Re: [PATCH] t/perf/run: Don't forget to copy config.mak.autogen & friends to build area
- From
Kirill Smelkov <kirr@nexedi.com>
- Date
- Sep 13, 2016, 06:58 UTC
- Message-ID
- <20160913065800.cpye2rqgizszgsuc@teco.navytux.spb.ru>
- In-Reply-To
- <xmqqpoo8yasu.fsf@gitster.mtv.corp.google.com>
On Mon, Sep 12, 2016 at 04:10:09PM -0700, Junio C Hamano wrote:
Show 30 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
>
> > In other words, something along this line, perhaps.
> > ...
>
> Not quite. There is no guanratee that the user is using autoconf at
> all. It should be more like this, I think.
>
> t/perf/run | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/t/perf/run b/t/perf/run
> index aa383c2..7ec3734 100755
> --- a/t/perf/run
> +++ b/t/perf/run
> @@ -30,7 +30,13 @@ unpack_git_rev () {
> }
> build_git_rev () {
> rev=$1
> - cp -t build/$rev ../../{config.mak,config.mak.autogen,config.status}
> + for config in config.mak config.mak.autogen config.status
> + do
> + if test -f "../../$config"
> + then
> + cp "../../$config" "build/$rev/"
> + fi
> + done
> (cd build/$rev && make $GIT_PERF_MAKE_OPTS) ||
> die "failed to build revision '$mydir'"
> }Junio, thanks for encouraging feedback and for catching the *-isms. What you propose is good (and we also automatically fix error when there was no config.mak - it was working but cp was giving an error to stderr but script was continuing normally).
I would amend your squash the following way:
* `test -f` -> `test -e`, because -f tests whether a file exists _and_ is regular file. Some people might have config.mak as a symlink for example. We don't want to miss them too.
Please find updated patch below:
---- 8< ---- From: Kirill Smelkov <kirr@nexedi.com> Subject: [PATCH] t/perf/run: Don't forget to copy config.mak.autogen & friends to build area
Otherwise for people who use autotools-based configure in main worktree, the performance testing results will be inconsistent as work and build trees could be using e.g. different optimization levels.
See e.g.
http://public-inbox.org/git/20160818175222.bmm3ivjheokf2qzl@sigill.intra.peff.net/
for example.
NOTE config.status has to be copied because otherwise without it the build would want to run reconfigure this way loosing just copied config.mak.autogen.
Signed-off-by: Kirill Smelkov <kirr@nexedi.com> --- t/perf/run | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/t/perf/run b/t/perf/run index cfd7012..e8adeda 100755 --- a/t/perf/run +++ b/t/perf/run @@ -30,7 +30,13 @@ unpack_git_rev () { } build_git_rev () { rev=$1 - cp ../../config.mak build/$rev/config.mak + for config in config.mak config.mak.autogen config.status + do + if test -e "../../$config" + then + cp "../../$config" "build/$rev/" + fi + done (cd build/$rev && make $GIT_PERF_MAKE_OPTS) || die "failed to build revision '$mydir'" }
-- 2.9.2.701.gf965a18.dirty