Hello Christoph,
On 2019-11-20 10:22, christoph.goettsch...@microdoc.com wrote:
Hi,
please review the following change.
Bug: https://bugs.openjdk.java.net/browse/JDK-8234535
Webrev: https://cr.openjdk.java.net/~cgo/8234535/webrev.00/
The removed lines are completely out of place and have no use at all. for
the CC variable, for instance, the script does the following:
CC_OLD="$CC"
CC="$BUILD_CC"
CC="$CC_OLD"
without executing any function between those lines. I think that, while
restructuring the build system, those lines have been copied to the wrong
location. I copied them around the "FLAGS_SETUP_CFLAGS_CPU_DEP" call for
the BUILD_CC, in order to make configure not use the target CFLAGS while
discovering possible CFLAGS for the BUILD_CC, which was happening in our
case.
Wow, I had not noticed that. Your fix looks correct.
I am also not sure if the variable BUILD_CC_DISABLE_WARNING_PREFIX is
actually used anywhere. I could not find any user and it might be possible
to simply remove it.
It's used to override DISABLE_WARNING_PREFIX in buildjdk-spec.gmk so I
think it serves a purpose, even if we don't actually support different
toolchain types for cross and native compiler. If we ever will, then
this variable will be needed, so I think it should stay.
I tried the change and compiled on an amd64 linux machine for amd64, and
cross-compiled for linux on armv7 and linux on aarch64. I don't have
access to other cross-compilation environments and would like to ask
others to review and try out the change.
I applied the patch and tried a few of our configurations. Since we use
the same (or close to the same) compiler versions across architectures,
I don't actually see any difference in the generated spec files. The
reason you see this is the large version difference between your compilers.
Looks good, thanks for finding and fixing this!
/Erik
Thanks,
Christoph
