* Re: [Qemu-devel] [PATCH] make: explicit dependencies for ACPI gen sources
2015-08-04 16:21 [Qemu-devel] [PATCH] make: explicit dependencies for ACPI gen sources Victor Kaplansky
@ 2015-08-04 20:34 ` Michael S. Tsirkin
2015-08-04 21:21 ` Peter Maydell
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Michael S. Tsirkin @ 2015-08-04 20:34 UTC (permalink / raw)
To: Victor Kaplansky
Cc: Paolo Bonzini, qemu-devel, Eduardo Habkost, Richard Henderson
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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH] make: explicit dependencies for ACPI gen sources
2015-08-04 16:21 [Qemu-devel] [PATCH] make: explicit dependencies for ACPI gen sources Victor Kaplansky
2015-08-04 20:34 ` Michael S. Tsirkin
@ 2015-08-04 21:21 ` Peter Maydell
2015-08-04 21:29 ` Michael S. Tsirkin
2015-08-05 9:50 ` Paolo Bonzini
3 siblings, 0 replies; 7+ messages in thread
From: Peter Maydell @ 2015-08-04 21:21 UTC (permalink / raw)
To: Victor Kaplansky
Cc: Paolo Bonzini, Richard Henderson, QEMU Developers,
Eduardo Habkost, Michael S. Tsirkin
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] make: explicit dependencies for ACPI gen sources
2015-08-04 16:21 [Qemu-devel] [PATCH] make: explicit dependencies for ACPI gen sources Victor Kaplansky
2015-08-04 20:34 ` Michael S. Tsirkin
2015-08-04 21:21 ` Peter Maydell
@ 2015-08-04 21:29 ` Michael S. Tsirkin
2015-08-05 9:50 ` Paolo Bonzini
3 siblings, 0 replies; 7+ messages in thread
From: Michael S. Tsirkin @ 2015-08-04 21:29 UTC (permalink / raw)
To: Victor Kaplansky
Cc: Paolo Bonzini, qemu-devel, Eduardo Habkost, Richard Henderson
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] make: explicit dependencies for ACPI gen sources
2015-08-04 16:21 [Qemu-devel] [PATCH] make: explicit dependencies for ACPI gen sources Victor Kaplansky
` (2 preceding siblings ...)
2015-08-04 21:29 ` Michael S. Tsirkin
@ 2015-08-05 9:50 ` Paolo Bonzini
2015-08-05 13:59 ` Michael S. Tsirkin
3 siblings, 1 reply; 7+ messages in thread
From: Paolo Bonzini @ 2015-08-05 9:50 UTC (permalink / raw)
To: Victor Kaplansky, qemu-devel
Cc: Michael S. Tsirkin, Eduardo Habkost, Richard Henderson
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)
> --
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH] make: explicit dependencies for ACPI gen sources
2015-08-05 9:50 ` Paolo Bonzini
@ 2015-08-05 13:59 ` Michael S. Tsirkin
2015-08-05 14:28 ` Victor Kaplansky
0 siblings, 1 reply; 7+ messages in thread
From: Michael S. Tsirkin @ 2015-08-05 13:59 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Victor Kaplansky, qemu-devel, Eduardo Habkost, Richard Henderson
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)
> > --
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] make: explicit dependencies for ACPI gen sources
2015-08-05 13:59 ` Michael S. Tsirkin
@ 2015-08-05 14:28 ` Victor Kaplansky
0 siblings, 0 replies; 7+ messages in thread
From: Victor Kaplansky @ 2015-08-05 14:28 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Paolo Bonzini, qemu-devel, Eduardo Habkost, Richard Henderson
> ----- 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
^ permalink raw reply [flat|nested] 7+ messages in thread