Erik:

The improved version looks good to me as well.

Tim

On 01/23/19 16:23, Vladimir Kozlov wrote:
Thank you, Erik

This looks good.

Vladimir

On 1/23/19 4:16 PM, Erik Joelsson wrote:
Hello again,

Here is a better fix as described below. Configure finds and sets
LD_JAOTC specifically tailored for the needs of jaotc for each platform.

http://cr.openjdk.java.net/~erikj/8217613/webrev.02/index.html

Verified with local test using TEST_OPTS=AOT_MODULES=java.base on
Macosx, Linux and Windows, as well as mach5 run of the same.

/Erik

On 2019-01-23 15:55, Igor Ignatyev wrote:
so far it has affected only my local runs, so we are in no hurry to
get this fixed ;) you can work on a better fix, I, meanwhile, will
apply your patch to my local ws as a workaround.

-- Igor

On Jan 23, 2019, at 3:10 PM, Erik Joelsson
<erik.joels...@oracle.com> wrote:

On 2019-01-23 13:38, Igor Ignatyev wrote:
Hi Erik,

I don't like that it's based on the assumption that ld and
clang/gcc are in the same directory, but this assumption seems to
be always true for now. so unless there is an easy way to get ld
path, I'm fine w/ this fix.
I don't either, and I was in a bit of a hurry so didn't think
further. It would of course be cleaner to have configure find ld for
us in a separate variable since this is only a problem in the
local/configured case.

/Erik

-- Igor

On Jan 23, 2019, at 1:18 PM, Vladimir Kozlov
<vladimir.koz...@oracle.com> wrote:

Looks good.

Thanks,
Vladimir

On 1/23/19 12:03 PM, Erik Joelsson wrote:
The TEST_OPTS=AOT_MODULES option does not work when running tests
locally on Macosx. This is because jaot expects the linker to be
"ld", while we define the linker (LD) to be clang on Macosx. When
running tests on already built JDKs using run-test-prebuilt, we
already setup LD to point to "ld" rather than "clang", so the
same should be done when running local tests.
This patch rewrites LD for jaotc, on Macosx and Linux. Though we
did not have a problem on Linux, the run-test-prebuilt case does
set LD to "ld" on Linux so better have it consistent.
I also added -XX:+UnlockDiagnosticVMOptions to the verification
command to make it work on release builds where this flag is
default off, as well as cleaned up some whitespace.
Bug: https://bugs.openjdk.java.net/browse/JDK-8217613
Webrev: http://cr.openjdk.java.net/~erikj/8217613/webrev.01/
/Erik

Reply via email to