Patchwork [dylan,master] autotools.bbclass: Fix race with sed-native

login
register
mail settings
Submitter Richard Tollerton
Date Oct. 4, 2013, 11:35 p.m.
Message ID <1380929753-10606-1-git-send-email-rich.tollerton@ni.com>
Download mbox | patch
Permalink /patch/59285/
State New
Headers show

Comments

Richard Tollerton - Oct. 4, 2013, 11:35 p.m.
Suppose the following:
1) foo.do_configure/do_build runs in parallel with
   sed-native.do_configure;
2) foo.do_configure/do_build makes use of sed (i.e. inherits autotools);
3) A previously built sed-native already exists in the native sysroot,
   and in the sstate cache.

Then sed-native may be deleted from its sysroot via
sstate_clean_manifest while foo.do_configure/do_build is using it,
leading to an irreproducible build failure.

This fix does for sed-native what's already done for libtool-native,
with some additional light refactoring.

Signed-off-by: Richard Tollerton <rich.tollerton@ni.com>
---
 meta/classes/autotools.bbclass | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)
Saul Wold - Oct. 5, 2013, 3:22 a.m.
On 10/04/2013 04:35 PM, Richard Tollerton wrote:
> Suppose the following:
> 1) foo.do_configure/do_build runs in parallel with
>     sed-native.do_configure;
> 2) foo.do_configure/do_build makes use of sed (i.e. inherits autotools);
> 3) A previously built sed-native already exists in the native sysroot,
>     and in the sstate cache.
>
> Then sed-native may be deleted from its sysroot via
> sstate_clean_manifest while foo.do_configure/do_build is using it,
> leading to an irreproducible build failure.
>
> This fix does for sed-native what's already done for libtool-native,
> with some additional light refactoring.
>
This also starts to create more front end bottle next on autotools 
related items before we can really start to parallelize the build.

More thought is required on this.

> Signed-off-by: Richard Tollerton <rich.tollerton@ni.com>
> ---
>   meta/classes/autotools.bbclass | 26 +++++++++++++++++---------
>   1 file changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/meta/classes/autotools.bbclass b/meta/classes/autotools.bbclass
> index 66c0f5d..2bb2aa3 100644
> --- a/meta/classes/autotools.bbclass
> +++ b/meta/classes/autotools.bbclass
> @@ -7,17 +7,25 @@ def autotools_dep_prepend(d):
>
>       if pn in ['autoconf-native', 'automake-native', 'help2man-native']:
>           return deps
> -    deps += 'autoconf-native automake-native '
> +    deps += 'autoconf-native automake-native gnu-config-native '
>
You don't mention anything about this change.

Sau!
> -    if not pn in ['libtool', 'libtool-native'] and not pn.endswith("libtool-cross"):
> -        deps += 'libtool-native '
> -        if not bb.data.inherits_class('native', d) \
> -                        and not bb.data.inherits_class('nativesdk', d) \
> -                        and not bb.data.inherits_class('cross', d) \
> -                        and not d.getVar('INHIBIT_DEFAULT_DEPS', True):
> -            deps += 'libtool-cross '
> +    if pn in ['libtool', 'libtool-native'] or pn.endswith("libtool-cross"):
> +        return deps
> +    deps += 'libtool-native '
> +
> +    if d.getVar('INHIBIT_DEFAULT_DEPS', True):
> +        return deps
> +
> +    if pn not in ['sed-native']:
> +        deps += 'sed-native '
> +
> +    if bb.data.inherits_class('native', d) \
> +       or bb.data.inherits_class('nativesdk', d) \
> +       or bb.data.inherits_class('cross', d):
> +        return deps
> +    deps += 'libtool-cross '
>
> -    return deps + 'gnu-config-native '
> +    return deps
>
>   EXTRA_OEMAKE = ""
>
>
Richard Purdie - Oct. 5, 2013, 7:37 a.m.
On Fri, 2013-10-04 at 20:22 -0700, Saul Wold wrote:
> On 10/04/2013 04:35 PM, Richard Tollerton wrote:
> > Suppose the following:
> > 1) foo.do_configure/do_build runs in parallel with
> >     sed-native.do_configure;
> > 2) foo.do_configure/do_build makes use of sed (i.e. inherits autotools);
> > 3) A previously built sed-native already exists in the native sysroot,
> >     and in the sstate cache.
> >
> > Then sed-native may be deleted from its sysroot via
> > sstate_clean_manifest while foo.do_configure/do_build is using it,
> > leading to an irreproducible build failure.
> >
> > This fix does for sed-native what's already done for libtool-native,
> > with some additional light refactoring.
> >
> This also starts to create more front end bottle next on autotools 
> related items before we can really start to parallelize the build.
> 
> More thought is required on this.

