Re: [PATCH v9 2/9] environment: move excludes_file into repo_config_values

["Pablo Sabater" <pabloosabaterr@gmail.com>]

On Thu Jul 9, 2026 at 6:11 PM CEST, Tian Yuchen wrote:
> The global variable 'excludes_file' is used to track the path to the
> global ignore file. If this variable is NULL,
> 'setup_standard_excludes()'

Nit: Strange line break here.

> in 'dir.c' forcefully evaluates and assigns the XDG default path to it.
>
> Continue the libification effort by encapsulating this lazy-loading
> fallback logic into a proper getter and moving the variable into
> 'struct repo_config_values'.
>
> Since 'excludes_file' is a dynamically allocated string, it requires
> proper heap memory management. It is safely freed using the newly
> introduced `repo_config_values_clear()` function when the repository
> is torn down.
>
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
> Signed-off-by: Tian Yuchen <cat@malon.dev>
> ---
>  dir.c         |  4 ++--
>  environment.c | 15 ++++++++++++---
>  environment.h |  4 +++-
>  3 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/dir.c b/dir.c
> index 7a73690fbc..4f87a52b3c 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -3481,11 +3481,11 @@ static GIT_PATH_FUNC(git_path_info_exclude, "info/exclude")
>
>  void setup_standard_excludes(struct dir_struct *dir)
>  {
> +	const char *excludes_file = repo_excludes_file(the_repository);
> +
>  	dir->exclude_per_dir = ".gitignore";
>
>  	/* core.excludesfile defaulting to $XDG_CONFIG_HOME/git/ignore */
> -	if (!excludes_file)
> -		excludes_file = xdg_config_home("ignore");
>  	if (excludes_file && !access_or_warn(excludes_file, R_OK, 0))
>  		add_patterns_from_file_1(dir, excludes_file,
>  					 dir->untracked ? &dir->internal.ss_excludes_file : NULL);
> diff --git a/environment.c b/environment.c
> index 13677484de..5950592d63 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -57,7 +57,6 @@ enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT;
>  enum fsync_component fsync_components = FSYNC_COMPONENTS_DEFAULT;
>  char *editor_program;
>  char *askpass_program;
> -char *excludes_file;
>  enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
>  enum eol core_eol = EOL_UNSET;
>  int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
> @@ -134,6 +133,14 @@ int is_bare_repository(void)
>  	return is_bare_repository_cfg && !repo_get_work_tree(the_repository);
>  }
>
> +const char *repo_excludes_file(struct repository *repo)
> +{
> +	if (!repo_config_values(repo)->excludes_file)
> +		repo_config_values(repo)->excludes_file = xdg_config_home("ignore");
> +
> +	return repo_config_values(repo)->excludes_file;
> +}

repo_config_values() returns a pointer so there should be no need to
call the function 3 times.

We could have the function be called once and use it then:

  const char *repo_excludes_file(struct repository *repo)
  {
	  struct repo_config_values *cfg = repo_config_values(repo);

	  if (!cfg->excludes_file)
		  cfg->excludes_file = xdg_config_home("ignore");

	  return cfg->excludes_file;
  }

> +
>  int have_git_dir(void)
>  {
>  	return startup_info->have_repository
> @@ -461,8 +468,8 @@ int git_default_core_config(const char *var, const char *value,
>  	}
>
>  	if (!strcmp(var, "core.excludesfile")) {
> -		FREE_AND_NULL(excludes_file);
> -		return git_config_pathname(&excludes_file, var, value);
> +		FREE_AND_NULL(cfg->excludes_file);
> +		return git_config_pathname(&cfg->excludes_file, var, value);
>  	}
>
>  	if (!strcmp(var, "core.whitespace")) {
> @@ -715,6 +722,7 @@ int git_default_config(const char *var, const char *value,
>  void repo_config_values_init(struct repo_config_values *cfg)
>  {
>  	cfg->attributes_file = NULL;
> +	cfg->excludes_file = NULL;
>  	cfg->apply_sparse_checkout = 0;
>  	cfg->branch_track = BRANCH_TRACK_REMOTE;
>  	cfg->trust_ctime = 1;
> @@ -744,4 +752,5 @@ void repo_config_values_clear(struct repository *repo)
>  	cfg = repo_config_values(repo);
>
>  	FREE_AND_NULL(cfg->attributes_file);
> +	FREE_AND_NULL(cfg->excludes_file);
>  }
> diff --git a/environment.h b/environment.h
> index c4a6a45704..2e8352de7f 100644
> --- a/environment.h
> +++ b/environment.h
> @@ -90,6 +90,7 @@ struct repository;
>  struct repo_config_values {
>  	/* section "core" config values */
>  	char *attributes_file;
> +	char *excludes_file;
>  	int apply_sparse_checkout;
>  	int trust_ctime;
>  	int check_stat;
> @@ -133,6 +134,8 @@ int git_default_config(const char *, const char *,
>  int git_default_core_config(const char *var, const char *value,
>  			    const struct config_context *ctx, void *cb);
>
> +const char *repo_excludes_file(struct repository *repo);
> +
>  void repo_config_values_init(struct repo_config_values *cfg);
>
>  /*
> @@ -217,7 +220,6 @@ extern char *git_log_output_encoding;
>
>  extern char *editor_program;
>  extern char *askpass_program;
> -extern char *excludes_file;
>
>  /*
>   * The character that begins a commented line in user-editable file

The rest looks fine.

Regards,
Pablo