Re: [PATCH v2 2/2] perf-lib.sh: remove GIT_TEST_INSTALLED from perf-lib.sh

[Jeff King <peff@peff.net>]

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
> things:
> 
>  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
> like:
> 
>     _____.p0000-perf-lib-sanity.1.times
> 
> And with $PWD/../../ you'd get:
> 
>     _home_avar_g_git_t_perf______.p0000-perf-lib-sanity.1.times
> 
> Now this is all pretty & consistent. Any path to a "git" will always be
> turned into the absolute path, e.g.:
> 
>     _home_avar_g_git.p0000-perf-lib-sanity.1.times
> 
> 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.

-Peff