filter-repo: ensure branches are updated as we go

When we prune a commit for being empty, there is no update to the branch
associated with the commit in the fast-import stream.  If the parent
commit had been associated with a different branch, then the branch
associated with the pruned commit would not be updated without
additional measures.  In the past, we resolved this by recording that
the branch needed an update in _seen_refs.  While this works, it is a
bit more complicated than just issuing an immediate Reset.  Also, note
that we need to avoid calling callbacks on that Reset because those
could rename branches (again, if the commit-callback already renamed
once) causing us to not update the intended branch.

There was actually one testcase where the old method didn't work: when a
branch was pruned away to nothing.  A testcase accidentally encoded the
wrong behavior, hiding this problem.  Fix the testcase to check for
correct behavior.

Signed-off-by: Elijah Newren <newren@gmail.com>

git-filter-repo

@@ -50,6 +50,8 @@ __all__ = ["Blob", "Reset", "FileChange", "Commit", "Tag", "Progress",
            "string_to_date", "date_to_string",
            "record_id_rename", "GitUtils", "FilteringOptions", "RepoFilter"]
 
+deleted_hash = b'0'*40
+
 def gettext_poison(msg):
   if "GIT_TEST_GETTEXT_POISON" in os.environ: # pragma: no cover
     return "# GETTEXT POISON #"
@@ -3091,18 +3093,16 @@ class RepoFilter(object):
 
     # Now print the resulting commit, or if prunable skip it
     if not commit.dumped:
+      self._seen_refs[commit.branch] = None # was seen, doesn't need (another) reset
       if not self._prunable(commit, new_1st_parent,
                             aux_info['had_file_changes'], orig_parents):
-        self._seen_refs[commit.branch] = None # was seen, doesn't need reset
         commit.dump(self._output)
         self._record_remapping(commit, orig_parents)
       else:
         rewrite_to = new_1st_parent or commit.first_parent()
-        # We skip empty commits, but want to keep track to make sure our branch
-        # still gets set and/or updated appropriately.
-        if rewrite_to:
-          self._seen_refs[commit.branch] = rewrite_to # need reset
         commit.skip(new_id = rewrite_to)
+        reset = Reset(commit.branch, rewrite_to or deleted_hash)
+        reset.dump(self._output)
         self._commit_renames[commit.original_id] = None
 
     # Show progress
@@ -3289,18 +3289,6 @@ class RepoFilter(object):
 
   def _handle_final_commands(self):
     self._finalize_handled = True
-    for ref, value in self._seen_refs.items():
-      if value is not None:
-        # Create a reset
-        reset = Reset(ref, value)
-
-        # Call any user callback to allow them to modify the reset
-        if self._reset_callback:
-          self._reset_callback(reset, self.callback_metadata())
-
-        # Now print the resulting reset
-        reset.dump(self._output)
-
     self._done_callback and self._done_callback()
 
     if not self._args.quiet:
@@ -3364,7 +3352,6 @@ class RepoFilter(object):
     return refs_to_nuke
 
   def _record_metadata(self, metadata_dir, orig_refs, refs_nuked):
-    deleted_hash = b'0'*40
     self._flush_renames()
     with open(os.path.join(metadata_dir, b'commit-map'), 'bw') as f:
       f.write(("%-40s %s\n" % (_("old"), _("new"))).encode())

t/t9390-filter-repo.sh

@@ -1061,13 +1061,15 @@ test_expect_success '--target' '
 		git checkout -b other &&
 		echo hello >world &&
 		git add world &&
-		git commit -m init
+		git commit -m init &&
+		git checkout -b unique
 	) &&
-	git -C target rev-parse other >target/expect &&
+	git -C target rev-parse unique >target/expect &&
 	git filter-repo --source analyze_me --target target --path fake_submodule --force --debug &&
-	git -C target rev-parse other >target/actual &&
-	test_cmp target/expect target/actual &&
-	test 2 = $(git -C target rev-list --count master)
+	test 2 = $(git -C target rev-list --count master) &&
+	test_must_fail git -C target rev-parse other &&
+	git -C target rev-parse unique >target/actual &&
+	test_cmp target/expect target/actual
 '
 
 test_expect_success '--refs' '

t/t9391/unusual.py

@@ -125,4 +125,4 @@ filter._input = stream
 filter._setup_output()
 filter._sanity_checks_handled = True
 filter.run()
-assert counts == collections.Counter({fr.Blob: 1, fr.Commit: 3, fr.Reset: 1})
+assert counts == collections.Counter({fr.Blob: 1, fr.Commit: 3})