Re: [PATCH v4 2/2] replay: add --revert mode to reverse commit changes

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Siddharth

On 13/03/2026 05:40, Siddharth Asthana wrote:
> Add a `--revert <branch>` mode to git replay that undoes the changes
> introduced by the specified commits. Like --onto and --advance, --revert
> is a standalone mode: it takes a branch argument and updates that branch
> with the newly created revert commits.
> 
> At GitLab, we need this in Gitaly for reverting commits directly on bare
> repositories without requiring a working tree checkout.
> 
> The approach is the same as sequencer.c's do_pick_commit() -- cherry-pick
> and revert are just the same three-way merge with swapped arguments:
> 
>    - Cherry-pick: merge(ancestor=parent, ours=current, theirs=commit)
>    - Revert: merge(ancestor=commit, ours=current, theirs=parent)
> 
> We swap the base and pickme trees passed to merge_incore_nonrecursive()
> to reverse the diff direction.
> 
> Reverts are processed newest-first (matching git revert behavior) to
> reduce conflicts by peeling off changes from the top. Each revert
> builds on the result of the previous one via the last_commit fallback
> in the main replay loop, rather than relying on the parent-mapping
> used for cherry-pick.
> 
> Revert commit messages follow the usual git revert conventions: prefixed
> with "Revert" (or "Reapply" when reverting a revert), and including
> "This reverts commit <hash>.". The author is set to the current user
> rather than preserving the original author, matching git revert behavior.

This addresses all my comments on the previous version. I've one minor 
comment below but I'm not sure its worth a re-roll on its own.

>   
>   test_expect_success 'cannot advance target ... ordering would be ill-defined' '
> -	echo "fatal: cannot advance target with multiple sources because ordering would be ill-defined" >expect &&
> +	cat >expect <<-\EOF &&
> +	fatal: '"'"'--advance'"'"' cannot be used with multiple revision ranges because the ordering would be ill-defined

This quoting is a bit strange - we'd normally drop the '\' form '\EOF' 
above use ${SQ} instead. git grep shows there are 220 instances of ${SQ} 
vs 31 instances of '"'"' in the test suite.

Thanks

Phillip


> +	EOF
>   	test_must_fail git replay --advance=main main topic1 topic2 2>actual &&
>   	test_cmp expect actual
>   '
> @@ -398,4 +399,105 @@ test_expect_success 'invalid replay.refAction value' '
>   	test_grep "invalid.*replay.refAction.*value" error
>   '
>   
> +test_expect_success 'argument to --revert must be a reference' '
> +	echo "fatal: argument to --revert must be a reference" >expect &&
> +	oid=$(git rev-parse main) &&
> +	test_must_fail git replay --revert=$oid topic1..topic2 2>actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'cannot revert with multiple sources' '
> +	cat >expect <<-\EOF &&
> +	fatal: '"'"'--revert'"'"' cannot be used with multiple revision ranges because the ordering would be ill-defined
> +	EOF
> +	test_must_fail git replay --revert main main topic1 topic2 2>actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'using replay --revert to revert commits' '
> +	# Reuse existing topic4 branch (has commits I and J on top of main)
> +	START=$(git rev-parse topic4) &&
> +	test_when_finished "git branch -f topic4 $START" &&
> +
> +	# Revert commits I and J
> +	git replay --revert topic4 topic4~2..topic4 &&
> +
> +	# Verify the revert commits were created (newest-first ordering
> +	# means J is reverted first, then I on top)
> +	git log --format=%s -4 topic4 >actual &&
> +	cat >expect <<-\EOF &&
> +	Revert "I"
> +	Revert "J"
> +	J
> +	I
> +	EOF
> +	test_cmp expect actual &&
> +
> +	# Verify commit message format includes hash (tip is Revert "I")
> +	test_commit_message topic4 <<-EOF &&
> +	Revert "I"
> +
> +	This reverts commit $(git rev-parse I).
> +	EOF
> +
> +	# Verify reflog message
> +	git reflog topic4 -1 --format=%gs >reflog-msg &&
> +	echo "replay --revert topic4" >expect-reflog &&
> +	test_cmp expect-reflog reflog-msg
> +'
> +
> +test_expect_success 'using replay --revert in bare repo' '
> +	# Reuse existing topic4 in bare repo
> +	START=$(git -C bare rev-parse topic4) &&
> +	test_when_finished "git -C bare update-ref refs/heads/topic4 $START" &&
> +
> +	# Revert commit J in bare repo
> +	git -C bare replay --revert topic4 topic4~1..topic4 &&
> +
> +	# Verify revert was created
> +	git -C bare log -1 --format=%s topic4 >actual &&
> +	echo "Revert \"J\"" >expect &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'revert of revert uses Reapply' '
> +	# Use topic4 and first revert J, then revert the revert
> +	START=$(git rev-parse topic4) &&
> +	test_when_finished "git branch -f topic4 $START" &&
> +
> +	# First revert J
> +	git replay --revert topic4 topic4~1..topic4 &&
> +	REVERT_J=$(git rev-parse topic4) &&
> +
> +	# Now revert the revert - should become Reapply
> +	git replay --revert topic4 topic4~1..topic4 &&
> +
> +	# Verify Reapply prefix and message format
> +	test_commit_message topic4 <<-EOF
> +	Reapply "J"
> +
> +	This reverts commit $REVERT_J.
> +	EOF
> +'
> +
> +test_expect_success 'git replay --revert with conflict' '
> +	# conflict branch has C.conflict which conflicts with topic1s C
> +	test_expect_code 1 git replay --revert conflict B..topic1
> +'
> +
> +test_expect_success 'git replay --revert incompatible with --contained' '
> +	test_must_fail git replay --revert topic4 --contained topic4~1..topic4 2>error &&
> +	test_grep "requires --onto" error
> +'
> +
> +test_expect_success 'git replay --revert incompatible with --onto' '
> +	test_must_fail git replay --revert topic4 --onto main topic4~1..topic4 2>error &&
> +	test_grep "cannot be used together" error
> +'
> +
> +test_expect_success 'git replay --revert incompatible with --advance' '
> +	test_must_fail git replay --revert topic4 --advance main topic4~1..topic4 2>error &&
> +	test_grep "cannot be used together" error
> +'
> +
>   test_done