Fix rpm pkg warnings and remove fallback to slow process fork

Author: bdrx312

changelog/68341.fixed.md

@@ -0,0 +1,2 @@
+Fixed pkg.installed state from showing warning if python rpm package not installed.
+Fixed pkg.installed state from showing warning and using slow process fork for version comparison when rpmdevtools is installed

salt/modules/rpm_lowpkg.py

@@ -2,6 +2,8 @@
 Support for rpm
 """
 
+from __future__ import annotations
+
 import datetime
 import logging
 import os
@@ -11,7 +13,6 @@
 import salt.utils.itertools
 import salt.utils.path
 import salt.utils.pkg.rpm
-import salt.utils.versions
 from salt.exceptions import CommandExecutionError, SaltInvocationError
 from salt.utils.versions import LooseVersion
 
@@ -698,14 +699,23 @@ def version_cmp(ver1, ver2, ignore_epoch=False):
     """
 
     def normalize(x):
-        return str(x).split(":", 1)[-1] if ignore_epoch else str(x)
+        return str(x).split(":", maxsplit=1)[-1] if ignore_epoch else str(x)
 
     ver1 = normalize(ver1)
     ver2 = normalize(ver2)
 
-    try:
-        cmp_func = None
-        if HAS_RPM:
+    (ver1_e, ver1_v, ver1_r) = salt.utils.pkg.rpm.version_to_evr(ver1)
+    (ver2_e, ver2_v, ver2_r) = salt.utils.pkg.rpm.version_to_evr(ver2)
+    # If one EVR is missing a release but not the other and they
+    # otherwise would be equal, ignore the release. This can happen if
+    # e.g. you are checking if a package version 3.2 is satisfied by
+    # 3.2-1.
+    if not ver1_r or not ver2_r:
+        ver1_r = ver2_r = ""
+
+    if HAS_RPM:
+        try:
+            cmp_func = None
             try:
                 cmp_func = rpm.labelCompare
             except AttributeError:
@@ -716,91 +726,27 @@ def normalize(x):
                     "labelCompare function. Not using rpm.labelCompare for "
                     "version comparison."
                 )
