* Re: [PATCH v3 02/11] doc: interpret-trailers: replace “lines” with “metadata”
From: Matt Hunter @ 2026-06-16 21:39 UTC (permalink / raw)
To: Kristoffer Haugsbakk, git
Cc: Christian Couder, jackmanb, Linus Arver, D. Ben Knoble
In-Reply-To: <26c2fcb2-2618-4bb9-a6e4-a6135934556d@app.fastmail.com>
On Tue Jun 16, 2026 at 4:32 PM EDT, Kristoffer Haugsbakk wrote:
> On Thu, Jun 11, 2026, at 05:10, Matt Hunter wrote:
>> On Wed Jun 10, 2026 at 5:21 PM EDT, kristofferhaugsbakk wrote:
>>>[snip]
>>> DESCRIPTION
>>> -----------
>>> -Add or parse _trailer_ lines at the end of the otherwise
>>> +Add or parse trailers metadata at the end of the otherwise
>>
>> fwiw, I think "trailer metadata" reads more naturally.
>
> You’re right that your version reads more naturally. I went back and
> forth on this.
After reading your points, I started debating the grammatical
correctness of it to myself too. Though I think I stand by my original
knee-jerk response.
>
> 1. We’re introducing the jargon, and the format is often discussed as
> plural “trailers”, with its constituent parts being singular
> “trailer”
> 2. What this replaces uses “trailer”, but it rescues the plural mood
> with “lines”
> 3. This is very soon going to go into the constituent parts, including
> each trailer, so we’re contrasting the concept name (trailers) with
> its parts
>
> But I think your version is overall better. It reads better and there is
> no way to confuse “trailer metadata” (trailers as a collection) with
> just a single “trailer”.
Yes, "metadata" reads as a hint to me that interpret-trailers works with
the overall trailer block too.
Another thought I had after seeing this again is that the text could
just say "Add or parse trailers at the end ...", but "metadata" is a lot
more useful, since it's an introduction to what trailers _are_. So the
patch is an improvement over the original context imo.
^ permalink raw reply
* Re: [PATCH v2 07/17] odb/source-packed: wire up `reprepare()` callback
From: Justin Tobler @ 2026-06-16 21:53 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Karthik Nayak
In-Reply-To: <20260609-pks-odb-source-packed-v2-7-839089132c8b@pks.im>
On 26/06/09 10:51AM, Patrick Steinhardt wrote:
> Move the logic to prepare and reprepare the "packed" source into
> "odb/source-packed.c" and wire it up as the `reprepare()` callback.
>
> Note that "preparing" a source is not yet generic. Eventually, it would
> probably make sense to turn the existing `reprepare()` callback into a
> `prepare()` callback with an optional flag to force re-preparing. But
> this step will be handled in a separate patch series.
I do find the prepare vs reprepare semantics a bit confusing. The
mentioned change above would be nice to see in the future. :)
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
[snip]
> static void odb_source_packed_reparent(const char *name UNUSED,
> const char *old_cwd,
> const char *new_cwd,
> @@ -58,6 +214,7 @@ struct odb_source_packed *odb_source_packed_new(struct odb_source_files *parent)
>
> packed->base.free = odb_source_packed_free;
> packed->base.close = odb_source_packed_close;
> + packed->base.reprepare = odb_source_packed_reprepare;
We moved `packfile_store_reprepare()` logic into "odb/source-packed.c"
and setup the callback making it accessible via `odb_source_reprepare()`
instead. Makes sense.
> if (!is_absolute_path(parent->base.path))
> chdir_notify_register(NULL, odb_source_packed_reparent, packed);
> diff --git a/odb/source-packed.h b/odb/source-packed.h
> index 68e64cabab..9d4796261a 100644
> --- a/odb/source-packed.h
> +++ b/odb/source-packed.h
> @@ -81,4 +81,13 @@ static inline struct odb_source_packed *odb_source_packed_downcast(struct odb_so
> return container_of(source, struct odb_source_packed, base);
> }
>
> +/*
> + * Prepare the source by loading packfiles and multi-pack indices for
> + * all alternates. This becomes a no-op if the source is already prepared.
> + *
> + * It shouldn't typically be necessary to call this function directly, as
> + * functions that access the source know to prepare it.
> + */
> +void odb_source_packed_prepare(struct odb_source_packed *source);
The logic for `packfile_store_prepare()` is also moved into
"odb/source-packed.c" and made accessible this function since there
isn't a matching callback.
The other changes are mostly 1:1 moves of associated logic and callsite
updates. Looks good.
-Justin
^ permalink raw reply
* Re: [PATCH v2 11/17] odb/source-packed: wire up `for_each_object()` callback
From: Justin Tobler @ 2026-06-16 22:10 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Karthik Nayak
In-Reply-To: <20260609-pks-odb-source-packed-v2-11-839089132c8b@pks.im>
On 26/06/09 10:51AM, Patrick Steinhardt wrote:
> Move `packfile_store_for_each_object()` and its associated helpers from
> "packfile.c" into "odb/source-packed.c" and wire it up as the
> `for_each_object()` callback of the "packed" source.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
[snip]
> @@ -291,6 +548,7 @@ struct odb_source_packed *odb_source_packed_new(struct odb_source_files *parent)
> packed->base.reprepare = odb_source_packed_reprepare;
> packed->base.read_object_info = odb_source_packed_read_object_info;
> packed->base.read_object_stream = odb_source_packed_read_object_stream;
> + packed->base.for_each_object = odb_source_packed_for_each_object;
Wiring up the callback. Looks good.
> if (!is_absolute_path(parent->base.path))
> chdir_notify_register(NULL, odb_source_packed_reparent, packed);
> diff --git a/packfile.c b/packfile.c
> index 42c84397eb..b8d6054c16 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -1362,8 +1362,8 @@ static void add_delta_base_cache(struct packed_git *p, off_t base_offset,
> hashmap_add(&delta_base_cache, &ent->ent);
> }
>
> -static int packed_object_info_with_index_pos(struct packed_git *p, off_t obj_offset,
> - uint32_t *maybe_index_pos, struct object_info *oi)
> +int packed_object_info_with_index_pos(struct packed_git *p, off_t obj_offset,
> + uint32_t *maybe_index_pos, struct object_info *oi)
Looks like we are also exposing `packed_object_info_with_index_pos()`
now. Not sure yet if this is also intended to be temporary like
`find_pack_entry()` in a previous patch though though.
-Justin
^ permalink raw reply
* Re: [PATCH] cat-file: speed up default format
From: René Scharfe @ 2026-06-16 22:14 UTC (permalink / raw)
To: Jeff King; +Cc: Git List
In-Reply-To: <20260616111237.GA687438@coredump.intra.peff.net>
On 6/16/26 1:12 PM, Jeff King wrote:
> On Mon, Jun 15, 2026 at 11:53:07PM +0200, René Scharfe wrote:
>
>> We can also store literal characters in there. An opcode plus with a
>> payload char incurs an overhead of 50%, which sounds high, but at least
>> the default format only has two of them and it's much better than
>> storing pointer plus size for an overhead of more than 90% in case of a
>> single char.
>
> True, and it's a size win if the literal portions tend to be small
> (fewer than 15 bytes). You do lose out on the ability to strbuf_add()
> them in one go, though. So lots more strbuf_grow() checks, etc. If you
> really wanted to get fancy, you could follow the opcode with a length
> represented as a variable-sized integer, followed by the literal bytes.
Or an opcode that shovels a fixed-size string. Depends on how much
literal text people include in their formats.
> I'm not sure that Git's formatting code needs to squeeze out quite that
> much performance, though.
Good point. Near-native performance would be necessary to make peephole
optimizations like the special handling of the default format
unnecessary, which I understand exists to speed up Gitaly [1], but I
guess most users don't have such high demands. And there's no point in
removing a few lines of duplicate code if the necessary machinery adds a
lot of complexity. Though the code discussed so far was not too crazy
IMHO.
[1] https://gitlab.com/gitlab-org/gitaly/-/blob/master/internal/git/gitpipe/catfile_info.go
> OK, so we managed another 1%. But I'm skeptical that this linear opcode
> technique is where we want to go in the long run, if we're ever going to
> unify formatters.
Agreed.
René
^ permalink raw reply
* Re: [PATCH] cat-file: speed up default format
From: Junio C Hamano @ 2026-06-16 22:21 UTC (permalink / raw)
To: René Scharfe; +Cc: Jeff King, Git List
In-Reply-To: <47e3cf16-217e-45d4-91e2-5a1abb4ee49e@web.de>
René Scharfe <l.s.r@web.de> writes:
> On 6/16/26 1:12 PM, Jeff King wrote:
>> On Mon, Jun 15, 2026 at 11:53:07PM +0200, René Scharfe wrote:
>>
>>> We can also store literal characters in there. An opcode plus with a
>>> payload char incurs an overhead of 50%, which sounds high, but at least
>>> the default format only has two of them and it's much better than
>>> storing pointer plus size for an overhead of more than 90% in case of a
>>> single char.
>>
>> True, and it's a size win if the literal portions tend to be small
>> (fewer than 15 bytes). You do lose out on the ability to strbuf_add()
>> them in one go, though. So lots more strbuf_grow() checks, etc. If you
>> really wanted to get fancy, you could follow the opcode with a length
>> represented as a variable-sized integer, followed by the literal bytes.
>
> Or an opcode that shovels a fixed-size string. Depends on how much
> literal text people include in their formats.
>
>> I'm not sure that Git's formatting code needs to squeeze out quite that
>> much performance, though.
>
> Good point. Near-native performance would be necessary to make peephole
> optimizations like the special handling of the default format
> unnecessary, which I understand exists to speed up Gitaly [1], but I
> guess most users don't have such high demands. And there's no point in
> removing a few lines of duplicate code if the necessary machinery adds a
> lot of complexity. Though the code discussed so far was not too crazy
> IMHO.
>
> [1] https://gitlab.com/gitlab-org/gitaly/-/blob/master/internal/git/gitpipe/catfile_info.go
>
>> OK, so we managed another 1%. But I'm skeptical that this linear opcode
>> technique is where we want to go in the long run, if we're ever going to
>> unify formatters.
>
> Agreed.
>
> René
Obviously I agree with the conclusion, but it was fun to watch cute
experiments from the sidelines. Thanks for entertainment ;-)
^ permalink raw reply
* [PATCH v2 0/7] Introduce fetch.followRemoteHEAD config variable
From: Matt Hunter @ 2026-06-16 22:25 UTC (permalink / raw)
To: git; +Cc: Bence Ferdinandy, Jeff King, Junio C Hamano
In-Reply-To: <20260612055947.1499497-1-m@lfurio.us>
git-fetch presently offers some useful ways to control how remote HEAD
symbolic-refs are (or aren't) updated when fetching from remote
repositories. Namely this is done via the
'remote.<name>.followRemoteHEAD' configuration variable.
However, this setting can be somewhat painful to use if you prefer a
default other than "create" and often work with multiple different
remote repositories.
This series introduces the variable 'fetch.followRemoteHEAD', which
provides a configurable default in place of per-remote settings.
'fetch.followRemoteHEAD' functions exactly the same as the original
variable, except that it doesn't allow warning suppression via
'warn-if-not-$branch'. Given that different remotes will vary their
HEAD and set of branches independently, setting a false-positive
globally in this way doesn't make logical sense.
While it is not mentioned by any of the patches in this series, note
also that the behavior introduced by 012bc566bad7 (remote set-head: set
followRemoteHEAD to "warn" if "always") is unaffected by this series,
and this feature continues to work for only the
'remote.<name>.followRemoteHEAD' variable.
---
Changes in v2:
- Don't die() if the value of fetch.followRemoteHEAD is unrecognized.
- Use case-sensitive matching for fetch.followRemoteHEAD values.
- Avoid the phrase "configuration option".
- Minor documentation wording changes.
- Link to v1: https://patch.msgid.link/20260612055947.1499497-1-m@lfurio.us
Matt Hunter (7):
fetch: fixup set_head advice for warn-if-not-branch
doc: explain fetchRemoteHEADWarn advice
t5510: cleanup remote in followRemoteHEAD dangling ref test
fetch: rename function report_set_head
fetch: refactor do_fetch handling of followRemoteHEAD
fetch: add configuration variable fetch.followRemoteHEAD
fetch: fixup a misaligned comment
Documentation/config/advice.adoc | 4 ++
Documentation/config/fetch.adoc | 19 ++++++
Documentation/config/remote.adoc | 21 +++---
builtin/fetch.c | 49 ++++++++++----
remote.h | 14 ++--
t/t5510-fetch.sh | 106 +++++++++++++++++++++++++++++++
6 files changed, 183 insertions(+), 30 deletions(-)
Range-diff against v1:
1: 779fb9bfc59f = 1: 2106228f7b98 fetch: fixup set_head advice for warn-if-not-branch
2: aacc2856bc77 = 2: b1c58c06e0c7 doc: explain fetchRemoteHEADWarn advice
3: f5272eaafbcc = 3: c1d11e8883e6 t5510: cleanup remote in followRemoteHEAD dangling ref test
4: 43a17027c13e = 4: 6306c8212fc0 fetch: rename function report_set_head
5: c719435d9675 ! 5: 3c7257094686 fetch: refactor do_fetch handling of followRemoteHEAD
@@ Commit message
Update enum follow_remote_head_settings to include the value
FOLLOW_REMOTE_UNCONFIGURED as the new zero-initialized value for
- followRemoteHEAD. This will allow us to distinguish between the option
- being unset vs. explicitly set to 'create', which is ultimately the
- system default. The unnecessary indentation is removed.
+ followRemoteHEAD. This will allow us to distinguish between the
+ variable being unset vs. explicitly set to 'create', which is ultimately
+ the system default. The unnecessary indentation is removed.
The do_fetch function is likewise updated to perform its own decision
making to determine the effective followRemoteHEAD mode, falling back to
the system default if necessary. This will enable the next patch to
- introduce a user-configurable fallback default option.
+ introduce a user-configurable default.
- Function set_head now accepts this value as an argument rather than only
+ Function set_head now accepts the mode as an argument rather than only
considering the value defined by the remote.
The use of the 'warn-if-not-$branch' value is awkward in the context of
- a global default option, since the branches will differ between
- individual remotes. For this reason, it's left out of this scheme and
- handling of the no_warn_branch variable is untouched. Since a
- remote-specific setting for followRemoteHEAD takes priority, we can
- assume that if remote->no_warn_branch is set, then the remote is also
- asserting FOLLOW_REMOTE_WARN as the effective operating mode, and it
- will be honored by do_fetch.
+ a global default, since the branches will differ between individual
+ remotes. For this reason, it's left out of this scheme and handling of
+ the no_warn_branch variable is untouched. Since a remote-specific
+ value for followRemoteHEAD takes priority, we can assume that if
+ remote->no_warn_branch is set, then the remote is also asserting
+ FOLLOW_REMOTE_WARN as the effective operating mode, and it will be
+ honored by do_fetch.
Signed-off-by: Matt Hunter <m@lfurio.us>
6: 56f6fc8ded2d ! 6: af9f99b1ceb2 fetch: add configuration option fetch.followRemoteHEAD
@@ Metadata
Author: Matt Hunter <m@lfurio.us>
## Commit message ##
- fetch: add configuration option fetch.followRemoteHEAD
+ fetch: add configuration variable fetch.followRemoteHEAD
- 'fetch.followRemoteHEAD' is added as a generic option used by all
+ 'fetch.followRemoteHEAD' is added as a generic setting used by all
remotes for which 'remote.<name>.followRemoteHEAD' is undefined. If
- both options are undefined, a builtin default of "create" is in effect,
- matching the previous behavior.
+ both variables are undefined, a builtin default of "create" is in
+ effect, matching the previous behavior.
As mentioned in the previous patch, 'fetch.followRemoteHEAD' supports
all of the values that its 'remote' counterpart does _except_
@@ Commit message
repositories.
Documentation and advice messages for both of the followRemoteHEAD
- options are reworded to better capture the relationship between the two.
+ variables are reworded to better capture the relationship between the
+ two.
The added tests assert feature parity between the two followRemoteHEAD
- options, as well as the fact that 'remote.<name>.followRemoteHEAD'
+ variables, as well as the fact that 'remote.<name>.followRemoteHEAD'
always supersedes this new configurable default.
+ Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Matt Hunter <m@lfurio.us>
## Documentation/config/fetch.adoc ##
@@ Documentation/config/fetch.adoc: the new bundle URI.
remove the value for the `fetch.bundleCreationToken` value before fetching.
+
+`fetch.followRemoteHEAD`::
-+ When fetching using a default refspec, this option determines how to handle
++ When fetching using a default refspec, this setting determines how to handle
+ differences between a fetched remote's `HEAD` and the local
+ `remotes/<name>/HEAD` symbolic-ref. Its value is one of
++
@@ Documentation/config/remote.adoc: Blank values signal to ignore all previous val
- Setting it to "always" will silently update `remotes/<name>/HEAD` to
- the value on the remote. Finally, setting it to "never" will never
- change or create the local reference.
-+ When fetching this remote using its default refspec, this option determines
++ When fetching this remote using its default refspec, this setting determines
+ how to handle differences between the remote's `HEAD` and the local
-+ `remotes/<name>/HEAD` symbolic-ref. Overrides the setting for
++ `remotes/<name>/HEAD` symbolic-ref. Overrides the value of
+ `fetch.followRemoteHEAD`. See `fetch.followRemoteHEAD` for a description of
+ accepted values.
++
-+In addition to the values supported by `fetch.followRemoteHEAD`, this option may
-+also take on the value "warn-if-not-`$branch`", which behaves like "warn", but
-+ignores the warning if the remote's `HEAD` is `remotes/<name>/$branch`.
++In addition to the values supported by `fetch.followRemoteHEAD`, this setting
++may also take on the value "warn-if-not-`$branch`", which behaves like "warn",
++but ignores the warning if the remote's `HEAD` is `remotes/<name>/$branch`.
## builtin/fetch.c ##
@@ builtin/fetch.c: static struct string_list negotiation_include = STRING_LIST_INIT_NODUP;
@@ builtin/fetch.c: static int git_fetch_config(const char *k, const char *v,
+ if (!strcmp(k, "fetch.followremotehead")) {
+ if (!v)
+ return config_error_nonbool(k);
-+ else if (!strcasecmp(v, "never"))
++ else if (!strcmp(v, "never"))
+ fetch_config->follow_remote_head = FOLLOW_REMOTE_NEVER;
-+ else if (!strcasecmp(v, "create"))
++ else if (!strcmp(v, "create"))
+ fetch_config->follow_remote_head = FOLLOW_REMOTE_CREATE;
-+ else if (!strcasecmp(v, "warn"))
++ else if (!strcmp(v, "warn"))
+ fetch_config->follow_remote_head = FOLLOW_REMOTE_WARN;
-+ else if (!strcasecmp(v, "always"))
++ else if (!strcmp(v, "always"))
+ fetch_config->follow_remote_head = FOLLOW_REMOTE_ALWAYS;
-+ else
-+ die(_("invalid value for '%s': '%s'"),
-+ "fetch.followRemoteHEAD", v);
+ }
+
return git_default_config(k, v, ctx, cb);
@@ builtin/fetch.c: static const char *strip_refshead(const char *name){
- "will disable the warning until the remote changes HEAD to something else.");
+ N_("Run 'git remote set-head %s %s' to follow the change, or modify\n"
+ "either of the 'remote.%s.followRemoteHEAD' or 'fetch.followRemoteHEAD'\n"
-+ "configuration options to handle the situation differently.\n\n"
++ "configuration variables to handle the situation differently.\n\n"
+
-+ "Using this specific option\n\n"
++ "Using this specific setting\n\n"
+ " git config set remote.%s.followRemoteHEAD warn-if-not-%s\n\n"
+ "will suppress the warning until the remote changes HEAD to something else.");
7: 5e0bdd0f00b4 = 7: 5c80107f6488 fetch: fixup a misaligned comment
base-commit: 0fae78c9d55efe705877ea537fe42c59164ccd94
--
2.54.0
^ permalink raw reply
* [PATCH v2 1/7] fetch: fixup set_head advice for warn-if-not-branch
From: Matt Hunter @ 2026-06-16 22:25 UTC (permalink / raw)
To: git; +Cc: Bence Ferdinandy, Jeff King, Junio C Hamano
In-Reply-To: <20260616222606.1003521-1-m@lfurio.us>
Specifying the word 'branch' in the command is not correct - a mismatch
with both the implementation in remote.c and the documentation.
Signed-off-by: Matt Hunter <m@lfurio.us>
---
builtin/fetch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index c1d7c672f4e0..82969e230f5a 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1700,7 +1700,7 @@ static void set_head_advice_msg(const char *remote, const char *head_name)
N_("Run 'git remote set-head %s %s' to follow the change, or set\n"
"'remote.%s.followRemoteHEAD' configuration option to a different value\n"
"if you do not want to see this message. Specifically running\n"
- "'git config set remote.%s.followRemoteHEAD warn-if-not-branch-%s'\n"
+ "'git config set remote.%s.followRemoteHEAD warn-if-not-%s'\n"
"will disable the warning until the remote changes HEAD to something else.");
advise_if_enabled(ADVICE_FETCH_SET_HEAD_WARN, _(message_advice_set_head),
--
2.54.0
^ permalink raw reply related
* [PATCH v2 2/7] doc: explain fetchRemoteHEADWarn advice
From: Matt Hunter @ 2026-06-16 22:25 UTC (permalink / raw)
To: git; +Cc: Bence Ferdinandy, Jeff King, Junio C Hamano
In-Reply-To: <20260616222606.1003521-1-m@lfurio.us>
When the user sets 'remote.<name>.followRemoteHEAD' to
'warn[-if-not-$branch]', git-fetch will report when a fetched HEAD
disagrees with the locally-configured remote's HEAD. This additional
advice instructs the user how to deal with these warnings, but was
previously undocumented in git-config.
Signed-off-by: Matt Hunter <m@lfurio.us>
---
Documentation/config/advice.adoc | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/config/advice.adoc b/Documentation/config/advice.adoc
index 257db5891817..c3c190ba6a4f 100644
--- a/Documentation/config/advice.adoc
+++ b/Documentation/config/advice.adoc
@@ -48,6 +48,10 @@ all advice messages.
to create a local branch after the fact.
diverging::
Shown when a fast-forward is not possible.
+ fetchRemoteHEADWarn::
+ Shown when linkgit:git-fetch[1] reveals that a remote `HEAD`
+ differs from what is set locally and the user has opted into
+ receiving a warning in this situation.
fetchShowForcedUpdates::
Shown when linkgit:git-fetch[1] takes a long time
to calculate forced updates after ref updates, or to warn
--
2.54.0
^ permalink raw reply related
* [PATCH v2 3/7] t5510: cleanup remote in followRemoteHEAD dangling ref test
From: Matt Hunter @ 2026-06-16 22:25 UTC (permalink / raw)
To: git; +Cc: Bence Ferdinandy, Jeff King, Junio C Hamano
In-Reply-To: <20260616222606.1003521-1-m@lfurio.us>
A later patch will introduce a new test which closely mirrors this one.
Update this test to remove the 'custom-head' remote it creates.
Otherwise, the two tests will conflict with each other, as the second
one to execute will fail to create this remote (which already exists,
thanks to the first test).
Signed-off-by: Matt Hunter <m@lfurio.us>
---
t/t5510-fetch.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index eca9a973b5cb..43190630e714 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -251,6 +251,7 @@ test_expect_success 'followRemoteHEAD does not kick in with refspecs' '
'
test_expect_success 'followRemoteHEAD create does not overwrite dangling symref' '
+ test_when_finished "git -C two remote remove custom-head" &&
git -C two remote add -m does-not-exist custom-head ../one &&
test_config -C two remote.custom-head.followRemoteHEAD create &&
git -C two fetch custom-head &&
--
2.54.0
^ permalink raw reply related
* [PATCH v2 4/7] fetch: rename function report_set_head
From: Matt Hunter @ 2026-06-16 22:25 UTC (permalink / raw)
To: git; +Cc: Bence Ferdinandy, Jeff King, Junio C Hamano
In-Reply-To: <20260616222606.1003521-1-m@lfurio.us>
Update to the slightly more obvious name 'warn_set_head', which matches
the verbiage of the followRemoteHEAD options.
Signed-off-by: Matt Hunter <m@lfurio.us>
---
builtin/fetch.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 82969e230f5a..9a45e1e7a44d 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1707,7 +1707,7 @@ static void set_head_advice_msg(const char *remote, const char *head_name)
remote, head_name, remote, remote, head_name);
}
-static void report_set_head(const char *remote, const char *head_name,
+static void warn_set_head(const char *remote, const char *head_name,
struct strbuf *buf_prev, int updateres) {
struct strbuf buf_prefix = STRBUF_INIT;
const char *prev_head = NULL;
@@ -1787,7 +1787,7 @@ static int set_head(const struct ref *remote_refs, struct remote *remote)
if (verbosity >= 0 &&
follow_remote_head == FOLLOW_REMOTE_WARN &&
(!no_warn_branch || strcmp(no_warn_branch, head_name)))
- report_set_head(remote->name, head_name, &b_local_head, was_detached);
+ warn_set_head(remote->name, head_name, &b_local_head, was_detached);
cleanup:
free(head_name);
--
2.54.0
^ permalink raw reply related
* [PATCH v2 5/7] fetch: refactor do_fetch handling of followRemoteHEAD
From: Matt Hunter @ 2026-06-16 22:25 UTC (permalink / raw)
To: git; +Cc: Bence Ferdinandy, Jeff King, Junio C Hamano
In-Reply-To: <20260616222606.1003521-1-m@lfurio.us>
Update enum follow_remote_head_settings to include the value
FOLLOW_REMOTE_UNCONFIGURED as the new zero-initialized value for
followRemoteHEAD. This will allow us to distinguish between the
variable being unset vs. explicitly set to 'create', which is ultimately
the system default. The unnecessary indentation is removed.
The do_fetch function is likewise updated to perform its own decision
making to determine the effective followRemoteHEAD mode, falling back to
the system default if necessary. This will enable the next patch to
introduce a user-configurable default.
Function set_head now accepts the mode as an argument rather than only
considering the value defined by the remote.
The use of the 'warn-if-not-$branch' value is awkward in the context of
a global default, since the branches will differ between individual
remotes. For this reason, it's left out of this scheme and handling of
the no_warn_branch variable is untouched. Since a remote-specific
value for followRemoteHEAD takes priority, we can assume that if
remote->no_warn_branch is set, then the remote is also asserting
FOLLOW_REMOTE_WARN as the effective operating mode, and it will be
honored by do_fetch.
Signed-off-by: Matt Hunter <m@lfurio.us>
---
builtin/fetch.c | 14 ++++++++++----
remote.h | 14 ++++++++------
2 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 9a45e1e7a44d..3cc7efdd83a0 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1729,12 +1729,12 @@ static void warn_set_head(const char *remote, const char *head_name,
strbuf_release(&buf_prefix);
}
-static int set_head(const struct ref *remote_refs, struct remote *remote)
+static int set_head(const struct ref *remote_refs, struct remote *remote,
+ int follow_remote_head)
{
int result = 0, create_only, baremirror, was_detached;
struct strbuf b_head = STRBUF_INIT, b_remote_head = STRBUF_INIT,
b_local_head = STRBUF_INIT;
- int follow_remote_head = remote->follow_remote_head;
const char *no_warn_branch = remote->no_warn_branch;
char *head_name = NULL;
struct ref *ref, *matches;
@@ -1901,6 +1901,7 @@ static int do_fetch(struct transport *transport,
struct ref_update_display_info_array display_array = { 0 };
struct strmap rejected_refs = STRMAP_INIT;
int summary_width = 0;
+ int follow_remote_head;
if (tags == TAGS_DEFAULT) {
if (transport->remote->fetch_tags == 2)
@@ -1916,6 +1917,11 @@ static int do_fetch(struct transport *transport,
goto cleanup;
}
+ if (transport->remote->follow_remote_head)
+ follow_remote_head = transport->remote->follow_remote_head;
+ else
+ follow_remote_head = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;
+
if (rs->nr) {
refspec_ref_prefixes(rs, &transport_ls_refs_options.ref_prefixes);
} else {
@@ -1924,7 +1930,7 @@ static int do_fetch(struct transport *transport,
if (transport->remote->fetch.nr) {
refspec_ref_prefixes(&transport->remote->fetch,
&transport_ls_refs_options.ref_prefixes);
- if (transport->remote->follow_remote_head != FOLLOW_REMOTE_NEVER)
+ if (follow_remote_head != FOLLOW_REMOTE_NEVER)
do_set_head = 1;
}
if (branch && branch_has_merge_config(branch) &&
@@ -2131,7 +2137,7 @@ static int do_fetch(struct transport *transport,
* Way too many cases where this can go wrong so let's just
* ignore errors and fail silently for now.
*/
- set_head(remote_refs, transport->remote);
+ set_head(remote_refs, transport->remote, follow_remote_head);
}
cleanup:
diff --git a/remote.h b/remote.h
index 54b17e4b028b..72a54d84ad51 100644
--- a/remote.h
+++ b/remote.h
@@ -62,12 +62,14 @@ struct remote_state {
void remote_state_clear(struct remote_state *remote_state);
struct remote_state *remote_state_new(void);
- enum follow_remote_head_settings {
- FOLLOW_REMOTE_NEVER = -1,
- FOLLOW_REMOTE_CREATE = 0,
- FOLLOW_REMOTE_WARN = 1,
- FOLLOW_REMOTE_ALWAYS = 2,
- };
+#define BUILTIN_FOLLOW_REMOTE_HEAD_DFLT FOLLOW_REMOTE_CREATE
+enum follow_remote_head_settings {
+ FOLLOW_REMOTE_UNCONFIGURED = 0,
+ FOLLOW_REMOTE_NEVER,
+ FOLLOW_REMOTE_CREATE,
+ FOLLOW_REMOTE_WARN,
+ FOLLOW_REMOTE_ALWAYS,
+};
struct remote {
struct hashmap_entry ent;
--
2.54.0
^ permalink raw reply related
* [PATCH v2 6/7] fetch: add configuration variable fetch.followRemoteHEAD
From: Matt Hunter @ 2026-06-16 22:25 UTC (permalink / raw)
To: git; +Cc: Bence Ferdinandy, Jeff King, Junio C Hamano
In-Reply-To: <20260616222606.1003521-1-m@lfurio.us>
'fetch.followRemoteHEAD' is added as a generic setting used by all
remotes for which 'remote.<name>.followRemoteHEAD' is undefined. If
both variables are undefined, a builtin default of "create" is in
effect, matching the previous behavior.
As mentioned in the previous patch, 'fetch.followRemoteHEAD' supports
all of the values that its 'remote' counterpart does _except_
warn-if-not-$branch, due to its tighter coupling to individual remote
repositories.
Documentation and advice messages for both of the followRemoteHEAD
variables are reworded to better capture the relationship between the
two.
The added tests assert feature parity between the two followRemoteHEAD
variables, as well as the fact that 'remote.<name>.followRemoteHEAD'
always supersedes this new configurable default.
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Matt Hunter <m@lfurio.us>
---
Documentation/config/fetch.adoc | 19 ++++++
Documentation/config/remote.adoc | 21 +++----
builtin/fetch.c | 29 +++++++--
t/t5510-fetch.sh | 105 +++++++++++++++++++++++++++++++
4 files changed, 157 insertions(+), 17 deletions(-)
diff --git a/Documentation/config/fetch.adoc b/Documentation/config/fetch.adoc
index 04ac90912d3a..00435e9a16d9 100644
--- a/Documentation/config/fetch.adoc
+++ b/Documentation/config/fetch.adoc
@@ -126,3 +126,22 @@ the new bundle URI.
The creation token values are chosen by the provider serving the specific
bundle URI. If you modify the URI at `fetch.bundleURI`, then be sure to
remove the value for the `fetch.bundleCreationToken` value before fetching.
+
+`fetch.followRemoteHEAD`::
+ When fetching using a default refspec, this setting determines how to handle
+ differences between a fetched remote's `HEAD` and the local
+ `remotes/<name>/HEAD` symbolic-ref. Its value is one of
++
+--
+`create`;;
+ Create `remotes/<name>/HEAD` if a ref exists on the remote, but not locally.
+ An existing symbolic-ref will not be touched. This is the default value.
+`warn`;;
+ Display a warning if the remote advertises a different `HEAD` than what is
+ set locally. Behaves like "create" if the local symbolic-ref doesn't exist.
+`always`;;
+ Silently update `remotes/<name>/HEAD` whenever the remote advertises a new
+ value.
+`never`;;
+ Never create or modify the `remotes/<name>/HEAD` symbolic-ref.
+--
diff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc
index eb9c8a3c4884..04724bc51628 100644
--- a/Documentation/config/remote.adoc
+++ b/Documentation/config/remote.adoc
@@ -157,15 +157,12 @@ Blank values signal to ignore all previous values, allowing a reset of
the list from broader config scenarios.
remote.<name>.followRemoteHEAD::
- How linkgit:git-fetch[1] should handle updates to `remotes/<name>/HEAD`
- when fetching using the configured refspecs of a remote.
- The default value is "create", which will create `remotes/<name>/HEAD`
- if it exists on the remote, but not locally; this will not touch an
- already existing local reference. Setting it to "warn" will print
- a message if the remote has a different value than the local one;
- in case there is no local reference, it behaves like "create".
- A variant on "warn" is "warn-if-not-$branch", which behaves like
- "warn", but if `HEAD` on the remote is `$branch` it will be silent.
- Setting it to "always" will silently update `remotes/<name>/HEAD` to
- the value on the remote. Finally, setting it to "never" will never
- change or create the local reference.
+ When fetching this remote using its default refspec, this setting determines
+ how to handle differences between the remote's `HEAD` and the local
+ `remotes/<name>/HEAD` symbolic-ref. Overrides the value of
+ `fetch.followRemoteHEAD`. See `fetch.followRemoteHEAD` for a description of
+ accepted values.
++
+In addition to the values supported by `fetch.followRemoteHEAD`, this setting
+may also take on the value "warn-if-not-`$branch`", which behaves like "warn",
+but ignores the warning if the remote's `HEAD` is `remotes/<name>/$branch`.
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 3cc7efdd83a0..1375fc4e0547 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -103,6 +103,7 @@ static struct string_list negotiation_include = STRING_LIST_INIT_NODUP;
struct fetch_config {
enum display_format display_format;
+ enum follow_remote_head_settings follow_remote_head;
int all;
int prune;
int prune_tags;
@@ -173,6 +174,19 @@ static int git_fetch_config(const char *k, const char *v,
"fetch.output", v);
}
+ if (!strcmp(k, "fetch.followremotehead")) {
+ if (!v)
+ return config_error_nonbool(k);
+ else if (!strcmp(v, "never"))
+ fetch_config->follow_remote_head = FOLLOW_REMOTE_NEVER;
+ else if (!strcmp(v, "create"))
+ fetch_config->follow_remote_head = FOLLOW_REMOTE_CREATE;
+ else if (!strcmp(v, "warn"))
+ fetch_config->follow_remote_head = FOLLOW_REMOTE_WARN;
+ else if (!strcmp(v, "always"))
+ fetch_config->follow_remote_head = FOLLOW_REMOTE_ALWAYS;
+ }
+
return git_default_config(k, v, ctx, cb);
}
@@ -1697,11 +1711,13 @@ static const char *strip_refshead(const char *name){
static void set_head_advice_msg(const char *remote, const char *head_name)
{
const char message_advice_set_head[] =
- N_("Run 'git remote set-head %s %s' to follow the change, or set\n"
- "'remote.%s.followRemoteHEAD' configuration option to a different value\n"
- "if you do not want to see this message. Specifically running\n"
- "'git config set remote.%s.followRemoteHEAD warn-if-not-%s'\n"
- "will disable the warning until the remote changes HEAD to something else.");
+ N_("Run 'git remote set-head %s %s' to follow the change, or modify\n"
+ "either of the 'remote.%s.followRemoteHEAD' or 'fetch.followRemoteHEAD'\n"
+ "configuration variables to handle the situation differently.\n\n"
+
+ "Using this specific setting\n\n"
+ " git config set remote.%s.followRemoteHEAD warn-if-not-%s\n\n"
+ "will suppress the warning until the remote changes HEAD to something else.");
advise_if_enabled(ADVICE_FETCH_SET_HEAD_WARN, _(message_advice_set_head),
remote, head_name, remote, remote, head_name);
@@ -1919,6 +1935,8 @@ static int do_fetch(struct transport *transport,
if (transport->remote->follow_remote_head)
follow_remote_head = transport->remote->follow_remote_head;
+ else if (config->follow_remote_head)
+ follow_remote_head = config->follow_remote_head;
else
follow_remote_head = BUILTIN_FOLLOW_REMOTE_HEAD_DFLT;
@@ -2477,6 +2495,7 @@ int cmd_fetch(int argc,
{
struct fetch_config config = {
.display_format = DISPLAY_FORMAT_FULL,
+ .follow_remote_head = FOLLOW_REMOTE_UNCONFIGURED,
.prune = -1,
.prune_tags = -1,
.show_forced_updates = 1,
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 43190630e714..6f0ae1bdd798 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -140,6 +140,16 @@ test_expect_success "fetch test remote HEAD change" '
)
'
+test_expect_success "fetch test default followRemoteHEAD never" '
+ git -C two update-ref --no-deref -d refs/remotes/origin/HEAD &&
+ test_config -C two fetch.followRemoteHEAD "never" &&
+ GIT_TRACE_PACKET=$PWD/trace.out git -C two fetch &&
+ # Confirm that we do not even ask for HEAD when we are
+ # not going to act on it.
+ test_grep ! "ref-prefix HEAD" trace.out &&
+ test_must_fail git -C two rev-parse --verify refs/remotes/origin/HEAD
+'
+
test_expect_success "fetch test followRemoteHEAD never" '
git -C two update-ref --no-deref -d refs/remotes/origin/HEAD &&
test_config -C two remote.origin.followRemoteHEAD "never" &&
@@ -150,6 +160,21 @@ test_expect_success "fetch test followRemoteHEAD never" '
test_must_fail git -C two rev-parse --verify refs/remotes/origin/HEAD
'
+test_expect_success "fetch test default followRemoteHEAD warn no change" '
+ git -C two rev-parse --verify refs/remotes/origin/other &&
+ git -C two remote set-head origin other &&
+ git -C two rev-parse --verify refs/remotes/origin/HEAD &&
+ git -C two rev-parse --verify refs/remotes/origin/main &&
+ test_config -C two fetch.followRemoteHEAD "warn" &&
+ git -C two fetch >output &&
+ echo "${SQ}HEAD${SQ} at ${SQ}origin${SQ} is ${SQ}main${SQ}," \
+ "but we have ${SQ}other${SQ} locally." >expect &&
+ test_cmp expect output &&
+ head=$(git -C two rev-parse refs/remotes/origin/HEAD) &&
+ branch=$(git -C two rev-parse refs/remotes/origin/other) &&
+ test "z$head" = "z$branch"
+'
+
test_expect_success "fetch test followRemoteHEAD warn no change" '
git -C two rev-parse --verify refs/remotes/origin/other &&
git -C two remote set-head origin other &&
@@ -165,6 +190,17 @@ test_expect_success "fetch test followRemoteHEAD warn no change" '
test "z$head" = "z$branch"
'
+test_expect_success "fetch test default followRemoteHEAD warn create" '
+ git -C two update-ref --no-deref -d refs/remotes/origin/HEAD &&
+ test_config -C two fetch.followRemoteHEAD "warn" &&
+ git -C two rev-parse --verify refs/remotes/origin/main &&
+ output=$(git -C two fetch) &&
+ test "z" = "z$output" &&
+ head=$(git -C two rev-parse refs/remotes/origin/HEAD) &&
+ branch=$(git -C two rev-parse refs/remotes/origin/main) &&
+ test "z$head" = "z$branch"
+'
+
test_expect_success "fetch test followRemoteHEAD warn create" '
git -C two update-ref --no-deref -d refs/remotes/origin/HEAD &&
test_config -C two remote.origin.followRemoteHEAD "warn" &&
@@ -176,6 +212,18 @@ test_expect_success "fetch test followRemoteHEAD warn create" '
test "z$head" = "z$branch"
'
+test_expect_success "fetch test default followRemoteHEAD warn detached" '
+ git -C two update-ref --no-deref -d refs/remotes/origin/HEAD &&
+ git -C two update-ref refs/remotes/origin/HEAD HEAD &&
+ HEAD=$(git -C two log --pretty="%H") &&
+ test_config -C two fetch.followRemoteHEAD "warn" &&
+ git -C two fetch >output &&
+ echo "${SQ}HEAD${SQ} at ${SQ}origin${SQ} is ${SQ}main${SQ}," \
+ "but we have a detached HEAD pointing to" \
+ "${SQ}${HEAD}${SQ} locally." >expect &&
+ test_cmp expect output
+'
+
test_expect_success "fetch test followRemoteHEAD warn detached" '
git -C two update-ref --no-deref -d refs/remotes/origin/HEAD &&
git -C two update-ref refs/remotes/origin/HEAD HEAD &&
@@ -188,6 +236,19 @@ test_expect_success "fetch test followRemoteHEAD warn detached" '
test_cmp expect output
'
+test_expect_success "fetch test default followRemoteHEAD warn quiet" '
+ git -C two rev-parse --verify refs/remotes/origin/other &&
+ git -C two remote set-head origin other &&
+ git -C two rev-parse --verify refs/remotes/origin/HEAD &&
+ git -C two rev-parse --verify refs/remotes/origin/main &&
+ test_config -C two fetch.followRemoteHEAD "warn" &&
+ output=$(git -C two fetch --quiet) &&
+ test "z" = "z$output" &&
+ head=$(git -C two rev-parse refs/remotes/origin/HEAD) &&
+ branch=$(git -C two rev-parse refs/remotes/origin/other) &&
+ test "z$head" = "z$branch"
+'
+
test_expect_success "fetch test followRemoteHEAD warn quiet" '
git -C two rev-parse --verify refs/remotes/origin/other &&
git -C two remote set-head origin other &&
@@ -229,6 +290,18 @@ test_expect_success "fetch test followRemoteHEAD warn-if-not-branch branch is di
test "z$head" = "z$branch"
'
+test_expect_success "fetch test default followRemoteHEAD always" '
+ git -C two rev-parse --verify refs/remotes/origin/other &&
+ git -C two remote set-head origin other &&
+ git -C two rev-parse --verify refs/remotes/origin/HEAD &&
+ git -C two rev-parse --verify refs/remotes/origin/main &&
+ test_config -C two fetch.followRemoteHEAD "always" &&
+ git -C two fetch &&
+ head=$(git -C two rev-parse refs/remotes/origin/HEAD) &&
+ branch=$(git -C two rev-parse refs/remotes/origin/main) &&
+ test "z$head" = "z$branch"
+'
+
test_expect_success "fetch test followRemoteHEAD always" '
git -C two rev-parse --verify refs/remotes/origin/other &&
git -C two remote set-head origin other &&
@@ -241,6 +314,28 @@ test_expect_success "fetch test followRemoteHEAD always" '
test "z$head" = "z$branch"
'
+test_expect_success 'per-remote followRemoteHEAD takes priority over fetch default' '
+ git -C two rev-parse --verify refs/remotes/origin/other &&
+ git -C two remote set-head origin other &&
+ git -C two rev-parse --verify refs/remotes/origin/HEAD &&
+ git -C two rev-parse --verify refs/remotes/origin/main &&
+ test_config -C two fetch.followRemoteHEAD "never" &&
+ test_config -C two remote.origin.followRemoteHEAD "always" &&
+ git -C two fetch &&
+ head=$(git -C two rev-parse refs/remotes/origin/HEAD) &&
+ branch=$(git -C two rev-parse refs/remotes/origin/main) &&
+ test "z$head" = "z$branch"
+'
+
+test_expect_success 'default followRemoteHEAD does not kick in with refspecs' '
+ git -C two remote set-head origin other &&
+ test_config -C two fetch.followRemoteHEAD always &&
+ git -C two fetch origin refs/heads/main:refs/remotes/origin/main &&
+ echo refs/remotes/origin/other >expect &&
+ git -C two symbolic-ref refs/remotes/origin/HEAD >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'followRemoteHEAD does not kick in with refspecs' '
git -C two remote set-head origin other &&
test_config -C two remote.origin.followRemoteHEAD always &&
@@ -250,6 +345,16 @@ test_expect_success 'followRemoteHEAD does not kick in with refspecs' '
test_cmp expect actual
'
+test_expect_success 'default followRemoteHEAD create does not overwrite dangling symref' '
+ test_when_finished "git -C two remote remove custom-head" &&
+ git -C two remote add -m does-not-exist custom-head ../one &&
+ test_config -C two fetch.followRemoteHEAD create &&
+ git -C two fetch custom-head &&
+ echo refs/remotes/custom-head/does-not-exist >expect &&
+ git -C two symbolic-ref refs/remotes/custom-head/HEAD >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'followRemoteHEAD create does not overwrite dangling symref' '
test_when_finished "git -C two remote remove custom-head" &&
git -C two remote add -m does-not-exist custom-head ../one &&
--
2.54.0
^ permalink raw reply related
* [PATCH v2 7/7] fetch: fixup a misaligned comment
From: Matt Hunter @ 2026-06-16 22:25 UTC (permalink / raw)
To: git; +Cc: Bence Ferdinandy, Jeff King, Junio C Hamano
In-Reply-To: <20260616222606.1003521-1-m@lfurio.us>
Signed-off-by: Matt Hunter <m@lfurio.us>
---
builtin/fetch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 1375fc4e0547..d942bf6aa029 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1789,7 +1789,7 @@ static int set_head(const struct ref *remote_refs, struct remote *remote,
strbuf_addf(&b_head, "refs/remotes/%s/HEAD", remote->name);
strbuf_addf(&b_remote_head, "refs/remotes/%s/%s", remote->name, head_name);
}
- /* make sure it's valid */
+ /* make sure it's valid */
if (!baremirror && !refs_ref_exists(refs, b_remote_head.buf)) {
result = 1;
goto cleanup;
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v2 16/17] midx: refactor interfaces to work on "packed" source
From: Justin Tobler @ 2026-06-16 22:37 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Karthik Nayak
In-Reply-To: <20260609-pks-odb-source-packed-v2-16-839089132c8b@pks.im>
On 26/06/09 10:51AM, Patrick Steinhardt wrote:
> Our interfaces used to interact with MIDXs all work on top of the
> generic `struct odb_source`. This doesn't make much sense though: a MIDX
> is strictly tied to the "packed" source, so passing in a generic source
> gives the false sense that it may also work with a different type of
> source.
A `struct odb_source_packed` now acts as the source specific to all
packfiles in the repository. Being that MIDXs only work with packfiles,
it also makes sense to me to use the appropriate concrete source here.
> Fix this conceptual weirdness and instead require the caller to pass in
> a "packed" source explicitly. This also makes the next commit easier to
> implement, where we drop the pointer to the "files" source in the
> "packed" source.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> builtin/multi-pack-index.c | 29 +++++------
> builtin/pack-objects.c | 3 +-
> builtin/repack.c | 8 ++-
> midx-write.c | 34 ++++++-------
> midx.c | 118 ++++++++++++++++++++++-----------------------
> midx.h | 30 ++++++------
> odb/source-packed.c | 12 ++---
> pack-bitmap.c | 8 +--
> pack-revindex.c | 6 +--
> repack-geometry.c | 3 +-
> repack-midx.c | 9 ++--
> repack.c | 6 +--
> t/helper/test-read-midx.c | 7 ++-
> 13 files changed, 144 insertions(+), 129 deletions(-)
>
> diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c
> index 00ffb36394..6e73c85cde 100644
> --- a/builtin/multi-pack-index.c
> +++ b/builtin/multi-pack-index.c
> @@ -10,6 +10,7 @@
> #include "trace2.h"
> #include "odb.h"
> #include "odb/source.h"
> +#include "odb/source-files.h"
> #include "replace-object.h"
> #include "repository.h"
>
> @@ -85,12 +86,12 @@ static int parse_object_dir(const struct option *opt, const char *arg,
> return 0;
> }
>
> -static struct odb_source *handle_object_dir_option(struct repository *repo)
> +static struct odb_source_files *handle_object_dir_option(struct repository *repo)
> {
> struct odb_source *source = odb_find_source(repo->objects, opts.object_dir);
> if (!source)
> source = odb_add_to_alternates_memory(repo->objects, opts.object_dir);
> - return source;
> + return odb_source_files_downcast(source);
Now that we are passing around concrete ODB sources to many of these
interfaces, callers may have to handle downcasting explicitly. Since now
the downcasting may happen a bit ealier than previously in some cases, I
wondered if this would alter any error flows. I don't think this would
be the case though because ultimately we only have the "files" source
currently anyways.
The updates to the function signatures and call sites accordingly in
this patch all look good to me.
-Justin
^ permalink raw reply
* Re: [PATCH v2 17/17] odb/source-packed: drop pointer to "files" parent source
From: Justin Tobler @ 2026-06-16 22:51 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Karthik Nayak
In-Reply-To: <20260609-pks-odb-source-packed-v2-17-839089132c8b@pks.im>
On 26/06/09 10:51AM, Patrick Steinhardt wrote:
> Over the last commits we have turned the packfile store into a proper
> object database source that can be used as a standalone backend. As
> such, it is no longer necessary to have it coupled to the "files" parent
> source.
>
> Remove the pointer to the owning "files" source so that the "packed"
> source can be used as a standalone entity.
Makes sense.
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
[snip]
> @@ -626,7 +625,7 @@ static void prepare_pack(const char *full_name, size_t full_name_len,
> report_garbage(PACKDIR_FILE_GARBAGE, full_name);
> }
>
> -static void prepare_packed_git_one(struct odb_source *source)
> +static void prepare_packed_git_one(struct odb_source_packed *source)
At first I was a bit confused to see this change here, but IIUC
previously this function was passed the "base" source of the "files"
ODB. Now that we have a proper "packed" source we can use that directly
instead.
[snip]
> -struct odb_source_packed *odb_source_packed_new(struct odb_source_files *parent)
> +struct odb_source_packed *odb_source_packed_new(struct object_database *odb,
> + const char *path,
> + bool local)
Since we no longer depend on the parent source the function signature is
updated accordingly. This also matches the "loose" ODB source.
[snip]
> diff --git a/odb/source-packed.h b/odb/source-packed.h
> index 9d4796261a..88994098c1 100644
> --- a/odb/source-packed.h
> +++ b/odb/source-packed.h
> @@ -10,7 +10,6 @@
> */
> struct odb_source_packed {
> struct odb_source base;
> - struct odb_source_files *files;
Nice. :)
-Justin
^ permalink raw reply
* Re: [PATCH v2 0/7] Introduce fetch.followRemoteHEAD config variable
From: Junio C Hamano @ 2026-06-16 23:18 UTC (permalink / raw)
To: Matt Hunter; +Cc: git, Bence Ferdinandy, Jeff King
In-Reply-To: <20260616222606.1003521-1-m@lfurio.us>
Matt Hunter <m@lfurio.us> writes:
> Changes in v2:
> - Don't die() if the value of fetch.followRemoteHEAD is unrecognized.
> - Use case-sensitive matching for fetch.followRemoteHEAD values.
> - Avoid the phrase "configuration option".
> - Minor documentation wording changes.
> - Link to v1: https://patch.msgid.link/20260612055947.1499497-1-m@lfurio.us
> @@ builtin/fetch.c: static int git_fetch_config(const char *k, const char *v,
> + if (!strcmp(k, "fetch.followremotehead")) {
> + if (!v)
> + return config_error_nonbool(k);
> -+ else if (!strcasecmp(v, "never"))
> ++ else if (!strcmp(v, "never"))
> + fetch_config->follow_remote_head = FOLLOW_REMOTE_NEVER;
> -+ else if (!strcasecmp(v, "create"))
> ++ else if (!strcmp(v, "create"))
> + fetch_config->follow_remote_head = FOLLOW_REMOTE_CREATE;
> -+ else if (!strcasecmp(v, "warn"))
> ++ else if (!strcmp(v, "warn"))
> + fetch_config->follow_remote_head = FOLLOW_REMOTE_WARN;
> -+ else if (!strcasecmp(v, "always"))
> ++ else if (!strcmp(v, "always"))
> + fetch_config->follow_remote_head = FOLLOW_REMOTE_ALWAYS;
> -+ else
> -+ die(_("invalid value for '%s': '%s'"),
> -+ "fetch.followRemoteHEAD", v);
> + }
Not dying on an unrecognised value is certainly better than dying,
but shouldn't we at least clear fetch_config->follow_remote_head
to some "unspecified" or "default" value? What does the existing
parser routine for remote.*.followremotehead do?
Ideally,
(1) If the "fetch" operation ends up with not needing to consult
the value of fetch.followRemoteHEAD at all (e.g., it is a
one-shot fetch that updates no remote-tracking hierarchy, or it
has a more specific per-remote setting that this variable is
meant to serve as a mere fallback), any bogus or unknown value
will not get any warning.
(2) If fetch.followRemoteHEAD ends up being _used_, and if it has
an unknown value, we should at least warn "we do not understand
what you wrote, 'awlays', and we ignore it", or die "we do not
understand 'reset', perhaps it is from a future version of Git?".
I do not think customization based on git_config() callback like the
above can easily implement such an ideal semantics.
And I suspect that the existing per-remote configuration that this
variable is meant to serve as a fallback definition would not work
in such an ideal way (i.e., even if we are doing one-shot fetch that
does not touch any remote-tracking hierarchies, "git fetch" may warn
if the value is not understood, and when we do need the value, the
code would only warn and does not die), so in that sense this new
code is not making things _worse_, even though it may be spreading
the same badness more widely X-<.
Thanks.
^ permalink raw reply
* [PATCH v3 00/17] odb: make packed object source a proper `struct odb_source`
From: Patrick Steinhardt @ 2026-06-17 6:39 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Justin Tobler
In-Reply-To: <20260604-pks-odb-source-packed-v1-0-2e7ab31b4b5c@pks.im>
Hi,
this patch series converts the "packed" source into a proper `struct
odb_source`. It's thus the equivalent to [1], which did the same thing
for the "loose" source.
This series here is unfortunately a bit bigger, mostly because I'm also
renaming `struct packfile_store` to `struct odb_source_packed`. Back
when I introduced the packfile store I didn't yet have the full vision
of how the final layout will look like, so I didn't have the foresight
yet to call it `struct odb_source_packed`. But now that the layout has
materialized I think it's sensible to adjust its naming to match all the
other sources that we have.
Also: I don't have anything else in the pipeline anymore that moves
around large pieces of our code in the vicinity of the object database.
So after this series got merged, subsequent changes should be of a more
incremental nature.
This series is built on top of 9ac3f193c0 (The 11th batch, 2026-06-02)
with ps/odb-source-loose at ef4778bcba (odb/source-loose: drop pointer
to the "files" source, 2026-06-01) merged into it.
Changes in v3:
- Add some more explanations to the commit messages.
- Link to v2: https://patch.msgid.link/20260609-pks-odb-source-packed-v2-0-839089132c8b@pks.im
Changes in v2:
- Split out `struct packfile_list` into a separate code unit to fix a
cyclic dependency between "packfile.h" and "odb/souurce-packed.h".
- Fix an extraneous newline.
- Link to v1: https://patch.msgid.link/20260604-pks-odb-source-packed-v1-0-2e7ab31b4b5c@pks.im
Thanks!
Patrick
[1]: <20260521-b4-pks-odb-source-loose-v1-0-6553b399be2d@pks.im>
---
Patrick Steinhardt (17):
packfile: rename `struct packfile_store` to `odb_source_packed`
packfile: split out packfile list logic
packfile: move packed source into "odb/" subsystem
odb/source-packed: store pointer to "files" instead of generic source
odb/source-packed: start converting to a proper `struct odb_source`
odb/source-packed: wire up `close()` callback
odb/source-packed: wire up `reprepare()` callback
packfile: use higher-level interface to implement `has_object_pack()`
odb/source-packed: wire up `read_object_info()` callback
odb/source-packed: wire up `read_object_stream()` callback
odb/source-packed: wire up `for_each_object()` callback
odb/source-packed: wire up `count_objects()` callback
odb/source-packed: wire up `find_abbrev_len()` callback
odb/source-packed: wire up `freshen_object()` callback
odb/source-packed: stub out remaining functions
midx: refactor interfaces to work on "packed" source
odb/source-packed: drop pointer to "files" parent source
Makefile | 2 +
builtin/cat-file.c | 4 +-
builtin/grep.c | 2 +-
builtin/multi-pack-index.c | 29 +-
builtin/pack-objects.c | 7 +-
builtin/repack.c | 8 +-
commit-graph.c | 4 +-
meson.build | 2 +
midx-write.c | 34 +-
midx.c | 118 +++----
midx.h | 30 +-
odb/source-files.c | 20 +-
odb/source-files.h | 4 +-
odb/source-packed.c | 764 +++++++++++++++++++++++++++++++++++++++++++
odb/source-packed.h | 94 ++++++
odb/source.h | 3 +
pack-bitmap.c | 8 +-
pack-revindex.c | 6 +-
packfile-list.c | 86 +++++
packfile-list.h | 28 ++
packfile.c | 784 +--------------------------------------------
packfile.h | 180 +----------
repack-geometry.c | 3 +-
repack-midx.c | 9 +-
repack.c | 6 +-
t/helper/test-read-midx.c | 7 +-
26 files changed, 1163 insertions(+), 1079 deletions(-)
Range-diff versus v2:
1: 9f917fe0cd = 1: 9c4cb4106c packfile: rename `struct packfile_store` to `odb_source_packed`
2: eb6740c34c = 2: 198b137c84 packfile: split out packfile list logic
3: 5c363883f1 = 3: fa91889861 packfile: move packed source into "odb/" subsystem
4: caa19f59bb ! 4: 5f0da3a334 odb/source-packed: store pointer to "files" instead of generic source
@@ Commit message
Make this relationship more explicit by storing a pointer to the "files"
source instead of storing a pointer to a generic `struct odb_source`.
- This will help make subsequent steps a bit clearer.
+ This will help make subsequent steps a bit clearer by making it more
+ obvious whether we're using the generic "base" source or the owning
+ "files" source.
Note that this is a temporary step, only. At the end of this series
- we will have dropped the parent pointer completely.
+ we will have dropped the "files" pointer completely.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
5: 8eb3cb17a1 ! 5: c9b1e1da26 odb/source-packed: start converting to a proper `struct odb_source`
@@ Commit message
odb_source`, as it's missing all of the callback implementations. These
will be wired up in subsequent commits.
+ Further note that we're also registering a `chdir_notify` callback to
+ reparent our path. This wasn't previously necessary (and still isn't at
+ this point in time) because all paths are taken from the owning "files"
+ source, and that source already handles the reparenting for us. But a
+ subsequent commit will change that so that we're using the path of the
+ "packed" source, and once that happens we'll need it to be updated when
+ changing the working directory.
+
Signed-off-by: Patrick Steinhardt <ps@pks.im>
## odb/source-files.c ##
6: 0b81cf21c8 = 6: 350cc18b4c odb/source-packed: wire up `close()` callback
7: 3024f236eb = 7: b42a633423 odb/source-packed: wire up `reprepare()` callback
8: e3553cae61 = 8: 2eda992ff9 packfile: use higher-level interface to implement `has_object_pack()`
9: 0d4d888c66 = 9: 81201eca00 odb/source-packed: wire up `read_object_info()` callback
10: 0a81bc811f = 10: 2f6ea598a5 odb/source-packed: wire up `read_object_stream()` callback
11: fc4b2477b5 = 11: 154ded231a odb/source-packed: wire up `for_each_object()` callback
12: 58c7c4c6c1 = 12: a35c357358 odb/source-packed: wire up `count_objects()` callback
13: b81e053716 = 13: 134c51a0cd odb/source-packed: wire up `find_abbrev_len()` callback
14: 424b9ee1b2 = 14: 542694cff2 odb/source-packed: wire up `freshen_object()` callback
15: f91f6355fe = 15: fa222d0cda odb/source-packed: stub out remaining functions
16: 877721f2ea = 16: cee3806dc7 midx: refactor interfaces to work on "packed" source
17: 5779eab630 = 17: 200fbcf4bb odb/source-packed: drop pointer to "files" parent source
---
base-commit: 06d49cec508464ced5d42541890ce5d749542a61
change-id: 20260602-pks-odb-source-packed-3826c352f059
^ permalink raw reply
* [PATCH v3 01/17] packfile: rename `struct packfile_store` to `odb_source_packed`
From: Patrick Steinhardt @ 2026-06-17 6:39 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Justin Tobler
In-Reply-To: <20260617-pks-odb-source-packed-v3-0-b5c7583cd795@pks.im>
Not too long ago, we have introduced the packfile store in b7983adb51
(packfile: introduce a new `struct packfile_store`, 2025-09-23). This
struct is responsible for managing all of our access to packfiles and is
used as one of the two sources of objects for the "files" source.
Back when I introduced this structure I didn't have the clear vision yet
that it will eventually also turn into a proper object database source,
and how exactly that infrastructure will look like. Now though it's
becoming increasingly clear that it does make sense to treat it just the
same as any of our other ODB sources.
The consequence is that the naming is now a bit out-of-date: it's just
another source and will be turned into a proper `struct odb_source` over
the next couple of commits, but it's not named accordingly.
Rename the structure to `odb_source_packed` to align it with this goal
and to bring it in line with the other sources we already have.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
odb/source-files.h | 4 ++--
packfile.c | 56 +++++++++++++++++++++++++++---------------------------
packfile.h | 32 +++++++++++++++----------------
3 files changed, 46 insertions(+), 46 deletions(-)
diff --git a/odb/source-files.h b/odb/source-files.h
index 23a3b4e04b..d7ac3c1c81 100644
--- a/odb/source-files.h
+++ b/odb/source-files.h
@@ -4,7 +4,7 @@
#include "odb/source.h"
struct odb_source_loose;
-struct packfile_store;
+struct odb_source_packed;
/*
* The files object database source uses a combination of loose objects and
@@ -13,7 +13,7 @@ struct packfile_store;
struct odb_source_files {
struct odb_source base;
struct odb_source_loose *loose;
- struct packfile_store *packed;
+ struct odb_source_packed *packed;
};
/* Allocate and initialize a new object source. */
diff --git a/packfile.c b/packfile.c
index 89366abfe3..a2d768d0ae 100644
--- a/packfile.c
+++ b/packfile.c
@@ -859,7 +859,7 @@ struct packed_git *add_packed_git(struct repository *r, const char *path,
return p;
}
-void packfile_store_add_pack(struct packfile_store *store,
+void packfile_store_add_pack(struct odb_source_packed *store,
struct packed_git *pack)
{
if (pack->pack_fd != -1)
@@ -869,7 +869,7 @@ void packfile_store_add_pack(struct packfile_store *store,
strmap_put(&store->packs_by_path, pack->pack_name, pack);
}
-struct packed_git *packfile_store_load_pack(struct packfile_store *store,
+struct packed_git *packfile_store_load_pack(struct odb_source_packed *store,
const char *idx_path, int local)
{
struct strbuf key = STRBUF_INIT;
@@ -1068,7 +1068,7 @@ static int sort_pack(const struct packfile_list_entry *a,
return -1;
}
-void packfile_store_prepare(struct packfile_store *store)
+void packfile_store_prepare(struct odb_source_packed *store)
{
if (store->initialized)
return;
@@ -1084,13 +1084,13 @@ void packfile_store_prepare(struct packfile_store *store)
store->initialized = true;
}
-void packfile_store_reprepare(struct packfile_store *store)
+void packfile_store_reprepare(struct odb_source_packed *store)
{
store->initialized = false;
packfile_store_prepare(store);
}
-struct packfile_list_entry *packfile_store_get_packs(struct packfile_store *store)
+struct packfile_list_entry *packfile_store_get_packs(struct odb_source_packed *store)
{
packfile_store_prepare(store);
@@ -1103,7 +1103,7 @@ struct packfile_list_entry *packfile_store_get_packs(struct packfile_store *stor
return store->packs.head;
}
-int packfile_store_count_objects(struct packfile_store *store,
+int packfile_store_count_objects(struct odb_source_packed *store,
enum odb_count_objects_flags flags UNUSED,
unsigned long *out)
{
@@ -2160,7 +2160,7 @@ static int fill_pack_entry(const struct object_id *oid,
return 1;
}
-static int find_pack_entry(struct packfile_store *store,
+static int find_pack_entry(struct odb_source_packed *store,
const struct object_id *oid,
struct pack_entry *e)
{
@@ -2183,7 +2183,7 @@ static int find_pack_entry(struct packfile_store *store,
return 0;
}
-int packfile_store_freshen_object(struct packfile_store *store,
+int packfile_store_freshen_object(struct odb_source_packed *store,
const struct object_id *oid)
{
struct pack_entry e;
@@ -2199,7 +2199,7 @@ int packfile_store_freshen_object(struct packfile_store *store,
return 1;
}
-int packfile_store_read_object_info(struct packfile_store *store,
+int packfile_store_read_object_info(struct odb_source_packed *store,
const struct object_id *oid,
struct object_info *oi,
enum object_info_flags flags)
@@ -2234,7 +2234,7 @@ int packfile_store_read_object_info(struct packfile_store *store,
return 0;
}
-static void maybe_invalidate_kept_pack_cache(struct packfile_store *store,
+static void maybe_invalidate_kept_pack_cache(struct odb_source_packed *store,
unsigned flags)
{
if (!store->kept_cache.packs)
@@ -2245,7 +2245,7 @@ static void maybe_invalidate_kept_pack_cache(struct packfile_store *store,
store->kept_cache.flags = 0;
}
-struct packed_git **packfile_store_get_kept_pack_cache(struct packfile_store *store,
+struct packed_git **packfile_store_get_kept_pack_cache(struct odb_source_packed *store,
unsigned flags)
{
maybe_invalidate_kept_pack_cache(store, flags);
@@ -2365,8 +2365,8 @@ int for_each_object_in_pack(struct packed_git *p,
return r;
}
-struct packfile_store_for_each_object_wrapper_data {
- struct packfile_store *store;
+struct odb_source_packed_for_each_object_wrapper_data {
+ struct odb_source_packed *store;
const struct object_info *request;
odb_for_each_object_cb cb;
void *cb_data;
@@ -2377,7 +2377,7 @@ static int packfile_store_for_each_object_wrapper(const struct object_id *oid,
uint32_t index_pos,
void *cb_data)
{
- struct packfile_store_for_each_object_wrapper_data *data = cb_data;
+ struct odb_source_packed_for_each_object_wrapper_data *data = cb_data;
if (data->request) {
off_t offset = nth_packed_object_offset(pack, index_pos);
@@ -2411,10 +2411,10 @@ static int match_hash(unsigned len, const unsigned char *a, const unsigned char
}
static int for_each_prefixed_object_in_midx(
- struct packfile_store *store,
+ struct odb_source_packed *store,
struct multi_pack_index *m,
const struct odb_for_each_object_options *opts,
- struct packfile_store_for_each_object_wrapper_data *data)
+ struct odb_source_packed_for_each_object_wrapper_data *data)
{
int ret;
@@ -2470,10 +2470,10 @@ static int for_each_prefixed_object_in_midx(
}
static int for_each_prefixed_object_in_pack(
- struct packfile_store *store,
+ struct odb_source_packed *store,
struct packed_git *p,
const struct odb_for_each_object_options *opts,
- struct packfile_store_for_each_object_wrapper_data *data)
+ struct odb_source_packed_for_each_object_wrapper_data *data)
{
uint32_t num, i, first = 0;
int len = opts->prefix_hex_len > p->repo->hash_algo->hexsz ?
@@ -2519,9 +2519,9 @@ static int for_each_prefixed_object_in_pack(
}
static int packfile_store_for_each_prefixed_object(
- struct packfile_store *store,
+ struct odb_source_packed *store,
const struct odb_for_each_object_options *opts,
- struct packfile_store_for_each_object_wrapper_data *data)
+ struct odb_source_packed_for_each_object_wrapper_data *data)
{
struct packfile_list_entry *e;
struct multi_pack_index *m;
@@ -2566,13 +2566,13 @@ static int packfile_store_for_each_prefixed_object(
return ret;
}
-int packfile_store_for_each_object(struct packfile_store *store,
+int packfile_store_for_each_object(struct odb_source_packed *store,
const struct object_info *request,
odb_for_each_object_cb cb,
void *cb_data,
const struct odb_for_each_object_options *opts)
{
- struct packfile_store_for_each_object_wrapper_data data = {
+ struct odb_source_packed_for_each_object_wrapper_data data = {
.store = store,
.request = request,
.cb = cb,
@@ -2707,7 +2707,7 @@ static void find_abbrev_len_for_pack(struct packed_git *p,
*out = len;
}
-int packfile_store_find_abbrev_len(struct packfile_store *store,
+int packfile_store_find_abbrev_len(struct odb_source_packed *store,
const struct object_id *oid,
unsigned min_len,
unsigned *out)
@@ -2832,16 +2832,16 @@ int parse_pack_header_option(const char *in, unsigned char *out, unsigned int *l
return 0;
}
-struct packfile_store *packfile_store_new(struct odb_source *source)
+struct odb_source_packed *packfile_store_new(struct odb_source *source)
{
- struct packfile_store *store;
+ struct odb_source_packed *store;
CALLOC_ARRAY(store, 1);
store->source = source;
strmap_init(&store->packs_by_path);
return store;
}
-void packfile_store_free(struct packfile_store *store)
+void packfile_store_free(struct odb_source_packed *store)
{
for (struct packfile_list_entry *e = store->packs.head; e; e = e->next)
free(e->pack);
@@ -2851,7 +2851,7 @@ void packfile_store_free(struct packfile_store *store)
free(store);
}
-void packfile_store_close(struct packfile_store *store)
+void packfile_store_close(struct odb_source_packed *store)
{
for (struct packfile_list_entry *e = store->packs.head; e; e = e->next) {
if (e->pack->do_not_close)
@@ -2988,7 +2988,7 @@ int packfile_read_object_stream(struct odb_read_stream **out,
}
int packfile_store_read_object_stream(struct odb_read_stream **out,
- struct packfile_store *store,
+ struct odb_source_packed *store,
const struct object_id *oid)
{
struct pack_entry e;
diff --git a/packfile.h b/packfile.h
index 49d6bdecf6..9cec15bc50 100644
--- a/packfile.h
+++ b/packfile.h
@@ -79,7 +79,7 @@ struct packed_git *packfile_list_find_oid(struct packfile_list_entry *packs,
/*
* A store that manages packfiles for a given object database.
*/
-struct packfile_store {
+struct odb_source_packed {
struct odb_source *source;
/*
@@ -138,19 +138,19 @@ struct packfile_store {
* Allocate and initialize a new empty packfile store for the given object
* database source.
*/
-struct packfile_store *packfile_store_new(struct odb_source *source);
+struct odb_source_packed *packfile_store_new(struct odb_source *source);
/*
* Free the packfile store and all its associated state. All packfiles
* tracked by the store will be closed.
*/
-void packfile_store_free(struct packfile_store *store);
+void packfile_store_free(struct odb_source_packed *store);
/*
* Close all packfiles associated with this store. The packfiles won't be
* free'd, so they can be re-opened at a later point in time.
*/
-void packfile_store_close(struct packfile_store *store);
+void packfile_store_close(struct odb_source_packed *store);
/*
* Prepare the packfile store by loading packfiles and multi-pack indices for
@@ -159,7 +159,7 @@ void packfile_store_close(struct packfile_store *store);
* It shouldn't typically be necessary to call this function directly, as
* functions that access the store know to prepare it.
*/
-void packfile_store_prepare(struct packfile_store *store);
+void packfile_store_prepare(struct odb_source_packed *store);
/*
* Clear the packfile caches and try to look up any new packfiles that have
@@ -167,20 +167,20 @@ void packfile_store_prepare(struct packfile_store *store);
*
* This function must be called under the `odb_read_lock()`.
*/
-void packfile_store_reprepare(struct packfile_store *store);
+void packfile_store_reprepare(struct odb_source_packed *store);
/*
* Add the pack to the store so that contained objects become accessible via
* the store. This moves ownership into the store.
*/
-void packfile_store_add_pack(struct packfile_store *store,
+void packfile_store_add_pack(struct odb_source_packed *store,
struct packed_git *pack);
/*
* Get all packs managed by the given store, including packfiles that are
* referenced by multi-pack indices.
*/
-struct packfile_list_entry *packfile_store_get_packs(struct packfile_store *store);
+struct packfile_list_entry *packfile_store_get_packs(struct odb_source_packed *store);
struct repo_for_each_pack_data {
struct odb_source *source;
@@ -239,7 +239,7 @@ static inline void repo_for_each_pack_data_next(struct repo_for_each_pack_data *
repo_for_each_pack_data_next(&eack_pack_data))
int packfile_store_read_object_stream(struct odb_read_stream **out,
- struct packfile_store *store,
+ struct odb_source_packed *store,
const struct object_id *oid);
/*
@@ -248,7 +248,7 @@ int packfile_store_read_object_stream(struct odb_read_stream **out,
* not found, 0 if it was and read successfully, and a negative error code in
* case the object was corrupted.
*/
-int packfile_store_read_object_info(struct packfile_store *store,
+int packfile_store_read_object_info(struct odb_source_packed *store,
const struct object_id *oid,
struct object_info *oi,
enum object_info_flags flags);
@@ -258,10 +258,10 @@ int packfile_store_read_object_info(struct packfile_store *store,
* either the newly opened packfile or the preexisting packfile. Returns a
* `NULL` pointer in case the packfile could not be opened.
*/
-struct packed_git *packfile_store_load_pack(struct packfile_store *store,
+struct packed_git *packfile_store_load_pack(struct odb_source_packed *store,
const char *idx_path, int local);
-int packfile_store_freshen_object(struct packfile_store *store,
+int packfile_store_freshen_object(struct odb_source_packed *store,
const struct object_id *oid);
enum kept_pack_type {
@@ -276,7 +276,7 @@ enum kept_pack_type {
*
* Return 0 on success, a negative error code otherwise.
*/
-int packfile_store_count_objects(struct packfile_store *store,
+int packfile_store_count_objects(struct odb_source_packed *store,
enum odb_count_objects_flags flags,
unsigned long *out);
@@ -285,7 +285,7 @@ int packfile_store_count_objects(struct packfile_store *store,
* combination of `kept_pack_type` flags. The cache is computed on demand and
* will be recomputed whenever the flags change.
*/
-struct packed_git **packfile_store_get_kept_pack_cache(struct packfile_store *store,
+struct packed_git **packfile_store_get_kept_pack_cache(struct odb_source_packed *store,
unsigned flags);
struct pack_window {
@@ -365,13 +365,13 @@ int for_each_object_in_pack(struct packed_git *p,
*
* The flags parameter is a combination of `odb_for_each_object_flags`.
*/
-int packfile_store_for_each_object(struct packfile_store *store,
+int packfile_store_for_each_object(struct odb_source_packed *store,
const struct object_info *request,
odb_for_each_object_cb cb,
void *cb_data,
const struct odb_for_each_object_options *opts);
-int packfile_store_find_abbrev_len(struct packfile_store *store,
+int packfile_store_find_abbrev_len(struct odb_source_packed *store,
const struct object_id *oid,
unsigned min_len,
unsigned *out);
--
2.55.0.rc0.786.g65d90a0328.dirty
^ permalink raw reply related
* [PATCH v3 02/17] packfile: split out packfile list logic
From: Patrick Steinhardt @ 2026-06-17 6:39 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Justin Tobler
In-Reply-To: <20260617-pks-odb-source-packed-v3-0-b5c7583cd795@pks.im>
In the next commit we're about to introduce the "packed" object database
source. This source will embed a packfile list, and consequently we'll
have to include "packfile.h" to make the struct definition available.
This will unfortunately lead to a cyclic dependency that we cannot
resolve with a forward declaration.
Split out the code that relates to the packfile list into a separate
compilation unit so that both "packfile.h" and "odb/source-packed.h" can
include it.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Makefile | 1 +
meson.build | 1 +
packfile-list.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
packfile-list.h | 28 +++++++++++++++++++
packfile.c | 83 -------------------------------------------------------
packfile.h | 23 +--------------
6 files changed, 117 insertions(+), 105 deletions(-)
diff --git a/Makefile b/Makefile
index 0976a69b4c..ed1731548e 100644
--- a/Makefile
+++ b/Makefile
@@ -1233,6 +1233,7 @@ LIB_OBJS += pack-refs.o
LIB_OBJS += pack-revindex.o
LIB_OBJS += pack-write.o
LIB_OBJS += packfile.o
+LIB_OBJS += packfile-list.o
LIB_OBJS += pager.o
LIB_OBJS += parallel-checkout.o
LIB_OBJS += parse.o
diff --git a/meson.build b/meson.build
index 3247697f74..12913fc948 100644
--- a/meson.build
+++ b/meson.build
@@ -421,6 +421,7 @@ libgit_sources = [
'pack-revindex.c',
'pack-write.c',
'packfile.c',
+ 'packfile-list.c',
'pager.c',
'parallel-checkout.c',
'parse.c',
diff --git a/packfile-list.c b/packfile-list.c
new file mode 100644
index 0000000000..01fb913abf
--- /dev/null
+++ b/packfile-list.c
@@ -0,0 +1,86 @@
+#include "git-compat-util.h"
+#include "packfile.h"
+#include "packfile-list.h"
+
+void packfile_list_clear(struct packfile_list *list)
+{
+ struct packfile_list_entry *e, *next;
+
+ for (e = list->head; e; e = next) {
+ next = e->next;
+ free(e);
+ }
+
+ list->head = list->tail = NULL;
+}
+
+static struct packfile_list_entry *packfile_list_remove_internal(struct packfile_list *list,
+ struct packed_git *pack)
+{
+ struct packfile_list_entry *e, *prev;
+
+ for (e = list->head, prev = NULL; e; prev = e, e = e->next) {
+ if (e->pack != pack)
+ continue;
+
+ if (prev)
+ prev->next = e->next;
+ if (list->head == e)
+ list->head = e->next;
+ if (list->tail == e)
+ list->tail = prev;
+
+ return e;
+ }
+
+ return NULL;
+}
+
+void packfile_list_remove(struct packfile_list *list, struct packed_git *pack)
+{
+ free(packfile_list_remove_internal(list, pack));
+}
+
+void packfile_list_prepend(struct packfile_list *list, struct packed_git *pack)
+{
+ struct packfile_list_entry *entry;
+
+ entry = packfile_list_remove_internal(list, pack);
+ if (!entry) {
+ entry = xmalloc(sizeof(*entry));
+ entry->pack = pack;
+ }
+ entry->next = list->head;
+
+ list->head = entry;
+ if (!list->tail)
+ list->tail = entry;
+}
+
+void packfile_list_append(struct packfile_list *list, struct packed_git *pack)
+{
+ struct packfile_list_entry *entry;
+
+ entry = packfile_list_remove_internal(list, pack);
+ if (!entry) {
+ entry = xmalloc(sizeof(*entry));
+ entry->pack = pack;
+ }
+ entry->next = NULL;
+
+ if (list->tail) {
+ list->tail->next = entry;
+ list->tail = entry;
+ } else {
+ list->head = list->tail = entry;
+ }
+}
+
+struct packed_git *packfile_list_find_oid(struct packfile_list_entry *packs,
+ const struct object_id *oid)
+{
+ for (; packs; packs = packs->next)
+ if (find_pack_entry_one(oid, packs->pack))
+ return packs->pack;
+ return NULL;
+}
diff --git a/packfile-list.h b/packfile-list.h
new file mode 100644
index 0000000000..1b05e2aa36
--- /dev/null
+++ b/packfile-list.h
@@ -0,0 +1,28 @@
+#ifndef PACKFILE_LIST_H
+#define PACKFILE_LIST_H
+
+struct object_id;
+
+struct packfile_list {
+ struct packfile_list_entry *head, *tail;
+};
+
+struct packfile_list_entry {
+ struct packfile_list_entry *next;
+ struct packed_git *pack;
+};
+
+void packfile_list_clear(struct packfile_list *list);
+void packfile_list_remove(struct packfile_list *list, struct packed_git *pack);
+void packfile_list_prepend(struct packfile_list *list, struct packed_git *pack);
+void packfile_list_append(struct packfile_list *list, struct packed_git *pack);
+
+/*
+ * Find the pack within the "packs" list whose index contains the object
+ * "oid". For general object lookups, you probably don't want this; use
+ * find_pack_entry() instead.
+ */
+struct packed_git *packfile_list_find_oid(struct packfile_list_entry *packs,
+ const struct object_id *oid);
+
+#endif
diff --git a/packfile.c b/packfile.c
index a2d768d0ae..27ea4a8436 100644
--- a/packfile.c
+++ b/packfile.c
@@ -48,89 +48,6 @@ static size_t pack_mapped;
#define SZ_FMT PRIuMAX
static inline uintmax_t sz_fmt(size_t s) { return s; }
-void packfile_list_clear(struct packfile_list *list)
-{
- struct packfile_list_entry *e, *next;
-
- for (e = list->head; e; e = next) {
- next = e->next;
- free(e);
- }
-
- list->head = list->tail = NULL;
-}
-
-static struct packfile_list_entry *packfile_list_remove_internal(struct packfile_list *list,
- struct packed_git *pack)
-{
- struct packfile_list_entry *e, *prev;
-
- for (e = list->head, prev = NULL; e; prev = e, e = e->next) {
- if (e->pack != pack)
- continue;
-
- if (prev)
- prev->next = e->next;
- if (list->head == e)
- list->head = e->next;
- if (list->tail == e)
- list->tail = prev;
-
- return e;
- }
-
- return NULL;
-}
-
-void packfile_list_remove(struct packfile_list *list, struct packed_git *pack)
-{
- free(packfile_list_remove_internal(list, pack));
-}
-
-void packfile_list_prepend(struct packfile_list *list, struct packed_git *pack)
-{
- struct packfile_list_entry *entry;
-
- entry = packfile_list_remove_internal(list, pack);
- if (!entry) {
- entry = xmalloc(sizeof(*entry));
- entry->pack = pack;
- }
- entry->next = list->head;
-
- list->head = entry;
- if (!list->tail)
- list->tail = entry;
-}
-
-void packfile_list_append(struct packfile_list *list, struct packed_git *pack)
-{
- struct packfile_list_entry *entry;
-
- entry = packfile_list_remove_internal(list, pack);
- if (!entry) {
- entry = xmalloc(sizeof(*entry));
- entry->pack = pack;
- }
- entry->next = NULL;
-
- if (list->tail) {
- list->tail->next = entry;
- list->tail = entry;
- } else {
- list->head = list->tail = entry;
- }
-}
-
-struct packed_git *packfile_list_find_oid(struct packfile_list_entry *packs,
- const struct object_id *oid)
-{
- for (; packs; packs = packs->next)
- if (find_pack_entry_one(oid, packs->pack))
- return packs->pack;
- return NULL;
-}
-
void pack_report(struct repository *repo)
{
fprintf(stderr,
diff --git a/packfile.h b/packfile.h
index 9cec15bc50..4e3d701a3a 100644
--- a/packfile.h
+++ b/packfile.h
@@ -6,6 +6,7 @@
#include "odb.h"
#include "odb/source-files.h"
#include "oidset.h"
+#include "packfile-list.h"
#include "repository.h"
#include "strmap.h"
@@ -54,28 +55,6 @@ struct packed_git {
char pack_name[FLEX_ARRAY]; /* more */
};
-struct packfile_list {
- struct packfile_list_entry *head, *tail;
-};
-
-struct packfile_list_entry {
- struct packfile_list_entry *next;
- struct packed_git *pack;
-};
-
-void packfile_list_clear(struct packfile_list *list);
-void packfile_list_remove(struct packfile_list *list, struct packed_git *pack);
-void packfile_list_prepend(struct packfile_list *list, struct packed_git *pack);
-void packfile_list_append(struct packfile_list *list, struct packed_git *pack);
-
-/*
- * Find the pack within the "packs" list whose index contains the object
- * "oid". For general object lookups, you probably don't want this; use
- * find_pack_entry() instead.
- */
-struct packed_git *packfile_list_find_oid(struct packfile_list_entry *packs,
- const struct object_id *oid);
-
/*
* A store that manages packfiles for a given object database.
*/
--
2.55.0.rc0.786.g65d90a0328.dirty
^ permalink raw reply related
* [PATCH v3 03/17] packfile: move packed source into "odb/" subsystem
From: Patrick Steinhardt @ 2026-06-17 6:39 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Justin Tobler
In-Reply-To: <20260617-pks-odb-source-packed-v3-0-b5c7583cd795@pks.im>
In subsequent patches we'll be turning `struct odb_source_packed` into a
proper `struct odb_source`. As a first step towards this goal, move its
struct out of "packfile.{c,h}" and into "odb/source-packed.{c,h}".
This detaches the implementation of the packfile object source from the
generic packfile code, following the same convention already used by the
"files" and "in-memory" sources.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Makefile | 1 +
meson.build | 1 +
odb/source-files.c | 2 +-
odb/source-packed.c | 11 ++++++++
odb/source-packed.h | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++
packfile.c | 9 -------
packfile.h | 66 +-----------------------------------------------
7 files changed, 87 insertions(+), 75 deletions(-)
diff --git a/Makefile b/Makefile
index ed1731548e..113fa45993 100644
--- a/Makefile
+++ b/Makefile
@@ -1218,6 +1218,7 @@ LIB_OBJS += odb/source.o
LIB_OBJS += odb/source-files.o
LIB_OBJS += odb/source-inmemory.o
LIB_OBJS += odb/source-loose.o
+LIB_OBJS += odb/source-packed.o
LIB_OBJS += odb/streaming.o
LIB_OBJS += odb/transaction.o
LIB_OBJS += oid-array.o
diff --git a/meson.build b/meson.build
index 12913fc948..ca235801cf 100644
--- a/meson.build
+++ b/meson.build
@@ -406,6 +406,7 @@ libgit_sources = [
'odb/source-files.c',
'odb/source-inmemory.c',
'odb/source-loose.c',
+ 'odb/source-packed.c',
'odb/streaming.c',
'odb/transaction.c',
'oid-array.c',
diff --git a/odb/source-files.c b/odb/source-files.c
index 5bdd042922..191562f316 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -269,7 +269,7 @@ struct odb_source_files *odb_source_files_new(struct object_database *odb,
CALLOC_ARRAY(files, 1);
odb_source_init(&files->base, odb, ODB_SOURCE_FILES, path, local);
files->loose = odb_source_loose_new(odb, path, local);
- files->packed = packfile_store_new(&files->base);
+ files->packed = odb_source_packed_new(&files->base);
files->base.free = odb_source_files_free;
files->base.close = odb_source_files_close;
diff --git a/odb/source-packed.c b/odb/source-packed.c
new file mode 100644
index 0000000000..1e94b47ea0
--- /dev/null
+++ b/odb/source-packed.c
@@ -0,0 +1,11 @@
+#include "git-compat-util.h"
+#include "odb/source-packed.h"
+
+struct odb_source_packed *odb_source_packed_new(struct odb_source *source)
+{
+ struct odb_source_packed *store;
+ CALLOC_ARRAY(store, 1);
+ store->source = source;
+ strmap_init(&store->packs_by_path);
+ return store;
+}
diff --git a/odb/source-packed.h b/odb/source-packed.h
new file mode 100644
index 0000000000..327be4ad65
--- /dev/null
+++ b/odb/source-packed.h
@@ -0,0 +1,72 @@
+#ifndef ODB_SOURCE_PACKED_H
+#define ODB_SOURCE_PACKED_H
+
+#include "odb/source.h"
+#include "packfile-list.h"
+#include "strmap.h"
+
+/*
+ * A store that manages packfiles for a given object database.
+ */
+struct odb_source_packed {
+ struct odb_source *source;
+
+ /*
+ * The list of packfiles in the order in which they have been most
+ * recently used.
+ */
+ struct packfile_list packs;
+
+ /*
+ * Cache of packfiles which are marked as "kept", either because there
+ * is an on-disk ".keep" file or because they are marked as "kept" in
+ * memory.
+ *
+ * Should not be accessed directly, but via
+ * `packfile_store_get_kept_pack_cache()`. The list of packs gets
+ * invalidated when the stored flags and the flags passed to
+ * `packfile_store_get_kept_pack_cache()` mismatch.
+ */
+ struct {
+ struct packed_git **packs;
+ unsigned flags;
+ } kept_cache;
+
+ /* The multi-pack index that belongs to this specific packfile store. */
+ struct multi_pack_index *midx;
+
+ /*
+ * A map of packfile names to packed_git structs for tracking which
+ * packs have been loaded already.
+ */
+ struct strmap packs_by_path;
+
+ /*
+ * Whether packfiles have already been populated with this store's
+ * packs.
+ */
+ bool initialized;
+
+ /*
+ * Usually, packfiles will be reordered to the front of the `packs`
+ * list whenever an object is looked up via them. This has the effect
+ * that packs that contain a lot of accessed objects will be located
+ * towards the front.
+ *
+ * This is usually desireable, but there are exceptions. One exception
+ * is when the looking up multiple objects in a loop for each packfile.
+ * In that case, we may easily end up with an infinite loop as the
+ * packfiles get reordered to the front repeatedly.
+ *
+ * Setting this field to `true` thus disables these reorderings.
+ */
+ bool skip_mru_updates;
+};
+
+/*
+ * Allocate and initialize a new empty packfile store for the given object
+ * database source.
+ */
+struct odb_source_packed *odb_source_packed_new(struct odb_source *source);
+
+#endif
diff --git a/packfile.c b/packfile.c
index 27ea4a8436..99be5789ef 100644
--- a/packfile.c
+++ b/packfile.c
@@ -2749,15 +2749,6 @@ int parse_pack_header_option(const char *in, unsigned char *out, unsigned int *l
return 0;
}
-struct odb_source_packed *packfile_store_new(struct odb_source *source)
-{
- struct odb_source_packed *store;
- CALLOC_ARRAY(store, 1);
- store->source = source;
- strmap_init(&store->packs_by_path);
- return store;
-}
-
void packfile_store_free(struct odb_source_packed *store)
{
for (struct packfile_list_entry *e = store->packs.head; e; e = e->next)
diff --git a/packfile.h b/packfile.h
index 4e3d701a3a..2d0bb7adbe 100644
--- a/packfile.h
+++ b/packfile.h
@@ -5,10 +5,10 @@
#include "object.h"
#include "odb.h"
#include "odb/source-files.h"
+#include "odb/source-packed.h"
#include "oidset.h"
#include "packfile-list.h"
#include "repository.h"
-#include "strmap.h"
/* in odb.h */
struct object_info;
@@ -55,70 +55,6 @@ struct packed_git {
char pack_name[FLEX_ARRAY]; /* more */
};
-/*
- * A store that manages packfiles for a given object database.
- */
-struct odb_source_packed {
- struct odb_source *source;
-
- /*
- * The list of packfiles in the order in which they have been most
- * recently used.
- */
- struct packfile_list packs;
-
- /*
- * Cache of packfiles which are marked as "kept", either because there
- * is an on-disk ".keep" file or because they are marked as "kept" in
- * memory.
- *
- * Should not be accessed directly, but via
- * `packfile_store_get_kept_pack_cache()`. The list of packs gets
- * invalidated when the stored flags and the flags passed to
- * `packfile_store_get_kept_pack_cache()` mismatch.
- */
- struct {
- struct packed_git **packs;
- unsigned flags;
- } kept_cache;
-
- /* The multi-pack index that belongs to this specific packfile store. */
- struct multi_pack_index *midx;
-
- /*
- * A map of packfile names to packed_git structs for tracking which
- * packs have been loaded already.
- */
- struct strmap packs_by_path;
-
- /*
- * Whether packfiles have already been populated with this store's
- * packs.
- */
- bool initialized;
-
- /*
- * Usually, packfiles will be reordered to the front of the `packs`
- * list whenever an object is looked up via them. This has the effect
- * that packs that contain a lot of accessed objects will be located
- * towards the front.
- *
- * This is usually desireable, but there are exceptions. One exception
- * is when the looking up multiple objects in a loop for each packfile.
- * In that case, we may easily end up with an infinite loop as the
- * packfiles get reordered to the front repeatedly.
- *
- * Setting this field to `true` thus disables these reorderings.
- */
- bool skip_mru_updates;
-};
-
-/*
- * Allocate and initialize a new empty packfile store for the given object
- * database source.
- */
-struct odb_source_packed *packfile_store_new(struct odb_source *source);
-
/*
* Free the packfile store and all its associated state. All packfiles
* tracked by the store will be closed.
--
2.55.0.rc0.786.g65d90a0328.dirty
^ permalink raw reply related
* [PATCH v3 04/17] odb/source-packed: store pointer to "files" instead of generic source
From: Patrick Steinhardt @ 2026-06-17 6:39 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Justin Tobler
In-Reply-To: <20260617-pks-odb-source-packed-v3-0-b5c7583cd795@pks.im>
The `struct odb_source_packed` holds a pointer to its owning parent
source. The way that Git is currently structured, this parent is always
the "files" source. In subsequent commits we're going to detangle that
so that the "packed" source doesn't have any owning parent source at
all, which makes it usable as a completely standalone source.
Detangling this mess is somewhat intricate though, and is made even more
intricate because it's not always clear which kind of source one is
holding at a specific point in time -- either the parent "files" source,
or the child "packed" source.
Make this relationship more explicit by storing a pointer to the "files"
source instead of storing a pointer to a generic `struct odb_source`.
This will help make subsequent steps a bit clearer by making it more
obvious whether we're using the generic "base" source or the owning
"files" source.
Note that this is a temporary step, only. At the end of this series
we will have dropped the "files" pointer completely.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
odb/source-files.c | 2 +-
odb/source-packed.c | 4 ++--
odb/source-packed.h | 4 ++--
packfile.c | 12 ++++++------
4 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/odb/source-files.c b/odb/source-files.c
index 191562f316..e04525fb08 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -269,7 +269,7 @@ struct odb_source_files *odb_source_files_new(struct object_database *odb,
CALLOC_ARRAY(files, 1);
odb_source_init(&files->base, odb, ODB_SOURCE_FILES, path, local);
files->loose = odb_source_loose_new(odb, path, local);
- files->packed = odb_source_packed_new(&files->base);
+ files->packed = odb_source_packed_new(files);
files->base.free = odb_source_files_free;
files->base.close = odb_source_files_close;
diff --git a/odb/source-packed.c b/odb/source-packed.c
index 1e94b47ea0..12e785be48 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -1,11 +1,11 @@
#include "git-compat-util.h"
#include "odb/source-packed.h"
-struct odb_source_packed *odb_source_packed_new(struct odb_source *source)
+struct odb_source_packed *odb_source_packed_new(struct odb_source_files *parent)
{
struct odb_source_packed *store;
CALLOC_ARRAY(store, 1);
- store->source = source;
+ store->files = parent;
strmap_init(&store->packs_by_path);
return store;
}
diff --git a/odb/source-packed.h b/odb/source-packed.h
index 327be4ad65..3c2d229a17 100644
--- a/odb/source-packed.h
+++ b/odb/source-packed.h
@@ -9,7 +9,7 @@
* A store that manages packfiles for a given object database.
*/
struct odb_source_packed {
- struct odb_source *source;
+ struct odb_source_files *files;
/*
* The list of packfiles in the order in which they have been most
@@ -67,6 +67,6 @@ struct odb_source_packed {
* Allocate and initialize a new empty packfile store for the given object
* database source.
*/
-struct odb_source_packed *odb_source_packed_new(struct odb_source *source);
+struct odb_source_packed *odb_source_packed_new(struct odb_source_files *parent);
#endif
diff --git a/packfile.c b/packfile.c
index 99be5789ef..862a24ad49 100644
--- a/packfile.c
+++ b/packfile.c
@@ -802,7 +802,7 @@ struct packed_git *packfile_store_load_pack(struct odb_source_packed *store,
p = strmap_get(&store->packs_by_path, key.buf);
if (!p) {
- p = add_packed_git(store->source->odb->repo, idx_path,
+ p = add_packed_git(store->files->base.odb->repo, idx_path,
strlen(idx_path), local);
if (p)
packfile_store_add_pack(store, p);
@@ -990,8 +990,8 @@ void packfile_store_prepare(struct odb_source_packed *store)
if (store->initialized)
return;
- prepare_multi_pack_index_one(store->source);
- prepare_packed_git_one(store->source);
+ prepare_multi_pack_index_one(&store->files->base);
+ prepare_packed_git_one(&store->files->base);
sort_packs(&store->packs.head, sort_pack);
for (struct packfile_list_entry *e = store->packs.head; e; e = e->next)
@@ -1029,7 +1029,7 @@ int packfile_store_count_objects(struct odb_source_packed *store,
unsigned long count = 0;
int ret;
- m = get_multi_pack_index(store->source);
+ m = get_multi_pack_index(&store->files->base);
if (m)
count += m->num_objects + m->num_objects_in_base;
@@ -2450,7 +2450,7 @@ static int packfile_store_for_each_prefixed_object(
store->skip_mru_updates = true;
- m = get_multi_pack_index(store->source);
+ m = get_multi_pack_index(&store->files->base);
if (m) {
ret = for_each_prefixed_object_in_midx(store, m, opts, data);
if (ret)
@@ -2632,7 +2632,7 @@ int packfile_store_find_abbrev_len(struct odb_source_packed *store,
struct packfile_list_entry *e;
struct multi_pack_index *m;
- m = get_multi_pack_index(store->source);
+ m = get_multi_pack_index(&store->files->base);
if (m)
find_abbrev_len_for_midx(m, oid, min_len, &min_len);
--
2.55.0.rc0.786.g65d90a0328.dirty
^ permalink raw reply related
* [PATCH v3 05/17] odb/source-packed: start converting to a proper `struct odb_source`
From: Patrick Steinhardt @ 2026-06-17 6:39 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Justin Tobler
In-Reply-To: <20260617-pks-odb-source-packed-v3-0-b5c7583cd795@pks.im>
Start converting `struct odb_source_packed` into a proper pluggable
`struct odb_source` by embedding the base struct and assigning it the
new `ODB_SOURCE_PACKED` type. Furthermore, wire up lifecycle management
of this source by implementing the `free` callback and taking ownership
of the chdir notifications.
Note that the packed source is not yet functional as a standalone `struct
odb_source`, as it's missing all of the callback implementations. These
will be wired up in subsequent commits.
Further note that we're also registering a `chdir_notify` callback to
reparent our path. This wasn't previously necessary (and still isn't at
this point in time) because all paths are taken from the owning "files"
source, and that source already handles the reparenting for us. But a
subsequent commit will change that so that we're using the path of the
"packed" source, and once that happens we'll need it to be updated when
changing the working directory.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
odb/source-files.c | 2 +-
odb/source-packed.c | 49 ++++++++++++++++++++++++++++++++++++++++++++-----
odb/source-packed.h | 12 ++++++++++++
odb/source.h | 3 +++
packfile.c | 10 ----------
packfile.h | 6 ------
6 files changed, 60 insertions(+), 22 deletions(-)
diff --git a/odb/source-files.c b/odb/source-files.c
index e04525fb08..3608808e7c 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -29,7 +29,7 @@ static void odb_source_files_free(struct odb_source *source)
struct odb_source_files *files = odb_source_files_downcast(source);
chdir_notify_unregister(NULL, odb_source_files_reparent, files);
odb_source_free(&files->loose->base);
- packfile_store_free(files->packed);
+ odb_source_free(&files->packed->base);
odb_source_release(&files->base);
free(files);
}
diff --git a/odb/source-packed.c b/odb/source-packed.c
index 12e785be48..f81a990cbd 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -1,11 +1,50 @@
#include "git-compat-util.h"
+#include "abspath.h"
+#include "chdir-notify.h"
#include "odb/source-packed.h"
+#include "packfile.h"
+
+static void odb_source_packed_reparent(const char *name UNUSED,
+ const char *old_cwd,
+ const char *new_cwd,
+ void *cb_data)
+{
+ struct odb_source_packed *packed = cb_data;
+ char *path = reparent_relative_path(old_cwd, new_cwd,
+ packed->base.path);
+ free(packed->base.path);
+ packed->base.path = path;
+}
+
+static void odb_source_packed_free(struct odb_source *source)
+{
+ struct odb_source_packed *packed = odb_source_packed_downcast(source);
+
+ chdir_notify_unregister(NULL, odb_source_packed_reparent, packed);
+
+ for (struct packfile_list_entry *e = packed->packs.head; e; e = e->next)
+ free(e->pack);
+ packfile_list_clear(&packed->packs);
+
+ strmap_clear(&packed->packs_by_path, 0);
+ odb_source_release(&packed->base);
+ free(packed);
+}
struct odb_source_packed *odb_source_packed_new(struct odb_source_files *parent)
{
- struct odb_source_packed *store;
- CALLOC_ARRAY(store, 1);
- store->files = parent;
- strmap_init(&store->packs_by_path);
- return store;
+ struct odb_source_packed *packed;
+
+ CALLOC_ARRAY(packed, 1);
+ odb_source_init(&packed->base, parent->base.odb, ODB_SOURCE_PACKED,
+ parent->base.path, parent->base.local);
+ packed->files = parent;
+ strmap_init(&packed->packs_by_path);
+
+ packed->base.free = odb_source_packed_free;
+
+ if (!is_absolute_path(parent->base.path))
+ chdir_notify_register(NULL, odb_source_packed_reparent, packed);
+
+ return packed;
}
diff --git a/odb/source-packed.h b/odb/source-packed.h
index 3c2d229a17..68e64cabab 100644
--- a/odb/source-packed.h
+++ b/odb/source-packed.h
@@ -9,6 +9,7 @@
* A store that manages packfiles for a given object database.
*/
struct odb_source_packed {
+ struct odb_source base;
struct odb_source_files *files;
/*
@@ -69,4 +70,15 @@ struct odb_source_packed {
*/
struct odb_source_packed *odb_source_packed_new(struct odb_source_files *parent);
+/*
+ * Cast the given object database source to the packed backend. This will cause
+ * a BUG in case the source doesn't use this backend.
+ */
+static inline struct odb_source_packed *odb_source_packed_downcast(struct odb_source *source)
+{
+ if (source->type != ODB_SOURCE_PACKED)
+ BUG("trying to downcast source of type '%d' to packed", source->type);
+ return container_of(source, struct odb_source_packed, base);
+}
+
#endif
diff --git a/odb/source.h b/odb/source.h
index 8bcb67787e..6865e1f71a 100644
--- a/odb/source.h
+++ b/odb/source.h
@@ -17,6 +17,9 @@ enum odb_source_type {
/* The "loose" backend that uses loose objects, only. */
ODB_SOURCE_LOOSE,
+ /* The "packed" backend that uses packfiles. */
+ ODB_SOURCE_PACKED,
+
/* The "in-memory" backend that stores objects in memory. */
ODB_SOURCE_INMEMORY,
};
diff --git a/packfile.c b/packfile.c
index 862a24ad49..6d492216de 100644
--- a/packfile.c
+++ b/packfile.c
@@ -2749,16 +2749,6 @@ int parse_pack_header_option(const char *in, unsigned char *out, unsigned int *l
return 0;
}
-void packfile_store_free(struct odb_source_packed *store)
-{
- for (struct packfile_list_entry *e = store->packs.head; e; e = e->next)
- free(e->pack);
- packfile_list_clear(&store->packs);
-
- strmap_clear(&store->packs_by_path, 0);
- free(store);
-}
-
void packfile_store_close(struct odb_source_packed *store)
{
for (struct packfile_list_entry *e = store->packs.head; e; e = e->next) {
diff --git a/packfile.h b/packfile.h
index 2d0bb7adbe..e8bc9349f8 100644
--- a/packfile.h
+++ b/packfile.h
@@ -55,12 +55,6 @@ struct packed_git {
char pack_name[FLEX_ARRAY]; /* more */
};
-/*
- * Free the packfile store and all its associated state. All packfiles
- * tracked by the store will be closed.
- */
-void packfile_store_free(struct odb_source_packed *store);
-
/*
* Close all packfiles associated with this store. The packfiles won't be
* free'd, so they can be re-opened at a later point in time.
--
2.55.0.rc0.786.g65d90a0328.dirty
^ permalink raw reply related
* [PATCH v3 06/17] odb/source-packed: wire up `close()` callback
From: Patrick Steinhardt @ 2026-06-17 6:39 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Justin Tobler
In-Reply-To: <20260617-pks-odb-source-packed-v3-0-b5c7583cd795@pks.im>
Wire up a new `close()` callback for the packed source and call it from
the "files" source via the generic `odb_source_close()` interface.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
odb/source-files.c | 2 +-
odb/source-packed.c | 16 ++++++++++++++++
packfile.c | 12 ------------
packfile.h | 6 ------
4 files changed, 17 insertions(+), 19 deletions(-)
diff --git a/odb/source-files.c b/odb/source-files.c
index 3608808e7c..9b0fa9ccdc 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -38,7 +38,7 @@ static void odb_source_files_close(struct odb_source *source)
{
struct odb_source_files *files = odb_source_files_downcast(source);
odb_source_close(&files->loose->base);
- packfile_store_close(files->packed);
+ odb_source_close(&files->packed->base);
}
static void odb_source_files_reprepare(struct odb_source *source)
diff --git a/odb/source-packed.c b/odb/source-packed.c
index f81a990cbd..74805be1dd 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -1,6 +1,7 @@
#include "git-compat-util.h"
#include "abspath.h"
#include "chdir-notify.h"
+#include "midx.h"
#include "odb/source-packed.h"
#include "packfile.h"
@@ -16,6 +17,20 @@ static void odb_source_packed_reparent(const char *name UNUSED,
packed->base.path = path;
}
+static void odb_source_packed_close(struct odb_source *source)
+{
+ struct odb_source_packed *packed = odb_source_packed_downcast(source);
+
+ for (struct packfile_list_entry *e = packed->packs.head; e; e = e->next) {
+ if (e->pack->do_not_close)
+ BUG("want to close pack marked 'do-not-close'");
+ close_pack(e->pack);
+ }
+ if (packed->midx)
+ close_midx(packed->midx);
+ packed->midx = NULL;
+}
+
static void odb_source_packed_free(struct odb_source *source)
{
struct odb_source_packed *packed = odb_source_packed_downcast(source);
@@ -42,6 +57,7 @@ struct odb_source_packed *odb_source_packed_new(struct odb_source_files *parent)
strmap_init(&packed->packs_by_path);
packed->base.free = odb_source_packed_free;
+ packed->base.close = odb_source_packed_close;
if (!is_absolute_path(parent->base.path))
chdir_notify_register(NULL, odb_source_packed_reparent, packed);
diff --git a/packfile.c b/packfile.c
index 6d492216de..e5386145a7 100644
--- a/packfile.c
+++ b/packfile.c
@@ -2749,18 +2749,6 @@ int parse_pack_header_option(const char *in, unsigned char *out, unsigned int *l
return 0;
}
-void packfile_store_close(struct odb_source_packed *store)
-{
- for (struct packfile_list_entry *e = store->packs.head; e; e = e->next) {
- if (e->pack->do_not_close)
- BUG("want to close pack marked 'do-not-close'");
- close_pack(e->pack);
- }
- if (store->midx)
- close_midx(store->midx);
- store->midx = NULL;
-}
-
struct odb_packed_read_stream {
struct odb_read_stream base;
struct packed_git *pack;
diff --git a/packfile.h b/packfile.h
index e8bc9349f8..9dc3a13112 100644
--- a/packfile.h
+++ b/packfile.h
@@ -55,12 +55,6 @@ struct packed_git {
char pack_name[FLEX_ARRAY]; /* more */
};
-/*
- * Close all packfiles associated with this store. The packfiles won't be
- * free'd, so they can be re-opened at a later point in time.
- */
-void packfile_store_close(struct odb_source_packed *store);
-
/*
* Prepare the packfile store by loading packfiles and multi-pack indices for
* all alternates. This becomes a no-op if the store is already prepared.
--
2.55.0.rc0.786.g65d90a0328.dirty
^ permalink raw reply related
* [PATCH v3 07/17] odb/source-packed: wire up `reprepare()` callback
From: Patrick Steinhardt @ 2026-06-17 6:39 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Justin Tobler
In-Reply-To: <20260617-pks-odb-source-packed-v3-0-b5c7583cd795@pks.im>
Move the logic to prepare and reprepare the "packed" source into
"odb/source-packed.c" and wire it up as the `reprepare()` callback.
Note that "preparing" a source is not yet generic. Eventually, it would
probably make sense to turn the existing `reprepare()` callback into a
`prepare()` callback with an optional flag to force re-preparing. But
this step will be handled in a separate patch series.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/grep.c | 2 +-
midx.c | 2 +-
odb/source-files.c | 2 +-
odb/source-packed.c | 157 +++++++++++++++++++++++++++++++++++++++++++++++++++
odb/source-packed.h | 9 +++
packfile.c | 160 +---------------------------------------------------
packfile.h | 17 ------
7 files changed, 172 insertions(+), 177 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 6a09571903..8080d1bf5e 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -1363,7 +1363,7 @@ int cmd_grep(int argc,
odb_prepare_alternates(the_repository->objects);
for (source = the_repository->objects->sources; source; source = source->next) {
struct odb_source_files *files = odb_source_files_downcast(source);
- packfile_store_prepare(files->packed);
+ odb_source_packed_prepare(files->packed);
}
}
diff --git a/midx.c b/midx.c
index efbfbb13f4..00bbd137b2 100644
--- a/midx.c
+++ b/midx.c
@@ -102,7 +102,7 @@ static int midx_read_object_offsets(const unsigned char *chunk_start,
struct multi_pack_index *get_multi_pack_index(struct odb_source *source)
{
struct odb_source_files *files = odb_source_files_downcast(source);
- packfile_store_prepare(files->packed);
+ odb_source_packed_prepare(files->packed);
return files->packed->midx;
}
diff --git a/odb/source-files.c b/odb/source-files.c
index 9b0fa9ccdc..7b1e0ac565 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -45,7 +45,7 @@ static void odb_source_files_reprepare(struct odb_source *source)
{
struct odb_source_files *files = odb_source_files_downcast(source);
odb_source_reprepare(&files->loose->base);
- packfile_store_reprepare(files->packed);
+ odb_source_reprepare(&files->packed->base);
}
static int odb_source_files_read_object_info(struct odb_source *source,
diff --git a/odb/source-packed.c b/odb/source-packed.c
index 74805be1dd..e8e2e5bb48 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -1,10 +1,166 @@
#include "git-compat-util.h"
#include "abspath.h"
#include "chdir-notify.h"
+#include "dir.h"
+#include "mergesort.h"
#include "midx.h"
#include "odb/source-packed.h"
#include "packfile.h"
+void (*report_garbage)(unsigned seen_bits, const char *path);
+
+static void report_helper(const struct string_list *list,
+ int seen_bits, int first, int last)
+{
+ if (seen_bits == (PACKDIR_FILE_PACK|PACKDIR_FILE_IDX))
+ return;
+
+ for (; first < last; first++)
+ report_garbage(seen_bits, list->items[first].string);
+}
+
+static void report_pack_garbage(struct string_list *list)
+{
+ int baselen = -1, first = 0, seen_bits = 0;
+
+ if (!report_garbage)
+ return;
+
+ string_list_sort(list);
+
+ for (size_t i = 0; i < list->nr; i++) {
+ const char *path = list->items[i].string;
+ if (baselen != -1 &&
+ strncmp(path, list->items[first].string, baselen)) {
+ report_helper(list, seen_bits, first, i);
+ baselen = -1;
+ seen_bits = 0;
+ }
+ if (baselen == -1) {
+ const char *dot = strrchr(path, '.');
+ if (!dot) {
+ report_garbage(PACKDIR_FILE_GARBAGE, path);
+ continue;
+ }
+ baselen = dot - path + 1;
+ first = i;
+ }
+ if (!strcmp(path + baselen, "pack"))
+ seen_bits |= 1;
+ else if (!strcmp(path + baselen, "idx"))
+ seen_bits |= 2;
+ }
+ report_helper(list, seen_bits, first, list->nr);
+}
+
+struct prepare_pack_data {
+ struct odb_source *source;
+ struct string_list *garbage;
+};
+
+static void prepare_pack(const char *full_name, size_t full_name_len,
+ const char *file_name, void *_data)
+{
+ struct prepare_pack_data *data = (struct prepare_pack_data *)_data;
+ struct odb_source_files *files = odb_source_files_downcast(data->source);
+ size_t base_len = full_name_len;
+
+ if (strip_suffix_mem(full_name, &base_len, ".idx") &&
+ !(files->packed->midx &&
+ midx_contains_pack(files->packed->midx, file_name))) {
+ char *trimmed_path = xstrndup(full_name, full_name_len);
+ packfile_store_load_pack(files->packed,
+ trimmed_path, data->source->local);
+ free(trimmed_path);
+ }
+
+ if (!report_garbage)
+ return;
+
+ if (!strcmp(file_name, "multi-pack-index") ||
+ !strcmp(file_name, "multi-pack-index.d"))
+ return;
+ if (starts_with(file_name, "multi-pack-index") &&
+ (ends_with(file_name, ".bitmap") || ends_with(file_name, ".rev")))
+ return;
+ if (ends_with(file_name, ".idx") ||
+ ends_with(file_name, ".rev") ||
+ ends_with(file_name, ".pack") ||
+ ends_with(file_name, ".bitmap") ||
+ ends_with(file_name, ".keep") ||
+ ends_with(file_name, ".promisor") ||
+ ends_with(file_name, ".mtimes"))
+ string_list_append(data->garbage, full_name);
+ else
+ report_garbage(PACKDIR_FILE_GARBAGE, full_name);
+}
+
+static void prepare_packed_git_one(struct odb_source *source)
+{
+ struct string_list garbage = STRING_LIST_INIT_DUP;
+ struct prepare_pack_data data = {
+ .source = source,
+ .garbage = &garbage,
+ };
+
+ for_each_file_in_pack_dir(source->path, prepare_pack, &data);
+
+ report_pack_garbage(data.garbage);
+ string_list_clear(data.garbage, 0);
+}
+
+DEFINE_LIST_SORT(static, sort_packs, struct packfile_list_entry, next);
+
+static int sort_pack(const struct packfile_list_entry *a,
+ const struct packfile_list_entry *b)
+{
+ int st;
+
+ /*
+ * Local packs tend to contain objects specific to our
+ * variant of the project than remote ones. In addition,
+ * remote ones could be on a network mounted filesystem.
+ * Favor local ones for these reasons.
+ */
+ st = a->pack->pack_local - b->pack->pack_local;
+ if (st)
+ return -st;
+
+ /*
+ * Younger packs tend to contain more recent objects,
+ * and more recent objects tend to get accessed more
+ * often.
+ */
+ if (a->pack->mtime < b->pack->mtime)
+ return 1;
+ else if (a->pack->mtime == b->pack->mtime)
+ return 0;
+ return -1;
+}
+
+void odb_source_packed_prepare(struct odb_source_packed *source)
+{
+ if (source->initialized)
+ return;
+
+ prepare_multi_pack_index_one(&source->files->base);
+ prepare_packed_git_one(&source->files->base);
+
+ sort_packs(&source->packs.head, sort_pack);
+ for (struct packfile_list_entry *e = source->packs.head; e; e = e->next)
+ if (!e->next)
+ source->packs.tail = e;
+
+ source->initialized = true;
+}
+
+static void odb_source_packed_reprepare(struct odb_source *source)
+{
+ struct odb_source_packed *packed = odb_source_packed_downcast(source);
+ packed->initialized = false;
+ odb_source_packed_prepare(packed);
+}
+
static void odb_source_packed_reparent(const char *name UNUSED,
const char *old_cwd,
const char *new_cwd,
@@ -58,6 +214,7 @@ struct odb_source_packed *odb_source_packed_new(struct odb_source_files *parent)
packed->base.free = odb_source_packed_free;
packed->base.close = odb_source_packed_close;
+ packed->base.reprepare = odb_source_packed_reprepare;
if (!is_absolute_path(parent->base.path))
chdir_notify_register(NULL, odb_source_packed_reparent, packed);
diff --git a/odb/source-packed.h b/odb/source-packed.h
index 68e64cabab..9d4796261a 100644
--- a/odb/source-packed.h
+++ b/odb/source-packed.h
@@ -81,4 +81,13 @@ static inline struct odb_source_packed *odb_source_packed_downcast(struct odb_so
return container_of(source, struct odb_source_packed, base);
}
+/*
+ * Prepare the source by loading packfiles and multi-pack indices for
+ * all alternates. This becomes a no-op if the source is already prepared.
+ *
+ * It shouldn't typically be necessary to call this function directly, as
+ * functions that access the source know to prepare it.
+ */
+void odb_source_packed_prepare(struct odb_source_packed *source);
+
#endif
diff --git a/packfile.c b/packfile.c
index e5386145a7..65631f674f 100644
--- a/packfile.c
+++ b/packfile.c
@@ -8,7 +8,6 @@
#include "pack.h"
#include "repository.h"
#include "dir.h"
-#include "mergesort.h"
#include "packfile.h"
#include "delta.h"
#include "hash-lookup.h"
@@ -812,52 +811,6 @@ struct packed_git *packfile_store_load_pack(struct odb_source_packed *store,
return p;
}
-void (*report_garbage)(unsigned seen_bits, const char *path);
-
-static void report_helper(const struct string_list *list,
- int seen_bits, int first, int last)
-{
- if (seen_bits == (PACKDIR_FILE_PACK|PACKDIR_FILE_IDX))
- return;
-
- for (; first < last; first++)
- report_garbage(seen_bits, list->items[first].string);
-}
-
-static void report_pack_garbage(struct string_list *list)
-{
- int i, baselen = -1, first = 0, seen_bits = 0;
-
- if (!report_garbage)
- return;
-
- string_list_sort(list);
-
- for (i = 0; i < list->nr; i++) {
- const char *path = list->items[i].string;
- if (baselen != -1 &&
- strncmp(path, list->items[first].string, baselen)) {
- report_helper(list, seen_bits, first, i);
- baselen = -1;
- seen_bits = 0;
- }
- if (baselen == -1) {
- const char *dot = strrchr(path, '.');
- if (!dot) {
- report_garbage(PACKDIR_FILE_GARBAGE, path);
- continue;
- }
- baselen = dot - path + 1;
- first = i;
- }
- if (!strcmp(path + baselen, "pack"))
- seen_bits |= 1;
- else if (!strcmp(path + baselen, "idx"))
- seen_bits |= 2;
- }
- report_helper(list, seen_bits, first, list->nr);
-}
-
void for_each_file_in_pack_subdir(const char *objdir,
const char *subdir,
each_file_in_pack_dir_fn fn,
@@ -900,116 +853,9 @@ void for_each_file_in_pack_dir(const char *objdir,
for_each_file_in_pack_subdir(objdir, NULL, fn, data);
}
-struct prepare_pack_data {
- struct odb_source *source;
- struct string_list *garbage;
-};
-
-static void prepare_pack(const char *full_name, size_t full_name_len,
- const char *file_name, void *_data)
-{
- struct prepare_pack_data *data = (struct prepare_pack_data *)_data;
- struct odb_source_files *files = odb_source_files_downcast(data->source);
- size_t base_len = full_name_len;
-
- if (strip_suffix_mem(full_name, &base_len, ".idx") &&
- !(files->packed->midx &&
- midx_contains_pack(files->packed->midx, file_name))) {
- char *trimmed_path = xstrndup(full_name, full_name_len);
- packfile_store_load_pack(files->packed,
- trimmed_path, data->source->local);
- free(trimmed_path);
- }
-
- if (!report_garbage)
- return;
-
- if (!strcmp(file_name, "multi-pack-index") ||
- !strcmp(file_name, "multi-pack-index.d"))
- return;
- if (starts_with(file_name, "multi-pack-index") &&
- (ends_with(file_name, ".bitmap") || ends_with(file_name, ".rev")))
- return;
- if (ends_with(file_name, ".idx") ||
- ends_with(file_name, ".rev") ||
- ends_with(file_name, ".pack") ||
- ends_with(file_name, ".bitmap") ||
- ends_with(file_name, ".keep") ||
- ends_with(file_name, ".promisor") ||
- ends_with(file_name, ".mtimes"))
- string_list_append(data->garbage, full_name);
- else
- report_garbage(PACKDIR_FILE_GARBAGE, full_name);
-}
-
-static void prepare_packed_git_one(struct odb_source *source)
-{
- struct string_list garbage = STRING_LIST_INIT_DUP;
- struct prepare_pack_data data = {
- .source = source,
- .garbage = &garbage,
- };
-
- for_each_file_in_pack_dir(source->path, prepare_pack, &data);
-
- report_pack_garbage(data.garbage);
- string_list_clear(data.garbage, 0);
-}
-
-DEFINE_LIST_SORT(static, sort_packs, struct packfile_list_entry, next);
-
-static int sort_pack(const struct packfile_list_entry *a,
- const struct packfile_list_entry *b)
-{
- int st;
-
- /*
- * Local packs tend to contain objects specific to our
- * variant of the project than remote ones. In addition,
- * remote ones could be on a network mounted filesystem.
- * Favor local ones for these reasons.
- */
- st = a->pack->pack_local - b->pack->pack_local;
- if (st)
- return -st;
-
- /*
- * Younger packs tend to contain more recent objects,
- * and more recent objects tend to get accessed more
- * often.
- */
- if (a->pack->mtime < b->pack->mtime)
- return 1;
- else if (a->pack->mtime == b->pack->mtime)
- return 0;
- return -1;
-}
-
-void packfile_store_prepare(struct odb_source_packed *store)
-{
- if (store->initialized)
- return;
-
- prepare_multi_pack_index_one(&store->files->base);
- prepare_packed_git_one(&store->files->base);
-
- sort_packs(&store->packs.head, sort_pack);
- for (struct packfile_list_entry *e = store->packs.head; e; e = e->next)
- if (!e->next)
- store->packs.tail = e;
-
- store->initialized = true;
-}
-
-void packfile_store_reprepare(struct odb_source_packed *store)
-{
- store->initialized = false;
- packfile_store_prepare(store);
-}
-
struct packfile_list_entry *packfile_store_get_packs(struct odb_source_packed *store)
{
- packfile_store_prepare(store);
+ odb_source_packed_prepare(store);
if (store->midx) {
struct multi_pack_index *m = store->midx;
@@ -2083,7 +1929,7 @@ static int find_pack_entry(struct odb_source_packed *store,
{
struct packfile_list_entry *l;
- packfile_store_prepare(store);
+ odb_source_packed_prepare(store);
if (store->midx && fill_midx_entry(store->midx, oid, e))
return 1;
@@ -2130,7 +1976,7 @@ int packfile_store_read_object_info(struct odb_source_packed *store,
* been added since the last time we have prepared the packfile store.
*/
if (flags & OBJECT_INFO_SECOND_READ)
- packfile_store_reprepare(store);
+ odb_source_reprepare(&store->base);
if (!find_pack_entry(store, oid, &e))
return 1;
diff --git a/packfile.h b/packfile.h
index 9dc3a13112..9674e573ae 100644
--- a/packfile.h
+++ b/packfile.h
@@ -55,23 +55,6 @@ struct packed_git {
char pack_name[FLEX_ARRAY]; /* more */
};
-/*
- * Prepare the packfile store by loading packfiles and multi-pack indices for
- * all alternates. This becomes a no-op if the store is already prepared.
- *
- * It shouldn't typically be necessary to call this function directly, as
- * functions that access the store know to prepare it.
- */
-void packfile_store_prepare(struct odb_source_packed *store);
-
-/*
- * Clear the packfile caches and try to look up any new packfiles that have
- * appeared since last preparing the packfiles store.
- *
- * This function must be called under the `odb_read_lock()`.
- */
-void packfile_store_reprepare(struct odb_source_packed *store);
-
/*
* Add the pack to the store so that contained objects become accessible via
* the store. This moves ownership into the store.
--
2.55.0.rc0.786.g65d90a0328.dirty
^ permalink raw reply related
* [PATCH v3 08/17] packfile: use higher-level interface to implement `has_object_pack()`
From: Patrick Steinhardt @ 2026-06-17 6:39 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, Justin Tobler
In-Reply-To: <20260617-pks-odb-source-packed-v3-0-b5c7583cd795@pks.im>
In `has_object_pack()` we're checking whether a specific object exists
as part of a packfile. This is done by calling the low-level function
`find_pack_entry()`, but this function will eventually be moved into
"odb/source-packed.c" and made file-local.
Refactor the code to use `packfile_store_read_object_info()` instead.
This refactoring is functionally equivalent as that function will call
`find_pack_entry()` itself and then return immediately when it ain't got
no object info pointer as parameter.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
packfile.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/packfile.c b/packfile.c
index 65631f674f..b35afd7797 100644
--- a/packfile.c
+++ b/packfile.c
@@ -2049,14 +2049,12 @@ struct packed_git **packfile_store_get_kept_pack_cache(struct odb_source_packed
int has_object_pack(struct repository *r, const struct object_id *oid)
{
struct odb_source *source;
- struct pack_entry e;
odb_prepare_alternates(r->objects);
for (source = r->objects->sources; source; source = source->next) {
struct odb_source_files *files = odb_source_files_downcast(source);
- int ret = find_pack_entry(files->packed, oid, &e);
- if (ret)
- return ret;
+ if (!packfile_store_read_object_info(files->packed, oid, NULL, 0))
+ return 1;
}
return 0;
--
2.55.0.rc0.786.g65d90a0328.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