Subject: Re: [PATCH v3 1/7] fetch: increase test coverage of fetches

From: Junio C Hamano <[email-masked]>
Date: Wed, 02 Mar 2022 16:32:48 -0800

Junio C Hamano <[email-masked]> writes:

> Patrick Steinhardt <[email-masked]> writes:
>
>> +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
>
> I think I've reviewed the previous round of these patches in
> detail.  I by mistake sent a comment for this step in v2, but I
> think the same puzzlement exists in this round, too.

Namely:

I have been wondering if we need to do this from the hook?  If we
have this ref before we start "fetch", would it have the same
effect, or "fetch" notices that this interfering ref exists and
removes it to make room for storing refs/tags/tag1, making the whole
thing fail to fail?

> +				exit \$!

In any case, "exit 0" or "exit \$?" would be understandable, but
exit with "$!", which is ...?  The process ID of the most recent
background command?  Puzzled.