-        else:
-            log.warning(
-                "Please install a package that provides rpm.labelCompare for "
-                "more accurate version comparisons."
-            )
-
-        # If one EVR is missing a release but not the other and they
-        # otherwise would be equal, ignore the release. This can happen if
-        # e.g. you are checking if a package version 3.2 is satisfied by
-        # 3.2-1.
-        (ver1_e, ver1_v, ver1_r) = salt.utils.pkg.rpm.version_to_evr(ver1)
-        (ver2_e, ver2_v, ver2_r) = salt.utils.pkg.rpm.version_to_evr(ver2)
-
-        if not ver1_r or not ver2_r:
-            ver1_r = ver2_r = ""
-
-        if cmp_func is None:
-            ver1 = f"{ver1_e}:{ver1_v}-{ver1_r}"
-            ver2 = f"{ver2_e}:{ver2_v}-{ver2_r}"
-
-            if salt.utils.path.which("rpmdev-vercmp"):
-                log.warning(
-                    "Installing the rpmdevtools package may surface dev tools in"
-                    " production."
+            if cmp_func is not None:
+                cmp_result = cmp_func(
+                    (ver1_e, ver1_v, ver1_r), (ver2_e, ver2_v, ver2_r)
                 )
-
-                # rpmdev-vercmp always uses epochs, even when zero
-                def _ensure_epoch(ver):
-                    def _prepend(ver):
-                        return f"0:{ver}"
-
-                    try:
-                        if ":" not in ver:
-                            return _prepend(ver)
-                    except TypeError:
-                        return _prepend(ver)
-                    return ver
-
-                ver1 = _ensure_epoch(ver1)
-                ver2 = _ensure_epoch(ver2)
-                result = __salt__["cmd.run_all"](
-                    ["rpmdev-vercmp", ver1, ver2],
-                    python_shell=False,
-                    redirect_stderr=True,
-                    ignore_retcode=True,
-                )
-                # rpmdev-vercmp returns 0 on equal, 11 on greater-than, and
-                # 12 on less-than.
-                if result["retcode"] == 0:
-                    return 0
-                elif result["retcode"] == 11:
-                    return 1
-                elif result["retcode"] == 12:
-                    return -1
-                else:
-                    # We'll need to fall back to salt.utils.versions.version_cmp()
-                    log.warning(
-                        "Failed to interpret results of rpmdev-vercmp output. "
-                        "This is probably a bug, and should be reported. "
-                        "Return code was %s. Output: %s",
-                        result["retcode"],
-                        result["stdout"],
+                if cmp_result not in (-1, 0, 1):
+                    raise CommandExecutionError(
+                        f"Comparison result '{cmp_result}' is invalid"
                     )
-            else:
-                log.warning(
-                    "Falling back on salt.utils.versions.version_cmp() for version"
-                    " comparisons"
-                )
-        else:
-            cmp_result = cmp_func((ver1_e, ver1_v, ver1_r), (ver2_e, ver2_v, ver2_r))
-            if cmp_result not in (-1, 0, 1):
-                raise CommandExecutionError(
-                    f"Comparison result '{cmp_result}' is invalid"
-                )
-            return cmp_result
-
-    except Exception as exc:  # pylint: disable=broad-except
-        log.warning(
-            "Failed to compare version '%s' to '%s' using RPM: %s", ver1, ver2, exc
+                return cmp_result
+        except Exception as exc:  # pylint: disable=broad-except
+            log.warning(
+                "Failed to compare version '%s' to '%s' using RPM: %s", ver1, ver2, exc
+            )
+    else:
+        log.info(
+            "Install a package that provides rpm.labelCompare for "
+            "faster version comparisons."
         )
-
-    # We would already have normalized the versions at the beginning of this
-    # function if ignore_epoch=True, so avoid unnecessary work and just pass
-    # False for this value.
-    return salt.utils.versions.version_cmp(ver1, ver2, ignore_epoch=False)
+    return salt.utils.pkg.rpm.evr_compare(
+        (ver1_e, ver1_v, ver1_r), (ver2_e, ver2_v, ver2_r)
+    )
 
 
 def checksum(*paths, **kwargs):

salt/utils/pkg/rpm.py

@@ -2,6 +2,8 @@
 Common functions for working with RPM packages
 """
 
+from __future__ import annotations
+
 import collections
 import datetime
 import logging
@@ -157,6 +159,156 @@ def combine_comments(comments):
     return "".join(ret)
 
 
+def evr_compare(
+    evr1: tuple[str | None, str | None, str | None],
+    evr2: tuple[str | None, str | None, str | None],
+) -> int:
+    """
+    Compare two RPM package identifiers using full epoch–version–release semantics.
+
+    This is a pure‑Python equivalent of ``rpm.labelCompare()``, returning the same
+    ordering as the system RPM library without requiring the ``python3-rpm`` bindings.
+
+    The comparison is performed in three stages:
+
+    1. **Epoch** — compared numerically; missing or empty values are treated as 0.
+    2. **Version** — compared using RPM's ``rpmvercmp`` rules:
+       - Split into digit, alpha, and tilde (``~``) segments.
+       - Tilde sorts before all other characters (e.g. ``1.0~beta`` < ``1.0``).
+       - Numeric segments are compared as integers, ignoring leading zeros.
+       - Numeric segments sort before alpha segments.
+    3. **Release** — compared with the same rules as version.
+
+    :param evr1: The first ``(epoch, version, release)`` triple to compare.
+                 Each element may be a string or ``None``.
+    :param evr2: The second ``(epoch, version, release)`` triple to compare.
+                 Each element may be a string or ``None``.
+    :return: ``-1`` if ``evr1`` is considered older than ``evr2``,
+             ``0`` if they are considered equal,
+             ``1`` if ``evr1`` is considered newer than ``evr2``.
+
+    .. note::
+       This comparison is **not** the same as PEP 440, ``LooseVersion``, or
+       ``StrictVersion``. It is intended for RPM package metadata and will match
+       the ordering used by tools like ``rpm``, ``dnf``, and ``yum``.
+
+    .. code-block:: python
+
+       >>> label_compare(("0", "1.2.3", "1"), ("0", "1.2.3", "2"))
+       -1
+       >>> label_compare(("1", "1.0", "1"), ("0", "9.9", "9"))
+       1
+       >>> label_compare(("0", "1.0~beta", "1"), ("0", "1.0", "1"))
+       -1
+    """
+    epoch1, version1, release1 = evr1
+    epoch2, version2, release2 = evr2
+    epoch1 = int(epoch1 or 0)
+    epoch2 = int(epoch2 or 0)
+    if epoch1 != epoch2:
+        return 1 if epoch1 > epoch2 else -1
+    cmp_versions = _rpmvercmp(version1 or "", version2 or "")
+    if cmp_versions != 0:
+        return cmp_versions
+    return _rpmvercmp(release1 or "", release2 or "")
+
+
+def _rpmvercmp(a: str, b: str) -> int:
+    """
+    Pure-Python comparator matching RPM's rpmvercmp().
+    Handles separators, tilde (~), caret (^), numeric/alpha segments.
+    """
+    # Fast path: identical strings
+    if a == b:
+        return 0
+
+    # Work with mutable indices instead of C char* pointers
+    i = j = 0
+    la, lb = len(a), len(b)
+
+    def isalnum_(c: str) -> bool:
+        return c.isalnum()
+
+    while i < la or j < lb:
+        # Skip separators: anything not alnum, not ~, not ^
+        while i < la and not (isalnum_(a[i]) or a[i] in "~^"):
+            i += 1
+        while j < lb and not (isalnum_(b[j]) or b[j] in "~^"):
+            j += 1
+
+        # Tilde: sorts before everything else
+        if i < la and a[i] == "~" or j < lb and b[j] == "~":
+            if not (i < la and a[i] == "~"):
+                return 1
+            if not (j < lb and b[j] == "~"):
+                return -1
+            i += 1
+            j += 1
+            continue
+
+        # Caret: like tilde except base (end) loses to caret
+        if i < la and a[i] == "^" or j < lb and b[j] == "^":
+            if i >= la:
+                return -1
+            if j >= lb:
+                return 1
+            if not (i < la and a[i] == "^"):
+                return 1
+            if not (j < lb and b[j] == "^"):
+                return -1
+            i += 1
+            j += 1
+            continue
+
+        # If either ran out now, stop
+        if not (i < la and j < lb):
+            break
+
+        # Segment start positions
+        si, sj = i, j
+
+        # Decide type from left side
+        isnum = a[i].isdigit()
+        if isnum:
+            while i < la and a[i].isdigit():
+                i += 1
+            while j < lb and b[j].isdigit():
+                j += 1
+        else:
+            while i < la and a[i].isalpha():
+                i += 1
+            while j < lb and b[j].isalpha():
+                j += 1
+
+        # If right side had no same‑type run, types differ
+        if sj == j:
+            return 1 if isnum else -1
+
+        seg_a = a[si:i]
+        seg_b = b[sj:j]
+
+        if isnum:
+            # Strip leading zeros
+            seg_a_nz = seg_a.lstrip("0")
+            seg_b_nz = seg_b.lstrip("0")
+            # Compare by length
+            if len(seg_a_nz) != len(seg_b_nz):
+                return 1 if len(seg_a_nz) > len(seg_b_nz) else -1
+            # Same length: lexicographic
+            if seg_a_nz != seg_b_nz:
+                return 1 if seg_a_nz > seg_b_nz else -1
+        else:
+            # Alpha vs alpha
+            if seg_a != seg_b:
+                return 1 if seg_a > seg_b else -1
+        # else equal segment → loop continues
+
+    # Tail handling
+    if i >= la and j >= lb:
+        return 0
+    return -1 if i >= la else 1
+
+
 def version_to_evr(verstring):
     """
     Split the package version string into epoch, version and release.