On Tue, 30 Apr 2024 13:50:22 GMT, Julian Waters <jwat...@openjdk.org> wrote:
>> make/modules/java.desktop/lib/AwtLibraries.gmk line 102: >> >>> 100: $(eval $(call SetupJdkLibrary, BUILD_LIBAWT, \ >>> 101: NAME := awt, \ >>> 102: LANG := $(if $(filter $(OPENJDK_TARGET_OS), windows), C++, C), \ >> >> No, this is not okay. You need to do this as for the LIBJSOUND_LINK_TYPE >> above. The same goes for the one below, too. > > Oh, I'm surprised! I thought that you'd prefer the more lambda-like approach. > I guess the other way of LIBAWT_LINK_TYPE works too in that case There are two reasons: 1) To keep up with the style elsewhere, were we use "ifeq (... platform)" to setup platform-specific arguments. Even if this was not an ideal style, it would still make sense to keep to one way of doing it. 2) I actually think that is better. We have gravitated towards that solution over the years. The make syntax is hard to read and easy to get wrong. We try to make the arguments in the Setup calls trivial, and if we can't do that in place, then we create a "local" variable (by prefixing it with the name of the lib) outside the Setup call, were we can use more space to clearly show what is going on. In fact, I really dislike the `$(if...)` syntax, and use it only if I must. It is hopeless to see what is the if-clause and the else-clause, and it is way too easy to get a "false positive" since you do not compare the variable with another value, but checks if it evaluates to non-empty. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18927#discussion_r1584903238
