LGTM! -- Igor
> On Jan 23, 2019, at 4:16 PM, Erik Joelsson <erik.joels...@oracle.com> 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