git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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
Previous: Junio C HamanoNext: Junio C Hamano
Message 11 of 14 in “git init BUG when gitconfig has includeIf”
  1. Heghedus RazvanMay 21, 2024
  2. Junio C HamanoMay 21, 2024
  3. Heghedus RazvanMay 21, 2024
  4. Patrick SteinhardtMay 22, 2024
  5. Heghedus RazvanMay 22, 2024
  6. Patrick SteinhardtMay 22, 2024
  7. setup: fix bug with "includeIf.onbranch" when initializing dirPatrick Steinhardt, May 22, 2024
  8. Heghedus RazvanMay 22, 2024
  9. Junio C HamanoMay 22, 2024
  10. Junio C HamanoMay 23, 2024
  11. Patrick SteinhardtMay 23, 2024
  12. setup: fix bug with "includeIf.onbranch" when initializing dirJunio C Hamano, May 23, 2024
  13. [rPATCH v2] setup: fix bug with "includeIf.onbranch" when initializing dirJunio C Hamano, May 23, 2024
  14. Patrick SteinhardtMay 23, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.