[GSOC PATCH v5 0/3] environment: remove sparse-checkout related global variables
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.0Ayush Chandekar <ayu.chandekar@gmail.com>