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

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> 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

> 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.

Sounds quite sensible.  Do what most users expect by default without
failing, let users give explicit preference, and when the explicitly
expressed preference cannot be fulfilled, loudly error out.  I like
it.

Thanks.