Subject: Re: [PATCH] Makefile: make COMPUTE_HEADER_DEPENDENCIES=auto work with DEVOPTS=pedantic

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Thu, 23 Sep 2021 00:40:04 +0200

On Wed, Sep 22 2021, Jeff King wrote:

> It's a shame we can't just try to do the _real_ compiles using the
> auto-dependency stuff, and then fall back if they fail. But I think
> there's a chicken-and-egg problem there with "make" doing real work, and
> figuring out the dependencies to do real work.

Isn't this just a chicken & egg problem because we've made it a chicken
& egg problem? I.e. this WIP hack seems to work for me to avoid it:

===============================================================================
diff --git a/Makefile b/Makefile
index 0fe0a9aaa19..68e1714a100 100644
--- a/Makefile
+++ b/Makefile
@@ -1953,6 +1953,7 @@ endif
 ifneq ($(findstring s,$(MAKEFLAGS)),s)
 ifndef V
 	QUIET_CC       = @echo '   ' CC $@;
+	QUIET_DEP      = @echo '   ' DEP $@;
 	QUIET_AR       = @echo '   ' AR $@;
 	QUIET_LINK     = @echo '   ' LINK $@;
 	QUIET_BUILT_IN = @echo '   ' BUILTIN $@;
@@ -2454,24 +2455,6 @@ endif
 .PHONY: objects
 objects: $(OBJECTS)
 
-dep_files := $(foreach f,$(OBJECTS),$(dir $f).depend/$(notdir $f).d)
-dep_dirs := $(addsuffix .depend,$(sort $(dir $(OBJECTS))))
-
-ifeq ($(COMPUTE_HEADER_DEPENDENCIES),yes)
-$(dep_dirs):
-	@mkdir -p $@
-
-missing_dep_dirs := $(filter-out $(wildcard $(dep_dirs)),$(dep_dirs))
-dep_file = $(dir $@).depend/$(notdir $@).d
-dep_args = -MF $(dep_file) -MQ $@ -MMD -MP
-endif
-
-ifneq ($(COMPUTE_HEADER_DEPENDENCIES),yes)
-dep_dirs =
-missing_dep_dirs =
-dep_args =
-endif
-
 compdb_dir = compile_commands
 
 ifeq ($(GENERATE_COMPILATION_DATABASE),yes)
@@ -2489,10 +2472,22 @@ endif
 ASM_SRC := $(wildcard $(OBJECTS:o=S))
 ASM_OBJ := $(ASM_SRC:S=o)
 C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS))
+C_DEP_OBJ := $(OBJECTS:o=dep)
 
 .SUFFIXES:
 
-$(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
+ifeq ($(COMPUTE_HEADER_DEPENDENCIES),yes)
+$(C_DEP_OBJ): $(GENERATED_H)
+$(C_DEP_OBJ): %.dep : %.c command-list.h
+	$(QUIET_DEP)$(CC) -MF $*.dep -MQ $*.o -MM $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<
+
+ALL.dep: $(C_DEP_OBJ)
+	$(QUIET_DEP)cat $(C_DEP_OBJ) >$@
+
+include ALL.dep
+endif
+
+$(C_OBJ): %.o: %.c %.dep GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
 	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<
 $(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
 	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<
===============================================================================

This is just stealing a pattern we already use for
Documentation/doc.dep.

I.e. here every *.c file depends on a corresponding *.dep file, we
generate the *.dep files and *.c files seperately, instead of making the
*.dep a "while we're at it" as we make the *.o.

It's on obvious WIP, e.g. there's no ASM_DEP_OBJ yet, and e.g. a
"grep.dep" itself should in turn depend on say "strbuf.h", since if we
touch strbuf.h we should re-generate the grep.dep file before we make
the "grep.o" from the "grep.c".

It also means that if you have a clean tree and make "grep.o" we'll
generate all the *.dep files and the ALL.dep first, although skipping
that could be done with some clever use of $(MAKECMDGOALS) if anyone
cared.

It also makes "git clean -dxf; make -j8 all" take ~17s on my box instead
of ~13s.

I wonder if it's worth doing anyway, the cost of doing incremental
compiles isn't much affected.

I think the cases where the current schema bites us are rather obscure
though, but maybe I haven't thought of some obvious ones.

I.e. if we don't have the dep file we'll make it for the first time when
making the grep.o file, then include those generated dependencies.

If one of those dependencies changes we'll update the dependencies by
virtue of re-making the grep.o. I *think* the only edge cases are if
you've created a grep.o with COMPUTE_HEADER_DEPENDENCIES=yes, then
recompiled and it's gained a new dependency while using
COMPUTE_HEADER_DEPENDENCIES=no, and finally you try to recompile with
COMPUTE_HEADER_DEPENDENCIES=yes and it does nothing, because you're
referencing a stale ".depends" directory.

But that could be solved by just making the grep.o depend on its own
dependency file, which we don't do now, but could easily do.

So maybe I've talked myself out of there being an inherent dependency
graph violation with the current schema, i.e. the current one seems like
an easily solved bug.

One good thing the above approach still gives you is that you can use
GCC or Clang to make the dependency graph, but then use another compiler
to compile, i.e. we could have a $(GCC_LIKE) in addition to $(CC), so if
I'm compiling with xlc or suncc I can still use the present GCC just to
create the dependency graph.