[stable-2.16] dnf fixes (#83084)

* dnf: fix installing a package based the file it provides (#82744) Fixes #82461 (cherry picked from commit a28709f92d) * dnf: utilize the API for the installed checks (#82725) Fixes #71808 Fixes #76463 Fixes #81018 (cherry picked from commit f1ded0f417) * setup_rpm_repo/create_repo: "Arch dependent binaries in noarch package" (#83108) This fixes "Arch dependent binaries in noarch package" error cause by including files created by make_elf function in noarch packages. While the error only manifests itself on EL 7 and 8 it is better to use files suitable for noarch packages to prevent the error potentially re-occuring in the future. (cherry picked from commit 87bead3dcf)
1 month ago · 28092180b0
parent 1f4eb2160b
commit 28092180b0
6 changed files with 168 additions and 171 deletions
--- a/changelogs/fragments/82461-dnf-provides.yml
+++ b/changelogs/fragments/82461-dnf-provides.yml
@ -0,0 +1,2 @@
+bugfixes:
+  - dnf - fix an issue when installing a package by specifying a file it provides could result in installing a different package providing the same file than the package already installed resulting in resolution failure (https://github.com/ansible/ansible/issues/82461)
--- a/changelogs/fragments/dnf-installed-checks-api.yml
+++ b/changelogs/fragments/dnf-installed-checks-api.yml
@ -0,0 +1,3 @@
+bugfixes:
+  - Mirror the behavior of dnf on the command line when handling NEVRAs with omitted epoch (https://github.com/ansible/ansible/issues/71808)
+  - Fix NEVRA parsing of package names that include digit(s) in them (https://github.com/ansible/ansible/issues/76463, https://github.com/ansible/ansible/issues/81018)
--- a/lib/ansible/modules/dnf.py
+++ b/lib/ansible/modules/dnf.py
@ -380,7 +380,6 @@ EXAMPLES = '''
 '''

 import os
-import re
 import sys

 from ansible.module_utils.common.text.converters import to_native, to_text
@ -482,94 +481,6 @@ class DnfModule(YumDnf):

        return result

-    def _split_package_arch(self, packagename):
-        # This list was auto generated on a Fedora 28 system with the following one-liner
-        #   printf '[ '; for arch in $(ls /usr/lib/rpm/platform); do  printf '"%s", ' ${arch%-linux}; done; printf ']\n'
-        redhat_rpm_arches = [
-            "aarch64", "alphaev56", "alphaev5", "alphaev67", "alphaev6", "alpha",
-            "alphapca56", "amd64", "armv3l", "armv4b", "armv4l", "armv5tejl", "armv5tel",
-            "armv5tl", "armv6hl", "armv6l", "armv7hl", "armv7hnl", "armv7l", "athlon",
-            "geode", "i386", "i486", "i586", "i686", "ia32e", "ia64", "m68k", "mips64el",
-            "mips64", "mips64r6el", "mips64r6", "mipsel", "mips", "mipsr6el", "mipsr6",
-            "noarch", "pentium3", "pentium4", "ppc32dy4", "ppc64iseries", "ppc64le", "ppc64",
-            "ppc64p7", "ppc64pseries", "ppc8260", "ppc8560", "ppciseries", "ppc", "ppcpseries",
-            "riscv64", "s390", "s390x", "sh3", "sh4a", "sh4", "sh", "sparc64", "sparc64v",
-            "sparc", "sparcv8", "sparcv9", "sparcv9v", "x86_64"
-        ]
-
-        name, delimiter, arch = packagename.rpartition('.')
-        if name and arch and arch in redhat_rpm_arches:
-            return name, arch
-        return packagename, None
-
-    def _packagename_dict(self, packagename):
-        """
-        Return a dictionary of information for a package name string or None
-        if the package name doesn't contain at least all NVR elements
-        """
-
-        if packagename[-4:] == '.rpm':
-            packagename = packagename[:-4]
-
-        rpm_nevr_re = re.compile(r'(\S+)-(?:(\d*):)?(.*)-(~?\w+[\w.+]*)')
-        try:
-            arch = None
-            nevr, arch = self._split_package_arch(packagename)
-            if arch:
-                packagename = nevr
-            rpm_nevr_match = rpm_nevr_re.match(packagename)
-            if rpm_nevr_match:
-                name, epoch, version, release = rpm_nevr_re.match(packagename).groups()
-                if not version or not version.split('.')[0].isdigit():
-                    return None
-            else:
-                return None
-        except AttributeError as e:
-            self.module.fail_json(
-                msg='Error attempting to parse package: %s, %s' % (packagename, to_native(e)),
-                rc=1,
-                results=[]
-            )
-
-        if not epoch:
-            epoch = "0"
-
-        if ':' in name:
-            epoch_name = name.split(":")
-
-            epoch = epoch_name[0]
-            name = ''.join(epoch_name[1:])
-
-        result = {
-            'name': name,
-            'epoch': epoch,
-            'release': release,
-            'version': version,
-        }
-
-        return result
-
-    # Original implementation from yum.rpmUtils.miscutils (GPLv2+)
-    #   http://yum.baseurl.org/gitweb?p=yum.git;a=blob;f=rpmUtils/miscutils.py
-    def _compare_evr(self, e1, v1, r1, e2, v2, r2):
-        # return 1: a is newer than b
-        # 0: a and b are the same version
-        # -1: b is newer than a
-        if e1 is None:
-            e1 = '0'
-        else:
-            e1 = str(e1)
-        v1 = str(v1)
-        r1 = str(r1)
-        if e2 is None:
-            e2 = '0'
-        else:
-            e2 = str(e2)
-        v2 = str(v2)
-        r2 = str(r2)
-        rc = dnf.rpm.rpm.labelCompare((e1, v1, r1), (e2, v2, r2))
-        return rc
-
    def _ensure_dnf(self):
        locale = get_best_parsable_locale(self.module)
        os.environ['LC_ALL'] = os.environ['LC_MESSAGES'] = locale
@ -578,7 +489,6 @@ class DnfModule(YumDnf):
        global dnf
        try:
            import dnf
-            import dnf.cli
            import dnf.const
            import dnf.exceptions
            import dnf.package
@ -809,43 +719,22 @@ class DnfModule(YumDnf):
        self.module.exit_json(msg="", results=results)

    def _is_installed(self, pkg):
-        installed = self.base.sack.query().installed()
-
-        package_spec = {}
-        name, arch = self._split_package_arch(pkg)
-        if arch:
-            package_spec['arch'] = arch
-
-        package_details = self._packagename_dict(pkg)
-        if package_details:
-            package_details['epoch'] = int(package_details['epoch'])
-            package_spec.update(package_details)
-        else:
-            package_spec['name'] = name
-
-        return bool(installed.filter(**package_spec))
+        return bool(
+            dnf.subject.Subject(pkg).get_best_query(sack=self.base.sack).installed().run()
+        )

    def _is_newer_version_installed(self, pkg_name):
-        candidate_pkg = self._packagename_dict(pkg_name)
-        if not candidate_pkg:
-            # The user didn't provide a versioned rpm, so version checking is
-            # not required
-            return False
-
-        installed = self.base.sack.query().installed()
-        installed_pkg = installed.filter(name=candidate_pkg['name']).run()
-        if installed_pkg:
-            installed_pkg = installed_pkg[0]
-
-            # this looks weird but one is a dict and the other is a dnf.Package
-            evr_cmp = self._compare_evr(
-                installed_pkg.epoch, installed_pkg.version, installed_pkg.release,
-                candidate_pkg['epoch'], candidate_pkg['version'], candidate_pkg['release'],
-            )
-
-            return evr_cmp == 1
-        else:
+        try:
+            if isinstance(pkg_name, dnf.package.Package):
+                available = pkg_name
+            else:
+                available = sorted(
+                    dnf.subject.Subject(pkg_name).get_best_query(sack=self.base.sack).available().run()
+                )[-1]
+            installed = sorted(self.base.sack.query().installed().filter(name=available.name).run())[-1]
+        except IndexError:
            return False
+        return installed > available

    def _mark_package_install(self, pkg_spec, upgrade=False):
        """Mark the package for install."""
@ -917,17 +806,6 @@ class DnfModule(YumDnf):
                    "results": []
                }

-    def _whatprovides(self, filepath):
-        self.base.read_all_repos()
-        available = self.base.sack.query().available()
-        # Search in file
-        files_filter = available.filter(file=filepath)
-        # And Search in provides
-        pkg_spec = files_filter.union(available.filter(provides=filepath)).run()
-
-        if pkg_spec:
-            return pkg_spec[0].name
-
    def _parse_spec_group_file(self):
        pkg_specs, grp_specs, module_specs, filenames = [], [], [], []
        already_loaded_comps = False  # Only load this if necessary, it's slow
@ -939,11 +817,13 @@ class DnfModule(YumDnf):
            elif name.endswith(".rpm"):
                filenames.append(name)
            elif name.startswith('/'):
-                # like "dnf install /usr/bin/vi"
-                pkg_spec = self._whatprovides(name)
-                if pkg_spec:
-                    pkg_specs.append(pkg_spec)
-                    continue
+                # dnf install /usr/bin/vi
+                installed = self.base.sack.query().filter(provides=name, file=name).installed().run()
+                if installed:
+                    pkg_specs.append(installed[0].name)  # should be only one?
+                elif not self.update_only:
+                    # not installed, pass the filename for dnf to process
+                    pkg_specs.append(name)
            elif name.startswith("@") or ('/' in name):
                if not already_loaded_comps:
                    self.base.read_comps()
@ -1005,7 +885,7 @@ class DnfModule(YumDnf):
        else:
            for pkg in pkgs:
                try:
-                    if self._is_newer_version_installed(self._package_dict(pkg)['nevra']):
+                    if self._is_newer_version_installed(pkg):
                        if self.allow_downgrade:
                            self.base.package_install(pkg, strict=self.base.conf.strict)
                    else:
--- a/lib/ansible/modules/dnf5.py
+++ b/lib/ansible/modules/dnf5.py
@ -361,19 +361,37 @@ def is_newer_version_installed(base, spec):
        spec_nevra = next(iter(libdnf5.rpm.Nevra.parse(spec)))
    except RuntimeError:
        return False
-    spec_name = spec_nevra.get_name()
-    v = spec_nevra.get_version()
-    r = spec_nevra.get_release()
-    if not v or not r:
+
+    spec_version = spec_nevra.get_version()
+    if not spec_version:
        return False
-    spec_evr = "{}:{}-{}".format(spec_nevra.get_epoch() or "0", v, r)

-    query = libdnf5.rpm.PackageQuery(base)
-    query.filter_installed()
-    query.filter_name([spec_name])
-    query.filter_evr([spec_evr], libdnf5.common.QueryCmp_GT)
+    installed = libdnf5.rpm.PackageQuery(base)
+    installed.filter_installed()
+    installed.filter_name([spec_nevra.get_name()])
+    installed.filter_latest_evr()
+    try:
+        installed_package = list(installed)[-1]
+    except IndexError:
+        return False

-    return query.size() > 0
+    target = libdnf5.rpm.PackageQuery(base)
+    target.filter_name([spec_nevra.get_name()])
+    target.filter_version([spec_version])
+    spec_release = spec_nevra.get_release()
+    if spec_release:
+        target.filter_release([spec_release])
+    spec_epoch = spec_nevra.get_epoch()
+    if spec_epoch:
+        target.filter_epoch([spec_epoch])
+    target.filter_latest_evr()
+    try:
+        target_package = list(target)[-1]
+    except IndexError:
+        return False
+
+    # FIXME https://github.com/rpm-software-management/dnf5/issues/1104
+    return libdnf5.rpm.rpmvercmp(installed_package.get_evr(), target_package.get_evr()) == 1


 def package_to_dict(package):
@ -606,13 +624,7 @@ class Dnf5Module(YumDnf):
            for spec in self.names:
                if is_newer_version_installed(base, spec):
                    if self.allow_downgrade:
-                        if upgrade:
-                            if is_installed(base, spec):
-                                goal.add_upgrade(spec, settings)
-                            else:
-                                goal.add_install(spec, settings)
-                        else:
-                            goal.add_install(spec, settings)
+                        goal.add_install(spec, settings)
                elif is_installed(base, spec):
                    if upgrade:
                        goal.add_upgrade(spec, settings)
--- a/test/integration/targets/dnf/tasks/repo.yml
+++ b/test/integration/targets/dnf/tasks/repo.yml
@ -307,3 +307,88 @@
        - dinginessentail-with-weak-dep
        - dinginessentail-weak-dep
        state: absent
+
+- name: >
+    test that when a package providing a file is installed then installing by specifying the file doesn't result in
+    installing a different package providing the same file
+  block:
+    - dnf:
+        name: provides_foo_b
+        state: "{{ item }}"
+      loop:
+        - absent
+        - present
+
+    - dnf:
+        name: /foo.gif
+        state: present
+      register: dnf_result
+
+    - command: rpm -q package_foo_a
+      ignore_errors: true
+      register: rpm_result
+
+    - assert:
+        that:
+          - dnf_result is not changed
+          - rpm_result.rc == 1
+  always:
+    - name: Clean up
+      dnf:
+        name: "{{ item }}"
+        state: absent
+      loop:
+        - provides_foo_b
+
+- name: ensure that a package named "$str-$number-$str" is parsed correctly
+  block:
+    - dnf:
+        name: number-11-name-11.0
+        state: "{{ item }}"
+      loop:
+        - absent
+        - present
+
+    - dnf:
+        name: number-11-name
+        state: present
+      register: dnf_result
+
+    - assert:
+        that:
+          - dnf_result is not changed
+
+    - dnf:
+        name: number-11-name
+        state: latest
+        update_only: true
+      register: dnf_result
+
+    - assert:
+        that:
+          - dnf_result is changed
+  always:
+    - name: Clean up
+      dnf:
+        name: number-11-name
+        state: absent
+
+- name: test that epochs are handled the same way as via DNF on the command line
+  block:
+    - dnf:
+        name: "{{ item }}"
+        state: present
+      loop:
+        - "epochone-1.0-1.noarch"
+        - "epochone-1.1-1.noarch"
+      register: dnf_results
+
+    - assert:
+        that:
+          - dnf_results["results"][0] is changed
+          - dnf_results["results"][1] is changed
+  always:
+    - name: Clean up
+      dnf:
+        name: epochone
+        state: absent
--- a/test/integration/targets/setup_rpm_repo/library/create_repo.py
+++ b/test/integration/targets/setup_rpm_repo/library/create_repo.py
@ -15,10 +15,12 @@ from ansible.module_utils.common.respawn import has_respawned, probe_interpreter
 HAS_RPMFLUFF = True
 can_use_rpm_weak_deps = None
 try:
-    from rpmfluff import SimpleRpmBuild
+    from rpmfluff import SimpleRpmBuild, GeneratedSourceFile, make_gif
    from rpmfluff import YumRepoBuild
 except ImportError:
    try:
+        from rpmfluff.make import make_gif
+        from rpmfluff.sourcefile import GeneratedSourceFile
        from rpmfluff.rpmbuild import SimpleRpmBuild
        from rpmfluff.yumrepobuild import YumRepoBuild
    except ImportError:
@ -35,20 +37,25 @@ if HAS_RPMFLUFF:
            pass


-RPM = namedtuple('RPM', ['name', 'version', 'release', 'epoch', 'recommends', 'arch'])
-
+RPM = namedtuple('RPM', ['name', 'version', 'release', 'epoch', 'recommends', 'file', 'arch'])

 SPECS = [
-    RPM('dinginessentail', '1.0', '1', None, None, None),
-    RPM('dinginessentail', '1.0', '2', '1', None, None),
-    RPM('dinginessentail', '1.1', '1', '1', None, None),
-    RPM('dinginessentail-olive', '1.0', '1', None, None, None),
-    RPM('dinginessentail-olive', '1.1', '1', None, None, None),
-    RPM('landsidescalping', '1.0', '1', None, None, None),
-    RPM('landsidescalping', '1.1', '1', None, None, None),
-    RPM('dinginessentail-with-weak-dep', '1.0', '1', None, ['dinginessentail-weak-dep'], None),
-    RPM('dinginessentail-weak-dep', '1.0', '1', None, None, None),
-    RPM('noarchfake', '1.0', '1', None, None, 'noarch'),
+    RPM('dinginessentail', '1.0', '1', None, None, None, None),
+    RPM('dinginessentail', '1.0', '2', '1', None, None, None),
+    RPM('dinginessentail', '1.1', '1', '1', None, None, None),
+    RPM('dinginessentail-olive', '1.0', '1', None, None, None, None),
+    RPM('dinginessentail-olive', '1.1', '1', None, None, None, None),
+    RPM('landsidescalping', '1.0', '1', None, None, None, None),
+    RPM('landsidescalping', '1.1', '1', None, None, None, None),
+    RPM('dinginessentail-with-weak-dep', '1.0', '1', None, ['dinginessentail-weak-dep'], None, None),
+    RPM('dinginessentail-weak-dep', '1.0', '1', None, None, None, None),
+    RPM('noarchfake', '1.0', '1', None, None, None, 'noarch'),
+    RPM('provides_foo_a', '1.0', '1', None, None, 'foo.gif', 'noarch'),
+    RPM('provides_foo_b', '1.0', '1', None, None, 'foo.gif', 'noarch'),
+    RPM('number-11-name', '11.0', '1', None, None, None, None),
+    RPM('number-11-name', '11.1', '1', None, None, None, None),
+    RPM('epochone', '1.0', '1', '1', None, None, "noarch"),
+    RPM('epochone', '1.1', '1', '1', None, None, "noarch"),
 ]


@ -66,6 +73,14 @@ def create_repo(arch='x86_64'):
            for recommend in spec.recommends:
                pkg.add_recommends(recommend)

+        if spec.file:
+            pkg.add_installed_file(
+                "/" + spec.file,
+                GeneratedSourceFile(
+                    spec.file, make_gif()
+                )
+            )
+
        pkgs.append(pkg)

    # HACK: EPEL6 version of rpmfluff can't do multi-arch packaging, so we'll just build separately and copy