Subject: Re: [PATCH 0/1] replay: add --revert option to reverse commit changes

From: Siddharth Asthana <[email-masked]>
Date: Thu, 27 Nov 2025 00:48:53 +0530

On 25/11/25 22:55, Johannes Schindelin wrote:
> Hi Siddharth,
>
> On Tue, 25 Nov 2025, Siddharth Asthana wrote:
>
>> The `git replay` command currently supports cherry-picking commits for
>> server-side history rewriting, but lacks the ability to revert them.
>> This patch adds a `--revert` option to enable reversing commits directly
>> on bare repositories.
>>
>> At GitLab, we use replay in Gitaly for efficient server-side operations.
>> Adding revert functionality enables us to reverse problematic commits
>> without client-side roundtrips, reducing network overhead.
>>
>> The implementation leverages the insight that cherry-pick and revert are
>> essentially the same merge operation with swapped arguments. By swapping
>> the base and pickme trees when calling `merge_incore_nonrecursive()`, we
>> effectively reverse the diff direction. The existing conflict handling,
>> ref updates, and atomic transaction support work unchanged.

Hi Johannes,
Thanks for the review!

> Are you reverting rebased Merge Requests commit by commit? If not, I would
> suggest the shortcut to use `merge-tree` directly for the entire Merge
> Request.

That's a great point. At GitLab, we have use cases for both approaches:

1.  For quick undoing an entire MR, the `merge-tree` approach you 
suggest is indeed more efficient and avoids unnecessary intermediate 
conflicts.

2. For commit-by-commit reverts, we need individual revert commits with 
proper attribution (which commit is being reverted) for auditability and 
history clarity. This is particularly useful when only specific commits 
from a merged branch need to be reverted.

I will add a note in the documentation mentioning the `merge-tree` 
alternative for whole-MR reverts.

> That is, if `$BASE` corresponds to the base branch onto which the
> Merge Request was rebased, and `$TIP` corresponds to the Merge Request's
> rebased tip commit, then the following will revert that Merge Request:
>
> 	git merge-tree --merge-base $TIP HEAD $BASE
>
> The upside is that this can potentially avoid a lot of unnecessary merge
> conflicts. The downside is that it does not revert the rebased Merge
> Request commit by commit.
>
> The patch itself looks fine to me, if a bit too extensive on the side of
> adding tests

Agreed. Looking at the tests again, I can consolidate several of them:
- The bare repo test can be merged with the basic revert test
- The multiple commits test overlaps with the basic functionality

I will trim down to essential coverage in v2: basic revert, conflict 
handling, and Reapply behavior.

Thanks,
Siddharth

> : Remember, a nimble test suite that catches a bug once is
> better than a long-running test suite that would catch a bug several times
> _iff_ it didn't tax the developer's patience so much that it is
> interrupted and aborted. You probably agree that Git's CI runtimes are
> already counter-productively long.
>
> Ciao,
> Johannes
>
>> The revert message generation logic is extracted into a new shared
>> `sequencer_format_revert_header()` function in `sequencer.c`, allowing
>> code reuse between `sequencer.c` and `builtin/replay.c`. The commit
>> messages follow `git revert` conventions, including "Revert"/"Reapply"
>> prefixes and the original commit SHA.
>>
>> This patch includes comprehensive tests covering various scenarios:
>> bare repositories, --advance mode, conflicts, reapply behavior, and
>> multiple commits.
>>
>> Siddharth Asthana (1):
>>    replay: add --revert option to reverse commit changes
>>
>>   Documentation/git-replay.adoc |  35 +++++++-
>>   builtin/replay.c              |  86 ++++++++++++++----
>>   sequencer.c                   |  23 +++++
>>   sequencer.h                   |   8 ++
>>   t/t3650-replay-basics.sh      | 160 ++++++++++++++++++++++++++++++++++
>>   5 files changed, 295 insertions(+), 17 deletions(-)
>>
>> -- 
>> 2.51.0
>>
>>