Hi David, thanks again for your efforts.
Here is a version that ran successfully through jdk-submit (mach5-one-clanger-JDK-8234185-3-20191205-1051-7247172): http://cr.openjdk.java.net/~clanger/webrevs/8234185.2/ I avoid the inclusion of jdk_util.h in src/java.base/windows/native/libjava/canonicalize_md.c. Do you think this is good to go? Somebody in Oracle could then eventually clean up things by decoupling the installer from OpenJDK's canonicalize_md.c. I'd open a bug for this. Thanks Christoph > -----Original Message----- > From: David Holmes <david.hol...@oracle.com> > Sent: Dienstag, 3. Dezember 2019 23:52 > To: Langer, Christoph <christoph.lan...@sap.com> > Cc: core-libs-dev@openjdk.java.net; hotspot-runtime- > d...@openjdk.java.net; Alan Bateman <alan.bate...@oracle.com>; gerard > ziemski <gerard.ziem...@oracle.com> > Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function between > libjava, hotspot and libinstrument > > Hi Christoph, > > On 3/12/2019 7:26 pm, Langer, Christoph wrote: > > 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/ > > I'm afraid I cannot get this to work at our end. I get the following errors: > > t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46): > error C2143: syntax error: missing ')' before '*' > t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46): > error C2143: syntax error: missing '{' before '*' > t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46): > warning C4142: 'size_t': benign redefinition of type > C:\ade\mesos\work_dir\jib- > ma~1\install\jpg\infra\buildd~1\devkit~1\vs2017~3.0\devkit~1.gz\VC\includ > e\vcruntime.h(184): > note: see declaration of 'size_t' > t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46): > error C2370: 'size_t': redefinition; different storage class > C:\ade\mesos\work_dir\jib- > ma~1\install\jpg\infra\buildd~1\devkit~1\vs2017~3.0\devkit~1.gz\VC\includ > e\vcruntime.h(184): > note: see declaration of 'size_t' > t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46): > error C2146: syntax error: missing ';' before identifier 'info_size' > t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46): > error C2059: syntax error: ')' > > This pertains to the line: > > JDK_GetVersionInfo0(jdk_version_info* info, size_t info_size); > > There is also a second problem in our closed code that will take more > effort from someone familiar with that code to resolve. I will file an > issue for our install team to pick up. > > I would ask that this not be pushed for the moment while others figure > out how best to handle this. > > Thanks, > David > ----- > > > > 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- > d...@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
