Merge pull request #20389 from jfindlay/fix_pip_venv

fix pip venv regression, deprecate activate param

doc/topics/releases/2014.7.2.rst

@@ -15,3 +15,7 @@ Version 2014.7.2 is a bugfix release for :doc:`2014.7.0
   to lowercase their npm package names for them.  The :py:mod:`npm module
   <salt.modules.npm>` was never affected by mandatory lowercasing.
   (:issue:`20329`)
+- Deprecate the `activate` parameter for pip.install for both the
+  :py:mod:`module <salt.modules.pip>` and the :py:mod:`state <salt.state.pip>`.
+  If `bin_env` is given and points to a virtualenv, there is no need to
+  activate that virtualenv in a shell for pip to install to the virtualenv.

salt/modules/pip.py

@@ -72,23 +72,6 @@ def _get_cached_requirements(requirements, saltenv):
     return cached_requirements
 
 
-def _get_env_activate(bin_env):
-    '''
-    Return the path to the activate binary
-    '''
-    if not bin_env:
-        raise CommandNotFoundError('Could not find a `activate` binary')
-
-    if os.path.isdir(bin_env):
-        if salt.utils.is_windows():
-            activate_bin = os.path.join(bin_env, 'Scripts', 'activate.bat')
-        else:
-            activate_bin = os.path.join(bin_env, 'bin', 'activate')
-        if os.path.isfile(activate_bin):
-            return activate_bin
-    raise CommandNotFoundError('Could not find a `activate` binary')
-
-
 def install(pkgs=None,  # pylint: disable=R0912,R0913,R0914
             requirements=None,
             env=None,
@@ -230,6 +213,11 @@ def install(pkgs=None,  # pylint: disable=R0912,R0913,R0914
     activate
         Activates the virtual environment, if given via bin_env,
         before running install.
+
+        .. deprecated:: 2014.7.2
+            If `bin_env` is given, pip will already be sourced from that
+            virualenv, making `activate` effectively a noop.
+
     pre_releases
         Include pre-releases in the available versions
     cert
@@ -272,6 +260,14 @@ def install(pkgs=None,  # pylint: disable=R0912,R0913,R0914
         )
         bin_env = env
 
+    if activate:
+        salt.utils.warn_until(
+                'Boron',
+                'Passing \'activate\' to the pip module is deprecated. If '
+                'bin_env refers to a virtualenv, there is no need to activate '
+                'that virtualenv before using pip to install packages in it.'
+        )
+
     if isinstance(__env__, string_types):
         salt.utils.warn_until(
             'Boron',
@@ -303,10 +299,6 @@ def install(pkgs=None,  # pylint: disable=R0912,R0913,R0914
 
     cmd = [_get_pip_bin(bin_env), 'install']
 
-    if activate and bin_env:
-        if not salt.utils.is_windows():
-            cmd = ['.', _get_env_activate(bin_env), '&&'] + cmd
-
     cleanup_requirements = []
     if requirements is not None:
         if isinstance(requirements, string_types):

salt/states/pip_state.py

@@ -243,6 +243,10 @@ def installed(name,
         Activates the virtual environment, if given via bin_env,
         before running install.
 
+        .. deprecated:: 2014.7.2
+            If `bin_env` is given, pip will already be sourced from that
+            virualenv, making `activate` effectively a noop.
+
     pre_releases
         Include pre-releases in the available versions
 

tests/unit/modules/pip_test.py

@@ -298,7 +298,7 @@ class PipTestCase(TestCase):
             )
 
     @patch('os.path')
-    def test_install_fix_activate_env(self, mock_path):
+    def test_install_venv(self, mock_path):
         mock_path.is_file.return_value = True
         mock_path.isdir.return_value = True
 
@@ -307,9 +307,9 @@ class PipTestCase(TestCase):
         mock_path.join = join
         mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
         with patch.dict(pip.__salt__, {'cmd.run_all': mock}):
-            pip.install('mock', bin_env='/test_env', activate=True)
+            pip.install('mock', bin_env='/test_env')
             mock.assert_called_once_with(
-                '. /test_env/bin/activate && /test_env/bin/pip install '
+                '/test_env/bin/pip install '
                 '\'mock\'',
                 env={'VIRTUAL_ENV': '/test_env'},
                 saltenv='base',