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

[David Holmes]

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.

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