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

From: Siddharth Asthana <siddharthasthana31@gmail.com>
Date: Nov 26, 2025, 19:50 UTC
Message-ID: <d4362def-9be6-4191-8846-f3180a5a5d59@gmail.com>
In-Reply-To: <CABPp-BEF1RVs7n4xf6ihyPUmxrhgpG6VEpZLrX=MTgcBWBRbfA@mail.gmail.com>

On 26/11/25 23:05, Elijah Newren wrote:

Show 71 quoted lines

> On Wed, Nov 26, 2025 at 3:10 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>> Hi Siddharth
>>
>> On 25/11/2025 17:00, Siddharth Asthana wrote:
>>> diff --git a/Documentation/git-replay.adoc b/Documentation/git-replay.adoc
>>> index dcb26e8a8e..ad7dc08622 100644
>>> --- a/Documentation/git-replay.adoc
>>> +++ b/Documentation/git-replay.adoc
>>> @@ -54,6 +54,18 @@ which uses the target only as a starting point without updating it.
>>> [...]
>>> +To revert a range of commits:
>>> +
>>> +------------
>>> +$ git replay --revert --onto main feature~3..feature
>>> +------------
>>> +
>>> +This creates new commits on top of 'main' that reverse the changes introduced
>>> +by the last three commits on 'feature'. The 'feature' branch is updated to
>>> +point at the last of these revert commits. The 'main' branch is not updated
>>> +in this case.
>> I'm struggling to understand when I'd want to do this. Why would I want
>> to update 'feature' to point to the reverted version of its last tree
>> commits rebased onto 'main'? In order to understand I ran the first
>> tests case which does
>>
>>          git replay --onto topic1 --revert topic1..topic2
>>
>> after fixing it by adding --ref-action=print the resulting commit log
>> looks like
>>
>> commit d337fab78e90008835f74e890039b464a0308cbe
>> Author: author@name <bogus@email@address>
>> Date:   Thu Apr 7 15:30:13 2005 -0700
>>
>>       Revert "E
>>       "
>>
>>       This reverts commit bceb3acd81ddd36ba0da391fffa48949a1337276.
>>
>> commit 47f0cc1c1f1911c0047a4d79d79f7c19c6c7151a
>> Author: author@name <bogus@email@address>
>> Date:   Thu Apr 7 15:30:13 2005 -0700
>>
>>       Revert "D
>>       "
>>
>>       This reverts commit d953cf2dcc1da8b51934e43fd83dac72d0e267c7.
>>
>>
>> The commits are empty because the original they are reverting each
>> create a new file which is then present in the base revision but not in
>> either of the merge heads when we revert. This suggests to me that it is
>> not a very realistic test and I'm still scratching my head to see where
>> "git replay --onto <commit> --revert" is useful.
>>
>> If '--revert' does not make sense with '--onto' then perhaps it should
>> be a new mode that takes a ref and acts like '--advance' but reverts the
>> commits rather than cherry-picking them. When reverting a range of
>> commits it would reduce the likelihood of conflicts to revert then in
>> reverse order so we should either recommend passing '--reverse' or make
>> that the default when '--revert' is given.
>>
>> As you can see in the log output above the new function to format the
>> revert subject lines is buggy. If you had used test_commit_message() to
>> check the commit message, rather than just grepping for ^Revert the
>> tests would have picked that up.
>>
>> Thanks
>>
>> Phillip
> I was going to say the same thing, but from a different angle.

Hi Elijah,

thanks for the architectural clarity!

Show 6 quoted lines

>
> The sequencer in git is used for three different types of operations:
> rebasing, cherry-picking, and reverting a range (with a sequence of
> reverts rather than one big revert).  In replay, these correspond to
> --onto, --advance, and the new thing you are trying to add.  As such,
> it should be its own new mode.

This makes complete sense. I was treating `--revert` as a modifier when it should be a third mode alongside `--onto` and `--advance`.

I will restructure so that the user specifies exactly one of:
   --onto <newbase>
   --advance <branch>
   --revert <target>

Where `--revert <target>` applies the reverts on top of <target> and updates that ref.

>
> (I do tend to see ranges reverted by a single big revert, the way
> Johannes suggested, rather than as a range of individual reverts,

The commit-by-commit approach is useful when you need:
- Individual revert commits with proper "This reverts commit X" messages
- The ability to later cherry-pick specific reverts
- Clear history showing which commit caused which revert

But I will add documentation noting the `merge-tree` alternative for cases where a single combined revert is preferred.

Thanks, Siddharth

Show 7 quoted lines

>   so
> to me the utility of the new mode looks low, but perhaps others find
> more utility in it.  Or maybe the intent is to only use it with a
> revision range that is only one commit long?)
>
> Phillip also went into more detail about why "--onto $COMMIT --revert"
> specifically doesn't make sense.  I'd also say

> "--advance $BRANCH
> --revert" doesn't read well because to users, "revert" means going
> back while "advance" means going forward,

Exactly - combining these is semantically confusing even if it could be made to work technically.

Show 10 quoted lines

>   so it's a rather confusing
> command line to make them wrap their head around.
>
> And yes, Siddharth, you were right that the new mode should be
> incompatible with --contained, but that's because --contained is a
> special modifier of --onto.  --onto, --advance, and --revert are three
> different modes that are incompatible with each other.  Once you've
> checked for that incompatibility between the three modes, then you can
> either check that whenever --contained is specified, either --onto is
> as well, or neither --advance nor --revert are.

Right. The check becomes:
1. Exactly one of --onto, --advance, --revert must be specified
2. --contained requires --onto

This is much cleaner than my current approach of pairwise incompatibility checks.