| Message ID | 1359042475-4592-1-git-send-email-afaerber@suse.de |
|---|---|
| State | New |
| Headers | show |
Thanks, applied. On Thu, Jan 24, 2013 at 3:47 PM, Andreas Färber <afaerber@suse.de> 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. > > Reported-by: Gerhard Wiesinger <lists@wiesinger.com> > Cc: qemu-stable@nongnu.org > Signed-off-by: Andreas Färber <afaerber@suse.de> > --- > scripts/make_device_config.sh | 2 +- > 1 Datei geändert, 1 Zeile hinzugefügt(+), 1 Zeile entfernt(-) > > diff --git a/scripts/make_device_config.sh b/scripts/make_device_config.sh > index 5d14885..0778fe2 100644 > --- a/scripts/make_device_config.sh > +++ b/scripts/make_device_config.sh > @@ -25,4 +25,4 @@ done > process_includes $src > $dest > > cat $src $all_includes | grep -v '^include' > $dest > -echo "$1: $all_includes" > $dep > +echo "`basename $1`: $all_includes" > $dep > -- > 1.7.10.4 > >
On 24 January 2013 15:47, Andreas Färber <afaerber@suse.de> 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. -- PMM
Am 21.02.2013 13:34, schrieb Peter Maydell: > On 24 January 2013 15:47, Andreas Färber <afaerber@suse.de> 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
Il 21/02/2013 13:52, Andreas Färber ha scritto: > 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 Makefile.target also includes Makefile.objs and calls the unnesting machinery in rules.mak. The machinery then includes the .d files. Paolo > , so we may have discovered a > different issue that if fixed may reintroduce the original bug again...
Am 21.02.2013 14:19, schrieb Paolo Bonzini: > Il 21/02/2013 13:52, Andreas Färber ha scritto: >> 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 > > Makefile.target also includes Makefile.objs and calls the unnesting > machinery in rules.mak. The machinery then includes the .d files. Sure, I'm aware of the unnesting. What I meant is that unnest-vars in rules.mak seems to -include only subdirectories added to the to-be-unnested variables $(var) iiuc. Where does ./*.d get -include'd though? There's no obj-y += ./ to trigger that. Andreas P.S. I'm preparing a revert of my patch and intend to change the file placement to fix the conflict and avoid more misunderstandings. > > Paolo > >> , so we may have discovered a >> different issue that if fixed may reintroduce the original bug again... >
Il 21/02/2013 15:28, Andreas Färber ha scritto: >>> However I don't see in the curent Makefile.target and rules.mak where >>> the *.d files are being included today >> >> Makefile.target also includes Makefile.objs and calls the unnesting >> machinery in rules.mak. The machinery then includes the .d files. > > Sure, I'm aware of the unnesting. What I meant is that unnest-vars in > rules.mak seems to -include only subdirectories added to the > to-be-unnested variables $(var) iiuc. Where does ./*.d get -include'd > though? There's no obj-y += ./ to trigger that. Ah, I see what you mean now. $(dir foo.o) is ./ and that causes rules.mak to include ./*.d. Paolo
diff --git a/scripts/make_device_config.sh b/scripts/make_device_config.sh index 5d14885..0778fe2 100644 --- a/scripts/make_device_config.sh +++ b/scripts/make_device_config.sh @@ -25,4 +25,4 @@ done process_includes $src > $dest cat $src $all_includes | grep -v '^include' > $dest -echo "$1: $all_includes" > $dep +echo "`basename $1`: $all_includes" > $dep
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. Reported-by: Gerhard Wiesinger <lists@wiesinger.com> Cc: qemu-stable@nongnu.org Signed-off-by: Andreas Färber <afaerber@suse.de> --- scripts/make_device_config.sh | 2 +- 1 Datei geändert, 1 Zeile hinzugefügt(+), 1 Zeile entfernt(-)