From: Karthik Nayak Date: Tue, 14 Apr 2026 09:07:01 GMT Subject: Re: [PATCH v2 5/8] environment: move "precomposed_unicode" into `struct repo_config_values` Message-ID: In-Reply-To: <20260324123750.157143-6-belkid98@gmail.com> Olamide Caleb Bello writes: > The `core.precomposeunicode` configuration is currently stored in the > global variable `precomposed_unicode`, which makes it shared across > repository instances within a single process. > > Store it instead in `repo_config_values` so the value is associated > with the repository from which it was read. This preserves existing > behavior while avoiding cross-repository state leakage and is another > step toward eliminating repository-dependent global state. > > Update all references to use repo_config_values(). > > Mentored-by: Christian Couder > Mentored-by: Usman Akinyemi > Signed-off-by: Olamide Caleb Bello > --- > compat/precompose_utf8.c | 20 +++++++++++++------- > environment.c | 4 ++-- > environment.h | 2 +- > upload-pack.c | 3 ++- > 4 files changed, 18 insertions(+), 11 deletions(-) > > diff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c > index 43b3be0114..0e94dbd862 100644 > --- a/compat/precompose_utf8.c > +++ b/compat/precompose_utf8.c > @@ -48,16 +48,18 @@ void probe_utf8_pathname_composition(void) > static const char *auml_nfc = "\xc3\xa4"; > static const char *auml_nfd = "\x61\xcc\x88"; > int output_fd; > - if (precomposed_unicode != -1) > + struct repo_config_values *cfg = repo_config_values(the_repository); > + > + if (cfg->precomposed_unicode != -1) > return; /* We found it defined in the global config, respect it */ > repo_git_path_replace(the_repository, &path, "%s", auml_nfc); > output_fd = open(path.buf, O_CREAT|O_EXCL|O_RDWR, 0600); > if (output_fd >= 0) { > close(output_fd); > repo_git_path_replace(the_repository, &path, "%s", auml_nfd); > - precomposed_unicode = access(path.buf, R_OK) ? 0 : 1; > + cfg->precomposed_unicode = access(path.buf, R_OK) ? 0 : 1; > repo_config_set(the_repository, "core.precomposeunicode", > - precomposed_unicode ? "true" : "false"); > + cfg->precomposed_unicode ? "true" : "false"); > repo_git_path_replace(the_repository, &path, "%s", auml_nfc); > if (unlink(path.buf)) > die_errno(_("failed to unlink '%s'"), path.buf); > @@ -69,14 +71,16 @@ const char *precompose_string_if_needed(const char *in) > { > size_t inlen; > size_t outlen; > + struct repo_config_values *cfg = repo_config_values(the_repository); > + > if (!in) > return NULL; > if (has_non_ascii(in, (size_t)-1, &inlen)) { > iconv_t ic_prec; > char *out; > - if (precomposed_unicode < 0) > - repo_config_get_bool(the_repository, "core.precomposeunicode", &precomposed_unicode); > - if (precomposed_unicode != 1) > + if (cfg->precomposed_unicode < 0) > + repo_config_get_bool(the_repository, "core.precomposeunicode", &cfg->precomposed_unicode); So if the variable is unset, we parse the config again. My question is why doesn't this flow already have the config parsed, or in other words, is there a way we reach here without the repository being setup. Would be nice to add this in the commit message. > + if (cfg->precomposed_unicode != 1) > return in; > ic_prec = iconv_open(repo_encoding, path_encoding); > if (ic_prec == (iconv_t) -1) > @@ -130,7 +134,9 @@ PREC_DIR *precompose_utf8_opendir(const char *dirname) > > struct dirent_prec_psx *precompose_utf8_readdir(PREC_DIR *prec_dir) > { > + struct repo_config_values *cfg = repo_config_values(the_repository); > struct dirent *res; > + > res = readdir(prec_dir->dirp); > if (res) { > size_t namelenz = strlen(res->d_name) + 1; /* \0 */ > @@ -149,7 +155,7 @@ struct dirent_prec_psx *precompose_utf8_readdir(PREC_DIR *prec_dir) > prec_dir->dirent_nfc->d_ino = res->d_ino; > prec_dir->dirent_nfc->d_type = res->d_type; > > - if ((precomposed_unicode == 1) && has_non_ascii(res->d_name, (size_t)-1, NULL)) { > + if ((cfg->precomposed_unicode == 1) && has_non_ascii(res->d_name, (size_t)-1, NULL)) { > if (prec_dir->ic_precompose == (iconv_t)-1) { > die("iconv_open(%s,%s) failed, but needed:\n" > " precomposed unicode is not supported.\n" > diff --git a/environment.c b/environment.c > index d0d3a4b7d2..739b647ebe 100644 > --- a/environment.c > +++ b/environment.c > @@ -72,7 +72,6 @@ enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE; > int grafts_keep_true_parents; > int core_sparse_checkout_cone; > int sparse_expect_files_outside_of_patterns; > -int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */ > unsigned long pack_size_limit_cfg; > > #ifndef PROTECT_HFS_DEFAULT > @@ -532,7 +531,7 @@ int git_default_core_config(const char *var, const char *value, > } > > if (!strcmp(var, "core.precomposeunicode")) { > - precomposed_unicode = git_config_bool(var, value); > + cfg->precomposed_unicode = git_config_bool(var, value); We parse a bool value, but.... > return 0; > } > > @@ -723,4 +722,5 @@ void repo_config_values_init(struct repo_config_values *cfg) > cfg->check_stat = 1; > cfg->zlib_compression_level = Z_BEST_SPEED; > cfg->pack_compression_level = Z_DEFAULT_COMPRESSION; > + cfg->precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */ > } But set -1 to showcase that this is not set. We should add that comment here. > diff --git a/environment.h b/environment.h > index 514576b67a..508cb1afbc 100644 > --- a/environment.h > +++ b/environment.h > @@ -95,6 +95,7 @@ struct repo_config_values { > int check_stat; > int zlib_compression_level; > int pack_compression_level; > + int precomposed_unicode; > > /* section "branch" config values */ > enum branch_track branch_track; > @@ -174,7 +175,6 @@ extern char *apply_default_whitespace; > extern char *apply_default_ignorewhitespace; > extern unsigned long pack_size_limit_cfg; > > -extern int precomposed_unicode; > extern int protect_hfs; > extern int protect_ntfs;