Skip to contentSkip to conversation

autogenerated dependency files are not being utilized properly

PATCH Applied Brian Norris 2 participants 4 messages Aug 27, 2015 – Aug 28, 2015 Memory technology devices (MTD)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 autogenerated dependency files are not being utilized properly

TL;DR
=====

Auto-generated dependency rules are not being written correctly, so changes to dependent files (e.g., headers) do not actually trigger rebuilds.

The problem
===========

It appears that when a dependency generation flag is passed directly to the preprocessor (with '-Wp,...'), it loses information about the output path. So, it just makes up the output name as $(basename).o, with no path information. This yields .*.c.dep files that look like this:

  flash_lock.o: flash_lock.c /usr/include/stdc-predef.h flash_unlock.c \
   (...)

and

  nanddump.o: nanddump.c /usr/include/stdc-predef.h /usr/include/ctype.h \
   (...)
   include/libmtd.h

This is the case for both in-tree *and* out-of-tree builds. Naturally, this is a problem for out-of-tree builds. But it is also a problem for in-tree builds, because we use rules like this for builds:

  $(BUILDDIR)/%.o: %.c

and make doesn't recognize $(BUILDDIR)/%.o as the same as %.o even when $(BUILDDIR) == $(PWD).

Example failures
================

## Rebuilding after touching common header doesn't recompile anything

  $ make
  (...)
  $ touch include/libmtd.h
  $ make
    CHK     include/version.h

## Same for out-of-tree builds

  $ BUILDDIR=test make
  (...)
  $ touch include/libmtd.h
  $ BUILDDIR=test make
    CHK     include/version.h

I noticed this when seeing that flash_lock would not get rebuilt when modifying flash_unlock.c (where 99% of the source code lies):

  $ make
  (...)
  $ touch flash_unlock.c
  $ make
    CHK     include/version.h
    CC      flash_unlock.o
    LD      flash_unlock

The fix
=======

Just pass -MD straight to the compiler, and make sure to specify the output file for the dependency info with -MF.

Cc Mike Frysinger <[email protected]>
Cc David Woodhouse <[email protected]>
---
 common.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
common.mk +1 -1
index ba87377a00b6..7c09ab0a46d0 100644
@@ -80,7 +80,7 @@ ifneq ($(BUILDDIR),$(CURDIR))
 	$(Q)mkdir -p $(dir $@)
 endif
 	$(call BECHO,CC)
-	$(Q)$(CC) $(CPPFLAGS) $(CFLAGS) -c -o $@ $< -g -Wp,-MD,$(BUILDDIR)/.$(<F).dep
+	$(Q)$(CC) $(CPPFLAGS) $(CFLAGS) -c -o $@ $< -g -MD -MF $(BUILDDIR)/.$(<F).dep
 
 .SUFFIXES:
 
Signature · 1 line
2.5.0.457.gab17608
Acked-by on v1 1/1
#1 lore raw

common.mk ifneq ($(BUILDDIR),$(CURDIR)) · line 83

Quoted 2 lines · Brian Norris
common.mk +1 -1
-	$(Q)$(CC) $(CPPFLAGS) $(CFLAGS) -c -o $@ $< -g -Wp,-MD,$(BUILDDIR)/.$(<F).dep
+	$(Q)$(CC) $(CPPFLAGS) $(CFLAGS) -c -o $@ $< -g -MD -MF $(BUILDDIR)/.$(<F).dep

fwiw, this has been here since the beginning. dropping the -Wp makes sense to me. independently, i think it should use -MMD, but others might prefer the -MD behavior.

-mike

#2 lore raw
Quoted 5 lines · Mike Frysinger

common.mk ifneq ($(BUILDDIR),$(CURDIR)) · line 83

Quoted 2 lines · Brian Norris
common.mk +1 -1
-	$(Q)$(CC) $(CPPFLAGS) $(CFLAGS) -c -o $@ $< -g -Wp,-MD,$(BUILDDIR)/.$(<F).dep
+	$(Q)$(CC) $(CPPFLAGS) $(CFLAGS) -c -o $@ $< -g -MD -MF $(BUILDDIR)/.$(<F).dep

fwiw, this has been here since the beginning.

Yeah, at first I thought it was added in your build system refactoring a few years back, but then I traced it back to the beginning of time^Wgit history. Anyway, I figured you'd have insight.

Quoted 3 lines

dropping the -Wp makes
sense to me. independently, i think it should use -MMD, but others might prefer the -MD behavior.

-MMD makes sense to me too, but yes it is independent. I can send another patch.

Quoted 1 line

Thanks for the review!

Brian

author Applied
#3 lore raw
In reply to Brian Norris
Quoted 6 lines · Brian Norris

TL;DR
=====

Auto-generated dependency rules are not being written correctly, so changes to dependent files (e.g., headers) do not actually trigger rebuilds.

...

Applied to mtd-utils.git