Re: [PATCH] perf: do allow `GIT_PERF_*` to be overridden again

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 20, 2025, 21:12 UTC
Message-ID: <xmqqfri28rlp.fsf@gitster.g>
In-Reply-To: <20250419035407.GA93039@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 12 quoted lines

> On Fri, Apr 04, 2025 at 10:56:07AM +0000, Johannes Schindelin via GitGitGadget wrote:
>
>> However, in 4638e8806e3a (Makefile: use common template for
>> GIT-BUILD-OPTIONS, 2024-12-06), a subtle change of behavior was
>> introduced: Whereas before, a couple of build-time options (the
>> `GIT_PERF_*` ones included) were written to `GIT-BUILD-OPTIONS` only
>> when their values were non-empty. With this commit, they are also
>> written when they are empty.
>
> It doesn't look like Junio picked this up, so I wanted to chime in that
> this regression bit me today, too (specifically for GIT_PERF_LARGE_REPO,
> but also another variable which I'll detail in a moment).

This was lost in the cracks. Thanks for bringing it back to our attention. I think what happened was that I saw whack-a-mole aspect of the root cause, which makes this "the tip of the iceberg", and felt it was more sensible to wait before a real solution, like ...

> So I think we either need to rewrite the "run" script's fallback code,
> or teach the GIT-BUILD-OPTIONS writer to avoid mentioning unset
> variables (which is the real source of the problem in 4638e8806e3a).

... this was raised. And then I completely forgot about the topic, as nothing happened since then.

> ...yes, this is definitely the tip of the iceberg. I don't mind doing
> this patch as an incremental step forward (and because it is an
> improvement in behavior even if 4638e8806e3a were reverted). But the
> issue is far from solved overall.

I do not mind it as an incremental band-aid.

Thanks, all.