[Outreachy PATCH v3 0/3] store repo specific config values in new `struct repo_config_values`

[Olamide Caleb Bello <belkid98@gmail.com>]

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 <phillip.wood123@gmail.com>
    @@ Commit message
         Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>

      ## 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 <phillip.wood123@gmail.com>
    @@ 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


-- 
2.34.1