[PATCH] package.bbclass: allow shell-style wildcards in PRIVATE_LIBS

[PATCH] package.bbclass: allow shell-style wildcards in PRIVATE_LIBS

[Alexander Kanavin]

PRIVATE_LIBS is used to exclude 'private' libraries from getting added to
automatic runtime dependency resolution. This variable currently has to list
all libraries by name, which becomes a maintenance issue if the list
of such libraries frequently changes, or is very large.

This change allows using shell-style wildcards in the variable, similar
to how FILES lists what gets packaged.

Signed-off-by: Alexander Kanavin <alex.kanavin@gmail.com>
---
 meta/classes/package.bbclass | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
index 114d6559f5e..aa8451ffe8b 100644
--- a/meta/classes/package.bbclass
+++ b/meta/classes/package.bbclass
@@ -1646,7 +1646,8 @@ python package_do_shlibs() {
                 prov = (this_soname, ldir, pkgver)
                 if not prov in sonames:
                     # if library is private (only used by package) then do not build shlib for it
-                    if not private_libs or this_soname not in private_libs:
+                    import fnmatch
+                    if not private_libs or len([i for i in private_libs if fnmatch.fnmatch(this_soname, i)]) == 0:
                         sonames.add(prov)
                 if libdir_re.match(os.path.dirname(file)):
                     needs_ldconfig = True
@@ -1829,7 +1830,8 @@ python package_do_shlibs() {
             # /opt/abc/lib/libfoo.so.1 and contains /usr/bin/abc depending on system library libfoo.so.1
             # but skipping it is still better alternative than providing own
             # version and then adding runtime dependency for the same system library
-            if private_libs and n[0] in private_libs:
+            import fnmatch
+            if private_libs and len([i for i in private_libs if fnmatch.fnmatch(n[0], i)]) > 0:
                 bb.debug(2, '%s: Dependency %s covered by PRIVATE_LIBS' % (pkg, n[0]))
                 continue
             if n[0] in shlib_provider.keys():
-- 
2.17.1

Re: [PATCH] package.bbclass: allow shell-style wildcards in PRIVATE_LIBS

[Quentin Schulz]

Hi Alexander,

On Tue, Sep 03, 2019 at 06:32:41PM +0200, Alexander Kanavin wrote:
> PRIVATE_LIBS is used to exclude 'private' libraries from getting added to
> automatic runtime dependency resolution. This variable currently has to list
> all libraries by name, which becomes a maintenance issue if the list
> of such libraries frequently changes, or is very large.
> 
> This change allows using shell-style wildcards in the variable, similar
> to how FILES lists what gets packaged.
> 
> Signed-off-by: Alexander Kanavin <alex.kanavin@gmail.com>
> ---
>  meta/classes/package.bbclass | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
> index 114d6559f5e..aa8451ffe8b 100644
> --- a/meta/classes/package.bbclass
> +++ b/meta/classes/package.bbclass
> @@ -1646,7 +1646,8 @@ python package_do_shlibs() {
>                  prov = (this_soname, ldir, pkgver)
>                  if not prov in sonames:
>                      # if library is private (only used by package) then do not build shlib for it
> -                    if not private_libs or this_soname not in private_libs:
> +                    import fnmatch
> +                    if not private_libs or len([i for i in private_libs if fnmatch.fnmatch(this_soname, i)]) == 0:

We only need to know if this_soname is matching one of the patterns, we
don't need to check for each and every pattern.

Something like:

for pattern in private_libs:
	if fnmatch.fnmatch(this_soname, pattern)]) == 0:
		sonames.add(prov)
		break

would be possible?

AFAICT, private_libs is either the content of PRIVATE_LIBS_PN or
PRIVATE_LIBS or "" then passed to .split(). This should be safe for the
forloop without a check.

>                          sonames.add(prov)
>                  if libdir_re.match(os.path.dirname(file)):
>                      needs_ldconfig = True
> @@ -1829,7 +1830,8 @@ python package_do_shlibs() {
>              # /opt/abc/lib/libfoo.so.1 and contains /usr/bin/abc depending on system library libfoo.so.1
>              # but skipping it is still better alternative than providing own
>              # version and then adding runtime dependency for the same system library
> -            if private_libs and n[0] in private_libs:
> +            import fnmatch
> +            if private_libs and len([i for i in private_libs if fnmatch.fnmatch(n[0], i)]) > 0:

Same here?

Just a suggestion :)

BR,
Quentin
-- 
StreamUnlimited Engineering GmbH
High Tech Campus Vienna, Gutheil-Schoder-Gasse 10, 1100 Vienna, Austria
quentin.schulz@streamunlimited.com, www.streamunlimited.com

Re: [PATCH] package.bbclass: allow shell-style wildcards in PRIVATE_LIBS

[Alexander Kanavin]

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

On Wed, 4 Sep 2019 at 09:32, Quentin Schulz <
quentin.schulz@streamunlimited.com> wrote:

> >                      # if library is private (only used by package) then
> do not build shlib for it
> > -                    if not private_libs or this_soname not in
> private_libs:
> > +                    import fnmatch
> > +                    if not private_libs or len([i for i in private_libs
> if fnmatch.fnmatch(this_soname, i)]) == 0:
>
> We only need to know if this_soname is matching one of the patterns, we
> don't need to check for each and every pattern.
>
> Something like:
>
> for pattern in private_libs:
>         if fnmatch.fnmatch(this_soname, pattern)]) == 0:
>                 sonames.add(prov)
>                 break
>
> would be possible?
>

