On Sun, Jan 28, 2024 at 10:18:16PM -0500, Jeff King wrote: > We build the UNIT_TEST_BIN directory (t/unit-tests/bin) on the fly with > "mkdir -p". And so the recipe for UNIT_TEST_PROGS, which put their > output in that directory, depend on UNIT_TEST_BIN to make sure it's > there. > > But using a normal dependency leads to weird outcomes, because the > timestamp of the directory is important. For example, try this: > > $ make > [...builds everything...] > > [now re-build one unit test] > $ touch t/unit-tests/t-ctype.c > $ make > SUBDIR templates > CC t/unit-tests/t-ctype.o > LINK t/unit-tests/bin/t-ctype > > So far so good. Now running make again should build nothing. But it > doesn't! > > $ make > SUBDIR templates > LINK t/unit-tests/bin/t-basic > LINK t/unit-tests/bin/t-mem-pool > LINK t/unit-tests/bin/t-strbuf > > Er, what? Let's rebuild again: > > $ make > SUBDIR templates > LINK t/unit-tests/bin/t-ctype > > Weird. And now we ping-pong back and forth forever: > > $ make > SUBDIR templates > LINK t/unit-tests/bin/t-basic > LINK t/unit-tests/bin/t-mem-pool > LINK t/unit-tests/bin/t-strbuf > $ make > SUBDIR templates > LINK t/unit-tests/bin/t-ctype > > What happens is that writing t/unit-tests/bin/t-ctype updates the mtime > of the directory t/unit-tests/bin. And then on the next invocation of > make, all of those other tests are now older and so get rebuilt. And > back and forth forever. > > We can fix this by using an order-only prereq. This is a GNU-ism that > tells make to only care that the dependency exists at all, and to ignore > its mtime. It was designed for exactly this sort of situation (the > documentation example even uses "mkdir"). > > We already rely on GNU make, so that's not a problem. This particular > feature was added in GNU make 3.80, released in October 2002. This is > obviously quite old by date, but it's also worth thinking about macOS, > as Apple stopped updating packages that switched to GPLv3 tools. In this > their dev tools ship GNU make 3.81, which is recent enough. > > If it is a problem, there are two alternatives: > > - we can just "mkdir -p" in the recipe to build the individual > binaries. This will mean some redundant "mkdir" calls, but only when > actually invoking the compiler. > > - we could stop making the directory on the fly, and just add it with > a .gitignore of "*". This would work fine, but might be awkward when > moving back and forth in history. A third alternative is to use $(call mkdir_p_parent_template) in the recipe and get rid of the thus unnecessary UNIT_TEST_BIN dependency and target. It will only run mkdir when needed, and it's a well established pattern in our Makefile, so you won't have to spend a paragraph or two arguing about potential problems with GNU-isms :) On a related note, 'make clean' doesn't remove this 't/unit-tests/bin' directory. > Signed-off-by: Jeff King <peff@xxxxxxxx> > --- > I may be overly paranoid about the ".gitignore" strategy. I feel like > I've been bitten by this in the past by things switching from source to > build (I think with git-remote-testgit). But that's an actual built > file. Git would probably be OK with the "bin/" directory coming and > going as a tracked entity, because the index really only cares about > the file "bin/.gitignore". Still, this make fix was easy enough. > > Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index 1a62e48759..958f4cd0bf 100644 > --- a/Makefile > +++ b/Makefile > @@ -3866,7 +3866,7 @@ fuzz-all: $(FUZZ_PROGRAMS) > $(UNIT_TEST_BIN): > @mkdir -p $(UNIT_TEST_BIN) > > -$(UNIT_TEST_PROGS): $(UNIT_TEST_BIN)/%$X: $(UNIT_TEST_DIR)/%.o $(UNIT_TEST_DIR)/test-lib.o $(GITLIBS) GIT-LDFLAGS $(UNIT_TEST_BIN) > +$(UNIT_TEST_PROGS): $(UNIT_TEST_BIN)/%$X: $(UNIT_TEST_DIR)/%.o $(UNIT_TEST_DIR)/test-lib.o $(GITLIBS) GIT-LDFLAGS | $(UNIT_TEST_BIN) > $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \ > $(filter %.o,$^) $(filter %.a,$^) $(LIBS) > > -- > 2.43.0.797.g29b680fc68 > >
