Re: [PATCH] Makefile: fix dependency for $(UNIT_TEST_DIR)/clar/clar.o

Hi,

I had a closer look at how the header dependencies are handled in the Makefile and I think a cleaner and more idiomatic way to fix the problem would be to add clar.suite to GENERATED_H. 

I’ll try to send a v2 tomorrow so maybe hold on to merging it to next for the moment, Junio. 

Thanks
Philippe. 

Le 7 oct. 2024 à 23:58, Patrick Steinhardt [off-list ref] a écrit :

On Mon, Oct 07, 2024 at 05:53:41PM -0700, Junio C Hamano wrote:

quoted

"Philippe Blain via GitGitGadget" [off-list ref] writes:

quoted

From: Philippe Blain <redacted>

The clar source file '$(UNIT_TEST_DIR)/clar/clar.c' includes the
generated 'clar.suite', but this dependency is not taken into account by
our Makefile, so that it is possible for a parallel build to fail if
Make tries to build 'clar.o' before 'clar.suite' is generated.

Correctly specify the dependency.

Signed-off-by: Philippe Blain <redacted>
---
   Makefile: fix dependency for $(UNIT_TEST_DIR)/clar/clar.o

   Hi Patrick,

   I tried building v2.47.0 and stumbled onto this small issue. It
   reproduces for me from a fresh clone on my old 2009 Mac with make -j -l
   2.5, it's a little curious that no one ran into this yet.

I suspect that nobody tells make to build clar.o (and nothing else).

Instead, the t/unit-tests/bin/unit-tests target is what is typically
built, which is part of $(CLAR_TEST_PROG) that has clar.suite as one
of its dependencies.

   $ make
   $ rm -f t/unit-tests/clar.suite t/unit-tests/clar/clar.o
   $ make -j1 t/unit-tests/bin/unit-tests
   GEN t/unit-tests/clar.suite
   CC t/unit-tests/clar/clar.o
   LINK t/unit-tests/bin/unit-tests

What is possible to happen from the broken dependencies is when I
did not remove clar.o in the above experiment.  We may rebuild
clar.suite and then link clar.o that is outdated without realizing.

Makes sense. In any case, the fix looks good to me, thanks!

Patrick