Re: [PATCH] setup: fix bug with "includeIf.onbranch" when initializing dir
- From
Patrick Steinhardt <ps@pks.im>
- Date
- May 23, 2024, 05:26 UTC
- Message-ID
- <Zk7Tej-AII3Ge6Ge@tanuki>
- In-Reply-To
- <xmqq4jap1hs7.fsf@gitster.g>
On Wed, May 22, 2024 at 05:41:28PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
[snip]
Show 24 quoted lines
> > +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.
Show 24 quoted lines
> > + 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