From: Bello Olamide Date: Thu, 22 Jan 2026 15:29:01 GMT Subject: Re: [Outreachy PATCH v3 2/3] environment: environment: stop using core.sparseCheckout globally Message-ID: In-Reply-To: <18b5d932-8a5a-4f33-a803-ef6f0c7d2750@gmail.com> On Thu, 22 Jan 2026 at 15:41, Phillip Wood wrote: > > 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. Okay I understand. Thank you for clarifying. > > > 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. Okay noted... Sorry but I have a question. I observed that the address of "repo" is passed to builtin/backfill.c, is gotten from git.c:handle_builtin which passed run_builtin "the_repository" as a parameter. Won't the address of "repo" and "the_repository be the same"? > > > > > 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. Okay noted > > > 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. Okay noted. > > 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. >