In WPT importer, abort if there are exportable-but-not-yet-exported commits.

third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py

 # Copyright 2014 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Fetches a copy of the latest state of a W3C test repository and commits.
 
 If this script is given the argument --auto-update, it will also:
  1. Upload a CL.
  2. Trigger try jobs and wait for them to complete.
  3. Make any changes that are required for new failing tests.
  4. Commit the CL.
 
 If this script is given the argument --auto-update, it will also attempt to
 upload a CL, trigger try jobs, and make any changes that are required for
 new failing tests before committing.
 """
 
 import argparse
 import json
 import logging
 import re
 
 from webkitpy.common.net.git_cl import GitCL
 from webkitpy.common.webkit_finder import WebKitFinder
 from webkitpy.layout_tests.models.test_expectations import TestExpectations, TestExpectationParser
+from webkitpy.w3c.common import WPT_REPO_URL, CSS_REPO_URL, WPT_DEST_NAME, CSS_DEST_NAME, exportable_commits_since
+from webkitpy.w3c.local_wpt import LocalWPT
+from webkitpy.w3c.test_importer import TestImporter
 from webkitpy.w3c.update_w3c_test_expectations import W3CExpectationsLineAdder
-from webkitpy.w3c.test_importer import TestImporter
-from webkitpy.w3c.common import WPT_REPO_URL, CSS_REPO_URL, WPT_DEST_NAME, CSS_DEST_NAME
 
 # Settings for how often to check try job results and how long to wait.
 POLL_DELAY_SECONDS = 2 * 60
 TIMEOUT_SECONDS = 180 * 60
 
 _log = logging.getLogger(__file__)
 
 
 class DepsUpdater(object):
 
@@ skipping 13 matching lines @@
 
         if not self.checkout_is_okay(options.allow_local_commits):
             return 1
 
         self.git_cl = GitCL(self.host, auth_refresh_token_json=options.auth_refresh_token_json)
 
         _log.info('Noting the current Chromium commit.')
         _, show_ref_output = self.run(['git', 'show-ref', 'HEAD'])
         chromium_commit = show_ref_output.split()[0]
 
+        assert options.target in ('wpt', 'css')
+        dest_dir_name = WPT_DEST_NAME
+        repo_url = WPT_REPO_URL
+        if options.target != 'wpt':
+            dest_dir_name = CSS_DEST_NAME
+            repo_url = CSS_REPO_URL
+
+        temp_repo_path = self.path_from_webkit_base(dest_dir_name)
+        _log.info('Cloning %s into %s.', repo_url, temp_repo_path)
+        self.run(['git', 'clone', repo_url, temp_repo_path])

[jeffcarp, 2017/01/26 00:17:15]

It would be nice to deduplicate the cloning code. Could LocalWPT.fetch be used
here? (that would also require LocalWPT to be instantiated above and be passed
into exportable_but_not_exported_commits)

[qyearsley, 2017/01/26 00:30:07]

That would be possible, although this code is also used to update the csswg-test
repo now, and currently the LocalWPT.fetch would only fetch the
web-platform-tests repo. It might be confusing to change LocalWPT to take a repo
URL and also handle csswg-test if passed the URL... anyway maybe I should add a
comment here noting that we want to simplify this after csswg-test is merged
into web-platform-tests?

[jeffcarp, 2017/01/27 02:05:41]

Excellent, sgtm

+
         if options.target == 'wpt':
-            import_commit = self.update(WPT_DEST_NAME, WPT_REPO_URL, options.keep_w3c_repos_around, options.revision)
+            commits = self.exportable_but_not_exported_commits(temp_repo_path)
+            if commits:
+                _log.error('There were exportable but not-yet-exported commits: %r', commits)
+                _log.error('Aborting import to prevent clobbering these commits.')
+                return 1
+
+        import_commit = self.update(dest_dir_name, temp_repo_path, options.keep_w3c_repos_around, options.revision)
+
+        if options.target == 'wpt':
             self._copy_resources()
-        elif options.target == 'css':
-            import_commit = self.update(CSS_DEST_NAME, CSS_REPO_URL, options.keep_w3c_repos_around, options.revision)
-        else:
-            raise AssertionError("Unsupported target %s" % options.target)
 
         has_changes = self._has_changes()
         if not has_changes:
             _log.info('Done: no changes to import.')
             return 0
 
         commit_message = self._commit_message(chromium_commit, import_commit)
         self._commit_changes(commit_message)
         _log.info('Done: changes imported and committed.')
 
@@ skipping 36 matching lines @@
         if self.fs.exists(self.path_from_webkit_base(WPT_DEST_NAME)):
             _log.warning('WebKit/%s exists; aborting.', WPT_DEST_NAME)
             return False
 
         if self.fs.exists(self.path_from_webkit_base(CSS_DEST_NAME)):
             _log.warning('WebKit/%s repo exists; aborting.', CSS_DEST_NAME)
             return False
 
         return True
 
+    def exportable_but_not_exported_commits(self, wpt_path):
+        """Checks for commits that might be overwritten by importing.
+
+        Args:
+            wpt_path: The path to a local checkout of web-platform-tests.
+
+        Returns:
+            A list of commits in the Chromium repo that are exportable
+            but not yet exported to the web-platform-tests repo.
+        """
+        local_wpt = LocalWPT(self.host, path=wpt_path)
+        assert self.host.filesystem.exists(wpt_path)
+        _, chromium_commit = local_wpt.most_recent_chromium_commit()
+        return exportable_commits_since(chromium_commit.sha, self.host, local_wpt)
+
     def _copy_resources(self):
         """Copies resources from wpt to LayoutTests/resources.
 
         We copy idlharness.js and testharness.js in wpt to LayoutTests/resources
         in order to use them in non-imported tests.
 
         If this method is changed, the lists of files expected to be identical
         in LayoutTests/PRESUBMIT.py should also be changed.
         """
         resources_to_copy_from_wpt = [
@@ skipping 16 matching lines @@
         stages the generated MANIFEST.json in the git index, ready to commit.
         """
         manifest_command = self.finder.path_from_webkit_base('Tools', 'Scripts', 'webkitpy', 'thirdparty', 'wpt', 'wpt', 'manifest')
         if 'css' in dest_path:
             # Do nothing for csswg-test.
             return
         _log.info('Generating MANIFEST.json')
         self.run([manifest_command, '--work', '--tests-root', dest_path])
         self.run(['git', 'add', self.fs.join(dest_path, 'MANIFEST.json')])
 
