{"thread":{"id":"24672","subject":"[PATCH 0/2] add missing dependencies on headers","startedAt":"2010-08-08T21:19:10Z","lastAt":"2010-08-09T04:01:06Z","messageCount":6,"participants":["Jonathan Nieder","Jeff King","Tay Ray Chuan"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"147462","messageId":"20100808211910.GA7210@burratino","threadId":"24672","inReplyTo":null,"subject":"[PATCH 0/2] add missing dependencies on headers","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-08T21:19:10Z","receivedAt":"2010-08-08T21:19:10Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From a local run of “make CHECK_HEADER_DEPENDENCIES=1”.\n\nJonathan Nieder (2):\n Makefile: add missing dependencies on url.h\n Makefile: add missing dependency on http.h\n\n Makefile | 3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n"},{"id":"147463","messageId":"20100808212510.GA6962@burratino","threadId":"24672","inReplyTo":"20100808211910.GA7210@burratino","subject":"[PATCH 1/2] Makefile: add missing dependencies on url.h","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-08T21:25:11Z","receivedAt":"2010-08-08T21:25:11Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"v1.7.2-rc0~56^2 and its parent (decode file:// and ssh://\nURLs, 2010-05-23) introduced a new url library. Update the\nMakefile with the relevant dependencies.\n\nDetected with \"make CHECK_HEADER_DEPENDENCIES=1\".\n\nCc: Jeff King \nSigned-off-by: Jonathan Nieder \n---\n Makefile | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex f33648d..9a233b0 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1854,6 +1854,7 @@ builtin/prune.o builtin/reflog.o reachable.o: reachable.h\n builtin/commit.o builtin/revert.o wt-status.o: wt-status.h\n builtin/tar-tree.o archive-tar.o: tar.h\n builtin/pack-objects.o: thread-utils.h\n+connect.o transport.o http-backend.o: url.h\n http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h\n http.o http-walker.o http-push.o remote-curl.o: http.h\n \n-- \n1.7.2.1.544.ga752d.dirty\n"},{"id":"147467","messageId":"20100808214859.GC6962@burratino","threadId":"24672","inReplyTo":"20100808211910.GA7210@burratino","subject":"[PATCH 2/2] Makefile: add missing dependency on http.h","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-08T21:48:59Z","receivedAt":"2010-08-08T21:48:59Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"v1.7.1-rc0~65^2~2 (http: init and cleanup separately from\nhttp-walker, 2010-03-02) introduced a direct dependency from\nhttp-fetch on the HTTP request library. Declare it.\n\nDetected with \"make CHECK_HEADER_DEPENDENCIES=1\".\n\nCc: Tay Ray Chuan \nSigned-off-by: Jonathan Nieder \n---\nAfter this series, “make COMPUTE_HEADER_DEPENDENCIES=YesPlease”\nsucceeds again.\n\nIdeas for future work:\n\n - speed up the dependency checker, run it regularly;\n - port COMPUTE_HEADER_DEPENDENCIES to non-gcc compilers so it can\n be used by default;\n - speed up the redundant-dependency detector[1] enough to make it\n usable;\n - autoconf test;\n - “make test-makefile” for cron jobs.\n\nAlas, this series did none of those things.\n\n[1] <20100131231103.GA5287@progeny.tock> which doesn’t seem to\nhave made it out into the world. Will resend as reply.\n\n Makefile | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 9a233b0..e151516 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1856,7 +1856,7 @@ builtin/tar-tree.o archive-tar.o: tar.h\n builtin/pack-objects.o: thread-utils.h\n connect.o transport.o http-backend.o: url.h\n http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h\n-http.o http-walker.o http-push.o remote-curl.o: http.h\n+http.o http-walker.o http-push.o http-fetch.o remote-curl.o: http.h\n \n xdiff-interface.o $(XDIFF_OBJS): \\\n \txdiff/xinclude.h xdiff/xmacros.h xdiff/xdiff.h xdiff/xtypes.h \\\n-- \n1.7.2.1.544.ga752d.dirty\n"},{"id":"147468","messageId":"20100808215220.GD6962@burratino","threadId":"24672","inReplyTo":"20100808214859.GC6962@burratino","subject":"[PATCH/RFC 15/12] Makefile: check for unnecessary dependencies with CHECK...","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-08T21:52:20Z","receivedAt":"2010-08-08T21:52:20Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"When the CHECK_HEADER_DEPENDENCIES facility is turned on, report\nfiles in three categories:\n\n a. detected dependencies missing from the hard-coded dependencies\n b. hard-coded dependencies that were neither detected nor in\n $(LIB_H)\n c. duplicate hard-coded dependencies\n\nUnfortunately, due to my ignorance of GNU make syntax, a and b\neach read the detected dependencies from disk independently,\nso this is very slow when using an old laptop drive.\n\nStale dependency rules can easily accumulate unnoticed, so\ndespite the slowdown, this should be a useful automated check.\nThe unnecessary dependencies (b) do not include files from\n$(LIB_H) to avoid noise from the $(GIT_OBJS): $(LIB_H) rule,\nwhich avoids patch noise adjusting dependencies as files start to\nuse different parts of libgit.\n\nSigned-off-by: Jonathan Nieder \n---\nJunio C Hamano wrote:\n> Jonathan Nieder writes:\n\n>> That would not be hard to do, but wouldn’t the $(GIT_OBJS): $(LIB_H)\n>> rule create a lot of noise?\n>>\n>> How about if it checks for duplicate dependencies and unnecessary\n>> dependencies that are not in LIB_H?\n>\n> That would be ideal, I think.\n\nHere is a mockup for that. I still have to speed this up before I\nfind it bearable to use. Currently this forks submakes twice to read\nin the same computed list of dependencies, but this should be\navoidable (maybe using $(call ...)?).\n\n Makefile | 53 ++++++++++++++++++++++++++++++++++++-----------------\n 1 files changed, 36 insertions(+), 17 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 93e1a92..ba4ea79 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1718,11 +1718,42 @@ endif\n \n ifdef CHECK_HEADER_DEPENDENCIES\n ifndef PRINT_HEADER_DEPENDENCIES\n-missing_deps = $(filter-out $(notdir $^), \\\n-\t$(notdir $(shell $(MAKE) -s $@ \\\n+dep = $(notdir $^)\n+all_dep = $(notdir $+)\n+computed_dep = $(notdir $(shell $(MAKE) -s $@ \\\n \t\tCHECK_HEADER_DEPENDENCIES=YesPlease \\\n \t\tUSE_COMPUTED_HEADER_DEPENDENCIES=YesPlease \\\n-\t\tPRINT_HEADER_DEPENDENCIES=YesPlease)))\n+\t\tPRINT_HEADER_DEPENDENCIES=YesPlease))\n+\n+missing_dep = $(filter-out $(dep), $(computed_dep))\n+\n+sloppy_dep := $(notdir $(LIB_H)) $(notdir $(dep_files))\n+extra_dep = $(filter-out $(sloppy_dep) $(computed_dep), $(dep))\n+\n+nondup_dep = $(foreach f, $(dep), \\\n+\t\t$(word $(words $(filter $f,$(all_dep))), $f))\n+# If the list of dependencies including duplicates has the same size\n+# as the list without, there are no dups.\n+dup_dep = $(if $(filter $(words $^),$(words $+)),, \\\n+\t\t$(filter-out $(nondup_dep), $(dep)))\n+\n+cmd_check_deps = @set -e; echo CHECK $@; \\\n+\tmissing_dep=\"$(missing_dep)\"; \\\n+\textra_dep=\"$(extra_dep)\"; \\\n+\tdup_dep=\"$(dup_dep)\"; \\\n+\tif test \"$$missing_dep\"; \\\n+\tthen \\\n+\t\techo missing dependencies: $$missing_dep; \\\n+\tfi; \\\n+\tif test \"$$extra_dep\"; \\\n+\tthen \\\n+\t\techo unnecessary dependencies: $$extra_dep; \\\n+\tfi; \\\n+\tif test \"$$dup_dep\"; \\\n+\tthen \\\n+\t\techo duplicate dependencies: $$dup_dep; \\\n+\tfi; \\\n+\ttest -z \"$$missing_dep$$extra_dep$$dup_dep\"\n endif\n endif\n \n@@ -1747,21 +1778,9 @@ endif\n ifndef PRINT_HEADER_DEPENDENCIES\n ifdef CHECK_HEADER_DEPENDENCIES\n $(C_OBJ): %.o: %.c $(dep_files) FORCE\n-\t@set -e; echo CHECK $@; \\\n-\tmissing_deps=\"$(missing_deps)\"; \\\n-\tif test \"$$missing_deps\"; \\\n-\tthen \\\n-\t\techo missing dependencies: $$missing_deps; \\\n-\t\tfalse; \\\n-\tfi\n+\t$(cmd_check_deps)\n $(ASM_OBJ): %.o: %.S $(dep_files) FORCE\n-\t@set -e; echo CHECK $@; \\\n-\tmissing_deps=\"$(missing_deps)\"; \\\n-\tif test \"$$missing_deps\"; \\\n-\tthen \\\n-\t\techo missing dependencies: $$missing_deps; \\\n-\t\tfalse; \\\n-\tfi\n+\t$(cmd_check_deps)\n endif\n endif\n \n-- \n1.7.0.rc1.457.gef7d.dirty\n"},{"id":"147471","messageId":"20100808230048.GC27206@coredump.intra.peff.net","threadId":"24672","inReplyTo":"20100808212510.GA6962@burratino","subject":"Re: [PATCH 1/2] Makefile: add missing dependencies on url.h","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-08-08T23:00:48Z","receivedAt":"2010-08-08T23:00:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Aug 08, 2010 at 04:25:11PM -0500, Jonathan Nieder wrote:\n\n> v1.7.2-rc0~56^2 and its parent (decode file:// and ssh://\n> URLs, 2010-05-23) introduced a new url library. Update the\n> Makefile with the relevant dependencies.\n> \n> Detected with \"make CHECK_HEADER_DEPENDENCIES=1\".\n\nMakes sense to me. Thanks.\n\n-Peff\n"},{"id":"147480","messageId":"AANLkTik-vjj0Nbv7XJoeiZo5W6E+ygk58C8KDpDJnAru@mail.gmail.com","threadId":"24672","inReplyTo":"20100808214859.GC6962@burratino","subject":"Re: [PATCH 2/2] Makefile: add missing dependency on http.h","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-08-09T04:01:06Z","receivedAt":"2010-08-09T04:01:06Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Mon, Aug 9, 2010 at 5:48 AM, Jonathan Nieder wrote:\n> v1.7.1-rc0~65^2~2 (http: init and cleanup separately from\n> http-walker, 2010-03-02) introduced a direct dependency from\n> http-fetch on the HTTP request library.  Declare it.\n>\n> Detected with \"make CHECK_HEADER_DEPENDENCIES=1\".\n>\n> Cc: Tay Ray Chuan \n> Signed-off-by: Jonathan Nieder \n\nWhoops - thanks for catching this.\n\n Acked-by: Tay Ray Chuan \n\n-- \nCheers,\nRay Chuan\n"}]}