store repo specific config values in new `struct repo_config_values`
Get series with b4
b4 am [email protected]Versions newest first · linked by author and title
- v7 Latest3 patches 7 repliesIn review
- v63 patches 8 repliesSuperseded
- v53 patches 6 repliesSuperseded
Earlier versions · 4
| Version | Posted | Patches | Replies | Status |
|---|---|---|---|---|
| v4 | 3 patches | 0 replies | Superseded | |
| v3 | 3 patches | 13 replies | Superseded | |
| v2 | 3 patches | 7 replies | Superseded | |
| v1 RFC | 3 patches | 2 replies | Superseded |
Changes in v7 from the cover letter
==============
- Added the code to reset the initialization of the_repository in the fuzz-commit-graph test in Patch 1.
- Changed the call to rep_config_values_init() in initialze_repository() by passing it the repo config_values_private_ struct instead of the accessor function in Patch 1.
- Modified the commit message in Patch 1
- Added extra lines between variable declarations and code
Patches in v7 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/3 | environment: stop storing `core.attributesFile` globallyattr.c environment.c environment.h +3 more | +45 | 0 replies | |
| 2/3 | environment: stop using core.sparseCheckout globallybuiltin/ builtin/ builtin/ +9 more | +46 | 3 replies | |
| 3/3 | environment: move "branch.autoSetupMerge" into `struct repo_config_values`branch.h builtin/ builtin/ +4 more | +19 | 0 replies |
Reviews trailers per patch
None of the 3 patches has a Reviewed-by or Acked-by.
| Patch | Christian Couder | Usman Akinyemi | Junio C Hamano |
|---|---|---|---|
| Series | Mentored-by | ||
| 1/3 | Mentored-by | Mentored-by | Helped-by |
| 2/3 | Mentored-by | Mentored-by | |
| 3/3 | Mentored-by | Mentored-by |
Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes. Scroll the table sideways for all 3 reviewers.
Cover letter [Outreachy PATCH v7 0/3] store repo specific config values in new `struct repo_config_values`
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.
In version v6, the fuzz-commit-graph failed due to the test being run multiples times in the same process. This makes us get a BUG due to the repository being reinitialized in the same process. But now resetting the initialization using memset before rerunning the test, the failure is not longer experienced.
1. https://lore.kernel.org/git/[email protected]/
2. https://lore.kernel.org/git/[email protected]/
3. https://lore.kernel.org/git/[email protected]
4. https://github.com/git/git/actions/runs/21633462506/job/62352191148?pr=2166
5. https://gitlab.com/gitlab-community/gitlab-org/git/-/pipelines/2303203992
Changes in v7:
==============
- Added the code to reset the initialization of the_repository in the fuzz-commit-graph test in Patch 1.
- Changed the call to rep_config_values_init() in initialze_repository() by passing it the repo config_values_private_ struct instead of the accessor function in Patch 1.
- Modified the commit message in Patch 1
- Added extra lines between variable declarations and code
Olamide Caleb Bello (3):
environment: stop storing core.attributesFile globally
environment: stop using core.sparseCheckout globally
environment: move "branch.autoSetupMerge" into `struct
repo_config_values`
attr.c | 7 ++++--- branch.h | 2 -- builtin/backfill.c | 3 ++- builtin/branch.c | 3 ++- builtin/checkout.c | 3 ++- builtin/clone.c | 4 +++- builtin/grep.c | 2 +- builtin/mv.c | 3 ++- builtin/push.c | 3 ++- builtin/sparse-checkout.c | 31 ++++++++++++++++++++----------- builtin/submodule--helper.c | 3 ++- builtin/worktree.c | 3 ++- dir.c | 4 +++- environment.c | 28 ++++++++++++++++++---------- environment.h | 17 +++++++++++++++-- oss-fuzz/fuzz-commit-graph.c | 1 + repository.c | 14 ++++++++++++++ repository.h | 7 +++++++ sparse-index.c | 7 +++++-- unpack-trees.c | 3 ++- wt-status.c | 4 +++- 21 files changed, 110 insertions(+), 42 deletions(-)
Range diff versus v6:
=====================
1: 7e3082125d ! 1: 48821a3848 environment: stop storing core.attributesFile globally
@@ Commit message
The core.attributeFile 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 it being overwritten by another repository when more than one
- Git repository run in the same Git process.
+ variable git_attributes_file. Storing this value in a global
+ variable can lead to it being overwritten by another repository when
+ more than one Git repository run in the same Git process.
Create a new struct repo_config_values to hold this value and other repository dependent values parsed by git_default_config(). This will ensure the current behaviour remains the same while also enabling the libification of Git.
- An accessor function 'repo_config_values()' is created and used to access
- the new struct member of the repository struct.
- This is to ensure that we detect if the struct repository has been
- initialized and also prevent double initialization of the repository.
-
- It is important to note that git_default_config() is a wrapper to other
- git_default_*_config() functions such as git_default_core_config().
- Therefore to access and modify this global variable,
- the change has to be made git_default_core_config().
+ An accessor function 'repo_config_values()' to ensure that we do not
+ access an uninitialized repository, or an instance of a different
+ repository than the current one.
@@ environment.h: extern int assume_unchanged;
extern int pack_compression_level; extern unsigned long pack_size_limit_cfg;
+ ## oss-fuzz/fuzz-commit-graph.c ##
+@@ oss-fuzz/fuzz-commit-graph.c: int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
+ {
+ struct commit_graph *g;
+
++ memset(the_repository, 0, sizeof(*the_repository));
+ initialize_repository(the_repository);
+
+ /*
+
## repository.c ##
@@ repository.c: static void set_default_hash_algo(struct repository *repo)
repo_set_hash_algo(repo, algo);
@@ repository.c: static void set_default_hash_algo(struct repository *repo)
+struct repo_config_values *repo_config_values(struct repository *repo) +{
++ if (repo != the_repository)
++ BUG("trying to read config from wrong repository instance");
+ if(!repo->initialized)
+ BUG("config values from uninitialized repository");
+ return &repo->config_values_private_;
@@ repository.c: static void set_default_hash_algo(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(repo));
++ repo_config_values_init(&repo->config_values_private_);
/*
* When a command runs inside a repository, it learns what
2: 8645b4f595 ! 2: 7d33f1ef0f environment: stop using core.sparseCheckout globally
@@ builtin/clone.c: static int git_sparse_checkout_init(const char *repo)
struct child_process cmd = CHILD_PROCESS_INIT;
int result = 0;
+ struct repo_config_values *cfg = repo_config_values(the_repository);
++
strvec_pushl(&cmd.args, "-C", repo, "sparse-checkout", "set", NULL);
/*
@@ dir.c: enum pattern_match_result path_matches_pattern_list(
{
- if (!core_apply_sparse_checkout)
+ struct repo_config_values *cfg = repo_config_values(the_repository);
++
+ if (!cfg->apply_sparse_checkout)
return 1;
if (istate->sparse_checkout_patterns)
@@ sparse-index.c: static int index_has_unmerged_entries(struct index_state *istate
{
- if (!core_apply_sparse_checkout || !core_sparse_checkout_cone)
+ struct repo_config_values *cfg = repo_config_values(the_repository);
++
+ if (!cfg->apply_sparse_checkout || !core_sparse_checkout_cone)
return 0;3: 60451b93a5 = 3: c3ecfa63b9 environment: move "branch.autoSetupMerge" into struct repo_config_values
Signature · 1 line
2.34.1