RFR [XXS] : cleanup macosx jspawnhelper build settings

[Baesken, Matthias]

Hi Erik, thanks for your ideas and comments.

> While your change is ok and can certainly be pushed on its own, there is
> so much more needing cleanup here.

I would like to do the other suggested cleanups  in a separate change.
Is this fine, can the original change be pushed ?

Best regards, Matthias


> -----Original Message-----
> From: Erik Joelsson [mailto:erik.joelsson at oracle.com]
> Sent: Donnerstag, 2. Februar 2017 17:45
> To: Baesken, Matthias <matthias.baesken at sap.com>; build-
> dev at openjdk.java.net
> Cc: Simonis, Volker <volker.simonis at sap.com>
> Subject: Re: RFR [XXS] : cleanup macosx jspawnhelper build settings
> 
> Hello Matthias,
> 
> While your change is ok and can certainly be pushed on its own, there is
> so much more needing cleanup here. If you don't mind, here is what I
> think needs doing:
> 
> * 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.
> 
> Thanks
> /Erik
> 
> On 2017-02-02 17:25, Baesken, Matthias wrote:
> > Hello,
> >      could I please have a  review for  the following small  change  that  cleans
> up  the special  macosx  BUILD_JSPAWNHELPER_DST_DIR setting that is not
> really
> >      needed any more after   CR 8066474: "Remove the lib/ directory from
> Linux and Solaris images"   changed the default setting .
> >
> >
> > Bug :
> >
> > https://bugs.openjdk.java.net/browse/JDK-8173834
> >
> > webrev :
> >
> > http://cr.openjdk.java.net/~mbaesken/webrevs/8173834.0/
> >
> >
> > Thanks, Matthias