diff mbox series

[v3] x86emul: fix test harness and fuzzer build dependencies

Message ID bb2f0618-77af-ce1d-66da-1ba403e3020c@suse.com (mailing list archive)
State Superseded
Headers show
Series [v3] x86emul: fix test harness and fuzzer build dependencies | expand

Commit Message

Jan Beulich Aug. 9, 2019, 3:40 p.m. UTC
Commit fd35f32b4b ("tools/x86emul: Use struct cpuid_policy in the
userspace test harnesses") didn't account for the dependencies of
cpuid-autogen.h to potentially change between incremental builds. In
particular the harness has a "run" goal which is supposed to be usable
independently of the rest of the tools sub-tree building, and both the
harness and the fuzzer code are also supposed to be buildable
independently. Therefore a re-build of the generated header needs to
be triggered first, which is achieved by introducing a new top-level
target pattern (for just the "run" part for now).

Finally cpuid.o did not have any dependencies added for it.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
TBD: Something similar would be nice for building both tools/tests/*/
      and tools/fuzz/*/, but I'm uncertain whether respective top level
      targets build-tests-% and build-fuzz-% would be welcome.
---
v3: Introduce top level run-tests-% target.
v2.1: Split controversial parts from (hopefully) non-controversial ones.
v2: Guard $(MAKE) invocations by $(MAKELEVEL) checks.

Comments

George Dunlap Aug. 14, 2019, 9:42 a.m. UTC | #1
On 8/9/19 4:40 PM, Jan Beulich wrote:
> Commit fd35f32b4b ("tools/x86emul: Use struct cpuid_policy in the
> userspace test harnesses") didn't account for the dependencies of
> cpuid-autogen.h to potentially change between incremental builds. In
> particular the harness has a "run" goal which is supposed to be usable
> independently of the rest of the tools sub-tree building, and both the
> harness and the fuzzer code are also supposed to be buildable
> independently. Therefore a re-build of the generated header needs to
> be triggered first, which is achieved by introducing a new top-level
> target pattern (for just the "run" part for now).
> 
> Finally cpuid.o did not have any dependencies added for it.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> TBD: Something similar would be nice for building both tools/tests/*/
>      and tools/fuzz/*/, but I'm uncertain whether respective top level
>      targets build-tests-% and build-fuzz-% would be welcome.
> ---
> v3: Introduce top level run-tests-% target.
> v2.1: Split controversial parts from (hopefully) non-controversial ones.
> v2: Guard $(MAKE) invocations by $(MAKELEVEL) checks.
> 
> --- a/Makefile
> +++ b/Makefile
> @@ -80,6 +80,9 @@ build-docs:
>  test:
>      $(MAKE) -C tools/python test
>  
> +run-tests-%: build-tools-public-headers tools/tests/%/
> +    $(MAKE) -C tools/tests/$* run
> +
>  # For most targets here,
>  #   make COMPONENT-TARGET
>  # is implemented, more or less, by

Hmm -- Thunderbird seems to know that there's only one space here, but
when I save this mail and try to `git am` it, I get hunk failures like this:

```
diff a/Makefile b/Makefile	(rejected hunks)
@@ -80,6 +80,9 @@ build-docs:
  test:
  	$(MAKE) -C tools/python test

+run-tests-%: build-tools-public-headers tools/tests/%/
+	$(MAKE) -C tools/tests/$* run
+
  # For most targets here,
  #   make COMPONENT-TARGET
  # is implemented, more or less, by
```

Note two spaces before the # rather than 1.  I looked at the raw email
and it has two spaces:

```
--- a/Makefile
+++ b/Makefile
@@ -80,6 +80,9 @@ build-docs:
  test:
  	$(MAKE) -C tools/python test

+run-tests-%: build-tools-public-headers tools/tests/%/
+	$(MAKE) -C tools/tests/$* run
+
  # For most targets here,
  #   make COMPONENT-TARGET
  # is implemented, more or less, by
```

Oh, weird -- but the version on gmail is a completely different
encoding: gmail has it as encoded base64, whereas my corporate mail has
it encoded as 7-bit.  What does everyone else see?

 -George
diff mbox series

Patch

--- a/Makefile
+++ b/Makefile
@@ -80,6 +80,9 @@  build-docs:
  test:
  	$(MAKE) -C tools/python test
  
+run-tests-%: build-tools-public-headers tools/tests/%/
+	$(MAKE) -C tools/tests/$* run
+
  # For most targets here,
  #   make COMPONENT-TARGET
  # is implemented, more or less, by
--- a/tools/fuzz/x86_instruction_emulator/Makefile
+++ b/tools/fuzz/x86_instruction_emulator/Makefile
@@ -26,13 +26,15 @@  GCOV_FLAGS := --coverage
  	$(CC) -c $(CFLAGS) $(GCOV_FLAGS) $< -o $@
  
  x86.h := $(addprefix $(XEN_ROOT)/tools/include/xen/asm/,\
-                     x86-vendors.h x86-defns.h msr-index.h)
+                     x86-vendors.h x86-defns.h msr-index.h) \
+         $(addprefix $(XEN_ROOT)/tools/include/xen/lib/x86/, \
+                     cpuid.h cpuid-autogen.h)
  x86_emulate.h := x86-emulate.h x86_emulate/x86_emulate.h $(x86.h)
  
  # x86-emulate.c will be implicit for both
  x86-emulate.o x86-emulate-cov.o: x86_emulate/x86_emulate.c $(x86_emulate.h)
  
-fuzz-emul.o fuzz-emulate-cov.o wrappers.o: $(x86_emulate.h)
+fuzz-emul.o fuzz-emulate-cov.o cpuid.o wrappers.o: $(x86_emulate.h)
  
  x86-insn-fuzzer.a: fuzz-emul.o x86-emulate.o cpuid.o
  	$(AR) rc $@ $^
--- a/tools/tests/x86_emulator/Makefile
+++ b/tools/tests/x86_emulator/Makefile
@@ -280,10 +280,12 @@  $(call cc-option-add,HOSTCFLAGS-x86_64,H
  HOSTCFLAGS += $(CFLAGS_xeninclude) -I. $(HOSTCFLAGS-$(XEN_COMPILE_ARCH))
  
  x86.h := $(addprefix $(XEN_ROOT)/tools/include/xen/asm/,\
-                     x86-vendors.h x86-defns.h msr-index.h)
+                     x86-vendors.h x86-defns.h msr-index.h) \
+         $(addprefix $(XEN_ROOT)/tools/include/xen/lib/x86/, \
+                     cpuid.h cpuid-autogen.h)
  x86_emulate.h := x86-emulate.h x86_emulate/x86_emulate.h $(x86.h)
  
-x86-emulate.o test_x86_emulator.o evex-disp8.o wrappers.o: %.o: %.c $(x86_emulate.h)
+x86-emulate.o cpuid.o test_x86_emulator.o evex-disp8.o wrappers.o: %.o: %.c $(x86_emulate.h)
  	$(HOSTCC) $(HOSTCFLAGS) -c -g -o $@ $<
  
  x86-emulate.o: x86_emulate/x86_emulate.c