[Qemu-devel] [PATCH] make: explicit dependencies for ACPI gen sources

[Qemu-devel] [PATCH] make: explicit dependencies for ACPI gen sources

[Victor Kaplansky]

Rebuilding QEMU after switching versions from git can cause make failure
if IASL is enabled. This is caused by including stale *.d info from
previous build.

To prevent this in the future we generate dependency info for generated
ACPI sources in *.dep instead regular *.d and include them explicitly.
This will allow us not to break future builds when one of such generated
sources is removed - by removing explicit include of removed dependency.

By itself this doesn't fix broken builds from the past and this will be
done by the following patch.

Signed-off-by: Victor Kaplansky <victork@redhat.com>
---
 hw/i386/Makefile.objs | 4 +++-
 rules.mak             | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index bd4f147..bcc7083 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -10,13 +10,15 @@ obj-y += acpi-build.o
 hw/i386/acpi-build.o: hw/i386/acpi-build.c \
 	hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex
 
+-include hw/i386/acpi-dsdt.dep hw/i386/q35-acpi-dsdt.dep
+
 iasl-option=$(shell if test -z "`$(1) $(2) 2>&1 > /dev/null`" \
     ; then echo "$(2)"; else echo "$(3)"; fi ;)
 
 ifdef IASL
 #IASL Present. Generate hex files from .dsl
 hw/i386/%.hex: $(SRC_PATH)/hw/i386/%.dsl $(SRC_PATH)/scripts/acpi_extract_preprocess.py $(SRC_PATH)/scripts/acpi_extract.py
-	$(call quiet-command, $(CPP) -x c -P $(QEMU_DGFLAGS) $(QEMU_INCLUDES) $< -o $*.dsl.i.orig, "  CPP $(TARGET_DIR)$*.dsl.i.orig")
+	$(call quiet-command, $(CPP) -x c -P $(QEMU_DGFLAGS_GEN) $(QEMU_INCLUDES) $< -o $*.dsl.i.orig, "  CPP $(TARGET_DIR)$*.dsl.i.orig")
 	$(call quiet-command, $(PYTHON) $(SRC_PATH)/scripts/acpi_extract_preprocess.py $*.dsl.i.orig > $*.dsl.i, "  ACPI_PREPROCESS $(TARGET_DIR)$*.dsl.i")
 	$(call quiet-command, $(IASL) $(call iasl-option,$(IASL),-Pn,) -vs -l -tc -p $* $*.dsl.i $(if $(V), , > /dev/null) 2>&1 ,"  IASL $(TARGET_DIR)$*.dsl.i")
 	$(call quiet-command, $(PYTHON) $(SRC_PATH)/scripts/acpi_extract.py $*.lst > $*.off, "  ACPI_EXTRACT $(TARGET_DIR)$*.off")
