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

[Ayush Chandekar <ayu.chandekar@gmail.com>]

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