From f24ab68f044e996cbe73b565edbcacc8df050a84 Mon Sep 17 00:00:00 2001 From: Matthew Gamble Date: Mon, 19 Dec 2016 17:05:17 +1100 Subject: [PATCH] Improve and optimise pacman package installation Previously, packages were installed one at a time in a loop. This caused a couple of problems. First, it was a performance issue - pacman would have to perform all of its checks once per package. This is unnecessarily costly, especially when you're trying to install several related packages at the same time. Second, if a package you're trying to install depends on a virtual package that is provided by several different packages (such as the "libgl" package on Arch) and you aren't also installing something that provides that virtual package at the same time, pacman will produce an interactive prompt to allow the user to select a relevant package. This is obviously incompatible with how ansible operates. Yes, this problem could be avoided by installing packages in a different order, but the order of installation shouldn't matter, and there may be situations where it is not possible to control the order of installation. With this refactoring, all of the above problems are avoided. The code will now work out all of the packages that need to be installed from any configured repositories and any packages that need to be installed from local files, and then install all the repository packages in one go and then all of the local file packages in one go. --- lib/ansible/modules/packaging/os/pacman.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/lib/ansible/modules/packaging/os/pacman.py b/lib/ansible/modules/packaging/os/pacman.py index 2cf8924dcae..1642f3ed60b 100644 --- a/lib/ansible/modules/packaging/os/pacman.py +++ b/lib/ansible/modules/packaging/os/pacman.py @@ -243,6 +243,8 @@ def install_packages(module, pacman_path, state, packages, package_files): package_err = [] message = "" + 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) @@ -253,17 +255,26 @@ def install_packages(module, pacman_path, state, packages, package_files): continue if package_files[i]: - params = '-U %s' % package_files[i] + to_install_files.append(package_files[i]) else: - params = '-S %s' % package + to_install_repos.append(package) - cmd = "%s %s --noconfirm --needed" % (pacman_path, params) + if to_install_repos: + cmd = "%s -S %s --noconfirm --needed" % (pacman_path, " ".join(to_install_repos)) rc, stdout, stderr = module.run_command(cmd, check_rc=False) if rc != 0: - module.fail_json(msg="failed to install %s" % (package)) + module.fail_json(msg="failed to install %s: %s" % (" ".join(to_install_repos), stderr)) - install_c += 1 + install_c += len(to_install_repos) + + if to_install_files: + cmd = "%s -U %s --noconfirm --needed" % (pacman_path, " ".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)) + + 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)