Re: [PATCH] setup: fix bug with "includeIf.onbranch" when initializing dir

[Patrick Steinhardt <ps@pks.im>]

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

On Wed, May 22, 2024 at 05:41:28PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
[snip]
> > +for from_format in files reftable
> > +do
> > +	for to_format in files reftable
> > +	do
> > +		if test "$from_format" = "$to_format"
> > +		then
> > +			continue
> > +		fi
> > +
> > +		test_expect_success "re-init with same format ($from_format)" '
> > +			test_when_finished "rm -rf refformat" &&
> > +			git init --ref-format=$from_format refformat &&
> > +			git init --ref-format=$from_format refformat &&
> > +			echo $from_format >expect &&
> > +			git -C refformat rev-parse --show-ref-format >actual &&
> > +			test_cmp expect actual
> > +		'
> 
> This is very iffy, isn't it?  This one wants to do the same format
> reinit, but it is behind "if from and to are the same, skip the rest"
> in the loop.
> 
> I think the "same format" loop should be lifted outside and
> immediately before the inner loop and we should be OK.

Yup, we can do that.

> > +		test_expect_success "re-init with different format fails ($from_format -> $to_format)" '
> > +			test_when_finished "rm -rf refformat" &&
> > +			git init --ref-format=$from_format refformat &&
> > +			cat >expect <<-EOF &&
> > +			fatal: attempt to reinitialize repository with different reference storage format
> > +			EOF
> > +			test_must_fail git init --ref-format=$to_format refformat 2>err &&
> > +			test_cmp expect err &&
> > +			echo $from_format >expect &&
> > +			git -C refformat rev-parse --show-ref-format >actual &&
> > +			test_cmp expect actual
> > +		'
> > +	done
> > +done
> 
> In any case, this "reinitialize to a different format is too late"
> test has nothing to do with the problem we are fixing with this
> patch, no?  It is a valuable set of tests in the post b4385bf0 world
> where we can choose between the two, but it is somewhat out of scope
> of the "we need to initialize the ref backend before we can do onbranch
> config".
> 
> I'll send your patch backported to v2.44.0, plus the change needed
> to review the merge of it into v2.45.0 codebase later.

Yeah, I wanted to play it safe and add some cases I felt were missing in
the existing test coverage so that there ideally aren't any other issues
and so that the proposed change doesn't regress functionality.

Patrick

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