From: Phillip Wood Date: Mon, 16 Mar 2026 16:57:52 GMT Subject: Re: [PATCH v4 2/2] replay: add --revert mode to reverse commit changes Message-ID: In-Reply-To: <20260313054035.26605-3-siddharthasthana31@gmail.com> Hi Siddharth On 13/03/2026 05:40, Siddharth Asthana wrote: > Add a `--revert ` 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 .". 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