tools/tests: Fix newly introduced Makefile

[XEN PATCH] tools/tests: Fix newly introduced Makefile

Posted by Anthony PERARD 6 months, 2 weeks ago

From: Anthony PERARD <anthony.perard@vates.tech>

Fix few issue with this new directory:
- clean generated files
- and ignore those generated files
- include the dependency files generated by `gcc`.
- rework prerequisites:
  "test-rangeset.o" also needs the generated files "list.h" and
  "rangeset.h". Technically, both only needs "harness.h" which needs
  the generated headers, but that's a bit simpler and the previous
  point will add the dependency on "harness.h" automatically.

This last point fix an issue where `make` might decide to build
"test-rangeset.o" before the other files are ready.

Fixes: 7bf777b42cad ("tootls/tests: introduce unit tests for rangesets")
Signed-off-by: Anthony PERARD <anthony.perard@vates.tech>
---

Make doesn't needs the *.h to generated the .c. So removing that
prerequisite means make can generate all 3 at the same time.
---
 tools/tests/rangeset/.gitignore | 4 ++++
 tools/tests/rangeset/Makefile   | 8 ++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)
 create mode 100644 tools/tests/rangeset/.gitignore

diff --git a/tools/tests/rangeset/.gitignore b/tools/tests/rangeset/.gitignore
new file mode 100644
index 0000000000..cdeb778535
--- /dev/null
+++ b/tools/tests/rangeset/.gitignore
@@ -0,0 +1,4 @@
+/list.h
+/rangeset.c
+/rangeset.h
+/test-rangeset
diff --git a/tools/tests/rangeset/Makefile b/tools/tests/rangeset/Makefile
index 70076eff34..3dafcbd054 100644
--- a/tools/tests/rangeset/Makefile
+++ b/tools/tests/rangeset/Makefile
@@ -12,7 +12,7 @@ run: $(TARGET)
 
 .PHONY: clean
 clean:
-	$(RM) -- *.o $(TARGET) $(DEPS_RM)
+	$(RM) -- *.o $(TARGET) $(DEPS_RM) list.h rangeset.h rangeset.c
 
 .PHONY: distclean
 distclean: clean
@@ -32,7 +32,7 @@ rangeset.h: $(XEN_ROOT)/xen/include/xen/rangeset.h
 list.h rangeset.h:
 	sed -e '/#include/d' <$< >$@
 
-rangeset.c: $(XEN_ROOT)/xen/common/rangeset.c list.h rangeset.h
+rangeset.c: $(XEN_ROOT)/xen/common/rangeset.c
 	# Remove includes and add the test harness header
 	sed -e '/#include/d' -e '1s/^/#include "harness.h"/' <$< >$@
 
@@ -42,5 +42,9 @@ CFLAGS += $(CFLAGS_xeninclude)
 
 LDFLAGS += $(APPEND_LDFLAGS)
 
+test-rangeset.o rangeset.o: list.h rangeset.h
+
 test-rangeset: rangeset.o test-rangeset.o
 	$(CC) $^ -o $@ $(LDFLAGS)
+
+-include $(DEPS_INCLUDE)
-- 
Anthony PERARD

Re: [XEN PATCH] tools/tests: Fix newly introduced Makefile

Posted by Roger Pau Monné 6 months, 2 weeks ago

On Mon, Apr 14, 2025 at 04:17:14PM +0200, Anthony PERARD wrote:
> From: Anthony PERARD <anthony.perard@vates.tech>
> 
> Fix few issue with this new directory:
> - clean generated files
> - and ignore those generated files
> - include the dependency files generated by `gcc`.
> - rework prerequisites:
>   "test-rangeset.o" also needs the generated files "list.h" and
>   "rangeset.h". Technically, both only needs "harness.h" which needs
>   the generated headers, but that's a bit simpler and the previous
>   point will add the dependency on "harness.h" automatically.
> 
> This last point fix an issue where `make` might decide to build
> "test-rangeset.o" before the other files are ready.

Oh, I see, yes, test-rangeset.o wasn't dependent on the generated
files.

> 
> Fixes: 7bf777b42cad ("tootls/tests: introduce unit tests for rangesets")
> Signed-off-by: Anthony PERARD <anthony.perard@vates.tech>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

> ---
> 
> Make doesn't needs the *.h to generated the .c. So removing that
> prerequisite means make can generate all 3 at the same time.
> ---
>  tools/tests/rangeset/.gitignore | 4 ++++
>  tools/tests/rangeset/Makefile   | 8 ++++++--
>  2 files changed, 10 insertions(+), 2 deletions(-)
>  create mode 100644 tools/tests/rangeset/.gitignore
> 
> diff --git a/tools/tests/rangeset/.gitignore b/tools/tests/rangeset/.gitignore
> new file mode 100644
> index 0000000000..cdeb778535
> --- /dev/null
> +++ b/tools/tests/rangeset/.gitignore
> @@ -0,0 +1,4 @@
> +/list.h
> +/rangeset.c
> +/rangeset.h
> +/test-rangeset
> diff --git a/tools/tests/rangeset/Makefile b/tools/tests/rangeset/Makefile
> index 70076eff34..3dafcbd054 100644
> --- a/tools/tests/rangeset/Makefile
> +++ b/tools/tests/rangeset/Makefile
> @@ -12,7 +12,7 @@ run: $(TARGET)
>  
>  .PHONY: clean
>  clean:
> -	$(RM) -- *.o $(TARGET) $(DEPS_RM)
> +	$(RM) -- *.o $(TARGET) $(DEPS_RM) list.h rangeset.h rangeset.c
>  
>  .PHONY: distclean
>  distclean: clean
> @@ -32,7 +32,7 @@ rangeset.h: $(XEN_ROOT)/xen/include/xen/rangeset.h
>  list.h rangeset.h:
>  	sed -e '/#include/d' <$< >$@
>  
> -rangeset.c: $(XEN_ROOT)/xen/common/rangeset.c list.h rangeset.h
> +rangeset.c: $(XEN_ROOT)/xen/common/rangeset.c
>  	# Remove includes and add the test harness header
>  	sed -e '/#include/d' -e '1s/^/#include "harness.h"/' <$< >$@
>  
> @@ -42,5 +42,9 @@ CFLAGS += $(CFLAGS_xeninclude)
>  
>  LDFLAGS += $(APPEND_LDFLAGS)
>  
> +test-rangeset.o rangeset.o: list.h rangeset.h
> +
>  test-rangeset: rangeset.o test-rangeset.o
>  	$(CC) $^ -o $@ $(LDFLAGS)
> +
> +-include $(DEPS_INCLUDE)

