Bug 151156 – [Win] webkitpy is applying abspath to DOS paths, yielding invalid paths

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Brent Fulgham]

Because we are in the process of moving from pure Cygwin to a pure Windows build, some of our Cygwin tools end up calling native Perl and Python. When this happens, the path types received by Cygwin scripts can sometimes be unexpected.

The most obvious example of this failure is:

> run-webkit-tests -1 fast

... yielding the error:

DumpRenderTree was not found at /home/trybot/WebKit/C:/cygwin/home/trybot/WebKit/WebKitBuild/Relese/bin32/DumpRenderTree

This is happening because a DOS style path is being passed through a Cygwin "abspath' command, which prefixes the passed path with the current directories absolute path. This is wrong behavior, and should not be used when running under Cygwin or native Windows.

[Brent Fulgham]

Created attachment 265320 [details]
Patch

[Brent Fulgham]

Committed r192330: <http://trac.webkit.org/changeset/192330>

[Alexey Proskuryakov]

Comment on attachment 265320 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=265320&action=review

> Tools/Scripts/webkitpy/port/base.py:781
> -            self._results_directory = self._filesystem.abspath(option_val)
> +            if sys.platform in ('win32', 'cygwin'):
> +                self._results_directory = option_val
> +            else:
> +                self._results_directory = self._filesystem.abspath(option_val)

Should self._filesystem.abspath do the right thing on Windows? Or do we expect to use Unix style paths most pf the time?

[Brent Fulgham]

