Re: [PATCH v3 6/8] packed-backend: add "packed-refs" entry consistency check

From: Patrick Steinhardt <ps@pks.im>
Date: Feb 12, 2025, 09:56 UTC
Message-ID: <Z6xwYlYFzbn3ft8u@pks.im>
In-Reply-To: <Z6RPzIGD-fSwIEPV@ArchLinux>

On Thu, Feb 06, 2025 at 01:59:40PM +0800, shejialuo wrote:

Show 11 quoted lines

> diff --git a/refs/packed-backend.c b/refs/packed-backend.c
> index c8bb93bb18..658f6bc7da 100644
> --- a/refs/packed-backend.c
> +++ b/refs/packed-backend.c
> @@ -1826,6 +1899,26 @@ static int packed_fsck_ref_content(struct fsck_options *o,
>  		line_number++;
>  	}
>  
> +	while (start < eof) {
> +		strbuf_reset(&packed_entry);
> +		strbuf_addf(&packed_entry, "packed-refs line %lu", line_number);

Instead of greedily computing the name of the line, can we pass in the line number? The motivation is that in a well-formatted packed-refs file we won't ever need this string at all, so it's wasteful to proactively compute it for every single line.

Show 36 quoted lines

> diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh
> index da321f16c6..3ab6b5bba5 100755
> --- a/t/t0602-reffiles-fsck.sh
> +++ b/t/t0602-reffiles-fsck.sh
> @@ -664,4 +664,46 @@ test_expect_success 'packed-refs header should be checked' '
>  	)
>  '
>  
> +test_expect_success 'packed-refs content should be checked' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		test_commit default &&
> +		git branch branch-1 &&
> +		git branch branch-2 &&
> +		git tag -a annotated-tag-1 -m tag-1 &&
> +		git tag -a annotated-tag-2 -m tag-2 &&
> +
> +		branch_1_oid=$(git rev-parse branch-1) &&
> +		branch_2_oid=$(git rev-parse branch-2) &&
> +		tag_1_oid=$(git rev-parse annotated-tag-1) &&
> +		tag_2_oid=$(git rev-parse annotated-tag-2) &&
> +		tag_1_peeled_oid=$(git rev-parse annotated-tag-1^{}) &&
> +		tag_2_peeled_oid=$(git rev-parse annotated-tag-2^{}) &&
> +		short_oid=$(printf "%s" $tag_1_peeled_oid | cut -c 1-4) &&
> +
> +		printf "# pack-refs with: peeled fully-peeled sorted \n"  >.git/packed-refs &&
> +		printf "%s\n" "$short_oid refs/heads/branch-1" >>.git/packed-refs &&
> +		printf "%sx\n" "$branch_1_oid" >>.git/packed-refs &&
> +		printf "%s   refs/heads/bad-branch\n" "$branch_2_oid" >>.git/packed-refs &&
> +		printf "%s refs/heads/branch.\n" "$branch_2_oid" >>.git/packed-refs &&
> +		printf "%s refs/tags/annotated-tag-3\n" "$tag_1_oid" >>.git/packed-refs &&
> +		printf "^%s\n" "$short_oid" >>.git/packed-refs &&
> +		printf "%s refs/tags/annotated-tag-4.\n" "$tag_2_oid" >>.git/packed-refs &&
> +		printf "^%s garbage\n" "$tag_2_peeled_oid" >>.git/packed-refs &&

This can be simplified using HERE docs.

        cat >.git/packed-refs <<-EOF
        # pack-refs with: peeled fully-peeled sorted 
        $short_oid refs/heads/branch-1
        ${branch_1_oid}x
        $branch_2_oid   refs/heads/bad-branch
        $branch_2_oid refs/heads/branch.
        $tag_1_oid refs/tags/annotated-tag-3
        ^$short_oid\n
        $tag_2_oid refs/tags/annotated-tag-4.
        ^$tag_2_peeled_oid garbage
        EOF

Patrick