* Re: [PATCH v4 03/10] reset: rename `reset_head()`
From: Phillip Wood @ 2026-06-10 13:10 UTC (permalink / raw)
To: Patrick Steinhardt, git
Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood
In-Reply-To: <20260610-b4-pks-history-drop-v4-3-70d5f0ae8c25@pks.im>
Hi Patrick
On 10/06/2026 09:52, Patrick Steinhardt wrote:
> 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.
That sounds good. Because we defer renaming the flags this patch is very
straight forward.
Thanks
Phillip
> 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"));
>
^ permalink raw reply
* Re: [PATCH v4 04/10] reset: modernize flags passed to `reset_working_tree()`
From: Phillip Wood @ 2026-06-10 13:10 UTC (permalink / raw)
To: Patrick Steinhardt, git
Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood
In-Reply-To: <20260610-b4-pks-history-drop-v4-4-70d5f0ae8c25@pks.im>
Hi Patrick
On 10/06/2026 09:52, Patrick Steinhardt wrote:
> 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.
This looks good
Thanks
Phillip
>
> 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)
>
^ permalink raw reply
* Re: [PATCH v4 06/10] reset: introduce ability to skip updating HEAD
From: Phillip Wood @ 2026-06-10 13:11 UTC (permalink / raw)
To: Patrick Steinhardt, git
Cc: Pablo Sabater, Junio C Hamano, Kristoffer Haugsbakk, Phillip Wood
In-Reply-To: <20260610-b4-pks-history-drop-v4-6-70d5f0ae8c25@pks.im>
Hi Patrick
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.
Thanks
Phillip
> Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> builtin/rebase.c | 14 ++++++++++----
> reset.c | 6 ++++--
> reset.h | 9 ++++++---
> sequencer.c | 4 +++-
> 4 files changed, 23 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..3ac99a51c0 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;
> @@ -129,7 +130,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 +198,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",
>
^ permalink raw reply
* Re: [PATCH v4] git-gui: silence install recipes under "make -s"
From: Harald Nordgren @ 2026-06-10 13:19 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, Harald Nordgren via GitGitGadget
In-Reply-To: <950f70ea-1615-402f-9cd4-3317bf177c5c@kdbg.org>
On Sat, Jun 6, 2026 at 1:47 PM Johannes Sixt <j6t@kdbg.org> wrote:
>
> Thanks, queued.
>
> -- Hannes
>
Hi!
Thanks for the help!
What does it mean for it to be queued here, should I expect it to show
up on seen or next?
Harald
^ permalink raw reply
* Re: [PATCH v2] Makefile: dedup archives in $(LIBS) so link recipes don't repeat them
From: Harald Nordgren @ 2026-06-10 13:24 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, Harald Nordgren via GitGitGadget
In-Reply-To: <pull.2314.v2.git.git.1780610623006.gitgitgadget@gmail.com>
Hi Johannes!
Maybe this could be interesting for you to look at too.
Harald
On Fri, Jun 5, 2026 at 12:03 AM Harald Nordgren via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> A handful of link recipes listed archive files twice: once explicitly
> via $(filter %.a,$^) and again implicitly through $(LIBS), which
> expanded to $(filter-out %.o,$(GITLIBS)) $(EXTLIBS). On macOS the
> linker warned about the duplicates:
>
> ld: warning: ignoring duplicate libraries: 'libgit.a', 'target/release/libgitcore.a'
>
> Redefine $(LIBS) to list archive prerequisites from $^ first, then
> the rest of the library list with those archives filtered out so each
> appears only once.
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
> Makefile: drop duplicate %.a from test-helper link rule
>
> Redefine $(LIBS) to list archive prerequisites from $^ first, then the
> rest of the library list to avoid brittleness in the future.
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2314%2FHaraldNordgren%2Fmakefile-test-helper-dedup-libs-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2314/HaraldNordgren/makefile-test-helper-dedup-libs-v2
> Pull-Request: https://github.com/git/git/pull/2314
>
> Range-diff vs v1:
>
> 1: f6166450b0 ! 1: 0ef442ea05 Makefile: drop duplicate %.a from link recipes
> @@ Metadata
> Author: Harald Nordgren <haraldnordgren@gmail.com>
>
> ## Commit message ##
> - Makefile: drop duplicate %.a from link recipes
> + Makefile: dedup archives in $(LIBS) so link recipes don't repeat them
>
> - Three link recipes list archive files twice on the link line: once
> - via $(filter %.a,$^) and again through $(LIBS), which expands to
> - $(filter-out %.o,$(GITLIBS)) $(EXTLIBS). On macOS the linker warns
> - about the duplicates:
> + A handful of link recipes listed archive files twice: once explicitly
> + via $(filter %.a,$^) and again implicitly through $(LIBS), which
> + expanded to $(filter-out %.o,$(GITLIBS)) $(EXTLIBS). On macOS the
> + linker warned about the duplicates:
>
> ld: warning: ignoring duplicate libraries: 'libgit.a', 'target/release/libgitcore.a'
>
> - Drop the redundant filter from the test-helper, fuzz-program, and
> - unit-test recipes so they match the pattern used by other link
> - recipes in the file.
> + Redefine $(LIBS) to list archive prerequisites from $^ first, then
> + the rest of the library list with those archives filtered out so each
> + appears only once.
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
>
> ## Makefile ##
> +@@ Makefile: endif
> + #
> + # where we use it as a dependency. Since we also pull object files
> + # from the dependency list, that would make each entry appear twice.
> +-LIBS = $(filter-out %.o, $(GITLIBS)) $(EXTLIBS)
> ++# Archives from $^ come first, then the rest with those archives
> ++# filtered out so each appears only once.
> ++LIBS = $(filter %.a,$^) $(filter-out $(filter %.a,$^),$(filter-out %.o,$(GITLIBS)) $(EXTLIBS))
> +
> + BASIC_CFLAGS += $(COMPAT_CFLAGS)
> + LIB_OBJS += $(COMPAT_OBJS)
> @@ Makefile: perf: all
> t/helper/test-tool$X: $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS)) $(UNIT_TEST_DIR)/test-lib.o
>
>
>
> Makefile | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index b31ecb0756..a828a66f28 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2503,7 +2503,9 @@ endif
> #
> # where we use it as a dependency. Since we also pull object files
> # from the dependency list, that would make each entry appear twice.
> -LIBS = $(filter-out %.o, $(GITLIBS)) $(EXTLIBS)
> +# Archives from $^ come first, then the rest with those archives
> +# filtered out so each appears only once.
> +LIBS = $(filter %.a,$^) $(filter-out $(filter %.a,$^),$(filter-out %.o,$(GITLIBS)) $(EXTLIBS))
>
> BASIC_CFLAGS += $(COMPAT_CFLAGS)
> LIB_OBJS += $(COMPAT_OBJS)
> @@ -3392,7 +3394,7 @@ perf: all
> t/helper/test-tool$X: $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS)) $(UNIT_TEST_DIR)/test-lib.o
>
> t/helper/test-%$X: t/helper/test-%.o GIT-LDFLAGS $(GITLIBS)
> - $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS)
> + $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
>
> check-sha1:: t/helper/test-tool$X
> t/helper/test-sha1.sh
> @@ -4015,13 +4017,13 @@ fuzz-all: $(FUZZ_PROGRAMS)
> $(FUZZ_PROGRAMS): %: %.o oss-fuzz/dummy-cmd-main.o $(GITLIBS) GIT-LDFLAGS
> $(QUIET_LINK)$(FUZZ_CXX) $(FUZZ_CXXFLAGS) -o $@ $(ALL_LDFLAGS) \
> -Wl,--allow-multiple-definition \
> - $(filter %.o,$^) $(filter %.a,$^) $(LIBS) $(LIB_FUZZING_ENGINE)
> + $(filter %.o,$^) $(LIBS) $(LIB_FUZZING_ENGINE)
>
> $(UNIT_TEST_PROGS): $(UNIT_TEST_BIN)/%$X: $(UNIT_TEST_DIR)/%.o $(UNIT_TEST_OBJS) \
> $(GITLIBS) GIT-LDFLAGS
> $(call mkdir_p_parent_template)
> $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \
> - $(filter %.o,$^) $(filter %.a,$^) $(LIBS)
> + $(filter %.o,$^) $(LIBS)
>
> GIT-TEST-SUITES: FORCE
> @FLAGS='$(CLAR_TEST_SUITES)'; \
>
> base-commit: 9ac3f193c05c2237e2b14ebaa1149e9fc8a1abe0
> --
> gitgitgadget
^ permalink raw reply
* Re: trailers: --only-trailers normalizes URLs to trailers
From: Kristoffer Haugsbakk @ 2026-06-10 14:21 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20260609004340.GF358144@coredump.intra.peff.net>
On Tue, Jun 9, 2026, at 02:43, Jeff King wrote:
> On Thu, Jun 04, 2026 at 11:27:51PM +0200, Kristoffer Haugsbakk wrote:
>
>> The following is a bug that follows straightforwardly from the documented
>> or discussed behavior. In that sense it is not a bug. But it is a bug in
>> the sense that it makes things inconvenient and violates a design goal.
>
> Yeah, though if you'll allow me to nitpick your subject a moment: I
> don't think --only-trailers is really the culprit here. It demonstrates
> the problem because it normalizes the "trailer" it found. But the loose
> trailer matching is the more fundamental issue. For example:
>
>[snip]
Yeah, this is more precise. I focused a ton on the normalized output
because that’s what makes it obvious. But the fundamental problem is
interpreting URLs like trailers.
>
>> > What's different between what you expected and what actually happened?
>>
>> In an ideal world to have some special-casing of URLs so that they are
>> not detected as trailers. Does anyone realistically want trailers like
>> this?:
>>
>> file: //...
>> http: //...
>> https: //...
>
> I could even see those as trailers, if somebody really wanted to allow
> arbitrary values that might just happen to start with "//". But without
> the whitespace after the colon, it is quite questionable.
>
>> Just special-casing `https` would go a long way.
>
> Agreed, though I think a rule like: ":// (with no whitespace)" is not a
> valid separator. Something like this:
Yes, matching on `://` strictly is a better proposal. No need to care
about `http`, `https`, `file`, etc. And both of these would *still* have
to be true for this change to be a false negative w.r.t. the user’s
intentions:
• They really input a trailer that looks like a URL, but it’s not meant
to be a URL
• They really wanted the value to start with `//`
And again I don’t think that is likely to ever happen (with a knock
on wood).
Thanks!
>
> diff --git a/trailer.c b/trailer.c
> index 6d8ec7fa8d..342ed81c78 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -635,8 +635,12 @@ static ssize_t find_separator(const char *line,
> const char *separators)
> int whitespace_found = 0;
> const char *c;
> for (c = line; *c; c++) {
> - if (strchr(separators, *c))
> + if (strchr(separators, *c)) {
> + /* special case to avoid accidental URL matches */
> + if (*c == ':' && c[1] == '/' && c[2] == '/')
> + return -1;
> return c - line;
> + }
> if (!whitespace_found && (isalnum(*c) || *c == '-'))
> continue;
> if (c != line && (*c == ' ' || *c == '\t')) {
^ permalink raw reply
* Re: [PATCH 3/3] replay: offer an option to linearize the commit topology
From: Toon Claes @ 2026-06-10 14:26 UTC (permalink / raw)
To: Junio C Hamano, Johannes Schindelin; +Cc: git
In-Reply-To: <xmqqtsrcvnjw.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Toon Claes <toon@iotcl.com> writes:
>
>> From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
>>
>> One of the stated goals of git-replay(1) is to allow implementing the
>> git-rebase(1) functionality on the server side.
>>
>> The default mode of git-rebase(1) is to act as if `--no-rebase-merges`
>> was given. This mode drops merge commits instead of replaying them, and
>> linearized the commit history into a sequence of the
>> regular (single-parent) commits.
>
> "linearized" -> "linearizes"?
Thanks.
>>
>> Add option `--linearize` to git-replay(1) do the same.
>
> "do the same" -> "to do the same"?
Ack.
>> Co-authored-by: Toon Claes <toon@iotcl.com>
>
> There is no sign-off by any of the authors?
My bad. I'll add mine.
@Johannes, can I re-add yours? I've removed it because I've made some
changes on top of the patch you wrote, but if you agree, I'll add your
Sign-off back.
>> @@ -430,12 +435,20 @@ int replay_revisions(struct rev_info *revs,
>> while ((commit = get_revision(revs))) {
>> const struct name_decoration *decoration;
>>
>> - if (commit->parents && commit->parents->next)
>> + if (opts->linearize && (!commit->parents || commit->parents->next))
>> + ; /* map current commit to the same as the previous commit */
>
> This uses the same treatment on either root commits or merge
> commits? If this were a mistake and this wants to handle merges but
> not roots, shouldn't it be more like
>
> if (opts->linearize && (commit->parents && commit->parents->next))
> ; /* map the merge to the previous */
>
>> + else if (commit->parents && commit->parents->next)
>> die(_("replaying merge commits is not supported yet!"));
>
> And because the next one is also about merges, perhaps the early
> part of this if/else if cascade can be written
>
> if (commit->parents && commit->parents->next) {
> /* We have a merge */
> if (!opts->linearize)
> die(_("can't replay a merge (yet)"));
> ; /* map current to the previous */
> } else {
> ...
>
> wouldn't it?
The way it was written in v1 was maybe a bit too smart and hard to
follow. I agree with your suggestion and will adopt this (with some
tweaks) in the next version.
> If the "map current to prev" is applicable to root, any root are
> mapped to the last_commit in the above, and if we saw a root as the
> first thing in the loop, last_commit is NULL, we do not do anything
> here, and after the if/else if/else cascade, we see last_commit is
> NULL and break out of the loop.
Yes, good observation. I did not test this.
> Perhaps we would want to have a test that replays all the way down
> to the root commit?
I'll add it.
--
Cheers,
Toon
^ permalink raw reply
* [PATCH] bash-completions: add --max-count-oldest
From: Mirko Faina @ 2026-06-10 14:38 UTC (permalink / raw)
To: git; +Cc: Mirko Faina
Add missing completion for log --max-count-oldest
Signed-off-by: Mirko Faina <mroik@delayed.space>
---
Unfortunately I forgot to add bash completions.
This is built upon 1ff279f340 (The 13th batch, 2026-06-09) with
jch/mf/revision-max-count-oldest bb4ce23284 (revision.c: implement
--max-count-oldest, 2026-05-19) merged into it.
contrib/completion/git-completion.bash | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index a8e7c6ddbf..e875787710 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2195,7 +2195,7 @@ __git_log_common_options="
--not --all
--branches --tags --remotes
--first-parent --merges --no-merges
- --max-count=
+ --max-count= --max-count-oldest=
--max-age= --since= --after=
--min-age= --until= --before=
--min-parents= --max-parents=
--
2.54.0.505.ga804828a04
^ permalink raw reply related
* [PATCH v2 0/3] Teach git-replay(1) to linearize merge commits
From: Toon Claes @ 2026-06-10 14:49 UTC (permalink / raw)
To: git; +Cc: Toon Claes, Johannes Schindelin, Johannes Schindelin
In-Reply-To: <20260608-toon-git-replay-drop-merges-v1-0-e3ee71fce7b4@iotcl.com>
As an alternative to dscho's patch series to replay merges[1], add
option to git-replay(1) to linearize merges. This mimics wath
git-rebase(1) does too with --no-rebase-merges (the default).
The first two patches do some refactoring. The third patch implements
the actual change. I was kindly helped by dscho to implement this
change.
The --linearize option is only added to git-replay(1) and not to
git-history(1) because in my opinion doesn't make much sense to do so,
but I'm happy to hear if anyone disagrees.
This series might conflict with Kristoffer's series to make
documentation changes[2], but should be trivial to resolve. And I don't
think there's a conflict with Patrick's series on adding "drop" to
git-history(1)[3].
dscho's series to replay merges[1] need a bit of rework to fit on top of
this, but I'm happy to help figuring that out. We've been discussing to
either name the option --flatten or --linearize, but I've decided on
"linearize" because the documentation of git-rebase(1) also mentions
"linearize".
[1]: <pull.2106.git.1778107405.gitgitgadget@gmail.com>
[2]: <V2_CV_doc_replay_config.767@msgid.xyz>
[3]: <20260603-b4-pks-history-drop-v2-0-742cb5b5176d@pks.im>
Signed-off-by: Toon Claes <toon@iotcl.com>
---
Changes in v2:
- Restructured the conditions to detect merge commits and added a line
of comment why the loop continues.
- Rewrote tests to use the history from the setup step and added a few
test cases.
- Re-added Johannes's Signed-off-by trailer. Johannes gave me the
patches with this trailer, and if I understand correctly, I can keep
it. Please let me know if that wrong.
- Link to v1: https://patch.msgid.link/20260608-toon-git-replay-drop-merges-v1-0-e3ee71fce7b4@iotcl.com
---
Johannes Schindelin (1):
replay: offer an option to linearize the commit topology
Toon Claes (2):
replay: refactor enum replay_mode into a bool
replay: add helper to put entry into mapped_commits
Documentation/git-replay.adoc | 5 ++
builtin/replay.c | 4 ++
replay.c | 114 ++++++++++++++++++++++++------------------
replay.h | 5 ++
t/t3650-replay-basics.sh | 26 ++++++++++
5 files changed, 105 insertions(+), 49 deletions(-)
Range-diff versus v1:
1: 7f3bc6f425 ! 1: 0975b142e3 replay: refactor enum replay_mode into a bool
@@ Commit message
- The value `REPLAY_MODE_REVERT` is used when option `--revert` is
passed to git-replay(1). When using this value the commits are
- possible in reverse order and the inverse of the changes are applied.
+ processed in reverse order and the inverse of the changes are
+ applied.
- The value `REPLAY_MODE_PICK` is used when either option `--onto` or
- `--advance` is used. In both cases the commits are pocessed in normal
- order, and the changes are applied as-is.
+ `--advance` is used. In both cases the commits are processed in
+ normal order, and the changes are applied as-is.
Since there are only two possible values of this enum, simplify the code
- by converting the enum into a bool. This avoid adding code paths that
- check for invalid vaues of the enum, and shortens code where the value
+ by converting the enum into a bool. This avoids adding code paths that
+ check for invalid values of the enum, and shortens code where the value
is checked with a ternary operator.
Signed-off-by: Toon Claes <toon@iotcl.com>
2: 0868871c78 ! 2: db88193624 replay: add helper to put entry into mapped_commits
@@ Commit message
replay: add helper to put entry into mapped_commits
The function replay_revisions() in replay.c is rather lengthy. Extract
- the logic to put commit entry into mapped_commits into a helper
- function.
+ the logic to put a commit entry into mapped_commits into a helper
+ function put_mapped_commit().
+
+ While at it, rename mapped_commit() to get_mapped_commit() to pair with
+ this new function.
Signed-off-by: Toon Claes <toon@iotcl.com>
3: a432ae753b ! 3: d0c220ec8e replay: offer an option to linearize the commit topology
@@ Commit message
The default mode of git-rebase(1) is to act as if `--no-rebase-merges`
was given. This mode drops merge commits instead of replaying them, and
- linearized the commit history into a sequence of the
+ linearizes the commit history into a sequence of the
regular (single-parent) commits.
- Add option `--linearize` to git-replay(1) do the same.
+ Add option `--linearize` to git-replay(1) to do the same.
Co-authored-by: Toon Claes <toon@iotcl.com>
+ Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
+ Signed-off-by: Toon Claes <toon@iotcl.com>
## Documentation/git-replay.adoc ##
@@ Documentation/git-replay.adoc: incompatible with `--contained` (which is a modifier for `--onto` only).
@@ replay.c: int replay_revisions(struct rev_info *revs,
const struct name_decoration *decoration;
- if (commit->parents && commit->parents->next)
-+ if (opts->linearize && (!commit->parents || commit->parents->next))
-+ ; /* map current commit to the same as the previous commit */
-+ else if (commit->parents && commit->parents->next)
- die(_("replaying merge commits is not supported yet!"));
-+ else {
+- die(_("replaying merge commits is not supported yet!"));
++ if (commit->parents && commit->parents->next) {
++ if (!opts->linearize)
++ die(_("replaying merge commits is not supported yet!"));
++ /*
++ * When linearizing, a merge commit itself is not picked,
++ * but refs that point to it might need updating.
++ */
++ } else {
+ struct commit *to_pick = reverse ? last_commit : onto;
+ last_commit =
+ pick_regular_commit(revs->repo, commit,
@@ t/t3650-replay-basics.sh: test_expect_success '--onto with --ref rejects multipl
test_grep "cannot be used with multiple revision ranges" err
'
-+test_expect_success 'linearize the commit topology' '
-+ test_tick &&
-+ N=$(git commit-tree -m N -p L -p I L:) &&
-+ N=$(git commit-tree -m N-child -p $N L:) &&
-+ git update-ref refs/heads/N $N &&
++test_expect_success 'replay merge commit fails' '
++ echo "fatal: replaying merge commits is not supported yet!" >expect &&
++ test_must_fail git replay --ref-action=print --onto main I..P 2>actual &&
++ test_cmp expect actual
++'
++
++test_expect_success 'replay to rebase merge commit with --linearize' '
++ git replay --ref-action=print --linearize --onto main I..topic-with-merge >result &&
++
++ test_line_count = 1 result &&
++
++ git log --format=%s $(cut -f 3 -d " " result) >actual &&
++ test_write_lines O N J M L B A >expect &&
++ test_cmp expect actual
++'
+
-+ git replay --ref-action=print --linearize \
-+ --onto A B..refs/heads/N >out &&
++test_expect_success 'replay to rebase merge commit with --linearize down to root commit' '
++ git replay --ref-action=print --linearize --onto main A..topic-with-merge >result &&
+
-+ test_line_count = 1 out &&
-+ read N1 N2 N3 N4 <out &&
++ test_line_count = 1 result &&
+
-+ cat >expect <<-EOF &&
-+ * N-child
-+ * I
-+ * L
-+ o A
-+ EOF
-+ git log --format=%s --graph --boundary A...$N3 >actual &&
++ git log --format=%s $(cut -f 3 -d " " result) >actual &&
++ test_write_lines O N J I M L B A >expect &&
+ test_cmp expect actual
+'
+
---
base-commit: 9ac3f193c05c2237e2b14ebaa1149e9fc8a1abe0
change-id: 20260604-toon-git-replay-drop-merges-807fa008d395
^ permalink raw reply
* [PATCH v2 1/3] replay: refactor enum replay_mode into a bool
From: Toon Claes @ 2026-06-10 14:49 UTC (permalink / raw)
To: git; +Cc: Toon Claes
In-Reply-To: <20260610-toon-git-replay-drop-merges-v2-0-5714a71c6d83@iotcl.com>
In 2760ee4983 (replay: add --revert mode to reverse commit changes,
2026-03-26) the enum `replay_mode` was introduced. This has two possible
values:
- The value `REPLAY_MODE_REVERT` is used when option `--revert` is
passed to git-replay(1). When using this value the commits are
processed in reverse order and the inverse of the changes are
applied.
- The value `REPLAY_MODE_PICK` is used when either option `--onto` or
`--advance` is used. In both cases the commits are processed in
normal order, and the changes are applied as-is.
Since there are only two possible values of this enum, simplify the code
by converting the enum into a bool. This avoids adding code paths that
check for invalid values of the enum, and shortens code where the value
is checked with a ternary operator.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
replay.c | 59 +++++++++++++++++++++++++----------------------------------
1 file changed, 25 insertions(+), 34 deletions(-)
diff --git a/replay.c b/replay.c
index 4ef8abb607..1f8e5b083b 100644
--- a/replay.c
+++ b/replay.c
@@ -18,11 +18,6 @@
*/
#define the_repository DO_NOT_USE_THE_REPOSITORY
-enum replay_mode {
- REPLAY_MODE_PICK,
- REPLAY_MODE_REVERT,
-};
-
static const char *short_commit_name(struct repository *repo,
struct commit *commit)
{
@@ -81,7 +76,7 @@ static struct commit *create_commit(struct repository *repo,
struct tree *tree,
struct commit *based_on,
struct commit *parent,
- enum replay_mode mode)
+ bool reverse)
{
struct object_id ret;
struct object *obj = NULL;
@@ -98,15 +93,13 @@ static struct commit *create_commit(struct repository *repo,
commit_list_insert(parent, &parents);
extra = read_commit_extra_headers(based_on, exclude_gpgsig);
- if (mode == REPLAY_MODE_REVERT) {
+ if (reverse) {
generate_revert_message(&msg, based_on, repo);
/* For revert, use current user as author (NULL = use default) */
- } else if (mode == REPLAY_MODE_PICK) {
+ } else {
find_commit_subject(message, &orig_message);
strbuf_addstr(&msg, orig_message);
author = get_author(message);
- } else {
- BUG("unexpected replay mode %d", mode);
}
reset_ident_date();
if (commit_tree_extended(msg.buf, msg.len, &tree->object.oid, parents,
@@ -269,7 +262,7 @@ static struct commit *pick_regular_commit(struct repository *repo,
struct commit *onto,
struct merge_options *merge_opt,
struct merge_result *result,
- enum replay_mode mode,
+ bool reverse,
enum replay_empty_commit_action empty)
{
struct commit *base, *replayed_base;
@@ -287,7 +280,21 @@ static struct commit *pick_regular_commit(struct repository *repo,
replayed_base_tree = repo_get_commit_tree(repo, replayed_base);
pickme_tree = repo_get_commit_tree(repo, pickme);
- if (mode == REPLAY_MODE_PICK) {
+ if (reverse) {
+ /* Revert: swap base and pickme to reverse the diff */
+ const char *pickme_name = short_commit_name(repo, pickme);
+ merge_opt->branch1 = short_commit_name(repo, replayed_base);
+ merge_opt->branch2 = xstrfmt("parent of %s", pickme_name);
+ merge_opt->ancestor = pickme_name;
+
+ merge_incore_nonrecursive(merge_opt,
+ pickme_tree,
+ replayed_base_tree,
+ base_tree,
+ result);
+
+ free((char *)merge_opt->branch2);
+ } else {
/* Cherry-pick: normal order */
merge_opt->branch1 = short_commit_name(repo, replayed_base);
merge_opt->branch2 = short_commit_name(repo, pickme);
@@ -303,22 +310,6 @@ static struct commit *pick_regular_commit(struct repository *repo,
result);
free((char *)merge_opt->ancestor);
- } else if (mode == REPLAY_MODE_REVERT) {
- /* Revert: swap base and pickme to reverse the diff */
- const char *pickme_name = short_commit_name(repo, pickme);
- merge_opt->branch1 = short_commit_name(repo, replayed_base);
- merge_opt->branch2 = xstrfmt("parent of %s", pickme_name);
- merge_opt->ancestor = pickme_name;
-
- merge_incore_nonrecursive(merge_opt,
- pickme_tree,
- replayed_base_tree,
- base_tree,
- result);
-
- free((char *)merge_opt->branch2);
- } else {
- BUG("unexpected replay mode %d", mode);
}
merge_opt->ancestor = NULL;
merge_opt->branch2 = NULL;
@@ -341,7 +332,7 @@ static struct commit *pick_regular_commit(struct repository *repo,
}
}
- return create_commit(repo, result->tree, pickme, replayed_base, mode);
+ return create_commit(repo, result->tree, pickme, replayed_base, reverse);
}
void replay_result_release(struct replay_result *result)
@@ -381,13 +372,13 @@ int replay_revisions(struct rev_info *revs,
char *revert;
const char *ref;
struct object_id old_oid;
- enum replay_mode mode = REPLAY_MODE_PICK;
+ bool reverse;
int ret;
advance = xstrdup_or_null(opts->advance);
revert = xstrdup_or_null(opts->revert);
- if (revert)
- mode = REPLAY_MODE_REVERT;
+ reverse = !!revert;
+
set_up_replay_mode(revs->repo, &revs->cmdline, opts->onto,
&detached_head, &advance, &revert, &onto, &update_refs);
@@ -430,8 +421,8 @@ int replay_revisions(struct rev_info *revs,
die(_("replaying merge commits is not supported yet!"));
last_commit = pick_regular_commit(revs->repo, commit, replayed_commits,
- mode == REPLAY_MODE_REVERT ? last_commit : onto,
- &merge_opt, &result, mode, opts->empty);
+ reverse ? last_commit : onto,
+ &merge_opt, &result, reverse, opts->empty);
if (!last_commit)
break;
--
2.53.0.1323.g189a785ab5
^ permalink raw reply related
* [PATCH v2 2/3] replay: add helper to put entry into mapped_commits
From: Toon Claes @ 2026-06-10 14:49 UTC (permalink / raw)
To: git; +Cc: Toon Claes
In-Reply-To: <20260610-toon-git-replay-drop-merges-v2-0-5714a71c6d83@iotcl.com>
The function replay_revisions() in replay.c is rather lengthy. Extract
the logic to put a commit entry into mapped_commits into a helper
function put_mapped_commit().
While at it, rename mapped_commit() to get_mapped_commit() to pair with
this new function.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
replay.c | 31 ++++++++++++++++++++-----------
1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/replay.c b/replay.c
index 1f8e5b083b..7921d7dba3 100644
--- a/replay.c
+++ b/replay.c
@@ -243,9 +243,9 @@ static void set_up_replay_mode(struct repository *repo,
strset_clear(&rinfo.positive_refs);
}
-static struct commit *mapped_commit(kh_oid_map_t *replayed_commits,
- struct commit *commit,
- struct commit *fallback)
+static struct commit *get_mapped_commit(kh_oid_map_t *replayed_commits,
+ struct commit *commit,
+ struct commit *fallback)
{
khint_t pos;
if (!commit)
@@ -256,6 +256,21 @@ static struct commit *mapped_commit(kh_oid_map_t *replayed_commits,
return kh_value(replayed_commits, pos);
}
+static void put_mapped_commit(kh_oid_map_t *replayed_commits,
+ struct commit *commit,
+ struct commit *new_commit)
+{
+ khint_t pos;
+ int ret;
+
+ pos = kh_put_oid_map(replayed_commits, commit->object.oid, &ret);
+ if (ret == 0)
+ BUG("Duplicate rewritten commit: %s\n",
+ oid_to_hex(&commit->object.oid));
+
+ kh_value(replayed_commits, pos) = new_commit;
+}
+
static struct commit *pick_regular_commit(struct repository *repo,
struct commit *pickme,
kh_oid_map_t *replayed_commits,
@@ -276,7 +291,7 @@ static struct commit *pick_regular_commit(struct repository *repo,
base_tree = lookup_tree(repo, repo->hash_algo->empty_tree);
}
- replayed_base = mapped_commit(replayed_commits, base, onto);
+ replayed_base = get_mapped_commit(replayed_commits, base, onto);
replayed_base_tree = repo_get_commit_tree(repo, replayed_base);
pickme_tree = repo_get_commit_tree(repo, pickme);
@@ -414,8 +429,6 @@ int replay_revisions(struct rev_info *revs,
replayed_commits = kh_init_oid_map();
while ((commit = get_revision(revs))) {
const struct name_decoration *decoration;
- khint_t pos;
- int hr;
if (commit->parents && commit->parents->next)
die(_("replaying merge commits is not supported yet!"));
@@ -427,11 +440,7 @@ int replay_revisions(struct rev_info *revs,
break;
/* Record commit -> last_commit mapping */
- pos = kh_put_oid_map(replayed_commits, commit->object.oid, &hr);
- if (hr == 0)
- BUG("Duplicate rewritten commit: %s\n",
- oid_to_hex(&commit->object.oid));
- kh_value(replayed_commits, pos) = last_commit;
+ put_mapped_commit(replayed_commits, commit, last_commit);
/* Update any necessary branches */
if (ref)
--
2.53.0.1323.g189a785ab5
^ permalink raw reply related
* [PATCH v2 3/3] replay: offer an option to linearize the commit topology
From: Toon Claes @ 2026-06-10 14:49 UTC (permalink / raw)
To: git; +Cc: Toon Claes, Johannes Schindelin, Johannes Schindelin
In-Reply-To: <20260610-toon-git-replay-drop-merges-v2-0-5714a71c6d83@iotcl.com>
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
One of the stated goals of git-replay(1) is to allow implementing the
git-rebase(1) functionality on the server side.
The default mode of git-rebase(1) is to act as if `--no-rebase-merges`
was given. This mode drops merge commits instead of replaying them, and
linearizes the commit history into a sequence of the
regular (single-parent) commits.
Add option `--linearize` to git-replay(1) to do the same.
Co-authored-by: Toon Claes <toon@iotcl.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Toon Claes <toon@iotcl.com>
---
Documentation/git-replay.adoc | 5 +++++
builtin/replay.c | 4 ++++
replay.c | 30 +++++++++++++++++++++++-------
replay.h | 5 +++++
t/t3650-replay-basics.sh | 26 ++++++++++++++++++++++++++
5 files changed, 63 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-replay.adoc b/Documentation/git-replay.adoc
index a32f72aead..41c96c7061 100644
--- a/Documentation/git-replay.adoc
+++ b/Documentation/git-replay.adoc
@@ -88,6 +88,11 @@ incompatible with `--contained` (which is a modifier for `--onto` only).
+
The default mode can be configured via the `replay.refAction` configuration variable.
+--linearize::
+ In this mode, `git replay` imitates `git rebase --no-rebase-merges`,
+ i.e. it cherry-picks only non-merge commits, each one on top of the
+ previous one.
+
<revision-range>::
Range of commits to replay; see "Specifying Ranges" in
linkgit:git-rev-parse[1]. In `--advance=<branch>` or
diff --git a/builtin/replay.c b/builtin/replay.c
index 39e3a86f6c..fedfe46dc6 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -111,6 +111,8 @@ int cmd_replay(int argc,
N_("mode"),
N_("control ref update behavior (update|print)"),
PARSE_OPT_NONEG),
+ OPT_BOOL(0, "linearize", &opts.linearize,
+ N_("ignore merge commits instead of replaying them")),
OPT_END()
};
@@ -132,6 +134,8 @@ int cmd_replay(int argc,
opts.contained, "--contained");
die_for_incompatible_opt2(!!opts.ref, "--ref",
!!opts.contained, "--contained");
+ die_for_incompatible_opt2(!!opts.revert, "--revert",
+ opts.linearize, "--linearize");
/* Parse ref action mode from command line or config */
ref_mode = get_ref_action_mode(repo, ref_action);
diff --git a/replay.c b/replay.c
index 7921d7dba3..81033fb889 100644
--- a/replay.c
+++ b/replay.c
@@ -277,12 +277,16 @@ static struct commit *pick_regular_commit(struct repository *repo,
struct commit *onto,
struct merge_options *merge_opt,
struct merge_result *result,
+ struct commit *replayed_base,
bool reverse,
enum replay_empty_commit_action empty)
{
- struct commit *base, *replayed_base;
+ struct commit *base;
struct tree *pickme_tree, *base_tree, *replayed_base_tree;
+ if (replayed_base && reverse)
+ BUG("Linearizing commits is not supported when replaying in reverse");
+
if (pickme->parents) {
base = pickme->parents->item;
base_tree = repo_get_commit_tree(repo, base);
@@ -291,7 +295,8 @@ static struct commit *pick_regular_commit(struct repository *repo,
base_tree = lookup_tree(repo, repo->hash_algo->empty_tree);
}
- replayed_base = get_mapped_commit(replayed_commits, base, onto);
+ if (!replayed_base)
+ replayed_base = get_mapped_commit(replayed_commits, base, onto);
replayed_base_tree = repo_get_commit_tree(repo, replayed_base);
pickme_tree = repo_get_commit_tree(repo, pickme);
@@ -430,12 +435,23 @@ int replay_revisions(struct rev_info *revs,
while ((commit = get_revision(revs))) {
const struct name_decoration *decoration;
- if (commit->parents && commit->parents->next)
- die(_("replaying merge commits is not supported yet!"));
+ if (commit->parents && commit->parents->next) {
+ if (!opts->linearize)
+ die(_("replaying merge commits is not supported yet!"));
+ /*
+ * When linearizing, a merge commit itself is not picked,
+ * but refs that point to it might need updating.
+ */
+ } else {
+ struct commit *to_pick = reverse ? last_commit : onto;
+ last_commit =
+ pick_regular_commit(revs->repo, commit,
+ replayed_commits, to_pick,
+ &merge_opt, &result,
+ opts->linearize ? last_commit : NULL,
+ reverse, opts->empty);
+ }
- last_commit = pick_regular_commit(revs->repo, commit, replayed_commits,
- reverse ? last_commit : onto,
- &merge_opt, &result, reverse, opts->empty);
if (!last_commit)
break;
diff --git a/replay.h b/replay.h
index 1851a07705..07e6fdcca3 100644
--- a/replay.h
+++ b/replay.h
@@ -62,6 +62,11 @@ struct replay_revisions_options {
* Defaults to REPLAY_EMPTY_COMMIT_DROP.
*/
enum replay_empty_commit_action empty;
+
+ /*
+ * Whether to linearize the commits (i.e. drop merge commits).
+ */
+ int linearize;
};
/* This struct is used as an out-parameter by `replay_revisions()`. */
diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
index 3353bc4a4d..64e0731188 100755
--- a/t/t3650-replay-basics.sh
+++ b/t/t3650-replay-basics.sh
@@ -565,4 +565,30 @@ test_expect_success '--onto with --ref rejects multiple revision ranges' '
test_grep "cannot be used with multiple revision ranges" err
'
+test_expect_success 'replay merge commit fails' '
+ echo "fatal: replaying merge commits is not supported yet!" >expect &&
+ test_must_fail git replay --ref-action=print --onto main I..P 2>actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'replay to rebase merge commit with --linearize' '
+ git replay --ref-action=print --linearize --onto main I..topic-with-merge >result &&
+
+ test_line_count = 1 result &&
+
+ git log --format=%s $(cut -f 3 -d " " result) >actual &&
+ test_write_lines O N J M L B A >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'replay to rebase merge commit with --linearize down to root commit' '
+ git replay --ref-action=print --linearize --onto main A..topic-with-merge >result &&
+
+ test_line_count = 1 result &&
+
+ git log --format=%s $(cut -f 3 -d " " result) >actual &&
+ test_write_lines O N J I M L B A >expect &&
+ test_cmp expect actual
+'
+
test_done
--
2.53.0.1323.g189a785ab5
^ permalink raw reply related
* Re: [PATCH v3] index-pack: retain child bases in delta cache
From: Junio C Hamano @ 2026-06-10 14:51 UTC (permalink / raw)
To: Arijit Banerjee
Cc: Jeff King, Arijit Banerjee via GitGitGadget, git,
Ævar Arnfjörð Bjarmason, Derrick Stolee,
Arijit Banerjee
In-Reply-To: <08B48BBE-4084-4619-94B0-503158B93BEF@gmail.com>
Arijit Banerjee <arijit91@gmail.com> writes:
> Apologies, my earlier replies were sent through GitHub's notification
> emails and appeared only as PR comments, so they did not reach the mailing
> list.
>
> On Thu, Jun 4, 2026, Jeff King wrote:
>> So I am happy with either v2 or v3.
>
> I also did not see a meaningful performance difference between v2 and v3.
> I am happy with either direction and defer to the maintainers on whether
> v3's more precise release is worth the added complexity.
I have no strong preference either way.
> On Wed, Jun 3, 2026, Derrick Stolee wrote:
>> Did you see any evidence that this change has the intended effect of
>> reducing process memory proactively instead of relying on cache evictions?
>
> I do not have strong RSS evidence. The spot checks showed no meaningful RSS
> change, and max RSS is not a good signal here because free_base_data()
> lowers Git's internal base_cache_used accounting but may not return pages
> to the OS or reduce the recorded peak.
>
> The evidence for v3 is therefore structural: it releases the cached data
> once all direct children have been dispatched and retain_data reaches zero,
> rather than waiting for cache-pressure eviction.
^ permalink raw reply
* [PATCH 0/9] refs: stop using `chdir_notify_reparent()`
From: Patrick Steinhardt @ 2026-06-10 14:57 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
Hi,
this patch series is a follow-up of the discussion at [1]. It converts
the reference backends to always use absolute paths internally, which
then allows us to drop the calls to `chdir_notify_reparent()`.
Unfortunately, the series has grown quite a bit larger than anticipated.
This is due to a couple of weirdnesses in how the reference database is
constructed with an "onbranch" condition. We essentially construct the
refdb twice and loose one, but we never noticed because the chdir
notification subsystem kept the pointer to it reachable.
Note that the first couple patches that touch "setup.c" aren't strictly
required. They are a remnant of a previous iteration where I tried to
solve the issue in a different way. But I ultimately figured that these
changes are worth it by themselves as they simplify "setup.c" a bit.
This series is built on top of 1ff279f340 (The 13th batch, 2026-06-09)
with ps/setup-centralize-odb-creation at 42b9d3dc9d (setup: construct
object database in `apply_repository_format()`, 2026-06-04) merged into
it.
Thanks!
Patrick
[1]: <aifAVpxanV31KUpC@pks.im>
---
Patrick Steinhardt (9):
setup: inline `check_and_apply_repository_format()`
setup: stop applying repository format twice
setup: don't apply "GIT_REFERENCE_BACKEND" without a repository
refs: unregister reference stores from "chdir_notify"
chdir-notify: drop unused `chdir_notify_reparent()`
repository: free main reference database
refs: fix recursing `get_main_ref_store()` with "onbranch" config
refs: drop local buffer in `refs_compute_filesystem_location()`
refs: always use absolute paths for reference stores
chdir-notify.c | 26 ------------
chdir-notify.h | 6 +--
refs.c | 35 ++++++++++++-----
refs/files-backend.c | 6 ---
refs/packed-backend.c | 4 +-
refs/reftable-backend.c | 3 --
repository.c | 5 +++
setup.c | 96 ++++++++++++++++++---------------------------
t/pack-refs-tests.sh | 6 +--
t/t0600-reffiles-backend.sh | 4 +-
t/t1423-ref-backend.sh | 9 +++--
t/t5510-fetch.sh | 2 +-
12 files changed, 83 insertions(+), 119 deletions(-)
---
base-commit: 255322df35357168daefec8523a3cdc849edd6c1
change-id: 20260609-b4-pks-refs-avoid-chdir-notify-reparent-a4eaf1edbcab
^ permalink raw reply
* [PATCH 1/9] setup: inline `check_and_apply_repository_format()`
From: Patrick Steinhardt @ 2026-06-10 14:57 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
In-Reply-To: <20260610-b4-pks-refs-avoid-chdir-notify-reparent-v1-0-56c864b01c43@pks.im>
We have two callsites of `check_and_apply_repository_format()`. In a
subsequent commit we'll want to adapt one of those callsites to change
the order in which we read and apply the repository format, at which
point the helper function will not really be a good fit for us anymore.
Inline the function to both of the callsites.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
setup.c | 47 ++++++++++++++++-------------------------------
1 file changed, 16 insertions(+), 31 deletions(-)
diff --git a/setup.c b/setup.c
index b4652651df..a9db1f2c23 100644
--- a/setup.c
+++ b/setup.c
@@ -1788,32 +1788,6 @@ int apply_repository_format(struct repository *repo,
return 0;
}
-/*
- * Check the repository format version in the path found in repo_get_git_dir(repo),
- * and die if it is a version we don't understand. Generally one would
- * set_git_dir() before calling this, and use it only for "are we in a valid
- * repo?".
- *
- * If successful and fmt is not NULL, fill fmt with data.
- */
-static void check_and_apply_repository_format(struct repository *repo,
- struct repository_format *fmt,
- enum apply_repository_format_flags flags)
-{
- struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;
- struct strbuf err = STRBUF_INIT;
-
- if (!fmt)
- fmt = &repo_fmt;
-
- check_repository_format_gently(repo_get_git_dir(repo), fmt, NULL);
- if (apply_repository_format(repo, fmt, flags, &err) < 0)
- die("%s", err.buf);
- startup_info->have_repository = 1;
-
- clear_repository_format(&repo_fmt);
-}
-
const char *enter_repo(struct repository *repo, const char *path, unsigned flags)
{
static struct strbuf validated_path = STRBUF_INIT;
@@ -1887,9 +1861,17 @@ const char *enter_repo(struct repository *repo, const char *path, unsigned flags
}
if (is_git_directory(".")) {
+ struct repository_format fmt = REPOSITORY_FORMAT_INIT;
+ struct strbuf err = STRBUF_INIT;
+
set_git_dir(repo, ".", 0);
- check_and_apply_repository_format(repo, NULL,
- APPLY_REPOSITORY_FORMAT_HONOR_ENV);
+ check_repository_format_gently(".", &fmt, NULL);
+ if (apply_repository_format(repo, &fmt, APPLY_REPOSITORY_FORMAT_HONOR_ENV, &err) < 0)
+ die("%s", err.buf);
+ startup_info->have_repository = 1;
+
+ clear_repository_format(&fmt);
+ strbuf_release(&err);
return path;
}
@@ -2820,6 +2802,7 @@ int init_db(struct repository *repo,
int exist_ok = flags & INIT_DB_EXIST_OK;
char *original_git_dir = real_pathdup(git_dir, 1);
struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;
+ struct strbuf err = STRBUF_INIT;
if (real_git_dir) {
struct stat st;
@@ -2846,9 +2829,10 @@ int init_db(struct repository *repo,
* config file, so this will not fail. What we are catching
* is an attempt to reinitialize new repository with an old tool.
*/
- check_and_apply_repository_format(repo, &repo_fmt,
- APPLY_REPOSITORY_FORMAT_HONOR_ENV);
-
+ check_repository_format_gently(repo_get_git_dir(repo), &repo_fmt, NULL);
+ if (apply_repository_format(repo, &repo_fmt, APPLY_REPOSITORY_FORMAT_HONOR_ENV, &err) < 0)
+ die("%s", err.buf);
+ startup_info->have_repository = 1;
repository_format_configure(repo, &repo_fmt, hash, ref_storage_format);
/*
@@ -2904,6 +2888,7 @@ int init_db(struct repository *repo,
}
clear_repository_format(&repo_fmt);
+ strbuf_release(&err);
free(original_git_dir);
return 0;
}
--
2.54.0.1189.g8c84645362.dirty
^ permalink raw reply related
* [PATCH 2/9] setup: stop applying repository format twice
From: Patrick Steinhardt @ 2026-06-10 14:57 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
In-Reply-To: <20260610-b4-pks-refs-avoid-chdir-notify-reparent-v1-0-56c864b01c43@pks.im>
When discovering the repository in "setup.c" we apply the final
repository format multiple times:
- Once via `repository_format_configure()`, where we configure the
repository format for both `struct repository_format` and `struct
repository`.
- And once via `apply_repository_format()`, where we then apply the
`struct repository_format` to the `struct repository` again.
As the format will be applied to the repository when applying the format
it's thus somewhat unnecessary to also apply it to the repository when
adapting the discovered format. The only reason we have to do this is
because we call `repository_format_configure()` after we have already
applied it.
Refactor the code so that we first configure the repository format
before applying it to the repository so that we can stop setting the
hash and reference storage format multiple times.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
setup.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/setup.c b/setup.c
index a9db1f2c23..2748155964 100644
--- a/setup.c
+++ b/setup.c
@@ -2710,8 +2710,7 @@ static int read_default_format_config(const char *key, const char *value,
return ret;
}
-static void repository_format_configure(struct repository *repo,
- struct repository_format *repo_fmt,
+static void repository_format_configure(struct repository_format *repo_fmt,
int hash, enum ref_storage_format ref_format)
{
struct default_format_config cfg = {
@@ -2748,7 +2747,6 @@ static void repository_format_configure(struct repository *repo,
} else if (cfg.hash != GIT_HASH_UNKNOWN) {
repo_fmt->hash_algo = cfg.hash;
}
- repo_set_hash_algo(repo, repo_fmt->hash_algo);
env = getenv("GIT_DEFAULT_REF_FORMAT");
if (repo_fmt->version >= 0 &&
@@ -2786,9 +2784,6 @@ static void repository_format_configure(struct repository *repo,
free(backend);
}
-
- repo_set_ref_storage_format(repo, repo_fmt->ref_storage_format,
- repo_fmt->ref_storage_payload);
}
int init_db(struct repository *repo,
@@ -2830,10 +2825,10 @@ int init_db(struct repository *repo,
* is an attempt to reinitialize new repository with an old tool.
*/
check_repository_format_gently(repo_get_git_dir(repo), &repo_fmt, NULL);
+ repository_format_configure(&repo_fmt, hash, ref_storage_format);
if (apply_repository_format(repo, &repo_fmt, APPLY_REPOSITORY_FORMAT_HONOR_ENV, &err) < 0)
die("%s", err.buf);
startup_info->have_repository = 1;
- repository_format_configure(repo, &repo_fmt, hash, ref_storage_format);
/*
* Ensure `core.hidedotfiles` is processed. This must happen after we
--
2.54.0.1189.g8c84645362.dirty
^ permalink raw reply related
* [PATCH 3/9] setup: don't apply "GIT_REFERENCE_BACKEND" without a repository
From: Patrick Steinhardt @ 2026-06-10 14:57 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
In-Reply-To: <20260610-b4-pks-refs-avoid-chdir-notify-reparent-v1-0-56c864b01c43@pks.im>
When discovering a repository we eventually also apply the
"GIT_REFERENCE_BACKEND" environment variable to the repository. There's
two problems with that:
- We do this unconditionally, which is rather pointless: we really
only have to configure the repository when we have found one.
- We have already applied the repository format at that point in time,
so we need to manually reapply it.
Move the logic around so that we only apply the environment variable
when a repository was discovered. This also allows us to drop the
explcit call to `repo_set_ref_storage_format()` because we now adjust
the format before we apply it via `apply_repository_format()`.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
setup.c | 40 ++++++++++++++++++++--------------------
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/setup.c b/setup.c
index 2748155964..7b2e50a8c5 100644
--- a/setup.c
+++ b/setup.c
@@ -1906,7 +1906,6 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
static struct strbuf cwd = STRBUF_INIT;
struct strbuf dir = STRBUF_INIT, gitdir = STRBUF_INIT, report = STRBUF_INIT;
const char *prefix = NULL;
- const char *ref_backend_uri;
struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;
/*
@@ -2023,6 +2022,8 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
startup_info->have_repository ||
/* GIT_DIR_EXPLICIT */
getenv(GIT_DIR_ENVIRONMENT)) {
+ const char *ref_backend_uri;
+
if (!repo->gitdir) {
const char *gitdir = getenv(GIT_DIR_ENVIRONMENT);
if (!gitdir)
@@ -2030,6 +2031,24 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
setup_git_env_internal(repo, gitdir);
}
+ /*
+ * The env variable should override the repository config
+ * for 'extensions.refStorage'.
+ */
+ ref_backend_uri = getenv(GIT_REFERENCE_BACKEND_ENVIRONMENT);
+ if (ref_backend_uri) {
+ char *format;
+
+ free(repo_fmt.ref_storage_payload);
+
+ parse_reference_uri(ref_backend_uri, &format, &repo_fmt.ref_storage_payload);
+ repo_fmt.ref_storage_format = ref_storage_format_by_name(format);
+ if (repo_fmt.ref_storage_format == REF_STORAGE_FORMAT_UNKNOWN)
+ die(_("unknown ref storage format: '%s'"), format);
+
+ free(format);
+ }
+
if (startup_info->have_repository) {
struct strbuf err = STRBUF_INIT;
@@ -2057,25 +2076,6 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
setenv(GIT_PREFIX_ENVIRONMENT, "", 1);
}
- /*
- * The env variable should override the repository config
- * for 'extensions.refStorage'.
- */
- ref_backend_uri = getenv(GIT_REFERENCE_BACKEND_ENVIRONMENT);
- if (ref_backend_uri) {
- char *backend, *payload;
- enum ref_storage_format format;
-
- parse_reference_uri(ref_backend_uri, &backend, &payload);
- format = ref_storage_format_by_name(backend);
- if (format == REF_STORAGE_FORMAT_UNKNOWN)
- die(_("unknown ref storage format: '%s'"), backend);
- repo_set_ref_storage_format(repo, format, payload);
-
- free(backend);
- free(payload);
- }
-
setup_original_cwd(repo);
strbuf_release(&dir);
--
2.54.0.1189.g8c84645362.dirty
^ permalink raw reply related
* [PATCH 4/9] refs: unregister reference stores from "chdir_notify"
From: Patrick Steinhardt @ 2026-06-10 14:57 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
In-Reply-To: <20260610-b4-pks-refs-avoid-chdir-notify-reparent-v1-0-56c864b01c43@pks.im>
When creating reference stores we register them with the "chdir_notify"
subsystem. This is required because some of the paths we track may be
relative paths, so we have to reparent them in case the current working
directory changes.
But while we register the reference stores, we never unregister them.
This can have multiple outcomes:
- For a repository's main reference database we essentially keep the
pointer alive. We never free that database, either, and our leak
checker doesn't notice because it's still registered.
- For submodule and worktree reference databases we do eventually free
them in `repo_clear()`, so we may keep pointers to free'd memory
registered. We never notice though as we don't tend to chdir around
in the middle of the process.
We never noticed either of these symptoms, but they are obviously bad.
Partially fix those issues by unregistering the reference stores when
releasing them. The leak of the main reference database will be fixed in
a subsequent commit.
Note that this requires us to use `chdir_notify_register()` instead of
`chdir_notify_parent()`, as there is no infrastructure to unregister the
latter. It ultimately doesn't matter much though: in a subsequent commit
we'll drop this infrastructure completely. We merely require this step
here so that we can fix the memory leaks ahead of time.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
refs/files-backend.c | 22 +++++++++++++++++++---
refs/packed-backend.c | 16 +++++++++++++++-
refs/reftable-backend.c | 16 +++++++++++++++-
3 files changed, 49 insertions(+), 5 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index a4c7858787..296981584b 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -100,6 +100,23 @@ static void clear_loose_ref_cache(struct files_ref_store *refs)
}
}
+static void files_ref_store_reparent(const char *name UNUSED,
+ const char *old_cwd,
+ const char *new_cwd,
+ void *payload)
+{
+ struct files_ref_store *refs = payload;
+ char *tmp;
+
+ tmp = reparent_relative_path(old_cwd, new_cwd, refs->base.gitdir);
+ free(refs->base.gitdir);
+ refs->base.gitdir = tmp;
+
+ tmp = reparent_relative_path(old_cwd, new_cwd, refs->gitcommondir);
+ free(refs->gitcommondir);
+ refs->gitcommondir = tmp;
+}
+
/*
* Create a new submodule ref cache and add it to the internal
* set of caches.
@@ -128,9 +145,7 @@ static struct ref_store *files_ref_store_init(struct repository *repo,
repo_config_get_bool(repo, "core.prefersymlinkrefs", &refs->prefer_symlink_refs);
- chdir_notify_reparent("files-backend $GIT_DIR", &refs->base.gitdir);
- chdir_notify_reparent("files-backend $GIT_COMMONDIR",
- &refs->gitcommondir);
+ chdir_notify_register(NULL, files_ref_store_reparent, refs);
strbuf_release(&refdir);
@@ -182,6 +197,7 @@ static void files_ref_store_release(struct ref_store *ref_store)
free(refs->gitcommondir);
ref_store_release(refs->packed_ref_store);
free(refs->packed_ref_store);
+ chdir_notify_unregister(NULL, files_ref_store_reparent, refs);
}
static void files_reflog_path(struct files_ref_store *refs,
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 0acde48c45..499cb55dfa 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -211,6 +211,19 @@ static size_t snapshot_hexsz(const struct snapshot *snapshot)
return snapshot->refs->base.repo->hash_algo->hexsz;
}
+static void packed_ref_store_reparent(const char *name UNUSED,
+ const char *old_cwd,
+ const char *new_cwd,
+ void *payload)
+{
+ struct packed_ref_store *refs = payload;
+ char *tmp;
+
+ tmp = reparent_relative_path(old_cwd, new_cwd, refs->path);
+ free(refs->path);
+ refs->path = tmp;
+}
+
/*
* Since packed-refs is only stored in the common dir, don't parse the
* payload and rely on the files-backend to set 'gitdir' correctly.
@@ -229,7 +242,7 @@ struct ref_store *packed_ref_store_init(struct repository *repo,
strbuf_addf(&sb, "%s/packed-refs", gitdir);
refs->path = strbuf_detach(&sb, NULL);
- chdir_notify_reparent("packed-refs", &refs->path);
+ chdir_notify_register(NULL, packed_ref_store_reparent, refs);
return ref_store;
}
@@ -274,6 +287,7 @@ static void packed_ref_store_release(struct ref_store *ref_store)
clear_snapshot(refs);
rollback_lock_file(&refs->lock);
delete_tempfile(&refs->tempfile);
+ chdir_notify_unregister(NULL, packed_ref_store_reparent, refs);
free(refs->path);
}
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 4ae22922de..8c93070677 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -365,6 +365,19 @@ static int reftable_be_config(const char *var, const char *value,
return 0;
}
+static void reftable_be_reparent(const char *name UNUSED,
+ const char *old_cwd,
+ const char *new_cwd,
+ void *payload)
+{
+ struct reftable_ref_store *refs = payload;
+ char *tmp;
+
+ tmp = reparent_relative_path(old_cwd, new_cwd, refs->base.gitdir);
+ free(refs->base.gitdir);
+ refs->base.gitdir = tmp;
+}
+
static struct ref_store *reftable_be_init(struct repository *repo,
const char *payload,
const char *gitdir,
@@ -447,7 +460,7 @@ static struct ref_store *reftable_be_init(struct repository *repo,
goto done;
}
- chdir_notify_reparent("reftables-backend $GIT_DIR", &refs->base.gitdir);
+ chdir_notify_register(NULL, reftable_be_reparent, refs);
done:
assert(refs->err != REFTABLE_API_ERROR);
@@ -474,6 +487,7 @@ static void reftable_be_release(struct ref_store *ref_store)
free(be);
}
strmap_clear(&refs->worktree_backends, 0);
+ chdir_notify_unregister(NULL, reftable_be_reparent, refs);
}
static int reftable_be_create_on_disk(struct ref_store *ref_store,
--
2.54.0.1189.g8c84645362.dirty
^ permalink raw reply related
* [PATCH 5/9] chdir-notify: drop unused `chdir_notify_reparent()`
From: Patrick Steinhardt @ 2026-06-10 14:57 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
In-Reply-To: <20260610-b4-pks-refs-avoid-chdir-notify-reparent-v1-0-56c864b01c43@pks.im>
With the preceding commit we've removed all callers of
`chdir_notify_reparent()`, so the function is unused now. Drop it.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
chdir-notify.c | 26 --------------------------
chdir-notify.h | 6 +-----
2 files changed, 1 insertion(+), 31 deletions(-)
diff --git a/chdir-notify.c b/chdir-notify.c
index f8bfe3cbef..1237a45e2e 100644
--- a/chdir-notify.c
+++ b/chdir-notify.c
@@ -43,32 +43,6 @@ void chdir_notify_unregister(const char *name, chdir_notify_callback cb,
}
}
-static void reparent_cb(const char *name,
- const char *old_cwd,
- const char *new_cwd,
- void *data)
-{
- char **path = data;
- char *tmp = *path;
-
- if (!tmp)
- return;
-
- *path = reparent_relative_path(old_cwd, new_cwd, tmp);
- free(tmp);
-
- if (name) {
- trace_printf_key(&trace_setup_key,
- "setup: reparent %s to '%s'",
- name, *path);
- }
-}
-
-void chdir_notify_reparent(const char *name, char **path)
-{
- chdir_notify_register(name, reparent_cb, path);
-}
-
int chdir_notify(const char *new_cwd)
{
struct strbuf old_cwd = STRBUF_INIT;
diff --git a/chdir-notify.h b/chdir-notify.h
index 81eb69d846..36b4114472 100644
--- a/chdir-notify.h
+++ b/chdir-notify.h
@@ -19,10 +19,7 @@
* chdir_notify_register("description", foo, data);
*
* In practice most callers will want to move a relative path to the new root;
- * they can use the reparent_relative_path() helper for that. If that's all
- * you're doing, you can also use the convenience function:
- *
- * chdir_notify_reparent("description", &my_path);
+ * they can use the reparent_relative_path() helper for that.
*
* Whenever a chdir event occurs, that will update my_path (if it's relative)
* to adjust for the new cwd by freeing any existing string and allocating a
@@ -43,7 +40,6 @@ typedef void (*chdir_notify_callback)(const char *name,
void chdir_notify_register(const char *name, chdir_notify_callback cb, void *data);
void chdir_notify_unregister(const char *name, chdir_notify_callback cb,
void *data);
-void chdir_notify_reparent(const char *name, char **path);
/*
*
--
2.54.0.1189.g8c84645362.dirty
^ permalink raw reply related
* [PATCH 6/9] repository: free main reference database
From: Patrick Steinhardt @ 2026-06-10 14:57 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
In-Reply-To: <20260610-b4-pks-refs-avoid-chdir-notify-reparent-v1-0-56c864b01c43@pks.im>
While we release worktree and submodule reference databases when
clearing a repository, we don't ever release the main reference
database. This memory leak went unnoticed because its pointer is
kept alive by the "chdir_notify" subsystem.
Fix the memory leak.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
repository.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/repository.c b/repository.c
index 187dd471c4..e2b5c6712b 100644
--- a/repository.c
+++ b/repository.c
@@ -421,6 +421,11 @@ void repo_clear(struct repository *repo)
FREE_AND_NULL(repo->remote_state);
}
+ if (repo->refs_private) {
+ ref_store_release(repo->refs_private);
+ FREE_AND_NULL(repo->refs_private);
+ }
+
strmap_for_each_entry(&repo->submodule_ref_stores, &iter, e)
ref_store_release(e->value);
strmap_clear(&repo->submodule_ref_stores, 1);
--
2.54.0.1189.g8c84645362.dirty
^ permalink raw reply related
* [PATCH 7/9] refs: fix recursing `get_main_ref_store()` with "onbranch" config
From: Patrick Steinhardt @ 2026-06-10 14:57 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
In-Reply-To: <20260610-b4-pks-refs-avoid-chdir-notify-reparent-v1-0-56c864b01c43@pks.im>
When we have an "onbranch" condition we need to ask the reference
database whether HEAD currently points at the configured branch. This
unfortunately creates a chicken-and-egg problem:
- The reference database needs to read the configuration so that it
can configure itself.
- The configuration needs to construct a reference database to fully
parse all of its conditionals.
The way we handle this is by simply excluding "onbranch" conditionals
when we haven't yet configured the reference database.
The mechanism for this is broken though: to verify whether or not we
have configured the reference database we check whether its format is
set to `REF_STORAGE_UNKNOWN` in `include_by_branch()`. But typically,
the format _is_ already known at that time because we set it up during
repository discovery in "setup.c".
The consequence is that we have recursion:
1. We call `get_main_ref_store()`.
2. We don't yet have a reference store, so we call `ref_store_init()`.
3. We parse the configuration required for the reference store.
4. We eventually end up in `include_by_branch()`.
5. We have already configured the reference storage format, so we end
up calling `get_main_ref_store()` again.
We still haven't finished (1) though, so `get_main_ref_store()` will now
call `ref_store_init()` a second time. The end result is that we have
constructed the same reference store twice.
Of course, as both reference stores would be assigned to `refs_private`,
we leak one of those two instances. This never surfaced as an actual
leak though because the pointer is kept alive by the "chdir_notify"
subsystem.
For now, we can fix the issue by explicitly unsetting the reference
storage format before constructing it. This makes the mentioned check
trigger as expected, and consequently we won't end up constructing a
second reference database at all. Ultimately, this means that we
consistently stop evaluating "onbranch" conditions when constructing the
main reference database.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
refs.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index d3caa9a633..e69b9b8ac8 100644
--- a/refs.c
+++ b/refs.c
@@ -2351,15 +2351,31 @@ void ref_store_release(struct ref_store *ref_store)
struct ref_store *get_main_ref_store(struct repository *r)
{
+ enum ref_storage_format format;
+
if (r->refs_private)
return r->refs_private;
if (!r->gitdir)
BUG("attempting to get main_ref_store outside of repository");
- r->refs_private = ref_store_init(r, r->ref_storage_format,
- r->gitdir, REF_STORE_ALL_CAPS);
+ /*
+ * When constructing the reference backend we'll end up reading the Git
+ * configuration. This means we'll also try to evaluate "onbranch"
+ * conditions.
+ *
+ * We cannot read branches when constructing the refdb, so it is not
+ * possible to evaluate those conditions in the first place. To gate
+ * their evaluation we check whether or not the reference storage
+ * format has been configured -- we thus have to temporarily set it to
+ * UNKNOWN here so that we don't end up recursing.
+ */
+ format = r->ref_storage_format;
+ r->ref_storage_format = REF_STORAGE_FORMAT_UNKNOWN;
+ r->refs_private = ref_store_init(r, format, r->gitdir, REF_STORE_ALL_CAPS);
r->refs_private = maybe_debug_wrap_ref_store(r->gitdir, r->refs_private);
+ r->ref_storage_format = format;
+
return r->refs_private;
}
--
2.54.0.1189.g8c84645362.dirty
^ permalink raw reply related
* [PATCH 8/9] refs: drop local buffer in `refs_compute_filesystem_location()`
From: Patrick Steinhardt @ 2026-06-10 14:57 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
In-Reply-To: <20260610-b4-pks-refs-avoid-chdir-notify-reparent-v1-0-56c864b01c43@pks.im>
We're using a local buffer in `refs_compute_filesystem_location()` that
is only used so that we can fill it and then call `strbuf_realpath()` on
its result. This roundtrip isn't necessary though: `strbuf_realpath()`
already knows to use a single buffer as both input and output at the
same time. So all this does is to add a bit of confusion and an extra
memory allocation.
Drop the local buffer.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
refs.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/refs.c b/refs.c
index e69b9b8ac8..4912510590 100644
--- a/refs.c
+++ b/refs.c
@@ -3571,8 +3571,6 @@ void refs_compute_filesystem_location(const char *gitdir, const char *payload,
bool *is_worktree, struct strbuf *refdir,
struct strbuf *ref_common_dir)
{
- struct strbuf sb = STRBUF_INIT;
-
*is_worktree = get_common_dir_noenv(ref_common_dir, gitdir);
if (!payload) {
@@ -3586,8 +3584,8 @@ void refs_compute_filesystem_location(const char *gitdir, const char *payload,
}
if (!is_absolute_path(payload)) {
- strbuf_addf(&sb, "%s/%s", ref_common_dir->buf, payload);
- strbuf_realpath(ref_common_dir, sb.buf, 1);
+ strbuf_addf(ref_common_dir, "/%s", payload);
+ strbuf_realpath(ref_common_dir, ref_common_dir->buf, 1);
} else {
strbuf_realpath(ref_common_dir, payload, 1);
}
@@ -3600,6 +3598,4 @@ void refs_compute_filesystem_location(const char *gitdir, const char *payload,
BUG("worktree path does not contain slash");
strbuf_addf(refdir, "/worktrees/%s", wt_id + 1);
}
-
- strbuf_release(&sb);
}
--
2.54.0.1189.g8c84645362.dirty
^ permalink raw reply related
* [PATCH 9/9] refs: always use absolute paths for reference stores
From: Patrick Steinhardt @ 2026-06-10 14:57 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
In-Reply-To: <20260610-b4-pks-refs-avoid-chdir-notify-reparent-v1-0-56c864b01c43@pks.im>
Both the "files" and "reftable" backends use
`refs_compute_filesystem_location()` to figure out the location of both
the git and common directories. Depending on how the function is called
we may or may not return an absolute path.
There isn't really a good reason to use relative paths though. Quite on
the contrary, because we sometimes use relative paths we are forced to
register for chdir(3p) notifications via `chdir_notify_reparent()`.
Adapt the function to always return absolute paths. This results in a
user-visible change in behaviour where we now unconditionally print
absolute paths in error messages. But arguably, that change in behaviour
is acceptable and may even be good in cases where a Git command may end
up accessing references across multiple different repositories.
Furthermore, drop the calls to `chdir_notify_reparent()`, which aren't
required anymore now that the paths are always absolute.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
refs.c | 11 ++++++++---
refs/files-backend.c | 22 ----------------------
refs/packed-backend.c | 18 +-----------------
refs/reftable-backend.c | 17 -----------------
t/pack-refs-tests.sh | 6 +++---
t/t0600-reffiles-backend.sh | 4 ++--
t/t1423-ref-backend.sh | 9 ++++++---
t/t5510-fetch.sh | 2 +-
8 files changed, 21 insertions(+), 68 deletions(-)
diff --git a/refs.c b/refs.c
index 4912510590..8679677bf7 100644
--- a/refs.c
+++ b/refs.c
@@ -3579,15 +3579,16 @@ void refs_compute_filesystem_location(const char *gitdir, const char *payload,
* worktree path, as the 'gitdir' here is already the worktree
* path and is different from 'commondir' denoted by 'ref_common_dir'.
*/
+ strbuf_reset(refdir);
strbuf_addstr(refdir, gitdir);
- return;
+ goto out;
}
if (!is_absolute_path(payload)) {
strbuf_addf(ref_common_dir, "/%s", payload);
- strbuf_realpath(ref_common_dir, ref_common_dir->buf, 1);
} else {
- strbuf_realpath(ref_common_dir, payload, 1);
+ strbuf_reset(ref_common_dir);
+ strbuf_addstr(ref_common_dir, payload);
}
strbuf_addbuf(refdir, ref_common_dir);
@@ -3598,4 +3599,8 @@ void refs_compute_filesystem_location(const char *gitdir, const char *payload,
BUG("worktree path does not contain slash");
strbuf_addf(refdir, "/worktrees/%s", wt_id + 1);
}
+
+out:
+ strbuf_realpath(ref_common_dir, ref_common_dir->buf, 1);
+ strbuf_realpath(refdir, refdir->buf, 1);
}
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 296981584b..762f392e67 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -21,7 +21,6 @@
#include "../lockfile.h"
#include "../path.h"
#include "../dir.h"
-#include "../chdir-notify.h"
#include "../setup.h"
#include "../worktree.h"
#include "../wrapper.h"
@@ -100,23 +99,6 @@ static void clear_loose_ref_cache(struct files_ref_store *refs)
}
}
-static void files_ref_store_reparent(const char *name UNUSED,
- const char *old_cwd,
- const char *new_cwd,
- void *payload)
-{
- struct files_ref_store *refs = payload;
- char *tmp;
-
- tmp = reparent_relative_path(old_cwd, new_cwd, refs->base.gitdir);
- free(refs->base.gitdir);
- refs->base.gitdir = tmp;
-
- tmp = reparent_relative_path(old_cwd, new_cwd, refs->gitcommondir);
- free(refs->gitcommondir);
- refs->gitcommondir = tmp;
-}
-
/*
* Create a new submodule ref cache and add it to the internal
* set of caches.
@@ -145,10 +127,7 @@ static struct ref_store *files_ref_store_init(struct repository *repo,
repo_config_get_bool(repo, "core.prefersymlinkrefs", &refs->prefer_symlink_refs);
- chdir_notify_register(NULL, files_ref_store_reparent, refs);
-
strbuf_release(&refdir);
-
return ref_store;
}
@@ -197,7 +176,6 @@ static void files_ref_store_release(struct ref_store *ref_store)
free(refs->gitcommondir);
ref_store_release(refs->packed_ref_store);
free(refs->packed_ref_store);
- chdir_notify_unregister(NULL, files_ref_store_reparent, refs);
}
static void files_reflog_path(struct files_ref_store *refs,
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 499cb55dfa..89e41a35a3 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -13,7 +13,6 @@
#include "packed-backend.h"
#include "../iterator.h"
#include "../lockfile.h"
-#include "../chdir-notify.h"
#include "../statinfo.h"
#include "../worktree.h"
#include "../wrapper.h"
@@ -211,19 +210,6 @@ static size_t snapshot_hexsz(const struct snapshot *snapshot)
return snapshot->refs->base.repo->hash_algo->hexsz;
}
-static void packed_ref_store_reparent(const char *name UNUSED,
- const char *old_cwd,
- const char *new_cwd,
- void *payload)
-{
- struct packed_ref_store *refs = payload;
- char *tmp;
-
- tmp = reparent_relative_path(old_cwd, new_cwd, refs->path);
- free(refs->path);
- refs->path = tmp;
-}
-
/*
* Since packed-refs is only stored in the common dir, don't parse the
* payload and rely on the files-backend to set 'gitdir' correctly.
@@ -239,10 +225,9 @@ struct ref_store *packed_ref_store_init(struct repository *repo,
base_ref_store_init(ref_store, repo, gitdir, &refs_be_packed);
refs->store_flags = opts->access_flags;
-
strbuf_addf(&sb, "%s/packed-refs", gitdir);
refs->path = strbuf_detach(&sb, NULL);
- chdir_notify_register(NULL, packed_ref_store_reparent, refs);
+
return ref_store;
}
@@ -287,7 +272,6 @@ static void packed_ref_store_release(struct ref_store *ref_store)
clear_snapshot(refs);
rollback_lock_file(&refs->lock);
delete_tempfile(&refs->tempfile);
- chdir_notify_unregister(NULL, packed_ref_store_reparent, refs);
free(refs->path);
}
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 8c93070677..8cc1dbbbdd 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -2,7 +2,6 @@
#include "../git-compat-util.h"
#include "../abspath.h"
-#include "../chdir-notify.h"
#include "../config.h"
#include "../dir.h"
#include "../environment.h"
@@ -365,19 +364,6 @@ static int reftable_be_config(const char *var, const char *value,
return 0;
}
-static void reftable_be_reparent(const char *name UNUSED,
- const char *old_cwd,
- const char *new_cwd,
- void *payload)
-{
- struct reftable_ref_store *refs = payload;
- char *tmp;
-
- tmp = reparent_relative_path(old_cwd, new_cwd, refs->base.gitdir);
- free(refs->base.gitdir);
- refs->base.gitdir = tmp;
-}
-
static struct ref_store *reftable_be_init(struct repository *repo,
const char *payload,
const char *gitdir,
@@ -460,8 +446,6 @@ static struct ref_store *reftable_be_init(struct repository *repo,
goto done;
}
- chdir_notify_register(NULL, reftable_be_reparent, refs);
-
done:
assert(refs->err != REFTABLE_API_ERROR);
strbuf_release(&ref_common_dir);
@@ -487,7 +471,6 @@ static void reftable_be_release(struct ref_store *ref_store)
free(be);
}
strmap_clear(&refs->worktree_backends, 0);
- chdir_notify_unregister(NULL, reftable_be_reparent, refs);
}
static int reftable_be_create_on_disk(struct ref_store *ref_store,
diff --git a/t/pack-refs-tests.sh b/t/pack-refs-tests.sh
index d76b087b09..357413ba3c 100644
--- a/t/pack-refs-tests.sh
+++ b/t/pack-refs-tests.sh
@@ -237,7 +237,7 @@ test_expect_success 'reject packed-refs with unterminated line' '
cp .git/packed-refs .git/packed-refs.bak &&
test_when_finished "mv .git/packed-refs.bak .git/packed-refs" &&
printf "%s" "$HEAD refs/zzzzz" >>.git/packed-refs &&
- echo "fatal: unterminated line in .git/packed-refs: $HEAD refs/zzzzz" >expected_err &&
+ echo "fatal: unterminated line in $(pwd)/.git/packed-refs: $HEAD refs/zzzzz" >expected_err &&
test_must_fail git for-each-ref >out 2>err &&
test_cmp expected_err err
'
@@ -246,7 +246,7 @@ test_expect_success 'reject packed-refs containing junk' '
cp .git/packed-refs .git/packed-refs.bak &&
test_when_finished "mv .git/packed-refs.bak .git/packed-refs" &&
printf "%s\n" "bogus content" >>.git/packed-refs &&
- echo "fatal: unexpected line in .git/packed-refs: bogus content" >expected_err &&
+ echo "fatal: unexpected line in $(pwd)/.git/packed-refs: bogus content" >expected_err &&
test_must_fail git for-each-ref >out 2>err &&
test_cmp expected_err err
'
@@ -255,7 +255,7 @@ test_expect_success 'reject packed-refs with a short SHA-1' '
cp .git/packed-refs .git/packed-refs.bak &&
test_when_finished "mv .git/packed-refs.bak .git/packed-refs" &&
printf "%.7s %s\n" $HEAD refs/zzzzz >>.git/packed-refs &&
- printf "fatal: unexpected line in .git/packed-refs: %.7s %s\n" $HEAD refs/zzzzz >expected_err &&
+ printf "fatal: unexpected line in $(pwd)/.git/packed-refs: %.7s %s\n" $HEAD refs/zzzzz >expected_err &&
test_must_fail git for-each-ref >out 2>err &&
test_cmp expected_err err
'
diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index 74bfa2e9ba..b17f0940c2 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -96,7 +96,7 @@ test_expect_success 'non-empty directory blocks create' - <<\EOT
: >.git/$prefix/foo/bar/baz.lock &&
test_when_finished "rm -f .git/$prefix/foo/bar/baz.lock" &&
cat >expected <<-EOF &&
- fatal: cannot lock ref '$prefix/foo': there is a non-empty directory '.git/$prefix/foo' blocking reference '$prefix/foo'
+ fatal: cannot lock ref '$prefix/foo': there is a non-empty directory '$(pwd)/.git/$prefix/foo' blocking reference '$prefix/foo'
EOF
printf "%s\n" "update $prefix/foo $C" |
test_must_fail git update-ref --stdin 2>output.err &&
@@ -135,7 +135,7 @@ test_expect_success 'non-empty directory blocks indirect create' - <<\EOT
: >.git/$prefix/foo/bar/baz.lock &&
test_when_finished "rm -f .git/$prefix/foo/bar/baz.lock" &&
cat >expected <<-EOF &&
- fatal: cannot lock ref '$prefix/symref': there is a non-empty directory '.git/$prefix/foo' blocking reference '$prefix/foo'
+ fatal: cannot lock ref '$prefix/symref': there is a non-empty directory '$(pwd)/.git/$prefix/foo' blocking reference '$prefix/foo'
EOF
printf "%s\n" "update $prefix/symref $C" |
test_must_fail git update-ref --stdin 2>output.err &&
diff --git a/t/t1423-ref-backend.sh b/t/t1423-ref-backend.sh
index fd47d77e8e..875857f2d0 100755
--- a/t/t1423-ref-backend.sh
+++ b/t/t1423-ref-backend.sh
@@ -145,7 +145,8 @@ do
test_commit 3 &&
git refs migrate --dry-run --ref-format=$to_format >out &&
- BACKEND_PATH="$dir/$(sed "s/.* ${SQ}.git\/\(.*\)${SQ}/\1/" out)" &&
+ BACKEND_PATH=$(sed "s/.* the result can be found at ${SQ}\(.*\)${SQ}$/\1/" out) &&
+ test_path_is_dir "$BACKEND_PATH" &&
test_refs_backend . $from_format "$to_format://$BACKEND_PATH" "$method"
)
'
@@ -160,7 +161,8 @@ do
test_commit 3 &&
git refs migrate --dry-run --ref-format=$to_format >out &&
- BACKEND_PATH="$dir/$(sed "s/.* ${SQ}.git\/\(.*\)${SQ}/\1/" out)" &&
+ BACKEND_PATH=$(sed "s/.* the result can be found at ${SQ}\(.*\)${SQ}$/\1/" out) &&
+ test_path_is_dir "$BACKEND_PATH" &&
test_refs_backend . $from_format "$to_format://$BACKEND_PATH" "$method" &&
@@ -187,7 +189,8 @@ do
test_commit 3 &&
git refs migrate --dry-run --ref-format=$to_format >out &&
- BACKEND_PATH="$dir/$(sed "s/.* ${SQ}.git\/\(.*\)${SQ}/\1/" out)" &&
+ BACKEND_PATH=$(sed "s/.* the result can be found at ${SQ}\(.*\)${SQ}$/\1/" out) &&
+ test_path_is_dir "$BACKEND_PATH" &&
run_with_uri . "$from_format" "$to_format://$BACKEND_PATH" \
"worktree add ../wt 2" "$method" &&
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index eca9a973b5..d5f84d99df 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -1741,7 +1741,7 @@ test_expect_success CASE_INSENSITIVE_FS,REFFILES 'D/F conflict on case insensiti
cd case_insensitive &&
git remote add origin -- ../case_sensitive_df &&
test_must_fail git fetch -f origin "refs/heads/*:refs/heads/*" 2>err &&
- test_grep "cannot lock ref ${SQ}refs/remotes/origin/foo${SQ}: there is a non-empty directory ${SQ}./refs/remotes/origin/foo${SQ} blocking reference ${SQ}refs/remotes/origin/foo${SQ}" err &&
+ test_grep "cannot lock ref ${SQ}refs/remotes/origin/foo${SQ}: there is a non-empty directory ${SQ}$(pwd)/refs/remotes/origin/foo${SQ} blocking reference ${SQ}refs/remotes/origin/foo${SQ}" err &&
git rev-parse refs/heads/main >expect &&
git rev-parse refs/heads/Foo/bar >actual &&
test_cmp expect actual
--
2.54.0.1189.g8c84645362.dirty
^ permalink raw reply related
* Re: [PATCH v4 0/8] Auto-configure advertised remotes via URL allowlist
From: Junio C Hamano @ 2026-06-10 15:11 UTC (permalink / raw)
To: Toon Claes
Cc: Christian Couder, git, Patrick Steinhardt, Taylor Blau,
Karthik Nayak, Elijah Newren, Kristoffer Haugsbakk
In-Reply-To: <877bo7294j.fsf@emacs.iotcl.com>
Toon Claes <toon@iotcl.com> writes:
>> Also I think it's easier to explain that 'acceptFromServerUrl' is a
>> different mechanism (that allows auto-configuration, contrary to
>> 'acceptFromServer') if these two variables are independent.
>
> True, although naming-wise it doesn't feel like that. But I no longer
> gonna keep picking on that, so ignore this comment please. :-)
>
>>> What do you think? If you disagree, I'm fine with the current approach
>>> and I think this version looks good.
>>
>> Thanks for your review and for being fine with the current approach if
>> I disagree.
>
> Thanks for explaining, I still agree moving on like this.
Sounds good. Shall we mark the topic for 'next' then?
Thanks, all.
^ permalink raw reply
* Re: [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root
From: Junio C Hamano @ 2026-06-10 15:21 UTC (permalink / raw)
To: Pablo Sabater
Cc: Chandra Pratap, phillip.wood, git, christian.couder, karthik.188,
jltobler, ayu.chandekar, siddharthasthana31
In-Reply-To: <CAN5EUNSFBC0+aoW1ceGjEiKWBRjzuzUEUjg8Xys5O9rDsJdkjg@mail.gmail.com>
Pablo Sabater <pabloosabaterr@gmail.com> writes:
>> > Do we want cascading or just a fixed indentation?
>> >
>> > * A parentless
>> > * B parentless
>> > * C parentless
>> > * D1 child
>> > * D parentless
>>
>> I am late to the party, but I cannot get how the latter is viable.
>> If "A" had parent "B" whose parent was "C" that is root, wouldn't we
>> see the same output? Or are we adding " parentless" at the end of
>> the one-liner log message?
>
> We wouldn't see the same output because A and B wouldn't get padded in
> that case. Vertical adjacency between indented commits doesn't imply
> relation because indentation means that they are "parentless",
Hmph, I guess such "the first column is special in that two commits
on consecutive lines with the asterisk on the same column, if only
that is on the first column, are parent-child, but it does not hold
in all other columns" was beyond my imagination. And that was why I
said I am late to the party. Do others find such a rule intuitive?
I didn't (and that is what led me to ask the question).
> Anyways, having more than 2 "parentless" commits one after the other
> is strange. Cascading is just having a depth counter and printing the
> padding depth times, so I'll keep it as it is more intuitive.
Is everbody happy with this version, or will we see an updated final
reroll to tie any loose ends? For example, do we need the above
"vertically adjacent commits are in parent-child relationship only
when they appear on the first column" given as a new instruction in
the documentation to help users read and understand what the graph
output is trying to tell them?
Thanks.
^ 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