Re: [PATCH v1] repository: move fetch_if_missing into struct repository
From: Patrick Steinhardt <hidden>
Date: 2026-07-15 06:35:53
On Wed, Jul 15, 2026 at 09:18:50AM +0800, Tian Yuchen wrote:
The global variable 'fetch_if_missing' controls whether a missing object check should prompt a lazy fetch from a promisor remote. In order to continue the libification effort, move it into 'struct repository' and initialize it to 1 by default to keep the previous behavior.
Right. I was also thinking about moving this into a non-global scope multiple times. I was approaching this a bit differently though: it's ultimately a property of the object database whether or not we want to accept missing objects, so I moved it in there instead. I don't really think there's a downside with your version, though. Quite on the contrary: we can really only perform the backfill fetches with a whole repository at hand anyway. So conceptually your version might even be more sensible.
Subsystems that already pass around a repository pointer, are updated to read this flag directly from their respective 'repo' instances. For the rest, we access 'the_repository'. Note that in builtin/fsck.c and builtin/index-pack.c, when running related commands with the '-h' parameter, the 'repo' pointer is not passed in. To prevent null pointer dereferences, we defer operations on the repo in until after parameter parsing is complete.
s/on the repo in/on the repo/
quoted hunk ↗ jump to hunk
diff --git a/builtin/index-pack.c b/builtin/index-pack.c index 0793dc595c..721d576938 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c@@ -1898,15 +1898,16 @@ int cmd_index_pack(int argc, int report_end_of_input = 0; int hash_algo = 0; + show_usage_if_asked(argc, argv, index_pack_usage); + /* * index-pack never needs to fetch missing objects except when * REF_DELTA bases are missing (which are explicitly handled). It only * accesses the repo to do hash collision checks and to check which * REF_DELTA bases need to be fetched. */ - fetch_if_missing = 0; - - show_usage_if_asked(argc, argv, index_pack_usage); + if (repo) + repo->fetch_if_missing = 0; disable_replace_refs();
Okay. This command can run without a repository, in which case we'll end up just indexing the pack. My assumption is that we'll probably end up using `the_repository` if so, as we still use `the_repository` in this file. So could this here cause a change in behaviour? If the answer is "maybe" I'd propose that we simply continue to use `the_repository` here.
quoted hunk ↗ jump to hunk
diff --git a/revision.c b/revision.c index e91d7e1f11..bb645654c3 100644 --- a/revision.c +++ b/revision.c@@ -2714,7 +2714,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg revs->ignore_missing = 1; } else if (opt && opt->allow_exclude_promisor_objects && !strcmp(arg, "--exclude-promisor-objects")) { - if (fetch_if_missing) + if (revs->repo->fetch_if_missing) BUG("exclude_promisor_objects can only be used when fetch_if_missing is 0"); revs->exclude_promisor_objects = 1; } else {
This one here also makes me wonder whether it could cause weird interactions in case a caller passes a repository other than `the_repository`. It ideally _shouldn't_, but it's hard to tell because we still use `the_repository` in lots of places here. Thanks! Patrick