Bug 188092

Summary: [LayoutTests][Win] Tests can fail if long paths are not enabled in registry.
Product: WebKit Reporter: Basuke Suzuki <Basuke.Suzuki>
Component: Tools / TestsAssignee: Basuke Suzuki <Basuke.Suzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, Basuke.Suzuki, bfulgham, dbates, ddkilzer, ews-watchlist, glenn, Hironori.Fujii, jbedard, lforschler, ross.kirsling, webkit-bug-importer
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=192293
Attachments:
Description Flags
PATCH
ews-watchlist: commit-queue-
Archive of layout-test-results from ews200 for win-future none

Description Basuke Suzuki 2018-07-26 23:10:22 PDT
Windows need a special care for path which is longer than 255 (OMG). Some tests are very long and causes error when generating a test result file which is usually longer than original.

i.e. 
C:\Users\basuke\webkit-trunk\LayoutTests\http\tests\storageAccess\equest-and-grant-storage-access-cross-origin-sandboxed-iframe-from-prevalent-domain-with-non-recent-user-interaction-and-try-access-from-right-frame.html
->
C:\Users\basuke\webkit-trunk\WebKitBuild\Release\bin64\layout-test-results\http\tests\storageAccess\request-and-grant-storage-access-cross-origin-sandboxed-iframe-from-prevalent-domain-with-non-recent-user-interaction-and-try-access-from-right-frame-actual.txt

link:
https://docs.microsoft.com/ja-jp/windows/desktop/FileIO/naming-a-file#maximum-path-length-limitation
Comment 1 Basuke Suzuki 2018-07-26 23:11:20 PDT
worker/7: IOError('[Errno 2] No such file or directory: u'C:\\Users\\basuke\\webkit-trunk\\WebKitBuild\\Release\\bin64\\layout-test-results\\http\\tests\\storageAccess\\request-and-grant-storage-access-cross-origin-sandboxed-iframe-from-prevalent-domain-with-non-recent-user-interaction-and-try-access-from-right-frame-actual.txt'') raised:
    File "C:\Users\basuke\webkit-trunk\Tools\Scripts\webkitpy\common\message_pool.py", line 257, in run
      worker.handle(message.name, message.src, *message.args)
    File "C:\Users\basuke\webkit-trunk\Tools\Scripts\webkitpy\layout_tests\controllers\layout_test_runner.py", line 270, in handle
      self._run_test(test_input, test_list_name)
    File "C:\Users\basuke\webkit-trunk\Tools\Scripts\webkitpy\layout_tests\controllers\layout_test_runner.py", line 294, in _run_test
      result = self._run_test_with_or_without_timeout(test_input, test_timeout_sec, stop_when_done)
    File "C:\Users\basuke\webkit-trunk\Tools\Scripts\webkitpy\layout_tests\controllers\layout_test_runner.py", line 337, in _run_test_with_or_without_timeout
      return self._run_test_in_this_thread(test_input, stop_when_done)
    File "C:\Users\basuke\webkit-trunk\Tools\Scripts\webkitpy\layout_tests\controllers\layout_test_runner.py", line 418, in _run_test_in_this_thread
      return self._run_single_test(self._driver, test_input, stop_when_done)
    File "C:\Users\basuke\webkit-trunk\Tools\Scripts\webkitpy\layout_tests\controllers\layout_test_runner.py", line 422, in _run_single_test
      self._name, driver, test_input, stop_when_done)
    File "C:\Users\basuke\webkit-trunk\Tools\Scripts\webkitpy\layout_tests\controllers\single_test_runner.py", line 46, in run_single_test
      return runner.run()
    File "C:\Users\basuke\webkit-trunk\Tools\Scripts\webkitpy\layout_tests\controllers\single_test_runner.py", line 105, in run
      return self._run_compare_test()
    File "C:\Users\basuke\webkit-trunk\Tools\Scripts\webkitpy\layout_tests\controllers\single_test_runner.py", line 124, in _run_compare_test
      test_result_writer.write_test_result(self._filesystem, self._port, self._results_directory, self._test_name, driver_output, expected_driver_output, test_result.failures)
    File "C:\Users\basuke\webkit-trunk\Tools\Scripts\webkitpy\layout_tests\controllers\test_result_writer.py", line 48, in write_test_result
      failure.write_failure(writer, driver_output, expected_driver_output, port)
    File "C:\Users\basuke\webkit-trunk\Tools\Scripts\webkitpy\layout_tests\models\test_failures.py", line 121, in write_failure
      writer.write_text_files(driver_output.text, expected_driver_output.text)
    File "C:\Users\basuke\webkit-trunk\Tools\Scripts\webkitpy\layout_tests\controllers\test_result_writer.py", line 154, in write_text_files
      self.write_output_files(".txt", actual_text, expected_text)
    File "C:\Users\basuke\webkit-trunk\Tools\Scripts\webkitpy\layout_tests\controllers\test_result_writer.py", line 138, in write_output_files
      self._write_binary_file(actual_filename, output)
    File "C:\Users\basuke\webkit-trunk\Tools\Scripts\webkitpy\layout_tests\controllers\test_result_writer.py", line 111, in _write_binary_file
      self._filesystem.write_binary_file(path, contents)
    File "C:\Users\basuke\webkit-trunk\Tools\Scripts\webkitpy\common\system\filesystem.py", line 228, in write_binary_file
      with file(path, 'wb') as f:
