[GSOC PATCH] environment: move access to "core.sparsecheckout" into repo_settings

[GSOC PATCH] environment: move access to "core.sparsecheckout" into repo_settings

[Ayush Chandekar]

The setting "core.sparsecheckout" is stored in the global
`core_apply_sparse_checkout` and is populated in config.c. Refactor the
code to store it inside the struct `repo_settings`. Also, create
functions to set and get the value of the setting and update all the
occurrences.

This also allows us to remove the definition `#define
USE_THE_REPOSITORY_VARIABLE` from the file 'builtin/backfill.c'.

This change is part of an ongoing effort to eliminate global variables,
improve modularity and help libify the codebase.

Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 builtin/backfill.c        |  5 +----
 builtin/clone.c           |  2 +-
 builtin/mv.c              |  2 +-
 builtin/sparse-checkout.c | 18 +++++++++---------
 builtin/worktree.c        |  2 +-
 config.c                  |  5 -----
 dir.c                     |  2 +-
 environment.c             |  1 -
 environment.h             |  1 -
 repo-settings.c           | 11 +++++++++++
 repo-settings.h           |  5 +++++
 sparse-index.c            |  4 ++--
 unpack-trees.c            |  2 +-
 wt-status.c               |  2 +-
 14 files changed, 34 insertions(+), 28 deletions(-)

diff --git a/builtin/backfill.c b/builtin/backfill.c
index fa82ad2f6f..d397b8e721 100644
--- a/builtin/backfill.c
+++ b/builtin/backfill.c
@@ -1,6 +1,3 @@
-/* We need this macro to access core_apply_sparse_checkout */
-#define USE_THE_REPOSITORY_VARIABLE
-
 #include "builtin.h"
 #include "git-compat-util.h"
 #include "config.h"
@@ -139,7 +136,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_settings_get_apply_sparse_checkout(repo);
 
 	result = do_backfill(&ctx);
 	backfill_context_clear(&ctx);
