* [PATCH v2 1/9] read-cache: split out function to drop unmerged entries to stage 0
2026-06-03 16:13 ` [PATCH v2 0/9] builtin/history: introduce " Patrick Steinhardt
@ 2026-06-03 16:14 ` Patrick Steinhardt
2026-06-03 16:14 ` [PATCH v2 2/9] reset: drop `USE_THE_REPOSITORY_VARIABLE` Patrick Steinhardt
` (7 subsequent siblings)
8 siblings, 0 replies; 23+ messages in thread
From: Patrick Steinhardt @ 2026-06-03 16:14 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano
In `repo_read_index_unmerged()` we read the index and then drop any
unmerged entries to stage 0. In a subsequent commit we'll want to
perform this operation on arbitrary indexes, not only the one of the
given repository.
Prepare for this by splitting out the functionality into a new function
that can act on an arbitrary index.
While at it, fix a signedness mismatch when iterating through the index
cache entries.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
read-cache-ll.h | 1 +
read-cache.c | 12 +++++++-----
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/read-cache-ll.h b/read-cache-ll.h
index 2c8b4b21b1..71b87615eb 100644
--- a/read-cache-ll.h
+++ b/read-cache-ll.h
@@ -309,6 +309,7 @@ int write_locked_index(struct index_state *, struct lock_file *lock, unsigned fl
void discard_index(struct index_state *);
void move_index_extensions(struct index_state *dst, struct index_state *src);
int unmerged_index(const struct index_state *);
+int index_state_unmerged_to_stage0(struct index_state *istate);
/**
* Returns 1 if istate differs from tree, 0 otherwise. If tree is NULL,
diff --git a/read-cache.c b/read-cache.c
index 21829102ae..799a5bc719 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -3403,13 +3403,15 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,
*/
int repo_read_index_unmerged(struct repository *repo)
{
- struct index_state *istate;
- int i;
+ repo_read_index(repo);
+ return index_state_unmerged_to_stage0(repo->index);
+}
+
+int index_state_unmerged_to_stage0(struct index_state *istate)
+{
int unmerged = 0;
- repo_read_index(repo);
- istate = repo->index;
- for (i = 0; i < istate->cache_nr; i++) {
+ for (unsigned int i = 0; i < istate->cache_nr; i++) {
struct cache_entry *ce = istate->cache[i];
struct cache_entry *new_ce;
int len;
--
2.54.0.1064.gd145956f57.dirty
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 2/9] reset: drop `USE_THE_REPOSITORY_VARIABLE`
2026-06-03 16:13 ` [PATCH v2 0/9] builtin/history: introduce " Patrick Steinhardt
2026-06-03 16:14 ` [PATCH v2 1/9] read-cache: split out function to drop unmerged entries to stage 0 Patrick Steinhardt
@ 2026-06-03 16:14 ` Patrick Steinhardt
2026-06-03 16:14 ` [PATCH v2 3/9] reset: modernize flags passed to `reset_head()` Patrick Steinhardt
` (6 subsequent siblings)
8 siblings, 0 replies; 23+ messages in thread
From: Patrick Steinhardt @ 2026-06-03 16:14 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano
In "reset.c" we still have references to `the_repository`, even though
the only entry point into the file already receives a repository as
parameter.
Update all uses of `the_repository` to instead use the passed-in repo
and drop `USE_THE_REPOSITORY_VARIABLE`.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
reset.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/reset.c b/reset.c
index 46e30e6394..3b3cb74dab 100644
--- a/reset.c
+++ b/reset.c
@@ -1,5 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
-
#include "git-compat-util.h"
#include "cache-tree.h"
#include "gettext.h"
@@ -13,7 +11,8 @@
#include "unpack-trees.h"
#include "hook.h"
-static int update_refs(const struct reset_head_opts *opts,
+static int update_refs(struct repository *repo,
+ const struct reset_head_opts *opts,
const struct object_id *oid,
const struct object_id *head)
{
@@ -42,19 +41,19 @@ static int update_refs(const struct reset_head_opts *opts,
prefix_len = msg.len;
if (update_orig_head) {
- if (!repo_get_oid(the_repository, "ORIG_HEAD", &oid_old_orig))
+ if (!repo_get_oid(repo, "ORIG_HEAD", &oid_old_orig))
old_orig = &oid_old_orig;
if (head) {
if (!reflog_orig_head) {
strbuf_addstr(&msg, "updating ORIG_HEAD");
reflog_orig_head = msg.buf;
}
- refs_update_ref(get_main_ref_store(the_repository),
+ refs_update_ref(get_main_ref_store(repo),
reflog_orig_head, "ORIG_HEAD",
orig_head ? orig_head : head,
old_orig, 0, UPDATE_REFS_MSG_ON_ERR);
} else if (old_orig)
- refs_delete_ref(get_main_ref_store(the_repository),
+ refs_delete_ref(get_main_ref_store(repo),
NULL, "ORIG_HEAD", old_orig, 0);
}
@@ -64,23 +63,23 @@ static int update_refs(const struct reset_head_opts *opts,
reflog_head = msg.buf;
}
if (!switch_to_branch)
- ret = refs_update_ref(get_main_ref_store(the_repository),
+ ret = refs_update_ref(get_main_ref_store(repo),
reflog_head, "HEAD", oid, head,
detach_head ? REF_NO_DEREF : 0,
UPDATE_REFS_MSG_ON_ERR);
else {
- ret = refs_update_ref(get_main_ref_store(the_repository),
+ ret = refs_update_ref(get_main_ref_store(repo),
reflog_branch ? reflog_branch : reflog_head,
switch_to_branch, oid, NULL, 0,
UPDATE_REFS_MSG_ON_ERR);
if (!ret)
- ret = refs_update_symref(get_main_ref_store(the_repository),
+ ret = refs_update_symref(get_main_ref_store(repo),
"HEAD", switch_to_branch,
reflog_head);
}
if (!ret && run_hook)
- run_hooks_l(the_repository, "post-checkout",
- oid_to_hex(head ? head : null_oid(the_hash_algo)),
+ run_hooks_l(repo, "post-checkout",
+ oid_to_hex(head ? head : null_oid(repo->hash_algo)),
oid_to_hex(oid), "1", NULL);
strbuf_release(&msg);
return ret;
@@ -126,7 +125,7 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
oid = &head_oid;
if (refs_only)
- return update_refs(opts, oid, head);
+ return update_refs(r, opts, oid, head);
action = reset_hard ? "reset" : "checkout";
setup_unpack_trees_porcelain(&unpack_tree_opts, action);
@@ -163,7 +162,7 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
goto leave_reset_head;
}
- tree = repo_parse_tree_indirect(the_repository, oid);
+ tree = repo_parse_tree_indirect(r, oid);
if (!tree) {
ret = error(_("unable to read tree (%s)"), oid_to_hex(oid));
goto leave_reset_head;
@@ -177,7 +176,7 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
}
if (oid != &head_oid || update_orig_head || switch_to_branch)
- ret = update_refs(opts, oid, head);
+ ret = update_refs(r, opts, oid, head);
leave_reset_head:
rollback_lock_file(&lock);
--
2.54.0.1064.gd145956f57.dirty
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 3/9] reset: modernize flags passed to `reset_head()`
2026-06-03 16:13 ` [PATCH v2 0/9] builtin/history: introduce " Patrick Steinhardt
2026-06-03 16:14 ` [PATCH v2 1/9] read-cache: split out function to drop unmerged entries to stage 0 Patrick Steinhardt
2026-06-03 16:14 ` [PATCH v2 2/9] reset: drop `USE_THE_REPOSITORY_VARIABLE` Patrick Steinhardt
@ 2026-06-03 16:14 ` Patrick Steinhardt
2026-06-03 18:01 ` Kristoffer Haugsbakk
2026-06-03 16:14 ` [PATCH v2 4/9] reset: introduce dry-run mode Patrick Steinhardt
` (5 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Patrick Steinhardt @ 2026-06-03 16:14 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano
The flags passed to `reset_head()` are declared as defines. This has
fallen a bit out of practice nowadays, where we instead prefer to use
enums.
Modernize the code accordingly.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/rebase.c | 2 +-
reset.c | 4 ++--
reset.h | 30 ++++++++++++++++++------------
sequencer.c | 2 +-
4 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index fa4f5d9306..6351a3aa32 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1876,7 +1876,7 @@ int cmd_rebase(int argc,
options.reflog_action, options.onto_name);
ropts.oid = &options.onto->object.oid;
ropts.orig_head = &options.orig_head->object.oid;
- ropts.flags = RESET_HEAD_DETACH | RESET_ORIG_HEAD |
+ ropts.flags = RESET_HEAD_DETACH | RESET_HEAD_ORIG_HEAD |
RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
ropts.head_msg = msg.buf;
ropts.default_reflog_action = options.reflog_action;
diff --git a/reset.c b/reset.c
index 3b3cb74dab..9ff14f5ed1 100644
--- a/reset.c
+++ b/reset.c
@@ -18,7 +18,7 @@ static int update_refs(struct repository *repo,
{
unsigned detach_head = opts->flags & RESET_HEAD_DETACH;
unsigned run_hook = opts->flags & RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
- unsigned update_orig_head = opts->flags & RESET_ORIG_HEAD;
+ unsigned update_orig_head = opts->flags & RESET_HEAD_ORIG_HEAD;
const struct object_id *orig_head = opts->orig_head;
const char *switch_to_branch = opts->branch;
const char *reflog_branch = opts->branch_msg;
@@ -91,7 +91,7 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
const char *switch_to_branch = opts->branch;
unsigned reset_hard = opts->flags & RESET_HEAD_HARD;
unsigned refs_only = opts->flags & RESET_HEAD_REFS_ONLY;
- unsigned update_orig_head = opts->flags & RESET_ORIG_HEAD;
+ unsigned update_orig_head = opts->flags & RESET_HEAD_ORIG_HEAD;
struct object_id *head = NULL, head_oid;
struct tree_desc desc[2] = { { NULL }, { NULL } };
struct lock_file lock = LOCK_INIT;
diff --git a/reset.h b/reset.h
index a28f81829d..97ced2601e 100644
--- a/reset.h
+++ b/reset.h
@@ -6,16 +6,22 @@
#define GIT_REFLOG_ACTION_ENVIRONMENT "GIT_REFLOG_ACTION"
-/* Request a detached checkout */
-#define RESET_HEAD_DETACH (1<<0)
-/* Request a reset rather than a checkout */
-#define RESET_HEAD_HARD (1<<1)
-/* Run the post-checkout hook */
-#define RESET_HEAD_RUN_POST_CHECKOUT_HOOK (1<<2)
-/* Only update refs, do not touch the worktree */
-#define RESET_HEAD_REFS_ONLY (1<<3)
-/* Update ORIG_HEAD as well as HEAD */
-#define RESET_ORIG_HEAD (1<<4)
+enum reset_head_flags {
+ /* Request a detached checkout */
+ RESET_HEAD_DETACH = (1 << 0),
+
+ /* Request a reset rather than a checkout */
+ RESET_HEAD_HARD = (1 << 1),
+
+ /* Run the post-checkout hook */
+ RESET_HEAD_RUN_POST_CHECKOUT_HOOK = (1 << 2),
+
+ /* Only update refs, do not touch the worktree */
+ RESET_HEAD_REFS_ONLY = (1 << 3),
+
+ /* Update ORIG_HEAD as well as HEAD */
+ RESET_HEAD_ORIG_HEAD = (1 << 4),
+};
struct reset_head_opts {
/*
@@ -33,7 +39,7 @@ struct reset_head_opts {
/*
* Flags defined above.
*/
- unsigned flags;
+ enum reset_head_flags flags;
/*
* Optional reflog message for branch, defaults to head_msg.
*/
@@ -45,7 +51,7 @@ struct reset_head_opts {
const char *head_msg;
/*
* Optional reflog message for ORIG_HEAD, if this omitted and flags
- * contains RESET_ORIG_HEAD then default_reflog_action must be given.
+ * contains RESET_HEAD_ORIG_HEAD then default_reflog_action must be given.
*/
const char *orig_head_msg;
/*
diff --git a/sequencer.c b/sequencer.c
index 1ee4b2875b..0b89a977b0 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4870,7 +4870,7 @@ static int checkout_onto(struct repository *r, struct replay_opts *opts,
struct reset_head_opts ropts = {
.oid = onto,
.orig_head = orig_head,
- .flags = RESET_HEAD_DETACH | RESET_ORIG_HEAD |
+ .flags = RESET_HEAD_DETACH | RESET_HEAD_ORIG_HEAD |
RESET_HEAD_RUN_POST_CHECKOUT_HOOK,
.head_msg = reflog_message(opts, "start", "checkout %s",
onto_name),
--
2.54.0.1064.gd145956f57.dirty
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 3/9] reset: modernize flags passed to `reset_head()`
2026-06-03 16:14 ` [PATCH v2 3/9] reset: modernize flags passed to `reset_head()` Patrick Steinhardt
@ 2026-06-03 18:01 ` Kristoffer Haugsbakk
0 siblings, 0 replies; 23+ messages in thread
From: Kristoffer Haugsbakk @ 2026-06-03 18:01 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Pablo Sabater, Junio C Hamano
On Wed, Jun 3, 2026, at 18:14, Patrick Steinhardt wrote:
> The flags passed to `reset_head()` are declared as defines. This has
> fallen a bit out of practice nowadays, where we instead prefer to use
> enums.
>
> Modernize the code accordingly.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
This seems to refer to this from CodingGuidelines (quoting
for reference):
When a function `F` accepts flags, those flags should be
So this goal makes sense.
>[snip]
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 4/9] reset: introduce dry-run mode
2026-06-03 16:13 ` [PATCH v2 0/9] builtin/history: introduce " Patrick Steinhardt
` (2 preceding siblings ...)
2026-06-03 16:14 ` [PATCH v2 3/9] reset: modernize flags passed to `reset_head()` Patrick Steinhardt
@ 2026-06-03 16:14 ` Patrick Steinhardt
2026-06-03 18:18 ` Kristoffer Haugsbakk
2026-06-03 23:49 ` Junio C Hamano
2026-06-03 16:14 ` [PATCH v2 5/9] reset: introduce ability to skip reference updates Patrick Steinhardt
` (4 subsequent siblings)
8 siblings, 2 replies; 23+ messages in thread
From: Patrick Steinhardt @ 2026-06-03 16:14 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano
In a subsequent commit we'll add add another caller to `reset_head()`
that wants to perform a dry-run check of whether it would be possible to
udpate the index and working tree when moving to a new commit. Introduce
a new flag that lets the caller perform this operation.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
reset.c | 44 +++++++++++++++++++++++++++++++++-----------
reset.h | 6 ++++++
2 files changed, 39 insertions(+), 11 deletions(-)
diff --git a/reset.c b/reset.c
index 9ff14f5ed1..a8d7eea4d6 100644
--- a/reset.c
+++ b/reset.c
@@ -92,11 +92,14 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
unsigned reset_hard = opts->flags & RESET_HEAD_HARD;
unsigned refs_only = opts->flags & RESET_HEAD_REFS_ONLY;
unsigned update_orig_head = opts->flags & RESET_HEAD_ORIG_HEAD;
+ unsigned dry_run = opts->flags & RESET_HEAD_DRY_RUN;
struct object_id *head = NULL, head_oid;
struct tree_desc desc[2] = { { NULL }, { NULL } };
struct lock_file lock = LOCK_INIT;
struct unpack_trees_options unpack_tree_opts = { 0 };
struct tree *tree;
+ struct index_state scratch_index = INDEX_STATE_INIT(r);
+ struct index_state *istate;
const char *action;
int ret = 0, nr = 0;
@@ -109,7 +112,7 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
if (opts->branch_msg && !opts->branch)
BUG("branch reflog message given without a branch");
- if (!refs_only && repo_hold_locked_index(r, &lock, LOCK_REPORT_ON_ERROR) < 0) {
+ if (!refs_only && !dry_run && repo_hold_locked_index(r, &lock, LOCK_REPORT_ON_ERROR) < 0) {
ret = -1;
goto leave_reset_head;
}
@@ -124,16 +127,36 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
if (!oid)
oid = &head_oid;
- if (refs_only)
- return update_refs(r, opts, oid, head);
+ if (refs_only) {
+ if (!dry_run)
+ return update_refs(r, opts, oid, head);
+ return 0;
+ }
+
+ if (dry_run) {
+ if (read_index_from(&scratch_index, r->index_file, r->gitdir) < 0 ||
+ index_state_unmerged_to_stage0(&scratch_index) < 0) {
+ ret = error(_("could not read index"));
+ goto leave_reset_head;
+ }
+
+ istate = &scratch_index;
+ } else {
+ if (repo_read_index_unmerged(r) < 0) {
+ ret = error(_("could not read index"));
+ goto leave_reset_head;
+ }
+ istate = r->index;
+ }
action = reset_hard ? "reset" : "checkout";
setup_unpack_trees_porcelain(&unpack_tree_opts, action);
unpack_tree_opts.head_idx = 1;
- unpack_tree_opts.src_index = r->index;
- unpack_tree_opts.dst_index = r->index;
+ unpack_tree_opts.src_index = istate;
+ unpack_tree_opts.dst_index = istate;
unpack_tree_opts.fn = reset_hard ? oneway_merge : twoway_merge;
- unpack_tree_opts.update = 1;
+ unpack_tree_opts.update = !dry_run;
+ unpack_tree_opts.dry_run = dry_run;
unpack_tree_opts.merge = 1;
unpack_tree_opts.preserve_ignored = 0; /* FIXME: !overwrite_ignore */
unpack_tree_opts.skip_cache_tree_update = 1;
@@ -141,11 +164,6 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
if (reset_hard)
unpack_tree_opts.reset = UNPACK_RESET_PROTECT_UNTRACKED;
- if (repo_read_index_unmerged(r) < 0) {
- ret = error(_("could not read index"));
- goto leave_reset_head;
- }
-
if (!reset_hard && !fill_tree_descriptor(r, &desc[nr++], &head_oid)) {
ret = error(_("failed to find tree of %s"),
oid_to_hex(&head_oid));
@@ -162,6 +180,9 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
goto leave_reset_head;
}
+ if (dry_run)
+ goto leave_reset_head;
+
tree = repo_parse_tree_indirect(r, oid);
if (!tree) {
ret = error(_("unable to read tree (%s)"), oid_to_hex(oid));
@@ -181,6 +202,7 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
leave_reset_head:
rollback_lock_file(&lock);
clear_unpack_trees_porcelain(&unpack_tree_opts);
+ release_index(&scratch_index);
while (nr)
free((void *)desc[--nr].buffer);
return ret;
diff --git a/reset.h b/reset.h
index 97ced2601e..9f696382c1 100644
--- a/reset.h
+++ b/reset.h
@@ -21,6 +21,12 @@ enum reset_head_flags {
/* Update ORIG_HEAD as well as HEAD */
RESET_HEAD_ORIG_HEAD = (1 << 4),
+
+ /*
+ * Perform a dry-run by performing the operation without updating
+ * any user-visible state.
+ */
+ RESET_HEAD_DRY_RUN = (1 << 5),
};
struct reset_head_opts {
--
2.54.0.1064.gd145956f57.dirty
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 4/9] reset: introduce dry-run mode
2026-06-03 16:14 ` [PATCH v2 4/9] reset: introduce dry-run mode Patrick Steinhardt
@ 2026-06-03 18:18 ` Kristoffer Haugsbakk
2026-06-03 23:49 ` Junio C Hamano
1 sibling, 0 replies; 23+ messages in thread
From: Kristoffer Haugsbakk @ 2026-06-03 18:18 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Pablo Sabater, Junio C Hamano
On Wed, Jun 3, 2026, at 18:14, Patrick Steinhardt wrote:
> In a subsequent commit we'll add add another caller to `reset_head()`
s/add add/add/
> that wants to perform a dry-run check of whether it would be possible to
> udpate the index and working tree when moving to a new commit. Introduce
s/udpate/update/
> a new flag that lets the caller perform this operation.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>[snip]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 4/9] reset: introduce dry-run mode
2026-06-03 16:14 ` [PATCH v2 4/9] reset: introduce dry-run mode Patrick Steinhardt
2026-06-03 18:18 ` Kristoffer Haugsbakk
@ 2026-06-03 23:49 ` Junio C Hamano
1 sibling, 0 replies; 23+ messages in thread
From: Junio C Hamano @ 2026-06-03 23:49 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Pablo Sabater
Patrick Steinhardt <ps@pks.im> writes:
> In a subsequent commit we'll add add another caller to `reset_head()`
add add?
> that wants to perform a dry-run check of whether it would be possible to
> udpate the index and working tree when moving to a new commit. Introduce
udpate?
> a new flag that lets the caller perform this operation.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> reset.c | 44 +++++++++++++++++++++++++++++++++-----------
> reset.h | 6 ++++++
> 2 files changed, 39 insertions(+), 11 deletions(-)
>
> diff --git a/reset.c b/reset.c
> index 9ff14f5ed1..a8d7eea4d6 100644
> --- a/reset.c
> +++ b/reset.c
> @@ -92,11 +92,14 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
> unsigned reset_hard = opts->flags & RESET_HEAD_HARD;
> unsigned refs_only = opts->flags & RESET_HEAD_REFS_ONLY;
> unsigned update_orig_head = opts->flags & RESET_HEAD_ORIG_HEAD;
> + unsigned dry_run = opts->flags & RESET_HEAD_DRY_RUN;
> struct object_id *head = NULL, head_oid;
> struct tree_desc desc[2] = { { NULL }, { NULL } };
> struct lock_file lock = LOCK_INIT;
> struct unpack_trees_options unpack_tree_opts = { 0 };
> struct tree *tree;
> + struct index_state scratch_index = INDEX_STATE_INIT(r);
> + struct index_state *istate;
> const char *action;
> int ret = 0, nr = 0;
>
> @@ -109,7 +112,7 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
> if (opts->branch_msg && !opts->branch)
> BUG("branch reflog message given without a branch");
>
> - if (!refs_only && repo_hold_locked_index(r, &lock, LOCK_REPORT_ON_ERROR) < 0) {
> + if (!refs_only && !dry_run && repo_hold_locked_index(r, &lock, LOCK_REPORT_ON_ERROR) < 0) {
> ret = -1;
> goto leave_reset_head;
> }
> @@ -124,16 +127,36 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
> if (!oid)
> oid = &head_oid;
>
> - if (refs_only)
> - return update_refs(r, opts, oid, head);
> + if (refs_only) {
> + if (!dry_run)
> + return update_refs(r, opts, oid, head);
> + return 0;
> + }
> +
> + if (dry_run) {
> + if (read_index_from(&scratch_index, r->index_file, r->gitdir) < 0 ||
> + index_state_unmerged_to_stage0(&scratch_index) < 0) {
> + ret = error(_("could not read index"));
> + goto leave_reset_head;
> + }
> +
> + istate = &scratch_index;
> + } else {
> + if (repo_read_index_unmerged(r) < 0) {
> + ret = error(_("could not read index"));
> + goto leave_reset_head;
> + }
> + istate = r->index;
> + }
>
> action = reset_hard ? "reset" : "checkout";
> setup_unpack_trees_porcelain(&unpack_tree_opts, action);
> unpack_tree_opts.head_idx = 1;
> - unpack_tree_opts.src_index = r->index;
> - unpack_tree_opts.dst_index = r->index;
> + unpack_tree_opts.src_index = istate;
> + unpack_tree_opts.dst_index = istate;
> unpack_tree_opts.fn = reset_hard ? oneway_merge : twoway_merge;
> - unpack_tree_opts.update = 1;
> + unpack_tree_opts.update = !dry_run;
> + unpack_tree_opts.dry_run = dry_run;
> unpack_tree_opts.merge = 1;
> unpack_tree_opts.preserve_ignored = 0; /* FIXME: !overwrite_ignore */
> unpack_tree_opts.skip_cache_tree_update = 1;
> @@ -141,11 +164,6 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
> if (reset_hard)
> unpack_tree_opts.reset = UNPACK_RESET_PROTECT_UNTRACKED;
>
> - if (repo_read_index_unmerged(r) < 0) {
> - ret = error(_("could not read index"));
> - goto leave_reset_head;
> - }
> -
> if (!reset_hard && !fill_tree_descriptor(r, &desc[nr++], &head_oid)) {
> ret = error(_("failed to find tree of %s"),
> oid_to_hex(&head_oid));
> @@ -162,6 +180,9 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
> goto leave_reset_head;
> }
>
> + if (dry_run)
> + goto leave_reset_head;
> +
> tree = repo_parse_tree_indirect(r, oid);
> if (!tree) {
> ret = error(_("unable to read tree (%s)"), oid_to_hex(oid));
> @@ -181,6 +202,7 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
> leave_reset_head:
> rollback_lock_file(&lock);
> clear_unpack_trees_porcelain(&unpack_tree_opts);
> + release_index(&scratch_index);
> while (nr)
> free((void *)desc[--nr].buffer);
> return ret;
> diff --git a/reset.h b/reset.h
> index 97ced2601e..9f696382c1 100644
> --- a/reset.h
> +++ b/reset.h
> @@ -21,6 +21,12 @@ enum reset_head_flags {
>
> /* Update ORIG_HEAD as well as HEAD */
> RESET_HEAD_ORIG_HEAD = (1 << 4),
> +
> + /*
> + * Perform a dry-run by performing the operation without updating
> + * any user-visible state.
> + */
> + RESET_HEAD_DRY_RUN = (1 << 5),
> };
>
> struct reset_head_opts {
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 5/9] reset: introduce ability to skip reference updates
2026-06-03 16:13 ` [PATCH v2 0/9] builtin/history: introduce " Patrick Steinhardt
` (3 preceding siblings ...)
2026-06-03 16:14 ` [PATCH v2 4/9] reset: introduce dry-run mode Patrick Steinhardt
@ 2026-06-03 16:14 ` Patrick Steinhardt
2026-06-03 23:51 ` Junio C Hamano
2026-06-03 16:14 ` [PATCH v2 6/9] reset: allow the caller to specify the current HEAD object Patrick Steinhardt
` (3 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Patrick Steinhardt @ 2026-06-03 16:14 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano
In a subsequent commit we'll introduce a new caller to `reset_head()`
that really only wants to update the index and working tree, without
updating any references. Introduce a new flag that lets the caller
perform this operation.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
reset.c | 7 ++++++-
reset.h | 3 +++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/reset.c b/reset.c
index a8d7eea4d6..ed9df6ca5c 100644
--- a/reset.c
+++ b/reset.c
@@ -93,6 +93,7 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
unsigned refs_only = opts->flags & RESET_HEAD_REFS_ONLY;
unsigned update_orig_head = opts->flags & RESET_HEAD_ORIG_HEAD;
unsigned dry_run = opts->flags & RESET_HEAD_DRY_RUN;
+ unsigned skip_ref_updates = opts->flags & RESET_HEAD_SKIP_REF_UPDATES;
struct object_id *head = NULL, head_oid;
struct tree_desc desc[2] = { { NULL }, { NULL } };
struct lock_file lock = LOCK_INIT;
@@ -112,6 +113,9 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
if (opts->branch_msg && !opts->branch)
BUG("branch reflog message given without a branch");
+ if (skip_ref_updates && (opts->branch || refs_only))
+ BUG("asked to perform ref updates and skip them at the same time");
+
if (!refs_only && !dry_run && repo_hold_locked_index(r, &lock, LOCK_REPORT_ON_ERROR) < 0) {
ret = -1;
goto leave_reset_head;
@@ -196,7 +200,8 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
goto leave_reset_head;
}
- if (oid != &head_oid || update_orig_head || switch_to_branch)
+ if (!skip_ref_updates &&
+ (oid != &head_oid || update_orig_head || switch_to_branch))
ret = update_refs(r, opts, oid, head);
leave_reset_head:
diff --git a/reset.h b/reset.h
index 9f696382c1..cb0700ffa7 100644
--- a/reset.h
+++ b/reset.h
@@ -27,6 +27,9 @@ enum reset_head_flags {
* any user-visible state.
*/
RESET_HEAD_DRY_RUN = (1 << 5),
+
+ /* Skip updating any references, only update the worktree and index. */
+ RESET_HEAD_SKIP_REF_UPDATES = (1 << 6),
};
struct reset_head_opts {
--
2.54.0.1064.gd145956f57.dirty
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 5/9] reset: introduce ability to skip reference updates
2026-06-03 16:14 ` [PATCH v2 5/9] reset: introduce ability to skip reference updates Patrick Steinhardt
@ 2026-06-03 23:51 ` Junio C Hamano
0 siblings, 0 replies; 23+ messages in thread
From: Junio C Hamano @ 2026-06-03 23:51 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Pablo Sabater
Patrick Steinhardt <ps@pks.im> writes:
> @@ -112,6 +113,9 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
> if (opts->branch_msg && !opts->branch)
> BUG("branch reflog message given without a branch");
>
> + if (skip_ref_updates && (opts->branch || refs_only))
> + BUG("asked to perform ref updates and skip them at the same time");
;-) That's certainly a careful safety valve.
Would we also want to catch skip_ref_updates && update_orig_head
being both set as a bogus request?
> if (!refs_only && !dry_run && repo_hold_locked_index(r, &lock, LOCK_REPORT_ON_ERROR) < 0) {
> ret = -1;
> goto leave_reset_head;
> @@ -196,7 +200,8 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
> goto leave_reset_head;
> }
>
> - if (oid != &head_oid || update_orig_head || switch_to_branch)
> + if (!skip_ref_updates &&
> + (oid != &head_oid || update_orig_head || switch_to_branch))
> ret = update_refs(r, opts, oid, head);
>
> leave_reset_head:
> diff --git a/reset.h b/reset.h
> index 9f696382c1..cb0700ffa7 100644
> --- a/reset.h
> +++ b/reset.h
> @@ -27,6 +27,9 @@ enum reset_head_flags {
> * any user-visible state.
> */
> RESET_HEAD_DRY_RUN = (1 << 5),
> +
> + /* Skip updating any references, only update the worktree and index. */
> + RESET_HEAD_SKIP_REF_UPDATES = (1 << 6),
> };
>
> struct reset_head_opts {
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 6/9] reset: allow the caller to specify the current HEAD object
2026-06-03 16:13 ` [PATCH v2 0/9] builtin/history: introduce " Patrick Steinhardt
` (4 preceding siblings ...)
2026-06-03 16:14 ` [PATCH v2 5/9] reset: introduce ability to skip reference updates Patrick Steinhardt
@ 2026-06-03 16:14 ` Patrick Steinhardt
2026-06-03 16:14 ` [PATCH v2 7/9] reset: stop assuming that the caller passes in a clean index Patrick Steinhardt
` (2 subsequent siblings)
8 siblings, 0 replies; 23+ messages in thread
From: Patrick Steinhardt @ 2026-06-03 16:14 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano
When calling `reset_head()` we automatically derive the commit that the
callers wants to move from by reading the HEAD commit. Some callers may
already have resolved it, or they may want to move from a different
commit that doesn't match HEAD.
Introduce a new `oid_from` option that lets the caller specify the
commit.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
reset.c | 5 ++++-
reset.h | 5 +++++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/reset.c b/reset.c
index ed9df6ca5c..7ff72de5d2 100644
--- a/reset.c
+++ b/reset.c
@@ -121,7 +121,10 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
goto leave_reset_head;
}
- if (!repo_get_oid(r, "HEAD", &head_oid)) {
+ if (opts->oid_from) {
+ oidcpy(&head_oid, opts->oid_from);
+ head = &head_oid;
+ } else if (!repo_get_oid(r, "HEAD", &head_oid)) {
head = &head_oid;
} else if (!oid || !reset_hard) {
ret = error(_("could not determine HEAD revision"));
diff --git a/reset.h b/reset.h
index cb0700ffa7..51ce114543 100644
--- a/reset.h
+++ b/reset.h
@@ -37,6 +37,11 @@ struct reset_head_opts {
* The commit to checkout/reset to. Defaults to HEAD.
*/
const struct object_id *oid;
+ /*
+ * The commit to checkout/reset from when doing a two-way merge. This
+ * is used as one of the sides to merge.
+ */
+ const struct object_id *oid_from;
/*
* Optional value to set ORIG_HEAD. Defaults to HEAD.
*/
--
2.54.0.1064.gd145956f57.dirty
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 7/9] reset: stop assuming that the caller passes in a clean index
2026-06-03 16:13 ` [PATCH v2 0/9] builtin/history: introduce " Patrick Steinhardt
` (5 preceding siblings ...)
2026-06-03 16:14 ` [PATCH v2 6/9] reset: allow the caller to specify the current HEAD object Patrick Steinhardt
@ 2026-06-03 16:14 ` Patrick Steinhardt
2026-06-03 16:14 ` [PATCH v2 8/9] builtin/history: split handling of ref updates into two phases Patrick Steinhardt
2026-06-03 16:14 ` [PATCH v2 9/9] builtin/history: implement "drop" subcommand Patrick Steinhardt
8 siblings, 0 replies; 23+ messages in thread
From: Patrick Steinhardt @ 2026-06-03 16:14 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano
In 652bd0211d (rebase: use 'skip_cache_tree_update' option, 2022-11-10),
we updated `reset_head()` to stop updating the index tree cache. This
was done as a performance optimization: the function is only called by
"sequencer.c" and "rebase.c", both of which assume a clean index before
they perform their operation, so we know that the end result will be a
clean index, too. Consequently, we can skip recomputing the cache as we
can instead use `prime_cache_tree()` directly.
In a subsequent commit we're about to add a new caller though where the
assumption doesn't hold anymore: the index may be dirty before calling
`reset_head()`, and consequently we cannot prime the cache with a given
tree anymore as the index and tree will mismatch.
Adapt the logic so that we only skip the cache tree update in case we're
doing a hard reset. While we could introduce logic that only skips the
update in case the incoming index was dirty already, that doesn't really
feel worth it: after all, the mentioned commit says itself that the
performance improvement was negligible anyway.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
reset.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/reset.c b/reset.c
index 7ff72de5d2..05eb80216c 100644
--- a/reset.c
+++ b/reset.c
@@ -166,10 +166,11 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
unpack_tree_opts.dry_run = dry_run;
unpack_tree_opts.merge = 1;
unpack_tree_opts.preserve_ignored = 0; /* FIXME: !overwrite_ignore */
- unpack_tree_opts.skip_cache_tree_update = 1;
init_checkout_metadata(&unpack_tree_opts.meta, switch_to_branch, oid, NULL);
- if (reset_hard)
+ if (reset_hard) {
+ unpack_tree_opts.skip_cache_tree_update = 1;
unpack_tree_opts.reset = UNPACK_RESET_PROTECT_UNTRACKED;
+ }
if (!reset_hard && !fill_tree_descriptor(r, &desc[nr++], &head_oid)) {
ret = error(_("failed to find tree of %s"),
@@ -196,7 +197,8 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
goto leave_reset_head;
}
- prime_cache_tree(r, r->index, tree);
+ if (reset_hard)
+ prime_cache_tree(r, r->index, tree);
if (write_locked_index(r->index, &lock, COMMIT_LOCK) < 0) {
ret = error(_("could not write index"));
--
2.54.0.1064.gd145956f57.dirty
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 8/9] builtin/history: split handling of ref updates into two phases
2026-06-03 16:13 ` [PATCH v2 0/9] builtin/history: introduce " Patrick Steinhardt
` (6 preceding siblings ...)
2026-06-03 16:14 ` [PATCH v2 7/9] reset: stop assuming that the caller passes in a clean index Patrick Steinhardt
@ 2026-06-03 16:14 ` Patrick Steinhardt
2026-06-03 16:14 ` [PATCH v2 9/9] builtin/history: implement "drop" subcommand Patrick Steinhardt
8 siblings, 0 replies; 23+ messages in thread
From: Patrick Steinhardt @ 2026-06-03 16:14 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano
The function `handle_reference_updates()` is used by git-history(1) to
update all references that refer to commits that have been rewritten. As
such, it performs two steps:
- It gathers the references that need to be updated in the first
place.
- It prepares and commits the reference transaction.
In a subsequent commit we'll want to handle those two steps separately.
Prepare for this by splitting up the function into two.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/history.c | 102 ++++++++++++++++++++++++++++++++++--------------------
1 file changed, 64 insertions(+), 38 deletions(-)
diff --git a/builtin/history.c b/builtin/history.c
index 0fc06fb204..4fadf38c32 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -333,21 +333,17 @@ static int handle_ref_update(struct ref_transaction *transaction,
NULL, NULL, 0, reflog_msg, err);
}
-static int handle_reference_updates(struct rev_info *revs,
- enum ref_action action,
- struct commit *original,
- struct commit *rewritten,
- const char *reflog_msg,
- int dry_run,
- enum replay_empty_commit_action empty)
+static int compute_pending_ref_updates(struct rev_info *revs,
+ enum ref_action action,
+ struct commit *original,
+ struct commit *rewritten,
+ enum replay_empty_commit_action empty,
+ struct replay_result *result)
{
const struct name_decoration *decoration;
struct replay_revisions_options opts = {
.empty = empty,
};
- struct replay_result result = { 0 };
- struct ref_transaction *transaction = NULL;
- struct strbuf err = STRBUF_INIT;
char hex[GIT_MAX_HEXSZ + 1];
bool detached_head;
int head_flags = 0;
@@ -359,34 +355,13 @@ static int handle_reference_updates(struct rev_info *revs,
opts.onto = oid_to_hex_r(hex, &rewritten->object.oid);
- ret = replay_revisions(revs, &opts, &result);
+ ret = replay_revisions(revs, &opts, result);
if (ret)
- goto out;
+ return ret;
if (action != REF_ACTION_BRANCHES && action != REF_ACTION_HEAD)
BUG("unsupported ref action %d", action);
- if (!dry_run) {
- transaction = ref_store_transaction_begin(get_main_ref_store(revs->repo), 0, &err);
- if (!transaction) {
- ret = error(_("failed to begin ref transaction: %s"), err.buf);
- goto out;
- }
- }
-
- for (size_t i = 0; i < result.updates_nr; i++) {
- ret = handle_ref_update(transaction,
- result.updates[i].refname,
- &result.updates[i].new_oid,
- &result.updates[i].old_oid,
- reflog_msg, &err);
- if (ret) {
- ret = error(_("failed to update ref '%s': %s"),
- result.updates[i].refname, err.buf);
- goto out;
- }
- }
-
/*
* `replay_revisions()` only updates references that are
* ancestors of `rewritten`, so we need to manually
@@ -414,14 +389,43 @@ static int handle_reference_updates(struct rev_info *revs,
!detached_head)
continue;
+ ALLOC_GROW(result->updates, result->updates_nr + 1, result->updates_alloc);
+ result->updates[result->updates_nr].refname = xstrdup(decoration->name);
+ result->updates[result->updates_nr].old_oid = original->object.oid;
+ result->updates[result->updates_nr].new_oid = rewritten->object.oid;
+ result->updates_nr++;
+ }
+
+ return 0;
+}
+
+static int apply_pending_ref_updates(struct repository *repo,
+ const struct replay_result *result,
+ const char *reflog_msg,
+ int dry_run)
+{
+ struct ref_transaction *transaction = NULL;
+ struct strbuf err = STRBUF_INIT;
+ int ret;
+
+ if (!dry_run) {
+ transaction = ref_store_transaction_begin(get_main_ref_store(repo),
+ 0, &err);
+ if (!transaction) {
+ ret = error(_("failed to begin ref transaction: %s"), err.buf);
+ goto out;
+ }
+ }
+
+ for (size_t i = 0; i < result->updates_nr; i++) {
ret = handle_ref_update(transaction,
- decoration->name,
- &rewritten->object.oid,
- &original->object.oid,
+ result->updates[i].refname,
+ &result->updates[i].new_oid,
+ &result->updates[i].old_oid,
reflog_msg, &err);
if (ret) {
ret = error(_("failed to update ref '%s': %s"),
- decoration->name, err.buf);
+ result->updates[i].refname, err.buf);
goto out;
}
}
@@ -435,11 +439,33 @@ static int handle_reference_updates(struct rev_info *revs,
out:
ref_transaction_free(transaction);
- replay_result_release(&result);
strbuf_release(&err);
return ret;
}
+static int handle_reference_updates(struct rev_info *revs,
+ enum ref_action action,
+ struct commit *original,
+ struct commit *rewritten,
+ const char *reflog_msg,
+ int dry_run,
+ enum replay_empty_commit_action empty)
+{
+ struct replay_result result = { 0 };
+ int ret;
+
+ ret = compute_pending_ref_updates(revs, action, original, rewritten,
+ empty, &result);
+ if (ret)
+ goto out;
+
+ ret = apply_pending_ref_updates(revs->repo, &result, reflog_msg, dry_run);
+
+out:
+ replay_result_release(&result);
+ return ret;
+}
+
static int commit_became_empty(struct repository *repo,
struct commit *original,
struct tree *result)
--
2.54.0.1064.gd145956f57.dirty
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 9/9] builtin/history: implement "drop" subcommand
2026-06-03 16:13 ` [PATCH v2 0/9] builtin/history: introduce " Patrick Steinhardt
` (7 preceding siblings ...)
2026-06-03 16:14 ` [PATCH v2 8/9] builtin/history: split handling of ref updates into two phases Patrick Steinhardt
@ 2026-06-03 16:14 ` Patrick Steinhardt
2026-06-03 19:04 ` Kristoffer Haugsbakk
2026-06-03 23:58 ` Junio C Hamano
8 siblings, 2 replies; 23+ messages in thread
From: Patrick Steinhardt @ 2026-06-03 16:14 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano
A common operation when editing the commit history is to drop a specific
commit from the history entirely, but this operation is not currently
covered by git-history(1).
A couple of noteworthy bits:
- This is the first git-history(1) command that will ultimately result
in changes to both the index and the working tree. We thus have to
add logic to merge resulting changes into those.
- It is still not possible to replay merge commits, so this limitation
is inherited for the new "drop" command.
- For now we refuse to drop root commits. While we _can_ indeed drop
root commits in the general case, there are edge cases where the
resulting history would become completely empty. This is thus left
to a subsequent patch series.
Other than that, most of the logic is rather straight-forward as we can
continue to build on the preexisting logic in git-history(1) for most of
the part.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/git-history.adoc | 38 ++-
builtin/history.c | 187 +++++++++++++++
t/meson.build | 1 +
t/t3454-history-drop.sh | 513 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 738 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc
index 2ba8121795..4eac732fd2 100644
--- a/Documentation/git-history.adoc
+++ b/Documentation/git-history.adoc
@@ -8,6 +8,7 @@ git-history - EXPERIMENTAL: Rewrite history
SYNOPSIS
--------
[synopsis]
+git history drop <commit> [--dry-run] [--update-refs=(branches|head)] [--empty=(drop|keep|abort)]
git history fixup <commit> [--dry-run] [--update-refs=(branches|head)] [--reedit-message] [--empty=(drop|keep|abort)]
git history reword <commit> [--dry-run] [--update-refs=(branches|head)]
git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--] [<pathspec>...]
@@ -51,13 +52,28 @@ be stateful operations. The limitation can be lifted once (if) Git learns about
first-class conflicts.
When using `fixup` with `--empty=drop`, dropping the root commit is not yet
-supported.
+supported. Likewise, `drop` cannot remove the root commit or a merge commit.
COMMANDS
--------
The following commands are available to rewrite history in different ways:
+`drop <commit>`::
+ Remove the specified commit from the history. All descendants of the
+ commit are replayed directly onto its parent.
++
+The root commit cannot be dropped as that may lead to edge cases where refs
+end up with no commits anymore. Merge commits cannot be dropped either; see
+LIMITATIONS.
++
+If `HEAD` points at a commit that is to be rewritten, the index and working
+tree are updated to match the new `HEAD`. The command aborts before any
+references are updated in case local modifications would be overwritten.
++
+If replaying any descendant would result in a conflict, the command aborts
+with an error.
+
`fixup <commit>`::
Apply the currently staged changes to the specified commit. This is
similar in nature to `git commit --fixup=<commit>` followed by `git
@@ -170,6 +186,26 @@ The staged addition of `unrelated.txt` has been incorporated into the `first`
commit. All descendant commits have been replayed on top of the rewritten
history.
+Drop a commit
+~~~~~~~~~~~~~
+
+----------
+$ git log --oneline
+abc1234 (HEAD -> main) third
+def5678 second
+ghi9012 first
+
+$ git history drop def5678
+
+$ git log --oneline
+jkl3456 (HEAD -> main) third
+ghi9012 first
+----------
+
+The `second` commit has been removed from the history, and `third` has been
+replayed directly on top of `first`. All branches that pointed at the dropped
+commit have been moved to its parent.
+
Split a commit
~~~~~~~~~~~~~~
diff --git a/builtin/history.c b/builtin/history.c
index 4fadf38c32..fa4f5e24ad 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -17,13 +17,17 @@
#include "read-cache.h"
#include "refs.h"
#include "replay.h"
+#include "reset.h"
#include "revision.h"
#include "sequencer.h"
#include "strvec.h"
#include "tree.h"
+#include "tree-walk.h"
#include "unpack-trees.h"
#include "wt-status.h"
+#define GIT_HISTORY_DROP_USAGE \
+ N_("git history drop <commit> [--dry-run] [--update-refs=(branches|head)] [--empty=(drop|keep|abort)]")
#define GIT_HISTORY_FIXUP_USAGE \
N_("git history fixup <commit> [--dry-run] [--update-refs=(branches|head)] [--reedit-message] [--empty=(drop|keep|abort)]")
#define GIT_HISTORY_REWORD_USAGE \
@@ -1001,12 +1005,194 @@ static int cmd_history_split(int argc,
return ret;
}
+static int update_worktree(struct repository *repo,
+ const struct commit *old_head,
+ const struct commit *new_head,
+ bool dry_run)
+{
+ struct reset_head_opts opts = {
+ .oid_from = &old_head->object.oid,
+ .oid = &new_head->object.oid,
+ .flags = RESET_HEAD_SKIP_REF_UPDATES,
+ };
+ if (dry_run)
+ opts.flags |= RESET_HEAD_DRY_RUN;
+ return reset_head(repo, &opts);
+}
+
+static int find_head_tree_change(struct repository *repo,
+ const struct replay_result *result,
+ struct commit **old_head,
+ struct commit **new_head,
+ bool *changed)
+{
+ const struct replay_ref_update *head_update = NULL;
+ struct commit *old_head_commit, *new_head_commit;
+ struct tree *old_head_tree, *new_head_tree;
+ const char *head_target;
+ int head_flags;
+
+ *changed = false;
+
+ head_target = refs_resolve_ref_unsafe(get_main_ref_store(repo),
+ "HEAD", RESOLVE_REF_NO_RECURSE,
+ NULL, &head_flags);
+ if (!head_target)
+ return error(_("cannot look up HEAD"));
+ if (!(head_flags & REF_ISSYMREF))
+ head_target = "HEAD";
+
+ for (size_t i = 0; i < result->updates_nr; i++) {
+ if (!strcmp(result->updates[i].refname, head_target)) {
+ head_update = &result->updates[i];
+ break;
+ }
+ }
+
+ if (!head_update)
+ return 0;
+
+ old_head_commit = lookup_commit_reference(repo, &head_update->old_oid);
+ new_head_commit = lookup_commit_reference(repo, &head_update->new_oid);
+ if (!old_head_commit || !new_head_commit)
+ return error(_("cannot resolve HEAD commit"));
+
+ old_head_tree = repo_get_commit_tree(repo, old_head_commit);
+ new_head_tree = repo_get_commit_tree(repo, new_head_commit);
+ if (!old_head_tree || !new_head_tree)
+ return error(_("cannot resolve tree for HEAD"));
+
+ if (oideq(&old_head_tree->object.oid, &new_head_tree->object.oid))
+ return 0;
+
+ *old_head = old_head_commit;
+ *new_head = new_head_commit;
+ *changed = true;
+
+ return 0;
+}
+
+static int cmd_history_drop(int argc,
+ const char **argv,
+ const char *prefix,
+ struct repository *repo)
+{
+ const char * const usage[] = {
+ GIT_HISTORY_DROP_USAGE,
+ NULL,
+ };
+ enum replay_empty_commit_action empty = REPLAY_EMPTY_COMMIT_DROP;
+ enum ref_action action = REF_ACTION_DEFAULT;
+ int dry_run = 0;
+ struct option options[] = {
+ OPT_CALLBACK_F(0, "update-refs", &action, "(branches|head)",
+ N_("control which refs should be updated"),
+ PARSE_OPT_NONEG, parse_ref_action),
+ OPT_BOOL('n', "dry-run", &dry_run,
+ N_("perform a dry-run without updating any refs")),
+ OPT_CALLBACK_F(0, "empty", &empty, "(drop|keep|abort)",
+ N_("how to handle descendants that become empty"),
+ PARSE_OPT_NONEG, parse_opt_empty),
+ OPT_END(),
+ };
+ struct strbuf reflog_msg = STRBUF_INIT;
+ struct commit *original, *rewritten;
+ struct rev_info revs = { 0 };
+ struct replay_result result = { 0 };
+ struct commit *old_head, *new_head;
+ bool head_moves = false;
+ int ret;
+
+ argc = parse_options(argc, argv, prefix, options, usage, 0);
+ if (argc != 1) {
+ ret = error(_("command expects a single revision"));
+ goto out;
+ }
+ repo_config(repo, git_default_config, NULL);
+
+ if (action == REF_ACTION_DEFAULT)
+ action = REF_ACTION_BRANCHES;
+
+ original = lookup_commit_reference_by_name(argv[0]);
+ if (!original) {
+ ret = error(_("commit cannot be found: %s"), argv[0]);
+ goto out;
+ }
+
+ if (!original->parents) {
+ ret = error(_("cannot drop root commit %s: "
+ "it has no parent to replay onto"),
+ argv[0]);
+ goto out;
+ } else if (original->parents->next) {
+ ret = error(_("cannot drop merge commit: %s"), argv[0]);
+ goto out;
+ }
+
+ ret = setup_revwalk(repo, action, original, &revs);
+ if (ret)
+ goto out;
+
+ rewritten = original->parents->item;
+
+ ret = compute_pending_ref_updates(&revs, action, original, rewritten,
+ empty, &result);
+ if (ret) {
+ ret = error(_("failed replaying descendants"));
+ goto out;
+ }
+
+ /*
+ * If HEAD will move as a result of the rewrite then we'll have to
+ * merge in the changes into the worktree and index. This merge can of
+ * course conflict, which will cause the whole operation to abort.
+ *
+ * If we had already updated the refs at that point then we'd have an
+ * inconsistent repository state. So we first perform a dry-run merge
+ * here before updating refs.
+ */
+ if (!dry_run && !is_bare_repository()) {
+ ret = find_head_tree_change(repo, &result, &old_head,
+ &new_head, &head_moves);
+ if (ret < 0)
+ goto out;
+
+ if (head_moves && update_worktree(repo, old_head, new_head, true) < 0) {
+ ret = error(_("dropping this commit would "
+ "overwrite local changes; aborting"));
+ goto out;
+ }
+ }
+
+ strbuf_addf(&reflog_msg, "drop: dropping %s", argv[0]);
+ ret = apply_pending_ref_updates(repo, &result, reflog_msg.buf, dry_run);
+ if (ret < 0) {
+ ret = error(_("failed to update references"));
+ goto out;
+ }
+
+ if (head_moves && update_worktree(repo, old_head, new_head, false) < 0) {
+ ret = error(_("could not update working tree to new commit %s"),
+ oid_to_hex(&new_head->object.oid));
+ goto out;
+ }
+
+ ret = 0;
+
+out:
+ replay_result_release(&result);
+ strbuf_release(&reflog_msg);
+ release_revisions(&revs);
+ return ret;
+}
+
int cmd_history(int argc,
const char **argv,
const char *prefix,
struct repository *repo)
{
const char * const usage[] = {
+ GIT_HISTORY_DROP_USAGE,
GIT_HISTORY_FIXUP_USAGE,
GIT_HISTORY_REWORD_USAGE,
GIT_HISTORY_SPLIT_USAGE,
@@ -1014,6 +1200,7 @@ int cmd_history(int argc,
};
parse_opt_subcommand_fn *fn = NULL;
struct option options[] = {
+ OPT_SUBCOMMAND("drop", &fn, cmd_history_drop),
OPT_SUBCOMMAND("fixup", &fn, cmd_history_fixup),
OPT_SUBCOMMAND("reword", &fn, cmd_history_reword),
OPT_SUBCOMMAND("split", &fn, cmd_history_split),
diff --git a/t/meson.build b/t/meson.build
index 2af8d01279..d5e71056b2 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -399,6 +399,7 @@ integration_tests = [
't3451-history-reword.sh',
't3452-history-split.sh',
't3453-history-fixup.sh',
+ 't3454-history-drop.sh',
't3500-cherry.sh',
't3501-revert-cherry-pick.sh',
't3502-cherry-pick-merge.sh',
diff --git a/t/t3454-history-drop.sh b/t/t3454-history-drop.sh
new file mode 100755
index 0000000000..37d8413e7e
--- /dev/null
+++ b/t/t3454-history-drop.sh
@@ -0,0 +1,513 @@
+#!/bin/sh
+
+test_description='tests for git-history drop subcommand'
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-log-graph.sh"
+
+expect_graph () {
+ cat >expect &&
+ lib_test_cmp_graph --format=%s "$@"
+}
+
+expect_log () {
+ git log --format="%s" "$@" >actual &&
+ cat >expect &&
+ test_cmp expect actual
+}
+
+test_expect_success 'errors on missing commit argument' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit initial &&
+ test_must_fail git history drop 2>err &&
+ test_grep "command expects a single revision" err
+ )
+'
+
+test_expect_success 'errors on too many arguments' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit initial &&
+ test_must_fail git history drop HEAD HEAD 2>err &&
+ test_grep "command expects a single revision" err
+ )
+'
+
+test_expect_success 'errors on unknown revision' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit initial &&
+ test_must_fail git history drop does-not-exist 2>err &&
+ test_grep "commit cannot be found: does-not-exist" err
+ )
+'
+
+test_expect_success 'errors with invalid --empty= value' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ test_commit -C repo initial &&
+ test_commit -C repo second &&
+ test_must_fail git -C repo history drop --empty=bogus HEAD 2>err &&
+ test_grep "unrecognized.*--empty.*bogus" err
+'
+
+test_expect_success 'drops a commit in the middle and replays descendants' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit first &&
+ test_commit second &&
+ test_commit third &&
+
+ git symbolic-ref HEAD >expect &&
+ git history drop HEAD~ &&
+ git symbolic-ref HEAD >actual &&
+ test_cmp expect actual &&
+
+ expect_log <<-\EOF &&
+ third
+ first
+ EOF
+
+ test_must_fail git show HEAD:second.t &&
+ test_path_is_missing second.t &&
+
+ git reflog >reflog &&
+ test_grep "drop: dropping HEAD~" reflog
+ )
+'
+
+test_expect_success 'drops the HEAD commit' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit first &&
+ test_commit second &&
+
+ git history drop HEAD &&
+
+ expect_log <<-\EOF
+ first
+ EOF
+ )
+'
+
+test_expect_success 'drops a commit on detached HEAD' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit first &&
+ test_commit second &&
+ test_commit third &&
+ git checkout --detach HEAD &&
+
+ git history drop HEAD~ &&
+
+ expect_log <<-\EOF
+ third
+ first
+ EOF
+ )
+'
+
+# Note: in this case it would actually be fine to drop the root commit, as we
+# do have a descendant commit, and no reference points to the root commit
+# directly. So this is something that we may relax eventually.
+test_expect_success 'refuses to drop the root commit' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit first &&
+ test_commit second &&
+
+ test_must_fail git history drop HEAD~ 2>err &&
+ test_grep "cannot drop root commit" err
+ )
+'
+
+# In contrast to the above case, we actually don't want to drop the root commit
+# here as that would cause us to end up with an empty commit graph.
+test_expect_success 'refuses to drop the root commit when branch becomes empty' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit first &&
+
+ test_must_fail git history drop HEAD 2>err &&
+ test_grep "cannot drop root commit" err
+ )
+'
+
+test_expect_success 'refuses to drop a merge commit' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit base &&
+ git branch branch &&
+ test_commit ours &&
+ git switch branch &&
+ test_commit theirs &&
+ git switch - &&
+ git merge theirs &&
+
+ test_must_fail git history drop HEAD 2>err &&
+ test_grep "cannot drop merge commit" err
+ )
+'
+
+test_expect_success 'refuses when descendants contain a merge commit' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit base &&
+ test_commit middle &&
+ git branch branch &&
+ test_commit ours &&
+ git switch branch &&
+ test_commit theirs &&
+ git switch - &&
+ git merge theirs &&
+
+ test_must_fail git history drop middle 2>err &&
+ test_grep "replaying merge commits is not supported yet" err
+ )
+'
+
+test_expect_success 'works in a bare repository' '
+ test_when_finished "rm -rf repo repo.git" &&
+
+ git init repo &&
+ test_commit -C repo first &&
+ test_commit -C repo second &&
+ test_commit -C repo third &&
+
+ git clone --bare repo repo.git &&
+ (
+ cd repo.git &&
+
+ git history drop HEAD~ &&
+ expect_log <<-\EOF
+ third
+ first
+ EOF
+ )
+'
+
+test_expect_success 'updates branches on other lines of descent' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit base &&
+ test_commit target &&
+ git branch theirs &&
+ test_commit ours &&
+ git switch theirs &&
+ test_commit theirs &&
+
+ expect_graph --branches <<-\EOF &&
+ * theirs
+ | * ours
+ |/
+ * target
+ * base
+ EOF
+
+ git history drop target &&
+
+ expect_graph --branches <<-\EOF
+ * ours
+ | * theirs
+ |/
+ * base
+ EOF
+ )
+'
+
+test_expect_success 'moves branch pointing at dropped commit to its parent' '
+ test_when_finished "rm -rf repo" &&
+ git init repo --initial-branch=main &&
+ (
+ cd repo &&
+ test_commit first &&
+ test_commit second &&
+ git branch points-at-second &&
+ test_commit third &&
+
+ git rev-parse first >expect &&
+ git history drop second &&
+ git rev-parse points-at-second >actual &&
+ test_cmp expect actual &&
+
+ expect_log --format="%s %D" --branches <<-\EOF
+ third HEAD -> main
+ first tag: first, points-at-second
+ EOF
+ )
+'
+
+test_expect_success '--dry-run prints ref updates without modifying repo' '
+ test_when_finished "rm -rf repo" &&
+ git init repo --initial-branch=main &&
+ (
+ cd repo &&
+ test_commit base &&
+ git branch branch &&
+ test_commit middle &&
+ test_commit ours &&
+ git switch branch &&
+ test_commit theirs &&
+
+ git refs list >refs-expect &&
+ git history drop --dry-run main~ >updates &&
+ git refs list >refs-actual &&
+ test_cmp refs-expect refs-actual &&
+ test_grep "update refs/heads/main" updates &&
+
+ git update-ref --stdin <updates &&
+ expect_log main <<-\EOF
+ ours
+ base
+ EOF
+ )
+'
+
+test_expect_success '--update-refs=head updates only HEAD' '
+ test_when_finished "rm -rf repo" &&
+ git init repo --initial-branch=main &&
+ (
+ cd repo &&
+ test_commit base &&
+ test_commit target &&
+ git branch theirs &&
+ test_commit ours &&
+ git switch theirs &&
+ test_commit theirs &&
+
+ # When told to update HEAD only, the command refuses to
+ # rewrite commits that are not an ancestor of HEAD.
+ test_must_fail git history drop --update-refs=head main 2>err &&
+ test_grep "rewritten commit must be an ancestor of HEAD" err &&
+
+ expect_graph --branches <<-\EOF &&
+ * theirs
+ | * ours
+ |/
+ * target
+ * base
+ EOF
+
+ git switch main &&
+ git history drop --update-refs=head target &&
+
+ expect_graph --branches <<-\EOF
+ * ours
+ | * theirs
+ | * target
+ |/
+ * base
+ EOF
+ )
+'
+
+test_expect_success 'conflict with replayed commit aborts cleanly' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit base &&
+ test_commit conflict-a file &&
+ test_commit conflict-b file &&
+
+ git refs list >refs-expect &&
+ test_must_fail git history drop HEAD~ 2>err &&
+ test_grep "failed replaying descendants" err &&
+ git refs list >refs-actual &&
+ test_cmp refs-expect refs-actual
+ )
+'
+
+# Build a history where a descendant of the drop target reverts the change
+# introduced by the drop target. After dropping, the descendant's diff applies
+# against a tree that already lacks the change, so it becomes empty.
+setup_empty_descendant_repo () {
+ git init "$1" &&
+ (
+ cd "$1" &&
+ echo C1 >file &&
+ git add file &&
+ git commit -m "base" &&
+ git tag base &&
+ echo C2 >file &&
+ git add file &&
+ git commit -m "drop-me" &&
+ git tag drop-me &&
+ test_commit middle &&
+ echo C1 >file &&
+ git add file &&
+ git commit -m "revert-drop-me" &&
+ git tag revert-drop-me
+ )
+}
+
+test_expect_success '--empty=drop drops descendants that become empty' '
+ test_when_finished "rm -rf repo" &&
+ setup_empty_descendant_repo repo &&
+ (
+ cd repo &&
+
+ git history drop --empty=drop drop-me &&
+
+ expect_log <<-\EOF
+ middle
+ base
+ EOF
+ )
+'
+
+test_expect_success '--empty=keep keeps descendants that become empty' '
+ test_when_finished "rm -rf repo" &&
+ setup_empty_descendant_repo repo &&
+ (
+ cd repo &&
+
+ git history drop --empty=keep drop-me &&
+
+ expect_log <<-\EOF &&
+ revert-drop-me
+ middle
+ base
+ EOF
+ git diff HEAD~ HEAD >diff &&
+ test_must_be_empty diff
+ )
+'
+
+test_expect_success '--empty=abort errors out when a descendant becomes empty' '
+ test_when_finished "rm -rf repo" &&
+ setup_empty_descendant_repo repo &&
+ (
+ cd repo &&
+
+ test_must_fail git history drop --empty=abort drop-me 2>err &&
+ test_grep "became empty after replay" err
+ )
+'
+
+test_expect_success 'updates index and worktree when HEAD moves' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit first &&
+ test_commit second &&
+ test_commit third &&
+
+ git history drop second &&
+
+ # Worktree should no longer contain second.t.
+ test_path_is_missing second.t &&
+ test_path_is_file first.t &&
+ test_path_is_file third.t &&
+
+ # Index and worktree should both match the new HEAD.
+ git status --porcelain --untracked-files=no >status &&
+ test_must_be_empty status
+ )
+'
+
+test_expect_success 'updates worktree when dropping HEAD itself' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit first &&
+ test_commit second &&
+
+ git history drop HEAD &&
+
+ test_path_is_missing second.t &&
+ test_path_is_file first.t &&
+
+ git status --porcelain --untracked-files=no >status &&
+ test_must_be_empty status
+ )
+'
+
+test_expect_success 'preserves unrelated unstaged modifications' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit first &&
+ echo first-content >unrelated.txt &&
+ git add unrelated.txt &&
+ git commit -m "add unrelated" &&
+ test_commit second &&
+ test_commit third &&
+
+ echo locally-modified >unrelated.txt &&
+
+ git diff >diff-expect &&
+ git history drop second &&
+ git diff >diff-actual &&
+ test_cmp diff-expect diff-actual &&
+ test_path_is_missing second.t
+ )
+'
+
+test_expect_success 'preserves unrelated staged changes' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit first &&
+ echo first-content >unrelated.txt &&
+ git add unrelated.txt &&
+ git commit -m "add unrelated" &&
+ test_commit second &&
+ test_commit third &&
+
+ echo staged-change >unrelated.txt &&
+ git add unrelated.txt &&
+
+ git diff --cached >diff-expect &&
+ git history drop second &&
+ git diff --cached >diff-actual &&
+ test_cmp diff-expect diff-actual &&
+ test_path_is_missing second.t
+ )
+'
+
+test_expect_success 'aborts when local modifications would be overwritten' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit base &&
+ test_commit conflict &&
+
+ echo local-edit >conflict.t &&
+ git diff >diff-expect &&
+ test_must_fail git history drop HEAD 2>err &&
+ test_grep "would overwrite local changes" err &&
+ git diff >diff-actual &&
+ test_cmp diff-expect diff-actual
+ )
+'
+
+test_done
--
2.54.0.1064.gd145956f57.dirty
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 9/9] builtin/history: implement "drop" subcommand
2026-06-03 16:14 ` [PATCH v2 9/9] builtin/history: implement "drop" subcommand Patrick Steinhardt
@ 2026-06-03 19:04 ` Kristoffer Haugsbakk
2026-06-03 23:58 ` Junio C Hamano
1 sibling, 0 replies; 23+ messages in thread
From: Kristoffer Haugsbakk @ 2026-06-03 19:04 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Pablo Sabater, Junio C Hamano
On Wed, Jun 3, 2026, at 18:14, Patrick Steinhardt wrote:
>[snip]
> ---
> Documentation/git-history.adoc | 38 ++-
> builtin/history.c | 187 +++++++++++++++
> t/meson.build | 1 +
> t/t3454-history-drop.sh | 513 +++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 738 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-history.adoc
> b/Documentation/git-history.adoc
> index 2ba8121795..4eac732fd2 100644
> --- a/Documentation/git-history.adoc
> +++ b/Documentation/git-history.adoc
> @@ -8,6 +8,7 @@ git-history - EXPERIMENTAL: Rewrite history
> SYNOPSIS
> --------
> [synopsis]
> +git history drop <commit> [--dry-run] [--update-refs=(branches|head)]
> [--empty=(drop|keep|abort)]
> git history fixup <commit> [--dry-run] [--update-refs=(branches|head)]
> [--reedit-message] [--empty=(drop|keep|abort)]
> git history reword <commit> [--dry-run] [--update-refs=(branches|head)]
> git history split <commit> [--dry-run] [--update-refs=(branches|head)]
> [--] [<pathspec>...]
> @@ -51,13 +52,28 @@ be stateful operations. The limitation can be
> lifted once (if) Git learns about
> first-class conflicts.
>
> When using `fixup` with `--empty=drop`, dropping the root commit is not yet
> -supported.
> +supported. Likewise, `drop` cannot remove the root commit or a merge commit.
>
> COMMANDS
> --------
>
> The following commands are available to rewrite history in different ways:
>
> +`drop <commit>`::
> + Remove the specified commit from the history. All descendants of the
> + commit are replayed directly onto its parent.
> ++
> +The root commit cannot be dropped as that may lead to edge cases where refs
> +end up with no commits anymore. Merge commits cannot be dropped either; see
> +LIMITATIONS.
Should section names be “bare” or quoted like "LIMITATIONS"?
I don’t know.
Maybe add “above” since it’s a previous section.
> ++
> +If `HEAD` points at a commit that is to be rewritten, the index and working
>[snip]
> +Drop a commit
> +~~~~~~~~~~~~~
> +
> +----------
> +$ git log --oneline
> +abc1234 (HEAD -> main) third
> +def5678 second
> +ghi9012 first
> +
> +$ git history drop def5678
I know this is only the most simple example. And I might be dragging in
something beyond the scope of this example. But I recall one
demonstration on the first git-history(1) series which used a lot of
revision expressions and someone saying that they couldn’t imagine a
workflow where this would be more interactive than bringing up the
git-rebase(1) todo editor.
(I couldn’t find back to this right now.)
Although it is slower in terms of machine cycles, the keyboard instinct
for dropping a nearby commit might be to do `git rebase -i @~10`
(sufficiently high number) and navigating quickly in the configured
editor, deleting the line or using the keybind for `drop`. This example
which by implication brings up the log in order to paste the abbreviated
hash isn’t as ergonomic in comparison.
But using a revision expression like searching the subject with
`main^{/second}`, while not quicker probably, does distinguish itself
from git-rebase(1) by being a pretty fast ad hoc invocation that can be
done in one command without futzing with some weird sed(1) editor in
order to navigate to the `second` line and deleting it, or
something. And that’s a small win in isolation, but it segues much more
naturally into letting you script, say, dropping the last commit that
starts with the subject `TEMP`.
Or maybe revision expressions is too much in this context?
> +
> +$ git log --oneline
>[snip]
> diff --git a/t/t3454-history-drop.sh b/t/t3454-history-drop.sh
> new file mode 100755
> index 0000000000..37d8413e7e
> --- /dev/null
> +++ b/t/t3454-history-drop.sh
> @@ -0,0 +1,513 @@
> +#!/bin/sh
> +
> +test_description='tests for git-history drop subcommand'
> +
> +. ./test-lib.sh
> +. "$TEST_DIRECTORY/lib-log-graph.sh"
> +
> +expect_graph () {
> + cat >expect &&
> + lib_test_cmp_graph --format=%s "$@"
> +}
> +
> +expect_log () {
> + git log --format="%s" "$@" >actual &&
> + cat >expect &&
> + test_cmp expect actual
> +}
> +
> +test_expect_success 'errors on missing commit argument' '
> + test_when_finished "rm -rf repo" &&
> + git init repo &&
> + (
> + cd repo &&
> + test_commit initial &&
> + test_must_fail git history drop 2>err &&
> + test_grep "command expects a single revision" err
Why not `test_cmp` since it’s a fixed error?
Same for a few other tests like `errors on unknown revision`.
> + )
> +'
>[snip]
> +test_expect_success 'errors with invalid --empty= value' '
> + test_when_finished "rm -rf repo" &&
> + git init repo &&
> + test_commit -C repo initial &&
> + test_commit -C repo second &&
> + test_must_fail git -C repo history drop --empty=bogus HEAD 2>err &&
> + test_grep "unrecognized.*--empty.*bogus" err
> +'
Style related I guess. Most tests here use a subshell but this one uses
`git -C`? Why is that?
>[snip]
> +test_expect_success 'updates branches on other lines of descent' '
> + test_when_finished "rm -rf repo" &&
> + git init repo &&
> + (
> + cd repo &&
> + test_commit base &&
> + test_commit target &&
> + git branch theirs &&
> + test_commit ours &&
> + git switch theirs &&
> + test_commit theirs &&
> +
> + expect_graph --branches <<-\EOF &&
> + * theirs
> + | * ours
> + |/
> + * target
> + * base
> + EOF
Oh, `expect_graph` is a cool tool.
> +
> + git history drop target &&
> +
> + expect_graph --branches <<-\EOF
> + * ours
> + | * theirs
> + |/
> + * base
> + EOF
> + )
> +'
>[snip]
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 9/9] builtin/history: implement "drop" subcommand
2026-06-03 16:14 ` [PATCH v2 9/9] builtin/history: implement "drop" subcommand Patrick Steinhardt
2026-06-03 19:04 ` Kristoffer Haugsbakk
@ 2026-06-03 23:58 ` Junio C Hamano
1 sibling, 0 replies; 23+ messages in thread
From: Junio C Hamano @ 2026-06-03 23:58 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Pablo Sabater
Patrick Steinhardt <ps@pks.im> writes:
> +static int update_worktree(struct repository *repo,
> + const struct commit *old_head,
> + const struct commit *new_head,
> + bool dry_run)
> +{
> + struct reset_head_opts opts = {
> + .oid_from = &old_head->object.oid,
> + .oid = &new_head->object.oid,
> + .flags = RESET_HEAD_SKIP_REF_UPDATES,
> + };
> + if (dry_run)
> + opts.flags |= RESET_HEAD_DRY_RUN;
> + return reset_head(repo, &opts);
> +}
> + ...
> + /*
> + * If HEAD will move as a result of the rewrite then we'll have to
> + * merge in the changes into the worktree and index. This merge can of
> + * course conflict, which will cause the whole operation to abort.
> + *
> + * If we had already updated the refs at that point then we'd have an
> + * inconsistent repository state. So we first perform a dry-run merge
> + * here before updating refs.
> + */
> + if (!dry_run && !is_bare_repository()) {
> + ret = find_head_tree_change(repo, &result, &old_head,
> + &new_head, &head_moves);
> + if (ret < 0)
> + goto out;
> +
> + if (head_moves && update_worktree(repo, old_head, new_head, true) < 0) {
> + ret = error(_("dropping this commit would "
> + "overwrite local changes; aborting"));
> + goto out;
> + }
> + }
This block is skipped under --dry-run, but update_worktree is
equipped to (and indeed run unconditionally here) run in the dry-run
mode. Does it mean that "git history drop --dry-run" that user runs
to see which refs may be updated will not get warned about possible
worktree conflicts that would prevent the real run from happening?
Unless there is a compelling reason not to, I think --dry-run should
be a close simulation of what would happen without it.
Thanks.
^ permalink raw reply [flat|nested] 23+ messages in thread