RFR [XS] : jspawnhelper build settings cleanup

[Erik Joelsson]

Hello Matthias,

Thanks for taking this on!

You can still remove "INCLUDE_FILES := jspawnhelper.c".

Another thing that struck me when looking at this is the inconsistency 
of sometimes using $(MODULE) and sometimes explicitly using java.base in 
the paths. I think $(MODULE) is better in this context.

Finally you could adopt the newer formatting style of using a space 
after comma at line 139 and end the eval-call with ', \' after the last 
argument and the double parenthesis on a new line on its own.

You are correct that JEXEC is suffering from many of the same issues. 
You are very welcome to fix that too, in this or another bug.

/Erik

On 2017-02-08 09:00, Baesken, Matthias wrote:
>
> Hello all ,
>
> Erik suggested to do further cleanups in  
> make/launcher/Launcher-java.base.gmk
>
>  In the  jspawnhelper build section.
>
> Those were the suggestions :
>
> * Inline BUILD_JSPAWNHELPER_SRC, JSPAWNHELPER_CFLAGS,
>
> BUILD_JSPAWNHELPER_DST_DIR and LINK_JSPAWNHELPER_OBJECTS. Since these
>
> variables aren't conditionally changed anywhere, there is really no need
>
> for the indirection.
>
> * The whole business of "BUILD_JSPAWNHELPER_LDFLAGS +=
>
> $(COMPILER_TARGET_BITS_FLAG)64" is confusing to me. Don't we trust the
>
> compiler for a 64 bit target to produce a 64 bit binary given the
>
> standard CFLAGS_JDKEXE and LDFLAGS_JDKLIB? I suspect this is just very
>
> old and confused code
>
> * The src dir only has the one src file, no need to explicitly list it
>
> for include.
>
> * The adding of childproc.o to LIBS can be accomplished using the
>
> parameter EXTRA_OBJECT_FILES. By using that you automatically get the
>
> dependency declaration so you can remove the line
>
> "$(BUILD_JSPAWNHELPER): $(LINK_JSPAWNHELPER_OBJECTS)"
>
> * The ifeq ($(BUILD_JSPAWNHELPER), 1) is also annoying, just move the
>
> single conditional into it's place.
>
> I prepared a webrev for this.
>
> After removing  the BUILD_JSPAWNHELPER_LDFLAGS += 
>  $(COMPILER_TARGET_BITS_FLAG)64
>
> I checked that  the generated executable is still 64bit on 
> AIX/Solaris/Mac-OSX (however  the  -m64 seems to be gone on Mac after 
> this change
>
> Compared to before when generating the executable).
>
> Btw the  BUILD_JEXEC in    make/launcher/Launcher-java.base.gmk  
>    looks also a bit strange (similar issues as the jspawnhelper section).
>
> Bug :
>
> https://bugs.openjdk.java.net/browse/JDK-8174086
>
>   webrev  jdk10 :
>
> http://cr.openjdk.java.net/~mbaesken/webrevs/8174086.0/ 
> <http://cr.openjdk.java.net/%7Embaesken/webrevs/8174086.0/>
>
> Please  review my change .
>
> Thanks ,Matthias
>