Hi Junio, On Sun, 27 Jan 2019, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes: > > > No, not really. Actually, not at all. > > > >> It would be, under this new arrangement of the code, most natural for > >> Those who want to use gnu-time to arrange it to be somehow added to > >> $BREW_INSTALL_PACKAGES, no? > > > > The purpose of BREW_INSTALL_PACKAGES is to list the packages necessary to > > build Git and run its test suite, and the only reason why this is no > > longer a hard-coded list of packages is that it depends on the CI platform > > (or more concretely, on the available macOS agents of said CI platform) > > which packages need to be installed to do so. > > > > The gnu-time package is not such a package, and it is unlikely to be > > dependent on the particular CI you want to use. > > Those who want to do perf tests in the current setup would need to > install gnu-time because the current setup is only Travis, whose > macOS agent does not have it preinstalled. Other CI platforms' > macOS agents may already have it, they may not want to get an error > by trying to install it there. I am not sure how that is different > from the situation for gettext etc.? The big difference is that gettext is needed to build Git and run its test suite. While gnu-time is only needed if you want to run the perf tests, which is not a part of the CI configuration we have, neither Travis nor Azure Pipelines. So as long as we do not run the perf tests as part of the CI runs, that optional dependency should *not* be included in *CI_TYPE* specific sections of the code. Since the perf test reference in this comment that you keep talking about is so clearly intended for some human being who wants to run the scripts in ci/ interactively (which pretty much contradicts the "ci" in the name a bit), I would even argue that it already is too hidden in the depths of the scripts to be useful. But sticking it even deeper into the CI_TYPE-specific sections? That would make it *even harder* to find! So no, I think this suggestion to move that comment into exactly those sections (and of course, repeat it, identically, because with this here patch series we support *two* CI types) is something that we really want to let slide. Ciao, Dscho
