sync: skip tags on commits not synced to the destination (bug 2082281)

git_hg_sync/repo_synchronizer.py

@@ -20,6 +20,8 @@
 
 REQUEST_USER_ENV_VAR = "AUTOLAND_REQUEST_USER"
 
+MANUAL_TAG_PUSH_DOC_URL = "https://mozilla-hub.atlassian.net/wiki/spaces/EW/pages/1592754924/Git-Hg-Sync+-+Manually+pushing+a+missing+tag+to+HgMO"
+
 
 class RepoSyncError(Exception):
     """Base exception class for git to mercurial synchronization errors"""
@@ -28,6 +30,15 @@ class RepoSyncError(Exception):
 class MercurialMetadataNotFoundError(RepoSyncError):
     """Raised when a commit is not found"""
 
+    def __init__(self, operation: SyncTagOperation, destination_url: str) -> None:
+        super().__init__(
+            f"Commit {operation.source_commit} for tag {operation.tag} has no "
+            f"Mercurial metadata for {destination_url}; it is likely not on a branch "
+            "synced to that repository. Once it is, push the tag manually: "
+            f"{MANUAL_TAG_PUSH_DOC_URL}"
+        )
+        self.operation = operation
+
 
 class RepoSynchronizer:
     def __init__(
@@ -212,29 +223,35 @@ def sync(
             if not self._commit_has_mercurial_metadata(
                 repo, tag_operation.source_commit
             ):
-                rollback_candidate = self._get_current_cinnabar_state(repo)
-
                 # Tagging can only be done on a commit that already has mercurial
                 # metadata.
                 #
-                # Add mercurial metadata to new commits from synced branches.
-                retry(
-                    "adding mercurial metadata to new git commits for tagging",
-                    lambda: repo.git.execute(
-                        ["git"]
-                        + ["-c", "cinnabar.data=force"]
-                        + ["push"]
-                        + ["--dry-run"]
-                        + [destination_remote]
-                        + refs_to_push,
-                    ),
-                )
+                # Add mercurial metadata to new commits from synced branches. This
+                # only needs doing once, and only if there are branches to push.
+                if refs_to_push and rollback_candidate is None:
+                    rollback_candidate = self._get_current_cinnabar_state(repo)
+                    retry(
+                        "adding mercurial metadata to new git commits for tagging",
+                        lambda: repo.git.execute(
+                            ["git"]
+                            + ["-c", "cinnabar.data=force"]
+                            + ["push"]
+                            + ["--dry-run"]
+                            + [destination_remote]
+                            + refs_to_push,
+                        ),
+                    )
 
-                # Make sure it worked.
+                # If the commit still has no metadata, it is not on any branch synced
+                # to the destination, and retrying the message will never fix that.
+                # Skip this tag rather than failing (and requeuing) the whole message.
                 if not self._commit_has_mercurial_metadata(
                     repo, tag_operation.source_commit
                 ):
-                    raise MercurialMetadataNotFoundError(tag_operation)
+                    exc = MercurialMetadataNotFoundError(tag_operation, destination_url)
+                    sentry_sdk.capture_exception(exc)
+                    logger.error(f"Skipping tag: {exc}")
+                    continue
 
             hg_sha = self._git2hg(repo, tag_operation.source_commit)
             tag_message = f"No bug - Tagging {hg_sha} with {tag_operation.tag} {tag_operation.tag_message_suffix}"

tests/test_repo_synchronizer.py

@@ -6,7 +6,7 @@
 import pytest
 from git import Repo
 from git.exc import GitCommandError
-from utils import hg_cat, hg_log, hg_rev
+from utils import hg_cat, hg_log, hg_rev, hg_run
 
 from git_hg_sync import repo_synchronizer
 from git_hg_sync.__main__ import get_connection, get_queue
@@ -273,6 +273,88 @@ def test_sync_process_no_duplicate_tags_on_error(
     )
 
 
+@pytest.mark.parametrize("with_branch_operation", [False, True])
+def test_sync_process_tag_unsynced_commit(
+    git_source: Path,
+    hg_destination: Path,
+    tmp_path: Path,
+    monkeypatch: pytest.MonkeyPatch,
+    with_branch_operation: bool,
+) -> None:
+    """Tagging a commit that isn't synced to the destination should skip that tag,
+    and still process the rest of the operations."""
+    branch = "bar"
+    tag_branch = "tags"
+    good_tag = "goodtag"
+    bad_tag = "badtag"
+    tag_suffix = "some suffix"
+
+    repo = Repo(git_source)
+
+    # A commit that will be synced to Mercurial (or already is).
+    if with_branch_operation:
+        bar_path = git_source / "bar.txt"
+        bar_path.write_text("BAR CONTENT")
+        repo.index.add([bar_path])
+        synced_commit_sha = repo.index.commit("add bar.txt").hexsha
+    else:
+        synced_commit_sha = repo.rev_parse("HEAD").hexsha
+
+    # A commit that will never be synced to Mercurial.
+    baz_path = git_source / "baz.txt"
+    baz_path.write_text("BAZ CONTENT")
+    repo.index.add([baz_path])
+    unsynced_commit_sha = repo.index.commit("add baz.txt").hexsha
+
+    git_local_repo_path = tmp_path / "clones" / "myrepo"
+    syncrepos = RepoSynchronizer(git_local_repo_path, str(git_source))
+    operations: list[SyncBranchOperation | SyncTagOperation] = [
+        SyncTagOperation(
+            source_commit=unsynced_commit_sha,
+            tag=bad_tag,
+            tags_destination_branch=tag_branch,
+            tag_message_suffix=tag_suffix,
+        ),
+        SyncTagOperation(
+            source_commit=synced_commit_sha,
+            tag=good_tag,
+            tags_destination_branch=tag_branch,
+            tag_message_suffix=tag_suffix,
+        ),
+    ]
+    if with_branch_operation:
+        operations.insert(
+            0,
+            SyncBranchOperation(
+                source_commit=synced_commit_sha, destination_branch=branch
+            ),
+        )
+
+    request_user = "request_user@example.com"
+
+    logger_mock = mock.MagicMock()
+    monkeypatch.setattr(repo_synchronizer, "logger", logger_mock)
+    syncrepos.sync(str(hg_destination), operations, request_user)
+
+    assert any(
+        unsynced_commit_sha in error.args[0] and bad_tag in error.args[0]
+        for error in logger_mock.error.call_args_list
+    ), "Expected an error about the tag on the unsynced commit."
+
+    if with_branch_operation:
+        assert "BAR CONTENT" in hg_cat(hg_destination, "bar.txt", branch)
+
+    tags = hg_run(hg_destination, ["hg", "tags", "-T", "{tag}\n"]).split()
+    assert good_tag in tags
+    assert bad_tag not in tags
+    assert hg_rev(hg_destination, good_tag) == Repo(git_local_repo_path).git.cinnabar(
+        ["git2hg", synced_commit_sha]
+    )
+
+    # The unsynced commit must not have leaked into Mercurial.
+    assert not hg_log(hg_destination, "file('path:baz.txt')")
+
+
 def test_sync_process_multiple_destinations(
     make_hg_repo: Callable,
     git_source: Path,