On Wed, May 22, 2024 at 05:41:28PM -0700, Junio C Hamano wrote: > Patrick Steinhardt <ps@xxxxxx> 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:
signature.asc
Description: PGP signature