From: Phillip Wood <phillip.wood123@gmail.com>
Date: Thu, 26 Feb 2026 14:45:50 GMT
Subject: Re: [PATCH v3 2/2] replay: add --revert mode to reverse commit changes
Message-ID: <405b0d34-c2ad-498d-93a1-2e7925ae11f1@gmail.com>
In-Reply-To: <20260218234215.89326-3-siddharthasthana31@gmail.com>

Hi Siddharth

On 18/02/2026 23:42, Siddharth Asthana wrote:
> @@ -42,6 +42,25 @@ The history is replayed on top of the <branch> and <branch> is updated to
>   point at the tip of the resulting history. This is different from `--onto`,
>   which uses the target only as a starting point without updating it.
>   
> +--revert <branch>::
> +	Starting point at which to create the reverted commits; must be a
> +	branch name.
> ++
> +When `--revert` is specified, the commits in the revision range are reverted
> +(their changes are undone) and the reverted commits are created on top of
> +<branch>. The <branch> is then updated to point at the new commits. This is
> +the same as running `git revert <revision-range>` but does not update the
> +working tree.
> ++
> +The commit messages follow `git revert` conventions: they are prefixed with
> +"Revert" and include "This reverts commit <hash>." When reverting a commit
> +whose message starts with "Revert", the new message uses "Reapply" instead.
> +Unlike cherry-pick which preserves the original author, revert commits use
> +the current user as the author, matching the behavior of `git revert`.
> ++
> +This option is mutually exclusive with `--onto` and `--advance`. It is also
> +incompatible with `--contained` (which is a modifier for `--onto` only).

We seem to have lost

     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.

from V2 which is a shame.

I do think we should seriously consider reverting commits in the reverse 
order that they were created (i.e. do not set '--reverse' when setting 
up the rev-list options) to reduce the likely-hood of conflicts when 
reverting a sequence of commits.

> @@ -152,6 +172,15 @@ all commits they have since `base`, playing them on top of
>   `origin/main`. These three branches may have commits on top of `base`
>   that they have in common, but that does not need to be the case.
>   
> +To revert commits on a branch:
> +
> +------------
> +$ git replay --revert main main~2..main

It might be more realistic to revert some commits from a different 
branch, for example

   git replay --revert main topic~2..topic

> +static void set_up_branch_mode(struct repository *repo,
> +			       char **branch_name,
> +			       const char *option_name,
> +			       struct ref_info *rinfo,
> +			       struct commit **onto)
> [...]
> +	if (rinfo->positive_refexprs > 1)
> +		die(_("cannot %s target with multiple sources because ordering would be ill-defined"),
> +		    option_name + 2); /* skip "--" prefix */

This is a bit of a nasty hack as it stuffs an English word into the 
middle of a translated sentence. Using the option name as below might be 
nicer

     die(_("'%s' cannot be used with multiple revision ranges because 
the ordering would be ill defined", option_name);
> @@ -226,25 +269,46 @@ static struct commit *pick_regular_commit(struct repository *repo,
>  [...]
> -	/* Drop commits that become empty */
> -	if (oideq(&replayed_base_tree->object.oid, &result->tree->object.oid) &&
> +	/* Drop commits that become empty (only for picks) */

Why? What's the advantage in creating empty revert commits?

> +	if (mode == REPLAY_MODE_PICK &&
> +	    oideq(&replayed_base_tree->object.oid, &result->tree->object.oid) &&

Thanks

Phillip