git-export: RMST-431: split push and delete tags

cronjobs/src/commands/_git_export_git_tools.py

@@ -108,25 +108,37 @@ def push_mirror(
     # 1) Force update all local branches
     for b in sorted(branches):
         to_push.append(f"+{b}:{b}")
-    # 2) Force update all local tags and delete old tags
+    # 2) Force update all local tags
+    to_delete = []
     for t in sorted(tags):
         deleting = t.startswith("-")
         name = t[1:]
         full = name if name.startswith("refs/tags/") else f"refs/tags/{name}"
         if deleting:
-            to_push.append(f":{full}")
+            to_delete.append(f":{full}")
         else:
             to_push.append(f"+{full}:{full}")
 
-    if not to_push:
+    if not to_push and not to_delete:
         print("Everything up-to-date.")
         return
 
+    # First push the new content, and then delete old tags in a second push.
+    # We do that because if the remote times out or fails with tags deletion,
+    # at least the new content is pushed.
     remote = repo.remotes[REMOTE_NAME]
-    print(f"Pushing to remote {remote.url}:\n - {'\n - '.join(to_push)}")
-    # This is the critical bit: non-fast-forward updates require the '+' force.
-    # The deletions use the ':refs/...'; '+' is ignored for deleted refspecs.
-    remote.push(to_push, callbacks=callbacks)
+    if to_push:
+        print(f"Pushing to remote {remote.url}:\n - {'\n - '.join(to_push)}")
+        # This is the critical bit: non-fast-forward updates require the '+' force.
+        # The deletions use the ':refs/...'; '+' is ignored for deleted refspecs.
+        remote.push(to_push, callbacks=callbacks)
+    else:
+        print("No new commit or tag to push.")
+    if to_delete:
+        print(f"Deleting from remote {remote.url}:\n - {'\n - '.join(to_delete)}")
+        remote.push(to_delete, callbacks=callbacks)
+    else:
+        print("No tag to delete.")
 
 
 def make_lfs_pointer(sha256_hex: str, size: int) -> bytes:

cronjobs/tests/commands/test_git_export_git_tools.py

@@ -8,6 +8,7 @@
     delete_old_tags,
     iter_tree,
     parse_lfs_pointer,
+    push_mirror,
     reset_repo,
     tree_upsert_blobs,
     truncate_branch,
@@ -47,6 +48,12 @@ def mock_ls_remotes():
         yield mock_ls
 
 
+@pytest.fixture
+def mock_remote_push():
+    with mock.patch("pygit2.Remote.push") as mock_push:
+        yield mock_push
+
+
 def test_valid_lfs_pointer():
     data = b"""version https://git-lfs.github.com/spec/v1
 oid sha256:1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef
@@ -100,6 +107,69 @@ def test_reset_repo_creates_local_branches(tmp_repo, mock_ls_remotes):
     assert local_ref.target == remote_ref.target
 
 
+@pytest.mark.parametrize(
+    ("branches", "tags", "expected"),
+    [
+        (
+            [
+                "v1/common",
+            ],
+            [
+                "-refs/tags/timestamp/123",
+                "+refs/tags/timestamp/456",
+            ],
+            [
+                [
+                    "+v1/common:v1/common",
+                    "+refs/tags/timestamp/456:refs/tags/timestamp/456",
+                ],
+                [":refs/tags/timestamp/123"],
+            ],
+        ),
+        (
+            [],
+            [
+                # Test that tags are normalized.
+                "+timestamp/789",
+            ],
+            [
+                [
+                    "+refs/tags/timestamp/789:refs/tags/timestamp/789",
+                ]
+                # And no call for delete.
+            ],
+        ),
+        (
+            [],
+            [
+                "-refs/tags/timestamp/123",
+            ],
+            [
+                [
+                    ":refs/tags/timestamp/123",
+                ]
+                # Only 1 call for delete.
+            ],
+        ),
+    ],
+)
+def test_push_mirror_pushes_branches_and_tags_then_deletes(
+    tmp_repo, mock_remote_push, branches, tags, expected
+):
+    repo = tmp_repo
+
+    push_mirror(
+        repo,
+        branches,
+        tags,
+        callbacks=None,
+    )
+
+    assert mock_remote_push.call_count == len(expected)
+    for expect in expected:
+        mock_remote_push.assert_any_call(expect, callbacks=None)
+
+
 def test_reset_repo_resets_local_branches_to_remote(tmp_repo, mock_ls_remotes):
     repo = tmp_repo
     commit = repo.revparse_single("main")