Patchwork fix tool/libuser makefile race

login
register
mail settings
Submitter Paolo Bonzini
Date Dec. 18, 2009, 10:40 a.m.
Message ID <1261132832-7688-1-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/41380/
State New
Headers show

Comments

Paolo Bonzini - Dec. 18, 2009, 10:40 a.m.
I just had this race happen on me while building qemu.  The problematic
file in my case was cutils.o.  I'm using GNU make's order-only
dependencies to avoid that "make recurse-all" builds the tools as well.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)
Juan Quintela - Dec. 18, 2009, 4:16 p.m.
Paolo Bonzini <pbonzini@redhat.com> wrote:
> I just had this race happen on me while building qemu.  The problematic
> file in my case was cutils.o.  I'm using GNU make's order-only
> dependencies to avoid that "make recurse-all" builds the tools as well.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  Makefile |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index d555bb2..373a861 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -79,7 +79,10 @@ romsubdir-%:
>  
>  ALL_SUBDIRS=$(TARGET_DIRS) $(patsubst %,pc-bios/%, $(ROMS))
>  
> -recurse-all: $(SUBDIR_RULES) $(ROMSUBDIR_RULES)
> +# Some files are shared between the tools and the emulators.  So there
> +# can be a race when the main makefile starts building xyz.o, while
> +# the recursive make sees a partially built xyz.o and ar(1) fails.
> +recurse-all: $(SUBDIR_RULES) $(ROMSUBDIR_RULES) | $(TOOLS)
>  
>  #######################################################################
>  # QObject

I still don't understand why that is needed :( (make mysteries).

All shared stuff between $(TOOLS) and $(SUBDIR_RULES) are made from the
same Makefile invocation.  I don't see where the recursive make can
build a file that is also built by the normal Makefile.

And if we have one file that is build in both places, that is the bug
that we have to fix.

Notice that cutils.o (from Makefile) is built in the root dir, and
cutils.o (from Makefile.user) is built into libuser/ directory.

I am confused how one can interfer with the other.

VPATH vs vpath problems?

Later, Juan.
Paolo Bonzini - Dec. 18, 2009, 4:22 p.m.
> Notice that cutils.o (from Makefile) is built in the root dir, and
> cutils.o (from Makefile.user) is built into libuser/ directory.

Ah, since that was the file that was built, that would be a problem
with my reasoning.

Definitely I saw the following in the log

    AR     libuser.a
    CC     cutils.o
    error: invalid cutils.o

or something like that.

> VPATH vs vpath problems?

Definitely a possibility.

Paolo
Andreas Färber - Dec. 18, 2009, 6:59 p.m.
Hey,

Am 18.12.2009 um 17:16 schrieb Juan Quintela:

> Paolo Bonzini <pbonzini@redhat.com> wrote:
>> I just had this race happen on me while building qemu.  The  
>> problematic
>> file in my case was cutils.o.  I'm using GNU make's order-only
>> dependencies to avoid that "make recurse-all" builds the tools as  
>> well.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>> Makefile |    5 ++++-
>> 1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index d555bb2..373a861 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -79,7 +79,10 @@ romsubdir-%:
>>
>> ALL_SUBDIRS=$(TARGET_DIRS) $(patsubst %,pc-bios/%, $(ROMS))
>>
>> -recurse-all: $(SUBDIR_RULES) $(ROMSUBDIR_RULES)
>> +# Some files are shared between the tools and the emulators.  So  
>> there
>> +# can be a race when the main makefile starts building xyz.o, while
>> +# the recursive make sees a partially built xyz.o and ar(1) fails.
>> +recurse-all: $(SUBDIR_RULES) $(ROMSUBDIR_RULES) | $(TOOLS)
>>
>> #######################################################################
>> # QObject
>
> I still don't understand why that is needed :( (make mysteries).
>
> All shared stuff between $(TOOLS) and $(SUBDIR_RULES) are made from  
> the
> same Makefile invocation.  I don't see where the recursive make can
> build a file that is also built by the normal Makefile.

