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

[Nick Alcock]

On 18 Jan 2024, Kris Van Hees verbalised:

> On Mon, Jan 15, 2024 at 07:34:31PM +0000, Nick Alcock via DTrace-devel wrote:
>> +	case x$(HAVE_$(1)) in \
>> +	xyes) echo '#define HAVE_$(1) 1' > $(objdir)/.config/config.$(1).h; \
>> +	     echo 'HAVE_$(1)=y' > $(objdir)/.config/config.$(1).mk;; \
>> +	xno) echo '/* #undef HAVE_$(1) */' > $(objdir)/.config/config.$(1).h; \
>> +	     echo '# HAVE_$(1) undefined' > $(objdir)/.config/config.$(1).mk;; \
>> +	*) if printf '#include <%s.h>\nint main(void) { %s; }' "$(3)" "$(2)" | \
>> +	       $(CC) $(filter-out --coverage,$(CFLAGS) $(LDFLAGS)) -D_GNU_SOURCE -Werror=implicit-function-declaration -c -o /dev/null -x c - >/dev/null 2>&1; then \
>> +	       echo '#define HAVE_$(1) 1' > $(objdir)/.config/config.$(1).h; \
>> +	       echo 'HAVE_$(1)=y' > $(objdir)/.config/config.$(1).mk; \
>> +	   else \
>> +	       echo '/* #undef HAVE_$(1) */' > $(objdir)/.config/config.$(1).h; \
>> +	       echo '# HAVE_$(1) undefined' > $(objdir)/.config/config.$(1).mk; \
>> +	   fi;; \
>
> Would it be possible to first check if HAVE_$(1) has a value, and if not, do
> the check and set HAVE_$(1) to yes or no based on that test, and then do the
> case statement?  That was you avoid the duplication of the code handling the
> yes/no cases.

Why didn't I think of that?

> Also, the case statement now has two default clauses.  I expect that you meant

What?!

... oh sheesh, what was I thinking.

> the third case to be x) (as in: no value) and the last one remain *).  But with
> the above change I propose, that would get split in two different portions
> anyway (first see if HAVE_$(1) is set, and then have the conditional for
> yes/no/*).

Agreed. Obviously better, implemented. (Or something like that anyway --
I can't set a make variable from a recipe...)

> And of course the same de-duplication of code in the other cases in this file
> as well per the same concept.

Yep! I tried to turn the common parts into a callable, but gave up: the
result was longer than the original. It might be worth revisiting if we
ever grow more than two instances.

>> @@ -103,3 +153,6 @@ endif
>>  $(eval $(call check-header-rule,FUSE_NUMA,fuse_set_numa,fuse/fuse_lowlevel))
>>  $(eval $(call check-header-symbol-rule,CLOSE_RANGE,close_range(3,~0U,0),c,unistd))
>
> This will still give rather odd output in the help about checking for the
> presence of "close_range(3,~0U,0) libc and unistd.h") which is exposing the
> details of the check in a way that is unnecessary and a bit confusing.  I'd
> say you probably should strip of /\(.*$/ from $2 in that check so that the
> help output doesn't include the arguments that are needed for the check
> itself.

True! Or at least do it in the help output (if we did it in that actual
call, it would remove it from the test itself :) ).

>>  $(eval $(call check-header-rule,GETTID,gettid,unistd))
>> +
>> +help::
>> +	printf "\n" >&2
>> diff --git a/Makeoptions b/Makeoptions
>> index cec68a0af2bc..25234092c6dd 100644
>> --- a/Makeoptions
>> +++ b/Makeoptions
>> @@ -1,7 +1,7 @@
>>  # The implementation of the configurable make options.
>>  #
>>  # Oracle Linux DTrace.
>> -# Copyright (c) 2011, 2022, Oracle and/or its affiliates. All rights reserved.
>> +# Copyright (c) 2011, 2023, Oracle and/or its affiliates. All rights reserved.
>
> 2024

Tempus fugit, and now I have my got-the-year-wrong for this year (fixed).

-- 
NULL && (void)