Git development
 help / color / mirror / Atom feed
* [PATCH v1] repository: move fetch_if_missing into struct repository
@ 2026-07-15  1:18 Tian Yuchen
  2026-07-15  3:27 ` Junio C Hamano
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Tian Yuchen @ 2026-07-15  1:18 UTC (permalink / raw)
  To: git
  Cc: ps, five231003, hariom18599, Tian Yuchen, Christian Couder,
	Ayush Chandekar, Olamide Caleb Bello

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.

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.

Additionally, update the partial clone documentation to reflect
that this is now a per-repository flag.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 Documentation/technical/partial-clone.adoc |  2 +-
 builtin/fetch-pack.c                       |  2 +-
 builtin/fsck.c                             |  6 +++---
 builtin/index-pack.c                       |  9 +++++----
 builtin/pack-objects.c                     | 14 +++++++-------
 builtin/prune.c                            |  2 +-
 builtin/rev-list.c                         | 10 +++++-----
 git.c                                      |  2 +-
 midx-write.c                               |  2 +-
 odb.c                                      |  4 +---
 odb.h                                      |  8 --------
 repository.c                               |  1 +
 repository.h                               |  6 ++++++
 revision.c                                 |  2 +-
 setup.c                                    |  2 +-
 15 files changed, 35 insertions(+), 37 deletions(-)

diff --git a/Documentation/technical/partial-clone.adoc b/Documentation/technical/partial-clone.adoc
index e513e391ea..18718a3840 100644
--- a/Documentation/technical/partial-clone.adoc
+++ b/Documentation/technical/partial-clone.adoc
@@ -159,7 +159,7 @@ and prefetch those objects in bulk.
 - `repack` in GC has been updated to not touch promisor packfiles at all,
   and to only repack other objects.
 
-- The global variable "fetch_if_missing" is used to control whether an
+- The per-repository flag "fetch_if_missing" is used to control whether an
   object lookup will attempt to dynamically fetch a missing object or
   report an error.
 +
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 316badd969..c5edd7b80f 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -67,7 +67,7 @@ int cmd_fetch_pack(int argc,
 	struct packet_reader reader;
 	enum protocol_version version;
 
-	fetch_if_missing = 0;
+	the_repository->fetch_if_missing = 0;
 
 	packet_trace_identity("fetch-pack");
 
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 248f8ff5a0..aa31c69486 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -1017,15 +1017,15 @@ int cmd_fsck(int argc,
 		.ref = NULL
 	};
 
-	/* fsck knows how to handle missing promisor objects */
-	fetch_if_missing = 0;
-
 	errors_found = 0;
 	disable_replace_refs();
 	save_commit_buffer = 0;
 
 	argc = parse_options(argc, argv, prefix, fsck_opts, fsck_usage, 0);
 
+	/* fsck knows how to handle missing promisor objects */
+	repo->fetch_if_missing = 0;
+
 	fsck_options_init(&fsck_walk_options, repo, FSCK_OPTIONS_DEFAULT);
 	fsck_walk_options.walk = mark_object;
 
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
@@ -1881,7 +1881,7 @@ static void repack_local_links(void)
 int cmd_index_pack(int argc,
 		   const char **argv,
 		   const char *prefix,
-		   struct repository *repo UNUSED)
+		   struct repository *repo)
 {
 	int i, fix_thin_pack = 0, verify = 0, stat_only = 0, rev_index;
 	const char *curr_index;
@@ -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();
 
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 8a1709a1ab..c6536b1f65 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -4059,7 +4059,7 @@ static void add_unreachable_loose_objects(struct rev_info *revs);
 
 static void read_stdin_packs(enum stdin_packs_mode mode, int rev_list_unpacked)
 {
-	int prev_fetch_if_missing = fetch_if_missing;
+	int prev_fetch_if_missing = the_repository->fetch_if_missing;
 	struct rev_info revs;
 
 	/*
@@ -4067,7 +4067,7 @@ static void read_stdin_packs(enum stdin_packs_mode mode, int rev_list_unpacked)
 	 * walk is best-effort though we don't want to perform backfill fetches
 	 * for them.
 	 */
-	fetch_if_missing = 0;
+	the_repository->fetch_if_missing = 0;
 
 	repo_init_revisions(the_repository, &revs, NULL);
 	/*
@@ -4115,7 +4115,7 @@ static void read_stdin_packs(enum stdin_packs_mode mode, int rev_list_unpacked)
 	trace2_data_intmax("pack-objects", the_repository, "stdin_packs_hints",
 			   stdin_packs_hints_nr);
 
-	fetch_if_missing = prev_fetch_if_missing;
+	the_repository->fetch_if_missing = prev_fetch_if_missing;
 }
 
 static void add_cruft_object_entry(const struct object_id *oid, enum object_type type,
@@ -4451,14 +4451,14 @@ static int option_parse_missing_action(const struct option *opt UNUSED,
 
 	if (!strcmp(arg, "allow-any")) {
 		arg_missing_action = MA_ALLOW_ANY;
-		fetch_if_missing = 0;
+		the_repository->fetch_if_missing = 0;
 		fn_show_object = show_object__ma_allow_any;
 		return 0;
 	}
 
 	if (!strcmp(arg, "allow-promisor")) {
 		arg_missing_action = MA_ALLOW_PROMISOR;
-		fetch_if_missing = 0;
+		the_repository->fetch_if_missing = 0;
 		fn_show_object = show_object__ma_allow_promisor;
 		return 0;
 	}
@@ -5247,7 +5247,7 @@ int cmd_pack_objects(int argc,
 				  exclude_promisor_objects_best_effort,
 				  "--exclude-promisor-objects-best-effort");
 	if (exclude_promisor_objects) {
-		fetch_if_missing = 0;
+		the_repository->fetch_if_missing = 0;
 
 		/* --stdin-packs handles promisor objects separately. */
 		if (!stdin_packs) {
@@ -5256,7 +5256,7 @@ int cmd_pack_objects(int argc,
 		}
 	} else if (exclude_promisor_objects_best_effort) {
 		use_internal_rev_list = 1;
-		fetch_if_missing = 0;
+		the_repository->fetch_if_missing = 0;
 		option_parse_missing_action(NULL, "allow-any", 0);
 		/* revs configured below */
 	}
diff --git a/builtin/prune.c b/builtin/prune.c
index 55635a891f..a7e4678d11 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -194,7 +194,7 @@ int cmd_prune(int argc,
 	if (show_progress == -1)
 		show_progress = isatty(2);
 	if (exclude_promisor_objects) {
-		fetch_if_missing = 0;
+		repo->fetch_if_missing = 0;
 		revs.exclude_promisor_objects = 1;
 	}
 
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 8f63003709..a6a0c5559e 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -509,25 +509,25 @@ static inline int parse_missing_action_value(const char *value)
 
 	if (!strcmp(value, "allow-any")) {
 		arg_missing_action = MA_ALLOW_ANY;
-		fetch_if_missing = 0;
+		the_repository->fetch_if_missing = 0;
 		return 1;
 	}
 
 	if (!strcmp(value, "print")) {
 		arg_missing_action = MA_PRINT;
-		fetch_if_missing = 0;
+		the_repository->fetch_if_missing = 0;
 		return 1;
 	}
 
 	if (!strcmp(value, "print-info")) {
 		arg_missing_action = MA_PRINT_INFO;
-		fetch_if_missing = 0;
+		the_repository->fetch_if_missing = 0;
 		return 1;
 	}
 
 	if (!strcmp(value, "allow-promisor")) {
 		arg_missing_action = MA_ALLOW_PROMISOR;
-		fetch_if_missing = 0;
+		the_repository->fetch_if_missing = 0;
 		return 1;
 	}
 
@@ -745,7 +745,7 @@ int cmd_rev_list(int argc,
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
 		if (!strcmp(arg, "--exclude-promisor-objects")) {
-			fetch_if_missing = 0;
+			the_repository->fetch_if_missing = 0;
 			revs.exclude_promisor_objects = 1;
 		} else if (skip_prefix(arg, "--missing=", &arg)) {
 			parse_missing_action_value(arg);
diff --git a/git.c b/git.c
index 36f08891ef..315d2e160e 100644
--- a/git.c
+++ b/git.c
@@ -202,7 +202,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 			if (envchanged)
 				*envchanged = 1;
 		} else if (!strcmp(cmd, "--no-lazy-fetch")) {
-			fetch_if_missing = 0;
+			the_repository->fetch_if_missing = 0;
 			setenv(NO_LAZY_FETCH_ENVIRONMENT, "1", 1);
 			if (envchanged)
 				*envchanged = 1;
diff --git a/midx-write.c b/midx-write.c
index 19e1cd10b7..e7313c9d2c 100644
--- a/midx-write.c
+++ b/midx-write.c
@@ -865,7 +865,7 @@ static void find_commits_for_midx_bitmap(struct commit_stack *commits,
 	 * complain later that we don't have reachability closure (and fail
 	 * appropriately).
 	 */
-	fetch_if_missing = 0;
+	ctx->repo->fetch_if_missing = 0;
 	revs.exclude_promisor_objects = 1;
 
 	if (prepare_revision_walk(&revs))
diff --git a/odb.c b/odb.c
index 965ef68e4e..664256e1a4 100644
--- a/odb.c
+++ b/odb.c
@@ -528,8 +528,6 @@ void disable_obj_read_lock(void)
 	pthread_mutex_destroy(&obj_read_mutex);
 }
 
-int fetch_if_missing = 1;
-
 static int register_all_submodule_sources(struct object_database *odb)
 {
 	int ret = odb->submodule_source_paths.nr;
@@ -595,7 +593,7 @@ static int do_oid_object_info_extended(struct object_database *odb,
 			continue;
 
 		/* Check if it is a missing object */
-		if (fetch_if_missing && repo_has_promisor_remote(odb->repo) &&
+		if (odb->repo->fetch_if_missing && repo_has_promisor_remote(odb->repo) &&
 		    !already_retried &&
 		    !(flags & OBJECT_INFO_SKIP_FETCH_OBJECT)) {
 			promisor_remote_get_direct(odb->repo, real, 1);
diff --git a/odb.h b/odb.h
index 0030467a52..1dca583fcb 100644
--- a/odb.h
+++ b/odb.h
@@ -14,14 +14,6 @@ struct repository;
 struct strbuf;
 struct strvec;
 
-/*
- * Set this to 0 to prevent odb_read_object_info_extended() from fetching missing
- * blobs. This has a difference only if extensions.partialClone is set.
- *
- * Its default value is 1.
- */
-extern int fetch_if_missing;
-
 /*
  * Compute the exact path an alternate is at and returns it. In case of
  * error NULL is returned and the human readable error is added to `err`
diff --git a/repository.c b/repository.c
index 187dd471c4..b959f7a028 100644
--- a/repository.c
+++ b/repository.c
@@ -73,6 +73,7 @@ void initialize_repository(struct repository *repo)
 	ALLOC_ARRAY(repo->index, 1);
 	index_state_init(repo->index, repo);
 	repo->check_deprecated_config = true;
+	repo->fetch_if_missing = 1;
 	repo_config_values_init(&repo->config_values_private_);
 
 	/*
diff --git a/repository.h b/repository.h
index 36e2db2633..e8bd6ef0e7 100644
--- a/repository.h
+++ b/repository.h
@@ -169,6 +169,12 @@ struct repository {
 	/* True if commit-graph has been disabled within this process. */
 	int commit_graph_disabled;
 
+	/*
+	 * Controls whether the repository should lazily fetch missing
+	 * objects from promisor remotes. Defaults to 1.
+	 */
+	int fetch_if_missing;
+
 	/*
 	 * Lazily-populated cache mapping hook event names to configured hooks.
 	 * NULL until first hook use.
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 {
diff --git a/setup.c b/setup.c
index b4652651df..ce2a80ac31 100644
--- a/setup.c
+++ b/setup.c
@@ -1064,7 +1064,7 @@ static void setup_git_env_internal(struct repository *repo,
 		set_alternate_shallow_file(repo, shallow_file, 0);
 
 	if (git_env_bool(NO_LAZY_FETCH_ENVIRONMENT, 0))
-		fetch_if_missing = 0;
+		the_repository->fetch_if_missing = 0;
 }
 
 static void set_git_dir_1(struct repository *repo, const char *path)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v1] repository: move fetch_if_missing into struct repository
  2026-07-15  1:18 [PATCH v1] repository: move fetch_if_missing into struct repository Tian Yuchen
@ 2026-07-15  3:27 ` Junio C Hamano
  2026-07-15  4:58   ` Tian Yuchen
  2026-07-15  6:35 ` Patrick Steinhardt
  2026-07-16  7:29 ` [PATCH v2] " Tian Yuchen
  2 siblings, 1 reply; 14+ messages in thread
From: Junio C Hamano @ 2026-07-15  3:27 UTC (permalink / raw)
  To: Tian Yuchen
  Cc: git, ps, five231003, hariom18599, Christian Couder,
	Ayush Chandekar, Olamide Caleb Bello

Tian Yuchen <cat@malon.dev> writes:

> 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.
> ...
> diff --git a/setup.c b/setup.c
> index b4652651df..ce2a80ac31 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -1064,7 +1064,7 @@ static void setup_git_env_internal(struct repository *repo,
>  		set_alternate_shallow_file(repo, shallow_file, 0);
>  
>  	if (git_env_bool(NO_LAZY_FETCH_ENVIRONMENT, 0))
> -		fetch_if_missing = 0;
> +		the_repository->fetch_if_missing = 0;
>  }

Could a caller pass a "repo" that is not the_repository?  In other
words, shouldn't this be

		repo->fetch_if_missing = 0;

instead?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1] repository: move fetch_if_missing into struct repository
  2026-07-15  3:27 ` Junio C Hamano
