Bug 1962882 - Make mozversioncontrol.repo.Repository into a real abstract base class r=ahal

Differential Revision: https://phabricator.services.mozilla.com/D246861

python/mozversioncontrol/mozversioncontrol/repo/base.py

@@ -32,7 +32,7 @@ def get_tool_path(tool: Optional[Union[str, Path]] = None):
     return str(path)
 
 
-class Repository:
+class Repository(abc.ABC):
     """A class wrapping utility methods around version control repositories.
 
     This class is abstract and never instantiated. Obtain an instance by
@@ -42,8 +42,6 @@ class Repository:
     all methods require this.
     """
 
-    __metaclass__ = abc.ABCMeta
-
     def __init__(self, path: Path, tool: Optional[str] = None):
         self.path = str(path.resolve())
         self._tool = Path(get_tool_path(tool)) if tool else None
@@ -127,11 +125,13 @@ class Repository:
         """True if git cinnabar is installed."""
         return False
 
-    @abc.abstractproperty
+    @property
+    @abc.abstractmethod
     def name(self):
         """Name of the tool."""
 
-    @abc.abstractproperty
+    @property
+    @abc.abstractmethod
     def head_ref(self):
         """Hash of HEAD revision."""
 
@@ -140,7 +140,8 @@ class Repository:
     def is_cinnabar_repo(self) -> bool:
         """True if the repo is a git cinnabar repo"""
 
-    @abc.abstractproperty
+    @property
+    @abc.abstractmethod
     def base_ref(self):
         """Hash of revision the current topic branch is based on."""
 
@@ -155,7 +156,8 @@ class Repository:
     def base_ref_as_commit(self):
         """Git hash of revision the current topic branch is based on."""
 
-    @abc.abstractproperty
+    @property
+    @abc.abstractmethod
     def branch(self):
         """Current branch or bookmark the checkout has active."""
 

python/mozversioncontrol/mozversioncontrol/repo/jj.py

@@ -71,7 +71,7 @@ class JujutsuRepository(Repository):
         """
         self._run("log", "-n0")
 
-    def resolve_to_change(self, revset: str) -> Optional[str]:
+    def _resolve_to_change(self, revset: str) -> Optional[str]:
         change_id = self._run_read_only(
             "log", "--no-graph", "-n1", "-r", revset, "-T", "change_id.short()"
         ).rstrip()
@@ -88,17 +88,17 @@ class JujutsuRepository(Repository):
         # directly to a git command, it must be converted to a commit id first
         # (eg via resolve_to_commit). This isn't done here because
         # callers should be aware when they're dropping down to git semantics.
-        return self.resolve_to_change("@")
+        return self._resolve_to_change("@")
 
     def is_cinnabar_repo(self) -> bool:
         return self._git.is_cinnabar_repo()
 
     @property
     def base_ref(self):
-        ref = self.resolve_to_change("latest(roots(::@ & mutable())-)")
+        ref = self._resolve_to_change("latest(roots(::@ & mutable())-)")
         return ref if ref else self.head_ref
 
-    def resolve_to_commit(self, revset):
+    def _resolve_to_commit(self, revset):
         commit = self._run_read_only(
             "log", "--no-graph", "-r", f"latest({revset})", "-T", "commit_id"
         ).rstrip()
@@ -108,7 +108,7 @@ class JujutsuRepository(Repository):
         return self._git.base_ref_as_hg()
 
     def base_ref_as_commit(self):
-        return self.resolve_to_commit(self.base_ref)
+        return self._resolve_to_commit(self.base_ref)
 
     @property
     def branch(self):
@@ -175,7 +175,7 @@ class JujutsuRepository(Repository):
     def diff_stream(self, rev=None, extensions=(), exclude_file=None, context=8):
         if rev is None:
             rev = "latest((@ ~ empty()) | @-)"
-        rev = self.resolve_to_commit(rev)
+        rev = self._resolve_to_commit(rev)
         return self._git.diff_stream(
             rev=rev, extensions=extensions, exclude_file=exclude_file, context=context
         )
@@ -322,10 +322,10 @@ class JujutsuRepository(Repository):
 
         return list(reversed(self._run_read_only(*cmd).splitlines()))
 
-    def looks_like_change_id(self, id):
+    def _looks_like_change_id(self, id):
         return len(id) > 0 and all(letter >= "k" and letter <= "z" for letter in id)
 
-    def looks_like_commit_id(self, id):
+    def _looks_like_commit_id(self, id):
         return len(id) > 0 and all(letter in string.hexdigits for letter in id)
 
     def get_commit_patches(self, nodes: List[str]) -> List[bytes]:
@@ -333,7 +333,7 @@ class JujutsuRepository(Repository):
         # Warning: tests, at least, may call this with change ids rather than
         # commit ids.
         nodes = [
-            id if self.looks_like_commit_id(id) else self.resolve_to_commit(id)
+            id if self._looks_like_commit_id(id) else self._resolve_to_commit(id)
             for id in nodes
         ]
         return [
@@ -366,7 +366,7 @@ class JujutsuRepository(Repository):
                 p.write_text(content)
             # Update the jj commit with the changes we just made.
             self._snapshot()
-            yield self.resolve_to_change("@")
+            yield self._resolve_to_change("@")
         finally:
             self._run("operation", "restore", opid)
 

python/mozversioncontrol/mozversioncontrol/repo/source.py

@@ -2,6 +2,7 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this,
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
+import contextlib
 import os
 from pathlib import Path
 from typing import Dict, Union
@@ -62,6 +63,9 @@ class SrcRepository(Repository):
     def get_changed_files(self, diff_filter="ADM", mode="unstaged", rev=None):
         return []
 
+    def diff_stream(self, rev=None, extensions=(), exclude_file=None, context=None):
+        pass
+
     def get_outgoing_files(self, diff_filter="ADM", upstream=None):
         return []
 
@@ -71,6 +75,9 @@ class SrcRepository(Repository):
     def forget_add_remove_files(self, *paths: Union[str, Path]):
         pass
 
+    def get_ignored_files_finder(self):
+        return FileListFinder([])
+
     def git_ignore(self, path):
         """This function reads the mozilla-central/.gitignore file and creates a
         list of the patterns to ignore
