Re: [Qemu-devel] [PATCH RESEND for-1.4] make_device_config.sh: Fix target path in generated dependency file

[Andreas Färber]

Am 21.02.2013 13:34, schrieb Peter Maydell:
> On 24 January 2013 15:47, Andreas Färber <address@hidden> wrote:
>> config-devices.mak.d is included from Makefile.target, i.e. from inside
>> the *-softmmu/ directory. It included the directory path, so never
>> applied to the actual ./config-devices.mak. Symptoms were spurious
>> build failures due to missing dependency on default-configs/pci.mak.
>>
>> Fix this by using `basename` to strip the directory path.
> 
> This patch seems to break the case it claims to be fixing:
> 
> (1) touching arm-softmmu.mak correctly provokes us to rebuild:
> 
> cam-vm-266:precise:qemu$ make V=1 arm-softmmu/config-devices.mak
> make: `arm-softmmu/config-devices.mak' is up to date.
> cam-vm-266:precise:qemu$ touch default-configs/arm-softmmu.mak
> cam-vm-266:precise:qemu$ make V=1 arm-softmmu/config-devices.mak
> /bin/sh 
> /home/petmay01/linaro/qemu-from-laptop/qemu/scripts/make_device_config.sh
> arm-softmmu/config-devices.mak default-configs/arm-softmmu.mak
> cat  arm-softmmu/config-devices.mak | grep =y | sort -u > 
> config-all-devices.mak
> make: `arm-softmmu/config-devices.mak' is up to date.
> 
> (2) but touching pci.mak does not:
> 
> cam-vm-266:precise:qemu$ make V=1 arm-softmmu/config-devices.mak
> make: `arm-softmmu/config-devices.mak' is up to date.
> cam-vm-266:precise:qemu$ touch default-configs/pci.mak
> cam-vm-266:precise:qemu$ make V=1 arm-softmmu/config-devices.mak
> make: `arm-softmmu/config-devices.mak' is up to date.
> 
> (3) git revert 23bf49b5ec and rm arm-softmmu/config-devices.mak*
> and then things work:
> cam-vm-266:precise:qemu$ make V=1 arm-softmmu/config-devices.mak
> make: `arm-softmmu/config-devices.mak' is up to date.
> cam-vm-266:precise:qemu$ touch default-configs/arm-softmmu.mak
> cam-vm-266:precise:qemu$ make V=1 arm-softmmu/config-devices.mak
> /bin/sh 
> /home/petmay01/linaro/qemu-from-laptop/qemu/scripts/make_device_config.sh
> arm-softmmu/config-devices.mak default-configs/arm-softmmu.mak
> cat  arm-softmmu/config-devices.mak | grep =y | sort -u > 
> config-all-devices.mak
> make: `arm-softmmu/config-devices.mak' is up to date.
> cam-vm-266:precise:qemu$ touch default-configs/pci.mak
> cam-vm-266:precise:qemu$ make V=1 arm-softmmu/config-devices.mak
> /bin/sh 
> /home/petmay01/linaro/qemu-from-laptop/qemu/scripts/make_device_config.sh
> arm-softmmu/config-devices.mak default-configs/arm-softmmu.mak
> cat  arm-softmmu/config-devices.mak | grep =y | sort -u > 
> config-all-devices.mak
> make: `arm-softmmu/config-devices.mak' is up to date.
> 
> 
> In particular the commit message claims "config-devices.mak.d
> is included from Makefile.target" but this is wrong -- this
> is done from the top level Makefile via the line
> "-include $(SUBDIR_DEVICES_MAK_DEP)". This is why you need
> to have the full path in the emitted dependency line.

As discussed on IRC, what happened here it seems is that at the time of
writing the patch (two releases ago) we had -include *.d in
Makefile.target, so the generated .mak.d files were effectively included
from two Makefiles, working for one but not for the other. So
effectively we were observing race conditions between Makefile and
x86_64-softmmu/Makefile a.k.a. Makefile.target.
This seems to have gotten fixed through Paolo's nested-vars handling in
rules.mak, which only does -include *.d for directories listed in obj-y
etc. So my patch was broken and is now completely broken. :(

However I don't see in the curent Makefile.target and rules.mak where
the *.d files are being included today, so we may have discovered a
different issue that if fixed may reintroduce the original bug again...

Paolo?

Andreas

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