@ 2026-07-15  4:58   ` Tian Yuchen
  0 siblings, 0 replies; 14+ messages in thread
From: Tian Yuchen @ 2026-07-15  4:58 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, ps, five231003, hariom18599, Christian Couder,
	Ayush Chandekar, Olamide Caleb Bello

On 7/15/26 11:27, Junio C Hamano wrote:
> Tian Yuchen <cat@malon.dev> writes:
> 
>> 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.
>> ...
>> diff --git a/setup.c b/setup.c
>> index b4652651df..ce2a80ac31 100644
>> --- a/setup.c
>> +++ b/setup.c
>> @@ -1064,7 +1064,7 @@ static void setup_git_env_internal(struct repository *repo,
>>   		set_alternate_shallow_file(repo, shallow_file, 0);
>>   
>>   	if (git_env_bool(NO_LAZY_FETCH_ENVIRONMENT, 0))
>> -		fetch_if_missing = 0;
>> +		the_repository->fetch_if_missing = 0;
>>   }
> 
> Could a caller pass a "repo" that is not the_repository?  In other
> words, shouldn't this be
> 
> 		repo->fetch_if_missing = 0;
> 
> instead?

Thanks, will change in the next reroll!

Regards, yuchen

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1] repository: move fetch_if_missing into struct repository
  2026-07-15  1:18 [PATCH v1] repository: move fetch_if_missing into struct repository Tian Yuchen
  2026-07-15  3:27 ` Junio C Hamano
@ 2026-07-15  6:35 ` Patrick Steinhardt
  2026-07-16  7:06   ` Tian Yuchen
  2026-07-16 15:28   ` Junio C Hamano
  2026-07-16  7:29 ` [PATCH v2] " Tian Yuchen
  2 siblings, 2 replies; 14+ messages in thread
From: Patrick Steinhardt @ 2026-07-15  6:35 UTC (permalink / raw)
  To: Tian Yuchen
  Cc: git, five231003, hariom18599, Christian Couder, Ayush Chandekar,
	Olamide Caleb Bello

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/

> 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.

> 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1] repository: move fetch_if_missing into struct repository
  2026-07-15  6:35 ` Patrick Steinhardt
@ 2026-07-16  7:06   ` Tian Yuchen
  2026-07-16 15:28   ` Junio C Hamano
  1 sibling, 0 replies; 14+ messages in thread
From: Tian Yuchen @ 2026-07-16  7:06 UTC (permalink / raw)
  To: Patrick Steinhardt
  Cc: git, five231003, hariom18599, Christian Couder, Ayush Chandekar,
	Olamide Caleb Bello

On 7/15/26 14:35, Patrick Steinhardt wrote:
> 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/
> 
>> 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?

Hummm...

> 
> If the answer is "maybe" I'd propose that we simply continue to use
> `the_repository` here.
> 
>> 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.
> 

This makes sense to me. Let's use the_repository then.

> Thanks!
> 
> Patrick

Regards, yuchen

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2] repository: move fetch_if_missing into struct repository
  2026-07-15  1:18 [PATCH v1] repository: move fetch_if_missing into struct repository Tian Yuchen
  2026-07-15  3:27 ` Junio C Hamano
  2026-07-15  6:35 ` Patrick Steinhardt
@ 2026-07-16  7:29 ` Tian Yuchen
  2026-08-01 15:53   ` Tian Yuchen
                     ` (2 more replies)
  2 siblings, 3 replies; 14+ messages in thread
From: Tian Yuchen @ 2026-07-16  7:29 UTC (permalink / raw)
  To: git
  Cc: ps, five231003, hariom18599, Tian Yuchen, Christian Couder,
	Ayush Chandekar, Olamide Caleb Bello

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.

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 until after parameter parsing is complete.

Additionally, update the partial clone documentation to reflect
that this is now a per-repository flag.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---

Change since V1:

- Following Patrick's advice, use the_repository whenever possible
  without re-introducing #define USE_THE_REPOSITORY_VARIABLE.

 Documentation/technical/partial-clone.adoc |  2 +-
 builtin/fetch-pack.c                       |  2 +-
 builtin/fsck.c                             |  6 +++---
 builtin/index-pack.c                       |  7 ++++---
 builtin/pack-objects.c                     | 14 +++++++-------
 builtin/prune.c                            |  2 +-
 builtin/rev-list.c                         | 10 +++++-----
 git.c                                      |  2 +-
 midx-write.c                               |  2 +-
 odb.c                                      |  4 +---
 odb.h                                      |  8 --------
 repository.c                               |  1 +
 repository.h                               |  6 ++++++
 revision.c                                 |  2 +-
 setup.c                                    |  2 +-
 15 files changed, 34 insertions(+), 36 deletions(-)

