[1/4] package_manager: Change complementary package handling to not include soft dependencies

Message ID 20220626094506.352336-1-richard.purdie@linuxfoundation.org
State Accepted, archived
Commit b44b0b9294675f89aa51ff84f532664f4c479677
Headers show
Series [1/4] package_manager: Change complementary package handling to not include soft dependencies | expand

Commit Message

Richard Purdie June 26, 2022, 9:45 a.m. UTC
From: Ross Burton <ross.burton@arm.com>

We've some long standing bugs where the RDEPENDS from -dev packages causes
problems, e.g. dropbear and openssh components on an image working fine together
but then the SDK failing to build as the main openssh and dropbear packages
conflict with each other (pulled in by openssh-dev and dropbear-dev).

We propose changing the behavour of complementary package installation to
ignore RRECOMMENDS. If we then change the ${PN}-dev dependency on ${PN}
to a RRECOMMENDS, we can avoid many of the issues people run into yet still
have the desired behaviour of ${PN}-dev pulling in ${PN}.

This therefore changes the package manager code so that it doesn't follow
RRECOMMENDS for completementary package globs.

[RP: Added deb support]
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 meta/lib/oe/package_manager/__init__.py     |  4 ++--
 meta/lib/oe/package_manager/deb/__init__.py | 10 +++++++---
 meta/lib/oe/package_manager/ipk/__init__.py |  4 +++-
 meta/lib/oe/package_manager/rpm/__init__.py |  4 ++--
 4 files changed, 14 insertions(+), 8 deletions(-)

Comments

Ross Burton June 27, 2022, 4:12 p.m. UTC | #1
Whilst I agree with this series in general (obviously…), some commentary.

First, the impact of this can’t be understated.  If you enable dev-pkgs on core-image-base, and extra 424 packages are installed *ignoring any package with -dev in its name*. This includes behaviour-changing packages such as the full-fat versions of coreutils, util-linux, findutils, most of Python, a whole slew of libraries.  That is *not* good and this is a step in the right direction.

There are still some edge cases though, repeating the same test with core-image-base installs these extra non-dev packages:

alsa-topology-conf
libatopology2
libavahi-client3
libavahi-glib1
libavahi-gobject0
libc-malloc-debug0
libc6-extra-nss
libc6-thread-db
libc6-utils
libfdisk1
libform5
libformw5
libgcc1
libmenu5
libmenuw5
libncurses5
libncursesw5
libnl-3-cli
libnl-idiag-3-200
libnl-nf-3-200
libnl-xfrm-3-200
libnss-db2
libpanel5
libpanelw5
libpcrecpp0
libpcreposix0
libsmartcols1
libss2
libstdc++6
libtic5
libticw5
libx11-xcb1
libxcb-composite0
libxcb-damage0
libxcb-dpms0
libxcb-dri2-0
libxcb-dri3-0
libxcb-glx0
libxcb-present0
libxcb-randr0
libxcb-record0
libxcb-render0
libxcb-res0
libxcb-screensaver0
libxcb-shape0
libxcb-shm0
libxcb-sync1
libxcb-xf86dri0
libxcb-xfixes0
libxcb-xinerama0
libxcb-xinput0
libxcb-xkb1
libxcb-xtest0
libxcb-xv0
libxcb-xvmc0

The bulk of these are likely from the -dev package depending on the real package to satisfy dangling symlinks.  This may actually be desirable behaviour?  Not sure I’d want a broken -dev package.

Ross



> On 26 Jun 2022, at 10:45, Richard Purdie <richard.purdie@linuxfoundation.org> wrote:
>
> From: Ross Burton <ross.burton@arm.com>
>
> We've some long standing bugs where the RDEPENDS from -dev packages causes
> problems, e.g. dropbear and openssh components on an image working fine together
> but then the SDK failing to build as the main openssh and dropbear packages
> conflict with each other (pulled in by openssh-dev and dropbear-dev).
>
> We propose changing the behavour of complementary package installation to
> ignore RRECOMMENDS. If we then change the ${PN}-dev dependency on ${PN}
> to a RRECOMMENDS, we can avoid many of the issues people run into yet still
> have the desired behaviour of ${PN}-dev pulling in ${PN}.
>
> This therefore changes the package manager code so that it doesn't follow
> RRECOMMENDS for completementary package globs.
>
> [RP: Added deb support]
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
> meta/lib/oe/package_manager/__init__.py     |  4 ++--
> meta/lib/oe/package_manager/deb/__init__.py | 10 +++++++---
> meta/lib/oe/package_manager/ipk/__init__.py |  4 +++-
> meta/lib/oe/package_manager/rpm/__init__.py |  4 ++--
> 4 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/meta/lib/oe/package_manager/__init__.py b/meta/lib/oe/package_manager/__init__.py
> index 80bc1a6bc6a..d3b45705ec4 100644
> --- a/meta/lib/oe/package_manager/__init__.py
> +++ b/meta/lib/oe/package_manager/__init__.py
> @@ -266,7 +266,7 @@ class PackageManager(object, metaclass=ABCMeta):
>         pass
>
>     @abstractmethod
> -    def install(self, pkgs, attempt_only=False):
> +    def install(self, pkgs, attempt_only=False, hard_depends_only=False):
>         """
>         Install a list of packages. 'pkgs' is a list object. If 'attempt_only' is
>         True, installation failures are ignored.
> @@ -396,7 +396,7 @@ class PackageManager(object, metaclass=ABCMeta):
>                 bb.note("Installing complementary packages ... %s (skipped already provided packages %s)" % (
>                     ' '.join(install_pkgs),
>                     ' '.join(skip_pkgs)))
> -                self.install(install_pkgs)
> +                self.install(install_pkgs, hard_depends_only=True)
>             except subprocess.CalledProcessError as e:
>                 bb.fatal("Could not compute complementary packages list. Command "
>                          "'%s' returned %d:\n%s" %
> diff --git a/meta/lib/oe/package_manager/deb/__init__.py b/meta/lib/oe/package_manager/deb/__init__.py
> index 86ddb130adf..b96ea0bad46 100644
> --- a/meta/lib/oe/package_manager/deb/__init__.py
> +++ b/meta/lib/oe/package_manager/deb/__init__.py
> @@ -289,14 +289,18 @@ class DpkgPM(OpkgDpkgPM):
>
>         self.deploy_dir_unlock()
>
> -    def install(self, pkgs, attempt_only=False):
> +    def install(self, pkgs, attempt_only=False, hard_depends_only=False):
>         if attempt_only and len(pkgs) == 0:
>             return
>
>         os.environ['APT_CONFIG'] = self.apt_conf_file
>
> -        cmd = "%s %s install --allow-downgrades --allow-remove-essential --allow-change-held-packages --allow-unauthenticated --no-remove %s" % \
> -              (self.apt_get_cmd, self.apt_args, ' '.join(pkgs))
> +        extra_args = ""
> +        if hard_depends_only:
> +            extra_args = "--no-install-recommends"
> +
> +        cmd = "%s %s install --allow-downgrades --allow-remove-essential --allow-change-held-packages --allow-unauthenticated --no-remove %s %s" % \
> +              (self.apt_get_cmd, self.apt_args, extra_args, ' '.join(pkgs))
>
>         try:
>             bb.note("Installing the following packages: %s" % ' '.join(pkgs))
> diff --git a/meta/lib/oe/package_manager/ipk/__init__.py b/meta/lib/oe/package_manager/ipk/__init__.py
> index 4cd3963111c..6fd2f021b68 100644
> --- a/meta/lib/oe/package_manager/ipk/__init__.py
> +++ b/meta/lib/oe/package_manager/ipk/__init__.py
> @@ -337,7 +337,7 @@ class OpkgPM(OpkgDpkgPM):
>
>         self.deploy_dir_unlock()
>
> -    def install(self, pkgs, attempt_only=False):
> +    def install(self, pkgs, attempt_only=False, hard_depends_only=False):
>         if not pkgs:
>             return
>
> @@ -346,6 +346,8 @@ class OpkgPM(OpkgDpkgPM):
>             cmd += " --add-exclude %s" % exclude
>         for bad_recommendation in (self.d.getVar("BAD_RECOMMENDATIONS") or "").split():
>             cmd += " --add-ignore-recommends %s" % bad_recommendation
> +        if hard_depends_only:
> +            cmd += " --no-install-recommends"
>         cmd += " install "
>         cmd += " ".join(pkgs)
>
> diff --git a/meta/lib/oe/package_manager/rpm/__init__.py b/meta/lib/oe/package_manager/rpm/__init__.py
> index b392581069c..d97dab32938 100644
> --- a/meta/lib/oe/package_manager/rpm/__init__.py
> +++ b/meta/lib/oe/package_manager/rpm/__init__.py
> @@ -181,7 +181,7 @@ class RpmPM(PackageManager):
>         os.environ['NATIVE_ROOT'] = self.d.getVar('STAGING_DIR_NATIVE')
>
>
> -    def install(self, pkgs, attempt_only = False):
> +    def install(self, pkgs, attempt_only=False, hard_depends_only=False):
>         if len(pkgs) == 0:
>             return
>         self._prepare_pkg_transaction()
> @@ -192,7 +192,7 @@ class RpmPM(PackageManager):
>
>         output = self._invoke_dnf((["--skip-broken"] if attempt_only else []) +
>                          (["-x", ",".join(exclude_pkgs)] if len(exclude_pkgs) > 0 else []) +
> -                         (["--setopt=install_weak_deps=False"] if self.d.getVar('NO_RECOMMENDATIONS') == "1" else []) +
> +                         (["--setopt=install_weak_deps=False"] if (hard_depends_only or self.d.getVar('NO_RECOMMENDATIONS') == "1") else []) +
>                          (["--nogpgcheck"] if self.d.getVar('RPM_SIGN_PACKAGES') != '1' else ["--setopt=gpgcheck=True"]) +
>                          ["install"] +
>                          pkgs)
> --
> 2.34.1
>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Richard Purdie June 27, 2022, 4:51 p.m. UTC | #2
On Mon, 2022-06-27 at 16:12 +0000, Ross Burton wrote:
> Whilst I agree with this series in general (obviously…), some
> commentary.
> 
> First, the impact of this can’t be understated.  If you enable dev-
> pkgs on core-image-base, and extra 424 packages are installed
> *ignoring any package with -dev in its name*. This includes
> behaviour-changing packages such as the full-fat versions of
> coreutils, util-linux, findutils, most of Python, a whole slew of
> libraries.  That is *not* good and this is a step in the right
> direction.
> 
> There are still some edge cases though, repeating the same test with
> core-image-base installs these extra non-dev packages:
> 
> alsa-topology-conf
> libatopology2
> libavahi-client3
> libavahi-glib1
> libavahi-gobject0
> libc-malloc-debug0
> libc6-extra-nss
> libc6-thread-db
> libc6-utils
> libfdisk1
> libform5
> libformw5
> libgcc1
> libmenu5
> libmenuw5
> libncurses5
> libncursesw5
> libnl-3-cli
> libnl-idiag-3-200
> libnl-nf-3-200
> libnl-xfrm-3-200
> libnss-db2
> libpanel5
> libpanelw5
> libpcrecpp0
> libpcreposix0
> libsmartcols1
> libss2
> libstdc++6
> libtic5
> libticw5
> libx11-xcb1
> libxcb-composite0
> libxcb-damage0
> libxcb-dpms0
> libxcb-dri2-0
> libxcb-dri3-0
> libxcb-glx0
> libxcb-present0
> libxcb-randr0
> libxcb-record0
> libxcb-render0
> libxcb-res0
> libxcb-screensaver0
> libxcb-shape0
> libxcb-shm0
> libxcb-sync1
> libxcb-xf86dri0
> libxcb-xfixes0
> libxcb-xinerama0
> libxcb-xinput0
> libxcb-xkb1
> libxcb-xtest0
> libxcb-xv0
> libxcb-xvmc0
> 
> The bulk of these are likely from the -dev package depending on the
> real package to satisfy dangling symlinks.  This may actually be
> desirable behaviour?  Not sure I’d want a broken -dev package.


I did a test build and changing package_fixsymlinks() in
package.bbclaass to use RRECOMMENDS instead of RDEPENDS does remove the
packages listed above too.

I'm torn on that, making that change would complete the picture and
you'd still have the dependency there, just as a slightly weaker level
and means dev-pkgs no longer radically changes the contents of the
image which is probably a good thing?

Cheers,

Richard

Patch

diff --git a/meta/lib/oe/package_manager/__init__.py b/meta/lib/oe/package_manager/__init__.py
index 80bc1a6bc6a..d3b45705ec4 100644
--- a/meta/lib/oe/package_manager/__init__.py
+++ b/meta/lib/oe/package_manager/__init__.py
@@ -266,7 +266,7 @@  class PackageManager(object, metaclass=ABCMeta):
         pass
 
     @abstractmethod
-    def install(self, pkgs, attempt_only=False):
+    def install(self, pkgs, attempt_only=False, hard_depends_only=False):
         """
         Install a list of packages. 'pkgs' is a list object. If 'attempt_only' is
         True, installation failures are ignored.
