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
