[Qemu-devel] [PATCH] Makefile: Don't build generated headers before Makefile is reread

[Qemu-devel] [PATCH] Makefile: Don't build generated headers before Makefile is reread

[Peter Maydell]

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
-- 
2.0.0

Re: [Qemu-devel] [PATCH] Makefile: Don't build generated headers before Makefile is reread

[Paolo Bonzini]

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

Re: [Qemu-devel] [PATCH] Makefile: Don't build generated headers before Makefile is reread

[Peter Maydell]

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

Re: [Qemu-devel] [PATCH] Makefile: Don't build generated headers before Makefile is reread

[Paolo Bonzini]

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

Re: [Qemu-devel] [PATCH] Makefile: Don't build generated headers before Makefile is reread

[Peter Maydell]

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

Re: [Qemu-devel] [PATCH] Makefile: Don't build generated headers before Makefile is reread

[Paolo Bonzini]

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

Re: [Qemu-devel] [PATCH] Makefile: Don't build generated headers before Makefile is reread

[Peter Maydell]

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

Re: [Qemu-devel] [PATCH] Makefile: Don't build generated headers before Makefile is reread

[Paolo Bonzini]

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

Re: [Qemu-devel] [PATCH] Makefile: Don't build generated headers before Makefile is reread

[Peter Maydell]

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