* [PATCH] sequencer: honor --empty when a fixup!/squash! empties its target
@ 2026-07-10 4:13 Farid Zakaria
2026-07-10 13:28 ` Phillip Wood
0 siblings, 1 reply; 5+ messages in thread
From: Farid Zakaria @ 2026-07-10 4:13 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Elijah Newren, Patrick Steinhardt, Junio C Hamano,
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] sequencer: honor --empty when a fixup!/squash! empties its target
2026-07-10 4:13 [PATCH] sequencer: honor --empty when a fixup!/squash! empties its target Farid Zakaria
@ 2026-07-10 13:28 ` Phillip Wood
2026-07-10 16:42 ` Farid Zakaria
2026-07-10 18:30 ` Yuxuan Chen
0 siblings, 2 replies; 5+ messages in thread
From: Phillip Wood @ 2026-07-10 13:28 UTC (permalink / raw)
To: Farid Zakaria, git
Cc: Phillip Wood, Elijah Newren, Patrick Steinhardt, Junio C Hamano
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
>
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] sequencer: honor --empty when a fixup!/squash! empties its target
2026-07-10 13:28 ` Phillip Wood
@ 2026-07-10 16:42 ` Farid Zakaria
2026-07-10 18:30 ` Yuxuan Chen
1 sibling, 0 replies; 5+ messages in thread
From: Farid Zakaria @ 2026-07-10 16:42 UTC (permalink / raw)
To: Phillip Wood, Farid Zakaria, git
Cc: Phillip Wood, Elijah Newren, Patrick Steinhardt, Junio C Hamano
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
>>
>>
>>
>>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] sequencer: honor --empty when a fixup!/squash! empties its target
2026-07-10 13:28 ` Phillip Wood
2026-07-10 16:42 ` Farid Zakaria
@ 2026-07-10 18:30 ` Yuxuan Chen
2026-07-13 13:18 ` phillip.wood123
1 sibling, 1 reply; 5+ messages in thread
From: Yuxuan Chen @ 2026-07-10 18:30 UTC (permalink / raw)
To: phillip.wood123@gmail.com
Cc: farid.m.zakaria@gmail.com, git@vger.kernel.org, gitster@pobox.com,
newren@gmail.com, phillip.wood@dunelm.org.uk, ps@pks.im,
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] sequencer: honor --empty when a fixup!/squash! empties its target
2026-07-10 18:30 ` Yuxuan Chen
@ 2026-07-13 13:18 ` phillip.wood123
0 siblings, 0 replies; 5+ messages in thread
From: phillip.wood123 @ 2026-07-13 13:18 UTC (permalink / raw)
To: Yuxuan Chen
Cc: farid.m.zakaria@gmail.com, git@vger.kernel.org, gitster@pobox.com,
newren@gmail.com, phillip.wood@dunelm.org.uk, ps@pks.im
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-07-13 13:18 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-10 4:13 [PATCH] sequencer: honor --empty when a fixup!/squash! empties its target Farid Zakaria
2026-07-10 13:28 ` Phillip Wood
2026-07-10 16:42 ` Farid Zakaria
2026-07-10 18:30 ` Yuxuan Chen
2026-07-13 13:18 ` phillip.wood123
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox