[DTrace-devel] [PATCH 1/5] build: overridable configuration variables

[Nick Alcock]

On 12 Dec 2023, Eugene Loh via DTrace-devel outgrape:

> Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
> subject to comments below.  (And, this whole stuff is pretty foreign to me.)

It's just make :) ("just", he says.)

> On 11/29/23 11:08, Nick Alcock via DTrace-devel wrote:
>> The Makeconfig checks are purely compile-time, so should always work: but if
>> they don't, it is sometimes desirable to be able to override them.
>
> I'm really puzzled by this text.  "Should always work", but if the impossible happens...?  Is there a clearer way to state the case
> for this patch?

But if there is a bug, if the configure tests misfire, if someone wants
to use fuse 2 even though fuse 3 is also on the system, I don't really
think I can enumerate every possible case :) perhaps "but if they choose
an option the user dislikes".

Rephrased a bit:

| The Makeconfig checks are purely compile-time, so should always work: but
| sometimes there are multiple choices, and if they choose an option the user
| dislikes, the user might well want to override them (e.g. picking FUSE 2
| even though FUSE 3 is already present).

>> This change causes every check-* invocation in Makeconfig to respond to
>> HAVE_* make variables set on the command line, and also to produce a line in
>> 'make help' of the general form
>>
>> HAVE_LIBSYSTEMD=[yes/no]	Override check for presence of sd_notify in libsystemd
>>
>> etc.
>>
>> (The escaping in make-override-help to figure out how many tabs to insert to
>> properly align the help will make your eyes bleed. One $ for every nested
>> $(eval $(call ...)): thank goodness they're all at the same level!)
>>
>> Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
>> ---
>>   Makeconfig  | 101 +++++++++++++++++++++++++++++++++++++++-------------
>>   Makeoptions |  13 ++++---
>>   2 files changed, 83 insertions(+), 31 deletions(-)
>>
>> diff --git a/Makeconfig b/Makeconfig
>> index 08cb48101f9a..f6a28d8f79f5 100644
>> --- a/Makeconfig
>> +++ b/Makeconfig
>> @@ -10,6 +10,23 @@
>>   CONFIG_H = $(objdir)/config.h
>>   CONFIG_MK = $(objdir)/config.mk
>>   +# Emit a help rule for an override.
>> +#
>> +# Syntax: $(call make-overrride-help,var,help-string)
>
> s/rrr/rr/

rrrr!

>> +define make-override-help
>> +help::
>
> Patch 4 introduces help-overrides.  I don't understand the big picture here, but might it not make sense to do the help-overrides
> stuff here in patch 1 rather than introducing stuff that will be changed a few patches later?

Good point. I'll shift the overrides patch to come right after this one:
I think that's still a bit clearer than jamming them together, since
they are a bit conceptually separate.

>> +	width=$$$$(printf '$(1)=[yes/no]' | wc -c); \
>> +	tabs=""; \
>
> Why isn't the default \t\t\t\t?  Arguably, it shouldn't matter since that case might never come up, but it'd be more straightforward
> that way.

Augmented to allow for that, and \t\t\t as well (and yes, they weren't
there because no existing text used them, but that's short-sighted.)

>> +	*) if printf '#include <%s.h>\nint main(void) { %s; }' "$(3)" "$(2)" | \
>
> Should this case be x) rather than *)?

Absolutely!

>> diff --git a/Makeoptions b/Makeoptions
>
> Okay though Makeoptions might as well be a separate patch.  In favor of separate patch:  it's unrelated to this patch.  In favor of
> leaving as is:  it's not a big deal one way or the other.
[...]
>> -	@printf "make debugging=yes [targets]   Disable optimization to make debugger use easier\n" >&2
[...]
>> +	@printf "make debugging=yes [targets]\tDisable optimization to make debugger use easier\n" >&2

We're turning spaces into tabs. If you don't do this at the same time as
the above, the help output looks ugly because the options are separated
by a different distance from the newly autogenerated stuff -- so it
actually *is* related.