RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument
David Holmes
david.holmes at oracle.com
Mon Nov 25 12:41:53 UTC 2019
Hi Christoph,
On 25/11/2019 10:38 pm, Langer, Christoph wrote:
> Hi David,
>
> thanks for your investigation. I'll prepare a fix to move back getPrefixed into canonicalize_md.c. However, could you please still fix your internal build in terms that it would have 'jdk_util.h' in the include path?
That should be simple enough to do.
David
> Thanks
> Christoph
>
>> -----Original Message-----
>> From: David Holmes <david.holmes at oracle.com>
>> Sent: Montag, 25. November 2019 01:02
>> To: Langer, Christoph <christoph.langer at sap.com>; Alan Bateman
>> <Alan.Bateman at oracle.com>; gerard ziemski <gerard.ziemski at oracle.com>
>> Cc: 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 25/11/2019 8:45 am, David Holmes wrote:
>>> On 25/11/2019 7:49 am, David Holmes wrote:
>>>> 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.
>>>
>>> It isn't the canonicalize function that is used, it is getPrefixed,
>>> which has now been moved to the io_util_md.c file. So a fix will be a
>>> bit more involved.
>>
>> I tried adding io_util_md.c to the library source list instead of
>> canonicalize_md.c but that just caused a slew of other compilation
>> failures, so I don't see any quick fix for us here.
>>
>> David
>>
>>>
>>> David
>>>
>>>>
>>>> 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
More information about the hotspot-runtime-dev
mailing list