A flaw in dep generation with gcc -MMD?

Hi,

My builtin/.depend/add.o.d says

add.o: .... cache.h ...

Shouldn't it be "builtin/add.o: ... cache.h ..."? I tried to touch cache.h and "make builtin/add.o". It did not remake builtin/add.o. If I modify add.o.d by hand and remake, it works.

Nguyen Thai Ngoc Duy wrote:

Interesting.  What compiler do you use?

Thanks for finding it, Jonathan

 $ head -1 builtin/.depend/add.o.d
 builtin/add.o: builtin/add.c cache.h git-compat-util.h compat/bswap.h \
 $ gcc --version | head -1
 gcc (Debian 4.6.2-4) 4.6.2

On Fri, Nov 18, 2011 at 10:41 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:

$ gcc --version gcc (Gentoo 4.4.4-r2 p1.2, pie-0.4.5) 4.4.4

Hmm.. I guess it's my compiler's fault then. Next question, how can I disable this feature?

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

FWIW, gcc 4.4.6 on debian does the correct thing too...

-Miles

On Fri, Nov 18, 2011 at 11:49 AM, Miles Bader <miles@gnu.org> wrote:

OK it's not gcc problem. I upgraded to 4.5.3 and still had the same problem. I used ccache though. Without ccache, gcc produced correct .o.d files.

On Fri, Nov 18, 2011 at 6:34 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:

"gcc -MF depfile -MMD -MP -c -o path/to/file.o" will produce "depfile" with target given by "-o". When ccache runs, it executes "gcc -MF depfile -MMD -MP -E" instead to get the final content for hashing. Notice that "-c -o" combination is replaced by "-E". The latter produces target without leading path.

Not sure if I should report this to ccache or gcc. In the meantime, may be we should recognize the situation and switch off COMPUTE_HEADER_DEPENDENCIES when ccache is used (maybe hard).

Nguyen Thai Ngoc Duy <pclouds <at> gmail.com> writes:

wrote:

I'm pretty sure you should report this against ccache; GCC seems to be behaving as documented. (I believe there is another flag that ccache should be passing in to tell GCC what target to use in the depfile.)

Of course, it'd be wise to make sure that this problem still occurs in the most recent version of ccache you can conveniently try it with before reporting...

Hi,

(restoring cc list.  Unfortunately gmane's web interface doesn't
 provide a built-in way to reply-to-all.  [1] has a hackish
 workaround.)
Samuel Bronson wrote:

[...]

Seems to have been fixed by v3.0pre0~187:

 commit e8354384
 Author: Andrea Bittau <a.bittau@cs.ucl.ac.uk>
 Date:   Sun Nov 1 19:39:58 2009 +0100

     Fix handling of the -MD and -MDD options

     From <http://lists.samba.org/archive/ccache/2007q2/000272.html>:

     The -MD and -MDD options automatically determine where the dependency file
     should land and what the target should look like based on the -o option.
     However, ccache drops -o and things mess up. The original patch was posted by
     Kaz Kylheku but I reworked it to make it work properly. Here is his post:

     http://lists.samba.org/archive/ccache/2006q4/000249.html

Workaround incoming in a few moments.

Thanks, Jonathan

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

I'm also using ccache (version 3.1.3) and get correct dependencies.

Andreas.

On Sat, Nov 19, 2011 at 2:27 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:

I use ccache-2.4. Jonathan pointed out 3.x has been fixed elsewhere in this thread . I'm going to ask Gentoo to stablize one of 3.x version.

Duy noticed that now that the COMPUTE_HEADER_DEPENDENCIES feature is turned on automatically for compilers that support it (see v1.7.8-rc0~142^2~1, 2011-08-18), there is no easy way to force it off. For example, setting COMPUTE_HEADER_DEPENDENCIES to the empty string in config.mak just tells the makefile to treat it as undefined and run a test command to see if the -MMD option is supported.

Introduce a new NO_COMPUTE_HEADER_DEPENDENCIES variable that forces the feature off. The new semantics:

 - If NO_COMPUTE_HEADER_DEPENDENCIES is set to a nonempty string,
   the -MMD option will not be used.  The build relies on hard-coded
   dependencies in the "ifndef USE_COMPUTED_HEADER_DEPENDENCIES"
   section of the Makefile instead.

 - If COMPUTE_HEADER_DEPENDENCIES is empty and NO_COMPUTE_... is
   nonempty, the build uses gcc's on-the-fly dependency generation
   feature.

 - If neither is nonempty, the makefile runs a quick test command
   to decide whether the compiler supports the -MMD option and
   whether to enable this feature.

Inspired-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Nguyen Thai Ngoc Duy wrote:

