From aa34de1c89849231e696ca0d8f744876c316db86 Mon Sep 17 00:00:00 2001 From: Julien Cristau Date: Wed, 14 Oct 2026 16:49:30 +0000 Subject: [PATCH] sync: skip tags on commits not synced to the destination (bug 2082281) If a tag points to a commit that isn't on a branch synced to the destination repository, it can't get Mercurial metadata, and the whole message would fail and be requeued forever, blocking the queue. Log an error and report to Sentry instead, pointing to the manual tag push documentation, and carry on with the rest of the message. Also don't attempt to add metadata with a dry-run push when there are no branches to push (the push would then fail with a confusing "nothing changed on remote" error), and only do it once per sync, so the rollback candidate isn't overwritten after metadata was already forced. --- git_hg_sync/repo_synchronizer.py | 49 +++++++++++++------ tests/test_repo_synchronizer.py | 84 +++++++++++++++++++++++++++++++- 2 files changed, 116 insertions(+), 17 deletions(-) diff --git a/git_hg_sync/repo_synchronizer.py b/git_hg_sync/repo_synchronizer.py index d9bd087..6e4d9e2 100644 --- a/git_hg_sync/repo_synchronizer.py +++ b/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}" diff --git a/tests/test_repo_synchronizer.py b/tests/test_repo_synchronizer.py index 930d11d..22c862d 100644 --- a/tests/test_repo_synchronizer.py +++ b/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,