RFR (M) 8191521: handle long relative path specified in -Xbootclasspath/a on windows

[Calvin Cheung]

Hi Ralf,

Your latest webrev looks good.

Go ahead and push the change after more testing.

thanks,

Calvin

On 9/27/19 3:23 AM, Schmelter, Ralf wrote:
> Hi Calvin and Christoph,
>
> I will run the tests with the final patch for one more day and if nothing pops up, I will submit the change. Any objections?
>
> Best regards,
> Ralf
>
> -----Original Message-----
> From: Langer, Christoph <christoph.langer at sap.com>
> Sent: Freitag, 27. September 2019 11:18
> To: Schmelter, Ralf <ralf.schmelter at sap.com>
> Cc: Calvin Cheung <calvin.cheung at oracle.com>; hotspot-runtime-dev at openjdk.java.net
> Subject: RE: RFR (M) 8191521: handle long relative path specified in -Xbootclasspath/a on windows
>
> Hi Ralf,
>
> this looks good now, thanks for adding the explanations to the test.
>
> Maybe you can mention os::same_files in the comments in line 470 as well, but no need for a new webrev ��
>
> Best regards
> Christoph
>
>> -----Original Message-----
>> From: Schmelter, Ralf <ralf.schmelter at sap.com>
>> Sent: Freitag, 27. September 2019 11:10
>> To: Langer, Christoph <christoph.langer at sap.com>
>> Cc: Calvin Cheung <calvin.cheung at oracle.com>; hotspot-runtime-
>> dev at openjdk.java.net
>> Subject: RE: RFR (M) 8191521: handle long relative path specified in -
>> Xbootclasspath/a on windows
>>
>> Hi Christoph,
>>
>> thanks for the review. I've made a new incremental webrev
>> http://cr.openjdk.java.net/~rschmelter/webrevs/8191521/webrev.3.inc/
>> containing the changes to the test and LongBCP.java.
>>
>> Best regards,
>> Ralf
>>
>> -----Original Message-----
>> From: Langer, Christoph <christoph.langer at sap.com>
>> Sent: Mittwoch, 25. September 2019 15:52
>> To: Schmelter, Ralf <ralf.schmelter at sap.com>
>> Cc: Calvin Cheung <calvin.cheung at oracle.com>; hotspot-runtime-
>> dev at openjdk.java.net
>> Subject: RE: RFR (M) 8191521: handle long relative path specified in -
>> Xbootclasspath/a on windows
>>
>> Hi Ralf,
>>
>> I eventually found the time to look at this change now. Sorry for the delay.
>>
>> Thanks a lot for working on this patch. It'll probably fix quite some issues for
>> the JDK on Windows when working with unusual path combinations.
>>
>> I've looked at
>> http://cr.openjdk.java.net/~rschmelter/webrevs/8191521/webrev.2/, which
>> seemingly adds the check for null paths at the beginning of
>> wide_abs_unc_path in os_windows.cpp. This patch version has been running
>> in our nightly tests now for a long time and the results look very good - no
>> obvious regressions.
>>
>> As for the patch: It looks good to me in general.
>>
>> os_windows.cpp: no comment.
>>
>> The test in test_windows.cpp maybe deserves a few more comments to
>> ease the understanding for future developers. I would move the comments
>> from line 89 and 90 to line 451 where the actual test function starts. Can you
>> then please elaborate a bit more that the test by default runs in the
>> regression test mode (e.g. TEST) which verifies the functionality of os::stat,
>> os::open and os::dir_is_empty. And furthermore there is the EXAMPLES
>> mode to illustrate the behavior of certain Windows APIs and the BENCH
>> mode for a simple benchmark of file operations.
>>
>> I'm also wondering, if you want to test os::same_files as well (which makes
>> use of the new wide_abs_unc_path method).
>>
>> As for the modified testcase LongBCP.java: You should remove "import
>> jdk.test.lib.Platform;" in line 42 as well. It's not needed any more after your
>> change.
>>
>> Thanks & Best regards
>> Christoph
>>
>>
>>> -----Original Message-----
>>> From: hotspot-runtime-dev <hotspot-runtime-dev-
>>> bounces at openjdk.java.net> On Behalf Of Calvin Cheung
>>> Sent: Freitag, 26. Juli 2019 20:59
>>> To: Schmelter, Ralf <ralf.schmelter at sap.com>; hotspot-runtime-
>>> dev at openjdk.java.net
>>> Subject: Re: RFR (M) 8191521: handle long relative path specified in -
>>> Xbootclasspath/a on windows
>>>
>>> Hi Ralf,
>>>
>>> On 7/26/19 5:20 AM, Schmelter, Ralf wrote:
>>>> Hi Calvin,
>>>>
>>>> I've updated the webrev with your suggestions. The tests now disable
>> the
>>>> UNC path portion, if you don't have share <DRIVE>$ for <DRIVE>:
>>>> So all tests should now run on your machine.
>>> The update looks good and the gtest passed. I saw the following in the
>>> test log:
>>>
>>> [----------] 2 tests from os_windows
>>> [ RUN      ] os_windows.reserve_memory_special_test_vm
>>> [       OK ] os_windows.reserve_memory_special_test_vm (0 ms)
>>> [ RUN      ] os_windows.handle_long_paths_test_vm
>>> Disabled UNC path test, since T: is not mapped as share T$.
>>> [       OK ] os_windows.handle_long_paths_test_vm (13835 ms)
>>> [----------] 2 tests from os_windows (13835 ms total)
>>>
>>> thanks,
>>>
>>> Calvin
>>>
>>>> http://cr.openjdk.java.net/~rschmelter/webrevs/8191521/webrev.1/
>>>>
>>>> Best regards,
>>>> Ralf