On Thu, Apr 24, 2025 at 04:13:15AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@xxxxxx> writes:
>
> >> I was more expecting something simple like:
> >>
> >> if time.found()
> >> ...
> >> else
> >> error('Benchmarking requires the `time` command')
> >> endif
> >>
> >> in the same way as in meson.build elsewhere we have things like:
> >>
> >> if not msgfmt.found() and gettext_option.enabled()
> >> error('Internationalization via libintl requires msgfmt')
> >> endif
> >
> > But erroring out by default doesn't really feel nice to the general
> > developer. I'd claim that 99% of the time, developers will only end up
> > running the test suite, never the benchmarks. So the default should
> > match that and not require GNU time to be available, if you ask me.
>
> Maybe a stupid question, but does "time" a shell built-in count when
> "if time.found()" is evaluated?
>
> $ type --all time
> time is a shell keyword
> time is /usr/bin/time
> time is /bin/time
No, it doesn't, and that is intentional. Our benchmarks explicitly
require GNU time because it surfaces more information than the shell
builtin.
We even have a `USR_BIN_TIME` prereq in our test lib, which is
unfortunately way too limiting for some platforms like NixOS. Maybe I
should use that as an excuse to also refactor this prereq while at it to
instead use a path-based lookup of time. On the other hand it's only
used by a single test anyway, so it doesn't really feel worth it.
Patrick
