[PATCH] sequencer: honor --empty when a fixup!/squash! empties its target

[PATCH] sequencer: honor --empty when a fixup!/squash! empties its target

[Farid Zakaria]

When "git rebase --autosquash" melds a "fixup!" or "squash!" commit into
its target, the result can be a commit that no longer changes anything
relative to its parent, for example when the melded change reverts the
target.  Rather than dropping or keeping this empty commit, the rebase
stops with

	You asked to amend the most recent commit, but doing so would
	make it empty. ...

and the "--empty" option has no effect on it.  This makes backing a
change out of a series awkward: reverting a commit as a "fixup!" and
running "git rebase --autosquash --empty=drop" ought to remove both the
commit and its revert, but it halts instead.

The reason is that allow_empty() decides emptiness with
is_index_unchanged(), which compares the index to HEAD.  A "fixup!" is
applied by amending HEAD, so the commit it produces has HEAD's parent as
its parent; it is empty when the index matches the tree of that parent,
not of HEAD.  A meld that cancels out its target is therefore never
recognized as having become empty, and falls through to "git commit
--amend", which refuses to create an empty commit.

Teach is_index_unchanged() to compare against the tree of HEAD's parent
when amending, and teach allow_empty() to classify the result as "became
empty" (and thus subject to --empty) unless the commit being melded into
was itself already empty, in which case it "started empty" and is
governed by allow_empty as before.

When --empty=drop applies, the emptied commit has already been created
by the preceding "pick", so drop it by moving HEAD back to its parent.
Do so before the rewritten-commit list is flushed, so that --update-refs
and the other rewrite consumers map the dropped commit to its parent.

Signed-off-by: Farid Zakaria <farid.m.zakaria@gmail.com>
---
At Meta we maintain a fork of LLVM that we regularly rebase onto
upstream.  A set of internal patches rides on top, and we keep each one
as a single commit by folding follow-up changes into it with autosquash
"fixup!" commits.  That works well for evolving a patch, but not for
retiring one: to back an internal patch out today we delete it from the
history by hand with an interactive rebase and then force-push, which is
easy to get wrong on a shared branch.

It would be nicer to retire a patch the same way we amend one: commit a
revert of it as a "fixup!" and let autosquash fold the two together.
The net change is empty, so the commit should just drop out of the
series.  Today it does not -- the rebase stops instead.

For example, starting from a commit we want to retire:

    $ git log --oneline
    4d5e6f7 add feature patch
    9a1b2c3 base

    # revert the feature and mark the revert as a fixup of it
    $ git revert --no-edit HEAD
    $ git commit --amend -m "fixup! add feature patch"

    $ git rebase -i --autosquash --empty=drop 9a1b2c3
    Rebasing (2/2)
    You asked to amend the most recent commit, but doing so would
    make it empty. You can repeat your command with --allow-empty [...]
    Could not apply 8e9f0a1... # fixup! add feature patch

The "--empty=drop" is ignored.  "--empty" only governs commits that are
picked empty, whereas a "fixup!" is applied by amending, and the
emptiness of an amended commit is measured against the wrong parent.  So
the rebase falls through to "git commit --amend", which refuses to
create an empty commit, and halts.

With this patch the emptied commit is recognized and handled according
to "--empty", the same as any other commit that becomes empty during a
rebase:

    $ git rebase -i --autosquash --empty=drop 9a1b2c3
    Rebasing (2/2)
    dropping 8e9f0a1... fixup! add feature patch -- resulting commit is empty
    Successfully rebased and updated refs/heads/main.

    $ git log --oneline
    9a1b2c3 base

"--empty=keep" retains it as an empty commit, and "--empty=stop" (the
default under "-i") halts so the user can decide -- matching how these
options already behave for commits that become empty when picked.

One open question, for a possible follow-up.  A natural next step would
be a "revert!" autosquash directive (and a "git commit --revert" to
create it), mirroring "fixup!"/"squash!", so
that retiring a patch would not require generating the reverse diff by
hand.  I have deliberately left it out of this series, because its
semantics are not obvious: in particular, whether a "revert!" commit
should carry the reverse patch as its own content (and thus be an
ordinary fixup that this patch already drops), or be an empty marker
that instructs the rebase to revert the target commit during the meld.
Opinions on whether such a directive is wanted, and which of those two
shapes is preferred, would be welcome before I attempt it.
---
base-commit: f60db8d575adb79761d363e026fb49bddf330c73
---
 Documentation/git-rebase.adoc | 12 ++++++
 sequencer.c                   | 96 +++++++++++++++++++++++++++++++++++++++----
 t/t3415-rebase-autosquash.sh  | 64 +++++++++++++++++++++++++++++
 3 files changed, 163 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-rebase.adoc b/Documentation/git-rebase.adoc
index f6c22d1598..7eb8bbe95f 100644
--- a/Documentation/git-rebase.adoc
+++ b/Documentation/git-rebase.adoc
@@ -282,6 +282,11 @@ by `git log --cherry-mark ...`) are detected and dropped as a
 preliminary step (unless `--reapply-cherry-picks` or `--keep-base` is
 passed).
 +
+A commit can also become empty as a result of `--autosquash`, when a
+`fixup!` or `squash!` commit cancels out all of the changes of the
+commit it is melded into.  Such a commit is treated the same way and is
+dropped, kept, or stopped at according to this option.
++
 See also INCOMPATIBLE OPTIONS below.
 
 --no-keep-empty::
@@ -591,6 +596,13 @@ changed from `pick` to `squash`, `fixup` or `fixup -C`, respectively, and they
 are moved right after the commit they modify.  The `--interactive` option can
 be used to review and edit the todo list before proceeding.
 +
+If melding a `fixup!` or `squash!` commit cancels out all of the changes of
+the commit it is applied to, the result is an empty commit.  The handling of
+these empty commits can be configured with the `--empty` option: the emptied
+commit is dropped, kept, or stopped at.  This makes it possible to back a
+change out of a series by committing a revert of it as a `fixup!` and letting
+`--autosquash --empty=drop` remove both.
++
 The recommended way to create commits with squash markers is by using the
 `--squash`, `--fixup`, `--fixup=amend:` or `--fixup=reword:` options of
 linkgit:git-commit[1], which take the target commit as an argument and
diff --git a/sequencer.c b/sequencer.c
index 0fe8fed6c3..435b100e3d 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -823,7 +823,7 @@ static struct object_id *get_cache_tree_oid(struct index_state *istate)
 	return &istate->cache_tree->oid;
 }
 
-static int is_index_unchanged(struct repository *r)
+static int is_index_unchanged(struct repository *r, int amend)
 {
 	struct object_id head_oid, *cache_tree_oid;
 	const struct object_id *head_tree_oid;
@@ -856,7 +856,26 @@ static int is_index_unchanged(struct repository *r)
 		if (repo_parse_commit(r, head_commit))
 			return -1;
 
-		head_tree_oid = get_commit_tree_oid(head_commit);
+		if (amend) {
+			/*
+			 * When amending (e.g. melding a "fixup!" or "squash!"),
+			 * the commit we are about to create replaces HEAD, so
+			 * its parent is HEAD's parent.  It is therefore empty
+			 * when the index matches the tree of HEAD's parent
+			 * rather than the tree of HEAD itself.
+			 */
+			if (head_commit->parents) {
+				struct commit *parent =
+					head_commit->parents->item;
+				if (repo_parse_commit(r, parent))
+					return -1;
+				head_tree_oid = get_commit_tree_oid(parent);
+			} else {
+				head_tree_oid = the_hash_algo->empty_tree;
+			}
+		} else {
+			head_tree_oid = get_commit_tree_oid(head_commit);
+		}
 	}
 
 	if (!(cache_tree_oid = get_cache_tree_oid(istate)))
