public inbox for git@vger.kernel.org
 help / color / mirror / Atom feed
* [Outreachy PATCH v6 1/3] environment: stop storing `core.attributesFile` globally
  2026-02-03 15:42 [Outreachy PATCH v6 0/3] store repo specific config values in new `struct repo_config_values` Olamide Caleb Bello
@ 2026-02-03 15:42 ` Olamide Caleb Bello
  2026-02-04 16:39   ` Phillip Wood
  2026-02-07  1:14   ` Junio C Hamano
  0 siblings, 2 replies; 15+ messages in thread
From: Olamide Caleb Bello @ 2026-02-03 15:42 UTC (permalink / raw)
  To: git
  Cc: toon, phillip.wood123, gitster, christian.couder,
	usmanakinyemi202, kaartic.sivaraam, me, karthik.188,
	Olamide Caleb Bello

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.

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()`.

Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>
---
 attr.c        |  7 ++++---
 environment.c | 12 +++++++++---
 environment.h | 11 ++++++++++-
 repository.c  | 12 ++++++++++++
 repository.h  |  7 +++++++
 5 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/attr.c b/attr.c
index 4999b7e09d..75369547b3 100644
--- a/attr.c
+++ b/attr.c
@@ -881,10 +881,11 @@ const char *git_attr_system_file(void)
 
 const char *git_attr_global_file(void)
 {
-	if (!git_attributes_file)
-		git_attributes_file = xdg_config_home("attributes");
+	struct repo_config_values *cfg = repo_config_values(the_repository);
+	if (!cfg->attributes_file)
+		cfg->attributes_file = xdg_config_home("attributes");
 
-	return git_attributes_file;
+	return cfg->attributes_file;
 }
 
 int git_attr_system_is_enabled(void)
diff --git a/environment.c b/environment.c
index a770b5921d..4b5c701e80 100644
--- a/environment.c
+++ b/environment.c
@@ -53,7 +53,6 @@ char *git_commit_encoding;
 char *git_log_output_encoding;
 char *apply_default_whitespace;
 char *apply_default_ignorewhitespace;
-char *git_attributes_file;
 int zlib_compression_level = Z_BEST_SPEED;
 int pack_compression_level = Z_DEFAULT_COMPRESSION;
 int fsync_object_files = -1;
