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
