Re: [PATCH v2 5/8] environment: move "precomposed_unicode" into `struct repo_config_values`
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- Apr 14, 2026, 09:07 UTC
- Message-ID
- <CAOLa=ZRE8O7UANkzr4p9__ReV1OX3KBBpqbKpfCJ+EvyziwtTA@mail.gmail.com>
- In-Reply-To
- <20260324123750.157143-6-belkid98@gmail.com>
Olamide Caleb Bello <belkid98@gmail.com> writes:
Show 63 quoted lines
> 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 <christian.couder@gmail.com>
> Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
> Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>
> ---
> 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.
Show 41 quoted lines
> + 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....
Show 9 quoted lines
> 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.
Show 19 quoted lines
> 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;