Re: bug in en/header-split-cache-h-part-3, was Re: What's cooking in git.git (Jun 2023, #05; Tue, 20)

From: Jeff King <peff@peff.net>
Date: Jun 21, 2023, 20:26 UTC
Message-ID: <20230621202642.GA1423@coredump.intra.peff.net>
In-Reply-To: <xmqqttv0hhjv.fsf@gitster.g>

On Wed, Jun 21, 2023 at 10:05:40AM -0700, Junio C Hamano wrote:

Show 10 quoted lines

> > So I think we'd at least want to fix the Makefile before graduating this
> > topic any further. But IMHO it would also be worth adjusting the topic's
> > start point so that we don't have a big chunk of commits which fail to
> > build in the final history.
> 
> Hmph, meaning (1) revert the merge of the topic to 'next', (2)
> rebase the topic on top of the current 'master', instead of 4bd872e0,
> which was a merge of the prerequisite series into then-current
> master, (3) apply the Makefile (plus setup.c) fix, and then (4)
> merge the result back to 'next'?

Yeah. I guess the real build problem is actually in the merge of split-2 (it conflicted with a simultaneous topic, hence the fix coming in the merge). So another option to address that here would be to amend the 4bd872e0ed (Merge branch 'en/header-split-cache-h-part-2' into en/header-split-cache-h-part-3, 2023-05-08) to include that fixup.

As for the others, I'd consider:

  1. (optional) Drop the #ifndef at the very start of the series, before
     we touch anything, with the rationale that it is not doing anything
     and masks errors. I don't _think_ this can ever backfire, because
     we unconditionally set DEFAULT_GIT_TEMPLATE_DIR (unlike some other
     things like DEFAULT_PAGER, where the Makefile might leave it
     unset). But we can also leave this out, or do it as a separate
     topic, if we want to minimize changes / risk of screwing something
     up.

  2. Squash the Makefile fix into the "adopt shared init-db" patch
     (currently 0d652b238).

And that would leave the result fully bisectable. But if we prefer to keep the history closer to reality, I can prepare the Makefile thing as a patch on top.

-Peff