Re: [PATCH v2] repository: move fetch_if_missing into struct repository

[Patrick Steinhardt <ps@pks.im>]

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