Yes, although there should only be very few patterns, and most of the time
just one: *.so*. Not worth it I'd say. There are other spots in do_package
which truly slow down things and could use a bit of optimization.

Alex

[-- Attachment #2: Type: text/html, Size: 1466 bytes --]

Re: [PATCH] package.bbclass: allow shell-style wildcards in PRIVATE_LIBS

[Alexander Kanavin]

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

On Wed, 4 Sep 2019 at 09:32, Quentin Schulz <
quentin.schulz@streamunlimited.com> wrote:

> > +++ b/meta/classes/package.bbclass
> > @@ -1646,7 +1646,8 @@ python package_do_shlibs() {
> >                  prov = (this_soname, ldir, pkgver)
> >                  if not prov in sonames:
> >                      # if library is private (only used by package) then
> do not build shlib for it
> > -                    if not private_libs or this_soname not in
> private_libs:
> > +                    import fnmatch
> > +                    if not private_libs or len([i for i in private_libs
> if fnmatch.fnmatch(this_soname, i)]) == 0:
>
> We only need to know if this_soname is matching one of the patterns, we
> don't need to check for each and every pattern.
>
> Something like:
>
> for pattern in private_libs:
>         if fnmatch.fnmatch(this_soname, pattern)]) == 0:
>                 sonames.add(prov)
>                 break
>
> would be possible?
>

Actually, this suggested code snippet looks altogether wrong to me. We do
need to check the file against every pattern, and only if it matches none
of them, it gets added to sonames. Note the original code says 'this_soname
not in private_libs', which also means going over every item in the list.

Alex

[-- Attachment #2: Type: text/html, Size: 1800 bytes --]

Re: [PATCH] package.bbclass: allow shell-style wildcards in PRIVATE_LIBS

[Quentin Schulz]

Hi Alex,

On Wed, Sep 04, 2019 at 12:11:29PM +0200, Alexander Kanavin wrote:
> On Wed, 4 Sep 2019 at 09:32, Quentin Schulz <
> quentin.schulz@streamunlimited.com> wrote:
> 
> > > +++ b/meta/classes/package.bbclass
> > > @@ -1646,7 +1646,8 @@ python package_do_shlibs() {
> > >                  prov = (this_soname, ldir, pkgver)
> > >                  if not prov in sonames:
> > >                      # if library is private (only used by package) then
> > do not build shlib for it
> > > -                    if not private_libs or this_soname not in
> > private_libs:
> > > +                    import fnmatch
> > > +                    if not private_libs or len([i for i in private_libs
> > if fnmatch.fnmatch(this_soname, i)]) == 0:
> >
> > We only need to know if this_soname is matching one of the patterns, we
> > don't need to check for each and every pattern.
> >
> > Something like:
> >
> > for pattern in private_libs:
> >         if fnmatch.fnmatch(this_soname, pattern)]) == 0:
> >                 sonames.add(prov)
> >                 break
> >
> > would be possible?
> >
> 
> Actually, this suggested code snippet looks altogether wrong to me. We do

Indeed.

> need to check the file against every pattern, and only if it matches none
> of them, it gets added to sonames. Note the original code says 'this_soname

So we just need to invert the logic actually.

matched = False
for pattern in private_libs:
	if fnmatch.fnmatch(this_soname, pattern)]) == 0:
		matched = True
		break

if not matched:
	sonames.add(prov)

> not in private_libs', which also means going over every item in the list.
> 

I'd hope Python devs did optimization on this one and stopped at the
first match in the list, but I'm not one of them so...

BR,
Quentin
-- 
StreamUnlimited Engineering GmbH
High Tech Campus Vienna, Gutheil-Schoder-Gasse 10, 1100 Vienna, Austria
quentin.schulz@streamunlimited.com, www.streamunlimited.com

