Allow version specifiers for pip install (#41792)

Allow version specifiers for pip install.

lib/ansible/modules/packaging/language/pip.py

@@ -22,8 +22,8 @@ version_added: "0.7"
 options:
   name:
     description:
-      - The name of a Python library to install or the url of the remote package.
+      - The name of a Python library to install or the url(bzr+,hg+,git+,svn+) of the remote package.
-      - As of 2.2 you can supply a list of names.
+      - This can be a list (since 2.2) and contain version specifiers (since 2.7).
   version:
     description:
       - The version number to install of the Python library specified in the I(name) parameter.
@@ -111,6 +111,7 @@ notes:
 requirements:
 - pip
 - virtualenv
+- setuptools
 author:
 - Matt Wright (@mattupstate)
 '''
@@ -122,8 +123,17 @@ EXAMPLES = '''
 
 # Install (Bottle) python package on version 0.11.
 - pip:
-    name: bottle
+    name: bottle==0.11
-    version: 0.11
+
+# Install (bottle) python package with version specifiers
+- pip:
+    name: bottle>0.10,<0.20,!=0.11
+
+# Install multi python packages with version specifiers
+- pip:
+    name:
+      - django>1.11.0,<1.12.0
+      - bottle>0.10,<0.20,!=0.11
 
 # Install (MyApp) using one of the remote protocols (bzr+,hg+,git+,svn+). You do not have to supply '-e' option in extra_args.
 - pip:
@@ -222,6 +232,16 @@ import os
 import re
 import sys
 import tempfile
+import operator
+import shlex
+from distutils.version import LooseVersion
+
+try:
+    from pkg_resources import Requirement
+
+    HAS_SETUPTOOLS = True
+except ImportError:
+    HAS_SETUPTOOLS = False
 
 from ansible.module_utils.basic import AnsibleModule, is_executable
 from ansible.module_utils._text import to_native
@@ -234,6 +254,53 @@ from ansible.module_utils.six import PY3
 _SPECIAL_PACKAGE_CHECKERS = {'setuptools': 'import setuptools; print(setuptools.__version__)',
                              'pip': 'import pkg_resources; print(pkg_resources.get_distribution("pip").version)'}
 
+_VCS_RE = re.compile(r'(svn|git|hg|bzr)\+')
+
+op_dict = {">=": operator.ge, "<=": operator.le, ">": operator.gt,
+           "<": operator.lt, "==": operator.eq, "!=": operator.ne, "~=": operator.ge}
+
+
+def _is_vcs_url(name):
+    """Test whether a name is a vcs url or not."""
+    return re.match(_VCS_RE, name)
+
+
+def _is_package_name(name):
+    """Test whether the name is a package name or a version specifier."""
+    return not name.lstrip().startswith(tuple(op_dict.keys()))
+
+
+def _recover_package_name(names):
+    """Recover package names as list from user's raw input.
+
+    :input: a mixed and invalid list of names or version specifiers
+    :return: a list of valid package name
+
+    eg.
+    input: ['django>1.11.1', '<1.11.3', 'ipaddress', 'simpleproject>1.1.0', '<2.0.0']
+    return: ['django>1.11.1,<1.11.3', 'ipaddress', 'simpleproject>1.1.0,<2.0.0']
+
+    input: ['django>1.11.1,<1.11.3,ipaddress', 'simpleproject>1.1.0,<2.0.0']
+    return: ['django>1.11.1,<1.11.3', 'ipaddress', 'simpleproject>1.1.0,<2.0.0']
+    """
+    # rebuild input name to a flat list so we can tolerate any combination of input
+    tmp = []
+    for one_line in names:
+        tmp.extend(one_line.split(","))
+    names = tmp
+
+    # reconstruct the names
+    name_parts = []
+    package_names = []
+    for name in names:
+        if _is_package_name(name):
+            if name_parts:
+                package_names.append(",".join(name_parts))
+            name_parts = []
+        name_parts.append(name)
+    package_names.append(",".join(name_parts))
+    return package_names
+
 
 def _get_cmd_options(module, cmd):
     thiscmd = cmd + " --help"
@@ -246,19 +313,11 @@ def _get_cmd_options(module, cmd):
     return cmd_options
 
 
-def _get_full_name(name, version=None):
-    if version is None or version == "":
-        resp = name
-    else:
-        resp = name + '==' + version
-    return resp
-
-
 def _get_packages(module, pip, chdir):
     '''Return results of pip command to get packages.'''
     # Try 'pip list' command first.
     command = '%s list --format=freeze' % pip
-    lang_env = dict(LANG='C', LC_ALL='C', LC_MESSAGES='C')
+    lang_env = {'LANG': 'C', 'LC_ALL': 'C', 'LC_MESSAGES': 'C'}
     rc, out, err = module.run_command(command, cwd=chdir, environ_update=lang_env)
 
     # If there was an error (pip version too old) then use 'pip freeze'.
@@ -268,10 +327,10 @@ def _get_packages(module, pip, chdir):
         if rc != 0:
             _fail(module, command, out, err)
 
-    return (command, out, err)
+    return command, out, err
 
 
-def _is_present(name, version, installed_pkgs, pkg_command):
+def _is_present(module, req, installed_pkgs, pkg_command):
     '''Return whether or not package is installed.'''
     for pkg in installed_pkgs:
         if '==' in pkg:
@@ -279,7 +338,7 @@ def _is_present(name, version, installed_pkgs, pkg_command):
         else:
             continue
 
-        if pkg_name == name and (version is None or version == pkg_version):
+        if pkg_name.lower() == req.package_name and req.is_satisfied_by(pkg_version):
             return True
 
     return False
@@ -417,12 +476,65 @@ def setup_virtualenv(module, env, chdir, out, err):
     return out, err
 
 
+class Package:
+    """Python distribution package metadata wrapper.
+
+    A wrapper class for Requirement, which provides
+    API to parse package name, version specifier,
+    test whether a package is already satisfied.
+    """
+
+    def __init__(self, name_string, version_string=None):
+        self._plain_package = False
+        self.package_name = name_string
+        self._requirement = None
+
+        if version_string:
+            version_string = version_string.lstrip()
+            separator = '==' if version_string[0].isdigit() else ' '
+            name_string = separator.join((name_string, version_string))
+        try:
+            self._requirement = Requirement.parse(name_string)
+            # old pkg_resource will replace 'setuptools' with 'distribute' when it already installed
+            if self._requirement.project_name == "distribute":
+                self.package_name = "setuptools"
+            else:
+                self.package_name = self._requirement.project_name
+            self._plain_package = True
+        except ValueError as e:
+            pass
+
+    @property
+    def has_version_specifier(self):
+        if self._plain_package:
+            return bool(self._requirement.specs)
+        return False
+
+    def is_satisfied_by(self, version_to_test):
+        if not self._plain_package:
+            return False
+        try:
+            return self._requirement.specifier.contains(version_to_test)
+        except AttributeError:
+            # old setuptools has no specifier, do fallback
+            version_to_test = LooseVersion(version_to_test)
+            return all(
+                op_dict[op](version_to_test, LooseVersion(ver))
+                for op, ver in self._requirement.specs
+            )
+
+    def __str__(self):
+        if self._plain_package:
+            return to_native(self._requirement)
+        return self.package_name
+
+
 def main():
     state_map = dict(
-        present='install',
+        present=['install'],
-        absent='uninstall -y',
+        absent=['uninstall', '-y'],
-        latest='install -U',
+        latest=['install', '-U'],
-        forcereinstall='install -U --force-reinstall',
+        forcereinstall=['install', '-U', '--force-reinstall'],
     )
 
     module = AnsibleModule(
@@ -447,6 +559,9 @@ def main():
         supports_check_mode=True,
     )
 
+    if not HAS_SETUPTOOLS:
+        module.fail_json(msg="No setuptools found in remote host, please install it first.")
+
     state = module.params['state']
     name = module.params['name']
     version = module.params['version']
@@ -488,7 +603,7 @@ def main():
 
         pip = _get_pip(module, env, module.params['executable'])
 
-        cmd = '%s %s' % (pip, state_map[state])
+        cmd = [pip] + state_map[state]
 
         # If there's a virtualenv we want things we install to be able to use other
         # installations that exist as binaries within this virtualenv. Example: we
@@ -505,10 +620,27 @@ def main():
         has_vcs = False
         if name:
             for pkg in name:
-                if bool(pkg and re.match(r'(svn|git|hg|bzr)\+', pkg)):
+                if pkg and _is_vcs_url(pkg):
                     has_vcs = True
                     break
 
+            # convert raw input package names to Package instances
+            packages = [Package(pkg) for pkg in _recover_package_name(name)]
+            # check invalid combination of arguments
+            if version is not None:
+                if len(packages) > 1:
+                    module.fail_json(
+                        msg="'version' argument is ambiguous when installing multiple package distributions. "
+                            "Please specify version restrictions next to each package in 'name' argument."
+                    )
+                if packages[0].has_version_specifier:
+                    module.fail_json(
+                        msg="The 'version' argument conflicts with any version specifier provided along with a package name. "
+                            "Please keep the version specifier, but remove the 'version' argument."
+                    )
+                # if the version specifier is provided by version, append that into the package
+                packages[0] = Package(packages[0].package_name, version)
+
         if module.params['editable']:
             args_list = []  # used if extra_args is not used at all
             if extra_args:
@@ -519,13 +651,12 @@ def main():
                 extra_args = ' '.join(args_list)
 
         if extra_args:
-            cmd += ' %s' % extra_args
+            cmd.extend(shlex.split(extra_args))
 
         if name:
-            for pkg in name:
+            cmd.extend(to_native(p) for p in packages)
-                cmd += ' %s' % _get_full_name(pkg, version)
         elif requirements:
-            cmd += ' -r %s' % requirements
+            cmd.extend(['-r', requirements])
         else:
             module.exit_json(
                 changed=False,
@@ -556,8 +687,8 @@ def main():
                                 pkg_list.append(formatted_dep)
                                 out += '%s\n' % formatted_dep
 
-                for pkg in name:
+                for package in packages:
-                    is_present = _is_present(pkg, version, pkg_list, pkg_cmd)
+                    is_present = _is_present(module, package, pkg_list, pkg_cmd)
                     if (state == 'present' and not is_present) or (state == 'absent' and is_present):
                         changed = True
                         break

test/integration/targets/pip/tasks/pip.yml

@@ -315,3 +315,162 @@
   assert:
     that:
       - pip_install_empty_version_string is successful
+
+# test version specifiers
+- name: make sure no test_package installed now
+  pip:
+    name: "{{ pip_test_packages }}"
+    state: absent
+
+- name: install package with version specifiers
+  pip:
+    name: "{{ pip_test_package }}"
+    version: "<100,!=1.0,>0.0.0"
+  register: version
+
+- name: assert package installed correctly
+  assert:
+    that: "version.changed"
+
+- name: reinstall package
+  pip:
+    name: "{{ pip_test_package }}"
+    version: "<100,!=1.0,>0.0.0"
+  register: version2
+
+- name: assert no changes ocurred
+  assert:
+    that: "not version2.changed"
+
+- name: test the check_mod
+  pip:
+    name: "{{ pip_test_package }}"
+    version: "<100,!=1.0,>0.0.0"
+  check_mode: yes
+  register: version3
+
+- name: assert no changes
+  assert:
+    that: "not version3.changed"
+
+- name: test the check_mod with unsatisfied version
+  pip:
+    name: "{{ pip_test_package }}"
+    version: ">100.0.0"
+  check_mode: yes
+  register: version4
+
+- name: assert changed
+  assert:
+    that: "version4.changed"
+
+- name: uninstall test packages for next test
+  pip:
+    name: "{{ pip_test_packages }}"
+    state: absent
+
+- name: test invalid combination of arguments
+  pip:
+    name: "{{ pip_test_pkg_ver }}"
+    version: "1.11.1"
+  ignore_errors: yes
+  register: version5
+
+- name: assert the invalid combination should fail
+  assert:
+    that: "version5 is failed"
+
+- name: another invalid combination of arguments
+  pip:
+    name: "{{ pip_test_pkg_ver[0] }}"
+    version: "<100.0.0"
+  ignore_errors: yes
+  register: version6
+
+- name: assert invalid combination should fail
+  assert:
+    that: "version6 is failed"
+
+- name: try to install invalid package
+  pip:
+    name: "{{ pip_test_pkg_ver_unsatisfied }}"
+  ignore_errors: yes
+  register: version7
+
+- name: assert install should fail
+  assert:
+    that: "version7 is failed"
+
+- name: test install multi-packages with version specifiers
+  pip:
+    name: "{{ pip_test_pkg_ver }}"
+  register: version8
+
+- name: assert packages installed correctly
+  assert:
+    that: "version8.changed"
+
+- name: test install multi-packages with check_mode
+  pip:
+    name: "{{ pip_test_pkg_ver }}"
+  check_mode: yes
+  register: version9
+
+- name: assert no change
+  assert:
+    that: "not version9.changed"
+
+- name: test install unsatisfied multi-packages with check_mode
+  pip:
+    name: "{{ pip_test_pkg_ver_unsatisfied }}"
+  check_mode: yes
+  register: version10
+
+- name: assert changes needed
+  assert:
+    that: "version10.changed"
+
+- name: uninstall packages for next test
+  pip:
+    name: "{{ pip_test_packages }}"
+    state: absent
+
+- name: test install multi package provided by one single string
+  pip:
+    name: "{{pip_test_pkg_ver[0]}},{{pip_test_pkg_ver[1]}}"
+  register: version11
+
+- name: assert the install ran correctly
+  assert:
+    that: "version11.changed"
+
+- name: test install multi package provided by one single string with check_mode
+  pip:
+    name: "{{pip_test_pkg_ver[0]}},{{pip_test_pkg_ver[1]}}"
+  check_mode: yes
+  register: version12
+
+- name: assert no changes needed
+  assert:
+    that: "not version12.changed"
+
+- name: test module can parse the combination of multi-packages one line and git url
+  pip:
+    name:
+      - git+https://github.com/dvarrazzo/pyiso8601#egg=pyiso8601
+      - "{{pip_test_pkg_ver[0]}},{{pip_test_pkg_ver[1]}}"
+
+- name: test the invalid package name
+  pip:
+    name: djan=+-~!@#$go>1.11.1,<1.11.3
+  ignore_errors: yes
+  register: version13
+
+- name: the invalid package should make module failed
+  assert:
+    that: "version13 is failed"
+
+- name: clean up
+  pip:
+    name: "{{ pip_test_packages }}"
+    state: absent

test/integration/targets/pip/vars/main.yml

@@ -2,6 +2,12 @@ pip_test_package: sampleproject
 pip_test_packages:
   - sampleproject
   - decorator
+pip_test_pkg_ver:
+  - sampleproject<=100, !=9.0.0,>=0.0.1 
+  - decorator<100 ,!=9,>=0.0.1
+pip_test_pkg_ver_unsatisfied:
+  - sampleproject>= 999.0.0
+  - decorator >999.0
 pip_test_modules:
   - sample
   - decorator

test/units/modules/packaging/language/test_pip.py

@@ -22,3 +22,14 @@ def test_failure_when_pip_absent(mocker, capfd):
     results = json.loads(out)
     assert results['failed']
     assert 'pip needs to be installed' in results['msg']
+
+
+@pytest.mark.parametrize('patch_ansible_module, test_input, expected', [
+    [None, ['django>1.11.1', '<1.11.2', 'ipaddress', 'simpleproject<2.0.0', '>1.1.0'],
+        ['django>1.11.1,<1.11.2', 'ipaddress', 'simpleproject<2.0.0,>1.1.0']],
+    [None, ['django>1.11.1,<1.11.2,ipaddress', 'simpleproject<2.0.0,>1.1.0'],
+        ['django>1.11.1,<1.11.2', 'ipaddress', 'simpleproject<2.0.0,>1.1.0']],
+    [None, ['django>1.11.1', '<1.11.2', 'ipaddress,simpleproject<2.0.0,>1.1.0'],
+        ['django>1.11.1,<1.11.2', 'ipaddress', 'simpleproject<2.0.0,>1.1.0']]])
+def test_recover_package_name(test_input, expected):
+    assert pip._recover_package_name(test_input) == expected