RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument

[David Holmes]

On 25/11/2019 7:33 am, David Holmes wrote:
> Hi Christoph,
> 
> On 23/11/2019 12:04 am, Langer, Christoph wrote:
>> Hi,
>>
>> I'd like to push this change. However, running it through jdk-submit 
>> shows reproducible errors:
>>
>> Job: mach5-one-clanger-JDK-8234185-1-20191122-0927-6913189
>> BuildId: 2019-11-22-0926373.christoph.langer.source
>> No failed tests
>> Tasks Summary
>> •    NA: 0
>> •    NOTHING_TO_RUN: 0
>> •    KILLED: 0
>> •    PASSED: 76
>> •    UNABLE_TO_RUN: 0
>> •    EXECUTED_WITH_FAILURE: 1
>> •    FAILED: 0
>> •    HARNESS_ERROR: 0
>> Build
>> 1 Executed with failure
>> o    windows-x64-install-windows-x64-build-19 error while building, 
>> return value: 2
>>
>>
>> Job: mach5-one-clanger-JDK-8234185-20191121-2313-6898791
>> BuildId: 2019-11-21-2311357.christoph.langer.source
>> No failed tests
>> Tasks Summary
>> •    NA: 0
>> •    NOTHING_TO_RUN: 0
>> •    KILLED: 0
>> •    PASSED: 76
>> •    UNABLE_TO_RUN: 0
>> •    EXECUTED_WITH_FAILURE: 1
>> •    FAILED: 0
>> •    HARNESS_ERROR: 0
>> Build
>> 1 Executed with failure
>> o    windows-x64-install-windows-x64-build-19 error while building, 
>> return value: 2
>>
>>
>> David already had a look and let me know that the following was the 
>> reason:
>>
>> t:/workspace/open/src/java.base/windows/native/libjava/canonicalize_md.c(41): 
>> fatal error C1083: Cannot open include file: 'jdk_util.h': No such 
>> file or directory
>>
>> This is not explainable to me as I see this running through my local 
>> build and our nightly builds without problems. I also can't explain 
>> jdk_util.h can't be opened at this place - it should be there and part 
>> of the include directories...
>>
>> I'd appreciate any help...
> 
> I just dug a little deeper and this is failing in part of our closed 
> build for the install repo. There is a library there that is using 
> canonicalize_md.c directly - i.e. it adds that file to its source files 
> list. The build instructions don't include that directory on the include 
> directory list - hence the failure. But it will also fail due to the 
> name change you made.

Actually it appears that the other source code doesn't actually refer to 
the canonicalize function at all, so a simple fix may be possible at the 
build level on our side. I'm testing that now.

David
-----

> Someone will need to work with you to make the necessary changes to our 
> code.
> 
> David
> 
>> Thanks
>> Christoph
>>
>>
>>> -----Original Message-----
>>> From: Langer, Christoph
>>> Sent: Donnerstag, 21. November 2019 14:19
>>> To: Alan Bateman <Alan.Bateman at oracle.com>; core-libs-
>>> dev at openjdk.java.net; hotspot-runtime-dev at openjdk.java.net
>>> Subject: RE: RFR: 8234185: Cleanup usage of canonicalize function 
>>> between
>>> libjava, hotspot and libinstrument
>>>
>>> Hi Alan,
>>>
>>> thanks for the review. I'll push it then after running through 
>>> jdk-submit.
>>>
>>> /Christoph
>>>
>>>> -----Original Message-----
>>>> From: Alan Bateman <Alan.Bateman at oracle.com>
>>>> Sent: Donnerstag, 21. November 2019 09:51
>>>> To: Langer, Christoph <christoph.langer at sap.com>; core-libs-
>>>> dev at openjdk.java.net; hotspot-runtime-dev at openjdk.java.net
>>>> Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function 
>>>> between
>>>> libjava, hotspot and libinstrument
>>>>
>>>> On 14/11/2019 15:37, Langer, Christoph wrote:
>>>>> Hi,
>>>>>
>>>>> please review this cleanup change regarding function "canonicalize" of
>>>> libjava.
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8234185
>>>>> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8234185.0/
>>>>>
>>>>>
>>>>> The goal is to cleanup how this function is defined and used. One 
>>>>> thing is,
>>>> that there was an unnecessary wrapper function "Canonicalize" in 
>>>> jni_util.c.
>>>> It wrapped the call to "canonicalize". We can get rid of this wrapper.
>>>> Unfortunately, it is not possible to just export "canonicalize" 
>>>> since this will
>>>> conflict with a method signature from the math library, at least on 
>>>> modern
>>>> Linuxes. So I decided to call the method JDK_Canonicalize and will 
>>>> correctly
>>>> define it in jdk_util.h which can be included everywhere.
>>>>>
>>>> I think this change is okay. My main concern when initially seeing this
>>>> go by was that it would leak the \\?\ or \\?\UNC\ prefix into the
>>>> canonical File when it wasn't there previously, this would of course
>>>> have several implications. But I think you have it right and this 
>>>> is, as
>>>> you position, just refactoring/cleanup.
>>>>
>>>> -Alan