From: Ayush Chandekar <ayu.chandekar@gmail•com>
To: ayu.chandekar@gmail•com
Cc: christian.couder@gmail•com, git@vger•kernel.org,
shyamthakkar001@gmail•com, phillip.wood123@gmail•com, ps@pks•im,
gitster@pobox•com, ben.knoble@gmail•com
Subject: [GSOC PATCH v6 0/3] environment: remove sparse-checkout related global variables
Date: Sat, 19 Jul 2025 05:41:25 +0530 [thread overview]
Message-ID: <cover.1752882401.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 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
next prev parent reply other threads:[~2025-07-19 0:11 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 ` [GSOC PATCH v5 0/3] environment: remove sparse-checkout related global variables Ayush Chandekar
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 ` Ayush Chandekar [this message]
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.1752882401.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=phillip.wood123@gmail$(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