From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
git@vger.kernel.org, Patrick Steinhardt <ps@pks.im>,
Johannes Schindelin <johannes.schindelin@gmx.de>,
Taylor Blau <me@ttaylorr.com>
Subject: Re: [PATCH] perf: do allow `GIT_PERF_*` to be overridden again
Date: Sun, 20 Apr 2025 14:12:34 -0700 [thread overview]
Message-ID: <xmqqfri28rlp.fsf@gitster.g> (raw)
In-Reply-To: <20250419035407.GA93039@coredump.intra.peff.net> (Jeff King's message of "Fri, 18 Apr 2025 23:54:07 -0400")
Jeff King <peff@peff.net> writes:
> 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.
next prev parent reply other threads:[~2025-04-20 21:12 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-04 10:56 [PATCH] perf: do allow `GIT_PERF_*` to be overridden again Johannes Schindelin via GitGitGadget
2025-04-04 12:13 ` Derrick Stolee
2025-04-19 3:54 ` Jeff King
2025-04-20 21:12 ` Junio C Hamano [this message]
2025-04-22 10:41 ` Jeff King
2025-04-22 15:37 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqfri28rlp.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=johannes.schindelin@gmx.de \
--cc=me@ttaylorr.com \
--cc=peff@peff.net \
--cc=ps@pks.im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.