diff mbox series

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

Message ID pull.1810.v2.git.1728667787227.gitgitgadget@gmail.com (mailing list archive)
State Accepted
Commit ea3422662d21c33c349cc71e9cdbfe235e8f6905
Headers show
Series [v2] Makefile: fix dependency for $(UNIT_TEST_DIR)/clar/clar.o | expand

Commit Message

Philippe Blain Oct. 11, 2024, 5:29 p.m. UTC 
From: Philippe Blain <levraiphilippeblain@gmail.com>

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 <levraiphilippeblain@gmail.com>
---
    Makefile: fix dependency for $(UNIT_TEST_DIR)/clar/clar.o
    
    v2: Compared to v1, I just moved the new line one line up to keep the
    dependencies of $(CLAR_TEST_OBJS) together.
    
    Regarding what I wrote in [1], I took a closer look and actually
    clar.suite is already added to GENERATED_H. The issue really is that for
    generated headers, the dependency must be specified manually. This is
    the case for other generated headers (command-list.h, etc). So my
    initial approach is correct.
    
    [1]
    https://lore.kernel.org/git/C05B01E0-5007-4FB9-94CD-CBE74E79F9B7@gmail.com/
    
    v1: 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 found it best to declare the dependency directly on clar.o, instead of
    adjusting the dependencies of CLAR_TEST_OBJS on the line above, since it
    is really only this clar.c that includes clar.suite

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1810%2Fphil-blain%2Fclar-build-dep-fix-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1810/phil-blain/clar-build-dep-fix-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1810

Range-diff vs v1:

 1:  42ae65c5f82 ! 1:  f20c8a0f57b Makefile: fix dependency for $(UNIT_TEST_DIR)/clar/clar.o
     @@ Commit message
      
       ## Makefile ##
      @@ Makefile: $(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUI
     + 	done >$@
       $(UNIT_TEST_DIR)/clar.suite: $(UNIT_TEST_DIR)/clar-decls.h
       	$(QUIET_GEN)awk -f $(UNIT_TEST_DIR)/clar-generate.awk $< >$(UNIT_TEST_DIR)/clar.suite
     - $(CLAR_TEST_OBJS): $(UNIT_TEST_DIR)/clar-decls.h
      +$(UNIT_TEST_DIR)/clar/clar.o: $(UNIT_TEST_DIR)/clar.suite
     + $(CLAR_TEST_OBJS): $(UNIT_TEST_DIR)/clar-decls.h
       $(CLAR_TEST_OBJS): EXTRA_CPPFLAGS = -I$(UNIT_TEST_DIR)
       $(CLAR_TEST_PROG): $(UNIT_TEST_DIR)/clar.suite $(CLAR_TEST_OBJS) $(GITLIBS) GIT-LDFLAGS
     - 	$(call mkdir_p_parent_template)


 Makefile | 1 +
 1 file changed, 1 insertion(+)


base-commit: 777489f9e09c8d0dd6b12f9d90de6376330577a2

Comments

Patrick Steinhardt Oct. 14, 2024, 11:59 a.m. UTC | #1 
On Fri, Oct 11, 2024 at 05:29:47PM +0000, Philippe Blain via GitGitGadget wrote:
> From: Philippe Blain <levraiphilippeblain@gmail.com>
> 
> 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 <levraiphilippeblain@gmail.com>

Thanks, this version looks good to me.

Patrick
Taylor Blau Oct. 15, 2024, 12:34 a.m. UTC | #2 
On Mon, Oct 14, 2024 at 01:59:25PM +0200, Patrick Steinhardt wrote:
> On Fri, Oct 11, 2024 at 05:29:47PM +0000, Philippe Blain via GitGitGadget wrote:
> > From: Philippe Blain <levraiphilippeblain@gmail.com>
> >
> > 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 <levraiphilippeblain@gmail.com>
>
> Thanks, this version looks good to me.

Thanks, both. Junio had already marked this one to merge to 'next', so I
will go ahead and merge it down in the next integration cycle (which
should be tomorrow, assuming I remember how to do it correctly).

Thanks,
Taylor
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index 2dde1fd2b8b..76926537ccd 100644
--- a/Makefile
+++ b/Makefile
@@ -3910,6 +3910,7 @@  $(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUI
 	done >$@
 $(UNIT_TEST_DIR)/clar.suite: $(UNIT_TEST_DIR)/clar-decls.h
 	$(QUIET_GEN)awk -f $(UNIT_TEST_DIR)/clar-generate.awk $< >$(UNIT_TEST_DIR)/clar.suite
+$(UNIT_TEST_DIR)/clar/clar.o: $(UNIT_TEST_DIR)/clar.suite
 $(CLAR_TEST_OBJS): $(UNIT_TEST_DIR)/clar-decls.h
 $(CLAR_TEST_OBJS): EXTRA_CPPFLAGS = -I$(UNIT_TEST_DIR)
 $(CLAR_TEST_PROG): $(UNIT_TEST_DIR)/clar.suite $(CLAR_TEST_OBJS) $(GITLIBS) GIT-LDFLAGS