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

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 1789 bytes --]

On Wed, Mar 02, 2022 at 04:32:48PM -0800, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Patrick Steinhardt <ps@pks.im> 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.

Oof, this was supposed to be `exit \$?`, thanks for catching this. But
your above comment is right: we can indeed just create the D/F conflict
outside of the hook and thus avoid the hook script altogether. Thanks!

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]