[PATCH] package_manager: rpm: remove update-alternatives correctly

[PATCH] package_manager: rpm: remove update-alternatives correctly

[Mikko Ylinen]

With "read-only-rootfs" in IMAGE_FEATURES, packages in ROOTFS_RO_UNNEEDED
are removed when building the rootfs.

rootfs.py has a note about some of the assumptions to that removal:

 "Make sure update-alternatives is last on the command line, so
  that it is removed last. This makes sure that its database is
  available while uninstalling packages, allowing alternative
  symlinks of packages to be uninstalled to be managed correctly."

However, it turns out rpm does not care about "last on the command
line" and update-alternatives provider is removed before other the
packages get to run their %preun scripts for update-alternatives.

This leaves broken alternative symlinks in rootfs.

The fix is to remove packages one by one (reversed command line
list did not work either) and process update-alternatives provider
last.

And while we're at it, make logging more verbose to better see
what's happening (and to get it consistent with opkg's remove() logging).

Signed-off-by: Mikko Ylinen <mikko.ylinen@linux.intel.com>
---
 meta/lib/oe/package_manager.py | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/meta/lib/oe/package_manager.py b/meta/lib/oe/package_manager.py
index 994e462..c122902 100644
--- a/meta/lib/oe/package_manager.py
+++ b/meta/lib/oe/package_manager.py
@@ -614,13 +614,16 @@ class RpmPM(PackageManager):
             self._invoke_dnf(["remove"] + pkgs)
         else:
             cmd = bb.utils.which(os.getenv('PATH'), "rpm")
-            args = ["-e", "--nodeps", "--root=%s" %self.target_rootfs]
+            args = ["-e", "-v", "--nodeps", "--root=%s" %self.target_rootfs]
 
-            try:
-                output = subprocess.check_output([cmd] + args + pkgs, stderr=subprocess.STDOUT).decode("utf-8")
-            except subprocess.CalledProcessError as e:
-                bb.fatal("Could not invoke rpm. Command "
-                     "'%s' returned %d:\n%s" % (' '.join([cmd] + args + pkgs), e.returncode, e.output.decode("utf-8")))
+            for pkg in pkgs:
+                try:
+                    bb.note("Running %s" % ' '.join([cmd] + args + [pkg]))
+                    output = subprocess.check_output([cmd] + args + [pkg], stderr=subprocess.STDOUT).decode("utf-8")
+                    bb.note(output)
+                except subprocess.CalledProcessError as e:
+                    bb.fatal("Could not invoke rpm. Command "
+                         "'%s' returned %d:\n%s" % (' '.join([cmd] + args + [pkg]), e.returncode, e.output.decode("utf-8")))
 
     def upgrade(self):
         self._prepare_pkg_transaction()
-- 
2.1.4

Re: [PATCH] package_manager: rpm: remove update-alternatives correctly

[Alexander Kanavin]

On 09/26/2017 03:03 PM, Mikko Ylinen wrote:
> @@ -614,13 +614,16 @@ class RpmPM(PackageManager):
>               self._invoke_dnf(["remove"] + pkgs)
>           else:
>               cmd = bb.utils.which(os.getenv('PATH'), "rpm")
> -            args = ["-e", "--nodeps", "--root=%s" %self.target_rootfs]
> +            args = ["-e", "-v", "--nodeps", "--root=%s" %self.target_rootfs]
>   
> -            try:
> -                output = subprocess.check_output([cmd] + args + pkgs, stderr=subprocess.STDOUT).decode("utf-8")
> -            except subprocess.CalledProcessError as e:
> -                bb.fatal("Could not invoke rpm. Command "
> -                     "'%s' returned %d:\n%s" % (' '.join([cmd] + args + pkgs), e.returncode, e.output.decode("utf-8")))
> +            for pkg in pkgs:
> +                try:
> +                    bb.note("Running %s" % ' '.join([cmd] + args + [pkg]))
> +                    output = subprocess.check_output([cmd] + args + [pkg], stderr=subprocess.STDOUT).decode("utf-8")
> +                    bb.note(output)
> +                except subprocess.CalledProcessError as e:
> +                    bb.fatal("Could not invoke rpm. Command "
> +                         "'%s' returned %d:\n%s" % (' '.join([cmd] + args + [pkg]), e.returncode, e.output.decode("utf-8")))
>   
>       def upgrade(self):
>           self._prepare_pkg_transaction()
> 

I'm worried this might significantly slow down image creation. How about 
fixing this in rootfs.py: first remove everything except 
update-alternatives, then remove that?


Alex

[PATCH v2 0/2] remove update-alternatives correctly

[Mikko Ylinen]

