On Mon, 22 May 2023 21:24:18 GMT, Kevin Rushforth <[email protected]> wrote:
>> John Neffenger has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains five commits:
>>
>> - Merge branch 'master' into allow-armhf-i386-ppc64el-s390x
>> - Warn instead of failing on unknown architectures
>> - Merge branch 'master' into allow-armhf-i386-ppc64el-s390x
>> - Merge branch 'master' into allow-armhf-i386-ppc64el-s390x
>> - Allow building on armhf, i386, ppc64el, and s390x
>
> buildSrc/linux.gradle line 48:
>
>> 46: "-Wextra", "-Wall", "-Wformat-security", "-Wno-unused",
>> "-Wno-parentheses", "-Werror=trampolines"] // warning flags
>> 47:
>> 48: if (OS_ARCH == "i386") {
>
> Why was this change needed, and is it sufficient? It seems that there is a
> larger problem (likely out of scope for this fix) as to how `IS_64` is set
> that might need follow-up.
The change was needed because the `-m32` option is appropriate only for the
`i386` architecture. I believe the change is sufficient, but there does seem to
be a problem in how the `IS_64` property is set and used throughout the build
files. The property is set with:
ext.IS_64 = OS_ARCH.toLowerCase().contains("64")
Among the Java architecture names known to the build file or supported by
Debian, that means:
* `IS_64` is true for `amd64`, `aarch64`, `ppc64le`, `loongarch64`, and
`riscv64`, while
* `IS_64` is false for `arm`, `i386`, and `s390x`.
The previous code, then, would set the `-m32` option for `arm`, `i386`, and
`s390x`. Yet the option is valid only for the [following target machines][1]:
* RS/6000 and PowerPC (Debian `powerpc`)
* SPARC (Debian `sparc`)
* x86 (Debian `i386`)
So as long as nobody wants to build JavaFX for the 32-bit PowerPC or 32-bit
SPARC architectures, the code change is fine. I don't know of anyone still
supporting operating systems with Java on those architectures, so we should be
safe.
[1]: https://gcc.gnu.org/onlinedocs/gcc/Option-Summary.html
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1124#discussion_r1230020859
