From: Siddharth Asthana Date: Wed, 26 Nov 2025 19:50:15 GMT Subject: Re: [PATCH 1/1] replay: add --revert option to reverse commit changes Message-ID: In-Reply-To: On 26/11/25 23:05, Elijah Newren wrote: > On Wed, Nov 26, 2025 at 3:10 AM Phillip Wood 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 >> Date: Thu Apr 7 15:30:13 2005 -0700 >> >> Revert "E >> " >> >> This reverts commit bceb3acd81ddd36ba0da391fffa48949a1337276. >> >> commit 47f0cc1c1f1911c0047a4d79d79f7c19c6c7151a >> Author: author@name >> 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 --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! > > 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   --advance   --revert Where `--revert ` applies the reverts on top of 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 > 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. > 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.