Skip to content

Commit

Permalink
request for comments - pacman: speed up most operations when working …
Browse files Browse the repository at this point in the history
…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 <[email protected]>

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

Co-authored-by: Felix Fontein <[email protected]>

* changelog

* bump copyright year and add my name to authors

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

Co-authored-by: Felix Fontein <[email protected]>

* maintainer entry

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit 1580f3c)
  • Loading branch information
jraby authored and patchback[bot] committed Feb 8, 2022
1 parent 815638f commit 2743b65
Show file tree
Hide file tree
Showing 4 changed files with 1,396 additions and 330 deletions.
2 changes: 1 addition & 1 deletion .github/BOTMETA.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
5 changes: 5 additions & 0 deletions changelogs/fragments/3907-pacman-speedup.yml
Original file line number Diff line number Diff line change
@@ -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)
Loading

0 comments on commit 2743b65

Please sign in to comment.