Bug 1545629 [wpt PR 16335] - Make manifest --work the default; drop the git code, a=testonly
authorSam Sneddon <me@gsnedders.com>
Fri, 17 May 2019 14:35:30 +0000
changeset 477191 e8e78314b1c61f43527821d9d572d80d698b5e03
parent 477190 70cb0cbcc7fd76fca1ca94181f3f83d98837e4d4
child 477192 9712b7e17485467b0b5fddd88dfb81040c8e9111
push id36116
push usershindli@mozilla.com
push dateThu, 06 Jun 2019 10:00:05 +0000
treeherdermozilla-central@fee989d27558 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstestonly
bugs1545629, 16335
milestone69.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1545629 [wpt PR 16335] - Make manifest --work the default; drop the git code, a=testonly Automatic update from web-platform-tests Make manifest --work the default; drop the git code (#16335) See https://github.com/web-platform-tests/rfcs/blob/master/rfcs/manifest-working-copy.md -- wpt-commits: ef764e63e01abacf095258118621ca42c667ab80 wpt-pr: 16335
testing/web-platform/tests/tools/manifest/download.py
testing/web-platform/tests/tools/manifest/manifest.py
testing/web-platform/tests/tools/manifest/tests/test_utils.py
testing/web-platform/tests/tools/manifest/tests/test_vcs.py
testing/web-platform/tests/tools/manifest/update.py
testing/web-platform/tests/tools/manifest/utils.py
testing/web-platform/tests/tools/manifest/vcs.py
--- a/testing/web-platform/tests/tools/manifest/download.py
+++ b/testing/web-platform/tests/tools/manifest/download.py
@@ -10,17 +10,17 @@ from datetime import datetime, timedelta
 
 from six.moves.urllib.request import urlopen
 
 try:
     import zstandard
 except ImportError:
     zstandard = None
 
-from .vcs import Git
+from .utils import git
 
 from . import log
 
 here = os.path.dirname(__file__)
 
 wpt_root = os.path.abspath(os.path.join(here, os.pardir, os.pardir))
 logger = log.get_logger()
 
@@ -35,19 +35,19 @@ def should_download(manifest_path, rebui
     mtime = datetime.fromtimestamp(os.path.getmtime(manifest_path))
     if mtime < datetime.now() - rebuild_time:
         return True
     logger.info("Skipping manifest download because existing file is recent")
     return False
 
 
 def merge_pr_tags(repo_root, max_count=50):
-    git = Git.get_func(repo_root)
+    gitfunc = git(repo_root)
     tags = []
-    for line in git("log", "--format=%D", "--max-count=%s" % max_count).split("\n"):
+    for line in gitfunc("log", "--format=%D", "--max-count=%s" % max_count).split("\n"):
         for ref in line.split(", "):
             if ref.startswith("tag: merge_pr_"):
                 tags.append(ref[5:])
     return tags
 
 
 def score_name(name):
     """Score how much we like each filename, lower wins, None rejects"""
--- a/testing/web-platform/tests/tools/manifest/manifest.py
+++ b/testing/web-platform/tests/tools/manifest/manifest.py
@@ -465,17 +465,17 @@ def _load(logger, tests_root, manifest, 
 
 def load_and_update(tests_root,
                     manifest_path,
                     url_base,
                     update=True,
                     rebuild=False,
                     metadata_path=None,
                     cache_root=None,
-                    working_copy=False,
+                    working_copy=True,
                     types=None,
                     meta_filters=None,
                     write_manifest=True,
                     allow_cached=True):
     logger = get_logger()
 
     manifest = None
     if not rebuild:
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/tools/manifest/tests/test_utils.py
@@ -0,0 +1,14 @@
+import os
+import subprocess
+
+import mock
+
+from .. import utils
+
+
+def test_git_for_path_no_git():
+    this_dir = os.path.dirname(__file__)
+    with mock.patch(
+            "subprocess.check_output",
+            side_effect=subprocess.CalledProcessError(1, "foo")):
+        assert utils.git(this_dir) is None
deleted file mode 100644
--- a/testing/web-platform/tests/tools/manifest/tests/test_vcs.py
+++ /dev/null
@@ -1,14 +0,0 @@
-import os
-import subprocess
-
-import mock
-
-from .. import vcs
-
-
-def test_git_for_path_no_git():
-    this_dir = os.path.dirname(__file__)
-    with mock.patch(
-            "subprocess.check_output",
-            side_effect=subprocess.CalledProcessError(1, "foo")):
-        assert vcs.Git.for_path(this_dir, "/", this_dir) is None
--- a/testing/web-platform/tests/tools/manifest/update.py
+++ b/testing/web-platform/tests/tools/manifest/update.py
@@ -12,17 +12,17 @@ here = os.path.dirname(__file__)
 wpt_root = os.path.abspath(os.path.join(here, os.pardir, os.pardir))
 
 logger = get_logger()
 
 
 def update(tests_root,
            manifest,
            manifest_path=None,
-           working_copy=False,
+           working_copy=True,
            cache_root=None,
            rebuild=False):
     logger.warning("Deprecated; use manifest.load_and_update instead")
     logger.info("Updating manifest")
 
     tree = vcs.get_tree(tests_root, manifest, manifest_path, cache_root,
                         working_copy, rebuild)
     return manifest.update(tree)
@@ -36,37 +36,33 @@ def update_from_cli(**kwargs):
     if not kwargs["rebuild"] and kwargs["download"]:
         download_from_github(path, tests_root)
 
     manifest.load_and_update(tests_root,
                              path,
                              kwargs["url_base"],
                              update=True,
                              rebuild=kwargs["rebuild"],
-                             cache_root=kwargs["cache_root"],
-                             working_copy=kwargs["work"])
+                             cache_root=kwargs["cache_root"])
 
 
 def abs_path(path):
     return os.path.abspath(os.path.expanduser(path))
 
 
 def create_parser():
     parser = argparse.ArgumentParser()
     parser.add_argument(
         "-p", "--path", type=abs_path, help="Path to manifest file.")
     parser.add_argument(
         "--tests-root", type=abs_path, default=wpt_root, help="Path to root of tests.")
     parser.add_argument(
         "-r", "--rebuild", action="store_true", default=False,
         help="Force a full rebuild of the manifest.")
     parser.add_argument(
-        "--work", action="store_true", default=False,
-        help="Build from the working tree rather than the latest commit")
-    parser.add_argument(
         "--url-base", action="store", default="/",
         help="Base url to use as the mount point for tests in this manifest.")
     parser.add_argument(
         "--no-download", dest="download", action="store_false", default=True,
         help="Never attempt to download the manifest.")
     parser.add_argument(
         "--cache-root", action="store", default=os.path.join(wpt_root, ".wptcache"),
         help="Path in which to store any caches (default <tests_root>/.wptcache/")
--- a/testing/web-platform/tests/tools/manifest/utils.py
+++ b/testing/web-platform/tests/tools/manifest/utils.py
@@ -1,10 +1,11 @@
+import os
 import platform
-import os
+import subprocess
 
 from six import BytesIO
 
 def rel_path_to_url(rel_path, url_base="/"):
     assert not os.path.isabs(rel_path), rel_path
     if url_base[0] != "/":
         url_base = "/" + url_base
     if url_base[-1] != "/":
@@ -27,16 +28,37 @@ def to_os_path(path):
     assert os.path.sep == "/" or platform.system() == "Windows"
     if "\\" in path:
         raise ValueError("normalised path contains \\")
     if "/" == os.path.sep:
         return path
     return path.replace("/", os.path.sep)
 
 
+def git(path):
+    def gitfunc(cmd, *args):
+        full_cmd = ["git", cmd] + list(args)
+        try:
+            return subprocess.check_output(full_cmd, cwd=path, stderr=subprocess.STDOUT)
+        except Exception as e:
+            if platform.uname()[0] == "Windows" and isinstance(e, WindowsError):
+                full_cmd[0] = "git.bat"
+                return subprocess.check_output(full_cmd, cwd=path, stderr=subprocess.STDOUT)
+            else:
+                raise
+
+    try:
+        # this needs to be a command that fails if we aren't in a git repo
+        gitfunc("rev-parse", "--show-toplevel")
+    except (subprocess.CalledProcessError, OSError):
+        return None
+    else:
+        return gitfunc
+
+
 class ContextManagerBytesIO(BytesIO):
     def __enter__(self):
         return self
 
     def __exit__(self, *args, **kwargs):
         self.close()
 
 
--- a/testing/web-platform/tests/tools/manifest/vcs.py
+++ b/testing/web-platform/tests/tools/manifest/vcs.py
@@ -1,84 +1,48 @@
 import json
 import os
-import platform
 import stat
-import subprocess
 from collections import deque
 
-from six import iteritems
-
 from .sourcefile import SourceFile
+from .utils import git
 
 MYPY = False
 if MYPY:
     # MYPY is set to True when run under Mypy.
     from typing import Dict, Optional
 
 
 def get_tree(tests_root, manifest, manifest_path, cache_root,
-             working_copy=False, rebuild=False):
+             working_copy=True, rebuild=False):
     tree = None
     if cache_root is None:
         cache_root = os.path.join(tests_root, ".wptcache")
     if not os.path.exists(cache_root):
         try:
             os.makedirs(cache_root)
         except IOError:
             cache_root = None
 
     if not working_copy:
-        tree = Git.for_path(tests_root,
-                            manifest.url_base,
-                            manifest_path=manifest_path,
-                            cache_path=cache_root,
-                            rebuild=rebuild)
+        raise ValueError("working_copy=False unsupported")
+
     if tree is None:
         tree = FileSystem(tests_root,
                           manifest.url_base,
                           manifest_path=manifest_path,
                           cache_path=cache_root,
                           rebuild=rebuild)
     return tree
 
 
-class Git(object):
-    def __init__(self, repo_root, url_base, cache_path, manifest_path=None,
-                 rebuild=False):
-        self.root = repo_root
-        self.git = Git.get_func(repo_root)
-        self.url_base = url_base
-        # rebuild is a noop for now since we don't cache anything
-
-    @staticmethod
-    def get_func(repo_path):
-        def git(cmd, *args):
-            full_cmd = ["git", cmd] + list(args)
-            try:
-                return subprocess.check_output(full_cmd, cwd=repo_path, stderr=subprocess.STDOUT)
-            except Exception as e:
-                if platform.uname()[0] == "Windows" and isinstance(e, WindowsError):
-                    full_cmd[0] = "git.bat"
-                    return subprocess.check_output(full_cmd, cwd=repo_path, stderr=subprocess.STDOUT)
-                else:
-                    raise
-        return git
-
-    @classmethod
-    def for_path(cls, path, url_base, cache_path, manifest_path=None, rebuild=False):
-        git = Git.get_func(path)
-        try:
-            # this needs to be a command that fails if we aren't in a git repo
-            git("rev-parse", "--show-toplevel")
-        except (subprocess.CalledProcessError, OSError):
-            return None
-        else:
-            return cls(path, url_base, cache_path,
-                       manifest_path=manifest_path, rebuild=rebuild)
+class GitHasher(object):
+    def __init__(self, path):
+        self.git = git(path)
 
     def _local_changes(self):
         """get a set of files which have changed between HEAD and working copy"""
         changes = set()
 
         cmd = ["status", "-z", "--ignore-submodules=all"]
         data = self.git(*cmd)
 
@@ -90,49 +54,31 @@ class Git(object):
             else:
                 status = line[:2]
                 if b"R" in status or b"C" in status:
                     in_rename = True
                 changes.add(line[3:])
 
         return changes
 
-    def _show_file(self, path):
-        path = os.path.relpath(os.path.abspath(path), self.root)
-        return self.git("show", "HEAD:%s" % path)
-
     def hash_cache(self):
         # type: () -> Dict[str, Optional[str]]
         """
         A dict of rel_path -> current git object id if the working tree matches HEAD else None
         """
         hash_cache = {}
 
         cmd = ["ls-tree", "-r", "-z", "HEAD"]
         local_changes = self._local_changes()
         for result in self.git(*cmd).split("\0")[:-1]:
             data, rel_path = result.rsplit("\t", 1)
             hash_cache[rel_path] = None if rel_path in local_changes else data.split(" ", 3)[2]
 
         return hash_cache
 
-    def __iter__(self):
-        for rel_path, hash in iteritems(self.hash_cache()):
-            if hash is None:
-                contents = self._show_file(rel_path)
-            else:
-                contents = None
-            yield SourceFile(self.root,
-                             rel_path,
-                             self.url_base,
-                             hash,
-                             contents=contents), True
-
-    def dump_caches(self):
-        pass
 
 
 class FileSystem(object):
     def __init__(self, root, url_base, cache_path, manifest_path=None, rebuild=False):
         from gitignore import gitignore
         self.root = os.path.abspath(root)
         self.url_base = url_base
         self.ignore_cache = None
@@ -140,17 +86,17 @@ class FileSystem(object):
         if cache_path is not None:
             if manifest_path is not None:
                 self.mtime_cache = MtimeCache(cache_path, root, manifest_path, rebuild)
             if gitignore.has_ignore(root):
                 self.ignore_cache = GitIgnoreCache(cache_path, root, rebuild)
         self.path_filter = gitignore.PathFilter(self.root,
                                                 extras=[".git/"],
                                                 cache=self.ignore_cache)
-        git = Git.for_path(root, url_base, cache_path)
+        git = GitHasher(root)
         if git is not None:
             self.hash_cache = git.hash_cache()
         else:
             self.hash_cache = {}
 
     def __iter__(self):
         mtime_cache = self.mtime_cache
         for dirpath, dirnames, filenames in self.path_filter(walk(self.root)):