[Qemu-devel] [PATCH 0/4] qemu timestamp and related makefile cleanups

[Qemu-devel] [PATCH 0/4] qemu timestamp and related makefile cleanups

[Michael S. Tsirkin]

I recently had some build failures because of a stable
timestamp file. The following patchset addresses some of the
issue I found when going over the makefiles.


Michael S. Tsirkin (4):
  rules.mak: cleanup config generation rules
  Makefile: clean timestamp generation rule
  rules/mak: make clean should blow away timestamp files
  Makefile: fix make clean on libcacard

 libcacard/Makefile  |  2 +-
 rules.mak           | 14 +++++++++-----
 trace/Makefile.objs |  4 ++--
 3 files changed, 12 insertions(+), 8 deletions(-)

-- 
MST

[Qemu-devel] [PATCH 1/4] rules.mak: cleanup config generation rules

[Michael S. Tsirkin]

This addresses two issues with config generation
1. rule generating timestamp has side effect.
Thus cleanup on error does not work.
2. rule for handling timestamp is too generic.
It can create any missing .h file.
As a result when .h file is removed, build
might try to create it using this rule which
results in build errors.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 rules.mak | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/rules.mak b/rules.mak
index 6d82c0d..d11a5b4 100644
--- a/rules.mak
+++ b/rules.mak
@@ -82,12 +82,11 @@ TRACETOOL=$(PYTHON) $(SRC_PATH)/scripts/tracetool.py
 
 # Generate timestamp files for .h include files
 
-%.h: %.h-timestamp
-	@test -f $@ || cp $< $@
+config-%.h: config-%.h-timestamp
+	@cmp $< $@ >/dev/null 2>&1 || cp $< $@
 
-%.h-timestamp: %.mak
-	$(call quiet-command, sh $(SRC_PATH)/scripts/create_config < $< > $@, "  GEN   $(TARGET_DIR)$*.h")
-	@cmp $@ $*.h >/dev/null 2>&1 || cp $@ $*.h
+config-%.h-timestamp: config-%.mak
+	$(call quiet-command, sh $(SRC_PATH)/scripts/create_config < $< > $@, "  GEN   $(TARGET_DIR)config-$*.h")
 
 # will delete the target of a rule if commands exit with a nonzero exit status
 .DELETE_ON_ERROR:
-- 
MST

[Qemu-devel] [PATCH 4/4] Makefile: fix make clean on libcacard

[Michael S. Tsirkin]

libcacard lacks a clean target. Need to fix it,
meanwhile mark this target phony so it does not
stop parallel make clean.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 libcacard/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libcacard/Makefile b/libcacard/Makefile
index 47827a0..a3518c1 100644
--- a/libcacard/Makefile
+++ b/libcacard/Makefile
@@ -35,7 +35,7 @@ libcacard.pc: $(SRC_PATH)/libcacard/libcacard.pc.in
 		-e 's|@PREFIX@|$(prefix)|' $< > libcacard.pc,\
 	"  GEN   $@")
 
-.PHONY: install-libcacard
+.PHONY: install-libcacard clean
 
 install: install-libcacard
 install-libcacard: libcacard.pc libcacard.la
-- 
MST

Re: [Qemu-devel] [PATCH 4/4] Makefile: fix make clean on libcacard

[Stefan Hajnoczi]

On Tue, Jan 15, 2013 at 06:36:50PM +0200, Michael S. Tsirkin wrote:
> libcacard lacks a clean target. Need to fix it,
> meanwhile mark this target phony so it does not
> stop parallel make clean.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  libcacard/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libcacard/Makefile b/libcacard/Makefile
> index 47827a0..a3518c1 100644
> --- a/libcacard/Makefile
> +++ b/libcacard/Makefile
> @@ -35,7 +35,7 @@ libcacard.pc: $(SRC_PATH)/libcacard/libcacard.pc.in
>  		-e 's|@PREFIX@|$(prefix)|' $< > libcacard.pc,\
>  	"  GEN   $@")
>  
> -.PHONY: install-libcacard
> +.PHONY: install-libcacard clean