"make COMPUTE_HEADER_DEPENDENCIES=" works. But there's no way aside from "override COMPUTE_HEADER_DEPENDENCIES=" to do that in config.mak. And yuck.

By the way, I'm not convinced it's your compiler's fault. It might be my compiler's fault. More reading to do...

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

Jonathan Nieder wrote:

Erm.  This should say:

  "- If COMPUTE_HEADER_DEPENDENCIES is nonempty and NO_COMPUTE_... is
     empty,"

Sorry for the noise.

Jonathan Nieder <jrnieder@gmail.com> writes:

Eek. At least at the end user UI level, couldn't we do this as a tristate? E.g. "YesPlease" (or anything that begins with Y if you are ambitious) to explicitly enable, empty (or "auto") to autodetect, and anything else to decline?

Even better, couldn't we either (1) rearrange .dep/ files somehow, so that compiler difference does not matter, or (2) have dep_check to perform a trial run to detect versions of compilers that produce the output that we cannot use?

Junio C Hamano wrote:

Ah, I didn't mind the UI so much.  Handling

	COMPUTE_HEADER_DEPENDENCIES_FORCE = (yes | no | auto)

should be doable. I'd suggest making any other value error out, so typos don't result in mysterious behavior.

Yes, I'm working on an incantation all the compilers like (it shouldn't be hard --- adding an -MQ option should be enough, but I want to understand the bug first). But even with such a fix, I think it will be important to have a way to turn the feature off. When someone using a compiler without -MMD support reports a bug, wouldn't it be nice to be able to reproduce it?

Now that the COMPUTE_HEADER_DEPENDENCIES feature is turned on automatically for compilers that support it (see v1.7.8-rc0~142^2~1, 2011-08-18), there is no easy way to force it off. For example, setting COMPUTE_HEADER_DEPENDENCIES to the empty string in config.mak just tells the makefile to treat it as undefined and run a test command to see if the -MMD option is supported.

So allow setting COMPUTE_HEADER_DEPENDENCIES=no to explicitly force the feature off. The new semantics:

 - "yes" means to explicitly enable the feature
 - "no" means to disable it
 - "auto" means to autodetect

The default is still "auto". Any value other than these three will cause the build to error out with a descriptive message so typos and stale settings in config.mak don't result in mysterious behavior.

	Makefile:1278: *** please set COMPUTE_HEADER_DEPENDENCIES to
	yes, no, or auto (not "1").  Stop.

So now when someone using a compiler without -MMD support reports trouble building git, you can reproduce it by running "make COMPUTE_HEADER_DEPENDENCIES=no".

Suggested-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Improved-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Junio C Hamano wrote:

Nice idea.  Here it is.

 Makefile |   31 ++++++++++++++++++++++++-------
 1 files changed, 24 insertions(+), 7 deletions(-)

When the streaming filter API was introduced in v1.7.7-rc0~60^2~7 (2011-05-20), we forgot to add its header to LIB_H. Most translation units depend on streaming.h via cache.h.

v1.7.5-rc0~48 (Fix sparse warnings, 2011-03-22) introduced undeclared dependencies by url.o on url.h and thread-utils.o on thread-utils.h.

Noticed by make CHECK_HEADER_DEPENDENCIES=1.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Some makefile buglets found while testing.

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

On Fri, Nov 18, 2011 at 4:58 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:

Tested-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

"gcc -MF depfile -MMD -MP -c -o path/to/file.o" produces a makefile snippet named "depfile" describing what files are needed to build the target given by "-o". When ccache versions before v3.0pre0~187 (Fix handling of the -MD and -MDD options, 2009-11-01) run, they execute

	gcc -MF depfile -MMD -MP -E

instead to get the final content for hashing. Notice that the "-c -o" combination is replaced by "-E". The result is a target name without a leading path.

Thus when building git with such versions of ccache with COMPUTE_HEADER_DEPENDENCIES enabled, the generated makefile snippets define dependencies for the wrong target:

	$ make builtin/add.o
	GIT_VERSION = 1.7.8.rc3
	    * new build flags or prefix
	    CC builtin/add.o
	$ head -1 builtin/.depend/add.o.d
	add.o: builtin/add.c cache.h git-compat-util.h compat/bswap.h strbuf.h \

After a change in a header file, object files in a subdirectory are not automatically rebuilt by "make":

	$ touch cache.h
	$ make builtin/add.o
	$

Luckily we can prevent trouble by explicitly supplying the name of the target to ccache and gcc, using the -MQ option. Do so.

Reported-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Hi,

Nguyen Thai Ngoc Duy wrote:

The following seems to do the trick for me. Thanks again for catching it.

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

On Sat, Nov 19, 2011 at 6:23 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:

Works for me too.