[GSOC PATCH v6 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 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.0Ayush Chandekar <ayu.chandekar@gmail.com>