@@ -396,7 +396,7 @@  class PackageManager(object, metaclass=ABCMeta):
                 bb.note("Installing complementary packages ... %s (skipped already provided packages %s)" % (
                     ' '.join(install_pkgs),
                     ' '.join(skip_pkgs)))
-                self.install(install_pkgs)
+                self.install(install_pkgs, hard_depends_only=True)
             except subprocess.CalledProcessError as e:
                 bb.fatal("Could not compute complementary packages list. Command "
                          "'%s' returned %d:\n%s" %
diff --git a/meta/lib/oe/package_manager/deb/__init__.py b/meta/lib/oe/package_manager/deb/__init__.py
index 86ddb130adf..b96ea0bad46 100644
--- a/meta/lib/oe/package_manager/deb/__init__.py
+++ b/meta/lib/oe/package_manager/deb/__init__.py
@@ -289,14 +289,18 @@  class DpkgPM(OpkgDpkgPM):
 
         self.deploy_dir_unlock()
 
-    def install(self, pkgs, attempt_only=False):
+    def install(self, pkgs, attempt_only=False, hard_depends_only=False):
         if attempt_only and len(pkgs) == 0:
             return
 
         os.environ['APT_CONFIG'] = self.apt_conf_file
 
-        cmd = "%s %s install --allow-downgrades --allow-remove-essential --allow-change-held-packages --allow-unauthenticated --no-remove %s" % \
-              (self.apt_get_cmd, self.apt_args, ' '.join(pkgs))
+        extra_args = ""
+        if hard_depends_only:
+            extra_args = "--no-install-recommends"
+
+        cmd = "%s %s install --allow-downgrades --allow-remove-essential --allow-change-held-packages --allow-unauthenticated --no-remove %s %s" % \
+              (self.apt_get_cmd, self.apt_args, extra_args, ' '.join(pkgs))
 
         try:
             bb.note("Installing the following packages: %s" % ' '.join(pkgs))
diff --git a/meta/lib/oe/package_manager/ipk/__init__.py b/meta/lib/oe/package_manager/ipk/__init__.py
index 4cd3963111c..6fd2f021b68 100644
--- a/meta/lib/oe/package_manager/ipk/__init__.py
+++ b/meta/lib/oe/package_manager/ipk/__init__.py
@@ -337,7 +337,7 @@  class OpkgPM(OpkgDpkgPM):
 
         self.deploy_dir_unlock()
 
-    def install(self, pkgs, attempt_only=False):
+    def install(self, pkgs, attempt_only=False, hard_depends_only=False):
         if not pkgs:
             return
 
@@ -346,6 +346,8 @@  class OpkgPM(OpkgDpkgPM):
             cmd += " --add-exclude %s" % exclude
         for bad_recommendation in (self.d.getVar("BAD_RECOMMENDATIONS") or "").split():
             cmd += " --add-ignore-recommends %s" % bad_recommendation
+        if hard_depends_only:
+            cmd += " --no-install-recommends"
         cmd += " install "
         cmd += " ".join(pkgs)
 
diff --git a/meta/lib/oe/package_manager/rpm/__init__.py b/meta/lib/oe/package_manager/rpm/__init__.py
index b392581069c..d97dab32938 100644
--- a/meta/lib/oe/package_manager/rpm/__init__.py
+++ b/meta/lib/oe/package_manager/rpm/__init__.py
@@ -181,7 +181,7 @@  class RpmPM(PackageManager):
         os.environ['NATIVE_ROOT'] = self.d.getVar('STAGING_DIR_NATIVE')
 
 
-    def install(self, pkgs, attempt_only = False):
+    def install(self, pkgs, attempt_only=False, hard_depends_only=False):
         if len(pkgs) == 0:
             return
         self._prepare_pkg_transaction()
@@ -192,7 +192,7 @@  class RpmPM(PackageManager):
 
         output = self._invoke_dnf((["--skip-broken"] if attempt_only else []) +
                          (["-x", ",".join(exclude_pkgs)] if len(exclude_pkgs) > 0 else []) +
-                         (["--setopt=install_weak_deps=False"] if self.d.getVar('NO_RECOMMENDATIONS') == "1" else []) +
+                         (["--setopt=install_weak_deps=False"] if (hard_depends_only or self.d.getVar('NO_RECOMMENDATIONS') == "1") else []) +
                          (["--nogpgcheck"] if self.d.getVar('RPM_SIGN_PACKAGES') != '1' else ["--setopt=gpgcheck=True"]) +
                          ["install"] +
                          pkgs)