libcacard/Makefile is not supposed to have a clean target since
992aeb8eb53e5846a957cf333f2e1ec8cb6e0c04 ("libcacard: rewrite Makefile
in non-recursive style").

I have a patch on qemu-devel to remove libcacard from the make clean
loop since its objects are already handled by the global clean now that
the file is sourced rather than invoked recursively.

Stefan

Re: [Qemu-devel] [PATCH 4/4] Makefile: fix make clean on libcacard

[Michael S. Tsirkin]

On Thu, Jan 17, 2013 at 11:33:03AM +0100, Stefan Hajnoczi wrote:
> On Tue, Jan 15, 2013 at 06:36:50PM +0200, Michael S. Tsirkin wrote:
> > libcacard lacks a clean target. Need to fix it,
> > meanwhile mark this target phony so it does not
> > stop parallel make clean.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  libcacard/Makefile | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libcacard/Makefile b/libcacard/Makefile
> > index 47827a0..a3518c1 100644
> > --- a/libcacard/Makefile
> > +++ b/libcacard/Makefile
> > @@ -35,7 +35,7 @@ libcacard.pc: $(SRC_PATH)/libcacard/libcacard.pc.in
> >  		-e 's|@PREFIX@|$(prefix)|' $< > libcacard.pc,\
> >  	"  GEN   $@")
> >  
> > -.PHONY: install-libcacard
> > +.PHONY: install-libcacard clean
> 
> libcacard/Makefile is not supposed to have a clean target since
> 992aeb8eb53e5846a957cf333f2e1ec8cb6e0c04 ("libcacard: rewrite Makefile
> in non-recursive style").
> 
> I have a patch on qemu-devel to remove libcacard from the make clean
> loop since its objects are already handled by the global clean now that
> the file is sourced rather than invoked recursively.
> 
> Stefan

That will help too. Can you give me the msg id? Maybe I'll just
put it on my tree.

-- 
MST

Re: [Qemu-devel] [PATCH 4/4] Makefile: fix make clean on libcacard

[Stefan Hajnoczi]

On Thu, Jan 17, 2013 at 12:50:18PM +0200, Michael S. Tsirkin wrote:
> On Thu, Jan 17, 2013 at 11:33:03AM +0100, Stefan Hajnoczi wrote:
> > On Tue, Jan 15, 2013 at 06:36:50PM +0200, Michael S. Tsirkin wrote:
> > > libcacard lacks a clean target. Need to fix it,
> > > meanwhile mark this target phony so it does not
> > > stop parallel make clean.
> > > 
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > ---
> > >  libcacard/Makefile | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/libcacard/Makefile b/libcacard/Makefile
> > > index 47827a0..a3518c1 100644
> > > --- a/libcacard/Makefile
> > > +++ b/libcacard/Makefile
> > > @@ -35,7 +35,7 @@ libcacard.pc: $(SRC_PATH)/libcacard/libcacard.pc.in
> > >  		-e 's|@PREFIX@|$(prefix)|' $< > libcacard.pc,\
> > >  	"  GEN   $@")
> > >  
> > > -.PHONY: install-libcacard
> > > +.PHONY: install-libcacard clean
> > 
> > libcacard/Makefile is not supposed to have a clean target since
> > 992aeb8eb53e5846a957cf333f2e1ec8cb6e0c04 ("libcacard: rewrite Makefile
> > in non-recursive style").
> > 
> > I have a patch on qemu-devel to remove libcacard from the make clean
> > loop since its objects are already handled by the global clean now that
> > the file is sourced rather than invoked recursively.
> > 
> > Stefan
> 
> That will help too. Can you give me the msg id? Maybe I'll just
> put it on my tree.

<1358236046-17242-1-git-send-email-stefanha@redhat.com>

Stefan

[Qemu-devel] [PATCH 3/4] rules/mak: make clean should blow away timestamp files

[Michael S. Tsirkin]

Using a global pattern makes it easier to clean out
old generated files.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 rules.mak | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/rules.mak b/rules.mak
index d11a5b4..edc2552 100644
--- a/rules.mak
+++ b/rules.mak
@@ -88,6 +88,11 @@ config-%.h: config-%.h-timestamp
 config-%.h-timestamp: config-%.mak
 	$(call quiet-command, sh $(SRC_PATH)/scripts/create_config < $< > $@, "  GEN   $(TARGET_DIR)config-$*.h")
 
+.PHONY: clean-timestamp
+clean-timestamp:
+	rm -f *.timestamp
+clean: clean-timestamp
+
 # will delete the target of a rule if commands exit with a nonzero exit status
 .DELETE_ON_ERROR:
 
-- 
MST

Re: [Qemu-devel] [PATCH 3/4] rules/mak: make clean should blow away timestamp files

[Stefan Hajnoczi]

On Tue, Jan 15, 2013 at 06:57:34PM +0200, Michael S. Tsirkin wrote:
> Using a global pattern makes it easier to clean out
> old generated files.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  rules.mak | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/rules.mak b/rules.mak
> index d11a5b4..edc2552 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -88,6 +88,11 @@ config-%.h: config-%.h-timestamp
>  config-%.h-timestamp: config-%.mak
>  	$(call quiet-command, sh $(SRC_PATH)/scripts/create_config < $< > $@, "  GEN   $(TARGET_DIR)config-$*.h")
>  
> +.PHONY: clean-timestamp
> +clean-timestamp:
> +	rm -f *.timestamp

Timestamp files also live in subdirectories and I think we're slowly
moving away from recursive make, so this may not clean all timestamp
files.

Stefan

Re: [Qemu-devel] [PATCH 3/4] rules/mak: make clean should blow away timestamp files

[Michael S. Tsirkin]

On Thu, Jan 17, 2013 at 11:28:46AM +0100, Stefan Hajnoczi wrote:
> On Tue, Jan 15, 2013 at 06:57:34PM +0200, Michael S. Tsirkin wrote:
> > Using a global pattern makes it easier to clean out
> > old generated files.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  rules.mak | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/rules.mak b/rules.mak
> > index d11a5b4..edc2552 100644
> > --- a/rules.mak
> > +++ b/rules.mak
> > @@ -88,6 +88,11 @@ config-%.h: config-%.h-timestamp
> >  config-%.h-timestamp: config-%.mak
> >  	$(call quiet-command, sh $(SRC_PATH)/scripts/create_config < $< > $@, "  GEN   $(TARGET_DIR)config-$*.h")
> >  
> > +.PHONY: clean-timestamp
> > +clean-timestamp:
> > +	rm -f *.timestamp
> 
> Timestamp files also live in subdirectories and I think we're slowly
> moving away from recursive make, so this may not clean all timestamp
> files.
> 
> Stefan

Yes but it helps whenever we include rules.mak which is in most places.

-- 
MST

Re: [Qemu-devel] [PATCH 3/4] rules/mak: make clean should blow away timestamp files

[Stefan Hajnoczi]

On Thu, Jan 17, 2013 at 12:48:51PM +0200, Michael S. Tsirkin wrote:
> On Thu, Jan 17, 2013 at 11:28:46AM +0100, Stefan Hajnoczi wrote:
> > On Tue, Jan 15, 2013 at 06:57:34PM +0200, Michael S. Tsirkin wrote:
> > > Using a global pattern makes it easier to clean out
> > > old generated files.
> > > 
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > ---
> > >  rules.mak | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/rules.mak b/rules.mak
> > > index d11a5b4..edc2552 100644
> > > --- a/rules.mak
> > > +++ b/rules.mak
> > > @@ -88,6 +88,11 @@ config-%.h: config-%.h-timestamp
> > >  config-%.h-timestamp: config-%.mak
> > >  	$(call quiet-command, sh $(SRC_PATH)/scripts/create_config < $< > $@, "  GEN   $(TARGET_DIR)config-$*.h")
> > >  
> > > +.PHONY: clean-timestamp
> > > +clean-timestamp:
> > > +	rm -f *.timestamp
> > 
> > Timestamp files also live in subdirectories and I think we're slowly
> > moving away from recursive make, so this may not clean all timestamp
> > files.
> > 
> > Stefan
> 
> Yes but it helps whenever we include rules.mak which is in most places.

Okay, something to keep an eye on when Makefiles are converted to
non-recursive.

Stefan

[Qemu-devel] [PATCH 2/4] Makefile: clean timestamp generation rule

[Michael S. Tsirkin]

create timestamp by rule without sideeffects.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 trace/Makefile.objs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/trace/Makefile.objs b/trace/Makefile.objs
index 27fe26b..dde9d57 100644
--- a/trace/Makefile.objs
+++ b/trace/Makefile.objs
@@ -4,24 +4,24 @@
 # Auto-generated header for tracing routines
 
 $(obj)/generated-tracers.h: $(obj)/generated-tracers.h-timestamp
+	@cmp -s $< $@ || cp $< $@
 $(obj)/generated-tracers.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
 	$(call quiet-command,$(TRACETOOL) \
 		--format=h \
 		--backend=$(TRACE_BACKEND) \
 		< $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
-	@cmp -s $@ $(patsubst %-timestamp,%,$@) || cp $@ $(patsubst %-timestamp,%,$@)
 
 ######################################################################
 # Auto-generated tracing routines (non-DTrace)
 
 ifneq ($(TRACE_BACKEND),dtrace)
 $(obj)/generated-tracers.c: $(obj)/generated-tracers.c-timestamp
+	@cmp -s $< $@ || cp $< $@
 $(obj)/generated-tracers.c-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
 	$(call quiet-command,$(TRACETOOL) \
 		--format=c \
 		--backend=$(TRACE_BACKEND) \
 		< $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
-	@cmp -s $@ $(patsubst %-timestamp,%,$@) || cp $@ $(patsubst %-timestamp,%,$@)
 
 $(obj)/generated-tracers.o: $(obj)/generated-tracers.c $(obj)/generated-tracers.h
 endif
-- 
MST

Re: [Qemu-devel] [PATCH 2/4] Makefile: clean timestamp generation rule

[Stefan Hajnoczi]

On Tue, Jan 15, 2013 at 06:57:37PM +0200, Michael S. Tsirkin wrote:
> create timestamp by rule without sideeffects.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  trace/Makefile.objs | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>