@@ -327,6 +326,8 @@ static enum fsync_component parse_fsync_components(const char *var, const char *
 static int git_default_core_config(const char *var, const char *value,
 				   const struct config_context *ctx, void *cb)
 {
+	struct repo_config_values *cfg = repo_config_values(the_repository);
+
 	/* This needs a better name */
 	if (!strcmp(var, "core.filemode")) {
 		trust_executable_bit = git_config_bool(var, value);
@@ -364,8 +365,8 @@ static int git_default_core_config(const char *var, const char *value,
 	}
 
 	if (!strcmp(var, "core.attributesfile")) {
-		FREE_AND_NULL(git_attributes_file);
-		return git_config_pathname(&git_attributes_file, var, value);
+		FREE_AND_NULL(cfg->attributes_file);
+		return git_config_pathname(&cfg->attributes_file, var, value);
 	}
 
 	if (!strcmp(var, "core.bare")) {
@@ -756,3 +757,8 @@ 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 = NULL;
+}
diff --git a/environment.h b/environment.h
index 51898c99cd..dfc31b794d 100644
--- a/environment.h
+++ b/environment.h
@@ -84,6 +84,14 @@ extern const char * const local_repo_env[];
 
 struct strvec;
 
+struct repository;
+struct repo_config_values {
+	/* section "core" config values */
+	char *attributes_file;
+};
+
+struct repo_config_values *repo_config_values(struct repository *repo);
+
 /*
  * Wrapper of getenv() that returns a strdup value. This value is kept
  * in argv to be freed later.
@@ -107,6 +115,8 @@ 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
@@ -152,7 +162,6 @@ extern int assume_unchanged;
 extern int warn_on_object_refname_ambiguity;
 extern char *apply_default_whitespace;
 extern char *apply_default_ignorewhitespace;
-extern char *git_attributes_file;
 extern int zlib_compression_level;
 extern int pack_compression_level;
 extern unsigned long pack_size_limit_cfg;
diff --git a/repository.c b/repository.c
index c7e75215ac..a9b727540f 100644
--- a/repository.c
+++ b/repository.c
@@ -50,13 +50,25 @@ static void set_default_hash_algo(struct repository *repo)
 	repo_set_hash_algo(repo, algo);
 }
 
+struct repo_config_values *repo_config_values(struct repository *repo)
+{
+	if(!repo->initialized)
+		BUG("config values from uninitialized repository");
+	return &repo->config_values_private_;
+}
+
 void initialize_repository(struct repository *repo)
 {
+	if (repo->initialized)
+		BUG("repository initialized already");
+	repo->initialized = true;
+
 	repo->remote_state = remote_state_new();
 	repo->parsed_objects = parsed_object_pool_new(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));
 
 	/*
 	 * When a command runs inside a repository, it learns what
diff --git a/repository.h b/repository.h
index 6063c4b846..9717e45000 100644
--- a/repository.h
+++ b/repository.h
@@ -3,6 +3,7 @@
 
 #include "strmap.h"
 #include "repo-settings.h"
+#include "environment.h"
 
 struct config_set;
 struct git_hash_algo;
@@ -148,6 +149,9 @@ struct repository {
 	/* Repository's compatibility hash algorithm. */
 	const struct git_hash_algo *compat_hash_algo;
 
+	/* Repository's config values parsed by git_default_config() */
+	struct repo_config_values config_values_private_;
+
 	/* Repository's reference storage format, as serialized on disk. */
 	enum ref_storage_format ref_storage_format;
 
@@ -171,6 +175,9 @@ struct repository {
 
 	/* Should repo_config() check for deprecated settings */
 	bool check_deprecated_config;
+
+	/* Has this repository instance been initialized? */
+	bool initialized;
 };
 
 #ifdef USE_THE_REPOSITORY_VARIABLE
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [Outreachy PATCH v6 1/3] environment: stop storing `core.attributesFile` globally
  2026-02-03 15:42 ` [Outreachy PATCH v6 1/3] environment: stop storing `core.attributesFile` globally Olamide Caleb Bello
@ 2026-02-04 16:39   ` Phillip Wood
  2026-02-09  8:47     ` Bello Olamide
  2026-02-07  1:14   ` Junio C Hamano
  1 sibling, 1 reply; 15+ messages in thread
From: Phillip Wood @ 2026-02-04 16:39 UTC (permalink / raw)
  To: Olamide Caleb Bello, git
  Cc: toon, gitster, christian.couder, usmanakinyemi202,
	kaartic.sivaraam, me, karthik.188

On 03/02/2026 15:42, Olamide Caleb Bello wrote:
> 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.
> 
> 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.

Sounds sensible. This paragraph could be reflowed.

> 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()`.

I'm not sure what this paragraph is saying with regard to the changes in 
this patch.

> --- a/environment.c
> +++ b/environment.c
> @@ -756,3 +757,8 @@ 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 = NULL;
> +}

Should we be free()ing cfg->attributes_file when the repository instance 
is free()d? At the moment we're using "the_repository" which points to a 
static instance so it does not make any practical difference but once we 
start storing the config per-repository instance we will need to free 
the config when the repository instance is free()d.

> diff --git a/repository.c b/repository.c
> index c7e75215ac..a9b727540f 100644
> --- a/repository.c
> +++ b/repository.c
> @@ -50,13 +50,25 @@ static void set_default_hash_algo(struct repository *repo)
>   	repo_set_hash_algo(repo, algo);
>   }
>   
> +struct repo_config_values *repo_config_values(struct repository *repo)
> +{
> +	if(!repo->initialized)
> +		BUG("config values from uninitialized repository");

This check and the one in initialize_repository() below assume that the 
repository instance is zeroed out when it is created, that's a 
reasonable requirement but we should probably document it as our other 
data structures tend not to require that they're zeroed out before they 
are initialized. For example

	struct strbuf buf;
	strbuf_init(&buf, 0);

is perfectly fine as strbuf_init() does not assume the instance passed 
to it has been zeroed out.

As we only support retrieving values from "the_repository" at the moment 
we should perhaps add

	if (repo != the_repository)
		BUG("trying to read config from wrong repository instance");

Everything else looks fine to me

Thanks

Phillip

> +	return &repo->config_values_private_;
> +}
> +
>   void initialize_repository(struct repository *repo)
>   {
> +	if (repo->initialized)
> +		BUG("repository initialized already");
> +	repo->initialized = true;
> +
>   	repo->remote_state = remote_state_new();
>   	repo->parsed_objects = parsed_object_pool_new(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));
>   
>   	/*
>   	 * When a command runs inside a repository, it learns what
> diff --git a/repository.h b/repository.h
> index 6063c4b846..9717e45000 100644
> --- a/repository.h
> +++ b/repository.h
> @@ -3,6 +3,7 @@
>   
>   #include "strmap.h"
>   #include "repo-settings.h"
> +#include "environment.h"
>   
>   struct config_set;
>   struct git_hash_algo;
> @@ -148,6 +149,9 @@ struct repository {
>   	/* Repository's compatibility hash algorithm. */
>   	const struct git_hash_algo *compat_hash_algo;
>   
> +	/* Repository's config values parsed by git_default_config() */
> +	struct repo_config_values config_values_private_;
> +
>   	/* Repository's reference storage format, as serialized on disk. */
>   	enum ref_storage_format ref_storage_format;
>   
> @@ -171,6 +175,9 @@ struct repository {
>   
>   	/* Should repo_config() check for deprecated settings */
>   	bool check_deprecated_config;
> +
> +	/* Has this repository instance been initialized? */
> +	bool initialized;
>   };
>   
>   #ifdef USE_THE_REPOSITORY_VARIABLE


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Outreachy PATCH v6 1/3] environment: stop storing `core.attributesFile` globally
  2026-02-03 15:42 ` [Outreachy PATCH v6 1/3] environment: stop storing `core.attributesFile` globally Olamide Caleb Bello
  2026-02-04 16:39   ` Phillip Wood
@ 2026-02-07  1:14   ` Junio C Hamano
  2026-02-08 11:14     ` Phillip Wood
  1 sibling, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2026-02-07  1:14 UTC (permalink / raw)
  To: Olamide Caleb Bello
  Cc: git, toon, phillip.wood123, christian.couder, usmanakinyemi202,
	kaartic.sivaraam, me, karthik.188

Olamide Caleb Bello <belkid98@gmail.com> writes:

> Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>
> ---
>  attr.c        |  7 ++++---
>  environment.c | 12 +++++++++---
>  environment.h | 11 ++++++++++-
>  repository.c  | 12 ++++++++++++
>  repository.h  |  7 +++++++
>  5 files changed, 42 insertions(+), 7 deletions(-)

I bisected our recent CI failures that break fuzz smoke test down to
this change.

$ make -j32 \
            NO_CURL=NoThanks \
            CC=clang \
            FUZZ_CXX=clang++ \
            CFLAGS="-fsanitize=fuzzer-no-link,address" \
            LIB_FUZZING_ENGINE="-fsanitize=fuzzer,address" \
            fuzz-all >/dev/null &&
$ oss-fuzz/fuzz-commit-graph -verbosity=0 -runs=1

INFO: Running with entropic power schedule (0xFF, 100).
INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes
INFO: A corpus is not provided, starting from an empty corpus
BUG: repository.c:63: repository initialized already
==2050473== ERROR: libFuzzer: deadly signal
    #0 0x56169aaf4065 in __sanitizer_print_stack_trace (/home/gitster/git.git/oss-fuzz/fuzz-commit-graph+0x32a065) (BuildId: ec362419d512b5bd707ae18eef56a6a12a18fc92)
    #1 0x56169aa4835c in fuzzer::PrintStackTrace() (/home/gitster/git.git/oss-fuzz/fuzz-commit-graph+0x27e35c) (BuildId: ec362419d512b5bd707ae18eef56a6a12a18fc92)
    #2 0x56169aa2d2d7 in fuzzer::Fuzzer::CrashCallback() (/home/gitster/git.git/oss-fuzz/fuzz-commit-graph+0x2632d7) (BuildId: ec362419d512b5bd707ae18eef56a6a12a18fc92)
    #3 0x7fb4c6f59def  (/lib/x86_64-linux-gnu/libc.so.6+0x3fdef) (BuildId: 61e1dea1f540b3b4b4d8ec76716e409cec096ece)
    #4 0x7fb4c6fae95b in __pthread_kill_implementation nptl/pthread_kill.c:43:17
    #5 0x7fb4c6f59cc1 in raise signal/../sysdeps/posix/raise.c:26:13
    #6 0x7fb4c6f424ab in abort stdlib/abort.c:73:3
    #7 0x56169ae91316 in BUG_vfl usage.c
    #8 0x56169ae8f527 in BUG_fl (/home/gitster/git.git/oss-fuzz/fuzz-commit-graph+0x6c5527) (BuildId: ec362419d512b5bd707ae18eef56a6a12a18fc92)
    #9 0x56169ad79d07 in initialize_repository (/home/gitster/git.git/oss-fuzz/fuzz-commit-graph+0x5afd07) (BuildId: ec362419d512b5bd707ae18eef56a6a12a18fc92)
    #10 0x56169ab2c4e2 in LLVMFuzzerTestOneInput (/home/gitster/git.git/oss-fuzz/fuzz-commit-graph+0x3624e2) (BuildId: ec362419d512b5bd707ae18eef56a6a12a18fc92)
    #11 0x56169aa2e9da in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/home/gitster/git.git/oss-fuzz/fuzz-commit-graph+0x2649da) (BuildId: ec362419d512b5bd707ae18eef56a6a12a18fc92)
    #12 0x56169aa2dfe9 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) (/home/gitster/git.git/oss-fuzz/fuzz-commit-graph+0x263fe9) (BuildId: ec362419d512b5bd707ae18eef56a6a12a18fc92)
    #13 0x56169aa2fdaf in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/home/gitster/git.git/oss-fuzz/fuzz-commit-graph+0x265daf) (BuildId: ec362419d512b5bd707ae18eef56a6a12a18fc92)
    #14 0x56169aa30390 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/home/gitster/git.git/oss-fuzz/fuzz-commit-graph+0x266390) (BuildId: ec362419d512b5bd707ae18eef56a6a12a18fc92)
    #15 0x56169aa1cb65 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/home/gitster/git.git/oss-fuzz/fuzz-commit-graph+0x252b65) (BuildId: ec362419d512b5bd707ae18eef56a6a12a18fc92)
    #16 0x56169aa48ec6 in main (/home/gitster/git.git/oss-fuzz/fuzz-commit-graph+0x27eec6) (BuildId: ec362419d512b5bd707ae18eef56a6a12a18fc92)
    #17 0x7fb4c6f43ca7 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #18 0x7fb4c6f43d64 in __libc_start_main csu/../csu/libc-start.c:360:3
    #19 0x56169aa10ec0 in _start (/home/gitster/git.git/oss-fuzz/fuzz-commit-graph+0x246ec0) (BuildId: ec362419d512b5bd707ae18eef56a6a12a18fc92)

NOTE: libFuzzer has rudimentary signal handlers.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Outreachy PATCH v6 1/3] environment: stop storing `core.attributesFile` globally
  2026-02-07  1:14   ` Junio C Hamano
@ 2026-02-08 11:14     ` Phillip Wood
  2026-02-09  8:54       ` Bello Olamide
  2026-02-10  8:40       ` Bello Olamide
  0 siblings, 2 replies; 15+ messages in thread
From: Phillip Wood @ 2026-02-08 11:14 UTC (permalink / raw)
  To: Junio C Hamano, Olamide Caleb Bello
  Cc: git, toon, christian.couder, usmanakinyemi202, kaartic.sivaraam,
	me, karthik.188



On 07/02/2026 01:14, Junio C Hamano wrote:
> Olamide Caleb Bello <belkid98@gmail.com> writes:
> 
>> Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
>> Mentored-by: Christian Couder <christian.couder@gmail.com>
>> Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
>> Helped-by: Junio C Hamano <gitster@pobox.com>
>> Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>
>> ---
>>   attr.c        |  7 ++++---
>>   environment.c | 12 +++++++++---
>>   environment.h | 11 ++++++++++-
>>   repository.c  | 12 ++++++++++++
>>   repository.h  |  7 +++++++
>>   5 files changed, 42 insertions(+), 7 deletions(-)
> 
> I bisected our recent CI failures that break fuzz smoke test down to
> this change.

The documentation for the LibFuzzer [1] notes

     * The fuzzing engine will execute the fuzz target many times with
       different inputs in the same process.

and the first thing that the callback in oss-fuzz/fuzz-commit-graph.c 
does is

	initialize_repository(the_repository);

so I think the problem is that the assumption that a process will only 
initialize "the_repository" once is incompatible with the way LibFuzzer 
works. Maybe we should add

	memset(the_repository, 0, sizeof(*the_repository));

before the call in initialize_repository()?

Thanks

Phillip

[1] https://llvm.org/docs/LibFuzzer.html

> $ make -j32 \
>              NO_CURL=NoThanks \
>              CC=clang \
>              FUZZ_CXX=clang++ \
>              CFLAGS="-fsanitize=fuzzer-no-link,address" \
>              LIB_FUZZING_ENGINE="-fsanitize=fuzzer,address" \
>              fuzz-all >/dev/null &&
> $ oss-fuzz/fuzz-commit-graph -verbosity=0 -runs=1
> 
> INFO: Running with entropic power schedule (0xFF, 100).
> INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes
> INFO: A corpus is not provided, starting from an empty corpus
> BUG: repository.c:63: repository initialized already
> ==2050473== ERROR: libFuzzer: deadly signal
>      #0 0x56169aaf4065 in __sanitizer_print_stack_trace (/home/gitster/git.git/oss-fuzz/fuzz-commit-graph+0x32a065) (BuildId: ec362419d512b5bd707ae18eef56a6a12a18fc92)
>      #1 0x56169aa4835c in fuzzer::PrintStackTrace() (/home/gitster/git.git/oss-fuzz/fuzz-commit-graph+0x27e35c) (BuildId: ec362419d512b5bd707ae18eef56a6a12a18fc92)
>      #2 0x56169aa2d2d7 in fuzzer::Fuzzer::CrashCallback() (/home/gitster/git.git/oss-fuzz/fuzz-commit-graph+0x2632d7) (BuildId: ec362419d512b5bd707ae18eef56a6a12a18fc92)
>      #3 0x7fb4c6f59def  (/lib/x86_64-linux-gnu/libc.so.6+0x3fdef) (BuildId: 61e1dea1f540b3b4b4d8ec76716e409cec096ece)
>      #4 0x7fb4c6fae95b in __pthread_kill_implementation nptl/pthread_kill.c:43:17
>      #5 0x7fb4c6f59cc1 in raise signal/../sysdeps/posix/raise.c:26:13
>      #6 0x7fb4c6f424ab in abort stdlib/abort.c:73:3
>      #7 0x56169ae91316 in BUG_vfl usage.c
>      #8 0x56169ae8f527 in BUG_fl (/home/gitster/git.git/oss-fuzz/fuzz-commit-graph+0x6c5527) (BuildId: ec362419d512b5bd707ae18eef56a6a12a18fc92)
>      #9 0x56169ad79d07 in initialize_repository (/home/gitster/git.git/oss-fuzz/fuzz-commit-graph+0x5afd07) (BuildId: ec362419d512b5bd707ae18eef56a6a12a18fc92)
>      #10 0x56169ab2c4e2 in LLVMFuzzerTestOneInput (/home/gitster/git.git/oss-fuzz/fuzz-commit-graph+0x3624e2) (BuildId: ec362419d512b5bd707ae18eef56a6a12a18fc92)
>      #11 0x56169aa2e9da in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/home/gitster/git.git/oss-fuzz/fuzz-commit-graph+0x2649da) (BuildId: ec362419d512b5bd707ae18eef56a6a12a18fc92)
>      #12 0x56169aa2dfe9 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) (/home/gitster/git.git/oss-fuzz/fuzz-commit-graph+0x263fe9) (BuildId: ec362419d512b5bd707ae18eef56a6a12a18fc92)
>      #13 0x56169aa2fdaf in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/home/gitster/git.git/oss-fuzz/fuzz-commit-graph+0x265daf) (BuildId: ec362419d512b5bd707ae18eef56a6a12a18fc92)
>      #14 0x56169aa30390 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/home/gitster/git.git/oss-fuzz/fuzz-commit-graph+0x266390) (BuildId: ec362419d512b5bd707ae18eef56a6a12a18fc92)
>      #15 0x56169aa1cb65 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/home/gitster/git.git/oss-fuzz/fuzz-commit-graph+0x252b65) (BuildId: ec362419d512b5bd707ae18eef56a6a12a18fc92)
>      #16 0x56169aa48ec6 in main (/home/gitster/git.git/oss-fuzz/fuzz-commit-graph+0x27eec6) (BuildId: ec362419d512b5bd707ae18eef56a6a12a18fc92)
>      #17 0x7fb4c6f43ca7 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
>      #18 0x7fb4c6f43d64 in __libc_start_main csu/../csu/libc-start.c:360:3
>      #19 0x56169aa10ec0 in _start (/home/gitster/git.git/oss-fuzz/fuzz-commit-graph+0x246ec0) (BuildId: ec362419d512b5bd707ae18eef56a6a12a18fc92)
> 
> NOTE: libFuzzer has rudimentary signal handlers.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Outreachy PATCH v6 1/3] environment: stop storing `core.attributesFile` globally
  2026-02-04 16:39   ` Phillip Wood
@ 2026-02-09  8:47     ` Bello Olamide
  0 siblings, 0 replies; 15+ messages in thread
From: Bello Olamide @ 2026-02-09  8:47 UTC (permalink / raw)
  To: phillip.wood
  Cc: git, toon, gitster, christian.couder, usmanakinyemi202,
	kaartic.sivaraam, me, karthik.188

On Wed, 4 Feb 2026 at 17:39, Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> On 03/02/2026 15:42, Olamide Caleb Bello wrote:
> > 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.
> >
> > 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.
>
> Sounds sensible. This paragraph could be reflowed.

Okay

>
> > 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()`.
>
> I'm not sure what this paragraph is saying with regard to the changes in
> this patch.

Okay I will fix it.

>
> > --- a/environment.c
> > +++ b/environment.c
> > @@ -756,3 +757,8 @@ 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 = NULL;
> > +}
>
> Should we be free()ing cfg->attributes_file when the repository instance
> is free()d? At the moment we're using "the_repository" which points to a
> static instance so it does not make any practical difference but once we
> start storing the config per-repository instance we will need to free
> the config when the repository instance is free()d.

Okay I will keep this in mind.

>
> > diff --git a/repository.c b/repository.c
> > index c7e75215ac..a9b727540f 100644
> > --- a/repository.c
> > +++ b/repository.c
> > @@ -50,13 +50,25 @@ static void set_default_hash_algo(struct repository *repo)
> >       repo_set_hash_algo(repo, algo);
> >   }
> >
> > +struct repo_config_values *repo_config_values(struct repository *repo)
> > +{
> > +     if(!repo->initialized)
> > +             BUG("config values from uninitialized repository");
>
> This check and the one in initialize_repository() below assume that the
> repository instance is zeroed out when it is created, that's a
> reasonable requirement but we should probably document it as our other
> data structures tend not to require that they're zeroed out before they
> are initialized. For example
>
>         struct strbuf buf;
>         strbuf_init(&buf, 0);
>
> is perfectly fine as strbuf_init() does not assume the instance passed
> to it has been zeroed out.
>
> As we only support retrieving values from "the_repository" at the moment
> we should perhaps add
>
>         if (repo != the_repository)
>                 BUG("trying to read config from wrong repository instance");

Okay noted

>
> Everything else looks fine to me
>

Thanks

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Outreachy PATCH v6 1/3] environment: stop storing `core.attributesFile` globally
  2026-02-08 11:14     ` Phillip Wood
@ 2026-02-09  8:54       ` Bello Olamide
  2026-02-10  8:40       ` Bello Olamide
  1 sibling, 0 replies; 15+ messages in thread
From: Bello Olamide @ 2026-02-09  8:54 UTC (permalink / raw)
  To: phillip.wood
  Cc: Junio C Hamano, git, toon, christian.couder, usmanakinyemi202,
	kaartic.sivaraam, me, karthik.188

On Sun, 8 Feb 2026 at 12:14, Phillip Wood <phillip.wood123@gmail.com> wrote:
>
>
>
> On 07/02/2026 01:14, Junio C Hamano wrote:
> > Olamide Caleb Bello <belkid98@gmail.com> writes:
> >
> >> Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
> >> Mentored-by: Christian Couder <christian.couder@gmail.com>
> >> Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
> >> Helped-by: Junio C Hamano <gitster@pobox.com>
> >> Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>
> >> ---
> >>   attr.c        |  7 ++++---
> >>   environment.c | 12 +++++++++---
> >>   environment.h | 11 ++++++++++-
> >>   repository.c  | 12 ++++++++++++
> >>   repository.h  |  7 +++++++
> >>   5 files changed, 42 insertions(+), 7 deletions(-)
> >
> > I bisected our recent CI failures that break fuzz smoke test down to
> > this change.
>
> The documentation for the LibFuzzer [1] notes
>
>      * The fuzzing engine will execute the fuzz target many times with
>        different inputs in the same process.
>
> and the first thing that the callback in oss-fuzz/fuzz-commit-graph.c
> does is
>
>         initialize_repository(the_repository);
>
> so I think the problem is that the assumption that a process will only
> initialize "the_repository" once is incompatible with the way LibFuzzer
> works. Maybe we should add
>
>         memset(the_repository, 0, sizeof(*the_repository));
>
> before the call in initialize_repository()?
>
> Thanks
>
> Phillip
>
> [1] https://llvm.org/docs/LibFuzzer.html
>
Thank you Phillip, I will try this.
I noticed something like this.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Outreachy PATCH v6 1/3] environment: stop storing `core.attributesFile` globally
  2026-02-08 11:14     ` Phillip Wood
  2026-02-09  8:54       ` Bello Olamide
@ 2026-02-10  8:40       ` Bello Olamide
  1 sibling, 0 replies; 15+ messages in thread
From: Bello Olamide @ 2026-02-10  8:40 UTC (permalink / raw)
  To: phillip.wood
  Cc: Junio C Hamano, git, toon, christian.couder, usmanakinyemi202,
	kaartic.sivaraam, me, karthik.188

On Sun, 8 Feb 2026 at 12:14, Phillip Wood <phillip.wood123@gmail.com> wrote:
>
>
>
> On 07/02/2026 01:14, Junio C Hamano wrote:
> > Olamide Caleb Bello <belkid98@gmail.com> writes:
> >
> >> Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
> >> Mentored-by: Christian Couder <christian.couder@gmail.com>
> >> Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
> >> Helped-by: Junio C Hamano <gitster@pobox.com>
> >> Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>
> >> ---
> >>   attr.c        |  7 ++++---
> >>   environment.c | 12 +++++++++---
> >>   environment.h | 11 ++++++++++-
> >>   repository.c  | 12 ++++++++++++
> >>   repository.h  |  7 +++++++
> >>   5 files changed, 42 insertions(+), 7 deletions(-)
> >
> > I bisected our recent CI failures that break fuzz smoke test down to
> > this change.
>
> The documentation for the LibFuzzer [1] notes
>
>      * The fuzzing engine will execute the fuzz target many times with
>        different inputs in the same process.
>
> and the first thing that the callback in oss-fuzz/fuzz-commit-graph.c
> does is
>
>         initialize_repository(the_repository);
>
> so I think the problem is that the assumption that a process will only
> initialize "the_repository" once is incompatible with the way LibFuzzer
> works. Maybe we should add
>
>         memset(the_repository, 0, sizeof(*the_repository));
>
> before the call in initialize_repository()?
>
> Thanks
>
> Phillip
>
> [1] https://llvm.org/docs/LibFuzzer.html
>

Hello Phillip,
thank you for your reviews and assistance so far.

So I moved the code
 memset(the_repository, 0, sizeof(*the_repository))
into the fuzz-commit-graph.c Fuzzer test before the call to
initialize_repository().
This made the fuzzer smoke tests pass.

But the line below,
`if (repo != the_repository)
    BUG("trying to read config from wrong repository instance")`,
in the repo_config_values() accessor function
to make sure we are reading the config_values only for the_repository ,
makes many tests fail.

I believe this is because repo_init() calls initialize_repository() and
repo_init() is also called in repo_submodule_init() when creating a
subrepo from a super project, and this subrepo passed to repo_init() is
not the_repository.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Outreachy PATCH v6 1/3] environment: stop storing `core.attributesFile` globally
@ 2026-02-10 10:17 Bello Caleb Olamide
  2026-02-10 15:07 ` Phillip Wood
  2026-02-11  9:31 ` Phillip Wood
  0 siblings, 2 replies; 15+ messages in thread
