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

From: Elijah Newren <newren@gmail.com>
Date: Nov 26, 2025, 17:35 UTC
Message-ID: <CABPp-BEF1RVs7n4xf6ihyPUmxrhgpG6VEpZLrX=MTgcBWBRbfA@mail.gmail.com>
In-Reply-To: <d563b68b-e01d-4b18-bd84-86f36e61a70d@gmail.com>

On Wed, Nov 26, 2025 at 3:10 AM Phillip Wood <phillip.wood123@gmail.com> wrote:

Show 72 quoted lines

>
> 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.

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.

(I do tend to see ranges reverted by a single big revert, the way Johannes suggested, rather than as a range of individual reverts, 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, 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.