Hi David, thanks for taking care of this.
This would be my updated patch that could hopefully be enabled by just adding the include directory where "jdk_util.h" is located. It would be really great if that'd work. I think it would open up a path to automatically include io_util_md.h by including io_util.h. http://cr.openjdk.java.net/~clanger/webrevs/8234185.1/ Cheers Christoph > -----Original Message----- > From: David Holmes <david.hol...@oracle.com> > Sent: Dienstag, 3. Dezember 2019 03:13 > To: Langer, Christoph <christoph.lan...@sap.com>; Alan Bateman > <alan.bate...@oracle.com>; gerard ziemski <gerard.ziem...@oracle.com> > Cc: core-libs-dev@openjdk.java.net; hotspot-runtime- > d...@openjdk.java.net > Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function between > libjava, hotspot and libinstrument > > Hi Christoph, > > Can you please post your updated patch for review and I will use it to > check the fix for our internal build. Once you have your fix reviewed I > will push both fixes together. > > Thanks, > David > > On 25/11/2019 10:41 pm, David Holmes wrote: > > 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.hol...@oracle.com> > >>> Sent: Montag, 25. November 2019 01:02 > >>> To: Langer, Christoph <christoph.lan...@sap.com>; Alan Bateman > >>> <alan.bate...@oracle.com>; gerard ziemski > <gerard.ziem...@oracle.com> > >>> Cc: core-libs-dev@openjdk.java.net; hotspot-runtime- > >>> d...@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.bate...@oracle.com>; core-libs- > >>>>>>>> d...@openjdk.java.net; hotspot-runtime-...@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.bate...@oracle.com> > >>>>>>>>> Sent: Donnerstag, 21. November 2019 09:51 > >>>>>>>>> To: Langer, Christoph <christoph.lan...@sap.com>; core-libs- > >>>>>>>>> d...@openjdk.java.net; hotspot-runtime- > d...@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