@@ -1786,7 +1805,7 @@ static int is_original_commit_empty(struct commit *commit)
  */
 static int allow_empty(struct repository *r,
 		       struct replay_opts *opts,
-		       struct commit *commit)
+		       struct commit *commit, int amend)
 {
 	int index_unchanged, originally_empty;
 
@@ -1798,13 +1817,33 @@ static int allow_empty(struct repository *r,
 	 * drop_redundant_commits determine whether the commit should be kept or
 	 * dropped. If neither is specified, halt.
 	 */
-	index_unchanged = is_index_unchanged(r);
+	index_unchanged = is_index_unchanged(r, amend);
 	if (index_unchanged < 0)
 		return index_unchanged;
 	if (!index_unchanged)
 		return 0; /* we do not have to say --allow-empty */
 
-	originally_empty = is_original_commit_empty(commit);
+	/*
+	 * When amending (melding a "fixup!"/"squash!"), the resulting commit
+	 * replaces HEAD, so whether it "started" empty or "became" empty is
+	 * decided by whether the commit being melded into was itself empty: if
+	 * HEAD had content that the fixup cancelled out, the commit became empty
+	 * and is subject to keep/drop_redundant; if HEAD was already empty, the
+	 * commit started empty and is subject to allow_empty as usual.
+	 */
+	if (amend) {
+		struct object_id head_oid;
+		struct commit *head_commit;
+
+		if (repo_get_oid(r, "HEAD", &head_oid))
+			return error(_("could not resolve HEAD commit"));
+		head_commit = lookup_commit_reference(r, &head_oid);
+		if (!head_commit)
+			return -1;
+		originally_empty = is_original_commit_empty(head_commit);
+	} else {
+		originally_empty = is_original_commit_empty(commit);
+	}
 	if (originally_empty < 0)
 		return originally_empty;
 	if (originally_empty)
@@ -2260,6 +2299,30 @@ static const char *reflog_message(struct replay_opts *opts,
 	return buf.buf;
 }
 
+/*
+ * A "fixup!"/"squash!" that melds into HEAD may empty it out.  In that case,
+ * with --empty=drop, we want to drop the commit entirely.  Since the commit
+ * being amended has already been created (by the preceding "pick"), and the
+ * index and worktree already match the tree of its parent, dropping it is a
+ * matter of moving HEAD back to that parent.
+ */
+static int reset_head_to_parent(struct repository *r, struct replay_opts *opts,
+				struct object_id *head)
+{
+	struct commit *head_commit = lookup_commit_reference(r, head);
+
+	if (!head_commit || repo_parse_commit(r, head_commit))
+		return error(_("could not parse HEAD commit"));
+	if (!head_commit->parents)
+		return error(_("cannot drop the root commit"));
+
+	return refs_update_ref(get_main_ref_store(r),
+			       reflog_message(opts, "fixup",
+					      "dropping emptied commit"),
+			       "HEAD", &head_commit->parents->item->object.oid,
+			       head, 0, UPDATE_REFS_MSG_ON_ERR);
+}
+
 static int do_pick_commit(struct repository *r,
 			  struct todo_item *item,
 			  struct replay_opts *opts,
@@ -2493,7 +2556,7 @@ static int do_pick_commit(struct repository *r,
 	}
 
 	drop_commit = 0;
-	allow = allow_empty(r, opts, commit);
+	allow = allow_empty(r, opts, commit, flags & AMEND_MSG);
 	if (allow < 0) {
 		res = allow;
 		goto leave;
@@ -2506,9 +2569,24 @@ static int do_pick_commit(struct repository *r,
 		unlink(git_path_merge_msg(r));
 		refs_delete_ref(get_main_ref_store(r), "", "AUTO_MERGE",
 				NULL, REF_NO_DEREF);
-		fprintf(stderr,
-			_("dropping %s %s -- patch contents already upstream\n"),
-			oid_to_hex(&commit->object.oid), msg.subject);
+		if (flags & AMEND_MSG) {
+			/*
+			 * The "fixup!"/"squash!" emptied out the commit it was
+			 * melded into; that commit was already created by the
+			 * preceding "pick", so drop it by moving HEAD back to
+			 * its parent.
+			 */
+			res = reset_head_to_parent(r, opts, &head);
+			if (res)
+				goto leave;
+			fprintf(stderr,
+				_("dropping %s %s -- resulting commit is empty\n"),
+				oid_to_hex(&commit->object.oid), msg.subject);
+		} else {
+			fprintf(stderr,
+				_("dropping %s %s -- patch contents already upstream\n"),
+				oid_to_hex(&commit->object.oid), msg.subject);
+		}
 	} /* else allow == 0 and there's nothing special to do */
 	if (!opts->no_commit && !drop_commit) {
 		if (author || command == TODO_REVERT || (flags & AMEND_MSG))
diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
index 5033411a43..508dcc7527 100755
--- a/t/t3415-rebase-autosquash.sh
+++ b/t/t3415-rebase-autosquash.sh
@@ -510,4 +510,68 @@ test_expect_success 'pick and fixup respect commit.cleanup' '
 	test_commit_message HEAD -m "something"
 '
 
+test_expect_success 'fixup! that empties its target is dropped with --empty=drop' '
+	git reset --hard base &&
+	test_commit --no-tag addX fileX 1 &&
+	test_commit --no-tag changeX fileX 2 &&
+	test_commit --no-tag later fileW hello &&
+	echo 1 >fileX &&
+	git commit -m "fixup! changeX" fileX &&
+
+	git rebase -i --autosquash --empty=drop HEAD~4 &&
+
+	git log --format=%s >actual &&
+	! grep changeX actual &&
+	grep addX actual &&
+	grep later actual &&
+	echo 1 >expect &&
+	test_cmp expect fileX &&
+	echo hello >expect &&
+	test_cmp expect fileW
+'
+
+test_expect_success 'fixup! that empties its target is kept with --empty=keep' '
+	git reset --hard base &&
+	test_commit --no-tag addY fileY 1 &&
+	test_commit --no-tag changeY fileY 2 &&
+	echo 1 >fileY &&
+	git commit -m "fixup! changeY" fileY &&
+
+	git rebase -i --autosquash --empty=keep HEAD~3 &&
+
+	git log --format=%s >actual &&
+	grep changeY actual &&
+	: "the retained commit is empty" &&
+	git diff --exit-code HEAD~1 HEAD &&
+	echo 1 >expect &&
+	test_cmp expect fileY
+'
+
+test_expect_success 'fixup! that empties its target stops with --empty=stop' '
+	git reset --hard base &&
+	test_commit --no-tag addZ fileZ 1 &&
+	test_commit --no-tag changeZ fileZ 2 &&
+	echo 1 >fileZ &&
+	git commit -m "fixup! changeZ" fileZ &&
+
+	test_when_finished "git rebase --abort" &&
+	test_must_fail git rebase -i --autosquash --empty=stop HEAD~3
+'
+
+test_expect_success 'squash! that empties its target is dropped with --empty=drop' '
+	git reset --hard base &&
+	test_commit --no-tag addS fileS 1 &&
+	test_commit --no-tag changeS fileS 2 &&
+	echo 1 >fileS &&
+	git commit -m "squash! changeS" fileS &&
+
+	git rebase -i --autosquash --empty=drop HEAD~3 &&
+
+	git log --format=%s >actual &&
+	! grep changeS actual &&
+	grep addS actual &&
+	echo 1 >expect &&
+	test_cmp expect fileS
+'
+
 test_done

Re: [PATCH] sequencer: honor --empty when a fixup!/squash! empties its target

[Phillip Wood]

Hi Farid

On 10/07/2026 05:13, Farid Zakaria wrote:
> When "git rebase --autosquash" melds a "fixup!" or "squash!" commit into
> its target, the result can be a commit that no longer changes anything
> relative to its parent, for example when the melded change reverts the
> target.  Rather than dropping or keeping this empty commit, the rebase
> stops with
> 
> 	You asked to amend the most recent commit, but doing so would
> 	make it empty. ...
> 
> and the "--empty" option has no effect on it.  This makes backing a
> change out of a series awkward: reverting a commit as a "fixup!" and
> running "git rebase --autosquash --empty=drop" ought to remove both the
> commit and its revert, but it halts instead.

I agree this is a use case that we want to support

> The reason is that allow_empty() decides emptiness with
> is_index_unchanged(), which compares the index to HEAD.  A "fixup!" is
> applied by amending HEAD, so the commit it produces has HEAD's parent as
> its parent; it is empty when the index matches the tree of that parent,
> not of HEAD.  A meld that cancels out its target is therefore never
> recognized as having become empty, and falls through to "git commit
> --amend", which refuses to create an empty commit.

and with this diagnosis.

> Teach is_index_unchanged() to compare against the tree of HEAD's parent
> when amending, and teach allow_empty() to classify the result as "became
> empty" (and thus subject to --empty) unless the commit being melded into
> was itself already empty, in which case it "started empty" and is
> governed by allow_empty as before.

However, I think that rather than changing the current check which 
changes the behavior of a fixup commit that becomes empty we should add 
an additional check to see if applying the fixup makes the target commit 
empty. With the patch here a fixup commit that becomes empty is only 
seen as empty if the commit being fixed up is empty in which case we 
always accept the fixup, whereas the current behavior is always to 
respect what --empty says. When I'm planning out a series of commits I 
sometimes create empty commits where the messages says what I'm 
intending to do and then I create fixups for them when I get round to 
writing the code. If one of those fixups becomes empty I want to know 
about it because it means I need to drop the empty commit that's being 
fixed up as well.

> When --empty=drop applies, the emptied commit has already been created
> by the preceding "pick", so drop it by moving HEAD back to its parent.
> Do so before the rewritten-commit list is flushed, so that --update-refs
> and the other rewrite consumers map the dropped commit to its parent.

If we're dropping the commit then we should not record it as rewritten 
so we need to remove the rewritten-pending file. Any labels and 
update-ref commands that come immediately after the dropped commit will 
see HEAD pointing to the dropped commits rewritten parent.

> Signed-off-by: Farid Zakaria <farid.m.zakaria@gmail.com>
> ---
> At Meta we maintain a fork of LLVM that we regularly rebase onto
> upstream.  A set of internal patches rides on top, and we keep each one
> as a single commit by folding follow-up changes into it with autosquash
> "fixup!" commits.  That works well for evolving a patch, but not for
> retiring one: to back an internal patch out today we delete it from the
> history by hand with an interactive rebase and then force-push, which is
> easy to get wrong on a shared branch.

You'll still need a forced push though because you're dropping the 
commit. I think the change you're proposing to git would be useful but 
you could automate your existing workflow by setting GIT_SEQUENCE_EDITOR 
to a script that drops the commit and it's fixups from the todo list.

> One open question, for a possible follow-up.  A natural next step would
> be a "revert!" autosquash directive (and a "git commit --revert" to
> create it), mirroring "fixup!"/"squash!", so
> that retiring a patch would not require generating the reverse diff by
> hand.  I have deliberately left it out of this series, because its
> semantics are not obvious: in particular, whether a "revert!" commit
> should carry the reverse patch as its own content (and thus be an
> ordinary fixup that this patch already drops), or be an empty marker
> that instructs the rebase to revert the target commit during the meld.
> Opinions on whether such a directive is wanted, and which of those two
> shapes is preferred, would be welcome before I attempt it.

I think having support for creating and squashing revert! (or possibly 
drop!) commits is a good idea (I've a feeling there is some discussion 
about that in the gitgitgadget issue tracker). Using an empty commit has 
a marker has the advantage that applying it cannot create conflicts, so 
you only have to deal with the conflicts caused by the commit being 
dropped, not the by fixup not applying cleanly.

Thanks

Phillip

> ---
> base-commit: f60db8d575adb79761d363e026fb49bddf330c73
> ---
>   Documentation/git-rebase.adoc | 12 ++++++
>   sequencer.c                   | 96 +++++++++++++++++++++++++++++++++++++++----
>   t/t3415-rebase-autosquash.sh  | 64 +++++++++++++++++++++++++++++
>   3 files changed, 163 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/git-rebase.adoc b/Documentation/git-rebase.adoc
> index f6c22d1598..7eb8bbe95f 100644
> --- a/Documentation/git-rebase.adoc
> +++ b/Documentation/git-rebase.adoc
> @@ -282,6 +282,11 @@ by `git log --cherry-mark ...`) are detected and dropped as a
>   preliminary step (unless `--reapply-cherry-picks` or `--keep-base` is
>   passed).
>   +
> +A commit can also become empty as a result of `--autosquash`, when a
> +`fixup!` or `squash!` commit cancels out all of the changes of the
> +commit it is melded into.  Such a commit is treated the same way and is
> +dropped, kept, or stopped at according to this option.
> ++
>   See also INCOMPATIBLE OPTIONS below.
>   
>   --no-keep-empty::
> @@ -591,6 +596,13 @@ changed from `pick` to `squash`, `fixup` or `fixup -C`, respectively, and they
>   are moved right after the commit they modify.  The `--interactive` option can
>   be used to review and edit the todo list before proceeding.
>   +
> +If melding a `fixup!` or `squash!` commit cancels out all of the changes of
> +the commit it is applied to, the result is an empty commit.  The handling of
> +these empty commits can be configured with the `--empty` option: the emptied
> +commit is dropped, kept, or stopped at.  This makes it possible to back a
> +change out of a series by committing a revert of it as a `fixup!` and letting
> +`--autosquash --empty=drop` remove both.
> ++
>   The recommended way to create commits with squash markers is by using the
>   `--squash`, `--fixup`, `--fixup=amend:` or `--fixup=reword:` options of
>   linkgit:git-commit[1], which take the target commit as an argument and
> diff --git a/sequencer.c b/sequencer.c
> index 0fe8fed6c3..435b100e3d 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -823,7 +823,7 @@ static struct object_id *get_cache_tree_oid(struct index_state *istate)
>   	return &istate->cache_tree->oid;
>   }
>   
> -static int is_index_unchanged(struct repository *r)
> +static int is_index_unchanged(struct repository *r, int amend)
>   {
>   	struct object_id head_oid, *cache_tree_oid;
>   	const struct object_id *head_tree_oid;
> @@ -856,7 +856,26 @@ static int is_index_unchanged(struct repository *r)
>   		if (repo_parse_commit(r, head_commit))
>   			return -1;
>   
> -		head_tree_oid = get_commit_tree_oid(head_commit);
> +		if (amend) {
> +			/*
> +			 * When amending (e.g. melding a "fixup!" or "squash!"),
> +			 * the commit we are about to create replaces HEAD, so
> +			 * its parent is HEAD's parent.  It is therefore empty
> +			 * when the index matches the tree of HEAD's parent
> +			 * rather than the tree of HEAD itself.
> +			 */
> +			if (head_commit->parents) {
> +				struct commit *parent =
> +					head_commit->parents->item;
> +				if (repo_parse_commit(r, parent))
> +					return -1;
> +				head_tree_oid = get_commit_tree_oid(parent);
> +			} else {
> +				head_tree_oid = the_hash_algo->empty_tree;
> +			}
> +		} else {
> +			head_tree_oid = get_commit_tree_oid(head_commit);
> +		}
>   	}
>   
>   	if (!(cache_tree_oid = get_cache_tree_oid(istate)))
> @@ -1786,7 +1805,7 @@ static int is_original_commit_empty(struct commit *commit)
>    */
>   static int allow_empty(struct repository *r,
>   		       struct replay_opts *opts,
> -		       struct commit *commit)
> +		       struct commit *commit, int amend)
>   {
>   	int index_unchanged, originally_empty;
>   
> @@ -1798,13 +1817,33 @@ static int allow_empty(struct repository *r,
>   	 * drop_redundant_commits determine whether the commit should be kept or
>   	 * dropped. If neither is specified, halt.
>   	 */
> -	index_unchanged = is_index_unchanged(r);
> +	index_unchanged = is_index_unchanged(r, amend);
>   	if (index_unchanged < 0)
>   		return index_unchanged;
>   	if (!index_unchanged)
>   		return 0; /* we do not have to say --allow-empty */
>   
> -	originally_empty = is_original_commit_empty(commit);
> +	/*
> +	 * When amending (melding a "fixup!"/"squash!"), the resulting commit
> +	 * replaces HEAD, so whether it "started" empty or "became" empty is
> +	 * decided by whether the commit being melded into was itself empty: if
> +	 * HEAD had content that the fixup cancelled out, the commit became empty
> +	 * and is subject to keep/drop_redundant; if HEAD was already empty, the
> +	 * commit started empty and is subject to allow_empty as usual.
> +	 */
> +	if (amend) {
> +		struct object_id head_oid;
> +		struct commit *head_commit;
> +
> +		if (repo_get_oid(r, "HEAD", &head_oid))
> +			return error(_("could not resolve HEAD commit"));
> +		head_commit = lookup_commit_reference(r, &head_oid);
> +		if (!head_commit)
> +			return -1;
> +		originally_empty = is_original_commit_empty(head_commit);
> +	} else {
> +		originally_empty = is_original_commit_empty(commit);
> +	}
>   	if (originally_empty < 0)
>   		return originally_empty;
>   	if (originally_empty)
> @@ -2260,6 +2299,30 @@ static const char *reflog_message(struct replay_opts *opts,
>   	return buf.buf;
>   }
>   
> +/*
> + * A "fixup!"/"squash!" that melds into HEAD may empty it out.  In that case,
> + * with --empty=drop, we want to drop the commit entirely.  Since the commit
> + * being amended has already been created (by the preceding "pick"), and the
> + * index and worktree already match the tree of its parent, dropping it is a
> + * matter of moving HEAD back to that parent.
> + */
> +static int reset_head_to_parent(struct repository *r, struct replay_opts *opts,
> +				struct object_id *head)
> +{
> +	struct commit *head_commit = lookup_commit_reference(r, head);
> +
> +	if (!head_commit || repo_parse_commit(r, head_commit))
> +		return error(_("could not parse HEAD commit"));
> +	if (!head_commit->parents)
> +		return error(_("cannot drop the root commit"));
> +
> +	return refs_update_ref(get_main_ref_store(r),
> +			       reflog_message(opts, "fixup",
> +					      "dropping emptied commit"),
> +			       "HEAD", &head_commit->parents->item->object.oid,
> +			       head, 0, UPDATE_REFS_MSG_ON_ERR);
> +}
> +
>   static int do_pick_commit(struct repository *r,
>   			  struct todo_item *item,
>   			  struct replay_opts *opts,
> @@ -2493,7 +2556,7 @@ static int do_pick_commit(struct repository *r,
>   	}
>   
>   	drop_commit = 0;
> -	allow = allow_empty(r, opts, commit);
> +	allow = allow_empty(r, opts, commit, flags & AMEND_MSG);
>   	if (allow < 0) {
>   		res = allow;
>   		goto leave;
> @@ -2506,9 +2569,24 @@ static int do_pick_commit(struct repository *r,
>   		unlink(git_path_merge_msg(r));
>   		refs_delete_ref(get_main_ref_store(r), "", "AUTO_MERGE",
>   				NULL, REF_NO_DEREF);
> -		fprintf(stderr,
> -			_("dropping %s %s -- patch contents already upstream\n"),
> -			oid_to_hex(&commit->object.oid), msg.subject);
> +		if (flags & AMEND_MSG) {
> +			/*
> +			 * The "fixup!"/"squash!" emptied out the commit it was
> +			 * melded into; that commit was already created by the
> +			 * preceding "pick", so drop it by moving HEAD back to
> +			 * its parent.
> +			 */
> +			res = reset_head_to_parent(r, opts, &head);
> +			if (res)
> +				goto leave;
> +			fprintf(stderr,
> +				_("dropping %s %s -- resulting commit is empty\n"),
> +				oid_to_hex(&commit->object.oid), msg.subject);
> +		} else {
> +			fprintf(stderr,
> +				_("dropping %s %s -- patch contents already upstream\n"),
> +				oid_to_hex(&commit->object.oid), msg.subject);
> +		}
>   	} /* else allow == 0 and there's nothing special to do */
>   	if (!opts->no_commit && !drop_commit) {
>   		if (author || command == TODO_REVERT || (flags & AMEND_MSG))
> diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
> index 5033411a43..508dcc7527 100755
> --- a/t/t3415-rebase-autosquash.sh
> +++ b/t/t3415-rebase-autosquash.sh
> @@ -510,4 +510,68 @@ test_expect_success 'pick and fixup respect commit.cleanup' '
>   	test_commit_message HEAD -m "something"
>   '
>   
> +test_expect_success 'fixup! that empties its target is dropped with --empty=drop' '
> +	git reset --hard base &&
> +	test_commit --no-tag addX fileX 1 &&
> +	test_commit --no-tag changeX fileX 2 &&
> +	test_commit --no-tag later fileW hello &&
> +	echo 1 >fileX &&
> +	git commit -m "fixup! changeX" fileX &&
> +
> +	git rebase -i --autosquash --empty=drop HEAD~4 &&
> +
> +	git log --format=%s >actual &&
> +	! grep changeX actual &&
> +	grep addX actual &&
> +	grep later actual &&
> +	echo 1 >expect &&
> +	test_cmp expect fileX &&
> +	echo hello >expect &&
> +	test_cmp expect fileW
> +'
> +
> +test_expect_success 'fixup! that empties its target is kept with --empty=keep' '
> +	git reset --hard base &&
> +	test_commit --no-tag addY fileY 1 &&
> +	test_commit --no-tag changeY fileY 2 &&
> +	echo 1 >fileY &&
> +	git commit -m "fixup! changeY" fileY &&
> +
> +	git rebase -i --autosquash --empty=keep HEAD~3 &&
> +
> +	git log --format=%s >actual &&
> +	grep changeY actual &&
> +	: "the retained commit is empty" &&
> +	git diff --exit-code HEAD~1 HEAD &&
> +	echo 1 >expect &&
> +	test_cmp expect fileY
> +'
> +
> +test_expect_success 'fixup! that empties its target stops with --empty=stop' '
> +	git reset --hard base &&
> +	test_commit --no-tag addZ fileZ 1 &&
> +	test_commit --no-tag changeZ fileZ 2 &&
> +	echo 1 >fileZ &&
> +	git commit -m "fixup! changeZ" fileZ &&
> +
> +	test_when_finished "git rebase --abort" &&
> +	test_must_fail git rebase -i --autosquash --empty=stop HEAD~3
> +'
> +
> +test_expect_success 'squash! that empties its target is dropped with --empty=drop' '
> +	git reset --hard base &&
> +	test_commit --no-tag addS fileS 1 &&
> +	test_commit --no-tag changeS fileS 2 &&
> +	echo 1 >fileS &&
> +	git commit -m "squash! changeS" fileS &&
> +
> +	git rebase -i --autosquash --empty=drop HEAD~3 &&
> +
> +	git log --format=%s >actual &&
> +	! grep changeS actual &&
> +	grep addS actual &&
> +	echo 1 >expect &&
> +	test_cmp expect fileS
> +'
> +
>   test_done
> 
> 
> 
> 

Re: [PATCH] sequencer: honor --empty when a fixup!/squash! empties its target

[Farid Zakaria]

On Fri Jul 10, 2026 at 6:28 AM PDT, Phillip Wood wrote:

Phillip,

Thank you for responding. This is my first submission to the Git mailing
list.

I want to be forthecoming that I'm not familiar with the Git codebase,
and (maybe unsurprisingly) I have been leveraging LLMs to help me
understand the code and write the patch. I consulted the Git
contribution guidelines and it says that should be OK as long as it's
not "slop". I'm diligent to remaining in the loop (HITL) and reviewing
the code and tests to the best of my understanding of the codebase.

I will send the V2 shortly. Thank you!

> Hi Farid
>
> On 10/07/2026 05:13, Farid Zakaria wrote:
>> When "git rebase --autosquash" melds a "fixup!" or "squash!" commit into
>> its target, the result can be a commit that no longer changes anything
>> relative to its parent, for example when the melded change reverts the
>> target.  Rather than dropping or keeping this empty commit, the rebase
>> stops with
>> 
>> 	You asked to amend the most recent commit, but doing so would
>> 	make it empty. ...
>> 
>> and the "--empty" option has no effect on it.  This makes backing a
>> change out of a series awkward: reverting a commit as a "fixup!" and
>> running "git rebase --autosquash --empty=drop" ought to remove both the
>> commit and its revert, but it halts instead.
>
> I agree this is a use case that we want to support
>
>> The reason is that allow_empty() decides emptiness with
>> is_index_unchanged(), which compares the index to HEAD.  A "fixup!" is
>> applied by amending HEAD, so the commit it produces has HEAD's parent as
>> its parent; it is empty when the index matches the tree of that parent,
>> not of HEAD.  A meld that cancels out its target is therefore never
>> recognized as having become empty, and falls through to "git commit
>> --amend", which refuses to create an empty commit.
>
> and with this diagnosis.
>
>> Teach is_index_unchanged() to compare against the tree of HEAD's parent
>> when amending, and teach allow_empty() to classify the result as "became
>> empty" (and thus subject to --empty) unless the commit being melded into
>> was itself already empty, in which case it "started empty" and is
>> governed by allow_empty as before.
>
> However, I think that rather than changing the current check which 
> changes the behavior of a fixup commit that becomes empty we should add 
> an additional check to see if applying the fixup makes the target commit 
> empty. With the patch here a fixup commit that becomes empty is only 
> seen as empty if the commit being fixed up is empty in which case we 
> always accept the fixup, whereas the current behavior is always to 
> respect what --empty says. When I'm planning out a series of commits I 
> sometimes create empty commits where the messages says what I'm 
> intending to do and then I create fixups for them when I get round to 
> writing the code. If one of those fixups becomes empty I want to know 
> about it because it means I need to drop the empty commit that's being 
> fixed up as well.
>

Thank you for this catch. I will apply the changes you suggest in V2 and
add test cases for this missing behavior.

>> When --empty=drop applies, the emptied commit has already been created
>> by the preceding "pick", so drop it by moving HEAD back to its parent.
>> Do so before the rewritten-commit list is flushed, so that --update-refs
>> and the other rewrite consumers map the dropped commit to its parent.
>
> If we're dropping the commit then we should not record it as rewritten 
> so we need to remove the rewritten-pending file. Any labels and 
> update-ref commands that come immediately after the dropped commit will 
> see HEAD pointing to the dropped commits rewritten parent.
>

I will address this in V2 as well.

>> Signed-off-by: Farid Zakaria <farid.m.zakaria@gmail.com>
>> ---
>> At Meta we maintain a fork of LLVM that we regularly rebase onto
>> upstream.  A set of internal patches rides on top, and we keep each one
>> as a single commit by folding follow-up changes into it with autosquash
>> "fixup!" commits.  That works well for evolving a patch, but not for
>> retiring one: to back an internal patch out today we delete it from the
>> history by hand with an interactive rebase and then force-push, which is
>> easy to get wrong on a shared branch.
>
> You'll still need a forced push though because you're dropping the 
> commit. I think the change you're proposing to git would be useful but 
> you could automate your existing workflow by setting GIT_SEQUENCE_EDITOR 
> to a script that drops the commit and it's fixups from the todo list.
>

True. I guess I should have clarified we run with a script that already
uses GIT_SEQUENCE_EDITOR and force-pushes. I just wanted to avoid cases
where a developer has to intervene in the rebase and force-push.
Developers must still intervene and force-push when a conflict arises in
our workflow though....

>> One open question, for a possible follow-up.  A natural next step would
>> be a "revert!" autosquash directive (and a "git commit --revert" to
>> create it), mirroring "fixup!"/"squash!", so
>> that retiring a patch would not require generating the reverse diff by
>> hand.  I have deliberately left it out of this series, because its
>> semantics are not obvious: in particular, whether a "revert!" commit
>> should carry the reverse patch as its own content (and thus be an
>> ordinary fixup that this patch already drops), or be an empty marker
>> that instructs the rebase to revert the target commit during the meld.
>> Opinions on whether such a directive is wanted, and which of those two
>> shapes is preferred, would be welcome before I attempt it.
>
> I think having support for creating and squashing revert! (or possibly 
> drop!) commits is a good idea (I've a feeling there is some discussion 
> about that in the gitgitgadget issue tracker). Using an empty commit has 
> a marker has the advantage that applying it cannot create conflicts, so 
> you only have to deal with the conflicts caused by the commit being 
> dropped, not the by fixup not applying cleanly.
>

This seems like a nice ergonomic improvement but I chose to leave it out
since it seems particularly thorny to get right.
I know for us at Meta, we use Phabricator (similar to Gerrit) and
working with empty commits is I think problematic for the code review
tooling (unsubstantiated).

If this current patch series makes it's way through though I can tackle
an approach as an RFC.

> Thanks
>
> Phillip
>
>> ---
>> base-commit: f60db8d575adb79761d363e026fb49bddf330c73
>> ---
>>   Documentation/git-rebase.adoc | 12 ++++++
>>   sequencer.c                   | 96 +++++++++++++++++++++++++++++++++++++++----
>>   t/t3415-rebase-autosquash.sh  | 64 +++++++++++++++++++++++++++++
>>   3 files changed, 163 insertions(+), 9 deletions(-)
>> 
>> diff --git a/Documentation/git-rebase.adoc b/Documentation/git-rebase.adoc
>> index f6c22d1598..7eb8bbe95f 100644
>> --- a/Documentation/git-rebase.adoc
>> +++ b/Documentation/git-rebase.adoc
>> @@ -282,6 +282,11 @@ by `git log --cherry-mark ...`) are detected and dropped as a
>>   preliminary step (unless `--reapply-cherry-picks` or `--keep-base` is
>>   passed).
>>   +
>> +A commit can also become empty as a result of `--autosquash`, when a
>> +`fixup!` or `squash!` commit cancels out all of the changes of the
>> +commit it is melded into.  Such a commit is treated the same way and is
>> +dropped, kept, or stopped at according to this option.
>> ++
>>   See also INCOMPATIBLE OPTIONS below.
>>   
>>   --no-keep-empty::
>> @@ -591,6 +596,13 @@ changed from `pick` to `squash`, `fixup` or `fixup -C`, respectively, and they
>>   are moved right after the commit they modify.  The `--interactive` option can
>>   be used to review and edit the todo list before proceeding.
>>   +
>> +If melding a `fixup!` or `squash!` commit cancels out all of the changes of
>> +the commit it is applied to, the result is an empty commit.  The handling of
>> +these empty commits can be configured with the `--empty` option: the emptied
>> +commit is dropped, kept, or stopped at.  This makes it possible to back a
>> +change out of a series by committing a revert of it as a `fixup!` and letting
>> +`--autosquash --empty=drop` remove both.
>> ++
>>   The recommended way to create commits with squash markers is by using the
>>   `--squash`, `--fixup`, `--fixup=amend:` or `--fixup=reword:` options of
>>   linkgit:git-commit[1], which take the target commit as an argument and
>> diff --git a/sequencer.c b/sequencer.c
>> index 0fe8fed6c3..435b100e3d 100644
>> --- a/sequencer.c
>> +++ b/sequencer.c
>> @@ -823,7 +823,7 @@ static struct object_id *get_cache_tree_oid(struct index_state *istate)
>>   	return &istate->cache_tree->oid;
>>   }
>>   
>> -static int is_index_unchanged(struct repository *r)
>> +static int is_index_unchanged(struct repository *r, int amend)
>>   {
>>   	struct object_id head_oid, *cache_tree_oid;
>>   	const struct object_id *head_tree_oid;
>> @@ -856,7 +856,26 @@ static int is_index_unchanged(struct repository *r)
>>   		if (repo_parse_commit(r, head_commit))
>>   			return -1;
>>   
>> -		head_tree_oid = get_commit_tree_oid(head_commit);
>> +		if (amend) {
>> +			/*
>> +			 * When amending (e.g. melding a "fixup!" or "squash!"),
>> +			 * the commit we are about to create replaces HEAD, so
>> +			 * its parent is HEAD's parent.  It is therefore empty
>> +			 * when the index matches the tree of HEAD's parent
>> +			 * rather than the tree of HEAD itself.
>> +			 */
>> +			if (head_commit->parents) {
>> +				struct commit *parent =
>> +					head_commit->parents->item;
>> +				if (repo_parse_commit(r, parent))
>> +					return -1;
>> +				head_tree_oid = get_commit_tree_oid(parent);
>> +			} else {
>> +				head_tree_oid = the_hash_algo->empty_tree;
>> +			}
>> +		} else {
>> +			head_tree_oid = get_commit_tree_oid(head_commit);
>> +		}
>>   	}
>>   
>>   	if (!(cache_tree_oid = get_cache_tree_oid(istate)))
>> @@ -1786,7 +1805,7 @@ static int is_original_commit_empty(struct commit *commit)
>>    */
>>   static int allow_empty(struct repository *r,
>>   		       struct replay_opts *opts,
>> -		       struct commit *commit)
>> +		       struct commit *commit, int amend)
>>   {
>>   	int index_unchanged, originally_empty;
>>   
>> @@ -1798,13 +1817,33 @@ static int allow_empty(struct repository *r,
>>   	 * drop_redundant_commits determine whether the commit should be kept or
>>   	 * dropped. If neither is specified, halt.
>>   	 */
>> -	index_unchanged = is_index_unchanged(r);
>> +	index_unchanged = is_index_unchanged(r, amend);
>>   	if (index_unchanged < 0)
>>   		return index_unchanged;
>>   	if (!index_unchanged)
>>   		return 0; /* we do not have to say --allow-empty */
>>   
>> -	originally_empty = is_original_commit_empty(commit);
>> +	/*
>> +	 * When amending (melding a "fixup!"/"squash!"), the resulting commit
>> +	 * replaces HEAD, so whether it "started" empty or "became" empty is
>> +	 * decided by whether the commit being melded into was itself empty: if
>> +	 * HEAD had content that the fixup cancelled out, the commit became empty
>> +	 * and is subject to keep/drop_redundant; if HEAD was already empty, the
>> +	 * commit started empty and is subject to allow_empty as usual.
>> +	 */
>> +	if (amend) {
>> +		struct object_id head_oid;
>> +		struct commit *head_commit;
>> +
>> +		if (repo_get_oid(r, "HEAD", &head_oid))
>> +			return error(_("could not resolve HEAD commit"));
>> +		head_commit = lookup_commit_reference(r, &head_oid);
>> +		if (!head_commit)
>> +			return -1;
>> +		originally_empty = is_original_commit_empty(head_commit);
>> +	} else {
>> +		originally_empty = is_original_commit_empty(commit);
>> +	}
>>   	if (originally_empty < 0)
>>   		return originally_empty;
>>   	if (originally_empty)
>> @@ -2260,6 +2299,30 @@ static const char *reflog_message(struct replay_opts *opts,
>>   	return buf.buf;
>>   }
>>   
>> +/*
>> + * A "fixup!"/"squash!" that melds into HEAD may empty it out.  In that case,
>> + * with --empty=drop, we want to drop the commit entirely.  Since the commit
>> + * being amended has already been created (by the preceding "pick"), and the
>> + * index and worktree already match the tree of its parent, dropping it is a
>> + * matter of moving HEAD back to that parent.
>> + */
>> +static int reset_head_to_parent(struct repository *r, struct replay_opts *opts,
>> +				struct object_id *head)
>> +{
>> +	struct commit *head_commit = lookup_commit_reference(r, head);
>> +
>> +	if (!head_commit || repo_parse_commit(r, head_commit))
>> +		return error(_("could not parse HEAD commit"));
>> +	if (!head_commit->parents)
>> +		return error(_("cannot drop the root commit"));
>> +
>> +	return refs_update_ref(get_main_ref_store(r),
>> +			       reflog_message(opts, "fixup",
>> +					      "dropping emptied commit"),
>> +			       "HEAD", &head_commit->parents->item->object.oid,
>> +			       head, 0, UPDATE_REFS_MSG_ON_ERR);
>> +}
>> +
>>   static int do_pick_commit(struct repository *r,
>>   			  struct todo_item *item,
>>   			  struct replay_opts *opts,
>> @@ -2493,7 +2556,7 @@ static int do_pick_commit(struct repository *r,
>>   	}
>>   
>>   	drop_commit = 0;
>> -	allow = allow_empty(r, opts, commit);
>> +	allow = allow_empty(r, opts, commit, flags & AMEND_MSG);
>>   	if (allow < 0) {
>>   		res = allow;
>>   		goto leave;
>> @@ -2506,9 +2569,24 @@ static int do_pick_commit(struct repository *r,
>>   		unlink(git_path_merge_msg(r));
>>   		refs_delete_ref(get_main_ref_store(r), "", "AUTO_MERGE",
>>   				NULL, REF_NO_DEREF);
>> -		fprintf(stderr,
>> -			_("dropping %s %s -- patch contents already upstream\n"),
>> -			oid_to_hex(&commit->object.oid), msg.subject);
>> +		if (flags & AMEND_MSG) {
>> +			/*
>> +			 * The "fixup!"/"squash!" emptied out the commit it was
>> +			 * melded into; that commit was already created by the
>> +			 * preceding "pick", so drop it by moving HEAD back to
>> +			 * its parent.
>> +			 */
>> +			res = reset_head_to_parent(r, opts, &head);
>> +			if (res)
>> +				goto leave;
>> +			fprintf(stderr,
>> +				_("dropping %s %s -- resulting commit is empty\n"),
>> +				oid_to_hex(&commit->object.oid), msg.subject);
>> +		} else {
>> +			fprintf(stderr,
>> +				_("dropping %s %s -- patch contents already upstream\n"),
>> +				oid_to_hex(&commit->object.oid), msg.subject);
>> +		}
>>   	} /* else allow == 0 and there's nothing special to do */
>>   	if (!opts->no_commit && !drop_commit) {
>>   		if (author || command == TODO_REVERT || (flags & AMEND_MSG))
>> diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
>> index 5033411a43..508dcc7527 100755
>> --- a/t/t3415-rebase-autosquash.sh
>> +++ b/t/t3415-rebase-autosquash.sh
>> @@ -510,4 +510,68 @@ test_expect_success 'pick and fixup respect commit.cleanup' '
>>   	test_commit_message HEAD -m "something"
>>   '
>>   
>> +test_expect_success 'fixup! that empties its target is dropped with --empty=drop' '
>> +	git reset --hard base &&
>> +	test_commit --no-tag addX fileX 1 &&
>> +	test_commit --no-tag changeX fileX 2 &&
>> +	test_commit --no-tag later fileW hello &&
>> +	echo 1 >fileX &&
>> +	git commit -m "fixup! changeX" fileX &&
>> +
>> +	git rebase -i --autosquash --empty=drop HEAD~4 &&
>> +
>> +	git log --format=%s >actual &&
>> +	! grep changeX actual &&
>> +	grep addX actual &&
>> +	grep later actual &&
>> +	echo 1 >expect &&
>> +	test_cmp expect fileX &&
>> +	echo hello >expect &&
>> +	test_cmp expect fileW
>> +'
>> +
>> +test_expect_success 'fixup! that empties its target is kept with --empty=keep' '
>> +	git reset --hard base &&
>> +	test_commit --no-tag addY fileY 1 &&
>> +	test_commit --no-tag changeY fileY 2 &&
>> +	echo 1 >fileY &&
>> +	git commit -m "fixup! changeY" fileY &&
>> +
>> +	git rebase -i --autosquash --empty=keep HEAD~3 &&
>> +
>> +	git log --format=%s >actual &&
>> +	grep changeY actual &&
>> +	: "the retained commit is empty" &&
>> +	git diff --exit-code HEAD~1 HEAD &&
>> +	echo 1 >expect &&
>> +	test_cmp expect fileY
>> +'
>> +
>> +test_expect_success 'fixup! that empties its target stops with --empty=stop' '
>> +	git reset --hard base &&
>> +	test_commit --no-tag addZ fileZ 1 &&
>> +	test_commit --no-tag changeZ fileZ 2 &&
>> +	echo 1 >fileZ &&
>> +	git commit -m "fixup! changeZ" fileZ &&
>> +
>> +	test_when_finished "git rebase --abort" &&
>> +	test_must_fail git rebase -i --autosquash --empty=stop HEAD~3
>> +'
>> +
>> +test_expect_success 'squash! that empties its target is dropped with --empty=drop' '
>> +	git reset --hard base &&
>> +	test_commit --no-tag addS fileS 1 &&
>> +	test_commit --no-tag changeS fileS 2 &&
>> +	echo 1 >fileS &&
>> +	git commit -m "squash! changeS" fileS &&
>> +
>> +	git rebase -i --autosquash --empty=drop HEAD~3 &&
>> +
>> +	git log --format=%s >actual &&
>> +	! grep changeS actual &&
>> +	grep addS actual &&
>> +	echo 1 >expect &&
>> +	test_cmp expect fileS
>> +'
>> +
>>   test_done
>> 
>> 
>> 
>> 

Re: [PATCH] sequencer: honor --empty when a fixup!/squash! empties its target

[Yuxuan Chen]

From: Yuxuan Chen <i@yuxuan.ch>

Hi Phillip,

I'm Yuxuan, and I work with Farid at Meta. Thank you for reviewing this patch.
It addresses a workflow problem for us, and we appreciate your feedback.

Regarding

> Using an empty commit has a marker has the advantage that applying it cannot
> create conflicts, so you only have to deal with the conflicts caused by the
> commit being dropped, not the by fixup not applying cleanly.

I am concerned, however, that representing a `drop!` commit as an empty marker
would be semantically unsound. We expect `rebase --autosquash` to drop the
target commit, but until that rebase happens, the repository is not in a state
where we consider the target commit dropped: the target's changes are still
present, and the empty marker changes nothing. Therefore, I think a `drop!`
commit should contain the inverse of the patch we intend to drop. That way,
the repository state reflects the intended removal even before autosquash
rewrites the history.

I recognize that applying the inverse patch may cause conflicts. However,
this is not a new problem; `git revert` has the same issue when the inverse
patch does not apply cleanly. Such conflicts reflect the actual difficulty of
undoing the change at that point in the history.

Thanks,
Yuxuan

Re: [PATCH] sequencer: honor --empty when a fixup!/squash! empties its target

[phillip.wood123]

Hi Yuxuan

On 10/07/2026 19:30, Yuxuan Chen wrote:
> 
>> Using an empty commit has a marker has the advantage that applying it cannot
>> create conflicts, so you only have to deal with the conflicts caused by the
>> commit being dropped, not the by fixup not applying cleanly.
> 
> I am concerned, however, that representing a `drop!` commit as an empty marker
> would be semantically unsound. We expect `rebase --autosquash` to drop the
> target commit, but until that rebase happens, the repository is not in a state
> where we consider the target commit dropped: the target's changes are still
> present, and the empty marker changes nothing. Therefore, I think a `drop!`
> commit should contain the inverse of the patch we intend to drop. That way,
> the repository state reflects the intended removal even before autosquash
> rewrites the history.

That's a good point. Looking at the gitgitgadget issue tracker [1], 
there is a suggestion to add a new option to revert that behaves like

     git revert -n <commit> &&
     git commit -m 'drop! '"$(git show -s --oneline <commit>)"

and then "git rebase --autosquash" would replace "pick" with "drop" for 
the commit we want to drop and drop the "drop!" commit as well. That 
avoids conflicts when dropping the commit and means anything built on 
top of the "drop!" commit before the rebase does not see the changes in 
the commit that we want to drop because it has been reverted. That seems 
to be the best of both worlds.

> I recognize that applying the inverse patch may cause conflicts. However,
> this is not a new problem; `git revert` has the same issue when the inverse
> patch does not apply cleanly. Such conflicts reflect the actual difficulty of
> undoing the change at that point in the history.
I agree conflicts are a fact of life when rebasing, but I think it is 
worth avoiding them where we can.

Thanks

Phillip

[1] https://github.com/gitgitgadget/git/issues/259

[PATCH v4] sequencer: honor --empty when a fixup!/squash! empties its target

[Farid Zakaria]

When "git rebase --autosquash" squashes a "fixup!" or "squash!" commit
into its target, the result can be a commit that no longer changes
anything relative to its parent, for example when the squashed change
reverts the target.  Rather than dropping or keeping that commit, the
rebase stops with

	You asked to amend the most recent commit, but doing so would
	make it empty. ...

and "--empty" has no effect on it.  This makes backing a change out of a
series awkward: reverting a commit as a "fixup!" and running "git rebase
--autosquash --empty=drop" ought to remove both the commit and its
revert, but it halts instead.

A "fixup" is applied by amending HEAD, so the commit it produces is
empty when the index matches the tree of HEAD's parent rather than the
tree of HEAD.  allow_empty() only knows about the latter, so it never
notices that the fixup cancelled the commit out and "git commit --amend"
is left to refuse to create the empty commit.

Check for this case separately and honor "--empty" for it, subject to
two restrictions.

First, "--empty" only governs commits that become empty, so a commit
that was picked empty to begin with must be left alone.  To tell the two
apart, record in "struct replay_ctx" what the "pick" that created the
commit at HEAD was, and write it to "$GIT_DIR/rebase-merge/fixup-target"
so that it survives a stop for conflict resolution.  Only a commit
created by a "pick" is a candidate: when the todo list has been edited
so that a chain starts after "reset", "exec" or "break", we do not know
how the commit at HEAD came to be and keep it.

Second, only the last fixup of a chain may drop the commit.  Were an
earlier one to drop it, the fixups still to come would be squashed into
the previous commit instead, so a commit emptied mid-chain is kept --
empty for the time being -- and the decision is deferred to the end of
the chain.

With "--empty=drop" the emptied commit has already been created by the
"pick", so drop it by moving HEAD back to its parent and report the new
PICK_RESULT_DROPPED_HEAD, so that neither that commit nor any of the
fixups squashed into it is recorded as rewritten and the post-rewrite
machinery has nothing to report.  A "label" or "update-ref" that follows
then sees HEAD at the parent.

A conflicted fixup that the user resolves by undoing the commit it is
being squashed into leaves the same empty commit behind, so give
commit_staged_changes() the same treatment.

Signed-off-by: Farid Zakaria <farid.m.zakaria@gmail.com>
---
Changes in v4 (thanks again to Phillip Wood's review):
- Rebased onto 'master' now that pw/rebase-drop-notes-with-commit has
  graduated; the drop is reported with a new member of that topic's
  "enum pick_result" instead of an out-parameter.
- Only a commit that *becomes* empty is dropped: whether the "pick" that
  created it was empty is now remembered in "struct replay_ctx" and
  written to "$GIT_DIR/rebase-merge/fixup-target" so it survives a stop
  for conflict resolution.  A commit picked empty is left alone.
- Only the final fixup of a chain may drop the commit.  One that empties
  it mid-chain keeps it, empty for the time being, so that the remaining
  fixups still land on it rather than on the previous commit.
- A chain that is not preceded by a "pick" -- because the todo list was
  edited to start it with "reset", "exec" or "break" -- never drops HEAD.
- commit_staged_changes() honors "--empty=drop" as well, for a
  conflicted fixup that the user resolves by undoing its target.
- allow_empty() is left in place rather than being bypassed, so a fixup
  whose own contents are already upstream is still reported.
- Dropped the tests that expected an already-empty commit to be dropped;
  added tests for the mid-chain, no-preceding-pick and conflict cases,
  and moved the post-rewrite check to t5407 next to the one added by
  pw/rebase-drop-notes-with-commit.
- Reworded the documentation to talk about squashing rather than
  melding, matching the rest of git-rebase(1).
- Link to v3: https://lore.kernel.org/r/20260711-fz-autosquash-empty-v3-1-d227b63eb511@gmail.com

sequencer: let autosquash drop a commit it empties out

At Meta we maintain a fork of LLVM that we regularly rebase onto
upstream.  A set of internal patches rides on top, and we keep each one
as a single commit by folding follow-up changes into it with autosquash
"fixup!" commits.  That works well for evolving a patch, but not for
retiring one: to back an internal patch out today we hand-edit the
interactive rebase todo list to delete the commit and its scattered
fixups, which is fiddly and easy to get wrong.  (The history is rewritten
either way, so a force-push is still needed; what this avoids is the
manual todo surgery.)

It would be nicer to retire a patch the same way we amend one: commit a
revert of it as a "fixup!" and let autosquash fold the two together.
The net change is empty, so the commit should just drop out of the
series.  Today it does not -- the rebase stops instead.

For example, starting from a commit we want to retire:

    $ git log --oneline
    4d5e6f7 add feature patch
    9a1b2c3 base

    # revert the feature and mark the revert as a fixup of it
    $ git revert --no-edit HEAD
    $ git commit --amend -m "fixup! add feature patch"

    $ git rebase -i --autosquash --empty=drop 9a1b2c3
    Rebasing (2/2)
    You asked to amend the most recent commit, but doing so would
    make it empty. You can repeat your command with --allow-empty [...]
    Could not apply 8e9f0a1... # fixup! add feature patch

The "--empty=drop" is ignored.  "--empty" only governs commits that are
picked empty, whereas a "fixup!" is applied by amending, and the
emptiness of an amended commit is measured against the wrong parent.  So
the rebase falls through to "git commit --amend", which refuses to
create an empty commit, and halts.

With this patch the emptied commit is recognized and handled according
to "--empty", the same as any other commit that becomes empty during a
rebase:

    $ git rebase -i --autosquash --empty=drop 9a1b2c3
    Rebasing (2/2)
    dropping 8e9f0a1... fixup! add feature patch -- squashing it in empties the commit
    Successfully rebased and updated refs/heads/main.

    $ git log --oneline
    9a1b2c3 base

"--empty=keep" retains it as an empty commit, and "--empty=stop" (the
default under "-i") halts so the user can decide -- matching how these
options already behave for commits that become empty when picked.

As "--empty" only governs commits that *become* empty, a commit that was
picked empty to begin with is never dropped, and neither is one that a
fixup empties out in the middle of a chain, since the fixups that follow
it would then be squashed into the previous commit instead.

Changes in v3:
 * Switch the new tests' assertions from grep to test_grep for better
   diagnostics (per review).
 * Link to v2: https://lore.kernel.org/r/20260710-fz-autosquash-empty-v2-1-fa1e277e05f8@gmail.com

Changes in v2 (thanks to Phillip Wood's review):
 * An emptied fixup/squash now honors --empty in all cases, including
   when the commit it was folded into started out empty; v1 kept that
   case regardless of --empty.
 * On drop, the dropped commit and its fixup are no longer recorded as
   rewritten, so nothing spurious reaches the post-rewrite machinery.
 * Added tests for the empty-placeholder + fixup cases and for the
   not-recorded-as-rewritten behavior; adjusted t3415 "abort last squash".
 * Link to v1: https://lore.kernel.org/r/20260709-fz-autosquash-empty-v1-1-84cb494c3613@gmail.com
---
 Documentation/git-rebase.adoc |  11 ++
 sequencer.c                   | 299 +++++++++++++++++++++++++++++++++++++++++-
 t/t3415-rebase-autosquash.sh  | 151 +++++++++++++++++++++
 t/t5407-post-rewrite-hook.sh  |  21 +++
 4 files changed, 478 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-rebase.adoc b/Documentation/git-rebase.adoc
index f6c22d1598..a171d3831a 100644
--- a/Documentation/git-rebase.adoc
+++ b/Documentation/git-rebase.adoc
@@ -282,6 +282,11 @@ by `git log --cherry-mark ...`) are detected and dropped as a
 preliminary step (unless `--reapply-cherry-picks` or `--keep-base` is
 passed).
 +
+A commit can also become empty when `--autosquash` squashes a `fixup!`
+or `squash!` commit into it that cancels out all of its changes.  This
+option governs such a commit as well; it is dropped, kept, or stopped
+at just like a commit that becomes empty when it is picked.
++
 See also INCOMPATIBLE OPTIONS below.
 
 --no-keep-empty::
@@ -591,6 +596,12 @@ changed from `pick` to `squash`, `fixup` or `fixup -C`, respectively, and they
 are moved right after the commit they modify.  The `--interactive` option can
 be used to review and edit the todo list before proceeding.
 +
+Squashing a `fixup!` or `squash!` commit into its target can cancel out all
+of the changes of that target, leaving an empty commit behind.  What happens
+then is governed by the `--empty` option, so a change can be backed out of a
+series by committing a revert of it as a `fixup!` and letting
+`--autosquash --empty=drop` remove the two together.
++
 The recommended way to create commits with squash markers is by using the
 `--squash`, `--fixup`, `--fixup=amend:` or `--fixup=reword:` options of
 linkgit:git-commit[1], which take the target commit as an argument and
diff --git a/sequencer.c b/sequencer.c
index 65afd100d9..685e822203 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -210,6 +210,31 @@ static GIT_PATH_FUNC(rebase_path_no_reschedule_failed_exec, "rebase-merge/no-res
 static GIT_PATH_FUNC(rebase_path_drop_redundant_commits, "rebase-merge/drop_redundant_commits")
 static GIT_PATH_FUNC(rebase_path_keep_redundant_commits, "rebase-merge/keep_redundant_commits")
 static GIT_PATH_FUNC(rebase_path_trailer, "rebase-merge/trailer")
+/*
+ * The file that remembers, across a stop for conflict resolution, what
+ * "enum fixup_target" recorded about the commit a chain of fixup and
+ * squash commands is being applied to.
+ */
+static GIT_PATH_FUNC(rebase_path_fixup_target, "rebase-merge/fixup-target")
+
+/*
+ * What we know about the commit that the current chain of fixup and squash
+ * commands is being applied to.  A commit is only dropped when squashing
+ * the fixups into it empties it out if it was picked with changes of its
+ * own, so anything but FIXUP_TARGET_PICKED_NONEMPTY keeps it.
+ */
+enum fixup_target {
+	/*
+	 * The commit was not created by a "pick", either because it was
+	 * dropped or because the todo list starts the chain with some
+	 * other command such as "reset", "exec" or "break".
+	 */
+	FIXUP_TARGET_UNKNOWN = 0,
+	/* The commit was picked empty, so the fixups did not empty it. */
+	FIXUP_TARGET_PICKED_EMPTY,
+	/* The commit was picked with changes, so the fixups may empty it. */
+	FIXUP_TARGET_PICKED_NONEMPTY
+};
 
 /*
  * A 'struct replay_ctx' represents the private state of the sequencer.
@@ -234,6 +259,11 @@ struct replay_ctx {
 	 * Whether message contains a commit message.
 	 */
 	unsigned have_message :1;
+	/*
+	 * What the commit that the current chain of fixup and squash
+	 * commands is being applied to was picked as.
+	 */
+	enum fixup_target fixup_target;
 };
 
 struct replay_ctx* replay_ctx_new(void)
@@ -587,6 +617,38 @@ static int write_message(const void *buf, size_t len, const char *filename,
 	return 0;
 }
 
+/* The two names that rebase_path_fixup_target() stores. */
+static const char *const fixup_target_name[] = {
+	[FIXUP_TARGET_PICKED_EMPTY] = "picked-empty",
+	[FIXUP_TARGET_PICKED_NONEMPTY] = "picked-non-empty"
+};
+
+/*
+ * Remember what the commit that the current chain of fixup and squash
+ * commands is being applied to was picked as, both in the sequencer state
+ * and on disk so that it survives a stop for conflict resolution.
+ */
+static int set_fixup_target(struct replay_opts *opts, enum fixup_target target)
+{
+	struct replay_ctx *ctx = opts->ctx;
+	const char *name;
+
+	if (ctx->fixup_target == target)
+		return 0;
+
+	ctx->fixup_target = target;
+	if (!is_rebase_i(opts))
+		return 0;
+
+	if (target == FIXUP_TARGET_UNKNOWN) {
+		unlink(rebase_path_fixup_target());
+		return 0;
+	}
+
+	name = fixup_target_name[target];
+	return write_message(name, strlen(name), rebase_path_fixup_target(), 1);
+}
+
 int read_oneliner(struct strbuf *buf,
 	const char *path, unsigned flags)
 {
@@ -1818,6 +1880,41 @@ static int allow_empty(struct repository *r,
 		return 0;
 }
 
+/*
+ * A "fixup" or "squash" is applied by amending HEAD, so the commit it
+ * produces is empty when the index matches the tree of HEAD's parent,
+ * rather than the tree of HEAD itself that is_index_unchanged() looks at.
+ * Returns 1 if amending HEAD would leave it empty, 0 if not, and negative
+ * on error.
+ */
+static int is_amended_head_empty(struct repository *r)
+{
+	const struct object_id *parent_tree_oid;
+	struct object_id *cache_tree_oid;
+	struct commit *head;
+
+	head = lookup_commit_reference_by_name("HEAD");
+	if (!head || repo_parse_commit(r, head))
+		return error(_("could not parse HEAD commit"));
+
+	if (head->parents) {
+		struct commit *parent = head->parents->item;
+
+		if (repo_parse_commit(r, parent))
+			return error(_("could not parse parent commit %s"),
+				     oid_to_hex(&parent->object.oid));
+		parent_tree_oid = get_commit_tree_oid(parent);
+	} else {
+		parent_tree_oid = the_hash_algo->empty_tree; /* HEAD is root */
+	}
+
+	cache_tree_oid = get_cache_tree_oid(r->index);
+	if (!cache_tree_oid)
+		return -1;
+
+	return oideq(cache_tree_oid, parent_tree_oid);
+}
+
 static struct {
 	char c;
 	const char *str;
@@ -2273,11 +2370,39 @@ static const char *reflog_message(struct replay_opts *opts,
 	return buf.buf;
 }
 
+/*
+ * Drop the commit at HEAD by moving HEAD back to its parent.  The index and
+ * the worktree already match the tree of that parent, so nothing else needs
+ * to be updated.  "action" names the command doing the dropping and is only
+ * used for the reflog message.
+ */
+static int drop_head_commit(struct repository *r, struct replay_opts *opts,
+			    const char *action)
+{
+	struct commit *head = lookup_commit_reference_by_name("HEAD");
+
+	if (!head || repo_parse_commit(r, head))
+		return error(_("could not parse HEAD commit"));
+	if (!head->parents)
+		return error(_("cannot drop the root commit"));
+
+	return refs_update_ref(get_main_ref_store(r),
+			       reflog_message(opts, action,
+					      "dropping emptied commit"),
+			       "HEAD", &head->parents->item->object.oid,
+			       &head->object.oid, 0, UPDATE_REFS_MSG_ON_ERR);
+}
+
 enum pick_result {
 	PICK_RESULT_ERROR = -1,
 	PICK_RESULT_OK,
 	PICK_RESULT_CONFLICTS,
 	PICK_RESULT_DROPPED,
+	/*
+	 * The fixups were squashed into a commit that they emptied out, so
+	 * that commit was dropped along with them.
+	 */
+	PICK_RESULT_DROPPED_HEAD,
 };
 
 static enum pick_result do_pick_commit(struct repository *r,
@@ -2293,7 +2418,7 @@ static enum pick_result do_pick_commit(struct repository *r,
 	const char *base_label, *next_label, *reflog_action;
 	char *author = NULL;
 	struct commit_message msg = { NULL, NULL, NULL, NULL };
-	int res, unborn = 0, reword = 0, allow, drop_commit = 0;
+	int res, unborn = 0, reword = 0, allow, drop_commit = 0, drop_head = 0;
 	enum todo_command command = item->command;
 	struct commit *commit = item->commit;
 
@@ -2303,6 +2428,20 @@ static enum pick_result do_pick_commit(struct repository *r,
 	else
 		reflog_action = sequencer_reflog_action(opts);
 
+	/*
+	 * Remember whether this commit is picked with changes of its own, as
+	 * only such a commit is dropped when the fixups that follow it empty
+	 * it out again.
+	 */
+	if (is_rebase_i(opts) && command == TODO_PICK) {
+		int empty = is_original_commit_empty(commit);
+
+		if (empty < 0 ||
+		    set_fixup_target(opts, empty ? FIXUP_TARGET_PICKED_EMPTY :
+				     FIXUP_TARGET_PICKED_NONEMPTY))
+			return PICK_RESULT_ERROR;
+	}
+
 	if (opts->no_commit) {
 		/*
 		 * We do not intend to commit immediately.  We just want to
@@ -2540,7 +2679,51 @@ static enum pick_result do_pick_commit(struct repository *r,
 			_("dropping %s %s -- patch contents already upstream\n"),
 			oid_to_hex(&commit->object.oid), msg.subject);
 	} /* else allow == 0 and there's nothing special to do */
-	if (!opts->no_commit && !drop_commit) {
+
+	/*
+	 * allow_empty() above only notices a commit that adds nothing to
+	 * HEAD.  A "fixup" or "squash" can also cancel out the changes of
+	 * the commit it is squashed into, which leaves that commit empty
+	 * instead, so check for that here and honor --empty for it.
+	 */
+	if ((flags & AMEND_MSG) && !drop_commit &&
+	    ctx->fixup_target == FIXUP_TARGET_PICKED_NONEMPTY) {
+		int emptied = is_amended_head_empty(r);
+
+		if (emptied < 0) {
+			res = emptied;
+			goto leave;
+		}
+
+		if (emptied && (!final_fixup || opts->keep_redundant_commits)) {
+			/*
+			 * Keep the commit, empty for now, when more fixups
+			 * are still to be squashed into it, as dropping it
+			 * here would squash them into the previous commit
+			 * instead.  Also keep it when --empty=keep asks us to.
+			 */
+			flags |= ALLOW_EMPTY;
+		} else if (emptied && opts->drop_redundant_commits) {
+			unlink(git_path_merge_msg(r));
+			refs_delete_ref(get_main_ref_store(r), "", "AUTO_MERGE",
+					NULL, REF_NO_DEREF);
+			res = drop_head_commit(r, opts,
+					       command_to_string(command));
+			if (res)
+				goto leave;
+			drop_head = 1;
+			fprintf(stderr,
+				_("dropping %s %s -- squashing it in empties the commit\n"),
+				oid_to_hex(&commit->object.oid), msg.subject);
+		}
+		/*
+		 * Otherwise --empty=stop is in effect, and "git commit
+		 * --amend" below refuses to make the commit empty, which
+		 * halts the rebase.
+		 */
+	}
+
+	if (!opts->no_commit && !drop_commit && !drop_head) {
 		if (author || command == TODO_REVERT || (flags & AMEND_MSG))
 			res = do_commit(r, msg_file, author, reflog_action,
 					opts, flags,
@@ -2587,6 +2770,8 @@ static enum pick_result do_pick_commit(struct repository *r,
 		return PICK_RESULT_ERROR;
 	else if (res > 0)
 		return PICK_RESULT_CONFLICTS;
+	else if (drop_head)
+		return PICK_RESULT_DROPPED_HEAD;
 	else if (drop_commit)
 		return PICK_RESULT_DROPPED;
 	else
@@ -3318,6 +3503,17 @@ static int read_populate_opts(struct replay_opts *opts)
 		}
 		strbuf_reset(&buf);
 
+		if (read_oneliner(&buf, rebase_path_fixup_target(),
+				  READ_ONELINER_SKIP_IF_EMPTY)) {
+			enum fixup_target target;
+
+			for (target = FIXUP_TARGET_PICKED_EMPTY;
+			     target <= FIXUP_TARGET_PICKED_NONEMPTY; target++)
+				if (!strcmp(buf.buf, fixup_target_name[target]))
+					ctx->fixup_target = target;
+			strbuf_reset(&buf);
+		}
+
 		if (read_oneliner(&ctx->current_fixups,
 				  rebase_path_current_fixups(),
 				  READ_ONELINER_SKIP_IF_EMPTY)) {
@@ -5057,9 +5253,26 @@ static int pick_one_commit(struct repository *r,
 				    peek_command(todo_list, 1));
 		return 0;
 	} else if (pick_res == PICK_RESULT_DROPPED) {
+		/*
+		 * When a "pick" is dropped HEAD stays where it was, so a
+		 * "fixup" that follows would be squashed into a commit we
+		 * know nothing about.  A dropped "fixup" on the other hand
+		 * leaves the commit it targets untouched.
+		 */
+		if (!is_fixup(item->command))
+			set_fixup_target(opts, FIXUP_TARGET_UNKNOWN);
 		if (is_final_fixup(todo_list))
 			flush_rewritten_pending();
 		return 0;
+	} else if (pick_res == PICK_RESULT_DROPPED_HEAD) {
+		/*
+		 * The commit the fixups were squashed into is gone, so
+		 * neither it nor any of them were rewritten and there is
+		 * nothing left for the post-rewrite machinery to report.
+		 */
+		unlink(rebase_path_rewritten_pending());
+		set_fixup_target(opts, FIXUP_TARGET_UNKNOWN);
+		return 0;
 	} else if (pick_res == PICK_RESULT_CONFLICTS &&
 		   is_fixup(item->command)) {
 		return error_failed_squash(r, item->commit, opts,
@@ -5115,6 +5328,16 @@ static int pick_commits(struct repository *r,
 
 		if (save_todo(todo_list, opts, reschedule))
 			return -1;
+
+		/*
+		 * Only a commit created by a "pick" is dropped when the
+		 * fixups squashed into it empty it out, so forget about the
+		 * last "pick" as soon as any other command runs.
+		 */
+		if (item->command != TODO_PICK && !is_fixup(item->command) &&
+		    !is_noop(item->command))
+			set_fixup_target(opts, FIXUP_TARGET_UNKNOWN);
+
 		if (is_rebase_i(opts)) {
 			if (item->command != TODO_COMMENT) {
 				FILE *f = fopen(rebase_path_msgnum(), "w");
@@ -5520,6 +5743,53 @@ static int commit_staged_changes(struct repository *r,
 		}
 	}
 
+	/*
+	 * If resolving the conflicts of the last "fixup" or "squash" of a
+	 * chain undid the commit they are being squashed into, honor
+	 * --empty for that commit just as do_pick_commit() does when the
+	 * chain applies cleanly.
+	 */
+	if ((flags & AMEND_MSG) && opts->drop_redundant_commits &&
+	    ctx->fixup_target == FIXUP_TARGET_PICKED_NONEMPTY &&
+	    !is_fixup(peek_command(todo_list, 0))) {
+		int emptied = is_amended_head_empty(r);
+
+		if (emptied < 0) {
+			ret = emptied;
+			goto out;
+		}
+		if (emptied) {
+			ret = drop_head_commit(r, opts, "continue");
+			if (ret)
+				goto out;
+
+			/*
+			 * Neither the dropped commit nor the fixups squashed
+			 * into it were rewritten, so leave nothing behind for
+			 * the post-rewrite machinery to report.
+			 */
+			unlink(rebase_path_stopped_sha());
+			unlink(rebase_path_rewritten_pending());
+			set_fixup_target(opts, FIXUP_TARGET_UNKNOWN);
+
+			unlink(rebase_path_amend());
+			unlink(rebase_path_fixup_msg());
+			unlink(rebase_path_squash_msg());
+			unlink(git_path_merge_head(r));
+			unlink(git_path_merge_msg(r));
+			refs_delete_ref(get_main_ref_store(r), "", "AUTO_MERGE",
+					NULL, REF_NO_DEREF);
+			if (ctx->current_fixup_count > 0) {
+				unlink(rebase_path_current_fixups());
+				strbuf_reset(&ctx->current_fixups);
+				ctx->current_fixup_count = 0;
+			}
+
+			ret = 0;
+			goto out;
+		}
+	}
+
 	if (run_git_commit(final_fixup ? NULL : rebase_path_message(),
 			   reflog_action, opts, flags)) {
 		ret = error(_("could not commit staged changes."));
@@ -6488,6 +6758,7 @@ int todo_list_write_to_file(struct repository *r, struct todo_list *todo_list,
 
 /* skip picking commits whose parents are unchanged */
 static int skip_unnecessary_picks(struct repository *r,
+				  struct replay_opts *opts,
 				  struct todo_list *todo_list,
 				  struct object_id *base_oid)
 {
@@ -6527,8 +6798,28 @@ static int skip_unnecessary_picks(struct repository *r,
 		todo_list->current = 0;
 		todo_list->done_nr += i;
 
-		if (is_fixup(peek_command(todo_list, 0)))
+		if (is_fixup(peek_command(todo_list, 0))) {
+			/*
+			 * The picks that were skipped never reach
+			 * do_pick_commit(), so record here what the last of
+			 * them left at HEAD for the fixups that follow it.
+			 */
+			struct commit *base = lookup_commit_reference(r,
+								      base_oid);
+			int empty;
+
+			if (!base)
+				return error(_("could not parse commit '%s'"),
+					     oid_to_hex(base_oid));
+			empty = is_original_commit_empty(base);
+			if (empty < 0 ||
+			    set_fixup_target(opts,
+					     empty ? FIXUP_TARGET_PICKED_EMPTY :
+					     FIXUP_TARGET_PICKED_NONEMPTY))
+				return -1;
+
 			record_in_rewritten(base_oid, peek_command(todo_list, 0));
+		}
 	}
 
 	return 0;
@@ -6727,7 +7018,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
 		BUG("invalid todo list after expanding IDs:\n%s",
 		    new_todo.buf.buf);
 
-	if (opts->allow_ff && skip_unnecessary_picks(r, &new_todo, &oid)) {
+	if (opts->allow_ff && skip_unnecessary_picks(r, opts, &new_todo, &oid)) {
 		todo_list_release(&new_todo);
 		return error(_("could not skip unnecessary pick commands"));
 	}
diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
index 07a5a11678..06b501b2af 100755
--- a/t/t3415-rebase-autosquash.sh
+++ b/t/t3415-rebase-autosquash.sh
@@ -510,4 +510,155 @@ test_expect_success 'pick and fixup respect commit.cleanup' '
 	test_commit_message HEAD -m "something"
 '
 
+test_expect_success 'fixup! that empties its target is dropped with --empty=drop' '
+	git reset --hard base &&
+	test_commit --no-tag addX fileX 1 &&
+	test_commit --no-tag changeX fileX 2 &&
+	test_commit --no-tag later fileW hello &&
+	echo 1 >fileX &&
+	git commit -m "fixup! changeX" fileX &&
+
+	git rebase -i --autosquash --empty=drop HEAD~4 &&
+
+	git log --format=%s >actual &&
+	test_grep ! changeX actual &&
+	test_grep addX actual &&
+	test_grep later actual &&
+	echo 1 >expect &&
+	test_cmp expect fileX &&
+	echo hello >expect &&
+	test_cmp expect fileW
+'
+
+test_expect_success 'fixup! that empties its target is kept with --empty=keep' '
+	git reset --hard base &&
+	test_commit --no-tag addY fileY 1 &&
+	test_commit --no-tag changeY fileY 2 &&
+	echo 1 >fileY &&
+	git commit -m "fixup! changeY" fileY &&
+
+	git rebase -i --autosquash --empty=keep HEAD~3 &&
+
+	git log --format=%s >actual &&
+	test_grep changeY actual &&
+	: "the commit that was kept is empty" &&
+	git diff --exit-code HEAD~1 HEAD &&
+	echo 1 >expect &&
+	test_cmp expect fileY
+'
+
+test_expect_success 'fixup! that empties its target halts by default' '
+	git reset --hard base &&
+	test_commit --no-tag addZ fileZ 1 &&
+	test_commit --no-tag changeZ fileZ 2 &&
+	echo 1 >fileZ &&
+	git commit -m "fixup! changeZ" fileZ &&
+
+	test_when_finished "git rebase --abort" &&
+	test_must_fail git rebase -i --autosquash HEAD~3
+'
+
+test_expect_success 'squash! that empties its target is dropped with --empty=drop' '
+	git reset --hard base &&
+	test_commit --no-tag addS fileS 1 &&
+	test_commit --no-tag changeS fileS 2 &&
+	echo 1 >fileS &&
+	git commit -m "squash! changeS" fileS &&
+
+	git rebase -i --autosquash --empty=drop HEAD~3 &&
+
+	git log --format=%s >actual &&
+	test_grep ! changeS actual &&
+	test_grep addS actual &&
+	echo 1 >expect &&
+	test_cmp expect fileS
+'
+
+test_expect_success 'a target emptied in the middle of a chain is not dropped' '
+	git reset --hard base &&
+	test_commit --no-tag addM fileM 1 &&
+	test_commit --no-tag changeM fileM 2 &&
+	echo 1 >fileM &&
+	git commit -m "fixup! changeM" fileM &&
+	test_commit --no-tag "fixup! changeM" fileN later &&
+
+	git rebase -i --autosquash --empty=drop HEAD~4 &&
+
+	: "the second fixup! refills the commit the first one emptied" &&
+	git log --format=%s >actual &&
+	test_grep changeM actual &&
+	echo 1 >expect &&
+	test_cmp expect fileM &&
+	echo later >expect &&
+	test_cmp expect fileN
+'
+
+test_expect_success 'a commit picked empty is kept when a fixup! leaves it empty' '
+	git reset --hard base &&
+	git commit --allow-empty -m placeholder &&
+	git commit --allow-empty -m "fixup! placeholder" &&
+
+	git rebase -i --autosquash --empty=drop HEAD~2 &&
+
+	: "--empty only governs commits that become empty" &&
+	git log --format=%s >actual &&
+	test_grep placeholder actual &&
+	git diff --exit-code HEAD~1 HEAD
+'
+
+test_expect_success 'fixup! filling in an empty commit keeps a non-empty commit' '
+	git reset --hard base &&
+	git commit --allow-empty -m placeholder &&
+	test_commit --no-tag "fixup! placeholder" fileP content &&
+
+	git rebase -i --autosquash --empty=drop HEAD~2 &&
+
+	git log --format=%s >actual &&
+	test_grep placeholder actual &&
+	echo content >expect &&
+	test_cmp expect fileP &&
+	test_must_fail git diff --exit-code HEAD~1 HEAD
+'
+
+test_expect_success 'a fixup! not preceded by a pick does not drop its target' '
+	git reset --hard base &&
+	test_commit --no-tag addQ fileQ 1 &&
+	test_commit --no-tag changeQ fileQ 2 &&
+	echo 1 >fileQ &&
+	git commit -m "fixup! changeQ" fileQ &&
+
+	: "an exec between the pick and the fixup hides what was picked" &&
+	test_when_finished "git rebase --abort" &&
+	set_fake_editor &&
+	test_must_fail env FAKE_LINES="1 2 exec_true 3" \
+		git rebase -i --autosquash --empty=drop HEAD~3
+'
+
+test_expect_success 'resolving a conflicted fixup! by emptying its target drops it' '
+	git reset --hard base &&
+	test_commit --no-tag addC fileC 1 &&
+	test_commit --no-tag changeC fileC 2 &&
+	test_commit --no-tag otherC fileC 3 &&
+	echo 1 >fileC &&
+	git commit -m "fixup! changeC" fileC &&
+
+	test_when_finished "test_might_fail git rebase --abort" &&
+	: "the fixup! is built on otherC, so it conflicts with changeC" &&
+	test_must_fail git rebase -i --autosquash --empty=drop HEAD~4 &&
+
+	: "resolve it by undoing changeC, which leaves changeC empty" &&
+	echo 1 >fileC &&
+	git add fileC &&
+	: "changeC is now gone, so otherC conflicts with addC" &&
+	test_must_fail git rebase --continue &&
+	echo 3 >fileC &&
+	git add fileC &&
+	git rebase --continue &&
+
+	git log --format=%s >actual &&
+	test_grep ! changeC actual &&
+	test_grep addC actual &&
+	test_grep otherC actual
+'
+
 test_done
diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh
index ca8a10fbb1..a35671fea2 100755
--- a/t/t5407-post-rewrite-hook.sh
+++ b/t/t5407-post-rewrite-hook.sh
@@ -333,4 +333,25 @@ test_expect_success 'rebase with commits that become empty' '
 	verify_hook_input
 '
 
+test_expect_success 'rebase drops a commit that its fixup empties' '
+	git checkout -b empty-fixup A &&
+	test_commit --no-tag P1 file1 one &&
+	test_commit --no-tag P2 file1 two &&
+	test_commit --no-tag P3 file2 three &&
+	echo one >file1 &&
+	git commit -m "fixup! P2" file1 &&
+	p1=$(git rev-parse HEAD~3) &&
+	p3=$(git rev-parse HEAD~1) &&
+	clear_hook_input &&
+
+	git rebase -i --autosquash --empty=drop B &&
+
+	echo rebase >expected.args &&
+	cat >expected.data <<-EOF &&
+	$p1 $(git rev-parse HEAD~1)
+	$p3 $(git rev-parse HEAD)
+	EOF
+	verify_hook_input
+'
+
 test_done

---
base-commit: f78ce2f7b6df702f93d40b85d6bda92a3f65da79
change-id: 20260709-fz-autosquash-empty-b6692cf36c60

Re: [PATCH v4] sequencer: honor --empty when a fixup!/squash! empties its target

[Phillip Wood]

Hi Farid

On 27/08/2026 19:19, Farid Zakaria wrote:
> When "git rebase --autosquash" squashes a "fixup!" or "squash!" commit
> into its target, the result can be a commit that no longer changes
> anything relative to its parent, for example when the squashed change
> reverts the target.  Rather than dropping or keeping that commit, the
> rebase stops with
> 
> 	You asked to amend the most recent commit, but doing so would
> 	make it empty. ...
> 
> and "--empty" has no effect on it.  This makes backing a change out of a
> series awkward: reverting a commit as a "fixup!" and running "git rebase
> --autosquash --empty=drop" ought to remove both the commit and its
> revert, but it halts instead.
> 
> A "fixup" is applied by amending HEAD, so the commit it produces is
> empty when the index matches the tree of HEAD's parent rather than the
> tree of HEAD.  allow_empty() only knows about the latter, so it never
> notices that the fixup cancelled the commit out and "git commit --amend"
> is left to refuse to create the empty commit.
> 
> Check for this case separately and honor "--empty" for it, subject to
> two restrictions.
> 
> First, "--empty" only governs commits that become empty, so a commit
> that was picked empty to begin with must be left alone.  To tell the two
> apart, record in "struct replay_ctx" what the "pick" that created the
> commit at HEAD was, and write it to "$GIT_DIR/rebase-merge/fixup-target"
> so that it survives a stop for conflict resolution.  Only a commit
> created by a "pick" is a candidate: when the todo list has been edited
> so that a chain starts after "reset", "exec" or "break", we do not know
> how the commit at HEAD came to be and keep it.
> 
> Second, only the last fixup of a chain may drop the commit.  Were an
> earlier one to drop it, the fixups still to come would be squashed into
> the previous commit instead, so a commit emptied mid-chain is kept --
> empty for the time being -- and the decision is deferred to the end of
> the chain.
> 
> With "--empty=drop" the emptied commit has already been created by the
> "pick", so drop it by moving HEAD back to its parent and report the new
> PICK_RESULT_DROPPED_HEAD, so that neither that commit nor any of the
> fixups squashed into it is recorded as rewritten and the post-rewrite
> machinery has nothing to report.  A "label" or "update-ref" that follows
> then sees HEAD at the parent.
> 
> A conflicted fixup that the user resolves by undoing the commit it is
> being squashed into leaves the same empty commit behind, so give
> commit_staged_changes() the same treatment.

This all sounds good - lets look at the implementation ...

> Signed-off-by: Farid Zakaria <farid.m.zakaria@gmail.com>
> @@ -2303,6 +2428,20 @@ static enum pick_result do_pick_commit(struct repository *r,
>   	else
>   		reflog_action = sequencer_reflog_action(opts);
>   
> +	/*
> +	 * Remember whether this commit is picked with changes of its own, as
> +	 * only such a commit is dropped when the fixups that follow it empty
> +	 * it out again.
> +	 */
> +	if (is_rebase_i(opts) && command == TODO_PICK) {
> +		int empty = is_original_commit_empty(commit);

So we only support dropping a "pick" that becomes empty, not a "reword"
or "edit". It would be strange to reword or edit a commit that is
destined to be dropped so that makes sense.

> +
> +		if (empty < 0 ||
> +		    set_fixup_target(opts, empty ? FIXUP_TARGET_PICKED_EMPTY :
> +				     FIXUP_TARGET_PICKED_NONEMPTY))
> +			return PICK_RESULT_ERROR;
> +	}
> +
>   	if (opts->no_commit) {
>   		/*
>   		 * We do not intend to commit immediately.  We just want to
> @@ -2540,7 +2679,51 @@ static enum pick_result do_pick_commit(struct repository *r,
>   			_("dropping %s %s -- patch contents already upstream\n"),
>   			oid_to_hex(&commit->object.oid), msg.subject);
>   	} /* else allow == 0 and there's nothing special to do */
> -	if (!opts->no_commit && !drop_commit) {
> +
> +	/*
> +	 * allow_empty() above only notices a commit that adds nothing to
> +	 * HEAD.  A "fixup" or "squash" can also cancel out the changes of
> +	 * the commit it is squashed into, which leaves that commit empty
> +	 * instead, so check for that here and honor --empty for it.
> +	 */
> +	if ((flags & AMEND_MSG) && !drop_commit &&

If we have a chain of fixups that looks like

	pick C
	fixup revert-C
	fixup becomes-empty

The "!drop_commit" above means that we wont drop "C" even though
squashing "revert-C" made it empty because "drop_commit == 1" after
"fixup becomes-empty".

> -	if (!opts->no_commit && !drop_commit) {
> +
> +	/*
> +	 * allow_empty() above only notices a commit that adds nothing to
> +	 * HEAD.  A "fixup" or "squash" can also cancel out the changes of
> +	 * the commit it is squashed into, which leaves that commit empty
> +	 * instead, so check for that here and honor --empty for it.
> +	 */
> +	if ((flags & AMEND_MSG) && !drop_commit &&
> +	    ctx->fixup_target == FIXUP_TARGET_PICKED_NONEMPTY) {
> +		int emptied = is_amended_head_empty(r);
> +
> +		if (emptied < 0) {
> +			res = emptied;
> +			goto leave;
> +		}
> +
> +		if (emptied && (!final_fixup || opts->keep_redundant_commits)) {

I'm not sure we need to check "emptied" here it should be fine to set
ALLOW_EMPTY unconditionally. That would allow us to move the call to
is_amended_head_empty() into the conditional block below, so we only
call it on the final fixup when dropping empty commits.

> +			/*
> +			 * Keep the commit, empty for now, when more fixups
> +			 * are still to be squashed into it, as dropping it
> +			 * here would squash them into the previous commit
> +			 * instead.  Also keep it when --empty=keep asks us to.
> +			 */
> +			flags |= ALLOW_EMPTY;

With a chain of fixups that looks like

	pick C
	fixup becomes-empty

and --empty=stop we wont stop because we've added ALLOW_EMPTY to flags.
However, because we're amending a non-empty commit it turns out the
existing code doesn't stop either, as it sees an non-empty commit when
it amends HEAD, so we're not making things any worse. With

	pick empty
	fixup becomes-empty

then the behavior is unchanged and we do stop. That inconsistency is
something we should fix but not as part of this series.

> +		} else if (emptied && opts->drop_redundant_commits) {
> +			unlink(git_path_merge_msg(r));
> +			refs_delete_ref(get_main_ref_store(r), "", "AUTO_MERGE",
> +					NULL, REF_NO_DEREF);
> +			res = drop_head_commit(r, opts,
> +					       command_to_string(command));
> +			if (res)
> +				goto leave;
> +			drop_head = 1;
> +			fprintf(stderr,
> +				_("dropping %s %s -- squashing it in empties the commit\n"),
> +				oid_to_hex(&commit->object.oid), msg.subject);
> +		}
> +		/*
> +		 * Otherwise --empty=stop is in effect, and "git commit
> +		 * --amend" below refuses to make the commit empty, which
> +		 * halts the rebase.
> +		 */
> +	}
> +
> +	if (!opts->no_commit && !drop_commit && !drop_head) {
>  		if (author || command == TODO_REVERT || (flags & AMEND_MSG))

> @@ -5057,9 +5253,26 @@ static int pick_one_commit(struct repository *r,
>   				    peek_command(todo_list, 1));
>   		return 0;
>   	} else if (pick_res == PICK_RESULT_DROPPED) {
> +		/*
> +		 * When a "pick" is dropped HEAD stays where it was, so a
> +		 * "fixup" that follows would be squashed into a commit we
> +		 * know nothing about. 

Dropping a commit and then squashing fixups into the previous pick is a
bug which we can fix using the changes in this patch. We should not drop
a pick that's followed by fixups, instead we should wait to the end of
the fixup chain to see if it is still empty. That doesn't necessarily
need to be part of this series though.

> + A dropped "fixup" on the other hand
> +		 * leaves the commit it targets untouched.
> +		 */
> +		if (!is_fixup(item->command))
> +			set_fixup_target(opts, FIXUP_TARGET_UNKNOWN);
>   		if (is_final_fixup(todo_list))
>   			flush_rewritten_pending();
>   		return 0;
> +	} else if (pick_res == PICK_RESULT_DROPPED_HEAD) {
> +		/*
> +		 * The commit the fixups were squashed into is gone, so
> +		 * neither it nor any of them were rewritten and there is
> +		 * nothing left for the post-rewrite machinery to report.
> +		 */
> +		unlink(rebase_path_rewritten_pending());
> +		set_fixup_target(opts, FIXUP_TARGET_UNKNOWN);

I guess this doesn't do any harm, but do we actually need it? We know
there are no more fixups in the chain so the next command will call
set_fixup_target() anyway.

> +		return 0;
>   	} else if (pick_res == PICK_RESULT_CONFLICTS &&
>   		   is_fixup(item->command)) {
>   		return error_failed_squash(r, item->commit, opts,
> @@ -5115,6 +5328,16 @@ static int pick_commits(struct repository *r,
>   
>   		if (save_todo(todo_list, opts, reschedule))
>   			return -1;
> +
> +		/*
> +		 * Only a commit created by a "pick" is dropped when the
> +		 * fixups squashed into it empty it out, so forget about the
> +		 * last "pick" as soon as any other command runs.
> +		 */
> +		if (item->command != TODO_PICK && !is_fixup(item->command) &&
> +		    !is_noop(item->command))
> +			set_fixup_target(opts, FIXUP_TARGET_UNKNOWN);
> +

It is a bit unfortunate that this is separated from setting the fixup
target for a "pick" command, it would be easier to follow if we did that
here rather than in do_pick_commit() and moved this below the next
line to give

>   		if (is_rebase_i(opts)) {

	if (item->command == TODO_PICK) {
		int empty = is_original_commit_empty(commit);

		if (empty < 0 ||
		    set_fixup_target(opts, empty ? FIXUP_TARGET_PICKED_EMPTY :
				     FIXUP_TARGET_PICKED_NONEMPTY))
			return -1;
	} else if (item->command != noop && !is_fixup(item->command)) {
		set_fixup_target(opts, FIXUP_TARGET_UNKNOWN);
	}

>   			if (item->command != TODO_COMMENT) {
>   				FILE *f = fopen(rebase_path_msgnum(), "w");
> @@ -5520,6 +5743,53 @@ static int commit_staged_changes(struct repository *r,
>   		}
>   	}
>   
> +	/*
> +	 * If resolving the conflicts of the last "fixup" or "squash" of a
> +	 * chain undid the commit they are being squashed into, honor
> +	 * --empty for that commit just as do_pick_commit() does when the
> +	 * chain applies cleanly.
> +	 */
> +	if ((flags & AMEND_MSG) && opts->drop_redundant_commits &&
> +	    ctx->fixup_target == FIXUP_TARGET_PICKED_NONEMPTY &&
> +	    !is_fixup(peek_command(todo_list, 0))) {
> +		int emptied = is_amended_head_empty(r);
> +
> +		if (emptied < 0) {
> +			ret = emptied;
> +			goto out;
> +		}
> +		if (emptied) {
> +			ret = drop_head_commit(r, opts, "continue");
> +			if (ret)
> +				goto out;
> +
> +			/*
> +			 * Neither the dropped commit nor the fixups squashed
> +			 * into it were rewritten, so leave nothing behind for
> +			 * the post-rewrite machinery to report.
> +			 */
> +			unlink(rebase_path_stopped_sha());
> +			unlink(rebase_path_rewritten_pending());
> +			set_fixup_target(opts, FIXUP_TARGET_UNKNOWN);
> +
> +			unlink(rebase_path_amend());
> +			unlink(rebase_path_fixup_msg());
> +			unlink(rebase_path_squash_msg());
> +			unlink(git_path_merge_head(r));
> +			unlink(git_path_merge_msg(r));
> +			refs_delete_ref(get_main_ref_store(r), "", "AUTO_MERGE",
> +					NULL, REF_NO_DEREF);
> +			if (ctx->current_fixup_count > 0) {
> +				unlink(rebase_path_current_fixups());
> +				strbuf_reset(&ctx->current_fixups);
> +				ctx->current_fixup_count = 0;
> +			}

This cleanup looks like it could easily go stale - can we have a single
place where we clean up and jump to that? Would a label just below "if
(run_git_commit(...))" do the job? That way we only need to remember to
clean up the files that "git commit" removes here.

> +			ret = 0;
> +			goto out;
> +		}
> +	}
> +
>   	if (run_git_commit(final_fixup ? NULL : rebase_path_message(),
>   			   reflog_action, opts, flags)) {
>   		ret = error(_("could not commit staged changes."));

> @@ -6527,8 +6798,28 @@ static int skip_unnecessary_picks(struct repository *r,
>   		todo_list->current = 0;
>   		todo_list->done_nr += i;
>   
> -		if (is_fixup(peek_command(todo_list, 0)))
> +		if (is_fixup(peek_command(todo_list, 0))) {
> +			/*
> +			 * The picks that were skipped never reach
> +			 * do_pick_commit(), so record here what the last of
> +			 * them left at HEAD for the fixups that follow it.
> +			 */
> +			struct commit *base = lookup_commit_reference(r,
> +								      base_oid);

If we changed the loop above to remember the commit as "struct commit
*base_commit", rather than the object id as "struct object_id base_oid"
we wouldn't need to lookup the commit again here. The loop above breaks
on the first command that isn't a "pick" so the logic here looks sound.

> +			int empty;
> +
> +			if (!base)
> +				return error(_("could not parse commit '%s'"),
> +					     oid_to_hex(base_oid));
> +			empty = is_original_commit_empty(base);
> +			if (empty < 0 ||
> +			    set_fixup_target(opts,
> +					     empty ? FIXUP_TARGET_PICKED_EMPTY :
> +					     FIXUP_TARGET_PICKED_NONEMPTY))
> +				return -1;
> +
>   			record_in_rewritten(base_oid, peek_command(todo_list, 0));
> +		}
>   	}


> diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
> index 07a5a11678..06b501b2af 100755
> --- a/t/t3415-rebase-autosquash.sh
> +++ b/t/t3415-rebase-autosquash.sh
> @@ -510,4 +510,155 @@ test_expect_success 'pick and fixup respect commit.cleanup' '
>   	test_commit_message HEAD -m "something"
>   '
>   
> +test_expect_success 'fixup! that empties its target is dropped with --empty=drop' '
> +	git reset --hard base &&
> +	test_commit --no-tag addX fileX 1 &&
> +	test_commit --no-tag changeX fileX 2 &&
> +	test_commit --no-tag later fileW hello &&
> +	echo 1 >fileX &&
> +	git commit -m "fixup! changeX" fileX &&

I think those two commands are just

	test_commit --no-tag "fixup! changeX" fileX 1

> +
> +	git rebase -i --autosquash --empty=drop HEAD~4 &&
> +
> +	git log --format=%s >actual &&
> +	test_grep ! changeX actual &&
> +	test_grep addX actual &&
> +	test_grep later actual &&

It would be easier to see what this was checking if we used test_cmp()
rather than a bunch of test_grep() calls.

The test coverage looks reasonable, I would add

test_expect_success 'fixup! that becomes empty, after its target has become empty' '
	git checkout -f first-commit &&
	test_commit two file1 2 &&
	test_commit --no-tag "fixup! two" file1 0 &&
	test_commit --no-tag "fixup! two" file3 3 &&
	test_commit four file2 4 &&

	git rebase --autosquash --empty=drop --reapply-cherry-picks base 2>err &&
	test_grep "contents already upstream" err &&
	test_grep "squashing it in empties the commit" err &&
	test_commit_message HEAD -m four &&
	git diff --exit-code four HEAD &&
	test_cmp_rev base HEAD~1
'

to check that we still drop the amended commit when the final fixup
becomes empty.

This is looking good, hopefully with a couple of tweaks it'll be ready
to be merged.

Thanks

Phillip

> +	echo 1 >expect &&
> +	test_cmp expect fileX &&
> +	echo hello >expect &&
> +	test_cmp expect fileW
> +'
> +
> +test_expect_success 'fixup! that empties its target is kept with --empty=keep' '
> +	git reset --hard base &&
> +	test_commit --no-tag addY fileY 1 &&
> +	test_commit --no-tag changeY fileY 2 &&
> +	echo 1 >fileY &&
> +	git commit -m "fixup! changeY" fileY &&
> +
> +	git rebase -i --autosquash --empty=keep HEAD~3 &&
> +
> +	git log --format=%s >actual &&
> +	test_grep changeY actual &&
> +	: "the commit that was kept is empty" &&
> +	git diff --exit-code HEAD~1 HEAD &&
> +	echo 1 >expect &&
> +	test_cmp expect fileY
> +'
> +
> +test_expect_success 'fixup! that empties its target halts by default' '
> +	git reset --hard base &&
> +	test_commit --no-tag addZ fileZ 1 &&
> +	test_commit --no-tag changeZ fileZ 2 &&
> +	echo 1 >fileZ &&
> +	git commit -m "fixup! changeZ" fileZ &&
> +
> +	test_when_finished "git rebase --abort" &&
> +	test_must_fail git rebase -i --autosquash HEAD~3
> +'
> +
> +test_expect_success 'squash! that empties its target is dropped with --empty=drop' '
> +	git reset --hard base &&
> +	test_commit --no-tag addS fileS 1 &&
> +	test_commit --no-tag changeS fileS 2 &&
> +	echo 1 >fileS &&
> +	git commit -m "squash! changeS" fileS &&
> +
> +	git rebase -i --autosquash --empty=drop HEAD~3 &&
> +
> +	git log --format=%s >actual &&
> +	test_grep ! changeS actual &&
> +	test_grep addS actual &&
> +	echo 1 >expect &&
> +	test_cmp expect fileS
> +'
> +
> +test_expect_success 'a target emptied in the middle of a chain is not dropped' '
> +	git reset --hard base &&
> +	test_commit --no-tag addM fileM 1 &&
> +	test_commit --no-tag changeM fileM 2 &&
> +	echo 1 >fileM &&
> +	git commit -m "fixup! changeM" fileM &&
> +	test_commit --no-tag "fixup! changeM" fileN later &&
> +
> +	git rebase -i --autosquash --empty=drop HEAD~4 &&
> +
> +	: "the second fixup! refills the commit the first one emptied" &&
> +	git log --format=%s >actual &&
> +	test_grep changeM actual &&
> +	echo 1 >expect &&
> +	test_cmp expect fileM &&
> +	echo later >expect &&
> +	test_cmp expect fileN
> +'
> +
> +test_expect_success 'a commit picked empty is kept when a fixup! leaves it empty' '
> +	git reset --hard base &&
> +	git commit --allow-empty -m placeholder &&
> +	git commit --allow-empty -m "fixup! placeholder" &&
> +
> +	git rebase -i --autosquash --empty=drop HEAD~2 &&
> +
> +	: "--empty only governs commits that become empty" &&
> +	git log --format=%s >actual &&
> +	test_grep placeholder actual &&
> +	git diff --exit-code HEAD~1 HEAD
> +'
> +
> +test_expect_success 'fixup! filling in an empty commit keeps a non-empty commit' '
> +	git reset --hard base &&
> +	git commit --allow-empty -m placeholder &&
> +	test_commit --no-tag "fixup! placeholder" fileP content &&
> +
> +	git rebase -i --autosquash --empty=drop HEAD~2 &&
> +
> +	git log --format=%s >actual &&
> +	test_grep placeholder actual &&
> +	echo content >expect &&
> +	test_cmp expect fileP &&
> +	test_must_fail git diff --exit-code HEAD~1 HEAD
> +'
> +
> +test_expect_success 'a fixup! not preceded by a pick does not drop its target' '
> +	git reset --hard base &&
> +	test_commit --no-tag addQ fileQ 1 &&
> +	test_commit --no-tag changeQ fileQ 2 &&
> +	echo 1 >fileQ &&
> +	git commit -m "fixup! changeQ" fileQ &&
> +
> +	: "an exec between the pick and the fixup hides what was picked" &&
> +	test_when_finished "git rebase --abort" &&
> +	set_fake_editor &&
> +	test_must_fail env FAKE_LINES="1 2 exec_true 3" \
> +		git rebase -i --autosquash --empty=drop HEAD~3
> +'
> +
> +test_expect_success 'resolving a conflicted fixup! by emptying its target drops it' '
> +	git reset --hard base &&
> +	test_commit --no-tag addC fileC 1 &&
> +	test_commit --no-tag changeC fileC 2 &&
> +	test_commit --no-tag otherC fileC 3 &&
> +	echo 1 >fileC &&
> +	git commit -m "fixup! changeC" fileC &&
> +
> +	test_when_finished "test_might_fail git rebase --abort" &&
> +	: "the fixup! is built on otherC, so it conflicts with changeC" &&
> +	test_must_fail git rebase -i --autosquash --empty=drop HEAD~4 &&
> +
> +	: "resolve it by undoing changeC, which leaves changeC empty" &&
> +	echo 1 >fileC &&
> +	git add fileC &&
> +	: "changeC is now gone, so otherC conflicts with addC" &&
> +	test_must_fail git rebase --continue &&
> +	echo 3 >fileC &&
> +	git add fileC &&
> +	git rebase --continue &&
> +
> +	git log --format=%s >actual &&
> +	test_grep ! changeC actual &&
> +	test_grep addC actual &&
> +	test_grep otherC actual
> +'
> +
>   test_done
> diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh
> index ca8a10fbb1..a35671fea2 100755
> --- a/t/t5407-post-rewrite-hook.sh
> +++ b/t/t5407-post-rewrite-hook.sh
> @@ -333,4 +333,25 @@ test_expect_success 'rebase with commits that become empty' '
>   	verify_hook_input
>   '
>   
> +test_expect_success 'rebase drops a commit that its fixup empties' '
> +	git checkout -b empty-fixup A &&
> +	test_commit --no-tag P1 file1 one &&
> +	test_commit --no-tag P2 file1 two &&
> +	test_commit --no-tag P3 file2 three &&
> +	echo one >file1 &&
> +	git commit -m "fixup! P2" file1 &&
> +	p1=$(git rev-parse HEAD~3) &&
> +	p3=$(git rev-parse HEAD~1) &&
> +	clear_hook_input &&
> +
> +	git rebase -i --autosquash --empty=drop B &&
> +
> +	echo rebase >expected.args &&
> +	cat >expected.data <<-EOF &&
> +	$p1 $(git rev-parse HEAD~1)
> +	$p3 $(git rev-parse HEAD)
> +	EOF
> +	verify_hook_input
> +'
> +
>   test_done
> 
> ---
> base-commit: f78ce2f7b6df702f93d40b85d6bda92a3f65da79
> change-id: 20260709-fz-autosquash-empty-b6692cf36c60
> 
>