From afd626e0b38babf40fcd6ce96684111ae987ee73 Mon Sep 17 00:00:00 2001 From: Alexander Bethke Date: Fri, 7 Dec 2018 17:38:42 +0100 Subject: [PATCH] Fix two regressions in flatpak and flatpak_remote modules (#49651) * Fix python 3 compatibility in flatpak_remote module * Fix flatpak module for recent versions of flatpak * Add reminder for contributors to run integration tests manually There has been several Python version compatibility regressions introduced into the module which could have been avoided by the integration tests. (cherry picked from commit 27fd8e4dc6a45c81b418647c1b8bd97a52ee8755) --- lib/ansible/modules/packaging/os/flatpak.py | 30 ++++++++++++++++--- .../modules/packaging/os/flatpak_remote.py | 30 ++++++++++++++++--- 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/lib/ansible/modules/packaging/os/flatpak.py b/lib/ansible/modules/packaging/os/flatpak.py index 42de3b82355..49efc35ee90 100644 --- a/lib/ansible/modules/packaging/os/flatpak.py +++ b/lib/ansible/modules/packaging/os/flatpak.py @@ -6,6 +6,27 @@ # Copyright: (c) 2017 Ansible Project # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +# ATTENTION CONTRIBUTORS! +# +# TL;DR: Run this module's integration tests manually before opening a pull request +# +# Long explanation: +# The integration tests for this module are currently NOT run on the Ansible project's continuous +# delivery pipeline. So please: When you make changes to this module, make sure that you run the +# included integration tests manually for both Python 2 and Python 3: +# +# Python 2: +# ansible-test integration -v --docker fedora28 --docker-privileged --allow-unsupported --python 2.7 flatpak +# Python 3: +# ansible-test integration -v --docker fedora28 --docker-privileged --allow-unsupported --python 3.6 flatpak +# +# Because of external dependencies, the current integration tests are somewhat too slow and brittle +# to be included right now. I have plans to rewrite the integration tests based on a local flatpak +# repository so that they can be included into the normal CI pipeline. +# //oolongbrothers + + from __future__ import (absolute_import, division, print_function) __metaclass__ = type @@ -22,7 +43,7 @@ description: - Allows users to add or remove flatpaks. - See the M(flatpak_remote) module for managing flatpak remotes. author: -- John Kwiatkoski (@jaykayy) +- John Kwiatkoski (@JayKayy) - Alexander Bethke (@oolongbrothers) requirements: - flatpak @@ -82,7 +103,7 @@ EXAMPLES = r''' flatpak: name: org.gnome.gedit state: present - method: user + method: user - name: Install the Gnome Calendar flatpak from the gnome remote system-wide flatpak: @@ -127,6 +148,7 @@ stdout: import subprocess from ansible.module_utils.six.moves.urllib.parse import urlparse from ansible.module_utils.basic import AnsibleModule +from ansible.module_utils._text import to_native def install_flat(module, binary, remote, name, method): @@ -144,7 +166,7 @@ def uninstall_flat(module, binary, name, method): """Remove an existing flatpak.""" global result installed_flat_name = _match_installed_flat_name(module, binary, name, method) - command = "{0} uninstall --{1} {2}".format(binary, method, installed_flat_name) + command = "{0} uninstall -y --{1} {2}".format(binary, method, installed_flat_name) _flatpak_command(module, module.check_mode, command) result['changed'] = True @@ -203,7 +225,7 @@ def _flatpak_command(module, noop, command): result['stderr'] = stderr_data if result['rc'] != 0: module.fail_json(msg="Failed to execute flatpak command", **result) - return stdout_data + return to_native(stdout_data) def main(): diff --git a/lib/ansible/modules/packaging/os/flatpak_remote.py b/lib/ansible/modules/packaging/os/flatpak_remote.py index 8425e1b2190..508af3c927e 100644 --- a/lib/ansible/modules/packaging/os/flatpak_remote.py +++ b/lib/ansible/modules/packaging/os/flatpak_remote.py @@ -6,6 +6,27 @@ # Copyright: (c) 2017 Ansible Project # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +# ATTENTION CONTRIBUTORS! +# +# TL;DR: Run this module's integration tests manually before opening a pull request +# +# Long explanation: +# The integration tests for this module are currently NOT run on the Ansible project's continuous +# delivery pipeline. So please: When you make changes to this module, make sure that you run the +# included integration tests manually for both Python 2 and Python 3: +# +# Python 2: +# ansible-test integration -v --docker fedora28 --docker-privileged --allow-unsupported --python 2.7 flatpak_remote +# Python 3: +# ansible-test integration -v --docker fedora28 --docker-privileged --allow-unsupported --python 3.6 flatpak_remote +# +# Because of external dependencies, the current integration tests are somewhat too slow and brittle +# to be included right now. I have plans to rewrite the integration tests based on a local flatpak +# repository so that they can be included into the normal CI pipeline. +# //oolongbrothers + + from __future__ import (absolute_import, division, print_function) __metaclass__ = type @@ -26,7 +47,7 @@ description: - Existing remotes will not be updated. - See the M(flatpak) module for managing flatpaks. author: -- John Kwiatkoski (@jaykayy) +- John Kwiatkoski (@JayKayy) - Alexander Bethke (@oolongbrothers) requirements: - flatpak @@ -120,6 +141,7 @@ stdout: import subprocess from ansible.module_utils.basic import AnsibleModule +from ansible.module_utils._text import to_bytes, to_native def add_remote(module, binary, name, flatpakrepo_url, method): @@ -147,7 +169,7 @@ def remote_exists(module, binary, name, method): output = _flatpak_command(module, False, command) for line in output.splitlines(): listed_remote = line.split() - if listed_remote[0] == name: + if listed_remote[0] == to_native(name): return True return False @@ -168,7 +190,7 @@ def _flatpak_command(module, noop, command): result['stderr'] = stderr_data if result['rc'] != 0: module.fail_json(msg="Failed to execute flatpak command", **result) - return stdout_data + return to_native(stdout_data) def main(): @@ -205,7 +227,7 @@ def main(): if not binary: module.fail_json(msg="Executable '%s' was not found on the system." % executable, **result) - remote_already_exists = remote_exists(module, binary, bytes(name, 'utf-8'), method) + remote_already_exists = remote_exists(module, binary, to_bytes(name), method) if state == 'present' and not remote_already_exists: add_remote(module, binary, name, flatpakrepo_url, method)