Re: [PATCH] package.bbclass: allow shell-style wildcards in PRIVATE_LIBS

[Alexander Kanavin]

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

On Wed, 4 Sep 2019 at 14:55, Quentin Schulz <
quentin.schulz@streamunlimited.com> wrote:

> So we just need to invert the logic actually.
>
> matched = False
> for pattern in private_libs:
>         if fnmatch.fnmatch(this_soname, pattern)]) == 0:
>                 matched = True
>                 break
>
> if not matched:
>         sonames.add(prov)
>

I think you are looking for the "for...else" feature in Python :) Still,
the original code is not adding any lines at all while yours adds eight so
I'd just keep the patch as it is.

Alex

[-- Attachment #2: Type: text/html, Size: 938 bytes --]

Re: [PATCH] package.bbclass: allow shell-style wildcards in PRIVATE_LIBS

[Quentin Schulz]

Hi Alex,

On Wed, Sep 04, 2019 at 04:52:10PM +0200, Alexander Kanavin wrote:
> On Wed, 4 Sep 2019 at 14:55, Quentin Schulz <
> quentin.schulz@streamunlimited.com> wrote:
> 
> > So we just need to invert the logic actually.
> >
> > matched = False
> > for pattern in private_libs:
> >         if fnmatch.fnmatch(this_soname, pattern)]) == 0:
> >                 matched = True
> >                 break
> >
> > if not matched:
> >         sonames.add(prov)
> >
> 
> I think you are looking for the "for...else" feature in Python :) Still,

I'm discovering this one, thanks!

> the original code is not adding any lines at all while yours adds eight so
> I'd just keep the patch as it is.
> 

Added lines vs added execution time.

Anyway, glad to have this feature one day in Yocto, I was about to use
it in some project and was upset for having to name each and every lib
in the package.

Thanks!
Quentin
-- 
StreamUnlimited Engineering GmbH
High Tech Campus Vienna, Gutheil-Schoder-Gasse 10, 1100 Vienna, Austria
quentin.schulz@streamunlimited.com, www.streamunlimited.com

Re: [PATCH] package.bbclass: allow shell-style wildcards in PRIVATE_LIBS

[Alexander Kanavin]

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

On Wed, 4 Sep 2019 at 16:57, Quentin Schulz <
quentin.schulz@streamunlimited.com> wrote:

> > the original code is not adding any lines at all while yours adds eight
> so
> > I'd just keep the patch as it is.
> >
>
> Added lines vs added execution time.
>

I am fairly certain do_package spends the bulk of its time elsewhere, so
you are really optimizing the wrong spot here :)

Alex

[-- Attachment #2: Type: text/html, Size: 733 bytes --]

[PATCH] package.bbclass: allow shell-style wildcards in PRIVATE_LIBS

[Alexander Kanavin]

PRIVATE_LIBS is used to exclude 'private' libraries from getting added to
automatic runtime dependency resolution. This variable currently has to list
all libraries by name, which becomes a maintenance issue if the list
of such libraries frequently changes, or is very large.

This change allows using shell-style wildcards in the variable, similar
to how FILES lists what gets packaged.

Signed-off-by: Alexander Kanavin <alex.kanavin@gmail.com>
---
 meta/classes/package.bbclass | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
index 114d6559f5e..aa8451ffe8b 100644
--- a/meta/classes/package.bbclass
+++ b/meta/classes/package.bbclass
@@ -1646,7 +1646,8 @@ python package_do_shlibs() {
                 prov = (this_soname, ldir, pkgver)
                 if not prov in sonames:
                     # if library is private (only used by package) then do not build shlib for it
-                    if not private_libs or this_soname not in private_libs:
+                    import fnmatch
+                    if not private_libs or len([i for i in private_libs if fnmatch.fnmatch(this_soname, i)]) == 0:
                         sonames.add(prov)
                 if libdir_re.match(os.path.dirname(file)):
                     needs_ldconfig = True
@@ -1829,7 +1830,8 @@ python package_do_shlibs() {
             # /opt/abc/lib/libfoo.so.1 and contains /usr/bin/abc depending on system library libfoo.so.1
             # but skipping it is still better alternative than providing own
             # version and then adding runtime dependency for the same system library
-            if private_libs and n[0] in private_libs:
+            import fnmatch
+            if private_libs and len([i for i in private_libs if fnmatch.fnmatch(n[0], i)]) > 0:
                 bb.debug(2, '%s: Dependency %s covered by PRIVATE_LIBS' % (pkg, n[0]))
                 continue
             if n[0] in shlib_provider.keys():
-- 
2.17.1