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

Reply via email to