From: Bello Caleb Olamide @ 2026-02-10 10:17 UTC (permalink / raw)
  To: Bello Olamide
  Cc: Junio C Hamano, git, toon, christian.couder, usmanakinyemi202,
	kaartic.sivaraam, me, karthik.188, phillip.wood

This is how I implemented the suggestion

diff --git a/oss-fuzz/fuzz-commit-graph.c b/oss-fuzz/fuzz-commit-graph.c
index fb8b8787a4..59bbb849d1 100644
--- a/oss-fuzz/fuzz-commit-graph.c
+++ b/oss-fuzz/fuzz-commit-graph.c
@@ -10,6 +10,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
 {
 	struct commit_graph *g;
 
+	memset(the_repository, 0, sizeof(*the_repository));
 	initialize_repository(the_repository);
 
 	/*
diff --git a/repository.c b/repository.c
index c7e75215ac..0af40b016e 100644
--- a/repository.c
+++ b/repository.c
@@ -50,13 +50,27 @@ static void set_default_hash_algo(struct repository *repo)
 	repo_set_hash_algo(repo, algo);
 }
 
+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_;
+}
+
 void initialize_repository(struct repository *repo)
 {
+	if (repo->initialized)
+		BUG("repository initialized already");
+	repo->initialized = true;
+
 	repo->remote_state = remote_state_new();
 	repo->parsed_objects = parsed_object_pool_new(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));
 
 	/*
 	 * When a command runs inside a repository, it learns what

Some of the tests that fail are related to the submodule and a
couple output is shown below

./t7412-submodule-absorbgitdirs.sh  -i -v
...
Initialized empty Git repository in /home/ubuntu/Code/open_source/git/t/trash directory.t7412-submodule-absorbgitdirs/sub1/.git/
[master (root-commit) 50e526b] first
 Author: A U Thor <author@example.com>
 1 file changed, 1 insertion(+)
 create mode 100644 first.t
BUG: repository.c:56: trying to read config from wrong repository instance
Aborted (core dumped)
not ok 1 - setup a real submodule
#
# cwd="$(pwd)" &&
# git init sub1 &&
# test_commit -C sub1 first &&
# git submodule add ./sub1 &&
# test_tick &&
# git commit -m superproject
#
1..1

./t4027-diff-submodule.sh  -i -v
...
Initialized empty Git repository in /home/ubuntu/Code/open_source/git/t/trash directory.t4027-diff-submodule/sub/.git/
[master (root-commit) 4431e0b] submodule
 Author: A U Thor <author@example.com>
 1 file changed, 1 insertion(+)
 create mode 100644 world
BUG: repository.c:56: trying to read config from wrong repository instance
Aborted (core dumped)
not ok 1 - setup
#
# test_tick &&
# test_create_repo sub &&
# (
# cd sub &&
# echo hello >world &&
# git add world &&
# git commit -m submodule
# ) &&
#
# test_tick &&
# echo frotz >nitfol &&
# git add nitfol sub &&
# git commit -m superproject &&
#
# (
# cd sub &&
# echo goodbye >world &&
# git add world &&
# git commit -m "submodule #2"
# ) &&
#
# git -C sub rev-list HEAD >revs &&
# set x $(cat revs) &&
# echo ":160000 160000 $3 $ZERO_OID M sub" >expect &&
# subtip=$3 subprev=$2
#
1..1

Thanks

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [Outreachy PATCH v6 1/3] environment: stop storing `core.attributesFile` globally
  2026-02-10 10:17 [Outreachy PATCH v6 1/3] environment: stop storing `core.attributesFile` globally Bello Caleb Olamide
@ 2026-02-10 15:07 ` Phillip Wood
  2026-02-11  8:05   ` Bello Olamide
  2026-02-11  9:31 ` Phillip Wood
  1 sibling, 1 reply; 15+ messages in thread
From: Phillip Wood @ 2026-02-10 15:07 UTC (permalink / raw)
  To: Bello Caleb Olamide
  Cc: Junio C Hamano, git, toon, christian.couder, usmanakinyemi202,
	kaartic.sivaraam, me, karthik.188, phillip.wood

On 10/02/2026 10:17, Bello Caleb Olamide wrote:

> Initialized empty Git repository in /home/ubuntu/Code/open_source/git/t/trash directory.t4027-diff-submodule/sub/.git/
> [master (root-commit) 4431e0b] submodule
>   Author: A U Thor <author@example.com>
>   1 file changed, 1 insertion(+)
>   create mode 100644 world
> BUG: repository.c:56: trying to read config from wrong repository instance
> Aborted (core dumped)

What does the backtrace show if you load the coredump into gdb? If 
you're using systemd you should be able to run

     coredumpctl gdb

to start gdb on the last coredump (you can list them with "coredumpctl 
list" if you need to select a different one) and then you can run

     bt full

in gdb to get a backtrace.

If you have an actual coredump file then you can just run "gdb 
path/to/coredump"

Thanks

Phillip

> not ok 1 - setup
> #
> # test_tick &&
> # test_create_repo sub &&
> # (
> # cd sub &&
> # echo hello >world &&
> # git add world &&
> # git commit -m submodule
> # ) &&
> #
> # test_tick &&
> # echo frotz >nitfol &&
> # git add nitfol sub &&
> # git commit -m superproject &&
> #
> # (
> # cd sub &&
> # echo goodbye >world &&
> # git add world &&
> # git commit -m "submodule #2"
> # ) &&
> #
> # git -C sub rev-list HEAD >revs &&
> # set x $(cat revs) &&
> # echo ":160000 160000 $3 $ZERO_OID M sub" >expect &&
> # subtip=$3 subprev=$2
> #
> 1..1
> 
> Thanks
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Outreachy PATCH v6 1/3] environment: stop storing `core.attributesFile` globally
  2026-02-10 15:07 ` Phillip Wood
@ 2026-02-11  8:05   ` Bello Olamide
  0 siblings, 0 replies; 15+ messages in thread
From: Bello Olamide @ 2026-02-11  8:05 UTC (permalink / raw)
  To: phillip.wood
  Cc: Junio C Hamano, git, toon, christian.couder, usmanakinyemi202,
	kaartic.sivaraam, me, karthik.188

On Tue, 10 Feb 2026 at 16:07, Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> On 10/02/2026 10:17, Bello Caleb Olamide wrote:
>
> > Initialized empty Git repository in /home/ubuntu/Code/open_source/git/t/trash directory.t4027-diff-submodule/sub/.git/
> > [master (root-commit) 4431e0b] submodule
> >   Author: A U Thor <author@example.com>
> >   1 file changed, 1 insertion(+)
> >   create mode 100644 world
> > BUG: repository.c:56: trying to read config from wrong repository instance
> > Aborted (core dumped)
>
> What does the backtrace show if you load the coredump into gdb? If
> you're using systemd you should be able to run
>
>      coredumpctl gdb
>
> to start gdb on the last coredump (you can list them with "coredumpctl
> list" if you need to select a different one) and then you can run
>
>      bt full
>
> in gdb to get a backtrace.
>
> If you have an actual coredump file then you can just run "gdb
> path/to/coredump"
>

Thank you Phillip.
I have been able to generate the backtrace

coredumpctl gdb
           PID: 43422 (git)
           UID: 1000 (ubuntu)
           GID: 1000 (ubuntu)
        Signal: 6 (ABRT)
     Timestamp: Wed 2026-02-11 08:43:11 WAT (15s ago)
  Command Line: git submodule--helper add -- ./S
    Executable: /home/ubuntu/Code/open_source/git/git
 Control Group:
/user.slice/user-1000.slice/user@1000.service/app.slice/app-org.gnome.Terminal.slice/vte-spawn-29116a64-3a3d-4a87-b321-1dd9a0e862c7.scope
          Unit: user@1000.service
     User Unit: vte-spawn-29116a64-3a3d-4a87-b321-1dd9a0e862c7.scope
         Slice: user-1000.slice
     Owner UID: 1000 (ubuntu)
       Boot ID: a3430056a91d43f49b602796f4eafc8c
    Machine ID: 4fb1781ac9f64d5cad47e77f1ed4f268
      Hostname: ubuntu
       Storage:
/var/lib/systemd/coredump/core.git.1000.a3430056a91d43f49b602796f4eafc8c.43422.1770795791000000.zst
(present)
     Disk Size: 43.4K
       Message: Process 43422 (git) of user 1000 dumped core.

                Found module /home/ubuntu/Code/open_source/git/git
with build-id: d3afb1a6b38e0303f833747d30b120a5c520f40e
                Found module linux-vdso.so.1 with build-id:
c1c6868625bceb6f487c419392bd09e4edbfc5d9
                Found module libc.so.6 with build-id:
9f32d43c341bff10b9e7196738eedcfc4f3cc36c
                Found module libz.so.1 with build-id:
b781927da654e744ed29ff39815bef9c750eaf24
                Found module libc_malloc_debug.so.0 with build-id:
1e0d2faf0cfdf8b3b9940dc5937792836187f911
                Stack trace of thread 43422:
                #0  0x0000ffffb8802008 __pthread_kill_implementation
(libc.so.6 + 0x82008)
                #1  0x0000ffffb87ba83c __GI_raise (libc.so.6 + 0x3a83c)
                #2  0x0000ffffb87a7134 __GI_abort (libc.so.6 + 0x27134)
                #3  0x0000aaaab14af398 n/a
(/home/ubuntu/Code/open_source/git/git + 0x39f398)
                #4  0x0000aaaab14af398 n/a
(/home/ubuntu/Code/open_source/git/git + 0x39f398)
                #5  0x0000aaaab14af454 n/a
(/home/ubuntu/Code/open_source/git/git + 0x39f454)
                #6  0x0000aaaab14257ac n/a
(/home/ubuntu/Code/open_source/git/git + 0x3157ac)
                #7  0x0000aaaab1425890 n/a
(/home/ubuntu/Code/open_source/git/git + 0x315890)
                #8  0x0000aaaab142609c n/a
(/home/ubuntu/Code/open_source/git/git + 0x31609c)
                #9  0x0000aaaab142625c n/a
(/home/ubuntu/Code/open_source/git/git + 0x31625c)
                #10 0x0000aaaab13eeedc n/a
(/home/ubuntu/Code/open_source/git/git + 0x2deedc)
                #11 0x0000aaaab13eeab8 n/a
(/home/ubuntu/Code/open_source/git/git + 0x2deab8)
                #12 0x0000aaaab1249088 n/a
(/home/ubuntu/Code/open_source/git/git + 0x139088)
                #13 0x0000aaaab1249650 n/a
(/home/ubuntu/Code/open_source/git/git + 0x139650)
                #14 0x0000aaaab1249c84 n/a
(/home/ubuntu/Code/open_source/git/git + 0x139c84)
                #15 0x0000aaaab1132314 n/a
(/home/ubuntu/Code/open_source/git/git + 0x22314)
                #16 0x0000aaaab11328b0 n/a
(/home/ubuntu/Code/open_source/git/git + 0x228b0)
                #17 0x0000aaaab1132bd4 n/a
(/home/ubuntu/Code/open_source/git/git + 0x22bd4)
                #18 0x0000aaaab1133074 n/a
(/home/ubuntu/Code/open_source/git/git + 0x23074)
                #19 0x0000aaaab125e86c n/a
(/home/ubuntu/Code/open_source/git/git + 0x14e86c)
                #20 0x0000ffffb87a7400 __libc_start_call_main
(libc.so.6 + 0x27400)
                #21 0x0000ffffb89cf370 n/a (n/a + 0x0)
                #22 0x0000ffffb89cf370 n/a (n/a + 0x0)
                #23 0x3d455441445f524f n/a (n/a + 0x0)

GNU gdb (Ubuntu 12.1-0ubuntu1~22.04.2) 12.1
Copyright (C) 2022 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "aarch64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /home/ubuntu/Code/open_source/git/git...
[New LWP 43422]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/aarch64-linux-gnu/libthread_db.so.1".
Core was generated by `git submodule--helper add -- ./S'.
Program terminated with signal SIGABRT, Aborted.
#0  __pthread_kill_implementation (threadid=281473778995232,
signo=signo@entry=6,
    no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44
44 ./nptl/pthread_kill.c: No such file or directory.
(gdb) bt full
#0  __pthread_kill_implementation (threadid=281473778995232,
signo=signo@entry=6,
    no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44
        tid = 43422
        ret = 0
        pd = 0xffffb89c5020
        old_mask = {__val = {16453191242184589568, 281474327489088,
187650095641156,
            281474327489568, 281474327489568, 281474327489520,
18446743528248704984,
            17298308644996116495, 17298308644996116495, 17298308644996116480,
            16453191242184589568, 281474327488672, 187650095637040,
281474327488752,
            18446743042917400560, 281474327489088}}
        ret = <optimized out>
#1  0x0000ffffb8802054 in __pthread_kill_internal (signo=6,
threadid=<optimized out>)
    at ./nptl/pthread_kill.c:78
No locals.
#2  0x0000ffffb87ba83c in __GI_raise (sig=sig@entry=6) at
../sysdeps/posix/raise.c:26
        ret = <optimized out>
#3  0x0000ffffb87a7134 in __GI_abort () at ./stdlib/abort.c:79
        save_stage = 1
        act = {__sigaction_handler = {sa_handler = 0xaaaad0012bfb,
            sa_sigaction = 0xaaaad0012bfb}, sa_mask = {__val =
{187650610900023, 0, 0,
              0, 0, 0, 4294967295, 0, 187651416064000, 0, 281473778608136,
              281474327489232, 187650095641436, 281474327489264,
281473778608284,
              187650610899888}}, sa_flags = -1, sa_restorer = 0x88}
        sigs = {__val = {32, 187650096456616, 243494161448, 187650096456672,
            281474327489824, 8099004987637978434, 3348833620946678639,
            18374721914061273699, 281474327488992, 281474904195073,
187650610899963,
            187650610899963, 187650610899963, 187650610899963, 187650610899964,
            187650610900023}}
#4  0x0000aaaab14af398 in BUG_vfl (file=0xaaaab15763e0 "repository.c", line=56,
    fmt=0xaaaab15763a8 "trying to read config from wrong repository instance",
    params=...) at usage.c:350
        params_copy = {__stack = 0xffffd94dac20, __gr_top = 0xffffd94dac20,
          __vr_top = 0xffffd94dabf0, __gr_offs = -40, __vr_offs = -128}
        in_bug = 1
#5  0x0000aaaab14af454 in BUG_fl (file=0xaaaab15763e0 "repository.c", line=56,
    fmt=0xaaaab15763a8 "trying to read config from wrong repository instance")
--Type <RET> for more, q to quit, c to continue without paging--
    at usage.c:360
        ap = {__stack = 0xffffd94dac20, __gr_top = 0xffffd94dac20,
          __vr_top = 0xffffd94dabf0, __gr_offs = -40, __vr_offs = -128}
#6  0x0000aaaab14257ac in repo_config_values (repo=0xaaaad0012900) at
repository.c:56
No locals.
#7  0x0000aaaab1425890 in initialize_repository (repo=0xaaaad0012900)
    at repository.c:73
No locals.
#8  0x0000aaaab142609c in repo_init (repo=0xaaaad0012900,
    gitdir=0xaaaad0012b10 "/home/ubuntu/Code/open_source/git/t/trash
directory.t7422-submodule-output/S/.git",
    worktree=0xaaaad0012bb0 "/home/ubuntu/Code/open_source/git/t/trash
directory.t7422-submodule-output/S") at repository.c:284
        format = {version = -1, precious_objects = 0, partial_clone = 0x0,
          worktree_config = 0, relative_worktrees = 0, is_bare = -1,
hash_algo = 1,
          compat_hash_algo = 0, ref_storage_format = REF_STORAGE_FORMAT_FILES,
          sparse_index = 0, work_tree = 0x0, unknown_extensions = {items = 0x0,
            nr = 0, alloc = 0, strdup_strings = 1, cmp = 0x0},
v1_only_extensions = {
            items = 0x0, nr = 0, alloc = 0, strdup_strings = 1, cmp = 0x0}}
#9  0x0000aaaab142625c in repo_submodule_init (subrepo=0xaaaad0012900,
    superproject=0xaaaab1656c98 <the_repo>, path=0xaaaad0012080 "S",
    treeish_name=0xaaaab1558cc8 <null_oid_sha1>) at repository.c:329
        gitdir = {alloc = 136, len = 81,
          buf = 0xaaaad0012b10
"/home/ubuntu/Code/open_source/git/t/trash
directory.t7422-submodule-output/S/.git"}
        worktree = {alloc = 136, len = 76,
          buf = 0xaaaad0012bb0
"/home/ubuntu/Code/open_source/git/t/trash
directory.t7422-submodule-output/S"}
        ret = 0
#10 0x0000aaaab13eeedc in repo_get_submodule_ref_store (
    repo=0xaaaab1656c98 <the_repo>, submodule=0xaaaad0012080 "S") at refs.c:2258
        submodule_sb = {alloc = 24, len = 6, buf = 0xaaaad0012500 "S/.git"}
        refs = 0x0
        to_free = 0x0
        len = 1
        subrepo = 0xaaaad0012900
--Type <RET> for more, q to quit, c to continue without paging--
#11 0x0000aaaab13eeab8 in repo_resolve_gitlink_ref (r=0xaaaab1656c98 <the_repo>,
    submodule=0xaaaad0012080 "S", refname=0xaaaab1537d50 "HEAD",
oid=0xffffd94dae80)
    at refs.c:2141
        refs = 0x1
        flags = 1
#12 0x0000aaaab1249088 in die_on_repo_without_commits (path=0xaaaad0012080 "S")
    at builtin/submodule--helper.c:3423
        oid = {hash = '\000' <repeats 24 times>, "\200
\001\u042a\252\000", algo = 0}
        sb = {alloc = 24, len = 1, buf = 0xaaaad00125f0 "S"}
#13 0x0000aaaab1249650 in module_add (argc=1, argv=0xaaaad0010ce0, prefix=0x0,
    repo=0xaaaab1656c98 <the_repo>) at builtin/submodule--helper.c:3522
        force = 0
        quiet = 0
        progress = 0
        dissociate = 0
        add_data = {prefix = 0x0, branch = 0x0, reference_path = 0x0,
          sm_path = 0xaaaad0012080 "S", sm_name = 0x0, repo =
0xaaaad0010ae0 "./S",
          realrepo = 0xaaaad0012590
"/home/ubuntu/Code/open_source/git/t/trash
directory.t7422-submodule-output/S", ref_storage_format =
REF_STORAGE_FORMAT_UNKNOWN,
          depth = -1, force = 0, quiet = 0, progress = 0, dissociate = 0}
        ref_storage_format = 0x0
        to_free = 0xaaaad0012590
"/home/ubuntu/Code/open_source/git/t/trash
directory.t7422-submodule-output/S"
        existing = 0xaaaab156aa08
        buf = {alloc = 0, len = 0, buf = 0xaaaab16572f8 <strbuf_slopbuf> ""}
        sm_name_to_free = 0x0
        options = {{type = OPTION_STRING, short_name = 98,
            long_name = 0xaaaab1539658 "branch", value = 0xffffd94daf60,
            precision = 0, argh = 0xaaaab1539658 "branch",
            help = 0xaaaab1539c80 "branch of repository to add as submodule",
            flags = 0, callback = 0x0, defval = 0, ll_callback = 0x0, extra = 0,
            subcommand_fn = 0x0}, {type = OPTION_COUNTUP, short_name = 102,
            long_name = 0xaaaab1538070 "force", value =
0xffffd94daee8, precision = 4,
            argh = 0x0,
            help = 0xaaaab1539cb0 "allow adding an otherwise ignored
submodule path",
            flags = (PARSE_OPT_NOARG | PARSE_OPT_NOCOMPLETE), callback = 0x0,
--Type <RET> for more, q to quit, c to continue without paging--
            defval = 0, ll_callback = 0x0, extra = 0, subcommand_fn = 0x0}, {
            type = OPTION_COUNTUP, short_name = 113,
            long_name = 0xaaaab15375e0 "quiet", value =
0xffffd94daeec, precision = 4,
            argh = 0x0, help = 0xaaaab1539748 "print only error messages",
            flags = PARSE_OPT_NOARG, callback = 0x0, defval = 0,
ll_callback = 0x0,
            extra = 0, subcommand_fn = 0x0}, {type = OPTION_SET_INT,
short_name = 0,
            long_name = 0xaaaab15387a0 "progress", value = 0xffffd94daef0,
            precision = 4, argh = 0x0, help = 0xaaaab15387b0 "force
cloning progress",
            flags = PARSE_OPT_NOARG, callback = 0x0, defval = 1,
ll_callback = 0x0,
            extra = 0, subcommand_fn = 0x0}, {type = OPTION_STRING,
short_name = 0,
            long_name = 0xaaaab15386a8 "reference", value = 0xffffd94daf68,
            precision = 0, argh = 0xaaaab1539ce8 "repository",
            help = 0xaaaab15386c0 "reference repository", flags = 0,
callback = 0x0,
            defval = 0, ll_callback = 0x0, extra = 0, subcommand_fn = 0x0}, {
            type = OPTION_STRING, short_name = 0,
            long_name = 0xaaaab15386d8 "ref-format", value = 0xffffd94daf00,
            precision = 0, argh = 0xaaaab15386e8 "format",
            help = 0xaaaab15386f0 "specify the reference format to
use", flags = 0,
            callback = 0x0, defval = 0, ll_callback = 0x0, extra = 0,
            subcommand_fn = 0x0}, {type = OPTION_SET_INT, short_name = 0,
            long_name = 0xaaaab1538718 "dissociate", value = 0xffffd94daef4,
            precision = 4, argh = 0x0,
            help = 0xaaaab1539cf8 "borrow the objects from reference
repositories",
            flags = PARSE_OPT_NOARG, callback = 0x0, defval = 1,
ll_callback = 0x0,
            extra = 0, subcommand_fn = 0x0}, {type = OPTION_STRING,
short_name = 0,
            long_name = 0xaaaab1538648 "name", value = 0xffffd94daf78,
precision = 0,
            argh = 0xaaaab1538648 "name",
            help = 0xaaaab1539d28 "sets the submodule's name to the
given string instead of defaulting to its path", flags = 0, callback =
0x0, defval = 0, ll_callback = 0x0,
            extra = 0, subcommand_fn = 0x0}, {type = OPTION_INTEGER,
short_name = 0,
            long_name = 0xaaaab1538750 "depth", value =
0xffffd94daf94, precision = 4,
            argh = 0xaaaab1537cf0 "n",
            help = 0xaaaab1538758 "depth for shallow clones", flags = 0,
            callback = 0x0, defval = 0, ll_callback = 0x0, extra = 0,
            subcommand_fn = 0x0}, {type = OPTION_END, short_name = 0,
long_name = 0x0,
            value = 0x0, precision = 0, argh = 0x0, help = 0x0, flags = 0,
--Type <RET> for more, q to quit, c to continue without paging--
            callback = 0x0, defval = 0, ll_callback = 0x0, extra = 0,
            subcommand_fn = 0x0}}
        usage = {
          0xaaaab1539d78 "git submodule add [<options>] [--]
<repository> [<path>]",
          0x0}
        sb = {alloc = 0, len = 0, buf = 0xaaaab16572f8 <strbuf_slopbuf> ""}
        ret = 1
#14 0x0000aaaab1249c84 in cmd_submodule__helper (argc=3, argv=0xaaaad0010ce0,
    prefix=0x0, repo=0xaaaab1656c98 <the_repo>) at
builtin/submodule--helper.c:3616
        fn = 0xaaaab12490dc <module_add>
        usage = {0xaaaab1539ec0 "git submodule--helper <command>", 0x0}
        options = {{type = OPTION_SUBCOMMAND, short_name = 0,
            long_name = 0xaaaab15384b8 "clone", value =
0xffffd94db430, precision = 0,
            argh = 0x0, help = 0x0, flags = 0, callback = 0x0, defval = 0,
            ll_callback = 0x0, extra = 0,
            subcommand_fn = 0xaaaab124403c <module_clone>}, {type =
OPTION_SUBCOMMAND,
            short_name = 0, long_name = 0xaaaab1539b98 "add", value =
0xffffd94db430,
            precision = 0, argh = 0x0, help = 0x0, flags = 0, callback = 0x0,
            defval = 0, ll_callback = 0x0, extra = 0,
            subcommand_fn = 0xaaaab12490dc <module_add>}, {type =
OPTION_SUBCOMMAND,
            short_name = 0, long_name = 0xaaaab1538f78 "update",
            value = 0xffffd94db430, precision = 0, argh = 0x0, help =
0x0, flags = 0,
            callback = 0x0, defval = 0, ll_callback = 0x0, extra = 0,
            subcommand_fn = 0xaaaab1246e44 <module_update>}, {
            type = OPTION_SUBCOMMAND, short_name = 0,
            long_name = 0xaaaab1537500 "foreach", value = 0xffffd94db430,
            precision = 0, argh = 0x0, help = 0x0, flags = 0, callback = 0x0,
            defval = 0, ll_callback = 0x0, extra = 0,
            subcommand_fn = 0xaaaab123fe24 <module_foreach>}, {
            type = OPTION_SUBCOMMAND, short_name = 0,
            long_name = 0xaaaab1539110 "init", value = 0xffffd94db430,
precision = 0,
            argh = 0x0, help = 0x0, flags = 0, callback = 0x0, defval = 0,
            ll_callback = 0x0, extra = 0,
            subcommand_fn = 0xaaaab1240418 <module_init>}, {type =
OPTION_SUBCOMMAND,
            short_name = 0, long_name = 0xaaaab15378e8 "status",
            value = 0xffffd94db430, precision = 0, argh = 0x0, help =
0x0, flags = 0,
--Type <RET> for more, q to quit, c to continue without paging--
            callback = 0x0, defval = 0, ll_callback = 0x0, extra = 0,
            subcommand_fn = 0xaaaab1240ca8 <module_status>}, {
            type = OPTION_SUBCOMMAND, short_name = 0,
            long_name = 0xaaaab1537e48 "sync", value = 0xffffd94db430,
precision = 0,
            argh = 0x0, help = 0x0, flags = 0, callback = 0x0, defval = 0,
            ll_callback = 0x0, extra = 0,
            subcommand_fn = 0xaaaab1242a0c <module_sync>}, {type =
OPTION_SUBCOMMAND,
            short_name = 0, long_name = 0xaaaab1539ee0 "deinit",
            value = 0xffffd94db430, precision = 0, argh = 0x0, help =
0x0, flags = 0,
            callback = 0x0, defval = 0, ll_callback = 0x0, extra = 0,
            subcommand_fn = 0xaaaab1243074 <module_deinit>}, {
            type = OPTION_SUBCOMMAND, short_name = 0,
            long_name = 0xaaaab1539ee8 "summary", value = 0xffffd94db430,
            precision = 0, argh = 0x0, help = 0x0, flags = 0, callback = 0x0,
            defval = 0, ll_callback = 0x0, extra = 0,
            subcommand_fn = 0xaaaab1242164 <module_summary>}, {
            type = OPTION_SUBCOMMAND, short_name = 0,
            long_name = 0xaaaab1539ef0 "push-check", value = 0xffffd94db430,
            precision = 0, argh = 0x0, help = 0x0, flags = 0, callback = 0x0,
            defval = 0, ll_callback = 0x0, extra = 0,
            subcommand_fn = 0xaaaab12476f4 <push_check>}, {type =
OPTION_SUBCOMMAND,
            short_name = 0, long_name = 0xaaaab1539f00 "absorbgitdirs",
            value = 0xffffd94db430, precision = 0, argh = 0x0, help =
0x0, flags = 0,
            callback = 0x0, defval = 0, ll_callback = 0x0, extra = 0,
            subcommand_fn = 0xaaaab12479a8 <absorb_git_dirs>}, {
            type = OPTION_SUBCOMMAND, short_name = 0,
            long_name = 0xaaaab1539f10 "set-url", value = 0xffffd94db430,
            precision = 0, argh = 0x0, help = 0x0, flags = 0, callback = 0x0,
            defval = 0, ll_callback = 0x0, extra = 0,
            subcommand_fn = 0xaaaab1247b40 <module_set_url>}, {
            type = OPTION_SUBCOMMAND, short_name = 0,
            long_name = 0xaaaab1539f18 "set-branch", value = 0xffffd94db430,
            precision = 0, argh = 0x0, help = 0x0, flags = 0, callback = 0x0,
            defval = 0, ll_callback = 0x0, extra = 0,
            subcommand_fn = 0xaaaab1247d74 <module_set_branch>}, {
            type = OPTION_SUBCOMMAND, short_name = 0,
--Type <RET> for more, q to quit, c to continue without paging--
            long_name = 0xaaaab1539f28 "create-branch", value = 0xffffd94db430,
            precision = 0, argh = 0x0, help = 0x0, flags = 0, callback = 0x0,
            defval = 0, ll_callback = 0x0, extra = 0,
            subcommand_fn = 0xaaaab1248038 <module_create_branch>}, {
            type = OPTION_END, short_name = 0, long_name = 0x0, value = 0x0,
            precision = 0, argh = 0x0, help = 0x0, flags = 0, callback = 0x0,
            defval = 0, ll_callback = 0x0, extra = 0, subcommand_fn = 0x0}}
#15 0x0000aaaab1132314 in run_builtin (p=0xaaaab1624be8 <commands+3024>, argc=4,
    argv=0xaaaad0010ce0, repo=0xaaaab1656c98 <the_repo>) at git.c:506
        status = 0
        help = 0
        no_repo = 0
        st = {st_dev = 187650095680628, st_ino = 281474327493248,
          st_mode = 2974518388, st_nlink = 43690, st_uid = 3645750856,
st_gid = 65535,
          st_rdev = 5, __pad1 = 187650097155112, st_size = 281473779032128,
          st_blksize = -1322915768, __pad2 = 43690, st_blocks = 40, st_atim = {
            tv_sec = 281474327493280, tv_nsec = 187650091976020}, st_mtim = {
            tv_sec = 5, tv_nsec = 8}, st_ctim = {tv_sec = 281474327493408,
            tv_nsec = 187650091985032}, __glibc_reserved = {8, 0}}
        prefix = 0x0
        run_setup = 1
        __PRETTY_FUNCTION__ = "run_builtin"
#16 0x0000aaaab11328b0 in handle_builtin (args=0xffffd94dbc50) at git.c:779
        argv_copy = 0xaaaad0010ce0
        ret = 43690
        cmd = 0xaaaad00109b0 "submodule--helper"
        builtin = 0xaaaab1624be8 <commands+3024>
#17 0x0000aaaab1132bd4 in run_argv (args=0xffffd94dbc50) at git.c:862
        done_alias = 0
        expanded_aliases = {items = 0x0, nr = 0, alloc = 0, strdup_strings = 1,
          cmp = 0x0}
#18 0x0000aaaab1133074 in cmd_main (argc=4, argv=0xffffd94dbe50) at git.c:984
        was_alias = 65535
        args = {v = 0xaaaad00109d0, nr = 4, alloc = 24}
        cmd = 0xffffd94dc7ff "submodule--helper"
        done_help = 0
--Type <RET> for more, q to quit, c to continue without paging--
#19 0x0000aaaab125e86c in main (argc=5, argv=0xffffd94dbe48) at common-main.c:9
        result = 0

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Outreachy PATCH v6 1/3] environment: stop storing `core.attributesFile` globally
  2026-02-10 10:17 [Outreachy PATCH v6 1/3] environment: stop storing `core.attributesFile` globally Bello Caleb Olamide
  2026-02-10 15:07 ` Phillip Wood
@ 2026-02-11  9:31 ` Phillip Wood
  2026-02-11 12:05   ` Bello Olamide
  2026-02-11 16:46   ` Junio C Hamano
  1 sibling, 2 replies; 15+ messages in thread
From: Phillip Wood @ 2026-02-11  9:31 UTC (permalink / raw)
  To: Bello Caleb Olamide
  Cc: Junio C Hamano, git, toon, christian.couder, usmanakinyemi202,
	kaartic.sivaraam, me, karthik.188, phillip.wood

Thanks for the backtrace which helped me spot the problem though I 
should have spotted this yesterday. The problem is in 
initialize_repository()

>   void initialize_repository(struct repository *repo)
>   {
> +	if (repo->initialized)
> +		BUG("repository initialized already");
> +	repo->initialized = true;
> +
>   	repo->remote_state = remote_state_new();
>   	repo->parsed_objects = parsed_object_pool_new(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));

Here you need to use repo->config_values_private_ instead of using the 
accessor as it is fine to initialize the config values to their defaults 
in any instance, it is only when we read them that we want to assert 
that we're reading from "the_repository".

Thanks

Phillip

>   
>   	/*
>   	 * When a command runs inside a repository, it learns what
> 
> Some of the tests that fail are related to the submodule and a
> couple output is shown below
> 
> ./t7412-submodule-absorbgitdirs.sh  -i -v
> ...
> Initialized empty Git repository in /home/ubuntu/Code/open_source/git/t/trash directory.t7412-submodule-absorbgitdirs/sub1/.git/
> [master (root-commit) 50e526b] first
>   Author: A U Thor <author@example.com>
>   1 file changed, 1 insertion(+)
>   create mode 100644 first.t
> BUG: repository.c:56: trying to read config from wrong repository instance
> Aborted (core dumped)
> not ok 1 - setup a real submodule
> #
> # cwd="$(pwd)" &&
> # git init sub1 &&
> # test_commit -C sub1 first &&
> # git submodule add ./sub1 &&
> # test_tick &&
> # git commit -m superproject
> #
> 1..1
> 
> ./t4027-diff-submodule.sh  -i -v
> ...
> Initialized empty Git repository in /home/ubuntu/Code/open_source/git/t/trash directory.t4027-diff-submodule/sub/.git/
> [master (root-commit) 4431e0b] submodule
>   Author: A U Thor <author@example.com>
>   1 file changed, 1 insertion(+)
>   create mode 100644 world
> BUG: repository.c:56: trying to read config from wrong repository instance
> Aborted (core dumped)
> not ok 1 - setup
> #
> # test_tick &&
> # test_create_repo sub &&
> # (
> # cd sub &&
> # echo hello >world &&
> # git add world &&
> # git commit -m submodule
> # ) &&
> #
> # test_tick &&
> # echo frotz >nitfol &&
> # git add nitfol sub &&
> # git commit -m superproject &&
> #
> # (
> # cd sub &&
> # echo goodbye >world &&
> # git add world &&
> # git commit -m "submodule #2"
> # ) &&
> #
> # git -C sub rev-list HEAD >revs &&
> # set x $(cat revs) &&
> # echo ":160000 160000 $3 $ZERO_OID M sub" >expect &&
> # subtip=$3 subprev=$2
> #
> 1..1
> 
> Thanks
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Outreachy PATCH v6 1/3] environment: stop storing `core.attributesFile` globally
  2026-02-11  9:31 ` Phillip Wood
@ 2026-02-11 12:05   ` Bello Olamide
  2026-02-11 16:46   ` Junio C Hamano
  1 sibling, 0 replies; 15+ messages in thread
From: Bello Olamide @ 2026-02-11 12:05 UTC (permalink / raw)
  To: phillip.wood
  Cc: Junio C Hamano, git, toon, christian.couder, usmanakinyemi202,
	kaartic.sivaraam, me, karthik.188

On Wed, 11 Feb 2026 at 10:31, Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> Thanks for the backtrace which helped me spot the problem though I
> should have spotted this yesterday. The problem is in
> initialize_repository()
>
> >   void initialize_repository(struct repository *repo)
> >   {
> > +     if (repo->initialized)
> > +             BUG("repository initialized already");
> > +     repo->initialized = true;
> > +
> >       repo->remote_state = remote_state_new();
> >       repo->parsed_objects = parsed_object_pool_new(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));
>
> Here you need to use repo->config_values_private_ instead of using the
> accessor as it is fine to initialize the config values to their defaults
> in any instance, it is only when we read them that we want to assert
> that we're reading from "the_repository".
>

Okay thank you very much.
I will send an updated version.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Outreachy PATCH v6 1/3] environment: stop storing `core.attributesFile` globally
  2026-02-11  9:31 ` Phillip Wood
  2026-02-11 12:05   ` Bello Olamide
@ 2026-02-11 16:46   ` Junio C Hamano
  2026-02-12 10:33     ` Phillip Wood
  1 sibling, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2026-02-11 16:46 UTC (permalink / raw)
  To: Phillip Wood
  Cc: Bello Caleb Olamide, git, toon, christian.couder,
	usmanakinyemi202, kaartic.sivaraam, me, karthik.188, phillip.wood

Phillip Wood <phillip.wood123@gmail.com> writes:

> Thanks for the backtrace which helped me spot the problem though I 
> should have spotted this yesterday. The problem is in 
> initialize_repository()
>
>>   void initialize_repository(struct repository *repo)
>>   {
>> +	if (repo->initialized)
>> +		BUG("repository initialized already");
>> +	repo->initialized = true;
>> +
>>   	repo->remote_state = remote_state_new();
>>   	repo->parsed_objects = parsed_object_pool_new(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));
>
> Here you need to use repo->config_values_private_ instead of using the 
> accessor as it is fine to initialize the config values to their defaults 
> in any instance, it is only when we read them that we want to assert 
> that we're reading from "the_repository".

Sorry, but ...

At the beginning of repo_config_values() in the patch, there is a
check to ensure that repo->initialized is true and otherwise you get
an error.  But the initialization is already done in the early part
of initialize_repository() as quoted above.  So I do not see what
difference it would make if we rewrote the last line as

	repo_config_values_init(&repo->config_values_private_);

I am confused.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Outreachy PATCH v6 1/3] environment: stop storing `core.attributesFile` globally
  2026-02-11 16:46   ` Junio C Hamano
@ 2026-02-12 10:33     ` Phillip Wood
  2026-02-12 17:13       ` Junio C Hamano
  0 siblings, 1 reply; 15+ messages in thread
From: Phillip Wood @ 2026-02-12 10:33 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Bello Caleb Olamide, git, toon, christian.couder,
	usmanakinyemi202, kaartic.sivaraam, me, karthik.188, phillip.wood

On 11/02/2026 16:46, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
> At the beginning of repo_config_values() in the patch, there is a
> check to ensure that repo->initialized is true and otherwise you get
> an error.  But the initialization is already done in the early part
> of initialize_repository() as quoted above.  So I do not see what
> difference it would make if we rewrote the last line as

In Bello's patch there is a second assertion in repo_config_values() 
that checks "repo == the_repository" and that one fails. I suggested 
adding it because the config values are still global rather than per 
repository so we should only be reading them from "the_repository".

Thanks

Phillip

> 	repo_config_values_init(&repo->config_values_private_);
> 
> I am confused.
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Outreachy PATCH v6 1/3] environment: stop storing `core.attributesFile` globally
  2026-02-12 10:33     ` Phillip Wood
@ 2026-02-12 17:13       ` Junio C Hamano
  0 siblings, 0 replies; 15+ messages in thread
From: Junio C Hamano @ 2026-02-12 17:13 UTC (permalink / raw)
  To: Phillip Wood
  Cc: Bello Caleb Olamide, git, toon, christian.couder,
	usmanakinyemi202, kaartic.sivaraam, me, karthik.188, phillip.wood

Phillip Wood <phillip.wood123@gmail.com> writes:

> On 11/02/2026 16:46, Junio C Hamano wrote:
>> Phillip Wood <phillip.wood123@gmail.com> writes:
>> 
>> At the beginning of repo_config_values() in the patch, there is a
>> check to ensure that repo->initialized is true and otherwise you get
>> an error.  But the initialization is already done in the early part
>> of initialize_repository() as quoted above.  So I do not see what
>> difference it would make if we rewrote the last line as
>
> In Bello's patch there is a second assertion in repo_config_values() 
> that checks "repo == the_repository" and that one fails. I suggested 
> adding it because the config values are still global rather than per 
> repository so we should only be reading them from "the_repository".

Ah, OK.  It may indeed be a good safety valve with the current
codebase.  I am not sure what the upgrade path would look like from
there, though.

Thanks.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2026-02-12 17:13 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-10 10:17 [Outreachy PATCH v6 1/3] environment: stop storing `core.attributesFile` globally Bello Caleb Olamide
2026-02-10 15:07 ` Phillip Wood
2026-02-11  8:05   ` Bello Olamide
2026-02-11  9:31 ` Phillip Wood
2026-02-11 12:05   ` Bello Olamide
2026-02-11 16:46   ` Junio C Hamano
2026-02-12 10:33     ` Phillip Wood
2026-02-12 17:13       ` Junio C Hamano
  -- strict thread matches above, loose matches on Subject: below --
2026-02-03 15:42 [Outreachy PATCH v6 0/3] store repo specific config values in new `struct repo_config_values` Olamide Caleb Bello
2026-02-03 15:42 ` [Outreachy PATCH v6 1/3] environment: stop storing `core.attributesFile` globally Olamide Caleb Bello
2026-02-04 16:39   ` Phillip Wood
2026-02-09  8:47     ` Bello Olamide
2026-02-07  1:14   ` Junio C Hamano
2026-02-08 11:14     ` Phillip Wood
2026-02-09  8:54       ` Bello Olamide
2026-02-10  8:40       ` Bello Olamide

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox