public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Ayush Chandekar <ayu.chandekar@gmail•com>
To: ayu.chandekar@gmail•com
Cc: christian.couder@gmail•com, git@vger•kernel.org,
	shyamthakkar001@gmail•com, gitster@pobox•com, ps@pks•im,
	ben.knoble@gmail•com
Subject: [GSOC PATCH v5 0/3] environment: remove sparse-checkout related global variables
Date: Tue,  1 Jul 2025 00:57:45 +0530	[thread overview]
Message-ID: <cover.1751309770.git.ayu.chandekar@gmail.com> (raw)
In-Reply-To: <20250603131806.14915-1-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


  parent reply	other threads:[~2025-06-30 19:28 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-03 13:18 [GSOC PATCH] environment: move access to "core.sparsecheckout" into repo_settings Ayush Chandekar
2025-06-03 13:41 ` Patrick Steinhardt
2025-06-03 16:20   ` Ayush Chandekar
2025-06-04  2:20     ` Ben Knoble
2025-06-04  7:28       ` Patrick Steinhardt
2025-06-04 23:48       ` Ayush Chandekar
2025-06-08  0:31 ` [GSOC PATCH v2] " Ayush Chandekar
2025-06-08  6:39   ` Christian Couder
2025-06-11 17:34 ` [PATCH v3] " Ayush Chandekar
2025-06-11 21:06   ` Junio C Hamano
2025-06-11 21:33     ` Junio C Hamano
2025-06-13  6:57       ` Ayush Chandekar
2025-06-17 12:06 ` [GSOC PATCH v4 0/3] environment: remove sparse-checkout related global variables Ayush Chandekar
2025-06-17 12:06   ` [GSOC PATCH v4 1/3] environment: move access to "core.sparsecheckout" into repo_settings Ayush Chandekar
2025-06-17 16:15     ` Junio C Hamano
2025-06-17 12:06   ` [GSOC PATCH v4 2/3] environment: move access to "core.sparsecheckoutcone" " Ayush Chandekar
2025-06-17 16:29     ` Junio C Hamano
2025-06-17 12:06   ` [GSOC PATCH v4 3/3] environment: remove the global variable 'sparse_expect_files_outside_of_patterns' Ayush Chandekar
2025-06-17 16:30     ` Junio C Hamano
2025-06-30 19:27 ` Ayush Chandekar [this message]
2025-06-30 19:27   ` [GSOC PATCH v5 1/3] environment: move access to "core.sparsecheckout" into repo_settings Ayush Chandekar
2025-06-30 19:27   ` [GSOC PATCH v5 2/3] environment: move access to "core.sparsecheckoutcone" " Ayush Chandekar
2025-06-30 19:27   ` [GSOC PATCH v5 3/3] environment: remove the global variable 'sparse_expect_files_outside_of_patterns' Ayush Chandekar
2025-07-01 13:18     ` Phillip Wood
2025-07-01 23:53       ` Ayush Chandekar
2025-07-02  9:01         ` Phillip Wood
2025-07-11 19:24           ` Ayush Chandekar
2025-07-02  9:21         ` Junio C Hamano
2025-07-11 19:35           ` Ayush Chandekar
2025-06-30 21:08   ` [GSOC PATCH v5 0/3] environment: remove sparse-checkout related global variables Junio C Hamano
2025-07-09  0:18   ` Junio C Hamano
2025-07-09  1:39     ` Ayush Chandekar
2025-07-19  0:11 ` [GSOC PATCH v6 " Ayush Chandekar
2025-07-19  0:11   ` [GSOC PATCH v6 1/3] environment: move access to "core.sparsecheckout" into repo_settings Ayush Chandekar
2025-07-19  0:11   ` [GSOC PATCH v6 2/3] environment: move access to "core.sparsecheckoutcone" " Ayush Chandekar
2025-07-19  0:11   ` [GSOC PATCH v6 3/3] environment: remove the global variable 'sparse_expect_files_outside_of_patterns' Ayush Chandekar
2025-07-23 22:14   ` [GSOC PATCH v6 0/3] environment: remove sparse-checkout related global variables Junio C Hamano
2025-07-24 13:25     ` Derrick Stolee
2025-07-24 18:44       ` Junio C Hamano
2025-07-29 11:36       ` Ayush Chandekar
2025-07-29 12:19         ` Derrick Stolee
2025-07-29 12:53         ` Ayush Chandekar
2025-07-30  8:53       ` Phillip Wood
2025-07-30 15:52         ` Junio C Hamano
2025-07-26 23:55     ` Ayush Chandekar
2025-08-10 15:36   ` Ayush Chandekar
2025-08-26 12:20     ` Derrick Stolee
2025-08-27 21:31       ` Ayush Chandekar
2025-09-05 14:15         ` Junio C Hamano
2025-09-05 17:10           ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1751309770.git.ayu.chandekar@gmail.com \
    --to=ayu.chandekar@gmail$(echo .)com \
    --cc=ben.knoble@gmail$(echo .)com \
    --cc=christian.couder@gmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    --cc=ps@pks$(echo .)im \
    --cc=shyamthakkar001@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox