[Qemu-devel] [PATCH] make: automatically include dependencies in recursive subdir rules

[Qemu-devel] [PATCH] make: automatically include dependencies in recursive subdir rules

[Anthony Liguori]

I think I understand enough of what's going on in these rules to ensure this is
right.  But I could certainly use a second or third opinion...

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 Makefile  |    4 ++--
 rules.mak |    1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index cce45fb..593bd9b 100644
--- a/Makefile
+++ b/Makefile
@@ -405,5 +405,5 @@ tar:
 Makefile: $(GENERATED_HEADERS)
 
 # Include automatically generated dependency files
--include $(wildcard *.d audio/*.d slirp/*.d block/*.d net/*.d ui/*.d qapi/*.d)
--include $(wildcard qga/*.d hw/*.d hw/usb/*.d qom/*.d)
+# All subdir dependencies come automatically from our recursive subdir rules
+-include $(wildcard *.d)
diff --git a/rules.mak b/rules.mak
index 4bc5e52..8c5ef95 100644
--- a/rules.mak
+++ b/rules.mak
@@ -94,6 +94,7 @@ define unnest-dir
 $(foreach var,$(nested-vars),$(call push-var,$(var),$1/))
 $(eval obj := $(obj)/$1)
 $(eval include $(SRC_PATH)/$1/Makefile.objs)
+$(eval -include $(wildcard $(SRC_PATH)/$1/*.d))
 $(eval obj := $(patsubst %/$1,%,$(obj)))
 $(foreach var,$(nested-vars),$(call pop-var,$(var),$1/))
 endef
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH] make: automatically include dependencies in recursive subdir rules

[Blue Swirl]

On Sun, Jun 17, 2012 at 2:57 PM, Anthony Liguori <aliguori@us.ibm.com> wrote:
> I think I understand enough of what's going on in these rules to ensure this is
> right.  But I could certainly use a second or third opinion...
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  Makefile  |    4 ++--
>  rules.mak |    1 +
>  2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index cce45fb..593bd9b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -405,5 +405,5 @@ tar:
>  Makefile: $(GENERATED_HEADERS)
>
>  # Include automatically generated dependency files
> --include $(wildcard *.d audio/*.d slirp/*.d block/*.d net/*.d ui/*.d qapi/*.d)
> --include $(wildcard qga/*.d hw/*.d hw/usb/*.d qom/*.d)
> +# All subdir dependencies come automatically from our recursive subdir rules
> +-include $(wildcard *.d)

include $(wildcard *.d */*.d */*/*.d */*/*/*.d)
?

> diff --git a/rules.mak b/rules.mak
> index 4bc5e52..8c5ef95 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -94,6 +94,7 @@ define unnest-dir
>  $(foreach var,$(nested-vars),$(call push-var,$(var),$1/))
>  $(eval obj := $(obj)/$1)
>  $(eval include $(SRC_PATH)/$1/Makefile.objs)
> +$(eval -include $(wildcard $(SRC_PATH)/$1/*.d))

This assumes a build in the source directory, s/SRC_PATH/BUILD_DIR/.

>  $(eval obj := $(patsubst %/$1,%,$(obj)))
>  $(foreach var,$(nested-vars),$(call pop-var,$(var),$1/))
>  endef
> --
> 1.7.5.4
>
>

Re: [Qemu-devel] [PATCH] make: automatically include dependencies in recursive subdir rules

[Andreas Färber]

Am 17.06.2012 16:57, schrieb Anthony Liguori:
> I think I understand enough of what's going on in these rules to ensure this is
> right.  But I could certainly use a second or third opinion...
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  Makefile  |    4 ++--
>  rules.mak |    1 +
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index cce45fb..593bd9b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -405,5 +405,5 @@ tar:
>  Makefile: $(GENERATED_HEADERS)
>  
>  # Include automatically generated dependency files
> --include $(wildcard *.d audio/*.d slirp/*.d block/*.d net/*.d ui/*.d qapi/*.d)
> --include $(wildcard qga/*.d hw/*.d hw/usb/*.d qom/*.d)

This was based on my QOM Makefile patch and doesn't apply to master.

If I understand this patch correctly, we could further apply this diff
on top:

diff --git a/Makefile.dis b/Makefile.dis
index 3e1fcaf..09060f0 100644
--- a/Makefile.dis
+++ b/Makefile.dis
@@ -20,4 +20,4 @@ clean:
 	rm -f *.o *.d *.a *~

 # Include automatically generated dependency files
--include $(wildcard *.d */*.d)
+-include $(wildcard *.d)
diff --git a/Makefile.hw b/Makefile.hw
index 2bcbaff..3e794e1 100644
--- a/Makefile.hw
+++ b/Makefile.hw
@@ -23,4 +23,4 @@ clean:
 	rm -f $(addsuffix /*.d, $(dir $(sort $(hw-obj-y))))

 # Include automatically generated dependency files
--include $(patsubst %.o, %.d, $(hw-obj-y))
+-include $(wildcard *.d)
diff --git a/Makefile.target b/Makefile.target
index 2907aad..550d889 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -216,4 +216,4 @@ GENERATED_HEADERS += config-target.h
 Makefile: $(GENERATED_HEADERS)

 # Include automatically generated dependency files
--include $(wildcard *.d */*.d)
+-include $(wildcard *.d)
diff --git a/Makefile.user b/Makefile.user
index b717820..0ffefe8 100644
--- a/Makefile.user
+++ b/Makefile.user
@@ -23,4 +23,4 @@ clean:
 	done

 # Include automatically generated dependency files
--include $(wildcard *.d */*.d)
+-include $(wildcard *.d)

/-F

> +# All subdir dependencies come automatically from our recursive subdir rules
> +-include $(wildcard *.d)
> diff --git a/rules.mak b/rules.mak
> index 4bc5e52..8c5ef95 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -94,6 +94,7 @@ define unnest-dir
>  $(foreach var,$(nested-vars),$(call push-var,$(var),$1/))
>  $(eval obj := $(obj)/$1)
>  $(eval include $(SRC_PATH)/$1/Makefile.objs)
> +$(eval -include $(wildcard $(SRC_PATH)/$1/*.d))
>  $(eval obj := $(patsubst %/$1,%,$(obj)))
>  $(foreach var,$(nested-vars),$(call pop-var,$(var),$1/))
>  endef


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] make: automatically include dependencies in recursive subdir rules

[Andreas Färber]

Am 17.06.2012 17:20, schrieb Blue Swirl:
> On Sun, Jun 17, 2012 at 2:57 PM, Anthony Liguori <aliguori@us.ibm.com> wrote:
>> I think I understand enough of what's going on in these rules to ensure this is
>> right.  But I could certainly use a second or third opinion...
>>
>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>> ---
>>  Makefile  |    4 ++--
>>  rules.mak |    1 +
>>  2 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index cce45fb..593bd9b 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -405,5 +405,5 @@ tar:
>>  Makefile: $(GENERATED_HEADERS)
>>
>>  # Include automatically generated dependency files
>> --include $(wildcard *.d audio/*.d slirp/*.d block/*.d net/*.d ui/*.d qapi/*.d)
>> --include $(wildcard qga/*.d hw/*.d hw/usb/*.d qom/*.d)
>> +# All subdir dependencies come automatically from our recursive subdir rules
>> +-include $(wildcard *.d)
> 
> include $(wildcard *.d */*.d */*/*.d */*/*/*.d)
> ?

No. Due to our directory structure not all dependency files apply to
Makefile.

Anyway, the problem is that any such "fixed" wildcard expression still
silently fails when another level gets added. We've seen that simply no
one notices, and the resulting errors are weird and non-telling.
Therefore it was suggested to have some automatic way of deducing the
dependency files from .o files. (See the "Makefile: Fix dependencies for
QOM" thread.)

>> diff --git a/rules.mak b/rules.mak
>> index 4bc5e52..8c5ef95 100644
>> --- a/rules.mak
>> +++ b/rules.mak
>> @@ -94,6 +94,7 @@ define unnest-dir
>>  $(foreach var,$(nested-vars),$(call push-var,$(var),$1/))
>>  $(eval obj := $(obj)/$1)
>>  $(eval include $(SRC_PATH)/$1/Makefile.objs)
>> +$(eval -include $(wildcard $(SRC_PATH)/$1/*.d))
> 
> This assumes a build in the source directory, s/SRC_PATH/BUILD_DIR/.

I've tested with just
+$(eval -include $(wildcard $1/*.d))
and did
$ touch include/qemu/object.h

>From what I see, using Anthony's original patch plus the modification
above (not the other ones), {i386,x86_64}-softmmu/hw/kvm/apic.o is still
not getting rebuilt despite its included hw/apic_internal.h depending on
sysbus.h -> qdev.h -> qemu/object.h.

They're being added via obj-$(CONFIG_KVM) += kvm/filename.o, not via the
recursive directory rule, so are not caught by Anthony's rule.

Maybe fix by some explicit Makefile-specific rule such as this?

diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index d43f1df..1e8423a 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -11,3 +11,5 @@ obj-$(CONFIG_KVM) += kvm/clock.o kvm/apic.o
kvm/i8259.o kvm/ioapic.o kvm/i8254.o
 obj-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o

 obj-y := $(addprefix ../,$(obj-y))
+
+-include $(wildcard ../kvm/*.d)

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] make: automatically include dependencies in recursive subdir rules

[Andreas Färber]

Am 17.06.2012 18:23, schrieb Andreas Färber:
> I've tested with just
> +$(eval -include $(wildcard $1/*.d))
> and did
> $ touch include/qemu/object.h
> 
> From what I see, using Anthony's original patch plus the modification
> above (not the other ones), {i386,x86_64}-softmmu/hw/kvm/apic.o is still
> not getting rebuilt despite its included hw/apic_internal.h depending on
> sysbus.h -> qdev.h -> qemu/object.h.
> 
> They're being added via obj-$(CONFIG_KVM) += kvm/filename.o, not via the
> recursive directory rule, so are not caught by Anthony's rule.
> 
> Maybe fix by some explicit Makefile-specific rule such as this?
> 
> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
> index d43f1df..1e8423a 100644
> --- a/hw/i386/Makefile.objs
> +++ b/hw/i386/Makefile.objs
> @@ -11,3 +11,5 @@ obj-$(CONFIG_KVM) += kvm/clock.o kvm/apic.o
> kvm/i8259.o kvm/ioapic.o kvm/i8254.o
>  obj-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
> 
>  obj-y := $(addprefix ../,$(obj-y))
> +
> +-include $(wildcard ../kvm/*.d)

If using hw/kvm/*.d instead, it actually works. ;)

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] make: automatically include dependencies in recursive subdir rules

[Anthony Liguori]

On 06/17/2012 12:00 PM, Andreas Färber wrote:
> Am 17.06.2012 18:23, schrieb Andreas Färber:
>> I've tested with just
>> +$(eval -include $(wildcard $1/*.d))
>> and did
>> $ touch include/qemu/object.h
>>
>>  From what I see, using Anthony's original patch plus the modification
>> above (not the other ones), {i386,x86_64}-softmmu/hw/kvm/apic.o is still
>> not getting rebuilt despite its included hw/apic_internal.h depending on
>> sysbus.h ->  qdev.h ->  qemu/object.h.
>>
>> They're being added via obj-$(CONFIG_KVM) += kvm/filename.o, not via the
>> recursive directory rule, so are not caught by Anthony's rule.
>>
>> Maybe fix by some explicit Makefile-specific rule such as this?
>>
>> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
>> index d43f1df..1e8423a 100644
>> --- a/hw/i386/Makefile.objs
>> +++ b/hw/i386/Makefile.objs
>> @@ -11,3 +11,5 @@ obj-$(CONFIG_KVM) += kvm/clock.o kvm/apic.o
>> kvm/i8259.o kvm/ioapic.o kvm/i8254.o
>>   obj-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
>>
>>   obj-y := $(addprefix ../,$(obj-y))
>> +
>> +-include $(wildcard ../kvm/*.d)
>
> If using hw/kvm/*.d instead, it actually works. ;)

We can also just add a hw/kvm/Makefile.objs and then add kvm/ to the subdir 
rules.  Seems to fix the problem you identified.

Just doing a full build for v2.  Will send out when it completes.

Regards,

Anthony Liguori

>
> Andreas
>

Re: [Qemu-devel] [PATCH] make: automatically include dependencies in recursive subdir rules

[Andreas Färber]

Am 17.06.2012 20:46, schrieb Anthony Liguori:
> On 06/17/2012 12:00 PM, Andreas Färber wrote:
>> Am 17.06.2012 18:23, schrieb Andreas Färber:
>>> I've tested with just
>>> +$(eval -include $(wildcard $1/*.d))
>>> and did
>>> $ touch include/qemu/object.h
>>>
>>>  From what I see, using Anthony's original patch plus the modification
>>> above (not the other ones), {i386,x86_64}-softmmu/hw/kvm/apic.o is still
>>> not getting rebuilt despite its included hw/apic_internal.h depending on
>>> sysbus.h ->  qdev.h ->  qemu/object.h.
>>>
>>> They're being added via obj-$(CONFIG_KVM) += kvm/filename.o, not via the
>>> recursive directory rule, so are not caught by Anthony's rule.
>>>
>>> Maybe fix by some explicit Makefile-specific rule such as this?
>>>
>>> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
>>> index d43f1df..1e8423a 100644
>>> --- a/hw/i386/Makefile.objs
>>> +++ b/hw/i386/Makefile.objs
>>> @@ -11,3 +11,5 @@ obj-$(CONFIG_KVM) += kvm/clock.o kvm/apic.o
>>> kvm/i8259.o kvm/ioapic.o kvm/i8254.o
>>>   obj-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
>>>
>>>   obj-y := $(addprefix ../,$(obj-y))
>>> +
>>> +-include $(wildcard ../kvm/*.d)
>>
>> If using hw/kvm/*.d instead, it actually works. ;)
> 
> We can also just add a hw/kvm/Makefile.objs and then add kvm/ to the
> subdir rules.  Seems to fix the problem you identified.

No, that would be wrong wrt ppc/s390/arm. But we could move the files
from hw/kvm/ to hw/i386/.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] make: automatically include dependencies in recursive subdir rules

[Anthony Liguori]

On 06/17/2012 02:53 PM, Andreas Färber wrote:
> Am 17.06.2012 20:46, schrieb Anthony Liguori:
>> On 06/17/2012 12:00 PM, Andreas Färber wrote:
>>> If using hw/kvm/*.d instead, it actually works. ;)
>>
>> We can also just add a hw/kvm/Makefile.objs and then add kvm/ to the
>> subdir rules.  Seems to fix the problem you identified.
>
> No, that would be wrong wrt ppc/s390/arm. But we could move the files
> from hw/kvm/ to hw/i386/.

Hrm, no, I don't think that's necessary.

Look at v2 of my patch.  kvm/ is only added by target hw makefiles 
(hw/i386/Makefile.objs).  So these devices are only even attempted to be added 
for i386.

Then within kvm/Makefile.objs, everything is guarded with CONFIG_KVM.

I think a better approach would be to:

hw/Makefiles.obj:
   obj-$(CONFIG_KVM) += kvm/

hw/kvm/Makefiles.obj:
   obj-$(CONFIG_I386) += apic.o pit.o ...

That way there could be ppc-specific KVM devices.

But that's a bigger refactoring as these devices need to be built target-specific.

Regards,

Anthony Liguori

>
> Andreas
>