[Qemu-devel] [PATCH] make: remove generated objects from target dirs

[Qemu-devel] [PATCH] make: remove generated objects from target dirs

[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)"),)
 	$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $* V="$(V)" TARGET_DIR="$*/" all,)
 
 ifneq ($(wildcard config-host.mak),)

Re: [Qemu-devel] [PATCH] make: remove generated objects from target dirs

[Peter Maydell]

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

Re: [Qemu-devel] [PATCH] make: remove generated objects from target dirs

[Michael S. Tsirkin]

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

Re: [Qemu-devel] [PATCH] make: remove generated objects from target dirs

[Peter Maydell]

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

Re: [Qemu-devel] [PATCH] make: remove generated objects from target dirs

[Michael S. Tsirkin]

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.

-- 
MST

Re: [Qemu-devel] [PATCH] make: remove generated objects from target dirs

[Andreas Färber]

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),)

-- 
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: remove generated objects from target dirs

[Michael S. Tsirkin]

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