On Wed, Jul 26, 2017 at 05:58:09PM +0200, Christian Couder wrote: > 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. > 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. > 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
