Re: [PATCH] setup: recognize bare repositories with packed-refs

[Josh Steadmon <steadmon@google.com>]

On 2023.11.28 15:28, Adam Majer wrote:
> In a garbage collected bare git repository, the refs/ subdirectory is
> empty.  In use-cases when such a repository is directly added into
> another repository, it no longer is detected as valid. Git doesn't
> preserve empty paths so refs/ subdirectory is not present. Simply
> creating an empty refs/ subdirectory fixes this problem.
> 
> Looking more carefully, there are two backends to handle various refs in
> git -- the files backend that uses refs/ subdirectory and the
> packed-refs backend that uses packed-refs file. If references are not
> found in refs/ subdirectory (or directory doesn't exist), the
> packed-refs directory will be consulted. Garbage collected repository
> will have all its references in packed-refs file.
> 
> To allow the use-case when packed-refs is the only source of refs and
> refs/ subdirectory is simply not present, augment 'is_git_directory()'
> setup function to look for packed-refs file as an alternative to refs/
> subdirectory.
> 
> Signed-off-by: Adam Majer <adamm@zombino.com>
> ---
>  setup.c       | 10 +++++++---
>  t/t6500-gc.sh |  9 +++++++++
>  2 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/setup.c b/setup.c
> index fc592dc6dd..2a6dda6ae9 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -348,7 +348,7 @@ int get_common_dir_noenv(struct strbuf *sb, const char *gitdir)
>   *
>   *  - either an objects/ directory _or_ the proper
>   *    GIT_OBJECT_DIRECTORY environment variable
> - *  - a refs/ directory
> + *  - a refs/ directory or packed-refs file
>   *  - either a HEAD symlink or a HEAD file that is formatted as
>   *    a proper "ref:", or a regular file HEAD that has a properly
>   *    formatted sha1 object name.
> @@ -384,8 +384,12 @@ int is_git_directory(const char *suspect)
>  
>  	strbuf_setlen(&path, len);
>  	strbuf_addstr(&path, "/refs");
> -	if (access(path.buf, X_OK))
> -		goto done;
> +	if (access(path.buf, X_OK)) {
> +		strbuf_setlen(&path, len);
> +		strbuf_addstr(&path, "/packed-refs");
> +		if (access(path.buf, R_OK))
> +			goto done;
> +	}
>  
>  	ret = 1;
>  done:
> diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
> index 18fe1c25e6..4ad1690817 100755
> --- a/t/t6500-gc.sh
> +++ b/t/t6500-gc.sh
> @@ -214,6 +214,15 @@ test_expect_success 'gc.repackFilter launches repack with a filter' '
>  	grep -E "^trace: (built-in|exec|run_command): git repack .* --filter=blob:none ?.*" trace.out
>  '
>  
> +test_expect_success 'GCed bare repos without empty refs/ still recognized' '
> +	GIT_DIR="$PWD"/bare.git git cat-file -e master &&
> +	test_dir_is_empty bare.git/refs/heads &&
> +	test_dir_is_empty bare.git/refs/tags &&
> +	test_file_not_empty bare.git/packed-refs &&
> +	rm -r bare.git/refs &&
> +	GIT_DIR="$PWD"/bare.git git cat-file -e master
> +'
> +
>  test_expect_success 'gc.repackFilterTo store filtered out objects' '
>  	test_when_finished "rm -rf bare.git filtered.git" &&
>  
> -- 
> 2.43.0.1.g67290e5b65

Thanks for the fixes. This looks good to me. BTW, in the future please
add a version number when you send updated patches (e.g. add "-v 2" to
your command-line if you're using git-format-patch).

Reviewed-by: Josh Steadmon <steadmon@google.com>