[PATCH] Makefile: determine the list of header files using a glob

[PATCH] Makefile: determine the list of header files using a glob

[Johannes Sixt]

From: Johannes Sixt <j6t@kdbg.org>

The list of header files was incomplete because a number of header files
were added to the code base, but were not added to the list LIB_H that we
have in the Makefile. This meant that no rebuild was triggered if one of
the missing headers was changed because we do not have automatic
dependency tracking, either.

Sidestep the issue by computing the list using $(wildcard).

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 Makefile |   63 +------------------------------------------------------------
 1 files changed, 2 insertions(+), 61 deletions(-)

diff --git a/Makefile b/Makefile
index 5a0b3d4..279c7e4 100644
--- a/Makefile
+++ b/Makefile
@@ -326,7 +326,6 @@ BUILTIN_OBJS =
 BUILT_INS =
 COMPAT_CFLAGS =
 COMPAT_OBJS =
-LIB_H =
 LIB_OBJS =
 PROGRAMS =
 SCRIPT_PERL =
@@ -429,65 +428,7 @@ export PERL_PATH
 LIB_FILE=libgit.a
 XDIFF_LIB=xdiff/lib.a

-LIB_H += advice.h
-LIB_H += archive.h
-LIB_H += attr.h
-LIB_H += blob.h
-LIB_H += builtin.h
-LIB_H += cache.h
-LIB_H += cache-tree.h
-LIB_H += commit.h
-LIB_H += compat/bswap.h
-LIB_H += compat/cygwin.h
-LIB_H += compat/mingw.h
-LIB_H += csum-file.h
-LIB_H += decorate.h
-LIB_H += delta.h
-LIB_H += diffcore.h
-LIB_H += diff.h
-LIB_H += dir.h
-LIB_H += fsck.h
-LIB_H += git-compat-util.h
-LIB_H += graph.h
-LIB_H += grep.h
-LIB_H += hash.h
-LIB_H += help.h
-LIB_H += levenshtein.h
-LIB_H += list-objects.h
-LIB_H += ll-merge.h
-LIB_H += log-tree.h
-LIB_H += mailmap.h
-LIB_H += merge-recursive.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 += parse-options.h
-LIB_H += patch-ids.h
-LIB_H += pkt-line.h
-LIB_H += progress.h
-LIB_H += quote.h
-LIB_H += reflog-walk.h
-LIB_H += refs.h
-LIB_H += remote.h
-LIB_H += rerere.h
-LIB_H += revision.h
-LIB_H += run-command.h
-LIB_H += sha1-lookup.h
-LIB_H += sideband.h
-LIB_H += sigchain.h
-LIB_H += strbuf.h
-LIB_H += string-list.h
-LIB_H += submodule.h
-LIB_H += tag.h
-LIB_H += transport.h
-LIB_H += tree.h
-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 = $(wildcard *.h */*.h compat/*/*.h)

 LIB_OBJS += abspath.o
 LIB_OBJS += advice.o
@@ -1611,7 +1552,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)) git.o: $(LIB_H)
 builtin-revert.o wt-status.o: wt-status.h

 $(LIB_FILE): $(LIB_OBJS)
-- 
1.6.6.rc0.43.g50037

Re: [PATCH] Makefile: determine the list of header files using a glob

[Mike Hommey]

On Fri, Nov 27, 2009 at 09:04:53AM +0100, Johannes Sixt wrote:
> From: Johannes Sixt <j6t@kdbg.org>
> 
> The list of header files was incomplete because a number of header files
> were added to the code base, but were not added to the list LIB_H that we
> have in the Makefile. This meant that no rebuild was triggered if one of
> the missing headers was changed because we do not have automatic
> dependency tracking, either.
> 
> Sidestep the issue by computing the list using $(wildcard).

I don't know if the current Makefile works with Solaris' make, or if GNU
make has to be used, but $(wildcard) is definitely not supported by
Solaris' make.

Mike

Re: [PATCH] Makefile: determine the list of header files using a glob

[Johannes Sixt]

Mike Hommey schrieb:
> I don't know if the current Makefile works with Solaris' make,...

No, it doesn't. You have to use GNU make anyway.

-- Hannes

Re: [PATCH] Makefile: determine the list of header files using a glob

[Mike Hommey]

On Fri, Nov 27, 2009 at 09:50:47AM +0100, Johannes Sixt wrote:
> Mike Hommey schrieb:
> > I don't know if the current Makefile works with Solaris' make,...
> 
> No, it doesn't. You have to use GNU make anyway.

Then it's fine. But shouldn't that be noted somewhere, like in the
INSTALL file ?

Mike

Re: [PATCH] Makefile: determine the list of header files using a glob

[Johannes Schindelin]

Hi,

On Fri, 27 Nov 2009, Johannes Sixt wrote:

> From: Johannes Sixt <j6t@kdbg.org>
> 
> The list of header files was incomplete because a number of header files
> were added to the code base, but were not added to the list LIB_H that we
> have in the Makefile. This meant that no rebuild was triggered if one of
> the missing headers was changed because we do not have automatic
> dependency tracking, either.
> 
> Sidestep the issue by computing the list using $(wildcard).

Funny; I thought that not all header files are library header files, i.e. 
not all header changes should trigger a full new build of libgit.a.

Am I wrong?

Ciao,
Dscho

P.S.: Something that comes to mind is the http.h header, which should 
really be independent of libgit.a.  Which reminds me: do we _still_ tell 
libgit.a at _compile time_ whether git-remote-http is compiled with cURL?

[PATCH/RFC 0/2] Lazily generate header dependencies

[Jonathan Nieder]

Johannes Schindelin wrote:

> Funny; I thought that not all header files are library header files, i.e. 
> not all header changes should trigger a full new build of libgit.a.

