[PATCH v2 1/4] tests: fixup domid make fragment

dmukhin@xen.org posted 4 patches 4 weeks, 1 day ago
[PATCH v2 1/4] tests: fixup domid make fragment
Posted by dmukhin@xen.org 4 weeks, 1 day ago
From: Denis Mukhin <dmukhin@ford.com> 

There can be multiple test harnesses per one test target (e.g. harness.h
and harness2.h). Account for that by further parametrizing existing
emit-harness-nested-rule().

Add guard against HOSTCC != CC (similarly to how its done in PDX unit test).

Account for multiple test targets in install and uninstall make targets.

Introduce CFLAGS dedicated for find-next-bit.c only to avoid contaminating
global CFLAGS.

Honor mocked hypervisor header over tools/include/xen symlinks.

Finally, add some clarifications for the functions.

Amends: 2d5065060710 ("xen/domain: unify domain ID allocation")
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v1:
- updated commentaries
- added ability to select the harness header filename
- fixup for picking up mocked header files
---
 tools/tests/domid/Makefile | 63 ++++++++++++++++++++++++++------------
 1 file changed, 44 insertions(+), 19 deletions(-)

diff --git a/tools/tests/domid/Makefile b/tools/tests/domid/Makefile
index 753129029ed9..dd22a25b038a 100644
--- a/tools/tests/domid/Makefile
+++ b/tools/tests/domid/Makefile
@@ -4,36 +4,55 @@
 #
 # Copyright 2025 Ford Motor Company
 
-XEN_ROOT=$(CURDIR)/../../..
-include $(XEN_ROOT)/tools/Rules.mk
-
 TESTS := test-domid
 
+XEN_ROOT = $(CURDIR)/../../..
+include $(XEN_ROOT)/tools/Rules.mk
+
 define list-c-headers
 $(shell sed -n \
     's/^[ \t]*# *include[ \t]*[<"]\([^">]*\)[">].*/\1/p' $(1) 2>/dev/null)
 endef
 
-# NB: $1 cannot be a list
+# Generate mock environment by replicating header file hierarchy;
+# each header file will point to a harness header.
+#
+# $1 target
+# $2 list of test harnesses
 define emit-harness-nested-rule
-$(1): $(CURDIR)/harness.h
-	mkdir -p $$(@D);
-	ln -sf $$< $$@;
+$(1): $(2)
+	set -e; \
+	mkdir -p $$(@D); \
+	for i in $(2); do [ -e $$@ ] || ln -s $$$$i $$@; done
 
 endef
 
+# Helper function to emit mocked hypervisor code dependencies.
+#
+# $1 Harness file name.
+# $2 Mocked hypervisor file name.
+# $3 List of dependencies to mock.
 define emit-harness-rules
-$(foreach x,$(2),$(call emit-harness-nested-rule,$(CURDIR)/generated/$(x)))
-$(1:.c=.o): $(addprefix $(CURDIR)/generated/,$(2))
+$(foreach x,$(3),$(call emit-harness-nested-rule,\
+                        $(CURDIR)/generated/$(x),\
+                        $(addprefix $(CURDIR)/,$(1))))
+$(2:.c=.o): $(addprefix $(CURDIR)/generated/,$(3))
 endef
 
 define emit-harness-deps
-$(if $(strip $(2)),$(call emit-harness-rules,$1,$2),)
+$(if $(strip $(3)),$(call emit-harness-rules,$1,$2,$3),)
 endef
 
+# Emit dependencies for mocked hypervisor code.
+#
+# $1 Hypervisor file name.
+# $2 Hypervisor source path.
+# $3 Harness header file name (optional).
 define vpath-with-harness-deps
 vpath $(1) $(2)
-$(call emit-harness-deps,$(1),$(call list-c-headers,$(2)$(1)))
+$(call emit-harness-deps,$(or $(strip $(3)),harness.h),\
+                         $(1),\
+                         $(call list-c-headers,$(2)$(1)))
 endef
 
 .PHONY: all