Please have a look at the sed-native changes that went into master
recently as I think this issue has been fixed differently...

Cheers,

Richard
Richard Tollerton - Oct. 8, 2013, 2:44 a.m.
richard.purdie@linuxfoundation.org writes:

> On Fri, 2013-10-04 at 20:22 -0700, Saul Wold wrote:
>> On 10/04/2013 04:35 PM, Richard Tollerton wrote:
>> This also starts to create more front end bottle next on autotools 
>> related items before we can really start to parallelize the build.
>> 
>> More thought is required on this.
>
> Please have a look at the sed-native changes that went into master
> recently as I think this issue has been fixed differently...

Indeed, I agree that commit db2eb325 would fix the race... aaaand this
also makes me 2-for-2 this month on reinventing the wheel. :'(

But this brings up a question I've been meaning to ask.

Build reproducibility is reduced (all other things being equal) as more
external dependencies are introduced. So I would have expected that The
Right Thing here would have been to specify -native packages as explicit
dependencies (just like it's always important to comprehensively
enumerate dependencies). Not to add specific short-circuits to use the
system package. sed's probably a bad example here since it is rather
well-behaved -- surely this sort of issue comes up for many other
packages, though...

I know that the OE build is horribly slow, but is it really so slow as
to require short-circuiting native package dependencies such as this?
Has there been a general architectural decision made over this sort of
thing? (I'm sorry, I tried to search for one on the mailing list and
wiki, but couldn't find any.)

>
> Cheers,
>
> Richard
Richard Tollerton - Oct. 8, 2013, 2:55 a.m.
sgw@linux.intel.com writes:

>> diff --git a/meta/classes/autotools.bbclass b/meta/classes/autotools.bbclass
>> index 66c0f5d..2bb2aa3 100644
>> --- a/meta/classes/autotools.bbclass
>> +++ b/meta/classes/autotools.bbclass
>> @@ -7,17 +7,25 @@ def autotools_dep_prepend(d):
>>
>>       if pn in ['autoconf-native', 'automake-native', 'help2man-native']:
>>           return deps
>> -    deps += 'autoconf-native automake-native '
>> +    deps += 'autoconf-native automake-native gnu-config-native '
>>
> You don't mention anything about this change.

This might be a moot topic, but maybe I did miss something here:

I thought it was implied in the “light refactoring”. In the original
code, 'gnu-config-native ' would have always been appended by this point
in the code. I thought the "if ...: return deps" idiom would greatly
simplify this function, but in order to use it, 'gnu-config-native '
needs to be appended before each return. So just append it here.

> Sau!
Richard Purdie - Oct. 8, 2013, 12:19 p.m.
On Mon, 2013-10-07 at 21:44 -0500, Richard Tollerton wrote:
> richard.purdie@linuxfoundation.org writes:
> 
> > On Fri, 2013-10-04 at 20:22 -0700, Saul Wold wrote:
> >> On 10/04/2013 04:35 PM, Richard Tollerton wrote:
> >> This also starts to create more front end bottle next on autotools 
> >> related items before we can really start to parallelize the build.
> >> 
> >> More thought is required on this.
> >
> > Please have a look at the sed-native changes that went into master
> > recently as I think this issue has been fixed differently...
> 
> Indeed, I agree that commit db2eb325 would fix the race... aaaand this
> also makes me 2-for-2 this month on reinventing the wheel. :'(
> 
> But this brings up a question I've been meaning to ask.
> 
> Build reproducibility is reduced (all other things being equal) as more
> external dependencies are introduced. So I would have expected that The
> Right Thing here would have been to specify -native packages as explicit
> dependencies (just like it's always important to comprehensively
> enumerate dependencies). Not to add specific short-circuits to use the
> system package. sed's probably a bad example here since it is rather
> well-behaved -- surely this sort of issue comes up for many other
> packages, though...
> 
> I know that the OE build is horribly slow, but is it really so slow as
> to require short-circuiting native package dependencies such as this?
> Has there been a general architectural decision made over this sort of
> thing? (I'm sorry, I tried to search for one on the mailing list and
> wiki, but couldn't find any.)

Some native utilities cause big problems in the dependency chain. Things
like tar and gzip have caused us big problems in the past. We've made a
conscious decision to require specific versions of tar, gzip and git for
example to ensure builds work as expected.

The list of things we assume are ok is relatively small and we look at
new issues on a case by case basis. I think sed is well enough
established and well enough behaved to be something we can rely on. In
general we don't rely on much. Do you have any other specific things you
worry about? When you start to look at it, the list is rather small...

Cheers,

Richard
Enrico Scholz - Oct. 8, 2013, 1:03 p.m.
Richard Purdie
<richard.purdie-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
writes:

> The list of things we assume are ok is relatively small and we look at
> new issues on a case by case basis. I think sed is well enough
> established and well enough behaved to be something we can rely on. In
> general we don't rely on much. Do you have any other specific things you
> worry about? 

ccache... it causes big problems when CC='ccache real-gcc' is set and
ccache-native gets rebuilt in parallel.  A customer had problems when
building apr-utils (libtool reported missing '--tag=cc' option) which
are very likely related to a no-ccache -> ccache-available situation.

It is very difficult to recover from such things because CCACHE is in
BB_HASHBASE_WHITELIST.



Enrico
Richard Tollerton - Oct. 8, 2013, 5:47 p.m.
richard.purdie@linuxfoundation.org writes:

>> I know that the OE build is horribly slow, but is it really so slow as
>> to require short-circuiting native package dependencies such as this?
>> Has there been a general architectural decision made over this sort of
>> thing? (I'm sorry, I tried to search for one on the mailing list and
>> wiki, but couldn't find any.)
>
> Some native utilities cause big problems in the dependency chain. Things
> like tar and gzip have caused us big problems in the past. We've made a
> conscious decision to require specific versions of tar, gzip and git for
> example to ensure builds work as expected.

I can level with that. However, it doesn't seem obvious that such a
decision was conscientiously made, judging from the code/documentation
(at least to a neophyte like me). Is there a place I've missed where
such technical decisions have been documented? (Or should there be? Or
should I just ask these sorts of questions on IRC?)

> The list of things we assume are ok is relatively small and we look at
> new issues on a case by case basis. I think sed is well enough
> established and well enough behaved to be something we can rely on. In
> general we don't rely on much. Do you have any other specific things you
> worry about? When you start to look at it, the list is rather small...

The biggest thing I can think of is gcc-native. My understanding of gcc
documentation is that it's recommended to build a cross-compiler with a
bootstrap, or at least the same version of compiler, and we're not doing
that. (Right?)

Otherwise I'm mainly asking in order to understand the decisionmaking.

>
> Cheers,
>
> Richard
Richard Purdie - Oct. 10, 2013, 9 a.m.
On Tue, 2013-10-08 at 12:47 -0500, Richard Tollerton wrote:
> richard.purdie@linuxfoundation.org writes:
> 
> >> I know that the OE build is horribly slow, but is it really so slow as
> >> to require short-circuiting native package dependencies such as this?
> >> Has there been a general architectural decision made over this sort of
> >> thing? (I'm sorry, I tried to search for one on the mailing list and
> >> wiki, but couldn't find any.)
> >
> > Some native utilities cause big problems in the dependency chain. Things
> > like tar and gzip have caused us big problems in the past. We've made a
> > conscious decision to require specific versions of tar, gzip and git for
> > example to ensure builds work as expected.
> 
> I can level with that. However, it doesn't seem obvious that such a
> decision was conscientiously made, judging from the code/documentation
> (at least to a neophyte like me). Is there a place I've missed where
> such technical decisions have been documented? (Or should there be? Or
> should I just ask these sorts of questions on IRC?)

There have been mailing list discussions that have happened as issues
have been noticed and patches which documented what was being changed
and why. The "formal" documentation is probably the ASSUME_PROVIDED
variable. The project makes lots of decisions and its hard to document
everything formally, help in doing that would be great but it would be
hard. I guess asking people is the best way of getting information (as
you're doing now) and/or looking at the mailing list archives and the
git history.

For example, searching for "git-native" shows:

http://git.yoctoproject.org/cgit.cgi/poky-contrib/commit/?h=rpurdie/t2&id=617835990edf1d7ca4fc8c08993b1b6725f12290

> > The list of things we assume are ok is relatively small and we look at
> > new issues on a case by case basis. I think sed is well enough
> > established and well enough behaved to be something we can rely on. In
> > general we don't rely on much. Do you have any other specific things you
> > worry about? When you start to look at it, the list is rather small...
> 
> The biggest thing I can think of is gcc-native. My understanding of gcc
> documentation is that it's recommended to build a cross-compiler with a
> bootstrap, or at least the same version of compiler, and we're not doing
> that. (Right?)

We don't build a gcc-native, no. Equally, we've not run in many issues
due to that so its not been something we've thought necessary.

Cheers,

Richard
Phil Blundell - Oct. 10, 2013, 10:29 a.m.
On Thu, 2013-10-10 at 10:00 +0100, Richard Purdie wrote:
> On Tue, 2013-10-08 at 12:47 -0500, Richard Tollerton wrote:
> > The biggest thing I can think of is gcc-native. My understanding of gcc
> > documentation is that it's recommended to build a cross-compiler with a
> > bootstrap, or at least the same version of compiler, and we're not doing
> > that. (Right?)
> 
> We don't build a gcc-native, no. Equally, we've not run in many issues
> due to that so its not been something we've thought necessary.

I think there might be some difficulty with building a Java-enabled
cross compiler if you don't have a matching host copy of ecj.jar on
hand, but we currently force Java off by default anyway and as far as I
know nobody is using it.  If we did want to start supporting Java as a
first class cross compiler target then it might be necessary to build
some host-side gcc bits.

Other than that, you're right, there is no particularly
rational/compelling reason to build a native gcc before compiling the
cross compiler.  In theory it would be a good thing for folks who are
building on systems where the installed native compiler is especially
old or lame (because your cross compiler would run faster if you built a
decent native compiler first and then used that to build the cross one)
but, in practice, it would be impossible to run OE on such a system
anyway without manually upgrading a bunch of tools and anybody who has
gone to that trouble would almost certainly have installed gcc already.

p.

Patch

diff --git a/meta/classes/autotools.bbclass b/meta/classes/autotools.bbclass
index 66c0f5d..2bb2aa3 100644
--- a/meta/classes/autotools.bbclass
+++ b/meta/classes/autotools.bbclass
@@ -7,17 +7,25 @@  def autotools_dep_prepend(d):
 
     if pn in ['autoconf-native', 'automake-native', 'help2man-native']:
         return deps
-    deps += 'autoconf-native automake-native '
+    deps += 'autoconf-native automake-native gnu-config-native '
 
-    if not pn in ['libtool', 'libtool-native'] and not pn.endswith("libtool-cross"):
-        deps += 'libtool-native '
-        if not bb.data.inherits_class('native', d) \
-                        and not bb.data.inherits_class('nativesdk', d) \
-                        and not bb.data.inherits_class('cross', d) \
-                        and not d.getVar('INHIBIT_DEFAULT_DEPS', True):
-            deps += 'libtool-cross '
+    if pn in ['libtool', 'libtool-native'] or pn.endswith("libtool-cross"):
+        return deps
+    deps += 'libtool-native '
+
+    if d.getVar('INHIBIT_DEFAULT_DEPS', True):
+        return deps
+
+    if pn not in ['sed-native']:
+        deps += 'sed-native '
+
+    if bb.data.inherits_class('native', d) \
+       or bb.data.inherits_class('nativesdk', d) \
+       or bb.data.inherits_class('cross', d):
+        return deps
+    deps += 'libtool-cross '
 
-    return deps + 'gnu-config-native '
+    return deps
 
 EXTRA_OEMAKE = ""