RFR: 8352284: EXTRA_CFLAGS incorrectly applied to BUILD_LIBJVM src/hotspot C++ source files [v2]

[Magnus Ihse Bursie]

On Sat, 22 Mar 2025 10:09:46 GMT, Patrick Zhang <qpzhang at openjdk.org> wrote:

>> Building jdk with `--with-extra-cflags='-Wno-incompatible-pointer-types'` triggers 1000+ warning messages like `cc1plus: warning: command-line option ‘-Wno-incompatible-pointer-types’ is valid for C/ObjC but not for C++`. 
>> 
>> The root cause is that `JVM_CFLAGS ` which contains both `EXTRA_CXXFLAGS` and `EXTRA_CFLAGS` when compiling `src/hotspot` C++ source files and building `BUILD_LIBJVM`.
>> 
>> This PR does:
>> 1. Not to append `EXTRA_CFLAGS` or `EXTRA_CXXFLAGS` into `JVM_CFLAGS` before calling `SetupJdkLibrary`, instead let `SetupCompilerFlags` accept and merge `EXTRA_CFLAGS` and `EXTRA_CXXFLAGS` passed from `SetupJdkLibrary` as parameters, so CPP compilation will only see `EXTRA_CXXFLAGS` as expected.
>> 2. Correct `PCH_COMMAND` to use `EXTRA_CXXFLAGS` as precompiled.hpp.gch should not be compiled with `EXTRA_CFLAGS`.
>> 3. Fixed `STATIC_LIB_CFLAGS` in `Flags.gmk` to `-DSTATIC_BUILD=1`, which was missed by [cbab40bc](https://github.com/openjdk/jdk/commit/cbab40bce45a2f58906be49c841178fa1dfd457e#diff-ab3ce05e795360030f19402fd0c2fad1dc1f7c5e7acc993cc4a2096cf31ccf40R114-R121) for the refactor of building static libs.
>> 
>> Tests: Passed jdk building on an AArch64 Linux system and tier1 sanity tests, also passed OpenJDK GHA Sanity Checks.
>
> Patrick Zhang has updated the pull request incrementally with three additional commits since the last revision:
> 
>  - Fixed a typo
>    
>    Signed-off-by: Patrick Zhang <patrick at os.amperecomputing.com>
>  - Added comments to describe why EXTRA_CFLAGS is excluded from JVM_CFLAGS
>    
>    Signed-off-by: Patrick Zhang <patrick at os.amperecomputing.com>
>  - Revert the changes of adding new params to SetupNativeCompilation
>    
>    Signed-off-by: Patrick Zhang <patrick at os.amperecomputing.com>

I've now re-read it all, and I stand behind my position that the comment should be removed.

I agree that the CFLAGS handling in the build system is obscure, and surprising. There is an old JBS issue from 2012 (!) tracking this: https://bugs.openjdk.org/browse/JDK-8001877 (see also https://bugs.openjdk.org/browse/JDK-8333089). It's a shame that this has not been fixed, but the problem is that the current situation is so complicated that it will take quite an effort to disentangle the mess, which combined with the limited ways to raise abstraction available in make makes this a much larger undertaking than maybe is apparent at first glance. 

So in this context, having the proposed comment at that place only just adds to the confusion.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/24115#issuecomment-2766258358