Maybe something like this could help?

Jonathan Nieder (2):
  Makefile: use target-specific variable to pass flags to cc
  Makefile: automatically track header dependencies

 .gitignore |    1 +
 Makefile   |   46 +++++++++++++++++++++++++++++-----------------
 2 files changed, 30 insertions(+), 17 deletions(-)

[PATCH/RFC 1/2] Makefile: use target-specific variable to pass flags to cc

[Jonathan Nieder]

Remove some duplicated Makefile code by reusing the %.o: %.c rule
even for objects that need to be built with special flags.  This
makes the relevant -D parameters more prominent on the command
line and means any changes to the rules for compilation only have
to happen in one place.

Target-specific variables have been supported in GNU make since
version 3.77 from 1998.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Makefile |   31 +++++++++++++++----------------
 1 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/Makefile b/Makefile
index 5a0b3d4..ed0f461 100644
--- a/Makefile
+++ b/Makefile
@@ -1440,19 +1440,18 @@ strip: $(PROGRAMS) git$X
 	$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X
 
 git.o: git.c common-cmds.h GIT-CFLAGS
-	$(QUIET_CC)$(CC) -DGIT_VERSION='"$(GIT_VERSION)"' \
-		'-DGIT_HTML_PATH="$(htmldir_SQ)"' \
-		$(ALL_CFLAGS) -o $@ -c $(filter %.c,$^)
+git.o: ALL_CFLAGS += -DGIT_VERSION='"$(GIT_VERSION)"' \
+	'-DGIT_HTML_PATH="$(htmldir_SQ)"'
 
 git$X: git.o $(BUILTIN_OBJS) $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ git.o \
 		$(BUILTIN_OBJS) $(ALL_LDFLAGS) $(LIBS)
 
 builtin-help.o: builtin-help.c common-cmds.h GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) \
-		'-DGIT_HTML_PATH="$(htmldir_SQ)"' \
-		'-DGIT_MAN_PATH="$(mandir_SQ)"' \
-		'-DGIT_INFO_PATH="$(infodir_SQ)"' $<
+builtin-help.o: ALL_CFLAGS += \
+	'-DGIT_HTML_PATH="$(htmldir_SQ)"' \
+	'-DGIT_MAN_PATH="$(mandir_SQ)"' \
+	'-DGIT_INFO_PATH="$(infodir_SQ)"'
 
 $(BUILT_INS): git$X
 	$(QUIET_BUILT_IN)$(RM) $@ && \
@@ -1568,24 +1567,24 @@ git.o git.spec \
 	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
 
 exec_cmd.o: exec_cmd.c GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) \
-		'-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
-		'-DBINDIR="$(bindir_relative_SQ)"' \
-		'-DPREFIX="$(prefix_SQ)"' \
-		$<
+exec_cmd.o: ALL_CFLAGS += \
+	'-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
+	'-DBINDIR="$(bindir_relative_SQ)"' \
+	'-DPREFIX="$(prefix_SQ)"'
 
 builtin-init-db.o: builtin-init-db.c GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) -DDEFAULT_GIT_TEMPLATE_DIR='"$(template_dir_SQ)"' $<
+builtin-init-db.o: ALL_CFLAGS += \
+	-DDEFAULT_GIT_TEMPLATE_DIR='"$(template_dir_SQ)"'
 
 config.o: config.c GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) -DETC_GITCONFIG='"$(ETC_GITCONFIG_SQ)"' $<
+config.o: ALL_CFLAGS += -DETC_GITCONFIG='"$(ETC_GITCONFIG_SQ)"'
 
 http.o: http.c GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) -DGIT_USER_AGENT='"git/$(GIT_VERSION)"' $<
+http.o: ALL_CFLAGS += -DGIT_USER_AGENT='"git/$(GIT_VERSION)"'
 
 ifdef NO_EXPAT
 http-walker.o: http-walker.c http.h GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) -DNO_EXPAT $<
+http-walker.o: ALL_CFLAGS += -DNO_EXPAT
 endif
 
 git-%$X: %.o $(GITLIBS)
-- 
1.6.5.3

[PATCH/RFC 2/2] Makefile: automatically compute header dependencies

[Jonathan Nieder]

Use the gcc -MMD -MP -MF options to generate dependency rules as a
byproduct when building .o files.

A bit remains to be done:

 - add the same support to the .c.s rule
 - make this optional (not all compilers support this, and not all
   developers necessarily want to litter the directory with .*.o.d
   files)
 - document what gcc version introduced these options
 - find equivalent options for other compilers (e.g., Intel C,
   SunWSPro, MSVC)

but this should give the idea.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Good idea?  Bad idea?

Good night,
Jonathan

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

diff --git a/.gitignore b/.gitignore
index ac02a58..c7b2736 100644
--- a/.gitignore
+++ b/.gitignore
@@ -170,6 +170,7 @@
 *.exe
 *.[aos]
 *.py[co]
+.*.o.d
 *+
 /config.mak
 /autom4te.cache
diff --git a/Makefile b/Makefile
index ed0f461..af3f874 100644
--- a/Makefile
+++ b/Makefile
@@ -488,6 +488,7 @@ LIB_H += unpack-trees.h
 LIB_H += userdiff.h
 LIB_H += utf8.h
 LIB_H += wt-status.h
+LIB_H :=
 
 LIB_OBJS += abspath.o
 LIB_OBJS += advice.o
@@ -1559,13 +1560,23 @@ git.o git.spec \
 	$(patsubst %.perl,%,$(SCRIPT_PERL)) \
 	: GIT-VERSION-FILE
 
+dep_file = $(dir $@).$(notdir $@).d
+dep_args = -MF $(dep_file) -MMD -MP
+
 %.o: %.c GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
