[PATCH v2] package: Record PE and PR values for shlib dependencies

[PATCH v2] package: Record PE and PR values for shlib dependencies

[Böszörményi Zoltán]

When downgrading a package or using a substitute with lower version,
the way to do it is adding or increasing PE and there may be other
reasons to set PE.

But it doesn't directly help dependant packages because the shlib
records only contain PV.

Let's add the PE value into the shlib records for packages where
it's set.

The in-memory variables storing the versions now use the PE:PV
notation but the on-disk files must use something else because
the : character is already used as field delimiter in the package.list
files storing the shlib records. Use # instead in the files,
so the file format doesn't change. Conversion occurs on reading
and writing the package.list files.

v2:

Also store PR in a new 4th field in the shlib records. If it's set,
i.e. it's different from "r0" then it will also be used in the
packages for shared library dependencies.

Signed-off-by: Zoltán Böszörményi <zboszor@pr.hu>
---
 meta/classes/package.bbclass | 41 ++++++++++++++++++++++++++---------------
 meta/lib/oe/package.py       |  4 +++-
 2 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
index 7dc759699f..95a3be774f 100644
--- a/meta/classes/package.bbclass
+++ b/meta/classes/package.bbclass
@@ -1560,7 +1560,7 @@ python package_do_shlibs() {
     # Take shared lock since we're only reading, not writing
     lf = bb.utils.lockfile(d.expand("${PACKAGELOCK}"))
 
-    def linux_so(file, needed, sonames, renames, pkgver):
+    def linux_so(file, needed, sonames, renames, pkgver, pkgpr):
         needs_ldconfig = False
         ldir = os.path.dirname(file).replace(pkgdest + "/" + pkg, '')
         cmd = d.getVar('OBJDUMP') + " -p " + pipes.quote(file) + " 2>/dev/null"
@@ -1582,7 +1582,7 @@ python package_do_shlibs() {
             m = re.match("\s+SONAME\s+([^\s]*)", l)
             if m:
                 this_soname = m.group(1)
-                prov = (this_soname, ldir, pkgver)
+                prov = (this_soname, ldir, pkgver, pkgpr)
                 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:
@@ -1593,7 +1593,7 @@ python package_do_shlibs() {
                     renames.append((file, os.path.join(os.path.dirname(file), this_soname)))
         return needs_ldconfig
 
-    def darwin_so(file, needed, sonames, renames, pkgver):
+    def darwin_so(file, needed, sonames, renames, pkgver, pkgpr):
         if not os.path.exists(file):
             return
         ldir = os.path.dirname(file).replace(pkgdest + "/" + pkg, '')
@@ -1618,7 +1618,7 @@ python package_do_shlibs() {
             combos = get_combinations(name)
             for combo in combos:
                 if not combo in sonames:
-                    prov = (combo, ldir, pkgver)
+                    prov = (combo, ldir, pkgver, pkgpr)
                     sonames.append(prov)
         if file.endswith('.dylib') or file.endswith('.so'):
             rpath = []
@@ -1645,13 +1645,13 @@ python package_do_shlibs() {
                 if name and name not in needed[pkg]:
                      needed[pkg].append((name, file, []))
 
-    def mingw_dll(file, needed, sonames, renames, pkgver):
+    def mingw_dll(file, needed, sonames, renames, pkgver, pkgpr):
         if not os.path.exists(file):
             return
 
         if file.endswith(".dll"):
             # assume all dlls are shared objects provided by the package
-            sonames.append((os.path.basename(file), os.path.dirname(file).replace(pkgdest + "/" + pkg, ''), pkgver))
+            sonames.append((os.path.basename(file), os.path.dirname(file).replace(pkgdest + "/" + pkg, ''), pkgver, pkgpr))
 
         if (file.endswith(".dll") or file.endswith(".exe")):
             # use objdump to search for "DLL Name: .*\.dll"
@@ -1686,6 +1686,12 @@ python package_do_shlibs() {
         if not pkgver:
             pkgver = ver
 
+        pkgpe = d.getVar('PE')
+        if pkgpe:
+            pkgver = pkgpe + ':' + pkgver
+
+        pkgpr = d.getVar('PR')
+
         needed[pkg] = []
         sonames = list()
         renames = list()
@@ -1694,11 +1700,11 @@ python package_do_shlibs() {
                 if cpath.islink(file):
                     continue
                 if targetos == "darwin" or targetos == "darwin8":
-                    darwin_so(file, needed, sonames, renames, pkgver)
+                    darwin_so(file, needed, sonames, renames, pkgver, pkgpr)
                 elif targetos.startswith("mingw"):
-                    mingw_dll(file, needed, sonames, renames, pkgver)
+                    mingw_dll(file, needed, sonames, renames, pkgver, pkgpr)
                 elif os.access(file, os.X_OK) or lib_re.match(file):
-                    ldconfig = linux_so(file, needed, sonames, renames, pkgver)
+                    ldconfig = linux_so(file, needed, sonames, renames, pkgver, pkgpr)
                     needs_ldconfig = needs_ldconfig or ldconfig
         for (old, new) in renames:
             bb.note("Renaming %s to %s" % (old, new))
@@ -1710,14 +1716,14 @@ python package_do_shlibs() {
             fd = open(shlibs_file, 'w')
             for s in sonames:
                 if s[0] in shlib_provider and s[1] in shlib_provider[s[0]]:
-                    (old_pkg, old_pkgver) = shlib_provider[s[0]][s[1]]
+                    (old_pkg, old_pkgver, old_pkgpr) = shlib_provider[s[0]][s[1]]
                     if old_pkg != pkg:
-                        bb.warn('%s-%s was registered as shlib provider for %s, changing it to %s-%s because it was built later' % (old_pkg, old_pkgver, s[0], pkg, pkgver))
+                        bb.warn('%s-%s-%s was registered as shlib provider for %s, changing it to %s-%s-%s because it was built later' % (old_pkg, old_pkgver, old_pkgpr, s[0], pkg, pkgver, pkgpr))
                 bb.debug(1, 'registering %s-%s as shlib provider for %s' % (pkg, pkgver, s[0]))
-                fd.write(s[0] + ':' + s[1] + ':' + s[2] + '\n')
+                fd.write(s[0] + ':' + s[1] + ':' + s[2].replace(':', '#', 1) + ':' + s[3] + '\n')
                 if s[0] not in shlib_provider:
                     shlib_provider[s[0]] = {}
-                shlib_provider[s[0]][s[1]] = (pkg, pkgver)
+                shlib_provider[s[0]][s[1]] = (pkg, pkgver, pkgpr)
             fd.close()
         if needs_ldconfig and use_ldconfig:
             bb.debug(1, 'adding ldconfig call to postinst for %s' % pkg)
@@ -1769,15 +1775,20 @@ python package_do_shlibs() {
                         match = p
                         break
                 if match:
-                    (dep_pkg, ver_needed) = shlib_provider[n[0]][match]
+                    (dep_pkg, ver_needed, pr_needed) = shlib_provider[n[0]][match]
 
                     bb.debug(2, '%s: Dependency %s requires package %s (used by files: %s)' % (pkg, n[0], dep_pkg, n[1]))
 
                     if dep_pkg == pkg:
                         continue
 
+                    if pr_needed and pr_needed != 'r0':
+                        pr_needed = '-' + pr_needed
+                    else:
+                        pr_needed = ''
+
                     if ver_needed:
-                        dep = "%s (>= %s)" % (dep_pkg, ver_needed)
+                        dep = "%s (>= %s%s)" % (dep_pkg, ver_needed, pr_needed)
                     else:
                         dep = dep_pkg
                     if not dep in deps:
diff --git a/meta/lib/oe/package.py b/meta/lib/oe/package.py
index 1e5c3aa8e1..32d7d24317 100644
--- a/meta/lib/oe/package.py
+++ b/meta/lib/oe/package.py
@@ -256,9 +256,11 @@ def read_shlib_providers(d):
                 fd.close()
                 for l in lines:
                     s = l.strip().split(":")
+                    while len(s) < 4:
+                        s.append('')
                     if s[0] not in shlib_provider:
                         shlib_provider[s[0]] = {}
-                    shlib_provider[s[0]][s[1]] = (dep_pkg, s[2])
+                    shlib_provider[s[0]][s[1]] = (dep_pkg, s[2].replace('#', ':', 1), s[3])
     return shlib_provider
 
 
-- 
2.14.3

Re: [PATCH v2] package: Record PE and PR values for shlib dependencies

[Richard Purdie]

On Fri, 2018-01-12 at 14:46 +0100, Böszörményi Zoltán wrote:
> When downgrading a package or using a substitute with lower version,
> the way to do it is adding or increasing PE and there may be other
> reasons to set PE.
> 
> But it doesn't directly help dependant packages because the shlib
> records only contain PV.
> 
> Let's add the PE value into the shlib records for packages where
> it's set.
> 
> The in-memory variables storing the versions now use the PE:PV
> notation but the on-disk files must use something else because
> the : character is already used as field delimiter in the
> package.list
> files storing the shlib records. Use # instead in the files,
> so the file format doesn't change. Conversion occurs on reading
> and writing the package.list files.

Can you explain a bit more about why/how this causes a problem?

PE is needed to maintain consistency of package feeds but the shlibs
code is primarily used for build time dependency analysis to figure out
which packages need to depend upon what and ensure runtime dependencies
are met. I'm not sure that should care about PE.

For PR, again its primarily for package feeds so that updates are
detected and I'm not sure why the shlibs code should need to care about
it.

Perhaps if you explain more about the issues you're having which this
fixes I'll better understand the problem/need.

Cheers,

Richard

Re: [PATCH v2] package: Record PE and PR values for shlib dependencies

[Böszörményi Zoltán]

2018-01-14 12:27 keltezéssel, Richard Purdie írta:
> On Fri, 2018-01-12 at 14:46 +0100, Böszörményi Zoltán wrote:
>> When downgrading a package or using a substitute with lower version,
>> the way to do it is adding or increasing PE and there may be other
>> reasons to set PE.
>>
>> But it doesn't directly help dependant packages because the shlib
>> records only contain PV.
>>
>> Let's add the PE value into the shlib records for packages where
>> it's set.
>>
>> The in-memory variables storing the versions now use the PE:PV
>> notation but the on-disk files must use something else because
>> the : character is already used as field delimiter in the
>> package.list
>> files storing the shlib records. Use # instead in the files,
>> so the file format doesn't change. Conversion occurs on reading
>> and writing the package.list files.
> 
> Can you explain a bit more about why/how this causes a problem?
> 
> PE is needed to maintain consistency of package feeds but the shlibs
> code is primarily used for build time dependency analysis to figure out
> which packages need to depend upon what and ensure runtime dependencies
> are met. I'm not sure that should care about PE.

See my mail about describing the use case at
http://lists.openembedded.org/pipermail/openembedded-devel/2018-January/116291.html

Practically it can prevent proper dist-ugprade because
library package dependencies can be installed later
than binaries requiring them. If this happens, running
any binary may crash in the pkg_postinst scriptlet.

My use case was the eglibc_linaro-2.19 -> glibc_2.24 transition
but any upgrade where the package already has a PE value set
may be in the same situation. E.g. glib-2.0 1:2.48.2 in Morty vs
1:2.52.3 in Rocko.

A bbappend file can also modify PV. In case PE is missing from
the shlib deps, you can even end up with the same situation
in the same distro with extra layers.

> For PR, again its primarily for package feeds so that updates are
> detected and I'm not sure why the shlibs code should need to care about
> it.

Besides PE, it is also better to always ship libraries and
binaries from the same build, i.e. with the same PV-PR value.

Such an example is e.g. util-linux with util-linux-mount and
libmount1.

The above described situation may occur if a patch is backported
that adds a new library function which is also used at the same
time from binaries in the same patch. All without increasing
the actual package version. This may be a rare case but I am sure
it is not without an example in Yocto. I am not sure if a recipe
review enforces changing PR in this case, though, but it should.

> Perhaps if you explain more about the issues you're having which this
> fixes I'll better understand the problem/need.

I think I just did.

Automatically relying on the full [PE:]PV[-PR] (note the optional
parts which are also done automatically in the patch) adds even
better consistency in the repo and upgrades can be done in the
correct order.

Upgrades should even work if I just cherry-pick some upgrades
but not all, i.e. "opkg upgrade <something-using-glib-2.0>"
should pull the libglib-2.0_0 version it was built with which is
not happening now because the library dependency is only (>= 2.52.3)
in Rocko but 1:2.48.2 from Morty satisfies it.

OPKG 0.3.x has a dist-upgrade mode and Rocko also introduced dnf,
so I assume at some point a proper distro upgrade will get supported.

Best regards,
Zoltán Böszörményi

Re: [PATCH v2] package: Record PE and PR values for shlib dependencies

[Böszörményi Zoltán]

Ping.

Do you have any objection against the idea of this patch
after the described observations?

Thanks in advance,
Zoltán Böszörményi

2018-01-16 11:10 keltezéssel, Böszörményi Zoltán írta:
> 2018-01-14 12:27 keltezéssel, Richard Purdie írta:
>> On Fri, 2018-01-12 at 14:46 +0100, Böszörményi Zoltán wrote:
>>> When downgrading a package or using a substitute with lower version,
>>> the way to do it is adding or increasing PE and there may be other
>>> reasons to set PE.
>>>
>>> But it doesn't directly help dependant packages because the shlib
>>> records only contain PV.
>>>
>>> Let's add the PE value into the shlib records for packages where
>>> it's set.
>>>
>>> The in-memory variables storing the versions now use the PE:PV
>>> notation but the on-disk files must use something else because
>>> the : character is already used as field delimiter in the
>>> package.list
>>> files storing the shlib records. Use # instead in the files,
>>> so the file format doesn't change. Conversion occurs on reading
>>> and writing the package.list files.
>>
>> Can you explain a bit more about why/how this causes a problem?
>>
>> PE is needed to maintain consistency of package feeds but the shlibs
>> code is primarily used for build time dependency analysis to figure out
>> which packages need to depend upon what and ensure runtime dependencies
>> are met. I'm not sure that should care about PE.
> 
> See my mail about describing the use case at
> http://lists.openembedded.org/pipermail/openembedded-devel/2018-January/116291.html
> 
> Practically it can prevent proper dist-ugprade because
> library package dependencies can be installed later
> than binaries requiring them. If this happens, running
> any binary may crash in the pkg_postinst scriptlet.
> 
> My use case was the eglibc_linaro-2.19 -> glibc_2.24 transition
> but any upgrade where the package already has a PE value set
> may be in the same situation. E.g. glib-2.0 1:2.48.2 in Morty vs
> 1:2.52.3 in Rocko.
> 
> A bbappend file can also modify PV. In case PE is missing from
> the shlib deps, you can even end up with the same situation
> in the same distro with extra layers.
> 
>> For PR, again its primarily for package feeds so that updates are
>> detected and I'm not sure why the shlibs code should need to care about
>> it.
> 
> Besides PE, it is also better to always ship libraries and
> binaries from the same build, i.e. with the same PV-PR value.
> 
> Such an example is e.g. util-linux with util-linux-mount and
> libmount1.
> 
> The above described situation may occur if a patch is backported
> that adds a new library function which is also used at the same
> time from binaries in the same patch. All without increasing
> the actual package version. This may be a rare case but I am sure
> it is not without an example in Yocto. I am not sure if a recipe
> review enforces changing PR in this case, though, but it should.
> 
>> Perhaps if you explain more about the issues you're having which this
>> fixes I'll better understand the problem/need.
> 
> I think I just did.
> 
> Automatically relying on the full [PE:]PV[-PR] (note the optional
> parts which are also done automatically in the patch) adds even
> better consistency in the repo and upgrades can be done in the
> correct order.
> 
> Upgrades should even work if I just cherry-pick some upgrades
> but not all, i.e. "opkg upgrade <something-using-glib-2.0>"
> should pull the libglib-2.0_0 version it was built with which is
> not happening now because the library dependency is only (>= 2.52.3)
> in Rocko but 1:2.48.2 from Morty satisfies it.
> 
> OPKG 0.3.x has a dist-upgrade mode and Rocko also introduced dnf,
> so I assume at some point a proper distro upgrade will get supported.
> 
> Best regards,
> Zoltán Böszörményi