RFR [XXS] : cleanup macosx jspawnhelper build settings

[Erik Joelsson]

Fine with me.

/Erik


On 2017-02-03 12:08, Baesken, Matthias wrote:
> 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