On Wed, 19 Mar 2025 14:43:28 GMT, Patrick Zhang <qpzh...@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. I think I understand what you are trying to achieve, but this isn't the right solution. The `EXTRA_[C|CXX]FLAGS` variables are not meant to be parameters to SetupNativeCompilation. In flags.m4 you can see that the intent is to not even export `EXTRA_[C|CXX]FLAGS` from spec.gmk: # FIXME: For compatibility, export this as EXTRA_CFLAGS for now. EXTRA_CFLAGS="$MACHINE_FLAG $USER_CFLAGS" EXTRA_CXXFLAGS="$MACHINE_FLAG $USER_CXXFLAGS" EXTRA_LDFLAGS="$MACHINE_FLAG $USER_LDFLAGS" EXTRA_ASFLAGS="$USER_ASFLAGS" We don't want library/executable specific makefiles to have to worry about the extra flags supplied by the user. Those should be handled in lower layers. Given that, I agree with the change in JvmFlags.gmk. We shouldn't add `$(EXTRA_CFLAGS)` there. If we wanted those added, it should be done in configure, where `EXTRA_CXXFLAGS` are added, but not doing that is your whole goal here. Wouldn't just the change in JvmFlags.gmk be enough to solve your issue? ------------- PR Review: https://git.openjdk.org/jdk/pull/24115#pullrequestreview-2699645789