@@ -41,7 +60,11 @@ all: $(TESTS)
 
 .PHONY: run
 run: $(TESTS)
+ifeq ($(CC),$(HOSTCC))
 	set -e; $(foreach t,$(TESTS),./$(t);)
+else
+	$(warning HOSTCC != CC, will not run test)
+endif
 
 .PHONY: clean
 clean:
@@ -55,25 +78,27 @@ distclean: clean
 .PHONY: install
 install: all
 	$(INSTALL_DIR) $(DESTDIR)$(LIBEXEC)/tests
-	$(INSTALL_PROG) test-domid $(DESTDIR)$(LIBEXEC)/tests
+	set -e; $(foreach t,$(TESTS),$(INSTALL_PROG) $t $(DESTDIR)$(LIBEXEC)/tests;)
 
 .PHONY: uninstall
 uninstall:
-	$(RM) -- $(DESTDIR)$(LIBEXEC)/tests/test-domid
+	set -e; $(foreach t,$(TESTS),$(RM) -- $(DESTDIR)$(LIBEXEC)/tests/$t;)
 
 CFLAGS += -D__XEN_TOOLS__
+
 # find-next-bit.c
-CFLAGS += '-DEXPORT_SYMBOL(x)=' \
+CFLAGS-find-next-bit.c += '-DEXPORT_SYMBOL(x)=' \
           -Dfind_first_bit \
           -Dfind_first_zero_bit \
           -Dfind_next_bit \
           -Dfind_next_bit_le \
           -Dfind_next_zero_bit_le
-CFLAGS += $(APPEND_CFLAGS)
-CFLAGS += $(CFLAGS_xeninclude)
-CFLAGS += -I./generated/
 
-LDFLAGS += $(APPEND_LDFLAGS)
+find-next-bit.o: CFLAGS += $(CFLAGS-find-next-bit.c)
+
+# Honor mocked hypervisor header over tools/include/xen symlinks
+CFLAGS += -I$(CURDIR)/generated/
+CFLAGS += $(CFLAGS_xeninclude)
 
 vpath find-next-bit.c $(XEN_ROOT)/xen/lib/
 
@@ -83,6 +108,6 @@ vpath find-next-bit.c $(XEN_ROOT)/xen/lib/
 $(eval $(call vpath-with-harness-deps,domid.c,$(XEN_ROOT)/xen/common/))
 
 test-domid: domid.o find-next-bit.o test-domid.o
-	$(CC) $^ -o $@ $(LDFLAGS)
+	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $^
 
 -include $(DEPS_INCLUDE)
