Re: [GSOC PATCH v2 2/2] attr: use `repo_settings_get_attributesfile_path()` and update callers
- From
shejialuo <shejialuo@gmail.com>
- Date
- Mar 11, 2025, 14:39 UTC
- Message-ID
- <Z9BLMLXJ7Desl-n6@ArchLinux>
- In-Reply-To
- <20250310151048.69825-3-ayu.chandekar@gmail.com>
On Mon, Mar 10, 2025 at 08:40:48PM +0530, Ayush Chandekar wrote:
Show 39 quoted lines
> 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?
Show 23 quoted lines
> 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.
Show 9 quoted lines
> @@ -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