v2: move the fix to rootfs.py and implement 2-stage removal
based on the feedback. split the fix and the logging improvements
in two commits.

Mikko Ylinen (2):
  rootfs.py: remove update-alternatives correctly
  package_manager: rpm: improve logging

 meta/lib/oe/package_manager.py |  4 +++-
 meta/lib/oe/rootfs.py          | 17 ++++++++++++-----
 2 files changed, 15 insertions(+), 6 deletions(-)

-- 
2.1.4

[PATCH 1/2] rootfs.py: remove update-alternatives correctly

[Mikko Ylinen]

With "read-only-rootfs" in IMAGE_FEATURES, packages in ROOTFS_RO_UNNEEDED
are removed when building the rootfs. The list of packages to remove is
passed to the package manager and the list is sorted so that
update-alternatives provider is the last entry. This is with the
assumption that the last entry on the list/command line is removed last.

However, it turns out rpm does not care about "last on the command
line" and update-alternatives provider is removed before other the
packages get to run their %preun scripts for update-alternatives.

This leaves broken alternative symlinks in rootfs.

The fix is to first remove all but update-alternatives provider and
after that update-alternatives provider in its own remove() call.

Signed-off-by: Mikko Ylinen <mikko.ylinen@linux.intel.com>
---
 meta/lib/oe/rootfs.py | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/meta/lib/oe/rootfs.py b/meta/lib/oe/rootfs.py
index 9d4727e..5f62015 100644
--- a/meta/lib/oe/rootfs.py
+++ b/meta/lib/oe/rootfs.py
@@ -261,15 +261,22 @@ class Rootfs(object, metaclass=ABCMeta):
             # Remove components that we don't need if it's a read-only rootfs
             unneeded_pkgs = self.d.getVar("ROOTFS_RO_UNNEEDED").split()
             pkgs_installed = image_list_installed_packages(self.d)
-            # Make sure update-alternatives is last on the command line, so
-            # that it is removed last. This makes sure that its database is
-            # available while uninstalling packages, allowing alternative
-            # symlinks of packages to be uninstalled to be managed correctly.
+            # Make sure update-alternatives is removed last. This is
+            # because its database has to available while uninstalling
+            # other packages, allowing alternative symlinks of packages
+            # to be uninstalled or to be managed correctly otherwise.
             provider = self.d.getVar("VIRTUAL-RUNTIME_update-alternatives")
             pkgs_to_remove = sorted([pkg for pkg in pkgs_installed if pkg in unneeded_pkgs], key=lambda x: x == provider)
 
+            # update-alternatives provider is removed in its own remove()
+            # call because all package managers do not guarantee the packages
+            # are removed in the order they given in the list (which is
+            # passed to the command line). The sorting done earlier is
+            # utilized to implement the 2-stage removal.
+            if len(pkgs_to_remove) > 1:
+                self.pm.remove(pkgs_to_remove[:-1], False)
             if len(pkgs_to_remove) > 0:
-                self.pm.remove(pkgs_to_remove, False)
+                self.pm.remove([pkgs_to_remove[-1]], False)
 
         if delayed_postinsts:
             self._save_postinsts()
-- 
2.1.4

[PATCH 2/2] package_manager: rpm: improve logging

[Mikko Ylinen]

To be able to better debug remove() behaviour, add more logging
to rpm calls via bb.note(). The change also makes remove() logging
more consistent with other package managers' remove() (e.g., opkg).

Signed-off-by: Mikko Ylinen <mikko.ylinen@linux.intel.com>
---
 meta/lib/oe/package_manager.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/meta/lib/oe/package_manager.py b/meta/lib/oe/package_manager.py
index 994e462..eb0824e 100644
--- a/meta/lib/oe/package_manager.py
+++ b/meta/lib/oe/package_manager.py
@@ -614,10 +614,12 @@ class RpmPM(PackageManager):
             self._invoke_dnf(["remove"] + pkgs)
         else:
             cmd = bb.utils.which(os.getenv('PATH'), "rpm")
-            args = ["-e", "--nodeps", "--root=%s" %self.target_rootfs]
+            args = ["-e", "-v", "--nodeps", "--root=%s" %self.target_rootfs]
 
             try:
+                bb.note("Running %s" % ' '.join([cmd] + args + pkgs))
                 output = subprocess.check_output([cmd] + args + pkgs, stderr=subprocess.STDOUT).decode("utf-8")
+                bb.note(output)
             except subprocess.CalledProcessError as e:
                 bb.fatal("Could not invoke rpm. Command "
                      "'%s' returned %d:\n%s" % (' '.join([cmd] + args + pkgs), e.returncode, e.output.decode("utf-8")))
-- 
2.1.4