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

From: Patrick Steinhardt <ps@pks.im>
Date: Apr 24, 2025, 04:31 UTC
Message-ID: <aAm-pcIfrFt6mjlV@pks.im>
In-Reply-To: <CAP8UFD1zqAk1_Y0ECu4=iyeF4k4LagfLgHWkEr8iz8ZnwkVT8A@mail.gmail.com>

On Wed, Apr 23, 2025 at 04:44:20PM +0200, Christian Couder wrote:

Show 32 quoted lines

> On Tue, Apr 22, 2025 at 9:53 AM Patrick Steinhardt <ps@pks.im> wrote:
> >
> > On Tue, Apr 22, 2025 at 09:27:57AM +0200, Christian Couder wrote:
> 
> > > In https://lore.kernel.org/git/CAP8UFD2jKwYzmc40knXY7k+FQabjZbGTqs9fowF=-0OqfNYp_w@mail.gmail.com/
> > > 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