Comment 2 Basuke Suzuki 2018-07-26 23:23:43 PDT
Created attachment 345901 [details]
PATCH
Comment 3 EWS Watchlist 2018-07-27 06:30:09 PDT
Comment on attachment 345901 [details]
PATCH

Attachment 345901 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8672057

New failing tests:
http/tests/security/canvas-remote-read-remote-video-localhost.html
Comment 4 EWS Watchlist 2018-07-27 06:30:21 PDT
Created attachment 345913 [details]
Archive of layout-test-results from ews200 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews200  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 5 Jonathan Bedard 2018-08-01 09:30:55 PDT
Do we need to apply this everywhere in the Filesystem class?  Ie, creating/deleting directory, listing elements in a directory, ect.
Comment 6 Brent Fulgham 2018-08-01 09:38:50 PDT
(In reply to Jonathan Bedard from comment #5)
> Do we need to apply this everywhere in the Filesystem class?  Ie,
> creating/deleting directory, listing elements in a directory, ect.

That's probably the wisest course of action. There must still be some old WinAPI calls getting hit that need to know they are dealing with modern file lengths. So gross.
Comment 7 Basuke Suzuki 2018-08-01 13:29:04 PDT
(In reply to Brent Fulgham from comment #6)
> (In reply to Jonathan Bedard from comment #5)
> > Do we need to apply this everywhere in the Filesystem class?  Ie,
> > creating/deleting directory, listing elements in a directory, ect.
> 
> That's probably the wisest course of action. There must still be some old
> WinAPI calls getting hit that need to know they are dealing with modern file
> lengths. So gross.

The wisest course of action must be just taking away from Windows :), but yes, at the end we have to do to whole those cases. Should I do that on this patch? Or open another bug?
Comment 8 Jonathan Bedard 2018-08-01 13:37:29 PDT
(In reply to Basuke Suzuki from comment #7)
> (In reply to Brent Fulgham from comment #6)
> > (In reply to Jonathan Bedard from comment #5)
> > > Do we need to apply this everywhere in the Filesystem class?  Ie,
> > > creating/deleting directory, listing elements in a directory, ect.
> > 
> > That's probably the wisest course of action. There must still be some old
> > WinAPI calls getting hit that need to know they are dealing with modern file
> > lengths. So gross.
> 
> The wisest course of action must be just taking away from Windows :), but
> yes, at the end we have to do to whole those cases. Should I do that on this
> patch? Or open another bug?

I would be for including it in this change.
Comment 9 Basuke Suzuki 2018-08-01 13:50:11 PDT
(In reply to Jonathan Bedard from comment #8)
> (In reply to Basuke Suzuki from comment #7)
> > (In reply to Brent Fulgham from comment #6)
> > > (In reply to Jonathan Bedard from comment #5)
> > > > Do we need to apply this everywhere in the Filesystem class?  Ie,
> > > > creating/deleting directory, listing elements in a directory, ect.
> > > 
> > > That's probably the wisest course of action. There must still be some old
> > > WinAPI calls getting hit that need to know they are dealing with modern file
> > > lengths. So gross.
> > 
> > The wisest course of action must be just taking away from Windows :), but
> > yes, at the end we have to do to whole those cases. Should I do that on this
> > patch? Or open another bug?
> 
> I would be for including it in this change.

Okay.
Comment 10 Fujii Hironori 2018-08-01 18:59:23 PDT
(In reply to Basuke Suzuki from comment #0)
> link:
> https://docs.microsoft.com/ja-jp/windows/desktop/FileIO/naming-a-file#maximum-path-length-limitation

According to this document, there is a regstory key to opt-in the long path since Windows 10 version 1607.
HKLM\SYSTEM\CurrentControlSet\Control\FileSystem LongPathsEnabled

Did you try?
Comment 11 Basuke Suzuki 2018-10-02 07:49:44 PDT
(In reply to Fujii Hironori from comment #10)
> (In reply to Basuke Suzuki from comment #0)
> > link:
> > https://docs.microsoft.com/ja-jp/windows/desktop/FileIO/naming-a-file#maximum-path-length-limitation
> 
> According to this document, there is a regstory key to opt-in the long path
> since Windows 10 version 1607.
> HKLM\SYSTEM\CurrentControlSet\Control\FileSystem LongPathsEnabled
> 
> Did you try?

After a long silence, I finally figured out this was not supported by Python 2.7. It looks like LongPathsEnabled support is required for app to be enabled in the manifest and it is supported starting from Python 3.6.

https://docs.python.org/3/using/windows.html#removing-the-max-path-limitation
Comment 12 Alex Christensen 2021-11-01 12:49:24 PDT
Comment on attachment 345901 [details]
PATCH

This has been requesting review for more than one year.  If this is still needed, please rebase and re-request review.
Comment 13 EWS 2022-04-20 17:31:24 PDT
Committed r293132 (249834@main): <https://commits.webkit.org/249834@main>

Reviewed commits have been landed. Closing PR #333 and removing active labels.