On Thu, Mar 03 2022, Patrick Steinhardt wrote:
> [[PGP Signed Part:Undecided]]
> In the testcase to exercise backfilling of tags for fetches we evoke a
> failure of the backfilling mechanism by creating a reference that later
> on causes a D/F conflict. Because the assumption was that git-fetch(1)
> would notice the D/F conflict early on this conflicting reference was
> created via the reference-transaction hook just when we were about to
> write the backfilled tag. As it turns out though this is not the case,
> and the fetch fails in the same way when we create the conflicting ref
> up front.
>
> Simplify the test setup creating the reference up front, which allows us
> to get rid of the hook script.
>
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> ---
>
> This simplifies the test setup of t5503 as discussed in [1]. The patch
> applies on top of Junio's ps/fetch-atomic (583bc41923 (fetch: make
> `--atomic` flag cover pruning of refs, 2022-02-17)).
FWIW for something in "next" already the OID will be stable, so it's OK
(and better) to mention 583bc419235 in the commit message itself.
> Patrick
>
> t/t5503-tagfollow.sh | 20 +++++---------------
> 1 file changed, 5 insertions(+), 15 deletions(-)
>
> diff --git a/t/t5503-tagfollow.sh b/t/t5503-tagfollow.sh
> index e72fdc2534..a3c01014b7 100755
> --- a/t/t5503-tagfollow.sh
> +++ b/t/t5503-tagfollow.sh
> @@ -212,21 +212,11 @@ test_expect_success 'atomic fetch with backfill should use single transaction' '
> test_expect_success 'backfill failure causes command to fail' '
> git init clone5 &&
>
> - write_script clone5/.git/hooks/reference-transaction <<-EOF &&
> - while read oldrev newrev reference
> - do
> - if test "\$reference" = refs/tags/tag1
> - then
> - # Create a nested tag below the actual tag we
> - # wanted to write, which causes a D/F conflict
> - # later when we want to commit refs/tags/tag1.
> - # We cannot just `exit 1` here given that this
> - # would cause us to die immediately.
> - git update-ref refs/tags/tag1/nested $B
> - exit \$!
> - fi
> - done
> - EOF
> + # Create a tag that is nested below the tag we are about to fetch via
> + # the backfill mechanism. This causes a D/F conflict when backfilling
> + # and should thus cause the command to fail.
> + empty_blob=$(git -C clone5 hash-object -w --stdin </dev/null) &&
> + git -C clone5 update-ref refs/tags/tag1/nested $empty_blob &&
This looks better, but FWIW for the discussion about quoted
v.s. unquoted here-doc in this case it's also OK for the pre-image (and
arguably better) to do (and any issues of $? v.s. $! etc. aside):
diff --git a/t/t5503-tagfollow.sh b/t/t5503-tagfollow.sh
index e72fdc25346..eebf0ddc4c2 100755
--- a/t/t5503-tagfollow.sh
+++ b/t/t5503-tagfollow.sh
@@ -212,18 +212,18 @@ test_expect_success 'atomic fetch with backfill should use single transaction' '
test_expect_success 'backfill failure causes command to fail' '
git init clone5 &&
- write_script clone5/.git/hooks/reference-transaction <<-EOF &&
+ write_script clone5/.git/hooks/reference-transaction <<-\EOF &&
while read oldrev newrev reference
do
- if test "\$reference" = refs/tags/tag1
+ if test "$reference" = refs/tags/tag1
then
# Create a nested tag below the actual tag we
# wanted to write, which causes a D/F conflict
# later when we want to commit refs/tags/tag1.
# We cannot just `exit 1` here given that this
# would cause us to die immediately.
- git update-ref refs/tags/tag1/nested $B
- exit \$!
+ git update-ref refs/tags/tag1/nested '$B'
+ exit $!
fi
done
EOF
I.e. to end the quote and interpolate $B. IMO also more readable as
e.g. shell syntax highlighting will show that we're interpolating that
outside-test $B.
>
> test_must_fail git -C clone5 fetch .. $B:refs/heads/something &&
> test $B = $(git -C clone5 rev-parse --verify refs/heads/something) &&
(In the pre-image, but..) this way of using "test" will hide segfaults
etc. from the invoked git command.
Better to:
echo $B >expect &&
git ... >actual &&
test_cmp expect actual
Or better yet "test_cmp_rev" in this case, but all of that's in "next"
already, so purely optional (and also more food for the "hiding exit
status" microproject) :)