Think of my porposed Makefile patch!

> And if we have one file that is build in both places, that is the bug
> that we have to fix.
>
> Notice that cutils.o (from Makefile) is built in the root dir, and
> cutils.o (from Makefile.user) is built into libuser/ directory.
>
> I am confused how one can interfer with the other.
>
> VPATH vs vpath problems?

libuser builds some .o files from .c files in $(SRC_PATH). They end up  
in libuser. For those that were built already in .., the .o files are  
taken from there instead.

Andreas

>
> Later, Juan.
>
>
Paolo Bonzini - Dec. 20, 2009, 6:22 p.m.
On 12/18/2009 07:59 PM, Andreas Färber wrote:
>
> Think of my porposed Makefile patch!

Which one?

Paolo
Andreas Färber - Dec. 20, 2009, 6:30 p.m.
Am 20.12.2009 um 19:22 schrieb Paolo Bonzini:

> On 12/18/2009 07:59 PM, Andreas Färber wrote:
>>
>> Think of my porposed Makefile patch!
>
> Which one?

In particular my add-on patch for libuser.a, which tackled exactly  
that issue:

http://www.mail-archive.com/qemu-devel@nongnu.org/msg20948.html

Andreas
Paolo Bonzini - Dec. 20, 2009, 6:38 p.m.
On 12/20/2009 07:30 PM, Andreas Färber wrote:
>
> Am 20.12.2009 um 19:22 schrieb Paolo Bonzini:
>
>> On 12/18/2009 07:59 PM, Andreas Färber wrote:
>>>
>>> Think of my porposed Makefile patch!
>>
>> Which one?
>
> In particular my add-on patch for libuser.a, which tackled exactly that
> issue:
>
> http://www.mail-archive.com/qemu-devel@nongnu.org/msg20948.html

Ah, thanks.  Just to be clear, this issue was on Linux, not on Solaris. 
  It is completely unrelated to --whole-archive and is a real bug.

Paolo
Andreas Färber - Dec. 20, 2009, 6:57 p.m.
Am 20.12.2009 um 19:38 schrieb Paolo Bonzini:

> On 12/20/2009 07:30 PM, Andreas Färber wrote:
>>
>> Am 20.12.2009 um 19:22 schrieb Paolo Bonzini:
>>
>>> On 12/18/2009 07:59 PM, Andreas Färber wrote:
>>>>
>>>> Think of my porposed Makefile patch!
>>>
>>> Which one?
>>
>> In particular my add-on patch for libuser.a, which tackled exactly  
>> that
>> issue:
>>
>> http://www.mail-archive.com/qemu-devel@nongnu.org/msg20948.html
>
> Ah, thanks.  Just to be clear, this issue was on Linux, not on  
> Solaris.  It is completely unrelated to --whole-archive and is a  
> real bug.

I did understand that. My patch was for Linux (there being no solaris- 
user) and acknowledged that there is such an unexpected mixture of  
libuser.a and libqemu_common.a object files, so it wasn't an all new  
discovery now.
I didn't attempt to change that behavior though, just Made It Work(tm)  
for regular builds.

Since we're bound to GNU make anyway, I have no objections to your  
patch if it works.

Andreas

Patch

diff --git a/Makefile b/Makefile
index d555bb2..373a861 100644
--- a/Makefile
+++ b/Makefile
@@ -79,7 +79,10 @@  romsubdir-%:
 
 ALL_SUBDIRS=$(TARGET_DIRS) $(patsubst %,pc-bios/%, $(ROMS))
 
-recurse-all: $(SUBDIR_RULES) $(ROMSUBDIR_RULES)
+# Some files are shared between the tools and the emulators.  So there
+# can be a race when the main makefile starts building xyz.o, while
+# the recursive make sees a partially built xyz.o and ar(1) fails.
+recurse-all: $(SUBDIR_RULES) $(ROMSUBDIR_RULES) | $(TOOLS)
 
 #######################################################################
 # QObject