| Message ID | 1403974776-23966-1-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Rejected |
| Headers | show |
Il 28/06/2014 18:59, Peter Maydell ha scritto: > Having a direct dependency > Makefile: $(GENERATED_HEADERS) > can result in not-from-clean builds failing sometimes, because it means > that when Make does its "is any makefile or include out of date and > needing a rebuild?" check, as well as possibly running configure (to > update config-host.mak) it will also rebuild GENERATED_HEADERS under > the "old" config-host.mak regime. If the makefile rules for rebuilding > the generated headers have changed in a way that means they're not > compatible with the old config-host.mak variable names, the build will > fail. Even if it does work, it's wasted effort since we'll need to > rebuild them with the right config-host.mak settings immediately. > > Instead, make the generated headers depend on config-host.mak > and config-target.mak. This means we'll definitely rebuild them > if the configuration changes, but it will be done after Make > reloads its makefiles and includes so will happen with the > correct set of config-host.mak settings. > > We rely on the various .o files having correct autogenerated > dependency rules to cause the generated headers to be generated > as and when they are needed. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > The specific example of this is the change from CONFIG_TRACE_BACKEND > to CONFIG_TRACE_BACKENDS, which I am still hitting on a pretty > regular basis. This patch fixes that problem and I don't think it > has any unpleasant side effects; Paolo, have I missed anything? > > Makefile | 8 +++----- > Makefile.target | 2 +- > 2 files changed, 4 insertions(+), 6 deletions(-) > > diff --git a/Makefile b/Makefile > index 145adb6..dca33b4 100644 > --- a/Makefile > +++ b/Makefile > @@ -546,11 +546,9 @@ ifdef SIGNCODE > endif # SIGNCODE > endif # CONFIG_WIN > > -# Add a dependency on the generated files, so that they are always > -# rebuilt before other object files > -ifneq ($(filter-out %clean,$(MAKECMDGOALS)),$(if $(MAKECMDGOALS),,fail)) > -Makefile: $(GENERATED_HEADERS) > -endif > +# Make the generated files depend on config-host.mak so that if > +# the configuration settings change we will rebuild them > +$(GENERATED_HEADERS): config-host.mak > > # Include automatically generated dependency files > # Dependencies in Makefile.objs files come from our recursive subdir rules > diff --git a/Makefile.target b/Makefile.target > index 6089d29..ea8614a 100644 > --- a/Makefile.target > +++ b/Makefile.target > @@ -198,4 +198,4 @@ ifdef CONFIG_TRACE_SYSTEMTAP > endif > > GENERATED_HEADERS += config-target.h > -Makefile: $(GENERATED_HEADERS) > +$(GENERATED_HEADERS): config-target.mak config-devices.mak config-devices.mak is not reflected in any C header file. Apart from this, Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Paolo
On 30 June 2014 13:09, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 28/06/2014 18:59, Peter Maydell ha scritto: > >> Having a direct dependency >> Makefile: $(GENERATED_HEADERS) >> can result in not-from-clean builds failing sometimes, because it means >> that when Make does its "is any makefile or include out of date and >> needing a rebuild?" check, as well as possibly running configure (to >> update config-host.mak) it will also rebuild GENERATED_HEADERS under >> the "old" config-host.mak regime. If the makefile rules for rebuilding >> the generated headers have changed in a way that means they're not >> compatible with the old config-host.mak variable names, the build will >> fail. Even if it does work, it's wasted effort since we'll need to >> rebuild them with the right config-host.mak settings immediately. >> >> Instead, make the generated headers depend on config-host.mak >> and config-target.mak. This means we'll definitely rebuild them >> if the configuration changes, but it will be done after Make >> reloads its makefiles and includes so will happen with the >> correct set of config-host.mak settings. >> >> We rely on the various .o files having correct autogenerated >> dependency rules to cause the generated headers to be generated >> as and when they are needed. >> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >> --- >> The specific example of this is the change from CONFIG_TRACE_BACKEND >> to CONFIG_TRACE_BACKENDS, which I am still hitting on a pretty >> regular basis. This patch fixes that problem and I don't think it >> has any unpleasant side effects; Paolo, have I missed anything? >> >> Makefile | 8 +++----- >> Makefile.target | 2 +- >> 2 files changed, 4 insertions(+), 6 deletions(-) >> >> diff --git a/Makefile b/Makefile >> index 145adb6..dca33b4 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -546,11 +546,9 @@ ifdef SIGNCODE >> endif # SIGNCODE >> endif # CONFIG_WIN >> >> -# Add a dependency on the generated files, so that they are always >> -# rebuilt before other object files >> -ifneq ($(filter-out %clean,$(MAKECMDGOALS)),$(if $(MAKECMDGOALS),,fail)) >> -Makefile: $(GENERATED_HEADERS) >> -endif >> +# Make the generated files depend on config-host.mak so that if >> +# the configuration settings change we will rebuild them >> +$(GENERATED_HEADERS): config-host.mak >> >> # Include automatically generated dependency files >> # Dependencies in Makefile.objs files come from our recursive subdir >> rules >> diff --git a/Makefile.target b/Makefile.target >> index 6089d29..ea8614a 100644 >> --- a/Makefile.target >> +++ b/Makefile.target >> @@ -198,4 +198,4 @@ ifdef CONFIG_TRACE_SYSTEMTAP >> endif >> >> GENERATED_HEADERS += config-target.h >> -Makefile: $(GENERATED_HEADERS) >> +$(GENERATED_HEADERS): config-target.mak config-devices.mak > > > config-devices.mak is not reflected in any C header file. Apart from this, > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Do you mean "...and therefore should not be listed on the RHS of this dependency" ? thanks -- PMM
Il 30/06/2014 14:13, Peter Maydell ha scritto: >> > config-devices.mak is not reflected in any C header file. Apart from this, >> > >> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > Do you mean "...and therefore should not be listed on the > RHS of this dependency" ? It need not be listed; it doesn't hurt to leave it in, so whatever the submitter wants to do is fine. :) Paolo
On 30 June 2014 13:34, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 30/06/2014 14:13, Peter Maydell ha scritto: > >>> > config-devices.mak is not reflected in any C header file. Apart from >>> > this, >>> > >>> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> >> >> Do you mean "...and therefore should not be listed on the >> RHS of this dependency" ? > > > It need not be listed; it doesn't hurt to leave it in, so whatever the > submitter wants to do is fine. :) In that case I will leave it be rather than re-rolling :-) (Maybe one day we'll have a header that does reflect it...) thanks -- PMM
Il 30/06/2014 14:42, Peter Maydell ha scritto: > In that case I will leave it be rather than re-rolling :-) > (Maybe one day we'll have a header that does reflect it...) So far we never had the need and Blue Swirl objected to it (in favor of run-time probing of QOM classes), but you never know. Paolo
On 30 June 2014 13:09, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 28/06/2014 18:59, Peter Maydell ha scritto: > >> Having a direct dependency >> Makefile: $(GENERATED_HEADERS) >> can result in not-from-clean builds failing sometimes, because it means >> that when Make does its "is any makefile or include out of date and >> needing a rebuild?" check, as well as possibly running configure (to >> update config-host.mak) it will also rebuild GENERATED_HEADERS under >> the "old" config-host.mak regime. If the makefile rules for rebuilding >> the generated headers have changed in a way that means they're not >> compatible with the old config-host.mak variable names, the build will >> fail. Even if it does work, it's wasted effort since we'll need to >> rebuild them with the right config-host.mak settings immediately. >> >> Instead, make the generated headers depend on config-host.mak >> and config-target.mak. This means we'll definitely rebuild them >> if the configuration changes, but it will be done after Make >> reloads its makefiles and includes so will happen with the >> correct set of config-host.mak settings. >> >> We rely on the various .o files having correct autogenerated >> dependency rules to cause the generated headers to be generated >> as and when they are needed. > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Any objections to this going into 2.1 for rc1 ? -- PMM
Il 04/07/2014 16:33, Peter Maydell ha scritto: > On 30 June 2014 13:09, Paolo Bonzini <pbonzini@redhat.com> wrote: >> Il 28/06/2014 18:59, Peter Maydell ha scritto: >> >>> Having a direct dependency >>> Makefile: $(GENERATED_HEADERS) >>> can result in not-from-clean builds failing sometimes, because it means >>> that when Make does its "is any makefile or include out of date and >>> needing a rebuild?" check, as well as possibly running configure (to >>> update config-host.mak) it will also rebuild GENERATED_HEADERS under >>> the "old" config-host.mak regime. If the makefile rules for rebuilding >>> the generated headers have changed in a way that means they're not >>> compatible with the old config-host.mak variable names, the build will >>> fail. Even if it does work, it's wasted effort since we'll need to >>> rebuild them with the right config-host.mak settings immediately. >>> >>> Instead, make the generated headers depend on config-host.mak >>> and config-target.mak. This means we'll definitely rebuild them >>> if the configuration changes, but it will be done after Make >>> reloads its makefiles and includes so will happen with the >>> correct set of config-host.mak settings. >>> >>> We rely on the various .o files having correct autogenerated >>> dependency rules to cause the generated headers to be generated >>> as and when they are needed. > >> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > > > Any objections to this going into 2.1 for rc1 ? Not from me. Paolo
On 4 July 2014 15:33, Peter Maydell <peter.maydell@linaro.org> wrote: > On 30 June 2014 13:09, Paolo Bonzini <pbonzini@redhat.com> wrote: >> Il 28/06/2014 18:59, Peter Maydell ha scritto: >> >>> Having a direct dependency >>> Makefile: $(GENERATED_HEADERS) >>> can result in not-from-clean builds failing sometimes, because it means >>> that when Make does its "is any makefile or include out of date and >>> needing a rebuild?" check, as well as possibly running configure (to >>> update config-host.mak) it will also rebuild GENERATED_HEADERS under >>> the "old" config-host.mak regime. If the makefile rules for rebuilding >>> the generated headers have changed in a way that means they're not >>> compatible with the old config-host.mak variable names, the build will >>> fail. Even if it does work, it's wasted effort since we'll need to >>> rebuild them with the right config-host.mak settings immediately. >>> >>> Instead, make the generated headers depend on config-host.mak >>> and config-target.mak. This means we'll definitely rebuild them >>> if the configuration changes, but it will be done after Make >>> reloads its makefiles and includes so will happen with the >>> correct set of config-host.mak settings. >>> >>> We rely on the various .o files having correct autogenerated >>> dependency rules to cause the generated headers to be generated >>> as and when they are needed. > >> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > Any objections to this going into 2.1 for rc1 ? Unfortunately it doesn't seem to work correctly for a build from clean: e104462:trusty:qemu-for-merges$ make -C build/alldbg V=1 make: Entering directory `/home/petmay01/linaro/qemu-for-merges/build/alldbg' libtool --mode=compile --tag=CC ccache gcc -I/home/petmay01/linaro/qemu-for-merges/tcg -I/home/petmay01/linaro/qemu-for-merges/tcg/i386 -I/home/petmay01/linaro/qemu-for-merges/linux-headers -I/home/petmay01/linaro/qemu-for-merges/build/alldbg/linux-headers -I. -I/home/petmay01/linaro/qemu-for-merges -I/home/petmay01/linaro/qemu-for-merges/include -I/home/petmay01/linaro/qemu-for-merges/libcacard -Ilibcacard -Werror -fPIE -DPIE -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -Wendif-labels -Wmissing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition -Wtype-limits -fstack-protector-all -I/usr/include/p11-kit-1 -I/usr/include/p11-kit-1 -I/usr/include/libpng12 -I/usr/include/spice-server -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/spice-1 -I/usr/include/libusb-1.0 -I/usr/include/pixman-1 -I/home/petmay01/linaro/qemu-for-merges/tests -MMD -MP -MT libcacard/cac.lo -MF libcacard/cac.d -pthread -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -g -c -o libcacard/cac.lo /home/petmay01/linaro/qemu-for-merges/libcacard/cac.c libtool: compile: ccache gcc -I/home/petmay01/linaro/qemu-for-merges/tcg -I/home/petmay01/linaro/qemu-for-merges/tcg/i386 -I/home/petmay01/linaro/qemu-for-merges/linux-headers -I/home/petmay01/linaro/qemu-for-merges/build/alldbg/linux-headers -I. -I/home/petmay01/linaro/qemu-for-merges -I/home/petmay01/linaro/qemu-for-merges/include -I/home/petmay01/linaro/qemu-for-merges/libcacard -Ilibcacard -Werror -DPIE -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -Wendif-labels -Wmissing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition -Wtype-limits -fstack-protector-all -I/usr/include/p11-kit-1 -I/usr/include/p11-kit-1 -I/usr/include/libpng12 -I/usr/include/spice-server -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/spice-1 -I/usr/include/libusb-1.0 -I/usr/include/pixman-1 -I/home/petmay01/linaro/qemu-for-merges/tests -MMD -MP -MT libcacard/cac.lo -MF libcacard/cac.d -pthread -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -g -c /home/petmay01/linaro/qemu-for-merges/libcacard/cac.c -fPIC -DPIC -o libcacard/.libs/cac.o In file included from /home/petmay01/linaro/qemu-for-merges/include/qemu-common.h:15:0, from /home/petmay01/linaro/qemu-for-merges/libcacard/cac.c:8: /home/petmay01/linaro/qemu-for-merges/include/qemu/compiler.h:6:25: fatal error: config-host.h: No such file or directory #include "config-host.h" ^ compilation terminated. We're trying to do a "build .o file and .d file in the same go" gcc command, so the fact that we don't have the generated header yet causes us to blow up. This would work if we built the .d files separately using -MG to tell gcc to ignore missing headers... thanks -- PMM
diff --git a/Makefile b/Makefile index 145adb6..dca33b4 100644 --- a/Makefile +++ b/Makefile @@ -546,11 +546,9 @@ ifdef SIGNCODE endif # SIGNCODE endif # CONFIG_WIN -# Add a dependency on the generated files, so that they are always -# rebuilt before other object files -ifneq ($(filter-out %clean,$(MAKECMDGOALS)),$(if $(MAKECMDGOALS),,fail)) -Makefile: $(GENERATED_HEADERS) -endif +# Make the generated files depend on config-host.mak so that if +# the configuration settings change we will rebuild them +$(GENERATED_HEADERS): config-host.mak # Include automatically generated dependency files # Dependencies in Makefile.objs files come from our recursive subdir rules diff --git a/Makefile.target b/Makefile.target index 6089d29..ea8614a 100644 --- a/Makefile.target +++ b/Makefile.target @@ -198,4 +198,4 @@ ifdef CONFIG_TRACE_SYSTEMTAP endif GENERATED_HEADERS += config-target.h -Makefile: $(GENERATED_HEADERS) +$(GENERATED_HEADERS): config-target.mak config-devices.mak
Having a direct dependency Makefile: $(GENERATED_HEADERS) can result in not-from-clean builds failing sometimes, because it means that when Make does its "is any makefile or include out of date and needing a rebuild?" check, as well as possibly running configure (to update config-host.mak) it will also rebuild GENERATED_HEADERS under the "old" config-host.mak regime. If the makefile rules for rebuilding the generated headers have changed in a way that means they're not compatible with the old config-host.mak variable names, the build will fail. Even if it does work, it's wasted effort since we'll need to rebuild them with the right config-host.mak settings immediately. Instead, make the generated headers depend on config-host.mak and config-target.mak. This means we'll definitely rebuild them if the configuration changes, but it will be done after Make reloads its makefiles and includes so will happen with the correct set of config-host.mak settings. We rely on the various .o files having correct autogenerated dependency rules to cause the generated headers to be generated as and when they are needed. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- The specific example of this is the change from CONFIG_TRACE_BACKEND to CONFIG_TRACE_BACKENDS, which I am still hitting on a pretty regular basis. This patch fixes that problem and I don't think it has any unpleasant side effects; Paolo, have I missed anything? Makefile | 8 +++----- Makefile.target | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-)