run-webkit-tests tests on win32 after r127302
Created attachment 166990 [details] Patch
Comment on attachment 166990 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=166990&action=review > Tools/Scripts/webkitpy/common/checkout/scm/svn.py:94 > + exit_code = executive.run_command(svn_info_args, cwd=path, return_exit_code=True, shell=(sys.platform == 'win32')) shell is the devil. Causes all sorts of oddness. Can we solve this w/o shell?
Using the default chromium install, we point to an svn.bat in depot_tools; without shell=True, calling "svn info" won't work. An alternative would be to make cls.executable a method and probe to figure out if we should use svn.exe or svn.bat.
Don't we already have hacks for svn.bat? http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/host.py#L82
Good question. I thought we had something like that but it must not be working. I will dig in and find out why not.
okay, turns out we don't actually call _initialize_scm() unless we're running on a bot; this was done to avoid the hit of calling out to svn if we didn't really need the revisions. Unfortunately, the code in json_result_generator doesn't know about that, and doesn't trap the error or call initialize_scm. It's easy enough to call initialize_scm() again, but it's harder (if not impossible) to figure out if we need to or not at that layer of the code without doing some refactoring.
I'll post a patch that fixes the crash for now and add a FIXME: to mention the slowdown and possible refactoring.
Created attachment 167003 [details] Patch
Comment on attachment 167003 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=167003&action=review > Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py:391 > + self._port.host._initialize_scm() > scm = SCMDetector(self._filesystem, self._executive).detect_scm_system(in_directory) > if scm: > return scm.svn_revision(in_directory) Um... Why can't it just use self._port.host.scm now then? Why re-detect?
(In reply to comment #9) > (From update of attachment 167003 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=167003&action=review > > > Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py:391 > > + self._port.host._initialize_scm() > > scm = SCMDetector(self._filesystem, self._executive).detect_scm_system(in_directory) > > if scm: > > return scm.svn_revision(in_directory) > > Um... Why can't it just use self._port.host.scm now then? Why re-detect? The directory matters, since we look once for the webkit checkout and once for the chromium checkout (and they might actually be different).
Comment on attachment 167003 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=167003&action=review >>> Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py:391 >>> return scm.svn_revision(in_directory) >> >> Um... Why can't it just use self._port.host.scm now then? Why re-detect? > > The directory matters, since we look once for the webkit checkout and once for the chromium checkout (and they might actually be different). I see. We should probably add an explaination of why we call _initialize_scm() here. Also, don't we have a non-private API for causing initialize_scm to fire? Could we just grab scm()? Or if we're going to call private functions on host, why not just call _engage_awesome_windows_hacks directly?
(In reply to comment #11) > (From update of attachment 167003 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=167003&action=review > > >>> Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py:391 > >>> return scm.svn_revision(in_directory) > >> > >> Um... Why can't it just use self._port.host.scm now then? Why re-detect? > > > > The directory matters, since we look once for the webkit checkout and once for the chromium checkout (and they might actually be different). > > I see. We should probably add an explaination of why we call _initialize_scm() here. Will do. Also, don't we have a non-private API for causing initialize_scm to fire? Could we just grab scm()? Or if we're going to call private functions on host, why not just call _engage_awesome_windows_hacks directly? grabbing scm() directly won't call initialize_scm(). We should either make that happen, or make initialize_scm() public IMO.
Created attachment 167018 [details] make initialize_scm() public
Comment on attachment 167018 [details] make initialize_scm() public View in context: https://bugs.webkit.org/attachment.cgi?id=167018&action=review > Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py:389 > + # We can't reuse an existing scm object because we need to look at the specific directories. Rather, because the specific directories we're interested in may come from a different checkout.
(In reply to comment #14) > (From update of attachment 167018 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=167018&action=review > > > Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py:389 > > + # We can't reuse an existing scm object because we need to look at the specific directories. > > Rather, because the specific directories we're interested in may come from a different checkout. That is clearer, thanks!
Committed r130356: <http://trac.webkit.org/changeset/130356>