| Message ID | AANLkTik--YoLr8OqDeYEcaxjmbsC6PM2aNbcXSKeHZE1@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Sun, Sep 26, 2010 at 10:05 AM, Blue Swirl <blauwirbel@gmail.com> wrote: > Add logic to detect changes in generated files. If the old > and new files are identical, don't touch the generated file. > This avoids a lot of churn since many files depend on trace.h. > > Signed-off-by: Blue Swirl <blauwirbel@gmail.com> > --- > Makefile | 18 ++++++++++++++++-- > 1 files changed, 16 insertions(+), 2 deletions(-) Looks good. Stefan
Blue Swirl <blauwirbel@gmail.com> writes: > Add logic to detect changes in generated files. If the old > and new files are identical, don't touch the generated file. > This avoids a lot of churn since many files depend on trace.h. > > Signed-off-by: Blue Swirl <blauwirbel@gmail.com> > --- > Makefile | 18 ++++++++++++++++-- > 1 files changed, 16 insertions(+), 2 deletions(-) > > diff --git a/Makefile b/Makefile > index ff39025..085e8ed 100644 > --- a/Makefile > +++ b/Makefile > @@ -107,10 +107,24 @@ ui/vnc.o: QEMU_CFLAGS += $(VNC_TLS_CFLAGS) > bt-host.o: QEMU_CFLAGS += $(BLUEZ_CFLAGS) > > trace.h: $(SRC_PATH)/trace-events config-host.mak > - $(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -h > < $< > $@," GEN $@") > + $(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -h > < $< > $@.tmp," GEN $@") > + @if test -f $@; then \ > + if ! cmp -s $@ $@.tmp; then \ > + mv $@.tmp $@; \ > + fi; \ > + else \ > + mv $@.tmp $@; \ > + fi Why the conditional? cmp -s fails fine when argument files don't exist. Note that common versions of make including GNU Make do not stat a rule's target to check whether the rule changed it. Instead, they assume it changed, and remake everything depending on it. armbru@blackfin:~/tmp$ cat Makefile foo: bar echo "Remaking foo" bar: [ -f $@ ] || touch $@ && echo "Touched bar" armbru@blackfin:~/tmp$ rm -f foo armbru@blackfin:~/tmp$ make [ -f bar ] || touch bar && echo "Touched bar" Touched bar echo "Remaking foo" Remaking foo armbru@blackfin:~/tmp$ make echo "Remaking foo" Remaking foo I doubt your patch avoids churn as advertized with such makes. [...]
On 09/27/2010 10:32 AM, Markus Armbruster wrote: > Why the conditional? cmp -s fails fine when argument files don't exist. > > Note that common versions of make including GNU Make do not stat a > rule's target to check whether the rule changed it. Instead, they > assume it changed, and remake everything depending on it. > > armbru@blackfin:~/tmp$ cat Makefile > foo: bar > echo "Remaking foo" > > bar: > [ -f $@ ] || touch $@&& echo "Touched bar" > armbru@blackfin:~/tmp$ rm -f foo > armbru@blackfin:~/tmp$ make > [ -f bar ] || touch bar&& echo "Touched bar" > Touched bar > echo "Remaking foo" > Remaking foo > armbru@blackfin:~/tmp$ make > echo "Remaking foo" > Remaking foo > > I doubt your patch avoids churn as advertized with such makes. Indeed, see how it's done for config-*.h. # Uses generic rule in rules.mak trace.h: trace.h-timestamp trace.h-timestamp: $(SRC_PATH)/trace-events config-host.mak $(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -h < $< > $@," GEN trace.h) @cmp $@ trace.h >/dev/null 2>&1 || cp $@ trace.h (untested). Paolo
On Mon, Sep 27, 2010 at 9:51 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > On 09/27/2010 10:32 AM, Markus Armbruster wrote: >> Why the conditional? cmp -s fails fine when argument files don't exist. >> >> Note that common versions of make including GNU Make do not stat a >> rule's target to check whether the rule changed it. Instead, they >> assume it changed, and remake everything depending on it. >> >> armbru@blackfin:~/tmp$ cat Makefile >> foo: bar >> echo "Remaking foo" >> >> bar: >> [ -f $@ ] || touch $@&& echo "Touched bar" >> armbru@blackfin:~/tmp$ rm -f foo >> armbru@blackfin:~/tmp$ make >> [ -f bar ] || touch bar&& echo "Touched bar" >> Touched bar >> echo "Remaking foo" >> Remaking foo >> armbru@blackfin:~/tmp$ make >> echo "Remaking foo" >> Remaking foo >> >> I doubt your patch avoids churn as advertized with such makes. > > Indeed, see how it's done for config-*.h. > > # Uses generic rule in rules.mak > trace.h: trace.h-timestamp > trace.h-timestamp: $(SRC_PATH)/trace-events config-host.mak > $(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -h < $< > $@," GEN trace.h) > @cmp $@ trace.h >/dev/null 2>&1 || cp $@ trace.h > > (untested). I just copied the rule from %/config-devices.mak. Is also that rule then incorrect? Perhaps there could be a macro for this, not unlike move-if-change script in various GNU packages?
On 09/27/2010 06:13 PM, Blue Swirl wrote: >> Indeed, see how it's done for config-*.h. >> >> # Uses generic rule in rules.mak >> trace.h: trace.h-timestamp >> trace.h-timestamp: $(SRC_PATH)/trace-events config-host.mak >> $(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -h< $< > $@," GEN trace.h) >> @cmp $@ trace.h>/dev/null 2>&1 || cp $@ trace.h >> >> (untested). > > I just copied the rule from %/config-devices.mak. Is also that rule > then incorrect? config-devices.mak will cause the Makefile to be reread, but no recompilations, so it's fine not to use a timestamp file. Headers are more complicated. > Perhaps there could be a macro for this, not unlike move-if-change > script in various GNU packages? The problem is not really the move-if-change script, but rather the timestamping rules. You can use $(eval) for that but it quickly becomes unmanageable. Paolo
Markus Armbruster <armbru@redhat.com> writes: > Blue Swirl <blauwirbel@gmail.com> writes: > >> Add logic to detect changes in generated files. If the old >> and new files are identical, don't touch the generated file. >> This avoids a lot of churn since many files depend on trace.h. >> >> Signed-off-by: Blue Swirl <blauwirbel@gmail.com> >> --- >> Makefile | 18 ++++++++++++++++-- >> 1 files changed, 16 insertions(+), 2 deletions(-) >> >> diff --git a/Makefile b/Makefile >> index ff39025..085e8ed 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -107,10 +107,24 @@ ui/vnc.o: QEMU_CFLAGS += $(VNC_TLS_CFLAGS) >> bt-host.o: QEMU_CFLAGS += $(BLUEZ_CFLAGS) >> >> trace.h: $(SRC_PATH)/trace-events config-host.mak >> - $(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -h >> < $< > $@," GEN $@") >> + $(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -h >> < $< > $@.tmp," GEN $@") >> + @if test -f $@; then \ >> + if ! cmp -s $@ $@.tmp; then \ >> + mv $@.tmp $@; \ >> + fi; \ >> + else \ >> + mv $@.tmp $@; \ >> + fi > > Why the conditional? cmp -s fails fine when argument files don't exist. > > Note that common versions of make including GNU Make do not stat a > rule's target to check whether the rule changed it. Instead, they > assume it changed, and remake everything depending on it. That was from memory. And my example makefile to illustrate the problem was flawed. So your patch might work reliably after all. The common make voodoo incantation for this is a stamp file, though, as Paolo pointed out. [...]
On 09/28/2010 10:58 AM, Markus Armbruster wrote: > That was from memory. And my example makefile to illustrate the problem > was flawed. So your patch might work reliably after all. The common > make voodoo incantation for this is a stamp file, though, as Paolo > pointed out. FWIW, Autoconf manual says that the stamp file is needed for low-resolution (1s or, for FAT, 2s) filesystems or something like that. Paolo
diff --git a/Makefile b/Makefile index ff39025..085e8ed 100644 --- a/Makefile +++ b/Makefile @@ -107,10 +107,24 @@ ui/vnc.o: QEMU_CFLAGS += $(VNC_TLS_CFLAGS) bt-host.o: QEMU_CFLAGS += $(BLUEZ_CFLAGS) trace.h: $(SRC_PATH)/trace-events config-host.mak - $(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -h < $< > $@," GEN $@") + $(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -h < $< > $@.tmp," GEN $@") + @if test -f $@; then \ + if ! cmp -s $@ $@.tmp; then \ + mv $@.tmp $@; \ + fi; \ + else \ + mv $@.tmp $@; \ + fi trace.c: $(SRC_PATH)/trace-events config-host.mak - $(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -c < $< > $@," GEN $@") + $(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -c < $< > $@.tmp," GEN $@") + @if test -f $@; then \ + if ! cmp -s $@ $@.tmp; then \ + mv $@.tmp $@; \ + fi; \ + else \ + mv $@.tmp $@; \
Add logic to detect changes in generated files. If the old and new files are identical, don't touch the generated file. This avoids a lot of churn since many files depend on trace.h. Signed-off-by: Blue Swirl <blauwirbel@gmail.com> --- Makefile | 18 ++++++++++++++++-- 1 files changed, 16 insertions(+), 2 deletions(-) + fi trace.o: trace.c $(GENERATED_HEADERS)