Re: [RFC][PATCH 1/4] package.bbclass: move reading shlibs providers to separate function

[Martin Jansa <martin.jansa@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 5543 bytes --]

On Thu, Sep 05, 2013 at 12:40:17PM +0100, Richard Purdie wrote:
> On Wed, 2013-08-28 at 10:33 +0200, Martin Jansa wrote:
> > On Sun, Jul 07, 2013 at 01:13:04AM +0200, Martin Jansa wrote:
> > > * prepare for reading shlibs providers only from dependency tree of
> > >   current recipe
> > > 
> > > [YOCTO #4628]
> > 
> > Any comment on this patchset?
> > 
> > I'm using first 3 commits for some time in world builds and they helped
> > me to discover some unexpected shlib providers (and fix them by setting
> > PRIVATE_LIBS).
> 
> I've run some tests with this and I do like the patches however there
> are some issues. Some are minor and easily fixed, some are more
> problematic but I'll try and list them:
> 
> * Patch 1/4 is missing a () from a funciton. Easily fixed, mentioned 
>   for completeness
> * In 2/4, I locally removed the continue from the "ignoring xxx" loop 
>   since it doesn't make the build any more deterministic, its still a 
>   race over which package builds first but it is a change in 
>   behaviour. We may decide to change behaviour but that should be a 
>   separate patch. Also need to update the message after the change.
> * In 3/4, the regexp is not anchored. Libraries places in subdirs of 
>   libdir should not match this code, neither should things in /foo/lib. 
>   The tweaks below ensure the regexp matches the correct things and 
>   avoids modules in ${libdir}/${PN}/. This is correct but gives another 
>   problem I'll come back to.
> * 4/4 isn't complete. Again I like the idea but we probably need help 
>   from bitbake for it. Its the wrong point in the development cycle to 
>   start thinking about it.
> 
> The problem in 3/4 is clear with something like gstreamer verses
> gstreamer1.0. These install into ${libdir}/PN/ so they are safe well
> behaved recipes but they trigger spurious warnings with the patch :(. We
> can "fix" with:
> 
> diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
> index 569599c..5fc6cda 100644
> --- a/meta/classes/package.bbclass
> +++ b/meta/classes/package.bbclass
> @@ -1337,7 +1337,7 @@ python package_do_shlibs() {
>              dir = dir[1:]
>          if shlibs_search_dirs_re_txt:
>              shlibs_search_dirs_re_txt += "|"
> -        shlibs_search_dirs_re_txt += "(^.*/%s/.*$)" % dir
> +        shlibs_search_dirs_re_txt += "(^/%s/[^/]*$)" % dir
>      shlibs_search_dirs_re = re.compile(shlibs_search_dirs_re_txt)
>      bb.debug(2, "will use following RE to search for provides sonames %s" % shlibs_search_dirs_re_txt)
>  
> @@ -1398,12 +1398,13 @@ python package_do_shlibs() {
>              if m:
>                  this_soname = m.group(1)
>                  if not this_soname in sonames:
> -                    if shlibs_search_dirs_re.match(file):
> +                    targetfile = file.replace(pkgdest + "/" + pkg, "")
> +                    if shlibs_search_dirs_re.match(targetfile):
>                          # if library is private (only used by package) then do not build shlib for it
>                          if not private_libs or -1 == private_libs.find(this_soname):
>                              sonames.append(this_soname)
>                      else:
> -                        bb.debug(2, "ignoring soname %s from %s, because path doesn't match %s" % (this_soname, file, shlibs_search_dirs_re_txt))
> +                        bb.debug(2, "ignoring soname %s from %s, because path doesn't match %s" % (this_soname, targetfile, shlibs_search_dirs_re_txt))
>                  if libdir_re.match(os.path.dirname(file)):
>                      needs_ldconfig = True
>                  if snap_symlinks and (os.path.basename(file) != this_soname):
> 
> So this is nice, however the shlibs code doesn't just handle the libs in
> the default search path. Its also used when say something in
> ${libdir}/${PN} links against something else in that directory. Bitbake
> looks up the soname and then adds in RDEPENDS on the appropriate
> packages. This does assume there is a valid RPATH in the library but
> that is usually the case.
> 
> So by including this code as it stands, we'd drop a number of
> autogenerated RDEPENDS for more unusual libraries in the system outside
> of ${libdir}. We can't do that.
> 
> So what can we do? When we process shlibs, we need to record the paths
> of the things providing given sonames. They're either in the default
> search path, or in specific directories. When we then process another
> library, we can look at the RPATH it uses and then only match things in
> the search paths (default or otherwise).
> 
> Sadly, this is fairly major surgery to the shlibs code and at the
> current point of the release cycle, not something we can start.
> 
> So in summary, I do like the patchset at lot and it is showing up real
> problems (emgd conflicting with mesa is something my builds are
> currently corrupted with) however I don't think the patches are right
> yet and we may need some more involved changes to get them there. I do
> think its worth doing but its probably 1.6 material.

Thanks for review, I'll include your changes in jansa/shlib-providers
branch and when jenkins gets less busy I'll build world with and without
these patches included to compare autogenerated RDEPENDS.

I agree it's too late for 1.5, I'll try to post updated version with
RDEPENDS-diff as soon as 1.6. window opens.

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 205 bytes --]