diff --git a/Documentation/technical/partial-clone.adoc b/Documentation/technical/partial-clone.adoc
index e513e391ea..18718a3840 100644
--- a/Documentation/technical/partial-clone.adoc
+++ b/Documentation/technical/partial-clone.adoc
@@ -159,7 +159,7 @@ and prefetch those objects in bulk.
 - `repack` in GC has been updated to not touch promisor packfiles at all,
   and to only repack other objects.
 
-- The global variable "fetch_if_missing" is used to control whether an
+- The per-repository flag "fetch_if_missing" is used to control whether an
   object lookup will attempt to dynamically fetch a missing object or
   report an error.
 +
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 316badd969..c5edd7b80f 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -67,7 +67,7 @@ int cmd_fetch_pack(int argc,
 	struct packet_reader reader;
 	enum protocol_version version;
 
-	fetch_if_missing = 0;
+	the_repository->fetch_if_missing = 0;
 
 	packet_trace_identity("fetch-pack");
 
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 248f8ff5a0..aa31c69486 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -1017,15 +1017,15 @@ int cmd_fsck(int argc,
 		.ref = NULL
 	};
 
-	/* fsck knows how to handle missing promisor objects */
-	fetch_if_missing = 0;
-
 	errors_found = 0;
 	disable_replace_refs();
 	save_commit_buffer = 0;
 
 	argc = parse_options(argc, argv, prefix, fsck_opts, fsck_usage, 0);
 
+	/* fsck knows how to handle missing promisor objects */
+	repo->fetch_if_missing = 0;
+
 	fsck_options_init(&fsck_walk_options, repo, FSCK_OPTIONS_DEFAULT);
 	fsck_walk_options.walk = mark_object;
 
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 0793dc595c..74f9694662 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)
+		the_repository->fetch_if_missing = 0;
 
 	disable_replace_refs();
 
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 8a1709a1ab..c6536b1f65 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -4059,7 +4059,7 @@ static void add_unreachable_loose_objects(struct rev_info *revs);
 
 static void read_stdin_packs(enum stdin_packs_mode mode, int rev_list_unpacked)
 {
-	int prev_fetch_if_missing = fetch_if_missing;
+	int prev_fetch_if_missing = the_repository->fetch_if_missing;
 	struct rev_info revs;
 
 	/*
@@ -4067,7 +4067,7 @@ static void read_stdin_packs(enum stdin_packs_mode mode, int rev_list_unpacked)
 	 * walk is best-effort though we don't want to perform backfill fetches
 	 * for them.
 	 */
-	fetch_if_missing = 0;
+	the_repository->fetch_if_missing = 0;
 
 	repo_init_revisions(the_repository, &revs, NULL);
 	/*
@@ -4115,7 +4115,7 @@ static void read_stdin_packs(enum stdin_packs_mode mode, int rev_list_unpacked)
 	trace2_data_intmax("pack-objects", the_repository, "stdin_packs_hints",
 			   stdin_packs_hints_nr);
 
-	fetch_if_missing = prev_fetch_if_missing;
+	the_repository->fetch_if_missing = prev_fetch_if_missing;
 }
 
 static void add_cruft_object_entry(const struct object_id *oid, enum object_type type,
@@ -4451,14 +4451,14 @@ static int option_parse_missing_action(const struct option *opt UNUSED,
 
 	if (!strcmp(arg, "allow-any")) {
 		arg_missing_action = MA_ALLOW_ANY;
-		fetch_if_missing = 0;
+		the_repository->fetch_if_missing = 0;
 		fn_show_object = show_object__ma_allow_any;
 		return 0;
 	}
 
 	if (!strcmp(arg, "allow-promisor")) {
 		arg_missing_action = MA_ALLOW_PROMISOR;
-		fetch_if_missing = 0;
+		the_repository->fetch_if_missing = 0;
 		fn_show_object = show_object__ma_allow_promisor;
 		return 0;
 	}
@@ -5247,7 +5247,7 @@ int cmd_pack_objects(int argc,
 				  exclude_promisor_objects_best_effort,
 				  "--exclude-promisor-objects-best-effort");
 	if (exclude_promisor_objects) {
-		fetch_if_missing = 0;
+		the_repository->fetch_if_missing = 0;
 
 		/* --stdin-packs handles promisor objects separately. */
 		if (!stdin_packs) {
@@ -5256,7 +5256,7 @@ int cmd_pack_objects(int argc,
 		}
 	} else if (exclude_promisor_objects_best_effort) {
 		use_internal_rev_list = 1;
-		fetch_if_missing = 0;
+		the_repository->fetch_if_missing = 0;
 		option_parse_missing_action(NULL, "allow-any", 0);
 		/* revs configured below */
 	}
diff --git a/builtin/prune.c b/builtin/prune.c
index 55635a891f..a7e4678d11 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -194,7 +194,7 @@ int cmd_prune(int argc,
 	if (show_progress == -1)
 		show_progress = isatty(2);
 	if (exclude_promisor_objects) {
-		fetch_if_missing = 0;
+		repo->fetch_if_missing = 0;
 		revs.exclude_promisor_objects = 1;
 	}
 
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 8f63003709..a6a0c5559e 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -509,25 +509,25 @@ static inline int parse_missing_action_value(const char *value)
 
 	if (!strcmp(value, "allow-any")) {
 		arg_missing_action = MA_ALLOW_ANY;
-		fetch_if_missing = 0;
+		the_repository->fetch_if_missing = 0;
 		return 1;
 	}
 
 	if (!strcmp(value, "print")) {
 		arg_missing_action = MA_PRINT;
-		fetch_if_missing = 0;
+		the_repository->fetch_if_missing = 0;
 		return 1;
 	}
 
 	if (!strcmp(value, "print-info")) {
 		arg_missing_action = MA_PRINT_INFO;
-		fetch_if_missing = 0;
+		the_repository->fetch_if_missing = 0;
 		return 1;
 	}
 
 	if (!strcmp(value, "allow-promisor")) {
 		arg_missing_action = MA_ALLOW_PROMISOR;
-		fetch_if_missing = 0;
+		the_repository->fetch_if_missing = 0;
 		return 1;
 	}
 
@@ -745,7 +745,7 @@ int cmd_rev_list(int argc,
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
 		if (!strcmp(arg, "--exclude-promisor-objects")) {
-			fetch_if_missing = 0;
+			the_repository->fetch_if_missing = 0;
 			revs.exclude_promisor_objects = 1;
 		} else if (skip_prefix(arg, "--missing=", &arg)) {
 			parse_missing_action_value(arg);
diff --git a/git.c b/git.c
index 36f08891ef..315d2e160e 100644
--- a/git.c
+++ b/git.c
@@ -202,7 +202,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 			if (envchanged)
 				*envchanged = 1;
 		} else if (!strcmp(cmd, "--no-lazy-fetch")) {
-			fetch_if_missing = 0;
+			the_repository->fetch_if_missing = 0;
 			setenv(NO_LAZY_FETCH_ENVIRONMENT, "1", 1);
 			if (envchanged)
 				*envchanged = 1;
diff --git a/midx-write.c b/midx-write.c
index 19e1cd10b7..e7313c9d2c 100644
--- a/midx-write.c
+++ b/midx-write.c
@@ -865,7 +865,7 @@ static void find_commits_for_midx_bitmap(struct commit_stack *commits,
 	 * complain later that we don't have reachability closure (and fail
 	 * appropriately).
 	 */
-	fetch_if_missing = 0;
+	ctx->repo->fetch_if_missing = 0;
 	revs.exclude_promisor_objects = 1;
 
 	if (prepare_revision_walk(&revs))
diff --git a/odb.c b/odb.c
index 965ef68e4e..664256e1a4 100644
--- a/odb.c
+++ b/odb.c
@@ -528,8 +528,6 @@ void disable_obj_read_lock(void)
 	pthread_mutex_destroy(&obj_read_mutex);
 }
 
-int fetch_if_missing = 1;
-
 static int register_all_submodule_sources(struct object_database *odb)
 {
 	int ret = odb->submodule_source_paths.nr;
@@ -595,7 +593,7 @@ static int do_oid_object_info_extended(struct object_database *odb,
 			continue;
 
 		/* Check if it is a missing object */
-		if (fetch_if_missing && repo_has_promisor_remote(odb->repo) &&
+		if (odb->repo->fetch_if_missing && repo_has_promisor_remote(odb->repo) &&
 		    !already_retried &&
 		    !(flags & OBJECT_INFO_SKIP_FETCH_OBJECT)) {
 			promisor_remote_get_direct(odb->repo, real, 1);
diff --git a/odb.h b/odb.h
index 0030467a52..1dca583fcb 100644
--- a/odb.h
+++ b/odb.h
@@ -14,14 +14,6 @@ struct repository;
 struct strbuf;
 struct strvec;
 
-/*
- * Set this to 0 to prevent odb_read_object_info_extended() from fetching missing
- * blobs. This has a difference only if extensions.partialClone is set.
- *
- * Its default value is 1.
- */
-extern int fetch_if_missing;
-
 /*
  * Compute the exact path an alternate is at and returns it. In case of
  * error NULL is returned and the human readable error is added to `err`
diff --git a/repository.c b/repository.c
index 187dd471c4..b959f7a028 100644
--- a/repository.c
+++ b/repository.c
@@ -73,6 +73,7 @@ void initialize_repository(struct repository *repo)
 	ALLOC_ARRAY(repo->index, 1);
 	index_state_init(repo->index, repo);
 	repo->check_deprecated_config = true;
+	repo->fetch_if_missing = 1;
 	repo_config_values_init(&repo->config_values_private_);
 
 	/*
diff --git a/repository.h b/repository.h
index 36e2db2633..e8bd6ef0e7 100644
--- a/repository.h
+++ b/repository.h
@@ -169,6 +169,12 @@ struct repository {
 	/* True if commit-graph has been disabled within this process. */
 	int commit_graph_disabled;
 
+	/*
+	 * Controls whether the repository should lazily fetch missing
+	 * objects from promisor remotes. Defaults to 1.
+	 */
+	int fetch_if_missing;
+
 	/*
 	 * Lazily-populated cache mapping hook event names to configured hooks.
 	 * NULL until first hook use.
diff --git a/revision.c b/revision.c
index e91d7e1f11..5f70aa81e6 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 (the_repository->fetch_if_missing)
 			BUG("exclude_promisor_objects can only be used when fetch_if_missing is 0");
 		revs->exclude_promisor_objects = 1;
 	} else {
diff --git a/setup.c b/setup.c
index b4652651df..ce2a80ac31 100644
--- a/setup.c
+++ b/setup.c
@@ -1064,7 +1064,7 @@ static void setup_git_env_internal(struct repository *repo,
 		set_alternate_shallow_file(repo, shallow_file, 0);
 
 	if (git_env_bool(NO_LAZY_FETCH_ENVIRONMENT, 0))
-		fetch_if_missing = 0;
+		the_repository->fetch_if_missing = 0;
 }
 
 static void set_git_dir_1(struct repository *repo, const char *path)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v1] repository: move fetch_if_missing into struct repository
  2026-07-15  6:35 ` Patrick Steinhardt
  2026-07-16  7:06   ` Tian Yuchen
@ 2026-07-16 15:28   ` Junio C Hamano
  1 sibling, 0 replies; 14+ messages in thread
From: Junio C Hamano @ 2026-07-16 15:28 UTC (permalink / raw)
  To: Patrick Steinhardt
  Cc: Tian Yuchen, git, five231003, hariom18599, Christian Couder,
	Ayush Chandekar, Olamide Caleb Bello

Patrick Steinhardt <ps@pks.im> writes:

>>  	/*
>>  	 * 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?

Meaning that even outside a repository, we could have read the
setting from ~/.gitconfig or some other places other than the
per-repository .git/config file?


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] repository: move fetch_if_missing into struct repository
  2026-07-16  7:29 ` [PATCH v2] " Tian Yuchen
@ 2026-08-01 15:53   ` Tian Yuchen
  2026-08-04  8:24   ` Patrick Steinhardt
  2026-08-07  9:41   ` [PATCH v3] " Tian Yuchen
  2 siblings, 0 replies; 14+ messages in thread
From: Tian Yuchen @ 2026-08-01 15:53 UTC (permalink / raw)
  To: git
  Cc: ps, five231003, hariom18599, Christian Couder, Ayush Chandekar,
	Olamide Caleb Bello

On 7/16/26 15:29, 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.
> 
> 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 until after parameter parsing is complete.
> 
> Additionally, update the partial clone documentation to reflect
> that this is now a per-repository flag.
> 
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
> Signed-off-by: Tian Yuchen <cat@malon.dev>
> ---
> 
> Change since V1:
> 
> - Following Patrick's advice, use the_repository whenever possible
>    without re-introducing #define USE_THE_REPOSITORY_VARIABLE.
> 
>   Documentation/technical/partial-clone.adoc |  2 +-
>   builtin/fetch-pack.c                       |  2 +-
>   builtin/fsck.c                             |  6 +++---
>   builtin/index-pack.c                       |  7 ++++---
>   builtin/pack-objects.c                     | 14 +++++++-------
>   builtin/prune.c                            |  2 +-
>   builtin/rev-list.c                         | 10 +++++-----
>   git.c                                      |  2 +-
>   midx-write.c                               |  2 +-
>   odb.c                                      |  4 +---
>   odb.h                                      |  8 --------
>   repository.c                               |  1 +
>   repository.h                               |  6 ++++++
>   revision.c                                 |  2 +-
>   setup.c                                    |  2 +-
>   15 files changed, 34 insertions(+), 36 deletions(-)
> 
> diff --git a/Documentation/technical/partial-clone.adoc b/Documentation/technical/partial-clone.adoc
> index e513e391ea..18718a3840 100644
> --- a/Documentation/technical/partial-clone.adoc
> +++ b/Documentation/technical/partial-clone.adoc
> @@ -159,7 +159,7 @@ and prefetch those objects in bulk.
>   - `repack` in GC has been updated to not touch promisor packfiles at all,
>     and to only repack other objects.
>   
> -- The global variable "fetch_if_missing" is used to control whether an
> +- The per-repository flag "fetch_if_missing" is used to control whether an
>     object lookup will attempt to dynamically fetch a missing object or
>     report an error.
>   +
> diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
> index 316badd969..c5edd7b80f 100644
> --- a/builtin/fetch-pack.c
> +++ b/builtin/fetch-pack.c
> @@ -67,7 +67,7 @@ int cmd_fetch_pack(int argc,
>   	struct packet_reader reader;
>   	enum protocol_version version;
>   
> -	fetch_if_missing = 0;
> +	the_repository->fetch_if_missing = 0;
>   
>   	packet_trace_identity("fetch-pack");
>   
> diff --git a/builtin/fsck.c b/builtin/fsck.c
> index 248f8ff5a0..aa31c69486 100644
> --- a/builtin/fsck.c
> +++ b/builtin/fsck.c
> @@ -1017,15 +1017,15 @@ int cmd_fsck(int argc,
>   		.ref = NULL
>   	};
>   
> -	/* fsck knows how to handle missing promisor objects */
> -	fetch_if_missing = 0;
> -
>   	errors_found = 0;
>   	disable_replace_refs();
>   	save_commit_buffer = 0;
>   
>   	argc = parse_options(argc, argv, prefix, fsck_opts, fsck_usage, 0);
>   
> +	/* fsck knows how to handle missing promisor objects */
> +	repo->fetch_if_missing = 0;
> +
>   	fsck_options_init(&fsck_walk_options, repo, FSCK_OPTIONS_DEFAULT);
>   	fsck_walk_options.walk = mark_object;
>   
> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
> index 0793dc595c..74f9694662 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)
> +		the_repository->fetch_if_missing = 0;
>   
>   	disable_replace_refs();
>   
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index 8a1709a1ab..c6536b1f65 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -4059,7 +4059,7 @@ static void add_unreachable_loose_objects(struct rev_info *revs);
>   
>   static void read_stdin_packs(enum stdin_packs_mode mode, int rev_list_unpacked)
>   {
> -	int prev_fetch_if_missing = fetch_if_missing;
> +	int prev_fetch_if_missing = the_repository->fetch_if_missing;
>   	struct rev_info revs;
>   
>   	/*
> @@ -4067,7 +4067,7 @@ static void read_stdin_packs(enum stdin_packs_mode mode, int rev_list_unpacked)
>   	 * walk is best-effort though we don't want to perform backfill fetches
>   	 * for them.
>   	 */
> -	fetch_if_missing = 0;
> +	the_repository->fetch_if_missing = 0;
>   
>   	repo_init_revisions(the_repository, &revs, NULL);
>   	/*
> @@ -4115,7 +4115,7 @@ static void read_stdin_packs(enum stdin_packs_mode mode, int rev_list_unpacked)
>   	trace2_data_intmax("pack-objects", the_repository, "stdin_packs_hints",
>   			   stdin_packs_hints_nr);
>   
> -	fetch_if_missing = prev_fetch_if_missing;
> +	the_repository->fetch_if_missing = prev_fetch_if_missing;
>   }
>   
>   static void add_cruft_object_entry(const struct object_id *oid, enum object_type type,
> @@ -4451,14 +4451,14 @@ static int option_parse_missing_action(const struct option *opt UNUSED,
>   
>   	if (!strcmp(arg, "allow-any")) {
>   		arg_missing_action = MA_ALLOW_ANY;
> -		fetch_if_missing = 0;
> +		the_repository->fetch_if_missing = 0;
>   		fn_show_object = show_object__ma_allow_any;
>   		return 0;
>   	}
>   
>   	if (!strcmp(arg, "allow-promisor")) {
>   		arg_missing_action = MA_ALLOW_PROMISOR;
> -		fetch_if_missing = 0;
> +		the_repository->fetch_if_missing = 0;
>   		fn_show_object = show_object__ma_allow_promisor;
>   		return 0;
>   	}
> @@ -5247,7 +5247,7 @@ int cmd_pack_objects(int argc,
>   				  exclude_promisor_objects_best_effort,
>   				  "--exclude-promisor-objects-best-effort");
>   	if (exclude_promisor_objects) {
> -		fetch_if_missing = 0;
> +		the_repository->fetch_if_missing = 0;
>   
>   		/* --stdin-packs handles promisor objects separately. */
>   		if (!stdin_packs) {
> @@ -5256,7 +5256,7 @@ int cmd_pack_objects(int argc,
>   		}
>   	} else if (exclude_promisor_objects_best_effort) {
>   		use_internal_rev_list = 1;
> -		fetch_if_missing = 0;
> +		the_repository->fetch_if_missing = 0;
>   		option_parse_missing_action(NULL, "allow-any", 0);
>   		/* revs configured below */
>   	}
> diff --git a/builtin/prune.c b/builtin/prune.c
> index 55635a891f..a7e4678d11 100644
> --- a/builtin/prune.c
> +++ b/builtin/prune.c
> @@ -194,7 +194,7 @@ int cmd_prune(int argc,
>   	if (show_progress == -1)
>   		show_progress = isatty(2);
>   	if (exclude_promisor_objects) {
> -		fetch_if_missing = 0;
> +		repo->fetch_if_missing = 0;
>   		revs.exclude_promisor_objects = 1;
>   	}
>   
> diff --git a/builtin/rev-list.c b/builtin/rev-list.c
> index 8f63003709..a6a0c5559e 100644
> --- a/builtin/rev-list.c
> +++ b/builtin/rev-list.c
> @@ -509,25 +509,25 @@ static inline int parse_missing_action_value(const char *value)
>   
>   	if (!strcmp(value, "allow-any")) {
>   		arg_missing_action = MA_ALLOW_ANY;
> -		fetch_if_missing = 0;
> +		the_repository->fetch_if_missing = 0;
>   		return 1;
>   	}
>   
>   	if (!strcmp(value, "print")) {
>   		arg_missing_action = MA_PRINT;
> -		fetch_if_missing = 0;
> +		the_repository->fetch_if_missing = 0;
>   		return 1;
>   	}
>   
>   	if (!strcmp(value, "print-info")) {
>   		arg_missing_action = MA_PRINT_INFO;
> -		fetch_if_missing = 0;
> +		the_repository->fetch_if_missing = 0;
>   		return 1;
>   	}
>   
>   	if (!strcmp(value, "allow-promisor")) {
>   		arg_missing_action = MA_ALLOW_PROMISOR;
> -		fetch_if_missing = 0;
> +		the_repository->fetch_if_missing = 0;
>   		return 1;
>   	}
>   
> @@ -745,7 +745,7 @@ int cmd_rev_list(int argc,
>   	for (i = 1; i < argc; i++) {
>   		const char *arg = argv[i];
>   		if (!strcmp(arg, "--exclude-promisor-objects")) {
> -			fetch_if_missing = 0;
> +			the_repository->fetch_if_missing = 0;
>   			revs.exclude_promisor_objects = 1;
>   		} else if (skip_prefix(arg, "--missing=", &arg)) {
>   			parse_missing_action_value(arg);
> diff --git a/git.c b/git.c
> index 36f08891ef..315d2e160e 100644
> --- a/git.c
> +++ b/git.c
> @@ -202,7 +202,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
>   			if (envchanged)
>   				*envchanged = 1;
>   		} else if (!strcmp(cmd, "--no-lazy-fetch")) {
> -			fetch_if_missing = 0;
> +			the_repository->fetch_if_missing = 0;
>   			setenv(NO_LAZY_FETCH_ENVIRONMENT, "1", 1);
>   			if (envchanged)
>   				*envchanged = 1;
> diff --git a/midx-write.c b/midx-write.c
> index 19e1cd10b7..e7313c9d2c 100644
> --- a/midx-write.c
> +++ b/midx-write.c
> @@ -865,7 +865,7 @@ static void find_commits_for_midx_bitmap(struct commit_stack *commits,
>   	 * complain later that we don't have reachability closure (and fail
>   	 * appropriately).
>   	 */
> -	fetch_if_missing = 0;
> +	ctx->repo->fetch_if_missing = 0;
>   	revs.exclude_promisor_objects = 1;
>   
>   	if (prepare_revision_walk(&revs))
> diff --git a/odb.c b/odb.c
> index 965ef68e4e..664256e1a4 100644
> --- a/odb.c
> +++ b/odb.c
> @@ -528,8 +528,6 @@ void disable_obj_read_lock(void)
>   	pthread_mutex_destroy(&obj_read_mutex);
>   }
>   
> -int fetch_if_missing = 1;
> -
>   static int register_all_submodule_sources(struct object_database *odb)
>   {
>   	int ret = odb->submodule_source_paths.nr;
> @@ -595,7 +593,7 @@ static int do_oid_object_info_extended(struct object_database *odb,
>   			continue;
>   
>   		/* Check if it is a missing object */
> -		if (fetch_if_missing && repo_has_promisor_remote(odb->repo) &&
> +		if (odb->repo->fetch_if_missing && repo_has_promisor_remote(odb->repo) &&
>   		    !already_retried &&
>   		    !(flags & OBJECT_INFO_SKIP_FETCH_OBJECT)) {
>   			promisor_remote_get_direct(odb->repo, real, 1);
> diff --git a/odb.h b/odb.h
> index 0030467a52..1dca583fcb 100644
> --- a/odb.h
> +++ b/odb.h
> @@ -14,14 +14,6 @@ struct repository;
>   struct strbuf;
>   struct strvec;
>   
> -/*
> - * Set this to 0 to prevent odb_read_object_info_extended() from fetching missing
> - * blobs. This has a difference only if extensions.partialClone is set.
> - *
> - * Its default value is 1.
> - */
> -extern int fetch_if_missing;
> -
>   /*
>    * Compute the exact path an alternate is at and returns it. In case of
>    * error NULL is returned and the human readable error is added to `err`
> diff --git a/repository.c b/repository.c
> index 187dd471c4..b959f7a028 100644
> --- a/repository.c
> +++ b/repository.c
> @@ -73,6 +73,7 @@ void initialize_repository(struct repository *repo)
>   	ALLOC_ARRAY(repo->index, 1);
>   	index_state_init(repo->index, repo);
>   	repo->check_deprecated_config = true;
> +	repo->fetch_if_missing = 1;
>   	repo_config_values_init(&repo->config_values_private_);
>   
>   	/*
> diff --git a/repository.h b/repository.h
> index 36e2db2633..e8bd6ef0e7 100644
> --- a/repository.h
> +++ b/repository.h
> @@ -169,6 +169,12 @@ struct repository {
>   	/* True if commit-graph has been disabled within this process. */
>   	int commit_graph_disabled;
>   
> +	/*
> +	 * Controls whether the repository should lazily fetch missing
> +	 * objects from promisor remotes. Defaults to 1.
> +	 */
> +	int fetch_if_missing;
> +
>   	/*
>   	 * Lazily-populated cache mapping hook event names to configured hooks.
>   	 * NULL until first hook use.
> diff --git a/revision.c b/revision.c
> index e91d7e1f11..5f70aa81e6 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 (the_repository->fetch_if_missing)
>   			BUG("exclude_promisor_objects can only be used when fetch_if_missing is 0");
>   		revs->exclude_promisor_objects = 1;
>   	} else {
> diff --git a/setup.c b/setup.c
> index b4652651df..ce2a80ac31 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -1064,7 +1064,7 @@ static void setup_git_env_internal(struct repository *repo,
>   		set_alternate_shallow_file(repo, shallow_file, 0);
>   
>   	if (git_env_bool(NO_LAZY_FETCH_ENVIRONMENT, 0))
> -		fetch_if_missing = 0;
> +		the_repository->fetch_if_missing = 0;
>   }
>   
>   static void set_git_dir_1(struct repository *repo, const char *path)

What are everyone's thoughts on this patch? It seems like it's been a 
while since it received responses. ^^

Regards, yuchen

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] repository: move fetch_if_missing into struct repository
  2026-07-16  7:29 ` [PATCH v2] " Tian Yuchen
  2026-08-01 15:53   ` Tian Yuchen
@ 2026-08-04  8:24   ` Patrick Steinhardt
  2026-08-04 17:38     ` Junio C Hamano
  2026-08-05 12:10     ` Tian Yuchen
  2026-08-07  9:41   ` [PATCH v3] " Tian Yuchen
  2 siblings, 2 replies; 14+ messages in thread
From: Patrick Steinhardt @ 2026-08-04  8:24 UTC (permalink / raw)
  To: Tian Yuchen
  Cc: git, five231003, hariom18599, Christian Couder, Ayush Chandekar,
	Olamide Caleb Bello

On Thu, Jul 16, 2026 at 03:29:54PM +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.
> 
> 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 until after parameter parsing is complete.
> 
> Additionally, update the partial clone documentation to reflect
> that this is now a per-repository flag.
> 
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
> Signed-off-by: Tian Yuchen <cat@malon.dev>
> ---
> 
> Change since V1:
> 
> - Following Patrick's advice, use the_repository whenever possible
>   without re-introducing #define USE_THE_REPOSITORY_VARIABLE.

It would be great to include the range-diff compared to the previous
version so that it's easier for the reviewer to spot what's changed.
Tools like b4 automate this for you :)

> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
> index 0793dc595c..74f9694662 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)
> +		the_repository->fetch_if_missing = 0;
>  
>  	disable_replace_refs();
>  

This one looks a bit weird -- we check for `repo`, but then set
`the_repository->fetch_if_missing`. We can probably just loose the
conditional completely, and furthermore we don't need to reorder any
code here at all anymore.

Other than that this patch looks good to me, thanks!

Patrick

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] repository: move fetch_if_missing into struct repository
  2026-08-04  8:24   ` Patrick Steinhardt
@ 2026-08-04 17:38     ` Junio C Hamano
  2026-08-05 12:34       ` Tian Yuchen
  2026-08-05 12:10     ` Tian Yuchen
  1 sibling, 1 reply; 14+ messages in thread
From: Junio C Hamano @ 2026-08-04 17:38 UTC (permalink / raw)
  To: Patrick Steinhardt
  Cc: Tian Yuchen, git, five231003, hariom18599, Christian Couder,
	Ayush Chandekar, Olamide Caleb Bello

Patrick Steinhardt <ps@pks.im> writes:

>> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
>> index 0793dc595c..74f9694662 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)
>> +		the_repository->fetch_if_missing = 0;
>>  
>>  	disable_replace_refs();
>>  
>
> This one looks a bit weird -- we check for `repo`, but then set
> `the_repository->fetch_if_missing`. We can probably just loose the
> conditional completely, and furthermore we don't need to reorder any
> code here at all anymore.

The 4-line comment is about disabling fetch-if-missing, so the code
movement is not even unnecessary, but it is harmful, I think.  If
the command can work without repository, incoming "repo" might be
NULL, and unconditionally doing

	repo->fetch_if_missing = 0;

may cause a crash.  But that is not an excuse to blindly add

	if (repo)

in front of such an assignment.

It gives you a chance to rethink what you are doing.

If a command can work without a repository, yet it cares about how
fetch_if_missing bit is set, it hints that it may be a mistake in
the first place to try associating fetch_if_missing bit with a
particular struct repository instance, as you must be prepared to
work with repo==NULL.

There could be at least three approaches you may have to think about
at that point.

 * Perhaps the command may not have to work outside a repository at
   all.  If so, then it is a bug for the caller to call this
   function with repo==NULL.  So we should just say

	repo->fetch_if_missing = 0;

   without "if (repo)" check at all here.  After all, the situation
   we might want to enable fetch_if_missing is where we have a place
   to fetch into, so by definition, we _should_ have a repository in
   such a case.

 * Perhaps the command may want to work outside a repository but it
   may be acceptable to operate in a degraded way.  By definition,
   when we are outside a repository, we have no object store to
   fetch objects lazily into, so fetch_if_missing MUST BE off.

   Because Git is primarily about working inside a repository,
   perhaps it may be acceptable, even when you are outside a
   repository, to assume that the_repository can be used as a
   back-up "fake repository" object, and fetch_if_missing and its
   friends that are necessary to have their meaning to be in that
   fake repository object.  If that the stance we are going to take,
   this part should probably say:

	(repo ? repo : the_repository)->fetch_if_missing = 0;

   We need to make sure that everybody who passes the code paths
   that ever reference fetch_if_missing would pass the_repository
   down when the command is running outside a repository, though.

 * Or perhaps there are some settings that really need to be
   available whether you are in a repository or not.  I think
   fetch_if_missing is a borderline case, but more generally, things
   like user.name should conceptually be available even outside a
   repository, with in-repository configuration files overriding
   them.  And it may be a mistake to force such settings to be
   stored in an instance of "struct repository" (or repo_settings
   that is part of it).  We would need a framework to represent a
   structure in which a basic setting, which does not belong to any
   repository (whose members may be the same as those in "struct
   repo_settings", so I think it is OK to use an instance of that
   struct to represnt this "basic settings that is global"), exists
   globally, and it is overriden by per repository setting, which is
   in "struct repo_settings" embedded in "struct repository".

The earlier choices require fewer changes than the later choices,
but the later choices are more concepturely pure, I think.





^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] repository: move fetch_if_missing into struct repository
  2026-08-04  8:24   ` Patrick Steinhardt
  2026-08-04 17:38     ` Junio C Hamano
@ 2026-08-05 12:10     ` Tian Yuchen
  1 sibling, 0 replies; 14+ messages in thread
From: Tian Yuchen @ 2026-08-05 12:10 UTC (permalink / raw)
  To: Patrick Steinhardt
  Cc: git, five231003, hariom18599, Christian Couder, Ayush Chandekar,
	Olamide Caleb Bello

Hi Patrick,

On 8/4/26 16:24, Patrick Steinhardt wrote:
> On Thu, Jul 16, 2026 at 03:29:54PM +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.
>>
>> 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 until after parameter parsing is complete.
>>
>> Additionally, update the partial clone documentation to reflect
>> that this is now a per-repository flag.
>>
>> Mentored-by: Christian Couder <christian.couder@gmail.com>
>> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
>> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
>> Signed-off-by: Tian Yuchen <cat@malon.dev>
>> ---
>>
>> Change since V1:
>>
>> - Following Patrick's advice, use the_repository whenever possible
>>    without re-introducing #define USE_THE_REPOSITORY_VARIABLE.
> 
> It would be great to include the range-diff compared to the previous
> version so that it's easier for the reviewer to spot what's changed.
> Tools like b4 automate this for you :)
> 

I see, I will include it in the next reroll!

Thanks! yuchen


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] repository: move fetch_if_missing into struct repository
  2026-08-04 17:38     ` Junio C Hamano
@ 2026-08-05 12:34       ` Tian Yuchen
  0 siblings, 0 replies; 14+ messages in thread
From: Tian Yuchen @ 2026-08-05 12:34 UTC (permalink / raw)
  To: Junio C Hamano, Patrick Steinhardt
  Cc: git, five231003, hariom18599, Christian Couder, Ayush Chandekar,
	Olamide Caleb Bello

On 8/5/26 01:38, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
>>> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
>>> index 0793dc595c..74f9694662 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)
>>> +		the_repository->fetch_if_missing = 0;
>>>   
>>>   	disable_replace_refs();
>>>   
>>
>> This one looks a bit weird -- we check for `repo`, but then set
>> `the_repository->fetch_if_missing`. We can probably just loose the
>> conditional completely, and furthermore we don't need to reorder any
>> code here at all anymore.
> 
> The 4-line comment is about disabling fetch-if-missing, so the code
> movement is not even unnecessary, but it is harmful, I think.  If
> the command can work without repository, incoming "repo" might be
> NULL, and unconditionally doing
> 
> 	repo->fetch_if_missing = 0;
> 
> may cause a crash.  But that is not an excuse to blindly add
> 
> 	if (repo)
> 
> in front of such an assignment.

Yes, this is a mistake. :(

> 
> It gives you a chance to rethink what you are doing.
> 
> If a command can work without a repository, yet it cares about how
> fetch_if_missing bit is set, it hints that it may be a mistake in
> the first place to try associating fetch_if_missing bit with a
> particular struct repository instance, as you must be prepared to
> work with repo==NULL.
> 
> There could be at least three approaches you may have to think about
> at that point.
> 
>   * Perhaps the command may not have to work outside a repository at
>     all.  If so, then it is a bug for the caller to call this
>     function with repo==NULL.  So we should just say
> 
> 	repo->fetch_if_missing = 0;
> 
>     without "if (repo)" check at all here.  After all, the situation
>     we might want to enable fetch_if_missing is where we have a place
>     to fetch into, so by definition, we _should_ have a repository in
>     such a case.
> 

Yep, this approach looks the most reasonable to me. I think I will apply 
this approach and update the commit message.

>   * Perhaps the command may want to work outside a repository but it
>     may be acceptable to operate in a degraded way.  By definition,
>     when we are outside a repository, we have no object store to
>     fetch objects lazily into, so fetch_if_missing MUST BE off.
> 
>     Because Git is primarily about working inside a repository,
>     perhaps it may be acceptable, even when you are outside a
>     repository, to assume that the_repository can be used as a
>     back-up "fake repository" object, and fetch_if_missing and its
>     friends that are necessary to have their meaning to be in that
>     fake repository object.  If that the stance we are going to take,
>     this part should probably say:
> 
> 	(repo ? repo : the_repository)->fetch_if_missing = 0;
> 
>     We need to make sure that everybody who passes the code paths
>     that ever reference fetch_if_missing would pass the_repository
>     down when the command is running outside a repository, though.
> 
>   * Or perhaps there are some settings that really need to be
>     available whether you are in a repository or not.  I think
>     fetch_if_missing is a borderline case, but more generally, things
>     like user.name should conceptually be available even outside a
>     repository, with in-repository configuration files overriding
>     them.  And it may be a mistake to force such settings to be
>     stored in an instance of "struct repository" (or repo_settings
>     that is part of it).  We would need a framework to represent a
>     structure in which a basic setting, which does not belong to any
>     repository (whose members may be the same as those in "struct
>     repo_settings", so I think it is OK to use an instance of that
>     struct to represnt this "basic settings that is global"), exists
>     globally, and it is overriden by per repository setting, which is
>     in "struct repo_settings" embedded in "struct repository".
> 

I'm not sure if I'm right, but I feel that 'fetch_if_missing' 
essentially carries two meanings simultaneously: "Does this repository 
allow lazy fetching?" and "Does this object lookup allow network 
behavior to be triggered?" In other words, suppose we consider 
'fetch_if_missing' to meet the third case, allowing it to exist in an 
ambiguous situation, i.e. it makes sense both within and outside the 
repository...Can we then consider the existence of this variable itself 
to have some semantic ambiguity? If so, do we really need to invent 
another mechanism to accommodate this ambiguity? Or should we 
temporarily apply minimal changes and discuss the ownership issue when 
the time is right (i.e. subsystems ready for 'repo' rather than 
'the_repository' only)? Perhaps this is what you mean by "a borderline 
case".

Furthermore: when we do libification, is the goal to eliminate global 
state, or to "package global state more reasonably"? The third approach 
seems to be the latter one.

> The earlier choices require fewer changes than the later choices,
> but the later choices are more concepturely pure, I think.
> 

All in all, I think your core point is that we shouldn't blindly put all 
global variables in a struct repository. I totally agree with that.

> 

Thanks! yuchen

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3] repository: move fetch_if_missing into struct repository
  2026-07-16  7:29 ` [PATCH v2] " Tian Yuchen
  2026-08-01 15:53   ` Tian Yuchen
  2026-08-04  8:24   ` Patrick Steinhardt
@ 2026-08-07  9:41   ` Tian Yuchen
  2026-08-07 17:03     ` Junio C Hamano
  2 siblings, 1 reply; 14+ messages in thread
From: Tian Yuchen @ 2026-08-07  9:41 UTC (permalink / raw)
  To: git; +Cc: ps, Tian Yuchen, Christian Couder, Ayush Chandekar,
	Olamide Caleb Bello

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.

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 until after parameter parsing is complete.

Additionally, update the partial clone documentation to reflect
that this is now a per-repository flag.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
Range-diff since v2:

    @@ builtin/fsck.c: int cmd_fsck(int argc,


      ## builtin/index-pack.c ##
    +@@ builtin/index-pack.c: static void repack_local_links(void)
    + int cmd_index_pack(int argc,
    +              const char **argv,
    +              const char *prefix,
    +-             struct repository *repo UNUSED)
    ++             struct repository *repo)
    + {
    +   int i, fix_thin_pack = 0, verify = 0, stat_only = 0, rev_index;
    +   const char *curr_index;
     @@ builtin/index-pack.c: int cmd_index_pack(int argc,
        int report_end_of_input = 0;
        int hash_algo = 0;
    @@ builtin/index-pack.c: int cmd_index_pack(int argc,
     -  fetch_if_missing = 0;
     -
     -  show_usage_if_asked(argc, argv, index_pack_usage);
    -+  if (repo)
    -+          the_repository->fetch_if_missing = 0;
    ++  (repo ? repo : the_repository)->fetch_if_missing = 0;

        disable_replace_refs();

 Documentation/technical/partial-clone.adoc |  2 +-
 builtin/fetch-pack.c                       |  2 +-
 builtin/fsck.c                             |  6 +++---
 builtin/index-pack.c                       |  8 ++++----
 builtin/pack-objects.c                     | 14 +++++++-------
 builtin/prune.c                            |  2 +-
 builtin/rev-list.c                         | 10 +++++-----
 git.c                                      |  2 +-
 midx-write.c                               |  2 +-
 odb.c                                      |  4 +---
 odb.h                                      |  8 --------
 repository.c                               |  1 +
 repository.h                               |  6 ++++++
 revision.c                                 |  2 +-
 setup.c                                    |  2 +-
 15 files changed, 34 insertions(+), 37 deletions(-)

diff --git a/Documentation/technical/partial-clone.adoc b/Documentation/technical/partial-clone.adoc
index e513e391ea..18718a3840 100644
--- a/Documentation/technical/partial-clone.adoc
+++ b/Documentation/technical/partial-clone.adoc
@@ -159,7 +159,7 @@ and prefetch those objects in bulk.
 - `repack` in GC has been updated to not touch promisor packfiles at all,
   and to only repack other objects.
 
-- The global variable "fetch_if_missing" is used to control whether an
+- The per-repository flag "fetch_if_missing" is used to control whether an
   object lookup will attempt to dynamically fetch a missing object or
   report an error.
 +
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 316badd969..c5edd7b80f 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -67,7 +67,7 @@ int cmd_fetch_pack(int argc,
 	struct packet_reader reader;
 	enum protocol_version version;
 
-	fetch_if_missing = 0;
+	the_repository->fetch_if_missing = 0;
 
 	packet_trace_identity("fetch-pack");
 
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 248f8ff5a0..aa31c69486 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -1017,15 +1017,15 @@ int cmd_fsck(int argc,
 		.ref = NULL
 	};
 
-	/* fsck knows how to handle missing promisor objects */
-	fetch_if_missing = 0;
-
 	errors_found = 0;
 	disable_replace_refs();
 	save_commit_buffer = 0;
 
 	argc = parse_options(argc, argv, prefix, fsck_opts, fsck_usage, 0);
 
+	/* fsck knows how to handle missing promisor objects */
+	repo->fetch_if_missing = 0;
+
 	fsck_options_init(&fsck_walk_options, repo, FSCK_OPTIONS_DEFAULT);
 	fsck_walk_options.walk = mark_object;
 
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 0793dc595c..1be374806d 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1881,7 +1881,7 @@ static void repack_local_links(void)
 int cmd_index_pack(int argc,
 		   const char **argv,
 		   const char *prefix,
-		   struct repository *repo UNUSED)
+		   struct repository *repo)
 {
 	int i, fix_thin_pack = 0, verify = 0, stat_only = 0, rev_index;
 	const char *curr_index;
@@ -1898,15 +1898,15 @@ 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);
+	(repo ? repo : the_repository)->fetch_if_missing = 0;
 
 	disable_replace_refs();
 
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 8a1709a1ab..c6536b1f65 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -4059,7 +4059,7 @@ static void add_unreachable_loose_objects(struct rev_info *revs);
 
 static void read_stdin_packs(enum stdin_packs_mode mode, int rev_list_unpacked)
 {
-	int prev_fetch_if_missing = fetch_if_missing;
+	int prev_fetch_if_missing = the_repository->fetch_if_missing;
 	struct rev_info revs;
 
 	/*
@@ -4067,7 +4067,7 @@ static void read_stdin_packs(enum stdin_packs_mode mode, int rev_list_unpacked)
 	 * walk is best-effort though we don't want to perform backfill fetches
 	 * for them.
 	 */
-	fetch_if_missing = 0;
+	the_repository->fetch_if_missing = 0;
 
 	repo_init_revisions(the_repository, &revs, NULL);
 	/*
@@ -4115,7 +4115,7 @@ static void read_stdin_packs(enum stdin_packs_mode mode, int rev_list_unpacked)
 	trace2_data_intmax("pack-objects", the_repository, "stdin_packs_hints",
 			   stdin_packs_hints_nr);
 
-	fetch_if_missing = prev_fetch_if_missing;
+	the_repository->fetch_if_missing = prev_fetch_if_missing;
 }
 
 static void add_cruft_object_entry(const struct object_id *oid, enum object_type type,
@@ -4451,14 +4451,14 @@ static int option_parse_missing_action(const struct option *opt UNUSED,
 
 	if (!strcmp(arg, "allow-any")) {
 		arg_missing_action = MA_ALLOW_ANY;
-		fetch_if_missing = 0;
+		the_repository->fetch_if_missing = 0;
 		fn_show_object = show_object__ma_allow_any;
 		return 0;
 	}
 
 	if (!strcmp(arg, "allow-promisor")) {
 		arg_missing_action = MA_ALLOW_PROMISOR;
-		fetch_if_missing = 0;
+		the_repository->fetch_if_missing = 0;
 		fn_show_object = show_object__ma_allow_promisor;
 		return 0;
 	}
@@ -5247,7 +5247,7 @@ int cmd_pack_objects(int argc,
 				  exclude_promisor_objects_best_effort,
 				  "--exclude-promisor-objects-best-effort");
 	if (exclude_promisor_objects) {
-		fetch_if_missing = 0;
+		the_repository->fetch_if_missing = 0;
 
 		/* --stdin-packs handles promisor objects separately. */
 		if (!stdin_packs) {
@@ -5256,7 +5256,7 @@ int cmd_pack_objects(int argc,
 		}
 	} else if (exclude_promisor_objects_best_effort) {
 		use_internal_rev_list = 1;
-		fetch_if_missing = 0;
+		the_repository->fetch_if_missing = 0;
 		option_parse_missing_action(NULL, "allow-any", 0);
 		/* revs configured below */
 	}
diff --git a/builtin/prune.c b/builtin/prune.c
index 55635a891f..a7e4678d11 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -194,7 +194,7 @@ int cmd_prune(int argc,
 	if (show_progress == -1)
 		show_progress = isatty(2);
 	if (exclude_promisor_objects) {
-		fetch_if_missing = 0;
+		repo->fetch_if_missing = 0;
 		revs.exclude_promisor_objects = 1;
 	}
 
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 8f63003709..a6a0c5559e 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -509,25 +509,25 @@ static inline int parse_missing_action_value(const char *value)
 
 	if (!strcmp(value, "allow-any")) {
 		arg_missing_action = MA_ALLOW_ANY;
-		fetch_if_missing = 0;
+		the_repository->fetch_if_missing = 0;
 		return 1;
 	}
 
 	if (!strcmp(value, "print")) {
 		arg_missing_action = MA_PRINT;
-		fetch_if_missing = 0;
+		the_repository->fetch_if_missing = 0;
 		return 1;
 	}
 
 	if (!strcmp(value, "print-info")) {
 		arg_missing_action = MA_PRINT_INFO;
-		fetch_if_missing = 0;
+		the_repository->fetch_if_missing = 0;
 		return 1;
 	}
 
 	if (!strcmp(value, "allow-promisor")) {
 		arg_missing_action = MA_ALLOW_PROMISOR;
-		fetch_if_missing = 0;
+		the_repository->fetch_if_missing = 0;
 		return 1;
 	}
 
@@ -745,7 +745,7 @@ int cmd_rev_list(int argc,
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
 		if (!strcmp(arg, "--exclude-promisor-objects")) {
-			fetch_if_missing = 0;
+			the_repository->fetch_if_missing = 0;
 			revs.exclude_promisor_objects = 1;
 		} else if (skip_prefix(arg, "--missing=", &arg)) {
 			parse_missing_action_value(arg);
diff --git a/git.c b/git.c
index 36f08891ef..315d2e160e 100644
--- a/git.c
+++ b/git.c
@@ -202,7 +202,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 			if (envchanged)
 				*envchanged = 1;
 		} else if (!strcmp(cmd, "--no-lazy-fetch")) {
-			fetch_if_missing = 0;
+			the_repository->fetch_if_missing = 0;
 			setenv(NO_LAZY_FETCH_ENVIRONMENT, "1", 1);
 			if (envchanged)
 				*envchanged = 1;
diff --git a/midx-write.c b/midx-write.c
index 19e1cd10b7..e7313c9d2c 100644
--- a/midx-write.c
+++ b/midx-write.c
@@ -865,7 +865,7 @@ static void find_commits_for_midx_bitmap(struct commit_stack *commits,
 	 * complain later that we don't have reachability closure (and fail
 	 * appropriately).
 	 */
-	fetch_if_missing = 0;
+	ctx->repo->fetch_if_missing = 0;
 	revs.exclude_promisor_objects = 1;
 
 	if (prepare_revision_walk(&revs))
diff --git a/odb.c b/odb.c
index 965ef68e4e..664256e1a4 100644
--- a/odb.c
+++ b/odb.c
@@ -528,8 +528,6 @@ void disable_obj_read_lock(void)
 	pthread_mutex_destroy(&obj_read_mutex);
 }
 
-int fetch_if_missing = 1;
-
 static int register_all_submodule_sources(struct object_database *odb)
 {
 	int ret = odb->submodule_source_paths.nr;
@@ -595,7 +593,7 @@ static int do_oid_object_info_extended(struct object_database *odb,
 			continue;
 
 		/* Check if it is a missing object */
-		if (fetch_if_missing && repo_has_promisor_remote(odb->repo) &&
+		if (odb->repo->fetch_if_missing && repo_has_promisor_remote(odb->repo) &&
 		    !already_retried &&
 		    !(flags & OBJECT_INFO_SKIP_FETCH_OBJECT)) {
 			promisor_remote_get_direct(odb->repo, real, 1);
diff --git a/odb.h b/odb.h
index 0030467a52..1dca583fcb 100644
--- a/odb.h
+++ b/odb.h
@@ -14,14 +14,6 @@ struct repository;
 struct strbuf;
 struct strvec;
 
-/*
- * Set this to 0 to prevent odb_read_object_info_extended() from fetching missing
- * blobs. This has a difference only if extensions.partialClone is set.
- *
- * Its default value is 1.
- */
-extern int fetch_if_missing;
-
 /*
  * Compute the exact path an alternate is at and returns it. In case of
  * error NULL is returned and the human readable error is added to `err`
diff --git a/repository.c b/repository.c
index 187dd471c4..b959f7a028 100644
--- a/repository.c
+++ b/repository.c
@@ -73,6 +73,7 @@ void initialize_repository(struct repository *repo)
 	ALLOC_ARRAY(repo->index, 1);
 	index_state_init(repo->index, repo);
 	repo->check_deprecated_config = true;
+	repo->fetch_if_missing = 1;
 	repo_config_values_init(&repo->config_values_private_);
 
 	/*
diff --git a/repository.h b/repository.h
index 36e2db2633..e8bd6ef0e7 100644
--- a/repository.h
+++ b/repository.h
@@ -169,6 +169,12 @@ struct repository {
 	/* True if commit-graph has been disabled within this process. */
 	int commit_graph_disabled;
 
+	/*
+	 * Controls whether the repository should lazily fetch missing
+	 * objects from promisor remotes. Defaults to 1.
+	 */
+	int fetch_if_missing;
+
 	/*
 	 * Lazily-populated cache mapping hook event names to configured hooks.
 	 * NULL until first hook use.
diff --git a/revision.c b/revision.c
index e91d7e1f11..5f70aa81e6 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 (the_repository->fetch_if_missing)
 			BUG("exclude_promisor_objects can only be used when fetch_if_missing is 0");
 		revs->exclude_promisor_objects = 1;
 	} else {
diff --git a/setup.c b/setup.c
index b4652651df..ce2a80ac31 100644
--- a/setup.c
+++ b/setup.c
@@ -1064,7 +1064,7 @@ static void setup_git_env_internal(struct repository *repo,
 		set_alternate_shallow_file(repo, shallow_file, 0);
 
 	if (git_env_bool(NO_LAZY_FETCH_ENVIRONMENT, 0))
-		fetch_if_missing = 0;
+		the_repository->fetch_if_missing = 0;
 }
 
 static void set_git_dir_1(struct repository *repo, const char *path)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] repository: move fetch_if_missing into struct repository
  2026-08-07  9:41   ` [PATCH v3] " Tian Yuchen
@ 2026-08-07 17:03     ` Junio C Hamano
  0 siblings, 0 replies; 14+ messages in thread
From: Junio C Hamano @ 2026-08-07 17:03 UTC (permalink / raw)
  To: Tian Yuchen
  Cc: git, ps, Christian Couder, Ayush Chandekar, Olamide Caleb Bello

Tian Yuchen <cat@malon.dev> writes:

> 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.
>
> 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 until after parameter parsing is complete.
>
> Additionally, update the partial clone documentation to reflect
> that this is now a per-repository flag.
>
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
> Signed-off-by: Tian Yuchen <cat@malon.dev>
> ---

Have you made a trial merge to 'seen' and 'next' to see how well
this change plays with other topics in flight?

> diff --git a/setup.c b/setup.c
> index b4652651df..ce2a80ac31 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -1064,7 +1064,7 @@ static void setup_git_env_internal(struct repository *repo,
>  		set_alternate_shallow_file(repo, shallow_file, 0);
>  
>  	if (git_env_bool(NO_LAZY_FETCH_ENVIRONMENT, 0))
> -		fetch_if_missing = 0;
> +		the_repository->fetch_if_missing = 0;
>  }

Pelease remind me the reason why we use the_repository here, even
though a few lines above we are already using the caller-supplied
repo.

Also setup_git_env_internal() no longer exists in 'seen', if this
topic wants to play well with cf1687a41c (setup: split up concerns
of `setup_git_env_internal()`, 2026-07-07), you'd need to coordinate
the other topics and their author(s) before relying on the presence
of what they are removing.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2026-08-07 17:03 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-15  1:18 [PATCH v1] repository: move fetch_if_missing into struct repository Tian Yuchen
2026-07-15  3:27 ` Junio C Hamano
2026-07-15  4:58   ` Tian Yuchen
2026-07-15  6:35 ` Patrick Steinhardt
2026-07-16  7:06   ` Tian Yuchen
2026-07-16 15:28   ` Junio C Hamano
2026-07-16  7:29 ` [PATCH v2] " Tian Yuchen
2026-08-01 15:53   ` Tian Yuchen
2026-08-04  8:24   ` Patrick Steinhardt
2026-08-04 17:38     ` Junio C Hamano
2026-08-05 12:34       ` Tian Yuchen
2026-08-05 12:10     ` Tian Yuchen
2026-08-07  9:41   ` [PATCH v3] " Tian Yuchen
2026-08-07 17:03     ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox