request for comments - pacman: speed up most operations when working with a package list (#3907)

* pacman: rewrite with a cache to speed up execution

- Use a cache (or inventory) to speed up lookups of:
  - installed packages and groups
  - available packages and groups
  - upgradable packages
- Call pacman with the list of pkgs instead of one call per package (for
  installations, upgrades and removals)
- Use pacman [--sync|--upgrade] --print-format [...] to gather list of
  changes. Parsing that instead of the regular output of pacman, which
  is error prone and can be changed by user configuration.
  This can introduce a TOCTOU problem but unless something else calls
  pacman between the invocations, it shouldn't be a concern.
- Given the above, "check mode" code is within the function that would
  carry out the actual operation. This should make it harder for the
  check code and the "real code" to diverge.
- Support for specifying alternate package name formats is a bit more
  robust. pacman is used to extract the name of the package when the
  specified package is a file or a URL.
  The "<repo>/<pkgname>" format is also supported.

For "state: latest" with a list of ~35 pkgs, this module is about 5
times faster than the original.

* Let fail() actually work

* all unhappy paths now end up calling fail()

* Update copyright

* Argument changes

update_cache_extra_args handled as a list like the others
moved the module setup to its own function for easier testing
update and upgrade have no defaults (None) to let required_one_of() do
its job properly

* update_cache exit path

Shift successful exit without name or upgrade under "update_cache".

It is an error if name or upgrade isn't specified and update_cache wasn't specified
either. (Caught by ansiblemodule required_one_of but still)

* Add pkgs to output on success only

Also align both format, only pkg name for now

* Multiple fixes

Move VersionTuple to top level for import from tests
Add removed pkgs to the exit json when removing packages
fixup list of upgraded pkgs reported on upgrades (was tuple of list for
no reason)
use list idiom for upgrades, like the rest
drop unused expand_package_groups function
skip empty lines when building inventory

* pacman: add tests

* python 2.x compat + pep8

* python 2.x some more

* Fix failure when pacman emits warnings

Add tests covering that failure case

* typo

* Whitespace

black failed me...

* Adjust documentation to fit implicit defaults

* fix test failures on older pythons

* remove file not intended for commit

* Test exception str with e.match

* Build inventory after cache update + adjust tests

* Apply suggestions from code review

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/packaging/os/pacman.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* changelog

* bump copyright year and add my name to authors

* Update changelogs/fragments/3907-pacman-speedup.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* maintainer entry

Co-authored-by: Felix Fontein <felix@fontein.de>

.github/BOTMETA.yml

@@ -819,7 +819,7 @@ files:
   $modules/packaging/os/opkg.py:
     maintainers: skinp
   $modules/packaging/os/pacman.py:
-    maintainers: elasticdog indrajitr tchernomax
+    maintainers: elasticdog indrajitr tchernomax jraby
     labels: pacman
     ignore: elasticdog
   $modules/packaging/os/pacman_key.py:

changelogs/fragments/3907-pacman-speedup.yml

@@ -0,0 +1,5 @@
+minor_changes:
+  - pacman - the module has been rewritten and is now much faster when using ``state=latest``.
+    Operations are now done all packages at once instead of package per package
+    and the configured output format of ``pacman`` no longer affect the module's operation.
+    (https://github.com/ansible-collections/community.general/pull/3907, https://github.com/ansible-collections/community.general/issues/3783, https://github.com/ansible-collections/community.general/issues/4079)

plugins/modules/packaging/os/pacman.py

@@ -4,12 +4,14 @@
 # Copyright: (c) 2012, Afterburn <https://github.com/afterburn>
 # Copyright: (c) 2013, Aaron Bull Schaefer <aaron@elasticdog.com>
 # Copyright: (c) 2015, Indrajit Raychaudhuri <irc+code@indrajit.com>
+# Copyright: (c) 2022, Jean Raby <jean@raby.sh>
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
 from __future__ import absolute_import, division, print_function
+
 __metaclass__ = type
 
-DOCUMENTATION = '''
+DOCUMENTATION = """
 ---
 module: pacman
 short_description: Manage packages with I(pacman)
@@ -19,6 +21,7 @@ author:
     - Indrajit Raychaudhuri (@indrajitr)
     - Aaron Bull Schaefer (@elasticdog) <aaron@elasticdog.com>
     - Maxime de Roucy (@tchernomax)
+    - Jean Raby (@jraby)
 options:
     name:
         description:
@@ -66,7 +69,7 @@ options:
             - Whether or not to refresh the master package lists.
             - This can be run as part of a package installation or as a separate step.
             - Alias C(update-cache) has been deprecated and will be removed in community.general 5.0.0.
-        default: no
+            - If not specified, it defaults to C(false).
         type: bool
         aliases: [ update-cache ]
 
@@ -80,7 +83,7 @@ options:
         description:
             - Whether or not to upgrade the whole system.
               Can't be used in combination with C(name).
-        default: no
+            - If not specified, it defaults to C(false).
         type: bool
 
     upgrade_extra_args:
@@ -94,9 +97,9 @@ notes:
     it is much more efficient to pass the list directly to the I(name) option.
   - To use an AUR helper (I(executable) option), a few extra setup steps might be required beforehand.
     For example, a dedicated build user with permissions to install packages could be necessary.
-'''
+"""
 
-RETURN = '''
+RETURN = """
 packages:
     description: a list of packages that have been changed
     returned: when upgrade is set to yes
@@ -116,9 +119,9 @@ stderr:
     type: str
     sample: "warning: libtool: local (2.4.6+44+gb9b44533-14) is newer than core (2.4.6+42+gb88cebd5-15)\nwarning ..."
     version_added: 4.1.0
-'''
+"""
 
-EXAMPLES = '''
+EXAMPLES = """
 - name: Install package foo from repo
   community.general.pacman:
     name: foo
@@ -180,357 +183,468 @@ EXAMPLES = '''
     name: baz
     state: absent
     force: yes
-'''
+"""
-
-import re
 
+import shlex
 from ansible.module_utils.basic import AnsibleModule
+from collections import defaultdict, namedtuple
 
 
-def get_version(pacman_output):
+Package = namedtuple("Package", ["name", "source"])
-    """Take pacman -Q or pacman -S output and get the Version"""
+VersionTuple = namedtuple("VersionTuple", ["current", "latest"])
-    fields = pacman_output.split()
-    if len(fields) == 2:
-        return fields[1]
-    return None
 
 
-def get_name(module, pacman_output):
+class Pacman(object):
-    """Take pacman -Q or pacman -S output and get the package name"""
+    def __init__(self, module):
-    fields = pacman_output.split()
+        self.m = module
-    if len(fields) == 2:
-        return fields[0]
-    module.fail_json(msg="get_name: fail to retrieve package name from pacman output")
 
+        self.m.run_command_environ_update = dict(LC_ALL="C")
+        p = self.m.params
 
-def query_package(module, pacman_path, name, state):
+        self._msgs = []
-    """Query the package status in both the local system and the repository. Returns a boolean to indicate if the package is installed, a second
+        self._stdouts = []
-    boolean to indicate if the package is up-to-date and a third boolean to indicate whether online information were available
+        self._stderrs = []
+        self.changed = False
+        self.exit_params = {}
+
+        self.pacman_path = self.m.get_bin_path(p["executable"], True)
+
+        # Normalize for old configs
+        if p["state"] == "installed":
+            self.target_state = "present"
+        elif p["state"] == "removed":
+            self.target_state = "absent"
+        else:
+            self.target_state = p["state"]
+
+    def add_exit_infos(self, msg=None, stdout=None, stderr=None):
+        if msg:
+            self._msgs.append(msg)
+        if stdout:
+            self._stdouts.append(stdout)
+        if stderr:
+            self._stderrs.append(stderr)
+
+    def _set_mandatory_exit_params(self):
+        msg = "\n".join(self._msgs)
+        stdouts = "\n".join(self._stdouts)
+        stderrs = "\n".join(self._stderrs)
+        if stdouts:
+            self.exit_params["stdout"] = stdouts
+        if stderrs:
+            self.exit_params["stderr"] = stderrs
+        self.exit_params["msg"] = msg  # mandatory, but might be empty
+
+    def fail(self, msg=None, stdout=None, stderr=None, **kwargs):
+        self.add_exit_infos(msg, stdout, stderr)
+        self._set_mandatory_exit_params()
+        if kwargs:
+            self.exit_params.update(**kwargs)
+        self.m.fail_json(**self.exit_params)
+
+    def success(self):
+        self._set_mandatory_exit_params()
+        self.m.exit_json(changed=self.changed, **self.exit_params)
+
+    def run(self):
+        if self.m.params["update_cache"]:
+            self.update_package_db()
+
+            if not (self.m.params["name"] or self.m.params["upgrade"]):
+                self.success()
+
+        self.inventory = self._build_inventory()
+        if self.m.params["upgrade"]:
+            self.upgrade()
+            self.success()
+
+        if self.m.params["name"]:
+            pkgs = self.package_list()
+
+            if self.target_state == "absent":
+                self.remove_packages(pkgs)
+                self.success()
+            else:
+                self.install_packages(pkgs)
+                self.success()
+
+        # This shouldn't happen...
+        self.fail("This is a bug")
+
+    def install_packages(self, pkgs):
+        pkgs_to_install = []
+        for p in pkgs:
+            if (
+                p.name not in self.inventory["installed_pkgs"]
+                or self.target_state == "latest"
+                and p.name in self.inventory["upgradable_pkgs"]
+            ):
+                pkgs_to_install.append(p)
+
+        if len(pkgs_to_install) == 0:
+            self.add_exit_infos("package(s) already installed")
+            return
+
+        self.changed = True
+        cmd_base = [
+            self.pacman_path,
+            "--sync",
+            "--noconfirm",
+            "--noprogressbar",
+            "--needed",
+        ]
+        if self.m.params["extra_args"]:
+            cmd_base.extend(self.m.params["extra_args"])
+
+        # Dry run first to gather what will be done
+        cmd = cmd_base + ["--print-format", "%n %v"] + [p.source for p in pkgs_to_install]
+        rc, stdout, stderr = self.m.run_command(cmd, check_rc=False)
+        if rc != 0:
+            self.fail("Failed to list package(s) to install", stdout=stdout, stderr=stderr)
+
+        name_ver = [l.strip() for l in stdout.splitlines()]
+        before = []
+        after = []
+        installed_pkgs = []
+        self.exit_params["packages"] = []
+        for p in name_ver:
+            name, version = p.split()
+            if name in self.inventory["installed_pkgs"]:
+                before.append("%s-%s" % (name, self.inventory["installed_pkgs"][name]))
+            after.append("%s-%s" % (name, version))
+            installed_pkgs.append(name)
+
+        self.exit_params["diff"] = {
+            "before": "\n".join(before) + "\n" if before else "",
+            "after": "\n".join(after) + "\n" if after else "",
+        }
+
+        if self.m.check_mode:
+            self.add_exit_infos("Would have installed %d packages" % len(installed_pkgs))
+            self.exit_params["packages"] = installed_pkgs
+            return
+
+        # actually do it
+        cmd = cmd_base + [p.source for p in pkgs_to_install]
+
+        rc, stdout, stderr = self.m.run_command(cmd, check_rc=False)
+        if rc != 0:
+            self.fail("Failed to install package(s)", stdout=stdout, stderr=stderr)
+
+        self.exit_params["packages"] = installed_pkgs
+        self.add_exit_infos(
+            "Installed %d package(s)" % len(installed_pkgs), stdout=stdout, stderr=stderr
+        )
+
+    def remove_packages(self, pkgs):
+        force_args = ["--nodeps", "--nodeps"] if self.m.params["force"] else []
+
+        # filter out pkgs that are already absent
+        pkg_names_to_remove = [p.name for p in pkgs if p.name in self.inventory["installed_pkgs"]]
+
+        if len(pkg_names_to_remove) == 0:
+            self.add_exit_infos("package(s) already absent")
+            return
+
+        # There's something to do, set this in advance
+        self.changed = True
+
+        cmd_base = [self.pacman_path, "--remove", "--noconfirm", "--noprogressbar"]
+        if self.m.params["extra_args"]:
+            cmd_base.extend(self.m.params["extra_args"])
+        if force_args:
+            cmd_base.extend(force_args)
+
+        # This is a bit of a TOCTOU but it is better than parsing the output of
+        # pacman -R, which is different depending on the user config (VerbosePkgLists)
+        # Start by gathering what would be removed
+        cmd = cmd_base + ["--print-format", "%n-%v"] + pkg_names_to_remove
+
+        rc, stdout, stderr = self.m.run_command(cmd, check_rc=False)
+        if rc != 0:
+            self.fail("failed to list package(s) to remove", stdout=stdout, stderr=stderr)
+
+        removed_pkgs = stdout.split()
+        self.exit_params["packages"] = removed_pkgs
+        self.exit_params["diff"] = {
+            "before": "\n".join(removed_pkgs) + "\n",  # trailing \n to avoid diff complaints
+            "after": "",
+        }
+
+        if self.m.check_mode:
+            self.exit_params["packages"] = removed_pkgs
+            self.add_exit_infos("Would have removed %d packages" % len(removed_pkgs))
+            return
+
+        # actually do it
+        cmd = cmd_base + pkg_names_to_remove
+
+        rc, stdout, stderr = self.m.run_command(cmd, check_rc=False)
+        if rc != 0:
+            self.fail("failed to remove package(s)", stdout=stdout, stderr=stderr)
+        self.exit_params["packages"] = removed_pkgs
+        self.add_exit_infos("Removed %d package(s)" % len(removed_pkgs), stdout=stdout, stderr=stderr)
+
+    def upgrade(self):
+        """Runs pacman --sync --sysupgrade if there are upgradable packages"""
+
+        if len(self.inventory["upgradable_pkgs"]) == 0:
+            self.add_exit_infos("Nothing to upgrade")
+            return
+
+        self.changed = True  # there are upgrades, so there will be changes
+
+        # Build diff based on inventory first.
+        diff = {"before": "", "after": ""}
+        for pkg, versions in self.inventory["upgradable_pkgs"].items():
+            diff["before"] += "%s-%s\n" % (pkg, versions.current)
+            diff["after"] += "%s-%s\n" % (pkg, versions.latest)
+        self.exit_params["diff"] = diff
+        self.exit_params["packages"] = self.inventory["upgradable_pkgs"].keys()
+
+        if self.m.check_mode:
+            self.add_exit_infos(
+                "%d packages would have been upgraded" % (len(self.inventory["upgradable_pkgs"]))
+            )
+        else:
+            cmd = [
+                self.pacman_path,
+                "--sync",
+                "--sys-upgrade",
+                "--quiet",
+                "--noconfirm",
+            ]
+            if self.m.params["upgrade_extra_args"]:
+                cmd += self.m.params["upgrade_extra_args"]
+            rc, stdout, stderr = self.m.run_command(cmd, check_rc=False)
+            if rc == 0:
+                self.add_exit_infos("System upgraded", stdout=stdout, stderr=stderr)
+            else:
+                self.fail("Could not upgrade", stdout=stdout, stderr=stderr)
+
+    def update_package_db(self):
+        """runs pacman --sync --refresh"""
+        if self.m.check_mode:
+            self.add_exit_infos("Would have updated the package db")
+            self.changed = True
+            return
+
+        cmd = [
+            self.pacman_path,
+            "--sync",
+            "--refresh",
+        ]
+        if self.m.params["update_cache_extra_args"]:
+            cmd += self.m.params["update_cache_extra_args"]
+        if self.m.params["force"]:
+            cmd += ["--refresh"]
+
+        rc, stdout, stderr = self.m.run_command(cmd, check_rc=False)
+
+        self.changed = True
+
+        if rc == 0:
+            self.add_exit_infos("Updated package db", stdout=stdout, stderr=stderr)
+        else:
+            self.fail("could not update package db", stdout=stdout, stderr=stderr)
+
+    def package_list(self):
+        """Takes the input package list and resolves packages groups to their package list using the inventory,
+        extracts package names from packages given as files or URLs using calls to pacman
+
+        Returns the expanded/resolved list as a list of Package
+        """
+        pkg_list = []
+        for pkg in self.m.params["name"]:
+            if not pkg:
+                continue
+
+            if pkg in self.inventory["available_groups"]:
+                # Expand group members
+                for group_member in self.inventory["available_groups"][pkg]:
+                    pkg_list.append(Package(name=group_member, source=group_member))
+            elif pkg in self.inventory["available_pkgs"]:
+                # just a regular pkg
+                pkg_list.append(Package(name=pkg, source=pkg))
+            else:
+                # Last resort, call out to pacman to extract the info,
+                # pkg is possibly in the <repo>/<pkgname> format, or a filename or a URL
+
+                # Start with <repo>/<pkgname> case
+                cmd = [self.pacman_path, "--sync", "--print-format", "%n", pkg]
+                rc, stdout, stderr = self.m.run_command(cmd, check_rc=False)
+                if rc != 0:
+                    # fallback to filename / URL
+                    cmd = [self.pacman_path, "--upgrade", "--print-format", "%n", pkg]
+                    rc, stdout, stderr = self.m.run_command(cmd, check_rc=False)
+                    if rc != 0:
+                        if self.target_state == "absent":
+                            continue  # Don't bark for unavailable packages when trying to remove them
+                        else:
+                            self.fail(
+                                msg="Failed to list package %s" % (pkg),
+                                cmd=cmd,
+                                stdout=stdout,
+                                stderr=stderr,
+                                rc=rc,
+                            )
+                pkg_name = stdout.strip()
+                pkg_list.append(Package(name=pkg_name, source=pkg))
+
+        return pkg_list
+
+    def _build_inventory(self):
+        """Build a cache datastructure used for all pkg lookups
+        Returns a dict:
+        {
+            "installed_pkgs": {pkgname: version},
+            "installed_groups": {groupname: set(pkgnames)},
+            "available_pkgs": {pkgname: version},
+            "available_groups": {groupname: set(pkgnames)},
+            "upgradable_pkgs": {pkgname: (current_version,latest_version)},
+        }
+
+        Fails the module if a package requested for install cannot be found
         """
 
-    lcmd = "%s --query %s" % (pacman_path, name)
+        installed_pkgs = {}
-    lrc, lstdout, lstderr = module.run_command(lcmd, check_rc=False)
+        dummy, stdout, dummy = self.m.run_command([self.pacman_path, "--query"], check_rc=True)
-    if lrc != 0:
+        # Format of a line: "pacman 6.0.1-2"
-        # package is not installed locally
+        for l in stdout.splitlines():
-        return False, False, False
+            l = l.strip()
-    else:
+            if not l:
-        # a non-zero exit code doesn't always mean the package is installed
-        # for example, if the package name queried is "provided" by another package
-        installed_name = get_name(module, lstdout)
-        if installed_name != name:
-            return False, False, False
-
-    # no need to check the repository if state is present or absent
-    # return False for package version check, because we didn't check it
-    if state == 'present' or state == 'absent':
-        return True, False, False
-
-    # get the version installed locally (if any)
-    lversion = get_version(lstdout)
-
-    rcmd = "%s --sync --print-format \"%%n %%v\" %s" % (pacman_path, name)
-    rrc, rstdout, rstderr = module.run_command(rcmd, check_rc=False)
-    # get the version in the repository
-    rversion = get_version(rstdout)
-
-    if rrc == 0:
-        # Return True to indicate that the package is installed locally, and the result of the version number comparison
-        # to determine if the package is up-to-date.
-        return True, (lversion == rversion), False
-
-    # package is installed but cannot fetch remote Version. Last True stands for the error
-    return True, True, True
-
-
-def update_package_db(module, pacman_path):
-    if module.params['force']:
-        module.params["update_cache_extra_args"] += " --refresh --refresh"
-
-    cmd = "%s --sync --refresh %s" % (pacman_path, module.params["update_cache_extra_args"])
-    rc, stdout, stderr = module.run_command(cmd, check_rc=False)
-
-    if rc == 0:
-        return stdout, stderr
-    else:
-        module.fail_json(msg="could not update package db", stdout=stdout, stderr=stderr)
-
-
-def upgrade(module, pacman_path):
-    cmdupgrade = "%s --sync --sysupgrade --quiet --noconfirm %s" % (pacman_path, module.params["upgrade_extra_args"])
-    cmdneedrefresh = "%s --query --upgrades" % (pacman_path)
-    rc, stdout, stderr = module.run_command(cmdneedrefresh, check_rc=False)
-    data = stdout.split('\n')
-    data.remove('')
-    packages = []
-    diff = {
-        'before': '',
-        'after': '',
-    }
-
-    if rc == 0:
-        # Match lines of `pacman -Qu` output of the form:
-        #   (package name) (before version-release) -> (after version-release)
-        # e.g., "ansible 2.7.1-1 -> 2.7.2-1"
-        regex = re.compile(r'([\w+\-.@]+) (\S+-\S+) -> (\S+-\S+)')
-        for p in data:
-            if '[ignored]' not in p:
-                m = regex.search(p)
-                packages.append(m.group(1))
-                if module._diff:
-                    diff['before'] += "%s-%s\n" % (m.group(1), m.group(2))
-                    diff['after'] += "%s-%s\n" % (m.group(1), m.group(3))
-        if module.check_mode:
-            if packages:
-                module.exit_json(changed=True, msg="%s package(s) would be upgraded" % (len(data)), packages=packages, diff=diff)
-            else:
-                module.exit_json(changed=False, msg='Nothing to upgrade', packages=packages)
-        rc, stdout, stderr = module.run_command(cmdupgrade, check_rc=False)
-        if rc == 0:
-            if packages:
-                module.exit_json(changed=True, msg='System upgraded', packages=packages, diff=diff, stdout=stdout, stderr=stderr)
-            else:
-                module.exit_json(changed=False, msg='Nothing to upgrade', packages=packages)
-        else:
-            module.fail_json(msg="Could not upgrade", stdout=stdout, stderr=stderr)
-    else:
-        module.exit_json(changed=False, msg='Nothing to upgrade', packages=packages)
-
-
-def remove_packages(module, pacman_path, packages):
-    data = []
-    diff = {
-        'before': '',
-        'after': '',
-    }
-
-    if module.params["force"]:
-        module.params["extra_args"] += " --nodeps --nodeps"
-
-    remove_c = 0
-    stdout_total = ""
-    stderr_total = ""
-    # Using a for loop in case of error, we can report the package that failed
-    for package in packages:
-        # Query the package first, to see if we even need to remove
-        installed, updated, unknown = query_package(module, pacman_path, package, 'absent')
-        if not installed:
                 continue
+            pkg, ver = l.split()
+            installed_pkgs[pkg] = ver
 
-        cmd = "%s --remove --noconfirm --noprogressbar %s %s" % (pacman_path, module.params["extra_args"], package)
+        installed_groups = defaultdict(set)
-        rc, stdout, stderr = module.run_command(cmd, check_rc=False)
+        dummy, stdout, dummy = self.m.run_command(
-
+            [self.pacman_path, "--query", "--group"], check_rc=True
-        if rc != 0:
+        )
-            module.fail_json(msg="failed to remove %s" % (package), stdout=stdout, stderr=stderr)
+        # Format of lines:
-
+        #     base-devel file
-        stdout_total += stdout
+        #     base-devel findutils
-        stderr_total += stderr
+        #     ...
-        if module._diff:
+        for l in stdout.splitlines():
-            d = stdout.split('\n')[2].split(' ')[2:]
+            l = l.strip()
-            for i, pkg in enumerate(d):
+            if not l:
-                d[i] = re.sub('-[0-9].*$', '', d[i].split('/')[-1])
-                diff['before'] += "%s\n" % pkg
-            data.append('\n'.join(d))
-
-        remove_c += 1
-
-    if remove_c > 0:
-        module.exit_json(changed=True, msg="removed %s package(s)" % remove_c, diff=diff, stdout=stdout_total, stderr=stderr_total)
-
-    module.exit_json(changed=False, msg="package(s) already absent")
-
-
-def install_packages(module, pacman_path, state, packages, package_files):
-    install_c = 0
-    package_err = []
-    message = ""
-    data = []
-    diff = {
-        'before': '',
-        'after': '',
-    }
-
-    to_install_repos = []
-    to_install_files = []
-    for i, package in enumerate(packages):
-        # if the package is installed and state == present or state == latest and is up-to-date then skip
-        installed, updated, latestError = query_package(module, pacman_path, package, state)
-        if latestError and state == 'latest':
-            package_err.append(package)
-
-        if installed and (state == 'present' or (state == 'latest' and updated)):
                 continue
+            group, pkgname = l.split()
+            installed_groups[group].add(pkgname)
 
-        if package_files[i]:
+        available_pkgs = {}
-            to_install_files.append(package_files[i])
+        dummy, stdout, dummy = self.m.run_command([self.pacman_path, "--sync", "--list"], check_rc=True)
+        # Format of a line: "core pacman 6.0.1-2"
+        for l in stdout.splitlines():
+            l = l.strip()
+            if not l:
+                continue
+            repo, pkg, ver = l.split()[:3]
+            available_pkgs[pkg] = ver
+
+        available_groups = defaultdict(set)
+        dummy, stdout, dummy = self.m.run_command(
+            [self.pacman_path, "--sync", "--group", "--group"], check_rc=True
+        )
+        # Format of lines:
+        #     vim-plugins vim-airline
+        #     vim-plugins vim-airline-themes
+        #     vim-plugins vim-ale
+        #     ...
+        for l in stdout.splitlines():
+            l = l.strip()
+            if not l:
+                continue
+            group, pkg = l.split()
+            available_groups[group].add(pkg)
+
+        upgradable_pkgs = {}
+        rc, stdout, stderr = self.m.run_command(
+            [self.pacman_path, "--query", "--upgrades"], check_rc=False
+        )
+
+        # non-zero exit with nothing in stdout -> nothing to upgrade, all good
+        # stderr can have warnings, so not checked here
+        if rc == 1 and stdout == "":
+            pass  # nothing to upgrade
+        elif rc == 0:
+            # Format of lines:
+            #     strace 5.14-1 -> 5.15-1
+            #     systemd 249.7-1 -> 249.7-2 [ignored]
+            for l in stdout.splitlines():
+                l = l.strip()
+                if not l:
+                    continue
+                if "[ignored]" in l:
+                    continue
+                s = l.split()
+                if len(s) != 4:
+                    self.fail(msg="Invalid line: %s" % l)
+
+                pkg = s[0]
+                current = s[1]
+                latest = s[3]
+                upgradable_pkgs[pkg] = VersionTuple(current=current, latest=latest)
         else:
-            to_install_repos.append(package)
+            # stuff in stdout but rc!=0, abort
+            self.fail(
+                "Couldn't get list of packages available for upgrade",
+                stdout=stdout,
+                stderr=stderr,
+                rc=rc,
+            )
 
-    if to_install_repos:
+        return dict(
-        cmd = "%s --sync --noconfirm --noprogressbar --needed %s %s" % (pacman_path, module.params["extra_args"], " ".join(to_install_repos))
+            installed_pkgs=installed_pkgs,
-        rc, stdout, stderr = module.run_command(cmd, check_rc=False)
+            installed_groups=installed_groups,
-
+            available_pkgs=available_pkgs,
-        if rc != 0:
+            available_groups=available_groups,
-            module.fail_json(msg="failed to install %s: %s" % (" ".join(to_install_repos), stderr), stdout=stdout, stderr=stderr)
+            upgradable_pkgs=upgradable_pkgs,
-
+        )
-        # As we pass `--needed` to pacman returns a single line of ` there is nothing to do` if no change is performed.
-        # The check for > 3 is here because we pick the 4th line in normal operation.
-        if len(stdout.split('\n')) > 3:
-            data = stdout.split('\n')[3].split(' ')[2:]
-            data = [i for i in data if i != '']
-            for i, pkg in enumerate(data):
-                data[i] = re.sub('-[0-9].*$', '', data[i].split('/')[-1])
-                if module._diff:
-                    diff['after'] += "%s\n" % pkg
-
-            install_c += len(to_install_repos)
-
-    if to_install_files:
-        cmd = "%s --upgrade --noconfirm --noprogressbar --needed %s %s" % (pacman_path, module.params["extra_args"], " ".join(to_install_files))
-        rc, stdout, stderr = module.run_command(cmd, check_rc=False)
-
-        if rc != 0:
-            module.fail_json(msg="failed to install %s: %s" % (" ".join(to_install_files), stderr), stdout=stdout, stderr=stderr)
-
-        # As we pass `--needed` to pacman returns a single line of ` there is nothing to do` if no change is performed.
-        # The check for > 3 is here because we pick the 4th line in normal operation.
-        if len(stdout.split('\n')) > 3:
-            data = stdout.split('\n')[3].split(' ')[2:]
-            data = [i for i in data if i != '']
-            for i, pkg in enumerate(data):
-                data[i] = re.sub('-[0-9].*$', '', data[i].split('/')[-1])
-                if module._diff:
-                    diff['after'] += "%s\n" % pkg
-
-            install_c += len(to_install_files)
-
-    if state == 'latest' and len(package_err) > 0:
-        message = "But could not ensure 'latest' state for %s package(s) as remote version could not be fetched." % (package_err)
-
-    if install_c > 0:
-        module.exit_json(changed=True, msg="installed %s package(s). %s" % (install_c, message), diff=diff, stdout=stdout, stderr=stderr)
-
-    module.exit_json(changed=False, msg="package(s) already installed. %s" % (message), diff=diff)
 
 
-def check_packages(module, pacman_path, packages, state):
+def setup_module():
-    would_be_changed = []
-    diff = {
-        'before': '',
-        'after': '',
-        'before_header': '',
-        'after_header': ''
-    }
-
-    for package in packages:
-        installed, updated, unknown = query_package(module, pacman_path, package, state)
-        if ((state in ["present", "latest"] and not installed) or
-                (state == "absent" and installed) or
-                (state == "latest" and not updated)):
-            would_be_changed.append(package)
-    if would_be_changed:
-        if state == "absent":
-            state = "removed"
-
-        if module._diff and (state == 'removed'):
-            diff['before_header'] = 'removed'
-            diff['before'] = '\n'.join(would_be_changed) + '\n'
-        elif module._diff and ((state == 'present') or (state == 'latest')):
-            diff['after_header'] = 'installed'
-            diff['after'] = '\n'.join(would_be_changed) + '\n'
-
-        module.exit_json(changed=True, msg="%s package(s) would be %s" % (
-            len(would_be_changed), state), diff=diff)
-    else:
-        module.exit_json(changed=False, msg="package(s) already %s" % state, diff=diff)
-
-
-def expand_package_groups(module, pacman_path, pkgs):
-    expanded = []
-
-    __, stdout, __ = module.run_command([pacman_path, "--sync", "--groups", "--quiet"], check_rc=True)
-    available_groups = stdout.splitlines()
-
-    for pkg in pkgs:
-        if pkg:  # avoid empty strings
-            if pkg in available_groups:
-                # A group was found matching the package name: expand it
-                cmd = [pacman_path, "--sync", "--groups", "--quiet", pkg]
-                rc, stdout, stderr = module.run_command(cmd, check_rc=True)
-                expanded.extend([name.strip() for name in stdout.splitlines()])
-            else:
-                expanded.append(pkg)
-
-    return expanded
-
-
-def main():
     module = AnsibleModule(
         argument_spec=dict(
-            name=dict(type='list', elements='str', aliases=['pkg', 'package']),
+            name=dict(type="list", elements="str", aliases=["pkg", "package"]),
-            state=dict(type='str', default='present', choices=['present', 'installed', 'latest', 'absent', 'removed']),
+            state=dict(
-            force=dict(type='bool', default=False),
+                type="str",
-            executable=dict(type='str', default='pacman'),
+                default="present",
-            extra_args=dict(type='str', default=''),
+                choices=["present", "installed", "latest", "absent", "removed"],
-            upgrade=dict(type='bool', default=False),
-            upgrade_extra_args=dict(type='str', default=''),
-            update_cache=dict(
-                type='bool', default=False, aliases=['update-cache'],
-                deprecated_aliases=[dict(name='update-cache', version='5.0.0', collection_name='community.general')]),
-            update_cache_extra_args=dict(type='str', default=''),
             ),
-        required_one_of=[['name', 'update_cache', 'upgrade']],
+            force=dict(type="bool", default=False),
-        mutually_exclusive=[['name', 'upgrade']],
+            executable=dict(type="str", default="pacman"),
+            extra_args=dict(type="str", default=""),
+            upgrade=dict(type="bool"),
+            upgrade_extra_args=dict(type="str", default=""),
+            update_cache=dict(
+                type="bool",
+                aliases=["update-cache"],
+                deprecated_aliases=[
+                    dict(
+                        name="update-cache",
+                        version="5.0.0",
+                        collection_name="community.general",
+                    )
+                ],
+            ),
+            update_cache_extra_args=dict(type="str", default=""),
+        ),
+        required_one_of=[["name", "update_cache", "upgrade"]],
+        mutually_exclusive=[["name", "upgrade"]],
         supports_check_mode=True,
     )
 
-    module.run_command_environ_update = dict(LC_ALL='C')
+    # Split extra_args as the shell would for easier handling later
+    for str_args in ["extra_args", "upgrade_extra_args", "update_cache_extra_args"]:
+        module.params[str_args] = shlex.split(module.params[str_args])
 
-    p = module.params
+    return module
 
-    # find pacman binary
-    pacman_path = module.get_bin_path(p['executable'], True)
 
-    # normalize the state parameter
+def main():
-    if p['state'] in ['present', 'installed']:
-        p['state'] = 'present'
-    elif p['state'] in ['absent', 'removed']:
-        p['state'] = 'absent'
 
-    if p["update_cache"] and not module.check_mode:
+    Pacman(setup_module()).run()
-        stdout, stderr = update_package_db(module, pacman_path)
-        if not (p['name'] or p['upgrade']):
-            module.exit_json(changed=True, msg='Updated the package master lists', stdout=stdout, stderr=stderr)
-
-    if p['update_cache'] and module.check_mode and not (p['name'] or p['upgrade']):
-        module.exit_json(changed=True, msg='Would have updated the package cache')
-
-    if p['upgrade']:
-        upgrade(module, pacman_path)
-
-    if p['name']:
-        pkgs = expand_package_groups(module, pacman_path, p['name'])
-
-        pkg_files = []
-        for i, pkg in enumerate(pkgs):
-            if not pkg:  # avoid empty strings
-                continue
-            elif re.match(r".*\.pkg\.tar(\.(gz|bz2|xz|lrz|lzo|Z|zst))?$", pkg):
-                # The package given is a filename, extract the raw pkg name from
-                # it and store the filename
-                pkg_files.append(pkg)
-                pkgs[i] = re.sub(r'-[0-9].*$', '', pkgs[i].split('/')[-1])
-            else:
-                pkg_files.append(None)
-
-        if module.check_mode:
-            check_packages(module, pacman_path, pkgs, p['state'])
-
-        if p['state'] in ['present', 'latest']:
-            install_packages(module, pacman_path, p['state'], pkgs, pkg_files)
-        elif p['state'] == 'absent':
-            remove_packages(module, pacman_path, pkgs)
-    else:
-        module.exit_json(changed=False, msg="No package specified to work on.")
 
 
 if __name__ == "__main__":

tests/unit/plugins/modules/packaging/os/test_pacman.py

@@ -0,0 +1,947 @@
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+from __future__ import absolute_import, division, print_function
+
+__metaclass__ = type
+
+import sys
+
+
+from ansible.module_utils import basic
+from ansible_collections.community.general.tests.unit.compat import mock, unittest
+from ansible_collections.community.general.tests.unit.compat.mock import patch
+from ansible_collections.community.general.tests.unit.plugins.modules.utils import (
+    AnsibleExitJson,
+    AnsibleFailJson,
+    set_module_args,
+    exit_json,
+    fail_json,
+)
+
+from ansible_collections.community.general.plugins.modules.packaging.os import pacman
+from ansible_collections.community.general.plugins.modules.packaging.os.pacman import (
+    Package,
+    VersionTuple,
+)
+
+import pytest
+import json
+
+
+def get_bin_path(self, arg, required=False):
+    """Mock AnsibleModule.get_bin_path"""
+    return arg
+
+
+# This inventory data is tightly coupled with the inventory test and the mock_valid_inventory fixture
+valid_inventory = {
+    "installed_pkgs": {
+        "file": "5.41-1",
+        "filesystem": "2021.11.11-1",
+        "findutils": "4.8.0-1",
+        "gawk": "5.1.1-1",
+        "gettext": "0.21-1",
+        "grep": "3.7-1",
+        "gzip": "1.11-1",
+        "pacman": "6.0.1-2",
+        "pacman-mirrorlist": "20211114-1",
+        "sed": "4.8-1",
+        "sqlite": "3.36.0-1",
+    },
+    "installed_groups": {
+        "base-devel": set(["gawk", "grep", "file", "findutils", "pacman", "sed", "gzip", "gettext"])
+    },
+    "available_pkgs": {
+        "acl": "2.3.1-1",
+        "amd-ucode": "20211027.1d00989-1",
+        "archlinux-keyring": "20211028-1",
+        "argon2": "20190702-3",
+        "attr": "2.5.1-1",
+        "audit": "3.0.6-5",
+        "autoconf": "2.71-1",
+        "automake": "1.16.5-1",
+        "b43-fwcutter": "019-3",
+        "gawk": "5.1.1-1",
+        "grep": "3.7-1",
+        "sqlite": "3.37.0-1",
+        "sudo": "1.9.8.p2-3",
+    },
+    "available_groups": {
+        "base-devel": set(
+            [
+                "libtool",
+                "gawk",
+                "which",
+                "texinfo",
+                "fakeroot",
+                "grep",
+                "findutils",
+                "autoconf",
+                "gzip",
+                "pkgconf",
+                "flex",
+                "patch",
+                "groff",
+                "m4",
+                "bison",
+                "gcc",
+                "gettext",
+                "make",
+                "file",
+                "pacman",
+                "sed",
+                "automake",
+                "sudo",
+                "binutils",
+            ]
+        ),
+        "some-group": set(["libtool", "sudo", "binutils"]),
+    },
+    "upgradable_pkgs": {
+        "sqlite": VersionTuple(current="3.36.0-1", latest="3.37.0-1"),
+    },
+}
+
+empty_inventory = {
+    "installed_pkgs": {},
+    "available_pkgs": {},
+    "installed_groups": {},
+    "available_groups": {},
+    "upgradable_pkgs": {},
+}
+
+
+class TestPacman:
+    @pytest.fixture(autouse=True)
+    def run_command(self, mocker):
+        self.mock_run_command = mocker.patch.object(basic.AnsibleModule, "run_command", autospec=True)
+
+    @pytest.fixture
+    def mock_package_list(self, mocker):
+        return mocker.patch.object(pacman.Pacman, "package_list", autospec=True)
+
+    @pytest.fixture(autouse=True)
+    def common(self, mocker):
+        self.mock_module = mocker.patch.multiple(
+            basic.AnsibleModule,
+            exit_json=exit_json,
+            fail_json=fail_json,
+            get_bin_path=get_bin_path,
+        )
+
+    @pytest.fixture
+    def mock_empty_inventory(self, mocker):
+        inv = empty_inventory
+        return mocker.patch.object(pacman.Pacman, "_build_inventory", return_value=inv)
+
+    @pytest.fixture
+    def mock_valid_inventory(self, mocker):
+        return mocker.patch.object(pacman.Pacman, "_build_inventory", return_value=valid_inventory)
+
+    def test_fail_without_required_args(self):
+        with pytest.raises(AnsibleFailJson) as e:
+            set_module_args({})
+            pacman.main()
+        assert e.match(r"one of the following is required")
+
+    def test_success(self, mock_empty_inventory):
+        set_module_args({"update_cache": True})  # Simplest args to let init go through
+        P = pacman.Pacman(pacman.setup_module())
+        with pytest.raises(AnsibleExitJson) as e:
+            P.success()
+
+    def test_fail(self, mock_empty_inventory):
+        set_module_args({"update_cache": True})
+        P = pacman.Pacman(pacman.setup_module())
+
+        args = dict(
+            msg="msg", stdout="something", stderr="somethingelse", cmd=["command", "with", "args"], rc=1
+        )
+        with pytest.raises(AnsibleFailJson) as e:
+            P.fail(**args)
+
+        assert all(item in e.value.args[0] for item in args)
+
+    @pytest.mark.parametrize(
+        "expected, run_command_side_effect, raises",
+        [
+            (
+                # Regular run
+                valid_inventory,
+                [
+                    [  # pacman --query
+                        0,
+                        """file 5.41-1
+                        filesystem 2021.11.11-1
+                        findutils 4.8.0-1
+                        gawk 5.1.1-1
+                        gettext 0.21-1
+                        grep 3.7-1
+                        gzip 1.11-1
+                        pacman 6.0.1-2
+                        pacman-mirrorlist 20211114-1
+                        sed 4.8-1
+                        sqlite 3.36.0-1
+                        """,
+                        "",
+                    ],
+                    (  # pacman --query --group
+                        0,
+                        """base-devel file
+                        base-devel findutils
+                        base-devel gawk
+                        base-devel gettext
+                        base-devel grep
+                        base-devel gzip
+                        base-devel pacman
+                        base-devel sed
+                        """,
+                        "",
+                    ),
+                    (  # pacman --sync --list
+                        0,
+                        """core acl 2.3.1-1 [installed]
+                        core amd-ucode 20211027.1d00989-1
+                        core archlinux-keyring 20211028-1 [installed]
+                        core argon2 20190702-3 [installed]
+                        core attr 2.5.1-1 [installed]
+                        core audit 3.0.6-5 [installed: 3.0.6-2]
+                        core autoconf 2.71-1
+                        core automake 1.16.5-1
+                        core b43-fwcutter 019-3
+                        core gawk 5.1.1-1 [installed]
+                        core grep 3.7-1 [installed]
+                        core sqlite 3.37.0-1 [installed: 3.36.0-1]
+                        code sudo 1.9.8.p2-3
+                        """,
+                        "",
+                    ),
+                    (  # pacman --sync --group --group
+                        0,
+                        """base-devel autoconf
+                        base-devel automake
+                        base-devel binutils
+                        base-devel bison
+                        base-devel fakeroot
+                        base-devel file
+                        base-devel findutils
+                        base-devel flex
+                        base-devel gawk
+                        base-devel gcc
+                        base-devel gettext
+                        base-devel grep
+                        base-devel groff
+                        base-devel gzip
+                        base-devel libtool
+                        base-devel m4
+                        base-devel make
+                        base-devel pacman
+                        base-devel patch
+                        base-devel pkgconf
+                        base-devel sed
+                        base-devel sudo
+                        base-devel texinfo
+                        base-devel which
+                        some-group libtool
+                        some-group sudo
+                        some-group binutils
+                        """,
+                        "",
+                    ),
+                    (  # pacman --query --upgrades
+                        0,
+                        """sqlite 3.36.0-1 -> 3.37.0-1
+                        systemd 249.6-3 -> 249.7-2 [ignored]
+                        """,
+                        "",
+                    ),
+                ],
+                None,
+            ),
+            (
+                # All good, but call to --query --upgrades return 1. aka nothing to upgrade
+                # with a pacman warning
+                empty_inventory,
+                [
+                    (0, "", ""),
+                    (0, "", ""),
+                    (0, "", ""),
+                    (0, "", ""),
+                    (
+                        1,
+                        "",
+                        "warning: config file /etc/pacman.conf, line 34: directive 'TotalDownload' in section 'options' not recognized.",
+                    ),
+                ],
+                None,
+            ),
+            (
+                # failure
+                empty_inventory,
+                [
+                    (0, "", ""),
+                    (0, "", ""),
+                    (0, "", ""),
+                    (0, "", ""),
+                    (
+                        1,
+                        "partial\npkg\\nlist",
+                        "some warning",
+                    ),
+                ],
+                AnsibleFailJson,
+            ),
+        ],
+    )
+    def test_build_inventory(self, expected, run_command_side_effect, raises):
+        self.mock_run_command.side_effect = run_command_side_effect
+
+        set_module_args({"update_cache": True})
+        if raises:
+            with pytest.raises(raises):
+                P = pacman.Pacman(pacman.setup_module())
+                P._build_inventory()
+        else:
+            P = pacman.Pacman(pacman.setup_module())
+            assert P._build_inventory() == expected
+
+    @pytest.mark.parametrize("check_mode_value", [True, False])
+    def test_upgrade_check_empty_inventory(self, mock_empty_inventory, check_mode_value):
+        set_module_args({"upgrade": True, "_ansible_check_mode": check_mode_value})
+        P = pacman.Pacman(pacman.setup_module())
+        with pytest.raises(AnsibleExitJson) as e:
+            P.run()
+        self.mock_run_command.call_count == 0
+        out = e.value.args[0]
+        assert "packages" not in out
+        assert not out["changed"]
+        assert "diff" not in out
+
+    def test_update_db_check(self, mock_empty_inventory):
+        set_module_args({"update_cache": True, "_ansible_check_mode": True})
+        P = pacman.Pacman(pacman.setup_module())
+
+        with pytest.raises(AnsibleExitJson) as e:
+            P.run()
+        self.mock_run_command.call_count == 0
+        out = e.value.args[0]
+        assert out["changed"]
+
+    @pytest.mark.parametrize(
+        "module_args,expected_call",
+        [
+            ({}, ["pacman", "--sync", "--refresh"]),
+            ({"force": True}, ["pacman", "--sync", "--refresh", "--refresh"]),
+            (
+                {"update_cache_extra_args": "--some-extra args"},
+                ["pacman", "--sync", "--refresh", "--some-extra", "args"],  # shlex test
+            ),
+            (
+                {"force": True, "update_cache_extra_args": "--some-extra args"},
+                ["pacman", "--sync", "--refresh", "--some-extra", "args", "--refresh"],
+            ),
+        ],
+    )
+    def test_update_db(self, mock_empty_inventory, module_args, expected_call):
+        args = {"update_cache": True}
+        args.update(module_args)
+        set_module_args(args)
+
+        self.mock_run_command.return_value = [0, "stdout", "stderr"]
+        with pytest.raises(AnsibleExitJson) as e:
+            P = pacman.Pacman(pacman.setup_module())
+            P.run()
+
+        self.mock_run_command.assert_called_with(mock.ANY, expected_call, check_rc=False)
+        out = e.value.args[0]
+        assert out["changed"]
+
+    @pytest.mark.parametrize(
+        "check_mode_value, run_command_data, upgrade_extra_args",
+        [
+            # just check
+            (True, None, None),
+            (
+                # for real
+                False,
+                {
+                    "args": ["pacman", "--sync", "--sys-upgrade", "--quiet", "--noconfirm"],
+                    "return_value": [0, "stdout", "stderr"],
+                },
+                None,
+            ),
+            (
+                # with extra args
+                False,
+                {
+                    "args": [
+                        "pacman",
+                        "--sync",
+                        "--sys-upgrade",
+                        "--quiet",
+                        "--noconfirm",
+                        "--some",
+                        "value",
+                    ],
+                    "return_value": [0, "stdout", "stderr"],
+                },
+                "--some value",
+            ),
+        ],
+    )
+    def test_upgrade(self, mock_valid_inventory, check_mode_value, run_command_data, upgrade_extra_args):
+        args = {"upgrade": True, "_ansible_check_mode": check_mode_value}
+        if upgrade_extra_args:
+            args["upgrade_extra_args"] = upgrade_extra_args
+        set_module_args(args)
+
+        if run_command_data and "return_value" in run_command_data:
+            self.mock_run_command.return_value = run_command_data["return_value"]
+
+        P = pacman.Pacman(pacman.setup_module())
+
+        with pytest.raises(AnsibleExitJson) as e:
+            P.run()
+        out = e.value.args[0]
+
+        if check_mode_value:
+            self.mock_run_command.call_count == 0
+
+        if run_command_data and "args" in run_command_data:
+            self.mock_run_command.assert_called_with(mock.ANY, run_command_data["args"], check_rc=False)
+            assert out["stdout"] == "stdout"
+            assert out["stderr"] == "stderr"
+
+        assert len(out["packages"]) == 1 and "sqlite" in out["packages"]
+        assert out["changed"]
+        assert out["diff"]["before"] and out["diff"]["after"]
+
+    def test_upgrade_fail(self, mock_valid_inventory):
+        set_module_args({"upgrade": True})
+        self.mock_run_command.return_value = [1, "stdout", "stderr"]
+        P = pacman.Pacman(pacman.setup_module())
+
+        with pytest.raises(AnsibleFailJson) as e:
+            P.run()
+        self.mock_run_command.call_count == 1
+        out = e.value.args[0]
+        assert out["failed"]
+        assert out["stdout"] == "stdout"
+        assert out["stderr"] == "stderr"
+
+    @pytest.mark.parametrize(
+        "state, pkg_names, expected, run_command_data, raises",
+        [
+            # regular packages, no resolving required
+            (
+                "present",
+                ["acl", "attr"],
+                [Package(name="acl", source="acl"), Package(name="attr", source="attr")],
+                None,
+                None,
+            ),
+            (
+                # group expansion
+                "present",
+                ["acl", "some-group", "attr"],
+                [
+                    Package(name="acl", source="acl"),
+                    Package(name="binutils", source="binutils"),
+                    Package(name="libtool", source="libtool"),
+                    Package(name="sudo", source="sudo"),
+                    Package(name="attr", source="attr"),
+                ],
+                None,
+                None,
+            ),
+            (
+                # <repo>/<pkgname> format -> call to pacman to resolve
+                "present",
+                ["community/elixir"],
+                [Package(name="elixir", source="community/elixir")],
+                {
+                    "calls": [
+                        mock.call(
+                            mock.ANY,
+                            ["pacman", "--sync", "--print-format", "%n", "community/elixir"],
+                            check_rc=False,
+                        )
+                    ],
+                    "side_effect": [(0, "elixir", "")],
+                },
+                None,
+            ),
+            (
+                # catch all -> call to pacman to resolve (--sync and --upgrade)
+                "present",
+                ["somepackage-12.3-x86_64.pkg.tar.zst"],
+                [Package(name="somepackage", source="somepackage-12.3-x86_64.pkg.tar.zst")],
+                {
+                    "calls": [
+                        mock.call(
+                            mock.ANY,
+                            [
+                                "pacman",
+                                "--sync",
+                                "--print-format",
+                                "%n",
+                                "somepackage-12.3-x86_64.pkg.tar.zst",
+                            ],
+                            check_rc=False,
+                        ),
+                        mock.call(
+                            mock.ANY,
+                            [
+                                "pacman",
+                                "--upgrade",
+                                "--print-format",
+                                "%n",
+                                "somepackage-12.3-x86_64.pkg.tar.zst",
+                            ],
+                            check_rc=False,
+                        ),
+                    ],
+                    "side_effect": [(1, "", "nope"), (0, "somepackage", "")],
+                },
+                None,
+            ),
+            (
+                # install a package that doesn't exist. call pacman twice and give up
+                "present",
+                ["unknown-package"],
+                [],
+                {
+                    # no call validation, since it will fail
+                    "side_effect": [(1, "", "nope"), (1, "", "stillnope")],
+                },
+                AnsibleFailJson,
+            ),
+            (
+                # Edge case: resolve a pkg that doesn't exist when trying to remove it (state == absent).
+                # will fallback to file + url format but not complain since it is already not there
+                # Can happen if a pkg is removed for the repos  (or if a repo is disabled/removed)
+                "absent",
+                ["unknown-package-to-remove"],
+                [],
+                {
+                    "calls": [
+                        mock.call(
+                            mock.ANY,
+                            ["pacman", "--sync", "--print-format", "%n", "unknown-package-to-remove"],
+                            check_rc=False,
+                        ),
+                        mock.call(
+                            mock.ANY,
+                            ["pacman", "--upgrade", "--print-format", "%n", "unknown-package-to-remove"],
+                            check_rc=False,
+                        ),
+                    ],
+                    "side_effect": [(1, "", "nope"), (1, "", "stillnope")],
+                },
+                None,  # Doesn't fail
+            ),
+        ],
+    )
+    def test_package_list(
+        self, mock_valid_inventory, state, pkg_names, expected, run_command_data, raises
+    ):
+        set_module_args({"name": pkg_names, "state": state})
+        P = pacman.Pacman(pacman.setup_module())
+        P.inventory = P._build_inventory()
+        if run_command_data:
+            self.mock_run_command.side_effect = run_command_data["side_effect"]
+
+        if raises:
+            with pytest.raises(raises):
+                P.package_list()
+        else:
+            assert sorted(P.package_list()) == sorted(expected)
+            if run_command_data:
+                assert self.mock_run_command.mock_calls == run_command_data["calls"]
+
+    @pytest.mark.parametrize("check_mode_value", [True, False])
+    @pytest.mark.parametrize(
+        "name, state, package_list",
+        [
+            (["already-absent"], "absent", [Package("already-absent", "already-absent")]),
+            (["grep"], "present", [Package("grep", "grep")]),
+        ],
+    )
+    def test_op_packages_nothing_to_do(
+        self, mock_valid_inventory, mock_package_list, check_mode_value, name, state, package_list
+    ):
+        set_module_args({"name": name, "state": state, "_ansible_check_mode": check_mode_value})
+        mock_package_list.return_value = package_list
+        P = pacman.Pacman(pacman.setup_module())
+        with pytest.raises(AnsibleExitJson) as e:
+            P.run()
+        out = e.value.args[0]
+        assert "packages" not in out
+        assert not out["changed"]
+        assert "diff" not in out
+        self.mock_run_command.call_count == 0
+
+    @pytest.mark.parametrize(
+        "module_args, expected_packages, run_command_data, raises",
+        [
+            (
+                # remove pkg: Check mode -- call to print format but that's it
+                {"_ansible_check_mode": True, "name": ["grep"], "state": "absent"},
+                ["grep-version"],
+                {
+                    "calls": [
+                        mock.call(
+                            mock.ANY,
+                            [
+                                "pacman",
+                                "--remove",
+                                "--noconfirm",
+                                "--noprogressbar",
+                                "--print-format",
+                                "%n-%v",
+                                "grep",
+                            ],
+                            check_rc=False,
+                        ),
+                    ],
+                    "side_effect": [(0, "grep-version", "")],
+                },
+                AnsibleExitJson,
+            ),
+            (
+                # remove pkg for real now -- with 2 packages
+                {"name": ["grep", "gawk"], "state": "absent"},
+                ["grep-version", "gawk-anotherversion"],
+                {
+                    "calls": [
+                        mock.call(
+                            mock.ANY,
+                            [
+                                "pacman",
+                                "--remove",
+                                "--noconfirm",
+                                "--noprogressbar",
+                                "--print-format",
+                                "%n-%v",
+                                "grep",
+                                "gawk",
+                            ],
+                            check_rc=False,
+                        ),
+                        mock.call(
+                            mock.ANY,
+                            ["pacman", "--remove", "--noconfirm", "--noprogressbar", "grep", "gawk"],
+                            check_rc=False,
+                        ),
+                    ],
+                    "side_effect": [
+                        (0, "grep-version\ngawk-anotherversion", ""),
+                        (0, "stdout", "stderr"),
+                    ],
+                },
+                AnsibleExitJson,
+            ),
+            (
+                # remove pkg   force + extra_args
+                {
+                    "name": ["grep"],
+                    "state": "absent",
+                    "force": True,
+                    "extra_args": "--some --extra arg",
+                },
+                ["grep-version"],
+                {
+                    "calls": [
+                        mock.call(
+                            mock.ANY,
+                            [
+                                "pacman",
+                                "--remove",
+                                "--noconfirm",
+                                "--noprogressbar",
+                                "--some",
+                                "--extra",
+                                "arg",
+                                "--nodeps",
+                                "--nodeps",
+                                "--print-format",
+                                "%n-%v",
+                                "grep",
+                            ],
+                            check_rc=False,
+                        ),
+                        mock.call(
+                            mock.ANY,
+                            [
+                                "pacman",
+                                "--remove",
+                                "--noconfirm",
+                                "--noprogressbar",
+                                "--some",
+                                "--extra",
+                                "arg",
+                                "--nodeps",
+                                "--nodeps",
+                                "grep",
+                            ],
+                            check_rc=False,
+                        ),
+                    ],
+                    "side_effect": [
+                        (0, "grep-version", ""),
+                        (0, "stdout", "stderr"),
+                    ],
+                },
+                AnsibleExitJson,
+            ),
+            (
+                # remove pkg -- Failure to list
+                {"name": ["grep"], "state": "absent"},
+                ["grep-3.7-1"],
+                {
+                    "calls": [
+                        mock.call(
+                            mock.ANY,
+                            [
+                                "pacman",
+                                "--remove",
+                                "--noconfirm",
+                                "--noprogressbar",
+                                "--print-format",
+                                "%n-%v",
+                                "grep",
+                            ],
+                            check_rc=False,
+                        )
+                    ],
+                    "side_effect": [
+                        (1, "stdout", "stderr"),
+                    ],
+                },
+                AnsibleFailJson,
+            ),
+            (
+                # remove pkg -- Failure to remove
+                {"name": ["grep"], "state": "absent"},
+                ["grep-3.7-1"],
+                {
+                    "calls": [
+                        mock.call(
+                            mock.ANY,
+                            [
+                                "pacman",
+                                "--remove",
+                                "--noconfirm",
+                                "--noprogressbar",
+                                "--print-format",
+                                "%n-%v",
+                                "grep",
+                            ],
+                            check_rc=False,
+                        ),
+                        mock.call(
+                            mock.ANY,
+                            ["pacman", "--remove", "--noconfirm", "--noprogressbar", "grep"],
+                            check_rc=False,
+                        ),
+                    ],
+                    "side_effect": [
+                        (0, "grep", ""),
+                        (1, "stdout", "stderr"),
+                    ],
+                },
+                AnsibleFailJson,
+            ),
+            (
+                # install pkg: Check mode
+                {"_ansible_check_mode": True, "name": ["sudo"], "state": "present"},
+                ["sudo"],
+                {
+                    "calls": [
+                        mock.call(
+                            mock.ANY,
+                            [
+                                "pacman",
+                                "--sync",
+                                "--noconfirm",
+                                "--noprogressbar",
+                                "--needed",
+                                "--print-format",
+                                "%n %v",
+                                "sudo",
+                            ],
+                            check_rc=False,
+                        ),
+                    ],
+                    "side_effect": [(0, "sudo version", "")],
+                },
+                AnsibleExitJson,
+            ),
+            (
+                # install 2 pkgs, one already present
+                {"name": ["sudo", "grep"], "state": "present"},
+                ["sudo"],
+                {
+                    "calls": [
+                        mock.call(
+                            mock.ANY,
+                            [
+                                "pacman",
+                                "--sync",
+                                "--noconfirm",
+                                "--noprogressbar",
+                                "--needed",
+                                "--print-format",
+                                "%n %v",
+                                "sudo",
+                            ],
+                            check_rc=False,
+                        ),
+                        mock.call(
+                            mock.ANY,
+                            [
+                                "pacman",
+                                "--sync",
+                                "--noconfirm",
+                                "--noprogressbar",
+                                "--needed",
+                                "sudo",
+                            ],
+                            check_rc=False,
+                        ),
+                    ],
+                    "side_effect": [(0, "sudo version", ""), (0, "", "")],
+                },
+                AnsibleExitJson,
+            ),
+            (
+                # install pkg, extra_args
+                {"name": ["sudo"], "state": "present", "extra_args": "--some --thing else"},
+                ["sudo"],
+                {
+                    "calls": [
+                        mock.call(
+                            mock.ANY,
+                            [
+                                "pacman",
+                                "--sync",
+                                "--noconfirm",
+                                "--noprogressbar",
+                                "--needed",
+                                "--some",
+                                "--thing",
+                                "else",
+                                "--print-format",
+                                "%n %v",
+                                "sudo",
+                            ],
+                            check_rc=False,
+                        ),
+                        mock.call(
+                            mock.ANY,
+                            [
+                                "pacman",
+                                "--sync",
+                                "--noconfirm",
+                                "--noprogressbar",
+                                "--needed",
+                                "--some",
+                                "--thing",
+                                "else",
+                                "sudo",
+                            ],
+                            check_rc=False,
+                        ),
+                    ],
+                    "side_effect": [(0, "sudo version", ""), (0, "", "")],
+                },
+                AnsibleExitJson,
+            ),
+            (
+                # latest pkg: Check mode
+                {"_ansible_check_mode": True, "name": ["sqlite"], "state": "latest"},
+                ["sqlite"],
+                {
+                    "calls": [
+                        mock.call(
+                            mock.ANY,
+                            [
+                                "pacman",
+                                "--sync",
+                                "--noconfirm",
+                                "--noprogressbar",
+                                "--needed",
+                                "--print-format",
+                                "%n %v",
+                                "sqlite",
+                            ],
+                            check_rc=False,
+                        ),
+                    ],
+                    "side_effect": [(0, "sqlite new-version", "")],
+                },
+                AnsibleExitJson,
+            ),
+            (
+                # latest pkg -- one already latest
+                {"name": ["sqlite", "grep"], "state": "latest"},
+                ["sqlite"],
+                {
+                    "calls": [
+                        mock.call(
+                            mock.ANY,
+                            [
+                                "pacman",
+                                "--sync",
+                                "--noconfirm",
+                                "--noprogressbar",
+                                "--needed",
+                                "--print-format",
+                                "%n %v",
+                                "sqlite",
+                            ],
+                            check_rc=False,
+                        ),
+                        mock.call(
+                            mock.ANY,
+                            [
+                                "pacman",
+                                "--sync",
+                                "--noconfirm",
+                                "--noprogressbar",
+                                "--needed",
+                                "sqlite",
+                            ],
+                            check_rc=False,
+                        ),
+                    ],
+                    "side_effect": [(0, "sqlite new-version", ""), (0, "", "")],
+                },
+                AnsibleExitJson,
+            ),
+        ],
+    )
+    def test_op_packages(
+        self,
+        mock_valid_inventory,
+        module_args,
+        expected_packages,
+        run_command_data,
+        raises,
+    ):
+        set_module_args(module_args)
+        self.mock_run_command.side_effect = run_command_data["side_effect"]
+
+        P = pacman.Pacman(pacman.setup_module())
+        with pytest.raises(raises) as e:
+            P.run()
+        out = e.value.args[0]
+
+        assert self.mock_run_command.mock_calls == run_command_data["calls"]
+        if raises == AnsibleExitJson:
+            assert out["packages"] == expected_packages
+            assert out["changed"]
+            assert "diff" in out
+        else:
+            assert out["stdout"] == "stdout"
+            assert out["stderr"] == "stderr"