Hi Kumar, (*) Highlights of the changes from the previous revision:
1) Conversion from UTF-16 to UTF-8. a) in webrev 4 was implemented in a standalone function JLI_DecodeArgs - the declaration and definition and usage affected 5 files: src/java.base/share/native/libjli/jli_util.h make/lib/CoreLibraries.gmk src/java.base/unix/native/libjli/java_md_common.c src/java.base/windows/native/libjli/java_md.c src/java.base/share/native/launcher/main.c b) webrew 5: was implemented directly in the main method and affected only 1 file: src/java.base/share/native/launcher/main.c 2) sprops.sun_stdout_encoding property setting this property turned out to be unnecessary to address the problem and all related changed were abolished - this affected 2 files: src/java.base/windows/native/libjava/java_props_md.c src/java.base/windows/native/libjava/Console_md.c 3) Conversion from UTF-8 to UTF-16. to interact from Java launcher with Windows file system - changes to the file were added in webrew 5: src/java.base/share/native/libjli/parse_manifest.c (*) All your comments have been fixed. Our only representative who has the right to post web reviews (Kirk Shoop) is on vacation now and will return on Monday 02/29/2016. We'll post the updated web review (6) as soon as he is back. (*) We should give credit to: - Vladimir Shcherbakov <vlas...@microsoft.com> - Valeriy Kopylov <valery.kopy...@akvelon.com> - Kirk Shoop <kirk.sh...@microsoft.com> Thanks, Vladimir. -----Original Message----- From: Kumar Srinivasan [mailto:kumar.x.sriniva...@oracle.com] Sent: Monday, February 22, 2016 8:18 AM To: Vladimir Shcherbakov <vlas...@microsoft.com> Cc: Naoto Sato <naoto.s...@oracle.com>; SHEN,XUEMING <xueming.s...@oracle.com>; core-libs-dev Libs <core-libs-dev@openjdk.java.net> Subject: Re: RFR 8124977 cmdline encoding challenges on Windows Few more comments on the tests: UnicodeCmdTestRun.java: The summary is fine, but it would be good to add a comment explaining what this test really does, random folks look at the tests when a test failure occurs, such a comment should help, similar to what you have in https://na01.safelinks.protection.outlook.com/?url=UnicodeCmdTest.java&data=01%7c01%7cvlashch%40microsoft.com%7c3ec7d35c50cc44070a4108d33ba3d6ea%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=cBKAOpf5sqd1yS3JK2EX44E41f%2fd3gq9fOua6R2Jr38%3d UnicodeCmdTest.java: it would be good add an error at line 33, + System.out.println("Error: 0 length argument"); System.exit(1); Thanks Kumar > > Hi Naoto, Sherman, can you please take a look. > I tested with the jprt build and test all tests pass. > > Hi Vladimir, et. al., > > It appears that there has been more simplifications from the previous > webrev.04. :-) > > It would've helped if you highlight the changes you have made from the > previous revision, unfortunately this is one of the deficiencies of > webrev. > > There are some inconsistencies in the coding conventions: > > parse_manifest.c > + if (q == 0) return -1; > > we expect the return to be on the next line. > > similarly main.c > > if (0 == q) > { > > I can fix those up. If I were to push this change, who should I > attribute the changes to ? ie. in the Contributed-by: line of the > commit info ? > Please note these have to be email addresses of the contributors. > > Thanks > Kumar > >> Hi Kumar, >> >> We posted another web review here: >> https://na01.safelinks.protection.outlook.com/?url=http:%2f%2fcr.open >> jdk.java.net%2f~kshoop%2f8124977%2fwebrev.05%2f&data=01%7C01%7Cvlashc >> h%40microsoft.com%7C3ec7d35c50cc44070a4108d33ba3d6ea%7C72f988bf86f141 >> af91ab2d7cd011db47%7C1&sdata=UWa60mcs9nyfqxB1CpJA%2b6h%2f6fPks0aDofCv >> k9ojssU%3d >> >> The patch was successfully tested. >> >> Test details: >> * Regression tests folder: jdk/test/tools/launcher/ >> * Builds were used: windows-x86_64-normal-server-fastdebug, >> windows-x86_64-normal-server-release, >> windows-x86-normal-server-release; >> * Platforms were used: Windows 7(64 bit), Windows 8.1, Windows >> Server 2012 R2 DC, Windows 10 ; >> * System locales were used: English (United States), Persian, >> Japanese (Japan), Chinese (Traditional, Taiwan), Russian (Russia); >> >> Thanks, >> Vladimir. >> >> -----Original Message----- >> From: Martin Sawicki >> Sent: Thursday, January 14, 2016 11:34 AM >> To: Kumar Srinivasan <kumar.x.sriniva...@oracle.com>; Vladimir >> Shcherbakov <vlas...@microsoft.com> >> Cc: core-libs-dev Libs <core-libs-dev@openjdk.java.net>; Naoto Sato >> <naoto.s...@oracle.com> >> Subject: RE: RFR 8124977 cmdline encoding challenges on Windows >> >> Thanks for the feedback. >> Investigating the regression failure. >> We'll get back as soon as we figure this out. (and yes, we'll run >> this through some localized Windows VMs) >> >> Cheers >> >> -----Original Message----- >> From: Kumar Srinivasan [mailto:kumar.x.sriniva...@oracle.com] >> Sent: Tuesday, January 12, 2016 2:35 PM >> To: Martin Sawicki <marc...@microsoft.com>; Vladimir Shcherbakov >> <vlas...@microsoft.com> >> Cc: core-libs-dev Libs <core-libs-dev@openjdk.java.net>; Naoto Sato >> <naoto.s...@oracle.com> >> Subject: Re: RFR 8124977 cmdline encoding challenges on Windows >> >> Hi Martin, Vladimir, >> >> It was suggested that this patch be tested on localized Windows >> machines and/or trying with the various Windows native encodings, >> appreciate if you can verify this as well. >> >> Thanks >> Kumar >> >> On 1/11/2016 1:10 PM, Kumar Srinivasan wrote: >>> Hi, >>> >>> Was on vacation, I started to prepare the patch from webrev.04 for >>> integration. Please note: made some adjustments to your patch to >>> pass jcheck, ie. usage of tabs and space at line endings, and >>> modifications to Copyright dates. >>> >>> Also fixed a minor bug on unix replaced JLI_TRUE with JNI_TRUE. >>> I have attached a patch to for your reference. >>> >>> However, there is a regression test failure on Windows, >>> jdk/test/tools/launcher/I18NTest.java >>> >>> ---Test info---- >>> Executed command: C:\mmm\jdk\bin\javac.exe i18nH▒lloWorld.java >>> >>> ++++Test Output++++ >>> javac: file not found: i18nHélloWorld.java ----End test info----- >>> >>> Have you run all the launcher regression tests with this changeset ? >>> >>> Thanks >>> Kumar >>> >>>> Hi Kumar, just wondering if there are any updates on processing >>>> this submission. >>>> Thanks! >>>> >>>> -----Original Message----- >>>> From: Vladimir Shcherbakov >>>> Sent: Wednesday, November 25, 2015 2:38 PM >>>> To: Kumar Srinivasan <kumar.x.sriniva...@oracle.com>; Martin >>>> Sawicki <marc...@microsoft.com> >>>> Cc: Kirk Shoop <kirk.sh...@microsoft.com>; core-libs-dev Libs >>>> <core-libs-dev@openjdk.java.net> >>>> Subject: RE: RFR 8124977 cmdline encoding challenges on Windows >>>> >>>> Hi Kumar, >>>> >>>> Please find updated webreview here: >>>> https://na01.safelinks.protection.outlook.com/?url=http:%2f%2fcr.op >>>> en >>>> jdk.java.net%2f~kshoop%2f8124977%2fwebrev.04%2f&data=01%7C01%7Cmarc >>>> in >>>> s%40microsoft.com%7C13ff309b775c4c019fc308d31ba0c43c%7C72f988bf86f1 >>>> 41 >>>> af91ab2d7cd011db47%7C1&sdata=3hhbO5mNPyTvtrTb4kCR42zsWGPGzDhqnmjpNf >>>> wn >>>> bIw%3d >>>> >>>> Thanks, >>>> Vladimir. >>>> >>>> -----Original Message----- >>>> From: Kumar Srinivasan [mailto:kumar.x.sriniva...@oracle.com] >>>> Sent: Sunday, November 22, 2015 8:14 AM >>>> To: Martin Sawicki <marc...@microsoft.com> >>>> Cc: Kirk Shoop <kirk.sh...@microsoft.com>; Vladimir Shcherbakov >>>> <vlas...@microsoft.com>; core-libs-dev Libs >>>> <core-libs-dev@openjdk.java.net> >>>> Subject: Re: RFR 8124977 cmdline encoding challenges on Windows >>>> >>>> >>>> Hi Martin, et. al., >>>> >>>> Sorry for not getting back earlier, I am very busy right now with >>>> my other large commitments for JDK9. >>>> >>>> I will sponsor this "enhancement/bug fix" sometime in the new year, >>>> meanwhile, there is the changeset [1] which is likely to cause >>>> merge conflicts, and perhaps logic issues. >>>> >>>> Thanks >>>> Kumar >>>> >>>> [1] >>>> https://na01.safelinks.protection.outlook.com/?url=http%3a%2f%2fhg. >>>> op >>>> enjdk.java.net%2fjdk9%2fdev%2fjdk%2frev%2f3b201a9ef918&data=01%7c01 >>>> %7 >>>> cvlashch%40microsoft.com%7c4d49ae546dba4d29b7be08d2f3589ee1%7c72f98 >>>> 8b >>>> f86f141af91ab2d7cd011db47%7c1&sdata=I2FKvBn82%2fxhW3D%2fi%2bRWaNOJk >>>> 7M >>>> g4lt2P0sdzLS%2fT9Q%3d >>>>> Hi all >>>>> Here's an updated webrev attempting to take into account the >>>>> various pieces of feedback we have received: >>>>> >>>>> Issue: >>>>> https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fbugs. >>>>> >>>>> openjdk.java.net%2fbrowse%2fJDK-8124977&data=01%7c01%7cvlashch%40m >>>>> ic >>>>> ro >>>>> soft.com%7c4d49ae546dba4d29b7be08d2f3589ee1%7c72f988bf86f141af91ab >>>>> 2d >>>>> 7c >>>>> d011db47%7c1&sdata=FjmfM%2fnPbWB%2fMsUU8uDzAUo3aPu3zOELVsJO%2fsUIq >>>>> 9E >>>>> %3 >>>>> d >>>>> Webrev: >>>>> https://na01.safelinks.protection.outlook.com/?url=http:%2f%2fcr.o >>>>> pe >>>>> nj >>>>> dk.java.net%2f~kshoop%2f8124977%2fwebrev.03%2f&data=01%7C01%7Cvlas >>>>> hc >>>>> h% >>>>> 40microsoft.com%7C4d49ae546dba4d29b7be08d2f3589ee1%7C72f988bf86f14 >>>>> 1a >>>>> f9 >>>>> 1ab2d7cd011db47%7C1&sdata=101HBPar2AZ63GJWyubWH0DiKmNI%2bOxknN667B >>>>> Jn >>>>> WY >>>>> 0%3d >>>>> >>>>> (Vladimir Shcherbakov is now working on this from our side) >>>>> >>>>> Looking forward to any other feedback. >>>>> Thanks >>>>> >>>>> -----Original Message----- >>>>> From: core-libs-dev >>>>> [mailto:core-libs-dev-boun...@openjdk.java.net] >>>>> On Behalf Of Kumar Srinivasan >>>>> Sent: Thursday, June 25, 2015 6:26 AM >>>>> To: Kirk Shoop (MS OPEN TECH) <kirk.sh...@microsoft.com> >>>>> Cc: Valery Kopylov (Akvelon) <v-val...@microsoft.com>; >>>>> core-libs-dev Libs <core-libs-dev@openjdk.java.net> >>>>> Subject: Re: RFR 8124977 cmdline encoding challenges on Windows >>>>> >>>>> Hi Kirk, >>>>> >>>>> Thanks for proposing this change. >>>>> >>>>> If you notice all the posix calls are wrapped in JLI_* this gives >>>>> us the ability to use "W" functions. I almost got it done, >>>>> several years ago, but we upgraded to VS2010 and my work based on >>>>> VS2003 keeled over, meanwhile my focus was "shifted" to something else. >>>>> >>>>> main.c: is really envisioned to be a stub compiled by the tool >>>>> launchers, like java, javac, javah, jar etc. I prefer to see all >>>>> the heavy logic in this file moved to the platform specific file >>>>> windows/java_md.* >>>>> >>>>> For the reason specified above we need to move fprintf or any >>>>> naked posix calls to JLI_* indirections. >>>>> >>>>> I don't see any tests ? The tests must be written in java and >>>>> placed in jdk/test/tools/launcher, there is a helper framework >>>>> TestHelper.java. >>>>> >>>>> There are other changes in nio, charsets etc, this will be >>>>> reviewed by my colleague specializing in that area (Sherman) cc'ed. >>>>> >>>>> >>>>> Thanks >>>>> >>>>> Kumar >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> On 6/22/2015 2:01 PM, Kirk Shoop (MS OPEN TECH) wrote: >>>>>> Hi, >>>>>> >>>>>> Issue: >>>>>> https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2f >>>>>> bu >>>>>> gs >>>>>> .openjdk.java.net%2fbrowse%2fJDK-8124977&data=01%7c01%7cvlashch%4 >>>>>> 0m >>>>>> ic >>>>>> rosoft.com%7c4d49ae546dba4d29b7be08d2f3589ee1%7c72f988bf86f141af9 >>>>>> 1a >>>>>> b2 >>>>>> d7cd011db47%7c1&sdata=FjmfM%2fnPbWB%2fMsUU8uDzAUo3aPu3zOELVsJO%2f >>>>>> sU >>>>>> Iq >>>>>> 9E%3d >>>>>> >>>>>> Webrev: >>>>>> https://na01.safelinks.protection.outlook.com/?url=http:%2f%2fcr. >>>>>> op >>>>>> en >>>>>> jdk.java.net%2f~kshoop%2f8124977%2f&data=01%7C01%7Cvlashch%40micr >>>>>> os >>>>>> of >>>>>> t.com%7C4d49ae546dba4d29b7be08d2f3589ee1%7C72f988bf86f141af91ab2d >>>>>> 7c >>>>>> d0 >>>>>> 11db47%7C1&sdata=RAA%2b5aIzKtrk5X85oLXKlPzbpSk%2bgJZRI%2b0QSI11B0 >>>>>> M% >>>>>> 3d >>>>>> >>>>>> This webrev intends to address interaction between Windows >>>>>> console and java apps. >>>>>> >>>>>> Two switches were added that change the behavior of the launcher. >>>>>> The defaults do not change the launcher behavior. >>>>>> >>>>>> -Dwindows.UnicodeConsole=true - switches on Unicode support >>>>>> in the Windows console. This optional switch causes the launcher >>>>>> to call GetCommandLineW() and parse the arguments in unicode. It >>>>>> also modifies how the codepage for console output is selected. >>>>>> >>>>>> -Dfile.encoding.unicode="UTF-8" - identifies Unicode >>>>>> charset to use; If not specified, UTF-8 is used by default. >>>>>> Ignored when windows.UnicodeConsole is not set to true. When the >>>>>> first switch is used, this optional switch allows the codepage >>>>>> for console output to be controlled. >>>>>> >>>>>> I would like to get feedback on the approach here and any >>>>>> additional work that is required solve these particular Unicode >>>>>> issues on Windows. >>>>>> >>>>>> Kirk >