@@ -147,6 +154,12 @@ class SrcRepository(Repository):
     def get_commits(self, head=None, limit=None, follow=None):
         return []
 
+    def get_commit_patches(self, nodes: str):
+        return []
+
+    def try_commit(self, commit_message: str, changed_files=None):
+        return contextlib.nullcontext()
+
     def get_last_modified_time_for_file(self, path: Path):
         """Return last modified in VCS time for the specified file."""
         raise MissingVCSTool

python/mozversioncontrol/test/conftest.py

@@ -65,6 +65,15 @@ SETUP = {
         jj bookmark track master@upstream
         """,
     ],
+    "src": [
+        """
+        echo "foo" > foo
+        echo "bar" > bar
+        mkdir config
+        echo 1.0 > config/milestone.txt
+        """,
+        "",
+    ],
 }
 
 
@@ -86,7 +95,7 @@ def shell(cmd, working_dir):
         subprocess.check_call(step, shell=True, cwd=working_dir)
 
 
-@pytest.fixture(params=["git", "hg", "jj"])
+@pytest.fixture(params=["git", "hg", "jj", "src"])
 def repo(tmpdir, request):
     if request.param == "jj":
         if os.getenv("MOZ_AVOID_JJ_VCS") not in (None, "0", ""):
@@ -101,6 +110,11 @@ def repo(tmpdir, request):
     steps = SETUP[vcs]
 
     if hasattr(request.module, "STEPS"):
+        if vcs == "src" and vcs not in request.module.STEPS:
+            # Special-case SourceRepository: most tests do not handle this case,
+            # so allow it to be skipped if STEPS is defined but not for src.
+            # (Tests without STEPS will need to skip manually.)
+            pytest.skip("not applicable for src repo")
         steps.extend(request.module.STEPS[vcs])
 
     repo_dir = (tmpdir / "repo").resolve()

python/mozversioncontrol/test/test_context_manager.py

@@ -12,13 +12,15 @@ def test_context_manager(repo):
         "git": ["show", "--no-patch"],
         "hg": ["tip"],
         "jj": ["show", "@-"],
+        "src": ["echo", "src"],
     }[repo.vcs]
 
     vcs = get_repository_object(repo.dir)
     output_subprocess = vcs._run(*cmd)
     if repo.vcs == "hg":
         assert vcs._client.server is None
-    assert "Initial commit" in output_subprocess
+    if repo.vcs != "src":
+        assert "Initial commit" in output_subprocess
 
     with vcs:
         if repo.vcs == "hg":

python/mozversioncontrol/test/test_push_to_try.py

@@ -14,6 +14,9 @@ from mozversioncontrol import MissingVCSExtension, get_repository_object
 
 
 def test_push_to_try(repo, monkeypatch):
+    if repo.vcs == "src":
+        pytest.skip("src repo cannot push")
+
     commit_message = "commit message"
     vcs = get_repository_object(repo.dir)
 

python/mozversioncontrol/test/test_try_commit.py

@@ -17,7 +17,8 @@ def test_try_commit(repo):
     # Create a non-empty commit.
     try:
         with vcs.try_commit(commit_message, {"try_task_config.json": "{}"}) as head:
-            assert vcs.get_changed_files(rev=head) == ["try_task_config.json"]
+            if vcs.name != "src":
+                assert vcs.get_changed_files(rev=head) == ["try_task_config.json"]
     except MissingVCSExtension:
         pytest.xfail("Requires the Mercurial evolve extension.")