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

From: Jeff King <peff@peff.net>
Date: Apr 22, 2025, 10:41 UTC
Message-ID: <20250422104125.GA1460472@coredump.intra.peff.net>
In-Reply-To: <xmqqfri28rlp.fsf@gitster.g>

On Sun, Apr 20, 2025 at 02:12:34PM -0700, Junio C Hamano wrote:

Show 13 quoted lines

> > 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.

Looks like this is in 'jch' now, but there's a mis-merge in e8cf2b99cd (Merge branch 'ps/meson-build-perf-bench' into jch, 2025-04-21).

The original patch in this thread did something like:

  git_perf_settings=$(...pull GIT_PERF_* from env...)
   . ../test-lib.sh
  eval "$git_perf_settings"

That is, we stash away the environment, then load test-lib.sh, which overwrites the environment, and then we restore (some of) the original values.

In that merge, the test-lib.sh inclusion is moved (and in fact is now accompanied by an explicit inclusion of GIT-BUILD-OPTIONS), and we now have:

   . "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS
   . "$GIT_SOURCE_DIR"/t/test-lib.sh
   git_perf_settings=$(...)
   eval "$git_perf_settings"

Which of course does nothing. We need to set $git_perf_settings before those other source lines (and the eval must remain after them).

-Peff