diff --git a/builtin/clone.c b/builtin/clone.c
index 91b9cd0d16..1d1cd880c4 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -621,7 +621,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;
+	repo_settings_set_apply_sparse_checkout(the_repository, 1);
 
 	cmd.git_cmd = 1;
 	if (run_command(&cmd)) {
diff --git a/builtin/mv.c b/builtin/mv.c
index 07548fe96a..dc9d0b1ab3 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -572,7 +572,7 @@ int cmd_mv(int argc,
 		rename_index_entry_at(the_repository->index, pos, dst);
 
 		if (ignore_sparse &&
-		    core_apply_sparse_checkout &&
+		    repo_settings_get_apply_sparse_checkout(the_repository) &&
 		    core_sparse_checkout_cone) {
 			/*
 			 * NEEDSWORK: we are *not* paying attention to
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 1bf01591b2..41e3a9d61d 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -62,7 +62,7 @@ static int sparse_checkout_list(int argc, const char **argv, const char *prefix,
 	int res;
 
 	setup_work_tree();
-	if (!core_apply_sparse_checkout)
+	if (!repo_settings_get_apply_sparse_checkout(the_repository))
 		die(_("this worktree is not sparse"));
 
 	argc = parse_options(argc, argv, prefix,
@@ -397,11 +397,11 @@ static int set_config(enum sparse_checkout_mode mode)
 
 static enum sparse_checkout_mode update_cone_mode(int *cone_mode) {
 	/* If not specified, use previous definition of cone mode */
-	if (*cone_mode == -1 && core_apply_sparse_checkout)
+	if (*cone_mode == -1 && repo_settings_get_apply_sparse_checkout(the_repository))
 		*cone_mode = core_sparse_checkout_cone;
 
 	/* Set cone/non-cone mode appropriately */
-	core_apply_sparse_checkout = 1;
+	repo_settings_set_apply_sparse_checkout(the_repository, 1);
 	if (*cone_mode == 1 || *cone_mode == -1) {
 		core_sparse_checkout_cone = 1;
 		return MODE_CONE_PATTERNS;
@@ -415,7 +415,7 @@ static int update_modes(int *cone_mode, int *sparse_index)
 	int mode, record_mode;
 
 	/* Determine if we need to record the mode; ensure sparse checkout on */
-	record_mode = (*cone_mode != -1) || !core_apply_sparse_checkout;
+	record_mode = (*cone_mode != -1) || !repo_settings_get_apply_sparse_checkout(the_repository);
 
 	mode = update_cone_mode(cone_mode);
 	if (record_mode && set_config(mode))
@@ -695,9 +695,9 @@ static int modify_pattern_list(struct strvec *args, int use_stdin,
 		break;
 	}
 
-	if (!core_apply_sparse_checkout) {
+	if (!repo_settings_get_apply_sparse_checkout(the_repository)) {
 		set_config(MODE_ALL_PATTERNS);
-		core_apply_sparse_checkout = 1;
+		repo_settings_set_apply_sparse_checkout(the_repository, 1);
 		changed_config = 1;
 	}
 
@@ -793,7 +793,7 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix,
 	int ret;
 
 	setup_work_tree();
-	if (!core_apply_sparse_checkout)
+	if (!repo_settings_get_apply_sparse_checkout(the_repository))
 		die(_("no sparse-checkout to add to"));
 
 	repo_read_index(the_repository);
@@ -902,7 +902,7 @@ static int sparse_checkout_reapply(int argc, const char **argv,
 	};
 
 	setup_work_tree();
-	if (!core_apply_sparse_checkout)
+	if (!repo_settings_get_apply_sparse_checkout(the_repository))
 		die(_("must be in a sparse-checkout to reapply sparsity patterns"));
 
 	reapply_opts.cone_mode = -1;
@@ -961,7 +961,7 @@ static int sparse_checkout_disable(int argc, const char **argv,
 	hashmap_init(&pl.recursive_hashmap, pl_hashmap_cmp, NULL, 0);
 	hashmap_init(&pl.parent_hashmap, pl_hashmap_cmp, NULL, 0);
 	pl.use_cone_patterns = 0;
-	core_apply_sparse_checkout = 1;
+	repo_settings_set_apply_sparse_checkout(the_repository, 1);
 
 	add_pattern("/*", empty_base, 0, &pl, 0);
 
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 88a36ea9f8..a6044dbe5f 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -536,7 +536,7 @@ static int add_worktree(const char *path, const char *refname,
 	 * If the current worktree has sparse-checkout enabled, then copy
 	 * the sparse-checkout patterns from the current worktree.
 	 */
-	if (core_apply_sparse_checkout)
+	if (repo_settings_get_apply_sparse_checkout(the_repository))
 		copy_sparse_checkout(sb_repo.buf);
 
 	/*
diff --git a/config.c b/config.c
index b18b5617fc..8fd4dd8c81 100644
--- a/config.c
+++ b/config.c
@@ -1612,11 +1612,6 @@ static int git_default_core_config(const char *var, const char *value,
 		return 0;
 	}
 
-	if (!strcmp(var, "core.sparsecheckout")) {
-		core_apply_sparse_checkout = git_config_bool(var, value);
-		return 0;
-	}
-
 	if (!strcmp(var, "core.sparsecheckoutcone")) {
 		core_sparse_checkout_cone = git_config_bool(var, value);
 		return 0;
diff --git a/dir.c b/dir.c
index a374972b62..2df307bb48 100644
--- a/dir.c
+++ b/dir.c
@@ -1503,7 +1503,7 @@ enum pattern_match_result path_matches_pattern_list(
 
 int init_sparse_checkout_patterns(struct index_state *istate)
 {
-	if (!core_apply_sparse_checkout)
+	if (!repo_settings_get_apply_sparse_checkout(the_repository))
 		return 1;
 	if (istate->sparse_checkout_patterns)
 		return 0;
diff --git a/environment.c b/environment.c
index c61d773e7e..a379a9149e 100644
--- a/environment.c
+++ b/environment.c
@@ -64,7 +64,6 @@ enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;
 #endif
 enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
 int grafts_keep_true_parents;
-int core_apply_sparse_checkout;
 int core_sparse_checkout_cone;
 int sparse_expect_files_outside_of_patterns;
 int merge_log_config = -1;
diff --git a/environment.h b/environment.h
index 3d98461a06..6a30512f3c 100644
--- a/environment.h
+++ b/environment.h
@@ -160,7 +160,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/repo-settings.c b/repo-settings.c
index 4129f8fb2b..406c70601c 100644
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -81,6 +81,7 @@ void prepare_repo_settings(struct repository *r)
 		      &r->settings.pack_use_bitmap_boundary_traversal,
 		      r->settings.pack_use_bitmap_boundary_traversal);
 	repo_cfg_bool(r, "core.usereplacerefs", &r->settings.read_replace_refs, 1);
+	repo_cfg_bool(r, "core.sparsecheckout", &r->settings.core_apply_sparse_checkout, 0);
 
 	/*
 	 * The GIT_TEST_MULTI_PACK_INDEX variable is special in that
@@ -227,3 +228,13 @@ void repo_settings_reset_shared_repository(struct repository *repo)
 {
 	repo->settings.shared_repository_initialized = 0;
 }
+
+int repo_settings_get_apply_sparse_checkout(struct repository *repo)
+{
+	return repo->settings.core_apply_sparse_checkout;
+}
+
+void repo_settings_set_apply_sparse_checkout(struct repository *repo, int value)
+{
+	repo->settings.core_apply_sparse_checkout = value;
+}
diff --git a/repo-settings.h b/repo-settings.h
index 2bf24b2597..e4e797e85c 100644
--- a/repo-settings.h
+++ b/repo-settings.h
@@ -67,6 +67,7 @@ struct repo_settings {
 	unsigned long big_file_threshold;
 
 	char *hooks_path;
+	int core_apply_sparse_checkout;
 };
 #define REPO_SETTINGS_INIT { \
 	.shared_repository = -1, \
@@ -98,4 +99,8 @@ int repo_settings_get_shared_repository(struct repository *repo);
 void repo_settings_set_shared_repository(struct repository *repo, int value);
 void repo_settings_reset_shared_repository(struct repository *repo);
 
+/* Read, set or reset the value for "core.sparsecheckout". */
+int repo_settings_get_apply_sparse_checkout(struct repository *repo);
+void repo_settings_set_apply_sparse_checkout(struct repository *repo, int value);
+
 #endif /* REPO_SETTINGS_H */
diff --git a/sparse-index.c b/sparse-index.c
index 5634abafaa..1565708190 100644
--- a/sparse-index.c
+++ b/sparse-index.c
@@ -150,7 +150,7 @@ 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)
+	if (!repo_settings_get_apply_sparse_checkout(istate->repo) || !core_sparse_checkout_cone)
 		return 0;
 
 	if (!(flags & SPARSE_INDEX_MEMORY_ONLY)) {
@@ -668,7 +668,7 @@ 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 ||
+	if (!repo_settings_get_apply_sparse_checkout(istate->repo) ||
 	    sparse_expect_files_outside_of_patterns)
 		return;
 
diff --git a/unpack-trees.c b/unpack-trees.c
index 471837f032..b3bf992067 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_settings_get_apply_sparse_checkout(repo) || !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 454601afa1..afe98a1bb9 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1773,7 +1773,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 (!repo_settings_get_apply_sparse_checkout(r) || 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.
-- 
2.43.0

Re: [GSOC PATCH] environment: move access to "core.sparsecheckout" into repo_settings

[Patrick Steinhardt]

On Tue, Jun 03, 2025 at 06:48:06PM +0530, Ayush Chandekar wrote:
> diff --git a/repo-settings.c b/repo-settings.c
> index 4129f8fb2b..406c70601c 100644
> --- a/repo-settings.c
> +++ b/repo-settings.c
> @@ -81,6 +81,7 @@ void prepare_repo_settings(struct repository *r)
>  		      &r->settings.pack_use_bitmap_boundary_traversal,
>  		      r->settings.pack_use_bitmap_boundary_traversal);
>  	repo_cfg_bool(r, "core.usereplacerefs", &r->settings.read_replace_refs, 1);
> +	repo_cfg_bool(r, "core.sparsecheckout", &r->settings.core_apply_sparse_checkout, 0);

The config is called "core.sparseCheckout", so why is the variable
called `core_apply_sparse_checkout`? `core_sparse_checkout` I would've
understood, but where does "apply" come from? Also, for brevity I think
we could just call it `settings.sparse_checkout`.

> @@ -227,3 +228,13 @@ void repo_settings_reset_shared_repository(struct repository *repo)
>  {
>  	repo->settings.shared_repository_initialized = 0;
>  }
> +
> +int repo_settings_get_apply_sparse_checkout(struct repository *repo)

Same remark here -- where does the "apply" part come from?

> +{
> +	return repo->settings.core_apply_sparse_checkout;
> +}
> +
> +void repo_settings_set_apply_sparse_checkout(struct repository *repo, int value)
> +{
> +	repo->settings.core_apply_sparse_checkout = value;
> +}

Getters and setters only really help in the case where they actually
provide a benefit. These don't though, so it's dubious whether we should
have them.

Also, shouldn't these functions call `prepare_repo_settings()`?
Otherwise we cannot guarantee that those settings have already been
parsed at all. And for the setter it could happen that the settings get
overwritten by the next caller of `prepare_repo_settings()`.

Patrick

Re: [GSOC PATCH] environment: move access to "core.sparsecheckout" into repo_settings

[Ayush Chandekar]

> The config is called "core.sparseCheckout", so why is the variable
> called `core_apply_sparse_checkout`? `core_sparse_checkout` I would've
> understood, but where does "apply" come from? Also, for brevity I think
> we could just call it `settings.sparse_checkout`.
>
Yes, I had this thought as well that adding "apply" doesn't make a lot of sense.
But I thought since the global variable has this name for a long time, there
must have been some reason. I can change the name if the "apply" doesn't hold
any value.

>
> > +{
> > +     return repo->settings.core_apply_sparse_checkout;
> > +}
> > +
> > +void repo_settings_set_apply_sparse_checkout(struct repository *repo, int value)
> > +{
> > +     repo->settings.core_apply_sparse_checkout = value;
> > +}
>
> Getters and setters only really help in the case where they actually
> provide a benefit. These don't though, so it's dubious whether we should
> have them.
>
> Also, shouldn't these functions call `prepare_repo_settings()`?
> Otherwise we cannot guarantee that those settings have already been
> parsed at all. And for the setter it could happen that the settings get
> overwritten by the next caller of `prepare_repo_settings()`.
>

Oh, yeah, you're right. So, if we use `prepare_repo_settings()` in
them, wouldn't
it be better to use getter and setter functions? Otherwise, I'd have to call
`prepare_repo_settings()` everywhere I'm using the setting.

Re: [GSOC PATCH] environment: move access to "core.sparsecheckout" into repo_settings

[Ben Knoble]

> Le 3 juin 2025 à 12:21, Ayush Chandekar <ayu.chandekar@gmail.com> a écrit :
> 
> 
>> The config is called "core.sparseCheckout", so why is the variable
>> called `core_apply_sparse_checkout`? `core_sparse_checkout` I would've
>> understood, but where does "apply" come from? Also, for brevity I think
>> we could just call it `settings.sparse_checkout`.
> Yes, I had this thought as well that adding "apply" doesn't make a lot of sense.
> But I thought since the global variable has this name for a long time, there
> must have been some reason. I can change the name if the "apply" doesn't hold
> any value.

Perhaps "git log -S core_apply_sparse_checkout config.c" or similar will reveal a reason? Or point us at a patch series that has some discussion?

> 
>>> +{
>>> +     return repo->settings.core_apply_sparse_checkout;
>>> +}
>>> +
>>> +void repo_settings_set_apply_sparse_checkout(struct repository *repo, int value)
>>> +{
>>> +     repo->settings.core_apply_sparse_checkout = value;
>>> +}
>> Getters and setters only really help in the case where they actually
>> provide a benefit. These don't though, so it's dubious whether we should
>> have them.

My thoughts exactly; see below.

>> Also, shouldn't these functions call `prepare_repo_settings()`?
>> Otherwise we cannot guarantee that those settings have already been
>> parsed at all. And for the setter it could happen that the settings get
>> overwritten by the next caller of `prepare_repo_settings()`.
> 
> Oh, yeah, you're right. So, if we use `prepare_repo_settings()` in
> them, wouldn't
> it be better to use getter and setter functions? Otherwise, I'd have to call
> `prepare_repo_settings()` everywhere I'm using the setting.

Aren’t most of the consumers builtins? And from a recent look, don’t they (all?) initialize the repo settings? I agree it is relatively painful to require developers to make sure that prepare_repo_settings has been called on each (new) code path that reads this variable, but OTOH I would expect that to be a straightforward audit during this change and then (see following) relatively easy to catch going forward. Is already a code convention that reading things in repo->settings depends on having prepared them?

Re: [GSOC PATCH] environment: move access to "core.sparsecheckout" into repo_settings

[Patrick Steinhardt]

On Tue, Jun 03, 2025 at 10:20:38PM -0400, Ben Knoble wrote:
> > Le 3 juin 2025 à 12:21, Ayush Chandekar <ayu.chandekar@gmail.com> a écrit :
> >>> +{
> >>> +     return repo->settings.core_apply_sparse_checkout;
> >>> +}
> >>> +
> >>> +void repo_settings_set_apply_sparse_checkout(struct repository *repo, int value)
> >>> +{
> >>> +     repo->settings.core_apply_sparse_checkout = value;
> >>> +}
> >> Getters and setters only really help in the case where they actually
> >> provide a benefit. These don't though, so it's dubious whether we should
> >> have them.
> 
> My thoughts exactly; see below.
> 
> >> Also, shouldn't these functions call `prepare_repo_settings()`?
> >> Otherwise we cannot guarantee that those settings have already been
> >> parsed at all. And for the setter it could happen that the settings get
> >> overwritten by the next caller of `prepare_repo_settings()`.
> > 
> > Oh, yeah, you're right. So, if we use `prepare_repo_settings()` in
> > them, wouldn't
> > it be better to use getter and setter functions? Otherwise, I'd have to call
> > `prepare_repo_settings()` everywhere I'm using the setting.
> 
> Aren’t most of the consumers builtins? And from a recent look, don’t
> they (all?) initialize the repo settings? I agree it is relatively
> painful to require developers to make sure that prepare_repo_settings
> has been called on each (new) code path that reads this variable, but
> OTOH I would expect that to be a straightforward audit during this
> change and then (see following) relatively easy to catch going
> forward. Is already a code convention that reading things in
> repo->settings depends on having prepared them?

Yes, it is a code convention. We have two patterns though:

  - Those that access the repo settings fields directly _always_ call
    `prepare_repo_settings()` manually beforehand.

  - Those that use a getter/setter rely on those to call
    `prepare_repo_settings()`.

So if you add the call to `prepare_repo_settings()` the getter and
setter do provide additional value. So in that case it may be sensible
to retain them indeed.

Patrick

Re: [GSOC PATCH] environment: move access to "core.sparsecheckout" into repo_settings

[Ayush Chandekar]

On Wed, Jun 4, 2025 at 7:50 AM Ben Knoble <ben.knoble@gmail.com> wrote:
>
>
> > Le 3 juin 2025 à 12:21, Ayush Chandekar <ayu.chandekar@gmail.com> a écrit :
> >
> > 
> >> The config is called "core.sparseCheckout", so why is the variable
> >> called `core_apply_sparse_checkout`? `core_sparse_checkout` I would've
> >> understood, but where does "apply" come from? Also, for brevity I think
> >> we could just call it `settings.sparse_checkout`.
> > Yes, I had this thought as well that adding "apply" doesn't make a lot of sense.
> > But I thought since the global variable has this name for a long time, there
> > must have been some reason. I can change the name if the "apply" doesn't hold
> > any value.
>
> Perhaps "git log -S core_apply_sparse_checkout config.c" or similar will reveal a reason? Or point us at a patch series that has some discussion?
>
I ran the command to see previous commits and also went through the
patch that introduced that variable, but there was no reasoning for
why it was named that way.
There was no comment as well for the patch that introduced it. I think
we can also get rid of the "core" (so it just becomes
"sparse_checkout") as there exist other core settings which don't have
"core" in their variable names and since we are changing the name
anyways.

> >
> >>> +{
> >>> +     return repo->settings.core_apply_sparse_checkout;
> >>> +}
> >>> +
> >>> +void repo_settings_set_apply_sparse_checkout(struct repository *repo, int value)
> >>> +{
> >>> +     repo->settings.core_apply_sparse_checkout = value;
> >>> +}
> >> Getters and setters only really help in the case where they actually
> >> provide a benefit. These don't though, so it's dubious whether we should
> >> have them.
>
> My thoughts exactly; see below.
>
> >> Also, shouldn't these functions call `prepare_repo_settings()`?
> >> Otherwise we cannot guarantee that those settings have already been
> >> parsed at all. And for the setter it could happen that the settings get
> >> overwritten by the next caller of `prepare_repo_settings()`.
> >
> > Oh, yeah, you're right. So, if we use `prepare_repo_settings()` in
> > them, wouldn't
> > it be better to use getter and setter functions? Otherwise, I'd have to call
> > `prepare_repo_settings()` everywhere I'm using the setting.
>
> Aren’t most of the consumers builtins? And from a recent look, don’t they (all?) initialize the repo settings? I agree it is relatively painful to require developers to make sure that prepare_repo_settings has been called on each (new) code path that reads this variable, but OTOH I would expect that to be a straightforward audit during this change and then (see following) relatively easy to catch going forward. Is already a code convention that reading things in repo->settings depends on having prepared them?

Yeah, as Patrick said, we have to call the `prepare_repo_settings()`
before we access these settings.
So, either we manually call the function or use it inside a
getter/setter function.

Thanks

[GSOC PATCH v2] environment: move access to "core.sparsecheckout" into repo_settings

[Ayush Chandekar]

The setting "core.sparsecheckout" is stored in the global
`core_apply_sparse_checkout` and is populated in config.c. Refactor the
code to store it in the variable `sparse_checkout` in the struct
`repo_settings`. Also, create functions to set and get the value of the
setting and update all the occurrences.

This also allows us to remove the definition `#define
USE_THE_REPOSITORY_VARIABLE` from the file 'builtin/backfill.c'.

This change is part of an ongoing effort to eliminate global variables,
improve modularity and help libify the codebase.

Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 builtin/backfill.c        |  5 +----
 builtin/clone.c           |  2 +-
 builtin/grep.c            |  2 +-
 builtin/mv.c              |  2 +-
 builtin/sparse-checkout.c | 20 ++++++++++----------
 builtin/worktree.c        |  2 +-
 config.c                  |  5 -----
 dir.c                     |  2 +-
 environment.c             |  1 -
 environment.h             |  1 -
 repo-settings.c           | 13 +++++++++++++
 repo-settings.h           |  5 +++++
 sparse-index.c            |  4 ++--
 unpack-trees.c            |  2 +-
 wt-status.c               |  2 +-
 15 files changed, 38 insertions(+), 30 deletions(-)

diff --git a/builtin/backfill.c b/builtin/backfill.c
index fa82ad2f6f..e16d116e98 100644
--- a/builtin/backfill.c
+++ b/builtin/backfill.c
@@ -1,6 +1,3 @@
-/* We need this macro to access core_apply_sparse_checkout */
-#define USE_THE_REPOSITORY_VARIABLE
-
 #include "builtin.h"
 #include "git-compat-util.h"
 #include "config.h"
@@ -139,7 +136,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_settings_get_sparse_checkout(repo);
 
 	result = do_backfill(&ctx);
 	backfill_context_clear(&ctx);
diff --git a/builtin/clone.c b/builtin/clone.c
index 91b9cd0d16..ba25e58258 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -621,7 +621,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;
+	repo_settings_set_sparse_checkout(the_repository, 1);
 
 	cmd.git_cmd = 1;
 	if (run_command(&cmd)) {
diff --git a/builtin/grep.c b/builtin/grep.c
index 3ce574a605..63342f5e36 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -481,7 +481,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"
 	 *	When running `grep` in the superproject, this setting is
 	 *	populated using the superproject's configs. However, once
 	 *	initialized, this config is globally accessible and is read by
diff --git a/builtin/mv.c b/builtin/mv.c
index 07548fe96a..95d24843d9 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -572,7 +572,7 @@ int cmd_mv(int argc,
 		rename_index_entry_at(the_repository->index, pos, dst);
 
 		if (ignore_sparse &&
-		    core_apply_sparse_checkout &&
+		    repo_settings_get_sparse_checkout(the_repository) &&
 		    core_sparse_checkout_cone) {
 			/*
 			 * NEEDSWORK: we are *not* paying attention to
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 1bf01591b2..662858b0a8 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -62,7 +62,7 @@ static int sparse_checkout_list(int argc, const char **argv, const char *prefix,
 	int res;
 
 	setup_work_tree();
-	if (!core_apply_sparse_checkout)
+	if (!repo_settings_get_sparse_checkout(the_repository))
 		die(_("this worktree is not sparse"));
 
 	argc = parse_options(argc, argv, prefix,
@@ -397,11 +397,11 @@ static int set_config(enum sparse_checkout_mode mode)
 
 static enum sparse_checkout_mode update_cone_mode(int *cone_mode) {
 	/* If not specified, use previous definition of cone mode */
-	if (*cone_mode == -1 && core_apply_sparse_checkout)
+	if (*cone_mode == -1 && repo_settings_get_sparse_checkout(the_repository))
 		*cone_mode = core_sparse_checkout_cone;
 
 	/* Set cone/non-cone mode appropriately */
-	core_apply_sparse_checkout = 1;
+	repo_settings_set_sparse_checkout(the_repository, 1);
 	if (*cone_mode == 1 || *cone_mode == -1) {
 		core_sparse_checkout_cone = 1;
 		return MODE_CONE_PATTERNS;
@@ -415,7 +415,7 @@ static int update_modes(int *cone_mode, int *sparse_index)
 	int mode, record_mode;
 
 	/* Determine if we need to record the mode; ensure sparse checkout on */
-	record_mode = (*cone_mode != -1) || !core_apply_sparse_checkout;
+	record_mode = (*cone_mode != -1) || !repo_settings_get_sparse_checkout(the_repository);
 
 	mode = update_cone_mode(cone_mode);
 	if (record_mode && set_config(mode))
@@ -695,9 +695,9 @@ static int modify_pattern_list(struct strvec *args, int use_stdin,
 		break;
 	}
 
-	if (!core_apply_sparse_checkout) {
+	if (!repo_settings_get_sparse_checkout(the_repository)) {
 		set_config(MODE_ALL_PATTERNS);
-		core_apply_sparse_checkout = 1;
+		repo_settings_set_sparse_checkout(the_repository, 1);
 		changed_config = 1;
 	}
 
@@ -793,7 +793,7 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix,
 	int ret;
 
 	setup_work_tree();
-	if (!core_apply_sparse_checkout)
+	if (!repo_settings_get_sparse_checkout(the_repository))
 		die(_("no sparse-checkout to add to"));
 
 	repo_read_index(the_repository);
@@ -902,7 +902,7 @@ static int sparse_checkout_reapply(int argc, const char **argv,
 	};
 
 	setup_work_tree();
-	if (!core_apply_sparse_checkout)
+	if (!repo_settings_get_sparse_checkout(the_repository))
 		die(_("must be in a sparse-checkout to reapply sparsity patterns"));
 
 	reapply_opts.cone_mode = -1;
@@ -935,7 +935,7 @@ static int sparse_checkout_disable(int argc, const char **argv,
 	struct pattern_list pl;
 
 	/*
-	 * We do not exit early if !core_apply_sparse_checkout; due to the
+	 * We do not exit early if !sparse_checkout; due to the
 	 * ability for users to manually muck things up between
 	 *   direct editing of .git/info/sparse-checkout
 	 *   running read-tree -m u HEAD or update-index --skip-worktree
@@ -961,7 +961,7 @@ static int sparse_checkout_disable(int argc, const char **argv,
 	hashmap_init(&pl.recursive_hashmap, pl_hashmap_cmp, NULL, 0);
 	hashmap_init(&pl.parent_hashmap, pl_hashmap_cmp, NULL, 0);
 	pl.use_cone_patterns = 0;
-	core_apply_sparse_checkout = 1;
+	repo_settings_set_sparse_checkout(the_repository, 1);
 
 	add_pattern("/*", empty_base, 0, &pl, 0);
 
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 88a36ea9f8..590bec334f 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -536,7 +536,7 @@ static int add_worktree(const char *path, const char *refname,
 	 * If the current worktree has sparse-checkout enabled, then copy
 	 * the sparse-checkout patterns from the current worktree.
 	 */
-	if (core_apply_sparse_checkout)
+	if (repo_settings_get_sparse_checkout(the_repository))
 		copy_sparse_checkout(sb_repo.buf);
 
 	/*
diff --git a/config.c b/config.c
index b18b5617fc..8fd4dd8c81 100644
--- a/config.c
+++ b/config.c
@@ -1612,11 +1612,6 @@ static int git_default_core_config(const char *var, const char *value,
 		return 0;
 	}
 
-	if (!strcmp(var, "core.sparsecheckout")) {
-		core_apply_sparse_checkout = git_config_bool(var, value);
-		return 0;
-	}
-
 	if (!strcmp(var, "core.sparsecheckoutcone")) {
 		core_sparse_checkout_cone = git_config_bool(var, value);
 		return 0;
diff --git a/dir.c b/dir.c
index a374972b62..323017e7a5 100644
--- a/dir.c
+++ b/dir.c
@@ -1503,7 +1503,7 @@ enum pattern_match_result path_matches_pattern_list(
 
 int init_sparse_checkout_patterns(struct index_state *istate)
 {
-	if (!core_apply_sparse_checkout)
+	if (!repo_settings_get_sparse_checkout(the_repository))
 		return 1;
 	if (istate->sparse_checkout_patterns)
 		return 0;
diff --git a/environment.c b/environment.c
index c61d773e7e..a379a9149e 100644
--- a/environment.c
+++ b/environment.c
@@ -64,7 +64,6 @@ enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;
 #endif
 enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
 int grafts_keep_true_parents;
-int core_apply_sparse_checkout;
 int core_sparse_checkout_cone;
 int sparse_expect_files_outside_of_patterns;
 int merge_log_config = -1;
diff --git a/environment.h b/environment.h
index 3d98461a06..6a30512f3c 100644
--- a/environment.h
+++ b/environment.h
@@ -160,7 +160,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/repo-settings.c b/repo-settings.c
index 4129f8fb2b..e11201e25b 100644
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -81,6 +81,7 @@ void prepare_repo_settings(struct repository *r)
 		      &r->settings.pack_use_bitmap_boundary_traversal,
 		      r->settings.pack_use_bitmap_boundary_traversal);
 	repo_cfg_bool(r, "core.usereplacerefs", &r->settings.read_replace_refs, 1);
+	repo_cfg_bool(r, "core.sparsecheckout", &r->settings.sparse_checkout, 0);
 
 	/*
 	 * The GIT_TEST_MULTI_PACK_INDEX variable is special in that
@@ -227,3 +228,15 @@ void repo_settings_reset_shared_repository(struct repository *repo)
 {
 	repo->settings.shared_repository_initialized = 0;
 }
+
+int repo_settings_get_sparse_checkout(struct repository *repo)
+{
+	prepare_repo_settings(repo);
+	return repo->settings.sparse_checkout;
+}
+
+void repo_settings_set_sparse_checkout(struct repository *repo, int value)
+{
+	prepare_repo_settings(repo);
+	repo->settings.sparse_checkout = value;
+}
diff --git a/repo-settings.h b/repo-settings.h
index 2bf24b2597..0d181e3f21 100644
--- a/repo-settings.h
+++ b/repo-settings.h
@@ -67,6 +67,7 @@ struct repo_settings {
 	unsigned long big_file_threshold;
 
 	char *hooks_path;
+	int sparse_checkout;
 };
 #define REPO_SETTINGS_INIT { \
 	.shared_repository = -1, \
@@ -98,4 +99,8 @@ int repo_settings_get_shared_repository(struct repository *repo);
 void repo_settings_set_shared_repository(struct repository *repo, int value);
 void repo_settings_reset_shared_repository(struct repository *repo);
 
+/* Read or set the value for "core.sparseCheckout". */
+int repo_settings_get_sparse_checkout(struct repository *repo);
+void repo_settings_set_sparse_checkout(struct repository *repo, int value);
+
 #endif /* REPO_SETTINGS_H */
diff --git a/sparse-index.c b/sparse-index.c
index 5634abafaa..1bedb35001 100644
--- a/sparse-index.c
+++ b/sparse-index.c
@@ -150,7 +150,7 @@ 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)
+	if (!repo_settings_get_sparse_checkout(istate->repo) || !core_sparse_checkout_cone)
 		return 0;
 
 	if (!(flags & SPARSE_INDEX_MEMORY_ONLY)) {
@@ -668,7 +668,7 @@ 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 ||
+	if (!repo_settings_get_sparse_checkout(istate->repo) ||
 	    sparse_expect_files_outside_of_patterns)
 		return;
 
diff --git a/unpack-trees.c b/unpack-trees.c
index 471837f032..c8eba27e5f 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_settings_get_sparse_checkout(repo) || !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 454601afa1..004f2945b7 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1773,7 +1773,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 (!repo_settings_get_sparse_checkout(r) || 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.
-- 
2.49.0

Re: [GSOC PATCH v2] environment: move access to "core.sparsecheckout" into repo_settings

[Christian Couder]

On Sun, Jun 8, 2025 at 2:32 AM Ayush Chandekar <ayu.chandekar@gmail.com> wrote:
>
> The setting "core.sparsecheckout" is stored in the global
> `core_apply_sparse_checkout` and is populated in config.c. Refactor the
> code to store it in the variable `sparse_checkout` in the struct
> `repo_settings`. Also, create functions to set and get the value of the
> setting and update all the occurrences.
>
> This also allows us to remove the definition `#define
> USE_THE_REPOSITORY_VARIABLE` from the file 'builtin/backfill.c'.
>
> This change is part of an ongoing effort to eliminate global variables,
> improve modularity and help libify the codebase.
>
> Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
> ---

When sending a v2 like this one, it's nice if you can describe what
changed since v1 here, after the line starting with 3 dashes "---".
Providing a range-diff here, when it makes sense, could be a good idea
and help reviewers too. Speaking of reviewers, thanking or just
mentioning them is nice too while at it.

>  builtin/backfill.c        |  5 +----
>  builtin/clone.c           |  2 +-
>  builtin/grep.c            |  2 +-
>  builtin/mv.c              |  2 +-
>  builtin/sparse-checkout.c | 20 ++++++++++----------
>  builtin/worktree.c        |  2 +-
>  config.c                  |  5 -----
>  dir.c                     |  2 +-
>  environment.c             |  1 -
>  environment.h             |  1 -
>  repo-settings.c           | 13 +++++++++++++
>  repo-settings.h           |  5 +++++
>  sparse-index.c            |  4 ++--
>  unpack-trees.c            |  2 +-
>  wt-status.c               |  2 +-
>  15 files changed, 38 insertions(+), 30 deletions(-)

[PATCH v3] environment: move access to "core.sparsecheckout" into repo_settings

[Ayush Chandekar]

The setting "core.sparsecheckout" is stored in the global
`core_apply_sparse_checkout` and is populated in config.c. Refactor the
code to store it in the variable `sparse_checkout` in the struct
`repo_settings`. Also, create functions to set and get the value of the
setting and update all the occurrences.

This also allows us to remove the definition `#define
USE_THE_REPOSITORY_VARIABLE` from the file 'builtin/backfill.c'.

This change is part of an ongoing effort to eliminate global variables,
improve modularity and help libify the codebase.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---

Note that this patch is the same as v2, the v3 just contains the range-diff 
of v2/v3 vs v1, and the 'Mentored-by' trailers.

I thank my mentors Christian and Ghanshyam, as well as Patrick and Ben, 
for reviewing my patch and suggesting changes.

Summary of the range-diff:
* Change the variable name from 'core_apply_sparse_checkout' to 'sparse_checkout'.
* Update the getter and setter function names to use 'sparse_checkout' instead of 'apply_sparse_checkout'.
* Call the function 'prepare_repo_settings()' in the getter/setter functions of the variable.

Range-diff:
1:  a385fc1dbe ! 1:  c49dcde03b environment: move access to "core.sparsecheckout" into repo_settings
    @@ Commit message
     
         The setting "core.sparsecheckout" is stored in the global
         `core_apply_sparse_checkout` and is populated in config.c. Refactor the
    -    code to store it inside the struct `repo_settings`. Also, create
    -    functions to set and get the value of the setting and update all the
    -    occurrences.
    +    code to store it in the variable `sparse_checkout` in the struct
    +    `repo_settings`. Also, create functions to set and get the value of the
    +    setting and update all the occurrences.
     
         This also allows us to remove the definition `#define
         USE_THE_REPOSITORY_VARIABLE` from the file 'builtin/backfill.c'.
    @@ Commit message
         This change is part of an ongoing effort to eliminate global variables,
         improve modularity and help libify the codebase.
     
    +    Mentored-by: Christian Couder <christian.couder@gmail.com>
    +    Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
         Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
     
      ## builtin/backfill.c ##
    @@ builtin/backfill.c: int cmd_backfill(int argc, const char **argv, const char *pr
      
      	if (ctx.sparse < 0)
     -		ctx.sparse = core_apply_sparse_checkout;
    -+		ctx.sparse = repo_settings_get_apply_sparse_checkout(repo);
    ++		ctx.sparse = repo_settings_get_sparse_checkout(repo);
      
      	result = do_backfill(&ctx);
      	backfill_context_clear(&ctx);
    @@ builtin/clone.c: static int git_sparse_checkout_init(const char *repo)
      	 * for the later checkout to use the sparse-checkout file.
      	 */
     -	core_apply_sparse_checkout = 1;
    -+	repo_settings_set_apply_sparse_checkout(the_repository, 1);
    ++	repo_settings_set_sparse_checkout(the_repository, 1);
      
      	cmd.git_cmd = 1;
      	if (run_command(&cmd)) {
     
    + ## builtin/grep.c ##
    +@@ builtin/grep.c: 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"
    + 	 *	When running `grep` in the superproject, this setting is
    + 	 *	populated using the superproject's configs. However, once
    + 	 *	initialized, this config is globally accessible and is read by
    +
      ## builtin/mv.c ##
     @@ builtin/mv.c: int cmd_mv(int argc,
      		rename_index_entry_at(the_repository->index, pos, dst);
      
      		if (ignore_sparse &&
     -		    core_apply_sparse_checkout &&
    -+		    repo_settings_get_apply_sparse_checkout(the_repository) &&
    ++		    repo_settings_get_sparse_checkout(the_repository) &&
      		    core_sparse_checkout_cone) {
      			/*
      			 * NEEDSWORK: we are *not* paying attention to
    @@ builtin/sparse-checkout.c: static int sparse_checkout_list(int argc, const char
      
      	setup_work_tree();
     -	if (!core_apply_sparse_checkout)
    -+	if (!repo_settings_get_apply_sparse_checkout(the_repository))
    ++	if (!repo_settings_get_sparse_checkout(the_repository))
      		die(_("this worktree is not sparse"));
      
      	argc = parse_options(argc, argv, prefix,
    @@ builtin/sparse-checkout.c: static int set_config(enum sparse_checkout_mode mode)
      static enum sparse_checkout_mode update_cone_mode(int *cone_mode) {
      	/* If not specified, use previous definition of cone mode */
     -	if (*cone_mode == -1 && core_apply_sparse_checkout)
    -+	if (*cone_mode == -1 && repo_settings_get_apply_sparse_checkout(the_repository))
    ++	if (*cone_mode == -1 && repo_settings_get_sparse_checkout(the_repository))
      		*cone_mode = core_sparse_checkout_cone;
      
      	/* Set cone/non-cone mode appropriately */
     -	core_apply_sparse_checkout = 1;
    -+	repo_settings_set_apply_sparse_checkout(the_repository, 1);
    ++	repo_settings_set_sparse_checkout(the_repository, 1);
      	if (*cone_mode == 1 || *cone_mode == -1) {
      		core_sparse_checkout_cone = 1;
      		return MODE_CONE_PATTERNS;
    @@ builtin/sparse-checkout.c: static int update_modes(int *cone_mode, int *sparse_i
      
      	/* Determine if we need to record the mode; ensure sparse checkout on */
     -	record_mode = (*cone_mode != -1) || !core_apply_sparse_checkout;
    -+	record_mode = (*cone_mode != -1) || !repo_settings_get_apply_sparse_checkout(the_repository);
    ++	record_mode = (*cone_mode != -1) || !repo_settings_get_sparse_checkout(the_repository);
      
      	mode = update_cone_mode(cone_mode);
      	if (record_mode && set_config(mode))
    @@ builtin/sparse-checkout.c: static int modify_pattern_list(struct strvec *args, i
      	}
      
     -	if (!core_apply_sparse_checkout) {
    -+	if (!repo_settings_get_apply_sparse_checkout(the_repository)) {
    ++	if (!repo_settings_get_sparse_checkout(the_repository)) {
      		set_config(MODE_ALL_PATTERNS);
     -		core_apply_sparse_checkout = 1;
    -+		repo_settings_set_apply_sparse_checkout(the_repository, 1);
    ++		repo_settings_set_sparse_checkout(the_repository, 1);
      		changed_config = 1;
      	}
      
    @@ builtin/sparse-checkout.c: static int sparse_checkout_add(int argc, const char *
      
      	setup_work_tree();
     -	if (!core_apply_sparse_checkout)
    -+	if (!repo_settings_get_apply_sparse_checkout(the_repository))
    ++	if (!repo_settings_get_sparse_checkout(the_repository))
      		die(_("no sparse-checkout to add to"));
      
      	repo_read_index(the_repository);
    @@ builtin/sparse-checkout.c: static int sparse_checkout_reapply(int argc, const ch
      
      	setup_work_tree();
     -	if (!core_apply_sparse_checkout)
    -+	if (!repo_settings_get_apply_sparse_checkout(the_repository))
    ++	if (!repo_settings_get_sparse_checkout(the_repository))
      		die(_("must be in a sparse-checkout to reapply sparsity patterns"));
      
      	reapply_opts.cone_mode = -1;
    +@@ builtin/sparse-checkout.c: static int sparse_checkout_disable(int argc, const char **argv,
    + 	struct pattern_list pl;
    + 
    + 	/*
    +-	 * We do not exit early if !core_apply_sparse_checkout; due to the
    ++	 * We do not exit early if !sparse_checkout; due to the
    + 	 * ability for users to manually muck things up between
    + 	 *   direct editing of .git/info/sparse-checkout
    + 	 *   running read-tree -m u HEAD or update-index --skip-worktree
     @@ builtin/sparse-checkout.c: static int sparse_checkout_disable(int argc, const char **argv,
      	hashmap_init(&pl.recursive_hashmap, pl_hashmap_cmp, NULL, 0);
      	hashmap_init(&pl.parent_hashmap, pl_hashmap_cmp, NULL, 0);
      	pl.use_cone_patterns = 0;
     -	core_apply_sparse_checkout = 1;
    -+	repo_settings_set_apply_sparse_checkout(the_repository, 1);
    ++	repo_settings_set_sparse_checkout(the_repository, 1);
      
      	add_pattern("/*", empty_base, 0, &pl, 0);
      
    @@ builtin/worktree.c: static int add_worktree(const char *path, const char *refnam
      	 * the sparse-checkout patterns from the current worktree.
      	 */
     -	if (core_apply_sparse_checkout)
    -+	if (repo_settings_get_apply_sparse_checkout(the_repository))
    ++	if (repo_settings_get_sparse_checkout(the_repository))
      		copy_sparse_checkout(sb_repo.buf);
      
      	/*
    @@ dir.c: enum pattern_match_result path_matches_pattern_list(
      int init_sparse_checkout_patterns(struct index_state *istate)
      {
     -	if (!core_apply_sparse_checkout)
    -+	if (!repo_settings_get_apply_sparse_checkout(the_repository))
    ++	if (!repo_settings_get_sparse_checkout(the_repository))
      		return 1;
      	if (istate->sparse_checkout_patterns)
      		return 0;
    @@ repo-settings.c: void prepare_repo_settings(struct repository *r)
      		      &r->settings.pack_use_bitmap_boundary_traversal,
      		      r->settings.pack_use_bitmap_boundary_traversal);
      	repo_cfg_bool(r, "core.usereplacerefs", &r->settings.read_replace_refs, 1);
    -+	repo_cfg_bool(r, "core.sparsecheckout", &r->settings.core_apply_sparse_checkout, 0);
    ++	repo_cfg_bool(r, "core.sparsecheckout", &r->settings.sparse_checkout, 0);
      
      	/*
      	 * The GIT_TEST_MULTI_PACK_INDEX variable is special in that
    @@ repo-settings.c: void repo_settings_reset_shared_repository(struct repository *r
      	repo->settings.shared_repository_initialized = 0;
      }
     +
    -+int repo_settings_get_apply_sparse_checkout(struct repository *repo)
    ++int repo_settings_get_sparse_checkout(struct repository *repo)
     +{
    -+	return repo->settings.core_apply_sparse_checkout;
    ++	prepare_repo_settings(repo);
    ++	return repo->settings.sparse_checkout;
     +}
     +
    -+void repo_settings_set_apply_sparse_checkout(struct repository *repo, int value)
    ++void repo_settings_set_sparse_checkout(struct repository *repo, int value)
     +{
    -+	repo->settings.core_apply_sparse_checkout = value;
    ++	prepare_repo_settings(repo);
    ++	repo->settings.sparse_checkout = value;
     +}
     
      ## repo-settings.h ##
    @@ repo-settings.h: struct repo_settings {
      	unsigned long big_file_threshold;
      
      	char *hooks_path;
    -+	int core_apply_sparse_checkout;
    ++	int sparse_checkout;
      };
      #define REPO_SETTINGS_INIT { \
      	.shared_repository = -1, \
    @@ repo-settings.h: int repo_settings_get_shared_repository(struct repository *repo
      void repo_settings_set_shared_repository(struct repository *repo, int value);
      void repo_settings_reset_shared_repository(struct repository *repo);
      
    -+/* Read, set or reset the value for "core.sparsecheckout". */
    -+int repo_settings_get_apply_sparse_checkout(struct repository *repo);
    -+void repo_settings_set_apply_sparse_checkout(struct repository *repo, int value);
    ++/* Read or set the value for "core.sparseCheckout". */
    ++int repo_settings_get_sparse_checkout(struct repository *repo);
    ++void repo_settings_set_sparse_checkout(struct repository *repo, int value);
     +
      #endif /* REPO_SETTINGS_H */
     
    @@ sparse-index.c: 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)
    -+	if (!repo_settings_get_apply_sparse_checkout(istate->repo) || !core_sparse_checkout_cone)
    ++	if (!repo_settings_get_sparse_checkout(istate->repo) || !core_sparse_checkout_cone)
      		return 0;
      
      	if (!(flags & SPARSE_INDEX_MEMORY_ONLY)) {
    @@ sparse-index.c: static void clear_skip_worktree_from_present_files_full(struct i
      void clear_skip_worktree_from_present_files(struct index_state *istate)
      {
     -	if (!core_apply_sparse_checkout ||
    -+	if (!repo_settings_get_apply_sparse_checkout(istate->repo) ||
    ++	if (!repo_settings_get_sparse_checkout(istate->repo) ||
      	    sparse_expect_files_outside_of_patterns)
      		return;
      
    @@ unpack-trees.c: int unpack_trees(unsigned len, struct tree_desc *t, struct unpac
      		update_sparsity_for_prefix(o->prefix, o->src_index);
      
     -	if (!core_apply_sparse_checkout || !o->update)
    -+	if (!repo_settings_get_apply_sparse_checkout(repo) || !o->update)
    ++	if (!repo_settings_get_sparse_checkout(repo) || !o->update)
      		o->skip_sparse_checkout = 1;
      	if (!o->skip_sparse_checkout) {
      		memset(&pl, 0, sizeof(pl));
    @@ wt-status.c: static void wt_status_check_sparse_checkout(struct repository *r,
      	int i;
      
     -	if (!core_apply_sparse_checkout || r->index->cache_nr == 0) {
    -+	if (!repo_settings_get_apply_sparse_checkout(r) || r->index->cache_nr == 0) {
    ++	if (!repo_settings_get_sparse_checkout(r) || 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.

 builtin/backfill.c        |  5 +----
 builtin/clone.c           |  2 +-
 builtin/grep.c            |  2 +-
 builtin/mv.c              |  2 +-
 builtin/sparse-checkout.c | 20 ++++++++++----------
 builtin/worktree.c        |  2 +-
 config.c                  |  5 -----
 dir.c                     |  2 +-
 environment.c             |  1 -
 environment.h             |  1 -
 repo-settings.c           | 13 +++++++++++++
 repo-settings.h           |  5 +++++
 sparse-index.c            |  4 ++--
 unpack-trees.c            |  2 +-
 wt-status.c               |  2 +-
 15 files changed, 38 insertions(+), 30 deletions(-)

diff --git a/builtin/backfill.c b/builtin/backfill.c
index fa82ad2f6f..e16d116e98 100644
--- a/builtin/backfill.c
+++ b/builtin/backfill.c
@@ -1,6 +1,3 @@
-/* We need this macro to access core_apply_sparse_checkout */
-#define USE_THE_REPOSITORY_VARIABLE
-
 #include "builtin.h"
 #include "git-compat-util.h"
 #include "config.h"
@@ -139,7 +136,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_settings_get_sparse_checkout(repo);
 
 	result = do_backfill(&ctx);
 	backfill_context_clear(&ctx);
diff --git a/builtin/clone.c b/builtin/clone.c
index 91b9cd0d16..ba25e58258 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -621,7 +621,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;
+	repo_settings_set_sparse_checkout(the_repository, 1);
 
 	cmd.git_cmd = 1;
 	if (run_command(&cmd)) {
diff --git a/builtin/grep.c b/builtin/grep.c
index 3ce574a605..63342f5e36 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -481,7 +481,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"
 	 *	When running `grep` in the superproject, this setting is
 	 *	populated using the superproject's configs. However, once
 	 *	initialized, this config is globally accessible and is read by
diff --git a/builtin/mv.c b/builtin/mv.c
index 07548fe96a..95d24843d9 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -572,7 +572,7 @@ int cmd_mv(int argc,
 		rename_index_entry_at(the_repository->index, pos, dst);
 
 		if (ignore_sparse &&
-		    core_apply_sparse_checkout &&
+		    repo_settings_get_sparse_checkout(the_repository) &&
 		    core_sparse_checkout_cone) {
 			/*
 			 * NEEDSWORK: we are *not* paying attention to
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 1bf01591b2..662858b0a8 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -62,7 +62,7 @@ static int sparse_checkout_list(int argc, const char **argv, const char *prefix,
 	int res;
 
 	setup_work_tree();
-	if (!core_apply_sparse_checkout)
+	if (!repo_settings_get_sparse_checkout(the_repository))
 		die(_("this worktree is not sparse"));
 
 	argc = parse_options(argc, argv, prefix,
@@ -397,11 +397,11 @@ static int set_config(enum sparse_checkout_mode mode)
 
 static enum sparse_checkout_mode update_cone_mode(int *cone_mode) {
 	/* If not specified, use previous definition of cone mode */
-	if (*cone_mode == -1 && core_apply_sparse_checkout)
+	if (*cone_mode == -1 && repo_settings_get_sparse_checkout(the_repository))
 		*cone_mode = core_sparse_checkout_cone;
 
 	/* Set cone/non-cone mode appropriately */
-	core_apply_sparse_checkout = 1;
+	repo_settings_set_sparse_checkout(the_repository, 1);
 	if (*cone_mode == 1 || *cone_mode == -1) {
 		core_sparse_checkout_cone = 1;
 		return MODE_CONE_PATTERNS;
@@ -415,7 +415,7 @@ static int update_modes(int *cone_mode, int *sparse_index)
 	int mode, record_mode;
 
 	/* Determine if we need to record the mode; ensure sparse checkout on */
-	record_mode = (*cone_mode != -1) || !core_apply_sparse_checkout;
+	record_mode = (*cone_mode != -1) || !repo_settings_get_sparse_checkout(the_repository);
 
 	mode = update_cone_mode(cone_mode);
 	if (record_mode && set_config(mode))
@@ -695,9 +695,9 @@ static int modify_pattern_list(struct strvec *args, int use_stdin,
 		break;
 	}
 
-	if (!core_apply_sparse_checkout) {
+	if (!repo_settings_get_sparse_checkout(the_repository)) {
 		set_config(MODE_ALL_PATTERNS);
-		core_apply_sparse_checkout = 1;
+		repo_settings_set_sparse_checkout(the_repository, 1);
 		changed_config = 1;
 	}
 
@@ -793,7 +793,7 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix,
 	int ret;
 
 	setup_work_tree();
-	if (!core_apply_sparse_checkout)
+	if (!repo_settings_get_sparse_checkout(the_repository))
 		die(_("no sparse-checkout to add to"));
 
 	repo_read_index(the_repository);
@@ -902,7 +902,7 @@ static int sparse_checkout_reapply(int argc, const char **argv,
 	};
 
 	setup_work_tree();
-	if (!core_apply_sparse_checkout)
+	if (!repo_settings_get_sparse_checkout(the_repository))
 		die(_("must be in a sparse-checkout to reapply sparsity patterns"));
 
 	reapply_opts.cone_mode = -1;
@@ -935,7 +935,7 @@ static int sparse_checkout_disable(int argc, const char **argv,
 	struct pattern_list pl;
 
 	/*
-	 * We do not exit early if !core_apply_sparse_checkout; due to the
+	 * We do not exit early if !sparse_checkout; due to the
 	 * ability for users to manually muck things up between
 	 *   direct editing of .git/info/sparse-checkout
 	 *   running read-tree -m u HEAD or update-index --skip-worktree
@@ -961,7 +961,7 @@ static int sparse_checkout_disable(int argc, const char **argv,
 	hashmap_init(&pl.recursive_hashmap, pl_hashmap_cmp, NULL, 0);
 	hashmap_init(&pl.parent_hashmap, pl_hashmap_cmp, NULL, 0);
 	pl.use_cone_patterns = 0;
-	core_apply_sparse_checkout = 1;
+	repo_settings_set_sparse_checkout(the_repository, 1);
 
 	add_pattern("/*", empty_base, 0, &pl, 0);
 
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 88a36ea9f8..590bec334f 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -536,7 +536,7 @@ static int add_worktree(const char *path, const char *refname,
 	 * If the current worktree has sparse-checkout enabled, then copy
 	 * the sparse-checkout patterns from the current worktree.
 	 */
-	if (core_apply_sparse_checkout)
+	if (repo_settings_get_sparse_checkout(the_repository))
 		copy_sparse_checkout(sb_repo.buf);
 
 	/*
diff --git a/config.c b/config.c
index b18b5617fc..8fd4dd8c81 100644
--- a/config.c
+++ b/config.c
@@ -1612,11 +1612,6 @@ static int git_default_core_config(const char *var, const char *value,
 		return 0;
 	}
 
-	if (!strcmp(var, "core.sparsecheckout")) {
-		core_apply_sparse_checkout = git_config_bool(var, value);
-		return 0;
-	}
-
 	if (!strcmp(var, "core.sparsecheckoutcone")) {
 		core_sparse_checkout_cone = git_config_bool(var, value);
 		return 0;
diff --git a/dir.c b/dir.c
index a374972b62..323017e7a5 100644
--- a/dir.c
+++ b/dir.c
@@ -1503,7 +1503,7 @@ enum pattern_match_result path_matches_pattern_list(
 
 int init_sparse_checkout_patterns(struct index_state *istate)
 {
-	if (!core_apply_sparse_checkout)
+	if (!repo_settings_get_sparse_checkout(the_repository))
 		return 1;
 	if (istate->sparse_checkout_patterns)
 		return 0;
diff --git a/environment.c b/environment.c
index c61d773e7e..a379a9149e 100644
--- a/environment.c
+++ b/environment.c
@@ -64,7 +64,6 @@ enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;
 #endif
 enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
 int grafts_keep_true_parents;
-int core_apply_sparse_checkout;
 int core_sparse_checkout_cone;
 int sparse_expect_files_outside_of_patterns;
 int merge_log_config = -1;
diff --git a/environment.h b/environment.h
index 3d98461a06..6a30512f3c 100644
--- a/environment.h
+++ b/environment.h
@@ -160,7 +160,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/repo-settings.c b/repo-settings.c
index 4129f8fb2b..e11201e25b 100644
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -81,6 +81,7 @@ void prepare_repo_settings(struct repository *r)
 		      &r->settings.pack_use_bitmap_boundary_traversal,
 		      r->settings.pack_use_bitmap_boundary_traversal);
 	repo_cfg_bool(r, "core.usereplacerefs", &r->settings.read_replace_refs, 1);
+	repo_cfg_bool(r, "core.sparsecheckout", &r->settings.sparse_checkout, 0);
 
 	/*
 	 * The GIT_TEST_MULTI_PACK_INDEX variable is special in that
@@ -227,3 +228,15 @@ void repo_settings_reset_shared_repository(struct repository *repo)
 {
 	repo->settings.shared_repository_initialized = 0;
 }
+
+int repo_settings_get_sparse_checkout(struct repository *repo)
+{
+	prepare_repo_settings(repo);
+	return repo->settings.sparse_checkout;
+}
+
+void repo_settings_set_sparse_checkout(struct repository *repo, int value)
+{
+	prepare_repo_settings(repo);
+	repo->settings.sparse_checkout = value;
+}
diff --git a/repo-settings.h b/repo-settings.h
index 2bf24b2597..0d181e3f21 100644
--- a/repo-settings.h
+++ b/repo-settings.h
@@ -67,6 +67,7 @@ struct repo_settings {
 	unsigned long big_file_threshold;
 
 	char *hooks_path;
+	int sparse_checkout;
 };
 #define REPO_SETTINGS_INIT { \
 	.shared_repository = -1, \
@@ -98,4 +99,8 @@ int repo_settings_get_shared_repository(struct repository *repo);
 void repo_settings_set_shared_repository(struct repository *repo, int value);
 void repo_settings_reset_shared_repository(struct repository *repo);
 
+/* Read or set the value for "core.sparseCheckout". */
+int repo_settings_get_sparse_checkout(struct repository *repo);
+void repo_settings_set_sparse_checkout(struct repository *repo, int value);
+
 #endif /* REPO_SETTINGS_H */
diff --git a/sparse-index.c b/sparse-index.c
index 5634abafaa..1bedb35001 100644
--- a/sparse-index.c
+++ b/sparse-index.c
@@ -150,7 +150,7 @@ 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)
+	if (!repo_settings_get_sparse_checkout(istate->repo) || !core_sparse_checkout_cone)
 		return 0;
 
 	if (!(flags & SPARSE_INDEX_MEMORY_ONLY)) {
@@ -668,7 +668,7 @@ 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 ||
+	if (!repo_settings_get_sparse_checkout(istate->repo) ||
 	    sparse_expect_files_outside_of_patterns)
 		return;
 
diff --git a/unpack-trees.c b/unpack-trees.c
index 471837f032..c8eba27e5f 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_settings_get_sparse_checkout(repo) || !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 454601afa1..004f2945b7 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1773,7 +1773,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 (!repo_settings_get_sparse_checkout(r) || 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.

-- 
2.49.0

Re: [PATCH v3] environment: move access to "core.sparsecheckout" into repo_settings

[Junio C Hamano]

Ayush Chandekar <ayu.chandekar@gmail.com> writes:

> The setting "core.sparsecheckout" is stored in the global
> `core_apply_sparse_checkout` and is populated in config.c. Refactor the
> code to store it in the variable `sparse_checkout` in the struct
> `repo_settings`. Also, create functions to set and get the value of the
> setting and update all the occurrences.
>
> This also allows us to remove the definition `#define
> USE_THE_REPOSITORY_VARIABLE` from the file 'builtin/backfill.c'.
>
> This change is part of an ongoing effort to eliminate global variables,
> improve modularity and help libify the codebase.
>
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
> Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
> ---

It may make sense to move the sparse-checkout bit from the
process-wide global to per-repository settings.  One advantage of
having it at the global level is that the code that does not require
to be in a repository can refer to it, but the nature of this variable
requires a repository to make sense anyway, so that advantage does
not apply to it.

But looking at members of repo.settings, among 25+ of them, there
are only a handful of variables with getter-setter pair, and this
patch seems to add yet another for a simple boolean.  Among existing
setters and getters, prevailing pattern seem to be

	/* getter */
	if (!repo->settings.foo)
		repo_cfg_ulong(repo, "core.foo", &repo->settings.foo, FOO_DEFAULT);
	return repo->settings.foo;

	/* setter */
	repo->settings.foo = value;

but there are more involved ones.

This new one adopts the most expensive pattern from
get_warn_ambiguous_refs() where prepare_repo_settings() is used, as
if it fears to be fed a repo instance that hasn't been prepared yet,
and it even does so for its setter, which I think is probably a
mistake.

And there are tons of members in repo-settings that are initialized
in prepare_repo_settings() (perhaps reading from the configuration
file or assigned their default values) and without having an
explicit setter/getter pair.  If the code paths that depends on the
value of these members are functioning correctly (and they should,
given how widely Git is used these days and not having heard about
bugs coming from these variables), would not it mean that in the
program start up sequence, prepare_repo_settings() is called early
enough so that all the code paths that care about these repository
settings values do not have to worry about having a getter setter
pair at all?

What is it so special about sparse-checkout bit that cannot be using
the same "initialize in prepare_repo_settings() and access it
directly thereafter" model, or "ah, do you need to know about this
bit?  Let me see if I already have figured it out, and otherwise let
me read from the configuration, and give it back to you" model?

My gut feeling, if I have to choose between "lazy loading" and
"popluate in prepare_repo_settings() and then access the member
directly thereafter" for this variable, I may pick the latter for
this particular variable.

Re: [PATCH v3] environment: move access to "core.sparsecheckout" into repo_settings

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> My gut feeling, if I have to choose between "lazy loading" and
> "popluate in prepare_repo_settings() and then access the member
> directly thereafter" for this variable, I may pick the latter for
> this particular variable.

I left the reason behind this choice unsaid, but let's spell it out.

Originally, this was read in git_config(git_default_config) into a
global, and that is probably because almost everybody that touches
the working tree files needs to know about it.  So populating it in
prepare_repo_settings() for everybody, even though the calling code
path does not even need it, would be OK---they were paying the cost
to read it when they read the default configuration variables.

It seems Patrick earlier made a confused comment on the two models
that may need a bit clarifying.

Here are the rules to follow.

 - "lazy loading" is not wrong. Initialize the member to an
   "uninitialized" state, never touch the member in
   prepare_repo_settings(), and have its getter check for the
   "uninitialized" state to lazily load it, or have its setter do
   its thing.  prepare_repo_settings() should not even be aware of
   the member, if we are going to give the member a getter/setter
   pair.

 - "Without getter/setter" is not wrong, either.  Load the member in
   prepare_repo_settings(), which will turn into a no-op once it is
   called to a repo instance.  Use the member directly afterwards.

You cannot mix and match.  If the variable is rarely used, you'd
want to catch the initial access and lazily load it, hence you are
required to have a getter/setter pair and lazily populate the member
in your getter.  If your variable is very commonly used, load it
once in prepare_repo_settings(), and because you are not going to do
anything special upon the first access, there is no need to have a
getter/setter pair.

Re: [PATCH v3] environment: move access to "core.sparsecheckout" into repo_settings

[Ayush Chandekar]

On Thu, Jun 12, 2025 at 3:03 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> > My gut feeling, if I have to choose between "lazy loading" and
> > "popluate in prepare_repo_settings() and then access the member
> > directly thereafter" for this variable, I may pick the latter for
> > this particular variable.
>
> I left the reason behind this choice unsaid, but let's spell it out.
>
> Originally, this was read in git_config(git_default_config) into a
> global, and that is probably because almost everybody that touches
> the working tree files needs to know about it.  So populating it in
> prepare_repo_settings() for everybody, even though the calling code
> path does not even need it, would be OK---they were paying the cost
> to read it when they read the default configuration variables.
>
> It seems Patrick earlier made a confused comment on the two models
> that may need a bit clarifying.
>
> Here are the rules to follow.
>
>  - "lazy loading" is not wrong. Initialize the member to an
>    "uninitialized" state, never touch the member in
>    prepare_repo_settings(), and have its getter check for the
>    "uninitialized" state to lazily load it, or have its setter do
>    its thing.  prepare_repo_settings() should not even be aware of
>    the member, if we are going to give the member a getter/setter
>    pair.
>
>  - "Without getter/setter" is not wrong, either.  Load the member in
>    prepare_repo_settings(), which will turn into a no-op once it is
>    called to a repo instance.  Use the member directly afterwards.
>
> You cannot mix and match.  If the variable is rarely used, you'd
> want to catch the initial access and lazily load it, hence you are
> required to have a getter/setter pair and lazily populate the member
> in your getter.  If your variable is very commonly used, load it
> once in prepare_repo_settings(), and because you are not going to do
> anything special upon the first access, there is no need to have a
> getter/setter pair.
>
>

Oh, now I understand it. I will keep this in mind when working on
these settings.
I will send a new version of this patch soon.

Thanks a lot, this helped clarify things.

[GSOC PATCH v4 0/3] environment: remove sparse-checkout related global variables

[Ayush Chandekar]

This patch series aims to remove global variables related to sparse-checkout from the global scope and to remove the definition '#define USE_THE_REPOSITORY_VARIABLE' from a few files.

It contains three patches:

1 - Remove the global variable 'core_apply_sparse_checkout' and move its setting to the 'struct repo_settings'. Also remove the definition '#define USE_THE_REPOSITORY_VARIABLE' from "builtin/backfill.c".

2 - Remove the global variable 'core_sparse_checkout_cone' and move its setting to the 'struct repo_settings'.

3 - Remove the global variable 'sparse_expect_files_outside_of_patterns` and localize it in the function which calls it. Also remove the definition '#define USE_THE_REPOSITORY_VARIABLE' from "sparse-index.c"

As Junio suggested while reviewing v3 patch, I removed the getter and setter functions for the 'core.sparsecheckout' settings. After making this change, I realized the other sparse-checkout-related global variables could be cleaned up as well, leading to the addition of two new patches in this series.

Ayush Chandekar (3):
  environment: move access to "core.sparsecheckout" into
    repo_settings
  environment: move access to "core.sparsecheckoutcone" into
    repo_settings
  environment: remove the global variable
    'sparse_expect_files_outside_of_patterns'

 builtin/backfill.c        |  7 ++----
 builtin/clone.c           |  2 +-
 builtin/grep.c            |  4 ++--
 builtin/mv.c              |  4 ++--
 builtin/sparse-checkout.c | 48 +++++++++++++++++++--------------------
 builtin/worktree.c        |  2 +-
 config.c                  | 24 --------------------
 dir.c                     |  4 ++--
 environment.c             |  3 ---
 environment.h             |  4 ----
 repo-settings.c           |  2 ++
 repo-settings.h           |  3 +++
 sparse-index.c            |  8 ++++---
 unpack-trees.c            |  2 +-
 wt-status.c               |  2 +-
 15 files changed, 46 insertions(+), 73 deletions(-)

-- 

Summary of the range-diff:
* Removed the getter and setter functions for the core.sparseCheckout setting. Instead, it is now initialized once in 'prepare_repo_settings()' and accessed directly, since it is commonly used.
* Added two new commits to eliminate the global variables 'core_sparse_checkout_cone' and 'sparse_expect_files_outside_of_patterns'.

Range-diff vs v3:
 1:  c49dcde03b ! 30:  e221c68ab5 environment: move access to "core.sparsecheckout" into repo_settings
    @@ Commit message
         The setting "core.sparsecheckout" is stored in the global
         `core_apply_sparse_checkout` and is populated in config.c. Refactor the
         code to store it in the variable `sparse_checkout` in the struct
    -    `repo_settings`. Also, create functions to set and get the value of the
    -    setting and update all the occurrences.
    +    `repo_settings`.
    +    It's fine not to lazily load it from the config, as the variable
    +    is used quite commonly.
     
         This also allows us to remove the definition `#define
         USE_THE_REPOSITORY_VARIABLE` from the file 'builtin/backfill.c'.
    @@ builtin/backfill.c
      #include "git-compat-util.h"
      #include "config.h"
     @@ builtin/backfill.c: int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit
    - 	repo_config(repo, git_default_config, NULL);
    + 			     0);
      
    + 	repo_config(repo, git_default_config, NULL);
    +-
    ++	prepare_repo_settings(repo);
      	if (ctx.sparse < 0)
     -		ctx.sparse = core_apply_sparse_checkout;
    -+		ctx.sparse = repo_settings_get_sparse_checkout(repo);
    ++		ctx.sparse = repo->settings.sparse_checkout;
      
      	result = do_backfill(&ctx);
      	backfill_context_clear(&ctx);
    @@ builtin/clone.c: static int git_sparse_checkout_init(const char *repo)
      	 * for the later checkout to use the sparse-checkout file.
      	 */
     -	core_apply_sparse_checkout = 1;
    -+	repo_settings_set_sparse_checkout(the_repository, 1);
    ++	the_repository->settings.sparse_checkout = 1;
      
      	cmd.git_cmd = 1;
      	if (run_command(&cmd)) {
    @@ builtin/mv.c: int cmd_mv(int argc,
      
      		if (ignore_sparse &&
     -		    core_apply_sparse_checkout &&
    -+		    repo_settings_get_sparse_checkout(the_repository) &&
    ++		    the_repository->settings.sparse_checkout &&
      		    core_sparse_checkout_cone) {
      			/*
      			 * NEEDSWORK: we are *not* paying attention to
    @@ builtin/sparse-checkout.c: static int sparse_checkout_list(int argc, const char
      
      	setup_work_tree();
     -	if (!core_apply_sparse_checkout)
    -+	if (!repo_settings_get_sparse_checkout(the_repository))
    ++	if (!the_repository->settings.sparse_checkout)
      		die(_("this worktree is not sparse"));
      
      	argc = parse_options(argc, argv, prefix,
    @@ builtin/sparse-checkout.c: static int set_config(enum sparse_checkout_mode mode)
      static enum sparse_checkout_mode update_cone_mode(int *cone_mode) {
      	/* If not specified, use previous definition of cone mode */
     -	if (*cone_mode == -1 && core_apply_sparse_checkout)
    -+	if (*cone_mode == -1 && repo_settings_get_sparse_checkout(the_repository))
    ++	if (*cone_mode == -1 && the_repository->settings.sparse_checkout)
      		*cone_mode = core_sparse_checkout_cone;
      
      	/* Set cone/non-cone mode appropriately */
     -	core_apply_sparse_checkout = 1;
    -+	repo_settings_set_sparse_checkout(the_repository, 1);
    ++	the_repository->settings.sparse_checkout = 1;
      	if (*cone_mode == 1 || *cone_mode == -1) {
      		core_sparse_checkout_cone = 1;
      		return MODE_CONE_PATTERNS;
    @@ builtin/sparse-checkout.c: static int update_modes(int *cone_mode, int *sparse_i
      
      	/* Determine if we need to record the mode; ensure sparse checkout on */
     -	record_mode = (*cone_mode != -1) || !core_apply_sparse_checkout;
    -+	record_mode = (*cone_mode != -1) || !repo_settings_get_sparse_checkout(the_repository);
    ++	record_mode = (*cone_mode != -1) || !the_repository->settings.sparse_checkout;
      
      	mode = update_cone_mode(cone_mode);
      	if (record_mode && set_config(mode))
    @@ builtin/sparse-checkout.c: static int modify_pattern_list(struct strvec *args, i
      	}
      
     -	if (!core_apply_sparse_checkout) {
    -+	if (!repo_settings_get_sparse_checkout(the_repository)) {
    ++	if (!the_repository->settings.sparse_checkout) {
      		set_config(MODE_ALL_PATTERNS);
     -		core_apply_sparse_checkout = 1;
    -+		repo_settings_set_sparse_checkout(the_repository, 1);
    ++		the_repository->settings.sparse_checkout = 1;
      		changed_config = 1;
      	}
      
    @@ builtin/sparse-checkout.c: static int sparse_checkout_add(int argc, const char *
      
      	setup_work_tree();
     -	if (!core_apply_sparse_checkout)
    -+	if (!repo_settings_get_sparse_checkout(the_repository))
    ++	if (!the_repository->settings.sparse_checkout)
      		die(_("no sparse-checkout to add to"));
      
      	repo_read_index(the_repository);
    @@ builtin/sparse-checkout.c: static int sparse_checkout_reapply(int argc, const ch
      
      	setup_work_tree();
     -	if (!core_apply_sparse_checkout)
    -+	if (!repo_settings_get_sparse_checkout(the_repository))
    ++	if (!the_repository->settings.sparse_checkout)
      		die(_("must be in a sparse-checkout to reapply sparsity patterns"));
      
      	reapply_opts.cone_mode = -1;
    @@ builtin/sparse-checkout.c: static int sparse_checkout_disable(int argc, const ch
      	hashmap_init(&pl.parent_hashmap, pl_hashmap_cmp, NULL, 0);
      	pl.use_cone_patterns = 0;
     -	core_apply_sparse_checkout = 1;
    -+	repo_settings_set_sparse_checkout(the_repository, 1);
    ++	the_repository->settings.sparse_checkout = 1;
      
      	add_pattern("/*", empty_base, 0, &pl, 0);
      
    @@ builtin/worktree.c: static int add_worktree(const char *path, const char *refnam
      	 * the sparse-checkout patterns from the current worktree.
      	 */
     -	if (core_apply_sparse_checkout)
    -+	if (repo_settings_get_sparse_checkout(the_repository))
    ++	if (the_repository->settings.sparse_checkout)
      		copy_sparse_checkout(sb_repo.buf);
      
      	/*
    @@ dir.c: enum pattern_match_result path_matches_pattern_list(
      int init_sparse_checkout_patterns(struct index_state *istate)
      {
     -	if (!core_apply_sparse_checkout)
    -+	if (!repo_settings_get_sparse_checkout(the_repository))
    ++	if (!istate->repo->settings.sparse_checkout)
      		return 1;
      	if (istate->sparse_checkout_patterns)
      		return 0;
    @@ repo-settings.c: void prepare_repo_settings(struct repository *r)
      
      	/*
      	 * The GIT_TEST_MULTI_PACK_INDEX variable is special in that
    -@@ repo-settings.c: void repo_settings_reset_shared_repository(struct repository *repo)
    - {
    - 	repo->settings.shared_repository_initialized = 0;
    - }
    -+
    -+int repo_settings_get_sparse_checkout(struct repository *repo)
    -+{
    -+	prepare_repo_settings(repo);
    -+	return repo->settings.sparse_checkout;
    -+}
    -+
    -+void repo_settings_set_sparse_checkout(struct repository *repo, int value)
    -+{
    -+	prepare_repo_settings(repo);
    -+	repo->settings.sparse_checkout = value;
    -+}
     
      ## repo-settings.h ##
     @@ repo-settings.h: struct repo_settings {
    @@ repo-settings.h: struct repo_settings {
      };
      #define REPO_SETTINGS_INIT { \
      	.shared_repository = -1, \
    -@@ repo-settings.h: int repo_settings_get_shared_repository(struct repository *repo);
    - void repo_settings_set_shared_repository(struct repository *repo, int value);
    - void repo_settings_reset_shared_repository(struct repository *repo);
    - 
    -+/* Read or set the value for "core.sparseCheckout". */
    -+int repo_settings_get_sparse_checkout(struct repository *repo);
    -+void repo_settings_set_sparse_checkout(struct repository *repo, int value);
    -+
    - #endif /* REPO_SETTINGS_H */
     
      ## sparse-index.c ##
     @@ sparse-index.c: static int index_has_unmerged_entries(struct index_state *istate)
    @@ sparse-index.c: 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)
    -+	if (!repo_settings_get_sparse_checkout(istate->repo) || !core_sparse_checkout_cone)
    ++	if (!istate->repo->settings.sparse_checkout || !core_sparse_checkout_cone)
      		return 0;
      
      	if (!(flags & SPARSE_INDEX_MEMORY_ONLY)) {
    @@ sparse-index.c: static void clear_skip_worktree_from_present_files_full(struct i
      void clear_skip_worktree_from_present_files(struct index_state *istate)
      {
     -	if (!core_apply_sparse_checkout ||
    -+	if (!repo_settings_get_sparse_checkout(istate->repo) ||
    ++	if (!istate->repo->settings.sparse_checkout ||
      	    sparse_expect_files_outside_of_patterns)
      		return;
      
    @@ unpack-trees.c: int unpack_trees(unsigned len, struct tree_desc *t, struct unpac
      		update_sparsity_for_prefix(o->prefix, o->src_index);
      
     -	if (!core_apply_sparse_checkout || !o->update)
    -+	if (!repo_settings_get_sparse_checkout(repo) || !o->update)
    ++	if (!repo->settings.sparse_checkout || !o->update)
      		o->skip_sparse_checkout = 1;
      	if (!o->skip_sparse_checkout) {
      		memset(&pl, 0, sizeof(pl));
    @@ wt-status.c: static void wt_status_check_sparse_checkout(struct repository *r,
      	int i;
      
     -	if (!core_apply_sparse_checkout || r->index->cache_nr == 0) {
    -+	if (!repo_settings_get_sparse_checkout(r) || r->index->cache_nr == 0) {
    ++	if (!r->settings.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.
 -:  ---------- > 31:  9a63884341 environment: move access to "core.sparsecheckoutcone" into repo_settings
 -:  ---------- > 32:  a9e1e23685 environment: remove the global variable 'sparse_expect_files_outside_of_patterns'


2.49.0

[GSOC PATCH v4 1/3] environment: move access to "core.sparsecheckout" into repo_settings

[Ayush Chandekar]

The setting "core.sparsecheckout" is stored in the global
`core_apply_sparse_checkout` and is populated in config.c. Refactor the
code to store it in the variable `sparse_checkout` in the struct
`repo_settings`.
It's fine not to lazily load it from the config, as the variable
is used quite commonly.

This also allows us to remove the definition `#define
USE_THE_REPOSITORY_VARIABLE` from the file 'builtin/backfill.c'.

This change is part of an ongoing effort to eliminate global variables,
improve modularity and help libify the codebase.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 builtin/backfill.c        |  7 ++-----
 builtin/clone.c           |  2 +-
 builtin/grep.c            |  2 +-
 builtin/mv.c              |  2 +-
 builtin/sparse-checkout.c | 20 ++++++++++----------
 builtin/worktree.c        |  2 +-
 config.c                  |  5 -----
 dir.c                     |  2 +-
 environment.c             |  1 -
 environment.h             |  1 -
 repo-settings.c           |  1 +
 repo-settings.h           |  1 +
 sparse-index.c            |  4 ++--
 unpack-trees.c            |  2 +-
 wt-status.c               |  2 +-
 15 files changed, 23 insertions(+), 31 deletions(-)

diff --git a/builtin/backfill.c b/builtin/backfill.c
index fa82ad2f6f..bf9e56bff3 100644
--- a/builtin/backfill.c
+++ b/builtin/backfill.c
@@ -1,6 +1,3 @@
-/* We need this macro to access core_apply_sparse_checkout */
-#define USE_THE_REPOSITORY_VARIABLE
-
 #include "builtin.h"
 #include "git-compat-util.h"
 #include "config.h"
@@ -137,9 +134,9 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit
 			     0);
 
 	repo_config(repo, git_default_config, NULL);
-
+	prepare_repo_settings(repo);
 	if (ctx.sparse < 0)
-		ctx.sparse = core_apply_sparse_checkout;
+		ctx.sparse = repo->settings.sparse_checkout;
 
 	result = do_backfill(&ctx);
 	backfill_context_clear(&ctx);
diff --git a/builtin/clone.c b/builtin/clone.c
index 91b9cd0d16..1bc9c1bada 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -621,7 +621,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->settings.sparse_checkout = 1;
 
 	cmd.git_cmd = 1;
 	if (run_command(&cmd)) {
diff --git a/builtin/grep.c b/builtin/grep.c
index 3ce574a605..63342f5e36 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -481,7 +481,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"
 	 *	When running `grep` in the superproject, this setting is
 	 *	populated using the superproject's configs. However, once
 	 *	initialized, this config is globally accessible and is read by
diff --git a/builtin/mv.c b/builtin/mv.c
index 07548fe96a..1e9f4d3eba 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -572,7 +572,7 @@ int cmd_mv(int argc,
 		rename_index_entry_at(the_repository->index, pos, dst);
 
 		if (ignore_sparse &&
-		    core_apply_sparse_checkout &&
+		    the_repository->settings.sparse_checkout &&
 		    core_sparse_checkout_cone) {
 			/*
 			 * NEEDSWORK: we are *not* paying attention to
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 1bf01591b2..869d574a03 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -62,7 +62,7 @@ static int sparse_checkout_list(int argc, const char **argv, const char *prefix,
 	int res;
 
 	setup_work_tree();
-	if (!core_apply_sparse_checkout)
+	if (!the_repository->settings.sparse_checkout)
 		die(_("this worktree is not sparse"));
 
 	argc = parse_options(argc, argv, prefix,
@@ -397,11 +397,11 @@ static int set_config(enum sparse_checkout_mode mode)
 
 static enum sparse_checkout_mode update_cone_mode(int *cone_mode) {
 	/* If not specified, use previous definition of cone mode */
-	if (*cone_mode == -1 && core_apply_sparse_checkout)
+	if (*cone_mode == -1 && the_repository->settings.sparse_checkout)
 		*cone_mode = core_sparse_checkout_cone;
 
 	/* Set cone/non-cone mode appropriately */
-	core_apply_sparse_checkout = 1;
+	the_repository->settings.sparse_checkout = 1;
 	if (*cone_mode == 1 || *cone_mode == -1) {
 		core_sparse_checkout_cone = 1;
 		return MODE_CONE_PATTERNS;
@@ -415,7 +415,7 @@ static int update_modes(int *cone_mode, int *sparse_index)
 	int mode, record_mode;
 
 	/* Determine if we need to record the mode; ensure sparse checkout on */
-	record_mode = (*cone_mode != -1) || !core_apply_sparse_checkout;
+	record_mode = (*cone_mode != -1) || !the_repository->settings.sparse_checkout;
 
 	mode = update_cone_mode(cone_mode);
 	if (record_mode && set_config(mode))
@@ -695,9 +695,9 @@ static int modify_pattern_list(struct strvec *args, int use_stdin,
 		break;
 	}
 
-	if (!core_apply_sparse_checkout) {
+	if (!the_repository->settings.sparse_checkout) {
 		set_config(MODE_ALL_PATTERNS);
-		core_apply_sparse_checkout = 1;
+		the_repository->settings.sparse_checkout = 1;
 		changed_config = 1;
 	}
 
@@ -793,7 +793,7 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix,
 	int ret;
 
 	setup_work_tree();
-	if (!core_apply_sparse_checkout)
+	if (!the_repository->settings.sparse_checkout)
 		die(_("no sparse-checkout to add to"));
 
 	repo_read_index(the_repository);
@@ -902,7 +902,7 @@ static int sparse_checkout_reapply(int argc, const char **argv,
 	};
 
 	setup_work_tree();
-	if (!core_apply_sparse_checkout)
+	if (!the_repository->settings.sparse_checkout)
 		die(_("must be in a sparse-checkout to reapply sparsity patterns"));
 
 	reapply_opts.cone_mode = -1;
@@ -935,7 +935,7 @@ static int sparse_checkout_disable(int argc, const char **argv,
 	struct pattern_list pl;
 
 	/*
-	 * We do not exit early if !core_apply_sparse_checkout; due to the
+	 * We do not exit early if !sparse_checkout; due to the
 	 * ability for users to manually muck things up between
 	 *   direct editing of .git/info/sparse-checkout
 	 *   running read-tree -m u HEAD or update-index --skip-worktree
@@ -961,7 +961,7 @@ static int sparse_checkout_disable(int argc, const char **argv,
 	hashmap_init(&pl.recursive_hashmap, pl_hashmap_cmp, NULL, 0);
 	hashmap_init(&pl.parent_hashmap, pl_hashmap_cmp, NULL, 0);
 	pl.use_cone_patterns = 0;
-	core_apply_sparse_checkout = 1;
+	the_repository->settings.sparse_checkout = 1;
 
 	add_pattern("/*", empty_base, 0, &pl, 0);
 
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 88a36ea9f8..92e1c92afc 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -536,7 +536,7 @@ static int add_worktree(const char *path, const char *refname,
 	 * If the current worktree has sparse-checkout enabled, then copy
 	 * the sparse-checkout patterns from the current worktree.
 	 */
-	if (core_apply_sparse_checkout)
+	if (the_repository->settings.sparse_checkout)
 		copy_sparse_checkout(sb_repo.buf);
 
 	/*
diff --git a/config.c b/config.c
index b18b5617fc..8fd4dd8c81 100644
--- a/config.c
+++ b/config.c
@@ -1612,11 +1612,6 @@ static int git_default_core_config(const char *var, const char *value,
 		return 0;
 	}
 
-	if (!strcmp(var, "core.sparsecheckout")) {
-		core_apply_sparse_checkout = git_config_bool(var, value);
-		return 0;
-	}
-
 	if (!strcmp(var, "core.sparsecheckoutcone")) {
 		core_sparse_checkout_cone = git_config_bool(var, value);
 		return 0;
diff --git a/dir.c b/dir.c
index a374972b62..8f0f7ca8a4 100644
--- a/dir.c
+++ b/dir.c
@@ -1503,7 +1503,7 @@ enum pattern_match_result path_matches_pattern_list(
 
 int init_sparse_checkout_patterns(struct index_state *istate)
 {
-	if (!core_apply_sparse_checkout)
+	if (!istate->repo->settings.sparse_checkout)
 		return 1;
 	if (istate->sparse_checkout_patterns)
 		return 0;
diff --git a/environment.c b/environment.c
index c61d773e7e..a379a9149e 100644
--- a/environment.c
+++ b/environment.c
@@ -64,7 +64,6 @@ enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;
 #endif
 enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
 int grafts_keep_true_parents;
-int core_apply_sparse_checkout;
 int core_sparse_checkout_cone;
 int sparse_expect_files_outside_of_patterns;
 int merge_log_config = -1;
diff --git a/environment.h b/environment.h
index 3d98461a06..6a30512f3c 100644
--- a/environment.h
+++ b/environment.h
@@ -160,7 +160,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/repo-settings.c b/repo-settings.c
index 4129f8fb2b..9270cca561 100644
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -81,6 +81,7 @@ void prepare_repo_settings(struct repository *r)
 		      &r->settings.pack_use_bitmap_boundary_traversal,
 		      r->settings.pack_use_bitmap_boundary_traversal);
 	repo_cfg_bool(r, "core.usereplacerefs", &r->settings.read_replace_refs, 1);
+	repo_cfg_bool(r, "core.sparsecheckout", &r->settings.sparse_checkout, 0);
 
 	/*
 	 * The GIT_TEST_MULTI_PACK_INDEX variable is special in that
diff --git a/repo-settings.h b/repo-settings.h
index 2bf24b2597..9caa7c57a3 100644
--- a/repo-settings.h
+++ b/repo-settings.h
@@ -67,6 +67,7 @@ struct repo_settings {
 	unsigned long big_file_threshold;
 
 	char *hooks_path;
+	int sparse_checkout;
 };
 #define REPO_SETTINGS_INIT { \
 	.shared_repository = -1, \
diff --git a/sparse-index.c b/sparse-index.c
index 5634abafaa..2428b20634 100644
--- a/sparse-index.c
+++ b/sparse-index.c
@@ -150,7 +150,7 @@ 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)
+	if (!istate->repo->settings.sparse_checkout || !core_sparse_checkout_cone)
 		return 0;
 
 	if (!(flags & SPARSE_INDEX_MEMORY_ONLY)) {
@@ -668,7 +668,7 @@ 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 ||
+	if (!istate->repo->settings.sparse_checkout ||
 	    sparse_expect_files_outside_of_patterns)
 		return;
 
diff --git a/unpack-trees.c b/unpack-trees.c
index 471837f032..02e32c4ba1 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->settings.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 454601afa1..ec2be98194 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1773,7 +1773,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->settings.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.
-- 
2.49.0

[GSOC PATCH v4 2/3] environment: move access to "core.sparsecheckoutcone" into repo_settings

[Ayush Chandekar]

The setting "core.sparsecheckoutcone" is stored in the global
`core_sparse_checkout_cone` and is populated in config.c. Refactor the
code to store it in the variable `sparse_checkout_cone` in the struct
`repo_settings`.
It's fine not to lazily load it from the config, as the variable
is used quite commonly.

This change is part of an ongoing effort to eliminate global variables,
improve modularity and help libify the codebase.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 builtin/grep.c            |  2 +-
 builtin/mv.c              |  2 +-
 builtin/sparse-checkout.c | 28 ++++++++++++++--------------
 config.c                  |  5 -----
 dir.c                     |  2 +-
 environment.c             |  1 -
 environment.h             |  1 -
 repo-settings.c           |  1 +
 repo-settings.h           |  2 ++
 sparse-index.c            |  2 +-
 10 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 63342f5e36..94d6245b85 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -491,7 +491,7 @@ static int grep_submodule(struct grep_opt *opt,
 	 *	dictate the behavior for the submodule, making it "forget" its
 	 *	sparse-checkout state.
 	 *
-	 * 3. "core_sparse_checkout_cone"
+	 * 3. "sparse_checkout_cone"
 	 *	ditto.
 	 *
 	 * Note that this list is not exhaustive.
diff --git a/builtin/mv.c b/builtin/mv.c
index 1e9f4d3eba..833fa761dd 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -573,7 +573,7 @@ int cmd_mv(int argc,
 
 		if (ignore_sparse &&
 		    the_repository->settings.sparse_checkout &&
-		    core_sparse_checkout_cone) {
+		    the_repository->settings.sparse_checkout_cone) {
 			/*
 			 * NEEDSWORK: we are *not* paying attention to
 			 * "out-to-out" move (<source> is out-of-cone and
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 869d574a03..e65a62f250 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -71,7 +71,7 @@ static int sparse_checkout_list(int argc, const char **argv, const char *prefix,
 
 	memset(&pl, 0, sizeof(pl));
 
-	pl.use_cone_patterns = core_sparse_checkout_cone;
+	pl.use_cone_patterns = the_repository->settings.sparse_checkout_cone;
 
 	sparse_filename = get_sparse_checkout_filename();
 	res = add_patterns_from_file_to_list(sparse_filename, "", 0, &pl, NULL, 0);
@@ -352,7 +352,7 @@ static int write_patterns_and_update(struct pattern_list *pl)
 	if (!fp)
 		die_errno(_("unable to fdopen %s"), get_lock_file_path(&lk));
 
-	if (core_sparse_checkout_cone)
+	if (the_repository->settings.sparse_checkout_cone)
 		write_cone_to_file(fp, pl);
 	else
 		write_patterns_to_file(fp, pl);
@@ -398,15 +398,15 @@ static int set_config(enum sparse_checkout_mode mode)
 static enum sparse_checkout_mode update_cone_mode(int *cone_mode) {
 	/* If not specified, use previous definition of cone mode */
 	if (*cone_mode == -1 && the_repository->settings.sparse_checkout)
-		*cone_mode = core_sparse_checkout_cone;
+		*cone_mode = the_repository->settings.sparse_checkout_cone;
 
 	/* Set cone/non-cone mode appropriately */
 	the_repository->settings.sparse_checkout = 1;
 	if (*cone_mode == 1 || *cone_mode == -1) {
-		core_sparse_checkout_cone = 1;
+		the_repository->settings.sparse_checkout_cone = 1;
 		return MODE_CONE_PATTERNS;
 	}
-	core_sparse_checkout_cone = 0;
+	the_repository->settings.sparse_checkout_cone = 0;
 	return MODE_ALL_PATTERNS;
 }
 
@@ -572,7 +572,7 @@ static void add_patterns_from_input(struct pattern_list *pl,
 				    FILE *file)
 {
 	int i;
-	if (core_sparse_checkout_cone) {
+	if (the_repository->settings.sparse_checkout_cone) {
 		struct strbuf line = STRBUF_INIT;
 
 		hashmap_init(&pl->recursive_hashmap, pl_hashmap_cmp, NULL, 0);
@@ -637,7 +637,7 @@ static void add_patterns_cone_mode(int argc, const char **argv,
 				use_stdin ? stdin : NULL);
 
 	memset(&existing, 0, sizeof(existing));
-	existing.use_cone_patterns = core_sparse_checkout_cone;
+	existing.use_cone_patterns = the_repository->settings.sparse_checkout_cone;
 
 	if (add_patterns_from_file_to_list(sparse_filename, "", 0,
 					   &existing, NULL, 0))
@@ -683,7 +683,7 @@ static int modify_pattern_list(struct strvec *args, int use_stdin,
 
 	switch (m) {
 	case ADD:
-		if (core_sparse_checkout_cone)
+		if (the_repository->settings.sparse_checkout_cone)
 			add_patterns_cone_mode(args->nr, args->v, pl, use_stdin);
 		else
 			add_patterns_literal(args->nr, args->v, pl, use_stdin);
@@ -719,7 +719,7 @@ static void sanitize_paths(struct strvec *args,
 	if (!args->nr)
 		return;
 
-	if (prefix && *prefix && core_sparse_checkout_cone) {
+	if (prefix && *prefix && the_repository->settings.sparse_checkout_cone) {
 		/*
 		 * The args are not pathspecs, so unfortunately we
 		 * cannot imitate how cmd_add() uses parse_pathspec().
@@ -736,10 +736,10 @@ static void sanitize_paths(struct strvec *args,
 	if (skip_checks)
 		return;
 
-	if (prefix && *prefix && !core_sparse_checkout_cone)
+	if (prefix && *prefix && !the_repository->settings.sparse_checkout_cone)
 		die(_("please run from the toplevel directory in non-cone mode"));
 
-	if (core_sparse_checkout_cone) {
+	if (the_repository->settings.sparse_checkout_cone) {
 		for (i = 0; i < args->nr; i++) {
 			if (args->v[i][0] == '/')
 				die(_("specify directories rather than patterns (no leading slash)"));
@@ -761,7 +761,7 @@ static void sanitize_paths(struct strvec *args,
 		if (S_ISSPARSEDIR(ce->ce_mode))
 			continue;
 
-		if (core_sparse_checkout_cone)
+		if (the_repository->settings.sparse_checkout_cone)
 			die(_("'%s' is not a directory; to treat it as a directory anyway, rerun with --skip-checks"), args->v[i]);
 		else
 			warning(_("pass a leading slash before paths such as '%s' if you want a single file (see NON-CONE PROBLEMS in the git-sparse-checkout manual)."), args->v[i]);
@@ -864,7 +864,7 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix,
 	 * non-cone mode, if nothing is specified, manually select just the
 	 * top-level directory (much as 'init' would do).
 	 */
-	if (!core_sparse_checkout_cone && !set_opts.use_stdin && argc == 0) {
+	if (!the_repository->settings.sparse_checkout_cone && !set_opts.use_stdin && argc == 0) {
 		for (int i = 0; i < default_patterns_nr; i++)
 			strvec_push(&patterns, default_patterns[i]);
 	} else {
@@ -1042,7 +1042,7 @@ static int sparse_checkout_check_rules(int argc, const char **argv, const char *
 		check_rules_opts.cone_mode = 1;
 
 	update_cone_mode(&check_rules_opts.cone_mode);
-	pl.use_cone_patterns = core_sparse_checkout_cone;
+	pl.use_cone_patterns = the_repository->settings.sparse_checkout_cone;
 	if (check_rules_opts.rules_file) {
 		fp = xfopen(check_rules_opts.rules_file, "r");
 		add_patterns_from_input(&pl, argc, argv, fp);
diff --git a/config.c b/config.c
index 8fd4dd8c81..707fe0707a 100644
--- a/config.c
+++ b/config.c
@@ -1612,11 +1612,6 @@ static int git_default_core_config(const char *var, const char *value,
 		return 0;
 	}
 
-	if (!strcmp(var, "core.sparsecheckoutcone")) {
-		core_sparse_checkout_cone = git_config_bool(var, value);
-		return 0;
-	}
-
 	if (!strcmp(var, "core.precomposeunicode")) {
 		precomposed_unicode = git_config_bool(var, value);
 		return 0;
diff --git a/dir.c b/dir.c
index 8f0f7ca8a4..8378996b72 100644
--- a/dir.c
+++ b/dir.c
@@ -3459,7 +3459,7 @@ int get_sparse_checkout_patterns(struct pattern_list *pl)
 	int res;
 	char *sparse_filename = get_sparse_checkout_filename();
 
-	pl->use_cone_patterns = core_sparse_checkout_cone;
+	pl->use_cone_patterns = the_repository->settings.sparse_checkout_cone;
 	res = add_patterns_from_file_to_list(sparse_filename, "", 0, pl, NULL, 0);
 
 	free(sparse_filename);
diff --git a/environment.c b/environment.c
index a379a9149e..7d46b80711 100644
--- a/environment.c
+++ b/environment.c
@@ -64,7 +64,6 @@ enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;
 #endif
 enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
 int grafts_keep_true_parents;
-int core_sparse_checkout_cone;
 int sparse_expect_files_outside_of_patterns;
 int merge_log_config = -1;
 int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
diff --git a/environment.h b/environment.h
index 6a30512f3c..00a5b332a0 100644
--- a/environment.h
+++ b/environment.h
@@ -160,7 +160,6 @@ extern int precomposed_unicode;
 extern int protect_hfs;
 extern int protect_ntfs;
 
-extern int core_sparse_checkout_cone;
 extern int sparse_expect_files_outside_of_patterns;
 
 enum rebase_setup_type {
diff --git a/repo-settings.c b/repo-settings.c
index 9270cca561..eebc1f941d 100644
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -82,6 +82,7 @@ void prepare_repo_settings(struct repository *r)
 		      r->settings.pack_use_bitmap_boundary_traversal);
 	repo_cfg_bool(r, "core.usereplacerefs", &r->settings.read_replace_refs, 1);
 	repo_cfg_bool(r, "core.sparsecheckout", &r->settings.sparse_checkout, 0);
+	repo_cfg_bool(r, "core.sparsecheckoutcone", &r->settings.sparse_checkout_cone, 0);
 
 	/*
 	 * The GIT_TEST_MULTI_PACK_INDEX variable is special in that
diff --git a/repo-settings.h b/repo-settings.h
index 9caa7c57a3..443e1399da 100644
--- a/repo-settings.h
+++ b/repo-settings.h
@@ -67,7 +67,9 @@ struct repo_settings {
 	unsigned long big_file_threshold;
 
 	char *hooks_path;
+
 	int sparse_checkout;
+	int sparse_checkout_cone;
 };
 #define REPO_SETTINGS_INIT { \
 	.shared_repository = -1, \
diff --git a/sparse-index.c b/sparse-index.c
index 2428b20634..444da8a753 100644
--- a/sparse-index.c
+++ b/sparse-index.c
@@ -150,7 +150,7 @@ static int index_has_unmerged_entries(struct index_state *istate)
 
 int is_sparse_index_allowed(struct index_state *istate, int flags)
 {
-	if (!istate->repo->settings.sparse_checkout || !core_sparse_checkout_cone)
+	if (!istate->repo->settings.sparse_checkout || !istate->repo->settings.sparse_checkout_cone)
 		return 0;
 
 	if (!(flags & SPARSE_INDEX_MEMORY_ONLY)) {
-- 
2.49.0

[GSOC PATCH v4 3/3] environment: remove the global variable 'sparse_expect_files_outside_of_patterns'

[Ayush Chandekar]

The global variable 'sparse_expect_files_outside_of_patterns' is used in
a single function named 'clear_skip_worktree_from_present_files()' in
sparse-index.c. Move its declaration inside that function, removing
unnecessary global state.

This also allows us to remove the definition '#define
USE_THE_REPOSITORY_VARIABLE' from the file 'sparse-index.c'.

This change is part of an ongoing effort to eliminate global variables,
improve modularity and help libify the codebase.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 config.c       | 14 --------------
 environment.c  |  1 -
 environment.h  |  2 --
 sparse-index.c |  4 +++-
 4 files changed, 3 insertions(+), 18 deletions(-)

diff --git a/config.c b/config.c
index 707fe0707a..d212329799 100644
--- a/config.c
+++ b/config.c
@@ -1636,17 +1636,6 @@ static int git_default_core_config(const char *var, const char *value,
 	return platform_core_config(var, value, ctx, cb);
 }
 
-static int git_default_sparse_config(const char *var, const char *value)
-{
-	if (!strcmp(var, "sparse.expectfilesoutsideofpatterns")) {
-		sparse_expect_files_outside_of_patterns = git_config_bool(var, value);
-		return 0;
-	}
-
-	/* Add other config variables here and to Documentation/config/sparse.adoc. */
-	return 0;
-}
-
 static int git_default_i18n_config(const char *var, const char *value)
 {
 	if (!strcmp(var, "i18n.commitencoding")) {
@@ -1808,9 +1797,6 @@ int git_default_config(const char *var, const char *value,
 		return 0;
 	}
 
-	if (starts_with(var, "sparse."))
-		return git_default_sparse_config(var, value);
-
 	/* Add other config variables here and to Documentation/config.adoc. */
 	return 0;
 }
diff --git a/environment.c b/environment.c
index 7d46b80711..d51e0a14aa 100644
--- a/environment.c
+++ b/environment.c
@@ -64,7 +64,6 @@ enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;
 #endif
 enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
 int grafts_keep_true_parents;
-int sparse_expect_files_outside_of_patterns;
 int merge_log_config = -1;
 int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
 unsigned long pack_size_limit_cfg;
diff --git a/environment.h b/environment.h
index 00a5b332a0..5121a28d3f 100644
--- a/environment.h
+++ b/environment.h
@@ -160,8 +160,6 @@ extern int precomposed_unicode;
 extern int protect_hfs;
 extern int protect_ntfs;
 
-extern int sparse_expect_files_outside_of_patterns;
-
 enum rebase_setup_type {
 	AUTOREBASE_NEVER = 0,
 	AUTOREBASE_LOCAL,
diff --git a/sparse-index.c b/sparse-index.c
index 444da8a753..5d87fc65c0 100644
--- a/sparse-index.c
+++ b/sparse-index.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
 #define DISABLE_SIGN_COMPARE_WARNINGS
 
 #include "git-compat-util.h"
@@ -668,6 +667,9 @@ static void clear_skip_worktree_from_present_files_full(struct index_state *ista
 
 void clear_skip_worktree_from_present_files(struct index_state *istate)
 {
+	int sparse_expect_files_outside_of_patterns = 0;
+	repo_config_get_bool(istate->repo, "sparse.expectfilesoutsideofpatterns", 
+		&sparse_expect_files_outside_of_patterns);
 	if (!istate->repo->settings.sparse_checkout ||
 	    sparse_expect_files_outside_of_patterns)
 		return;
-- 
2.49.0

Re: [GSOC PATCH v4 1/3] environment: move access to "core.sparsecheckout" into repo_settings

[Junio C Hamano]

Ayush Chandekar <ayu.chandekar@gmail.com> writes:

> diff --git a/builtin/backfill.c b/builtin/backfill.c
> index fa82ad2f6f..bf9e56bff3 100644
> --- a/builtin/backfill.c
> +++ b/builtin/backfill.c
> @@ -1,6 +1,3 @@
> -/* We need this macro to access core_apply_sparse_checkout */
> -#define USE_THE_REPOSITORY_VARIABLE
> -
>  #include "builtin.h"
>  #include "git-compat-util.h"
>  #include "config.h"
> @@ -137,9 +134,9 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit
>  			     0);
>  
>  	repo_config(repo, git_default_config, NULL);
> -
> +	prepare_repo_settings(repo);

At this point, because show_usage_with_options_if_asked() has
already been called and returned, we know repo is not NULL, since
the only time git.c:run_builtin() calls us with repo==NULL is when
there is "-h" with nothing else on the command line and that causes
show_usage_with_options_if_asked() to emit usage and exit.

OK.

>  	if (ctx.sparse < 0)
> -		ctx.sparse = core_apply_sparse_checkout;
> +		ctx.sparse = repo->settings.sparse_checkout;

This is safe for the same reason.

> diff --git a/builtin/clone.c b/builtin/clone.c
> index 91b9cd0d16..1bc9c1bada 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -621,7 +621,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->settings.sparse_checkout = 1;

Have anybody called prepare_repo_settings() on the repository yet?
What will prevent another call to the function from overwriting this
value later?

> diff --git a/builtin/mv.c b/builtin/mv.c
> index 07548fe96a..1e9f4d3eba 100644
> --- a/builtin/mv.c
> +++ b/builtin/mv.c
> @@ -572,7 +572,7 @@ int cmd_mv(int argc,
>  		rename_index_entry_at(the_repository->index, pos, dst);
>  
>  		if (ignore_sparse &&
> -		    core_apply_sparse_checkout &&
> +		    the_repository->settings.sparse_checkout &&
>  		    core_sparse_checkout_cone) {

Have anybody called prepare_repo_settings() on the repository yet
before the control comes here?

The guarantee of the original code being correct relied on the fact
that git_default_core_config() was called way before these places so
the global variable has been already initialized correctly.

The .sparse_checkout member is read in prepare_repo_settings() in
your new code; in order to give the correctness guarantee, there
needs to be some way to make sure prepare_repo_settings() has
already been called on the_repository before these places.

The same comment applies to all the code paths that access
the_repository->settings.sparse_checkout member instead of the
global.  As the source of their correctness guarantee is quite
different, a mechanical replacement from global to a struct member
is not sufficient.

Re: [GSOC PATCH v4 2/3] environment: move access to "core.sparsecheckoutcone" into repo_settings

[Junio C Hamano]

Ayush Chandekar <ayu.chandekar@gmail.com> writes:

> The setting "core.sparsecheckoutcone" is stored in the global
> `core_sparse_checkout_cone` and is populated in config.c. Refactor the
> code to store it in the variable `sparse_checkout_cone` in the struct
> `repo_settings`.
> It's fine not to lazily load it from the config, as the variable
> is used quite commonly.
>
> This change is part of an ongoing effort to eliminate global variables,
> improve modularity and help libify the codebase.
>
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
> Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
> ---

I think "the correctness guarantee comes from a different place now.
How are we making sure that these accesses are correct?" comment
applies equally here.

>  builtin/grep.c            |  2 +-
>  builtin/mv.c              |  2 +-

Looking at the output from

    $ git grep -n -e prepare_repo_settings \*.c

there are many builtin/*.c that makes a call to the function on
the_repository fairly early in its start-up sequence.  Unlike many
others these two do not seem to have any.

>  builtin/sparse-checkout.c | 28 ++++++++++++++--------------

This one does, immediately after calling git_config(), so it should
be fairly safe.

>  dir.c                     |  2 +-
>  sparse-index.c            |  2 +-

These two also need correctness guarantee.

You'd need to make sure any potential caller of the helper functions
have called prepare_repo_settings().  Those who wrote an access to
the global variable in the original would already have made sure
that the callers would have already read the configuration file, but
with the new code, that is no longer a guarantee for correctness.

Re: [GSOC PATCH v4 3/3] environment: remove the global variable 'sparse_expect_files_outside_of_patterns'

[Junio C Hamano]

Ayush Chandekar <ayu.chandekar@gmail.com> writes:

> diff --git a/sparse-index.c b/sparse-index.c
> index 444da8a753..5d87fc65c0 100644
> --- a/sparse-index.c
> +++ b/sparse-index.c
> @@ -1,4 +1,3 @@
> -#define USE_THE_REPOSITORY_VARIABLE
>  #define DISABLE_SIGN_COMPARE_WARNINGS
>  
>  #include "git-compat-util.h"
> @@ -668,6 +667,9 @@ static void clear_skip_worktree_from_present_files_full(struct index_state *ista
>  
>  void clear_skip_worktree_from_present_files(struct index_state *istate)
>  {
> +	int sparse_expect_files_outside_of_patterns = 0;
> +	repo_config_get_bool(istate->repo, "sparse.expectfilesoutsideofpatterns", 

There is a trailing whitespace here.

> +		&sparse_expect_files_outside_of_patterns);

[GSOC PATCH v5 0/3] environment: remove sparse-checkout related global variables

[Ayush Chandekar]

This patch series aims to remove global variables related to sparse-checkout from the global scope and to remove the definition '#define USE_THE_REPOSITORY_VARIABLE' from a few files.

It contains three patches:

1 - Remove the global variable 'core_apply_sparse_checkout' and move its setting to the 'struct repo_settings'. Also remove the definition '#define USE_THE_REPOSITORY_VARIABLE' from "builtin/backfill.c". 

2 - Remove the global variable 'core_sparse_checkout_cone' and move its setting to the 'struct repo_settings'.

3 - Remove the global variable 'sparse_expect_files_outside_of_patterns` and localize it in the function which calls it. Also remove the definition '#define USE_THE_REPOSITORY_VARIABLE' from "sparse-index.c"

Thanks a lot to Christian for mentoring, and to Junio, Patrick and Ben for reviewing.

Ayush Chandekar (2):
  environment: move access to "core.sparsecheckoutcone" into
    repo_settings
  environment: remove the global variable
    'sparse_expect_files_outside_of_patterns'
  environment: move access to "core.sparsecheckout" into repo_settings

 builtin/backfill.c        |  7 ++----
 builtin/clone.c           |  3 ++-
 builtin/grep.c            |  4 ++--
 builtin/mv.c              |  6 ++---
 builtin/sparse-checkout.c | 49 +++++++++++++++++++--------------------
 builtin/worktree.c        |  2 +-
 config.c                  | 24 -------------------
 dir.c                     |  5 ++--
 environment.c             |  3 ---
 environment.h             |  4 ----
 repo-settings.c           |  2 ++
 repo-settings.h           |  3 +++
 sparse-index.c            | 10 ++++----
 unpack-trees.c            |  3 ++-
 wt-status.c               |  3 ++-
 15 files changed, 52 insertions(+), 76 deletions(-)

-- 

Summary of range-diff:
* Ensure that `prepare_repo_settings()` is called in all code paths before accessing `settings.sparse_checkout` and `settings.sparse_checkout_cone`.

Range-diff with v4:
1:  e221c68ab5 ! 1:  ba9929d128 environment: move access to "core.sparsecheckout" into repo_settings
    @@ Commit message
         `core_apply_sparse_checkout` and is populated in config.c. Refactor the
         code to store it in the variable `sparse_checkout` in the struct
         `repo_settings`.
    -    It's fine not to lazily load it from the config, as the variable
    -    is used quite commonly.
    +
    +    Call `prepare_repo_settings()` where necessary to ensure the `struct
    +    repo_settings` is initialized before use:
    +    - In "builtin/backfill.c", "builtin/mv.c" and "builtin/clone.c" call
    +      `prepare_repo_settings()` since their respective `cmd_*()` functions
    +      did not call it earlier.
    +    - In "dir.c", the function using 'settings.sparse_checkout' is invoked
    +      in multiple files that do not call `prepare_repo_settings()`, hence
    +      add a call directly to that function.
    +    - In "sparse-checkout.c", add a call to `prepare_repo_settings()` inside
    +      `is_sparse_index_allowed()`, as it is used widely and relies on the
    +      setting.
    +    - In "wt-status.c", call `prepare_repo_settings()` before accessing
    +      the setting because the function using it is commonly used.
    +
    +    Avoid reduntant calls to `prepare_repo_settings()` where it is already
    +    present:
    +    - In "builtin/worktree.c", it is already invoked in `cmd_worktree()`
    +      before the setting is accessed.
    +    - In "unpack-tress.c", the function accessing the setting already calls
    +      it.
     
         This also allows us to remove the definition `#define
         USE_THE_REPOSITORY_VARIABLE` from the file 'builtin/backfill.c'.
    @@ builtin/backfill.c: int cmd_backfill(int argc, const char **argv, const char *pr
     
      ## builtin/clone.c ##
     @@ builtin/clone.c: static int git_sparse_checkout_init(const char *repo)
    + 	int result = 0;
    + 	strvec_pushl(&cmd.args, "-C", repo, "sparse-checkout", "set", NULL);
    + 
    ++	prepare_repo_settings(the_repository);
    + 	/*
      	 * We must apply the setting in the current process
      	 * for the later checkout to use the sparse-checkout file.
      	 */
    @@ builtin/grep.c: static int grep_submodule(struct grep_opt *opt,
      	 *	of these submodules are expanded unexpectedly.
      	 *
     -	 * 2. "core_apply_sparse_checkout"
    -+	 * 2. "sparse_checkout"
    ++	 * 2. "settings.sparse_checkout"
      	 *	When running `grep` in the superproject, this setting is
      	 *	populated using the superproject's configs. However, once
      	 *	initialized, this config is globally accessible and is read by
     
      ## builtin/mv.c ##
     @@ builtin/mv.c: int cmd_mv(int argc,
    + 						       &st,
    + 						       0);
      		rename_index_entry_at(the_repository->index, pos, dst);
    - 
    +-
    ++		prepare_repo_settings(the_repository);
      		if (ignore_sparse &&
     -		    core_apply_sparse_checkout &&
     +		    the_repository->settings.sparse_checkout &&
    @@ builtin/sparse-checkout.c: static int sparse_checkout_disable(int argc, const ch
      
      	add_pattern("/*", empty_base, 0, &pl, 0);
      
    +-	prepare_repo_settings(the_repository);
    + 	the_repository->settings.sparse_index = 0;
    + 
    + 	if (update_working_directory(&pl))
     
      ## builtin/worktree.c ##
     @@ builtin/worktree.c: static int add_worktree(const char *path, const char *refname,
    @@ dir.c: enum pattern_match_result path_matches_pattern_list(
      int init_sparse_checkout_patterns(struct index_state *istate)
      {
     -	if (!core_apply_sparse_checkout)
    ++	prepare_repo_settings(istate->repo);
     +	if (!istate->repo->settings.sparse_checkout)
      		return 1;
      	if (istate->sparse_checkout_patterns)
    @@ sparse-index.c: 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)
    ++	prepare_repo_settings(istate->repo);
     +	if (!istate->repo->settings.sparse_checkout || !core_sparse_checkout_cone)
      		return 0;
      
      	if (!(flags & SPARSE_INDEX_MEMORY_ONLY)) {
    +@@ sparse-index.c: int is_sparse_index_allowed(struct index_state *istate, int flags)
    + 		/*
    + 		 * Only convert to sparse if index.sparse is set.
    + 		 */
    +-		prepare_repo_settings(istate->repo);
    + 		if (!istate->repo->settings.sparse_index)
    + 			return 0;
    + 	}
     @@ sparse-index.c: static void clear_skip_worktree_from_present_files_full(struct index_state *ista
      
      void clear_skip_worktree_from_present_files(struct index_state *istate)
    @@ wt-status.c: static void wt_status_check_sparse_checkout(struct repository *r,
      	int i;
      
     -	if (!core_apply_sparse_checkout || r->index->cache_nr == 0) {
    ++	prepare_repo_settings(r);
     +	if (!r->settings.sparse_checkout || r->index->cache_nr == 0) {
      		/*
      		 * Don't compute percentage of checked out files if we
2:  9a63884341 ! 2:  5a2f61443b environment: move access to "core.sparsecheckoutcone" into repo_settings
    @@ Commit message
         `core_sparse_checkout_cone` and is populated in config.c. Refactor the
         code to store it in the variable `sparse_checkout_cone` in the struct
         `repo_settings`.
    -    It's fine not to lazily load it from the config, as the variable
    -    is used quite commonly.
    +
    +    Call `prepare_repo_settings()` where necessary to ensure the `struct
    +    repo_settings` is initialized before use:
    +    - In "dir.c", the function accessing the setting is usually called after
    +      `prepare_repo_settings()`, except for one code path in
    +      "unpack-trees.c", so add a call there.
    +
    +    Avoid redundant calls to `prepare_repo_settings()` where it is already
    +    present:
    +    - In "builtin/mv.c" and "builtin/sparse-checkout.c", it is already
    +      invoked in their respective `cmd_*()` functions.
    +    - In "sparse-index.c", `prepare_repo_settings` is already called before
    +      the setting is accessed.
     
         This change is part of an ongoing effort to eliminate global variables,
         improve modularity and help libify the codebase.
    @@ builtin/grep.c: static int grep_submodule(struct grep_opt *opt,
      	 *	sparse-checkout state.
      	 *
     -	 * 3. "core_sparse_checkout_cone"
    -+	 * 3. "sparse_checkout_cone"
    ++	 * 3. "settings.sparse_checkout_cone"
      	 *	ditto.
      	 *
      	 * Note that this list is not exhaustive.
     
      ## builtin/mv.c ##
     @@ builtin/mv.c: int cmd_mv(int argc,
    - 
    + 		prepare_repo_settings(the_repository);
      		if (ignore_sparse &&
      		    the_repository->settings.sparse_checkout &&
     -		    core_sparse_checkout_cone) {
    @@ repo-settings.h: struct repo_settings {
     
      ## sparse-index.c ##
     @@ sparse-index.c: static int index_has_unmerged_entries(struct index_state *istate)
    - 
      int is_sparse_index_allowed(struct index_state *istate, int flags)
      {
    + 	prepare_repo_settings(istate->repo);
     -	if (!istate->repo->settings.sparse_checkout || !core_sparse_checkout_cone)
     +	if (!istate->repo->settings.sparse_checkout || !istate->repo->settings.sparse_checkout_cone)
      		return 0;
      
      	if (!(flags & SPARSE_INDEX_MEMORY_ONLY)) {
    +
    + ## unpack-trees.c ##
    +@@ unpack-trees.c: enum update_sparsity_result update_sparsity(struct unpack_trees_options *o,
    + 		BUG("update_sparsity() called wrong");
    + 
    + 	trace_performance_enter();
    ++	prepare_repo_settings(the_repository);
    + 
    + 	/* If we weren't given patterns, use the recorded ones */
    + 	if (!pl) {
3:  578c087188 = 3:  45c84a6615 environment: remove the global variable 'sparse_expect_files_outside_of_patterns'


2.49.0

[GSOC PATCH v5 1/3] environment: move access to "core.sparsecheckout" into repo_settings

[Ayush Chandekar]

The setting "core.sparsecheckout" is stored in the global
`core_apply_sparse_checkout` and is populated in config.c. Refactor the
code to store it in the variable `sparse_checkout` in the struct
`repo_settings`.

Call `prepare_repo_settings()` where necessary to ensure the `struct
repo_settings` is initialized before use:
- In "builtin/backfill.c", "builtin/mv.c" and "builtin/clone.c" call
  `prepare_repo_settings()` since their respective `cmd_*()` functions
  did not call it earlier.
- In "dir.c", the function using 'settings.sparse_checkout' is invoked
  in multiple files that do not call `prepare_repo_settings()`, hence
  add a call directly to that function.
- In "sparse-checkout.c", add a call to `prepare_repo_settings()` inside
  `is_sparse_index_allowed()`, as it is used widely and relies on the
  setting.
- In "wt-status.c", call `prepare_repo_settings()` before accessing
  the setting because the function using it is commonly used.

Avoid reduntant calls to `prepare_repo_settings()` where it is already
present:
- In "builtin/worktree.c", it is already invoked in `cmd_worktree()`
  before the setting is accessed.
- In "unpack-tress.c", the function accessing the setting already calls
  it.

This also allows us to remove the definition `#define
USE_THE_REPOSITORY_VARIABLE` from the file 'builtin/backfill.c'.

This change is part of an ongoing effort to eliminate global variables,
improve modularity and help libify the codebase.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 builtin/backfill.c        |  7 ++-----
 builtin/clone.c           |  3 ++-
 builtin/grep.c            |  2 +-
 builtin/mv.c              |  4 ++--
 builtin/sparse-checkout.c | 21 ++++++++++-----------
 builtin/worktree.c        |  2 +-
 config.c                  |  5 -----
 dir.c                     |  3 ++-
 environment.c             |  1 -
 environment.h             |  1 -
 repo-settings.c           |  1 +
 repo-settings.h           |  1 +
 sparse-index.c            |  6 +++---
 unpack-trees.c            |  2 +-
 wt-status.c               |  3 ++-
 15 files changed, 28 insertions(+), 34 deletions(-)

diff --git a/builtin/backfill.c b/builtin/backfill.c
index fa82ad2f6f..bf9e56bff3 100644
--- a/builtin/backfill.c
+++ b/builtin/backfill.c
@@ -1,6 +1,3 @@
-/* We need this macro to access core_apply_sparse_checkout */
-#define USE_THE_REPOSITORY_VARIABLE
-
 #include "builtin.h"
 #include "git-compat-util.h"
 #include "config.h"
@@ -137,9 +134,9 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit
 			     0);
 
 	repo_config(repo, git_default_config, NULL);
-
+	prepare_repo_settings(repo);
 	if (ctx.sparse < 0)
-		ctx.sparse = core_apply_sparse_checkout;
+		ctx.sparse = repo->settings.sparse_checkout;
 
 	result = do_backfill(&ctx);
 	backfill_context_clear(&ctx);
diff --git a/builtin/clone.c b/builtin/clone.c
index 91b9cd0d16..6d70986f3e 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -617,11 +617,12 @@ static int git_sparse_checkout_init(const char *repo)
 	int result = 0;
 	strvec_pushl(&cmd.args, "-C", repo, "sparse-checkout", "set", NULL);
 
+	prepare_repo_settings(the_repository);
 	/*
 	 * 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->settings.sparse_checkout = 1;
 
 	cmd.git_cmd = 1;
 	if (run_command(&cmd)) {
diff --git a/builtin/grep.c b/builtin/grep.c
index 3ce574a605..6111fee60b 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -481,7 +481,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. "settings.sparse_checkout"
 	 *	When running `grep` in the superproject, this setting is
 	 *	populated using the superproject's configs. However, once
 	 *	initialized, this config is globally accessible and is read by
diff --git a/builtin/mv.c b/builtin/mv.c
index 07548fe96a..43ed2e3d0a 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -570,9 +570,9 @@ int cmd_mv(int argc,
 						       &st,
 						       0);
 		rename_index_entry_at(the_repository->index, pos, dst);
-
+		prepare_repo_settings(the_repository);
 		if (ignore_sparse &&
-		    core_apply_sparse_checkout &&
+		    the_repository->settings.sparse_checkout &&
 		    core_sparse_checkout_cone) {
 			/*
 			 * NEEDSWORK: we are *not* paying attention to
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 1bf01591b2..8329d29a27 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -62,7 +62,7 @@ static int sparse_checkout_list(int argc, const char **argv, const char *prefix,
 	int res;
 
 	setup_work_tree();
-	if (!core_apply_sparse_checkout)
+	if (!the_repository->settings.sparse_checkout)
 		die(_("this worktree is not sparse"));
 
 	argc = parse_options(argc, argv, prefix,
@@ -397,11 +397,11 @@ static int set_config(enum sparse_checkout_mode mode)
 
 static enum sparse_checkout_mode update_cone_mode(int *cone_mode) {
 	/* If not specified, use previous definition of cone mode */
-	if (*cone_mode == -1 && core_apply_sparse_checkout)
+	if (*cone_mode == -1 && the_repository->settings.sparse_checkout)
 		*cone_mode = core_sparse_checkout_cone;
 
 	/* Set cone/non-cone mode appropriately */
-	core_apply_sparse_checkout = 1;
+	the_repository->settings.sparse_checkout = 1;
 	if (*cone_mode == 1 || *cone_mode == -1) {
 		core_sparse_checkout_cone = 1;
 		return MODE_CONE_PATTERNS;
@@ -415,7 +415,7 @@ static int update_modes(int *cone_mode, int *sparse_index)
 	int mode, record_mode;
 
 	/* Determine if we need to record the mode; ensure sparse checkout on */
-	record_mode = (*cone_mode != -1) || !core_apply_sparse_checkout;
+	record_mode = (*cone_mode != -1) || !the_repository->settings.sparse_checkout;
 
 	mode = update_cone_mode(cone_mode);
 	if (record_mode && set_config(mode))
@@ -695,9 +695,9 @@ static int modify_pattern_list(struct strvec *args, int use_stdin,
 		break;
 	}
 
-	if (!core_apply_sparse_checkout) {
+	if (!the_repository->settings.sparse_checkout) {
 		set_config(MODE_ALL_PATTERNS);
-		core_apply_sparse_checkout = 1;
+		the_repository->settings.sparse_checkout = 1;
 		changed_config = 1;
 	}
 
@@ -793,7 +793,7 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix,
 	int ret;
 
 	setup_work_tree();
-	if (!core_apply_sparse_checkout)
+	if (!the_repository->settings.sparse_checkout)
 		die(_("no sparse-checkout to add to"));
 
 	repo_read_index(the_repository);
@@ -902,7 +902,7 @@ static int sparse_checkout_reapply(int argc, const char **argv,
 	};
 
 	setup_work_tree();
-	if (!core_apply_sparse_checkout)
+	if (!the_repository->settings.sparse_checkout)
 		die(_("must be in a sparse-checkout to reapply sparsity patterns"));
 
 	reapply_opts.cone_mode = -1;
@@ -935,7 +935,7 @@ static int sparse_checkout_disable(int argc, const char **argv,
 	struct pattern_list pl;
 
 	/*
-	 * We do not exit early if !core_apply_sparse_checkout; due to the
+	 * We do not exit early if !sparse_checkout; due to the
 	 * ability for users to manually muck things up between
 	 *   direct editing of .git/info/sparse-checkout
 	 *   running read-tree -m u HEAD or update-index --skip-worktree
@@ -961,11 +961,10 @@ static int sparse_checkout_disable(int argc, const char **argv,
 	hashmap_init(&pl.recursive_hashmap, pl_hashmap_cmp, NULL, 0);
 	hashmap_init(&pl.parent_hashmap, pl_hashmap_cmp, NULL, 0);
 	pl.use_cone_patterns = 0;
-	core_apply_sparse_checkout = 1;
+	the_repository->settings.sparse_checkout = 1;
 
 	add_pattern("/*", empty_base, 0, &pl, 0);
 
-	prepare_repo_settings(the_repository);
 	the_repository->settings.sparse_index = 0;
 
 	if (update_working_directory(&pl))
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 88a36ea9f8..92e1c92afc 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -536,7 +536,7 @@ static int add_worktree(const char *path, const char *refname,
 	 * If the current worktree has sparse-checkout enabled, then copy
 	 * the sparse-checkout patterns from the current worktree.
 	 */
-	if (core_apply_sparse_checkout)
+	if (the_repository->settings.sparse_checkout)
 		copy_sparse_checkout(sb_repo.buf);
 
 	/*
diff --git a/config.c b/config.c
index b18b5617fc..8fd4dd8c81 100644
--- a/config.c
+++ b/config.c
@@ -1612,11 +1612,6 @@ static int git_default_core_config(const char *var, const char *value,
 		return 0;
 	}
 
-	if (!strcmp(var, "core.sparsecheckout")) {
-		core_apply_sparse_checkout = git_config_bool(var, value);
-		return 0;
-	}
-
 	if (!strcmp(var, "core.sparsecheckoutcone")) {
 		core_sparse_checkout_cone = git_config_bool(var, value);
 		return 0;
diff --git a/dir.c b/dir.c
index a374972b62..5d1cb7a067 100644
--- a/dir.c
+++ b/dir.c
@@ -1503,7 +1503,8 @@ enum pattern_match_result path_matches_pattern_list(
 
 int init_sparse_checkout_patterns(struct index_state *istate)
 {
-	if (!core_apply_sparse_checkout)
+	prepare_repo_settings(istate->repo);
+	if (!istate->repo->settings.sparse_checkout)
 		return 1;
 	if (istate->sparse_checkout_patterns)
 		return 0;
diff --git a/environment.c b/environment.c
index c61d773e7e..a379a9149e 100644
--- a/environment.c
+++ b/environment.c
@@ -64,7 +64,6 @@ enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;
 #endif
 enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
 int grafts_keep_true_parents;
-int core_apply_sparse_checkout;
 int core_sparse_checkout_cone;
 int sparse_expect_files_outside_of_patterns;
 int merge_log_config = -1;
diff --git a/environment.h b/environment.h
index 3d98461a06..6a30512f3c 100644
--- a/environment.h
+++ b/environment.h
@@ -160,7 +160,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/repo-settings.c b/repo-settings.c
index 4129f8fb2b..9270cca561 100644
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -81,6 +81,7 @@ void prepare_repo_settings(struct repository *r)
 		      &r->settings.pack_use_bitmap_boundary_traversal,
 		      r->settings.pack_use_bitmap_boundary_traversal);
 	repo_cfg_bool(r, "core.usereplacerefs", &r->settings.read_replace_refs, 1);
+	repo_cfg_bool(r, "core.sparsecheckout", &r->settings.sparse_checkout, 0);
 
 	/*
 	 * The GIT_TEST_MULTI_PACK_INDEX variable is special in that
diff --git a/repo-settings.h b/repo-settings.h
index 2bf24b2597..9caa7c57a3 100644
--- a/repo-settings.h
+++ b/repo-settings.h
@@ -67,6 +67,7 @@ struct repo_settings {
 	unsigned long big_file_threshold;
 
 	char *hooks_path;
+	int sparse_checkout;
 };
 #define REPO_SETTINGS_INIT { \
 	.shared_repository = -1, \
diff --git a/sparse-index.c b/sparse-index.c
index 5634abafaa..8132c0f2fb 100644
--- a/sparse-index.c
+++ b/sparse-index.c
@@ -150,7 +150,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)
+	prepare_repo_settings(istate->repo);
+	if (!istate->repo->settings.sparse_checkout || !core_sparse_checkout_cone)
 		return 0;
 
 	if (!(flags & SPARSE_INDEX_MEMORY_ONLY)) {
@@ -172,7 +173,6 @@ int is_sparse_index_allowed(struct index_state *istate, int flags)
 		/*
 		 * Only convert to sparse if index.sparse is set.
 		 */
-		prepare_repo_settings(istate->repo);
 		if (!istate->repo->settings.sparse_index)
 			return 0;
 	}
@@ -668,7 +668,7 @@ 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 ||
+	if (!istate->repo->settings.sparse_checkout ||
 	    sparse_expect_files_outside_of_patterns)
 		return;
 
diff --git a/unpack-trees.c b/unpack-trees.c
index 471837f032..02e32c4ba1 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->settings.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 454601afa1..8651134bcb 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1773,7 +1773,8 @@ 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) {
+	prepare_repo_settings(r);
+	if (!r->settings.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.
-- 
2.49.0

[GSOC PATCH v5 2/3] environment: move access to "core.sparsecheckoutcone" into repo_settings

[Ayush Chandekar]

The setting "core.sparsecheckoutcone" is stored in the global
`core_sparse_checkout_cone` and is populated in config.c. Refactor the
code to store it in the variable `sparse_checkout_cone` in the struct
`repo_settings`.

Call `prepare_repo_settings()` where necessary to ensure the `struct
repo_settings` is initialized before use:
- In "dir.c", the function accessing the setting is usually called after
  `prepare_repo_settings()`, except for one code path in
  "unpack-trees.c", so add a call there.

Avoid redundant calls to `prepare_repo_settings()` where it is already
present:
- In "builtin/mv.c" and "builtin/sparse-checkout.c", it is already
  invoked in their respective `cmd_*()` functions.
- In "sparse-index.c", `prepare_repo_settings` is already called before
  the setting is accessed.

This change is part of an ongoing effort to eliminate global variables,
improve modularity and help libify the codebase.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 builtin/grep.c            |  2 +-
 builtin/mv.c              |  2 +-
 builtin/sparse-checkout.c | 28 ++++++++++++++--------------
 config.c                  |  5 -----
 dir.c                     |  2 +-
 environment.c             |  1 -
 environment.h             |  1 -
 repo-settings.c           |  1 +
 repo-settings.h           |  2 ++
 sparse-index.c            |  2 +-
 unpack-trees.c            |  1 +
 11 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 6111fee60b..8971a15ef4 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -491,7 +491,7 @@ static int grep_submodule(struct grep_opt *opt,
 	 *	dictate the behavior for the submodule, making it "forget" its
 	 *	sparse-checkout state.
 	 *
-	 * 3. "core_sparse_checkout_cone"
+	 * 3. "settings.sparse_checkout_cone"
 	 *	ditto.
 	 *
 	 * Note that this list is not exhaustive.
diff --git a/builtin/mv.c b/builtin/mv.c
index 43ed2e3d0a..2d1326a18f 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -573,7 +573,7 @@ int cmd_mv(int argc,
 		prepare_repo_settings(the_repository);
 		if (ignore_sparse &&
 		    the_repository->settings.sparse_checkout &&
-		    core_sparse_checkout_cone) {
+		    the_repository->settings.sparse_checkout_cone) {
 			/*
 			 * NEEDSWORK: we are *not* paying attention to
 			 * "out-to-out" move (<source> is out-of-cone and
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 8329d29a27..8a0ffba9d4 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -71,7 +71,7 @@ static int sparse_checkout_list(int argc, const char **argv, const char *prefix,
 
 	memset(&pl, 0, sizeof(pl));
 
-	pl.use_cone_patterns = core_sparse_checkout_cone;
+	pl.use_cone_patterns = the_repository->settings.sparse_checkout_cone;
 
 	sparse_filename = get_sparse_checkout_filename();
 	res = add_patterns_from_file_to_list(sparse_filename, "", 0, &pl, NULL, 0);
@@ -352,7 +352,7 @@ static int write_patterns_and_update(struct pattern_list *pl)
 	if (!fp)
 		die_errno(_("unable to fdopen %s"), get_lock_file_path(&lk));
 
-	if (core_sparse_checkout_cone)
+	if (the_repository->settings.sparse_checkout_cone)
 		write_cone_to_file(fp, pl);
 	else
 		write_patterns_to_file(fp, pl);
@@ -398,15 +398,15 @@ static int set_config(enum sparse_checkout_mode mode)
 static enum sparse_checkout_mode update_cone_mode(int *cone_mode) {
 	/* If not specified, use previous definition of cone mode */
 	if (*cone_mode == -1 && the_repository->settings.sparse_checkout)
-		*cone_mode = core_sparse_checkout_cone;
+		*cone_mode = the_repository->settings.sparse_checkout_cone;
 
 	/* Set cone/non-cone mode appropriately */
 	the_repository->settings.sparse_checkout = 1;
 	if (*cone_mode == 1 || *cone_mode == -1) {
-		core_sparse_checkout_cone = 1;
+		the_repository->settings.sparse_checkout_cone = 1;
 		return MODE_CONE_PATTERNS;
 	}
-	core_sparse_checkout_cone = 0;
+	the_repository->settings.sparse_checkout_cone = 0;
 	return MODE_ALL_PATTERNS;
 }
 
@@ -572,7 +572,7 @@ static void add_patterns_from_input(struct pattern_list *pl,
 				    FILE *file)
 {
 	int i;
-	if (core_sparse_checkout_cone) {
+	if (the_repository->settings.sparse_checkout_cone) {
 		struct strbuf line = STRBUF_INIT;
 
 		hashmap_init(&pl->recursive_hashmap, pl_hashmap_cmp, NULL, 0);
@@ -637,7 +637,7 @@ static void add_patterns_cone_mode(int argc, const char **argv,
 				use_stdin ? stdin : NULL);
 
 	memset(&existing, 0, sizeof(existing));
-	existing.use_cone_patterns = core_sparse_checkout_cone;
+	existing.use_cone_patterns = the_repository->settings.sparse_checkout_cone;
 
 	if (add_patterns_from_file_to_list(sparse_filename, "", 0,
 					   &existing, NULL, 0))
@@ -683,7 +683,7 @@ static int modify_pattern_list(struct strvec *args, int use_stdin,
 
 	switch (m) {
 	case ADD:
-		if (core_sparse_checkout_cone)
+		if (the_repository->settings.sparse_checkout_cone)
 			add_patterns_cone_mode(args->nr, args->v, pl, use_stdin);
 		else
 			add_patterns_literal(args->nr, args->v, pl, use_stdin);
@@ -719,7 +719,7 @@ static void sanitize_paths(struct strvec *args,
 	if (!args->nr)
 		return;
 
-	if (prefix && *prefix && core_sparse_checkout_cone) {
+	if (prefix && *prefix && the_repository->settings.sparse_checkout_cone) {
 		/*
 		 * The args are not pathspecs, so unfortunately we
 		 * cannot imitate how cmd_add() uses parse_pathspec().
@@ -736,10 +736,10 @@ static void sanitize_paths(struct strvec *args,
 	if (skip_checks)
 		return;
 
-	if (prefix && *prefix && !core_sparse_checkout_cone)
+	if (prefix && *prefix && !the_repository->settings.sparse_checkout_cone)
 		die(_("please run from the toplevel directory in non-cone mode"));
 
-	if (core_sparse_checkout_cone) {
+	if (the_repository->settings.sparse_checkout_cone) {
 		for (i = 0; i < args->nr; i++) {
 			if (args->v[i][0] == '/')
 				die(_("specify directories rather than patterns (no leading slash)"));
@@ -761,7 +761,7 @@ static void sanitize_paths(struct strvec *args,
 		if (S_ISSPARSEDIR(ce->ce_mode))
 			continue;
 
-		if (core_sparse_checkout_cone)
+		if (the_repository->settings.sparse_checkout_cone)
 			die(_("'%s' is not a directory; to treat it as a directory anyway, rerun with --skip-checks"), args->v[i]);
 		else
 			warning(_("pass a leading slash before paths such as '%s' if you want a single file (see NON-CONE PROBLEMS in the git-sparse-checkout manual)."), args->v[i]);
@@ -864,7 +864,7 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix,
 	 * non-cone mode, if nothing is specified, manually select just the
 	 * top-level directory (much as 'init' would do).
 	 */
-	if (!core_sparse_checkout_cone && !set_opts.use_stdin && argc == 0) {
+	if (!the_repository->settings.sparse_checkout_cone && !set_opts.use_stdin && argc == 0) {
 		for (int i = 0; i < default_patterns_nr; i++)
 			strvec_push(&patterns, default_patterns[i]);
 	} else {
@@ -1041,7 +1041,7 @@ static int sparse_checkout_check_rules(int argc, const char **argv, const char *
 		check_rules_opts.cone_mode = 1;
 
 	update_cone_mode(&check_rules_opts.cone_mode);
-	pl.use_cone_patterns = core_sparse_checkout_cone;
+	pl.use_cone_patterns = the_repository->settings.sparse_checkout_cone;
 	if (check_rules_opts.rules_file) {
 		fp = xfopen(check_rules_opts.rules_file, "r");
 		add_patterns_from_input(&pl, argc, argv, fp);
diff --git a/config.c b/config.c
index 8fd4dd8c81..707fe0707a 100644
--- a/config.c
+++ b/config.c
@@ -1612,11 +1612,6 @@ static int git_default_core_config(const char *var, const char *value,
 		return 0;
 	}
 
-	if (!strcmp(var, "core.sparsecheckoutcone")) {
-		core_sparse_checkout_cone = git_config_bool(var, value);
-		return 0;
-	}
-
 	if (!strcmp(var, "core.precomposeunicode")) {
 		precomposed_unicode = git_config_bool(var, value);
 		return 0;
diff --git a/dir.c b/dir.c
index 5d1cb7a067..d2b0a5aef6 100644
--- a/dir.c
+++ b/dir.c
@@ -3460,7 +3460,7 @@ int get_sparse_checkout_patterns(struct pattern_list *pl)
 	int res;
 	char *sparse_filename = get_sparse_checkout_filename();
 
-	pl->use_cone_patterns = core_sparse_checkout_cone;
+	pl->use_cone_patterns = the_repository->settings.sparse_checkout_cone;
 	res = add_patterns_from_file_to_list(sparse_filename, "", 0, pl, NULL, 0);
 
 	free(sparse_filename);
diff --git a/environment.c b/environment.c
index a379a9149e..7d46b80711 100644
--- a/environment.c
+++ b/environment.c
@@ -64,7 +64,6 @@ enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;
 #endif
 enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
 int grafts_keep_true_parents;
-int core_sparse_checkout_cone;
 int sparse_expect_files_outside_of_patterns;
 int merge_log_config = -1;
 int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
diff --git a/environment.h b/environment.h
index 6a30512f3c..00a5b332a0 100644
--- a/environment.h
+++ b/environment.h
@@ -160,7 +160,6 @@ extern int precomposed_unicode;
 extern int protect_hfs;
 extern int protect_ntfs;
 
-extern int core_sparse_checkout_cone;
 extern int sparse_expect_files_outside_of_patterns;
 
 enum rebase_setup_type {
diff --git a/repo-settings.c b/repo-settings.c
index 9270cca561..eebc1f941d 100644
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -82,6 +82,7 @@ void prepare_repo_settings(struct repository *r)
 		      r->settings.pack_use_bitmap_boundary_traversal);
 	repo_cfg_bool(r, "core.usereplacerefs", &r->settings.read_replace_refs, 1);
 	repo_cfg_bool(r, "core.sparsecheckout", &r->settings.sparse_checkout, 0);
+	repo_cfg_bool(r, "core.sparsecheckoutcone", &r->settings.sparse_checkout_cone, 0);
 
 	/*
 	 * The GIT_TEST_MULTI_PACK_INDEX variable is special in that
diff --git a/repo-settings.h b/repo-settings.h
index 9caa7c57a3..443e1399da 100644
--- a/repo-settings.h
+++ b/repo-settings.h
@@ -67,7 +67,9 @@ struct repo_settings {
 	unsigned long big_file_threshold;
 
 	char *hooks_path;
+
 	int sparse_checkout;
+	int sparse_checkout_cone;
 };
 #define REPO_SETTINGS_INIT { \
 	.shared_repository = -1, \
diff --git a/sparse-index.c b/sparse-index.c
index 8132c0f2fb..6fd782a8fc 100644
--- a/sparse-index.c
+++ b/sparse-index.c
@@ -151,7 +151,7 @@ static int index_has_unmerged_entries(struct index_state *istate)
 int is_sparse_index_allowed(struct index_state *istate, int flags)
 {
 	prepare_repo_settings(istate->repo);
-	if (!istate->repo->settings.sparse_checkout || !core_sparse_checkout_cone)
+	if (!istate->repo->settings.sparse_checkout || !istate->repo->settings.sparse_checkout_cone)
 		return 0;
 
 	if (!(flags & SPARSE_INDEX_MEMORY_ONLY)) {
diff --git a/unpack-trees.c b/unpack-trees.c
index 02e32c4ba1..0e9813bddf 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -2144,6 +2144,7 @@ enum update_sparsity_result update_sparsity(struct unpack_trees_options *o,
 		BUG("update_sparsity() called wrong");
 
 	trace_performance_enter();
+	prepare_repo_settings(the_repository);
 
 	/* If we weren't given patterns, use the recorded ones */
 	if (!pl) {
-- 
2.49.0

[GSOC PATCH v5 3/3] environment: remove the global variable 'sparse_expect_files_outside_of_patterns'

[Ayush Chandekar]

The global variable 'sparse_expect_files_outside_of_patterns' is used in
a single function named 'clear_skip_worktree_from_present_files()' in
sparse-index.c. Move its declaration inside that function, removing
unnecessary global state.

This also allows us to remove the definition '#define
USE_THE_REPOSITORY_VARIABLE' from the file 'sparse-index.c'.

This change is part of an ongoing effort to eliminate global variables,
improve modularity and help libify the codebase.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 config.c       | 14 --------------
 environment.c  |  1 -
 environment.h  |  2 --
 sparse-index.c |  4 +++-
 4 files changed, 3 insertions(+), 18 deletions(-)

diff --git a/config.c b/config.c
index 707fe0707a..d212329799 100644
--- a/config.c
+++ b/config.c
@@ -1636,17 +1636,6 @@ static int git_default_core_config(const char *var, const char *value,
 	return platform_core_config(var, value, ctx, cb);
 }
 
-static int git_default_sparse_config(const char *var, const char *value)
-{
-	if (!strcmp(var, "sparse.expectfilesoutsideofpatterns")) {
-		sparse_expect_files_outside_of_patterns = git_config_bool(var, value);
-		return 0;
-	}
-
-	/* Add other config variables here and to Documentation/config/sparse.adoc. */
-	return 0;
-}
-
 static int git_default_i18n_config(const char *var, const char *value)
 {
 	if (!strcmp(var, "i18n.commitencoding")) {
@@ -1808,9 +1797,6 @@ int git_default_config(const char *var, const char *value,
 		return 0;
 	}
 
-	if (starts_with(var, "sparse."))
-		return git_default_sparse_config(var, value);
-
 	/* Add other config variables here and to Documentation/config.adoc. */
 	return 0;
 }
diff --git a/environment.c b/environment.c
index 7d46b80711..d51e0a14aa 100644
--- a/environment.c
+++ b/environment.c
@@ -64,7 +64,6 @@ enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;
 #endif
 enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
 int grafts_keep_true_parents;
-int sparse_expect_files_outside_of_patterns;
 int merge_log_config = -1;
 int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
 unsigned long pack_size_limit_cfg;
diff --git a/environment.h b/environment.h
index 00a5b332a0..5121a28d3f 100644
--- a/environment.h
+++ b/environment.h
@@ -160,8 +160,6 @@ extern int precomposed_unicode;
 extern int protect_hfs;
 extern int protect_ntfs;
 
-extern int sparse_expect_files_outside_of_patterns;
-
 enum rebase_setup_type {
 	AUTOREBASE_NEVER = 0,
 	AUTOREBASE_LOCAL,
diff --git a/sparse-index.c b/sparse-index.c
index 6fd782a8fc..ff33b8516b 100644
--- a/sparse-index.c
+++ b/sparse-index.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
 #define DISABLE_SIGN_COMPARE_WARNINGS
 
 #include "git-compat-util.h"
@@ -668,6 +667,9 @@ static void clear_skip_worktree_from_present_files_full(struct index_state *ista
 
 void clear_skip_worktree_from_present_files(struct index_state *istate)
 {
+	int sparse_expect_files_outside_of_patterns = 0;
+	repo_config_get_bool(istate->repo, "sparse.expectfilesoutsideofpatterns",
+		&sparse_expect_files_outside_of_patterns);
 	if (!istate->repo->settings.sparse_checkout ||
 	    sparse_expect_files_outside_of_patterns)
 		return;
-- 
2.49.0

Re: [GSOC PATCH v5 0/3] environment: remove sparse-checkout related global variables

[Junio C Hamano]

Ayush Chandekar <ayu.chandekar@gmail.com> writes:

> This patch series aims to remove global variables related to
> sparse-checkout from the global scope and to remove the definition
> '#define USE_THE_REPOSITORY_VARIABLE' from a few files.
>
> It contains three patches:
>
> 1 - Remove the global variable 'core_apply_sparse_checkout' and
> move its setting to the 'struct repo_settings'. Also remove the
> definition '#define USE_THE_REPOSITORY_VARIABLE' from
> "builtin/backfill.c".
>
> 2 - Remove the global variable 'core_sparse_checkout_cone' and
> move its setting to the 'struct repo_settings'.
>
> 3 - Remove the global variable
> 'sparse_expect_files_outside_of_patterns` and localize it in the
> function which calls it. Also remove the definition '#define
> USE_THE_REPOSITORY_VARIABLE' from "sparse-index.c"

A call to prepare_repo_settings() function is not free even though
the .settings.initialized member makes second and subsequent calls
to it as cheap as possible.  It makes me a bit worried to see
patches that add new calls to it to places that are fairly deep in
the callchain (as opposed to in cmd_foo() for various built-in
commands).  As long as the control passes these places only once
before we do the heavy lifting and then after the heavy lifting,
the only thing left for us is to exit, we would be fine, but I do
not know if all new calls added in these patches are that kind.

Thanks.

Re: [GSOC PATCH v5 3/3] environment: remove the global variable 'sparse_expect_files_outside_of_patterns'

[Phillip Wood]

Hi Ayush

On 30/06/2025 20:27, Ayush Chandekar wrote:
> 
>   void clear_skip_worktree_from_present_files(struct index_state *istate)
>   {
> +	int sparse_expect_files_outside_of_patterns = 0;
> +	repo_config_get_bool(istate->repo, "sparse.expectfilesoutsideofpatterns",
> +		&sparse_expect_files_outside_of_patterns);

This changes the user facing behavior if 
sparse.expectfilesoutsideofpatterns is not a valid boolean value. 
Currently git will error out when it first starts because that config 
value is parsed by git_default_config() which is called by almost all 
git commands. This means that if someone sets an invalid value they get 
timely feedback that the value is invalid and git dies before doing 
anything. Now, if the value is invalid, git will only die if this 
function is called and it is likely to die in the middle of a command.

Thanks

Phillip

Re: [GSOC PATCH v5 3/3] environment: remove the global variable 'sparse_expect_files_outside_of_patterns'

[Ayush Chandekar]

On Tue, Jul 1, 2025 at 6:48 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> Hi Ayush

Hi Phillip,

>
> On 30/06/2025 20:27, Ayush Chandekar wrote:
> >
> >   void clear_skip_worktree_from_present_files(struct index_state *istate)
> >   {
> > +     int sparse_expect_files_outside_of_patterns = 0;
> > +     repo_config_get_bool(istate->repo, "sparse.expectfilesoutsideofpatterns",
> > +             &sparse_expect_files_outside_of_patterns);
>
> This changes the user facing behavior if
> sparse.expectfilesoutsideofpatterns is not a valid boolean value.
> Currently git will error out when it first starts because that config
> value is parsed by git_default_config() which is called by almost all
> git commands. This means that if someone sets an invalid value they get
> timely feedback that the value is invalid and git dies before doing
> anything. Now, if the value is invalid, git will only die if this
> function is called and it is likely to die in the middle of a command.
>
> Thanks
>
> Phillip
>

Yes, I get your point. However, if we look at settings which are
shifted to `struct repo_settings`, the behaviour is to set a
fallback/default value in case of an invalid input, instead of
throwing an error. This is done inside the `prepare_repo_settings()`
function, which is often called in the middle of a process.

Thanks

Ayush:)

Re: [GSOC PATCH v5 3/3] environment: remove the global variable 'sparse_expect_files_outside_of_patterns'

[Phillip Wood]

Hi Ayush

On 02/07/2025 00:53, Ayush Chandekar wrote:
> On Tue, Jul 1, 2025 at 6:48 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>> On 30/06/2025 20:27, Ayush Chandekar wrote:
>>>
>>>    void clear_skip_worktree_from_present_files(struct index_state *istate)
>>>    {
>>> +     int sparse_expect_files_outside_of_patterns = 0;
>>> +     repo_config_get_bool(istate->repo, "sparse.expectfilesoutsideofpatterns",
>>> +             &sparse_expect_files_outside_of_patterns);
>>
>> This changes the user facing behavior if
>> sparse.expectfilesoutsideofpatterns is not a valid boolean value.
>> Currently git will error out when it first starts because that config
>> value is parsed by git_default_config() which is called by almost all
>> git commands. This means that if someone sets an invalid value they get
>> timely feedback that the value is invalid and git dies before doing
>> anything. Now, if the value is invalid, git will only die if this
>> function is called and it is likely to die in the middle of a command.
> 
> Yes, I get your point. However, if we look at settings which are
> shifted to `struct repo_settings`, the behaviour is to set a
> fallback/default value in case of an invalid input, instead of
> throwing an error. This is done inside the `prepare_repo_settings()`
> function, which is often called in the middle of a process.

I'm a bit confused by this and I'm not quite sure what you're saying for 
a couple of reasons. Firstly this patch is not adding a new member to 
struct repo_settings, it is parsing the config directly and will die() 
in git_config_bool() if the config value is invalid. Secondly 
prepare_repo_settings() ends up calling git_config_bool() and so will 
also die if the config value is invalid rather than setting a default 
value. In the case of prepare_repo_settings() commands that do not want 
to die in the middle of an operation can call that function early on 
before they start doing any real work. Looking at the output of "git 
grep prepare_repo_settings()" many do exactly that. Here there is no 
option for a command to die() early on invalid config values if it wants to.

Thanks

Phillip

Re: [GSOC PATCH v5 3/3] environment: remove the global variable 'sparse_expect_files_outside_of_patterns'

[Junio C Hamano]

Ayush Chandekar <ayu.chandekar@gmail.com> writes:

> Yes, I get your point. However, if we look at settings which are
> shifted to `struct repo_settings`, the behaviour is to set a
> fallback/default value in case of an invalid input, instead of
> throwing an error.

So the user will not be told about misconfiguration like they used
to?  Is that an acceptable way forward, I have to wonder...

Re: [GSOC PATCH v5 0/3] environment: remove sparse-checkout related global variables

[Junio C Hamano]

Ayush Chandekar <ayu.chandekar@gmail.com> writes:

> This patch series aims to remove global variables related to
> sparse-checkout from the global scope and to remove the definition
> '#define USE_THE_REPOSITORY_VARIABLE' from a few files.

For this topic, it seems that the ball is in the author's court
after a few review comments.  What's the status of it?

If it is back-burnered for now, then we may eject the topic out of
'seen' to make room for a new topic that touches the same area with
overlapping changes.

Thanks.

Re: [GSOC PATCH v5 0/3] environment: remove sparse-checkout related global variables

[Ayush Chandekar]

Hi Junio,

On Wed, Jul 9, 2025 at 5:48 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Ayush Chandekar <ayu.chandekar@gmail.com> writes:
>
> > This patch series aims to remove global variables related to
> > sparse-checkout from the global scope and to remove the definition
> > '#define USE_THE_REPOSITORY_VARIABLE' from a few files.
>
> For this topic, it seems that the ball is in the author's court
> after a few review comments.  What's the status of it?
>
> If it is back-burnered for now, then we may eject the topic out of
> 'seen' to make room for a new topic that touches the same area with
> overlapping changes.
>
> Thanks.
>

Apologies for not keeping you updated. I had actually drafted a
response explaining that in 1/3, I only added two new calls to
`prepare_repo_settings()`. Some other calls were made in cmd_foo()
functions, and a few others were rearranged and removed as well. (I
should've made it clearer in the commit message). In fact, the commit
message mistaknely suggests that one extra call was added which wasn't
the case. For 2/3, only one new call was added. As for 3/3, I can move
the setting into the repo_settings struct, since
`prepare_repo_settings()` is already being called before the variable
is accessed.

At the time, I felt my draft wasn't convincinv enough, and then got
caught up with responses on other patch series and also was working on
another one.

You can eject this topic if you think another one is more important or
if this one still needs work.

Again, Sorry for not updating earlied, will do better next time.

Thanks,
Ayush

Re: [GSOC PATCH v5 3/3] environment: remove the global variable 'sparse_expect_files_outside_of_patterns'

[Ayush Chandekar]

Hi Phillip,

On Wed, Jul 2, 2025 at 2:31 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> Hi Ayush
>
> On 02/07/2025 00:53, Ayush Chandekar wrote:
> > On Tue, Jul 1, 2025 at 6:48 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
> >> On 30/06/2025 20:27, Ayush Chandekar wrote:
> >>>
> >>>    void clear_skip_worktree_from_present_files(struct index_state *istate)
> >>>    {
> >>> +     int sparse_expect_files_outside_of_patterns = 0;
> >>> +     repo_config_get_bool(istate->repo, "sparse.expectfilesoutsideofpatterns",
> >>> +             &sparse_expect_files_outside_of_patterns);
> >>
> >> This changes the user facing behavior if
> >> sparse.expectfilesoutsideofpatterns is not a valid boolean value.
> >> Currently git will error out when it first starts because that config
> >> value is parsed by git_default_config() which is called by almost all
> >> git commands. This means that if someone sets an invalid value they get
> >> timely feedback that the value is invalid and git dies before doing
> >> anything. Now, if the value is invalid, git will only die if this
> >> function is called and it is likely to die in the middle of a command.
> >
> > Yes, I get your point. However, if we look at settings which are
> > shifted to `struct repo_settings`, the behaviour is to set a
> > fallback/default value in case of an invalid input, instead of
> > throwing an error. This is done inside the `prepare_repo_settings()`
> > function, which is often called in the middle of a process.
>
> I'm a bit confused by this and I'm not quite sure what you're saying for
> a couple of reasons. Firstly this patch is not adding a new member to
> struct repo_settings, it is parsing the config directly and will die()
> in git_config_bool() if the config value is invalid. Secondly
> prepare_repo_settings() ends up calling git_config_bool() and so will
> also die if the config value is invalid rather than setting a default
> value. In the case of prepare_repo_settings() commands that do not want
> to die in the middle of an operation can call that function early on
> before they start doing any real work. Looking at the output of "git
> grep prepare_repo_settings()" many do exactly that. Here there is no
> option for a command to die() early on invalid config values if it wants to.
>
> Thanks
>
> Phillip
>

Yeah, you're right about the `prepare_repo_settings()` which throws
error and since they're called early on, we are notified about it
before any heavy operation takes place. I had it confused as it also
stores a default value if there isn't a config setting set. I can move
this setting into `struct repo_settings`, and since
`prepare_repo_settings()` is already called just before the function
that uses this variable, I don't have to add any extra call to it.

Thanks!
Ayush

Re: [GSOC PATCH v5 3/3] environment: remove the global variable 'sparse_expect_files_outside_of_patterns'

[Ayush Chandekar]

On Wed, Jul 2, 2025 at 2:51 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Ayush Chandekar <ayu.chandekar@gmail.com> writes:
>
> > Yes, I get your point. However, if we look at settings which are
> > shifted to `struct repo_settings`, the behaviour is to set a
> > fallback/default value in case of an invalid input, instead of
> > throwing an error.
>
> So the user will not be told about misconfiguration like they used
> to?  Is that an acceptable way forward, I have to wonder...
>

I was actually mistaken that `prepare_repo_settings()` does not throw
any error due to the fact that it sets default value when a config
setting is not set. So I thought, if there's invalid value in the
config, it would just set the default value, but I was wrong.

Thanks,
Ayush

[GSOC PATCH v6 0/3] environment: remove sparse-checkout related global variables

[Ayush Chandekar]

This patch series aims to remove global variables related to sparse-checkout from the global scope and to remove the definition '#define USE_THE_REPOSITORY_VARIABLE' from a few files.

It contains three patches:

1 - Remove the global variable 'core_apply_sparse_checkout' and move its setting to the 'struct repo_settings'. Also remove the definition '#define USE_THE_REPOSITORY_VARIABLE' from "builtin/backfill.c". 

2 - Remove the global variable 'core_sparse_checkout_cone' and move its setting to the 'struct repo_settings'.

3 - Remove the global variable 'sparse_expect_files_outside_of_patterns` and move its setting to 'struct repo_settings'. Also remove the definition '#define USE_THE_REPOSITORY_VARIABLE' from "sparse-index.c"

Thanks a lot to Christian for mentoring, and to Junio, Patrick, Phillip and Ben for reviewing

Ayush Chandekar (3):
  environment: move access to "core.sparsecheckout" into repo_settings
  environment: move access to "core.sparsecheckoutcone" into
    repo_settings
  environment: remove the global variable
    'sparse_expect_files_outside_of_patterns'

 builtin/backfill.c        |  7 ++----
 builtin/clone.c           |  3 ++-
 builtin/grep.c            |  4 ++--
 builtin/mv.c              |  6 ++---
 builtin/sparse-checkout.c | 49 +++++++++++++++++++--------------------
 builtin/worktree.c        |  2 +-
 config.c                  | 24 -------------------
 dir.c                     |  5 ++--
 environment.c             |  3 ---
 environment.h             |  4 ----
 repo-settings.c           |  3 +++
 repo-settings.h           |  4 ++++
 sparse-index.c            |  9 ++++---
 unpack-trees.c            |  2 +-
 wt-status.c               |  3 ++-
 15 files changed, 51 insertions(+), 77 deletions(-)

-- 

Discussions since v5:
* For 1/3 and 2/3, Junio told me that it was concerning to put so many calls to `prepare_repo_settings()` so I tried to minimize the calls and made sure that there's no useless calling.
* For 3/3, Phillip told me that it broke user-facing as it will be parsed quite late in the callchain and might throw an error mid operation which we do not want.

Main problem I was dealing with was `prepare_repo_settings()`. I think we can try to get useless calls to `prepare_repo_settings()`. Other than that, I agree with the approach that came up in the discussion here:
https://lore.kernel.org/git/32fceddc-c867-4a47-bde8-c873279edbc1@gmail.com/
which was adding a call to `prepare_repo_settings()` in `repo_config()`.

Summary of range-diff:
* Changed some calls to `prepare_repo_settings()` to make sure that it is added in code paths only where it has no been called before. 
  Also changed a mistake in commit message: s/sparse-checkout.c/sparse-index.c  and have explained it more.
* Shifted the global variable to `struct repo_settings`(the previous version just localized it leading to user experience issues) and changed the commit message

Range-diff with v5:
1:  54ea376768 ! 1:  d0e2042b30 environment: move access to "core.sparsecheckout" into repo_settings
    @@ Commit message
         - In "dir.c", the function using 'settings.sparse_checkout' is invoked
           in multiple files that do not call `prepare_repo_settings()`, hence
           add a call directly to that function.
    -    - In "sparse-checkout.c", add a call to `prepare_repo_settings()` inside
    -      `is_sparse_index_allowed()`, as it is used widely and relies on the
    -      setting.
    +    - In "sparse-index.c", remove a call to `prepare_repo_settings()`
    +      from the function `is_sparse_index_allowed()` as it is called
    +      everytime before the function is called, and add a call to
    +      `prepare_repo_settings()` inside `convert_to_sparse()`, as it is
    +      used widely without having a call to `prepare_repo_settings()`
    +      before and relies on the setting.
         - In "wt-status.c", call `prepare_repo_settings()` before accessing
           the setting because the function using it is commonly used.
     
    @@ dir.c: enum pattern_match_result path_matches_pattern_list(
      int init_sparse_checkout_patterns(struct index_state *istate)
      {
     -	if (!core_apply_sparse_checkout)
    -+	prepare_repo_settings(istate->repo);
     +	if (!istate->repo->settings.sparse_checkout)
      		return 1;
      	if (istate->sparse_checkout_patterns)
      		return 0;
    +@@ dir.c: static int path_in_sparse_checkout_1(const char *path,
    + 	enum pattern_match_result match = UNDECIDED;
    + 	const char *end, *slash;
    + 
    ++	prepare_repo_settings(istate->repo);
    + 	/*
    + 	 * We default to accepting a path if the path is empty, there are no
    + 	 * patterns, or the patterns are of the wrong type.
     
      ## environment.c ##
     @@ environment.c: enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;
    @@ sparse-index.c: 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)
    -+	prepare_repo_settings(istate->repo);
     +	if (!istate->repo->settings.sparse_checkout || !core_sparse_checkout_cone)
      		return 0;
      
    @@ sparse-index.c: int is_sparse_index_allowed(struct index_state *istate, int flag
      		if (!istate->repo->settings.sparse_index)
      			return 0;
      	}
    +@@ sparse-index.c: int is_sparse_index_allowed(struct index_state *istate, int flags)
    + 
    + int convert_to_sparse(struct index_state *istate, int flags)
    + {
    ++	prepare_repo_settings(istate->repo);
    + 	/*
    + 	 * If the index is already sparse, empty, or otherwise
    + 	 * cannot be converted to sparse, do not convert.
     @@ sparse-index.c: static void clear_skip_worktree_from_present_files_full(struct index_state *ista
      
      void clear_skip_worktree_from_present_files(struct index_state *istate)
2:  c4d37dc7c5 ! 2:  d799faca3f environment: move access to "core.sparsecheckoutcone" into repo_settings
    @@ Commit message
         code to store it in the variable `sparse_checkout_cone` in the struct
         `repo_settings`.
     
    -    Call `prepare_repo_settings()` where necessary to ensure the `struct
    -    repo_settings` is initialized before use:
    -    - In "dir.c", the function accessing the setting is usually called after
    -      `prepare_repo_settings()`, except for one code path in
    -      "unpack-trees.c", so add a call there.
    -
         Avoid redundant calls to `prepare_repo_settings()` where it is already
         present:
         - In "builtin/mv.c" and "builtin/sparse-checkout.c", it is already
           invoked in their respective `cmd_*()` functions.
    -    - In "sparse-index.c", `prepare_repo_settings` is already called before
    -      the setting is accessed.
    +    - In "sparse-index.c", `prepare_repo_settings()` is already called
    +      before the setting is accessed.
    +    - In "dir.c", `prepare_repo_settings()` is already called in all code
    +      paths before the setting is accessed.
     
         This change is part of an ongoing effort to eliminate global variables,
         improve modularity and help libify the codebase.
    @@ repo-settings.h: struct repo_settings {
     
      ## sparse-index.c ##
     @@ sparse-index.c: static int index_has_unmerged_entries(struct index_state *istate)
    + 
      int is_sparse_index_allowed(struct index_state *istate, int flags)
      {
    - 	prepare_repo_settings(istate->repo);
     -	if (!istate->repo->settings.sparse_checkout || !core_sparse_checkout_cone)
     +	if (!istate->repo->settings.sparse_checkout || !istate->repo->settings.sparse_checkout_cone)
      		return 0;
      
      	if (!(flags & SPARSE_INDEX_MEMORY_ONLY)) {
    -
    - ## unpack-trees.c ##
    -@@ unpack-trees.c: enum update_sparsity_result update_sparsity(struct unpack_trees_options *o,
    - 		BUG("update_sparsity() called wrong");
    - 
    - 	trace_performance_enter();
    -+	prepare_repo_settings(the_repository);
    - 
    - 	/* If we weren't given patterns, use the recorded ones */
    - 	if (!pl) {
3:  84cb67469e ! 3:  35137b6814 environment: remove the global variable 'sparse_expect_files_outside_of_patterns'
    @@ Metadata
      ## Commit message ##
         environment: remove the global variable 'sparse_expect_files_outside_of_patterns'
     
    -    The global variable 'sparse_expect_files_outside_of_patterns' is used in
    -    a single function named 'clear_skip_worktree_from_present_files()' in
    -    sparse-index.c. Move its declaration inside that function, removing
    -    unnecessary global state.
    +    The setting "sparse.expectFilesOutsideOfPatterns" is stored in the
    +    global variable 'sparse_expect_files_outside_of_patterns' and allows
    +    files marked with the `SKIP_WORKTREE` bit to be present in the worktree.
    +
    +    As this setting is closely related to repository, remove the global
    +    variable and store the setting in the `struct repo_settings` along
    +    with other sparse checkout related settings.
     
         This also allows us to remove the definition '#define
         USE_THE_REPOSITORY_VARIABLE' from the file 'sparse-index.c'.
    @@ environment.h: extern int precomposed_unicode;
      	AUTOREBASE_NEVER = 0,
      	AUTOREBASE_LOCAL,
     
    + ## repo-settings.c ##
    +@@ repo-settings.c: void prepare_repo_settings(struct repository *r)
    + 	repo_cfg_bool(r, "core.usereplacerefs", &r->settings.read_replace_refs, 1);
    + 	repo_cfg_bool(r, "core.sparsecheckout", &r->settings.sparse_checkout, 0);
    + 	repo_cfg_bool(r, "core.sparsecheckoutcone", &r->settings.sparse_checkout_cone, 0);
    ++	repo_cfg_bool(r, "sparse.expectfilesoutsideofpatterns", &r->settings.sparse_expect_files_outside_of_patterns, 0);
    + 
    + 	/*
    + 	 * The GIT_TEST_MULTI_PACK_INDEX variable is special in that
    +
    + ## repo-settings.h ##
    +@@ repo-settings.h: struct repo_settings {
    + 
    + 	int sparse_checkout;
    + 	int sparse_checkout_cone;
    ++	int sparse_expect_files_outside_of_patterns;
    + };
    + #define REPO_SETTINGS_INIT { \
    + 	.shared_repository = -1, \
    +
      ## sparse-index.c ##
     @@
     -#define USE_THE_REPOSITORY_VARIABLE
    @@ sparse-index.c
      
      #include "git-compat-util.h"
     @@ sparse-index.c: static void clear_skip_worktree_from_present_files_full(struct index_state *ista
    - 
      void clear_skip_worktree_from_present_files(struct index_state *istate)
      {
    -+	int sparse_expect_files_outside_of_patterns = 0;
    -+	repo_config_get_bool(istate->repo, "sparse.expectfilesoutsideofpatterns",
    -+		&sparse_expect_files_outside_of_patterns);
      	if (!istate->repo->settings.sparse_checkout ||
    - 	    sparse_expect_files_outside_of_patterns)
    +-	    sparse_expect_files_outside_of_patterns)
    ++	    istate->repo->settings.sparse_expect_files_outside_of_patterns)
      		return;
    + 
    + 	if (clear_skip_worktree_from_present_files_sparse(istate)) {

2.49.0

[GSOC PATCH v6 1/3] environment: move access to "core.sparsecheckout" into repo_settings

[Ayush Chandekar]

The setting "core.sparsecheckout" is stored in the global
`core_apply_sparse_checkout` and is populated in config.c. Refactor the
code to store it in the variable `sparse_checkout` in the struct
`repo_settings`.

Call `prepare_repo_settings()` where necessary to ensure the `struct
repo_settings` is initialized before use:
- In "builtin/backfill.c", "builtin/mv.c" and "builtin/clone.c" call
  `prepare_repo_settings()` since their respective `cmd_*()` functions
  did not call it earlier.
- In "dir.c", the function using 'settings.sparse_checkout' is invoked
  in multiple files that do not call `prepare_repo_settings()`, hence
  add a call directly to that function.
- In "sparse-index.c", remove a call to `prepare_repo_settings()`
  from the function `is_sparse_index_allowed()` as it is called
  everytime before the function is called, and add a call to
  `prepare_repo_settings()` inside `convert_to_sparse()`, as it is
  used widely without having a call to `prepare_repo_settings()`
  before and relies on the setting.
- In "wt-status.c", call `prepare_repo_settings()` before accessing
  the setting because the function using it is commonly used.

Avoid reduntant calls to `prepare_repo_settings()` where it is already
present:
- In "builtin/worktree.c", it is already invoked in `cmd_worktree()`
  before the setting is accessed.
- In "unpack-tress.c", the function accessing the setting already calls
  it.

This also allows us to remove the definition `#define
USE_THE_REPOSITORY_VARIABLE` from the file 'builtin/backfill.c'.

This change is part of an ongoing effort to eliminate global variables,
improve modularity and help libify the codebase.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 builtin/backfill.c        |  7 ++-----
 builtin/clone.c           |  3 ++-
 builtin/grep.c            |  2 +-
 builtin/mv.c              |  4 ++--
 builtin/sparse-checkout.c | 21 ++++++++++-----------
 builtin/worktree.c        |  2 +-
 config.c                  |  5 -----
 dir.c                     |  3 ++-
 environment.c             |  1 -
 environment.h             |  1 -
 repo-settings.c           |  1 +
 repo-settings.h           |  1 +
 sparse-index.c            |  6 +++---
 unpack-trees.c            |  2 +-
 wt-status.c               |  3 ++-
 15 files changed, 28 insertions(+), 34 deletions(-)

diff --git a/builtin/backfill.c b/builtin/backfill.c
index 80056abe47..48b2518743 100644
--- a/builtin/backfill.c
+++ b/builtin/backfill.c
@@ -1,6 +1,3 @@
-/* We need this macro to access core_apply_sparse_checkout */
-#define USE_THE_REPOSITORY_VARIABLE
-
 #include "builtin.h"
 #include "git-compat-util.h"
 #include "config.h"
@@ -137,9 +134,9 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit
 			     0);
 
 	repo_config(repo, git_default_config, NULL);
-
+	prepare_repo_settings(repo);
 	if (ctx.sparse < 0)
-		ctx.sparse = core_apply_sparse_checkout;
+		ctx.sparse = repo->settings.sparse_checkout;
 
 	result = do_backfill(&ctx);
 	backfill_context_clear(&ctx);
diff --git a/builtin/clone.c b/builtin/clone.c
index 6d08abed37..6b6d429fd8 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -619,11 +619,12 @@ static int git_sparse_checkout_init(const char *repo)
 	int result = 0;
 	strvec_pushl(&cmd.args, "-C", repo, "sparse-checkout", "set", NULL);
 
+	prepare_repo_settings(the_repository);
 	/*
 	 * 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->settings.sparse_checkout = 1;
 
 	cmd.git_cmd = 1;
 	if (run_command(&cmd)) {
diff --git a/builtin/grep.c b/builtin/grep.c
index 39273d9c0f..fbad1a72a2 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -481,7 +481,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. "settings.sparse_checkout"
 	 *	When running `grep` in the superproject, this setting is
 	 *	populated using the superproject's configs. However, once
 	 *	initialized, this config is globally accessible and is read by
diff --git a/builtin/mv.c b/builtin/mv.c
index 07548fe96a..43ed2e3d0a 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -570,9 +570,9 @@ int cmd_mv(int argc,
 						       &st,
 						       0);
 		rename_index_entry_at(the_repository->index, pos, dst);
-
+		prepare_repo_settings(the_repository);
 		if (ignore_sparse &&
-		    core_apply_sparse_checkout &&
+		    the_repository->settings.sparse_checkout &&
 		    core_sparse_checkout_cone) {
 			/*
 			 * NEEDSWORK: we are *not* paying attention to
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 1bf01591b2..8329d29a27 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -62,7 +62,7 @@ static int sparse_checkout_list(int argc, const char **argv, const char *prefix,
 	int res;
 
 	setup_work_tree();
-	if (!core_apply_sparse_checkout)
+	if (!the_repository->settings.sparse_checkout)
 		die(_("this worktree is not sparse"));
 
 	argc = parse_options(argc, argv, prefix,
@@ -397,11 +397,11 @@ static int set_config(enum sparse_checkout_mode mode)
 
 static enum sparse_checkout_mode update_cone_mode(int *cone_mode) {
 	/* If not specified, use previous definition of cone mode */
-	if (*cone_mode == -1 && core_apply_sparse_checkout)
+	if (*cone_mode == -1 && the_repository->settings.sparse_checkout)
 		*cone_mode = core_sparse_checkout_cone;
 
 	/* Set cone/non-cone mode appropriately */
-	core_apply_sparse_checkout = 1;
+	the_repository->settings.sparse_checkout = 1;
 	if (*cone_mode == 1 || *cone_mode == -1) {
 		core_sparse_checkout_cone = 1;
 		return MODE_CONE_PATTERNS;
@@ -415,7 +415,7 @@ static int update_modes(int *cone_mode, int *sparse_index)
 	int mode, record_mode;
 
 	/* Determine if we need to record the mode; ensure sparse checkout on */
-	record_mode = (*cone_mode != -1) || !core_apply_sparse_checkout;
+	record_mode = (*cone_mode != -1) || !the_repository->settings.sparse_checkout;
 
 	mode = update_cone_mode(cone_mode);
 	if (record_mode && set_config(mode))
@@ -695,9 +695,9 @@ static int modify_pattern_list(struct strvec *args, int use_stdin,
 		break;
 	}
 
-	if (!core_apply_sparse_checkout) {
+	if (!the_repository->settings.sparse_checkout) {
 		set_config(MODE_ALL_PATTERNS);
-		core_apply_sparse_checkout = 1;
+		the_repository->settings.sparse_checkout = 1;
 		changed_config = 1;
 	}
 
@@ -793,7 +793,7 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix,
 	int ret;
 
 	setup_work_tree();
-	if (!core_apply_sparse_checkout)
+	if (!the_repository->settings.sparse_checkout)
 		die(_("no sparse-checkout to add to"));
 
 	repo_read_index(the_repository);
@@ -902,7 +902,7 @@ static int sparse_checkout_reapply(int argc, const char **argv,
 	};
 
 	setup_work_tree();
-	if (!core_apply_sparse_checkout)
+	if (!the_repository->settings.sparse_checkout)
 		die(_("must be in a sparse-checkout to reapply sparsity patterns"));
 
 	reapply_opts.cone_mode = -1;
@@ -935,7 +935,7 @@ static int sparse_checkout_disable(int argc, const char **argv,
 	struct pattern_list pl;
 
 	/*
-	 * We do not exit early if !core_apply_sparse_checkout; due to the
+	 * We do not exit early if !sparse_checkout; due to the
 	 * ability for users to manually muck things up between
 	 *   direct editing of .git/info/sparse-checkout
 	 *   running read-tree -m u HEAD or update-index --skip-worktree
@@ -961,11 +961,10 @@ static int sparse_checkout_disable(int argc, const char **argv,
 	hashmap_init(&pl.recursive_hashmap, pl_hashmap_cmp, NULL, 0);
 	hashmap_init(&pl.parent_hashmap, pl_hashmap_cmp, NULL, 0);
 	pl.use_cone_patterns = 0;
-	core_apply_sparse_checkout = 1;
+	the_repository->settings.sparse_checkout = 1;
 
 	add_pattern("/*", empty_base, 0, &pl, 0);
 
-	prepare_repo_settings(the_repository);
 	the_repository->settings.sparse_index = 0;
 
 	if (update_working_directory(&pl))
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 2dceeeed8b..a3a1bb00e3 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -536,7 +536,7 @@ static int add_worktree(const char *path, const char *refname,
 	 * If the current worktree has sparse-checkout enabled, then copy
 	 * the sparse-checkout patterns from the current worktree.
 	 */
-	if (core_apply_sparse_checkout)
+	if (the_repository->settings.sparse_checkout)
 		copy_sparse_checkout(sb_repo.buf);
 
 	/*
diff --git a/config.c b/config.c
index 095a17bd42..da76bf4fde 100644
--- a/config.c
+++ b/config.c
@@ -1607,11 +1607,6 @@ static int git_default_core_config(const char *var, const char *value,
 		return 0;
 	}
 
-	if (!strcmp(var, "core.sparsecheckout")) {
-		core_apply_sparse_checkout = git_config_bool(var, value);
-		return 0;
-	}
-
 	if (!strcmp(var, "core.sparsecheckoutcone")) {
 		core_sparse_checkout_cone = git_config_bool(var, value);
 		return 0;
diff --git a/dir.c b/dir.c
index 02873f59ea..01d7574c09 100644
--- a/dir.c
+++ b/dir.c
@@ -1516,7 +1516,7 @@ enum pattern_match_result path_matches_pattern_list(
 
 int init_sparse_checkout_patterns(struct index_state *istate)
 {
-	if (!core_apply_sparse_checkout)
+	if (!istate->repo->settings.sparse_checkout)
 		return 1;
 	if (istate->sparse_checkout_patterns)
 		return 0;
@@ -1539,6 +1539,7 @@ static int path_in_sparse_checkout_1(const char *path,
 	enum pattern_match_result match = UNDECIDED;
 	const char *end, *slash;
 
+	prepare_repo_settings(istate->repo);
 	/*
 	 * We default to accepting a path if the path is empty, there are no
 	 * patterns, or the patterns are of the wrong type.
diff --git a/environment.c b/environment.c
index 7c2480b22e..3a21629f86 100644
--- a/environment.c
+++ b/environment.c
@@ -63,7 +63,6 @@ enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;
 #endif
 enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
 int grafts_keep_true_parents;
-int core_apply_sparse_checkout;
 int core_sparse_checkout_cone;
 int sparse_expect_files_outside_of_patterns;
 int merge_log_config = -1;
diff --git a/environment.h b/environment.h
index 3d806ced6e..1e1e83fff1 100644
--- a/environment.h
+++ b/environment.h
@@ -159,7 +159,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/repo-settings.c b/repo-settings.c
index 195c24e9c0..c3aa92c065 100644
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -84,6 +84,7 @@ void prepare_repo_settings(struct repository *r)
 		      &r->settings.pack_use_bitmap_boundary_traversal,
 		      r->settings.pack_use_bitmap_boundary_traversal);
 	repo_cfg_bool(r, "core.usereplacerefs", &r->settings.read_replace_refs, 1);
+	repo_cfg_bool(r, "core.sparsecheckout", &r->settings.sparse_checkout, 0);
 
 	/*
 	 * The GIT_TEST_MULTI_PACK_INDEX variable is special in that
diff --git a/repo-settings.h b/repo-settings.h
index d477885561..95900784f1 100644
--- a/repo-settings.h
+++ b/repo-settings.h
@@ -68,6 +68,7 @@ struct repo_settings {
 	unsigned long big_file_threshold;
 
 	char *hooks_path;
+	int sparse_checkout;
 };
 #define REPO_SETTINGS_INIT { \
 	.shared_repository = -1, \
diff --git a/sparse-index.c b/sparse-index.c
index 5634abafaa..c9e5a5efe1 100644
--- a/sparse-index.c
+++ b/sparse-index.c
@@ -150,7 +150,7 @@ 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)
+	if (!istate->repo->settings.sparse_checkout || !core_sparse_checkout_cone)
 		return 0;
 
 	if (!(flags & SPARSE_INDEX_MEMORY_ONLY)) {
@@ -172,7 +172,6 @@ int is_sparse_index_allowed(struct index_state *istate, int flags)
 		/*
 		 * Only convert to sparse if index.sparse is set.
 		 */
-		prepare_repo_settings(istate->repo);
 		if (!istate->repo->settings.sparse_index)
 			return 0;
 	}
@@ -196,6 +195,7 @@ int is_sparse_index_allowed(struct index_state *istate, int flags)
 
 int convert_to_sparse(struct index_state *istate, int flags)
 {
+	prepare_repo_settings(istate->repo);
 	/*
 	 * If the index is already sparse, empty, or otherwise
 	 * cannot be converted to sparse, do not convert.
@@ -668,7 +668,7 @@ 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 ||
+	if (!istate->repo->settings.sparse_checkout ||
 	    sparse_expect_files_outside_of_patterns)
 		return;
 
diff --git a/unpack-trees.c b/unpack-trees.c
index f38c761ab9..2e6d3f98f7 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->settings.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 454601afa1..8651134bcb 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1773,7 +1773,8 @@ 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) {
+	prepare_repo_settings(r);
+	if (!r->settings.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.
-- 
2.49.0

[GSOC PATCH v6 2/3] environment: move access to "core.sparsecheckoutcone" into repo_settings

[Ayush Chandekar]

The setting "core.sparsecheckoutcone" is stored in the global
`core_sparse_checkout_cone` and is populated in config.c. Refactor the
code to store it in the variable `sparse_checkout_cone` in the struct
`repo_settings`.

Avoid redundant calls to `prepare_repo_settings()` where it is already
present:
- In "builtin/mv.c" and "builtin/sparse-checkout.c", it is already
  invoked in their respective `cmd_*()` functions.
- In "sparse-index.c", `prepare_repo_settings()` is already called
  before the setting is accessed.
- In "dir.c", `prepare_repo_settings()` is already called in all code
  paths before the setting is accessed.

This change is part of an ongoing effort to eliminate global variables,
improve modularity and help libify the codebase.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 builtin/grep.c            |  2 +-
 builtin/mv.c              |  2 +-
 builtin/sparse-checkout.c | 28 ++++++++++++++--------------
 config.c                  |  5 -----
 dir.c                     |  2 +-
 environment.c             |  1 -
 environment.h             |  1 -
 repo-settings.c           |  1 +
 repo-settings.h           |  2 ++
 sparse-index.c            |  2 +-
 10 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index fbad1a72a2..bd5481cc44 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -491,7 +491,7 @@ static int grep_submodule(struct grep_opt *opt,
 	 *	dictate the behavior for the submodule, making it "forget" its
 	 *	sparse-checkout state.
 	 *
-	 * 3. "core_sparse_checkout_cone"
+	 * 3. "settings.sparse_checkout_cone"
 	 *	ditto.
 	 *
 	 * Note that this list is not exhaustive.
diff --git a/builtin/mv.c b/builtin/mv.c
index 43ed2e3d0a..2d1326a18f 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -573,7 +573,7 @@ int cmd_mv(int argc,
 		prepare_repo_settings(the_repository);
 		if (ignore_sparse &&
 		    the_repository->settings.sparse_checkout &&
-		    core_sparse_checkout_cone) {
+		    the_repository->settings.sparse_checkout_cone) {
 			/*
 			 * NEEDSWORK: we are *not* paying attention to
 			 * "out-to-out" move (<source> is out-of-cone and
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 8329d29a27..8a0ffba9d4 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -71,7 +71,7 @@ static int sparse_checkout_list(int argc, const char **argv, const char *prefix,
 
 	memset(&pl, 0, sizeof(pl));
 
-	pl.use_cone_patterns = core_sparse_checkout_cone;
+	pl.use_cone_patterns = the_repository->settings.sparse_checkout_cone;
 
 	sparse_filename = get_sparse_checkout_filename();
 	res = add_patterns_from_file_to_list(sparse_filename, "", 0, &pl, NULL, 0);
@@ -352,7 +352,7 @@ static int write_patterns_and_update(struct pattern_list *pl)
 	if (!fp)
 		die_errno(_("unable to fdopen %s"), get_lock_file_path(&lk));
 
-	if (core_sparse_checkout_cone)
+	if (the_repository->settings.sparse_checkout_cone)
 		write_cone_to_file(fp, pl);
 	else
 		write_patterns_to_file(fp, pl);
@@ -398,15 +398,15 @@ static int set_config(enum sparse_checkout_mode mode)
 static enum sparse_checkout_mode update_cone_mode(int *cone_mode) {
 	/* If not specified, use previous definition of cone mode */
 	if (*cone_mode == -1 && the_repository->settings.sparse_checkout)
-		*cone_mode = core_sparse_checkout_cone;
+		*cone_mode = the_repository->settings.sparse_checkout_cone;
 
 	/* Set cone/non-cone mode appropriately */
 	the_repository->settings.sparse_checkout = 1;
 	if (*cone_mode == 1 || *cone_mode == -1) {
-		core_sparse_checkout_cone = 1;
+		the_repository->settings.sparse_checkout_cone = 1;
 		return MODE_CONE_PATTERNS;
 	}
-	core_sparse_checkout_cone = 0;
+	the_repository->settings.sparse_checkout_cone = 0;
 	return MODE_ALL_PATTERNS;
 }
 
@@ -572,7 +572,7 @@ static void add_patterns_from_input(struct pattern_list *pl,
 				    FILE *file)
 {
 	int i;
-	if (core_sparse_checkout_cone) {
+	if (the_repository->settings.sparse_checkout_cone) {
 		struct strbuf line = STRBUF_INIT;
 
 		hashmap_init(&pl->recursive_hashmap, pl_hashmap_cmp, NULL, 0);
@@ -637,7 +637,7 @@ static void add_patterns_cone_mode(int argc, const char **argv,
 				use_stdin ? stdin : NULL);
 
 	memset(&existing, 0, sizeof(existing));
-	existing.use_cone_patterns = core_sparse_checkout_cone;
+	existing.use_cone_patterns = the_repository->settings.sparse_checkout_cone;
 
 	if (add_patterns_from_file_to_list(sparse_filename, "", 0,
 					   &existing, NULL, 0))
@@ -683,7 +683,7 @@ static int modify_pattern_list(struct strvec *args, int use_stdin,
 
 	switch (m) {
 	case ADD:
-		if (core_sparse_checkout_cone)
+		if (the_repository->settings.sparse_checkout_cone)
 			add_patterns_cone_mode(args->nr, args->v, pl, use_stdin);
 		else
 			add_patterns_literal(args->nr, args->v, pl, use_stdin);
@@ -719,7 +719,7 @@ static void sanitize_paths(struct strvec *args,
 	if (!args->nr)
 		return;
 
-	if (prefix && *prefix && core_sparse_checkout_cone) {
+	if (prefix && *prefix && the_repository->settings.sparse_checkout_cone) {
 		/*
 		 * The args are not pathspecs, so unfortunately we
 		 * cannot imitate how cmd_add() uses parse_pathspec().
@@ -736,10 +736,10 @@ static void sanitize_paths(struct strvec *args,
 	if (skip_checks)
 		return;
 
-	if (prefix && *prefix && !core_sparse_checkout_cone)
+	if (prefix && *prefix && !the_repository->settings.sparse_checkout_cone)
 		die(_("please run from the toplevel directory in non-cone mode"));
 
-	if (core_sparse_checkout_cone) {
+	if (the_repository->settings.sparse_checkout_cone) {
 		for (i = 0; i < args->nr; i++) {
 			if (args->v[i][0] == '/')
 				die(_("specify directories rather than patterns (no leading slash)"));
@@ -761,7 +761,7 @@ static void sanitize_paths(struct strvec *args,
 		if (S_ISSPARSEDIR(ce->ce_mode))
 			continue;
 
-		if (core_sparse_checkout_cone)
+		if (the_repository->settings.sparse_checkout_cone)
 			die(_("'%s' is not a directory; to treat it as a directory anyway, rerun with --skip-checks"), args->v[i]);
 		else
 			warning(_("pass a leading slash before paths such as '%s' if you want a single file (see NON-CONE PROBLEMS in the git-sparse-checkout manual)."), args->v[i]);
@@ -864,7 +864,7 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix,
 	 * non-cone mode, if nothing is specified, manually select just the
 	 * top-level directory (much as 'init' would do).
 	 */
-	if (!core_sparse_checkout_cone && !set_opts.use_stdin && argc == 0) {
+	if (!the_repository->settings.sparse_checkout_cone && !set_opts.use_stdin && argc == 0) {
 		for (int i = 0; i < default_patterns_nr; i++)
 			strvec_push(&patterns, default_patterns[i]);
 	} else {
@@ -1041,7 +1041,7 @@ static int sparse_checkout_check_rules(int argc, const char **argv, const char *
 		check_rules_opts.cone_mode = 1;
 
 	update_cone_mode(&check_rules_opts.cone_mode);
-	pl.use_cone_patterns = core_sparse_checkout_cone;
+	pl.use_cone_patterns = the_repository->settings.sparse_checkout_cone;
 	if (check_rules_opts.rules_file) {
 		fp = xfopen(check_rules_opts.rules_file, "r");
 		add_patterns_from_input(&pl, argc, argv, fp);
diff --git a/config.c b/config.c
index da76bf4fde..e13f7d8fe8 100644
--- a/config.c
+++ b/config.c
@@ -1607,11 +1607,6 @@ static int git_default_core_config(const char *var, const char *value,
 		return 0;
 	}
 
-	if (!strcmp(var, "core.sparsecheckoutcone")) {
-		core_sparse_checkout_cone = git_config_bool(var, value);
-		return 0;
-	}
-
 	if (!strcmp(var, "core.precomposeunicode")) {
 		precomposed_unicode = git_config_bool(var, value);
 		return 0;
diff --git a/dir.c b/dir.c
index 01d7574c09..5bed22a479 100644
--- a/dir.c
+++ b/dir.c
@@ -3473,7 +3473,7 @@ int get_sparse_checkout_patterns(struct pattern_list *pl)
 	int res;
 	char *sparse_filename = get_sparse_checkout_filename();
 
-	pl->use_cone_patterns = core_sparse_checkout_cone;
+	pl->use_cone_patterns = the_repository->settings.sparse_checkout_cone;
 	res = add_patterns_from_file_to_list(sparse_filename, "", 0, pl, NULL, 0);
 
 	free(sparse_filename);
diff --git a/environment.c b/environment.c
index 3a21629f86..cd5ec5c736 100644
--- a/environment.c
+++ b/environment.c
@@ -63,7 +63,6 @@ enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;
 #endif
 enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
 int grafts_keep_true_parents;
-int core_sparse_checkout_cone;
 int sparse_expect_files_outside_of_patterns;
 int merge_log_config = -1;
 int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
diff --git a/environment.h b/environment.h
index 1e1e83fff1..5642156d3d 100644
--- a/environment.h
+++ b/environment.h
@@ -159,7 +159,6 @@ extern int precomposed_unicode;
 extern int protect_hfs;
 extern int protect_ntfs;
 
-extern int core_sparse_checkout_cone;
 extern int sparse_expect_files_outside_of_patterns;
 
 enum rebase_setup_type {
diff --git a/repo-settings.c b/repo-settings.c
index c3aa92c065..505e402276 100644
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -85,6 +85,7 @@ void prepare_repo_settings(struct repository *r)
 		      r->settings.pack_use_bitmap_boundary_traversal);
 	repo_cfg_bool(r, "core.usereplacerefs", &r->settings.read_replace_refs, 1);
 	repo_cfg_bool(r, "core.sparsecheckout", &r->settings.sparse_checkout, 0);
+	repo_cfg_bool(r, "core.sparsecheckoutcone", &r->settings.sparse_checkout_cone, 0);
 
 	/*
 	 * The GIT_TEST_MULTI_PACK_INDEX variable is special in that
diff --git a/repo-settings.h b/repo-settings.h
index 95900784f1..1b43c4029c 100644
--- a/repo-settings.h
+++ b/repo-settings.h
@@ -68,7 +68,9 @@ struct repo_settings {
 	unsigned long big_file_threshold;
 
 	char *hooks_path;
+
 	int sparse_checkout;
+	int sparse_checkout_cone;
 };
 #define REPO_SETTINGS_INIT { \
 	.shared_repository = -1, \
diff --git a/sparse-index.c b/sparse-index.c
index c9e5a5efe1..3b51ea46e3 100644
--- a/sparse-index.c
+++ b/sparse-index.c
@@ -150,7 +150,7 @@ static int index_has_unmerged_entries(struct index_state *istate)
 
 int is_sparse_index_allowed(struct index_state *istate, int flags)
 {
-	if (!istate->repo->settings.sparse_checkout || !core_sparse_checkout_cone)
+	if (!istate->repo->settings.sparse_checkout || !istate->repo->settings.sparse_checkout_cone)
 		return 0;
 
 	if (!(flags & SPARSE_INDEX_MEMORY_ONLY)) {
-- 
2.49.0

[GSOC PATCH v6 3/3] environment: remove the global variable 'sparse_expect_files_outside_of_patterns'

[Ayush Chandekar]

The setting "sparse.expectFilesOutsideOfPatterns" is stored in the
global variable 'sparse_expect_files_outside_of_patterns' and allows
files marked with the `SKIP_WORKTREE` bit to be present in the worktree.

As this setting is closely related to repository, remove the global
variable and store the setting in the `struct repo_settings` along
with other sparse checkout related settings.

This also allows us to remove the definition '#define
USE_THE_REPOSITORY_VARIABLE' from the file 'sparse-index.c'.

This change is part of an ongoing effort to eliminate global variables,
improve modularity and help libify the codebase.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 config.c        | 14 --------------
 environment.c   |  1 -
 environment.h   |  2 --
 repo-settings.c |  1 +
 repo-settings.h |  1 +
 sparse-index.c  |  3 +--
 6 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/config.c b/config.c
index e13f7d8fe8..ea6de843f7 100644
--- a/config.c
+++ b/config.c
@@ -1631,17 +1631,6 @@ static int git_default_core_config(const char *var, const char *value,
 	return platform_core_config(var, value, ctx, cb);
 }
 
-static int git_default_sparse_config(const char *var, const char *value)
-{
-	if (!strcmp(var, "sparse.expectfilesoutsideofpatterns")) {
-		sparse_expect_files_outside_of_patterns = git_config_bool(var, value);
-		return 0;
-	}
-
-	/* Add other config variables here and to Documentation/config/sparse.adoc. */
-	return 0;
-}
-
 static int git_default_i18n_config(const char *var, const char *value)
 {
 	if (!strcmp(var, "i18n.commitencoding")) {
@@ -1803,9 +1792,6 @@ int git_default_config(const char *var, const char *value,
 		return 0;
 	}
 
-	if (starts_with(var, "sparse."))
-		return git_default_sparse_config(var, value);
-
 	/* Add other config variables here and to Documentation/config.adoc. */
 	return 0;
 }
diff --git a/environment.c b/environment.c
index cd5ec5c736..cf52fa9617 100644
--- a/environment.c
+++ b/environment.c
@@ -63,7 +63,6 @@ enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;
 #endif
 enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
 int grafts_keep_true_parents;
-int sparse_expect_files_outside_of_patterns;
 int merge_log_config = -1;
 int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
 unsigned long pack_size_limit_cfg;
diff --git a/environment.h b/environment.h
index 5642156d3d..aaa3025df3 100644
--- a/environment.h
+++ b/environment.h
@@ -159,8 +159,6 @@ extern int precomposed_unicode;
 extern int protect_hfs;
 extern int protect_ntfs;
 
-extern int sparse_expect_files_outside_of_patterns;
-
 enum rebase_setup_type {
 	AUTOREBASE_NEVER = 0,
 	AUTOREBASE_LOCAL,
diff --git a/repo-settings.c b/repo-settings.c
index 505e402276..5e0ba4ae23 100644
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -86,6 +86,7 @@ void prepare_repo_settings(struct repository *r)
 	repo_cfg_bool(r, "core.usereplacerefs", &r->settings.read_replace_refs, 1);
 	repo_cfg_bool(r, "core.sparsecheckout", &r->settings.sparse_checkout, 0);
 	repo_cfg_bool(r, "core.sparsecheckoutcone", &r->settings.sparse_checkout_cone, 0);
+	repo_cfg_bool(r, "sparse.expectfilesoutsideofpatterns", &r->settings.sparse_expect_files_outside_of_patterns, 0);
 
 	/*
 	 * The GIT_TEST_MULTI_PACK_INDEX variable is special in that
diff --git a/repo-settings.h b/repo-settings.h
index 1b43c4029c..695c0fd0ce 100644
--- a/repo-settings.h
+++ b/repo-settings.h
@@ -71,6 +71,7 @@ struct repo_settings {
 
 	int sparse_checkout;
 	int sparse_checkout_cone;
+	int sparse_expect_files_outside_of_patterns;
 };
 #define REPO_SETTINGS_INIT { \
 	.shared_repository = -1, \
diff --git a/sparse-index.c b/sparse-index.c
index 3b51ea46e3..552d26adc1 100644
--- a/sparse-index.c
+++ b/sparse-index.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
 #define DISABLE_SIGN_COMPARE_WARNINGS
 
 #include "git-compat-util.h"
@@ -669,7 +668,7 @@ 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 (!istate->repo->settings.sparse_checkout ||
-	    sparse_expect_files_outside_of_patterns)
+	    istate->repo->settings.sparse_expect_files_outside_of_patterns)
 		return;
 
 	if (clear_skip_worktree_from_present_files_sparse(istate)) {
-- 
2.49.0

Re: [GSOC PATCH v6 0/3] environment: remove sparse-checkout related global variables

[Junio C Hamano]

Ayush Chandekar <ayu.chandekar@gmail.com> writes:

> This patch series aims to remove global variables related to
> sparse-checkout from the global scope and to remove the definition
> '#define USE_THE_REPOSITORY_VARIABLE' from a few files.
>
> It contains three patches:
>
> 1 - Remove the global variable 'core_apply_sparse_checkout' and
> move its setting to the 'struct repo_settings'. Also remove the
> definition '#define USE_THE_REPOSITORY_VARIABLE' from
> "builtin/backfill.c".
>
> 2 - Remove the global variable 'core_sparse_checkout_cone' and
> move its setting to the 'struct repo_settings'.
>
> 3 - Remove the global variable
> 'sparse_expect_files_outside_of_patterns` and move its setting to
> 'struct repo_settings'. Also remove the definition '#define
> USE_THE_REPOSITORY_VARIABLE' from "sparse-index.c"
> --
> Discussions since v5:
>
> * For 1/3 and 2/3, Junio told me that it was concerning to put so
>   many calls to `prepare_repo_settings()` so I tried to minimize the
>   calls and made sure that there's no useless calling.

I didn't mean that the number of places is the problem.  What I
found troubling was that this is not done in any central place, so
it is hard to notice even if some random cmd_foo() failed to call
the function before doing its real work.  For example, shouldn't we
be able to, at least for built-in commands that have RUN_SETUP bit
set, centrally call prepare_repo_settings() somewhere late in
git.c:run_builtin() after we figure out what should be in
the_repository?  Now historically, setting up a repository may never
have involved opening and parsing tons of configuration files, so
such a change may be incurring extra overhead we did not have to
pay, so it needs a lot more thought than just trying to minimize the
number of calls, but some performance measurement.

> * For 3/3, Phillip told me that it broke user-facing as it will be
>   parsed quite late in the callchain and might throw an error mid
>   operation which we do not want.

So has the behaviour change caused by 3/3 been resolved?

A meta-level comment and a half.

 * Please do not use "-- " (that is a line that has dash dash and a
   single space and nothing else on it) lightly.  It is called
   signature line and often MUA pays attention to it when responding
   to a message with such a line by omitting everything after it
   (which is supposed to be your "who I am" advertisement) when
   quoting the original.  Since you had one before the "discussions
   since v5" section and the range-diff, I had to manually resurrect
   the part after the signature line while composing this message.

 * This throws everything in repo_settings, but these settings are
   inherently per repository and they are meaningful only when you
   are working with a repository.  What makes us choose to make them
   new members in the repo_settings structure, not direct members in
   the repository structure?

   Not an objection and not a suggestion to move them out of the
   repo_settings and to the repository proper.  Just wanted to hear
   the reasoning behind it (and have the rationale clearly
   documented, preferrably in the proposed log messages).

Re: [GSOC PATCH v6 0/3] environment: remove sparse-checkout related global variables

[Derrick Stolee]

On 7/23/25 6:14 PM, Junio C Hamano wrote:
> Ayush Chandekar <ayu.chandekar@gmail.com> writes:
> 
>> This patch series aims to remove global variables related to
>> sparse-checkout from the global scope and to remove the definition
>> '#define USE_THE_REPOSITORY_VARIABLE' from a few files.

Sorry that I missed early versions of this thread. It's an
interesting topic to me, but I've been distracted.

>> Discussions since v5:
>>
>> * For 1/3 and 2/3, Junio told me that it was concerning to put so
>>    many calls to `prepare_repo_settings()` so I tried to minimize the
>>    calls and made sure that there's no useless calling.
> 
> I didn't mean that the number of places is the problem.  What I
> found troubling was that this is not done in any central place, so
> it is hard to notice even if some random cmd_foo() failed to call
> the function before doing its real work.  For example, shouldn't we
> be able to, at least for built-in commands that have RUN_SETUP bit
> set, centrally call prepare_repo_settings() somewhere late in
> git.c:run_builtin() after we figure out what should be in
> the_repository?  Now historically, setting up a repository may never
> have involved opening and parsing tons of configuration files, so
> such a change may be incurring extra overhead we did not have to
> pay, so it needs a lot more thought than just trying to minimize the
> number of calls, but some performance measurement.

I think that the core issue here (and probably causing the issues
that were seen in the user-facing issues) is that the repo settings
struct was intended as a place to fill config for some one-off
"feature flags" and not to replace core functionality for a repo.

There are two ways to change the approach here to fix the problem
of needing prepare_repo_settings() everyhwere:

  1. With the idea that these sparse-checkout variables are
     critical to the functionality of the repo, they should move
     into the repository struct itself and be initialized along
     with all other values there. This changes the patches (and my
     follow-up series) significantly, but mechanically.

  2. If we are going to change the intention of the repo settings
     struct to move from "optional one-off feature flags" to
     "important information about the core behavior of a repo"
     then we should prepare_repo_settings() when initializing the
     repository struct.

My preference is (1). The only argument for (2) that I can think
of is that it is sometimes helpful to share only the settings for
a repo without sharing the whole repo. But that seems like a weak
reason right now.

>> * For 3/3, Phillip told me that it broke user-facing as it will be
>>    parsed quite late in the callchain and might throw an error mid
>>    operation which we do not want.
> 
> So has the behaviour change caused by 3/3 been resolved?

>   * This throws everything in repo_settings, but these settings are
>     inherently per repository and they are meaningful only when you
>     are working with a repository.  What makes us choose to make them
>     new members in the repo_settings structure, not direct members in
>     the repository structure?

(This is the same thought I expressed earlier in this message.)

Thanks,
-Stolee

Re: [GSOC PATCH v6 0/3] environment: remove sparse-checkout related global variables

[Junio C Hamano]

Derrick Stolee <stolee@gmail.com> writes:

> There are two ways to change the approach here to fix the problem
> of needing prepare_repo_settings() everyhwere:
>
>  1. With the idea that these sparse-checkout variables are
>     critical to the functionality of the repo, they should move
>     into the repository struct itself and be initialized along
>     with all other values there. This changes the patches (and my
>     follow-up series) significantly, but mechanically.
>
>  2. If we are going to change the intention of the repo settings
>     struct to move from "optional one-off feature flags" to
>     "important information about the core behavior of a repo"
>     then we should prepare_repo_settings() when initializing the
>     repository struct.
>
> My preference is (1). The only argument for (2) that I can think
> of is that it is sometimes helpful to share only the settings for
> a repo without sharing the whole repo. But that seems like a weak
> reason right now.

I do agree with the sentiment that being able to pass &repo.settings
to helper function makes us feel safer, but I agree that it is a
weak argument.  If we reexamine other things in repo_settings, it
may turn out that the same reasoning applies to them and we may be
better off to roll repo_settings into the repository itself (after
all, it is an embedded structure, not even a pointer in the main
structure that points at an indenendent repo_settings structure),
but that is totally outside the scope of this discussion.

Thanks.

Re: [GSOC PATCH v6 0/3] environment: remove sparse-checkout related global variables

[Ayush Chandekar]

On Thu, Jul 24, 2025 at 3:44 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Ayush Chandekar <ayu.chandekar@gmail.com> writes:
>
> >
> > * For 1/3 and 2/3, Junio told me that it was concerning to put so
> >   many calls to `prepare_repo_settings()` so I tried to minimize the
> >   calls and made sure that there's no useless calling.
>
> I didn't mean that the number of places is the problem.  What I
> found troubling was that this is not done in any central place, so
> it is hard to notice even if some random cmd_foo() failed to call
> the function before doing its real work.  For example, shouldn't we
> be able to, at least for built-in commands that have RUN_SETUP bit
> set, centrally call prepare_repo_settings() somewhere late in
> git.c:run_builtin() after we figure out what should be in
> the_repository?  Now historically, setting up a repository may never
> have involved opening and parsing tons of configuration files, so
> such a change may be incurring extra overhead we did not have to
> pay, so it needs a lot more thought than just trying to minimize the
> number of calls, but some performance measurement.
>

I was quite stumped as I don't know what the perfect solution for this
would be. I get your point that we have calls to the function all over
the place and would take some toll on the performance as well. As you
said that we can probably call the function in git.c:run_builtin() or
we can have a call to it in config.c:repo_config() so that just as the
other settings, we will have our repo_settings parsed, which were once
parsed through the same function(repo_config() or git_config()) and
since all the cmd_*() functions have a call to this, we will also be
able to call prepare_repo_settings() there itself.

> > * For 3/3, Phillip told me that it broke user-facing as it will be
> >   parsed quite late in the callchain and might throw an error mid
> >   operation which we do not want.
>
> So has the behaviour change caused by 3/3 been resolved?

Well, I am not parsing it at that place. But, I am relying on an
already existing call to prepre_repo_settings() before the function
using the setting is called repository.c:repo_read_index(). I tried to
narrow down to a cmd_foo() function so that I can shift a call to the
prepare_repo_settings() from repo_read_index() to it, but this
function is widely called and cannot be narrowed down so I had to
settle with it. I'm afraid the issue still isn't completely resolved

>
> A meta-level comment and a half.
>
>  * Please do not use "-- " (that is a line that has dash dash and a
>    single space and nothing else on it) lightly.  It is called
>    signature line and often MUA pays attention to it when responding
>    to a message with such a line by omitting everything after it
>    (which is supposed to be your "who I am" advertisement) when
>    quoting the original.  Since you had one before the "discussions
>    since v5" section and the range-diff, I had to manually resurrect
>    the part after the signature line while composing this message.
>

I am sorry for that. I will keep that in mind from next time.

>  * This throws everything in repo_settings, but these settings are
>    inherently per repository and they are meaningful only when you
>    are working with a repository.  What makes us choose to make them
>    new members in the repo_settings structure, not direct members in
>    the repository structure?
>
>    Not an objection and not a suggestion to move them out of the
>    repo_settings and to the repository proper.  Just wanted to hear
>    the reasoning behind it (and have the rationale clearly
>    documented, preferrably in the proposed log messages).

Yeah, so what I thought was that if it is a "core.foo" setting, I
would club it with other core.* settings in the struct repo_settings.
But other config settings like in the previous patch series,
"extensions.preciousObjects" or also in this series, the
"sparse.expectfilesoutsideofpatterns", I would put them in some local
context or if they're tied to a repository, I would store them in the
repository struct itself. But, as other "core.sparse_*" variables are
stored in the repo_settings, I thought it was better to store the
"sparse.expectfilesoutsideofpatterns" along with them rather than
storing it in the repository.

Thanks
Ayush

Re: [GSOC PATCH v6 0/3] environment: remove sparse-checkout related global variables

[Ayush Chandekar]

Hey Derrick,

On Thu, Jul 24, 2025 at 6:55 PM Derrick Stolee <stolee@gmail.com> wrote:
>
> On 7/23/25 6:14 PM, Junio C Hamano wrote:
> > Ayush Chandekar <ayu.chandekar@gmail.com> writes:
> >
> >> This patch series aims to remove global variables related to
> >> sparse-checkout from the global scope and to remove the definition
> >> '#define USE_THE_REPOSITORY_VARIABLE' from a few files.
>
> Sorry that I missed early versions of this thread. It's an
> interesting topic to me, but I've been distracted.
>

Thanks for joining the discussion!

> >> Discussions since v5:
> >>
> >> * For 1/3 and 2/3, Junio told me that it was concerning to put so
> >>    many calls to `prepare_repo_settings()` so I tried to minimize the
> >>    calls and made sure that there's no useless calling.
> >
> > I didn't mean that the number of places is the problem.  What I
> > found troubling was that this is not done in any central place, so
> > it is hard to notice even if some random cmd_foo() failed to call
> > the function before doing its real work.  For example, shouldn't we
> > be able to, at least for built-in commands that have RUN_SETUP bit
> > set, centrally call prepare_repo_settings() somewhere late in
> > git.c:run_builtin() after we figure out what should be in
> > the_repository?  Now historically, setting up a repository may never
> > have involved opening and parsing tons of configuration files, so
> > such a change may be incurring extra overhead we did not have to
> > pay, so it needs a lot more thought than just trying to minimize the
> > number of calls, but some performance measurement.
>
> I think that the core issue here (and probably causing the issues
> that were seen in the user-facing issues) is that the repo settings
> struct was intended as a place to fill config for some one-off
> "feature flags" and not to replace core functionality for a repo.
>

Oh, that is the complete opposite of what I had understood. I assumed
that repo_settings is used to hold some core repository-related config
settings, especially since there are already quite a few stored there,
and shifting these to the struct repository would probably clutter it.
Given that the existing configs in the struct repository are mostly
'repository_format_*' and having Patrick address that we embed the
repository_format in the repository as they were increasing[1], it let
me to think that we should try not to use the repository to store
these configs.

> There are two ways to change the approach here to fix the problem
> of needing prepare_repo_settings() everyhwere:
>
>   1. With the idea that these sparse-checkout variables are
>      critical to the functionality of the repo, they should move
>      into the repository struct itself and be initialized along
>      with all other values there. This changes the patches (and my
>      follow-up series) significantly, but mechanically.
>
>   2. If we are going to change the intention of the repo settings
>      struct to move from "optional one-off feature flags" to
>      "important information about the core behavior of a repo"
>      then we should prepare_repo_settings() when initializing the
>      repository struct.
>
> My preference is (1). The only argument for (2) that I can think
> of is that it is sometimes helpful to share only the settings for
> a repo without sharing the whole repo. But that seems like a weak
> reason right now.
>

Okay, I agree with your points. I can maybe send a new version to address this.

Do we also shift settings like index.sparse to the repository then?

> >> * For 3/3, Phillip told me that it broke user-facing as it will be
> >>    parsed quite late in the callchain and might throw an error mid
> >>    operation which we do not want.
> >
> > So has the behaviour change caused by 3/3 been resolved?
>
> >   * This throws everything in repo_settings, but these settings are
> >     inherently per repository and they are meaningful only when you
> >     are working with a repository.  What makes us choose to make them
> >     new members in the repo_settings structure, not direct members in
> >     the repository structure?
>
> (This is the same thought I expressed earlier in this message.)
>
> Thanks,
> -Stolee
>

Thanks
Ayush

Re: [GSOC PATCH v6 0/3] environment: remove sparse-checkout related global variables

[Derrick Stolee]

On 7/29/25 7:36 AM, Ayush Chandekar wrote:
> Hey Derrick,
> 
> On Thu, Jul 24, 2025 at 6:55 PM Derrick Stolee <stolee@gmail.com> wrote:
>>
>> There are two ways to change the approach here to fix the problem
>> of needing prepare_repo_settings() everyhwere:
>>
>>    1. With the idea that these sparse-checkout variables are
>>       critical to the functionality of the repo, they should move
>>       into the repository struct itself and be initialized along
>>       with all other values there. This changes the patches (and my
>>       follow-up series) significantly, but mechanically.
>>
>>    2. If we are going to change the intention of the repo settings
>>       struct to move from "optional one-off feature flags" to
>>       "important information about the core behavior of a repo"
>>       then we should prepare_repo_settings() when initializing the
>>       repository struct.
>>
>> My preference is (1). The only argument for (2) that I can think
>> of is that it is sometimes helpful to share only the settings for
>> a repo without sharing the whole repo. But that seems like a weak
>> reason right now.
>>
> 
> Okay, I agree with your points. I can maybe send a new version to address this.
> 
> Do we also shift settings like index.sparse to the repository then?

For now, it's important to focus this series on the globals being
converted. We can come back around to the ideas around removing
the settings struct and the prepare_repo_settings() method as a
separate series.

The index.sparse setting is something that is colocated in the
settings partly because of its interaction with feature.experimental
being handled in prepare_repo_settings() but also its very isolated
use. The sparse checkout globals are much more spread out across the
codebase.

Thanks,
-Stolee

Re: [GSOC PATCH v6 0/3] environment: remove sparse-checkout related global variables

[Ayush Chandekar]

On Tue, Jul 29, 2025 at 5:06 PM Ayush Chandekar <ayu.chandekar@gmail.com> wrote:
>
[snip]
>
> Oh, that is the complete opposite of what I had understood. I assumed
> that repo_settings is used to hold some core repository-related config
> settings, especially since there are already quite a few stored there,
> and shifting these to the struct repository would probably clutter it.
> Given that the existing configs in the struct repository are mostly
> 'repository_format_*' and having Patrick address that we embed the
> repository_format in the repository as they were increasing[1], it let
> me to think that we should try not to use the repository to store
> these configs.
>

It seems that I missed out on adding the reference for [1]
[1]: https://lore.kernel.org/git/aGPcJMfBCJuQLdtu@pks.im/

Re: [GSOC PATCH v6 0/3] environment: remove sparse-checkout related global variables

[Phillip Wood]

On 24/07/2025 14:25, Derrick Stolee wrote:
> 
> I think that the core issue here (and probably causing the issues
> that were seen in the user-facing issues) is that the repo settings
> struct was intended as a place to fill config for some one-off
> "feature flags" and not to replace core functionality for a repo.
> 
> There are two ways to change the approach here to fix the problem
> of needing prepare_repo_settings() everyhwere:
> 
>   1. With the idea that these sparse-checkout variables are
>      critical to the functionality of the repo, they should move
>      into the repository struct itself and be initialized along
>      with all other values there. This changes the patches (and my
>      follow-up series) significantly, but mechanically.

Patrick and I had a discussion about calling prepare_repo_settings() 
from repo_read_config() recently [1]. It turned out that does not work 
but I wonder if instead we could change git_default_config() to expect a 
repository pointer as the callback data and use that to initialize 
things. That would mean that we would not need to move code out of 
git_default_config() to remove global variables and we would retain the 
"last one wins" behavior when two or more config keys such are 
"merge.log" and "merge.summary" set the same variable. It would be 
fairly invasive though as we'd need to pass the repository pointer down 
through all the other callbacks that end up calling git_default_config().

Thanks

Phillip

[1] 
https://lore.kernel.org/git/f6479d6a-32a4-4a49-a75c-589978cb9a57@gmail.com/

>   2. If we are going to change the intention of the repo settings
>      struct to move from "optional one-off feature flags" to
>      "important information about the core behavior of a repo"
>      then we should prepare_repo_settings() when initializing the
>      repository struct.
> 
> My preference is (1). The only argument for (2) that I can think
> of is that it is sometimes helpful to share only the settings for
> a repo without sharing the whole repo. But that seems like a weak
> reason right now.
> 
>>> * For 3/3, Phillip told me that it broke user-facing as it will be
>>>    parsed quite late in the callchain and might throw an error mid
>>>    operation which we do not want.
>>
>> So has the behaviour change caused by 3/3 been resolved?
> 
>>   * This throws everything in repo_settings, but these settings are
>>     inherently per repository and they are meaningful only when you
>>     are working with a repository.  What makes us choose to make them
>>     new members in the repo_settings structure, not direct members in
>>     the repository structure?
> 
> (This is the same thought I expressed earlier in this message.)
> 
> Thanks,
> -Stolee
> 
> 

Re: [GSOC PATCH v6 0/3] environment: remove sparse-checkout related global variables

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> ... I wonder if instead we could change git_default_config() to expect
> a repository pointer as the callback data and use that to initialize
> things. That would mean that we would not need to move code out of
> git_default_config() to remove global variables and we would retain
> the "last one wins" behavior when two or more config keys such are
> "merge.log" and "merge.summary" set the same variable. It would be
> fairly invasive though as we'd need to pass the repository pointer
> down through all the other callbacks that end up calling
> git_default_config().

Sounds very painful, but it does sound like something worthwhile to
do.

Thanks.

Re: [GSOC PATCH v6 0/3] environment: remove sparse-checkout related global variables

[Ayush Chandekar]

Just an update, I'm still working on this patch series.

Thanks,
Ayush

Re: [GSOC PATCH v6 0/3] environment: remove sparse-checkout related global variables

[Derrick Stolee]

On 8/10/25 11:36 AM, Ayush Chandekar wrote:
> Just an update, I'm still working on this patch series.

Hi Ayush. Do you have an update on your progress? Perhaps there
is something you're stuck on and could use some help?

A few weeks ago, I played around with the ideas around updating
the location of these globals into the repository struct and
made this critical observation:

   It's "easy" to move the global into the_repository, but it
   becomes harder (and changes behavior) if we start referring
   to the data in each repository struct.

It may be good to separate the two things into different steps:

  1. Move the globals into the repository struct, but only set
     or read from the_repository->sparse_checkout[_cone].

  2. Replace the use of the_repository and instead refer to a
     specific repo. This may change behavior of the feature in
     the presence of submodules with different config than the
     root repo (tests before and after will be necessary).
     We'll also need to update the_repository during the very
     early config parsing but then update other repos during
     repo initialization.

Does this make sense based on your progress in this space?

Thanks,
-Stolee

Re: [GSOC PATCH v6 0/3] environment: remove sparse-checkout related global variables

[Ayush Chandekar]

On Tue, Aug 26, 2025 at 5:50 PM Derrick Stolee <stolee@gmail.com> wrote:
>
> On 8/10/25 11:36 AM, Ayush Chandekar wrote:
> > Just an update, I'm still working on this patch series.
>
> Hi Ayush. Do you have an update on your progress? Perhaps there
> is something you're stuck on and could use some help?
>

Hi Derrick, thanks for checking in!
I have made a few branches by trying out different ways: [1],[2] & [3]
I am quite close to sending the patch series now.

> A few weeks ago, I played around with the ideas around updating
> the location of these globals into the repository struct and
> made this critical observation:
>
>    It's "easy" to move the global into the_repository, but it
>    becomes harder (and changes behavior) if we start referring
>    to the data in each repository struct.
>
> It may be good to separate the two things into different steps:
>
>   1. Move the globals into the repository struct, but only set
>      or read from the_repository->sparse_checkout[_cone].
>
>   2. Replace the use of the_repository and instead refer to a
>      specific repo. This may change behavior of the feature in
>      the presence of submodules with different config than the
>      root repo (tests before and after will be necessary).
>      We'll also need to update the_repository during the very
>      early config parsing but then update other repos during
>      repo initialization.
>
> Does this make sense based on your progress in this space?
>

Yes, I was able to do the first step, the second step means that I
have to pass the repo struct to quite a few functions.

> Thanks,
> -Stolee
>

Thanks!
Ayush:)

[1]: https://github.com/ayu-ch/git/commits/sparse-checkout-6 (Stored
the variables in the 'struct repository' as int, and made a function
to read their values from the config)

[2]: https://github.com/ayu-ch/git/commits/sparse-checkout-7
(Christian suggested me that I can also store them as booleans so I
tried it.)

[3]: https://github.com/ayu-ch/git/commits/sparse-checkout-8 (Similar
to [1] but set their value as -1 when uninitialized. Tests seem to
fail though when I make this change.)

Re: [GSOC PATCH v6 0/3] environment: remove sparse-checkout related global variables

[Junio C Hamano]

Ayush Chandekar <ayu.chandekar@gmail.com> writes:

>> It may be good to separate the two things into different steps:
>>
>>   1. Move the globals into the repository struct, but only set
>>      or read from the_repository->sparse_checkout[_cone].
>>
>>   2. Replace the use of the_repository and instead refer to a
>>      specific repo. This may change behavior of the feature in
>>      the presence of submodules with different config than the
>>      root repo (tests before and after will be necessary).
>>      We'll also need to update the_repository during the very
>>      early config parsing but then update other repos during
>>      repo initialization.
>>
>> Does this make sense based on your progress in this space?
>
> Yes, I was able to do the first step, the second step means that I
> have to pass the repo struct to quite a few functions.

Ayush, what's the status of this effort?

Currently a topic by Derrick is built on top of this one, which
means it is stuck waiting for this topic to stabilize.  Should we
ask Derrick to rebuild his topic independent from this topic and let
it graduate sooner, and when you reroll this series, you'd base
yours on top of whatever the Git codebase looks like when it
happens?

Thanks.

Re: [GSOC PATCH v6 0/3] environment: remove sparse-checkout related global variables

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Ayush, what's the status of this effort?
>
> Currently a topic by Derrick is built on top of this one, which
> means it is stuck waiting for this topic to stabilize.  Should we
> ask Derrick to rebuild his topic independent from this topic and let
> it graduate sooner, and when you reroll this series, you'd base
> yours on top of whatever the Git codebase looks like when it
> happens?

I tried to (re)adjust Derrick's ds/sparse-checkout-clean topic to
build directly on top of 'master', which involved removing its first
step to make it depend on the global core_apply_sparse_checkout and
core_sparse_checkout_cone variables, which was not too bad.

The result is queued on 'seen' I just pushed out.  Derrick, could
you take a look to see if I screwed up any?  At least it seems to
pass the tests locally.

I've ejected the ac/deglobal-sparse-variables topic from 'seen' for
now, but perhaps it may want to be rebased on Derrick's series when
it is rerolled.  We'll see how fast the sparse-checkout-clean topic
can enter 'next' and graduate, hopefully soon enough.

Thanks.