[PATCH/RFC 15/12] Makefile: check for unnecessary dependencies with CHECK...

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Aug 8, 2010, 21:52 UTC
Message-ID: <20100808215220.GD6962@burratino>
In-Reply-To: <20100808214859.GC6962@burratino>

When the CHECK_HEADER_DEPENDENCIES facility is turned on, report files in three categories:

 a. detected dependencies missing from the hard-coded dependencies
 b. hard-coded dependencies that were neither detected nor in
    $(LIB_H)
 c. duplicate hard-coded dependencies

Unfortunately, due to my ignorance of GNU make syntax, a and b each read the detected dependencies from disk independently, so this is very slow when using an old laptop drive.

Stale dependency rules can easily accumulate unnoticed, so despite the slowdown, this should be a useful automated check. The unnecessary dependencies (b) do not include files from $(LIB_H) to avoid noise from the $(GIT_OBJS): $(LIB_H) rule, which avoids patch noise adjusting dependencies as files start to use different parts of libgit.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Junio C Hamano wrote:

> Jonathan Nieder <jrnieder@gmail.com> writes:

Show 7 quoted lines

>> That would not be hard to do, but wouldn’t the $(GIT_OBJS): $(LIB_H)
>> rule create a lot of noise?
>>
>> How about if it checks for duplicate dependencies and unnecessary
>> dependencies that are not in LIB_H?
>
> That would be ideal, I think.

Here is a mockup for that. I still have to speed this up before I find it bearable to use. Currently this forks submakes twice to read in the same computed list of dependencies, but this should be avoidable (maybe using $(call ...)?).

 Makefile |   53 ++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 36 insertions(+), 17 deletions(-)

diff --git a/Makefile b/Makefile
index 93e1a92..ba4ea79 100644
--- a/Makefile
+++ b/Makefile
@@ -1718,11 +1718,42 @@ endif
 
 ifdef CHECK_HEADER_DEPENDENCIES
 ifndef PRINT_HEADER_DEPENDENCIES
-missing_deps = $(filter-out $(notdir $^), \
-	$(notdir $(shell $(MAKE) -s $@ \
+dep = $(notdir $^)
+all_dep = $(notdir $+)
+computed_dep = $(notdir $(shell $(MAKE) -s $@ \
 		CHECK_HEADER_DEPENDENCIES=YesPlease \
 		USE_COMPUTED_HEADER_DEPENDENCIES=YesPlease \
-		PRINT_HEADER_DEPENDENCIES=YesPlease)))
+		PRINT_HEADER_DEPENDENCIES=YesPlease))
+
+missing_dep = $(filter-out $(dep), $(computed_dep))
+
+sloppy_dep := $(notdir $(LIB_H)) $(notdir $(dep_files))
+extra_dep = $(filter-out $(sloppy_dep) $(computed_dep), $(dep))
+
+nondup_dep = $(foreach f, $(dep), \
+		$(word $(words $(filter $f,$(all_dep))), $f))
+# If the list of dependencies including duplicates has the same size
+# as the list without, there are no dups.
+dup_dep = $(if $(filter $(words $^),$(words $+)),, \
+		$(filter-out $(nondup_dep), $(dep)))
+
+cmd_check_deps = @set -e; echo CHECK $@; \
+	missing_dep="$(missing_dep)"; \
+	extra_dep="$(extra_dep)"; \
+	dup_dep="$(dup_dep)"; \
+	if test "$$missing_dep"; \
+	then \
+		echo missing dependencies: $$missing_dep; \
+	fi; \
+	if test "$$extra_dep"; \
+	then \
+		echo unnecessary dependencies: $$extra_dep; \
+	fi; \
+	if test "$$dup_dep"; \
+	then \
+		echo duplicate dependencies: $$dup_dep; \
+	fi; \
+	test -z "$$missing_dep$$extra_dep$$dup_dep"
 endif
 endif
 
@@ -1747,21 +1778,9 @@ endif
 ifndef PRINT_HEADER_DEPENDENCIES
 ifdef CHECK_HEADER_DEPENDENCIES
 $(C_OBJ): %.o: %.c $(dep_files) FORCE
-	@set -e; echo CHECK $@; \
-	missing_deps="$(missing_deps)"; \
-	if test "$$missing_deps"; \
-	then \
-		echo missing dependencies: $$missing_deps; \
-		false; \
-	fi
+	$(cmd_check_deps)
 $(ASM_OBJ): %.o: %.S $(dep_files) FORCE
-	@set -e; echo CHECK $@; \
-	missing_deps="$(missing_deps)"; \
-	if test "$$missing_deps"; \
-	then \
-		echo missing dependencies: $$missing_deps; \
-		false; \
-	fi
+	$(cmd_check_deps)
 endif
 endif

-- 
1.7.0.rc1.457.gef7d.dirty