[PATCH 0/2] add missing dependencies on headers

From a local run of “make CHECK_HEADER_DEPENDENCIES=1”.

Jonathan Nieder (2):
  Makefile: add missing dependencies on url.h
  Makefile: add missing dependency on http.h

 Makefile |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

v1.7.2-rc0~56^2 and its parent (decode file:// and ssh://
URLs, 2010-05-23) introduced a new url library.  Update the
Makefile with the relevant dependencies.

Detected with "make CHECK_HEADER_DEPENDENCIES=1".

Cc: Jeff King <redacted>
Signed-off-by: Jonathan Nieder <redacted>
---
 Makefile |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index f33648d..9a233b0 100644
--- a/Makefile
+++ b/Makefile

@@ -1854,6 +1854,7 @@ builtin/prune.o builtin/reflog.o reachable.o: reachable.h
 builtin/commit.o builtin/revert.o wt-status.o: wt-status.h
 builtin/tar-tree.o archive-tar.o: tar.h
 builtin/pack-objects.o: thread-utils.h
+connect.o transport.o http-backend.o: url.h
 http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h
 http.o http-walker.o http-push.o remote-curl.o: http.h
 

-- 
1.7.2.1.544.ga752d.dirty

v1.7.1-rc0~65^2~2 (http: init and cleanup separately from
http-walker, 2010-03-02) introduced a direct dependency from
http-fetch on the HTTP request library.  Declare it.

Detected with "make CHECK_HEADER_DEPENDENCIES=1".

Cc: Tay Ray Chuan <redacted>
Signed-off-by: Jonathan Nieder <redacted>
---
After this series, “make COMPUTE_HEADER_DEPENDENCIES=YesPlease”
succeeds again.

Ideas for future work:

 - speed up the dependency checker, run it regularly;
 - port COMPUTE_HEADER_DEPENDENCIES to non-gcc compilers so it can
   be used by default;
 - speed up the redundant-dependency detector[1] enough to make it
   usable;
 - autoconf test;
 - “make test-makefile” for cron jobs.

Alas, this series did none of those things.

[1] [off-list ref] which doesn’t seem to
have made it out into the world.  Will resend as reply.

 Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 9a233b0..e151516 100644
--- a/Makefile
+++ b/Makefile

@@ -1856,7 +1856,7 @@ builtin/tar-tree.o archive-tar.o: tar.h
 builtin/pack-objects.o: thread-utils.h
 connect.o transport.o http-backend.o: url.h
 http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h
-http.o http-walker.o http-push.o remote-curl.o: http.h
+http.o http-walker.o http-push.o http-fetch.o remote-curl.o: http.h
 
 xdiff-interface.o $(XDIFF_OBJS): \
 	xdiff/xinclude.h xdiff/xmacros.h xdiff/xdiff.h xdiff/xtypes.h \

-- 
1.7.2.1.544.ga752d.dirty

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 <redacted>
---
Junio C Hamano wrote:

Jonathan Nieder [off-list ref] writes:

quoted

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

On Sun, Aug 08, 2010 at 04:25:11PM -0500, Jonathan Nieder wrote:

v1.7.2-rc0~56^2 and its parent (decode file:// and ssh://
URLs, 2010-05-23) introduced a new url library.  Update the
Makefile with the relevant dependencies.

Detected with "make CHECK_HEADER_DEPENDENCIES=1".

Makes sense to me. Thanks.

-Peff

Hi,

On Mon, Aug 9, 2010 at 5:48 AM, Jonathan Nieder [off-list ref] wrote:

v1.7.1-rc0~65^2~2 (http: init and cleanup separately from
http-walker, 2010-03-02) introduced a direct dependency from
http-fetch on the HTTP request library.  Declare it.

Detected with "make CHECK_HEADER_DEPENDENCIES=1".

Cc: Tay Ray Chuan <redacted>
Signed-off-by: Jonathan Nieder <redacted>

Whoops - thanks for catching this.

  Acked-by: Tay Ray Chuan [off-list ref]

-- 
Cheers,
Ray Chuan