git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v1 0/4] Teach 'run' perf script to read config files

From
Jeff King <peff@peff.net>
Date
Jul 26, 2017, 16:54 UTC
Message-ID
<20170726165427.xh6ykoxjdibfqasp@sigill.intra.peff.net>
In-Reply-To
<CAP8UFD2NNBN=6GHbQPjz19hQUb+k_43YZBKimaA=3M6m4RH7Tw@mail.gmail.com>
On Wed, Jul 26, 2017 at 05:58:09PM +0200, Christian Couder wrote:
Show 8 quoted lines
> Actually after taking another look at that, it looks like the following happens:
> 
> 1) the run script sources the original GIT-BUILD-OPTIONS file from
> ../.. relative to its location
> 2) a git version is built in "build/$rev" using GIT_PERF_MAKE_OPTS
> which generates a new GIT-BUILD-OPTIONS file in "build/$rev/"
> 3) when the actual perf scripts are run they source the original
> GIT-BUILD-OPTIONS file (through perf-lib.sh which sources test-lib.sh)

Right, the perf scripts are run in the context of the "outer" repository, and get their options from that one. I think that's intentional, and does the right thing for GIT_PERF_* options. It's possibly confusing if the tests really do want to know about the build options for a particular test-build (like asking if it was built with NO_PERL, for example).

I think in practice it works out OK, because we tend to do test-builds that are similar to what's in the outer repo (because we copy config.mak, and don't tend to add a lot of command-line options). But if you put exotic options into your GIT_PERF_MAKE_OPTS, they won't be reflected in the config that the test scripts see.

Show 6 quoted lines
> I wonder how useful 1) is, as the variables sourced from original
> GIT-BUILD-OPTIONS are not used inside the "run" script and not
> available to its child processes as they are not exported.
> Is it just so that if people add GIT_PERF_* variables to their
> config.mak before building they can then have those variables used by
> the run script?

Exactly. I put GIT_PERF_MAKE_OPTS in my config.mak, and they end up respected for each run without me having to specify them manually.

Show 7 quoted lines
> I also wonder if it would be better at step 3) to source the
> GIT-BUILD-OPTIONS file generated at step 2) instead of the original
> one, because they can be different as the options in
> $GIT_PERF_MAKE_OPTS will be baked into the new GIT-BUILD-OPTIONS file.
> (Of course if $GIT_PERF_MAKE_OPTS was added to config.mak before
> building, then they will be in the original one too. But
> $GIT_PERF_MAKE_OPTS should work without that.)

I think that would make some cases work (build options for the tested build), but I fear that it would break others (perf variables that probably should be coming from the "outer" layer). Remember that test builds may not be current versions and may not be forwarding those variables via GIT-BUILD-OPTIONS. I think it's important that the bundle of t/perf scripts act as a single unit that is driven primarily by the currently checked-out version (and it's up to those scripts to handle inconsistencies in old versions; see the $MODERN_GIT stuff I added a few months ago.

Right now I don't think it has been a big problem, because the build config tends to be the same. But if we introduce more "properties" that the user can tweak for a certain test run, this distinction is probably going to cause more bugs. I'd almost say that the perf scripts should be a project outside of git.git entirely, to eliminate confusion.

-Peff
Previous: Christian Couder
Message 16 of 16 in “Teach 'run' perf script to read config files”
  1. 0/4 Teach 'run' perf script to read config filesChristian Couder, Jul 13, 2017
  2. 2/4 perf/run: add get_var_from_env_or_config()Christian Couder, Jul 13, 2017
  3. 1/4 perf/run: add '--config' option to the 'run' scriptChristian Couder, Jul 13, 2017
  4. 3/4 perf/run: add GIT_PERF_DIRS_OR_REVSChristian Couder, Jul 13, 2017
  5. 4/4 perf/run: add calls to get_var_from_env_or_config()Christian Couder, Jul 13, 2017
  6. Jeff KingJul 13, 2017
  7. Junio C HamanoJul 13, 2017
  8. Jeff KingJul 13, 2017
  9. Junio C HamanoJul 13, 2017
  10. Christian CouderJul 13, 2017
  11. Christian CouderJul 13, 2017
  12. Jeff KingJul 13, 2017
  13. Christian CouderJul 14, 2017
  14. Jeff KingJul 14, 2017
  15. Christian CouderJul 26, 2017
  16. Jeff KingJul 26, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.