[PATCH v4 0/2] replay: add --revert mode to reverse commit changes
- From
Siddharth Asthana <siddharthasthana31@gmail.com>
- Date
- Mar 13, 2026, 05:40 UTC
- Message-ID
- <20260313054035.26605-1-siddharthasthana31@gmail.com>
- In-Reply-To
- <20260218234215.89326-1-siddharthasthana31@gmail.com>
Hi,
git replay currently supports cherry-picking (--advance) and rebasing (--onto), but not reverting. We need this at GitLab for Gitaly to reverse commits directly on bare repositories without a checkout.
The approach is the same as sequencer.c -- cherry-pick and revert are just the same three-way merge with swapped arguments. We swap the base and pickme trees passed to merge_incore_nonrecursive() to reverse the diff direction.
Patch 1 extracts the full revert message formatting logic into a new sequencer_format_revert_message() function, following Phillip's suggestion to move everything into one shared function rather than just the header. refer_to_commit() is updated to take a struct repository and a bool instead of replay_opts so it works outside the sequencer.
Patch 2 adds --revert <branch> as a standalone mode. Reverts are processed newest-first (matching git revert) to reduce conflicts by peeling off changes from the top.
The series is based on top of d181b9354c (The 13th batch, 2026-03-07).
CI: https://gitlab.com/gitlab-org/git/-/pipelines/2329880894 The msvc-meson / Chocolatey failures are pre-existing infrastructure issues unrelated to this series.
Changes in v4: - Replaced sequencer_format_revert_header() with a more complete sequencer_format_revert_message() that handles everything: subject prefix, commit reference via refer_to_commit(), and merge-parent references -- per Phillip - Updated refer_to_commit() signature to take (struct repository *r, bool use_commit_reference) instead of (struct replay_opts *opts) - Reverts are now newest-first (revs.reverse = 0 for --revert), chaining on last_commit rather than the parent mapping - Changed doc example to cross-branch scenario and restored the merge-tree NOTE - Updated error message format to "'--revert' cannot be used with multiple revision ranges..." (and same for --advance) - Empty revert commits are now dropped, consistent with cherry-pick - Link to v3: https://public-inbox.org/git/20260218234215.89326-1-siddharthasthana31@gmail.com/ - Link to v2: https://public-inbox.org/git/20251202201611.22137-1-siddharthasthana31@gmail.com/ - Link to v1: https://public-inbox.org/git/20251125170056.34489-1-siddharthasthana31@gmail.com/
Thanks, Siddharth
--- Siddharth Asthana (2): sequencer: extract revert message formatting into shared function replay: add --revert mode to reverse commit changes
Documentation/git-replay.adoc | 43 ++++++++- builtin/replay.c | 46 ++++++---- replay.c | 165 ++++++++++++++++++++++++---------- replay.h | 11 ++- sequencer.c | 78 +++++++++------- sequencer.h | 14 +++ t/t3650-replay-basics.sh | 114 +++++++++++++++++++++-- 7 files changed, 364 insertions(+), 107 deletions(-)
Range-diff versus v3:
1: 9d686bcdfe ! 1: bdc710b265 sequencer: extract revert message formatting into shared function
@@ Commit message
sequencer: extract revert message formatting into shared function
The logic for formatting revert commit messages (handling "Revert" and
- "Reapply" cases) is currently duplicated between sequencer.c and will be
- needed by builtin/replay.c.
+ "Reapply" cases, appending "This reverts commit <ref>.", and handling
+ merge-parent references) currently lives inline in do_pick_commit().
+ The upcoming replay --revert mode needs to reuse this logic.
- Extract this logic into a new sequencer_format_revert_header() function
- that can be shared. The function handles both regular reverts ("Revert
- "<subject>"") and revert-of-revert cases ("Reapply "<subject>"").
- When an oid is provided, the function appends the full commit hash and
- period; otherwise the caller should append the commit reference.
+ Extract all of this into a new sequencer_format_revert_message()
+ function. The function takes a repository, the subject line, commit,
+ parent, a use_commit_reference flag, and the output strbuf. It handles
+ both regular reverts ("Revert "<subject>"") and revert-of-revert cases
+ ("Reapply "<subject>""), and uses refer_to_commit() internally to
+ format the commit reference.
- Update do_pick_commit() to use the new helper, eliminating code
- duplication while preserving the special handling for commit_use_reference.
+ Update refer_to_commit() to take a struct repository parameter instead
+ of relying on the_repository, and a bool instead of reading from
+ replay_opts directly. This makes it usable from the new shared function
+ without pulling in sequencer-specific state.
Signed-off-by: Siddharth Asthana <siddharthasthana31@gmail.com>
## sequencer.c ##
+@@ sequencer.c: static int should_edit(struct replay_opts *opts) {
+ return opts->edit;
+ }
+
+-static void refer_to_commit(struct replay_opts *opts,
+- struct strbuf *msgbuf, struct commit *commit)
++static void refer_to_commit(struct repository *r, struct strbuf *msgbuf,
++ const struct commit *commit,
++ bool use_commit_reference)
+ {
+- if (opts->commit_use_reference) {
++ if (use_commit_reference) {
+ struct pretty_print_context ctx = {
+ .abbrev = DEFAULT_ABBREV,
+ .date_mode.type = DATE_SHORT,
+ };
+- repo_format_commit_message(the_repository, commit,
++ repo_format_commit_message(r, commit,
+ "%h (%s, %ad)", msgbuf, &ctx);
+ } else {
+ strbuf_addstr(msgbuf, oid_to_hex(&commit->object.oid));
@@ sequencer.c: static int do_pick_commit(struct repository *r,
*/
@@ sequencer.c: static int do_pick_commit(struct repository *r,
base = commit;
base_label = msg.label;
next = parent;
-@@ sequencer.c: static int do_pick_commit(struct repository *r,
- if (opts->commit_use_reference) {
- strbuf_commented_addf(&ctx->message, comment_line_str,
- "*** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***");
+ next_label = msg.parent_label;
+- if (opts->commit_use_reference) {
+- strbuf_commented_addf(&ctx->message, comment_line_str,
+- "*** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***");
- } else if (skip_prefix(msg.subject, "Revert \"", &orig_subject) &&
- /*
- * We don't touch pre-existing repeated reverts, because
@@ sequencer.c: static int do_pick_commit(struct repository *r,
- strbuf_addstr(&ctx->message, "Reapply \"");
- strbuf_addstr(&ctx->message, orig_subject);
- strbuf_addstr(&ctx->message, "\n");
-+ strbuf_addstr(&ctx->message, "\nThis reverts commit ");
- } else {
+- } else {
- strbuf_addstr(&ctx->message, "Revert \"");
- strbuf_addstr(&ctx->message, msg.subject);
- strbuf_addstr(&ctx->message, "\"\n");
-+ sequencer_format_revert_header(&ctx->message, msg.subject, NULL);
- }
+- }
- strbuf_addstr(&ctx->message, "\nThis reverts commit ");
- refer_to_commit(opts, &ctx->message, commit);
+- refer_to_commit(opts, &ctx->message, commit);
+-
+- if (commit->parents && commit->parents->next) {
+- strbuf_addstr(&ctx->message, ", reversing\nchanges made to ");
+- refer_to_commit(opts, &ctx->message, parent);
+- }
+- strbuf_addstr(&ctx->message, ".\n");
++ sequencer_format_revert_message(r, msg.subject, commit,
++ parent,
++ opts->commit_use_reference,
++ &ctx->message);
+ } else {
+ const char *p;
- if (commit->parents && commit->parents->next) {
@@ sequencer.c: int sequencer_pick_revisions(struct repository *r,
return res;
}
-+void sequencer_format_revert_header(struct strbuf *out,
-+ const char *orig_subject,
-+ const struct object_id *oid)
++void sequencer_format_revert_message(struct repository *r,
++ const char *subject,
++ const struct commit *commit,
++ const struct commit *parent,
++ bool use_commit_reference,
++ struct strbuf *message)
+{
-+ const char *revert_subject;
++ const char *orig_subject;
+
-+ if (skip_prefix(orig_subject, "Revert \"", &revert_subject) &&
-+ /*
-+ * We don't touch pre-existing repeated reverts, because
-+ * theoretically these can be nested arbitrarily deeply,
-+ * thus requiring excessive complexity to deal with.
-+ */
-+ !starts_with(revert_subject, "Revert \"")) {
-+ strbuf_addstr(out, "Reapply \"");
-+ strbuf_addstr(out, revert_subject);
-+ strbuf_addch(out, '\n');
++ if (use_commit_reference) {
++ strbuf_commented_addf(message, comment_line_str,
++ "*** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***");
++ } else if (skip_prefix(subject, "Revert \"", &orig_subject) &&
++ /*
++ * We don't touch pre-existing repeated reverts, because
++ * theoretically these can be nested arbitrarily deeply,
++ * thus requiring excessive complexity to deal with.
++ */
++ !starts_with(orig_subject, "Revert \"")) {
++ strbuf_addstr(message, "Reapply \"");
++ strbuf_addstr(message, orig_subject);
++ strbuf_addstr(message, "\n");
+ } else {
-+ strbuf_addstr(out, "Revert \"");
-+ strbuf_addstr(out, orig_subject);
-+ strbuf_addstr(out, "\"\n");
++ strbuf_addstr(message, "Revert \"");
++ strbuf_addstr(message, subject);
++ strbuf_addstr(message, "\"\n");
+ }
++ strbuf_addstr(message, "\nThis reverts commit ");
++ refer_to_commit(r, message, commit, use_commit_reference);
+
-+ strbuf_addstr(out, "\nThis reverts commit ");
-+ if (oid) {
-+ strbuf_addstr(out, oid_to_hex(oid));
-+ strbuf_addstr(out, ".\n");
++ if (commit->parents && commit->parents->next) {
++ strbuf_addstr(message, ", reversing\nchanges made to ");
++ refer_to_commit(r, message, parent, use_commit_reference);
+ }
++ strbuf_addstr(message, ".\n");
+}
+
void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag)
@@ sequencer.h: int sequencer_determine_whence(struct repository *r, enum commit_wh
int sequencer_get_update_refs_state(const char *wt_dir, struct string_list *refs);
+/*
-+ * Formats a revert commit message following standard Git conventions.
-+ * Handles both regular reverts ("Revert \"<subject>\"") and revert of revert
-+ * cases ("Reapply \"<subject>\""). Adds "This reverts commit <oid>." if oid
-+ * is provided, otherwise just adds "This reverts commit " and the caller
-+ * should append the commit reference.
++ * Formats a complete revert commit message following standard Git conventions.
++ * Handles regular reverts ("Revert \"<subject>\""), revert of revert cases
++ * ("Reapply \"<subject>\""), and the --reference style. Appends "This reverts
++ * commit <ref>." using either the abbreviated or full commit reference
++ * depending on use_commit_reference. Also handles merge-parent references.
+ */
-+void sequencer_format_revert_header(struct strbuf *out,
-+ const char *orig_subject,
-+ const struct object_id *oid);
++void sequencer_format_revert_message(struct repository *r,
++ const char *subject,
++ const struct commit *commit,
++ const struct commit *parent,
++ bool use_commit_reference,
++ struct strbuf *message);
+
#endif /* SEQUENCER_H */
2: 066269706e ! 2: bea6229575 replay: add --revert mode to reverse commit changes
@@ Commit message
We swap the base and pickme trees passed to merge_incore_nonrecursive()
to reverse the diff direction.
+ Reverts are processed newest-first (matching git revert behavior) to
+ reduce conflicts by peeling off changes from the top. Each revert
+ builds on the result of the previous one via the last_commit fallback
+ in the main replay loop, rather than relying on the parent-mapping
+ used for cherry-pick.
+
Revert commit messages follow the usual git revert conventions: prefixed
with "Revert" (or "Reapply" when reverting a revert), and including
"This reverts commit <hash>.". The author is set to the current user
@@ Commit message
Helped-by: Phillip Wood <phillip.wood123@gmail.com>
Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Helped-by: Junio C Hamano <gitster@pobox.com>
+ Helped-by: Toon Claes <toon@iotcl.com>
Signed-off-by: Siddharth Asthana <siddharthasthana31@gmail.com>
## Documentation/git-replay.adoc ##
@@ Documentation/git-replay.adoc: all commits they have since `base`, playing them
+To revert commits on a branch:
+
+------------
-+$ git replay --revert main main~2..main
++$ git replay --revert main topic~2..topic
+------------
+
-+This reverts the last two commits on `main`, creating two revert commits
-+on top of `main`, and updates `main` to point at the result.
++This reverts the last two commits from `topic`, creating revert commits on
++top of `main`, and updates `main` to point at the result. This is useful when
++commits from `topic` were previously merged or cherry-picked into `main` and
++need to be undone.
++
++NOTE: For reverting an entire merge request as a single commit (rather than
++commit-by-commit), consider using `git merge-tree --merge-base $TIP HEAD $BASE`
++which can avoid unnecessary merge conflicts.
+
GIT
---
@@ builtin/replay.c: int cmd_replay(int argc,
/* Parse ref action mode from command line or config */
ref_mode = get_ref_action_mode(repo, ref_action);
+@@ builtin/replay.c: int cmd_replay(int argc,
+ * some options changing these values if we think they could
+ * be useful.
+ */
+- revs.reverse = 1;
++ /*
++ * Cherry-pick/rebase need oldest-first ordering so that each
++ * replayed commit can build on its already-replayed parent.
++ * Revert needs newest-first ordering (like git revert) to
++ * reduce conflicts by peeling off changes from the top.
++ */
++ revs.reverse = opts.revert ? 0 : 1;
+ revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
+ revs.topo_order = 1;
+ revs.simplify_history = 0;
+@@ builtin/replay.c: int cmd_replay(int argc,
+ * Detect and warn if we override some user specified rev
+ * walking options.
+ */
+- if (revs.reverse != 1) {
+- warning(_("some rev walking options will be overridden as "
+- "'%s' bit in 'struct rev_info' will be forced"),
+- "reverse");
+- revs.reverse = 1;
++ {
++ int desired_reverse = opts.revert ? 0 : 1;
++ if (revs.reverse != desired_reverse) {
++ warning(_("some rev walking options will be overridden as "
++ "'%s' bit in 'struct rev_info' will be forced"),
++ "reverse");
++ revs.reverse = desired_reverse;
++ }
+ }
+ if (revs.sort_order != REV_SORT_IN_GRAPH_ORDER) {
+ warning(_("some rev walking options will be overridden as "
@@ builtin/replay.c: int cmd_replay(int argc,
goto cleanup;
@@ replay.c
#include "strmap.h"
#include "tree.h"
+-/*
+- * We technically need USE_THE_REPOSITORY_VARIABLE for DEFAULT_ABBREV, but
+- * do not want to use the_repository.
+- */
+-#define the_repository DO_NOT_USE_THE_REPOSITORY
+enum replay_mode {
+ REPLAY_MODE_PICK,
+ REPLAY_MODE_REVERT,
+};
-+
+
static const char *short_commit_name(struct repository *repo,
struct commit *commit)
- {
@@ replay.c: static char *get_author(const char *message)
return NULL;
}
@@ replay.c: static char *get_author(const char *message)
+ subject_len = find_commit_subject(message, &subject_start);
+ subject = xmemdupz(subject_start, subject_len);
+
-+ sequencer_format_revert_header(msg, subject, &commit->object.oid);
++ sequencer_format_revert_message(repo, subject, commit,
++ commit->parents ? commit->parents->item : NULL,
++ false, msg);
+
+ free(subject);
+ repo_unuse_commit_buffer(repo, commit, message);
@@ replay.c: static void get_ref_information(struct repository *repo,
+ }
+ *onto = peel_committish(repo, *branch_name, option_name);
+ if (rinfo->positive_refexprs > 1)
-+ die(_("cannot %s target with multiple sources because ordering would be ill-defined"),
-+ option_name + 2); /* skip "--" prefix */
++ die(_("'%s' cannot be used with multiple revision ranges "
++ "because the ordering would be ill-defined"),
++ option_name);
+}
+
static void set_up_replay_mode(struct repository *repo,
@@ replay.c: static struct commit *pick_regular_commit(struct repository *repo,
+ merge_opt->branch2 = NULL;
if (!result->clean)
return NULL;
-- /* Drop commits that become empty */
-- if (oideq(&replayed_base_tree->object.oid, &result->tree->object.oid) &&
-+ /* Drop commits that become empty (only for picks) */
-+ if (mode == REPLAY_MODE_PICK &&
-+ oideq(&replayed_base_tree->object.oid, &result->tree->object.oid) &&
+ /* Drop commits that become empty */
+ if (oideq(&replayed_base_tree->object.oid, &result->tree->object.oid) &&
!oideq(&pickme_tree->object.oid, &base_tree->object.oid))
return replayed_base;
- return create_commit(repo, result->tree, pickme, replayed_base);
@@ replay.c: int replay_revisions(struct rev_info *revs,
last_commit = pick_regular_commit(revs->repo, commit, replayed_commits,
- onto, &merge_opt, &result);
-+ onto, &merge_opt, &result, mode);
++ mode == REPLAY_MODE_REVERT ? last_commit : onto,
++ &merge_opt, &result, mode);
if (!last_commit)
break;
@@ t/t3650-replay-basics.sh: test_expect_success 'no base or negative ref gives no-
test_must_fail git replay --advance=main --contained \
topic1..topic2 2>actual &&
test_cmp expect actual
+ '
+
+ test_expect_success 'cannot advance target ... ordering would be ill-defined' '
+- echo "fatal: cannot advance target with multiple sources because ordering would be ill-defined" >expect &&
++ cat >expect <<-\EOF &&
++ fatal: '"'"'--advance'"'"' cannot be used with multiple revision ranges because the ordering would be ill-defined
++ EOF
+ test_must_fail git replay --advance=main main topic1 topic2 2>actual &&
+ test_cmp expect actual
+ '
@@ t/t3650-replay-basics.sh: test_expect_success 'invalid replay.refAction value' '
test_grep "invalid.*replay.refAction.*value" error
'
@@ t/t3650-replay-basics.sh: test_expect_success 'invalid replay.refAction value' '
+'
+
+test_expect_success 'cannot revert with multiple sources' '
-+ echo "fatal: cannot revert target with multiple sources because ordering would be ill-defined" >expect &&
++ cat >expect <<-\EOF &&
++ fatal: '"'"'--revert'"'"' cannot be used with multiple revision ranges because the ordering would be ill-defined
++ EOF
+ test_must_fail git replay --revert main main topic1 topic2 2>actual &&
+ test_cmp expect actual
+'
@@ t/t3650-replay-basics.sh: test_expect_success 'invalid replay.refAction value' '
+ # Revert commits I and J
+ git replay --revert topic4 topic4~2..topic4 &&
+
-+ # Verify the revert commits were created
++ # Verify the revert commits were created (newest-first ordering
++ # means J is reverted first, then I on top)
+ git log --format=%s -4 topic4 >actual &&
+ cat >expect <<-\EOF &&
-+ Revert "J"
+ Revert "I"
++ Revert "J"
+ J
+ I
+ EOF
+ test_cmp expect actual &&
+
-+ # Verify commit message format includes hash
++ # Verify commit message format includes hash (tip is Revert "I")
+ test_commit_message topic4 <<-EOF &&
-+ Revert "J"
++ Revert "I"
+
-+ This reverts commit $(git rev-parse J).
++ This reverts commit $(git rev-parse I).
+ EOF
+
+ # Verify reflog messagebase-commit: d181b9354cf85b44455ce3ca9e6af0b9559e0ae2