* Re: followRemoteHEAD management question
From: Matt Hunter @ 2026-06-12 6:11 UTC (permalink / raw)
To: Bence Ferdinandy, Jeff King; +Cc: git
In-Reply-To: <DJ6IBPYNOTTY.3QKEZQ28P713V@ferdinandy.com>
On Thu Jun 11, 2026 at 4:36 PM EDT, Bence Ferdinandy wrote:
> On Thu Jun 11, 2026 at 08:01, Jeff King <peff@peff.net> wrote:
>>
>> My initial thought is that it might affect clone as well as fetch. But I
>> guess this feature does not kick in for clone, as it has its own logic
>> for handling the remote-tracking HEAD. Though arguably it should be
>> possible to configure it not to create one in the first place.
>
> If memory serves well clone has set the remote/HEAD well before this and
> I think it indeed uses a different mechanism/logic.
I'm a little interested to try to look into the clone case as well, but
I think I'll save it for a later patch series and keep the scope of this
one as it is.
> Bit late to the party, but happy to review/test patches if they come.
Greatly appreciated!
>
> Best,
> Bence
The first version of my patches went out. You two are Cc'd on the cover
letter, but that didn't propagate to the patches themselves, oops.
^ permalink raw reply
* Re: [PATCH v2 3/3] doc: git-config: escape erroneous highlight markup
From: Jeff King @ 2026-06-12 6:17 UTC (permalink / raw)
To: Jean-Noël AVILA
Cc: Tuomas Ahola, git, Kristoffer Haugsbakk, Junio C Hamano
In-Reply-To: <20260612051605.GB593075@coredump.intra.peff.net>
On Fri, Jun 12, 2026 at 01:16:06AM -0400, Jeff King wrote:
> So...weird. groff wants to add extra space for some reason. It happens
> even if I drop the bolding, and just have "#" on a line by itself. I
> guess maybe it is the trailing "." of the previous line putting groff
> into "oh, I'm starting a new sentence" mode and it uses two spaces.
>
> But I think that is all outside the scope of your fix, and this is an
> existing issue that we are now just unlucky enough to hit. I'd be
> tempted to ignore it and possibly fix it later.
Poking at the groff manpage for possible fixes, I think it is either:
1. Just turn off extra spaces between sentences, like this:
.ss 12 0
The first argument "12" is "the minimum word space is 12/12ths of
the current font's word space. The second is "also add 0/12ths
between sentences" (so, no extra space).
That prevents the problem from happening anywhere, but maybe people
really like the extra spaces in other contexts.
2. There are some magic characters marked as "this doesn't start a new
sentence". We can set that flag for "#" like this:
.cflags 32 \#
I don't think docbook has parameter support for either of those, so we'd
have to do some kind of "shove this into the header" magic, which is a
bit gross.
-Peff
^ permalink raw reply
* Re: [PATCH 3/9] setup: don't apply "GIT_REFERENCE_BACKEND" without a repository
From: Patrick Steinhardt @ 2026-06-12 6:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Karthik Nayak
In-Reply-To: <xmqqa4t2wbb5.fsf@gitster.g>
On Wed, Jun 10, 2026 at 10:32:46AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > 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.
>
> Does the second point have a small typo, i.e., "if we have a
> repository, we have already applied the ref backend to it when we
> discovered it, so NO need to manually reapply"?
No, this is correct as-is. At the point in time where we handle
GIT_REFERENCE_BACKEND we have already discovered the repository format,
applied it to the repository, configured the reference database format
et al. So because we handle GIT_REFERENCE_BACKEND _after_ that whole
dance we basically have to re-configure the reference database format,
which is awkward.
Patrick
^ permalink raw reply
* Re: [PATCH 0/9] refs: stop using `chdir_notify_reparent()`
From: Patrick Steinhardt @ 2026-06-12 6:18 UTC (permalink / raw)
To: Jeff King; +Cc: git, Karthik Nayak
In-Reply-To: <20260611065346.GD2191159@coredump.intra.peff.net>
On Thu, Jun 11, 2026 at 02:53:46AM -0400, Jeff King wrote:
> On Wed, Jun 10, 2026 at 04:57:06PM +0200, Patrick Steinhardt wrote:
>
> > 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()`.
>
> We added chdir-notify to suport set_work_tree(). Commit 8500e0de3f
> (set_work_tree: use chdir_notify, 2018-03-30) mentions an optimization
> from 044bbbcb63 (Make git_dir a path relative to work_tree in
> setup_work_tree(), 2008-06-19). That commit demonstrates some measurable
> speedup from using relative versus absolute paths.
Oh, that is context I wasn't aware of. Not much of a surprise though,
given that this is from 2008 :) So thanks a lot for the pointer!
> If we move to a world of all absolute paths where chdir-notify is not
> necessary, will we lose that optimization?
Probably. Unfortunately, the commit doesn't have any repeatable
benchmarks in there, so it's hard to say whether we could still
reproduce those issues or not.
> I'm not sure how much it matters in practice these days, or if those
> timings could be repeated. And they weren't all _that_ big to start
> with. I guess it may depend on how deep your repo is within your
> filesystem, too.
Ideally, we'd have the best of both worlds: absolute paths everywhere
without the performance hit. A while back I had a discussion with
Torvalds on the securiy mailing list around this issue, and ultimately
the conclusion was that the best way forward would be to use openat(3p).
This wouldn't only allow us to optimize cases like this, but it also has
the added benefit that we're much less prone to TOCTOU-style issues and
we might even be able to use flags like O_BENEATH. So it would basically
be win-win. The only problem is of course that Windows doesn't have
openat(3p), so we'd have to emulate it, and that's where I always lost
the desire to do this.
When waking up this morning though I had the thought that we shouldn't
try to emulate openat(3p) directly, but instead create a higher-level
interface.
struct fsroot;
/*
* Open a new filesystem root at the given directory. All subsequent
* calls to open will be relative to this fsroot.
*/
struct fsroot *fsroot_new(const char *dir);
/*
* Create a new fsroot from a subdirectory relative to the given
* root directory.
*/
struct fsroot *fsroot_new_subdir(struct fsroot *r, const char *dir);
/*
* Open a new file relative to the given fsroot. This will use the
* equivalent of O_BENEATH so that we only ever open files that are
* located below the fsroot.
*/
int fsroot_open(struct fsroot *r, const char *path, int oflag, ...);
This is of course heavily inspired by similar interfaces that exist in
Go [1]. By having such a higher-level abstraction it should also be way
easier to port this to different platforms, where we can then add safety
features like O_BENEATH when available on any given platform.
The idea here would be that we can then convert some subsystems to use
those structures instead of tracking paths. I'd for example love for the
repository's working tree to use this mechanism so that we can squash a
whole class of potential security issues when checking out files that
end in locations we didn't intend to.
Thanks!
Patrick
[1]: https://pkg.go.dev/io/fs#FS
^ permalink raw reply
* Re: [PATCH v3 1/3] commit-reach: handle cycles in contains walk
From: Kristofer Karlsson @ 2026-06-12 6:53 UTC (permalink / raw)
To: Tamir Duberstein
Cc: git, Jeff King, Karthik Nayak, Junio C Hamano, Victoria Dye,
Derrick Stolee, Elijah Newren
In-Reply-To: <20260611-ref-filter-memoized-contains-v3-1-b26af3dba285@gmail.com>
On Fri, 12 Jun 2026 at 05:00, Tamir Duberstein <tamird@gmail.com> wrote:
>
> The memoized contains traversal used by git tag assumes that commit
> ancestry is acyclic. Replacement refs can violate that assumption,
> causing it to keep pushing an already active commit until memory is
> exhausted.
>
The cycle detection itself makes sense, but would it be simpler to
just die() when a cycle is found rather than falling back to a
second reachability walk?
A cycle in the commit graph means replacement refs are
misconfigured. The existing code already loops forever when it
hits one, so detecting and dying is strictly an improvement. The
fallback adds a second codepath through the function, discards all
cached results (so later candidates redo work), and papers over
what is really a broken invariant.
do_lookup_replace_object() already dies when replacement refs
chain deeper than MAXREPLACEDEPTH (which covers cycles), so the
existing contract treats this as a fatal configuration error.
parse_commit_or_die() sets the same precedent within the walk
itself.
Kristofer
^ permalink raw reply
* Re: [PATCH v2 1/1] environment.c: move 'protect_hfs' and 'protect_ntfs' into 'repo_config_values'
From: Christian Couder @ 2026-06-12 7:22 UTC (permalink / raw)
To: Junio C Hamano
Cc: Tian Yuchen, git, phillip.wood123, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <xmqqse6uwdnz.fsf@gitster.g>
On Wed, Jun 10, 2026 at 6:41 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Tian Yuchen <cat@malon.dev> writes:
>
> > +int repo_protect_ntfs(struct repository *repo)
> > +{
> > + return repo->gitdir ?
> > + repo_config_values(repo)->protect_ntfs :
> > + PROTECT_NTFS_DEFAULT;
> > +}
> > +
> > +int repo_protect_hfs(struct repository *repo)
> > +{
> > + return repo->gitdir ?
> > + repo_config_values(repo)->protect_hfs :
> > + PROTECT_HFS_DEFAULT;
> > +}
> > ...
> > @@ -123,6 +125,14 @@ int git_default_config(const char *, const char *,
> > int git_default_core_config(const char *var, const char *value,
> > const struct config_context *ctx, void *cb);
> >
> > +/*
> > + * Getters for the `protect_hfs` and `protect_ntfs` fields of `struct repo_config_values`.
> > + * They check `repo->gitdir` to prevent calling repo_config_values()
> > + * before the configuration is loaded or in bare environments.
> > + */
> > +int repo_protect_hfs(struct repository *repo);
> > +int repo_protect_ntfs(struct repository *repo);
>
> I briefly wondered what *should* happen when repo->gitdir is not
> ready, as it feels almost a bug for a caller to call these two
> functions before the repository is ready to be used.
>
> When repo is not ready, these return their respective default
> values. That's like the original code using the initial value of
> these global variables.
>
> IOW, this rewrite is bug-for-bug compatible, which is good.
>
> Shall we declare victory and mark the topic for 'next' now?
I would have preferred the commit subject to start with "environment:"
rather than "environment.c:" but it's a small nit and maybe you can
fix it while merging.
Otherwise the patch looks indeed good to me.
Thanks.
^ permalink raw reply
* Re: [PATCH v2 2/3] read-cache: move 'ce_mode_from_stat()' to 'read-cache.c'
From: Christian Couder @ 2026-06-12 7:43 UTC (permalink / raw)
To: Tian Yuchen; +Cc: git, ps, Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260610093635.139719-3-cat@malon.dev>
On Wed, Jun 10, 2026 at 11:37 AM Tian Yuchen <cat@malon.dev> wrote:
>
> The ce_mode_from_stat() function is declared as a static inline function
> in 'read-cache.h'. As we want to migrate configuration variables, this
> helper function will need access to corresponding repository-specific
> configuration logic. Move the implementation to 'read-cache.c' to
> cleanly encapsulate its dependencies.
>
> Note that the 'extern int trust_executable_bit, has_symlinks;' line is
> discarded because it's not necessary when the function lives in
> "read-cache.c".
>
> At present, this change has no visible impact, but it is crucial
> for our future plans to pass in the repo context. Comment
> has been added whilst we are at it.
We prefer it when comments like the one below are added in front of
the declaration of the function into the header file ("read-cache.h"
here), rather than the *.c file.
And yeah, I know that "read-cache.h" is a bad example right now
because no function has such a comment there yet.
> +/*
> + * Determine the appropriate index mode for a file based on its stat()
> + * information and the existing cache entry (if any).
> + *
> + * This function handles degradation for filesystems that lack
> + * symlink support or reliable executable bits.
> + */
> +unsigned int ce_mode_from_stat(const struct cache_entry *ce, unsigned int mode)
> +{
> + if (!has_symlinks && S_ISREG(mode) &&
> + ce && S_ISLNK(ce->ce_mode))
> + return ce->ce_mode;
> + if (!trust_executable_bit && S_ISREG(mode)) {
> + if (ce && S_ISREG(ce->ce_mode))
> + return ce->ce_mode;
> + return create_ce_mode(0666);
> + }
> + return create_ce_mode(mode);
> +}
^ permalink raw reply
* Re: [PATCH v2 3/3] environment: move trust_executable_bit into repo_config_values
From: Christian Couder @ 2026-06-12 7:48 UTC (permalink / raw)
To: Tian Yuchen; +Cc: git, ps, Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260610093635.139719-4-cat@malon.dev>
On Wed, Jun 10, 2026 at 11:37 AM Tian Yuchen <cat@malon.dev> wrote:
> +/*
> + * Getters for the `repo_trust_executable_bit` fields of `struct repo_config_values`.
s/Getters/Getter/
s/fields/field/
s/repo_trust_executable_bit/trust_executable_bit/
> + * They check `repo->gitdir` to prevent calling repo_config_values()
> + * before the configuration is loaded or in bare environments.
> + */
> +int repo_trust_executable_bit(struct repository *repo);
Thanks.
^ permalink raw reply
* Re: [PATCH v2 5/7] environment: split up concerns of `is_bare_repository_cfg`
From: Toon Claes @ 2026-06-12 7:59 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Justin Tobler
In-Reply-To: <20260611-b4-pks-setup-drop-global-state-v2-5-a6f7269c841d@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> The `is_bare_repository_cfg` variable tracks two different pieces of
> information:
>
> - It tracks whether the user has invoked git with the "--bare" flag,
> which makes us treat any discovered Git repository as if it was a
> bare repository.
>
> - Otherwise it tracks whether the discovered `the_repository` is bare.
>
> This makes the flag extremely confusing and creates a bit of a challenge
> when handling multiple repositories in the same process.
>
> Split up the concerns of this variable into two pieces:
>
> - `startup_info.force_bare_repository` tracks whether the user has
> passed the "--bare" flag. This is used as a hint to treat newly set
> up repositories as bare regardless of whether or not they have a
> worktree.
>
> - `struct repository::bare_cfg` tracks whether or not a repository is
> considered bare. This takes into account both whether the user has
> passed "--bare" and the discovered state of the repository itself.
>
> Whether or not a repository is bare is now resolved when checking the
> repository's format, and is then later applied to the repository itself
> via `apply_repository_format()`.
>
> This enables a subsequent change where we make `is_bare_repository()`
> not depend on global state anymore.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> builtin/init-db.c | 2 +-
> environment.c | 5 ++---
> environment.h | 1 -
> git.c | 2 +-
> repository.c | 1 +
> repository.h | 7 +++++++
> setup.c | 27 ++++++++++++++++++++-------
> setup.h | 6 ++++++
> worktree.c | 2 +-
> 9 files changed, 39 insertions(+), 14 deletions(-)
>
> diff --git a/builtin/init-db.c b/builtin/init-db.c
> index 52aa92fb0a..566732c9f4 100644
> --- a/builtin/init-db.c
> +++ b/builtin/init-db.c
> @@ -81,7 +81,7 @@ int cmd_init_db(int argc,
> const char *template_dir = NULL;
> char *template_dir_to_free = NULL;
> unsigned int flags = 0;
> - int bare = is_bare_repository_cfg;
> + int bare = startup_info->force_bare_repository ? 1 : -1;
> const char *object_format = NULL;
> const char *ref_format = NULL;
> const char *initial_branch = NULL;
> diff --git a/environment.c b/environment.c
> index 4e86335f25..9d7c908c55 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -48,7 +48,6 @@ int has_symlinks = 1;
> int minimum_abbrev = 4, default_abbrev = -1;
> int ignore_case;
> int assume_unchanged;
> -int is_bare_repository_cfg = -1; /* unspecified */
> int warn_on_object_refname_ambiguity = 1;
> char *git_commit_encoding;
> char *git_log_output_encoding;
> @@ -136,7 +135,7 @@ const char *getenv_safe(struct strvec *argv, const char *name)
> int is_bare_repository(void)
> {
> /* if core.bare is not 'false', let's see if there is a work tree */
> - return is_bare_repository_cfg && !repo_get_work_tree(the_repository);
> + return the_repository->bare_cfg && !repo_get_work_tree(the_repository);
> }
>
> int have_git_dir(void)
> @@ -342,7 +341,7 @@ int git_default_core_config(const char *var, const char *value,
> }
>
> if (!strcmp(var, "core.bare")) {
> - is_bare_repository_cfg = git_config_bool(var, value);
> + the_repository->bare_cfg = git_config_bool(var, value);
> return 0;
> }
>
> diff --git a/environment.h b/environment.h
> index 5d6e4e6c1b..afb5bcf197 100644
> --- a/environment.h
> +++ b/environment.h
> @@ -147,7 +147,6 @@ void repo_config_values_init(struct repo_config_values *cfg);
> */
> int have_git_dir(void);
>
> -extern int is_bare_repository_cfg;
> int is_bare_repository(void);
>
> /* Environment bits from configuration mechanism */
> diff --git a/git.c b/git.c
> index 36f08891ef..387eabe38c 100644
> --- a/git.c
> +++ b/git.c
> @@ -255,7 +255,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
> *envchanged = 1;
> } else if (!strcmp(cmd, "--bare")) {
> char *cwd = xgetcwd();
> - is_bare_repository_cfg = 1;
> + startup_info->force_bare_repository = true;
> setenv(GIT_DIR_ENVIRONMENT, cwd, 0);
> free(cwd);
> setenv(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, "0", 1);
> diff --git a/repository.c b/repository.c
> index 187dd471c4..c1e91eb0da 100644
> --- a/repository.c
> +++ b/repository.c
> @@ -73,6 +73,7 @@ void initialize_repository(struct repository *repo)
> ALLOC_ARRAY(repo->index, 1);
> index_state_init(repo->index, repo);
> repo->check_deprecated_config = true;
> + repo->bare_cfg = -1;
> repo_config_values_init(&repo->config_values_private_);
>
> /*
> diff --git a/repository.h b/repository.h
> index 36e2db2633..7d649e32e7 100644
> --- a/repository.h
> +++ b/repository.h
> @@ -117,6 +117,13 @@ struct repository {
> bool worktree_initialized;
> bool worktree_config_is_bogus;
>
> + /*
> + * Whether the repository is bare, as set by "core.bare" config or
> + * inferred during repository discovery. -1 means unset/unknown, 0
> + * means non-bare, 1 means bare.
> + */
> + int bare_cfg;
> +
> /*
> * Path from the root of the top-level superproject down to this
> * repository. This is only non-NULL if the repository is initialized
> diff --git a/setup.c b/setup.c
> index 71fc6b33da..32f14a8688 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -795,10 +795,22 @@ static int check_repository_format_gently(const char *gitdir,
> has_common = 0;
> }
>
> - if (!has_common) {
> - if (candidate->is_bare != -1)
> - is_bare_repository_cfg = candidate->is_bare;
> - } else {
> + if (startup_info->force_bare_repository) {
> + candidate->is_bare = 1;
> + FREE_AND_NULL(candidate->work_tree);
> + } else if (has_common) {
> + /*
> + * When sharing a common dir with another repository (e.g. a
> + * linked worktree), do not let this repository's config
> + * dictate bareness; it is inherited from the main worktree.
> + */
> + candidate->is_bare = -1;
> +
> + /*
> + * Furthermore, "core.worktree" is supposed to be ignored when
> + * we have a commondir configured, unless it comes from the
> + * per-worktree configuration.
> + */
> FREE_AND_NULL(candidate->work_tree);
> }
Looking at the diff, this is really hard to understand. But your
refactor makes sense and the after state is easier to comprehend.
> @@ -1138,7 +1150,7 @@ static const char *setup_explicit_git_dir(struct repository *repo,
> /* #3, #7, #11, #15, #19, #23, #27, #31 (see t1510) */
> if (work_tree_env)
> set_git_work_tree(repo, work_tree_env);
> - else if (is_bare_repository_cfg > 0) {
> + else if (repo_fmt->is_bare > 0) {
> if (repo_fmt->work_tree) {
> /* #22.2, #30 */
> warning("core.bare and core.worktree do not make sense");
> @@ -1225,7 +1237,7 @@ static const char *setup_discovered_git_dir(struct repository *repo,
> }
>
> /* #16.2, #17.2, #20.2, #21.2, #24, #25, #28, #29 (see t1510) */
> - if (is_bare_repository_cfg > 0) {
> + if (repo_fmt->is_bare > 0) {
> set_git_dir(repo, gitdir, (offset != cwd->len));
> if (chdir(cwd->buf))
> die_errno(_("cannot come back to cwd"));
> @@ -1762,6 +1774,7 @@ int apply_repository_format(struct repository *repo,
> alternate_object_directories = xstrdup_or_null(getenv(ALTERNATE_DB_ENVIRONMENT));
> }
>
> + repo->bare_cfg = format->is_bare;
> repo_set_hash_algo(repo, format->hash_algo);
> repo->objects = odb_new(repo, object_directory,
> alternate_object_directories);
> @@ -2571,7 +2584,7 @@ static int create_default_files(struct repository *repo,
> repo_settings_set_shared_repository(repo,
> init_shared_repository);
>
> - is_bare_repository_cfg = !work_tree;
> + repo->bare_cfg = !work_tree;
I'm curious, do we still need this? If I'm not mistaken, this function
is called after check_and_apply_repository_format(), which calls
apply_repository_format() and sets repo->bare_cfg too (see the diff
above). Or is it explained by what Justin said[1]?
[1]: <airTpB8Pm6TYoMhx@denethor>
--
Cheers,
Toon
^ permalink raw reply
* Re: [PATCH 7/7] treewide: drop USE_THE_REPOSITORY_VARIABLE
From: Toon Claes @ 2026-06-12 8:02 UTC (permalink / raw)
To: Patrick Steinhardt, git
In-Reply-To: <20260610-b4-pks-setup-drop-global-state-v1-7-5dff3eec8f06@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> Adapt a couple of trivial callers of `is_bare_repository()` to instead
> use a repository available via the caller's context so that we can drop
> the `USE_THE_REPOSITORY_VARIABLE` macro.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> builtin/repack.c | 3 +--
> mailmap.c | 6 ++----
> refs/reftable-backend.c | 4 +---
> setup.c | 3 +--
> 4 files changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/builtin/repack.c b/builtin/repack.c
> index bbc6f51639..d0465fb4f5 100644
> --- a/builtin/repack.c
> +++ b/builtin/repack.c
> @@ -1,4 +1,3 @@
> -#define USE_THE_REPOSITORY_VARIABLE
This makes it all worthwhile. Great work on untangling this!
--
Cheers,
Toon
^ permalink raw reply
* Re: [PATCH v2 1/7] builtin/init: stop modifying global `git_work_tree_cfg` variable
From: Toon Claes @ 2026-06-12 8:04 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Justin Tobler
In-Reply-To: <20260611-b4-pks-setup-drop-global-state-v2-1-a6f7269c841d@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> When executing git-init(1) we need to figure out the final location of
> the worktree. This location can be configured in a couple of ways: via
> an environment variable, via the preexisting "core.worktree" config in
> case we're reinitializing, or implicitly when reinitializing a non-bare
> repository.
Do you mean:
> case we're reinitializing, or implicitly when initializing a non-bare
> repository.
So the second 'init' without the 're'?
Obviously not worth a reroll on it's own.
--
Cheers,
Toon
^ permalink raw reply
* Re: [PATCH v2 0/7] setup: drop global state
From: Toon Claes @ 2026-06-12 8:06 UTC (permalink / raw)
To: Justin Tobler, Patrick Steinhardt; +Cc: git
In-Reply-To: <airVOrTboNDDGBak@denethor>
Justin Tobler <jltobler@gmail.com> writes:
> I find the additional explaination here quite helpful. Thanks.
Yeah, hard to follow series looking at the code only, but commit
messages make more bearable.
> The changes in this version of the series looks good to me.
I've only reviewed v2 and I agree this version looks good.
--
Cheers,
Toon
^ permalink raw reply
* Re: [PATCH v2 1/3] replay: refactor enum replay_mode into a bool
From: Toon Claes @ 2026-06-12 8:19 UTC (permalink / raw)
To: Justin Tobler; +Cc: git
In-Reply-To: <airORumUxyTsN7Bz@denethor>
Justin Tobler <jltobler@gmail.com> writes:
> Naive question: Do we expect there to only be two replay modes in the
> forseeable future? I suppose if other modes were added in the future
> this change would essentially be reverted.
The enum was mainly used to determine "direction": PICK to apply commits
forward, and REVERT to apply them in opposite order. But it's a bit
twofold, because REVERT also applies the inverse change. At the moment
--onto and --advance use PICK and --revert uses REVERT. There could be
added more options in the future, but I don't expect any of them to add
a new mode. And if there is ever a new mode needed, I think it's better
to re-add the enum then, or maybe a second bool makes sense then, who
knows...
--
Cheers,
Toon
^ permalink raw reply
* Re: [RFC PATCH] MyFirstContribution: mention trimming quoted text in replies
From: Weijie Yuan @ 2026-06-12 8:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqcxxwljue.fsf@gitster.g>
On Thu, Jun 11, 2026 at 04:48:25PM -0700, Junio C Hamano wrote:
> [...]
> > diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
> > index 607876f3d8..0e2a9313ce 100644
> > --- a/Documentation/MyFirstContribution.adoc
> > +++ b/Documentation/MyFirstContribution.adoc
> > @@ -1453,6 +1453,11 @@ effect which had not occurred to you. It is always okay to ask for clarification
> > if you aren't sure why a change was suggested, or what the reviewer is asking
> > you to do.
> >
> > +When replying to review comments, quote only the parts of the message that are
> > +relevant to your response. It is usually helpful to trim away unrelated context,
> > +such as large portions of the patch that are not being discussed, while keeping
> > +enough quoted text for readers to understand what you are responding to.
> > +
> > Make sure your email client has a plaintext email mode and it is turned on; the
> > Git list rejects HTML email. Please also follow the mailing list etiquette
> > outlined in the
>
> The insertion point is well chosen, immediately following the
> discussion on how to handle review comments and before the technical
> details of email client configuration. The text itself is clear and
> gives sound advice.
Thank you for your reply.
I first considered folding this into one of the existing paragraphs
above, but ended up making it a separate paragraph for better clarity.
Let's see whether others have better wording suggestions.
Now the expectations for contributors and reviewers are more aligned
when participating in review discussions :-)
Hopefully this addition will make review discussions a little easier to
focus and reduce the burden on reviewers.
Thanks!
^ permalink raw reply
* Re: [PATCH 2/9] setup: stop applying repository format twice
From: Karthik Nayak @ 2026-06-12 9:00 UTC (permalink / raw)
To: Patrick Steinhardt, git
In-Reply-To: <20260610-b4-pks-refs-avoid-chdir-notify-reparent-v1-2-56c864b01c43@pks.im>
[-- Attachment #1: Type: text/plain, Size: 3140 bytes --]
Patrick Steinhardt <ps@pks.im> writes:
> 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.
>
Okay so we're talking applying the repository format to the `struct
repository` specifically.
> 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.
This was a bit confusing to read at first. Okay since we already apply
the format in the second step, the first is not necessary.
> The only reason we have to do this is
> because we call `repository_format_configure()` after we have already
> applied it.
Right, so there is a need to do this.
>
> 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.
>
Makse sense.
> 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
The patch looks good.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [PATCH 4/9] refs: unregister reference stores from "chdir_notify"
From: Karthik Nayak @ 2026-06-12 9:18 UTC (permalink / raw)
To: Patrick Steinhardt, git
In-Reply-To: <20260610-b4-pks-refs-avoid-chdir-notify-reparent-v1-4-56c864b01c43@pks.im>
[-- Attachment #1: Type: text/plain, Size: 6726 bytes --]
Patrick Steinhardt <ps@pks.im> writes:
> 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.
>
So `ref_store_release()` is what is called to release a ref_store. So
in the former's case, we never release the ref_store even if the
repository is closed, wow.
> 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
Shouldn't this be s/chdir_notify_parent/chdir_notify_reparent ?
> 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.
Right, we can't unregister when using `chdir_notify_reparent()` because
it internally calls `chdir_notify_register()` with a private cb
function, and we need to supply the callback function during
un-registering. Makes sense.
>
> 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;
> +}
> +
Looks similar to `void reparent_cb()` but for both the directories.
> /*
> * 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
The changes here look good.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [PATCH 6/9] repository: free main reference database
From: Karthik Nayak @ 2026-06-12 9:20 UTC (permalink / raw)
To: Patrick Steinhardt, git
In-Reply-To: <20260610-b4-pks-refs-avoid-chdir-notify-reparent-v1-6-56c864b01c43@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1131 bytes --]
Patrick Steinhardt <ps@pks.im> writes:
> 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.
>
Funny, cause long ago I looked into this and thought I was clearly
missing something and eventually forgot about it. Good to know that I
was correct :)
> 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [PATCH v2] commit-reach: remove get_reachable_subset()
From: Weijie Yuan @ 2026-06-12 9:26 UTC (permalink / raw)
To: Junio C Hamano
Cc: Derrick Stolee, Kristofer Karlsson via GitGitGadget, git,
Kristofer Karlsson, Patrick Steinhardt
In-Reply-To: <xmqq7bo5nf31.fsf@gitster.g>
On Thu, Jun 11, 2026 at 10:48:18AM -0700, Junio C Hamano wrote:
> I wonder if we should talk about it in the SubmittingPatches and/or
> MyFirstContribution document?
Hi, I think it might be a good idea to cover these details in
MyFirstContribution, then cross-reference them from the part of
SubmittingPatches that discusses sending a new version.
More specifically, I think these details could fit around steps 3 and 4
of "A typical life cycle of a patch series" in SubmittingPatches,
starting around line 54. That section may need some reworking of the
existing wording, rather than just an additive change.
Also, for the part about sending a new version, I wonder whether it
would be better to summarize and fold in Patrick's explanation here,
thank you Patrick:
---
From: Patrick Steinhardt <ps@pks.im>
Message-ID: <aietF4BX1Ewt3cpG@pks.im>
> By the way, how long should I wait before sending new versions of my
> patches? I have 4 outstanding at the moment.
I typically aim to send at most one version per day per patch series.
This avoids that you're "flooding" the mailing list with too many
versions of the same series, allows you to address feedback from
multiple folks in batches, and it gives you enough time to think about
the feedback without having to rush anything.
Whether I actually do end up sending a series depends on a couple of
factors:
- How big is the series? The bigger it is the more time I give folks
to perform reviews.
- How substantial were the reviews you received? Is it just a couple
of small typos? Then it probably makes sense to wait one or two more
days to get some more involved reviews. Is it something that
requires signifciant rework? Then I'd send out soon so that others
don't review a patch series that will change significantly anyway.
- How close to being merged is the series? The closer it is the less
substantial the reviews will (hopefully) get, so it makes sense to
reroll a bit faster even if you only received minor feedback.
So there isn't really a golden rule to follow here, but a lot of this
depends on gut feeling. You probably won't have that feeling yet when
starting out in a new project, but that's fine. In case we see that
behaviour doesn't quite match the norm we'll typically give a hint that
the contributor should slow down or maybe send a new iteration.
Patrick
---
Patrick's point may be beyond the scope of this thread, so I only
mention it as an aside. Maybe these could be part of the same series.
Thanks.
^ permalink raw reply
* Re: [PATCH v2 1/7] builtin/init: stop modifying global `git_work_tree_cfg` variable
From: Patrick Steinhardt @ 2026-06-12 9:27 UTC (permalink / raw)
To: Toon Claes; +Cc: git, Justin Tobler
In-Reply-To: <87pl1wyyjw.fsf@emacs.iotcl.com>
On Fri, Jun 12, 2026 at 10:04:35AM +0200, Toon Claes wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > When executing git-init(1) we need to figure out the final location of
> > the worktree. This location can be configured in a couple of ways: via
> > an environment variable, via the preexisting "core.worktree" config in
> > case we're reinitializing, or implicitly when reinitializing a non-bare
> > repository.
>
> Do you mean:
>
> > case we're reinitializing, or implicitly when initializing a non-bare
> > repository.
>
> So the second 'init' without the 're'?
>
> Obviously not worth a reroll on it's own.
It can actually happen in both cases. I've queued the following change
locally. Thanks!
Patrick
1: 0808dbb336 ! 1: cc6999257c builtin/init: stop modifying global `git_work_tree_cfg` variable
@@ Commit message
When executing git-init(1) we need to figure out the final location of
the worktree. This location can be configured in a couple of ways: via
an environment variable, via the preexisting "core.worktree" config in
- case we're reinitializing, or implicitly when reinitializing a non-bare
- repository.
+ case we're reinitializing, or implicitly when (re)initializing a
+ non-bare repository.
When checking for the worktree location in "builtin/init-db.c" we
populate any potentially-discovered value both by setting the global
^ permalink raw reply
* Re: [PATCH v2 5/7] environment: split up concerns of `is_bare_repository_cfg`
From: Patrick Steinhardt @ 2026-06-12 9:28 UTC (permalink / raw)
To: Toon Claes; +Cc: git, Justin Tobler
In-Reply-To: <87wlw4yys1.fsf@emacs.iotcl.com>
On Fri, Jun 12, 2026 at 09:59:42AM +0200, Toon Claes wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > diff --git a/setup.c b/setup.c
> > index 71fc6b33da..32f14a8688 100644
> > --- a/setup.c
> > +++ b/setup.c
> > @@ -795,10 +795,22 @@ static int check_repository_format_gently(const char *gitdir,
> > has_common = 0;
> > }
> >
> > - if (!has_common) {
> > - if (candidate->is_bare != -1)
> > - is_bare_repository_cfg = candidate->is_bare;
> > - } else {
> > + if (startup_info->force_bare_repository) {
> > + candidate->is_bare = 1;
> > + FREE_AND_NULL(candidate->work_tree);
> > + } else if (has_common) {
> > + /*
> > + * When sharing a common dir with another repository (e.g. a
> > + * linked worktree), do not let this repository's config
> > + * dictate bareness; it is inherited from the main worktree.
> > + */
> > + candidate->is_bare = -1;
> > +
> > + /*
> > + * Furthermore, "core.worktree" is supposed to be ignored when
> > + * we have a commondir configured, unless it comes from the
> > + * per-worktree configuration.
> > + */
> > FREE_AND_NULL(candidate->work_tree);
> > }
>
> Looking at the diff, this is really hard to understand. But your
> refactor makes sense and the after state is easier to comprehend.
Yeah, I'm in the same boat. Honestly, I really hope that our test suite
has enough coverage so that this refactoring won't cause any significant
regressions. Which isn't exactly a statement of confidence, but rather a
statement of "oh boy, this is awfully complex and has lots of weird edge
cases".
> > @@ -2571,7 +2584,7 @@ static int create_default_files(struct repository *repo,
> > repo_settings_set_shared_repository(repo,
> > init_shared_repository);
> >
> > - is_bare_repository_cfg = !work_tree;
> > + repo->bare_cfg = !work_tree;
>
> I'm curious, do we still need this? If I'm not mistaken, this function
> is called after check_and_apply_repository_format(), which calls
> apply_repository_format() and sets repo->bare_cfg too (see the diff
> above). Or is it explained by what Justin said[1]?
We can't drop this because after we've applied the format we call
`repo_config()` with `git_default_core_config()`, which will potentially
set the `bare_cfg` variable if "core.bare" is set.
Patrick
^ permalink raw reply
* [GIT PULL] gitk: horizontal scrollbar for commit list
From: Johannes Sixt @ 2026-06-12 9:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
The following changes since commit c8c5df79df34b40119c4bf8e3079520762f258d1:
Merge branch 'jx/i18n-fix' of github.com:jiangxin/gitk (2026-03-20 09:23:32 +0100)
are available in the Git repository at:
https://github.com/j6t/gitk.git master
for you to fetch changes up to bad83ada0ebf9e293d570e6e7ca4f1cd7877f482:
Merge branch 'horizontal-scroll' of github.com:ramcdona/gitk (2026-06-12 11:30:22 +0200)
----------------------------------------------------------------
Johannes Sixt (1):
Merge branch 'horizontal-scroll' of github.com:ramcdona/gitk
Rob McDonald (1):
gitk: add horizontal scrollbar to the commit list pane
gitk | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
^ permalink raw reply
* [GIT PULL] git-gui: repo discovery with rev-parse; pick and gui subcommands; silent make -s
From: Johannes Sixt @ 2026-06-12 9:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
The following changes since commit bb52cdac6254c006e06bf0bb820268dcf024fc22:
git-gui: grey out comment lines in commit message (2026-03-04 08:04:37 +0100)
are available in the Git repository at:
https://github.com/j6t/git-gui.git master
for you to fetch changes up to 1b2c2a2edbaa1638becef4c3755b3e0633b9c304:
Merge branch 'ml/repo-discovery' (2026-06-12 11:05:28 +0200)
----------------------------------------------------------------
Harald Nordgren (1):
git-gui: silence install recipes under "make -s"
Johannes Sixt (2):
git-gui: remove unnecessary 'cd $_gitworktree' from do_gitk
Merge branch 'ml/repo-discovery'
Mark Levedahl (11):
git-gui: use HEAD as current branch when detached
git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE
git-gui: do not change global vars in choose_repository::pick
git-gui: use --absolute-git-dir
git-gui: use rev-parse exclusively to find a repository
git-gui: use git rev-parse for worktree discovery
git-gui: simplify [is_bare] to report if a worktree is known
git-gui: try harder to find worktree from gitdir
git-gui: allow specifying path '.' to the browser
git-gui: check browser/blame arguments carefully
git-gui: add gui and pick as explicit subcommands
Makefile | 6 +-
git-gui.sh | 376 ++++++++++++++++++++++++++--------------------
lib/choose_repository.tcl | 21 +--
3 files changed, 224 insertions(+), 179 deletions(-)
^ permalink raw reply
* Re: [PATCH 9/9] refs: always use absolute paths for reference stores
From: Karthik Nayak @ 2026-06-12 9:58 UTC (permalink / raw)
To: Patrick Steinhardt, git
In-Reply-To: <20260610-b4-pks-refs-avoid-chdir-notify-reparent-v1-9-56c864b01c43@pks.im>
[-- Attachment #1: Type: text/plain, Size: 7719 bytes --]
Patrick Steinhardt <ps@pks.im> writes:
> 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()`.
>
With the previous changes added, we register via
`chdir_notify_register()`
> 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.
>
Same here, should be `chdir_notify_register()`
> 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,
>
The changes look good to me.
[snip]
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* [PATCH 0/2] branch/push: suggest intended form when remote/branch slip given
From: Harald Nordgren via GitGitGadget @ 2026-06-12 11:10 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren
When the repository or upstream argument is a slip like "origin/main" or
"origin main", suggest the intended "git push origin main" or "git branch
--set-upstream-to=origin/main" form instead of failing with an unrelated
error.
Harald Nordgren (2):
branch: suggest <remote>/<branch> on upstream slip
push: suggest <remote> <branch> for a slash slip
Documentation/config/advice.adoc | 5 +++++
advice.c | 1 +
advice.h | 1 +
builtin/branch.c | 17 ++++++++++++++
builtin/push.c | 26 +++++++++++++++++++++-
t/t3200-branch.sh | 38 ++++++++++++++++++++++++++++++++
t/t5529-push-errors.sh | 31 ++++++++++++++++++++++++++
7 files changed, 118 insertions(+), 1 deletion(-)
base-commit: 3e65291872de10c3f0bf05ea8c24187e7a71ebf0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2331%2FHaraldNordgren%2Fsuggest-remote-branch-slips-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2331/HaraldNordgren/suggest-remote-branch-slips-v1
Pull-Request: https://github.com/git/git/pull/2331
--
gitgitgadget
^ permalink raw reply
* [PATCH 1/2] branch: suggest <remote>/<branch> on upstream slip
From: Harald Nordgren via GitGitGadget @ 2026-06-12 11:10 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2331.git.git.1781262619.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
"git branch --set-upstream-to origin main" reads the trailing word as
the local branch to operate on and dies with "branch 'main' does not
exist", pointing at the wrong problem.
When that branch is missing and "<remote>/<branch>" names a real
remote-tracking ref, suggest the intended
"git branch --set-upstream-to=<remote>/<branch>" form.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
builtin/branch.c | 17 +++++++++++++++++
t/t3200-branch.sh | 38 ++++++++++++++++++++++++++++++++++++++
2 files changed, 55 insertions(+)
diff --git a/builtin/branch.c b/builtin/branch.c
index 1572a4f9ef..7ad3efb908 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -957,6 +957,23 @@ int cmd_branch(int argc,
if (!refs_ref_exists(get_main_ref_store(the_repository), branch->refname)) {
if (!argc || branch_checked_out(branch->refname))
die(_("no commit on branch '%s' yet"), branch->name);
+ if (argc == 1 && !strchr(new_upstream, '/') &&
+ remote_is_configured(remote_get(new_upstream), 0)) {
+ struct strbuf remote_ref = STRBUF_INIT;
+
+ strbuf_addf(&remote_ref, "refs/remotes/%s/%s",
+ new_upstream, argv[0]);
+ if (refs_ref_exists(get_main_ref_store(the_repository),
+ remote_ref.buf)) {
+ int code = die_message(_("--set-upstream-to takes a single <remote>/<branch> argument"));
+ advise_if_enabled(ADVICE_SET_UPSTREAM_FAILURE,
+ _("Did you mean to use: git branch --set-upstream-to=%s/%s?"),
+ new_upstream, argv[0]);
+ strbuf_release(&remote_ref);
+ exit(code);
+ }
+ strbuf_release(&remote_ref);
+ }
die(_("branch '%s' does not exist"), branch->name);
}
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index e7829c2c4b..e2682a83a0 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -1022,6 +1022,44 @@ test_expect_success '--set-upstream-to fails on a missing dst branch' '
test_cmp expect err
'
+test_expect_success '--set-upstream-to suggests <remote>/<branch> on slip' '
+ test_when_finished "git remote remove slip-remote" &&
+ git remote add slip-remote . &&
+ git update-ref refs/remotes/slip-remote/slip-feature HEAD &&
+ test_must_fail git branch --set-upstream-to slip-remote slip-feature 2>err &&
+ test_grep "takes a single <remote>/<branch> argument" err &&
+ test_grep "hint: Did you mean to use: git branch --set-upstream-to=slip-remote/slip-feature?" err &&
+ test_must_fail git -c advice.setUpstreamFailure=false \
+ branch --set-upstream-to slip-remote slip-feature 2>err &&
+ test_grep ! "Did you mean" err
+'
+
+test_expect_success '--set-upstream-to does not suggest when no matching remote ref' '
+ test_when_finished "git remote remove slip-remote" &&
+ git remote add slip-remote . &&
+ test_must_fail git branch --set-upstream-to slip-remote no-such-branch 2>err &&
+ test_grep "branch ${SQ}no-such-branch${SQ} does not exist" err &&
+ test_grep ! "Did you mean" err
+'
+
+test_expect_success '--set-upstream-to to a local branch is not mistaken for a slip' '
+ git branch slip-local-upstream &&
+ git branch slip-local-target &&
+ git branch --set-upstream-to=slip-local-upstream slip-local-target 2>err &&
+ test_grep ! "Did you mean" err &&
+ echo refs/heads/slip-local-upstream >expect &&
+ git config branch.slip-local-target.merge >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '--set-upstream-to slip suggestion keeps a slashed branch name' '
+ test_when_finished "git remote remove slip-remote" &&
+ git remote add slip-remote . &&
+ git update-ref refs/remotes/slip-remote/slip/feature HEAD &&
+ test_must_fail git branch --set-upstream-to slip-remote slip/feature 2>err &&
+ test_grep "hint: Did you mean to use: git branch --set-upstream-to=slip-remote/slip/feature?" err
+'
+
test_expect_success '--set-upstream-to fails on a missing src branch' '
test_must_fail git branch --set-upstream-to does-not-exist main 2>err &&
test_grep "the requested upstream branch '"'"'does-not-exist'"'"' does not exist" err
--
gitgitgadget
^ permalink raw reply related
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