Bug 1869733: consider all official remotes when finding base ref and patches r=zeid

A previous patch added parsing `git remote -v`, preferring
`mozilla-unified` as the official upstream remote and falling
back to the first official-looking remote if unified was not
found. We assumed that a developer is either using mozilla-unified,
or is using a single-headed repo like central. This overlooks
the fact that it is possible to clone from central, and then
pull other repos in, creating a repo with multiple official
remotes.

Update the `get_upstream_remote` function to find all official
looking remotes instead of a single official remote, and change
the `get_remote_arg` function to return a list of `--remotes`
arguments to be passed to various Git commands. This allows
Git to take all official remotes into consideration and more
precisely find the commits which are not present on any official
upsteams.

Add a test for `get_mozilla_remote_args` while we are here.

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

python/mozversioncontrol/mozversioncontrol/__init__.py

@@ -10,6 +10,7 @@ import shutil
 import subprocess
 from pathlib import Path
 from typing import (
+    Iterator,
     List,
     Optional,
     Union,
@@ -687,41 +688,41 @@ class GitRepository(Repository):
     def head_ref(self):
         return self._run("rev-parse", "HEAD").strip()
 
-    def get_mozilla_upstream_remote(self) -> Optional[str]:
-        """Return the Mozilla-official upstream remote for this repo."""
+    def get_mozilla_upstream_remotes(self) -> Iterator[str]:
+        """Return the Mozilla-official upstream remotes for this repo."""
         out = self._run("remote", "-v")
         if not out:
-            return None
+            return
 
         remotes = out.splitlines()
         if not remotes:
-            return None
+            return
 
-        # Prefer mozilla-unified, then find any other official-looking remote next.
-        for upstream in ("hg.mozilla.org/mozilla-unified", "hg.mozilla.org"):
-            for line in remotes:
-                name, url, action = line.split()
+        for line in remotes:
+            name, url, action = line.split()
 
-                if upstream in url:
-                    return name
+            # Only consider fetch sources.
+            if action != "(fetch)":
+                continue
 
-        return None
+            # Return any `hg.mozilla.org` remotes, ignoring `try`.
+            if "hg.mozilla.org" in url and not url.endswith("hg.mozilla.org/try"):
+                yield name
 
-    def get_mozilla_remote_arg(self) -> str:
-        """Return a `--remotes` argument to limit revisions to relevant upstreams."""
-        official_remote = self.get_mozilla_upstream_remote()
+    def get_mozilla_remote_args(self) -> List[str]:
+        """Return a list of `--remotes` arguments to limit commits to official remotes."""
+        official_remotes = [
+            f"--remotes={remote}" for remote in self.get_mozilla_upstream_remotes()
+        ]
 
-        # Limit remotes to official Firefox repos where possible.
-        remote_arg = f"--remotes={official_remote}" if official_remote else "--remotes"
-
-        return remote_arg
+        return official_remotes if official_remotes else ["--remotes"]
 
     @property
     def base_ref(self):
-        remote_arg = self.get_mozilla_remote_arg()
+        remote_args = self.get_mozilla_remote_args()
 
         refs = self._run(
-            "rev-list", "HEAD", "--topo-order", "--boundary", "--not", remote_arg
+            "rev-list", "HEAD", "--topo-order", "--boundary", "--not", *remote_args
         ).splitlines()
         if refs:
             return refs[-1][1:]  # boundary starts with a prefix `-`
@@ -881,14 +882,14 @@ class GitRepository(Repository):
 
     def get_branch_nodes(self) -> List[str]:
         """Return a list of commit SHAs for nodes on the current branch."""
-        remote_arg = self.get_mozilla_remote_arg()
+        remote_args = self.get_mozilla_remote_args()
 
         return self._run(
             "log",
             "HEAD",
             "--reverse",
             "--not",
-            remote_arg,
+            *remote_args,
             "--pretty=%H",
         ).splitlines()
 

python/mozversioncontrol/test/python.toml

@@ -13,7 +13,9 @@ subsuite = "mozversioncontrol"
 
 ["test_get_commit_patches.py"]
 
-["test_get_upstream_remote.py"]
+["test_get_mozilla_remote_args.py"]
+
+["test_get_upstream_remotes.py"]
 
 ["test_push_to_try.py"]
 

python/mozversioncontrol/test/test_get_mozilla_remote_args.py

@@ -9,39 +9,38 @@ from mozversioncontrol import get_repository_object
 STEPS = {
     "hg": [],
     "git": [
+        "git remote add blah https://example.com/blah",
         """
         git remote add unified hg::https://hg.mozilla.org/mozilla-unified
         git remote add central hg::https://hg.mozilla.org/central
+        git remote add try hg::https://hg.mozilla.org/try
         """,
-        "git remote remove unified",
-        "git remote remove central",
     ],
 }
 
 
-def test_get_upstream_remote(repo):
+def test_get_upstream_remotes(repo):
     # Test is only relevant for Git.
     if not repo.vcs == "git":
         return
 
+    repo.execute_next_step()
+
     vcs = get_repository_object(repo.dir)
-    remote = vcs.get_mozilla_upstream_remote()
+    remotes = vcs.get_mozilla_remote_args()
+
+    assert remotes == [
+        "--remotes"
+    ], "Default `--remotes` passed without finding official remote."
 
     repo.execute_next_step()
-    remote = vcs.get_mozilla_upstream_remote()
-    assert remote == "unified", "Unified remote should be preferred."
 
-    repo.execute_next_step()
-    remote = vcs.get_mozilla_upstream_remote()
-    assert (
-        remote == "central"
-    ), "Any other official looking remote should be returned second."
+    remotes = sorted(vcs.get_mozilla_remote_args())
 
-    repo.execute_next_step()
-    remote = vcs.get_mozilla_upstream_remote()
-    assert (
-        remote is None
-    ), "`None` should be returned if an official remote isn't found."
+    assert remotes == [
+        "--remotes=central",
+        "--remotes=unified",
+    ], "Multiple non-try remote arguments should be found."
 
 
 if __name__ == "__main__":

python/mozversioncontrol/test/test_get_upstream_remotes.py

@@ -0,0 +1,45 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# 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 mozunit
+
+from mozversioncontrol import get_repository_object
+
+STEPS = {
+    "hg": [],
+    "git": [
+        "git remote add blah https://example.com/blah",
+        """
+        git remote add unified hg::https://hg.mozilla.org/mozilla-unified
+        git remote add central hg::https://hg.mozilla.org/central
+        git remote add try hg::https://hg.mozilla.org/try
+        """,
+    ],
+}
+
+
+def test_get_upstream_remotes(repo):
+    # Test is only relevant for Git.
+    if not repo.vcs == "git":
+        return
+
+    repo.execute_next_step()
+
+    vcs = get_repository_object(repo.dir)
+    remotes = vcs.get_mozilla_upstream_remotes()
+
+    assert list(remotes) == [], "No official remotes should be found."
+
+    repo.execute_next_step()
+
+    remotes = sorted(list(vcs.get_mozilla_upstream_remotes()))
+
+    assert remotes == [
+        "central",
+        "unified",
+    ], "Multiple non-try remotes should be found."
+
+
+if __name__ == "__main__":
+    mozunit.main()