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
>

Reply via email to