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

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

Index: third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py
index 3f3388e218092939a2005afaba0c7b8c48eb3698..e232b88963cdfe033e8b465082faf308ad9443fc 100644
--- a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py
+++ b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py
@@ -23,9 +23,10 @@ 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.update_w3c_test_expectations import W3CExpectationsLineAdder
+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.common import WPT_REPO_URL, CSS_REPO_URL, WPT_DEST_NAME, CSS_DEST_NAME
+from webkitpy.w3c.update_w3c_test_expectations import W3CExpectationsLineAdder
 
 # Settings for how often to check try job results and how long to wait.
 POLL_DELAY_SECONDS = 2 * 60
@@ -59,13 +60,28 @@ class DepsUpdater(object):
         _, 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':
+            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':
-            import_commit = self.update(WPT_DEST_NAME, WPT_REPO_URL, options.keep_w3c_repos_around, options.revision)
             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:
@@ -122,6 +138,21 @@ class DepsUpdater(object):
 
         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.
 
@@ -158,21 +189,18 @@ class DepsUpdater(object):
         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)