From: Bello Olamide Date: Tue, 20 Jan 2026 15:19:05 GMT Subject: Re: [Outreachy PATCH v3 0/3] store repo specific config values in new `struct repo_config_values` Message-ID: In-Reply-To: On Sat, 17 Jan 2026 at 22:02, Olamide Caleb Bello wrote: > > Hi Git Community, > Over the course of my ongoing internship, which focused on moving repo specific > global variables in environment.h into local scope, I have attempted to move some > variables into the struct repo-settings. > However there has been some design concerns as regards the use of > `prepare_repo_settings()` with respect to when and where to call the > function, and also the change in behaviours when the variable is lazily > loaded as discussed in [1] and [2]. > > After different deliberations, Phillip Wood proposed creating a new config > struct [3], adding it to the repository struct and passing the repo struct to > `git_default_config()` to store parsed repo specific config values per repo. > This ensures the current behaviours will be retained. > > I have experimented with this approach for some values and I would > appreciate feedbacks about this approach before we can move forward > and use it for more variables related to `git_default_config()`. > > For now, the parsed value is stored in `the_repository` in > `git_default_*_config()` and further efforts to pass the repository > parameter to `git_default_config()` as the callback parameter will > be looked into later on. > The link to the CI tests can be see in [4] > > 1. https://lore.kernel.org/git/43aaec10-2696-44c9-8728-2045b83dc5d3@gmail.com/ > 2. https://lore.kernel.org/git/a881499d-e236-4f8e-a217-b6bce69e3e3c@gmail.com/ > 3. https://lore.kernel.org/git/8899016f-eeef-404b-8da6-ff3a90e81cea@gmail.com/ > 4. https://gitlab.com/gitlab-community/gitlab-org/git/-/pipelines/2266020513 > > Changes in v3: > ============== > - Moved declaration and definition of repo_config_values_init into patch 1 > - Returned commit message in line 1 of builtin/backfill.c > > Olamide Caleb Bello (3): > environment: stop storing `core.attributesFile` globally > environment: environment: stop using core.sparseCheckout globally > environment: move "branch.autoSetupMerge" into `struct > repo_config_values` > > attr.c | 7 ++++--- > branch.h | 2 -- > builtin/backfill.c | 2 +- > builtin/branch.c | 2 +- > builtin/checkout.c | 2 +- > builtin/clone.c | 2 +- > builtin/grep.c | 2 +- > builtin/mv.c | 2 +- > builtin/push.c | 2 +- > builtin/sparse-checkout.c | 22 +++++++++++----------- > builtin/submodule--helper.c | 2 +- > builtin/worktree.c | 2 +- > dir.c | 2 +- > environment.c | 28 ++++++++++++++++++---------- > environment.h | 15 +++++++++++++-- > repository.c | 1 + > repository.h | 4 ++++ > sparse-index.c | 6 ++++-- > unpack-trees.c | 2 +- > wt-status.c | 2 +- > 20 files changed, 67 insertions(+), 42 deletions(-) > > Range diff versus v2: > ===================== > > 1: b6f8deaa40 ! 1: 1aa41da833 environment: stop storing `core.attributesFile` globally > @@ Metadata > ## Commit message ## > environment: stop storing `core.attributesFile` globally > > - The config value parsed in git_default_core_config() is loaded eagerly > + The config value is parsed in git_default_core_config(), loaded eagerly > and stored in the global variable `git_attributes_file`. > Storing this value in a global variable can lead to unexpected > behaviours when more than one Git repository run in the same Git process. > > - Move this value into a `struct repo_config_values` which holds all the > - values parsed by `git_default_config()` and can be accessed per > - repository via `git_default_config()`. This will ensure the current > - behaviour remains the same while also enabling the libification of Git. > + Create a new struct `repo_config_values` to hold this value and > + other repository dependent values parsed by `git_default_config()` and > + can be accessed per repository via `git_default_config()`. > + This will ensure the current behaviour remains the same while also > + enabling the libification of Git. > > It is important to note that `git_default_config()` is a wrapper to other > `git_default_*_config()` such as `git_default_core_config()`. > @@ environment.c: static int git_default_core_config(const char *var, const char *v > } > > if (!strcmp(var, "core.bare")) { > +@@ environment.c: int git_default_config(const char *var, const char *value, > + /* Add other config variables here and to Documentation/config.adoc. */ > + return 0; > + } > ++ > ++void repo_config_values_init(struct repo_config_values *cfg) > ++{ > ++ cfg->attributes_file_path = NULL; > ++} > > ## environment.h ## > @@ environment.h: extern const char * const local_repo_env[]; > @@ environment.h: extern const char * const local_repo_env[]; > /* > * Wrapper of getenv() that returns a strdup value. This value is kept > * in argv to be freed later. > +@@ environment.h: const char *strip_namespace(const char *namespaced_ref); > + int git_default_config(const char *, const char *, > + const struct config_context *, void *); > + > ++void repo_config_values_init(struct repo_config_values *cfg); > ++ > + /* > + * TODO: All the below state either explicitly or implicitly relies on > + * `the_repository`. We should eventually get rid of these and make the > @@ environment.h: extern int assume_unchanged; > extern int warn_on_object_refname_ambiguity; > extern char *apply_default_whitespace; > @@ environment.h: extern int assume_unchanged; > extern int pack_compression_level; > extern unsigned long pack_size_limit_cfg; > > + ## repository.c ## > +@@ repository.c: void initialize_repository(struct repository *repo) > + ALLOC_ARRAY(repo->index, 1); > + index_state_init(repo->index, repo); > + repo->check_deprecated_config = true; > ++ repo_config_values_init(&repo->config_values); > + > + /* > + * When a command runs inside a repository, it learns what > + > ## repository.h ## > @@ > > 2: 1e83c077f2 ! 2: fd95169de4 environment: environment: stop using core.sparseCheckout globally > @@ Commit message > when different Git repositories running in the same process access this > variable. > > - Move the parsed value into `struct repo_config_values` which holds all the > - values parsed by `git_default_config()` and can be accessed > - per repo via `git_default_config()`. This retains current > + Move the parsed value into `struct repo_config_values` to retains current > behaviours while achieving the repository scoped access. > > Suggested-by: Phillip Wood > @@ Commit message > Signed-off-by: Olamide Caleb Bello > > ## builtin/backfill.c ## > -@@ > --/* We need this macro to access core_apply_sparse_checkout */ > - #define USE_THE_REPOSITORY_VARIABLE > - > - #include "builtin.h" > @@ builtin/backfill.c: int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit > repo_config(repo, git_default_config, NULL); > > @@ environment.c: static int git_default_core_config(const char *var, const char *v > return 0; > } > > +@@ environment.c: int git_default_config(const char *var, const char *value, > + void repo_config_values_init(struct repo_config_values *cfg) > + { > + cfg->attributes_file_path = NULL; > ++ cfg->sparse_checkout = 0; > + } > > ## environment.h ## > @@ environment.h: struct strvec; > 3: 6e54e22ac7 ! 3: 9a411db9f8 environment: move "branch.autoSetupMerge" into `struct repo_config_values` > @@ Commit message > `git_branch_track`. This global variable can cause unexpected behaviours > when multiple Git repos run in the the same process. > > - Move this value into `struct repo_config_values` which holds all values > - parsed by `git_default_config()` and can be accessed per > - repo via `git_default_config()`. This would retain the same > + Move this value into `struct repo_config_values` to retain current > behaviours while achieving repository scoped access. > > Suggested-by: Phillip Wood > @@ environment.c: static int git_default_i18n_config(const char *var, const char *v > return 0; > } > if (!strcmp(var, "branch.autosetuprebase")) { > -@@ environment.c: int git_default_config(const char *var, const char *value, > - /* Add other config variables here and to Documentation/config.adoc. */ > - return 0; > - } > -+ > -+void repo_config_values_init(struct repo_config_values *cfg) > -+{ > +@@ environment.c: void repo_config_values_init(struct repo_config_values *cfg) > + { > + cfg->attributes_file_path = NULL; > + cfg->sparse_checkout = 0; > + cfg->git_branch_track = BRANCH_TRACK_REMOTE; > -+} > + } > > ## environment.h ## > @@ > @@ environment.h: struct repo_config_values { > }; > > /* > -@@ environment.h: const char *strip_namespace(const char *namespaced_ref); > - int git_default_config(const char *, const char *, > - const struct config_context *, void *); > - > -+void repo_config_values_init(struct repo_config_values *cfg); > -+ > - /* > - * TODO: All the below state either explicitly or implicitly relies on > - * `the_repository`. We should eventually get rid of these and make the > - > - ## repository.c ## > -@@ repository.c: void initialize_repository(struct repository *repo) > - ALLOC_ARRAY(repo->index, 1); > - index_state_init(repo->index, repo); > - repo->check_deprecated_config = true; > -+ repo_config_values_init(&repo->config_values); > - > - /* > - * When a command runs inside a repository, it learns what > > Hello Replying to this to get reviews and feedback. Thanks