On Wed, Feb 12, 2025 at 10:56:50AM +0100, Patrick Steinhardt wrote:
> On Thu, Feb 06, 2025 at 01:59:40PM +0800, shejialuo wrote:
> > 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.
>
I agree with you here. And I already have idea to do this. Let me
improve this in the next version.
> > 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
>
Thanks for the suggestion, I will improve this in the next version.
> Patrick
