Bug 89616 – nrwt: use scm instead of calling svn directly to get the revision in json results generator

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Dirk Pranke]

nrwt: call executive.run_command instead of subprocess.popen to generate json results

[Dirk Pranke]

note that we need to make sure that this works for chromium checkouts (since this code is used for chromium tests that aren't webkit checkouts); possibly that's why it didn't work before.

[Ojan Vafai]

I think the problem before was that the SCM code didn't work correctly under cygwin SVN. IIRC that has been fixed.

[Dirk Pranke]

We wouldn't care about cygwin svn for chromium, I don't think, since we don't run cygwin svn on the bots (just win32 svn). Users might be running cygwin svn, but they presumably wouldn't be generating json results.

OTOH, apple win does use cygwin svn, so we should presumably make sure it works there.

[Dirk Pranke]

resetting the owner in case someone else wants to take a look, as these bugs aren't on my immediate to-do list.

[Zan Dobersek]

Created attachment 156625 [details]
Using detect_scm_system

The current code checks for the .svn subdirectory in the LayoutTests directory. This doesn't work out well if using Subversion 1.7 or newer. OTOH, using detect_scm_system to detect the correct SCM also makes this work when Git is being used.

Is this approach possible or are there some other caveats?

[Zan Dobersek]

(In reply to comment #5)
> Is this approach possible or are there some other caveats?

One I just discovered is the resulting test-webkitpy failures.

[917/1577] webkitpy.layout_tests.run_webkit_tests_integrationtest.MainTest.test_repeat_each_iterations_num_tests erred:
  Traceback (most recent call last):
...
    File "/home/zan/Dev/webkit/weekend/Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py", line 388, in _get_svn_revision
      scm = detect_scm_system(in_directory, filesystem=self._filesystem)
    File "/home/zan/Dev/webkit/weekend/Tools/Scripts/webkitpy/common/checkout/scm/detection.py", line 81, in detect_scm_system
      return SCMDetector(filesystem, Executive()).detect_scm_system(path, patch_directories)
    File "/home/zan/Dev/webkit/weekend/Tools/Scripts/webkitpy/common/checkout/scm/detection.py", line 69, in detect_scm_system
      if SVN.in_working_directory(absolute_path):
    File "/home/zan/Dev/webkit/weekend/Tools/Scripts/webkitpy/common/checkout/scm/svn.py", line 93, in in_working_directory
      exit_code = Executive().run_command(svn_info_args, cwd=path, return_exit_code=True)
    File "/home/zan/Dev/webkit/weekend/Tools/Scripts/webkitpy/common/system/executive.py", line 402, in run_command
      close_fds=self._should_close_fds())
    File "/home/zan/Dev/webkit/weekend/Tools/Scripts/webkitpy/common/system/executive.py", line 458, in popen
      return subprocess.Popen(*args, **kwargs)
    File "/usr/lib/python2.7/subprocess.py", line 679, in __init__
      errread, errwrite)
    File "/usr/lib/python2.7/subprocess.py", line 1249, in _execute_child
      raise child_exception
  OSError: [Errno 2] No such file or directory: '/test.checkout/LayoutTests'

common.checkout.scm.svn runs commands on the real filesystem and not the mock filesystem used in settings.

[Zan Dobersek]

Created attachment 161689 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 161689 [details]
Patch

LGTM.

[WebKit Review Bot]

Comment on attachment 161689 [details]
Patch

Clearing flags on attachment: 161689

Committed r127302: <http://trac.webkit.org/changeset/127302>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Ojan Vafai]

This caused me to get the following error whenever I run the tests. I have a WebKit checkout embedded in a Chromium checkout. Don't know if that matters. Also, I'm on Ubuntu Precise. It's failing to run because /work/chromium/src/build does not exist. Not sure where that path is coming from.

ScriptError raised: Failed to run "[u'git', u'log', u'-25', u'/work/chromium/src/build']" exit_code: 128
    File "/work/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 110, in run
      unexpected_result_count = manager.run(args)
    File "/work/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 450, in run
      self._upload_json_files(summarized_results, result_summary, individual_test_timings)
    File "/work/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 570, in _upload_json_files
      self._options.master_name)
    File "/work/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/layout_package/json_layout_results_generator.py", line 76, in __init__
      self.generate_json_output()
    File "/work/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py", line 244, in generate_json_output
      json_object = self.get_json()
    File "/work/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py", line 284, in get_json
      self._insert_generic_metadata(results_for_builder)
    File "/work/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py", line 529, in _insert_generic_metadata
      self._get_svn_revision(path),
    File "/work/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py", line 391, in _get_svn_revision
      return scm.svn_revision(in_directory)
    File "/work/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/common/checkout/scm/git.py", line 250, in svn_revision
      git_log = self.run([self.executable_name, 'log', '-25', path])
    File "/work/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/common/checkout/scm/scm.py", line 76, in run
      decode_output=decode_output)
    File "/work/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 423, in run_command
      (error_handler or self.default_error_handler)(script_error)
    File "/work/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 340, in default_error_handler
      raise error
Failed to execute Tools/Scripts/new-run-webkit-tests at /work/chromium/src/third_party/WebKit/Tools/Scripts/run-webkit-tests line 124.

[Dirk Pranke]

That path is coming from chromium.py's implementation of repository_paths() ... how can you have a chromium checkout that doesn't have src/build ? 

(I just tripped over this myself, but this was from running the chromium port in a webkit checkout that had a hand-copied set of binaries and not a full chromium checkout).

[Kenichi Ishibashi]

(In reply to comment #12)
> That path is coming from chromium.py's implementation of repository_paths() ... how can you have a chromium checkout that doesn't have src/build ? 
> 
> (I just tripped over this myself, but this was from running the chromium port in a webkit checkout that had a hand-copied set of binaries and not a full chromium checkout).

I have the same problem. src/build is exists, but git log fails because src/build is outside repository when you put unmanaged WebKit repository in chromium's third_party directory. I have a patch to fix this. Will upload soon.

[Zan Dobersek]

I think the patch is OK but the error occurs as expected in such conditions (copying the WebKit checkout into Chromium checkout and similar).

I'm not sure what a proper fix would be, though. Seeing that using different checkouts together is common, that should definitely be taken into account.

[noel gordon]

Could we roll out until we have a proper fix?

[Kenichi Ishibashi]

(In reply to comment #15)
> Could we roll out until we have a proper fix?

I posted a proposal fix. https://bugs.webkit.org/show_bug.cgi?id=95662