From 21528abc3612e3dae0c195b81848dd9aa18d2a02 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Tue, 26 Jan 2010 09:44:47 -0600 Subject: [PATCH 01/15] Makefile: add missing header file dependencies LIB_H is missing exec_cmd.h and color.h. cache.h includes SHA1_HEADER, and thus so does almost everything else, so add that to LIB_H, too. xdiff-interface.h is not included by any header files, but so many source files use xdiff that it is simplest to include it in LIB_H, too. xdiff-interface.o uses the xdiff library heavily; let it depend on all xdiff headers to avoid needing to keep track of which headers it uses. Signed-off-by: Jonathan Nieder --- Makefile | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index fd7f51e8b9..bad8f804c3 100644 --- a/Makefile +++ b/Makefile @@ -447,6 +447,7 @@ LIB_H += blob.h LIB_H += builtin.h LIB_H += cache.h LIB_H += cache-tree.h +LIB_H += color.h LIB_H += commit.h LIB_H += compat/bswap.h LIB_H += compat/cygwin.h @@ -457,6 +458,7 @@ LIB_H += delta.h LIB_H += diffcore.h LIB_H += diff.h LIB_H += dir.h +LIB_H += exec_cmd.h LIB_H += fsck.h LIB_H += git-compat-util.h LIB_H += graph.h @@ -499,6 +501,8 @@ LIB_H += unpack-trees.h LIB_H += userdiff.h LIB_H += utf8.h LIB_H += wt-status.h +LIB_H += xdiff-interface.h +LIB_H += xdiff/xdiff.h LIB_OBJS += abspath.o LIB_OBJS += advice.o @@ -1281,10 +1285,12 @@ endif ifdef BLK_SHA1 SHA1_HEADER = "block-sha1/sha1.h" LIB_OBJS += block-sha1/sha1.o + LIB_H += block-sha1/sha1.h else ifdef PPC_SHA1 SHA1_HEADER = "ppc/sha1.h" LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o + LIB_H += ppc/sha1.h else SHA1_HEADER = EXTLIBS += $(LIB_4_CRYPTO) @@ -1620,9 +1626,9 @@ git-imap-send$X: imap-send.o $(GITLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \ $(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL) -http.o http-walker.o http-push.o: http.h +http.o http-walker.o http-push.o remote-curl.o: http.h -http.o http-walker.o: $(LIB_H) +http.o http-walker.o remote-curl.o: $(LIB_H) git-http-fetch$X: revision.o http.o http-walker.o http-fetch.o $(GITLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \ @@ -1637,14 +1643,25 @@ git-remote-curl$X: remote-curl.o http.o http-walker.o $(GITLIBS) $(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H) $(patsubst git-%$X,%.o,$(PROGRAMS)) git.o: $(LIB_H) $(wildcard */*.h) +builtin-branch.o builtin-checkout.o builtin-clone.o builtin-reset.o branch.o: branch.h +builtin-bundle.o bundle.o transport.o: bundle.h +builtin-bisect--helper.o builtin-rev-list.o bisect.o: bisect.h +builtin-clone.o builtin-fetch-pack.o transport.o: fetch-pack.h +builtin-send-pack.o transport.o: send-pack.h +builtin-log.o builtin-shortlog.o: shortlog.h +builtin-prune.o builtin-reflog.o reachable.o: reachable.h 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 +http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h $(LIB_FILE): $(LIB_OBJS) $(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(LIB_OBJS) XDIFF_OBJS=xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \ xdiff/xmerge.o xdiff/xpatience.o -$(XDIFF_OBJS): xdiff/xinclude.h xdiff/xmacros.h xdiff/xdiff.h xdiff/xtypes.h \ +xdiff-interface.o $(XDIFF_OBJS): \ + xdiff/xinclude.h xdiff/xmacros.h xdiff/xdiff.h xdiff/xtypes.h \ xdiff/xutils.h xdiff/xprepare.h xdiff/xdiffi.h xdiff/xemit.h $(XDIFF_LIB): $(XDIFF_OBJS) From daa99a917293d962ce565a04a09b0de08e32b8ba Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Tue, 26 Jan 2010 09:45:54 -0600 Subject: [PATCH 02/15] Makefile: make sure test helpers are rebuilt when headers change It is not worth the bother to maintain an up-to-date list of which headers each test helper uses, so depend on $(LIB_H) to catch them all. Signed-off-by: Jonathan Nieder --- Makefile | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/Makefile b/Makefile index bad8f804c3..1670ee87c3 100644 --- a/Makefile +++ b/Makefile @@ -402,6 +402,18 @@ PROGRAMS += git-upload-pack$X PROGRAMS += git-var$X PROGRAMS += git-http-backend$X +TEST_PROGRAMS += test-chmtime$X +TEST_PROGRAMS += test-ctype$X +TEST_PROGRAMS += test-date$X +TEST_PROGRAMS += test-delta$X +TEST_PROGRAMS += test-dump-cache-tree$X +TEST_PROGRAMS += test-genrandom$X +TEST_PROGRAMS += test-match-trees$X +TEST_PROGRAMS += test-parse-options$X +TEST_PROGRAMS += test-path-utils$X +TEST_PROGRAMS += test-sha1$X +TEST_PROGRAMS += test-sigchain$X + # List built-in command $C whose implementation cmd_$C() is not in # builtin-$C.o but is linked in as part of some other command. BUILT_INS += $(patsubst builtin-%.o,git-%$X,$(BUILTIN_OBJS)) @@ -695,6 +707,8 @@ BUILTIN_OBJS += builtin-verify-pack.o BUILTIN_OBJS += builtin-verify-tag.o BUILTIN_OBJS += builtin-write-tree.o +TEST_OBJS := $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS)) + GITLIBS = $(LIB_FILE) $(XDIFF_LIB) EXTLIBS = @@ -1642,7 +1656,7 @@ git-remote-curl$X: remote-curl.o http.o http-walker.o $(GITLIBS) $(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H) -$(patsubst git-%$X,%.o,$(PROGRAMS)) git.o: $(LIB_H) $(wildcard */*.h) +$(patsubst git-%$X,%.o,$(PROGRAMS)) $(TEST_OBJS) git.o: $(LIB_H) $(wildcard */*.h) builtin-branch.o builtin-checkout.o builtin-clone.o builtin-reset.o branch.o: branch.h builtin-bundle.o bundle.o transport.o: bundle.h builtin-bisect--helper.o builtin-rev-list.o bisect.o: bisect.h @@ -1732,18 +1746,6 @@ endif ### Testing rules -TEST_PROGRAMS += test-chmtime$X -TEST_PROGRAMS += test-ctype$X -TEST_PROGRAMS += test-date$X -TEST_PROGRAMS += test-delta$X -TEST_PROGRAMS += test-dump-cache-tree$X -TEST_PROGRAMS += test-genrandom$X -TEST_PROGRAMS += test-match-trees$X -TEST_PROGRAMS += test-parse-options$X -TEST_PROGRAMS += test-path-utils$X -TEST_PROGRAMS += test-sha1$X -TEST_PROGRAMS += test-sigchain$X - all:: $(TEST_PROGRAMS) # GNU make supports exporting all variables by "export" without parameters. @@ -1763,9 +1765,7 @@ test-delta$X: diff-delta.o patch-delta.o test-parse-options$X: parse-options.o -test-parse-options.o: parse-options.h - -.PRECIOUS: $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS)) +.PRECIOUS: $(TEST_OBJS) test-%$X: test-%.o $(GITLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) From 7a1894e3039bc554e2a193c895b2ab5300b7f823 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Tue, 26 Jan 2010 09:46:16 -0600 Subject: [PATCH 03/15] Makefile: remove wt-status.h from LIB_H A list of the few translation units using this header is half-populated already. Including the dependency on this header twice (once explicitly, once through LIB_H) makes it difficult to figure out where future headers should be added to the Makefile. Signed-off-by: Jonathan Nieder --- Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 1670ee87c3..8cf6383a9d 100644 --- a/Makefile +++ b/Makefile @@ -512,7 +512,6 @@ LIB_H += tree-walk.h LIB_H += unpack-trees.h LIB_H += userdiff.h LIB_H += utf8.h -LIB_H += wt-status.h LIB_H += xdiff-interface.h LIB_H += xdiff/xdiff.h @@ -1664,7 +1663,7 @@ builtin-clone.o builtin-fetch-pack.o transport.o: fetch-pack.h builtin-send-pack.o transport.o: send-pack.h builtin-log.o builtin-shortlog.o: shortlog.h builtin-prune.o builtin-reflog.o reachable.o: reachable.h -builtin-revert.o wt-status.o: wt-status.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 http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h From 066ddda6cdf2652a85430a979dbf1cd65fbfad0f Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Tue, 26 Jan 2010 09:46:37 -0600 Subject: [PATCH 04/15] Makefile: clean up http-walker.o dependency rules http-walker.o depends on http.h twice: once in the rule listing files that use http.h, and again in the rule explaining how to build it. Messy. Signed-off-by: Jonathan Nieder --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 8cf6383a9d..593801ae1c 100644 --- a/Makefile +++ b/Makefile @@ -1628,7 +1628,7 @@ http.o: http.c GIT-CFLAGS $(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) -DGIT_USER_AGENT='"git/$(GIT_VERSION)"' $< ifdef NO_EXPAT -http-walker.o: http-walker.c http.h GIT-CFLAGS +http-walker.o: http-walker.c GIT-CFLAGS $(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) -DNO_EXPAT $< endif From 3e6577b45e755b53c9cccb24d75916fa3f2e1916 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Tue, 26 Jan 2010 09:46:59 -0600 Subject: [PATCH 05/15] Makefile: drop dependency on $(wildcard */*.h) The files this pulls in are already pulled in by other dependency rules (some recently added). Signed-off-by: Jonathan Nieder --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 593801ae1c..98810b246b 100644 --- a/Makefile +++ b/Makefile @@ -1655,7 +1655,7 @@ git-remote-curl$X: remote-curl.o http.o http-walker.o $(GITLIBS) $(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H) -$(patsubst git-%$X,%.o,$(PROGRAMS)) $(TEST_OBJS) git.o: $(LIB_H) $(wildcard */*.h) +$(patsubst git-%$X,%.o,$(PROGRAMS)) $(TEST_OBJS) git.o: $(LIB_H) builtin-branch.o builtin-checkout.o builtin-clone.o builtin-reset.o branch.o: branch.h builtin-bundle.o bundle.o transport.o: bundle.h builtin-bisect--helper.o builtin-rev-list.o bisect.o: bisect.h From 75df714487fd5d40b370f2d0f993f347d0170599 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Tue, 26 Jan 2010 09:47:25 -0600 Subject: [PATCH 06/15] Makefile: transport.o depends on branch.h now Since commit e9fcd1e2 (Add push --set-upstream, 2010-01-16), transport.c uses branch.h. Signed-off-by: Jonathan Nieder --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 6f0f64753d..5ebd5b2487 100644 --- a/Makefile +++ b/Makefile @@ -1719,7 +1719,7 @@ $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o $(GITLIBS) $(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H) $(patsubst git-%$X,%.o,$(PROGRAMS)) $(TEST_OBJS) git.o: $(LIB_H) -builtin-branch.o builtin-checkout.o builtin-clone.o builtin-reset.o branch.o: branch.h +builtin-branch.o builtin-checkout.o builtin-clone.o builtin-reset.o branch.o transport.o: branch.h builtin-bundle.o bundle.o transport.o: bundle.h builtin-bisect--helper.o builtin-rev-list.o bisect.o: bisect.h builtin-clone.o builtin-fetch-pack.o transport.o: fetch-pack.h From beeb4564bb7133fd12e6811a701686982b10cc2f Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Tue, 26 Jan 2010 09:49:33 -0600 Subject: [PATCH 07/15] Makefile: rearrange dependency rules Put rules listing dependencies of compiled objects (.o files) on header files (.h files) in one place, to make them easier to compare and modify all at once. Add a GIT_OBJS variable listing objects that depend on LIB_H, for similar reasons. No change in build-time behavior intended. Signed-off-by: Jonathan Nieder --- Makefile | 49 +++++++++++++++++++++++++------------------------ 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/Makefile b/Makefile index 5ebd5b2487..1811a9e74e 100644 --- a/Makefile +++ b/Makefile @@ -1666,6 +1666,12 @@ git.o git.spec \ $(patsubst %.perl,%,$(SCRIPT_PERL)) \ : GIT-VERSION-FILE +GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) $(TEST_OBJS) \ + git.o http.o http-walker.o remote-curl.o \ + $(patsubst git-%$X,%.o,$(PROGRAMS)) +XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \ + xdiff/xmerge.o xdiff/xpatience.o + %.o: %.c GIT-CFLAGS $(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $< %.s: %.c GIT-CFLAGS FORCE @@ -1673,6 +1679,25 @@ git.o git.spec \ %.o: %.S GIT-CFLAGS $(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $< +$(GIT_OBJS): $(LIB_H) +builtin-branch.o builtin-checkout.o builtin-clone.o builtin-reset.o branch.o transport.o: branch.h +builtin-bundle.o bundle.o transport.o: bundle.h +builtin-bisect--helper.o builtin-rev-list.o bisect.o: bisect.h +builtin-clone.o builtin-fetch-pack.o transport.o: fetch-pack.h +builtin-send-pack.o transport.o: send-pack.h +builtin-log.o builtin-shortlog.o: shortlog.h +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 +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 + + +xdiff-interface.o $(XDIFF_OBJS): \ + xdiff/xinclude.h xdiff/xmacros.h xdiff/xdiff.h xdiff/xtypes.h \ + xdiff/xutils.h xdiff/xprepare.h xdiff/xdiffi.h xdiff/xemit.h + exec_cmd.s exec_cmd.o: ALL_CFLAGS += \ '-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \ '-DBINDIR="$(bindir_relative_SQ)"' \ @@ -1696,10 +1721,6 @@ git-imap-send$X: imap-send.o $(GITLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \ $(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL) -http.o http-walker.o http-push.o remote-curl.o: http.h - -http.o http-walker.o remote-curl.o: $(LIB_H) - git-http-fetch$X: revision.o http.o http-walker.o http-fetch.o $(GITLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \ $(LIBS) $(CURL_LIBCURL) @@ -1717,29 +1738,9 @@ $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o $(GITLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \ $(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT) -$(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H) -$(patsubst git-%$X,%.o,$(PROGRAMS)) $(TEST_OBJS) git.o: $(LIB_H) -builtin-branch.o builtin-checkout.o builtin-clone.o builtin-reset.o branch.o transport.o: branch.h -builtin-bundle.o bundle.o transport.o: bundle.h -builtin-bisect--helper.o builtin-rev-list.o bisect.o: bisect.h -builtin-clone.o builtin-fetch-pack.o transport.o: fetch-pack.h -builtin-send-pack.o transport.o: send-pack.h -builtin-log.o builtin-shortlog.o: shortlog.h -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 -http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h - $(LIB_FILE): $(LIB_OBJS) $(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(LIB_OBJS) -XDIFF_OBJS=xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \ - xdiff/xmerge.o xdiff/xpatience.o -xdiff-interface.o $(XDIFF_OBJS): \ - xdiff/xinclude.h xdiff/xmacros.h xdiff/xdiff.h xdiff/xtypes.h \ - xdiff/xutils.h xdiff/xprepare.h xdiff/xdiffi.h xdiff/xemit.h - $(XDIFF_LIB): $(XDIFF_OBJS) $(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(XDIFF_OBJS) From 30248886ce89e5467ce734908775ed90b9138e99 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Tue, 26 Jan 2010 09:51:24 -0600 Subject: [PATCH 08/15] Makefile: disable default implicit rules The git makefile never uses any default implicit rules. Unfortunately, if a prerequisite for one of the intended rules is missing, a default rule can be used in its place: $ make var.s CC var.s $ rm var.c $ make var.o as -o var.o var.s Avoiding the default rules avoids this hard-to-debug behavior. It also should speed things up a little in the normal case. Future patches may restrict the scope of the %.o: %.c pattern. This patch would then ensure that for targets not listed, we do not fall back to the default rule. Signed-off-by: Jonathan Nieder --- Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Makefile b/Makefile index 1811a9e74e..8d3299a8f0 100644 --- a/Makefile +++ b/Makefile @@ -1672,6 +1672,8 @@ GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) $(TEST_OBJS) \ XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \ xdiff/xmerge.o xdiff/xpatience.o +.SUFFIXES: + %.o: %.c GIT-CFLAGS $(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $< %.s: %.c GIT-CFLAGS FORCE From c373991375a4903dbb9bea69e2ce11ce819253e2 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Tue, 26 Jan 2010 09:52:11 -0600 Subject: [PATCH 09/15] Makefile: list generated object files in OBJECTS Set the OBJECTS variable to a comprehensive list of all object file targets. To make sure it is truly comprehensive, restrict the scope of the %.o pattern rule to only generate objects in this list. Attempts to build other object files will fail loudly: $ touch foo.c $ make foo.o make: *** No rule to make target `foo.o'. Stop. providing a reminder to add the new object to the OBJECTS list. The new variable is otherwise unused. The intent is for later patches to take advantage of it. Signed-off-by: Jonathan Nieder --- Makefile | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 8d3299a8f0..a3774f7172 100644 --- a/Makefile +++ b/Makefile @@ -1671,14 +1671,19 @@ GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) $(TEST_OBJS) \ $(patsubst git-%$X,%.o,$(PROGRAMS)) XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \ xdiff/xmerge.o xdiff/xpatience.o +OBJECTS := $(GIT_OBJS) $(XDIFF_OBJS) + +ASM_SRC := $(wildcard $(OBJECTS:o=S)) +ASM_OBJ := $(ASM_SRC:S=o) +C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS)) .SUFFIXES: -%.o: %.c GIT-CFLAGS +$(C_OBJ): %.o: %.c GIT-CFLAGS $(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $< %.s: %.c GIT-CFLAGS FORCE $(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $< -%.o: %.S GIT-CFLAGS +$(ASM_OBJ): %.o: %.S GIT-CFLAGS $(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $< $(GIT_OBJS): $(LIB_H) From dfea575017ddc2ae7c9e8dcb978f4557f07715d3 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Tue, 26 Jan 2010 09:52:49 -0600 Subject: [PATCH 10/15] Makefile: lazily compute header dependencies Use the gcc -MMD -MP -MF options to generate dependency rules as a byproduct when building .o files if the COMPUTE_HEADER_DEPENDENCIES variable is defined. That variable is left undefined by default for now. As each object file is built, write a makefile fragment containing its dependencies in the deps/ subdirectory of its containing directory. The deps/ directories should be generated if they are missing at the start of each build. So let each object file depend on $(missing_dep_dirs), which lists only the directories of this kind that are missing to avoid needlessly regenerating files when the directories' timestamps change. gcc learned the -MMD -MP -MF options in version 3.0, so most gcc users should have them by now. The dependencies this option computes are more specific than the rough estimates hard-coded in the Makefile, greatly speeding up rebuilds when only a little-used header file has changed. Signed-off-by: Jonathan Nieder --- .gitignore | 1 + Makefile | 49 ++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/.gitignore b/.gitignore index 8df8f88bea..7b3acb7664 100644 --- a/.gitignore +++ b/.gitignore @@ -177,6 +177,7 @@ *.exe *.[aos] *.py[co] +*.o.d *+ /config.mak /autom4te.cache diff --git a/Makefile b/Makefile index a3774f7172..df361737b0 100644 --- a/Makefile +++ b/Makefile @@ -217,6 +217,10 @@ all:: # DEFAULT_EDITOR='~/bin/vi', # DEFAULT_EDITOR='$GIT_FALLBACK_EDITOR', # DEFAULT_EDITOR='"C:\Program Files\Vim\gvim.exe" --nofork' +# +# Define COMPUTE_HEADER_DEPENDENCIES if your compiler supports the -MMD option +# and you want to avoid rebuilding objects when an unrelated header file +# changes. GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN @@ -1677,14 +1681,48 @@ ASM_SRC := $(wildcard $(OBJECTS:o=S)) ASM_OBJ := $(ASM_SRC:S=o) C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS)) +ifdef COMPUTE_HEADER_DEPENDENCIES +dep_dirs := $(addsuffix deps,$(sort $(dir $(OBJECTS)))) +$(dep_dirs): + mkdir -p $@ + +missing_dep_dirs := $(filter-out $(wildcard $(dep_dirs)),$(dep_dirs)) +else +dep_dirs = +missing_dep_dirs = +endif + .SUFFIXES: -$(C_OBJ): %.o: %.c GIT-CFLAGS - $(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $< +$(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) + $(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $< %.s: %.c GIT-CFLAGS FORCE $(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $< -$(ASM_OBJ): %.o: %.S GIT-CFLAGS - $(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $< +$(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs) + $(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $< + +ifdef COMPUTE_HEADER_DEPENDENCIES +# Take advantage of gcc's on-the-fly dependency generation +# See . +dep_files := $(wildcard $(foreach f,$(OBJECTS),$(dir f)deps/$(notdir $f).d)) +ifneq ($(dep_files),) +include $(dep_files) +endif + +dep_file = $(dir $@)deps/$(notdir $@).d +dep_args = -MF $(dep_file) -MMD -MP +else +dep_args = + +# Dependencies on header files, for platforms that do not support +# the gcc -MMD option. +# +# Dependencies on automatically generated headers such as common-cmds.h +# should _not_ be included here, since they are necessary even when +# building an object for the first time. +# +# XXX. Please check occasionally that these include all dependencies +# gcc detects! $(GIT_OBJS): $(LIB_H) builtin-branch.o builtin-checkout.o builtin-clone.o builtin-reset.o branch.o transport.o: branch.h @@ -1700,10 +1738,10 @@ builtin-pack-objects.o: thread-utils.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 - xdiff-interface.o $(XDIFF_OBJS): \ xdiff/xinclude.h xdiff/xmacros.h xdiff/xdiff.h xdiff/xtypes.h \ xdiff/xutils.h xdiff/xprepare.h xdiff/xdiffi.h xdiff/xemit.h +endif exec_cmd.s exec_cmd.o: ALL_CFLAGS += \ '-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \ @@ -2011,6 +2049,7 @@ clean: $(RM) $(ALL_PROGRAMS) $(BUILT_INS) git$X $(RM) $(TEST_PROGRAMS) $(RM) -r bin-wrappers + $(RM) -r $(dep_dirs) $(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h TAGS tags cscope* $(RM) -r autom4te.cache $(RM) config.log config.mak.autogen config.mak.append config.status config.cache From 1b22c99c14b30f0add6f108a5883f003a2d81e01 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Tue, 26 Jan 2010 09:54:23 -0600 Subject: [PATCH 11/15] Makefile: list standalone program object files in PROGRAM_OBJS Because of new commands like git-remote-http, the OBJECTS list contains fictitious objects such as remote-http.o. Thus any out-of-tree rules that require all $(OBJECTS) to be buildable are broken. Add a list of real program objects to avoid this problem. To avoid duplication of effort, calculate the command list in the PROGRAMS variable using the expansion of PROGRAM_OBJS. This calculation occurs at the time $(PROGRAMS) is expanded, so later additions to PROGRAM_OBJS will be reflected in it, provided they occur before the build rules begin on line 1489. Signed-off-by: Jonathan Nieder --- Makefile | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/Makefile b/Makefile index df361737b0..630687f26e 100644 --- a/Makefile +++ b/Makefile @@ -341,6 +341,7 @@ COMPAT_CFLAGS = COMPAT_OBJS = LIB_H = LIB_OBJS = +PROGRAM_OBJS = PROGRAMS = SCRIPT_PERL = SCRIPT_PYTHON = @@ -390,12 +391,15 @@ EXTRA_PROGRAMS = # ... and all the rest that could be moved out of bindir to gitexecdir PROGRAMS += $(EXTRA_PROGRAMS) -PROGRAMS += git-fast-import$X -PROGRAMS += git-imap-send$X -PROGRAMS += git-shell$X -PROGRAMS += git-show-index$X -PROGRAMS += git-upload-pack$X -PROGRAMS += git-http-backend$X + +PROGRAM_OBJS += fast-import.o +PROGRAM_OBJS += imap-send.o +PROGRAM_OBJS += shell.o +PROGRAM_OBJS += show-index.o +PROGRAM_OBJS += upload-pack.o +PROGRAM_OBJS += http-backend.o + +PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS)) TEST_PROGRAMS_NEED_X += test-chmtime TEST_PROGRAMS_NEED_X += test-ctype @@ -1139,11 +1143,12 @@ else REMOTE_CURL_PRIMARY = git-remote-http$X REMOTE_CURL_ALIASES = git-remote-https$X git-remote-ftp$X git-remote-ftps$X REMOTE_CURL_NAMES = $(REMOTE_CURL_PRIMARY) $(REMOTE_CURL_ALIASES) - PROGRAMS += $(REMOTE_CURL_NAMES) git-http-fetch$X + PROGRAM_OBJS += http-fetch.o + PROGRAMS += $(REMOTE_CURL_NAMES) curl_check := $(shell (echo 070908; curl-config --vernum) | sort -r | sed -ne 2p) ifeq "$(curl_check)" "070908" ifndef NO_EXPAT - PROGRAMS += git-http-push$X + PROGRAM_OBJS += http-push.o endif endif ifndef NO_EXPAT @@ -1163,7 +1168,7 @@ endif EXTLIBS += -lz ifndef NO_POSIX_ONLY_PROGRAMS - PROGRAMS += git-daemon$X + PROGRAM_OBJS += daemon.o endif ifndef NO_OPENSSL OPENSSL_LIBSSL = -lssl @@ -1670,9 +1675,8 @@ git.o git.spec \ $(patsubst %.perl,%,$(SCRIPT_PERL)) \ : GIT-VERSION-FILE -GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) $(TEST_OBJS) \ - git.o http.o http-walker.o remote-curl.o \ - $(patsubst git-%$X,%.o,$(PROGRAMS)) +GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \ + git.o http.o http-walker.o remote-curl.o XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \ xdiff/xmerge.o xdiff/xpatience.o OBJECTS := $(GIT_OBJS) $(XDIFF_OBJS) From f2fabbf76e458010852ac7ced30594c8ab96b9fd Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Tue, 26 Jan 2010 09:57:15 -0600 Subject: [PATCH 12/15] Teach Makefile to check header dependencies Add a target to use the gcc-generated makefile snippets for dependencies on header files to check the hard-coded dependencies. With this patch applied, if any dependencies are missing, then make clean make COMPUTE_HEADER_DEPENDENCIES=YesPlease make CHECK_HEADER_DEPENDENCIES=YesPlease will produce an error message like the following: CHECK fast-import.o missing dependencies: exec_cmd.h make: *** [fast-import.o] Error 1 Signed-off-by: Jonathan Nieder --- Makefile | 105 ++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 85 insertions(+), 20 deletions(-) diff --git a/Makefile b/Makefile index 630687f26e..45b1f54a63 100644 --- a/Makefile +++ b/Makefile @@ -221,6 +221,9 @@ all:: # Define COMPUTE_HEADER_DEPENDENCIES if your compiler supports the -MMD option # and you want to avoid rebuilding objects when an unrelated header file # changes. +# +# Define CHECK_HEADER_DEPENDENCIES to check for problems in the hard-coded +# dependency rules. GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN @@ -1088,6 +1091,14 @@ endif -include config.mak.autogen -include config.mak +ifdef CHECK_HEADER_DEPENDENCIES +USE_COMPUTED_HEADER_DEPENDENCIES = +endif + +ifdef COMPUTE_HEADER_DEPENDENCIES +USE_COMPUTED_HEADER_DEPENDENCIES = YesPlease +endif + ifdef SANE_TOOL_PATH SANE_TOOL_PATH_SQ = $(subst ','\'',$(SANE_TOOL_PATH)) BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix $(SANE_TOOL_PATH_SQ)|' @@ -1681,9 +1692,7 @@ XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \ xdiff/xmerge.o xdiff/xpatience.o OBJECTS := $(GIT_OBJS) $(XDIFF_OBJS) -ASM_SRC := $(wildcard $(OBJECTS:o=S)) -ASM_OBJ := $(ASM_SRC:S=o) -C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS)) +dep_files := $(foreach f,$(OBJECTS),$(dir $f)deps/$(notdir $f).d) ifdef COMPUTE_HEADER_DEPENDENCIES dep_dirs := $(addsuffix deps,$(sort $(dir $(OBJECTS)))) @@ -1691,33 +1700,89 @@ $(dep_dirs): mkdir -p $@ missing_dep_dirs := $(filter-out $(wildcard $(dep_dirs)),$(dep_dirs)) -else +dep_file = $(dir $@)deps/$(notdir $@).d +dep_args = -MF $(dep_file) -MMD -MP +ifdef CHECK_HEADER_DEPENDENCIES +$(error cannot compute header dependencies outside a normal build. \ +Please unset CHECK_HEADER_DEPENDENCIES and try again) +endif +endif + +ifndef COMPUTE_HEADER_DEPENDENCIES +ifndef CHECK_HEADER_DEPENDENCIES dep_dirs = missing_dep_dirs = +dep_args = endif +endif + +ifdef CHECK_HEADER_DEPENDENCIES +ifndef PRINT_HEADER_DEPENDENCIES +missing_deps = $(filter-out $(notdir $^), \ + $(notdir $(shell $(MAKE) -s $@ \ + CHECK_HEADER_DEPENDENCIES=YesPlease \ + USE_COMPUTED_HEADER_DEPENDENCIES=YesPlease \ + PRINT_HEADER_DEPENDENCIES=YesPlease))) +endif +endif + +ASM_SRC := $(wildcard $(OBJECTS:o=S)) +ASM_OBJ := $(ASM_SRC:S=o) +C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS)) .SUFFIXES: -$(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) - $(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $< -%.s: %.c GIT-CFLAGS FORCE - $(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $< -$(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs) - $(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $< +ifdef PRINT_HEADER_DEPENDENCIES +$(C_OBJ): %.o: %.c FORCE + echo $^ +$(ASM_OBJ): %.o: %.S FORCE + echo $^ -ifdef COMPUTE_HEADER_DEPENDENCIES -# Take advantage of gcc's on-the-fly dependency generation -# See . -dep_files := $(wildcard $(foreach f,$(OBJECTS),$(dir f)deps/$(notdir $f).d)) -ifneq ($(dep_files),) -include $(dep_files) +ifndef CHECK_HEADER_DEPENDENCIES +$(error cannot print header dependencies during a normal build. \ +Please set CHECK_HEADER_DEPENDENCIES and try again) +endif endif -dep_file = $(dir $@)deps/$(notdir $@).d -dep_args = -MF $(dep_file) -MMD -MP -else -dep_args = +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 +$(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 +endif +endif +ifndef CHECK_HEADER_DEPENDENCIES +$(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) + $(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $< +$(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs) + $(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $< +endif + +%.s: %.c GIT-CFLAGS FORCE + $(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $< + +ifdef USE_COMPUTED_HEADER_DEPENDENCIES +# Take advantage of gcc's on-the-fly dependency generation +# See . +dep_files_present := $(wildcard $(dep_files)) +ifneq ($(dep_files_present),) +include $(dep_files_present) +endif +else # Dependencies on header files, for platforms that do not support # the gcc -MMD option. # From ec5e0bb860bf98008d51b8235d19ed3407141295 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sun, 31 Jan 2010 15:23:53 -0600 Subject: [PATCH 13/15] Makefile: tuck away generated makefile fragments in .depend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When building with COMPUTE_HEADER_DEPENDENCIES on, save dependency information to .depend/ instead of deps/ so it does not show up in ‘ls’ output. Otherwise, the extra directories can be distracting. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 45b1f54a63..c9391390b9 100644 --- a/Makefile +++ b/Makefile @@ -1692,15 +1692,15 @@ XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \ xdiff/xmerge.o xdiff/xpatience.o OBJECTS := $(GIT_OBJS) $(XDIFF_OBJS) -dep_files := $(foreach f,$(OBJECTS),$(dir $f)deps/$(notdir $f).d) +dep_files := $(foreach f,$(OBJECTS),$(dir $f).depend/$(notdir $f).d) ifdef COMPUTE_HEADER_DEPENDENCIES -dep_dirs := $(addsuffix deps,$(sort $(dir $(OBJECTS)))) +dep_dirs := $(addsuffix .depend,$(sort $(dir $(OBJECTS)))) $(dep_dirs): mkdir -p $@ missing_dep_dirs := $(filter-out $(wildcard $(dep_dirs)),$(dep_dirs)) -dep_file = $(dir $@)deps/$(notdir $@).d +dep_file = $(dir $@).depend/$(notdir $@).d dep_args = -MF $(dep_file) -MMD -MP ifdef CHECK_HEADER_DEPENDENCIES $(error cannot compute header dependencies outside a normal build. \ From 010acc151981ebfb0e369ffdeaef63803c58ef2b Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sun, 31 Jan 2010 15:37:25 -0600 Subject: [PATCH 14/15] Makefile: always remove .depend directories on 'make clean' Even if COMPUTE_HEADER_DEPENDENCIES is not set, some .o.d files might be lying around from previous builds when it was. This is especially likely because using the CHECK_HEADER_DEPENDENCIES feature requires building sometimes with COMPUTE... on and sometimes with it off. At the end of such an exercise, to get a blank slate, the user ought to be able to just run 'make clean'. Make it so. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index c9391390b9..93e1a9218e 100644 --- a/Makefile +++ b/Makefile @@ -1693,9 +1693,9 @@ XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \ OBJECTS := $(GIT_OBJS) $(XDIFF_OBJS) dep_files := $(foreach f,$(OBJECTS),$(dir $f).depend/$(notdir $f).d) +dep_dirs := $(addsuffix .depend,$(sort $(dir $(OBJECTS)))) ifdef COMPUTE_HEADER_DEPENDENCIES -dep_dirs := $(addsuffix .depend,$(sort $(dir $(OBJECTS)))) $(dep_dirs): mkdir -p $@ From 5b8805e75cd54439e2b0208e9512420794bc1ecd Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sun, 28 Feb 2010 03:11:55 -0600 Subject: [PATCH 15/15] Makefile: clarify definition of TEST_OBJS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The definition of TEST_OBJS in commit daa99a91 (Makefile: make sure test helpers are rebuilt when headers change, 2010-01-26) moved a use of $X to before the platform-specific section where it gets defined. There are at least two ways to fix that: - Change the definition of TEST_OBJS to use the = delayed evaluation operator. This way, one need not worry about $(X) needing to be defined before TEST_OBJS is set. - Move the definition of TEST_OBJS to below the definition of $X. Carry out the second. The later site of definition makes the code more readable, since now a reader only has to look down one line to see what TEST_OBJS is meant to be used for. Oddly enough, with or without this change the behavior of the Makefile is the same. Since TEST_PROGRAMS is defined with delayed evaluation, the value of TEST_OBJS := $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS)) is independent of the value of $X when it is evaluated: the $X in the pattern and the $X in $(TEST_PROGRAMS) will simply always cancel out. Make sure $X has the expected expansion anyway to make the code and the reader’s sanity more robust in the face of future changes. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 93e1a9218e..9dd995df75 100644 --- a/Makefile +++ b/Makefile @@ -738,8 +738,6 @@ BUILTIN_OBJS += builtin-verify-pack.o BUILTIN_OBJS += builtin-verify-tag.o BUILTIN_OBJS += builtin-write-tree.o -TEST_OBJS := $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS)) - GITLIBS = $(LIB_FILE) $(XDIFF_LIB) EXTLIBS = @@ -1686,6 +1684,7 @@ git.o git.spec \ $(patsubst %.perl,%,$(SCRIPT_PERL)) \ : GIT-VERSION-FILE +TEST_OBJS := $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS)) GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \ git.o http.o http-walker.o remote-curl.o XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \