RFR: 8325621: Improve jspawnhelper version checks [v3]

[Magnus Ihse Bursie]

On Wed, 13 Mar 2024 09:49:13 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> make/modules/java.base/Launcher.gmk line 81:
>> 
>>> 79:       SRC := $(TOPDIR)/src/$(MODULE)/unix/native/jspawnhelper, \
>>> 80:       OPTIMIZATION := LOW, \
>>> 81:       CFLAGS := $(CFLAGS_JDKEXE) \
>> 
>> There is no need to introduce a break after `$(CFLAGS_JDKEXE)`, for thing like flags we try to fill the line.
>> Also, the indentation rules are that a broken line should be indented with four spaces. See e.g. the CFLAGS line below in CoreLibraries.gmk.
>
> My fault, I suggested Chad to do this. So what's the preferred formatting here? Like BUILD_JEXEC block above does it?
> 
> 
>       CFLAGS := $(CFLAGS_JDKEXE) $(VERSION_CFLAGS) \
>           -I$(TOPDIR)/src/$(MODULE)/unix/native/libjava, \

Yeah, that looks good. I was thinking just appending it at the end like:


     CFLAGS := $(CFLAGS_JDKEXE) -I$(TOPDIR)/src/$(MODULE)/unix/native/libjava \
          $(VERSION_CFLAGS), \


but your version definitely looks better!

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18204#discussion_r1522963660