* Re: [PATCH v2 4/5] builtin/refs: add "create" subcommand
From: Patrick Steinhardt @ 2026-06-30 10:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq5x31ukqv.fsf@gitster.g>
On Mon, Jun 29, 2026 at 01:58:32PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > + if (repo_get_oid_with_flags(repo, argv[1], &newoid, GET_OID_SKIP_AMBIGUITY_CHECK))
> > + die(_("invalid object ID: '%s'"), argv[1]);
> > + if (is_null_oid(&newoid))
> > + die(_("cannot create reference with null old object ID"));
>
> An apparent typo here, "with null old" -> "with null new object
> name".
>
> Other than that, I think this one is good.
Yes, indeed, good eyes.
Patrick
^ permalink raw reply
* git-blame vs. abbrev
From: Laszlo Ersek @ 2026-06-30 11:15 UTC (permalink / raw)
To: git
Hi,
when git-blame is passed the "-b" option ("Show blank SHA-1 for boundary
commits"), shouldn't git-blame *stop* reserving a commit hash nibble for
the caret that otherwise marks boundary commits?
More directly, I find it inconvenient that git-blame shows commit hashes
that are one nibble longer (13) than my "core.abbrev" (12) setting; that
makes cutting and pasting commit hashes from the git-blame output into a
git-rebase TODO list cumbersome. I briefly hoped that by setting
"blame.blankBoundary", I could get around that, but it doesn't seem to
work (I tried with Git 2.55). I now have an alias that passes
"--abbrev=11" explicitly, as a last resort, to git-blame. (Even a
potential "blame.abbrev" would be superior, but such a permanent setting
doesn't seem to exist.)
Thanks,
Laszlo Ersek
^ permalink raw reply
* Re: [PATCH 2/6] odb: make backend-specific fields optional
From: Patrick Steinhardt @ 2026-06-30 11:28 UTC (permalink / raw)
To: Justin Tobler; +Cc: git
In-Reply-To: <akKmwPGSAGEGKZjL@denethor>
On Mon, Jun 29, 2026 at 12:25:21PM -0500, Justin Tobler wrote:
> On 26/06/24 02:19PM, Patrick Steinhardt wrote:
> > diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> > index 8726485f1f..adc626ce30 100644
> > --- a/builtin/cat-file.c
> > +++ b/builtin/cat-file.c
> > @@ -269,32 +301,20 @@ struct object_info {
> > */
> > time_t *mtimep;
> >
> > + /*
> > + * Backend-specific information that tells the caller where exactly an
> > + * object was looked up from. This information should help disambiguate
> > + * object lookups in case the same object exists in multiple sources,
> > + * or multiple times in the same source.
> > + */
> > + struct object_info_source *sourcep;
>
> To me, the name `sourcep` makes me think a pointer to `struct
> odb_source`. This did confuse me slightly when initially reading, but
> I'm not sure it's worth it to be overly verbose here.
Yeah, good point. But as you say, I haven't been able to really come up
with a name that is not overly verbose. We could potentially rename the
structure itself to `odb_source_info` and then call the field itself
`source_infop`. Would that help?
Patrick
^ permalink raw reply
* Re: [PATCH 1/6] packfile: thread odb_source_packed through packed_object_info()
From: Patrick Steinhardt @ 2026-06-30 11:28 UTC (permalink / raw)
To: Justin Tobler; +Cc: git
In-Reply-To: <akKge0zmT3WSfdyz@denethor>
On Mon, Jun 29, 2026 at 12:01:47PM -0500, Justin Tobler wrote:
> On 26/06/24 02:19PM, Patrick Steinhardt wrote:
> > Add an optional `struct odb_source_packed *source` parameter to
> > `packed_object_info()` and `packed_object_info_with_index_pos()`. This
> > parameter is unused at this point in time, but it will be used in a
> > follow-up commit so that we can record the source of a specific object.
>
> Ok so `packed_object_info()` is responsible for populating `struct
> object_info` from the provided packfile and object offset. By
> additionally providing the object source, the ultimate goal is to store
> the this information in `struct object_info` or some equivalent
> structure.
>
> At first, I wondered if it would make more sense for `struct packed_git`
> to record the `struct odb_source_packed` it comes from, but maybe that
> wouldn't be the best layer to handle this bookkeeping?
Yeah, I was thinking about that, too. But I feel like that would be a
layering violation: a packfile can in theory live standalone without a
source. So tracking that information as part of the packfile itself just
feels wrong to me.
We could in theory adapt all callers of `packed_object_info()` to track
the origin of the packfiles. I _think_ that should be feasible at almost
all sites. But I'm just not sure myself whether that really buys us much
in the first place, because...
> > Note that callers in "odb/source-packed.c" pass the already-available
> > source, but all other callers pass `NULL` instead. This is fine though,
> > as we only care about populating this info when called via the packed
> > store.
>
> Hmmm, is this because knowing the ODB source the object comes from is
> only useful for callers from in "odb/source-packed.c"? Maybe this will
> become a bit more clear to me in subsequent patches.
... right now none none of the callers that call `packed_object_info()`
directly care about the source information at all. It's really only
callers of `odb_read_object_info()` that do.
So I understand that this feels a bit iffy. But arguably, the right way
to fix this is to stop using `packed_object_info()` altogether. It is an
internal implementation detail of the object source backend, and ideally
we shouldn't need to care about it.
I already have a patch series that fixes git-cat-file(1). The
commit-graph is a bigger building site, as I'm still not a 100% decided
on how to represent such auxiliary data structures with pluggable object
backends. And for the other commands I don't yet have a good answer.
Patrick
^ permalink raw reply
* Re: [PATCH 3/6] odb: add `source` field to struct object_info_source
From: Patrick Steinhardt @ 2026-06-30 11:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Justin Tobler, git
In-Reply-To: <xmqqmrwdul8y.fsf@gitster.g>
On Mon, Jun 29, 2026 at 01:47:41PM -0700, Junio C Hamano wrote:
> Justin Tobler <jltobler@gmail.com> writes:
>
> >> @@ -1424,6 +1424,10 @@ int packed_object_info_with_index_pos(struct odb_source_packed *source UNUSED,
> >> oi->whence = OI_PACKED;
> >>
> >> if (oi->sourcep) {
> >> + if (!source)
> >> + BUG("cannot request source without an owning source");
> >> + oi->sourcep->source = &source->base;
> >
> > And here it is set for the packed backend. Looks good.
> >
> > Naive question: I understand that some `packed_info_object()` callers
> > may not have the `struct odb_source` on hand, but when the `struct
> > packed_git` is intially setup, is it not always known the ODB source it
> > comes from? It makes me wonder if the ODB source should also be recorded
> > when `struct packed_git` is initialized.
I've addressed this comment on patch 1.
> As with your reaction to [PATCH 1/6], I do share this puzzlement: if
> the source can almost always be NULL, what is it good for and isn't
> it something that can be computed from the available information?
It's not almost always NULL, even though it looks like this because we
ended up adapting more callers to pass `NULL` than we adapted callers to
pass an actual source. But in the end it's rather the opposite: there
are very few low-level callers that don't have the source info
available, and everyone else instead uses `odb_read_object_info()`,
where we do have it available. But those callers don't need to be
adjusted, so they weren't visible in the diff.
> Perhaps it is the naming?
Yeah, as Justin pointed out, calling this `sourcep` is confusing.
> I am confused what the above quoted code actually is doing ("if you
> have a source, then grab its base and set it to .source member of
> the struct the out parameter points at", makes it sound like the out
> parameter sourcep should be pointing at a structure with .base
> member, not .source member, or perhaps the caller should be passing
> &oi->sourcep->source as *base to be assigned to, or something).
We have to return the generic source here, not the specialized source,
so that this interface can be used by every implementation. Other sites
would end up storing their own source, which of course would have a
different specialized backend.
So an alternative to write this would have been:
oi->sourcep->source = (struct odb_source *) source;
But by assigning the base we avoid having to cast.
Patrick
^ permalink raw reply
* Re: [PATCH v4 3/3] replay: offer an option to linearize the commit topology
From: Patrick Steinhardt @ 2026-06-30 11:32 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Toon Claes, git, Elijah Newren
In-Reply-To: <9e7d14c4-82f0-2b89-b07b-f219119a199b@gmx.de>
On Tue, Jun 30, 2026 at 11:44:47AM +0200, Johannes Schindelin wrote:
> On Tue, 30 Jun 2026, Patrick Steinhardt wrote:
> > On Fri, Jun 26, 2026 at 07:36:31AM +0200, Toon Claes wrote:
> > > Then there's the option of rebasing cousins left. That's something that
> > > isn't covered by Dscho's series yet. Maybe --replay-cousins?
> > >
> > > To reiterate what the final design could look like:
> > >
> > > * <nothing>: replay merges preserving topology.
> > > * "--linearize": flattens merges (only git-replay(1)).
> > > * "--no-merges": dies when the process tries to replay a merge.
> > > * "--replay-cousins": does what --rebase-merges=rebase-cousins does.
> >
> > Right. And if we tried to be consistent with git-rebase(1), then this
> > could be done as:
> >
> > - "--rebase-merges" to replay merges preserving topology, which is the
> > default once we support replaying them.
> >
> > - "--no-rebase-merges" to flatten commits.
> >
> > - "--rebase-merges=abort" to explicitly die when seeing merges.
> >
> > - "--rebase-merges=rebase-cousins"
>
> The `git rebase` options are unlikely to be a good precedent to follow.
> Their history is full of usability warts, and in hindsight, I would really
> have loved a more steady hand in developing and maintaining a good UX. The
> fact alone that this is called `rebase` speaks volumes about how hostile
> of a user experience this command surfaces.
>
> In any case, these options should use the much more natural term "replay"
> instead of "rebase".
>
> But then: you said that `--no-rebase-merges` should flatten the commits?
> That's not what this option name conveys to me; It would convey to me that
> the operation would _abort_ on encountering merge commits.
>
> In other words, I do think that the --linearize option is conceptually
> quite distinct from the different modes in which merge commits could be
> handled. As such, this option should probably not be conflated with
> the various `--replay-merges=<mode>` modes.
Fair enough. Arguments like this are basically what I want to read in
the commit message. As said in the below snippet: I'm not against
diverging from the git-rebase(1) interface, but if we do that we should
document why we think that the current interface is bad.
[snip]
> > Note that I'm not arguing that we should support all of these options
> > now. I'm merely arguing that we should try to be consistent, unless
> > there is a good argument not to do that. I'm fine with the interface if
> > there indeed is a good argument, but if so we should document why we
> > think that the current interface in git-rebase(1) is not a good fit for
> > this command.
Thanks!
Patrick
^ permalink raw reply
* [PATCH 00/13] setup: split up repository discovery and setup
From: Patrick Steinhardt @ 2026-06-30 11:47 UTC (permalink / raw)
To: git
Hi,
this patch series is the next set of refactorings to simplify how we
configure repositories in "setup.c".
The setup of the repository is essentially happening in two phases:
1. We discover the location of the repository as well as its format.
2. We then use this information to configure the repository.
So far so sensible. In our code base though these two phases are quite
intertwined with one another, as we continue to repeatedly call
`set_git_dir()` and `set_work_tree()` on the repository as we discover
its locations. This makes it hard to follow the logic, and it basically
leaves us with a partially-configured repository.
This patch series splits this up into two proper phases that are
completely separate from one another. The first phase now populates a
`struct repo_discovery` structure, without even having access to any
repository. The second phase then takes that structure and configures
the repository accordingly.
Ultimately, the motivation of this whole exercise is that eventually we
can unify configuration of the repository into `repo_init()` instead of
having bits and pieces thereof distributed across "repository.c" and
"setup.c".
This series is built on top of v2.55.0 with the following three branches
merged into it:
- ps/refs-onbranch-fixes at d6522d01df (refs: protect against
chicken-and-egg recursion, 2026-06-25).
- ps/setup-drop-global-state at 1ceee7431b (treewide: drop
USE_THE_REPOSITORY_VARIABLE, 2026-06-11).
- jk/repo-info-path-keys at 3ac28d832a (repo: add path.gitdir with
absolute and relative suffix formatting, 2026-06-24).
Thanks!
Patrick
---
Patrick Steinhardt (13):
setup: rename `check_repository_format_gently()`
setup: mark bogus worktree in `apply_repository_format()`
setup: unify setup of shallow file
setup: split up concerns of `setup_git_env_internal()`
setup: introduce explicit repository discovery
setup: embed repository format in discovery
setup: move prefix into repository
setup: drop static `cwd` variable
setup: propagate prefix via repository discovery
setup: make repository discovery self-contained
setup: drop redundant configuration of `startup_info->have_repository`
setup: pass worktree to `init_db()`
setup: mark `set_git_work_tree()` as file-local
builtin/clone.c | 8 +-
builtin/init-db.c | 34 ++--
builtin/repo.c | 8 +-
builtin/rev-parse.c | 5 +-
builtin/update-index.c | 4 +-
common-init.c | 20 +++
git.c | 2 +-
object-name.c | 4 +-
repository.c | 1 +
repository.h | 8 +
setup.c | 419 ++++++++++++++++++++++++++-----------------------
setup.h | 7 +-
trace.c | 4 +-
13 files changed, 283 insertions(+), 241 deletions(-)
---
base-commit: b340fc4c4f3850656b726ff757b42d2020215378
change-id: 20260618-pks-setup-split-discovery-and-setup-d7f23831803c
^ permalink raw reply
* [PATCH 01/13] setup: rename `check_repository_format_gently()`
From: Patrick Steinhardt @ 2026-06-30 11:47 UTC (permalink / raw)
To: git
In-Reply-To: <20260630-pks-setup-split-discovery-and-setup-v1-0-13864eb5a032@pks.im>
The function `check_repository_format_gently()` receives a format as
input. An unknowing reader may thus suspect that this function actually
checks the passed-in format for consistency. While the function indeed
checks the repository format, it actually serves two purposes:
- It reads the repository's format and populates the passed-in format
with that information.
- It then indeed checks whether the format is consistent.
Rename the function to `read_and_verify_repository_format()` to clarify
its functionality. While at it, reorder the parameters so that the
format comes first to better match other functions that pass around the
format.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
setup.c | 38 +++++++++++++++++++-------------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/setup.c b/setup.c
index 951ab9eedb..118416e350 100644
--- a/setup.c
+++ b/setup.c
@@ -749,9 +749,9 @@ static int check_repo_format(const char *var, const char *value,
return read_worktree_config(var, value, ctx, vdata);
}
-static int check_repository_format_gently(const char *gitdir,
- struct repository_format *candidate,
- int *nongit_ok)
+static int read_and_verify_repository_format(struct repository_format *format,
+ const char *gitdir,
+ int *nongit_ok)
{
struct strbuf sb = STRBUF_INIT;
struct strbuf err = STRBUF_INIT;
@@ -759,7 +759,7 @@ static int check_repository_format_gently(const char *gitdir,
has_common = get_common_dir(&sb, gitdir);
strbuf_addstr(&sb, "/config");
- read_repository_format(candidate, sb.buf);
+ read_repository_format(format, sb.buf);
strbuf_release(&sb);
/*
@@ -767,10 +767,10 @@ static int check_repository_format_gently(const char *gitdir,
* we treat a missing config as a silent "ok", even when nongit_ok
* is unset.
*/
- if (candidate->version < 0)
+ if (format->version < 0)
return 0;
- if (verify_repository_format(candidate, &err) < 0) {
+ if (verify_repository_format(format, &err) < 0) {
if (nongit_ok) {
warning("%s", err.buf);
strbuf_release(&err);
@@ -780,37 +780,37 @@ static int check_repository_format_gently(const char *gitdir,
die("%s", err.buf);
}
- string_list_clear(&candidate->unknown_extensions, 0);
- string_list_clear(&candidate->v1_only_extensions, 0);
+ string_list_clear(&format->unknown_extensions, 0);
+ string_list_clear(&format->v1_only_extensions, 0);
- if (candidate->worktree_config) {
+ if (format->worktree_config) {
/*
* pick up core.bare and core.worktree from per-worktree
* config if present
*/
strbuf_addf(&sb, "%s/config.worktree", gitdir);
- git_config_from_file(read_worktree_config, sb.buf, candidate);
+ git_config_from_file(read_worktree_config, sb.buf, format);
strbuf_release(&sb);
has_common = 0;
}
if (startup_info->force_bare_repository) {
- candidate->is_bare = 1;
- FREE_AND_NULL(candidate->work_tree);
+ format->is_bare = 1;
+ FREE_AND_NULL(format->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;
+ format->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);
+ FREE_AND_NULL(format->work_tree);
}
return 0;
@@ -1141,7 +1141,7 @@ static const char *setup_explicit_git_dir(struct repository *repo,
die(_("not a git repository: '%s'"), gitdirenv);
}
- if (check_repository_format_gently(gitdirenv, repo_fmt, nongit_ok)) {
+ if (read_and_verify_repository_format(repo_fmt, gitdirenv, nongit_ok)) {
free(gitfile);
return NULL;
}
@@ -1218,7 +1218,7 @@ static const char *setup_discovered_git_dir(struct repository *repo,
struct repository_format *repo_fmt,
int *nongit_ok)
{
- if (check_repository_format_gently(gitdir, repo_fmt, nongit_ok))
+ if (read_and_verify_repository_format(repo_fmt, gitdir, nongit_ok))
return NULL;
/* --work-tree is set without --git-dir; use discovered one */
@@ -1266,7 +1266,7 @@ static const char *setup_bare_git_dir(struct repository *repo,
{
int root_len;
- if (check_repository_format_gently(".", repo_fmt, nongit_ok))
+ if (read_and_verify_repository_format(repo_fmt, ".", nongit_ok))
return NULL;
setenv(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, "0", 1);
@@ -1874,7 +1874,7 @@ const char *enter_repo(struct repository *repo, const char *path, unsigned flags
struct strbuf err = STRBUF_INIT;
set_git_dir(repo, ".", 0);
- check_repository_format_gently(".", &fmt, NULL);
+ read_and_verify_repository_format(&fmt, ".", NULL);
if (apply_repository_format(repo, &fmt, APPLY_REPOSITORY_FORMAT_HONOR_ENV, &err) < 0)
die("%s", err.buf);
startup_info->have_repository = 1;
@@ -2836,7 +2836,7 @@ 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_repository_format_gently(repo_get_git_dir(repo), &repo_fmt, NULL);
+ read_and_verify_repository_format(&repo_fmt, repo_get_git_dir(repo), 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);
--
2.55.0.795.g602f6c329a.dirty
^ permalink raw reply related
* [PATCH 02/13] setup: mark bogus worktree in `apply_repository_format()`
From: Patrick Steinhardt @ 2026-06-30 11:47 UTC (permalink / raw)
To: git
In-Reply-To: <20260630-pks-setup-split-discovery-and-setup-v1-0-13864eb5a032@pks.im>
When a repository is configured to have both "core.worktree" and
"core.bare" we emit a warning and mark the worktree configuration as
bogus so that the next call to `setup_work_tree()` will cause us to die.
This allows us to still use the misconfigured repository, at least as
long as we don't try to use its worktree.
This condition is handled in `setup_explicit_git_dir()`. In a subsequent
commit we'll refactor this function so that it doesn't receive a repo as
input anymore though, and consequently we cannot set the "bogus" bit
anymore.
Move the logic into `apply_repository_format()` instead to prepare for
this. While at it, fix up formatting a bit.
Note that this change requires us to also explicitly unset the value of
"core.worktree" in case we have the "GIT_WORK_TREE" environment variable
set. This is because the environment variable overrides the repository's
configuration, and we don't want to warn or die in case the work tree
has been configured explicitly regardless of whether or not "core.bare"
is set.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
setup.c | 37 +++++++++++++++++++++----------------
1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/setup.c b/setup.c
index 118416e350..f54eac5e5a 100644
--- a/setup.c
+++ b/setup.c
@@ -1147,24 +1147,24 @@ static const char *setup_explicit_git_dir(struct repository *repo,
}
/* #3, #7, #11, #15, #19, #23, #27, #31 (see t1510) */
- if (work_tree_env)
+ if (work_tree_env) {
+ /*
+ * The environment variable overrides "core.worktree". This
+ * also has the consequence that we don't want to flag cases as
+ * bogus where we have both "core.worktree" and "core.bare", so
+ * we have to exlicitly unset the configuration.
+ */
+ FREE_AND_NULL(repo_fmt->work_tree);
set_git_work_tree(repo, work_tree_env);
- 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");
- repo->worktree_config_is_bogus = true;
- }
-
+ } else if (repo_fmt->is_bare > 0) {
/* #18, #26 */
set_git_dir(repo, gitdirenv, 0);
free(gitfile);
return NULL;
- }
- else if (repo_fmt->work_tree) { /* #6, #14 */
- if (is_absolute_path(repo_fmt->work_tree))
+ } else if (repo_fmt->work_tree) { /* #6, #14 */
+ if (is_absolute_path(repo_fmt->work_tree)) {
set_git_work_tree(repo, repo_fmt->work_tree);
- else {
+ } else {
char *core_worktree;
if (chdir(gitdirenv))
die_errno(_("cannot chdir to '%s'"), gitdirenv);
@@ -1176,15 +1176,14 @@ static const char *setup_explicit_git_dir(struct repository *repo,
set_git_work_tree(repo, core_worktree);
free(core_worktree);
}
- }
- else if (!git_env_bool(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, 1)) {
+ } else if (!git_env_bool(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, 1)) {
/* #16d */
set_git_dir(repo, gitdirenv, 0);
free(gitfile);
return NULL;
- }
- else /* #2, #10 */
+ } else { /* #2, #10 */
set_git_work_tree(repo, ".");
+ }
/* set_git_work_tree() must have been called by now */
worktree = repo_get_work_tree(repo);
@@ -1768,6 +1767,12 @@ int apply_repository_format(struct repository *repo,
if (verify_repository_format(format, err) < 0)
return -1;
+ if (format->is_bare > 0 && format->work_tree) {
+ /* #22.2, #30 */
+ warning("core.bare and core.worktree do not make sense");
+ repo->worktree_config_is_bogus = true;
+ }
+
if (flags & APPLY_REPOSITORY_FORMAT_HONOR_ENV) {
object_directory = xstrdup_or_null(getenv(DB_ENVIRONMENT));
alternate_object_directories = xstrdup_or_null(getenv(ALTERNATE_DB_ENVIRONMENT));
--
2.55.0.795.g602f6c329a.dirty
^ permalink raw reply related
* [PATCH 03/13] setup: unify setup of shallow file
From: Patrick Steinhardt @ 2026-06-30 11:47 UTC (permalink / raw)
To: git
In-Reply-To: <20260630-pks-setup-split-discovery-and-setup-v1-0-13864eb5a032@pks.im>
It is possible to configure an arbitrary "shallow" file via two
mechanisms, and the respective logic to handle these is split across two
locations:
- Via the "GIT_SHALLOW_FILE" environment variable, which is handled in
`setup_git_env_internal()`.
- Via the global "--shallow-file=" command line option, which is
handled in `handle_options()`.
We can rather easily unify this logic by not configuring the shallow
file in `handle_options()`, but instead overwriting the environment
variable. The environment variable itself is then handled inside of
`apply_repository_format()`, which is responsible for configuring a
discovered Git directory.
This new logic is similar in nature to how we handle the other global
options already, all of which end up setting an environment variable.
So for one this gives us more consistency. But more importantly, this
change means that `the_repository` will not contain any relevant state
anymore before we hit `apply_repository_format()` once we're at the end
of this patch series. Consequently, it will become possible for us to
completely discard `the_repository` and populate it anew.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
git.c | 2 +-
setup.c | 10 +++++-----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/git.c b/git.c
index 387eabe38c..e5f1811b6b 100644
--- a/git.c
+++ b/git.c
@@ -306,7 +306,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
} else if (!strcmp(cmd, "--shallow-file")) {
(*argv)++;
(*argc)--;
- set_alternate_shallow_file(the_repository, (*argv)[0], 1);
+ setenv(GIT_SHALLOW_FILE_ENVIRONMENT, (*argv)[0], 1);
if (envchanged)
*envchanged = 1;
} else if (!strcmp(cmd, "-C")) {
diff --git a/setup.c b/setup.c
index f54eac5e5a..5e6b959f68 100644
--- a/setup.c
+++ b/setup.c
@@ -1046,7 +1046,6 @@ static void setup_git_env_internal(struct repository *repo,
const char *git_dir)
{
char *git_replace_ref_base;
- const char *shallow_file;
const char *replace_ref_base;
struct set_gitdir_args args = { NULL };
struct strvec to_free = STRVEC_INIT;
@@ -1067,10 +1066,6 @@ static void setup_git_env_internal(struct repository *repo,
: "refs/replace/");
update_ref_namespace(NAMESPACE_REPLACE, git_replace_ref_base);
- shallow_file = getenv(GIT_SHALLOW_FILE_ENVIRONMENT);
- if (shallow_file)
- set_alternate_shallow_file(repo, shallow_file, 0);
-
if (git_env_bool(NO_LAZY_FETCH_ENVIRONMENT, 0))
fetch_if_missing = 0;
}
@@ -1774,8 +1769,13 @@ int apply_repository_format(struct repository *repo,
}
if (flags & APPLY_REPOSITORY_FORMAT_HONOR_ENV) {
+ const char *shallow_file;
+
object_directory = xstrdup_or_null(getenv(DB_ENVIRONMENT));
alternate_object_directories = xstrdup_or_null(getenv(ALTERNATE_DB_ENVIRONMENT));
+ shallow_file = getenv(GIT_SHALLOW_FILE_ENVIRONMENT);
+ if (shallow_file)
+ set_alternate_shallow_file(repo, shallow_file, 0);
}
repo->bare_cfg = format->is_bare;
--
2.55.0.795.g602f6c329a.dirty
^ permalink raw reply related
* [PATCH 04/13] setup: split up concerns of `setup_git_env_internal()`
From: Patrick Steinhardt @ 2026-06-30 11:47 UTC (permalink / raw)
To: git
In-Reply-To: <20260630-pks-setup-split-discovery-and-setup-v1-0-13864eb5a032@pks.im>
The function `setup_git_env_internal()` does two completely unrelated
things:
- It configures the repository's gitdir and propagates environment
variables into it.
- It configures a couple of global parameters via environment
variables.
The function is called when we initialize the repository's path, but
it's also called via `chdir_notify_register()` whenever we change the
current working directory. While we indeed have to reconfigure the
gitdir in case it's a relative path, it doesn't make sense to reapply
the global environment variables.
Split up concerns of this function along the above delineation. Handling
of the global environment variables is moved into `init_git()`, as they
can be considered part of our setup procedure.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
common-init.c | 20 ++++++++++++++++
setup.c | 73 +++++++++++++++++++++++------------------------------------
2 files changed, 48 insertions(+), 45 deletions(-)
diff --git a/common-init.c b/common-init.c
index 5cc73f058c..d26c9c1f20 100644
--- a/common-init.c
+++ b/common-init.c
@@ -5,7 +5,10 @@
#include "exec-cmd.h"
#include "gettext.h"
#include "attr.h"
+#include "odb.h"
+#include "parse.h"
#include "repository.h"
+#include "replace-object.h"
#include "setup.h"
#include "strbuf.h"
#include "trace2.h"
@@ -31,6 +34,22 @@ static void restore_sigpipe_to_default(void)
signal(SIGPIPE, SIG_DFL);
}
+static void setup_environment(void)
+{
+ char *git_replace_ref_base;
+ const char *replace_ref_base;
+
+ if (getenv(NO_REPLACE_OBJECTS_ENVIRONMENT))
+ disable_replace_refs();
+ replace_ref_base = getenv(GIT_REPLACE_REF_BASE_ENVIRONMENT);
+ git_replace_ref_base = xstrdup(replace_ref_base ? replace_ref_base
+ : "refs/replace/");
+ update_ref_namespace(NAMESPACE_REPLACE, git_replace_ref_base);
+
+ if (git_env_bool(NO_LAZY_FETCH_ENVIRONMENT, 0))
+ fetch_if_missing = 0;
+}
+
void init_git(const char **argv)
{
struct strbuf tmp = STRBUF_INIT;
@@ -51,6 +70,7 @@ void init_git(const char **argv)
git_setup_gettext();
initialize_repository(the_repository);
+ setup_environment();
attr_start();
diff --git a/setup.c b/setup.c
index 5e6b959f68..dd8514b822 100644
--- a/setup.c
+++ b/setup.c
@@ -10,7 +10,6 @@
#include "object-file.h"
#include "object-name.h"
#include "refs.h"
-#include "replace-object.h"
#include "repository.h"
#include "config.h"
#include "dir.h"
@@ -1042,38 +1041,19 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)
return error_code ? NULL : path;
}
-static void setup_git_env_internal(struct repository *repo,
- const char *git_dir)
+static void apply_gitdir_and_environment(struct repository *repo, const char *path)
{
- char *git_replace_ref_base;
- const char *replace_ref_base;
- struct set_gitdir_args args = { NULL };
struct strvec to_free = STRVEC_INIT;
+ struct set_gitdir_args args = {
+ .commondir = getenv_safe(&to_free, GIT_COMMON_DIR_ENVIRONMENT),
+ .graft_file = getenv_safe(&to_free, GRAFT_ENVIRONMENT),
+ .index_file = getenv_safe(&to_free, INDEX_ENVIRONMENT),
+ .disable_ref_updates = !!getenv(GIT_QUARANTINE_ENVIRONMENT),
+ };
- args.commondir = getenv_safe(&to_free, GIT_COMMON_DIR_ENVIRONMENT);
- args.graft_file = getenv_safe(&to_free, GRAFT_ENVIRONMENT);
- args.index_file = getenv_safe(&to_free, INDEX_ENVIRONMENT);
- if (getenv(GIT_QUARANTINE_ENVIRONMENT))
- args.disable_ref_updates = true;
+ repo_set_gitdir(repo, path, &args);
- repo_set_gitdir(repo, git_dir, &args);
strvec_clear(&to_free);
-
- if (getenv(NO_REPLACE_OBJECTS_ENVIRONMENT))
- disable_replace_refs();
- replace_ref_base = getenv(GIT_REPLACE_REF_BASE_ENVIRONMENT);
- git_replace_ref_base = xstrdup(replace_ref_base ? replace_ref_base
- : "refs/replace/");
- update_ref_namespace(NAMESPACE_REPLACE, git_replace_ref_base);
-
- if (git_env_bool(NO_LAZY_FETCH_ENVIRONMENT, 0))
- fetch_if_missing = 0;
-}
-
-static void set_git_dir_1(struct repository *repo, const char *path)
-{
- xsetenv(GIT_DIR_ENVIRONMENT, path, 1);
- setup_git_env_internal(repo, path);
}
static void update_relative_gitdir(const char *name UNUSED,
@@ -1087,11 +1067,12 @@ static void update_relative_gitdir(const char *name UNUSED,
trace_printf_key(&trace_setup_key,
"setup: move $GIT_DIR to '%s'",
path);
- set_git_dir_1(repo, path);
+ apply_gitdir_and_environment(repo, path);
+ xsetenv(GIT_DIR_ENVIRONMENT, path, 1);
free(path);
}
-static void set_git_dir(struct repository *repo, const char *path, int make_realpath)
+static void apply_and_export_relative_gitdir(struct repository *repo, const char *path, int make_realpath)
{
struct strbuf realpath = STRBUF_INIT;
@@ -1100,7 +1081,9 @@ static void set_git_dir(struct repository *repo, const char *path, int make_real
path = realpath.buf;
}
- set_git_dir_1(repo, path);
+ apply_gitdir_and_environment(repo, path);
+ xsetenv(GIT_DIR_ENVIRONMENT, path, 1);
+
if (!is_absolute_path(path))
chdir_notify_register(NULL, update_relative_gitdir, repo);
@@ -1153,7 +1136,7 @@ static const char *setup_explicit_git_dir(struct repository *repo,
set_git_work_tree(repo, work_tree_env);
} else if (repo_fmt->is_bare > 0) {
/* #18, #26 */
- set_git_dir(repo, gitdirenv, 0);
+ apply_and_export_relative_gitdir(repo, gitdirenv, 0);
free(gitfile);
return NULL;
} else if (repo_fmt->work_tree) { /* #6, #14 */
@@ -1173,7 +1156,7 @@ static const char *setup_explicit_git_dir(struct repository *repo,
}
} else if (!git_env_bool(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, 1)) {
/* #16d */
- set_git_dir(repo, gitdirenv, 0);
+ apply_and_export_relative_gitdir(repo, gitdirenv, 0);
free(gitfile);
return NULL;
} else { /* #2, #10 */
@@ -1185,14 +1168,14 @@ static const char *setup_explicit_git_dir(struct repository *repo,
/* both repo_get_work_tree() and cwd are already normalized */
if (!strcmp(cwd->buf, worktree)) { /* cwd == worktree */
- set_git_dir(repo, gitdirenv, 0);
+ apply_and_export_relative_gitdir(repo, gitdirenv, 0);
free(gitfile);
return NULL;
}
offset = dir_inside_of(cwd->buf, worktree);
if (offset >= 0) { /* cwd inside worktree? */
- set_git_dir(repo, gitdirenv, 1);
+ apply_and_export_relative_gitdir(repo, gitdirenv, 1);
if (chdir(worktree))
die_errno(_("cannot chdir to '%s'"), worktree);
strbuf_addch(cwd, '/');
@@ -1201,7 +1184,7 @@ static const char *setup_explicit_git_dir(struct repository *repo,
}
/* cwd outside worktree */
- set_git_dir(repo, gitdirenv, 0);
+ apply_and_export_relative_gitdir(repo, gitdirenv, 0);
free(gitfile);
return NULL;
}
@@ -1231,7 +1214,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 (repo_fmt->is_bare > 0) {
- set_git_dir(repo, gitdir, (offset != cwd->len));
+ apply_and_export_relative_gitdir(repo, gitdir, (offset != cwd->len));
if (chdir(cwd->buf))
die_errno(_("cannot come back to cwd"));
return NULL;
@@ -1240,7 +1223,7 @@ static const char *setup_discovered_git_dir(struct repository *repo,
/* #0, #1, #5, #8, #9, #12, #13 */
set_git_work_tree(repo, ".");
if (strcmp(gitdir, DEFAULT_GIT_DIR_ENVIRONMENT))
- set_git_dir(repo, gitdir, 0);
+ apply_and_export_relative_gitdir(repo, gitdir, 0);
if (offset >= cwd->len)
return NULL;
@@ -1280,10 +1263,10 @@ static const char *setup_bare_git_dir(struct repository *repo,
die_errno(_("cannot come back to cwd"));
root_len = offset_1st_component(cwd->buf);
strbuf_setlen(cwd, offset > root_len ? offset : root_len);
- set_git_dir(repo, cwd->buf, 0);
+ apply_and_export_relative_gitdir(repo, cwd->buf, 0);
}
else
- set_git_dir(repo, ".", 0);
+ apply_and_export_relative_gitdir(repo, ".", 0);
return NULL;
}
@@ -1878,7 +1861,7 @@ const char *enter_repo(struct repository *repo, const char *path, unsigned flags
struct repository_format fmt = REPOSITORY_FORMAT_INIT;
struct strbuf err = STRBUF_INIT;
- set_git_dir(repo, ".", 0);
+ apply_and_export_relative_gitdir(repo, ".", 0);
read_and_verify_repository_format(&fmt, ".", NULL);
if (apply_repository_format(repo, &fmt, APPLY_REPOSITORY_FORMAT_HONOR_ENV, &err) < 0)
die("%s", err.buf);
@@ -2022,7 +2005,7 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
startup_info->have_repository = 1;
/*
- * Not all paths through the setup code will call 'set_git_dir()' (which
+ * Not all paths through the setup code will call 'apply_and_export_relative_gitdir()' (which
* directly sets up the environment) so in order to guarantee that the
* environment is in a consistent state after setup, explicitly setup
* the environment if we have a repository.
@@ -2040,7 +2023,7 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
const char *gitdir = getenv(GIT_DIR_ENVIRONMENT);
if (!gitdir)
gitdir = DEFAULT_GIT_DIR_ENVIRONMENT;
- setup_git_env_internal(repo, gitdir);
+ apply_gitdir_and_environment(repo, gitdir);
}
if (startup_info->have_repository) {
@@ -2825,12 +2808,12 @@ int init_db(struct repository *repo,
if (!exist_ok && !stat(real_git_dir, &st))
die(_("%s already exists"), real_git_dir);
- set_git_dir(repo, real_git_dir, 1);
+ apply_and_export_relative_gitdir(repo, real_git_dir, 1);
git_dir = repo_get_git_dir(repo);
separate_git_dir(git_dir, original_git_dir);
}
else {
- set_git_dir(repo, git_dir, 1);
+ apply_and_export_relative_gitdir(repo, git_dir, 1);
git_dir = repo_get_git_dir(repo);
}
startup_info->have_repository = 1;
--
2.55.0.795.g602f6c329a.dirty
^ permalink raw reply related
* [PATCH 05/13] setup: introduce explicit repository discovery
From: Patrick Steinhardt @ 2026-06-30 11:47 UTC (permalink / raw)
To: git
In-Reply-To: <20260630-pks-setup-split-discovery-and-setup-v1-0-13864eb5a032@pks.im>
When setting up the global repository we intermix repository discovery
and repository configuration: we repeatedly call `set_git_work_tree()`
and `apply_and_export_relative_gitdir()` until we're happy with the
result. The result of this is then a partially-configured repository
that we use for further setup.
This process is quite hard to follow, as it's never quite clear which
parts of the repository have been configured already and which haven't.
Furthermore, it means that the repository configuration is distributed
across many different places instead of having it neatly contained in a
single location. Ultimately, this is the reason that we cannot use a
central function like `repo_init()`.
Refactor the logic so that we stop partially-configuring a repository
and instead populate a new `struct repo_discovery`. This allow us to
essentially split repository setup into two phases:
- The first phase only figures out parameters required to configure
the repository.
- The second phase then takes these parameters and applies them to the
repository.
Like this, we'll never end up with a partially-configured repository and
can eventually extend `repo_init()` to handle the full initialization
for us.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
setup.c | 155 ++++++++++++++++++++++++++++++++++++++++------------------------
1 file changed, 98 insertions(+), 57 deletions(-)
diff --git a/setup.c b/setup.c
index dd8514b822..06768de23f 100644
--- a/setup.c
+++ b/setup.c
@@ -1090,14 +1090,47 @@ static void apply_and_export_relative_gitdir(struct repository *repo, const char
strbuf_release(&realpath);
}
-static const char *setup_explicit_git_dir(struct repository *repo,
- const char *gitdirenv,
- struct strbuf *cwd,
- struct repository_format *repo_fmt,
- int *nongit_ok)
+struct repo_discovery {
+ char *gitdir;
+ char *worktree;
+};
+
+#define REPO_DISCOVERY_INIT { 0 }
+
+static void repo_discovery_release(struct repo_discovery *r)
+{
+ free(r->gitdir);
+ free(r->worktree);
+}
+
+static void repo_discovery_set_gitdir(struct repo_discovery *r,
+ const char *gitdir,
+ int make_realpath)
+{
+ free(r->gitdir);
+ if (make_realpath) {
+ struct strbuf realpath = STRBUF_INIT;
+ strbuf_realpath(&realpath, gitdir, 1);
+ r->gitdir = strbuf_detach(&realpath, NULL);
+ } else {
+ r->gitdir = xstrdup(gitdir);
+ }
+}
+
+static void repo_discovery_set_worktree(struct repo_discovery *r,
+ const char *worktree)
+{
+ free(r->worktree);
+ r->worktree = real_pathdup(worktree, 1);
+}
+
+static const char *repo_discover_explicit_gitdir(struct repo_discovery *discovery,
+ const char *gitdirenv,
+ struct strbuf *cwd,
+ struct repository_format *repo_fmt,
+ int *nongit_ok)
{
const char *work_tree_env = getenv(GIT_WORK_TREE_ENVIRONMENT);
- const char *worktree;
char *gitfile;
int offset;
@@ -1133,15 +1166,15 @@ static const char *setup_explicit_git_dir(struct repository *repo,
* we have to exlicitly unset the configuration.
*/
FREE_AND_NULL(repo_fmt->work_tree);
- set_git_work_tree(repo, work_tree_env);
+ repo_discovery_set_worktree(discovery, work_tree_env);
} else if (repo_fmt->is_bare > 0) {
/* #18, #26 */
- apply_and_export_relative_gitdir(repo, gitdirenv, 0);
+ repo_discovery_set_gitdir(discovery, gitdirenv, 0);
free(gitfile);
return NULL;
} else if (repo_fmt->work_tree) { /* #6, #14 */
if (is_absolute_path(repo_fmt->work_tree)) {
- set_git_work_tree(repo, repo_fmt->work_tree);
+ repo_discovery_set_worktree(discovery, repo_fmt->work_tree);
} else {
char *core_worktree;
if (chdir(gitdirenv))
@@ -1151,49 +1184,46 @@ static const char *setup_explicit_git_dir(struct repository *repo,
core_worktree = xgetcwd();
if (chdir(cwd->buf))
die_errno(_("cannot come back to cwd"));
- set_git_work_tree(repo, core_worktree);
+ repo_discovery_set_worktree(discovery, core_worktree);
free(core_worktree);
}
} else if (!git_env_bool(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, 1)) {
/* #16d */
- apply_and_export_relative_gitdir(repo, gitdirenv, 0);
+ repo_discovery_set_gitdir(discovery, gitdirenv, 0);
free(gitfile);
return NULL;
} else { /* #2, #10 */
- set_git_work_tree(repo, ".");
+ repo_discovery_set_worktree(discovery, ".");
}
- /* set_git_work_tree() must have been called by now */
- worktree = repo_get_work_tree(repo);
-
- /* both repo_get_work_tree() and cwd are already normalized */
- if (!strcmp(cwd->buf, worktree)) { /* cwd == worktree */
- apply_and_export_relative_gitdir(repo, gitdirenv, 0);
+ /* both the worktree and cwd are already normalized */
+ if (!strcmp(cwd->buf, discovery->worktree)) { /* cwd == worktree */
+ repo_discovery_set_gitdir(discovery, gitdirenv, 0);
free(gitfile);
return NULL;
}
- offset = dir_inside_of(cwd->buf, worktree);
- if (offset >= 0) { /* cwd inside worktree? */
- apply_and_export_relative_gitdir(repo, gitdirenv, 1);
- if (chdir(worktree))
- die_errno(_("cannot chdir to '%s'"), worktree);
+ offset = dir_inside_of(cwd->buf, discovery->worktree);
+ if (offset >= 0) { /* cwd inside discovery->worktree? */
+ repo_discovery_set_gitdir(discovery, gitdirenv, 1);
+ if (chdir(discovery->worktree))
+ die_errno(_("cannot chdir to '%s'"), discovery->worktree);
strbuf_addch(cwd, '/');
free(gitfile);
return cwd->buf + offset;
}
/* cwd outside worktree */
- apply_and_export_relative_gitdir(repo, gitdirenv, 0);
+ repo_discovery_set_gitdir(discovery, gitdirenv, 0);
free(gitfile);
return NULL;
}
-static const char *setup_discovered_git_dir(struct repository *repo,
- const char *gitdir,
- struct strbuf *cwd, int offset,
- struct repository_format *repo_fmt,
- int *nongit_ok)
+static const char *repo_discover_implicit_gitdir(struct repo_discovery *discovery,
+ const char *gitdir,
+ struct strbuf *cwd, int offset,
+ struct repository_format *repo_fmt,
+ int *nongit_ok)
{
if (read_and_verify_repository_format(repo_fmt, gitdir, nongit_ok))
return NULL;
@@ -1207,23 +1237,24 @@ static const char *setup_discovered_git_dir(struct repository *repo,
gitdir = to_free = real_pathdup(gitdir, 1);
if (chdir(cwd->buf))
die_errno(_("cannot come back to cwd"));
- ret = setup_explicit_git_dir(repo, gitdir, cwd, repo_fmt, nongit_ok);
+ ret = repo_discover_explicit_gitdir(discovery, gitdir, cwd,
+ repo_fmt, nongit_ok);
free(to_free);
return ret;
}
/* #16.2, #17.2, #20.2, #21.2, #24, #25, #28, #29 (see t1510) */
if (repo_fmt->is_bare > 0) {
- apply_and_export_relative_gitdir(repo, gitdir, (offset != cwd->len));
+ repo_discovery_set_gitdir(discovery, gitdir, (offset != cwd->len));
if (chdir(cwd->buf))
die_errno(_("cannot come back to cwd"));
return NULL;
}
/* #0, #1, #5, #8, #9, #12, #13 */
- set_git_work_tree(repo, ".");
+ repo_discovery_set_worktree(discovery, ".");
if (strcmp(gitdir, DEFAULT_GIT_DIR_ENVIRONMENT))
- apply_and_export_relative_gitdir(repo, gitdir, 0);
+ repo_discovery_set_gitdir(discovery, gitdir, 0);
if (offset >= cwd->len)
return NULL;
@@ -1236,10 +1267,10 @@ static const char *setup_discovered_git_dir(struct repository *repo,
}
/* #16.1, #17.1, #20.1, #21.1, #22.1 (see t1510) */
-static const char *setup_bare_git_dir(struct repository *repo,
- struct strbuf *cwd, int offset,
- struct repository_format *repo_fmt,
- int *nongit_ok)
+static const char *repo_discover_bare_gitdir(struct repo_discovery *discovery,
+ struct strbuf *cwd, int offset,
+ struct repository_format *repo_fmt,
+ int *nongit_ok)
{
int root_len;
@@ -1255,7 +1286,8 @@ static const char *setup_bare_git_dir(struct repository *repo,
gitdir = offset == cwd->len ? "." : xmemdupz(cwd->buf, offset);
if (chdir(cwd->buf))
die_errno(_("cannot come back to cwd"));
- return setup_explicit_git_dir(repo, gitdir, cwd, repo_fmt, nongit_ok);
+ return repo_discover_explicit_gitdir(discovery, gitdir, cwd,
+ repo_fmt, nongit_ok);
}
if (offset != cwd->len) {
@@ -1263,10 +1295,10 @@ static const char *setup_bare_git_dir(struct repository *repo,
die_errno(_("cannot come back to cwd"));
root_len = offset_1st_component(cwd->buf);
strbuf_setlen(cwd, offset > root_len ? offset : root_len);
- apply_and_export_relative_gitdir(repo, cwd->buf, 0);
+ repo_discovery_set_gitdir(discovery, cwd->buf, 0);
}
else
- apply_and_export_relative_gitdir(repo, ".", 0);
+ repo_discovery_set_gitdir(discovery, ".", 0);
return NULL;
}
@@ -1525,10 +1557,10 @@ static int is_implicit_bare_repo(const char *path)
* the discovered .git/ directory, if any. If `gitdir` is not absolute, it
* is relative to `dir` (i.e. *not* necessarily the cwd).
*/
-static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
- struct strbuf *gitdir,
- struct strbuf *report,
- int die_on_error)
+static enum discovery_result repo_discovery_find_dir(struct strbuf *dir,
+ struct strbuf *gitdir,
+ struct strbuf *report,
+ int die_on_error)
{
const char *env_ceiling_dirs = getenv(CEILING_DIRECTORIES_ENVIRONMENT);
struct string_list ceiling_dirs = STRING_LIST_INIT_DUP;
@@ -1695,7 +1727,7 @@ enum discovery_result discover_git_directory_reason(struct strbuf *commondir,
return GIT_DIR_CWD_FAILURE;
cwd_len = dir.len;
- result = setup_git_directory_gently_1(&dir, gitdir, NULL, 0);
+ result = repo_discovery_find_dir(&dir, gitdir, NULL, 0);
if (result <= 0) {
strbuf_release(&dir);
return result;
@@ -1902,6 +1934,7 @@ 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;
+ struct repo_discovery discovery = REPO_DISCOVERY_INIT;
const char *prefix = NULL;
struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;
@@ -1926,20 +1959,22 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
die_errno(_("Unable to read current working directory"));
strbuf_addbuf(&dir, &cwd);
- switch (setup_git_directory_gently_1(&dir, &gitdir, &report, 1)) {
+ switch (repo_discovery_find_dir(&dir, &gitdir, &report, 1)) {
case GIT_DIR_EXPLICIT:
- prefix = setup_explicit_git_dir(repo, gitdir.buf, &cwd, &repo_fmt, nongit_ok);
+ prefix = repo_discover_explicit_gitdir(&discovery, gitdir.buf, &cwd,
+ &repo_fmt, nongit_ok);
break;
case GIT_DIR_DISCOVERED:
if (dir.len < cwd.len && chdir(dir.buf))
die(_("cannot change to '%s'"), dir.buf);
- prefix = setup_discovered_git_dir(repo, gitdir.buf, &cwd, dir.len,
- &repo_fmt, nongit_ok);
+ prefix = repo_discover_implicit_gitdir(&discovery, gitdir.buf, &cwd, dir.len,
+ &repo_fmt, nongit_ok);
break;
case GIT_DIR_BARE:
if (dir.len < cwd.len && chdir(dir.buf))
die(_("cannot change to '%s'"), dir.buf);
- prefix = setup_bare_git_dir(repo, &cwd, dir.len, &repo_fmt, nongit_ok);
+ prefix = repo_discover_bare_gitdir(&discovery, &cwd, dir.len,
+ &repo_fmt, nongit_ok);
break;
case GIT_DIR_HIT_CEILING:
if (!nongit_ok)
@@ -1980,13 +2015,13 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
case GIT_DIR_CWD_FAILURE:
case GIT_DIR_INVALID_FORMAT:
/*
- * As a safeguard against setup_git_directory_gently_1 returning
+ * As a safeguard against repo_discovery_find_dir returning
* these values, fallthrough to BUG. Otherwise it is possible to
* set startup_info->have_repository to 1 when we did nothing to
* find a repository.
*/
default:
- BUG("unhandled setup_git_directory_gently_1() result");
+ BUG("unhandled repo_discovery_find_dir() result");
}
/*
@@ -2005,10 +2040,10 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
startup_info->have_repository = 1;
/*
- * Not all paths through the setup code will call 'apply_and_export_relative_gitdir()' (which
- * directly sets up the environment) so in order to guarantee that the
- * environment is in a consistent state after setup, explicitly setup
- * the environment if we have a repository.
+ * Not all paths through the setup code will have recorded a gitdir
+ * above, so in order to guarantee that the environment is in a
+ * consistent state after setup, explicitly set up the gitdir and
+ * environment if we have a repository.
*
* NEEDSWORK: currently we allow bogus GIT_DIR values to be set in some
* code paths so we also need to explicitly setup the environment if
@@ -2019,7 +2054,12 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
startup_info->have_repository ||
/* GIT_DIR_EXPLICIT */
getenv(GIT_DIR_ENVIRONMENT)) {
- if (!repo->gitdir) {
+ if (discovery.worktree)
+ set_git_work_tree(repo, discovery.worktree);
+
+ if (discovery.gitdir) {
+ apply_and_export_relative_gitdir(repo, discovery.gitdir, 0);
+ } else {
const char *gitdir = getenv(GIT_DIR_ENVIRONMENT);
if (!gitdir)
gitdir = DEFAULT_GIT_DIR_ENVIRONMENT;
@@ -2074,6 +2114,7 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
setup_original_cwd(repo);
+ repo_discovery_release(&discovery);
strbuf_release(&dir);
strbuf_release(&gitdir);
strbuf_release(&report);
--
2.55.0.795.g602f6c329a.dirty
^ permalink raw reply related
* [PATCH 06/13] setup: embed repository format in discovery
From: Patrick Steinhardt @ 2026-06-30 11:47 UTC (permalink / raw)
To: git
In-Reply-To: <20260630-pks-setup-split-discovery-and-setup-v1-0-13864eb5a032@pks.im>
All functions related to repository discovery receive both a `struct
repository_discovery` and `struct repository_format` as input, and the
expectation is that both will be populated. Refactor this so that the
repository format is part of the discovery result.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
setup.c | 60 +++++++++++++++++++++++++++++-------------------------------
1 file changed, 29 insertions(+), 31 deletions(-)
diff --git a/setup.c b/setup.c
index 06768de23f..0185257b2c 100644
--- a/setup.c
+++ b/setup.c
@@ -1091,14 +1091,18 @@ static void apply_and_export_relative_gitdir(struct repository *repo, const char
}
struct repo_discovery {
+ struct repository_format format;
char *gitdir;
char *worktree;
};
-#define REPO_DISCOVERY_INIT { 0 }
+#define REPO_DISCOVERY_INIT { \
+ .format = REPOSITORY_FORMAT_INIT, \
+}
static void repo_discovery_release(struct repo_discovery *r)
{
+ clear_repository_format(&r->format);
free(r->gitdir);
free(r->worktree);
}
@@ -1127,7 +1131,6 @@ static void repo_discovery_set_worktree(struct repo_discovery *r,
static const char *repo_discover_explicit_gitdir(struct repo_discovery *discovery,
const char *gitdirenv,
struct strbuf *cwd,
- struct repository_format *repo_fmt,
int *nongit_ok)
{
const char *work_tree_env = getenv(GIT_WORK_TREE_ENVIRONMENT);
@@ -1152,7 +1155,7 @@ static const char *repo_discover_explicit_gitdir(struct repo_discovery *discover
die(_("not a git repository: '%s'"), gitdirenv);
}
- if (read_and_verify_repository_format(repo_fmt, gitdirenv, nongit_ok)) {
+ if (read_and_verify_repository_format(&discovery->format, gitdirenv, nongit_ok)) {
free(gitfile);
return NULL;
}
@@ -1165,22 +1168,22 @@ static const char *repo_discover_explicit_gitdir(struct repo_discovery *discover
* bogus where we have both "core.worktree" and "core.bare", so
* we have to exlicitly unset the configuration.
*/
- FREE_AND_NULL(repo_fmt->work_tree);
+ FREE_AND_NULL(discovery->format.work_tree);
repo_discovery_set_worktree(discovery, work_tree_env);
- } else if (repo_fmt->is_bare > 0) {
+ } else if (discovery->format.is_bare > 0) {
/* #18, #26 */
repo_discovery_set_gitdir(discovery, gitdirenv, 0);
free(gitfile);
return NULL;
- } else if (repo_fmt->work_tree) { /* #6, #14 */
- if (is_absolute_path(repo_fmt->work_tree)) {
- repo_discovery_set_worktree(discovery, repo_fmt->work_tree);
+ } else if (discovery->format.work_tree) { /* #6, #14 */
+ if (is_absolute_path(discovery->format.work_tree)) {
+ repo_discovery_set_worktree(discovery, discovery->format.work_tree);
} else {
char *core_worktree;
if (chdir(gitdirenv))
die_errno(_("cannot chdir to '%s'"), gitdirenv);
- if (chdir(repo_fmt->work_tree))
- die_errno(_("cannot chdir to '%s'"), repo_fmt->work_tree);
+ if (chdir(discovery->format.work_tree))
+ die_errno(_("cannot chdir to '%s'"), discovery->format.work_tree);
core_worktree = xgetcwd();
if (chdir(cwd->buf))
die_errno(_("cannot come back to cwd"));
@@ -1222,14 +1225,13 @@ static const char *repo_discover_explicit_gitdir(struct repo_discovery *discover
static const char *repo_discover_implicit_gitdir(struct repo_discovery *discovery,
const char *gitdir,
struct strbuf *cwd, int offset,
- struct repository_format *repo_fmt,
int *nongit_ok)
{
- if (read_and_verify_repository_format(repo_fmt, gitdir, nongit_ok))
+ if (read_and_verify_repository_format(&discovery->format, gitdir, nongit_ok))
return NULL;
/* --work-tree is set without --git-dir; use discovered one */
- if (getenv(GIT_WORK_TREE_ENVIRONMENT) || repo_fmt->work_tree) {
+ if (getenv(GIT_WORK_TREE_ENVIRONMENT) || discovery->format.work_tree) {
char *to_free = NULL;
const char *ret;
@@ -1238,13 +1240,13 @@ static const char *repo_discover_implicit_gitdir(struct repo_discovery *discover
if (chdir(cwd->buf))
die_errno(_("cannot come back to cwd"));
ret = repo_discover_explicit_gitdir(discovery, gitdir, cwd,
- repo_fmt, nongit_ok);
+ nongit_ok);
free(to_free);
return ret;
}
/* #16.2, #17.2, #20.2, #21.2, #24, #25, #28, #29 (see t1510) */
- if (repo_fmt->is_bare > 0) {
+ if (discovery->format.is_bare > 0) {
repo_discovery_set_gitdir(discovery, gitdir, (offset != cwd->len));
if (chdir(cwd->buf))
die_errno(_("cannot come back to cwd"));
@@ -1269,25 +1271,24 @@ static const char *repo_discover_implicit_gitdir(struct repo_discovery *discover
/* #16.1, #17.1, #20.1, #21.1, #22.1 (see t1510) */
static const char *repo_discover_bare_gitdir(struct repo_discovery *discovery,
struct strbuf *cwd, int offset,
- struct repository_format *repo_fmt,
int *nongit_ok)
{
int root_len;
- if (read_and_verify_repository_format(repo_fmt, ".", nongit_ok))
+ if (read_and_verify_repository_format(&discovery->format, ".", nongit_ok))
return NULL;
setenv(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, "0", 1);
/* --work-tree is set without --git-dir; use discovered one */
- if (getenv(GIT_WORK_TREE_ENVIRONMENT) || repo_fmt->work_tree) {
+ if (getenv(GIT_WORK_TREE_ENVIRONMENT) || discovery->format.work_tree) {
static const char *gitdir;
gitdir = offset == cwd->len ? "." : xmemdupz(cwd->buf, offset);
if (chdir(cwd->buf))
die_errno(_("cannot come back to cwd"));
return repo_discover_explicit_gitdir(discovery, gitdir, cwd,
- repo_fmt, nongit_ok);
+ nongit_ok);
}
if (offset != cwd->len) {
@@ -1936,7 +1937,6 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
struct strbuf dir = STRBUF_INIT, gitdir = STRBUF_INIT, report = STRBUF_INIT;
struct repo_discovery discovery = REPO_DISCOVERY_INIT;
const char *prefix = NULL;
- struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;
/*
* We may have read an incomplete configuration before
@@ -1962,19 +1962,19 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
switch (repo_discovery_find_dir(&dir, &gitdir, &report, 1)) {
case GIT_DIR_EXPLICIT:
prefix = repo_discover_explicit_gitdir(&discovery, gitdir.buf, &cwd,
- &repo_fmt, nongit_ok);
+ nongit_ok);
break;
case GIT_DIR_DISCOVERED:
if (dir.len < cwd.len && chdir(dir.buf))
die(_("cannot change to '%s'"), dir.buf);
prefix = repo_discover_implicit_gitdir(&discovery, gitdir.buf, &cwd, dir.len,
- &repo_fmt, nongit_ok);
+ nongit_ok);
break;
case GIT_DIR_BARE:
if (dir.len < cwd.len && chdir(dir.buf))
die(_("cannot change to '%s'"), dir.buf);
prefix = repo_discover_bare_gitdir(&discovery, &cwd, dir.len,
- &repo_fmt, nongit_ok);
+ nongit_ok);
break;
case GIT_DIR_HIT_CEILING:
if (!nongit_ok)
@@ -2078,21 +2078,21 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
if (ref_backend_uri) {
char *format;
- free(repo_fmt.ref_storage_payload);
+ free(discovery.format.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)
+ parse_reference_uri(ref_backend_uri, &format, &discovery.format.ref_storage_payload);
+ discovery.format.ref_storage_format = ref_storage_format_by_name(format);
+ if (discovery.format.ref_storage_format == REF_STORAGE_FORMAT_UNKNOWN)
die(_("unknown ref storage format: '%s'"), format);
free(format);
}
- if (apply_repository_format(repo, &repo_fmt,
+ if (apply_repository_format(repo, &discovery.format,
APPLY_REPOSITORY_FORMAT_HONOR_ENV, &err) < 0)
die("%s", err.buf);
- clear_repository_format(&repo_fmt);
+ clear_repository_format(&discovery.format);
strbuf_release(&err);
}
}
@@ -2118,8 +2118,6 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
strbuf_release(&dir);
strbuf_release(&gitdir);
strbuf_release(&report);
- clear_repository_format(&repo_fmt);
-
return prefix;
}
--
2.55.0.795.g602f6c329a.dirty
^ permalink raw reply related
* [PATCH 07/13] setup: move prefix into repository
From: Patrick Steinhardt @ 2026-06-30 11:47 UTC (permalink / raw)
To: git
In-Reply-To: <20260630-pks-setup-split-discovery-and-setup-v1-0-13864eb5a032@pks.im>
The repository prefix is currently stored in the startup info. This
feels somewhat awkward though, as it is inherently a property of a given
repository.
Move the prefix into the repository accordingly.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/repo.c | 8 ++++----
builtin/rev-parse.c | 5 +++--
builtin/update-index.c | 4 ++--
object-name.c | 4 ++--
repository.c | 1 +
repository.h | 8 ++++++++
setup.c | 6 +++---
setup.h | 1 -
trace.c | 4 ++--
9 files changed, 25 insertions(+), 16 deletions(-)
diff --git a/builtin/repo.c b/builtin/repo.c
index 042d6de558..84e012f83f 100644
--- a/builtin/repo.c
+++ b/builtin/repo.c
@@ -84,7 +84,7 @@ static int get_path_commondir_absolute(struct repository *repo, struct strbuf *b
if (!common_dir)
return error(_("unable to get common directory"));
- format_path(buf, common_dir, startup_info->prefix, PATH_FORMAT_CANONICAL);
+ format_path(buf, common_dir, repo->prefix, PATH_FORMAT_CANONICAL);
return 0;
}
@@ -95,7 +95,7 @@ static int get_path_commondir_relative(struct repository *repo, struct strbuf *b
if (!common_dir)
return error(_("unable to get common directory"));
- format_path(buf, common_dir, startup_info->prefix, PATH_FORMAT_RELATIVE);
+ format_path(buf, common_dir, repo->prefix, PATH_FORMAT_RELATIVE);
return 0;
}
@@ -106,7 +106,7 @@ static int get_path_gitdir_absolute(struct repository *repo, struct strbuf *buf)
if (!git_dir)
return error(_("unable to get git directory"));
- format_path(buf, git_dir, startup_info->prefix, PATH_FORMAT_CANONICAL);
+ format_path(buf, git_dir, repo->prefix, PATH_FORMAT_CANONICAL);
return 0;
}
@@ -117,7 +117,7 @@ static int get_path_gitdir_relative(struct repository *repo, struct strbuf *buf)
if (!git_dir)
return error(_("unable to get git directory"));
- format_path(buf, git_dir, startup_info->prefix, PATH_FORMAT_RELATIVE);
+ format_path(buf, git_dir, repo->prefix, PATH_FORMAT_RELATIVE);
return 0;
}
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 5e04b0e2bd..43693454d5 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -255,7 +255,7 @@ static int show_file(const char *arg, int output_prefix)
show_default();
if ((filter & (DO_NONFLAGS|DO_NOREV)) == (DO_NONFLAGS|DO_NOREV)) {
if (output_prefix) {
- const char *prefix = startup_info->prefix;
+ const char *prefix = the_repository->prefix;
char *fname = prefix_filename(prefix, arg);
show(fname);
free(fname);
@@ -832,7 +832,8 @@ int cmd_rev_parse(int argc,
prefix = argv[++i];
if (!prefix)
die(_("--prefix requires an argument"));
- startup_info->prefix = prefix;
+ FREE_AND_NULL(the_repository->prefix);
+ the_repository->prefix = xstrdup(prefix);
output_prefix = 1;
continue;
}
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 3d6646c318..f43d150eb3 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -875,7 +875,7 @@ static enum parse_opt_result unresolve_callback(
const char *arg, int unset)
{
int *has_errors = opt->value;
- const char *prefix = startup_info->prefix;
+ const char *prefix = the_repository->prefix;
BUG_ON_OPT_NEG(unset);
BUG_ON_OPT_ARG(arg);
@@ -896,7 +896,7 @@ static enum parse_opt_result reupdate_callback(
const char *arg, int unset)
{
int *has_errors = opt->value;
- const char *prefix = startup_info->prefix;
+ const char *prefix = the_repository->prefix;
BUG_ON_OPT_NEG(unset);
BUG_ON_OPT_ARG(arg);
diff --git a/object-name.c b/object-name.c
index 46159466ac..fc70acc9e0 100644
--- a/object-name.c
+++ b/object-name.c
@@ -1708,8 +1708,8 @@ static char *resolve_relative_path(struct repository *r, const char *rel)
die(_("relative path syntax can't be used outside working tree"));
/* die() inside prefix_path() if resolved path is outside worktree */
- return prefix_path(the_repository, startup_info->prefix,
- startup_info->prefix ? strlen(startup_info->prefix) : 0,
+ return prefix_path(the_repository, the_repository->prefix,
+ the_repository->prefix ? strlen(the_repository->prefix) : 0,
rel);
}
diff --git a/repository.c b/repository.c
index 73d80bcffd..2ef0778846 100644
--- a/repository.c
+++ b/repository.c
@@ -376,6 +376,7 @@ void repo_clear(struct repository *repo)
FREE_AND_NULL(repo->gitdir);
FREE_AND_NULL(repo->commondir);
+ FREE_AND_NULL(repo->prefix);
FREE_AND_NULL(repo->graft_file);
FREE_AND_NULL(repo->index_file);
FREE_AND_NULL(repo->worktree);
diff --git a/repository.h b/repository.h
index 7d649e32e7..b767307911 100644
--- a/repository.h
+++ b/repository.h
@@ -52,6 +52,14 @@ struct repository {
*/
char *commondir;
+ /*
+ * The "prefix", a path to the current working directory relative to
+ * the work tree root, or NULL, if the current working directory is not
+ * a strict subdirectory of the work tree root. The prefix always ends
+ * with a '/' character.
+ */
+ char *prefix;
+
/*
* Holds any information related to accessing the raw object content.
*/
diff --git a/setup.c b/setup.c
index 0185257b2c..fc88ea2dbd 100644
--- a/setup.c
+++ b/setup.c
@@ -2030,7 +2030,7 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
* repository and that the caller expects startup_info to reflect
* this.
*
- * Regardless of the state of nongit_ok, startup_info->prefix and
+ * Regardless of the state of nongit_ok, the_repository->prefix and
* the GIT_PREFIX environment variable must always match. For details
* see Documentation/config/alias.adoc.
*/
@@ -2105,10 +2105,10 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
*/
if (prefix) {
prefix = precompose_string_if_needed(prefix);
- startup_info->prefix = prefix;
+ repo->prefix = xstrdup(prefix);
setenv(GIT_PREFIX_ENVIRONMENT, prefix, 1);
} else {
- startup_info->prefix = NULL;
+ FREE_AND_NULL(repo->prefix);
setenv(GIT_PREFIX_ENVIRONMENT, "", 1);
}
diff --git a/setup.h b/setup.h
index b9fd96bea6..c01a244fe9 100644
--- a/setup.h
+++ b/setup.h
@@ -299,7 +299,6 @@ struct startup_info {
bool force_bare_repository;
int have_repository;
- const char *prefix;
const char *original_cwd;
};
extern struct startup_info *startup_info;
diff --git a/trace.c b/trace.c
index 9b99460db8..515b99e7f5 100644
--- a/trace.c
+++ b/trace.c
@@ -299,7 +299,7 @@ static const char *quote_crnl(const char *path)
void trace_repo_setup(struct repository *r)
{
- const char *git_work_tree, *prefix = startup_info->prefix;
+ const char *git_work_tree, *prefix = r->prefix;
char *cwd;
if (!trace_want(&trace_setup_key))
@@ -310,7 +310,7 @@ void trace_repo_setup(struct repository *r)
if (!(git_work_tree = repo_get_work_tree(r)))
git_work_tree = "(null)";
- if (!startup_info->prefix)
+ if (!r->prefix)
prefix = "(null)";
trace_printf_key(&trace_setup_key, "setup: git_dir: %s\n", quote_crnl(repo_get_git_dir(r)));
--
2.55.0.795.g602f6c329a.dirty
^ permalink raw reply related
* [PATCH 08/13] setup: drop static `cwd` variable
From: Patrick Steinhardt @ 2026-06-30 11:47 UTC (permalink / raw)
To: git
In-Reply-To: <20260630-pks-setup-split-discovery-and-setup-v1-0-13864eb5a032@pks.im>
The current working directory is stored as part of a static strbuf
variable. This variable had to have a lifetime longer than its
containing function because the value we return typically points into
that buffer.
In the preceding commit we have moved the prefix into the repository
though. Consequently, we can now return the repository's prefix instead
of the local one and thus properly manage the lifecycle of this local
variable.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
setup.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/setup.c b/setup.c
index fc88ea2dbd..971024e5a6 100644
--- a/setup.c
+++ b/setup.c
@@ -1933,7 +1933,7 @@ void set_git_work_tree(struct repository *repo, const char *new_work_tree)
const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
{
- static struct strbuf cwd = STRBUF_INIT;
+ struct strbuf cwd = STRBUF_INIT;
struct strbuf dir = STRBUF_INIT, gitdir = STRBUF_INIT, report = STRBUF_INIT;
struct repo_discovery discovery = REPO_DISCOVERY_INIT;
const char *prefix = NULL;
@@ -2116,9 +2116,10 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
repo_discovery_release(&discovery);
strbuf_release(&dir);
+ strbuf_release(&cwd);
strbuf_release(&gitdir);
strbuf_release(&report);
- return prefix;
+ return repo->prefix;
}
int git_config_perm(const char *var, const char *value)
--
2.55.0.795.g602f6c329a.dirty
^ permalink raw reply related
* [PATCH 09/13] setup: propagate prefix via repository discovery
From: Patrick Steinhardt @ 2026-06-30 11:47 UTC (permalink / raw)
To: git
In-Reply-To: <20260630-pks-setup-split-discovery-and-setup-v1-0-13864eb5a032@pks.im>
In the preceding commits we have started to propagate all information
required for the configuration of the repository via a new `struct
repo_discovery`. The only exception is the repository's prefix, which we
still return via the return parameter.
This is conceptually fine, but somewhat inconsistent. Refactor this to
instead propagate the prefix via the repository discovery, too.
While at it, drop a static variable in `repo_discover_bare_gitdir()`.
We apply its value to the repository discovery anyway, so we don't have
to keep it around afterwards anymore.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
setup.c | 101 +++++++++++++++++++++++++++++-----------------------------------
1 file changed, 45 insertions(+), 56 deletions(-)
diff --git a/setup.c b/setup.c
index 971024e5a6..fc73276149 100644
--- a/setup.c
+++ b/setup.c
@@ -1094,6 +1094,7 @@ struct repo_discovery {
struct repository_format format;
char *gitdir;
char *worktree;
+ char *prefix;
};
#define REPO_DISCOVERY_INIT { \
@@ -1105,6 +1106,7 @@ static void repo_discovery_release(struct repo_discovery *r)
clear_repository_format(&r->format);
free(r->gitdir);
free(r->worktree);
+ free(r->prefix);
}
static void repo_discovery_set_gitdir(struct repo_discovery *r,
@@ -1128,10 +1130,10 @@ static void repo_discovery_set_worktree(struct repo_discovery *r,
r->worktree = real_pathdup(worktree, 1);
}
-static const char *repo_discover_explicit_gitdir(struct repo_discovery *discovery,
- const char *gitdirenv,
- struct strbuf *cwd,
- int *nongit_ok)
+static void repo_discover_explicit_gitdir(struct repo_discovery *discovery,
+ const char *gitdirenv,
+ struct strbuf *cwd,
+ int *nongit_ok)
{
const char *work_tree_env = getenv(GIT_WORK_TREE_ENVIRONMENT);
char *gitfile;
@@ -1149,16 +1151,13 @@ static const char *repo_discover_explicit_gitdir(struct repo_discovery *discover
if (!is_git_directory(gitdirenv)) {
if (nongit_ok) {
*nongit_ok = 1;
- free(gitfile);
- return NULL;
+ goto out;
}
die(_("not a git repository: '%s'"), gitdirenv);
}
- if (read_and_verify_repository_format(&discovery->format, gitdirenv, nongit_ok)) {
- free(gitfile);
- return NULL;
- }
+ if (read_and_verify_repository_format(&discovery->format, gitdirenv, nongit_ok))
+ goto out;
/* #3, #7, #11, #15, #19, #23, #27, #31 (see t1510) */
if (work_tree_env) {
@@ -1173,8 +1172,7 @@ static const char *repo_discover_explicit_gitdir(struct repo_discovery *discover
} else if (discovery->format.is_bare > 0) {
/* #18, #26 */
repo_discovery_set_gitdir(discovery, gitdirenv, 0);
- free(gitfile);
- return NULL;
+ goto out;
} else if (discovery->format.work_tree) { /* #6, #14 */
if (is_absolute_path(discovery->format.work_tree)) {
repo_discovery_set_worktree(discovery, discovery->format.work_tree);
@@ -1193,8 +1191,7 @@ static const char *repo_discover_explicit_gitdir(struct repo_discovery *discover
} else if (!git_env_bool(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, 1)) {
/* #16d */
repo_discovery_set_gitdir(discovery, gitdirenv, 0);
- free(gitfile);
- return NULL;
+ goto out;
} else { /* #2, #10 */
repo_discovery_set_worktree(discovery, ".");
}
@@ -1202,8 +1199,7 @@ static const char *repo_discover_explicit_gitdir(struct repo_discovery *discover
/* both the worktree and cwd are already normalized */
if (!strcmp(cwd->buf, discovery->worktree)) { /* cwd == worktree */
repo_discovery_set_gitdir(discovery, gitdirenv, 0);
- free(gitfile);
- return NULL;
+ goto out;
}
offset = dir_inside_of(cwd->buf, discovery->worktree);
@@ -1211,38 +1207,37 @@ static const char *repo_discover_explicit_gitdir(struct repo_discovery *discover
repo_discovery_set_gitdir(discovery, gitdirenv, 1);
if (chdir(discovery->worktree))
die_errno(_("cannot chdir to '%s'"), discovery->worktree);
- strbuf_addch(cwd, '/');
- free(gitfile);
- return cwd->buf + offset;
+ discovery->prefix = xstrfmt("%s/", cwd->buf + offset);
+ goto out;
}
/* cwd outside worktree */
repo_discovery_set_gitdir(discovery, gitdirenv, 0);
+
+out:
free(gitfile);
- return NULL;
}
-static const char *repo_discover_implicit_gitdir(struct repo_discovery *discovery,
- const char *gitdir,
- struct strbuf *cwd, int offset,
- int *nongit_ok)
+static void repo_discover_implicit_gitdir(struct repo_discovery *discovery,
+ const char *gitdir,
+ struct strbuf *cwd, int offset,
+ int *nongit_ok)
{
if (read_and_verify_repository_format(&discovery->format, gitdir, nongit_ok))
- return NULL;
+ return;
/* --work-tree is set without --git-dir; use discovered one */
if (getenv(GIT_WORK_TREE_ENVIRONMENT) || discovery->format.work_tree) {
char *to_free = NULL;
- const char *ret;
if (offset != cwd->len && !is_absolute_path(gitdir))
gitdir = to_free = real_pathdup(gitdir, 1);
if (chdir(cwd->buf))
die_errno(_("cannot come back to cwd"));
- ret = repo_discover_explicit_gitdir(discovery, gitdir, cwd,
- nongit_ok);
+ repo_discover_explicit_gitdir(discovery, gitdir, cwd,
+ nongit_ok);
free(to_free);
- return ret;
+ return;
}
/* #16.2, #17.2, #20.2, #21.2, #24, #25, #28, #29 (see t1510) */
@@ -1250,7 +1245,7 @@ static const char *repo_discover_implicit_gitdir(struct repo_discovery *discover
repo_discovery_set_gitdir(discovery, gitdir, (offset != cwd->len));
if (chdir(cwd->buf))
die_errno(_("cannot come back to cwd"));
- return NULL;
+ return;
}
/* #0, #1, #5, #8, #9, #12, #13 */
@@ -1258,37 +1253,34 @@ static const char *repo_discover_implicit_gitdir(struct repo_discovery *discover
if (strcmp(gitdir, DEFAULT_GIT_DIR_ENVIRONMENT))
repo_discovery_set_gitdir(discovery, gitdir, 0);
if (offset >= cwd->len)
- return NULL;
+ return;
/* Make "offset" point past the '/' (already the case for root dirs) */
if (offset != offset_1st_component(cwd->buf))
offset++;
- /* Add a '/' at the end */
- strbuf_addch(cwd, '/');
- return cwd->buf + offset;
+ discovery->prefix = xstrfmt("%s/", cwd->buf + offset);
}
/* #16.1, #17.1, #20.1, #21.1, #22.1 (see t1510) */
-static const char *repo_discover_bare_gitdir(struct repo_discovery *discovery,
- struct strbuf *cwd, int offset,
- int *nongit_ok)
+static void repo_discover_bare_gitdir(struct repo_discovery *discovery,
+ struct strbuf *cwd, int offset,
+ int *nongit_ok)
{
int root_len;
if (read_and_verify_repository_format(&discovery->format, ".", nongit_ok))
- return NULL;
+ return;
setenv(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, "0", 1);
/* --work-tree is set without --git-dir; use discovered one */
if (getenv(GIT_WORK_TREE_ENVIRONMENT) || discovery->format.work_tree) {
- static const char *gitdir;
-
- gitdir = offset == cwd->len ? "." : xmemdupz(cwd->buf, offset);
+ char *gitdir = offset == cwd->len ? xstrdup(".") : xmemdupz(cwd->buf, offset);
if (chdir(cwd->buf))
die_errno(_("cannot come back to cwd"));
- return repo_discover_explicit_gitdir(discovery, gitdir, cwd,
- nongit_ok);
+ repo_discover_explicit_gitdir(discovery, gitdir, cwd, nongit_ok);
+ free(gitdir);
+ return;
}
if (offset != cwd->len) {
@@ -1297,10 +1289,9 @@ static const char *repo_discover_bare_gitdir(struct repo_discovery *discovery,
root_len = offset_1st_component(cwd->buf);
strbuf_setlen(cwd, offset > root_len ? offset : root_len);
repo_discovery_set_gitdir(discovery, cwd->buf, 0);
- }
- else
+ } else {
repo_discovery_set_gitdir(discovery, ".", 0);
- return NULL;
+ }
}
static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_len)
@@ -1936,7 +1927,6 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
struct strbuf cwd = STRBUF_INIT;
struct strbuf dir = STRBUF_INIT, gitdir = STRBUF_INIT, report = STRBUF_INIT;
struct repo_discovery discovery = REPO_DISCOVERY_INIT;
- const char *prefix = NULL;
/*
* We may have read an incomplete configuration before
@@ -1961,20 +1951,19 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
switch (repo_discovery_find_dir(&dir, &gitdir, &report, 1)) {
case GIT_DIR_EXPLICIT:
- prefix = repo_discover_explicit_gitdir(&discovery, gitdir.buf, &cwd,
- nongit_ok);
+ repo_discover_explicit_gitdir(&discovery, gitdir.buf, &cwd,
+ nongit_ok);
break;
case GIT_DIR_DISCOVERED:
if (dir.len < cwd.len && chdir(dir.buf))
die(_("cannot change to '%s'"), dir.buf);
- prefix = repo_discover_implicit_gitdir(&discovery, gitdir.buf, &cwd, dir.len,
- nongit_ok);
+ repo_discover_implicit_gitdir(&discovery, gitdir.buf, &cwd, dir.len,
+ nongit_ok);
break;
case GIT_DIR_BARE:
if (dir.len < cwd.len && chdir(dir.buf))
die(_("cannot change to '%s'"), dir.buf);
- prefix = repo_discover_bare_gitdir(&discovery, &cwd, dir.len,
- nongit_ok);
+ repo_discover_bare_gitdir(&discovery, &cwd, dir.len, nongit_ok);
break;
case GIT_DIR_HIT_CEILING:
if (!nongit_ok)
@@ -2103,10 +2092,10 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
* out where the repository is, i.e. a preparation
* for calling repo_config_get_bool().
*/
- if (prefix) {
- prefix = precompose_string_if_needed(prefix);
+ if (discovery.prefix) {
+ const char *prefix = precompose_string_if_needed(discovery.prefix);
repo->prefix = xstrdup(prefix);
- setenv(GIT_PREFIX_ENVIRONMENT, prefix, 1);
+ setenv(GIT_PREFIX_ENVIRONMENT, repo->prefix, 1);
} else {
FREE_AND_NULL(repo->prefix);
setenv(GIT_PREFIX_ENVIRONMENT, "", 1);
--
2.55.0.795.g602f6c329a.dirty
^ permalink raw reply related
* [PATCH 10/13] setup: make repository discovery self-contained
From: Patrick Steinhardt @ 2026-06-30 11:47 UTC (permalink / raw)
To: git
In-Reply-To: <20260630-pks-setup-split-discovery-and-setup-v1-0-13864eb5a032@pks.im>
In the preceding commits we have introduced a separate repository
discovery phase and refactored the logic so that we have two clear
phases:
1. Repository discovery, which doesn't modify the repository itself at
all.
2. Repository configuration, which takes the information we have
discovered to set up the repository.
Extract the first phase into a new function `repo_discover()` to further
stress these two different phases.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
setup.c | 43 +++++++++++++++++++++++++------------------
1 file changed, 25 insertions(+), 18 deletions(-)
diff --git a/setup.c b/setup.c
index fc73276149..7715f3ea85 100644
--- a/setup.c
+++ b/setup.c
@@ -1922,20 +1922,10 @@ void set_git_work_tree(struct repository *repo, const char *new_work_tree)
repo_set_worktree(repo, new_work_tree);
}
-const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
+static void repo_discover(struct repo_discovery *discovery, int *nongit_ok)
{
struct strbuf cwd = STRBUF_INIT;
struct strbuf dir = STRBUF_INIT, gitdir = STRBUF_INIT, report = STRBUF_INIT;
- struct repo_discovery discovery = REPO_DISCOVERY_INIT;
-
- /*
- * We may have read an incomplete configuration before
- * setting-up the git directory. If so, clear the cache so
- * that the next queries to the configuration reload complete
- * configuration (including the per-repo config file that we
- * ignored previously).
- */
- repo_config_clear(repo);
/*
* Let's assume that we are in a git repository.
@@ -1951,19 +1941,19 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
switch (repo_discovery_find_dir(&dir, &gitdir, &report, 1)) {
case GIT_DIR_EXPLICIT:
- repo_discover_explicit_gitdir(&discovery, gitdir.buf, &cwd,
+ repo_discover_explicit_gitdir(discovery, gitdir.buf, &cwd,
nongit_ok);
break;
case GIT_DIR_DISCOVERED:
if (dir.len < cwd.len && chdir(dir.buf))
die(_("cannot change to '%s'"), dir.buf);
- repo_discover_implicit_gitdir(&discovery, gitdir.buf, &cwd, dir.len,
+ repo_discover_implicit_gitdir(discovery, gitdir.buf, &cwd, dir.len,
nongit_ok);
break;
case GIT_DIR_BARE:
if (dir.len < cwd.len && chdir(dir.buf))
die(_("cannot change to '%s'"), dir.buf);
- repo_discover_bare_gitdir(&discovery, &cwd, dir.len, nongit_ok);
+ repo_discover_bare_gitdir(discovery, &cwd, dir.len, nongit_ok);
break;
case GIT_DIR_HIT_CEILING:
if (!nongit_ok)
@@ -2013,6 +2003,27 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
BUG("unhandled repo_discovery_find_dir() result");
}
+ strbuf_release(&dir);
+ strbuf_release(&cwd);
+ strbuf_release(&gitdir);
+ strbuf_release(&report);
+}
+
+const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
+{
+ struct repo_discovery discovery = REPO_DISCOVERY_INIT;
+
+ /*
+ * We may have read an incomplete configuration before
+ * setting-up the git directory. If so, clear the cache so
+ * that the next queries to the configuration reload complete
+ * configuration (including the per-repo config file that we
+ * ignored previously).
+ */
+ repo_config_clear(repo);
+
+ repo_discover(&discovery, nongit_ok);
+
/*
* At this point, nongit_ok is stable. If it is non-NULL and points
* to a non-zero value, then this means that we haven't found a
@@ -2104,10 +2115,6 @@ const char *setup_git_directory_gently(struct repository *repo, int *nongit_ok)
setup_original_cwd(repo);
repo_discovery_release(&discovery);
- strbuf_release(&dir);
- strbuf_release(&cwd);
- strbuf_release(&gitdir);
- strbuf_release(&report);
return repo->prefix;
}
--
2.55.0.795.g602f6c329a.dirty
^ permalink raw reply related
* [PATCH 11/13] setup: drop redundant configuration of `startup_info->have_repository`
From: Patrick Steinhardt @ 2026-06-30 11:47 UTC (permalink / raw)
To: git
In-Reply-To: <20260630-pks-setup-split-discovery-and-setup-v1-0-13864eb5a032@pks.im>
In `init_db()` we set `startup_info->have_repository` twice: once before
reading and applying the repository format and once after. This is
redundant though, as configuring the repository format does not rely on
this variable at all.
Remove the first such site. While at it, fix up formatting a bit.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
setup.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/setup.c b/setup.c
index 7715f3ea85..4f37a7b642 100644
--- a/setup.c
+++ b/setup.c
@@ -2847,12 +2847,10 @@ int init_db(struct repository *repo,
apply_and_export_relative_gitdir(repo, real_git_dir, 1);
git_dir = repo_get_git_dir(repo);
separate_git_dir(git_dir, original_git_dir);
- }
- else {
+ } else {
apply_and_export_relative_gitdir(repo, git_dir, 1);
git_dir = repo_get_git_dir(repo);
}
- startup_info->have_repository = 1;
/*
* Check to see if the repository version is right.
--
2.55.0.795.g602f6c329a.dirty
^ permalink raw reply related
* [PATCH 12/13] setup: pass worktree to `init_db()`
From: Patrick Steinhardt @ 2026-06-30 11:47 UTC (permalink / raw)
To: git
In-Reply-To: <20260630-pks-setup-split-discovery-and-setup-v1-0-13864eb5a032@pks.im>
In the preceding commits we have refactored how we discover and set up
repositories so that we cannot end up with partially-configured repos.
Instead, we apply the gitdir, worktree and repository format in a single
location, only.
Initializing a new repository has the same antipattern though: while
most of the information for the new repository is passed via parameters,
the work tree is instead propagated by configuring the repository's work
tree.
Refactor the code so that we also pass the work tree as an explicit
parameter. Like this, configuration fo the repository happens in a
single spot, too, just as with repository discovery.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/clone.c | 8 ++++----
builtin/init-db.c | 34 ++++++++++------------------------
setup.c | 7 ++++++-
setup.h | 4 +++-
4 files changed, 23 insertions(+), 30 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index d60d1b60bc..9d08cd8722 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1116,7 +1116,6 @@ int cmd_clone(int argc,
die_errno(_("could not create work tree dir '%s'"),
work_tree);
junk_work_tree = work_tree;
- set_git_work_tree(the_repository, work_tree);
}
if (real_git_dir) {
@@ -1186,9 +1185,10 @@ int cmd_clone(int argc,
* repository, and reference backends may persist that information into
* their on-disk data structures.
*/
- init_db(the_repository, git_dir, real_git_dir, option_template, GIT_HASH_UNKNOWN,
- ref_storage_format, NULL,
- do_not_override_repo_unix_permissions, INIT_DB_QUIET | INIT_DB_SKIP_REFDB);
+ init_db(the_repository, git_dir, real_git_dir, work_tree, option_template,
+ GIT_HASH_UNKNOWN, ref_storage_format, NULL,
+ do_not_override_repo_unix_permissions,
+ INIT_DB_QUIET | INIT_DB_SKIP_REFDB);
if (real_git_dir) {
free((char *)git_dir);
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 566732c9f4..e96b1283b7 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -231,39 +231,25 @@ int cmd_init_db(int argc,
if (!bare) {
const char *git_dir_parent = strrchr(git_dir, '/');
- if (work_tree) {
- set_git_work_tree(the_repository, work_tree);
- } else {
- char *work_tree_cfg = NULL;
-
+ if (!work_tree) {
if (git_dir_parent) {
char *rel = xstrndup(git_dir, git_dir_parent - git_dir);
- work_tree_cfg = real_pathdup(rel, 1);
+ work_tree = real_pathdup(rel, 1);
free(rel);
+ } else {
+ work_tree = xgetcwd();
}
-
- if (!work_tree_cfg)
- work_tree_cfg = xgetcwd();
-
- set_git_work_tree(the_repository, work_tree_cfg);
-
- free(work_tree_cfg);
}
- if (access(repo_get_work_tree(the_repository), X_OK))
- die_errno (_("Cannot access work tree '%s'"),
- repo_get_work_tree(the_repository));
- }
- else {
- if (real_git_dir)
- die(_("--separate-git-dir incompatible with bare repository"));
- if (work_tree)
- set_git_work_tree(the_repository, work_tree);
+ if (access(work_tree, X_OK))
+ die_errno (_("Cannot access work tree '%s'"), work_tree);
+ } else if (real_git_dir) {
+ die(_("--separate-git-dir incompatible with bare repository"));
}
flags |= INIT_DB_EXIST_OK;
- ret = init_db(the_repository, git_dir, real_git_dir, template_dir, hash_algo,
- ref_storage_format, initial_branch,
+ ret = init_db(the_repository, git_dir, real_git_dir, work_tree,
+ template_dir, hash_algo, ref_storage_format, initial_branch,
init_shared_repository, flags);
free(template_dir_to_free);
diff --git a/setup.c b/setup.c
index 4f37a7b642..40e26862ca 100644
--- a/setup.c
+++ b/setup.c
@@ -2823,7 +2823,9 @@ static void repository_format_configure(struct repository_format *repo_fmt,
}
int init_db(struct repository *repo,
- const char *git_dir, const char *real_git_dir,
+ const char *git_dir,
+ const char *real_git_dir,
+ const char *worktree,
const char *template_dir, int hash,
enum ref_storage_format ref_storage_format,
const char *initial_branch,
@@ -2852,6 +2854,9 @@ int init_db(struct repository *repo,
git_dir = repo_get_git_dir(repo);
}
+ if (worktree)
+ set_git_work_tree(repo, worktree);
+
/*
* Check to see if the repository version is right.
* Note that a newly created repository does not have
diff --git a/setup.h b/setup.h
index c01a244fe9..bf3e3f3ea6 100644
--- a/setup.h
+++ b/setup.h
@@ -263,7 +263,9 @@ const char *get_template_dir(const char *option_template);
#define INIT_DB_SKIP_REFDB (1 << 2)
int init_db(struct repository *repo,
- const char *git_dir, const char *real_git_dir,
+ const char *git_dir,
+ const char *real_git_dir,
+ const char *worktree,
const char *template_dir, int hash_algo,
enum ref_storage_format ref_storage_format,
const char *initial_branch, int init_shared_repository,
--
2.55.0.795.g602f6c329a.dirty
^ permalink raw reply related
* [PATCH 13/13] setup: mark `set_git_work_tree()` as file-local
From: Patrick Steinhardt @ 2026-06-30 11:47 UTC (permalink / raw)
To: git
In-Reply-To: <20260630-pks-setup-split-discovery-and-setup-v1-0-13864eb5a032@pks.im>
In the preceding commit we have removed the last callers of
`set_git_work_tree()` that is located outside of "setup.c". Remove its
declaration and mark the function as file-local.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
setup.c | 2 +-
setup.h | 2 --
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/setup.c b/setup.c
index 40e26862ca..1be040e178 100644
--- a/setup.c
+++ b/setup.c
@@ -1904,7 +1904,7 @@ const char *enter_repo(struct repository *repo, const char *path, unsigned flags
* primarily to support git-clone to work in a new repository it just
* created, and is not meant to flip between different work trees.
*/
-void set_git_work_tree(struct repository *repo, const char *new_work_tree)
+static void set_git_work_tree(struct repository *repo, const char *new_work_tree)
{
if (repo->worktree_initialized) {
struct strbuf realpath = STRBUF_INIT;
diff --git a/setup.h b/setup.h
index bf3e3f3ea6..bb24ee8f0f 100644
--- a/setup.h
+++ b/setup.h
@@ -96,8 +96,6 @@ static inline int discover_git_directory(struct strbuf *commondir,
return 0;
}
-void set_git_work_tree(struct repository *repo, const char *tree);
-
/* Flags that can be passed to `enter_repo()`. */
enum {
/*
--
2.55.0.795.g602f6c329a.dirty
^ permalink raw reply related
* [PATCH v3 0/5] builtin/refs: add ability to write references
From: Patrick Steinhardt @ 2026-06-30 11:49 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <20260616-pks-refs-writing-subcommands-v1-0-9f5219b6109d@pks.im>
Hi,
Reference-related functionality in Git is currently spread across many
different commands: git-update-ref(1), git-for-each-ref(1),
git-show-ref(1), git-pack-refs(1) and git-symbolic-ref(1). This makes it
hard for users to discover what functionality we have available to work
with references.
We have thus started to consolidate this functionality into git-refs(1),
which is a toolbox of everything related to references. Until now, the
command doesn't handle functionality of git-update-ref(1).
This patch series backfills most of the functionality by introducing
three new commands:
- `git refs delete` to delete references. This is the equivalent of
`git update-ref -d`.
- `git refs update` to update references. This is the equivalent of
`git update-ref <refname> <oldvalue> <newvalue>`.
- `git refs rename` to rename a reference, including its reflog. This
does not have an equivalent in git-update-ref(1), but is inspired by
and supersedes [1].
Changes in v3:
- Fix confused error message.
- Link to v2: https://patch.msgid.link/20260617-pks-refs-writing-subcommands-v2-0-07f3d18336f9@pks.im
Changes in v2:
- Add a new "create" subcommand.
- Consistently quote in error messages.
- Consistently use `<old-value>` in the synopsis.
- Don't return negative exit codes.
- Improve documentation of "update" subcommand to mention that you can
create and delete branches.
- Add tests to verify that we can use "update" to do this, both in
racy and raceless ways.
- Add missing calls to `repo_config()`.
- Drop useless `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME` variable.
- Link to v1: https://patch.msgid.link/20260616-pks-refs-writing-subcommands-v1-0-9f5219b6109d@pks.im
Thanks!
Patrick
[1]: <xmqqv7brz9ba.fsf@gitster.g>
---
Patrick Steinhardt (5):
builtin/refs: drop `the_repository`
builtin/refs: add "delete" subcommand
builtin/refs: add "update" subcommand
builtin/refs: add "create" subcommand
builtin/refs: add "rename" subcommand
Documentation/git-refs.adoc | 40 +++++++
builtin/refs.c | 222 ++++++++++++++++++++++++++++++++++--
t/meson.build | 4 +
t/t1464-refs-delete.sh | 130 +++++++++++++++++++++
t/t1465-refs-update.sh | 268 ++++++++++++++++++++++++++++++++++++++++++++
t/t1466-refs-create.sh | 151 +++++++++++++++++++++++++
t/t1467-refs-rename.sh | 131 ++++++++++++++++++++++
7 files changed, 938 insertions(+), 8 deletions(-)
Range-diff versus v2:
1: 2341316537 = 1: 9dd7b70df4 builtin/refs: drop `the_repository`
2: 40efa6887b = 2: 72341f895e builtin/refs: add "delete" subcommand
3: 2f86fb281d = 3: bcca5fe8ee builtin/refs: add "update" subcommand
4: fb75fb72cf ! 4: 6fa75a36bd builtin/refs: add "create" subcommand
@@ builtin/refs.c: static int cmd_refs_optimize(int argc, const char **argv, const
+ if (repo_get_oid_with_flags(repo, argv[1], &newoid, GET_OID_SKIP_AMBIGUITY_CHECK))
+ die(_("invalid object ID: '%s'"), argv[1]);
+ if (is_null_oid(&newoid))
-+ die(_("cannot create reference with null old object ID"));
++ die(_("cannot create reference with null new object ID"));
+
+ ret = refs_update_ref(get_main_ref_store(repo), message, refname,
+ &newoid, null_oid(repo->hash_algo), flags,
@@ t/t1466-refs-create.sh (new)
+ (
+ cd repo &&
+ test_must_fail git refs create refs/heads/foo $ZERO_OID 2>err &&
-+ test_grep "null old object ID" err &&
++ test_grep "null new object ID" err &&
+ test_must_fail git refs exists refs/heads/foo
+ )
+'
5: 134b161ec7 = 5: 8e12fe028e builtin/refs: add "rename" subcommand
---
base-commit: 700432b2ba22603a0bcb71475c9c333d17c9b0d1
change-id: 20260616-pks-refs-writing-subcommands-7a77be5bda9b
^ permalink raw reply
* [PATCH v3 1/5] builtin/refs: drop `the_repository`
From: Patrick Steinhardt @ 2026-06-30 11:49 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <20260630-pks-refs-writing-subcommands-v3-0-deb04de1ecef@pks.im>
We still have a couple of uses of `the_repository` in "builtin/refs.c".
All of those are trivial to convert though as the command always
requires a repository to exist.
Convert them to use the passed-in repository and drop
`USE_THE_REPOSITORY_VARIABLE`.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/refs.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/builtin/refs.c b/builtin/refs.c
index e3125bc61b..f0faabf45a 100644
--- a/builtin/refs.c
+++ b/builtin/refs.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
#include "builtin.h"
#include "config.h"
#include "fsck.h"
@@ -23,7 +22,7 @@
N_("git refs optimize " PACK_REFS_OPTS)
static int cmd_refs_migrate(int argc, const char **argv, const char *prefix,
- struct repository *repo UNUSED)
+ struct repository *repo)
{
const char * const migrate_usage[] = {
REFS_MIGRATE_USAGE,
@@ -59,13 +58,13 @@ static int cmd_refs_migrate(int argc, const char **argv, const char *prefix,
goto out;
}
- if (the_repository->ref_storage_format == format) {
+ if (repo->ref_storage_format == format) {
err = error(_("repository already uses '%s' format"),
ref_storage_format_to_name(format));
goto out;
}
- if (repo_migrate_ref_storage_format(the_repository, format, flags, &errbuf) < 0) {
+ if (repo_migrate_ref_storage_format(repo, format, flags, &errbuf) < 0) {
err = error("%s", errbuf.buf);
goto out;
}
@@ -99,8 +98,8 @@ static int cmd_refs_verify(int argc, const char **argv, const char *prefix,
if (argc)
usage(_("'git refs verify' takes no arguments"));
- repo_config(the_repository, git_fsck_config, &fsck_refs_options);
- prepare_repo_settings(the_repository);
+ repo_config(repo, git_fsck_config, &fsck_refs_options);
+ prepare_repo_settings(repo);
worktrees = get_worktrees_without_reading_head();
for (size_t i = 0; worktrees[i]; i++)
@@ -124,7 +123,7 @@ static int cmd_refs_list(int argc, const char **argv, const char *prefix,
}
static int cmd_refs_exists(int argc, const char **argv, const char *prefix,
- struct repository *repo UNUSED)
+ struct repository *repo)
{
struct strbuf unused_referent = STRBUF_INIT;
struct object_id unused_oid;
@@ -145,7 +144,7 @@ static int cmd_refs_exists(int argc, const char **argv, const char *prefix,
die(_("'git refs exists' requires a reference"));
ref = *argv++;
- if (refs_read_raw_ref(get_main_ref_store(the_repository), ref,
+ if (refs_read_raw_ref(get_main_ref_store(repo), ref,
&unused_oid, &unused_referent, &unused_type,
&failure_errno)) {
if (failure_errno == ENOENT || failure_errno == EISDIR) {
--
2.55.0.795.g602f6c329a.dirty
^ permalink raw reply related
* [PATCH v3 2/5] builtin/refs: add "delete" subcommand
From: Patrick Steinhardt @ 2026-06-30 11:49 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <20260630-pks-refs-writing-subcommands-v3-0-deb04de1ecef@pks.im>
Reference-related functionality in Git is currently spread across many
different commands: git-update-ref(1), git-for-each-ref(1),
git-show-ref(1), git-pack-refs(1) and git-symbolic-ref(1). This makes it
hard for users to discover what functionality we have available to work
with references.
We have thus started to consolidate this functionality into git-refs(1),
which is a toolbox of everything related to references. Until now, the
command doesn't handle functionality of git-update-ref(1).
Fix this gap by introducing a new "delete" subcommand, which is the
equivalent of `git update-ref -d`.
Note that we're intentionally not using a generic "write" subcommand
with a "-d" flag. This is rather harder to discover, and subcommands
that are implmented as flags tend to be hard to reason about in the code
as we'd have to handle mutually-exclusive flags that stem from the other
subcommand-like modes.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/git-refs.adoc | 17 ++++++
builtin/refs.c | 51 +++++++++++++++++
t/meson.build | 1 +
t/t1464-refs-delete.sh | 130 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 199 insertions(+)
diff --git a/Documentation/git-refs.adoc b/Documentation/git-refs.adoc
index fa33680cc7..2633934463 100644
--- a/Documentation/git-refs.adoc
+++ b/Documentation/git-refs.adoc
@@ -20,6 +20,7 @@ git refs list [--count=<count>] [--shell|--perl|--python|--tcl]
[ --stdin | (<pattern>...)]
git refs exists <ref>
git refs optimize [--all] [--no-prune] [--auto] [--include <pattern>] [--exclude <pattern>]
+git refs delete [--message=<reason>] [--no-deref] <ref> [<old-value>]
DESCRIPTION
-----------
@@ -51,6 +52,12 @@ optimize::
usage. This subcommand is an alias for linkgit:git-pack-refs[1] and
offers identical functionality.
+delete::
+ Delete the given reference. This subcommand mirrors `git update-ref -d`
+ (see linkgit:git-update-ref[1]). When `<old-value>` is given, the
+ reference is only deleted after verifying that it currently contains
+ `<old-value>`.
+
OPTIONS
-------
@@ -90,6 +97,16 @@ The following options are specific to 'git refs optimize':
include::pack-refs-options.adoc[]
+The following options are specific to commands which write references:
+
+`--message=<reason>`::
+ Use the given <reason> string for the reflog entry associated with the
+ update. An empty message is rejected.
+
+`--no-deref`::
+ Operate on <ref> itself rather than the reference it points to via a
+ symbolic ref.
+
KNOWN LIMITATIONS
-----------------
diff --git a/builtin/refs.c b/builtin/refs.c
index f0faabf45a..edb7d61663 100644
--- a/builtin/refs.c
+++ b/builtin/refs.c
@@ -21,6 +21,9 @@
#define REFS_OPTIMIZE_USAGE \
N_("git refs optimize " PACK_REFS_OPTS)
+#define REFS_DELETE_USAGE \
+ N_("git refs delete [--message=<reason>] [--no-deref] <ref> [<old-value>]")
+
static int cmd_refs_migrate(int argc, const char **argv, const char *prefix,
struct repository *repo)
{
@@ -175,6 +178,52 @@ static int cmd_refs_optimize(int argc, const char **argv, const char *prefix,
return pack_refs_core(argc, argv, prefix, repo, refs_optimize_usage);
}
+static int cmd_refs_delete(int argc, const char **argv, const char *prefix,
+ struct repository *repo)
+{
+ static char const * const refs_delete_usage[] = {
+ REFS_DELETE_USAGE,
+ NULL
+ };
+ const char *message = NULL;
+ unsigned flags = 0;
+ struct option opts[] = {
+ OPT_STRING(0, "message", &message, N_("reason"),
+ N_("reason of the update")),
+ OPT_BIT(0 ,"no-deref", &flags,
+ N_("update <refname> not the one it points to"),
+ REF_NO_DEREF),
+ OPT_END(),
+ };
+ struct object_id oldoid;
+ const char *refname;
+ int ret;
+
+ argc = parse_options(argc, argv, prefix, opts, refs_delete_usage, 0);
+ if (argc < 1 || argc > 2)
+ usage(_("delete requires reference name and an optional old object ID"));
+
+ if (message && !*message)
+ die(_("refusing to perform update with empty message"));
+
+ repo_config(repo, git_default_config, NULL);
+
+ refname = argv[0];
+ if (argc == 2) {
+ if (repo_get_oid_with_flags(repo, argv[1], &oldoid, GET_OID_SKIP_AMBIGUITY_CHECK))
+ die(_("invalid old object ID: '%s'"), argv[1]);
+ if (is_null_oid(&oldoid))
+ die(_("cannot delete reference with null old object ID"));
+ }
+
+ ret = refs_delete_ref(get_main_ref_store(repo), message, refname,
+ argc == 2 ? &oldoid : NULL, flags);
+
+ if (ret < 0)
+ ret = 1;
+ return ret;
+}
+
int cmd_refs(int argc,
const char **argv,
const char *prefix,
@@ -186,6 +235,7 @@ int cmd_refs(int argc,
"git refs list " COMMON_USAGE_FOR_EACH_REF,
REFS_EXISTS_USAGE,
REFS_OPTIMIZE_USAGE,
+ REFS_DELETE_USAGE,
NULL,
};
parse_opt_subcommand_fn *fn = NULL;
@@ -195,6 +245,7 @@ int cmd_refs(int argc,
OPT_SUBCOMMAND("list", &fn, cmd_refs_list),
OPT_SUBCOMMAND("exists", &fn, cmd_refs_exists),
OPT_SUBCOMMAND("optimize", &fn, cmd_refs_optimize),
+ OPT_SUBCOMMAND("delete", &fn, cmd_refs_delete),
OPT_END(),
};
diff --git a/t/meson.build b/t/meson.build
index c5832fee05..1ccf08a3b5 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -223,6 +223,7 @@ integration_tests = [
't1461-refs-list.sh',
't1462-refs-exists.sh',
't1463-refs-optimize.sh',
+ 't1464-refs-delete.sh',
't1500-rev-parse.sh',
't1501-work-tree.sh',
't1502-rev-parse-parseopt.sh',
diff --git a/t/t1464-refs-delete.sh b/t/t1464-refs-delete.sh
new file mode 100755
index 0000000000..efff7d0574
--- /dev/null
+++ b/t/t1464-refs-delete.sh
@@ -0,0 +1,130 @@
+#!/bin/sh
+
+test_description='git refs delete'
+
+. ./test-lib.sh
+
+setup_repo () {
+ git init "$1" &&
+ test_commit -C "$1" A &&
+ test_commit -C "$1" B
+}
+
+test_expect_success 'delete without oldvalue verification' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ A=$(git -C repo rev-parse A) &&
+ git -C repo update-ref refs/heads/foo $A &&
+ git -C repo refs delete refs/heads/foo &&
+ test_must_fail git -C repo show-ref --verify -q refs/heads/foo
+'
+
+test_expect_success 'delete with matching oldvalue' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git update-ref refs/heads/foo $A &&
+ git refs delete refs/heads/foo $A &&
+ test_must_fail git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'delete with stale oldvalue fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ git update-ref refs/heads/foo $A &&
+ test_must_fail git refs delete refs/heads/foo $B 2>err &&
+ test_grep " but expected " err &&
+ git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'delete with null oldvalue fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git update-ref refs/heads/foo $A &&
+ test_must_fail git refs delete refs/heads/foo $ZERO_OID 2>err &&
+ test_grep "null old object ID" err &&
+ git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'delete with invalid oldvalue fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git update-ref refs/heads/foo $A &&
+ test_must_fail git refs delete refs/heads/foo invalid-oid 2>err &&
+ test_grep "invalid old object ID" err &&
+ git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'delete symref with --no-deref leaves target intact' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git update-ref refs/heads/foo $A &&
+ git symbolic-ref refs/heads/symref refs/heads/foo &&
+ git refs delete --no-deref refs/heads/symref &&
+ test_must_fail git refs exists refs/heads/symref &&
+ git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'delete with message records reason in reflog' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git update-ref refs/heads/foo $A &&
+ git symbolic-ref HEAD refs/heads/foo &&
+ git refs delete --message=delete-reason refs/heads/foo &&
+ test_must_fail git refs exists refs/heads/foo &&
+ test-tool ref-store main for-each-reflog-ent HEAD >actual &&
+ test_grep "delete-reason$" actual
+ )
+'
+
+test_expect_success 'delete with empty message fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git update-ref refs/heads/foo $A &&
+ test_must_fail git refs delete --message= refs/heads/foo 2>err &&
+ test_grep "empty message" err &&
+ git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'delete without arguments fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ test_must_fail git -C repo refs delete 2>err &&
+ test_grep "requires reference name" err
+'
+
+test_expect_success 'delete with too many arguments fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ test_must_fail git refs delete one two three 2>err &&
+ test_grep "requires reference name" err
+'
+
+test_done
--
2.55.0.795.g602f6c329a.dirty
^ permalink raw reply related
* [PATCH v3 3/5] builtin/refs: add "update" subcommand
From: Patrick Steinhardt @ 2026-06-30 11:49 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <20260630-pks-refs-writing-subcommands-v3-0-deb04de1ecef@pks.im>
Add a new "update" subcommand which mirrors `git update-ref <refname>
<oldoid> <newoid>`. This follows the same reasoning as the preceding
commit.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/git-refs.adoc | 12 ++
builtin/refs.c | 55 +++++++++
t/meson.build | 1 +
t/t1465-refs-update.sh | 268 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 336 insertions(+)
diff --git a/Documentation/git-refs.adoc b/Documentation/git-refs.adoc
index 2633934463..6475bdcc62 100644
--- a/Documentation/git-refs.adoc
+++ b/Documentation/git-refs.adoc
@@ -21,6 +21,7 @@ git refs list [--count=<count>] [--shell|--perl|--python|--tcl]
git refs exists <ref>
git refs optimize [--all] [--no-prune] [--auto] [--include <pattern>] [--exclude <pattern>]
git refs delete [--message=<reason>] [--no-deref] <ref> [<old-value>]
+git refs update [--message=<reason>] [--no-deref] [--create-reflog] <ref> <new-value> [<old-value>]
DESCRIPTION
-----------
@@ -58,6 +59,13 @@ delete::
reference is only deleted after verifying that it currently contains
`<old-value>`.
+update::
+ Update the given reference to point at `<new-value>`. If `<old-value>`
+ is given, the reference is only updated after verifying that it
+ currently contains `<old-value>`. As a special case, an all-zeroes
+ `<new-value>` deletes the branch, whereas an all-zeroes `<old-value>`
+ ensures that the branch does not yet exist.
+
OPTIONS
-------
@@ -99,6 +107,10 @@ include::pack-refs-options.adoc[]
The following options are specific to commands which write references:
+`--create-reflog`::
+ Create a reflog for the reference even if one would not ordinarily be
+ created.
+
`--message=<reason>`::
Use the given <reason> string for the reflog entry associated with the
update. An empty message is rejected.
diff --git a/builtin/refs.c b/builtin/refs.c
index edb7d61663..08453ae1c8 100644
--- a/builtin/refs.c
+++ b/builtin/refs.c
@@ -24,6 +24,9 @@
#define REFS_DELETE_USAGE \
N_("git refs delete [--message=<reason>] [--no-deref] <ref> [<old-value>]")
+#define REFS_UPDATE_USAGE \
+ N_("git refs update [--message=<reason>] [--no-deref] [--create-reflog] <ref> <new-value> [<old-value>]")
+
static int cmd_refs_migrate(int argc, const char **argv, const char *prefix,
struct repository *repo)
{
@@ -224,6 +227,56 @@ static int cmd_refs_delete(int argc, const char **argv, const char *prefix,
return ret;
}
+static int cmd_refs_update(int argc, const char **argv, const char *prefix,
+ struct repository *repo)
+{
+ static char const * const refs_update_usage[] = {
+ REFS_UPDATE_USAGE,
+ NULL
+ };
+ const char *message = NULL;
+ unsigned flags = 0;
+ struct option opts[] = {
+ OPT_STRING(0, "message", &message, N_("reason"),
+ N_("reason of the update")),
+ OPT_BIT(0 ,"no-deref", &flags,
+ N_("update <refname> not the one it points to"),
+ REF_NO_DEREF),
+ OPT_BIT(0, "create-reflog", &flags, N_("create a reflog"),
+ REF_FORCE_CREATE_REFLOG),
+ OPT_END(),
+ };
+ struct object_id newoid, oldoid;
+ const char *refname;
+ int ret;
+
+ argc = parse_options(argc, argv, prefix, opts, refs_update_usage, 0);
+ if (argc < 2 || argc > 3)
+ usage(_("update requires reference name, new value and an optional old value"));
+
+ if (message && !*message)
+ die(_("refusing to perform update with empty message"));
+
+ repo_config(repo, git_default_config, NULL);
+
+ refname = argv[0];
+ if (repo_get_oid_with_flags(repo, argv[1], &newoid,
+ GET_OID_SKIP_AMBIGUITY_CHECK))
+ die(_("invalid new object ID: '%s'"), argv[1]);
+ if (argc == 3 &&
+ repo_get_oid_with_flags(repo, argv[2], &oldoid,
+ GET_OID_SKIP_AMBIGUITY_CHECK))
+ die(_("invalid old object ID: '%s'"), argv[2]);
+
+ ret = refs_update_ref(get_main_ref_store(repo), message, refname,
+ &newoid, argc == 3 ? &oldoid : NULL, flags,
+ UPDATE_REFS_MSG_ON_ERR);
+
+ if (ret < 0)
+ ret = 1;
+ return ret;
+}
+
int cmd_refs(int argc,
const char **argv,
const char *prefix,
@@ -236,6 +289,7 @@ int cmd_refs(int argc,
REFS_EXISTS_USAGE,
REFS_OPTIMIZE_USAGE,
REFS_DELETE_USAGE,
+ REFS_UPDATE_USAGE,
NULL,
};
parse_opt_subcommand_fn *fn = NULL;
@@ -246,6 +300,7 @@ int cmd_refs(int argc,
OPT_SUBCOMMAND("exists", &fn, cmd_refs_exists),
OPT_SUBCOMMAND("optimize", &fn, cmd_refs_optimize),
OPT_SUBCOMMAND("delete", &fn, cmd_refs_delete),
+ OPT_SUBCOMMAND("update", &fn, cmd_refs_update),
OPT_END(),
};
diff --git a/t/meson.build b/t/meson.build
index 1ccf08a3b5..2063962dab 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -224,6 +224,7 @@ integration_tests = [
't1462-refs-exists.sh',
't1463-refs-optimize.sh',
't1464-refs-delete.sh',
+ 't1465-refs-update.sh',
't1500-rev-parse.sh',
't1501-work-tree.sh',
't1502-rev-parse-parseopt.sh',
diff --git a/t/t1465-refs-update.sh b/t/t1465-refs-update.sh
new file mode 100755
index 0000000000..a9becdda99
--- /dev/null
+++ b/t/t1465-refs-update.sh
@@ -0,0 +1,268 @@
+#!/bin/sh
+
+test_description='git refs update'
+
+. ./test-lib.sh
+
+setup_repo () {
+ git init "$1" &&
+ test_commit -C "$1" A &&
+ test_commit -C "$1" B
+}
+
+test_ref_matches () {
+ git rev-parse "$1" >expect &&
+ echo "$2" >actual &&
+ test_cmp expect actual
+}
+
+test_expect_success 'update creates a new reference' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs update refs/heads/foo $A &&
+ test_ref_matches refs/heads/foo "$A"
+ )
+'
+
+test_expect_success 'update an existing reference without oldvalue' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ git refs update refs/heads/foo $A &&
+ git refs update refs/heads/foo $B &&
+ test_ref_matches refs/heads/foo $B
+ )
+'
+
+test_expect_success 'update with matching oldvalue' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ git refs update refs/heads/foo $A &&
+ git refs update refs/heads/foo $B $A &&
+ test_ref_matches refs/heads/foo $B
+ )
+'
+
+test_expect_success 'update with stale oldvalue fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ git refs update refs/heads/foo $A &&
+ test_must_fail git refs update refs/heads/foo $B $B 2>err &&
+ test_grep " but expected " err &&
+ test_ref_matches refs/heads/foo $A
+ )
+'
+
+test_expect_success 'update can create a new branch with oldvalue' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs update refs/heads/foo $A $ZERO_OID 2>err &&
+ test_ref_matches refs/heads/foo $A
+ )
+'
+
+test_expect_success 'update can create a new branch without oldvalue' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs update refs/heads/foo $A 2>err &&
+ test_ref_matches refs/heads/foo $A
+ )
+'
+
+test_expect_success 'update refuses to create preexisting branch' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ git refs update refs/heads/foo $A &&
+ test_must_fail git refs update refs/heads/foo $B $ZERO_OID 2>err &&
+ test_grep "reference already exists" err &&
+ test_ref_matches refs/heads/foo $A
+ )
+'
+
+test_expect_success 'update can delete a branch with oldvalue' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs update refs/heads/foo $A 2>err &&
+ git refs update refs/heads/foo $ZERO_OID $A 2>err &&
+ test_must_fail git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'update can delete a branch without oldvalue' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs update refs/heads/foo $A 2>err &&
+ git refs update refs/heads/foo $ZERO_OID 2>err &&
+ test_must_fail git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'update refuses to delete a branch with mismatching value' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ git refs update refs/heads/foo $A 2>err &&
+ test_must_fail git refs update refs/heads/foo $ZERO_OID $B 2>err &&
+ test_grep " but expected " err &&
+ git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'update refuses to create preexisting branch' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ git refs update refs/heads/foo $A &&
+ test_must_fail git refs update refs/heads/foo $B $ZERO_OID 2>err &&
+ test_grep "reference already exists" err &&
+ test_ref_matches refs/heads/foo $A
+ )
+'
+
+
+test_expect_success 'update with invalid new value fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ test_must_fail git refs update refs/heads/foo invalid-oid 2>err &&
+ test_grep "invalid new object ID" err &&
+ test_must_fail git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'update with invalid old value fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ git refs update refs/heads/foo $A &&
+ test_must_fail git refs update refs/heads/foo $B invalid-oid 2>err &&
+ test_grep "invalid old object ID" err &&
+ test_ref_matches refs/heads/foo $A
+ )
+'
+
+test_expect_success 'update --no-deref rewrites the symref itself' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ git refs update refs/heads/foo $A &&
+ git symbolic-ref refs/heads/symref refs/heads/foo &&
+ git refs update --no-deref refs/heads/symref $B &&
+ test_must_fail git symbolic-ref refs/heads/symref &&
+ test_ref_matches refs/heads/symref $B &&
+ test_ref_matches refs/heads/foo $A
+ )
+'
+
+test_expect_success 'update does not create a reflog by default' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs update refs/foo $A &&
+ test_must_fail git reflog exists refs/foo
+ )
+'
+
+test_expect_success 'update creates a reflog with --create-reflog' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs update --create-reflog refs/foo $A &&
+ git reflog exists refs/foo
+ )
+'
+
+test_expect_success 'update with message records reason in reflog' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ git refs update refs/heads/foo $A &&
+ git refs update --message=update-reason refs/heads/foo $B &&
+ git reflog show refs/heads/foo >actual &&
+ test_grep "update-reason$" actual
+ )
+'
+
+test_expect_success 'update with empty message fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ git refs update refs/heads/foo $A &&
+ test_must_fail git refs update --message= refs/heads/foo $B 2>err &&
+ test_grep "empty message" err
+ )
+'
+
+test_expect_success 'update with too few arguments fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ test_must_fail git -C repo refs update refs/heads/foo 2>err &&
+ test_grep "requires reference name, new value" err
+'
+
+test_expect_success 'update with too many arguments fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ test_must_fail git refs update refs/heads/foo $A $B extra 2>err &&
+ test_grep "requires reference name, new value" err
+ )
+'
+
+test_done
--
2.55.0.795.g602f6c329a.dirty
^ permalink raw reply related
* [PATCH v3 4/5] builtin/refs: add "create" subcommand
From: Patrick Steinhardt @ 2026-06-30 11:49 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <20260630-pks-refs-writing-subcommands-v3-0-deb04de1ecef@pks.im>
The "update" subcommand cannot only update an existing reference, but it
can also create new branches and delete existing branches by specifying
the all-zeroes object ID as either old or new value. Despite that, we
already have the "delete" subcommand as a handy shortcut so that a user
can easily delete a branch. This relieves them of needing to understand
the more arcane uses of the "update" command, and of counting the number
of zeroes they need to pass.
But while we have a "delete" subcommand, we don't have an equivalent
that would allow the user to create a new branch, which creates a
certain asymmetry.
Add a new "create" subcommand to plug this gap.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/git-refs.adoc | 5 ++
builtin/refs.c | 52 +++++++++++++++
t/meson.build | 1 +
t/t1466-refs-create.sh | 151 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 209 insertions(+)
diff --git a/Documentation/git-refs.adoc b/Documentation/git-refs.adoc
index 6475bdcc62..e6a3528349 100644
--- a/Documentation/git-refs.adoc
+++ b/Documentation/git-refs.adoc
@@ -20,6 +20,7 @@ git refs list [--count=<count>] [--shell|--perl|--python|--tcl]
[ --stdin | (<pattern>...)]
git refs exists <ref>
git refs optimize [--all] [--no-prune] [--auto] [--include <pattern>] [--exclude <pattern>]
+git refs create [--message=<reason>] [--no-deref] [--create-reflog] <ref> <new-value>
git refs delete [--message=<reason>] [--no-deref] <ref> [<old-value>]
git refs update [--message=<reason>] [--no-deref] [--create-reflog] <ref> <new-value> [<old-value>]
@@ -53,6 +54,10 @@ optimize::
usage. This subcommand is an alias for linkgit:git-pack-refs[1] and
offers identical functionality.
+create::
+ Create the given reference, which must not already exist, pointing at
+ `<new-value>`.
+
delete::
Delete the given reference. This subcommand mirrors `git update-ref -d`
(see linkgit:git-update-ref[1]). When `<old-value>` is given, the
diff --git a/builtin/refs.c b/builtin/refs.c
index 08453ae1c8..1ebaf30149 100644
--- a/builtin/refs.c
+++ b/builtin/refs.c
@@ -21,6 +21,9 @@
#define REFS_OPTIMIZE_USAGE \
N_("git refs optimize " PACK_REFS_OPTS)
+#define REFS_CREATE_USAGE \
+ N_("git refs create [--message=<reason>] [--no-deref] [--create-reflog] <ref> <new-value>")
+
#define REFS_DELETE_USAGE \
N_("git refs delete [--message=<reason>] [--no-deref] <ref> [<old-value>]")
@@ -181,6 +184,53 @@ static int cmd_refs_optimize(int argc, const char **argv, const char *prefix,
return pack_refs_core(argc, argv, prefix, repo, refs_optimize_usage);
}
+static int cmd_refs_create(int argc, const char **argv, const char *prefix,
+ struct repository *repo)
+{
+ static char const * const refs_create_usage[] = {
+ REFS_CREATE_USAGE,
+ NULL
+ };
+ const char *message = NULL;
+ unsigned flags = 0;
+ struct option opts[] = {
+ OPT_STRING(0, "message", &message, N_("reason"),
+ N_("reason of the update")),
+ OPT_BIT(0 ,"no-deref", &flags,
+ N_("update <refname> not the one it points to"),
+ REF_NO_DEREF),
+ OPT_BIT(0, "create-reflog", &flags, N_("create a reflog"),
+ REF_FORCE_CREATE_REFLOG),
+ OPT_END(),
+ };
+ struct object_id newoid;
+ const char *refname;
+ int ret;
+
+ argc = parse_options(argc, argv, prefix, opts, refs_create_usage, 0);
+ if (argc != 2)
+ usage(_("create requires reference name and an object ID"));
+
+ if (message && !*message)
+ die(_("refusing to perform update with empty message"));
+
+ repo_config(repo, git_default_config, NULL);
+
+ refname = argv[0];
+ if (repo_get_oid_with_flags(repo, argv[1], &newoid, GET_OID_SKIP_AMBIGUITY_CHECK))
+ die(_("invalid object ID: '%s'"), argv[1]);
+ if (is_null_oid(&newoid))
+ die(_("cannot create reference with null new object ID"));
+
+ ret = refs_update_ref(get_main_ref_store(repo), message, refname,
+ &newoid, null_oid(repo->hash_algo), flags,
+ UPDATE_REFS_MSG_ON_ERR);
+
+ if (ret < 0)
+ ret = 1;
+ return ret;
+}
+
static int cmd_refs_delete(int argc, const char **argv, const char *prefix,
struct repository *repo)
{
@@ -288,6 +338,7 @@ int cmd_refs(int argc,
"git refs list " COMMON_USAGE_FOR_EACH_REF,
REFS_EXISTS_USAGE,
REFS_OPTIMIZE_USAGE,
+ REFS_CREATE_USAGE,
REFS_DELETE_USAGE,
REFS_UPDATE_USAGE,
NULL,
@@ -299,6 +350,7 @@ int cmd_refs(int argc,
OPT_SUBCOMMAND("list", &fn, cmd_refs_list),
OPT_SUBCOMMAND("exists", &fn, cmd_refs_exists),
OPT_SUBCOMMAND("optimize", &fn, cmd_refs_optimize),
+ OPT_SUBCOMMAND("create", &fn, cmd_refs_create),
OPT_SUBCOMMAND("delete", &fn, cmd_refs_delete),
OPT_SUBCOMMAND("update", &fn, cmd_refs_update),
OPT_END(),
diff --git a/t/meson.build b/t/meson.build
index 2063962dab..541e6f919c 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -225,6 +225,7 @@ integration_tests = [
't1463-refs-optimize.sh',
't1464-refs-delete.sh',
't1465-refs-update.sh',
+ 't1466-refs-create.sh',
't1500-rev-parse.sh',
't1501-work-tree.sh',
't1502-rev-parse-parseopt.sh',
diff --git a/t/t1466-refs-create.sh b/t/t1466-refs-create.sh
new file mode 100755
index 0000000000..cfb21bf863
--- /dev/null
+++ b/t/t1466-refs-create.sh
@@ -0,0 +1,151 @@
+#!/bin/sh
+
+test_description='git refs create'
+
+. ./test-lib.sh
+
+setup_repo () {
+ git init "$1" &&
+ test_commit -C "$1" A &&
+ test_commit -C "$1" B
+}
+
+test_ref_matches () {
+ git rev-parse "$1" >expect &&
+ echo "$2" >actual &&
+ test_cmp expect actual
+}
+
+test_expect_success 'create a new reference' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs create refs/heads/foo $A &&
+ test_ref_matches refs/heads/foo "$A"
+ )
+'
+
+test_expect_success 'create fails when the reference already exists' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ git refs create refs/heads/foo $A &&
+ test_must_fail git refs create refs/heads/foo $B 2>err &&
+ test_grep "reference already exists" err &&
+ test_ref_matches refs/heads/foo "$A"
+ )
+'
+
+test_expect_success 'create with null new value fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ test_must_fail git refs create refs/heads/foo $ZERO_OID 2>err &&
+ test_grep "null new object ID" err &&
+ test_must_fail git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'create with invalid new value fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ test_must_fail git refs create refs/heads/foo invalid-oid 2>err &&
+ test_grep "invalid object ID" err &&
+ test_must_fail git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'create does not create a reflog by default' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs create refs/foo $A &&
+ test_must_fail git reflog exists refs/foo
+ )
+'
+
+test_expect_success 'create creates a reflog with --create-reflog' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs create --create-reflog refs/foo $A &&
+ git reflog exists refs/foo
+ )
+'
+
+test_expect_success 'create with message records reason in reflog' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs create --message="create reason" refs/heads/foo $A &&
+ git reflog show refs/heads/foo >actual &&
+ test_grep "create reason$" actual
+ )
+'
+
+test_expect_success 'create with symref target creates target reference' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git symbolic-ref refs/heads/symref refs/heads/target &&
+ git refs create refs/heads/symref $A &&
+ git reflog exists refs/heads/target
+ )
+'
+
+test_expect_success 'create with symref target and --no-deref refuses to create reference' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git symbolic-ref refs/heads/symref refs/heads/target &&
+ test_must_fail git refs create --no-deref refs/heads/symref $A 2>err &&
+ test_grep "dangling symref already exists" err &&
+ test_must_fail git reflog exists refs/heads/target
+ )
+'
+
+test_expect_success 'create with empty message fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ test_must_fail git refs create --message= refs/heads/foo $A 2>err &&
+ test_grep "empty message" err &&
+ test_must_fail git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'create without arguments fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ test_must_fail git -C repo refs create 2>err &&
+ test_grep "requires reference name" err
+'
+
+test_expect_success 'create with too many arguments fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ test_must_fail git -C repo refs create refs/heads/foo a b 2>err &&
+ test_grep "requires reference name" err
+'
+
+test_done
--
2.55.0.795.g602f6c329a.dirty
^ 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