* [PATCH mtd-utils] autogenerated dependency files are not being utilized properly
@ 2015-08-27 22:48 Brian Norris
2015-08-27 23:56 ` Mike Frysinger
2015-08-28 16:04 ` Brian Norris
0 siblings, 2 replies; 4+ messages in thread
From: Brian Norris @ 2015-08-27 22:48 UTC (permalink / raw)
To: linux-mtd; +Cc: Brian Norris, Mike Frysinger, David Woodhouse
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.
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Cc: Mike Frysinger <vapier@gentoo.org>
Cc: David Woodhouse <David.Woodhouse@intel.com>
---
common.mk | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common.mk b/common.mk
index ba87377a00b6..7c09ab0a46d0 100644
--- a/common.mk
+++ b/common.mk
@@ -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:
--
2.5.0.457.gab17608
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH mtd-utils] autogenerated dependency files are not being utilized properly
2015-08-27 22:48 [PATCH mtd-utils] autogenerated dependency files are not being utilized properly Brian Norris
@ 2015-08-27 23:56 ` Mike Frysinger
2015-08-28 0:20 ` Brian Norris
2015-08-28 16:04 ` Brian Norris
1 sibling, 1 reply; 4+ messages in thread
From: Mike Frysinger @ 2015-08-27 23:56 UTC (permalink / raw)
To: Brian Norris; +Cc: linux-mtd, David Woodhouse
[-- Attachment #1: Type: text/plain, Size: 428 bytes --]
On 27 Aug 2015 15:48, Brian Norris wrote:
> - $(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.
Acked-by: Mike Frysinger <vapier@gentoo.org>
-mike
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH mtd-utils] autogenerated dependency files are not being utilized properly
2015-08-27 23:56 ` Mike Frysinger
@ 2015-08-28 0:20 ` Brian Norris
0 siblings, 0 replies; 4+ messages in thread
From: Brian Norris @ 2015-08-28 0:20 UTC (permalink / raw)
To: linux-mtd, David Woodhouse
On Thu, Aug 27, 2015 at 07:56:13PM -0400, Mike Frysinger wrote:
> On 27 Aug 2015 15:48, Brian Norris wrote:
> > - $(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.
> 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.
> Acked-by: Mike Frysinger <vapier@gentoo.org>
Thanks for the review!
Brian
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH mtd-utils] autogenerated dependency files are not being utilized properly
2015-08-27 22:48 [PATCH mtd-utils] autogenerated dependency files are not being utilized properly Brian Norris
2015-08-27 23:56 ` Mike Frysinger
@ 2015-08-28 16:04 ` Brian Norris
1 sibling, 0 replies; 4+ messages in thread
From: Brian Norris @ 2015-08-28 16:04 UTC (permalink / raw)
To: linux-mtd; +Cc: Mike Frysinger, David Woodhouse
On Thu, Aug 27, 2015 at 03:48:22PM -0700, Brian Norris wrote:
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-08-28 16:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-27 22:48 [PATCH mtd-utils] autogenerated dependency files are not being utilized properly Brian Norris
2015-08-27 23:56 ` Mike Frysinger
2015-08-28 0:20 ` Brian Norris
2015-08-28 16:04 ` Brian Norris
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).