Re: [Integrated] RFR: 8237944: webview native cl "-m32" unknown option for windows 32-bit build
Changeset: aa91ebbb Author:Guru Hb Date: 2020-02-03 08:51:25 + URL: https://git.openjdk.java.net/jfx/commit/aa91ebbb 8237944: webview native cl "-m32" unknown option for windows 32-bit build Reviewed-by: kcr ! modules/javafx.web/src/main/native/Tools/Scripts/webkitdirs.pm
Re: [Rev 01] RFR: 8237944: webview native cl "-m32" unknown option for windows 32-bit build
On Wed, 29 Jan 2020 15:36:52 GMT, Guru Hb wrote: >> Marked as reviewed by kcr (Lead). > >> the -m32 option seems to be ignored by the compiler: >> cl : Command line warning D9002 : ignoring unknown option '-m32' >> >> However, I agree it is better to conditionally remove it. > "-m32" required for compiling other ports of webkit (WebkitGTK, WPE) and for > our platform I haven't tested this feature (i.e cross compiling on a 64 bit > Linux). @guruhb this is ready to integrate unless @johanvos wants to review it. - PR: https://git.openjdk.java.net/jfx/pull/97
Re: [Rev 01] RFR: 8237944: webview native cl "-m32" unknown option for windows 32-bit build
On Wed, 29 Jan 2020 12:57:41 GMT, Kevin Rushforth wrote: >> The pull request has been updated with 1 additional commit. > > Marked as reviewed by kcr (Lead). > the -m32 option seems to be ignored by the compiler: > cl : Command line warning D9002 : ignoring unknown option '-m32' > > However, I agree it is better to conditionally remove it. "-m32" required for compiling other ports of webkit (WebkitGTK, WPE) and for our platform I haven't tested this feature (i.e cross compiling on a 64 bit Linux). - PR: https://git.openjdk.java.net/jfx/pull/97
Re: [Rev 01] RFR: 8237944: webview native cl "-m32" unknown option for windows 32-bit build
On Wed, 29 Jan 2020 12:58:00 GMT, Guru Hb wrote: >> cl : Command line warning D9002 : ignoring unknown option '-m32' >> >> post fix for "https://trac.webkit.org/changeset/242724/webkit"; makes use of >> cross compiling 32 bit JSC in a 64 bit and its holds good only for Linux. >> '-m32' flag is gcc specifc and on windows cl.exe (visual studio) doesn't >> recognize this flag. > > The pull request has been updated with 1 additional commit. Marked as reviewed by kcr (Lead). - PR: https://git.openjdk.java.net/jfx/pull/97
Re: [Rev 01] RFR: 8237944: webview native cl "-m32" unknown option for windows 32-bit build
> cl : Command line warning D9002 : ignoring unknown option '-m32' > > post fix for "https://trac.webkit.org/changeset/242724/webkit"; makes use of > cross compiling 32 bit JSC in a 64 bit and its holds good only for Linux. > '-m32' flag is gcc specifc and on windows cl.exe (visual studio) doesn't > recognize this flag. The pull request has been updated with 1 additional commit. - Added commits: - bbb6201b: inforporated review comments Changes: - all: https://git.openjdk.java.net/jfx/pull/97/files - new: https://git.openjdk.java.net/jfx/pull/97/files/0d9ca899..bbb6201b Webrevs: - full: https://webrevs.openjdk.java.net/jfx/97/webrev.01 - incr: https://webrevs.openjdk.java.net/jfx/97/webrev.00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jfx/pull/97.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/97/head:pull/97 PR: https://git.openjdk.java.net/jfx/pull/97
Re: [Rev 01] RFR: 8237944: webview native cl "-m32" unknown option for windows 32-bit build
On Tue, 28 Jan 2020 23:42:08 GMT, Kevin Rushforth wrote:
>> The pull request has been updated with 1 additional commit.
>
> modules/javafx.web/src/main/native/Tools/Scripts/webkitdirs.pm line 2300:
>
>> 2299:
>> 2300: if (architecture() eq "x86_64" && shouldBuild32Bit() && (isJava()
>> && !isCygwin())) {
>> 2301: # CMAKE_LIBRARY_ARCHITECTURE is needed to get the right .pc
>
> This will work, since all we build is the Java platform, but I'm not sure
> that the `isJava()` test is quite right. It suggests that the logic should be
> skipped for other platforms (i.e., that the entire test and body of the if
> block is Java platform-specific), which it isn't. I think this should be:
>
> ... && !(isJava() && isCygwin())
You are correct, Current fix will block for all the other platform, Done the
changes as suggested.
-
PR: https://git.openjdk.java.net/jfx/pull/97
Re: RFR: 8237944: webview native cl "-m32" unknown option for windows 32-bit build
On Tue, 28 Jan 2020 09:50:27 GMT, Guru Hb wrote:
> cl : Command line warning D9002 : ignoring unknown option '-m32'
>
> post fix for "https://trac.webkit.org/changeset/242724/webkit"; makes use of
> cross compiling 32 bit JSC in a 64 bit and its holds good only for Linux.
> '-m32' flag is gcc specifc and on windows cl.exe (visual studio) doesn't
> recognize this flag.
All my testing looks good. I added one comment for you to take a look at.
modules/javafx.web/src/main/native/Tools/Scripts/webkitdirs.pm line 2300:
> 2299:
> 2300: if (architecture() eq "x86_64" && shouldBuild32Bit() && (isJava()
> && !isCygwin())) {
> 2301: # CMAKE_LIBRARY_ARCHITECTURE is needed to get the right .pc
This will work, since all we build is the Java platform, but I'm not sure that
the `isJava()` test is quite right. It suggests that the logic should be
skipped for other platforms (i.e., that the entire test and body of the if
block is Java platform-specific), which it isn't. I think this should be:
... && !(isJava() && isCygwin())
-
PR: https://git.openjdk.java.net/jfx/pull/97
Re: RFR: 8237944: webview native cl "-m32" unknown option for windows 32-bit build
On Tue, 28 Jan 2020 21:03:01 GMT, Johan Vos wrote: >> cl : Command line warning D9002 : ignoring unknown option '-m32' >> >> post fix for "https://trac.webkit.org/changeset/242724/webkit"; makes use of >> cross compiling 32 bit JSC in a 64 bit and its holds good only for Linux. >> '-m32' flag is gcc specifc and on windows cl.exe (visual studio) doesn't >> recognize this flag. > > the -m32 option seems to be ignored by the compiler: > cl : Command line warning D9002 : ignoring unknown option '-m32' > > However, I agree it is better to conditionally remove it. Agreed. It's a good cleanup fix. I'll do a quick test and then review it. - PR: https://git.openjdk.java.net/jfx/pull/97
Re: RFR: 8237944: webview native cl "-m32" unknown option for windows 32-bit build
On Tue, 28 Jan 2020 09:50:27 GMT, Guru Hb wrote: > cl : Command line warning D9002 : ignoring unknown option '-m32' > > post fix for "https://trac.webkit.org/changeset/242724/webkit"; makes use of > cross compiling 32 bit JSC in a 64 bit and its holds good only for Linux. > '-m32' flag is gcc specifc and on windows cl.exe (visual studio) doesn't > recognize this flag. the -m32 option seems to be ignored by the compiler: cl : Command line warning D9002 : ignoring unknown option '-m32' However, I agree it is better to conditionally remove it. - PR: https://git.openjdk.java.net/jfx/pull/97
RFR: 8237944: webview native cl "-m32" unknown option for windows 32-bit build
cl : Command line warning D9002 : ignoring unknown option '-m32' post fix for "https://trac.webkit.org/changeset/242724/webkit"; makes use of cross compiling 32 bit JSC in a 64 bit and its holds good only for Linux. '-m32' flag is gcc specifc and on windows cl.exe (visual studio) doesn't recognize this flag. - Commits: - 0d9ca899: 8237944: webview native cl "-m32" unknown option for windows 32-bit build Changes: https://git.openjdk.java.net/jfx/pull/97/files Webrev: https://webrevs.openjdk.java.net/jfx/97/webrev.00 Issue: https://bugs.openjdk.java.net/browse/JDK-8237944 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jfx/pull/97.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/97/head:pull/97 PR: https://git.openjdk.java.net/jfx/pull/97
