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

Eugene Loh eugene.loh at oracle.com
Tue Dec 12 23:30:18 UTC 2023


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

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?

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

> +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?

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

> +	if [[ $$$${width} -gt 31 ]]; then \
> +	    tabs=' '; \
> +	elif [[ $$$${width} -gt 23 ]]; then \
> +	    tabs='\t'; \
> +	elif [[ $$$${width} -gt 15 ]]; then \
> +	    tabs='\t\t'; \
> +	fi; \
> +	printf "$(1)=[yes/no]$$$${tabs}Override check for$(2)\n" >&2
> +endef
> +
>   # Generate a makefile rule to check for the presence of FUNCTION
>   # in HEADER and emit an appropriate header file fragment into a
>   # file under $(objdir)/.config.
> @@ -20,14 +37,24 @@ CONFIG_MK = $(objdir)/config.mk
>   # Syntax: $(call check-header-rule,name,function,header)
>   define check-header-rule
>   $(objdir)/.config/config.$(1).h $(objdir)/.config/config.$(1).mk: $(objdir)/.config/.dir.stamp
> -	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
> +	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)" | \

Should this case be x) rather than *)?

> +	       $(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;; \
> +	*) echo "HAVE_$(1) must be yes or no, not $(HAVE_$(1))" >&2; \
> +	   false;; \
> +	esac
> +
> +$(eval $(call make-override-help,HAVE_$(1), presence of $(2) in $(3).h))
>   
>   $(CONFIG_H): $(objdir)/.config/config.$(1).h
>   $(CONFIG_MK): $(objdir)/.config/config.$(1).mk
> @@ -43,14 +70,24 @@ endef
>   # Syntax: $(call check-symbol,name,symbol,library)
>   define check-symbol-rule
>   $(objdir)/.config/config.$(1).h $(objdir)/.config/config.$(1).mk: $(objdir)/.config/.dir.stamp
> -	if echo 'void $(2)(); int main(void) { $(2)(); }' | \
> -		$(CC) $(filter-out --coverage,$(CFLAGS) $(LDFLAGS)) -o /dev/null -x c - -l$(3) >/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
> +	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;; \
> +	x) if echo 'void $(2)(); int main(void) { $(2)(); }' | \
> +	       $(CC) $(filter-out --coverage,$(CFLAGS) $(LDFLAGS)) -o /dev/null -x c - -l$(3) >/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;; \
> +	*) echo "HAVE_$(1) must be yes or no, not $(HAVE_$(1))" >&2; \
> +	   false;; \
> +	esac
> +
> +$(eval $(call make-override-help,HAVE_$(1), presence of $(2) in lib$(3)))
>   
>   $(CONFIG_H): $(objdir)/.config/config.$(1).h
>   $(CONFIG_MK): $(objdir)/.config/config.$(1).mk
> @@ -66,14 +103,24 @@ endef
>   # Syntax: $(call check-header-symbol-rule,name,symbol,library,header)
>   define check-header-symbol-rule
>   $(objdir)/.config/config.$(1).h $(objdir)/.config/config.$(1).mk: $(objdir)/.config/.dir.stamp
> -	if printf '#include <%s.h>\nint main(void) { %s; }' "$(4)" "$(2)" | \
> -		$(CC) $(filter-out --coverage,$(CFLAGS) $(LDFLAGS)) -D_GNU_SOURCE -Werror=implicit-function-declaration -o /dev/null -x c - -l$(3) >/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
> +	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; }' "$(4)" "$(2)" | \

Should this case be x) rather than *)?

> +	       $(CC) $(filter-out --coverage,$(CFLAGS) $(LDFLAGS)) -D_GNU_SOURCE -Werror=implicit-function-declaration -o /dev/null -x c - -l$(3) >/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;; \
> +	*) echo "HAVE_$(1) must be yes or no, not $(HAVE_$(1))" >&2; \
> +	   false;; \
> +	esac
> +
> +$(eval $(call make-override-help,HAVE_$(1), presence of $(2) in lib$(3) and $(4).h))
>   
>   $(CONFIG_H): $(objdir)/.config/config.$(1).h
>   $(CONFIG_MK): $(objdir)/.config/config.$(1).mk
> @@ -92,6 +139,9 @@ $(CONFIG_MK):
>   	echo 'CONFIGURED := yes' >> $(objdir)/config.mk
>   	cat $(objdir)/.config/*.mk >> $(objdir)/config.mk 2>/dev/null || true
>   
> +help::
> +	printf "Overrides for library and symbol searches:\n\n" >&2
> +
>   $(eval $(call check-symbol-rule,ELF_GETSHDRSTRNDX,elf_getshdrstrndx,elf))
>   $(eval $(call check-symbol-rule,LIBCTF,ctf_open,ctf))
>   $(eval $(call check-symbol-rule,STRRSTR,strrstr,c))
> @@ -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))
>   $(eval $(call check-header-rule,GETTID,gettid,unistd))
> +
> +help::
> +	printf "\n" >&2
> 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.

> 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.
>   # Licensed under the Universal Permissive License v 1.0 as shown at
>   # http://oss.oracle.com/licenses/upl.
>   
> @@ -13,13 +13,12 @@ libfuse2 ?= no
>   
>   help::
>   	@printf "Options:\n\n" >&2
> -	@printf "make debugging=yes [targets]   Disable optimization to make debugger use easier\n" >&2
> -	@printf "make coverage=yes [targets]    Turn on coverage support in the testsuite\n" >&2
> -	@printf "make verbose=yes [target]      Enable verbose building\n" >&2
> -	@printf "make dof_dbg=yes [targets]     Turn on especially noisy DOF parser debugging\n\n" >&2
> +	@printf "make debugging=yes [targets]\tDisable optimization to make debugger use easier\n" >&2
> +	@printf "make coverage=yes [targets]\tTurn on coverage support in the testsuite\n" >&2
> +	@printf "make verbose=yes [target]\tEnable verbose building\n" >&2
> +	@printf "make dof_dbg=yes [targets]\tTurn on especially noisy DOF parser debugging\n\n" >&2
>   	@printf "Dependencies:\n\n" >&2
> -	@printf "make libfuse2=yes [targets]    Build against libfuse 2 even if libfuse 3 is found\n" >&2
> -	@printf "\n" >&2
> +	@printf "make libfuse2=yes [targets]\tBuild against libfuse 2 even if libfuse 3 is found\n\n" >&2
>   
>   ifneq ($(debugging),no)
>   override CFLAGS += -O0 -g



More information about the DTrace-devel mailing list