Ayush Chandekar <ayu.chandekar@xxxxxxxxx> writes:
> 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.
>
We should also talk about the modifications made to pass around the
repository struct.
> Signed-off-by: Ayush Chandekar <ayu.chandekar@xxxxxxxxx>
> ---
> 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,
Nit: here and other places, can this be a 'const'?
> 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)
> @@ -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.
> @@ -1143,7 +1135,7 @@ static void determine_macros(struct all_attrs_item *all_attrs,
> * If check->check_nr is non-zero, only attributes in check[] are collected.
> * Otherwise all attributes are collected.
> */
> -static void collect_some_attrs(struct index_state *istate,
> +static void collect_some_attrs(struct repository *repo, struct index_state *istate,
> const struct object_id *tree_oid,
> const char *path, struct attr_check *check)
> {
> @@ -1164,7 +1156,7 @@ static void collect_some_attrs(struct index_state *istate,
> dirlen = 0;
> }
>
> - prepare_attr_stack(istate, tree_oid, path, dirlen, &check->stack);
> + prepare_attr_stack(repo, istate, tree_oid, path, dirlen, &check->stack);
> all_attrs_init(&g_attr_hashmap, check);
> determine_macros(check->all_attrs, check->stack);
>
> @@ -1310,7 +1302,7 @@ void git_check_attr(struct index_state *istate,
> int i;
> const struct object_id *tree_oid = default_attr_source();
>
> - collect_some_attrs(istate, tree_oid, path, check);
> + collect_some_attrs(the_repository, istate, tree_oid, path, check);
>
The other places in the same file we pass around the 'repository'
struct, but here we use 'the_repository'. Even below, we modify an
external function's signature to avail the 'repository' struct.
Can't we modify 'git_check_attr()' to also receive a 'repository'? If
not, perhaps it would be much simpler to simply pass 'the_repository'
everywhere and cleanup this file in another follow up series?
> for (i = 0; i < check->nr; i++) {
> unsigned int n = check->items[i].attr->attr_nr;
> @@ -1321,14 +1313,14 @@ void git_check_attr(struct index_state *istate,
> }
> }
>
> -void git_all_attrs(struct index_state *istate,
> +void git_all_attrs(struct repository *repo, struct index_state *istate,
> const char *path, struct attr_check *check)
> {
> int i;
> const struct object_id *tree_oid = default_attr_source();
>
> attr_check_reset(check);
> - collect_some_attrs(istate, tree_oid, path, check);
> + collect_some_attrs(repo, istate, tree_oid, path, check);
>
> for (i = 0; i < check->all_attrs_nr; i++) {
> const char *name = check->all_attrs[i].attr->name;
> diff --git a/attr.h b/attr.h
> index a04a521092..1ff058bef7 100644
> --- a/attr.h
> +++ b/attr.h
> @@ -213,11 +213,13 @@ void git_check_attr(struct index_state *istate,
> const char *path,
> struct attr_check *check);
>
> +struct repository;
> +
> /*
> * Retrieve all attributes that apply to the specified path.
> * check holds the attributes and their values.
> */
> -void git_all_attrs(struct index_state *istate,
> +void git_all_attrs(struct repository *repo, struct index_state *istate,
> const char *path, struct attr_check *check);
>
> enum git_attr_direction {
> @@ -232,9 +234,6 @@ void attr_start(void);
> /* Return the system gitattributes file. */
> const char *git_attr_system_file(void);
>
> -/* Return the global gitattributes file, if any. */
> -const char *git_attr_global_file(void);
> -
> /* Return whether the system gitattributes file is enabled and should be used. */
> int git_attr_system_is_enabled(void);
>
> diff --git a/builtin/check-attr.c b/builtin/check-attr.c
> index 7cf275b893..1b8a89dfb2 100644
> --- a/builtin/check-attr.c
> +++ b/builtin/check-attr.c
> @@ -70,7 +70,7 @@ static void check_attr(const char *prefix, struct attr_check *check,
> prefix_path(prefix, prefix ? strlen(prefix) : 0, file);
>
> if (collect_all) {
> - git_all_attrs(the_repository->index, full_path, check);
> + git_all_attrs(the_repository, the_repository->index, full_path, check);
> } else {
> git_check_attr(the_repository->index, full_path, check);
> }
> diff --git a/builtin/var.c b/builtin/var.c
> index ada642a9fe..8fbf5430a4 100644
> --- a/builtin/var.c
> +++ b/builtin/var.c
> @@ -71,7 +71,7 @@ static char *git_attr_val_system(int ident_flag UNUSED)
>
> static char *git_attr_val_global(int ident_flag UNUSED)
> {
> - char *file = xstrdup_or_null(git_attr_global_file());
> + char *file = xstrdup_or_null(repo_settings_get_attributesfile_path(the_repository));
> if (file) {
> normalize_path_copy(file, file);
> return file;
> --
> 2.48.GIT
Attachment:
signature.asc
Description: PGP signature
