RFR: JDK-8210729 Clean up macosx static library handling

Erik Joelsson erik.joelsson at oracle.com
Fri Sep 14 17:03:20 UTC 2018


Hello,

On 2018-09-14 00:33, Magnus Ihse Bursie wrote:
> On 2018-09-14 01:16, Erik Joelsson wrote:
>> Hello,
>>
>> Nice cleanup overall.
> Thanks!
>
>>
>> In CoreLibraries.gmk: I don't think it's safe to use 
>> $(BUILD_LIBFDLIB) directly as input to LIBS. That variable may 
>> contain other targets as well (such as debug symbol files and soon 
>> compile_commands.js snippets). 
> Ah, the compile_commands.js patch is surely going to break things, 
> where we have been sloppy about this. :-&
>
>> A safer variable to use is $(BUILD_FDLIBM_TARGET). I even documented 
>> this as an output variable in NativeCompilation.gmk. I realize this 
>> was already used before your patch however.
>
> Good point, you are absolutely correct. Updated webrev:
> http://cr.openjdk.java.net/~ihse/JDK-8210729-cleanup-macosx-static-library-handling/webrev.02 
>
>
Looks good!
>
>> I would expect a certain level of testing to be done for a change 
>> like this to make sure all affected libraries are verified.
> I've tested tier1-3. Do you think that's sufficient, or can you 
> recommend some additional testing that would be suitable?
>
A quick greping through the tests seems to indicate that fdlibm is 
tested by tests under test/jdk/java/lang/ which is part of tier1. For 
java.instrument, it seems to be tested by :jdk_instrument which is part 
of :jdk_svc which isn't included in tier1-3, so perhaps a separate run 
of :jdk_instrument would be good.

/Erik
> /Magnus
>
>>
>> /Erik
>>
>> On 2018-09-13 15:06, Magnus Ihse Bursie wrote:
>>> There's a bunch of interrelated smelly code regarding static 
>>> libraries on macosx.
>>>
>>> I started by turning libfdlibm into a static library on macosx, as 
>>> on all other platforms. It turned out that we don't have proper 
>>> support for static libraries on macosx, so I fixed this too.
>>>
>>> Secondly, I removed the libjli_static.a for macosx. There is no good 
>>> reason to have it. It's probably just a left-over from the old Apple 
>>> port to macosx. This in turn prompted some additional related 
>>> cleanup in LauncherCommon.gmk.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8210729
>>> WebRev: 
>>> http://cr.openjdk.java.net/~ihse/JDK-8210729-cleanup-macosx-static-library-handling/webrev.01
>>>
>>> /Magnus
>>>
>>
>




More information about the build-dev mailing list