From b89b688d52cdcb54694c84c110a866057e6ea7a2 Mon Sep 17 00:00:00 2001 From: Strahinja Kustudic Date: Mon, 20 Aug 2018 15:26:12 +0200 Subject: [PATCH] Fix pip idempotence in check mode PIP package names must be case insensitive, and must consider hyphens and underscores to be equivalent (https://www.python.org/dev/peps/pep-0426/#name), because of this the module didn't work correctly in check mode. For example if the passed package name had a different case or an underscore instead of a hyphen (or the other way around) compared to the installed package, check mode reported as changed, even though packages were installed. Now the module ignores case and hyphens/underscores in package names, so check mode works correctly. --- .../pip-fix-idempotence-in-check-mode.yaml | 2 ++ lib/ansible/modules/packaging/language/pip.py | 12 +++++++++-- test/integration/targets/pip/tasks/pip.yml | 21 +++++++++++++++++++ 3 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 changelogs/fragments/pip-fix-idempotence-in-check-mode.yaml diff --git a/changelogs/fragments/pip-fix-idempotence-in-check-mode.yaml b/changelogs/fragments/pip-fix-idempotence-in-check-mode.yaml new file mode 100644 index 00000000000..f3cc2ffd0a3 --- /dev/null +++ b/changelogs/fragments/pip-fix-idempotence-in-check-mode.yaml @@ -0,0 +1,2 @@ +bugfixes: +- "pip - idempotence in check mode now works correctly." diff --git a/lib/ansible/modules/packaging/language/pip.py b/lib/ansible/modules/packaging/language/pip.py index 8683a407545..9b39cd3fd33 100644 --- a/lib/ansible/modules/packaging/language/pip.py +++ b/lib/ansible/modules/packaging/language/pip.py @@ -350,10 +350,11 @@ def _is_present(module, req, installed_pkgs, pkg_command): for pkg in installed_pkgs: if '==' in pkg: pkg_name, pkg_version = pkg.split('==') + pkg_name = Package.canonicalize_name(pkg_name) else: continue - if pkg_name.lower() == req.package_name and req.is_satisfied_by(pkg_version): + if pkg_name == req.package_name and req.is_satisfied_by(pkg_version): return True return False @@ -499,6 +500,8 @@ class Package: test whether a package is already satisfied. """ + _CANONICALIZE_RE = re.compile(r'[-_.]+') + def __init__(self, name_string, version_string=None): self._plain_package = False self.package_name = name_string @@ -515,7 +518,7 @@ class Package: self.package_name = "setuptools" self._requirement.project_name = "setuptools" else: - self.package_name = self._requirement.project_name + self.package_name = Package.canonicalize_name(self._requirement.project_name) self._plain_package = True except ValueError as e: pass @@ -539,6 +542,11 @@ class Package: for op, ver in self._requirement.specs ) + @staticmethod + def canonicalize_name(name): + # This is taken from PEP 503. + return Package._CANONICALIZE_RE.sub("-", name).lower() + def __str__(self): if self._plain_package: return to_native(self._requirement) diff --git a/test/integration/targets/pip/tasks/pip.yml b/test/integration/targets/pip/tasks/pip.yml index dd84724f262..b7036de99de 100644 --- a/test/integration/targets/pip/tasks/pip.yml +++ b/test/integration/targets/pip/tasks/pip.yml @@ -213,6 +213,27 @@ that: - "not (q_check_mode is changed)" +# Case with package name that has a different package name case and an +# underscore instead of a hyphen +- name: check for Junit-XML package + pip: + name: Junit-XML + virtualenv: "{{ output_dir }}/pipenv" + state: present + +- name: check for Junit-XML package in check_mode + pip: + name: Junit-XML + virtualenv: "{{ output_dir }}/pipenv" + state: present + check_mode: True + register: diff_case_check_mode + +- name: make sure Junit-XML in check_mode doesn't report changed + assert: + that: + - "diff_case_check_mode is not changed" + # ansible#23204 - name: ensure is a fresh virtualenv file: