From: Phillip Wood <phillip.wood123@gmail.com>
Date: Thu, 22 Jan 2026 14:41:02 GMT
Subject: Re: [Outreachy PATCH v3 2/3] environment: environment: stop using core.sparseCheckout globally
Message-ID: <18b5d932-8a5a-4f33-a803-ef6f0c7d2750@gmail.com>
In-Reply-To: <fd95169de42891452b430814476d78c706e4a7e2.1768681947.git.belkid98@gmail.com>

Hi Olamide

On 17/01/2026 20:59, Olamide Caleb Bello wrote:
> The config value `core.sparseCheckout` is parsed in
> `git_default_core_config()` and stored globally in
> `core_appy_sparse_checkout`. This could cause unintended behaviours
> when different Git repositories running in the same process access this
> variable.
> 
> Move the parsed value into `struct repo_config_values` to retains current
> behaviours while achieving the repository scoped access.

It doesn't achieve repository scoped access though because we only ever 
populate the values in "the_repository", all other instances of "struct 
repository" are initialized by config_values_init() but not the config 
settings.

> diff --git a/builtin/backfill.c b/builtin/backfill.c
> index e80fc1b694..5fc8c51ed1 100644
> --- a/builtin/backfill.c
> +++ b/builtin/backfill.c
> @@ -139,7 +139,7 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit
>   	repo_config(repo, git_default_config, NULL);
>   
>   	if (ctx.sparse < 0)
> -		ctx.sparse = core_apply_sparse_checkout;
> +		ctx.sparse = repo->config_values.sparse_checkout;

Using "repo" rather than "the_repository" here is dangerous because only 
"the_repository" contains the parsed config. This applies throughout 
this patch.

>   
>   	result = do_backfill(&ctx);
>   	backfill_context_clear(&ctx);
> diff --git a/builtin/clone.c b/builtin/clone.c
> index b19b302b06..b6b19e83d1 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -623,7 +623,7 @@ static int git_sparse_checkout_init(const char *repo)
>   	 * We must apply the setting in the current process
>   	 * for the later checkout to use the sparse-checkout file.
>   	 */
> -	core_apply_sparse_checkout = 1;
> +	the_repository->config_values.sparse_checkout = 1;
>   
>   	cmd.git_cmd = 1;
>   	if (run_command(&cmd)) {
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 53cccf2d25..525edb5e9c 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -482,7 +482,7 @@ static int grep_submodule(struct grep_opt *opt,
>   	 *	"forget" the sparse-index feature switch. As a result, the index
>   	 *	of these submodules are expanded unexpectedly.
>   	 *
> -	 * 2. "core_apply_sparse_checkout"
> +	 * 2. "sparse_checkout"

That should be something like config_values.sparse_checkout to make it 
clear that "sparse_checkout" is the name of a member of a struct, not 
the name of a variable.

> diff --git a/environment.h b/environment.h
> index aea73ff25b..3b5ff7094a 100644
> --- a/environment.h
> +++ b/environment.h
> @@ -88,6 +88,7 @@ struct strvec;
>   struct repo_config_values {
>   	/* core config values */
>   	char *attributes_file_path;
> +	int sparse_checkout;

There are several other sparse checkout variables like 
core_sparse_checkout_cone that we'll need to convert in the future so 
"apply_sparse_checkout" or "sparse_checkout_apply" would be better names.

Thanks

Phillip


>   };
>   
>   /*
> @@ -169,7 +170,6 @@ extern int precomposed_unicode;
>   extern int protect_hfs;
>   extern int protect_ntfs;
>   
> -extern int core_apply_sparse_checkout;
>   extern int core_sparse_checkout_cone;
>   extern int sparse_expect_files_outside_of_patterns;
>   
> diff --git a/sparse-index.c b/sparse-index.c
> index 76f90da5f5..6dd8dd679d 100644
> --- a/sparse-index.c
> +++ b/sparse-index.c
> @@ -152,7 +152,8 @@ static int index_has_unmerged_entries(struct index_state *istate)
>   
>   int is_sparse_index_allowed(struct index_state *istate, int flags)
>   {
> -	if (!core_apply_sparse_checkout || !core_sparse_checkout_cone)
> +	struct repo_config_values *cfg = &istate->repo->config_values;
> +	if (!cfg->sparse_checkout || !core_sparse_checkout_cone)
>   		return 0;
>   
>   	if (!(flags & SPARSE_INDEX_MEMORY_ONLY)) {
> @@ -670,7 +671,8 @@ static void clear_skip_worktree_from_present_files_full(struct index_state *ista
>   
>   void clear_skip_worktree_from_present_files(struct index_state *istate)
>   {
> -	if (!core_apply_sparse_checkout ||
> +	struct repo_config_values *cfg = &istate->repo->config_values;
> +	if (!cfg->sparse_checkout ||
>   	    sparse_expect_files_outside_of_patterns)
>   		return;
>   
> diff --git a/unpack-trees.c b/unpack-trees.c
> index f38c761ab9..2bdfa1334c 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -1924,7 +1924,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
>   	if (o->prefix)
>   		update_sparsity_for_prefix(o->prefix, o->src_index);
>   
> -	if (!core_apply_sparse_checkout || !o->update)
> +	if (!repo->config_values.sparse_checkout || !o->update)
>   		o->skip_sparse_checkout = 1;
>   	if (!o->skip_sparse_checkout) {
>   		memset(&pl, 0, sizeof(pl));
> diff --git a/wt-status.c b/wt-status.c
> index e12adb26b9..a2e388606f 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -1764,7 +1764,7 @@ static void wt_status_check_sparse_checkout(struct repository *r,
>   	int skip_worktree = 0;
>   	int i;
>   
> -	if (!core_apply_sparse_checkout || r->index->cache_nr == 0) {
> +	if (!r->config_values.sparse_checkout || r->index->cache_nr == 0) {
>   		/*
>   		 * Don't compute percentage of checked out files if we
>   		 * aren't in a sparse checkout or would get division by 0.