(In reply to comment #3)
> Comment on attachment 265320 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=265320&action=review
> 
> > Tools/Scripts/webkitpy/port/base.py:781
> > -            self._results_directory = self._filesystem.abspath(option_val)
> > +            if sys.platform in ('win32', 'cygwin'):
> > +                self._results_directory = option_val
> > +            else:
> > +                self._results_directory = self._filesystem.abspath(option_val)
> 
> Should self._filesystem.abspath do the right thing on Windows? Or do we
> expect to use Unix style paths most pf the time?

This is just a weird edge case. "self._filesystem.abspath" is just a wrapper around the os.filesystem.abspath. On Cygwin that tries to covert things based on "/cygpath/C/..." (or whatever), and since it doesn't understand DOS paths, it doesn't properly combine the two paths.

When we run under native Windows, the abspath would probably do the right thing. We should probably revisit this special case once that work is finished.

[Csaba Osztrogonác]

(In reply to comment #2)
> Committed r192330: <http://trac.webkit.org/changeset/192330>

It broke layout testing on the Apple Windows bots:
ServerError raised: Failed to start httpd: httpd.exe: Could not open configuration file C:/xampp/apache/layout-test-results/httpd.conf: The system cannot find the path specified.  

Traceback (most recent call last):
  File "/home/buildbot/slave/win-release-tests/build/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 77, in main
    run_details = run(port, options, args, stderr)
  File "/home/buildbot/slave/win-release-tests/build/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 409, in run
    run_details = manager.run(args)
  File "/home/buildbot/slave/win-release-tests/build/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 196, in run
    int(self._options.child_processes), retrying=False)
  File "/home/buildbot/slave/win-release-tests/build/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 259, in _run_tests
    return self._runner.run_tests(self._expectations, test_inputs, tests_to_skip, num_workers, needs_http, needs_websockets, needs_web_platform_test_server, retrying)
  File "/home/buildbot/slave/win-release-tests/build/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 107, in run_tests
    self.start_servers()
  File "/home/buildbot/slave/win-release-tests/build/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 190, in start_servers
    self._port.start_http_server()
  File "/home/buildbot/slave/win-release-tests/build/Tools/Scripts/webkitpy/port/base.py", line 901, in start_http_server
    server.start()
  File "/home/buildbot/slave/win-release-tests/build/Tools/Scripts/webkitpy/layout_tests/servers/http_server_base.py", line 89, in start
    self._pid = self._spawn_process()
  File "/home/buildbot/slave/win-release-tests/build/Tools/Scripts/webkitpy/layout_tests/servers/apache_http_server.py", line 171, in _spawn_process
    raise http_server_base.ServerError('Failed to start %s: %s' % (self._name, err))
ServerError: Failed to start httpd: httpd.exe: Could not open configuration file C:/xampp/apache/layout-test-results/httpd.conf: The system cannot find the path specified.  

No handlers could be found for logger "webkitpy.port.win"




Additionally it broke a webkitpy test on the Apple Windows bots:
[452/1444] webkitpy.layout_tests.run_webkit_tests_integrationtest.RunTest.test_results_directory_relative failed:
  Traceback (most recent call last):
    File "/home/buildbot/slave/win-release-tests/build/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py", line 625, in test_results_directory_relative
      self.assertEqual(user.opened_urls, [path.abspath_to_uri(host.platform, '/tmp/cwd/foo/results.html')])
  AssertionError: Lists differ: ['file://foo/results.html'] != ['file:///tmp/cwd/foo/results....
  
  First differing element 0:
  file://foo/results.html
  file:///tmp/cwd/foo/results.html
  
  - ['file://foo/results.html']
  + ['file:///tmp/cwd/foo/results.html']
  ?         +++++++++

[Brent Fulgham]

(In reply to comment #5)
> (In reply to comment #2)
> > Committed r192330: <http://trac.webkit.org/changeset/192330>
> 
> It broke layout testing on the Apple Windows bots:
> ServerError raised: Failed to start httpd: httpd.exe: Could not open
> configuration file C:/xampp/apache/layout-test-results/httpd.conf: The
> system cannot find the path specified.  
> 

Whoops! I'll correct that.

[Brent Fulgham]

Created attachment 265421 [details]
Patch v2

[Brent Fulgham]

Comment on attachment 265421 [details]
Patch v2

This patch allows us to give Cygwin paths to Python libraries that expect UNIX style paths. Although we generally want to be dealing in native DOS paths on Windows, when the test infrastructure is run inside Cygwin Python, we need to give Python paths that it can handle properly.

[Brent Fulgham]

Committed r192395: <http://trac.webkit.org/changeset/192395>

[Csaba Osztrogonác]

(In reply to comment #9)
> Committed r192395: <http://trac.webkit.org/changeset/192395>

Layout testing is stilk broken on the Apple Windows bots.

[Brent Fulgham]

(In reply to comment #10)
> (In reply to comment #9)
> > Committed r192395: <http://trac.webkit.org/changeset/192395>
> 
> Layout testing is stilk broken on the Apple Windows bots.

Interesting. The tests run when triggered from the command line. The BuildSlave environment must be set differently.

[Brent Fulgham]

I see the problem. We launch Apache with a relative path to the configuration file:

08:18:57.398 6484 Starting httpd server, cmd="C:/xampp/apache/bin/httpd.exe -f "layout-test-results/httpd.conf" -C 'DocumentRoot "C:/cygwin/home/buildbot/slave/win-release-tests/build/LayoutTests/http/tests"' -c 'Alias /js-test-resources "C:/cygwin/home/buildbot/slave/win-release-tests/build/LayoutTests/resources"' -c 'Alias /media-resources "C:/cygwin/home/buildbot/slave/win-release-tests/build/LayoutTests/media"' -c 'TypesConfig "C:/cygwin/home/buildbot/slave/win-release-tests/build/LayoutTests/http/conf/mime.types"' -c 'CustomLog "layout-test-results/access_log.txt" common' -c 'ErrorLog "layout-test-results/error_log.txt"' -c 'PidFile C:/xampp/apache/logs/httpd.pid' -k start -C 'Listen 127.0.0.1:8000' -C 'Listen 127.0.0.1:8080' -C 'Listen 127.0.0.1:8443' -c 'SSLCertificateFile C:/cygwin/home/buildbot/slave/win-release-tests/build/LayoutTests/http/conf/webkit-httpd.pem'"

The -f "layout-tests-results/httpd.conf" is getting converted to the httpd.exe root directory -> "C:/xampp/apache/layout-test-results/httpd.conf".

We should be passing the full path to the 'httpd.conf', since it lives in the test folder.

[Brent Fulgham]

Fix landed in <https://trac.webkit.org/changeset/192431>.

[Ryan Haddad]

(In reply to comment #13)
> Fix landed in <https://trac.webkit.org/changeset/192431>.

The tests are now running, but they are exiting early after 500 failures. Do you think this could still be fallout from this change?

[Csaba Osztrogonác]

(In reply to comment #14)
> (In reply to comment #13)
> > Fix landed in <https://trac.webkit.org/changeset/192431>.
> 
> The tests are now running, but they are exiting early after 500 failures. Do
> you think this could still be fallout from this change?

I think the culprit is this change and the Windows layout 
testing is broken since a week. Brent, are you planning
to fix the regression your patches caused?

[Csaba Osztrogonác]

ping?

[Ryan Haddad]

(In reply to comment #16)
> ping?

I know that Brent was investigating this yesterday on one of the bots. I'll follow up with him.

[Brent Fulgham]

Test machines should be fixed with Commit r192635: <http://trac.webkit.org/changeset/192635>