+	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $<
 %.s: %.c GIT-CFLAGS
 	$(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
 %.o: %.S
 	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
 
+objects := $(wildcard *.o block-sha1/*.o arm/*.o ppc/*.o \
+		compat/*.o compat/*/*.o xdiff/*.o)
+dep_files := $(wildcard $(foreach f,$(objects),$(dir $f).$(notdir $f).d))
+ifneq ($(dep_files),)
+include $(dep_files)
+endif
+
 exec_cmd.o: exec_cmd.c GIT-CFLAGS
 exec_cmd.o: ALL_CFLAGS += \
 	'-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
@@ -1875,6 +1886,8 @@ distclean: clean
 clean:
 	$(RM) *.o block-sha1/*.o arm/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \
 		$(LIB_FILE) $(XDIFF_LIB)
+	$(RM) .*.o.d block-sha1/.*.o.d arm/.*.o.d ppc/.*.o.d compat/.*.o.d \
+		compat/*/.*.o.d xdiff/.*.o.d
 	$(RM) $(ALL_PROGRAMS) $(BUILT_INS) git$X
 	$(RM) $(TEST_PROGRAMS)
 	$(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h TAGS tags cscope*
-- 
1.6.5.3

Re: [PATCH] Makefile: determine the list of header files using a glob

[Junio C Hamano]

Mike Hommey <mh@glandium.org> writes:

> On Fri, Nov 27, 2009 at 09:50:47AM +0100, Johannes Sixt wrote:
>> Mike Hommey schrieb:
>> > I don't know if the current Makefile works with Solaris' make,...
>> 
>> No, it doesn't. You have to use GNU make anyway.
>
> Then it's fine. But shouldn't that be noted somewhere, like in the
> INSTALL file ?

Surely.  Please make it so.

Re: [PATCH] Makefile: determine the list of header files using a glob

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> Sidestep the issue by computing the list using $(wildcard).
>
> Funny; I thought that not all header files are library header files, i.e. 
> not all header changes should trigger a full new build of libgit.a.
>
> Am I wrong?

You are right.

Re: [PATCH/RFC 2/2] Makefile: automatically compute header dependencies

[Sverre Rabbelier]

Heya,

On Fri, Nov 27, 2009 at 18:50, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Good idea?  Bad idea?

Ugh, git/git is already a horror to 'ls', adding another n files...
:(. Which brings me back to "if only git had a seperate src/ and maybe
/build directories" :P.


-- 
Cheers,

Sverre Rabbelier

Re: [PATCH/RFC 2/2] Makefile: automatically compute header dependencies

[Jonathan Nieder]

Sverre Rabbelier wrote:

> Ugh, git/git is already a horror to 'ls', adding another n files...
> :(.

They are dotfiles, though depending on how your 'ls' works, that may or
may not help.

> Which brings me back to "if only git had a seperate src/ and maybe
> /build directories" :P.

Hmm, I don’t want to work on that in general, but a separate deps/
directory does not sound like a bad idea at all.

i.e., something vaguely like this.

 .gitignore |    1 +
 Makefile   |   20 ++++++++++++++++++--
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/.gitignore b/.gitignore
index ac02a58..803247f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -170,6 +170,7 @@
 *.exe
 *.[aos]
 *.py[co]
+*.o.d
 *+
 /config.mak
 /autom4te.cache
diff --git a/Makefile b/Makefile
index ed0f461..1cc149b 100644
--- a/Makefile
+++ b/Makefile
@@ -488,6 +488,7 @@ LIB_H += unpack-trees.h
 LIB_H += userdiff.h
 LIB_H += utf8.h
 LIB_H += wt-status.h
+LIB_H :=
 
 LIB_OBJS += abspath.o
 LIB_OBJS += advice.o
@@ -1559,13 +1560,23 @@ git.o git.spec \
 	$(patsubst %.perl,%,$(SCRIPT_PERL)) \
 	: GIT-VERSION-FILE
 
+dep_file = $(dir $@)deps/$(notdir $@).d
+dep_args = -MF $(dep_file) -MMD -MP
+
 %.o: %.c GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
+	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $<
 %.s: %.c GIT-CFLAGS
 	$(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
 %.o: %.S
 	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
 
+objects := $(wildcard *.o block-sha1/*.o ppc/*.o compat/*.o \
+		compat/*/*.o xdiff/*.o)
+dep_files := $(wildcard $(foreach f,$(objects),$(dir $f)deps/$(notdir $f).d))
+ifneq ($(dep_files),)
+include $(dep_files)
+endif
+
 exec_cmd.o: exec_cmd.c GIT-CFLAGS
 exec_cmd.o: ALL_CFLAGS += \
 	'-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
@@ -1657,6 +1668,9 @@ TRACK_CFLAGS = $(subst ','\'',$(ALL_CFLAGS)):\
              $(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ)
 
 GIT-CFLAGS: .FORCE-GIT-CFLAGS
+	mkdir -p deps block-sha1/deps ppc/deps compat/deps \
+		compat/regex/deps compat/nedmalloc/deps compat/fnmatch/deps \
+		xdiff/deps
 	@FLAGS='$(TRACK_CFLAGS)'; \
 	    if test x"$$FLAGS" != x"`cat GIT-CFLAGS 2>/dev/null`" ; then \
 		echo 1>&2 "    * new build flags or prefix"; \
@@ -1873,8 +1887,10 @@ distclean: clean
 	$(RM) configure
 
 clean:
-	$(RM) *.o block-sha1/*.o arm/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \
+	$(RM) *.o block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \
 		$(LIB_FILE) $(XDIFF_LIB)
+	$(RM) -r deps block-sha1/deps ppc/deps compat/deps \
+		compat/*/deps xdiff/deps
 	$(RM) $(ALL_PROGRAMS) $(BUILT_INS) git$X
 	$(RM) $(TEST_PROGRAMS)
 	$(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h TAGS tags cscope*
-- 
1.6.5.3

Re: [PATCH/RFC 2/2 v3] Makefile: lazily compute header dependencies

[Andreas Schwab]

Jonathan Nieder <jrnieder@gmail.com> writes:

>  GIT-CFLAGS: .FORCE-GIT-CFLAGS
> +	mkdir -p deps block-sha1/deps ppc/deps compat/deps \
> +		compat/regex/deps compat/nedmalloc/deps compat/fnmatch/deps \
> +		xdiff/deps

IMHO the list of directories should be factored out in a variable for
easier maintenance.

> @@ -1873,8 +1898,10 @@ distclean: clean
>  	$(RM) configure
>  
>  clean:
> -	$(RM) *.o block-sha1/*.o arm/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \
> +	$(RM) *.o block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \
>  		$(LIB_FILE) $(XDIFF_LIB)
> +	$(RM) -r deps block-sha1/deps ppc/deps compat/deps \
> +		compat/*/deps xdiff/deps

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

[PATCH/RFC 2/2 v3] Makefile: lazily compute header dependencies

[Jonathan Nieder]

Use the gcc -MMD -MP -MF options to generate dependencies as a
byproduct of building .o files.

This feature has to be optional (I don’t think MSVC, for example,
supports anything like this), so unless someone hooks in a rule
to check the static header dependencies for correctness, this
won’t help much with header dependency maintainance.  It is
enabled by setting the COMPUTE_HEADER_DEPENDENCIES variable,
unset by default.

The scope of the %.o: %.c pattern rule has been restricted to
make it easier to tell if a new object file has not been hooked
into the dependency generation machinery.

An unrelated fix also snuck in: the %.s: %.c pattern rule to
generate an assembler listing did not have correct dependencies.
It is meant to be invoked by hand and should always run.

To avoid litering the build directory with even more build
products, the generated Makefile fragments are squirreled away
into deps/ subdirectories of each directory containing object
files.  These directories are currently generated as a
side-effect of the GIT-CFLAGS rule, to guarantee they will be
available whenever the %.o: %.c and %.o: %.S pattern rules are
being used.  This is really not ideal, especially because it
requires hard-coding the list of directories with objects.

gcc learned the -MMD -MP -MF options in version 3.0, so most gcc
users should have them by now.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
I’ll send the .c.s and dependency fixes separately.

Thoughts? Advice?

Thanks,
Jonathan

 .gitignore |    1 +
 Makefile   |   63 ++++++++++++++++++++++++++++++++++++++++++-----------------
 2 files changed, 46 insertions(+), 18 deletions(-)

diff --git a/.gitignore b/.gitignore
index ac02a58..803247f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -170,6 +170,7 @@
 *.exe
 *.[aos]
 *.py[co]
+*.o.d
 *+
 /config.mak
 /autom4te.cache
diff --git a/Makefile b/Makefile
index ed0f461..fb20302 100644
--- a/Makefile
+++ b/Makefile
@@ -216,6 +216,9 @@ all::
 #   DEFAULT_EDITOR='~/bin/vi',
 #   DEFAULT_EDITOR='$GIT_FALLBACK_EDITOR',
 #   DEFAULT_EDITOR='"C:\Program Files\Vim\gvim.exe" --nofork'
+#
+# Define COMPUTE_HEADER_DEPENDENCIES if you want to avoid rebuilding objects
+# when an unrelated header file changes and your compiler supports it.
 
 GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1559,12 +1562,42 @@ git.o git.spec \
 	$(patsubst %.perl,%,$(SCRIPT_PERL)) \
 	: GIT-VERSION-FILE
 
-%.o: %.c GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
-%.s: %.c GIT-CFLAGS
+GIT_OBJS := http.o http-walker.o http-push.o \
+	$(LIB_OBJS) $(BUILTIN_OBJS) \
+	$(patsubst git-%$X,%.o,$(PROGRAMS)) git.o
+
+OBJECTS := $(GIT_OBJS) $(XDIFF_OBJS)
+
+ifndef COMPUTE_HEADER_DEPENDENCIES
+$(GIT_OBJS): $(LIB_H)
+
+$(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
+
+http.o http-walker.o http-push.o: http.h
+
+builtin-revert.o wt-status.o: wt-status.h
+
+$(patsubst git-%$X,%.o,$(PROGRAMS)) git.o: $(wildcard */*.h)
+else
+dep_files := $(wildcard $(foreach f,$(OBJECTS),$(dir $f)deps/$(notdir $f).d))
+
+ifneq ($(dep_files),)
+include $(dep_files)
+endif
+
+# Take advantage of gcc's dependency generation.
+# See <http://gcc.gnu.org/gcc-3.0/features.html>.
+dep_args = -MF $(dep_file) -MMD -MP
+dep_file = $(dir $@)deps/$(notdir $@).d
+endif
+
+$(OBJECTS): %.o: %.c GIT-CFLAGS
+	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $<
+%.s: %.c GIT-CFLAGS .FORCE-LISTING
 	$(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
-%.o: %.S
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
+$(OBJECTS): %.o: %.S GIT-CFLAGS
+	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $<
 
 exec_cmd.o: exec_cmd.c GIT-CFLAGS
 exec_cmd.o: ALL_CFLAGS += \
@@ -1594,10 +1627,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: http.h
-
-http.o http-walker.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)
@@ -1609,22 +1638,15 @@ git-remote-curl$X: 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)) git.o: $(LIB_H) $(wildcard */*.h)
-builtin-revert.o wt-status.o: wt-status.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/xutils.h xdiff/xprepare.h xdiff/xdiffi.h xdiff/xemit.h
 
 $(XDIFF_LIB): $(XDIFF_OBJS)
 	$(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(XDIFF_OBJS)
 
-
 doc:
 	$(MAKE) -C Documentation all
 
@@ -1657,6 +1679,9 @@ TRACK_CFLAGS = $(subst ','\'',$(ALL_CFLAGS)):\
              $(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ)
 
 GIT-CFLAGS: .FORCE-GIT-CFLAGS
+	mkdir -p deps block-sha1/deps ppc/deps compat/deps \
+		compat/regex/deps compat/nedmalloc/deps compat/fnmatch/deps \
+		xdiff/deps
 	@FLAGS='$(TRACK_CFLAGS)'; \
 	    if test x"$$FLAGS" != x"`cat GIT-CFLAGS 2>/dev/null`" ; then \
 		echo 1>&2 "    * new build flags or prefix"; \
@@ -1873,8 +1898,10 @@ distclean: clean
 	$(RM) configure
 
 clean:
-	$(RM) *.o block-sha1/*.o arm/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \
+	$(RM) *.o block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \
 		$(LIB_FILE) $(XDIFF_LIB)
+	$(RM) -r deps block-sha1/deps ppc/deps compat/deps \
+		compat/*/deps xdiff/deps
 	$(RM) $(ALL_PROGRAMS) $(BUILT_INS) git$X
 	$(RM) $(TEST_PROGRAMS)
 	$(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h TAGS tags cscope*
@@ -1899,7 +1926,7 @@ endif
 .PHONY: all install clean strip
 .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
 .PHONY: .FORCE-GIT-VERSION-FILE TAGS tags cscope .FORCE-GIT-CFLAGS
-.PHONY: .FORCE-GIT-BUILD-OPTIONS
+.PHONY: .FORCE-GIT-BUILD-OPTIONS .FORCE-LISTING
 
 ### Check documentation
 #
-- 
1.6.5.3

Re: [PATCH/RFC 2/2 v3] Makefile: lazily compute header dependencies

[Jonathan Nieder]

Andreas Schwab wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:

>>  GIT-CFLAGS: .FORCE-GIT-CFLAGS
>> +	mkdir -p deps block-sha1/deps ppc/deps compat/deps \
>> +		compat/regex/deps compat/nedmalloc/deps compat/fnmatch/deps \
>> +		xdiff/deps
> 
> IMHO the list of directories should be factored out in a variable for
> easier maintenance.

Good idea, thanks.

Perhaps the the value for such a variable could be computed at build time.
e.g.

depdirs := $(addsuffix /deps,$(sort $(dir $(OBJECTS))))

>> @@ -1873,8 +1898,10 @@ distclean: clean
>>  	$(RM) configure
>>  
>>  clean:
>> -	$(RM) *.o block-sha1/*.o arm/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \
>> +	$(RM) *.o block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \
>>  		$(LIB_FILE) $(XDIFF_LIB)
>> +	$(RM) -r deps block-sha1/deps ppc/deps compat/deps \
>> +		compat/*/deps xdiff/deps

Re: [PATCH] Makefile: determine the list of header files using a glob

[Mike Hommey]

(Nanako's bunch of reminders reminded me of this thread)

On Fri, Nov 27, 2009 at 10:28:38AM -0800, Junio C Hamano wrote:
> Mike Hommey <mh@glandium.org> writes:
> 
> > On Fri, Nov 27, 2009 at 09:50:47AM +0100, Johannes Sixt wrote:
> >> Mike Hommey schrieb:
> >> > I don't know if the current Makefile works with Solaris' make,...
> >> 
> >> No, it doesn't. You have to use GNU make anyway.
> >
> > Then it's fine. But shouldn't that be noted somewhere, like in the
> > INSTALL file ?
> 
> Surely.  Please make it so.

I had another idea in the interim. If GNU make is necessary, why not
make the Makefile an explicit GNU make only Makefile, by renaming it
GNUmakefile ?

That wouldn't remove the need to add a note in the INSTALL file, though.

Mike

Re: [PATCH] Makefile: determine the list of header files using a glob

[Junio C Hamano]

Mike Hommey <mh@glandium.org> writes:

> On Fri, Nov 27, 2009 at 10:28:38AM -0800, Junio C Hamano wrote:
>> Mike Hommey <mh@glandium.org> writes:
>> ...
>> > Then it's fine. But shouldn't that be noted somewhere, like in the
>> > INSTALL file ?
>> 
>> Surely.  Please make it so.
>
> I had another idea in the interim. If GNU make is necessary, why not
> make the Makefile an explicit GNU make only Makefile, by renaming it
> GNUmakefile ?
>
> That wouldn't remove the need to add a note in the INSTALL file, though.

I think you answered the question yourself.

Personally, when I am dealing with other people's projects, I dislike ones
that do not name their own Makefile "Makefile", as I traditionally have
done my local customization by having a higher precedence Makefile
("makefile" in the old world order, or "GNUmakefile" for gmake) that sets
things up and then call into upstream "Makefile".  For this project, since
I am the upstream, I do want to keep the file named "Makefile".

Re: [PATCH/RFC 0/2] Lazily generate header dependencies

[Nanako Shiraishi]

Junio, could you tell us what happened to this thread?

Makefile improvements.  No discussion.

Re: [PATCH/RFC 0/2] Lazily generate header dependencies

[Junio C Hamano]

Nanako Shiraishi <nanako3@lavabit.com> writes:

> Junio, could you tell us what happened to this thread?
>
> Makefile improvements.  No discussion.

I was mildly interested in the series, but after this:

http://thread.gmane.org/gmane.comp.version-control.git/133872/focus=133911

the progress seems to have stopped.

Re: [PATCH/RFC 0/2] Lazily generate header dependencies

[Johannes Schindelin]

Hi,

On Tue, 5 Jan 2010, Junio C Hamano wrote:

> Nanako Shiraishi <nanako3@lavabit.com> writes:
> 
> > Junio, could you tell us what happened to this thread?
> >
> > Makefile improvements.  No discussion.
> 
> I was mildly interested in the series, but after this:
> 
> http://thread.gmane.org/gmane.comp.version-control.git/133872/focus=133911
> 
> the progress seems to have stopped.

Like I said yesterday, I do not want to spend time chasing old threads.  
But if you spend just two more minutes to summarize what came out of that 
thread (two well spent minutes in the global time balance, if you ask me), 
I will gladly comment.

Ciao,
Dscho

Re: [PATCH/RFC 0/2] Lazily generate header dependencies

[Jonathan Nieder]

Hi,

Johannes Schindelin wrote:
> On Tue, 5 Jan 2010, Junio C Hamano wrote:

>> I was mildly interested in the series, but after this:
>> 
>> http://thread.gmane.org/gmane.comp.version-control.git/133872/focus=133911
>> 
>> the progress seems to have stopped.
>
> Like I said yesterday, I do not want to spend time chasing old threads.  
> But if you spend just two more minutes to summarize what came out of that 
> thread (two well spent minutes in the global time balance, if you ask me), 
> I will gladly comment.

I received some feedback (a suggestion to tuck the makefile snippets in
a separate directory and a follow-up suggestion to make the list of
directories needed for this more maintainable) but have not sent an updated
patch yet.  Hopefully tomorrow I will have more to comment on.

Apologies again for the long silence,
Jonathan

[PATCH v2 0/5] Lazily generate header dependencies

[Jonathan Nieder]

Nanako Shiraishi wrote:
> Junio, could you tell us what happened to this thread?
> 
> Makefile improvements.  No discussion.

My bad.  The previous version was very rough because I was not sure
yet how this could help in making the header dependency rules more
maintainable.  If all compilers worth using support something like
gcc's -MD option (does MSVC?), we could switch over completely;
otherwise, we need some way to use the generated dependencies to
check the static ones, or the static ones will go stale.

That is, maybe something like this.  With these patches applied,
running

	echo COMPUTE_HEADER_DEPENDENCIES=YesPlease >> config.mak
	make clean
	make
	make CHECK_HEADER_DEPENDENCIES=YesPlease

will fail unless the dependency rules in the Makefile account for
all #includes gcc noticed with the current configuration.

Patch 5 is a little rough around the edges, but I am hoping it
will convey the idea.

Enjoy,
Jonathan Nieder (5):
  Makefile: rearrange dependency rules
  Makefile: clear list of default rules
  Makefile: list generated object files in OBJECTS macro
  Makefile: lazily compute header dependencies
  Teach Makefile to check header dependencies

 .gitignore |    1 +
 Makefile   |  158 +++++++++++++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 130 insertions(+), 29 deletions(-)

[PATCH 1/5] Makefile: rearrange dependency rules

[Jonathan Nieder]

Collect header dependency rules after the pattern rules to make
it easier to modify them all at once.  No change in behavior is
intended.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Makefile |   26 +++++++++++++-------------
 1 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/Makefile b/Makefile
index 8ce6fd7..fa08535 100644
--- a/Makefile
+++ b/Makefile
@@ -1630,6 +1630,11 @@ git.o git.spec \
 	$(patsubst %.perl,%,$(SCRIPT_PERL)) \
 	: GIT-VERSION-FILE
 
+GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) git.o http.o http-walker.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
@@ -1637,6 +1642,14 @@ git.o git.spec \
 %.o: %.S GIT-CFLAGS
 	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
 
+$(GIT_OBJS): $(LIB_H)
+http.o http-walker.o http-push.o: http.h
+$(patsubst git-%$X,%.o,$(PROGRAMS)) git.o: $(wildcard */*.h)
+builtin-revert.o wt-status.o: wt-status.h
+
+$(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)"' \
@@ -1661,10 +1674,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: http.h
-
-http.o http-walker.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)
@@ -1676,18 +1685,9 @@ git-remote-curl$X: 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)) git.o: $(LIB_H) $(wildcard */*.h)
-builtin-revert.o wt-status.o: wt-status.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/xutils.h xdiff/xprepare.h xdiff/xdiffi.h xdiff/xemit.h
-
 $(XDIFF_LIB): $(XDIFF_OBJS)
 	$(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(XDIFF_OBJS)
 
-- 
1.6.6.rc2

[PATCH 2/5] Makefile: clear list of default rules

[Jonathan Nieder]

The git makefile never uses any default implicit rules.  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 increases performance and avoids
hard-to-debug behaviour.  Especially, once the scope of the
%.o: %.c pattern rule is restricted, we should not fall back
to the default %.o: %.c pattern rule.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
'make -d' reveals that GNU make still ponders the default rules with
this patch applied, though at least it does not use them any more.  Is
it possible to set something like the make '-r' option from within a
makefile?

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

diff --git a/Makefile b/Makefile
index fa08535..9a5d897 100644
--- a/Makefile
+++ b/Makefile
@@ -1635,6 +1635,8 @@ GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) git.o http.o http-walker.o \
 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
-- 
1.6.6.rc2

[PATCH 3/5] Makefile: add OBJECTS variable listing object files

[Jonathan Nieder]

To find the generated dependencies to include, we will need 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 that 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.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Makefile |   34 ++++++++++++++++++++--------------
 1 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/Makefile b/Makefile
index 9a5d897..87de3c3 100644
--- a/Makefile
+++ b/Makefile
@@ -388,6 +388,18 @@ SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH)) \
 # Empty...
 EXTRA_PROGRAMS =
 
+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
+
 # ... and all the rest that could be moved out of bindir to gitexecdir
 PROGRAMS += $(EXTRA_PROGRAMS)
 PROGRAMS += git-fast-import$X
@@ -1634,14 +1646,20 @@ GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) git.o http.o http-walker.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
+TEST_OBJS := $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS))
+OBJECTS := $(GIT_OBJS) $(XDIFF_OBJS) $(TEST_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)
@@ -1757,18 +1775,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.
-- 
1.6.6.rc2

[PATCH 4/5] Makefile: lazily compute header dependencies

[Jonathan Nieder]

Use the gcc -MMD -MP -MF options to generate dependency rules as
a byproduct when building .o files.

This feature has to be optional (MSVC does not seem to support
anything like this), so unfortunately it does not make the
Makefile much easier to maintain.  The feature is enabled by the
COMPUTE_HEADER_DEPENDENCIES variable, which is unset by default.

The generated Makefile fragments are saved in deps/
subdirectories of each directory containing object files.  These
directories are generated if missing at the start of each build.
A dependency on $(filter-out $(wildcard $(dep_dirs)),$(dep_dirs))
avoids 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 estimate hard-coded in the Makefile, greatly speeding up
rebuilds when only a little-used header file has changed.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Timings:

for arg in YesPlease ""
do
	{
		echo NO_CURL=1
		echo NO_TCLTK=1
		echo NO_PERL=1
		echo COMPUTE_HEADER_DEPENDENCIES="$arg"
	} >config.mak
	make
	make clean

	time -p make
	touch levenshtein.h
	time -p make

	make clean
done >/dev/null

Build	COMPUTE_HEADER_DEPENDENCIES	real	user	sys
first	YesPlease			55.06	45.13	5.23
second	YesPlease			3.13	2.04	0.79
first					55.45	45.49	4.99
second					53.14	43.19	4.70

 .gitignore |    1 +
 Makefile   |   40 ++++++++++++++++++++++++++++++++++++----
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/.gitignore b/.gitignore
index ac02a58..803247f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -170,6 +170,7 @@
 *.exe
 *.[aos]
 *.py[co]
+*.o.d
 *+
 /config.mak
 /autom4te.cache
diff --git a/Makefile b/Makefile
index 87de3c3..578843c 100644
--- a/Makefile
+++ b/Makefile
@@ -221,6 +221,9 @@ all::
 #   DEFAULT_EDITOR='~/bin/vi',
 #   DEFAULT_EDITOR='$GIT_FALLBACK_EDITOR',
 #   DEFAULT_EDITOR='"C:\Program Files\Vim\gvim.exe" --nofork'
+#
+# Define COMPUTE_HEADER_DEPENDENCIES if you want to avoid rebuilding objects
+# when an unrelated header file changes and your compiler supports it.
 
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1653,15 +1656,38 @@ 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_dir_dep := $(filter-out $(wildcard $(dep_dirs)),$(dep_dirs))
+
+$(dep_dirs):
+	mkdir -p $@
+else
+dep_dirs =
+dep_dir_dep =
+endif
+
 .SUFFIXES:
 
-$(C_OBJ): %.o: %.c GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
+$(C_OBJ): %.o: %.c GIT-CFLAGS $(dep_dir_dep)
+	$(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 $(dep_dir_dep)
+	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $<
+
+ifdef COMPUTE_HEADER_DEPENDENCIES
+# Take advantage of gcc's dependency generation
+# See <http://gcc.gnu.org/gcc-3.0/features.html>.
+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
 $(GIT_OBJS): $(LIB_H)
 http.o http-walker.o http-push.o: http.h
 $(patsubst git-%$X,%.o,$(PROGRAMS)) git.o: $(wildcard */*.h)
@@ -1670,6 +1696,9 @@ builtin-revert.o wt-status.o: wt-status.h
 $(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
 
+dep_args =
+endif
+
 exec_cmd.s exec_cmd.o: ALL_CFLAGS += \
 	'-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
 	'-DBINDIR="$(bindir_relative_SQ)"' \
@@ -1794,7 +1823,9 @@ test-delta$X: diff-delta.o patch-delta.o
 
 test-parse-options$X: parse-options.o
 
+ifndef COMPUTE_HEADER_DEPENDENCIES
 test-parse-options.o: parse-options.h
+endif
 
 .PRECIOUS: $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS))
 
@@ -1954,6 +1985,7 @@ clean:
 		$(LIB_FILE) $(XDIFF_LIB)
 	$(RM) $(ALL_PROGRAMS) $(BUILT_INS) git$X
 	$(RM) $(TEST_PROGRAMS)
+	$(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
-- 
1.6.6.rc2

[PATCH/RFC 5/5] Teach Makefile to check header dependencies

[Jonathan Nieder]

Portability means we cannot completely switch over to
automatically generated dependencies on header files, since some
compilers do not support them.  This would seem to lead to a
dangerous situation in which the hand-written dependency rules
are needed for some configurations but poorly maintained because
most configurations do not use them.

Luckily, there is a way out: as part of testing git, ask the
build system to verify that the hand-written dependency rules are
consistent with the automatically generated ones.  This patch is
a start towards that goal.

The actual patch requires a few steps:

 1. Separate out a USE_COMPUTED_HEADER_DEPENDENCIES option to
    tell make to use the makefile snippets stored in deps/*
    without necessarily regenerating them;

 2. Add a PRINT_HEADER_DEPENDENCIES option to turn on
    USE_COMPUTED_HEADER_DEPENDENCIES and make the %.o: %.c rule
    print its prerequisites instead of compiling anything;

 3. Add a CHECK_HEADER_DEPENDENCIES option to turn off
    USE_COMPUTED_HEADER_DEPENDENCIES and make the %.o: %.c rule
    check that its prerequisites includes all files listed by
    'make -s PRINT_HEADER_DEPENDENCIES=YesPlease $@' instead of
    compiling anything.

With this patch applied,

	echo COMPUTE_HEADER_DEPENDENCIES=YesPlease >> config.mak
	make clean
	make
	make CHECK_HEADER_DEPENDENCIES=YesPlease

produces a useful error message:

	CHECK fast-import.o
	missing dependencies: exec_cmd.h
	make: *** [fast-import.o] Error 1

Probably we should check for missing deps/%.o.d files to avoid
false negatives if some are missing.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
I look forward to your thoughts.

This is a bit clunky, but it is useful enough to detect a few problems
with the current dependency rules.  Patches should follow soon.

 Makefile |   74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 67 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index 578843c..e642a24 100644
--- a/Makefile
+++ b/Makefile
@@ -224,6 +224,8 @@ all::
 #
 # Define COMPUTE_HEADER_DEPENDENCIES if you want to avoid rebuilding objects
 # when an unrelated header file changes and your compiler supports it.
+#
+# Define CHECK_HEADER_DEPENDENCIES after a successful build to find problems.
 
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1064,6 +1066,28 @@ endif
 -include config.mak.autogen
 -include config.mak
 
+ifdef PRINT_HEADER_DEPENDENCIES
+CHECK_HEADER_DEPENDENCIES = YesPlease
+endif
+
+ifdef CHECK_HEADER_DEPENDENCIES
+ifndef COMPUTE_HEADER_DEPENDENCIES
+$(error checking dependencies requires build with COMPUTE_HEADER_DEPENDENCIES)
+endif
+endif
+
+ifdef COMPUTE_HEADER_DEPENDENCIES
+ifdef CHECK_HEADER_DEPENDENCIES
+ifdef PRINT_HEADER_DEPENDENCIES
+USE_COMPUTED_HEADER_DEPENDENCIES = YesPlease
+else
+USE_COMPUTED_HEADER_DEPENDENCIES =
+endif
+else
+USE_COMPUTED_HEADER_DEPENDENCIES = YesPlease
+endif
+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)|'
@@ -1669,14 +1693,48 @@ endif
 
 .SUFFIXES:
 
+ifdef CHECK_HEADER_DEPENDENCIES
+
+ifdef PRINT_HEADER_DEPENDENCIES
+$(C_OBJ): %.o: %.c FORCE
+	echo $^
+$(ASM_OBJ): %.o: %.S FORCE
+	echo $^
+else
+missing_deps = $(filter-out $^, \
+	$(shell $(MAKE) -s PRINT_HEADER_DEPENDENCIES=1 $@))
+
+$(C_OBJ): %.o: %.c 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 FORCE
+	@set -e; echo CHECK $@; \
+	missing_deps="$(missing_deps)"; \
+	if test "$$missing_deps"; \
+	then \
+		echo missing dependencies: $$missing_deps; \
+		false; \
+	fi
+endif
+
+else
+
 $(C_OBJ): %.o: %.c GIT-CFLAGS $(dep_dir_dep)
 	$(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 $(dep_dir_dep)
 	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $<
 
-ifdef COMPUTE_HEADER_DEPENDENCIES
+endif
+
+%.s: %.c GIT-CFLAGS FORCE
+	$(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
+
+ifdef USE_COMPUTED_HEADER_DEPENDENCIES
 # Take advantage of gcc's dependency generation
 # See <http://gcc.gnu.org/gcc-3.0/features.html>.
 dep_files := $(wildcard $(foreach f,$(OBJECTS),$(dir f)deps/$(notdir $f).d))
@@ -1684,9 +1742,6 @@ 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
 $(GIT_OBJS): $(LIB_H)
 http.o http-walker.o http-push.o: http.h
@@ -1695,7 +1750,12 @@ builtin-revert.o wt-status.o: wt-status.h
 
 $(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
 
+ifdef COMPUTE_HEADER_DEPENDENCIES
+dep_file = $(dir $@)deps/$(notdir $@).d
+dep_args = -MF $(dep_file) -MMD -MP
+else
 dep_args =
 endif
 
@@ -1823,7 +1883,7 @@ test-delta$X: diff-delta.o patch-delta.o
 
 test-parse-options$X: parse-options.o
 
-ifndef COMPUTE_HEADER_DEPENDENCIES
+ifndef USE_COMPUTED_HEADER_DEPENDENCIES
 test-parse-options.o: parse-options.h
 endif
 
-- 
1.6.6.rc2

Re: [PATCH v2 0/5] Lazily generate header dependencies

[Erik Faye-Lund]

On Thu, Jan 7, 2010 at 8:13 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Nanako Shiraishi wrote:
>> Junio, could you tell us what happened to this thread?
>>
>> Makefile improvements.  No discussion.
>
> My bad.  The previous version was very rough because I was not sure
> yet how this could help in making the header dependency rules more
> maintainable.  If all compilers worth using support something like
> gcc's -MD option (does MSVC?), we could switch over completely;
> otherwise, we need some way to use the generated dependencies to
> check the static ones, or the static ones will go stale.

Nope, there's no support for -MD in MSVC. It does have an "/MD"
option, but it means something completely different (link with
multithreaded DLL CRT). There IS the "/showIncludes" option [1], which
should make it possible to do some build-magic to generate the correct
dependency-files, though.

[1] http://msdn.microsoft.com/en-us/library/hdkef6tk(VS.71).aspx

-- 
Erik "kusma" Faye-Lund