RFR: 8328157: Move C[XX]FLAGS_JDK[LIB/EXE] to JdkNativeCompilation.gmk [v3]

Erik Joelsson erikj at openjdk.org
Thu Mar 14 19:50:39 UTC 2024


On Thu, 14 Mar 2024 15:16:53 GMT, Magnus Ihse Bursie <ihse at openjdk.org> wrote:

>> We are setting one of the flags `CFLAGS_JDKLIB`, `CXXFLAGS_JDKLIB`, `CFLAGS_JDKEXE` or `CXXFLAGS_JDKEXE` to `CFLAGS` or `CXXFLAGS`, respectively, in basically all calls to `SetupJdkLibrary` and `SetupJdkExecutable`.
>> 
>> These flag variables contain a lot of duplication.
>> 
>> The first step towards bringing some sanity to this mess is to move the setting of these variables into `SetupJdkLibrary/SetupJdkExecutable`.
>> 
>> In a few places these standard flags are not set, partially or fullly. This is handled by the new arguments `DEFAULT_CFLAGS := false` (to disable the entire setting) and `C[XX]FLAGS_FILTER_OUT` (which excludes some specific flag) to `SetupJdkLibrary/SetupJdkExecutable`.
>
> Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix syntax error

make/common/JdkNativeCompilation.gmk line 206:

> 204:       $$(foreach flag, $$($1_CXXFLAGS_FILTER_OUT), \
> 205:         $$(eval $1_CXXFLAGS := $(filter-out $$(flag), $$($1_CXXFLAGS)) \
> 206:       ))

There shouldn't be a need for a loop here.
Suggestion:

      $$(filter-out $$($1_CXXFLAGS_FILTER_OUT), $$($1_CXXFLAGS))

Avoiding eval calls is usually a good idea as `$` escaping gets really convoluted when nesting them. Also, the single `$` in the filter-out call can't possibly work?

The conditional is also not necessary as filter-out with an empty pattern is a noop, so all of this could be condensed to a single line, including the assignment.

Same thing applies to a couple of more places below.

make/modules/jdk.jpackage/Lib.gmk line 63:

> 61:     CXXFLAGS_FILTER_OUT := -MD, \
> 62:     CXXFLAGS := $(JPACKAGE_APPLAUNCHER_INCLUDES), \
> 63:     CFLAGS := $(JPACKAGE_APPLAUNCHER_INCLUDES), \

This removed `-MD`, but I don't see it adding `-MT`.

make/modules/jdk.jpackage/Lib.gmk line 140:

> 138:       OPTIMIZATION := LOW, \
> 139:       SRC := $(JPACKAGE_WIXHELPER_SRC), \
> 140:       CXXFLAGS_FILTER_OUT := -MD, \

Same here about `-MT`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18301#discussion_r1525301430
PR Review Comment: https://git.openjdk.org/jdk/pull/18301#discussion_r1525409694
PR Review Comment: https://git.openjdk.org/jdk/pull/18301#discussion_r1525410729


More information about the build-dev mailing list