-    def update(self, dest_dir_name, url, keep_w3c_repos_around, revision):
+    def update(self, dest_dir_name, temp_repo_path, keep_w3c_repos_around, revision):
         """Updates an imported repository.
 
         Args:
             dest_dir_name: The destination directory name.
-            url: URL of the git repository.
-            revision: Commit hash or None.
+            temp_repo_path: Path to local checkout of W3C test repo.
+            keep_w3c_repos_around: If True, the temp directory won't be cleaned up.
+            revision: A W3C test repo commit hash, or None.
 
         Returns:
             A string for the commit description "<destination>@<commitish>".
         """
-        temp_repo_path = self.path_from_webkit_base(dest_dir_name)
-        _log.info('Cloning %s into %s.', url, temp_repo_path)
-        self.run(['git', 'clone', url, temp_repo_path])
-
         if revision is not None:
             _log.info('Checking out %s', revision)
             self.run(['git', 'checkout', revision], cwd=temp_repo_path)
 
         self.run(['git', 'submodule', 'update', '--init', '--recursive'], cwd=temp_repo_path)
 
         _log.info('Noting the revision we are importing.')
         _, show_ref_output = self.run(['git', 'show-ref', 'origin/master'], cwd=temp_repo_path)
         master_commitish = show_ref_output.split()[0]
 
@@ skipping 265 matching lines @@
         """Returns a dict mapping source to dest name for layout tests that have been renamed."""
         out = self.check_run(['git', 'diff', 'origin/master', '-M100%', '--diff-filter=R', '--name-status'])
         renamed_tests = {}
         for line in out.splitlines():
             _, source_path, dest_path = line.split()
             source_test = self.finder.layout_test_name(source_path)
             dest_test = self.finder.layout_test_name(dest_path)
             if source_test and dest_test:
                 renamed_tests[source_test] = dest_test
         return renamed_tests