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