Re: [GSOC PATCH v2 2/2] attr: use `repo_settings_get_attributesfile_path()` and update callers

[shejialuo <shejialuo@gmail.com>]

On Mon, Mar 10, 2025 at 08:40:48PM +0530, Ayush Chandekar wrote:
> Update attribute-related functions to retrieve the "core.attributesfile"
> configuration via the new repository-scoped accessor
> `repo_settings_get_attributesfile_path()`. This improves behaviour in
> multi-repository contexts and aligns with the goal of minimizing
> reliance on global state.
> 
> Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
> ---
>  attr.c               | 28 ++++++++++------------------
>  attr.h               |  7 +++----
>  builtin/check-attr.c |  2 +-
>  builtin/var.c        |  2 +-
>  4 files changed, 15 insertions(+), 24 deletions(-)
> 
> diff --git a/attr.c b/attr.c
> index 0bd2750528..8f28463e8c 100644
> --- a/attr.c
> +++ b/attr.c
> @@ -879,14 +879,6 @@ const char *git_attr_system_file(void)
>  	return system_wide;
>  }
>  
> -const char *git_attr_global_file(void)
> -{
> -	if (!git_attributes_file)
> -		git_attributes_file = xdg_config_home("attributes");
> -
> -	return git_attributes_file;
> -}
> -
>  int git_attr_system_is_enabled(void)
>  {
>  	return !git_env_bool("GIT_ATTR_NOSYSTEM", 0);
> @@ -906,7 +898,7 @@ static void push_stack(struct attr_stack **attr_stack_p,
>  	}
>  }
>  
> -static void bootstrap_attr_stack(struct index_state *istate,
> +static void bootstrap_attr_stack(struct repository *repo, struct index_state *istate,

I have scanned the definition of the "struct index_state", there is a
"struct repository *repo" member in this data structure. This makes me
think why do we need to pass the "struct repository *repo" in the first
place. A design question, should we just use `istate->repo` directly?

>  				 const struct object_id *tree_oid,
>  				 struct attr_stack **stack)
>  {
> @@ -927,8 +919,8 @@ static void bootstrap_attr_stack(struct index_state *istate,
>  	}
>  
>  	/* home directory */
> -	if (git_attr_global_file()) {
> -		e = read_attr_from_file(git_attr_global_file(), flags);
> +	if (repo_settings_get_attributesfile_path(repo)) {
> +		e = read_attr_from_file(repo_settings_get_attributesfile_path(repo), flags);
>  		push_stack(stack, e, NULL, 0);
>  	}
>  
> @@ -946,7 +938,7 @@ static void bootstrap_attr_stack(struct index_state *istate,
>  	push_stack(stack, e, NULL, 0);
>  }
>  
> -static void prepare_attr_stack(struct index_state *istate,
> +static void prepare_attr_stack(struct repository *repo, struct index_state *istate,
>  			       const struct object_id *tree_oid,
>  			       const char *path, int dirlen,
>  			       struct attr_stack **stack)

If we use "istate->repo", we don't even need to change this function.

> @@ -969,7 +961,7 @@ static void prepare_attr_stack(struct index_state *istate,
>  	 * .gitattributes in deeper directories to shallower ones,
>  	 * and finally use the built-in set as the default.
>  	 */
> -	bootstrap_attr_stack(istate, tree_oid, stack);
> +	bootstrap_attr_stack(repo, istate, tree_oid, stack);
>  
>  	/*
>  	 * Pop the "info" one that is always at the top of the stack.

[snip]

Thanks,
Jialuo