From fc890030c1430ead948e96ccd3839832cc3e7246 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 20 Jun 2012 14:30:08 -0400 Subject: [PATCH 01/14] Makefile: sort LIB_H list This was mostly sorted already, but put things like "cache-tree.h" after "cache.h", even though "-" comes before "." (at least in the C locale). This will make it easier to keep the list sorted later by piping it through "sort". Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Makefile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 62de0b42b..72cdb56d3 100644 --- a/Makefile +++ b/Makefile @@ -595,8 +595,8 @@ LIB_H += attr.h LIB_H += blob.h LIB_H += builtin.h LIB_H += bulk-checkin.h -LIB_H += cache.h LIB_H += cache-tree.h +LIB_H += cache.h LIB_H += color.h LIB_H += commit.h LIB_H += compat/bswap.h @@ -636,13 +636,13 @@ LIB_H += mailmap.h LIB_H += merge-file.h LIB_H += merge-recursive.h LIB_H += mergesort.h -LIB_H += notes.h LIB_H += notes-cache.h LIB_H += notes-merge.h +LIB_H += notes.h LIB_H += object.h -LIB_H += pack.h LIB_H += pack-refs.h LIB_H += pack-revindex.h +LIB_H += pack.h LIB_H += parse-options.h LIB_H += patch-ids.h LIB_H += pkt-line.h @@ -668,8 +668,8 @@ LIB_H += submodule.h LIB_H += tag.h LIB_H += thread-utils.h LIB_H += transport.h -LIB_H += tree.h LIB_H += tree-walk.h +LIB_H += tree.h LIB_H += unpack-trees.h LIB_H += userdiff.h LIB_H += utf8.h From b8ba629264b85a2daf0df9c61d00873988d006b7 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 20 Jun 2012 14:30:56 -0400 Subject: [PATCH 02/14] Makefile: fold MISC_H into LIB_H We keep a list of most of the header files in LIB_H, but some are split out into MISC_H. The original point of LIB_H was that it would force recompilation of C files when any of the library headers changed. It was over-encompassing, since not all C files included all of the library headers; this made it simple to maintain, but meant that we sometimes recompiled when it was not necessary. Over time, some new headers were omitted from LIB_H, and rules were added to the Makefile for a few specific targets to explicitly depend on them. This avoided some unnecessary recompilation at the cost of having to maintain the dependency list of those targets manually (e.g., d349a03). Later, we needed a complete list of headers from which we should extract strings to localized. Thus 1b8b2e4 introduced MISC_H to mention all header files not included in LIB_H, and the concatenation of the two lists is fed to xgettext. Headers mentioned as dependencies must also be manually added to MISC_H to receive the benefits of localization. Having to update multiple locations manually is a pain and has led to errors. For example, see "git log -Swt-status.h Makefile" for some back-and-forth between the two locations. Or the fact that column.h was never added to MISC_H, and therefore was not localized (which is fixed by this patch). Moreover, the benefits of keeping these few headers out of LIB_H is not that great, for two reasons: 1. The better way to do this is by auto-computing the dependencies, which is more accurate and less work to maintain. If your compiler supports it, we turn on computed header dependencies by default these days. So these manual dependencies are used only for people who do not have gcc at all (which increases the chance of them becoming stale, as many developers will never even use them). 2. Even if you do not have gcc, the manual header dependencies do not help all that much. They obviously cannot help with an initial compilation (since their purpose is to avoid unnecessary recompilation when a header changes), which means they are only useful when building a new version of git in the working tree that held an existing build (e.g., after checkout or during a bisection). But since a change of a header in LIB_H will force recompilation, and given that the vast majority of headers are in LIB_H, most version changes will result in a full rebuild anyway. Let's just fold MISC_H into LIB_H and get rid of these manual rules. The worst case is some extra compilation, but even that is unlikely to matter due to the reasons above. The one exception is that we should keep common-cmds.h separate. Because it is generated, the computed dependencies do not handle it properly, and we must keep separate individual dependencies on it. Let's therefore rename MISC_H to GENERATED_H to make it more clear what should go in it. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Makefile | 45 ++++++++++++++++----------------------------- 1 file changed, 16 insertions(+), 29 deletions(-) diff --git a/Makefile b/Makefile index 72cdb56d3..500966b10 100644 --- a/Makefile +++ b/Makefile @@ -397,7 +397,7 @@ XDIFF_OBJS = VCSSVN_H = VCSSVN_OBJS = VCSSVN_TEST_OBJS = -MISC_H = +GENERATED_H = EXTRA_CPPFLAGS = LIB_H = LIB_OBJS = @@ -574,30 +574,22 @@ VCSSVN_H += vcs-svn/fast_export.h VCSSVN_H += vcs-svn/svndiff.h VCSSVN_H += vcs-svn/svndump.h -MISC_H += bisect.h -MISC_H += branch.h -MISC_H += bundle.h -MISC_H += common-cmds.h -MISC_H += fetch-pack.h -MISC_H += reachable.h -MISC_H += send-pack.h -MISC_H += shortlog.h -MISC_H += tar.h -MISC_H += thread-utils.h -MISC_H += url.h -MISC_H += walker.h -MISC_H += wt-status.h +GENERATED_H += common-cmds.h LIB_H += advice.h LIB_H += archive.h LIB_H += argv-array.h LIB_H += attr.h +LIB_H += bisect.h LIB_H += blob.h +LIB_H += branch.h LIB_H += builtin.h LIB_H += bulk-checkin.h +LIB_H += bundle.h LIB_H += cache-tree.h LIB_H += cache.h LIB_H += color.h +LIB_H += column.h LIB_H += commit.h LIB_H += compat/bswap.h LIB_H += compat/cygwin.h @@ -618,6 +610,7 @@ LIB_H += diff.h LIB_H += diffcore.h LIB_H += dir.h LIB_H += exec_cmd.h +LIB_H += fetch-pack.h LIB_H += fmt-merge-msg.h LIB_H += fsck.h LIB_H += gettext.h @@ -627,6 +620,7 @@ LIB_H += graph.h LIB_H += grep.h LIB_H += hash.h LIB_H += help.h +LIB_H += http.h LIB_H += kwset.h LIB_H += levenshtein.h LIB_H += list-objects.h @@ -649,6 +643,7 @@ LIB_H += pkt-line.h LIB_H += progress.h LIB_H += prompt.h LIB_H += quote.h +LIB_H += reachable.h LIB_H += reflog-walk.h LIB_H += refs.h LIB_H += remote.h @@ -656,9 +651,11 @@ LIB_H += rerere.h LIB_H += resolve-undo.h LIB_H += revision.h LIB_H += run-command.h +LIB_H += send-pack.h LIB_H += sequencer.h LIB_H += sha1-array.h LIB_H += sha1-lookup.h +LIB_H += shortlog.h LIB_H += sideband.h LIB_H += sigchain.h LIB_H += strbuf.h @@ -666,14 +663,18 @@ LIB_H += streaming.h LIB_H += string-list.h LIB_H += submodule.h LIB_H += tag.h +LIB_H += tar.h LIB_H += thread-utils.h LIB_H += transport.h LIB_H += tree-walk.h LIB_H += tree.h LIB_H += unpack-trees.h +LIB_H += url.h LIB_H += userdiff.h LIB_H += utf8.h LIB_H += varint.h +LIB_H += walker.h +LIB_H += wt-status.h LIB_H += xdiff-interface.h LIB_H += xdiff/xdiff.h @@ -2237,20 +2238,6 @@ else # gcc detects! $(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/index-pack.o builtin/grep.o builtin/pack-objects.o transport-helper.o thread-utils.o: thread-utils.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 -connect.o transport.o url.o http-backend.o: url.h -builtin/branch.o builtin/commit.o builtin/tag.o column.o help.o pager.o: column.h -http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h -http.o http-walker.o http-push.o http-fetch.o remote-curl.o: http.h url.h xdiff-interface.o $(XDIFF_OBJS): $(XDIFF_H) @@ -2347,7 +2334,7 @@ XGETTEXT_FLAGS_C = $(XGETTEXT_FLAGS) --language=C \ --keyword=_ --keyword=N_ --keyword="Q_:1,2" XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell XGETTEXT_FLAGS_PERL = $(XGETTEXT_FLAGS) --keyword=__ --language=Perl -LOCALIZED_C := $(C_OBJ:o=c) $(LIB_H) $(XDIFF_H) $(VCSSVN_H) $(MISC_H) +LOCALIZED_C := $(C_OBJ:o=c) $(LIB_H) $(XDIFF_H) $(VCSSVN_H) $(GENERATED_H) LOCALIZED_SH := $(SCRIPT_SH) LOCALIZED_PERL := $(SCRIPT_PERL) From 60d24dd2556f8b6020f9de37af3ec292c9f3769d Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Fri, 6 Jul 2012 22:39:18 -0500 Subject: [PATCH 03/14] Makefile: fold XDIFF_H and VCSSVN_H into LIB_H Just like MISC_H (see previous commit), there is no reason to track xdiff and vcs-svn headers separately from the rest of the headers. The only purpose of these variables is to keep track of recompilation dependencies. As a pleasant side effect, folding these into LIB_H lets us stop tracking GIT_OBJS and VCSSVN_TEST_OBJS separately from the list of all OBJECTS. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- Makefile | 62 +++++++++++++++++++++++--------------------------------- 1 file changed, 25 insertions(+), 37 deletions(-) diff --git a/Makefile b/Makefile index 500966b10..b24ca20d6 100644 --- a/Makefile +++ b/Makefile @@ -392,11 +392,8 @@ BUILTIN_OBJS = BUILT_INS = COMPAT_CFLAGS = COMPAT_OBJS = -XDIFF_H = XDIFF_OBJS = -VCSSVN_H = VCSSVN_OBJS = -VCSSVN_TEST_OBJS = GENERATED_H = EXTRA_CPPFLAGS = LIB_H = @@ -558,21 +555,21 @@ LIB_FILE=libgit.a XDIFF_LIB=xdiff/lib.a VCSSVN_LIB=vcs-svn/lib.a -XDIFF_H += xdiff/xinclude.h -XDIFF_H += xdiff/xmacros.h -XDIFF_H += xdiff/xdiff.h -XDIFF_H += xdiff/xtypes.h -XDIFF_H += xdiff/xutils.h -XDIFF_H += xdiff/xprepare.h -XDIFF_H += xdiff/xdiffi.h -XDIFF_H += xdiff/xemit.h - -VCSSVN_H += vcs-svn/line_buffer.h -VCSSVN_H += vcs-svn/sliding_window.h -VCSSVN_H += vcs-svn/repo_tree.h -VCSSVN_H += vcs-svn/fast_export.h -VCSSVN_H += vcs-svn/svndiff.h -VCSSVN_H += vcs-svn/svndump.h +LIB_H += xdiff/xinclude.h +LIB_H += xdiff/xmacros.h +LIB_H += xdiff/xdiff.h +LIB_H += xdiff/xtypes.h +LIB_H += xdiff/xutils.h +LIB_H += xdiff/xprepare.h +LIB_H += xdiff/xdiffi.h +LIB_H += xdiff/xemit.h + +LIB_H += vcs-svn/line_buffer.h +LIB_H += vcs-svn/sliding_window.h +LIB_H += vcs-svn/repo_tree.h +LIB_H += vcs-svn/fast_export.h +LIB_H += vcs-svn/svndiff.h +LIB_H += vcs-svn/svndump.h GENERATED_H += common-cmds.h @@ -2110,13 +2107,6 @@ version.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 -ifndef NO_CURL - GIT_OBJS += http.o http-walker.o remote-curl.o -endif - XDIFF_OBJS += xdiff/xdiffi.o XDIFF_OBJS += xdiff/xprepare.o XDIFF_OBJS += xdiff/xutils.o @@ -2132,9 +2122,14 @@ VCSSVN_OBJS += vcs-svn/fast_export.o VCSSVN_OBJS += vcs-svn/svndiff.o VCSSVN_OBJS += vcs-svn/svndump.o -VCSSVN_TEST_OBJS += test-line-buffer.o - -OBJECTS := $(GIT_OBJS) $(XDIFF_OBJS) $(VCSSVN_OBJS) +TEST_OBJS := $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS)) +OBJECTS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \ + $(XDIFF_OBJS) \ + $(VCSSVN_OBJS) \ + git.o +ifndef NO_CURL + OBJECTS += http.o http-walker.o remote-curl.o +endif dep_files := $(foreach f,$(OBJECTS),$(dir $f).depend/$(notdir $f).d) dep_dirs := $(addsuffix .depend,$(sort $(dir $(OBJECTS)))) @@ -2233,15 +2228,8 @@ else # 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) - -xdiff-interface.o $(XDIFF_OBJS): $(XDIFF_H) -$(VCSSVN_OBJS) $(VCSSVN_TEST_OBJS): $(LIB_H) $(VCSSVN_H) +$(OBJECTS): $(LIB_H) endif exec_cmd.sp exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \ @@ -2334,7 +2322,7 @@ XGETTEXT_FLAGS_C = $(XGETTEXT_FLAGS) --language=C \ --keyword=_ --keyword=N_ --keyword="Q_:1,2" XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell XGETTEXT_FLAGS_PERL = $(XGETTEXT_FLAGS) --keyword=__ --language=Perl -LOCALIZED_C := $(C_OBJ:o=c) $(LIB_H) $(XDIFF_H) $(VCSSVN_H) $(GENERATED_H) +LOCALIZED_C := $(C_OBJ:o=c) $(LIB_H) $(GENERATED_H) LOCALIZED_SH := $(SCRIPT_SH) LOCALIZED_PERL := $(SCRIPT_PERL) From eea8c32b92ab7eb49202cd1cf29bb652a0ca02fb Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 20 Jun 2012 14:31:18 -0400 Subject: [PATCH 04/14] Makefile: do not have git.o depend on common-cmds.h This dependency has been stale since 70827b1 (Split up builtin commands into separate files from git.c, 2006-04-21). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Makefile | 1 - 1 file changed, 1 deletion(-) diff --git a/Makefile b/Makefile index b24ca20d6..16bffc5cf 100644 --- a/Makefile +++ b/Makefile @@ -1970,7 +1970,6 @@ shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell strip: $(PROGRAMS) git$X $(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X -git.o: common-cmds.h git.sp git.s git.o: EXTRA_CPPFLAGS = \ '-DGIT_HTML_PATH="$(htmldir_SQ)"' \ '-DGIT_MAN_PATH="$(mandir_SQ)"' \ From c0219dd5d8cd11ea7a112081efd1db59d0bec105 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 20 Jun 2012 14:31:33 -0400 Subject: [PATCH 05/14] Makefile: apply dependencies consistently to sparse/asm targets When a C file "foo.c" depends on a generated header file, we note the dependency for the "foo.o" target. However, we should also note it for other targets that are built from foo.c, like "foo.sp" and "foo.s". These tend to be missed because the latter two are not part of the default build, and are typically built after a regular build which will generate the header. Let's be consistent about including them in dependencies. This also makes us more consistent with nearby lines which tack on EXTRA_CPPFLAGS when building certain files. These flags may sometimes require extra dependencies to be added (e.g., like GIT-VERSION-FILE; this is not the case for any of the updated lines in this patch, but it is establishing a style that will be used in later patches). Technically the ".sp" and ".s" targets do not care about these dependencies, because they are force-built (".sp" because it is a phony target, and ".s" because we explicitly force a rebuild). Since the blocks in question are about communicating "things built from foo.c depend on these flags", it frees the reader from having to know or care more about how those targets are implemented, and why it is OK for only "foo.o" to depend on GIT-VERSION-FILE while "foo.sp" and "foo.s" both are impacted by $(GIT_VERSION). And it helps future-proof us if those force-build details should ever change. This patch explicitly does not update the static header dependencies used when COMPUTED_HEADER_DEPENDENCIES is off. They are similar to the GIT-VERSION-FILE case above, in that technically "foo.s" would depend on its included headers, but it is irrelevant because we force-build it anyway. So it would be tempting to update them in the same way (for readability and future-proofing). However, those rules are meant as a fallback to the computed header dependencies, which do not handle ".s" and ".sp" at all (and are a much harder problem to solve, as gcc is the one generating those dependency lists). So let's leave that harder problem until (and if) somebody wants to change the ".sp" and ".s" rules, and keep the static header dependencies consistent with the computed ones. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 16bffc5cf..3f82b51b8 100644 --- a/Makefile +++ b/Makefile @@ -1979,9 +1979,9 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ git.o \ $(BUILTIN_OBJS) $(ALL_LDFLAGS) $(LIBS) -help.sp help.o: common-cmds.h +help.sp help.s help.o: common-cmds.h -builtin/help.sp builtin/help.o: common-cmds.h +builtin/help.sp builtin/help.s builtin/help.o: common-cmds.h builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \ '-DGIT_HTML_PATH="$(htmldir_SQ)"' \ '-DGIT_MAN_PATH="$(mandir_SQ)"' \ From 47eb28ec0cdb372f4354800e2e0ec713ad4c1992 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 20 Jun 2012 14:31:42 -0400 Subject: [PATCH 06/14] Makefile: do not replace @@GIT_USER_AGENT@@ in scripts No scripts actually care about this replacement. This was erroneously added by 42dcbb7 (version: add git_user_agent function, 2012-06-02). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Makefile | 1 - 1 file changed, 1 deletion(-) diff --git a/Makefile b/Makefile index 3f82b51b8..d41231529 100644 --- a/Makefile +++ b/Makefile @@ -2007,7 +2007,6 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \ -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \ -e 's|@@DIFF@@|$(DIFF_SQ)|' \ -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \ - -e 's|@@GIT_USER_AGENT@@|$(GIT_USER_AGENT_SQ)|g' \ -e 's|@@LOCALEDIR@@|$(localedir_SQ)|g' \ -e 's/@@NO_CURL@@/$(NO_CURL)/g' \ -e 's/@@USE_GETTEXT_SCHEME@@/$(USE_GETTEXT_SCHEME)/g' \ From 620c293abdc36f3db18edd61a622eae30f01ce8e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 20 Jun 2012 14:31:51 -0400 Subject: [PATCH 07/14] Makefile: split GIT_USER_AGENT from GIT-CFLAGS The default user-agent depends on the GIT_VERSION, which means that anytime you switch versions, it causes a full rebuild. Instead, let's split it out into its own file and restrict the dependency to version.o. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- .gitignore | 1 + Makefile | 11 +++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index bf66648e2..7329cfe5c 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ /GIT-CFLAGS /GIT-LDFLAGS /GIT-GUI-VARS +/GIT-USER-AGENT /GIT-VERSION-FILE /bin-wrappers/ /git diff --git a/Makefile b/Makefile index d41231529..2037caa61 100644 --- a/Makefile +++ b/Makefile @@ -1922,7 +1922,11 @@ endif GIT_USER_AGENT_SQ = $(subst ','\'',$(GIT_USER_AGENT)) GIT_USER_AGENT_CQ = "$(subst ",\",$(subst \,\\,$(GIT_USER_AGENT)))" GIT_USER_AGENT_CQ_SQ = $(subst ','\'',$(GIT_USER_AGENT_CQ)) -BASIC_CFLAGS += -DGIT_USER_AGENT='$(GIT_USER_AGENT_CQ_SQ)' +GIT-USER-AGENT: FORCE + @if test x'$(GIT_USER_AGENT_SQ)' != x"`cat GIT-USER-AGENT 2>/dev/null`"; then \ + echo >&2 " * new user-agent flag"; \ + echo '$(GIT_USER_AGENT_SQ)' >GIT-USER-AGENT; \ + fi ALL_CFLAGS += $(BASIC_CFLAGS) ALL_LDFLAGS += $(BASIC_LDFLAGS) @@ -1987,8 +1991,10 @@ builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \ '-DGIT_MAN_PATH="$(mandir_SQ)"' \ '-DGIT_INFO_PATH="$(infodir_SQ)"' +version.sp version.s version.o: GIT-USER-AGENT version.sp version.s version.o: EXTRA_CPPFLAGS = \ - '-DGIT_VERSION="$(GIT_VERSION)"' + '-DGIT_VERSION="$(GIT_VERSION)"' \ + '-DGIT_USER_AGENT=$(GIT_USER_AGENT_CQ_SQ)' $(BUILT_INS): git$X $(QUIET_BUILT_IN)$(RM) $@ && \ @@ -2710,6 +2716,7 @@ ifndef NO_TCLTK $(MAKE) -C git-gui clean endif $(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-GUI-VARS GIT-BUILD-OPTIONS + $(RM) GIT-USER-AGENT .PHONY: all install profile-clean clean strip .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell From 33ddbcb01208f93c20ac4b904ad3d76777144882 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Fri, 6 Jul 2012 23:42:11 -0500 Subject: [PATCH 08/14] Makefile: be silent when only GIT_USER_AGENT changes To avoid noise during builds, unlike the GIT-CFLAGS rule which prints "* new build flags or prefix" so the operator knows why all files are being rebuilt when it changes, GIT-USER-AGENT generation is silent. If this code breaks and a target depending on GIT-USER-AGENT ends up being rebuilt when it shouldn't be, the full dependency chain can be retrieved with "make --debug=b". Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- Makefile | 1 - 1 file changed, 1 deletion(-) diff --git a/Makefile b/Makefile index 2037caa61..aa0e4bbb0 100644 --- a/Makefile +++ b/Makefile @@ -1924,7 +1924,6 @@ GIT_USER_AGENT_CQ = "$(subst ",\",$(subst \,\\,$(GIT_USER_AGENT)))" GIT_USER_AGENT_CQ_SQ = $(subst ','\'',$(GIT_USER_AGENT_CQ)) GIT-USER-AGENT: FORCE @if test x'$(GIT_USER_AGENT_SQ)' != x"`cat GIT-USER-AGENT 2>/dev/null`"; then \ - echo >&2 " * new user-agent flag"; \ echo '$(GIT_USER_AGENT_SQ)' >GIT-USER-AGENT; \ fi From be1dbd0a933749526eb763fd00cd00c0bdf7c7a6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 20 Jun 2012 14:31:55 -0400 Subject: [PATCH 09/14] Makefile: split prefix flags from GIT-CFLAGS Most of the build targets do not care about the setting of $prefix (or its derivative variables), but will be rebuilt if the prefix changes. For most setups this doesn't matter (they set prefix once and never change it), but for a setup which puts each branch or version in its own prefix, this unnecessarily causes a full rebuild whenever the branc is changed. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- .gitignore | 1 + Makefile | 30 ++++++++++++++++++++++-------- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/.gitignore b/.gitignore index 7329cfe5c..c60c5a323 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ /GIT-CFLAGS /GIT-LDFLAGS /GIT-GUI-VARS +/GIT-PREFIX /GIT-USER-AGENT /GIT-VERSION-FILE /bin-wrappers/ diff --git a/Makefile b/Makefile index aa0e4bbb0..8b8164b8b 100644 --- a/Makefile +++ b/Makefile @@ -1973,6 +1973,7 @@ shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell strip: $(PROGRAMS) git$X $(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X +git.sp git.s git.o: GIT-PREFIX git.sp git.s git.o: EXTRA_CPPFLAGS = \ '-DGIT_HTML_PATH="$(htmldir_SQ)"' \ '-DGIT_MAN_PATH="$(mandir_SQ)"' \ @@ -1984,7 +1985,7 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS) help.sp help.s help.o: common-cmds.h -builtin/help.sp builtin/help.s builtin/help.o: common-cmds.h +builtin/help.sp builtin/help.s builtin/help.o: common-cmds.h GIT-PREFIX builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \ '-DGIT_HTML_PATH="$(htmldir_SQ)"' \ '-DGIT_MAN_PATH="$(mandir_SQ)"' \ @@ -2031,7 +2032,7 @@ $(SCRIPT_LIB) : % : %.sh ifndef NO_PERL $(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak -perl/perl.mak: GIT-CFLAGS perl/Makefile perl/Makefile.PL +perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL $(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' $(@F) $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl @@ -2075,7 +2076,7 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)) git-instaweb: % : unimplemented.sh endif # NO_PERL ifndef NO_PYTHON -$(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS +$(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS GIT-PREFIX $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py $(QUIET_GEN)$(RM) $@ $@+ && \ INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C git_remote_helpers -s \ @@ -2235,20 +2236,25 @@ else $(OBJECTS): $(LIB_H) endif +exec_cmd.sp exec_cmd.s exec_cmd.o: GIT-PREFIX exec_cmd.sp exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \ '-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \ '-DBINDIR="$(bindir_relative_SQ)"' \ '-DPREFIX="$(prefix_SQ)"' +builtin/init-db.sp builtin/init-db.s builtin/init-db.o: GIT-PREFIX builtin/init-db.sp builtin/init-db.s builtin/init-db.o: EXTRA_CPPFLAGS = \ -DDEFAULT_GIT_TEMPLATE_DIR='"$(template_dir_SQ)"' +config.sp config.s config.o: GIT-PREFIX config.sp config.s config.o: EXTRA_CPPFLAGS = \ -DETC_GITCONFIG='"$(ETC_GITCONFIG_SQ)"' +attr.sp attr.s attr.o: GIT-PREFIX attr.sp attr.s attr.o: EXTRA_CPPFLAGS = \ -DETC_GITATTRIBUTES='"$(ETC_GITATTRIBUTES_SQ)"' +gettext.sp gettext.s gettext.o: GIT-PREFIX gettext.sp gettext.s gettext.o: EXTRA_CPPFLAGS = \ -DGIT_LOCALE_PATH='"$(localedir_SQ)"' @@ -2372,14 +2378,22 @@ cscope: $(FIND_SOURCE_FILES) | xargs cscope -b ### Detect prefix changes -TRACK_CFLAGS = $(CC):$(subst ','\'',$(ALL_CFLAGS)):\ - $(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ):\ - $(localedir_SQ):$(USE_GETTEXT_SCHEME) +TRACK_PREFIX = $(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ):\ + $(localedir_SQ) + +GIT-PREFIX: FORCE + @FLAGS='$(TRACK_PREFIX)'; \ + if test x"$$FLAGS" != x"`cat GIT-PREFIX 2>/dev/null`" ; then \ + echo 1>&2 " * new prefix flags"; \ + echo "$$FLAGS" >GIT-PREFIX; \ + fi + +TRACK_CFLAGS = $(CC):$(subst ','\'',$(ALL_CFLAGS)):$(USE_GETTEXT_SCHEME) GIT-CFLAGS: FORCE @FLAGS='$(TRACK_CFLAGS)'; \ if test x"$$FLAGS" != x"`cat GIT-CFLAGS 2>/dev/null`" ; then \ - echo 1>&2 " * new build flags or prefix"; \ + echo 1>&2 " * new build flags"; \ echo "$$FLAGS" >GIT-CFLAGS; \ fi @@ -2715,7 +2729,7 @@ ifndef NO_TCLTK $(MAKE) -C git-gui clean endif $(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-GUI-VARS GIT-BUILD-OPTIONS - $(RM) GIT-USER-AGENT + $(RM) GIT-USER-AGENT GIT-PREFIX .PHONY: all install profile-clean clean strip .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell From 2b9391bc675f5435aee0ec9dc3a725c81591bf2d Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 20 Jun 2012 14:32:10 -0400 Subject: [PATCH 10/14] Makefile: do not replace @@GIT_VERSION@@ in shell scripts No shell script actually uses the replacement (it is used in some perl scripts, but cmd_munge_script only handles shell scripts). We can also therefore drop the dependency on GIT-VERSION-FILE. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Makefile | 3 --- 1 file changed, 3 deletions(-) diff --git a/Makefile b/Makefile index 8b8164b8b..faddf9f96 100644 --- a/Makefile +++ b/Makefile @@ -2012,7 +2012,6 @@ $(RM) $@ $@+ && \ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \ -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \ -e 's|@@DIFF@@|$(DIFF_SQ)|' \ - -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \ -e 's|@@LOCALEDIR@@|$(localedir_SQ)|g' \ -e 's/@@NO_CURL@@/$(NO_CURL)/g' \ -e 's/@@USE_GETTEXT_SCHEME@@/$(USE_GETTEXT_SCHEME)/g' \ @@ -2058,7 +2057,6 @@ gitweb: git-instaweb: git-instaweb.sh gitweb $(QUIET_GEN)$(RM) $@ $@+ && \ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \ - -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \ -e 's/@@NO_CURL@@/$(NO_CURL)/g' \ -e 's|@@GITWEBDIR@@|$(gitwebdir_SQ)|g' \ -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \ @@ -2107,7 +2105,6 @@ configure: configure.ac # These can record GIT_VERSION version.o git.spec \ - $(patsubst %.sh,%,$(SCRIPT_SH)) \ $(patsubst %.perl,%,$(SCRIPT_PERL)) \ : GIT-VERSION-FILE From e4dd89ab98466e4d8e5fdadb0576f7e074992f48 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 20 Jun 2012 14:32:16 -0400 Subject: [PATCH 11/14] Makefile: update scripts when build-time parameters change Currently, running: make SHELL_PATH=/bin/bash && make SHELL_PATH=/bin/sh will not rebuild any shell scripts in the second command, leading to incorrect results when building from an unclean working directory. This patch introduces a new dependency meta-file to notice the change. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- .gitignore | 1 + Makefile | 16 +++++++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index c60c5a323..6535cd73a 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,7 @@ /GIT-LDFLAGS /GIT-GUI-VARS /GIT-PREFIX +/GIT-SCRIPT-DEFINES /GIT-USER-AGENT /GIT-VERSION-FILE /bin-wrappers/ diff --git a/Makefile b/Makefile index faddf9f96..03da09605 100644 --- a/Makefile +++ b/Makefile @@ -2007,6 +2007,8 @@ common-cmds.h: ./generate-cmdlist.sh command-list.txt common-cmds.h: $(wildcard Documentation/git-*.txt) $(QUIET_GEN)./generate-cmdlist.sh > $@+ && mv $@+ $@ +SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\ + $(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ) define cmd_munge_script $(RM) $@ $@+ && \ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \ @@ -2019,12 +2021,20 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \ $@.sh >$@+ endef -$(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh +GIT-SCRIPT-DEFINES: FORCE + @FLAGS='$(SCRIPT_DEFINES)'; \ + if test x"$$FLAGS" != x"`cat $@ 2>/dev/null`" ; then \ + echo 1>&2 " * new script parameters"; \ + echo "$$FLAGS" >$@; \ + fi + + +$(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh GIT-SCRIPT-DEFINES $(QUIET_GEN)$(cmd_munge_script) && \ chmod +x $@+ && \ mv $@+ $@ -$(SCRIPT_LIB) : % : %.sh +$(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES $(QUIET_GEN)$(cmd_munge_script) && \ mv $@+ $@ @@ -2726,7 +2736,7 @@ ifndef NO_TCLTK $(MAKE) -C git-gui clean endif $(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-GUI-VARS GIT-BUILD-OPTIONS - $(RM) GIT-USER-AGENT GIT-PREFIX + $(RM) GIT-USER-AGENT GIT-PREFIX GIT-SCRIPT-DEFINES .PHONY: all install profile-clean clean strip .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell From b5295f322c8828e913a3ef126ddffc6862235b03 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 20 Jun 2012 14:32:19 -0400 Subject: [PATCH 12/14] Makefile: build instaweb similar to other scripts Instaweb would not properly rebuild if the build-time parameters changed. Fix this by depending on the GIT-SCRIPT-DEFINES meta-file and using $(cmd_munge_script) like all the other shell scripts. This requires adding a few new parametres to cmd_munge_script, but that doesn't hurt existing scripts. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Makefile | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/Makefile b/Makefile index 03da09605..87ef77706 100644 --- a/Makefile +++ b/Makefile @@ -2008,7 +2008,8 @@ common-cmds.h: $(wildcard Documentation/git-*.txt) $(QUIET_GEN)./generate-cmdlist.sh > $@+ && mv $@+ $@ SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\ - $(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ) + $(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\ + $(gitwebdir_SQ):$(PERL_PATH_SQ) define cmd_munge_script $(RM) $@ $@+ && \ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \ @@ -2018,6 +2019,8 @@ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \ -e 's/@@NO_CURL@@/$(NO_CURL)/g' \ -e 's/@@USE_GETTEXT_SCHEME@@/$(USE_GETTEXT_SCHEME)/g' \ -e $(BROKEN_PATH_FIX) \ + -e 's|@@GITWEBDIR@@|$(gitwebdir_SQ)|g' \ + -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \ $@.sh >$@+ endef @@ -2064,13 +2067,8 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl gitweb: $(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) all -git-instaweb: git-instaweb.sh gitweb - $(QUIET_GEN)$(RM) $@ $@+ && \ - sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \ - -e 's/@@NO_CURL@@/$(NO_CURL)/g' \ - -e 's|@@GITWEBDIR@@|$(gitwebdir_SQ)|g' \ - -e 's|@@PERL@@|$(PERL_PATH_SQ)|g' \ - $@.sh > $@+ && \ +git-instaweb: git-instaweb.sh gitweb GIT-SCRIPT-DEFINES + $(QUIET_GEN)$(cmd_munge_script) && \ chmod +x $@+ && \ mv $@+ $@ else # NO_PERL From 520a6cdce3531867110df012d62d4974376eba1f Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 20 Jun 2012 14:32:22 -0400 Subject: [PATCH 13/14] Makefile: move GIT-VERSION-FILE dependencies closer to use There is a list of all of the targets which depend on GIT-VERSION-FILE, but it can be quite far from the actual point where the targets actually use $(GIT_VERSION). This can make it hard to verify that each use of $(GIT_VERSION) has a matching dependency. This patch moves the dependency closer to the actual build instructions, which makes verification easier. This also fixes the generation of "configure", which did not properly mark the dependency. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Makefile | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/Makefile b/Makefile index 87ef77706..5e43050da 100644 --- a/Makefile +++ b/Makefile @@ -1991,7 +1991,7 @@ builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \ '-DGIT_MAN_PATH="$(mandir_SQ)"' \ '-DGIT_INFO_PATH="$(infodir_SQ)"' -version.sp version.s version.o: GIT-USER-AGENT +version.sp version.s version.o: GIT-VERSION-FILE GIT-USER-AGENT version.sp version.s version.o: EXTRA_CPPFLAGS = \ '-DGIT_VERSION="$(GIT_VERSION)"' \ '-DGIT_USER_AGENT=$(GIT_USER_AGENT_CQ_SQ)' @@ -2047,7 +2047,7 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL $(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' $(@F) -$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl +$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl GIT-VERSION-FILE $(QUIET_GEN)$(RM) $@ $@+ && \ INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \ sed -e '1{' \ @@ -2104,18 +2104,13 @@ $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : unimplemented.sh mv $@+ $@ endif # NO_PYTHON -configure: configure.ac +configure: configure.ac GIT-VERSION-FILE $(QUIET_GEN)$(RM) $@ $<+ && \ sed -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \ $< > $<+ && \ autoconf -o $@ $<+ && \ $(RM) $<+ -# These can record GIT_VERSION -version.o git.spec \ - $(patsubst %.perl,%,$(SCRIPT_PERL)) \ - : GIT-VERSION-FILE - XDIFF_OBJS += xdiff/xdiffi.o XDIFF_OBJS += xdiff/xprepare.o XDIFF_OBJS += xdiff/xutils.o @@ -2650,7 +2645,7 @@ quick-install-html: ### Maintainer's dist rules -git.spec: git.spec.in +git.spec: git.spec.in GIT-VERSION-FILE sed -e 's/@@VERSION@@/$(GIT_VERSION)/g' < $< > $@+ mv $@+ $@ From 7b63c77eeea7cc4bf3d29793bcb95dcf659e5126 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Fri, 6 Jul 2012 23:19:09 -0500 Subject: [PATCH 14/14] Makefile: document ground rules for target-specific dependencies When a source file makes use of a makefile variable, there should be a corresponding dependency on a file that changes when that variable changes to ensure the build output is not left stale when the variable changes. Document this, even though we are not following the rule perfectly yet. Based on an explanation from Jeff King. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- Makefile | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/Makefile b/Makefile index 5e43050da..af95b70fd 100644 --- a/Makefile +++ b/Makefile @@ -1973,6 +1973,39 @@ shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell strip: $(PROGRAMS) git$X $(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X +### Target-specific flags and dependencies + +# The generic compilation pattern rule and automatically +# computed header dependencies (falling back to a dependency on +# LIB_H) are enough to describe how most targets should be built, +# but some targets are special enough to need something a little +# different. +# +# - When a source file "foo.c" #includes a generated header file, +# we need to list that dependency for the "foo.o" target. +# +# We also list it from other targets that are built from foo.c +# like "foo.sp" and "foo.s", even though that is easy to forget +# to do because the generated header is already present around +# after a regular build attempt. +# +# - Some code depends on configuration kept in makefile +# variables. The target-specific variable EXTRA_CPPFLAGS can +# be used to convey that information to the C preprocessor +# using -D options. +# +# The "foo.o" target should have a corresponding dependency on +# a file that changes when the value of the makefile variable +# changes. For example, targets making use of the +# $(GIT_VERSION) variable depend on GIT-VERSION-FILE. +# +# Technically the ".sp" and ".s" targets do not need this +# dependency because they are force-built, but they get the +# same dependency for consistency. This way, you do not have to +# know how each target is implemented. And it means the +# dependencies here will not need to change if the force-build +# details change some day. + git.sp git.s git.o: GIT-PREFIX git.sp git.s git.o: EXTRA_CPPFLAGS = \ '-DGIT_HTML_PATH="$(htmldir_SQ)"' \