diff mbox series

[2/3] filter-branch: drop multiple-ancestor warning

Message ID YEj8zy5JX+KvlfGJ@coredump.intra.peff.net (mailing list archive)
State Accepted
Commit 98fe9e666fe4a595cf5396fd8d5c57b380c782b2
Headers show
Series sha256 fixes for filter-branch | expand

Commit Message

Jeff King March 10, 2021, 5:07 p.m. UTC
When a ref maps to a commit that is neither rewritten nor kept by
filter-branch (e.g., because it was eliminated by rev-list's pathspec
selection), we rewrite it to its nearest ancestor.

Since the initial commit in 6f6826c52b (Add git-filter-branch,
2007-06-03), we have warned when there are multiple such ancestors in
the map file. However, the warning code is impossible to trigger these
days. Since a0e46390d3 (filter-branch: fix ref rewriting with
--subdirectory-filter, 2008-08-12), we find the ancestor using "rev-list
-1", so it can only ever have a single value.

This code is made doubly confusing by the fact that we append to the map
file when mapping ancestors. However, this can never yield multiple
values because:

  - we explicitly check whether the map already exists, and if so, do
    nothing (so our "append" will always be to a file that does not
    exist)

  - even if we were to try mapping twice, the process to do so is
    deterministic. I.e., we'd always end up with the same ancestor for a
    given sha1. So warning about it would be pointless; there is no
    ambiguity.

So swap out the warning code for a BUG (which we'll simplify further in
the next commit). And let's stop using the append operator to make the
ancestor-mapping code less confusing.

Signed-off-by: Jeff King <peff@peff.net>
---
 git-filter-branch.sh | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

Comments

Junio C Hamano March 10, 2021, 10:16 p.m. UTC | #1
Jeff King <peff@peff.net> writes:

> When a ref maps to a commit that is neither rewritten nor kept by
> filter-branch (e.g., because it was eliminated by rev-list's pathspec
> selection), we rewrite it to its nearest ancestor.
>
> Since the initial commit in 6f6826c52b (Add git-filter-branch,
> 2007-06-03), we have warned when there are multiple such ancestors in
> the map file. However, the warning code is impossible to trigger these
> days. Since a0e46390d3 (filter-branch: fix ref rewriting with
> --subdirectory-filter, 2008-08-12), we find the ancestor using "rev-list
> -1", so it can only ever have a single value.

;-)
diff mbox series

Patch

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index fea7964617..a1e80bd552 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -492,7 +492,7 @@  then
 		sha1=$(git rev-parse "$ref"^0)
 		test -f "$workdir"/../map/$sha1 && continue
 		ancestor=$(git rev-list --simplify-merges -1 "$ref" "$@")
-		test "$ancestor" && echo $(map $ancestor) >> "$workdir"/../map/$sha1
+		test "$ancestor" && echo $(map $ancestor) >"$workdir"/../map/$sha1
 	done < "$tempdir"/heads
 fi
 
@@ -534,14 +534,7 @@  do
 		fi
 	;;
 	*)
-		# NEEDSWORK: possibly add -Werror, making this an error
-		warn "WARNING: '$ref' was rewritten into multiple commits:"
-		warn "$rewritten"
-		warn "WARNING: Ref '$ref' points to the first one now."
-		rewritten=$(echo "$rewritten" | head -n 1)
-		git update-ref -m "filter-branch: rewrite to first" \
-				"$ref" $rewritten $sha1 ||
-			die "Could not rewrite $ref"
+		die "BUG: multiple ancestors in map file?"
 	;;
 	esac
 	git update-ref -m "filter-branch: backup" "$orig_namespace$ref" $sha1 ||