This include is likely also needed by vpci/Makefile?

Thanks, Roger.

Re: [XEN PATCH] tools/tests: Fix newly introduced Makefile

Posted by Anthony PERARD 6 months, 2 weeks ago

On Tue, Apr 15, 2025 at 10:43:24AM +0200, Roger Pau Monné wrote:
> On Mon, Apr 14, 2025 at 04:17:14PM +0200, Anthony PERARD wrote:
> > From: Anthony PERARD <anthony.perard@vates.tech>
> > +
> > +-include $(DEPS_INCLUDE)
> 
> This include is likely also needed by vpci/Makefile?

No, because we don't build any object there, and build the binary
directly from source, for some reason. So there's no deps files been
generated.

Cheers,

-- 
Anthony PERARD

Re: [XEN PATCH] tools/tests: Fix newly introduced Makefile

Posted by dmkhn@proton.me 6 months, 2 weeks ago

On Mon, Apr 14, 2025 at 04:17:14PM +0200, Anthony PERARD wrote:
> From: Anthony PERARD <anthony.perard@vates.tech>
> 
> Fix few issue with this new directory:
> - clean generated files
> - and ignore those generated files
> - include the dependency files generated by `gcc`.
> - rework prerequisites:
>   "test-rangeset.o" also needs the generated files "list.h" and
>   "rangeset.h". Technically, both only needs "harness.h" which needs
>   the generated headers, but that's a bit simpler and the previous
>   point will add the dependency on "harness.h" automatically.
> 
> This last point fix an issue where `make` might decide to build
> "test-rangeset.o" before the other files are ready.
> 
> Fixes: 7bf777b42cad ("tootls/tests: introduce unit tests for rangesets")
> Signed-off-by: Anthony PERARD <anthony.perard@vates.tech>

Reviewed-by: Denis Mukhin <dmukhin@ford.com>

> ---
> 
> Make doesn't needs the *.h to generated the .c. So removing that
> prerequisite means make can generate all 3 at the same time.
> ---
>  tools/tests/rangeset/.gitignore | 4 ++++
>  tools/tests/rangeset/Makefile   | 8 ++++++--
>  2 files changed, 10 insertions(+), 2 deletions(-)
>  create mode 100644 tools/tests/rangeset/.gitignore
> 
> diff --git a/tools/tests/rangeset/.gitignore b/tools/tests/rangeset/.gitignore
> new file mode 100644
> index 0000000000..cdeb778535
> --- /dev/null
> +++ b/tools/tests/rangeset/.gitignore
> @@ -0,0 +1,4 @@
> +/list.h
> +/rangeset.c
> +/rangeset.h
> +/test-rangeset
> diff --git a/tools/tests/rangeset/Makefile b/tools/tests/rangeset/Makefile
> index 70076eff34..3dafcbd054 100644
> --- a/tools/tests/rangeset/Makefile
> +++ b/tools/tests/rangeset/Makefile
> @@ -12,7 +12,7 @@ run: $(TARGET)
> 
>  .PHONY: clean
>  clean:
> -	$(RM) -- *.o $(TARGET) $(DEPS_RM)
> +	$(RM) -- *.o $(TARGET) $(DEPS_RM) list.h rangeset.h rangeset.c
> 
>  .PHONY: distclean
>  distclean: clean
> @@ -32,7 +32,7 @@ rangeset.h: $(XEN_ROOT)/xen/include/xen/rangeset.h
>  list.h rangeset.h:
>  	sed -e '/#include/d' <$< >$@
> 
> -rangeset.c: $(XEN_ROOT)/xen/common/rangeset.c list.h rangeset.h
> +rangeset.c: $(XEN_ROOT)/xen/common/rangeset.c
>  	# Remove includes and add the test harness header
>  	sed -e '/#include/d' -e '1s/^/#include "harness.h"/' <$< >$@
> 
> @@ -42,5 +42,9 @@ CFLAGS += $(CFLAGS_xeninclude)
> 
>  LDFLAGS += $(APPEND_LDFLAGS)
> 
> +test-rangeset.o rangeset.o: list.h rangeset.h
> +
>  test-rangeset: rangeset.o test-rangeset.o
>  	$(CC) $^ -o $@ $(LDFLAGS)
> +
> +-include $(DEPS_INCLUDE)
> --
> Anthony PERARD
> 
>