| Message ID | 20120304091007.GA2252@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 4 March 2012 09:10, Michael S. Tsirkin <mst@redhat.com> wrote: > I ended up with qmp-commands.h in target directories, > which makes build fail as it is found before the > main header. > make clean fixes it, but it might get triggered > again when we make some header target-independent next. > It's easy to just make sure all such leftovers are > removed, so let's do this. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > diff --git a/Makefile b/Makefile > index e66e885..958a414 100644 > --- a/Makefile > +++ b/Makefile > @@ -89,6 +89,7 @@ qemu-options.def: $(SRC_PATH)/qemu-options.hx > SUBDIR_RULES=$(patsubst %,subdir-%, $(TARGET_DIRS)) > > subdir-%: $(GENERATED_HEADERS) > + $(call quiet-command,rm -f $(foreach header, $(GENERATED_HEADERS), "$*/$(header)"),) > $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $* V="$(V)" TARGET_DIR="$*/" all,) In general we don't have workarounds for "something moved directory and this broke builds not from clean" (source file moved from hw/ to . being one that's bitten me before), so why does just this one deserve to get an rm here rather than just asking the user to run 'make clean / distclean' ? -- PMM
On Sun, Mar 04, 2012 at 01:25:59PM +0000, Peter Maydell wrote: > On 4 March 2012 09:10, Michael S. Tsirkin <mst@redhat.com> wrote: > > I ended up with qmp-commands.h in target directories, > > which makes build fail as it is found before the > > main header. > > make clean fixes it, but it might get triggered > > again when we make some header target-independent next. > > It's easy to just make sure all such leftovers are > > removed, so let's do this. > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > > diff --git a/Makefile b/Makefile > > index e66e885..958a414 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -89,6 +89,7 @@ qemu-options.def: $(SRC_PATH)/qemu-options.hx > > SUBDIR_RULES=$(patsubst %,subdir-%, $(TARGET_DIRS)) > > > > subdir-%: $(GENERATED_HEADERS) > > + $(call quiet-command,rm -f $(foreach header, $(GENERATED_HEADERS), "$*/$(header)"),) > > $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $* V="$(V)" TARGET_DIR="$*/" all,) > > In general we don't have workarounds for "something > moved directory and this broke builds not from clean" Why don't we? It's cheaper than always doing make clean after pull. > (source file moved from hw/ to . being one that's bitten > me before), Why would that bite anyone? AFAIK files under source control are handled fine. It's the generated ones that are a problem. > so why does just this one deserve to get an > rm here rather than just asking the user to run > 'make clean / distclean' ? > > -- PMM
On 4 March 2012 13:31, Michael S. Tsirkin <mst@redhat.com> wrote: > On Sun, Mar 04, 2012 at 01:25:59PM +0000, Peter Maydell wrote: >> In general we don't have workarounds for "something >> moved directory and this broke builds not from clean" > > Why don't we? It's cheaper than always doing > make clean after pull. That's an argument for a general solution to the problem, not a one-off bandaid fix for the bit that happened to bite you. >> (source file moved from hw/ to . being one that's bitten >> me before), > > Why would that bite anyone? AFAIK files under source control > are handled fine. It's the generated ones that are > a problem. I forget the exact failure mode but I think the problem is that the old arm-softmmu/foo.d file is still lying around and claims that foo.o depends on the no-longer-present hw/foo.c. (If we didn't squash the directory structure, so that it was arm-softmmu/hw/foo.d and arm-softmmu/hw/foo.o, this problem wouldn't happen.) -- PMM
On Sun, Mar 04, 2012 at 01:44:26PM +0000, Peter Maydell wrote: > On 4 March 2012 13:31, Michael S. Tsirkin <mst@redhat.com> wrote: > > On Sun, Mar 04, 2012 at 01:25:59PM +0000, Peter Maydell wrote: > >> In general we don't have workarounds for "something > >> moved directory and this broke builds not from clean" > > > > Why don't we? It's cheaper than always doing > > make clean after pull. > > That's an argument for a general solution to the problem, > not a one-off bandaid fix for the bit that happened to > bite you. What I did is generic: I remove all generated headers. If you see more problems, let me know. > >> (source file moved from hw/ to . being one that's bitten > >> me before), > > > > Why would that bite anyone? AFAIK files under source control > > are handled fine. It's the generated ones that are > > a problem. > > I forget the exact failure mode but I think the problem > is that the old arm-softmmu/foo.d file is still lying > around and claims that foo.o depends on the no-longer-present > hw/foo.c. (If we didn't squash the directory structure, so > that it was arm-softmmu/hw/foo.d and arm-softmmu/hw/foo.o, > this problem wouldn't happen.) > > -- PMM Aha. A stale .d file, I see. Note that it's siginificantly different from my problem. I would think if .c is newer than .o we should just ignore .d, alternatively make the dependencies in .d somehow weaker than the implicit dependencies so they don't fail the build. I'll see whether I can come up with a solution.
Am 04.03.2012 10:10, schrieb Michael S. Tsirkin: > I ended up with qmp-commands.h in target directories, > which makes build fail as it is found before the > main header. > make clean fixes it, but it might get triggered > again when we make some header target-independent next. > It's easy to just make sure all such leftovers are > removed, so let's do this. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > diff --git a/Makefile b/Makefile > index e66e885..958a414 100644 > --- a/Makefile > +++ b/Makefile > @@ -89,6 +89,7 @@ qemu-options.def: $(SRC_PATH)/qemu-options.hx > SUBDIR_RULES=$(patsubst %,subdir-%, $(TARGET_DIRS)) > > subdir-%: $(GENERATED_HEADERS) > + $(call quiet-command,rm -f $(foreach header, $(GENERATED_HEADERS), "$*/$(header)"),) Nack. While this happens to fix an issue you encountered this is just plain wrong and dangerous. It takes a list of currently generated headers in the main directory and deletes them in all target folders on every compile; it's not forbidden to have a header of the same name in both directories, whether generated or not. I am all for making make clean do what it claims though. The real solution to this problem would be to make sure by careful review not to move files around in such a conflicting way (rename them instead) rather than posting a note to please run make clean or make distclean (which is fine if you have to do it once, bothersome for multiple repos, and unhelpful for bisecting). Andreas > $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $* V="$(V)" TARGET_DIR="$*/" all,) > > ifneq ($(wildcard config-host.mak),)
On Sun, Mar 04, 2012 at 05:03:14PM +0100, Andreas Färber wrote: > Am 04.03.2012 10:10, schrieb Michael S. Tsirkin: > > I ended up with qmp-commands.h in target directories, > > which makes build fail as it is found before the > > main header. > > make clean fixes it, but it might get triggered > > again when we make some header target-independent next. > > It's easy to just make sure all such leftovers are > > removed, so let's do this. > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > > diff --git a/Makefile b/Makefile > > index e66e885..958a414 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -89,6 +89,7 @@ qemu-options.def: $(SRC_PATH)/qemu-options.hx > > SUBDIR_RULES=$(patsubst %,subdir-%, $(TARGET_DIRS)) > > > > subdir-%: $(GENERATED_HEADERS) > > + $(call quiet-command,rm -f $(foreach header, $(GENERATED_HEADERS), "$*/$(header)"),) > > Nack. While this happens to fix an issue you encountered this is just > plain wrong and dangerous. It takes a list of currently generated > headers in the main directory and deletes them in all target folders on > every compile; it's not forbidden to have a header of the same name in > both directories, whether generated or not. Yes it is Forbidden. With a big F. And the reason is that it will break the build in a very confusing way. So if you do this silly thing, you pay the price, and this is way better than everyone who tries to build the tree pays the price. > I am all for making make clean do what it claims though. > > The real solution to this problem would be to make sure by careful > review not to move files around in such a conflicting way (rename them > instead) Too late for current tree. There are many such conflicts. > rather than posting a note to please run make clean or make > distclean (which is fine if you have to do it once, bothersome for > multiple repos, and unhelpful for bisecting). > > Andreas This means that a file name that was generated somewhere in the hierarchy at any point in the past can never be generated again anywhere else. That's impractical. > > $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $* V="$(V)" TARGET_DIR="$*/" all,) > > > > ifneq ($(wildcard config-host.mak),) > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
diff --git a/Makefile b/Makefile index e66e885..958a414 100644 --- a/Makefile +++ b/Makefile @@ -89,6 +89,7 @@ qemu-options.def: $(SRC_PATH)/qemu-options.hx SUBDIR_RULES=$(patsubst %,subdir-%, $(TARGET_DIRS)) subdir-%: $(GENERATED_HEADERS) + $(call quiet-command,rm -f $(foreach header, $(GENERATED_HEADERS), "$*/$(header)"),) $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $* V="$(V)" TARGET_DIR="$*/" all,) ifneq ($(wildcard config-host.mak),)
I ended up with qmp-commands.h in target directories, which makes build fail as it is found before the main header. make clean fixes it, but it might get triggered again when we make some header target-independent next. It's easy to just make sure all such leftovers are removed, so let's do this. Signed-off-by: Michael S. Tsirkin <mst@redhat.com>