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

Mon Nov 25 12:38:15 UTC 2019

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?

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