From: Phillip Wood Date: Fri, 23 Jan 2026 10:43:19 GMT Subject: Re: [Outreachy PATCH v3 2/3] environment: environment: stop using core.sparseCheckout globally Message-ID: <4f19e70f-8ab5-4322-ac71-76bc925b324a@gmail.com> In-Reply-To: On 22/01/2026 15:29, Bello Olamide wrote: > On Thu, 22 Jan 2026 at 15:41, Phillip Wood wrote: >> >>> 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. You don't need to be sorry for having a question - it shows you have been thinking about the feedback you have received which is very good. > 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"? Yes, but I think it is safer to explicitly say "the_repository" so that if any of the functions you convert here are ever passed another repository instance the code will keep working as expected. It also documents that the config value is only stored in "the_repository". Once we make these config values per-repository then we can use the repository instance passed to the function. Thanks Phillip >> >>> >>> 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. >>