-- 
2.52.0
Re: [PATCH v2 1/4] tests: fixup domid make fragment
Posted by Stefano Stabellini 2 weeks, 2 days ago
On Sat, 10 Jan 2026, dmukhin@xen.org wrote:
> From: Denis Mukhin <dmukhin@ford.com> 
> 
> There can be multiple test harnesses per one test target (e.g. harness.h
> and harness2.h). Account for that by further parametrizing existing
> emit-harness-nested-rule().
> 
> Add guard against HOSTCC != CC (similarly to how its done in PDX unit test).
> 
> Account for multiple test targets in install and uninstall make targets.
> 
> Introduce CFLAGS dedicated for find-next-bit.c only to avoid contaminating
> global CFLAGS.
> 
> Honor mocked hypervisor header over tools/include/xen symlinks.
> 
> Finally, add some clarifications for the functions.
> 
> Amends: 2d5065060710 ("xen/domain: unify domain ID allocation")
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
> Changes since v1:
> - updated commentaries
> - added ability to select the harness header filename
> - fixup for picking up mocked header files
> ---
>  tools/tests/domid/Makefile | 63 ++++++++++++++++++++++++++------------
>  1 file changed, 44 insertions(+), 19 deletions(-)
> 
> diff --git a/tools/tests/domid/Makefile b/tools/tests/domid/Makefile
> index 753129029ed9..dd22a25b038a 100644
> --- a/tools/tests/domid/Makefile
> +++ b/tools/tests/domid/Makefile
> @@ -4,36 +4,55 @@
>  #
>  # Copyright 2025 Ford Motor Company
>  
> -XEN_ROOT=$(CURDIR)/../../..
> -include $(XEN_ROOT)/tools/Rules.mk
> -
>  TESTS := test-domid
>  
> +XEN_ROOT = $(CURDIR)/../../..
> +include $(XEN_ROOT)/tools/Rules.mk
> +
>  define list-c-headers
>  $(shell sed -n \
>      's/^[ \t]*# *include[ \t]*[<"]\([^">]*\)[">].*/\1/p' $(1) 2>/dev/null)
>  endef
>  
> -# NB: $1 cannot be a list
> +# Generate mock environment by replicating header file hierarchy;
> +# each header file will point to a harness header.
> +#
> +# $1 target
> +# $2 list of test harnesses
>  define emit-harness-nested-rule
> -$(1): $(CURDIR)/harness.h
> -	mkdir -p $$(@D);
> -	ln -sf $$< $$@;
> +$(1): $(2)
> +	set -e; \
> +	mkdir -p $$(@D); \
> +	for i in $(2); do [ -e $$@ ] || ln -s $$$$i $$@; done
 
Are you sure this is correct? Everything seems to be escaped too many
times? Shouldn't this be:

  $(1): $(2)
        set -e; \
        mkdir -p "$(@D)"; \
        for i in $(2); do \
                [ -e "$@" ] || ln -s "$$i" "$@"; \
        done
  endef



>  endef
>  
> +# Helper function to emit mocked hypervisor code dependencies.
> +#
> +# $1 Harness file name.
> +# $2 Mocked hypervisor file name.
> +# $3 List of dependencies to mock.
>  define emit-harness-rules
> -$(foreach x,$(2),$(call emit-harness-nested-rule,$(CURDIR)/generated/$(x)))
> -$(1:.c=.o): $(addprefix $(CURDIR)/generated/,$(2))
> +$(foreach x,$(3),$(call emit-harness-nested-rule,\
> +                        $(CURDIR)/generated/$(x),\
> +                        $(addprefix $(CURDIR)/,$(1))))
> +$(2:.c=.o): $(addprefix $(CURDIR)/generated/,$(3))
>  endef
>  
>  define emit-harness-deps
> -$(if $(strip $(2)),$(call emit-harness-rules,$1,$2),)
> +$(if $(strip $(3)),$(call emit-harness-rules,$1,$2,$3),)
>  endef
>  
> +# Emit dependencies for mocked hypervisor code.
> +#
> +# $1 Hypervisor file name.
> +# $2 Hypervisor source path.
> +# $3 Harness header file name (optional).
>  define vpath-with-harness-deps
>  vpath $(1) $(2)
> -$(call emit-harness-deps,$(1),$(call list-c-headers,$(2)$(1)))
> +$(call emit-harness-deps,$(or $(strip $(3)),harness.h),\
> +                         $(1),\
> +                         $(call list-c-headers,$(2)$(1)))
>  endef
>  
>  .PHONY: all
> @@ -41,7 +60,11 @@ all: $(TESTS)
>  
>  .PHONY: run
>  run: $(TESTS)
> +ifeq ($(CC),$(HOSTCC))
>  	set -e; $(foreach t,$(TESTS),./$(t);)
> +else
> +	$(warning HOSTCC != CC, will not run test)
> +endif
>  
>  .PHONY: clean
>  clean:
> @@ -55,25 +78,27 @@ distclean: clean
>  .PHONY: install
>  install: all
>  	$(INSTALL_DIR) $(DESTDIR)$(LIBEXEC)/tests
> -	$(INSTALL_PROG) test-domid $(DESTDIR)$(LIBEXEC)/tests
> +	set -e; $(foreach t,$(TESTS),$(INSTALL_PROG) $t $(DESTDIR)$(LIBEXEC)/tests;)
>  
>  .PHONY: uninstall
>  uninstall:
> -	$(RM) -- $(DESTDIR)$(LIBEXEC)/tests/test-domid
> +	set -e; $(foreach t,$(TESTS),$(RM) -- $(DESTDIR)$(LIBEXEC)/tests/$t;)
>  
>  CFLAGS += -D__XEN_TOOLS__
> +
>  # find-next-bit.c
> -CFLAGS += '-DEXPORT_SYMBOL(x)=' \
> +CFLAGS-find-next-bit.c += '-DEXPORT_SYMBOL(x)=' \
>            -Dfind_first_bit \
>            -Dfind_first_zero_bit \
>            -Dfind_next_bit \
>            -Dfind_next_bit_le \
>            -Dfind_next_zero_bit_le
> -CFLAGS += $(APPEND_CFLAGS)
> -CFLAGS += $(CFLAGS_xeninclude)
> -CFLAGS += -I./generated/
>  
> -LDFLAGS += $(APPEND_LDFLAGS)

