Re: [PATCH] [RFC][GSoC][PATCH] attr: use local repository state in read_attr

[Tian Yuchen <a3205153416@gmail.com>]

On 2/7/26 19:40, Ayush Jha wrote:
> read_attr() currently relies on is_bare_repository(), which
> implicitly depends on the global the_repository.
> 
> As attr.c is a reusable library component used by multiple
> commands, this prevents correct behavior when operating on
> secondary repositories (e.g. submodules or in-process repos)
> whose bareness may differ from the_repository.
> 
> Update read_attr() to determine bareness using the repository
> associated with istate->repo, based on repository configuration
> and worktree presence, instead of relying on global state.
> 
> No functional change is intended for the primary repository case.
> 
> Signed-off-by: Ayush Jha <kumarayushjha123@gmail.com>
> ---
>   attr.c | 36 ++++++++++++++++++++++--------------
>   1 file changed, 22 insertions(+), 14 deletions(-)
> 
> diff --git a/attr.c b/attr.c
> index 4999b7e09d..f2d25b1863 100644
> --- a/attr.c
> +++ b/attr.c
> @@ -848,21 +848,29 @@ static struct attr_stack *read_attr(struct index_state *istate,
>   		res = read_attr_from_index(istate, path, flags);
>   	} else if (tree_oid) {
>   		res = read_attr_from_blob(istate, tree_oid, path, flags);
> -	} else if (!is_bare_repository()) {
> -		if (direction == GIT_ATTR_CHECKOUT) {
> -			res = read_attr_from_index(istate, path, flags);
> -			if (!res)
> -				res = read_attr_from_file(path, flags);
> -		} else if (direction == GIT_ATTR_CHECKIN) {
> -			res = read_attr_from_file(path, flags);
> -			if (!res)
> -				/*
> -				 * There is no checked out .gitattributes file
> -				 * there, but we might have it in the index.
> -				 * We allow operation in a sparsely checked out
> -				 * work tree, so read from it.
> -				 */
> +	} else {
> +		int is_bare;
> +		int is_bare_cfg = -1;
> +
> +		repo_config_get_bool(istate->repo, "core.bare", &is_bare_cfg);
> +		is_bare = is_bare_cfg && !repo_get_work_tree(istate->repo);
> +
> +		if (!is_bare) {
> +			if (direction == GIT_ATTR_CHECKOUT) {
>   				res = read_attr_from_index(istate, path, flags);
> +				if (!res)
> +					res = read_attr_from_file(path, flags);
> +			} else if (direction == GIT_ATTR_CHECKIN) {
> +				res = read_attr_from_file(path, flags);
> +				if (!res)
> +					/*
> +					 * There is no checked out .gitattributes file
> +					 * there, but we might have it in the index.
> +					 * We allow operation in a sparsely checked out
> +					 * work tree, so read from it.
> +					 */
> +					res = read_attr_from_index(istate, path, flags);
> +			}
>   		}
>   	}
>   

Hi Ayush,

I'm new to the community ;)

Initially, the concern that replacing 'is_bare_repository' with 
'repo_config_get_bool()' inside 'read_attr()' might introduce a 
performance regression came to my mind immediately. To verify this, I 
ran a benchmark using hyperfine, and the script was like:

 >#!/bin/bash
 >mkdir -p perf-test && cd perf-test
 >git init
 >
 >git config user.email "malon7782@yahoo.com"
 >git config user.name "ILOVEGIT"
 >
 >for i in {1..10000}; do
 >	echo "content" > "file_$i.txt"
 >done
 >
 >git add .
 >git commit -m "initial commit"
 >
 >echo "* test=auto" > .gitattributes
 >git add .gitattributes
 >git commit -m "add attr"

Then

 >'git ls-files > files_list.txt'

( I wrote it this way primarily because I didn't anticipate that too 
many files would return a 126 error code. So I improvised my switching 
to reading from standard input:/

It didn't result in a noticeable performance difference (Even if the 
number of loop was increased to 300000). Then I started to create a 
stricter benchmark environment to defeat the attribute stack caching:


 >mkdir -p dir_A dir_B
 >
 >echo "* text=auto" > dir_A/.gitattributes
 >echo "* text=auto" > dir_B/.gitattributes
 >
 >for i in {1..5000}; do
 >    echo "dir_A/file"
 >    echo "dir_B/file"
 >done > thrashing_list.txt

Still, no difference in performance. Has anyone else conducted any 
similar performance test? Were the results the same as mine? Is it 
normal, or there is something wrong with my test?

Regards,

Yuchen