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

From: Elijah Newren <newren@gmail.com>
Date: Jun 23, 2023, 06:33 UTC
Message-ID: <CABPp-BEQ0_UfUbdeFetCsvAnpO_=mvmjQk8JS0trKJtCL=uh1A@mail.gmail.com>
In-Reply-To: <xmqqjzvwfp6f.fsf@gitster.g>

On Wed, Jun 21, 2023 at 3:03 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 41 quoted lines

>
> Jeff King <peff@peff.net> writes:
>
> > 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.
>
> I've done the messiest, I guess ;-)
>
>  * revert merge of the part-3 topic and Dscho's cmake fix out of 'next'.
>
>  * rebase part-3 on top of the more recent 'master'.
>
>  * squash in the two hunks (including setup.c change) from you into
>    the "setup: adopt shared init-db & clone code" step.
>
>  * squash in Dscho's cmake fix into "cache.h: remove this
>    no-longer-used header" step.
>
> The result is not in 'next' yet until I hear something from those
> who have been involved in the topic, including Elijah and Dscho.

I did a range-diff to compare my original series to your newly rebased one, and read through all the differences (including Dscho's and Peff's suggested changes, as well as the various slight adjustments due to rebasing). I also rebuilt every patch in your rebase of the series to ensure they all build, and ran all tests on a couple of the patches to verify the pass (I didn't run all tests for each patch, because I did that for my original series and the differences in the range-diff suggested I only needed to spot check all the tests).

Anyway, your rebase of en/header-split-cache-h-part-3, including Dscho's and Peff's changes, all look good to me.

Thanks everyone!