* Re: [PATCH 5/7] environment: split up concerns of `is_bare_repository_cfg`
From: Patrick Steinhardt @ 2026-06-11 9:19 UTC (permalink / raw)
To: Justin Tobler; +Cc: git
In-Reply-To: <ainesoUOuhspKxHF@denethor>
On Wed, Jun 10, 2026 at 05:22:04PM -0500, Justin Tobler wrote:
> On 26/06/10 08:56AM, Patrick Steinhardt wrote:
> > diff --git a/builtin/init-db.c b/builtin/init-db.c
> > index 52aa92fb0a..566732c9f4 100644
> > --- a/builtin/init-db.c
> > +++ b/builtin/init-db.c
> > @@ -81,7 +81,7 @@ int cmd_init_db(int argc,
> > const char *template_dir = NULL;
> > char *template_dir_to_free = NULL;
> > unsigned int flags = 0;
> > - int bare = is_bare_repository_cfg;
> > + int bare = startup_info->force_bare_repository ? 1 : -1;
>
> Any particular reason to continue mapping `force_bare_repository=false`
> to -1? Or was this to just minimize changes?
The `-1` value doesn't mean "false", but it rather means "undecided".
The effect of this is that "core.bare" will eventually override this.
> > diff --git a/setup.c b/setup.c
> > index 71fc6b33da..2b690da8ca 100644
> > --- a/setup.c
> > +++ b/setup.c
> > @@ -795,10 +795,16 @@ static int check_repository_format_gently(const char *gitdir,
> > has_common = 0;
> > }
> >
> > - if (!has_common) {
> > - if (candidate->is_bare != -1)
> > - is_bare_repository_cfg = candidate->is_bare;
> > - } else {
> > + if (startup_info->force_bare_repository) {
> > + candidate->is_bare = 1;
> > + FREE_AND_NULL(candidate->work_tree);
> > + } else if (has_common) {
> > + /*
> > + * When sharing a common dir with another repository (e.g. a
> > + * linked worktree), do not let this repository's config
> > + * dictate bareness; it is inherited from the main worktree.
> > + */
> > + candidate->is_bare = -1;
> > FREE_AND_NULL(candidate->work_tree);
>
> Previously, when there was a common dir, `candidate->work_tree` was left
> untouched, but now we are expclicitly setting it. I'm not sure I fully
> understand this change.
I cannot blame you. All of this logic is so unbelievably tangled and
hard to follow.
In any case, I think you might have missed the fact that we `else if`
branch is now `has_common` as compared to `!has_common`? To explain the
different cases a bit:
- When we have `force_bare_repository` we are being told that the
repository should be treated as bare. So we set `is_bare` and also
clear the work tree that may have been discovered.
- When we have a commondir we know that we're in a worktree.
Previously we did nothing in this case, and that had the implicit
effect that `is_bare_repository_cfg` would remain at `-1`. So to
match that behaviour we have to also reset the candidate's bareness
to `-1`, so that we parse it via the repository's configuration at a
later point in time.
The other part here is that we also reset `candidate->work_tree`.
This is because the expectation is that `$GIT_DIR/common` should
override any "core.worktree" settings. Quoting git-config(1):
If GIT_COMMON_DIR environment variable is set, core.worktree is
ignored and not used for determining the root of working tree.
- When we don't have a commondir we previosuly had to also adapt the
global `is_bare_repository_cfg` variable. This part is not necessary
anymore, so we basically just drop this case altogether.
Patrick
^ permalink raw reply
* Re: [PATCH v4] git-gui: silence install recipes under "make -s"
From: Harald Nordgren @ 2026-06-11 11:17 UTC (permalink / raw)
To: Kristoffer Haugsbakk; +Cc: Johannes Sixt, git, Koji Nakamaru
In-Reply-To: <7b40cabe-d243-40dd-ab29-fc4dd91fa20d@app.fastmail.com>
Thanks for the explanation.
Harald
^ permalink raw reply
* Re: [PATCH v4 06/10] reset: introduce ability to skip updating HEAD
From: Patrick Steinhardt @ 2026-06-11 11:47 UTC (permalink / raw)
To: phillip.wood; +Cc: git, Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk
In-Reply-To: <cfade236-75bc-4679-a74a-6da82e6a5135@gmail.com>
On Wed, Jun 10, 2026 at 02:11:04PM +0100, Phillip Wood wrote:
> On 10/06/2026 09:52, Patrick Steinhardt wrote:
> > In a subsequent commit we'll introduce a new caller to
> > `reset_working_tree()` that really only wants to update the index and
> > working tree, without updating any references. Introduce a new flag that
> > makes the caller opt in to updating HEAD and adapt all callers to set
> > that flag.
> >
> > Note that in a previous iteration we instead introduced a flag that made
> > callers opt out of updating any references. This was somewhat awkward
> > though because we already have the `UPDATE_ORIG_HEAD` flag, so the
> > result was somewhat inconsistent.
>
> Thanks for doing this. I've grepped for all the callers of reset_head() to
> confirm this patch adds RESET_HEAD_UPDATE_HEAD to them all.
>
> I wonder if we should add a check for passing RESET_HEAD_UPDATE_ORIG_HEAD
> without RESET_HEAD_UPDATE_HEAD that calls BUG() as we don't support that.
> Everything else looks good.
Fair, can do. Thanks!
Patrick
^ permalink raw reply
* [PATCH v2] commit-reach: remove get_reachable_subset()
From: Kristofer Karlsson via GitGitGadget @ 2026-06-11 11:49 UTC (permalink / raw)
To: git
Cc: Derrick Stolee, Kristofer Karlsson, Kristofer Karlsson,
Kristofer Karlsson
In-Reply-To: <pull.2144.git.1781033285419.gitgitgadget@gmail.com>
From: Kristofer Karlsson <krka@spotify.com>
get_reachable_subset() and tips_reachable_from_bases() both answer
the same reachability question but use different traversal
strategies: priority queue vs depth-first search. Consolidate them
into tips_reachable_from_bases() with a mode parameter to select
between DFS and PQ traversal, preserving the preferred strategy for
each caller.
This works cleanly because prio_queue already supports LIFO mode
(when compare is NULL), so a single prio_queue acts as either a
stack or a heap depending on the mode.
The unified traversal pushes all unseen parents at once rather than
peeking and pushing one parent at a time. This eliminates merge
commit revisits entirely: a 2-parent merge now requires 1 visit
instead of 3. For DFS (LIFO) mode, the first parent is pushed
last so it ends up on top of the stack, preserving first-parent
traversal order.
Parsing is deferred to pop time for DFS since parent objects carry
valid flags without a full repo_parse_commit() call. PQ mode
parses before push so the heap can order by generation number.
Add exhaustive reachability tests that use every commit in the
grid as a tip, protecting against subtle traversal bugs such as
wrong parent ordering or premature pruning. The existing tests
are also extended to exercise both DFS and PQ modes.
The flag in remote.c changes from 1 (bit 0) to TMP_MARK (bit 4)
because tips_reachable_from_bases() uses SEEN (bit 0) internally.
TMP_MARK is already used for deduplication earlier in the same
function and is cleared before the reachability check.
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
commit-reach: remove get_reachable_subset()
This removes get_reachable_subset() and consolidates its only caller
into tips_reachable_from_bases() with a mode parameter to select between
DFS and priority queue traversal. Both functions answer the same
reachability query and use the same generation-number pruning strategy,
differing primarily in traversal order (DFS vs generation-ordered PQ).
They were introduced years apart and never consolidated.
The unified function uses prio_queue for both modes: LIFO (stack) when
compare is NULL, min-heap when given a comparator.
Changes since v1:
* Replaced the commit_stack + reverse-push pattern with a simpler
approach: push all unseen parents directly, with the first parent
pushed last so it lands on top of the LIFO stack. This preserves
first-parent DFS order without needing a temporary stack to reverse
parent order.
* Deferred repo_parse_commit() to pop time for DFS mode. Parent objects
carry valid flags without a full parse, so we avoid parsing
already-SEEN parents in the inner loop. PQ mode still parses before
push so the heap can order by generation number.
* Moved the generation floor check from the parent loop to pop time,
simplifying the hot path at the cost of occasionally enqueueing
commits that will later be discarded.
* Added SEEN flag on base commits before enqueueing, preventing
duplicate processing if bases overlap with the traversal.
* Added PQ mode to the existing test-reach tests so both DFS and PQ
paths are exercised by the test suite.
* Added two new reachability tests that use all 100 commits in the grid
as tips - the idea is to better detect subtle traversal bugs.
Notes for reviewers:
* The flag in remote.c changes from 1 to TMP_MARK because
tips_reachable_from_bases() uses SEEN (bit 0) internally. TMP_MARK is
already used earlier in the same function and is cleared before the
reachability block.
* Test helper and test names rename from get_reachable_subset to
tips_reachable_from_bases to match the function being exercised.
As Stolee noted in the v1 review, commit-graph is now stable and
expected for repositories where this matters, making the DFS approach
with dynamic floor raising an attractive default. The mode parameter
could be dropped in a future patch if we decide to use DFS everywhere,
but preserving each caller's existing strategy keeps this change
conservative and avoids any behavior change for add_missing_tags().
Performance was not a goal of this refactoring, but the simplified DFS
traversal turned out to be a pleasant surprise -- eliminating merge
commit revisits and deferring parse to pop time gives a consistent
speedup across repositories:
Benchmark results (median, 30 runs, pre-built binaries):
Repository Command Speedup
-----------------------------------------------------------
linux branch --merged=HEAD 1.09x faster
linux for-each-ref --merged=HEAD 1.14x faster
git branch --merged=HEAD neutral
git for-each-ref --merged=HEAD 1.12x faster
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2144%2Fspkrka%2Fkrka%2Fremove-get-reachable-subset-v2-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2144/spkrka/krka/remove-get-reachable-subset-v2-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2144
Range-diff vs v1:
1: df8e2de11b ! 1: da01032a32 commit-reach: remove get_reachable_subset()
@@ Metadata
## Commit message ##
commit-reach: remove get_reachable_subset()
- get_reachable_subset() and tips_reachable_from_bases() answer the
- same question: given a set of bases and a set of tips, which tips
- are reachable from at least one base?
+ get_reachable_subset() and tips_reachable_from_bases() both answer
+ the same reachability question but use different traversal
+ strategies: priority queue vs depth-first search. Consolidate them
+ into tips_reachable_from_bases() with a mode parameter to select
+ between DFS and PQ traversal, preserving the preferred strategy for
+ each caller.
- get_reachable_subset() was introduced in fcb2c0769d (2018-11-02)
- for add_missing_tags() in remote.c. tips_reachable_from_bases()
- was added in cbfe360b14 (2023-03-20) as part of the ahead-behind
- series. The two were never consolidated.
+ This works cleanly because prio_queue already supports LIFO mode
+ (when compare is NULL), so a single prio_queue acts as either a
+ stack or a heap depending on the mode.
- With a commit-graph, tips_reachable_from_bases() can have an edge:
- its DFS raises the generation floor as lower targets are found,
- pruning more aggressively than the static floor in
- get_reachable_subset(). Without generation numbers, some edge cases
- may be slower with DFS instead of BFS since the date-ordered
- prio_queue naturally stays near the top of the graph, but this
- should not matter in practice -- worst case both visit the full
- graph down from the bases.
+ The unified traversal pushes all unseen parents at once rather than
+ peeking and pushing one parent at a time. This eliminates merge
+ commit revisits entirely: a 2-parent merge now requires 1 visit
+ instead of 3. For DFS (LIFO) mode, the first parent is pushed
+ last so it ends up on top of the stack, preserving first-parent
+ traversal order.
+
+ Parsing is deferred to pop time for DFS since parent objects carry
+ valid flags without a full repo_parse_commit() call. PQ mode
+ parses before push so the heap can order by generation number.
+
+ Add exhaustive reachability tests that use every commit in the
+ grid as a tip, protecting against subtle traversal bugs such as
+ wrong parent ordering or premature pruning. The existing tests
+ are also extended to exercise both DFS and PQ modes.
The flag in remote.c changes from 1 (bit 0) to TMP_MARK (bit 4)
because tips_reachable_from_bases() uses SEEN (bit 0) internally.
@@ commit-reach.c: int can_all_from_reach(struct commit_list *from, struct commit_l
define_commit_slab(bit_arrays, struct bitmap *);
static struct bit_arrays bit_arrays;
+@@ commit-reach.c: static int compare_commit_and_index_by_generation(const void *va, const void *vb
+ void tips_reachable_from_bases(struct repository *r,
+ struct commit_list *bases,
+ struct commit **tips, size_t tips_nr,
+- int mark)
++ int mark, enum tips_reachable_mode mode)
+ {
+ struct commit_and_index *commits;
++ struct commit_list *p;
++ struct commit *c;
+ size_t min_generation_index = 0;
+ timestamp_t min_generation;
+- struct commit_list *stack = NULL;
++ struct prio_queue queue = { NULL };
+
+ if (!bases || !tips || !tips_nr)
+ return;
+
+ /*
+- * Do a depth-first search starting at 'bases' to search for the
+- * tips. Stop at the lowest (un-found) generation number. When
+- * finding the lowest commit, increase the minimum generation
+- * number to the next lowest (un-found) generation number.
++ * Search starting at 'bases' looking for the tips. Stop at the
++ * lowest un-found generation number, raising the floor as tips
++ * are found. Use DFS by default; with TIPS_REACHABLE_PQ,
++ * use a priority queue ordered by generation then commit date.
+ */
++ if (mode == TIPS_REACHABLE_PQ)
++ queue.compare = compare_commits_by_gen_then_commit_date;
+
+ CALLOC_ARRAY(commits, tips_nr);
+
+@@ commit-reach.c: void tips_reachable_from_bases(struct repository *r,
+
+ while (bases) {
+ repo_parse_commit(r, bases->item);
+- commit_list_insert(bases->item, &stack);
++ bases->item->object.flags |= SEEN;
++ prio_queue_put(&queue, bases->item);
+ bases = bases->next;
+ }
+
+- while (stack) {
+- int explored_all_parents = 1;
+- struct commit_list *p;
+- struct commit *c = stack->item;
++ while ((c = prio_queue_get(&queue))) {
++ struct commit *first_parent = NULL;
++
++ repo_parse_commit(r, c);
++
++ /* Skip if below the current generation floor. */
++ if (commit_graph_generation(c) < min_generation)
++ continue;
+
+ /* Does it match any of our tips? */
+ {
+@@ commit-reach.c: void tips_reachable_from_bases(struct repository *r,
+ }
+
+ for (p = c->parents; p; p = p->next) {
+- repo_parse_commit(r, p->item);
+-
+ /* Have we already explored this parent? */
+ if (p->item->object.flags & SEEN)
+ continue;
+
+- /* Is it below the current minimum generation? */
+- if (commit_graph_generation(p->item) < min_generation)
+- continue;
+-
+ /* Ok, we will explore from here on. */
+ p->item->object.flags |= SEEN;
+- explored_all_parents = 0;
+- commit_list_insert(p->item, &stack);
+- break;
++ /* Parse before pushing in PQ mode for ordering. */
++ if (mode == TIPS_REACHABLE_PQ)
++ repo_parse_commit(r, p->item);
++ if (!first_parent)
++ first_parent = p->item;
++ else
++ prio_queue_put(&queue, p->item);
+ }
+-
+- if (explored_all_parents)
+- pop_commit(&stack);
++ /*
++ * Add the first parent last so that it is on top of
++ * the LIFO queue, maintaining first-parent DFS order.
++ */
++ if (first_parent)
++ prio_queue_put(&queue, first_parent);
+ }
+
+ done:
+@@ commit-reach.c: done:
+ commits[i].commit->object.flags &= ~RESULT;
+ free(commits);
+ repo_clear_commit_marks(r, SEEN);
+- commit_list_free(stack);
++ clear_prio_queue(&queue);
+ }
+
+ /*
## commit-reach.h ##
@@ commit-reach.h: int can_all_from_reach_with_flag(struct object_array *from,
@@ commit-reach.h: int can_all_from_reach_with_flag(struct object_array *from,
struct ahead_behind_count {
/**
* As input, the *_index members indicate which positions in
+@@ commit-reach.h: void ahead_behind(struct repository *r,
+ * For all tip commits, add 'mark' to their flags if and only if they
+ * are reachable from one of the commits in 'bases'.
+ */
++enum tips_reachable_mode {
++ TIPS_REACHABLE_DFS,
++ TIPS_REACHABLE_PQ,
++};
+ void tips_reachable_from_bases(struct repository *r,
+ struct commit_list *bases,
+ struct commit **tips, size_t tips_nr,
+- int mark);
++ int mark, enum tips_reachable_mode mode);
+
+ /*
+ * Given a 'tip' commit and a list potential 'bases', return the index 'i' that
+
+ ## ref-filter.c ##
+@@ ref-filter.c: static void reach_filter(struct ref_array *array,
+ tips_reachable_from_bases(the_repository,
+ *check_reachable,
+ to_clear, array->nr,
+- UNINTERESTING);
++ UNINTERESTING, TIPS_REACHABLE_DFS);
+
+ old_nr = array->nr;
+ array->nr = 0;
## remote.c ##
@@ remote.c: static void add_missing_tags(struct ref *src, struct ref **dst, struct ref ***ds
@@ remote.c: static void add_missing_tags(struct ref *src, struct ref **dst, struct
+ commit_list_insert(sent_tips.items[i], &bases);
+ tips_reachable_from_bases(the_repository,
+ bases, src_commits.items,
-+ src_commits.nr, TMP_MARK);
++ src_commits.nr, TMP_MARK,
++ TIPS_REACHABLE_PQ);
+ commit_list_free(bases);
for_each_string_list_item(item, &src_tag) {
@@ t/helper/test-reach.c: int cmd__reach(int ac, const char **av)
- oid_to_hex(&list->item->object.oid));
- count++;
- }
-+ } else if (!strcmp(av[1], "tips_reachable_from_bases")) {
++ } else if (!strcmp(av[1], "tips_reachable_from_bases") ||
++ !strcmp(av[1], "tips_reachable_from_bases_pq")) {
++ enum tips_reachable_mode mode =
++ !strcmp(av[1], "tips_reachable_from_bases_pq")
++ ? TIPS_REACHABLE_PQ : TIPS_REACHABLE_DFS;
+ struct commit_list *bases = NULL;
+ struct commit_list *result = NULL;
+
@@ t/helper/test-reach.c: int cmd__reach(int ac, const char **av)
+ commit_list_insert(X_stack.items[i], &bases);
+ tips_reachable_from_bases(the_repository,
+ bases, Y_stack.items,
-+ Y_stack.nr, TMP_MARK);
++ Y_stack.nr, TMP_MARK,
++ mode);
+ commit_list_free(bases);
+
+ printf("tips_reachable_from_bases(X,Y)\n");
@@ t/t6600-test-reach.sh: test_expect_success 'get_reachable_subset:all' '
commit-5-6 | sort
) >expect &&
- test_all_modes get_reachable_subset
-+ test_all_modes tips_reachable_from_bases
++ test_all_modes tips_reachable_from_bases &&
++ test_all_modes tips_reachable_from_bases_pq
'
-test_expect_success 'get_reachable_subset:some' '
@@ t/t6600-test-reach.sh: test_expect_success 'get_reachable_subset:some' '
commit-1-7 | sort
) >expect &&
- test_all_modes get_reachable_subset
-+ test_all_modes tips_reachable_from_bases
++ test_all_modes tips_reachable_from_bases &&
++ test_all_modes tips_reachable_from_bases_pq
'
-test_expect_success 'get_reachable_subset:none' '
@@ t/t6600-test-reach.sh: test_expect_success 'get_reachable_subset:none' '
- echo "get_reachable_subset(X,Y)" >expect &&
- test_all_modes get_reachable_subset
+ echo "tips_reachable_from_bases(X,Y)" >expect &&
-+ test_all_modes tips_reachable_from_bases
++ test_all_modes tips_reachable_from_bases &&
++ test_all_modes tips_reachable_from_bases_pq
'
test_expect_success 'for-each-ref ahead-behind:linear' '
+@@ t/t6600-test-reach.sh: test_expect_success 'for-each-ref merged:duplicate at min generation' '
+ --format="%(refname)" --stdin
+ '
+
++test_expect_success 'for-each-ref merged:all reachable commits' '
++ for x in $(test_seq 1 10)
++ do
++ for y in $(test_seq 1 10)
++ do
++ echo "refs/heads/commit-$x-$y" || return 1
++ done
++ done >input &&
++ for x in $(test_seq 1 5)
++ do
++ for y in $(test_seq 1 5)
++ do
++ echo "refs/heads/commit-$x-$y" || return 1
++ done
++ done | sort >expect &&
++ run_all_modes git for-each-ref --merged=commit-5-5 \
++ --format="%(refname)" --stdin
++'
++
++test_expect_success 'for-each-ref merged:all reachable, multibase' '
++ for x in $(test_seq 1 10)
++ do
++ for y in $(test_seq 1 10)
++ do
++ echo "refs/heads/commit-$x-$y" || return 1
++ done
++ done >input &&
++ for x in $(test_seq 1 10)
++ do
++ for y in $(test_seq 1 10)
++ do
++ if { test $x -le 3 && test $y -le 7; } ||
++ { test $x -le 7 && test $y -le 3; }
++ then
++ echo "refs/heads/commit-$x-$y" || return 1
++ fi
++ done
++ done | sort >expect &&
++ run_all_modes git for-each-ref \
++ --merged=commit-3-7 \
++ --merged=commit-7-3 \
++ --format="%(refname)" --stdin
++'
++
+ # For get_branch_base_for_tip, we only care about
+ # first-parent history. Here is the test graph with
+ # second parents removed:
commit-reach.c | 131 +++++++++++-------------------------------
commit-reach.h | 19 ++----
ref-filter.c | 2 +-
remote.c | 20 +++----
t/helper/test-reach.c | 44 +++++++-------
t/t6600-test-reach.sh | 65 ++++++++++++++++++---
6 files changed, 129 insertions(+), 152 deletions(-)
diff --git a/commit-reach.c b/commit-reach.c
index 5df471a313..1cad7b211e 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -1013,79 +1013,6 @@ int can_all_from_reach(struct commit_list *from, struct commit_list *to,
return result;
}
-struct commit_list *get_reachable_subset(struct commit **from, size_t nr_from,
- struct commit **to, size_t nr_to,
- unsigned int reachable_flag)
-{
- struct commit **item;
- struct commit *current;
- struct commit_list *found_commits = NULL;
- struct commit **to_last = to + nr_to;
- struct commit **from_last = from + nr_from;
- timestamp_t min_generation = GENERATION_NUMBER_INFINITY;
- int num_to_find = 0;
-
- struct prio_queue queue = { compare_commits_by_gen_then_commit_date };
-
- for (item = to; item < to_last; item++) {
- timestamp_t generation;
- struct commit *c = *item;
-
- repo_parse_commit(the_repository, c);
- generation = commit_graph_generation(c);
- if (generation < min_generation)
- min_generation = generation;
-
- if (!(c->object.flags & PARENT1)) {
- c->object.flags |= PARENT1;
- num_to_find++;
- }
- }
-
- for (item = from; item < from_last; item++) {
- struct commit *c = *item;
- if (!(c->object.flags & PARENT2)) {
- c->object.flags |= PARENT2;
- repo_parse_commit(the_repository, c);
-
- prio_queue_put(&queue, *item);
- }
- }
-
- while (num_to_find && (current = prio_queue_get(&queue)) != NULL) {
- struct commit_list *parents;
-
- if (current->object.flags & PARENT1) {
- current->object.flags &= ~PARENT1;
- current->object.flags |= reachable_flag;
- commit_list_insert(current, &found_commits);
- num_to_find--;
- }
-
- for (parents = current->parents; parents; parents = parents->next) {
- struct commit *p = parents->item;
-
- repo_parse_commit(the_repository, p);
-
- if (commit_graph_generation(p) < min_generation)
- continue;
-
- if (p->object.flags & PARENT2)
- continue;
-
- p->object.flags |= PARENT2;
- prio_queue_put(&queue, p);
- }
- }
-
- clear_prio_queue(&queue);
-
- clear_commit_marks_many(nr_to, to, PARENT1);
- clear_commit_marks_many(nr_from, from, PARENT2);
-
- return found_commits;
-}
-
define_commit_slab(bit_arrays, struct bitmap *);
static struct bit_arrays bit_arrays;
@@ -1212,22 +1139,26 @@ static int compare_commit_and_index_by_generation(const void *va, const void *vb
void tips_reachable_from_bases(struct repository *r,
struct commit_list *bases,
struct commit **tips, size_t tips_nr,
- int mark)
+ int mark, enum tips_reachable_mode mode)
{
struct commit_and_index *commits;
+ struct commit_list *p;
+ struct commit *c;
size_t min_generation_index = 0;
timestamp_t min_generation;
- struct commit_list *stack = NULL;
+ struct prio_queue queue = { NULL };
if (!bases || !tips || !tips_nr)
return;
/*
- * Do a depth-first search starting at 'bases' to search for the
- * tips. Stop at the lowest (un-found) generation number. When
- * finding the lowest commit, increase the minimum generation
- * number to the next lowest (un-found) generation number.
+ * Search starting at 'bases' looking for the tips. Stop at the
+ * lowest un-found generation number, raising the floor as tips
+ * are found. Use DFS by default; with TIPS_REACHABLE_PQ,
+ * use a priority queue ordered by generation then commit date.
*/
+ if (mode == TIPS_REACHABLE_PQ)
+ queue.compare = compare_commits_by_gen_then_commit_date;
CALLOC_ARRAY(commits, tips_nr);
@@ -1245,14 +1176,19 @@ void tips_reachable_from_bases(struct repository *r,
while (bases) {
repo_parse_commit(r, bases->item);
- commit_list_insert(bases->item, &stack);
+ bases->item->object.flags |= SEEN;
+ prio_queue_put(&queue, bases->item);
bases = bases->next;
}
- while (stack) {
- int explored_all_parents = 1;
- struct commit_list *p;
- struct commit *c = stack->item;
+ while ((c = prio_queue_get(&queue))) {
+ struct commit *first_parent = NULL;
+
+ repo_parse_commit(r, c);
+
+ /* Skip if below the current generation floor. */
+ if (commit_graph_generation(c) < min_generation)
+ continue;
/* Does it match any of our tips? */
{
@@ -1276,25 +1212,26 @@ void tips_reachable_from_bases(struct repository *r,
}
for (p = c->parents; p; p = p->next) {
- repo_parse_commit(r, p->item);
-
/* Have we already explored this parent? */
if (p->item->object.flags & SEEN)
continue;
- /* Is it below the current minimum generation? */
- if (commit_graph_generation(p->item) < min_generation)
- continue;
-
/* Ok, we will explore from here on. */
p->item->object.flags |= SEEN;
- explored_all_parents = 0;
- commit_list_insert(p->item, &stack);
- break;
+ /* Parse before pushing in PQ mode for ordering. */
+ if (mode == TIPS_REACHABLE_PQ)
+ repo_parse_commit(r, p->item);
+ if (!first_parent)
+ first_parent = p->item;
+ else
+ prio_queue_put(&queue, p->item);
}
-
- if (explored_all_parents)
- pop_commit(&stack);
+ /*
+ * Add the first parent last so that it is on top of
+ * the LIFO queue, maintaining first-parent DFS order.
+ */
+ if (first_parent)
+ prio_queue_put(&queue, first_parent);
}
done:
@@ -1302,7 +1239,7 @@ done:
commits[i].commit->object.flags &= ~RESULT;
free(commits);
repo_clear_commit_marks(r, SEEN);
- commit_list_free(stack);
+ clear_prio_queue(&queue);
}
/*
diff --git a/commit-reach.h b/commit-reach.h
index 3f3a563d8a..71e60d727a 100644
--- a/commit-reach.h
+++ b/commit-reach.h
@@ -96,19 +96,6 @@ int can_all_from_reach_with_flag(struct object_array *from,
int can_all_from_reach(struct commit_list *from, struct commit_list *to,
int commit_date_cutoff);
-
-/*
- * Return a list of commits containing the commits in the 'to' array
- * that are reachable from at least one commit in the 'from' array.
- * Also add the given 'flag' to each of the commits in the returned list.
- *
- * This method uses the PARENT1 and PARENT2 flags during its operation,
- * so be sure these flags are not set before calling the method.
- */
-struct commit_list *get_reachable_subset(struct commit **from, size_t nr_from,
- struct commit **to, size_t nr_to,
- unsigned int reachable_flag);
-
struct ahead_behind_count {
/**
* As input, the *_index members indicate which positions in
@@ -144,10 +131,14 @@ void ahead_behind(struct repository *r,
* For all tip commits, add 'mark' to their flags if and only if they
* are reachable from one of the commits in 'bases'.
*/
+enum tips_reachable_mode {
+ TIPS_REACHABLE_DFS,
+ TIPS_REACHABLE_PQ,
+};
void tips_reachable_from_bases(struct repository *r,
struct commit_list *bases,
struct commit **tips, size_t tips_nr,
- int mark);
+ int mark, enum tips_reachable_mode mode);
/*
* Given a 'tip' commit and a list potential 'bases', return the index 'i' that
diff --git a/ref-filter.c b/ref-filter.c
index 1da4c0e60d..9c8896d347 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -3157,7 +3157,7 @@ static void reach_filter(struct ref_array *array,
tips_reachable_from_bases(the_repository,
*check_reachable,
to_clear, array->nr,
- UNINTERESTING);
+ UNINTERESTING, TIPS_REACHABLE_DFS);
old_nr = array->nr;
array->nr = 0;
diff --git a/remote.c b/remote.c
index 00723b385e..0324c25743 100644
--- a/remote.c
+++ b/remote.c
@@ -1459,9 +1459,8 @@ static void add_missing_tags(struct ref *src, struct ref **dst, struct ref ***ds
* sent to the other side.
*/
if (sent_tips.nr) {
- const int reachable_flag = 1;
- struct commit_list *found_commits;
struct commit_stack src_commits = COMMIT_STACK_INIT;
+ struct commit_list *bases = NULL;
for_each_string_list_item(item, &src_tag) {
struct ref *ref = item->util;
@@ -1479,11 +1478,13 @@ static void add_missing_tags(struct ref *src, struct ref **dst, struct ref ***ds
commit_stack_push(&src_commits, commit);
}
- found_commits = get_reachable_subset(sent_tips.items,
- sent_tips.nr,
- src_commits.items,
- src_commits.nr,
- reachable_flag);
+ for (size_t i = 0; i < sent_tips.nr; i++)
+ commit_list_insert(sent_tips.items[i], &bases);
+ tips_reachable_from_bases(the_repository,
+ bases, src_commits.items,
+ src_commits.nr, TMP_MARK,
+ TIPS_REACHABLE_PQ);
+ commit_list_free(bases);
for_each_string_list_item(item, &src_tag) {
struct ref *dst_ref;
@@ -1503,7 +1504,7 @@ static void add_missing_tags(struct ref *src, struct ref **dst, struct ref ***ds
* Is this tag, which they do not have, reachable from
* any of the commits we are sending?
*/
- if (!(commit->object.flags & reachable_flag))
+ if (!(commit->object.flags & TMP_MARK))
continue;
/* Add it in */
@@ -1513,9 +1514,8 @@ static void add_missing_tags(struct ref *src, struct ref **dst, struct ref ***ds
}
clear_commit_marks_many(src_commits.nr, src_commits.items,
- reachable_flag);
+ TMP_MARK);
commit_stack_clear(&src_commits);
- commit_list_free(found_commits);
}
string_list_clear(&src_tag, 0);
diff --git a/t/helper/test-reach.c b/t/helper/test-reach.c
index 5d86a96c17..66ee35e70d 100644
--- a/t/helper/test-reach.c
+++ b/t/helper/test-reach.c
@@ -7,6 +7,7 @@
#include "hex.h"
#include "object-name.h"
#include "ref-filter.h"
+#include "revision.h"
#include "setup.h"
#include "string-list.h"
#include "tag.h"
@@ -149,30 +150,31 @@ int cmd__reach(int ac, const char **av)
printf("%s(_,A,X,_):%d\n", av[1], commit_contains(&filter, A, X, &cache));
clear_contains_cache(&cache);
- } else if (!strcmp(av[1], "get_reachable_subset")) {
- const int reachable_flag = 1;
- int count = 0;
- struct commit_list *current;
- struct commit_list *list = get_reachable_subset(X_stack.items, X_stack.nr,
- Y_stack.items, Y_stack.nr,
- reachable_flag);
- printf("get_reachable_subset(X,Y)\n");
- for (current = list; current; current = current->next) {
- if (!(list->item->object.flags & reachable_flag))
- die(_("commit %s is not marked reachable"),
- oid_to_hex(&list->item->object.oid));
- count++;
- }
+ } else if (!strcmp(av[1], "tips_reachable_from_bases") ||
+ !strcmp(av[1], "tips_reachable_from_bases_pq")) {
+ enum tips_reachable_mode mode =
+ !strcmp(av[1], "tips_reachable_from_bases_pq")
+ ? TIPS_REACHABLE_PQ : TIPS_REACHABLE_DFS;
+ struct commit_list *bases = NULL;
+ struct commit_list *result = NULL;
+
+ for (size_t i = 0; i < X_stack.nr; i++)
+ commit_list_insert(X_stack.items[i], &bases);
+ tips_reachable_from_bases(the_repository,
+ bases, Y_stack.items,
+ Y_stack.nr, TMP_MARK,
+ mode);
+ commit_list_free(bases);
+
+ printf("tips_reachable_from_bases(X,Y)\n");
for (size_t i = 0; i < Y_stack.nr; i++) {
- if (Y_stack.items[i]->object.flags & reachable_flag)
- count--;
+ if (Y_stack.items[i]->object.flags & TMP_MARK)
+ commit_list_insert(Y_stack.items[i], &result);
}
+ print_sorted_commit_ids(result);
- if (count < 0)
- die(_("too many commits marked reachable"));
-
- print_sorted_commit_ids(list);
- commit_list_free(list);
+ clear_commit_marks_many(Y_stack.nr, Y_stack.items, TMP_MARK);
+ commit_list_free(result);
}
object_array_clear(&X_obj);
diff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh
index b5b314e570..b736d893d5 100755
--- a/t/t6600-test-reach.sh
+++ b/t/t6600-test-reach.sh
@@ -391,7 +391,7 @@ test_expect_success 'rev-list: symmetric difference topo-order' '
run_all_modes git rev-list --topo-order commit-3-8...commit-6-6
'
-test_expect_success 'get_reachable_subset:all' '
+test_expect_success 'tips_reachable_from_bases:all' '
cat >input <<-\EOF &&
X:commit-9-1
X:commit-8-3
@@ -403,15 +403,16 @@ test_expect_success 'get_reachable_subset:all' '
Y:commit-5-6
EOF
(
- echo "get_reachable_subset(X,Y)" &&
+ echo "tips_reachable_from_bases(X,Y)" &&
git rev-parse commit-3-3 \
commit-1-7 \
commit-5-6 | sort
) >expect &&
- test_all_modes get_reachable_subset
+ test_all_modes tips_reachable_from_bases &&
+ test_all_modes tips_reachable_from_bases_pq
'
-test_expect_success 'get_reachable_subset:some' '
+test_expect_success 'tips_reachable_from_bases:some' '
cat >input <<-\EOF &&
X:commit-9-1
X:commit-8-3
@@ -422,14 +423,15 @@ test_expect_success 'get_reachable_subset:some' '
Y:commit-5-6
EOF
(
- echo "get_reachable_subset(X,Y)" &&
+ echo "tips_reachable_from_bases(X,Y)" &&
git rev-parse commit-3-3 \
commit-1-7 | sort
) >expect &&
- test_all_modes get_reachable_subset
+ test_all_modes tips_reachable_from_bases &&
+ test_all_modes tips_reachable_from_bases_pq
'
-test_expect_success 'get_reachable_subset:none' '
+test_expect_success 'tips_reachable_from_bases:none' '
cat >input <<-\EOF &&
X:commit-9-1
X:commit-8-3
@@ -439,8 +441,9 @@ test_expect_success 'get_reachable_subset:none' '
Y:commit-7-6
Y:commit-2-8
EOF
- echo "get_reachable_subset(X,Y)" >expect &&
- test_all_modes get_reachable_subset
+ echo "tips_reachable_from_bases(X,Y)" >expect &&
+ test_all_modes tips_reachable_from_bases &&
+ test_all_modes tips_reachable_from_bases_pq
'
test_expect_success 'for-each-ref ahead-behind:linear' '
@@ -657,6 +660,50 @@ test_expect_success 'for-each-ref merged:duplicate at min generation' '
--format="%(refname)" --stdin
'
+test_expect_success 'for-each-ref merged:all reachable commits' '
+ for x in $(test_seq 1 10)
+ do
+ for y in $(test_seq 1 10)
+ do
+ echo "refs/heads/commit-$x-$y" || return 1
+ done
+ done >input &&
+ for x in $(test_seq 1 5)
+ do
+ for y in $(test_seq 1 5)
+ do
+ echo "refs/heads/commit-$x-$y" || return 1
+ done
+ done | sort >expect &&
+ run_all_modes git for-each-ref --merged=commit-5-5 \
+ --format="%(refname)" --stdin
+'
+
+test_expect_success 'for-each-ref merged:all reachable, multibase' '
+ for x in $(test_seq 1 10)
+ do
+ for y in $(test_seq 1 10)
+ do
+ echo "refs/heads/commit-$x-$y" || return 1
+ done
+ done >input &&
+ for x in $(test_seq 1 10)
+ do
+ for y in $(test_seq 1 10)
+ do
+ if { test $x -le 3 && test $y -le 7; } ||
+ { test $x -le 7 && test $y -le 3; }
+ then
+ echo "refs/heads/commit-$x-$y" || return 1
+ fi
+ done
+ done | sort >expect &&
+ run_all_modes git for-each-ref \
+ --merged=commit-3-7 \
+ --merged=commit-7-3 \
+ --format="%(refname)" --stdin
+'
+
# For get_branch_base_for_tip, we only care about
# first-parent history. Here is the test graph with
# second parents removed:
base-commit: 1ff279f3404a482a83fb04c7457e41ab26884aea
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v3 00/11] doc: interpret-trailers: explain key format
From: Junio C Hamano @ 2026-06-11 11:57 UTC (permalink / raw)
To: kristofferhaugsbakk
Cc: git, Kristoffer Haugsbakk, christian.couder, jackmanb,
Linus Arver, D . Ben Knoble
In-Reply-To: <xmqqcxxyt4op.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> kristofferhaugsbakk@fastmail.com writes:
>
>> Interdiff against v2:
>> diff --git a/Documentation/git-interpret-trailers.adoc b/Documentation/git-interpret-trailers.adoc
>> ...
By the way, what I queued last night was missing [10/11] as I used
"b4 am" to grab the latest thread messages by giving the message-id
of the cover letter, but somehow [10/11] had a bogus value in the
e-mail header.
Subject: [PATCH v3 10/11] doc: interpret-trailers: rewrite new-trailers paragraphs
Date: Wed, 10 Jun 2026 23:21:28 +0200
Message-ID: <>
X-Mailer: git-send-email 2.54.0.22.g9e26862b904
So, I reverted to the old and battle tested way to pick these 11
messages manually in my newsreader to replace the topic.
If you have a chance, could you investigate where the send-out
process went wrong and gave one message a bogus ID? I am worried if
you may have triggered a bug in send-email, in which case we would
want to fix it to avoid hurting other users.
Thanks.
^ permalink raw reply
* Re: [PATCH v3 00/11] doc: interpret-trailers: explain key format
From: Kristoffer Haugsbakk @ 2026-06-11 12:05 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Kristoffer Haugsbakk, Christian Couder, jackmanb,
Linus Arver, D. Ben Knoble
In-Reply-To: <xmqq1pedthkv.fsf@gitster.g>
On Thu, Jun 11, 2026, at 13:57, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> kristofferhaugsbakk@fastmail.com writes:
>>
>>> Interdiff against v2:
>>> diff --git a/Documentation/git-interpret-trailers.adoc b/Documentation/git-interpret-trailers.adoc
>>> ...
>
> By the way, what I queued last night was missing [10/11] as I used
> "b4 am" to grab the latest thread messages by giving the message-id
> of the cover letter, but somehow [10/11] had a bogus value in the
> e-mail header.
>
> Subject: [PATCH v3 10/11] doc: interpret-trailers: rewrite
> new-trailers paragraphs
> Date: Wed, 10 Jun 2026 23:21:28 +0200
> Message-ID: <>
> X-Mailer: git-send-email 2.54.0.22.g9e26862b904
>
> So, I reverted to the old and battle tested way to pick these 11
> messages manually in my newsreader to replace the topic.
>
> If you have a chance, could you investigate where the send-out
> process went wrong and gave one message a bogus ID? I am worried if
> you may have triggered a bug in send-email, in which case we would
> want to fix it to avoid hurting other users.
I’ll investigate. It’s 99.99% chance a problem on my end, created by me.
Sorry for the trouble!
^ permalink raw reply
* Re: [PATCH] t1400: have fifo test clean after itself
From: Patrick Steinhardt @ 2026-06-11 12:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqo6hit6rn.fsf@gitster.g>
On Wed, Jun 10, 2026 at 02:39:08PM -0700, Junio C Hamano wrote:
> One test in this script creates a pair of FIFOs, "in" and "out",
> that are named so generically that later tests may be tempted to use
> them. By the time those later tests run a command with its output
> redirected to the file (e.g., "git foobar >out"), however, nobody is
> reading from the lingering FIFO, and the test gets blocked forever.
>
> Clean them up when the test finishes.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> t/t1400-update-ref.sh | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
> index db7f5444da..477af544bc 100755
> --- a/t/t1400-update-ref.sh
> +++ b/t/t1400-update-ref.sh
> @@ -1610,6 +1610,7 @@ test_expect_success 'transaction cannot restart ongoing transaction' '
> '
>
> test_expect_success PIPE 'transaction flushes status updates' '
> + test_when_finished "rm -f in out" &&
> mkfifo in out &&
> (git update-ref --stdin <in >out &) &&
I'd expect that such a test that tried to reuse the sockets would
probably break quite obviously, but I guess you never really know. In
any case, it doesn't hurt to clean up after the test.
Thanks!
Patrick
^ permalink raw reply
* Re: [PATCH v3 00/11] doc: interpret-trailers: explain key format
From: Kristoffer Haugsbakk @ 2026-06-11 12:53 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Christian Couder, jackmanb, Linus Arver, D. Ben Knoble
In-Reply-To: <f738e97b-1aa6-492f-82df-c284a2f94c6a@app.fastmail.com>
On Thu, Jun 11, 2026, at 14:05, Kristoffer Haugsbakk wrote:
>>>>[snip]
>>>> ...
>>
>> By the way, what I queued last night was missing [10/11] as I used
>> "b4 am" to grab the latest thread messages by giving the message-id
>> of the cover letter, but somehow [10/11] had a bogus value in the
>> e-mail header.
>>
>> Subject: [PATCH v3 10/11] doc: interpret-trailers: rewrite
>> new-trailers paragraphs
>> Date: Wed, 10 Jun 2026 23:21:28 +0200
>> Message-ID: <>
>> X-Mailer: git-send-email 2.54.0.22.g9e26862b904
>>
>> So, I reverted to the old and battle tested way to pick these 11
>> messages manually in my newsreader to replace the topic.
>>
>> If you have a chance, could you investigate where the send-out
>> process went wrong and gave one message a bogus ID? I am worried if
>> you may have triggered a bug in send-email, in which case we would
>> want to fix it to avoid hurting other users.
>
> I’ll investigate. It’s 99.99% chance a problem on my end, created by me.
>
> Sorry for the trouble!
It was as stupid and embarrassing as it looked at first glance.
A search-and-replace on the format-patch output which was missing
a target value.
I’m very sorry again.
^ permalink raw reply
* Re: [PATCH v2] commit-reach: remove get_reachable_subset()
From: Derrick Stolee @ 2026-06-11 12:57 UTC (permalink / raw)
To: Kristofer Karlsson via GitGitGadget, git; +Cc: Kristofer Karlsson
In-Reply-To: <pull.2144.v2.git.1781178567862.gitgitgadget@gmail.com>
On 6/11/2026 7:49 AM, Kristofer Karlsson via GitGitGadget wrote:
> From: Kristofer Karlsson <krka@spotify.com>
> * Added PQ mode to the existing test-reach tests so both DFS and PQ
> paths are exercised by the test suite.
This is a substantial change that I don't think is merited. I
think that this makes the point of your change moot: we essentially
have two implementations in one complicated method instead of two
implementations that have different performance characteristics.
I'd rather leave the code as-is than take this complication. I don't
think your commit message justifies the merging of these
implementations, either.
Moreover, I thought the previous patch was fine, it just needed better
awareness of the performance implications of the change. Specifically,
it could be a regression for large repos without a commit-graph file
while simultaneously potentially being a performance boost for large
repos _with_ a commit-graph file.
_If_ we were to go this direction, then it should be two patches, with
the first introducing the new mode and testing it. The second patch
could change the callers of get_reachable_subset() and delete that
code.
Finally, a commentary: You seem to have a habit of responding to
review feedback only through new patch versions, but I'd rather see
some thoughts in the discussion thread as direct replies to the review,
especially if you think you will change direction like this. Saying
something like "Maybe I should update the method to have two walk modes"
in a reply would have given me an opportunity to respond and perhaps
avoided a new version that went in this direction.
Thanks,
-Stolee
^ permalink raw reply
* Re: [PATCH v2] update-ref: add --rename option
From: Patrick Steinhardt @ 2026-06-11 13:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20260610212800.2892146-1-gitster@pobox.com>
On Wed, Jun 10, 2026 at 02:28:00PM -0700, Junio C Hamano wrote:
> Add a "--rename" option to "git update-ref" with the syntax:
>
> $ git update-ref --rename <old-refname> <new-refname>
>
> It renames <old-refname> together with its reflog to <new-refname>
> (even when used on a local branch ref, the current value and the
> reflog of the ref are the only things that are renamed). As the
> command is a low-level plumbing command, attempts to rename branches
> are not warned, but we document it to draw attention of unsuspecting
> users and protect them from burning themselves.
This sentence reads a tiny bit awkward now and is probably an artifact
of the change between v1 and v2.
> Because the "--stdin" mode wants to operate on its refs in a
> reference transaction, and the API function refs_rename_ref() does
> not work well as part of a transaction, it is currently not possible
> to add a corresponding "rename" verb to the "--stdin" mode before
> the underlying API learns to rename refs atomically inside a
> transaction. It hence is left for a future refactoring.
Fair. I thought at times about extending reference transactions to also
fully support renames, but I never had a good use case where it would
really matter.
> * The initial draft I sent had a warning when the command is used
> to rename local branches, but that is unusual for plumbing
> commands that should do one thing it is designed for consistently
> well without being chatty. This version only has words of warning
> in the documentation.
Good, I just wanted to complain about that warning. I think it's good to
just accept this as-is. I really doubt that folks would go out of their
way to use git-update-ref(1) if all they want was to rename their
branch.
One thing that I'm missing from the commit message: what's the
motivation for this new mode?
> diff --git a/Documentation/git-update-ref.adoc b/Documentation/git-update-ref.adoc
> index 37a5019a8b..0c27efaa52 100644
> --- a/Documentation/git-update-ref.adoc
> +++ b/Documentation/git-update-ref.adoc
> @@ -39,6 +40,14 @@ the result of following the symbolic pointers.
> With `-d`, it deletes the named <ref> after verifying that it
> still contains <old-oid>.
>
> +With `--rename`, it renames <old-refname> together with its reflog to
> +<new-refname>. The command fails if <old-refname> does not exist, or
> +if <new-refname> already exists. Because `git update-ref` does not
> +update active worktree `HEAD` symbolic references or `.git/config`
> +tracking settings when you rename a local branch in the `refs/heads/`
> +hierarchy, think twice before using this command to rename a local
> +branch (use `git branch -m` instead).
I'd rephrase this slightly to first document behaviour and then draw the
conclusion that it shouldn't be used in many cases separately. For
example:
This command does not update any symbolic references pointing to
the renamed reference, and neither does it update `.git/config`
tracking settings. It is thus not recommended to use it for renaming
local branches. Use `git branch -m` instead.
> diff --git a/builtin/update-ref.c b/builtin/update-ref.c
> index 2d68c40ecb..65ee8af08c 100644
> --- a/builtin/update-ref.c
> +++ b/builtin/update-ref.c
> @@ -787,7 +789,7 @@ int cmd_update_ref(int argc,
> }
>
> if (read_stdin) {
> - if (delete || argc > 0)
> + if (delete || rename || argc > 0)
> usage_with_options(git_update_ref_usage, options);
> if (end_null)
> line_termination = '\0';
Okay, so here we make it mutually exclusive with "--delete".
> @@ -800,6 +802,32 @@ int cmd_update_ref(int argc,
> if (end_null)
> usage_with_options(git_update_ref_usage, options);
>
> + if (rename) {
> + const char *oldref, *newref;
> +
> + if (delete || argc != 2)
> + usage_with_options(git_update_ref_usage, options);
And here with "-d". Good.
> + if (!refs_ref_exists(get_main_ref_store(the_repository), oldref))
> + die("no ref named '%s'", oldref);
> +
> + if (refs_ref_exists(get_main_ref_store(the_repository), newref))
> + die("ref '%s' already exists", newref);
> +
> + if (refs_rename_ref(get_main_ref_store(the_repository),
> + oldref, newref, msg))
> + die("rename failed");
> + return 0;
> + }
Hm. I think we're not using "--deref" / "--no-deref" at all, but we
document this flag as accepted in the synopsis.
Thanks!
Patrick
^ permalink raw reply
* Re: [PATCH 0/3] config: allow disabling config includes
From: Derrick Stolee @ 2026-06-11 13:08 UTC (permalink / raw)
To: Jeff King; +Cc: Derrick Stolee via GitGitGadget, git, gitster
In-Reply-To: <20260611083943.GJ2191159@coredump.intra.peff.net>
On 6/11/2026 4:39 AM, Jeff King wrote:
> On Tue, Jun 09, 2026 at 08:59:22AM -0400, Derrick Stolee wrote:
> I'm not sure I entirely understand the problematic case, though. The
> user points to in-repo config (which we already tell people is a bad
> idea), and then that config breaks for some reason? Because the include
> is relative and git is run from another directory?
>> Or: are we venturing into territory where we don't even want to create a
>> new foot-gun? If there were another way to solve the situation that I'm
>> facing without these risks, then I'd be open to it. Any ideas?
>
> Yeah, the more I think on it, the more it seems like a foot-gun. Like I
> said, I'm not sure I entirely understand the use-case. If you could
> flesh out an example, that might help.
The case I'm struggling with is that our build system has sandboxing
restrictions to make sure the build is deterministic based on a certain
number of inputs. A tool we don't control is calling Git commands and
these users with included config are getting errors because the build
is looking at files in the repo that are not registered as build inputs.
Files within $SRCROOT/.git/ are ignored as "internal to Git" but when
the users update their config to include other files, this error occurs.
I'd much rather that this tool doesn't call Git at all, but I'm unable
to make that change to a third-party tool. But this environment variable
would make it possible to disable this behavior. And I'd also rather
that these users don't use includes in this way, but they are using a
checked-in file to share aliases and other quality-of-life things when
a human uses Git, not "critical" settings.
This series is my attempt to see if we can find a solution that enables
this behavior, but maybe we've found enough concerns with the idea that
we can push back on the users to say "stop doing that."
Thanks,
-Stolee
^ permalink raw reply
* [PATCH v5 00/10] builtin/history: introduce "drop" subcommand
From: Patrick Steinhardt @ 2026-06-11 13:27 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood
In-Reply-To: <20260601-b4-pks-history-drop-v1-0-643e32340d55@pks.im>
Hi,
this small patch series introduces the new "drop" subcommand for
git-history(1). As a reader might guess, the command does exactly that:
given a commit, it will drop that commit from the commit history and
replay descendant branches on top of it.
Changes in v5:
- Reject UPDATE_ORIG_HEAD without UPDATE_HEAD.
- Link to v4: https://patch.msgid.link/20260610-b4-pks-history-drop-v4-0-70d5f0ae8c25@pks.im
Changes in v4:
- Remove the `SKIP_REF_UPDATES` flag in favor of a new `UPDATE_HEAD`
flag, as suggested by Phillip.
- Rename `reset_head()` to `reset_working_tree()`. This better matches
the new scope of the function, and it helps us to catch any
in-flight patches that would now have to set the `UPDATE_HEAD` flag.
- Link to v3: https://patch.msgid.link/20260608-b4-pks-history-drop-v3-0-84ca8e43e937@pks.im
Changes in v3:
- Fix commit message typos.
- Make `update_orig_head` and `skip_ref_updates` mutually exclusive.
- Use fancy revisions to specify the commit to drop in the example
section.
- Detect conflicting changes in the index/working tree in dry-run
mode.
- Consistently use a subshell.
- Rename `RESET_HEAD_ORIG_HEAD` to `RESET_HEAD_UPDATE_ORIG_HEAD`.
-
- Link to v2: https://patch.msgid.link/20260603-b4-pks-history-drop-v2-0-742cb5b5176d@pks.im
Changes in v2:
- Reworked `update_worktree()` to use `reset_head()`, which required a
bunch of changes to `reset_head()`.
- Consistently mention the commit that cannot be dropped as part of
error messages.
- Adapt error message to not use backticks anymore.
- Drop redundant "--graph" flag in a test helper.
- Link to v1: https://patch.msgid.link/20260601-b4-pks-history-drop-v1-0-643e32340d55@pks.im
Thanks!
Patrick
---
Patrick Steinhardt (10):
read-cache: split out function to drop unmerged entries to stage 0
reset: drop `USE_THE_REPOSITORY_VARIABLE`
reset: rename `reset_head()`
reset: modernize flags passed to `reset_working_tree()`
reset: introduce dry-run mode
reset: introduce ability to skip updating HEAD
reset: allow the caller to specify the current HEAD object
reset: stop assuming that the caller passes in a clean index
builtin/history: split handling of ref updates into two phases
builtin/history: implement "drop" subcommand
Documentation/git-history.adoc | 38 ++-
builtin/history.c | 288 +++++++++++++++++++---
builtin/rebase.c | 41 ++--
read-cache-ll.h | 1 +
read-cache.c | 12 +-
reset.c | 102 +++++---
reset.h | 51 ++--
sequencer.c | 17 +-
t/meson.build | 1 +
t/t3454-history-drop.sh | 537 +++++++++++++++++++++++++++++++++++++++++
10 files changed, 971 insertions(+), 117 deletions(-)
Range-diff versus v4:
1: 82e26ff318 = 1: 6f74de2025 read-cache: split out function to drop unmerged entries to stage 0
2: 93e6e7276c = 2: 673658d03c reset: drop `USE_THE_REPOSITORY_VARIABLE`
3: 79c2877a65 = 3: 3a4745775a reset: rename `reset_head()`
4: 9e67c7ee84 = 4: d53df7446e reset: modernize flags passed to `reset_working_tree()`
5: 4da39e7cec = 5: 2c06054013 reset: introduce dry-run mode
6: 683897ac8d ! 6: b9f3438140 reset: introduce ability to skip updating HEAD
@@ reset.c: int reset_working_tree(struct repository *r,
unsigned update_orig_head = opts->flags & RESET_WORKING_TREE_UPDATE_ORIG_HEAD;
unsigned dry_run = opts->flags & RESET_WORKING_TREE_DRY_RUN;
struct object_id *head = NULL, head_oid;
+@@ reset.c: int reset_working_tree(struct repository *r,
+ if (opts->branch_msg && !opts->branch)
+ BUG("branch reflog message given without a branch");
+
++ if (update_orig_head && !update_head)
++ BUG("cannot update ORIG_HEAD without updating HEAD" );
++
+ if (!refs_only && !dry_run && repo_hold_locked_index(r, &lock, LOCK_REPORT_ON_ERROR) < 0) {
+ ret = -1;
+ goto leave_reset_head;
@@ reset.c: int reset_working_tree(struct repository *r,
oid = &head_oid;
7: 2d70838562 = 7: 9e7af4b77b reset: allow the caller to specify the current HEAD object
8: ad56a2d370 = 8: 90b12199e8 reset: stop assuming that the caller passes in a clean index
9: b47f608e92 = 9: a7e0e345be builtin/history: split handling of ref updates into two phases
10: f39ec74b0f = 10: 93bc3d46a4 builtin/history: implement "drop" subcommand
---
base-commit: 1666c1265231b0bc5f613fbbf3f0a9896cdef76e
change-id: 20260601-b4-pks-history-drop-28f6c6399e7b
^ permalink raw reply
* [PATCH v5 01/10] read-cache: split out function to drop unmerged entries to stage 0
From: Patrick Steinhardt @ 2026-06-11 13:27 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood
In-Reply-To: <20260611-b4-pks-history-drop-v5-0-34d35725559c@pks.im>
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.1189.g8c84645362.dirty
^ permalink raw reply related
* [PATCH v5 02/10] reset: drop `USE_THE_REPOSITORY_VARIABLE`
From: Patrick Steinhardt @ 2026-06-11 13:27 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood
In-Reply-To: <20260611-b4-pks-history-drop-v5-0-34d35725559c@pks.im>
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.1189.g8c84645362.dirty
^ permalink raw reply related
* [PATCH v5 03/10] reset: rename `reset_head()`
From: Patrick Steinhardt @ 2026-06-11 13:27 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood
In-Reply-To: <20260611-b4-pks-history-drop-v5-0-34d35725559c@pks.im>
In a subsequent commit we're about to adapt `reset_head()` so that the
reference update to HEAD is optional, only. At this point the function
starts to feel misnamed, as it doesn't necessarily have anything to do
with the HEAD reference anymore. The gist of the function then is that
we reset the working tree to a specific new commit, updating both the
index and the checked-out files.
Rename it to `reset_working_tree()` to better reflect that.
Note that we don't adjust the flags yet. This will happen in a
subsequent commit.
Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
---
builtin/rebase.c | 20 ++++++++++----------
reset.c | 5 +++--
reset.h | 4 ++--
sequencer.c | 8 ++++----
4 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index fa4f5d9306..22fbba3c62 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -592,7 +592,7 @@ static int finish_rebase(struct rebase_options *opts)
static int move_to_original_branch(struct rebase_options *opts)
{
struct strbuf branch_reflog = STRBUF_INIT, head_reflog = STRBUF_INIT;
- struct reset_head_opts ropts = { 0 };
+ struct reset_working_tree_options ropts = { 0 };
int ret;
if (!opts->head_name)
@@ -610,7 +610,7 @@ static int move_to_original_branch(struct rebase_options *opts)
ropts.flags = RESET_HEAD_REFS_ONLY;
ropts.branch_msg = branch_reflog.buf;
ropts.head_msg = head_reflog.buf;
- ret = reset_head(the_repository, &ropts);
+ ret = reset_working_tree(the_repository, &ropts);
strbuf_release(&branch_reflog);
strbuf_release(&head_reflog);
@@ -685,7 +685,7 @@ static int run_am(struct rebase_options *opts)
status = run_command(&format_patch);
if (status) {
- struct reset_head_opts ropts = { 0 };
+ struct reset_working_tree_options ropts = { 0 };
unlink(rebased_patches);
free(rebased_patches);
child_process_clear(&am);
@@ -693,7 +693,7 @@ static int run_am(struct rebase_options *opts)
ropts.oid = &opts->orig_head->object.oid;
ropts.branch = opts->head_name;
ropts.default_reflog_action = opts->reflog_action;
- reset_head(the_repository, &ropts);
+ reset_working_tree(the_repository, &ropts);
error(_("\ngit encountered an error while preparing the "
"patches to replay\n"
"these revisions:\n"
@@ -855,7 +855,7 @@ static int rebase_config(const char *var, const char *value,
static int checkout_up_to_date(struct rebase_options *options)
{
struct strbuf buf = STRBUF_INIT;
- struct reset_head_opts ropts = { 0 };
+ struct reset_working_tree_options ropts = { 0 };
int ret = 0;
strbuf_addf(&buf, "%s: checkout %s",
@@ -866,7 +866,7 @@ static int checkout_up_to_date(struct rebase_options *options)
if (!ropts.branch)
ropts.flags |= RESET_HEAD_DETACH;
ropts.head_msg = buf.buf;
- if (reset_head(the_repository, &ropts) < 0)
+ if (reset_working_tree(the_repository, &ropts) < 0)
ret = error(_("could not switch to %s"), options->switch_to);
strbuf_release(&buf);
@@ -1116,7 +1116,7 @@ int cmd_rebase(int argc,
int reschedule_failed_exec = -1;
int allow_preemptive_ff = 1;
int preserve_merges_selected = 0;
- struct reset_head_opts ropts = { 0 };
+ struct reset_working_tree_options ropts = { 0 };
struct option builtin_rebase_options[] = {
OPT_STRING(0, "onto", &options.onto_name,
N_("revision"),
@@ -1385,7 +1385,7 @@ int cmd_rebase(int argc,
rerere_clear(the_repository, &merge_rr);
string_list_clear(&merge_rr, 1);
ropts.flags = RESET_HEAD_HARD;
- if (reset_head(the_repository, &ropts) < 0)
+ if (reset_working_tree(the_repository, &ropts) < 0)
die(_("could not discard worktree changes"));
remove_branch_state(the_repository, 0);
if (read_basic_state(&options))
@@ -1410,7 +1410,7 @@ int cmd_rebase(int argc,
ropts.head_msg = head_msg.buf;
ropts.branch = options.head_name;
ropts.flags = RESET_HEAD_HARD;
- if (reset_head(the_repository, &ropts) < 0)
+ if (reset_working_tree(the_repository, &ropts) < 0)
die(_("could not move back to %s"),
oid_to_hex(&options.orig_head->object.oid));
strbuf_release(&head_msg);
@@ -1880,7 +1880,7 @@ int cmd_rebase(int argc,
RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
ropts.head_msg = msg.buf;
ropts.default_reflog_action = options.reflog_action;
- if (reset_head(the_repository, &ropts)) {
+ if (reset_working_tree(the_repository, &ropts)) {
ret = error(_("Could not detach HEAD"));
goto cleanup_autostash;
}
diff --git a/reset.c b/reset.c
index 3b3cb74dab..799596398b 100644
--- a/reset.c
+++ b/reset.c
@@ -12,7 +12,7 @@
#include "hook.h"
static int update_refs(struct repository *repo,
- const struct reset_head_opts *opts,
+ const struct reset_working_tree_options *opts,
const struct object_id *oid,
const struct object_id *head)
{
@@ -85,7 +85,8 @@ static int update_refs(struct repository *repo,
return ret;
}
-int reset_head(struct repository *r, const struct reset_head_opts *opts)
+int reset_working_tree(struct repository *r,
+ const struct reset_working_tree_options *opts)
{
const struct object_id *oid = opts->oid;
const char *switch_to_branch = opts->branch;
diff --git a/reset.h b/reset.h
index a28f81829d..f130152014 100644
--- a/reset.h
+++ b/reset.h
@@ -17,7 +17,7 @@
/* Update ORIG_HEAD as well as HEAD */
#define RESET_ORIG_HEAD (1<<4)
-struct reset_head_opts {
+struct reset_working_tree_options {
/*
* The commit to checkout/reset to. Defaults to HEAD.
*/
@@ -55,6 +55,6 @@ struct reset_head_opts {
const char *default_reflog_action;
};
-int reset_head(struct repository *r, const struct reset_head_opts *opts);
+int reset_working_tree(struct repository *r, const struct reset_working_tree_options *opts);
#endif
diff --git a/sequencer.c b/sequencer.c
index 1ee4b2875b..d73ecf0384 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4677,7 +4677,7 @@ static void create_autostash_internal(struct repository *r,
if (has_unstaged_changes(r, 1) ||
has_uncommitted_changes(r, 1)) {
struct child_process stash = CHILD_PROCESS_INIT;
- struct reset_head_opts ropts = { .flags = RESET_HEAD_HARD };
+ struct reset_working_tree_options ropts = { .flags = RESET_HEAD_HARD };
struct object_id oid;
strvec_pushl(&stash.args,
@@ -4707,7 +4707,7 @@ static void create_autostash_internal(struct repository *r,
if (!silent)
printf(_("Created autostash: %s\n"), buf.buf);
- if (reset_head(r, &ropts) < 0)
+ if (reset_working_tree(r, &ropts) < 0)
die(_("could not reset --hard"));
discard_index(r->index);
if (repo_read_index(r) < 0)
@@ -4867,7 +4867,7 @@ static int checkout_onto(struct repository *r, struct replay_opts *opts,
const char *onto_name, const struct object_id *onto,
const struct object_id *orig_head)
{
- struct reset_head_opts ropts = {
+ struct reset_working_tree_options ropts = {
.oid = onto,
.orig_head = orig_head,
.flags = RESET_HEAD_DETACH | RESET_ORIG_HEAD |
@@ -4876,7 +4876,7 @@ static int checkout_onto(struct repository *r, struct replay_opts *opts,
onto_name),
.default_reflog_action = sequencer_reflog_action(opts)
};
- if (reset_head(r, &ropts)) {
+ if (reset_working_tree(r, &ropts)) {
apply_autostash(rebase_path_autostash());
sequencer_remove_state(opts);
return error(_("could not detach HEAD"));
--
2.54.0.1189.g8c84645362.dirty
^ permalink raw reply related
* [PATCH v5 04/10] reset: modernize flags passed to `reset_working_tree()`
From: Patrick Steinhardt @ 2026-06-11 13:27 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood
In-Reply-To: <20260611-b4-pks-history-drop-v5-0-34d35725559c@pks.im>
The flags passed to `reset_working_tree()` are declared as defines. This
has fallen a bit out of practice nowadays, where we instead prefer to
use enums. Furthermore, the prefix of those flags does not match the
function name anymore after the rename in the preceding commit.
Adapt the code to follow modern best practices and adapt the flag names.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/rebase.c | 15 ++++++++-------
reset.c | 12 ++++++------
reset.h | 31 +++++++++++++++++++------------
sequencer.c | 9 ++++++---
4 files changed, 39 insertions(+), 28 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 22fbba3c62..06dcbaf5e8 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -607,7 +607,7 @@ static int move_to_original_branch(struct rebase_options *opts)
strbuf_addf(&head_reflog, "%s (finish): returning to %s",
opts->reflog_action, opts->head_name);
ropts.branch = opts->head_name;
- ropts.flags = RESET_HEAD_REFS_ONLY;
+ ropts.flags = RESET_WORKING_TREE_REFS_ONLY;
ropts.branch_msg = branch_reflog.buf;
ropts.head_msg = head_reflog.buf;
ret = reset_working_tree(the_repository, &ropts);
@@ -862,9 +862,9 @@ static int checkout_up_to_date(struct rebase_options *options)
options->reflog_action, options->switch_to);
ropts.oid = &options->orig_head->object.oid;
ropts.branch = options->head_name;
- ropts.flags = RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
+ ropts.flags = RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK;
if (!ropts.branch)
- ropts.flags |= RESET_HEAD_DETACH;
+ ropts.flags |= RESET_WORKING_TREE_DETACH;
ropts.head_msg = buf.buf;
if (reset_working_tree(the_repository, &ropts) < 0)
ret = error(_("could not switch to %s"), options->switch_to);
@@ -1384,7 +1384,7 @@ int cmd_rebase(int argc,
rerere_clear(the_repository, &merge_rr);
string_list_clear(&merge_rr, 1);
- ropts.flags = RESET_HEAD_HARD;
+ ropts.flags = RESET_WORKING_TREE_HARD;
if (reset_working_tree(the_repository, &ropts) < 0)
die(_("could not discard worktree changes"));
remove_branch_state(the_repository, 0);
@@ -1409,7 +1409,7 @@ int cmd_rebase(int argc,
ropts.oid = &options.orig_head->object.oid;
ropts.head_msg = head_msg.buf;
ropts.branch = options.head_name;
- ropts.flags = RESET_HEAD_HARD;
+ ropts.flags = RESET_WORKING_TREE_HARD;
if (reset_working_tree(the_repository, &ropts) < 0)
die(_("could not move back to %s"),
oid_to_hex(&options.orig_head->object.oid));
@@ -1876,8 +1876,9 @@ 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 |
- RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
+ ropts.flags = RESET_WORKING_TREE_DETACH |
+ RESET_WORKING_TREE_UPDATE_ORIG_HEAD |
+ RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK;
ropts.head_msg = msg.buf;
ropts.default_reflog_action = options.reflog_action;
if (reset_working_tree(the_repository, &ropts)) {
diff --git a/reset.c b/reset.c
index 799596398b..4ca7f23a25 100644
--- a/reset.c
+++ b/reset.c
@@ -16,9 +16,9 @@ static int update_refs(struct repository *repo,
const struct object_id *oid,
const struct object_id *head)
{
- 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 detach_head = opts->flags & RESET_WORKING_TREE_DETACH;
+ unsigned run_hook = opts->flags & RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK;
+ unsigned update_orig_head = opts->flags & RESET_WORKING_TREE_UPDATE_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;
@@ -90,9 +90,9 @@ int reset_working_tree(struct repository *r,
{
const struct object_id *oid = opts->oid;
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 reset_hard = opts->flags & RESET_WORKING_TREE_HARD;
+ unsigned refs_only = opts->flags & RESET_WORKING_TREE_REFS_ONLY;
+ unsigned update_orig_head = opts->flags & RESET_WORKING_TREE_UPDATE_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 f130152014..2e5826de99 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_working_tree_flags {
+ /* Request a detached checkout */
+ RESET_WORKING_TREE_DETACH = (1 << 0),
+
+ /* Request a reset rather than a checkout */
+ RESET_WORKING_TREE_HARD = (1 << 1),
+
+ /* Run the post-checkout hook */
+ RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK = (1 << 2),
+
+ /* Only update refs, do not touch the worktree */
+ RESET_WORKING_TREE_REFS_ONLY = (1 << 3),
+
+ /* Update ORIG_HEAD as well as HEAD */
+ RESET_WORKING_TREE_UPDATE_ORIG_HEAD = (1 << 4),
+};
struct reset_working_tree_options {
/*
@@ -33,7 +39,7 @@ struct reset_working_tree_options {
/*
* Flags defined above.
*/
- unsigned flags;
+ enum reset_working_tree_flags flags;
/*
* Optional reflog message for branch, defaults to head_msg.
*/
@@ -45,7 +51,8 @@ struct reset_working_tree_options {
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_WORKING_TREE_UPDATE_ORIG_HEAD then
+ * default_reflog_action must be given.
*/
const char *orig_head_msg;
/*
diff --git a/sequencer.c b/sequencer.c
index d73ecf0384..4efe831178 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4677,7 +4677,9 @@ static void create_autostash_internal(struct repository *r,
if (has_unstaged_changes(r, 1) ||
has_uncommitted_changes(r, 1)) {
struct child_process stash = CHILD_PROCESS_INIT;
- struct reset_working_tree_options ropts = { .flags = RESET_HEAD_HARD };
+ struct reset_working_tree_options ropts = {
+ .flags = RESET_WORKING_TREE_HARD,
+ };
struct object_id oid;
strvec_pushl(&stash.args,
@@ -4870,8 +4872,9 @@ static int checkout_onto(struct repository *r, struct replay_opts *opts,
struct reset_working_tree_options ropts = {
.oid = onto,
.orig_head = orig_head,
- .flags = RESET_HEAD_DETACH | RESET_ORIG_HEAD |
- RESET_HEAD_RUN_POST_CHECKOUT_HOOK,
+ .flags = RESET_WORKING_TREE_DETACH |
+ RESET_WORKING_TREE_UPDATE_ORIG_HEAD |
+ RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK,
.head_msg = reflog_message(opts, "start", "checkout %s",
onto_name),
.default_reflog_action = sequencer_reflog_action(opts)
--
2.54.0.1189.g8c84645362.dirty
^ permalink raw reply related
* [PATCH v5 05/10] reset: introduce dry-run mode
From: Patrick Steinhardt @ 2026-06-11 13:27 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood
In-Reply-To: <20260611-b4-pks-history-drop-v5-0-34d35725559c@pks.im>
In a subsequent commit we'll add another caller to `reset_working_tree()`
that wants to perform a dry-run check of whether it would be possible to
update 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 4ca7f23a25..99f2c1b012 100644
--- a/reset.c
+++ b/reset.c
@@ -93,11 +93,14 @@ int reset_working_tree(struct repository *r,
unsigned reset_hard = opts->flags & RESET_WORKING_TREE_HARD;
unsigned refs_only = opts->flags & RESET_WORKING_TREE_REFS_ONLY;
unsigned update_orig_head = opts->flags & RESET_WORKING_TREE_UPDATE_ORIG_HEAD;
+ unsigned dry_run = opts->flags & RESET_WORKING_TREE_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;
@@ -110,7 +113,7 @@ int reset_working_tree(struct repository *r,
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;
}
@@ -125,16 +128,36 @@ int reset_working_tree(struct repository *r,
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;
@@ -142,11 +165,6 @@ int reset_working_tree(struct repository *r,
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));
@@ -163,6 +181,9 @@ int reset_working_tree(struct repository *r,
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));
@@ -182,6 +203,7 @@ int reset_working_tree(struct repository *r,
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 2e5826de99..898e4a1e95 100644
--- a/reset.h
+++ b/reset.h
@@ -21,6 +21,12 @@ enum reset_working_tree_flags {
/* Update ORIG_HEAD as well as HEAD */
RESET_WORKING_TREE_UPDATE_ORIG_HEAD = (1 << 4),
+
+ /*
+ * Perform a dry-run by performing the operation without updating
+ * any user-visible state.
+ */
+ RESET_WORKING_TREE_DRY_RUN = (1 << 5),
};
struct reset_working_tree_options {
--
2.54.0.1189.g8c84645362.dirty
^ permalink raw reply related
* [PATCH v5 06/10] reset: introduce ability to skip updating HEAD
From: Patrick Steinhardt @ 2026-06-11 13:27 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood
In-Reply-To: <20260611-b4-pks-history-drop-v5-0-34d35725559c@pks.im>
In a subsequent commit we'll introduce a new caller to
`reset_working_tree()` that really only wants to update the index and
working tree, without updating any references. Introduce a new flag that
makes the caller opt in to updating HEAD and adapt all callers to set
that flag.
Note that in a previous iteration we instead introduced a flag that made
callers opt out of updating any references. This was somewhat awkward
though because we already have the `UPDATE_ORIG_HEAD` flag, so the
result was somewhat inconsistent.
Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/rebase.c | 14 ++++++++++----
reset.c | 9 +++++++--
reset.h | 9 ++++++---
sequencer.c | 4 +++-
4 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 06dcbaf5e8..10a306310c 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -607,7 +607,8 @@ static int move_to_original_branch(struct rebase_options *opts)
strbuf_addf(&head_reflog, "%s (finish): returning to %s",
opts->reflog_action, opts->head_name);
ropts.branch = opts->head_name;
- ropts.flags = RESET_WORKING_TREE_REFS_ONLY;
+ ropts.flags = RESET_WORKING_TREE_REFS_ONLY |
+ RESET_WORKING_TREE_UPDATE_HEAD;
ropts.branch_msg = branch_reflog.buf;
ropts.head_msg = head_reflog.buf;
ret = reset_working_tree(the_repository, &ropts);
@@ -693,6 +694,7 @@ static int run_am(struct rebase_options *opts)
ropts.oid = &opts->orig_head->object.oid;
ropts.branch = opts->head_name;
ropts.default_reflog_action = opts->reflog_action;
+ ropts.flags = RESET_WORKING_TREE_UPDATE_HEAD;
reset_working_tree(the_repository, &ropts);
error(_("\ngit encountered an error while preparing the "
"patches to replay\n"
@@ -862,7 +864,8 @@ static int checkout_up_to_date(struct rebase_options *options)
options->reflog_action, options->switch_to);
ropts.oid = &options->orig_head->object.oid;
ropts.branch = options->head_name;
- ropts.flags = RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK;
+ ropts.flags = RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK |
+ RESET_WORKING_TREE_UPDATE_HEAD;
if (!ropts.branch)
ropts.flags |= RESET_WORKING_TREE_DETACH;
ropts.head_msg = buf.buf;
@@ -1384,7 +1387,8 @@ int cmd_rebase(int argc,
rerere_clear(the_repository, &merge_rr);
string_list_clear(&merge_rr, 1);
- ropts.flags = RESET_WORKING_TREE_HARD;
+ ropts.flags = RESET_WORKING_TREE_HARD |
+ RESET_WORKING_TREE_UPDATE_HEAD;
if (reset_working_tree(the_repository, &ropts) < 0)
die(_("could not discard worktree changes"));
remove_branch_state(the_repository, 0);
@@ -1409,7 +1413,8 @@ int cmd_rebase(int argc,
ropts.oid = &options.orig_head->object.oid;
ropts.head_msg = head_msg.buf;
ropts.branch = options.head_name;
- ropts.flags = RESET_WORKING_TREE_HARD;
+ ropts.flags = RESET_WORKING_TREE_HARD |
+ RESET_WORKING_TREE_UPDATE_HEAD;
if (reset_working_tree(the_repository, &ropts) < 0)
die(_("could not move back to %s"),
oid_to_hex(&options.orig_head->object.oid));
@@ -1877,6 +1882,7 @@ int cmd_rebase(int argc,
ropts.oid = &options.onto->object.oid;
ropts.orig_head = &options.orig_head->object.oid;
ropts.flags = RESET_WORKING_TREE_DETACH |
+ RESET_WORKING_TREE_UPDATE_HEAD |
RESET_WORKING_TREE_UPDATE_ORIG_HEAD |
RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK;
ropts.head_msg = msg.buf;
diff --git a/reset.c b/reset.c
index 99f2c1b012..a744b0f0fc 100644
--- a/reset.c
+++ b/reset.c
@@ -92,6 +92,7 @@ int reset_working_tree(struct repository *r,
const char *switch_to_branch = opts->branch;
unsigned reset_hard = opts->flags & RESET_WORKING_TREE_HARD;
unsigned refs_only = opts->flags & RESET_WORKING_TREE_REFS_ONLY;
+ unsigned update_head = opts->flags & RESET_WORKING_TREE_UPDATE_HEAD;
unsigned update_orig_head = opts->flags & RESET_WORKING_TREE_UPDATE_ORIG_HEAD;
unsigned dry_run = opts->flags & RESET_WORKING_TREE_DRY_RUN;
struct object_id *head = NULL, head_oid;
@@ -113,6 +114,9 @@ int reset_working_tree(struct repository *r,
if (opts->branch_msg && !opts->branch)
BUG("branch reflog message given without a branch");
+ if (update_orig_head && !update_head)
+ BUG("cannot update ORIG_HEAD without updating HEAD" );
+
if (!refs_only && !dry_run && repo_hold_locked_index(r, &lock, LOCK_REPORT_ON_ERROR) < 0) {
ret = -1;
goto leave_reset_head;
@@ -129,7 +133,7 @@ int reset_working_tree(struct repository *r,
oid = &head_oid;
if (refs_only) {
- if (!dry_run)
+ if (update_head)
return update_refs(r, opts, oid, head);
return 0;
}
@@ -197,7 +201,8 @@ int reset_working_tree(struct repository *r,
goto leave_reset_head;
}
- if (oid != &head_oid || update_orig_head || switch_to_branch)
+ if (update_head &&
+ (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 898e4a1e95..38b2891b53 100644
--- a/reset.h
+++ b/reset.h
@@ -19,14 +19,17 @@ enum reset_working_tree_flags {
/* Only update refs, do not touch the worktree */
RESET_WORKING_TREE_REFS_ONLY = (1 << 3),
- /* Update ORIG_HEAD as well as HEAD */
- RESET_WORKING_TREE_UPDATE_ORIG_HEAD = (1 << 4),
+ /* Update HEAD */
+ RESET_WORKING_TREE_UPDATE_HEAD = (1 << 4),
+
+ /* Update ORIG_HEAD */
+ RESET_WORKING_TREE_UPDATE_ORIG_HEAD = (1 << 5),
/*
* Perform a dry-run by performing the operation without updating
* any user-visible state.
*/
- RESET_WORKING_TREE_DRY_RUN = (1 << 5),
+ RESET_WORKING_TREE_DRY_RUN = (1 << 6),
};
struct reset_working_tree_options {
diff --git a/sequencer.c b/sequencer.c
index 4efe831178..e905b1b2d9 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4678,7 +4678,8 @@ static void create_autostash_internal(struct repository *r,
has_uncommitted_changes(r, 1)) {
struct child_process stash = CHILD_PROCESS_INIT;
struct reset_working_tree_options ropts = {
- .flags = RESET_WORKING_TREE_HARD,
+ .flags = RESET_WORKING_TREE_HARD |
+ RESET_WORKING_TREE_UPDATE_HEAD,
};
struct object_id oid;
@@ -4873,6 +4874,7 @@ static int checkout_onto(struct repository *r, struct replay_opts *opts,
.oid = onto,
.orig_head = orig_head,
.flags = RESET_WORKING_TREE_DETACH |
+ RESET_WORKING_TREE_UPDATE_HEAD |
RESET_WORKING_TREE_UPDATE_ORIG_HEAD |
RESET_WORKING_TREE_RUN_POST_CHECKOUT_HOOK,
.head_msg = reflog_message(opts, "start", "checkout %s",
--
2.54.0.1189.g8c84645362.dirty
^ permalink raw reply related
* [PATCH v5 07/10] reset: allow the caller to specify the current HEAD object
From: Patrick Steinhardt @ 2026-06-11 13:27 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood
In-Reply-To: <20260611-b4-pks-history-drop-v5-0-34d35725559c@pks.im>
When calling `reset_working_tree()` 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 a744b0f0fc..955ea4ed5a 100644
--- a/reset.c
+++ b/reset.c
@@ -122,7 +122,10 @@ int reset_working_tree(struct repository *r,
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 38b2891b53..4c992ba671 100644
--- a/reset.h
+++ b/reset.h
@@ -37,6 +37,11 @@ struct reset_working_tree_options {
* 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.1189.g8c84645362.dirty
^ permalink raw reply related
* [PATCH v5 08/10] reset: stop assuming that the caller passes in a clean index
From: Patrick Steinhardt @ 2026-06-11 13:27 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood
In-Reply-To: <20260611-b4-pks-history-drop-v5-0-34d35725559c@pks.im>
In 652bd0211d (rebase: use 'skip_cache_tree_update' option, 2022-11-10),
we updated `reset_working_tree()` 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_working_tree()`, 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 955ea4ed5a..5a9ed807d7 100644
--- a/reset.c
+++ b/reset.c
@@ -167,10 +167,11 @@ int reset_working_tree(struct repository *r,
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"),
@@ -197,7 +198,8 @@ int reset_working_tree(struct repository *r,
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.1189.g8c84645362.dirty
^ permalink raw reply related
* [PATCH v5 09/10] builtin/history: split handling of ref updates into two phases
From: Patrick Steinhardt @ 2026-06-11 13:27 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood
In-Reply-To: <20260611-b4-pks-history-drop-v5-0-34d35725559c@pks.im>
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.1189.g8c84645362.dirty
^ permalink raw reply related
* [PATCH v5 10/10] builtin/history: implement "drop" subcommand
From: Patrick Steinhardt @ 2026-06-11 13:27 UTC (permalink / raw)
To: git; +Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood
In-Reply-To: <20260611-b4-pks-history-drop-v5-0-34d35725559c@pks.im>
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 | 186 ++++++++++++++
t/meson.build | 1 +
t/t3454-history-drop.sh | 537 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 761 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc
index 2ba8121795..28b477cd37 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 'main^{/second}'
+
+$ 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..e59f69233b 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,193 @@ 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_working_tree_options opts = {
+ .oid_from = &old_head->object.oid,
+ .oid = &new_head->object.oid,
+ };
+ if (dry_run)
+ opts.flags |= RESET_WORKING_TREE_DRY_RUN;
+ return reset_working_tree(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 (!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 (!dry_run && 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 +1199,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..0f33247212
--- /dev/null
+++ b/t/t3454-history-drop.sh
@@ -0,0 +1,537 @@
+#!/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 &&
+ (
+ cd repo &&
+ test_commit initial &&
+ test_commit second &&
+ test_must_fail git 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 '--dry-run detects conflicts with modified working tree' '
+ test_when_finished "rm -rf repo" &&
+ git init repo --initial-branch=main &&
+ (
+ cd repo &&
+ test_commit first &&
+ test_commit second modify-me &&
+ echo modified >modify-me &&
+
+ git refs list >refs-expect &&
+ git diff >diff-expect &&
+ test_must_fail git history drop --dry-run HEAD 2>err &&
+ test_grep "dropping this commit would overwrite local changes" err &&
+ git diff >diff-actual &&
+ git refs list >refs-actual &&
+
+ test_cmp diff-expect diff-actual &&
+ test_cmp refs-expect refs-actual
+ )
+'
+
+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.1189.g8c84645362.dirty
^ permalink raw reply related
* Re: [PATCH v2 3/3] doc: git-config: escape erroneous highlight markup
From: Tuomas Ahola @ 2026-06-11 13:28 UTC (permalink / raw)
To: Jeff King; +Cc: git, Kristoffer Haugsbakk, Junio C Hamano, Jean-Noël Avila
In-Reply-To: <20260611083139.GA2237523@coredump.intra.peff.net>
Jeff King <peff@peff.net> wrote:
> On Thu, Jun 11, 2026 at 11:02:42AM +0300, Tuomas Ahola wrote:
>
> > > > If _<message>_ begins with one or more whitespaces followed
> > > > -by "#", it is used as-is. If it begins with "#", a space is
> > > > +by "\#", it is used as-is. If it begins with "\#", a space is
> > > > prepended before it is used. Otherwise, a string " # " (a
> > > > space followed by a hash followed by a space) is prepended
> > >
> > > I saw the comment on round 1 about this second "#" on the line. But
> > > while we are here, should we be doing the one in the context, too?
> > >
> > > -Peff
> >
> > It seems adding that second backslash was already too much, as doc-diff (which
> > I neglected to run before submitting V2) shows:
> >
> > ```
> > $ ./doc-diff V1 V2
> >
> > If <message> begins with one or more whitespaces followed by "#",
> > - it is used as-is. If it begins with "#", a space is prepended
> > + it is used as-is. If it begins with "\#", a space is prepended
> > before it is used. Otherwise, a string " # " (a space followed by a
> > hash followed by a space) is prepended to it. The resulting string
> > is placed immediately after the value defined for the variable. The
> > ```
>
> Heh, it would not be the first time I am baffled by asciidoc's parsing. :)
>
> Adding a backslash to the third instance "fixes" the second one to me,
> but I wouldn't want to rely on that (plus it breaks the third instance).
>
> Using backticks does work, though it always opens a typographical
> question. When reading the source, you see `#`, so you get a punctuation
> delimiter but no typographical one. In the rendered output, you'll see
> it in a typewriter font (assuming we fix the config issue), but we'd
> lose the visible punctuation. I could live with that.
>
> But for " # ", it gets weirder. We need punctuation to call out the
> spaces, but what should happen to the quotes? They are not really part
> of the literal string, so should they go inside or outside the
> backticks? I think it may be a moot point as "` # `" is not parsed as
> you might hope by asciidoc. Doing `" # "` does work, and is probably OK
> enough here.
>
> -Peff
Okay, I'll change those to `#` and `" # "`. That seems to be the best,
or the least bad option right now. :-)
--Tuomas
^ permalink raw reply
* Re: [PATCH v4 0/8] Auto-configure advertised remotes via URL allowlist
From: Christian Couder @ 2026-06-11 13:29 UTC (permalink / raw)
To: Junio C Hamano
Cc: Toon Claes, git, Patrick Steinhardt, Taylor Blau, Karthik Nayak,
Elijah Newren, Kristoffer Haugsbakk
In-Reply-To: <xmqqh5naxwfc.fsf@gitster.g>
On Wed, Jun 10, 2026 at 5:11 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Toon Claes <toon@iotcl.com> writes:
> > Thanks for explaining, I still agree moving on like this.
>
> Sounds good. Shall we mark the topic for 'next' then?
Yes please. I think it's ready.
Thanks.
^ permalink raw reply
* Re: [PATCH v2] commit-reach: remove get_reachable_subset()
From: Kristofer Karlsson @ 2026-06-11 13:52 UTC (permalink / raw)
To: Derrick Stolee; +Cc: Kristofer Karlsson via GitGitGadget, git
In-Reply-To: <ffaf26b1-c55e-43c7-84b6-f810a54f7717@gmail.com>
On Thu, 11 Jun 2026 at 14:57, Derrick Stolee <stolee@gmail.com> wrote:
> Finally, a commentary: You seem to have a habit of responding to
> review feedback only through new patch versions, but I'd rather see
> some thoughts in the discussion thread as direct replies to the review,
> especially if you think you will change direction like this. Saying
> something like "Maybe I should update the method to have two walk modes"
> in a reply would have given me an opportunity to respond and perhaps
> avoided a new version that went in this direction.
That's fair, I apologize both for jumping ahead too quickly with a new
patch and also for evolving it into multiple logical changes
(both code removal and complex refactoring).
I will be more mindful going forward about letting the discussion
settle more before submitting followup patches.
I have no strong opinion on how to proceed - either park/abandon this
or go with v1 as-is. I think you're right that having two modes within
tips_reachable_from_bases is reducing the win here unless the mode is
truly seamless but the abstraction does leak through a bit.
Thanks,
Kristofer
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox