Re: [PATCH v2 2/2] perf-lib.sh: remove GIT_TEST_INSTALLED from perf-lib.sh
- Date: Tue, 7 May 2019 17:23:15 -0400
- From: Jeff King <peff@xxxxxxxx>
- Subject: Re: [PATCH v2 2/2] perf-lib.sh: remove GIT_TEST_INSTALLED from perf-lib.sh
On Tue, May 07, 2019 at 10:31:23AM +0200, Ævar Arnfjörð Bjarmason wrote:
> > I'm not sure I get what's going on here. Why do we need the realpath in
> > aggregate.perl? We'd want to generate the same filename that "run"
> > decided to store things in, which we'd generate from the command-line
> > arguments (either passed on to us by "run", or direct from the user if
> > they're printing a previous run).
> So this is part of the "has sucked since forever, future TODO" mentioned
> in 0/2.
> I.e. if you pass "../.." as a path to "run" we'll try to discover a
> built/installed "git" in a "bindir" there, and then we need to do two
> 1. Figure out a way to turn that into a filename sensible for the
> *.times files.
> 2. Print some header showing that path in the aggregate output.
> The "run" script will discover #1 for itself, that's what that "pwd &&
> tr -c ..." command is doing, but then we just pass "../.." again to
> aggregate.perl and have it figure it out again on its own, so it needs
> to duplicate the logic.
> Just having both discover the absolute path all the time for #1 made
> things a lot simpler, e.g. if you do ../.. on v2.21.0 you'll get things
> And with $PWD/../../ you'd get:
> Now this is all pretty & consistent. Any path to a "git" will always be
> turned into the absolute path, e.g.:
> And instead of "git" or ".." being printed in the aggregate header we
> print the path, e.g. "/home/avar/g/git".
OK. I sort of assumed we'd be sticking with the crappy "_____" for both
cases after your cleanup. But it really is changing behavior to name
things after the absolute path. I'd probably have split that out into
its own change, but I don't think it's worth revisiting at this point.
Thanks for explaining.