Subject: Re: [PATCH v3 0/5] meson: wire up support for benchmarks

From: Patrick Steinhardt <[email-masked]>
Date: Thu, 24 Apr 2025 06:31:33 +0200

On Wed, Apr 23, 2025 at 04:44:20PM +0200, Christian Couder wrote:
> On Tue, Apr 22, 2025 at 9:53 AM Patrick Steinhardt <[email-masked]> wrote:
> >
> > On Tue, Apr 22, 2025 at 09:27:57AM +0200, Christian Couder wrote:
> 
> > > In https://lore.kernel.org/git/CAP8UFD2jKwYzmc40knXY7k+FQabjZbGTqs9fowF=[email-masked]/
> > > I wrote:
> > >
> > > "I wonder what happens when we run `meson test -C build --benchmark`
> > > but 'time' is not found."
> > >
> > > because I wasn't sure if it would just do nothing in that case which
> > > might not be very user friendly.
> >
> > Ah, sorry, forgot to answer that question. What Meson does in that case
> > is to print "No tests defined". We could help improve usability a bit by
> > printing benchmarks as part of the auto-detected features after setup of
> > the build directory has finished. That would make it more discoverable
> > that benchmarks have been disabled.
> 
> 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.

An alternative could be to have a 'benchmarks' feature option wired up.
This feature would be set to 'auto', and then we can detect time like
this:

    time = find_program('time', dirs: program_path, required: get_option('benchmarks'))

This will then auto-enable or -disable the feature depending on the
availability of GNU time. But if the user passes `-Dbenchmarks=enabled`,
we would fail the setup in case the program wasn't found.

Patrick