APPEND_CFLAGS and APPEND_LDFLAGS are lost?


> +find-next-bit.o: CFLAGS += $(CFLAGS-find-next-bit.c)
> +
> +# Honor mocked hypervisor header over tools/include/xen symlinks
> +CFLAGS += -I$(CURDIR)/generated/
> +CFLAGS += $(CFLAGS_xeninclude)
>  
>  vpath find-next-bit.c $(XEN_ROOT)/xen/lib/
>  
> @@ -83,6 +108,6 @@ vpath find-next-bit.c $(XEN_ROOT)/xen/lib/
>  $(eval $(call vpath-with-harness-deps,domid.c,$(XEN_ROOT)/xen/common/))
>  
>  test-domid: domid.o find-next-bit.o test-domid.o
> -	$(CC) $^ -o $@ $(LDFLAGS)
> +	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $^

My understanding is that $(LDFLAGS) should be last, e.g.:

$(CC) $(CFLAGS) -o $@ $^ $(LDFLAGS)


>  -include $(DEPS_INCLUDE)
> -- 
> 2.52.0
>
Re: [PATCH v2 1/4] tests: fixup domid make fragment
Posted by Jan Beulich 4 weeks ago
On 11.01.2026 05:11, dmukhin@xen.org wrote:
> From: Denis Mukhin <dmukhin@ford.com> 
> 
> There can be multiple test harnesses per one test target (e.g. harness.h
> and harness2.h). Account for that by further parametrizing existing
> emit-harness-nested-rule().

Multiple harnesses within a single dir imo likely would share headers, but
use different .c files. Also, why would dependencies on headers need
recording at all? The Makefile includes $(DEPS_INCLUDE), so all dependency
concerns should be covered (or else the generic machinery would need
fixing). Imo all of this wants simplifying (dropping?) rather than further
complicating.

Jan
Re: [PATCH v2 1/4] tests: fixup domid make fragment
Posted by dmukhin@xen.org 3 weeks, 4 days ago
On Mon, Jan 12, 2026 at 12:16:46PM +0100, Jan Beulich wrote:
> On 11.01.2026 05:11, dmukhin@xen.org wrote:
> > From: Denis Mukhin <dmukhin@ford.com> 
> > 
> > There can be multiple test harnesses per one test target (e.g. harness.h
> > and harness2.h). Account for that by further parametrizing existing
> > emit-harness-nested-rule().
> 
> Multiple harnesses within a single dir imo likely would share headers, but
> use different .c files. Also, why would dependencies on headers need
> recording at all? The Makefile includes $(DEPS_INCLUDE), so all dependency
> concerns should be covered (or else the generic machinery would need
> fixing). Imo all of this wants simplifying (dropping?) rather than further
> complicating.

Whoops, I forgot to drop that multi-harness stuff.
Thanks!

> 
> Jan