Re: [PATCH 0/1] replay: add --revert option to reverse commit changes
- From
Siddharth Asthana <siddharthasthana31@gmail.com>
- Date
- Nov 26, 2025, 19:18 UTC
- Message-ID
- <27fef9e1-bf26-48af-b3df-35948937c891@gmail.com>
- In-Reply-To
- <4d75da90-7e85-573a-bb10-0c63a02b076c@gmx.de>
On 25/11/25 22:55, Johannes Schindelin wrote:
Show 18 quoted lines
> 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.
Show 12 quoted lines
> 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
Show 33 quoted lines
> : 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 >> >>