-
Notifications
You must be signed in to change notification settings - Fork 23.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
pacman: recognize if a target package is provided by another package #34574
Conversation
The test
|
bot_status |
Componentslib/ansible/modules/packaging/os/pacman.py Metadatawaiting_on: maintainer |
Hi @indrajitr, what's the state here? Do you need further information from me? |
@thomwiggers could you verify if this fixes the issue you reported. Thanks. |
@resmo Only if it's already installed.
|
Thanks, I missed that. New change should fix this (tested with svn uninstalled / svn installed / trying to install a non-existent package) |
The test
|
Removing the package doesn't work yet:
|
Alright. Think I checked all relevant combinations of options now. Current test playbook: - name: Test pacman module changes
hosts: localhost
become: true
tasks:
- name: prepare
pacman:
name: subversion
state: absent
- name: name=real state=absent action=remove
pacman:
name: subversion
state: absent
- name: name=real state=absent action=install
pacman:
name: subversion
state: present
- name: name=real state=present action=install
pacman:
name: subversion
state: present
- name: name=real state=present action=latest
pacman:
name: subversion
state: latest
- name: name=real state=present action=remove
pacman:
name: subversion
state: absent
- name: name=short state=absent action=remove
pacman:
name: svn
state: absent
- name: name=short state=absent action=install
pacman:
name: svn
state: present
- name: name=short state=present action=install
pacman:
name: svn
state: present
- name: name=short state=present action=latest
pacman:
name: svn
state: latest
- name: name=short state=present action=remove
pacman:
name: svn
state: absent
- name: name=bad state=absent action=remove
pacman:
name: not-a-real-package
state: absent
ignore_errors: yes
- name: name=bad state=absent action=install
pacman:
name: not-a-real-package
state: present
ignore_errors: yes
- name: name=bad state=absent action=latest
pacman:
name: not-a-real-package
state: present
ignore_errors: yes |
Appears to work for me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -138,21 +138,62 @@ def get_version(pacman_output): | |||
return None | |||
|
|||
|
|||
def find_provider(module, pacman_path, target_package): | |||
"""Find the package providing `target_package`""" | |||
cmd = "%s -Q %s" % (pacman_path, target_package) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cmd = "%s -Q %s" % (pacman_path, target_package) | |
cmd = "%s --query %s" % (pacman_path, target_package) |
I personally prefer if we use the long options.
if rc == 0: | ||
realname = output.split()[0] | ||
version = output.split()[1] | ||
return (realname, version) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# (realname, version)
return tuple(output.split())
no need for named variables
return (realname, version) | ||
|
||
# Unable to find package named `target_package` - scan full package dump | ||
cmd = "%s -Qqei" % pacman_path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cmd = "%s -Qqei" % pacman_path | |
cmd = "%s --query --quiet --explicit --info" % pacman_path |
# some unknown error | ||
return (None, None) | ||
|
||
target_package = target_package.lower() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
target_package = target_package.lower() |
I don't think you should do this, case is meaningful:
max@mde-oxalide % pacman -Q libreoffice-en-US
libreoffice-fresh 6.1.3-1
max@mde-oxalide % pacman -Q libreoffice-en-us
error: package 'libreoffice-en-us' was not found
return (realname, version) | ||
|
||
# Unable to find package named `target_package` - scan full package dump | ||
cmd = "%s -Qqei" % pacman_path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure this is necessary ? Can you provide an example when 'pacman -Q …' isn't sufficient ?
|
||
for line in lines: | ||
if line.startswith("Name"): | ||
pkg_name = line.split(':', 1)[1].strip().lower() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pkg_name = line.split(':', 1)[1].strip().lower() | |
pkg_name = line.partition(':')[-1].strip() |
idem, no need to lower.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're going to do partition, I'd rather use the fact that it'll always return a 3-tuple instead of using -1
to index the third element. i.e.:
line.partition(':')[2]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thomwiggers line.partition(':')[2]
yes, I have no problem with that.
if line.startswith("Provides"): | ||
pkg_provides = [x.strip().lower() for x in line.split(':', 1)[1].strip().split()] | ||
if line.startswith("Version"): | ||
pkg_version = line.split(':', 1)[1].strip() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pkg_version = line.split(':', 1)[1].strip() | |
pkg_version = line.partition(':')[-1].strip() |
if line.startswith("Version"): | ||
pkg_version = line.split(':', 1)[1].strip() | ||
|
||
if pkg_name == target_package or target_package in pkg_provides: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't work if pkg_provides contain a version:
max@mde-oxalide % pacman -Qqei perl | grep Provide
Provides : perl-archive-tar=2.30 perl-attribute-handlers=1.01 perl-autodie=2.29 perl-autoloader=5.74 perl-autouse=1.11 perl-b-debug=1.26 perl-base=2.27 perl-bignum=0.49 perl-carp=1.50 perl-compress-raw-bzip2=2.074 perl-compress-raw-zlib=2.076 perl-config-perl-v=0.29 perl-constant=1.33 perl-cpan-meta-requirements=2.140 perl-cpan-meta-yaml=0.018 perl-cpan-meta=2.150010 perl-cpan=2.20 perl-data-dumper=2.170 perl-db_file=1.840 perl-devel-ppport=3.40 perl-devel-selfstubber=1.06 perl-digest-md5=2.55 perl-digest-sha=6.01 perl-digest=1.17_01 perl-dumpvalue=1.18 perl-encode=2.97 perl-encoding-warnings=0.13 perl-env=1.04 perl-experimental=0.019 perl-exporter=5.73 perl-extutils-cbuilder=0.280230 perl-extutils-constant=0.25 perl-extutils-install=2.14 perl-extutils-makemaker=7.34 perl-extutils-manifest=1.70 perl-extutils-parsexs=3.39 perl-file-fetch=0.56 perl-file-path=2.15 perl-file-temp=0.2304 perl-filter-simple=0.95 perl-filter-util-call=1.58 perl-getopt-long=2.5 perl-http-tiny=0.070 perl-i18n-collate=1.02 perl-i18n-langtags=0.43 perl-if=0.0608 perl-io-compress=2.074 perl-io-socket-ip=0.39 perl-io-zlib=1.10 perl-io=1.39 perl-ipc-cmd=1.00 perl-ipc-sysv=2.07 perl-json-pp=2.97001 perl-lib=0.64 perl-libnet=3.11 perl-locale-codes=3.56 perl-locale-maketext-simple=0.21_01 perl-locale-maketext=1.29 perl-math-bigint-fastcalc=0.5006 perl-math-bigint=1.999811 perl-math-bigrat=0.2613 perl-math-complex=1.5901 perl-memoize=1.03_01 perl-mime-base64=3.15 perl-module-corelist=5.20180622 perl-module-load-conditional=0.68 perl-module-load=0.32 perl-module-loaded=0.08 perl-module-metadata=1.000033 perl-net-ping=2.62 perl-params-check=0.38 perl-parent=0.236 perl-pathtools=3.74 perl-perl-ostype=1.010 perl-perlfaq=5.021011 perl-perlio-via-quotedprint=0.08 perl-pod-checker=1.73 perl-pod-escapes=1.07 perl-pod-parser=1.63 perl-pod-perldoc=3.2801 perl-pod-simple=3.35 perl-pod-usage=1.69 perl-podlators=5.006 perl-safe=2.40 perl-scalar-list-utils=1.50 perl-search-dict=1.07 perl-selfloader=1.25 perl-socket=2.027 perl-storable=3.08 perl-sys-syslog=0.35 perl-term-ansicolor=4.06 perl-term-cap=1.17 perl-term-complete=1.403 perl-term-readline=1.17 perl-test-harness=3.42 perl-test-simple=1.302133 perl-test=1.31 perl-text-abbrev=1.02 perl-text-balanced=2.03 perl-text-parsewords=3.30 perl-text-tabs=2013.0523 perl-thread-queue=3.12 perl-thread-semaphore=2.13 perl-threads-shared=1.58 perl-threads=2.22 perl-tie-file=1.02 perl-tie-refhash=1.39 perl-time-hires=1.9759 perl-time-local=1.25 perl-time-piece=1.3204 perl-unicode-collate=1.25 perl-unicode-normalize=1.26 perl-version=0.9923 perl-xsloader=0.30
pkg_version = line.split(':', 1)[1].strip() | ||
|
||
if pkg_name == target_package or target_package in pkg_provides: | ||
if not pkg_version: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible ?
For me it would mean that it exist a package without version.
lversion = get_version(lstdout) | ||
|
||
rcmd = "%s -Si %s" % (pacman_path, name) | ||
rcmd = "%s -Si %s" % (pacman_path, realname) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rcmd = "%s -Si %s" % (pacman_path, realname) | |
rcmd = "%s --sync --info %s" % (pacman_path, realname) |
Thank you very much for your interest in Ansible. Ansible has migrated much of the content into separate repositories to allow for more rapid, independent development. We are closing this issue/PR because this content has been moved to one or more collection repositories.
For further information, please see: |
SUMMARY
Packages that are provided by a different package (example: 'cron' is provided by 'cronie') are not recognized by the pacman module and always assumed uninstalled. In particular, this leads to crashes when trying to install the package because pacman output is not in the expected format.
If querying package information for the target package fails, we look through all installed packages and try to find one that provides the target instead, then proceed with this package's name and version.
Fixes #24110
ISSUE TYPE
COMPONENT NAME
pacman
ANSIBLE VERSION