Hi Serguei, I could not fail these tests on my environment. I would like to see more detail bug information especially the environment variable. Could you share with me the test-result, please?
Regards, Chihiro 2020年3月12日(木) 10:32 Yasumasa Suenaga <suen...@oss.nttdata.com>: > > Hi, > > AFAICS failure tests which are listed in JBS (JDK-8240881) seems to be caused > by HotSpotVirtualMachine::getSystemProperties. > It would load the result of PrintSystemPropertiesDCmd via Properties::load. > So we have to compliant the spec of Properties. > > OTOH it is not described in spec (JDK-7120511: introducing > VM.system_properties, and help message). > > Thus I think we need to add new option for VM.system_properties for showing > raw values (e.g. -raw). > If do so, we need CSR. > > What do you think? > > > Yasumasa > > > On 2020/03/12 4:52, serguei.spit...@oracle.com wrote: > > Hi Chihiro, > > > > I've tested and pushed your fix but the impact of fix was underestimated. > > The fix caused several regressions and the following bug was filed: > > https://bugs.openjdk.java.net/browse/JDK-8240881 > > > > Now, I'm working on removing the fix of JDK-8222489 with the anti-delta. > > You can find and review my RFR posted on the serviceability-dev mailing > > list: > > RFR: 8240881: several tests are failing due to encoding failures > > > > You can file another bug as a replacement of JDK-8222489. > > I will help you with the information about test regressions caused by it. > > > > Thanks, > > Serguei > > > > > > On 3/10/20 02:54, serguei.spit...@oracle.com wrote: > >> Hi Chihiro, > >> > >> Yes, I'll sponsor it. > >> Thank you for the update. > >> > >> Thanks, > >> Serguei > >> > >> > >> On 3/8/20 06:05, Chihiro Ito wrote: > >>> Hi, > >>> > >>> I'm sorry. I included "JDK-" in the changeset title. I removed it and > >>> updated it. > >>> > >>> Change set : > >>> http://cr.openjdk.java.net/~cito/JDK-8222489/webrev.06/changeset > >>> > >>> Regards, > >>> Chihiro > >>> > >>> 2020年3月7日(土) 23:13 Chihiro Ito <chiroito...@gmail.com>: > >>>> Hi Serguei and Yasumasa, > >>>> > >>>> I update the copyright year and created the change set. > >>>> > >>>> Could you sponsor this, please? > >>>> > >>>> Webrev : http://cr.openjdk.java.net/~cito/JDK-8222489/webrev.06/ > >>>> Change set : > >>>> http://cr.openjdk.java.net/~cito/JDK-8222489/webrev.06/changeset > >>>> > >>>> Regards, > >>>> Chihiro > >>>> > >>>> > >>>> 2020年3月7日(土) 16:03 Yasumasa Suenaga <suen...@oss.nttdata.com>: > >>>> > >>>> > >>>>> Hi Chihiro, > >>>>> > >>>>> I'm also ok with webrev.05 after updating copyright year. > >>>>> > >>>>> > >>>>> Yasumasa > >>>>> > >>>>> > >>>>> On 2020/03/07 3:32, serguei.spit...@oracle.com wrote: > >>>>>> Hi Chichiro, > >>>>>> > >>>>>> I'm okay with the fix. > >>>>>> Could you, please, update the copyright date in || > >>>>>> src/java.base/share/classes/jdk/internal/vm/VMSupport.java before push? > >>>>>> > >>>>>> Thanks, > >>>>>> Serguei > >>>>>> > >>>>>> > >>>>>> On 3/6/20 07:24, Chihiro Ito wrote: > >>>>>>> Hi Serguei, > >>>>>>> > >>>>>>> Could you review this again, please? > >>>>>>> > >>>>>>> Regards, > >>>>>>> Chihiro > >>>>>>> > >>>>>>> > >>>>>>> 2020年2月27日(木) 22:11 Chihiro Ito<chiroito...@gmail.com>: > >>>>>>>> Hi Ralf, > >>>>>>>> > >>>>>>>> Thank you for your advice. > >>>>>>>> > >>>>>>>> 1. > >>>>>>>> The comment of serializePropertiesToByteArray in VMSupport is "The > >>>>>>>> stream written to the byte array is ISO 8859-1 encoded.". > >>>>>>>> But the previous implementation does not keep this. I think we need > >>>>>>>> to implement encode by ISO 8859-1. > >>>>>>>> > >>>>>>>> 2. > >>>>>>>> According to help, the feature of VM.system_properties is just > >>>>>>>> "Print system properties". The users should not use this output for > >>>>>>>> loading. The users use it when they want to see System Properties > >>>>>>>> soon. > >>>>>>>> > >>>>>>>> Regards, > >>>>>>>> Chihiro > >>>>>>>> > >>>>>>>> > >>>>>>>> 2020年2月26日(水) 18:53 Schmelter, Ralf<ralf.schmel...@sap.com>: > >>>>>>>>> Hi Chihiro, > >>>>>>>>> > >>>>>>>>> I have two remarks: > >>>>>>>>> > >>>>>>>>> 1. ISO Latin 1 characters which are not ASCII will not work with > >>>>>>>>> the code. While the Properties.store() method claims to create ISO > >>>>>>>>> Latin 1 String, it really only will create printable ASCII > >>>>>>>>> characters (apart from the comment, but it is ASCII too in this > >>>>>>>>> case). See Properties.saveConvert, where the char is checked for < > >>>>>>>>> 0x20 or > 0x7e and then printed as \uxxxx. This is important, since > >>>>>>>>> the bytes of the ByteArrayOutputStream are then send to the jcmd. > >>>>>>>>> And jcmd expects UTF-8 encoded strings, which is OK if we only used > >>>>>>>>> ASCII characters. But a ISO Latin 1 character >= 0x80 will break > >>>>>>>>> the encoding. Just try using \u00DC in your test. > >>>>>>>>> > >>>>>>>>> 2. Your change makes it impossible to load the output with > >>>>>>>>> properties.load(). The old output could be loaded, since it was a > >>>>>>>>> valid properties file. But yours is not. For example, consider the > >>>>>>>>> filename c:\test\new. Formerly it would be encoded as: > >>>>>>>>> C\:\\test\\new > >>>>>>>>> And now it is: > >>>>>>>>> C:\test\new > >>>>>>>>> But the properties code would see "\n" as the newline character in > >>>>>>>>> your encoding. In fact you cannot differentiate between \n, \t, \f > >>>>>>>>> and \r originally being one or two characters. > >>>>>>>>> > >>>>>>>>> Best regards, > >>>>>>>>> Ralf > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> From: > >>>>>>>>> serviceability-dev<serviceability-dev-boun...@openjdk.java.net> On > >>>>>>>>> Behalf Of Chihiro Ito > >>>>>>>>> Sent: Dienstag, 25. Februar 2020 04:45 > >>>>>>>>> To:serguei.spit...@oracle.com > >>>>>>>>> Cc:serviceability-dev@openjdk.java.net > >>>>>>>>> Subject: Re: RFR: JDK-8222489: jcmd VM.system_properties gives > >>>>>>>>> unusable paths on Windows > >>>>>>>>> > >>>>>>>>> Hi Serguei, > >>>>>>>>> > >>>>>>>>> Thanks for your review and advice. > >>>>>>>>> > >>>>>>>>> I modified these. > >>>>>>>>> Could you review this again, please? > >>>>>>>>> > >>>>>>>>> Webrev :http://cr.openjdk.java.net/~cito/JDK-8222489/webrev.05/ > >>>>>>>>> > >>>>>>>>> Regards, > >>>>>>>>> Chihiro > >>>>>>>>> > >> > >