diff --git a/rules.mak b/rules.mak
index aec27f8..6019a46 100644
--- a/rules.mak
+++ b/rules.mak
@@ -18,6 +18,7 @@ QEMU_CXXFLAGS = -D__STDC_LIMIT_MACROS $(filter-out -Wstrict-prototypes -Wmissing
 
 # Flags for dependency generation
 QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(*D)/$(*F).d
+QEMU_DGFLAGS_GEN += -MMD -MP -MT $@ -MF $(*D)/$(*F).dep
 
 # Same as -I$(SRC_PATH) -I., but for the nested source/object directories
 QEMU_INCLUDES += -I$(<D) -I$(@D)
-- 
--Victor

Re: [Qemu-devel] [PATCH] make: explicit dependencies for ACPI gen sources

[Michael S. Tsirkin]

On Tue, Aug 04, 2015 at 07:21:25PM +0300, Victor Kaplansky wrote:
> Rebuilding QEMU after switching versions from git can cause make failure
> if IASL is enabled. This is caused by including stale *.d info from
> previous build.
> 
> To prevent this in the future we generate dependency info for generated
> ACPI sources in *.dep instead regular *.d and include them explicitly.
> This will allow us not to break future builds when one of such generated
> sources is removed - by removing explicit include of removed dependency.
> 
> By itself this doesn't fix broken builds from the past and this will be
> done by the following patch.
> 
> Signed-off-by: Victor Kaplansky <victork@redhat.com>

I was always wondering about these build failures.
Thanks for looking into this!


> ---
>  hw/i386/Makefile.objs | 4 +++-
>  rules.mak             | 1 +
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
> index bd4f147..bcc7083 100644
> --- a/hw/i386/Makefile.objs
> +++ b/hw/i386/Makefile.objs
> @@ -10,13 +10,15 @@ obj-y += acpi-build.o
>  hw/i386/acpi-build.o: hw/i386/acpi-build.c \
>  	hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex
>  
> +-include hw/i386/acpi-dsdt.dep hw/i386/q35-acpi-dsdt.dep
> +
>  iasl-option=$(shell if test -z "`$(1) $(2) 2>&1 > /dev/null`" \
>      ; then echo "$(2)"; else echo "$(3)"; fi ;)
>  
>  ifdef IASL
>  #IASL Present. Generate hex files from .dsl
>  hw/i386/%.hex: $(SRC_PATH)/hw/i386/%.dsl $(SRC_PATH)/scripts/acpi_extract_preprocess.py $(SRC_PATH)/scripts/acpi_extract.py
> -	$(call quiet-command, $(CPP) -x c -P $(QEMU_DGFLAGS) $(QEMU_INCLUDES) $< -o $*.dsl.i.orig, "  CPP $(TARGET_DIR)$*.dsl.i.orig")
> +	$(call quiet-command, $(CPP) -x c -P $(QEMU_DGFLAGS_GEN) $(QEMU_INCLUDES) $< -o $*.dsl.i.orig, "  CPP $(TARGET_DIR)$*.dsl.i.orig")
>  	$(call quiet-command, $(PYTHON) $(SRC_PATH)/scripts/acpi_extract_preprocess.py $*.dsl.i.orig > $*.dsl.i, "  ACPI_PREPROCESS $(TARGET_DIR)$*.dsl.i")
>  	$(call quiet-command, $(IASL) $(call iasl-option,$(IASL),-Pn,) -vs -l -tc -p $* $*.dsl.i $(if $(V), , > /dev/null) 2>&1 ,"  IASL $(TARGET_DIR)$*.dsl.i")
>  	$(call quiet-command, $(PYTHON) $(SRC_PATH)/scripts/acpi_extract.py $*.lst > $*.off, "  ACPI_EXTRACT $(TARGET_DIR)$*.off")
> diff --git a/rules.mak b/rules.mak
> index aec27f8..6019a46 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -18,6 +18,7 @@ QEMU_CXXFLAGS = -D__STDC_LIMIT_MACROS $(filter-out -Wstrict-prototypes -Wmissing
>  
>  # Flags for dependency generation
>  QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(*D)/$(*F).d
> +QEMU_DGFLAGS_GEN += -MMD -MP -MT $@ -MF $(*D)/$(*F).dep
>  
>  # Same as -I$(SRC_PATH) -I., but for the nested source/object directories
>  QEMU_INCLUDES += -I$(<D) -I$(@D)


We also need to make sure make clean blows these away, right?

> -- 
> --Victor

Re: [Qemu-devel] [PATCH] make: explicit dependencies for ACPI gen sources

[Peter Maydell]

On 4 August 2015 at 17:21, Victor Kaplansky <victork@redhat.com> wrote:
> Rebuilding QEMU after switching versions from git can cause make failure
> if IASL is enabled. This is caused by including stale *.d info from
> previous build

>  # Flags for dependency generation
>  QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(*D)/$(*F).d
> +QEMU_DGFLAGS_GEN += -MMD -MP -MT $@ -MF $(*D)/$(*F).dep

A comment about why we need both of these and when each is supposed
to be used would probably be helpful to future readers.

-- PMM

Re: [Qemu-devel] [PATCH] make: explicit dependencies for ACPI gen sources

[Michael S. Tsirkin]

On Tue, Aug 04, 2015 at 07:21:25PM +0300, Victor Kaplansky wrote:
> Rebuilding QEMU after switching versions from git can cause make failure
> if IASL is enabled. This is caused by including stale *.d info from
> previous build.
> 
> To prevent this in the future we generate dependency info for generated
> ACPI sources in *.dep instead regular *.d and include them explicitly.
> This will allow us not to break future builds when one of such generated
> sources is removed - by removing explicit include of removed dependency.
> 
> By itself this doesn't fix broken builds from the past and this will be
> done by the following patch.

I wonder how do you plan to do this.
One simple way would be to rename .hex files to something else.
Without an implicit rule for making .hex from .dsl, stale
dependencies on .hex would be harmless.

> 
> Signed-off-by: Victor Kaplansky <victork@redhat.com>
> ---
>  hw/i386/Makefile.objs | 4 +++-
>  rules.mak             | 1 +
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
> index bd4f147..bcc7083 100644
> --- a/hw/i386/Makefile.objs
> +++ b/hw/i386/Makefile.objs
> @@ -10,13 +10,15 @@ obj-y += acpi-build.o
>  hw/i386/acpi-build.o: hw/i386/acpi-build.c \
>  	hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex
>  
> +-include hw/i386/acpi-dsdt.dep hw/i386/q35-acpi-dsdt.dep
> +
>  iasl-option=$(shell if test -z "`$(1) $(2) 2>&1 > /dev/null`" \
>      ; then echo "$(2)"; else echo "$(3)"; fi ;)
>  
>  ifdef IASL
>  #IASL Present. Generate hex files from .dsl
>  hw/i386/%.hex: $(SRC_PATH)/hw/i386/%.dsl $(SRC_PATH)/scripts/acpi_extract_preprocess.py $(SRC_PATH)/scripts/acpi_extract.py
> -	$(call quiet-command, $(CPP) -x c -P $(QEMU_DGFLAGS) $(QEMU_INCLUDES) $< -o $*.dsl.i.orig, "  CPP $(TARGET_DIR)$*.dsl.i.orig")
> +	$(call quiet-command, $(CPP) -x c -P $(QEMU_DGFLAGS_GEN) $(QEMU_INCLUDES) $< -o $*.dsl.i.orig, "  CPP $(TARGET_DIR)$*.dsl.i.orig")
>  	$(call quiet-command, $(PYTHON) $(SRC_PATH)/scripts/acpi_extract_preprocess.py $*.dsl.i.orig > $*.dsl.i, "  ACPI_PREPROCESS $(TARGET_DIR)$*.dsl.i")
>  	$(call quiet-command, $(IASL) $(call iasl-option,$(IASL),-Pn,) -vs -l -tc -p $* $*.dsl.i $(if $(V), , > /dev/null) 2>&1 ,"  IASL $(TARGET_DIR)$*.dsl.i")
>  	$(call quiet-command, $(PYTHON) $(SRC_PATH)/scripts/acpi_extract.py $*.lst > $*.off, "  ACPI_EXTRACT $(TARGET_DIR)$*.off")
> diff --git a/rules.mak b/rules.mak
> index aec27f8..6019a46 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -18,6 +18,7 @@ QEMU_CXXFLAGS = -D__STDC_LIMIT_MACROS $(filter-out -Wstrict-prototypes -Wmissing
>  
>  # Flags for dependency generation
>  QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(*D)/$(*F).d
> +QEMU_DGFLAGS_GEN += -MMD -MP -MT $@ -MF $(*D)/$(*F).dep
>  
>  # Same as -I$(SRC_PATH) -I., but for the nested source/object directories
>  QEMU_INCLUDES += -I$(<D) -I$(@D)
> -- 
> --Victor

Re: [Qemu-devel] [PATCH] make: explicit dependencies for ACPI gen sources

[Paolo Bonzini]

On 04/08/2015 18:21, Victor Kaplansky wrote:
> Rebuilding QEMU after switching versions from git can cause make failure
> if IASL is enabled. This is caused by including stale *.d info from
> previous build.
> 
> To prevent this in the future we generate dependency info for generated
> ACPI sources in *.dep instead regular *.d and include them explicitly.
> This will allow us not to break future builds when one of such generated
> sources is removed - by removing explicit include of removed dependency.
> 
> By itself this doesn't fix broken builds from the past and this will be
> done by the following patch.

Does this happen the same way if you are removing a .c file?  If not,
can the same solution be used for .c and .dsl files?

Paolo

> Signed-off-by: Victor Kaplansky <victork@redhat.com>
> ---
>  hw/i386/Makefile.objs | 4 +++-
>  rules.mak             | 1 +
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
> index bd4f147..bcc7083 100644
> --- a/hw/i386/Makefile.objs
> +++ b/hw/i386/Makefile.objs
> @@ -10,13 +10,15 @@ obj-y += acpi-build.o
>  hw/i386/acpi-build.o: hw/i386/acpi-build.c \
>  	hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex
>  
> +-include hw/i386/acpi-dsdt.dep hw/i386/q35-acpi-dsdt.dep
> +
>  iasl-option=$(shell if test -z "`$(1) $(2) 2>&1 > /dev/null`" \
>      ; then echo "$(2)"; else echo "$(3)"; fi ;)
>  
>  ifdef IASL
>  #IASL Present. Generate hex files from .dsl
>  hw/i386/%.hex: $(SRC_PATH)/hw/i386/%.dsl $(SRC_PATH)/scripts/acpi_extract_preprocess.py $(SRC_PATH)/scripts/acpi_extract.py
> -	$(call quiet-command, $(CPP) -x c -P $(QEMU_DGFLAGS) $(QEMU_INCLUDES) $< -o $*.dsl.i.orig, "  CPP $(TARGET_DIR)$*.dsl.i.orig")
> +	$(call quiet-command, $(CPP) -x c -P $(QEMU_DGFLAGS_GEN) $(QEMU_INCLUDES) $< -o $*.dsl.i.orig, "  CPP $(TARGET_DIR)$*.dsl.i.orig")
>  	$(call quiet-command, $(PYTHON) $(SRC_PATH)/scripts/acpi_extract_preprocess.py $*.dsl.i.orig > $*.dsl.i, "  ACPI_PREPROCESS $(TARGET_DIR)$*.dsl.i")
>  	$(call quiet-command, $(IASL) $(call iasl-option,$(IASL),-Pn,) -vs -l -tc -p $* $*.dsl.i $(if $(V), , > /dev/null) 2>&1 ,"  IASL $(TARGET_DIR)$*.dsl.i")
>  	$(call quiet-command, $(PYTHON) $(SRC_PATH)/scripts/acpi_extract.py $*.lst > $*.off, "  ACPI_EXTRACT $(TARGET_DIR)$*.off")
> diff --git a/rules.mak b/rules.mak
> index aec27f8..6019a46 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -18,6 +18,7 @@ QEMU_CXXFLAGS = -D__STDC_LIMIT_MACROS $(filter-out -Wstrict-prototypes -Wmissing
>  
>  # Flags for dependency generation
>  QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(*D)/$(*F).d
> +QEMU_DGFLAGS_GEN += -MMD -MP -MT $@ -MF $(*D)/$(*F).dep
>  
>  # Same as -I$(SRC_PATH) -I., but for the nested source/object directories
>  QEMU_INCLUDES += -I$(<D) -I$(@D)
> -- 

Re: [Qemu-devel] [PATCH] make: explicit dependencies for ACPI gen sources

[Michael S. Tsirkin]

On Wed, Aug 05, 2015 at 11:50:54AM +0200, Paolo Bonzini wrote:
> 
> 
> On 04/08/2015 18:21, Victor Kaplansky wrote:
> > Rebuilding QEMU after switching versions from git can cause make failure
> > if IASL is enabled. This is caused by including stale *.d info from
> > previous build.
> > 
> > To prevent this in the future we generate dependency info for generated
> > ACPI sources in *.dep instead regular *.d and include them explicitly.
> > This will allow us not to break future builds when one of such generated
> > sources is removed - by removing explicit include of removed dependency.
> > 
> > By itself this doesn't fix broken builds from the past and this will be
> > done by the following patch.
> 
> Does this happen the same way if you are removing a .c file?  If not,
> can the same solution be used for .c and .dsl files?
> 
> Paolo

The problem is with .hex, not with .dsl files.
It will happen with .c or .h file as well if .c/.h file is generated.


> > Signed-off-by: Victor Kaplansky <victork@redhat.com>
> > ---
> >  hw/i386/Makefile.objs | 4 +++-
> >  rules.mak             | 1 +
> >  2 files changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
> > index bd4f147..bcc7083 100644
> > --- a/hw/i386/Makefile.objs
> > +++ b/hw/i386/Makefile.objs
> > @@ -10,13 +10,15 @@ obj-y += acpi-build.o
> >  hw/i386/acpi-build.o: hw/i386/acpi-build.c \
> >  	hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex
> >  
> > +-include hw/i386/acpi-dsdt.dep hw/i386/q35-acpi-dsdt.dep
> > +
> >  iasl-option=$(shell if test -z "`$(1) $(2) 2>&1 > /dev/null`" \
> >      ; then echo "$(2)"; else echo "$(3)"; fi ;)
> >  
> >  ifdef IASL
> >  #IASL Present. Generate hex files from .dsl
> >  hw/i386/%.hex: $(SRC_PATH)/hw/i386/%.dsl $(SRC_PATH)/scripts/acpi_extract_preprocess.py $(SRC_PATH)/scripts/acpi_extract.py
> > -	$(call quiet-command, $(CPP) -x c -P $(QEMU_DGFLAGS) $(QEMU_INCLUDES) $< -o $*.dsl.i.orig, "  CPP $(TARGET_DIR)$*.dsl.i.orig")
> > +	$(call quiet-command, $(CPP) -x c -P $(QEMU_DGFLAGS_GEN) $(QEMU_INCLUDES) $< -o $*.dsl.i.orig, "  CPP $(TARGET_DIR)$*.dsl.i.orig")
> >  	$(call quiet-command, $(PYTHON) $(SRC_PATH)/scripts/acpi_extract_preprocess.py $*.dsl.i.orig > $*.dsl.i, "  ACPI_PREPROCESS $(TARGET_DIR)$*.dsl.i")
> >  	$(call quiet-command, $(IASL) $(call iasl-option,$(IASL),-Pn,) -vs -l -tc -p $* $*.dsl.i $(if $(V), , > /dev/null) 2>&1 ,"  IASL $(TARGET_DIR)$*.dsl.i")
> >  	$(call quiet-command, $(PYTHON) $(SRC_PATH)/scripts/acpi_extract.py $*.lst > $*.off, "  ACPI_EXTRACT $(TARGET_DIR)$*.off")
> > diff --git a/rules.mak b/rules.mak
> > index aec27f8..6019a46 100644
> > --- a/rules.mak
> > +++ b/rules.mak
> > @@ -18,6 +18,7 @@ QEMU_CXXFLAGS = -D__STDC_LIMIT_MACROS $(filter-out -Wstrict-prototypes -Wmissing
> >  
> >  # Flags for dependency generation
> >  QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(*D)/$(*F).d
> > +QEMU_DGFLAGS_GEN += -MMD -MP -MT $@ -MF $(*D)/$(*F).dep
> >  
> >  # Same as -I$(SRC_PATH) -I., but for the nested source/object directories
> >  QEMU_INCLUDES += -I$(<D) -I$(@D)
> > -- 

Re: [Qemu-devel] [PATCH] make: explicit dependencies for ACPI gen sources

[Victor Kaplansky]

> ----- Original Message -----
> From: "Michael S. Tsirkin" <mst@redhat.com>
> To: "Paolo Bonzini" <pbonzini@redhat.com>
> Cc: "Victor Kaplansky" <victork@redhat.com>, qemu-devel@nongnu.org, "Richard Henderson" <rth@twiddle.net>, "Eduardo Habkost" <ehabkost@redhat.com>
> Sent: Wednesday, August 5, 2015 4:59:56 PM
> Subject: Re: [PATCH] make: explicit dependencies for ACPI gen sources
> 
> On Wed, Aug 05, 2015 at 11:50:54AM +0200, Paolo Bonzini wrote:
> > 
> > 
> > On 04/08/2015 18:21, Victor Kaplansky wrote:
> > > Rebuilding QEMU after switching versions from git can cause make failure
> > > if IASL is enabled. This is caused by including stale *.d info from
> > > previous build.
> > > 
> > > To prevent this in the future we generate dependency info for generated
> > > ACPI sources in *.dep instead regular *.d and include them explicitly.
> > > This will allow us not to break future builds when one of such generated
> > > sources is removed - by removing explicit include of removed dependency.
> > > 
> > > By itself this doesn't fix broken builds from the past and this will be
> > > done by the following patch.
> > 
> > Does this happen the same way if you are removing a .c file?  If not,
> > can the same solution be used for .c and .dsl files?
> > 
> > Paolo
> 
> The problem is with .hex, not with .dsl files.
> It will happen with .c or .h file as well if .c/.h file is generated.

Right, if a foo.c is generated and has its own includes like foo.h, our rules
will automatically create dependency file foo.d:

foo.c: foo.h

foo.h

After you switch to a later commit which has removed foo.c (and all its 
sources), foo.d will be still there in the build tree and as we load all *.d 
files, foo.d will be loaded as well and this will break the build. It is
because foo.c has prerequisites, so make has to recreate it.  

I'm preparing another patch which will clean up the things, and will solve
the issue both for future commits and between old commits and new commits.
Switching between old commits will remain broken though.

-- Victor