dnf: utilize the API for the installed checks (#82725)

Fixes #71808
Fixes #76463
Fixes #81018

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)

lib/ansible/modules/dnf.py

@@ -387,7 +387,6 @@ EXAMPLES = '''
 '''
 
 import os
-import re
 import sys
 
 from ansible.module_utils.common.text.converters import to_native, to_text
@@ -479,94 +478,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
@@ -575,7 +486,6 @@ class DnfModule(YumDnf):
         global dnf
         try:
             import dnf
-            import dnf.cli
             import dnf.const
             import dnf.exceptions
             import dnf.package
@@ -813,43 +723,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."""
@@ -1000,7 +889,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:

lib/ansible/modules/dnf5.py

@@ -374,19 +374,37 @@ def is_newer_version_installed(base, spec):
         spec_nevra = next(iter(libdnf5.rpm.Nevra.parse(spec)))
     except (RuntimeError, StopIteration):
         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):
@@ -604,13 +622,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)

test/integration/targets/dnf/tasks/repo.yml

@@ -467,3 +467,56 @@
         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

test/integration/targets/setup_rpm_repo/library/create_repo.py

@@ -49,6 +49,10 @@ SPECS = [
     RPM('noarchfake', '1.0', '1', None, None, None, 'noarch'),
     RPM('provides_foo_a', '1.0', '1', None, None, 'foo.so', 'noarch'),
     RPM('provides_foo_b', '1.0', '1', None, None, 'foo.so', '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"),
 ]