Merge pull request #397 from PyAr/automatic-pip-upgrade

Get pip automatically upgraded to latest version (unless explicitly avoided)

Author: facundobatista

fades/envbuilder.py

@@ -121,7 +121,7 @@ def post_setup(self, context):
         self.env_bin_path = context.bin_path
 
 
-def create_venv(requested_deps, interpreter, is_current, options, pip_options):
+def create_venv(requested_deps, interpreter, is_current, options, pip_options, avoid_pip_upgrade):
     """Create a new virtualvenv with the requirements of this script."""
     # create virtualenv
     env = _FadesEnvBuilder()
@@ -135,7 +135,9 @@ def create_venv(requested_deps, interpreter, is_current, options, pip_options):
     installed = {}
     for repo in requested_deps.keys():
         if repo in (REPO_PYPI, REPO_VCS):
-            mgr = PipManager(env_bin_path, pip_installed=pip_installed, options=pip_options)
+            mgr = PipManager(
+                env_bin_path, pip_installed=pip_installed, options=pip_options,
+                avoid_pip_upgrade=avoid_pip_upgrade)
         else:
             logger.warning("Install from %r not implemented", repo)
             continue

fades/main.py

@@ -271,6 +271,10 @@ def go():
         '--freeze', action='store', metavar='FILEPATH',
         help="dump all the dependencies and its versions to the specified filepath "
              "(operating normally beyond that)")
+    parser.add_argument(
+        '--avoid-pip-upgrade', action='store_true',
+        help="disable the automatic pip upgrade that happens after the virtualenv is created "
+             "and before the dependencies begin to be installed.")
 
     mutexg = parser.add_mutually_exclusive_group()
     mutexg.add_argument(
@@ -416,7 +420,7 @@ def go():
 
         # Create a new venv
         venv_data, installed = envbuilder.create_venv(
-            indicated_deps, args.python, is_current, options, pip_options)
+            indicated_deps, args.python, is_current, options, pip_options, args.avoid_pip_upgrade)
         # store this new venv in the cache
         venvscache.store(installed, venv_data, interpreter, options)
 

fades/pipmanager.py

@@ -38,14 +38,15 @@
 class PipManager():
     """A manager for all PIP related actions."""
 
-    def __init__(self, env_bin_path, pip_installed=False, options=None):
+    def __init__(self, env_bin_path, pip_installed=False, options=None, avoid_pip_upgrade=False):
         """Init."""
         self.env_bin_path = env_bin_path
         self.pip_installed = pip_installed
         self.options = options
         self.pip_exe = os.path.join(self.env_bin_path, "pip")
         basedir = helpers.get_basedir()
         self.pip_installer_fname = os.path.join(basedir, "get-pip.py")
+        self.avoid_pip_upgrade = avoid_pip_upgrade
 
     def install(self, dependency):
         """Install a new dependency."""
@@ -54,6 +55,12 @@ def install(self, dependency):
                         "doing it manually (just wait a little, all should go well)")
             self._brute_force_install_pip()
 
+        # Always update pip to get latest behaviours (specially regarding security); this has
+        # the nice side effect of getting logged the pip version that is used.
+        if not self.avoid_pip_upgrade:
+            python_exe = os.path.join(self.env_bin_path, "python")
+            helpers.logged_exec([python_exe, '-m', 'pip', 'install', 'pip', '--upgrade'])
+
         # split to pass several tokens on multiword dependency (this is very specific for '-e' on
         # external requirements, but implemented generically; note that this does not apply for
         # normal reqs, because even if it originally is 'foo > 1.2', after parsing it loses the

man/fades.1

@@ -26,6 +26,7 @@ fades - A system that automatically handles the virtualenvs in the cases normall
 [\fB-a\fR][\fB--autoimport\fR]
 [\fB--freeze\fR]
 [\fB-m\fR][\fB--module\fR]
+[\fB--avoid-pip-upgrade\fR]
 [child_program [child_options]]
 
 \fBfades\fR can be used to execute directly your script, or put it with a #! at your script's beginning.
@@ -137,6 +138,10 @@ Will operate exactly as without the command, but also it will dump the revisions
 .BR -m ", " --module
 Run library module as a script (same behaviour than Python's \fB-m\fR parameter).
 
+.TP
+.BR --avoid-pip-upgrade
+Disable the automatic \fBpip\fR upgrade that happens after the virtualenv is created and before the dependencies begin to be installed.
+
 
 .SH EXAMPLES
 

tests/test_envbuilder.py

@@ -21,7 +21,7 @@
 import tempfile
 import unittest
 from datetime import datetime, timedelta
-from unittest.mock import Mock, patch
+from unittest.mock import Mock, patch, call
 
 from pkg_resources import parse_requirements
 
@@ -65,6 +65,7 @@ def test_create_simple(self):
         }
         interpreter = 'python3'
         is_current = True
+        avoid_pip_upgrade = False
         options = {"virtualenv_options": [],
                    "pyvenv_options": [],
                    }
@@ -74,8 +75,8 @@ def test_create_simple(self):
                 mock_create.return_value = ('env_path', 'env_bin_path', 'pip_installed')
                 mock_mgr_c.return_value = fake_manager = self.FakeManager()
                 fake_manager.really_installed = {'dep1': 'v1', 'dep2': 'v2'}
-                venv_data, installed = envbuilder.create_venv(requested, interpreter, is_current,
-                                                              options, pip_options)
+                venv_data, installed = envbuilder.create_venv(
+                    requested, interpreter, is_current, options, pip_options, avoid_pip_upgrade)
 
         self.assertEqual(venv_data, {
             'env_bin_path': 'env_bin_path',
@@ -88,21 +89,26 @@ def test_create_simple(self):
                 'dep2': 'v2',
             }
         })
+        expected_pipmanager_call = call(
+            'env_bin_path', pip_installed='pip_installed', options=[],
+            avoid_pip_upgrade=avoid_pip_upgrade)
+        self.assertEqual(mock_mgr_c.call_args, expected_pipmanager_call)
 
     def test_create_vcs(self):
         requested = {
             REPO_VCS: [parsing.VCSDependency("someurl")]
         }
         interpreter = 'python3'
         is_current = True
+        avoid_pip_upgrade = False
         options = {"virtualenv_options": [], "pyvenv_options": []}
         pip_options = []
         with patch.object(envbuilder._FadesEnvBuilder, 'create_env') as mock_create:
             with patch.object(envbuilder, 'PipManager') as mock_mgr_c:
                 mock_create.return_value = ('env_path', 'env_bin_path', 'pip_installed')
                 mock_mgr_c.return_value = self.FakeManager()
-                venv_data, installed = envbuilder.create_venv(requested, interpreter, is_current,
-                                                              options, pip_options)
+                venv_data, installed = envbuilder.create_venv(
+                    requested, interpreter, is_current, options, pip_options, avoid_pip_upgrade)
 
         self.assertEqual(venv_data, {
             'env_bin_path': 'env_bin_path',
@@ -117,6 +123,7 @@ def test_unknown_repo(self):
         }
         interpreter = 'python3'
         is_current = True
+        avoid_pip_upgrade = False
         options = {"virtualenv_options": [],
                    "pyvenv_options": [],
                    }
@@ -125,7 +132,8 @@ def test_unknown_repo(self):
             with patch.object(envbuilder, 'PipManager') as mock_mgr_c:
                 mock_create.return_value = ('env_path', 'env_bin_path', 'pip_installed')
                 mock_mgr_c.return_value = self.FakeManager()
-                envbuilder.create_venv(requested, interpreter, is_current, options, pip_options)
+                envbuilder.create_venv(
+                    requested, interpreter, is_current, options, pip_options, avoid_pip_upgrade)
 
         self.assertLoggedWarning("Install from 'unknown' not implemented")
 
@@ -135,6 +143,7 @@ def test_non_existing_dep(self):
         }
         interpreter = 'python3'
         is_current = True
+        avoid_pip_upgrade = False
         options = {'virtualenv_options': [],
                    'pyvenv_options': [],
                    }
@@ -147,11 +156,8 @@ def test_non_existing_dep(self):
                 with patch.object(envbuilder, 'destroy_venv', spec=True) as mock_destroy:
                     with self.assertRaises(FadesError) as cm:
                         envbuilder.create_venv(
-                            requested,
-                            interpreter,
-                            is_current,
-                            options,
-                            pip_options)
+                            requested, interpreter, is_current, options, pip_options,
+                            avoid_pip_upgrade)
                     self.assertEqual(str(cm.exception), 'Dependency installation failed')
                     mock_destroy.assert_called_once_with('env_path')
 
@@ -163,6 +169,7 @@ def test_different_versions(self):
         }
         interpreter = 'python3'
         is_current = True
+        avoid_pip_upgrade = False
         options = {"virtualenv_options": [],
                    "pyvenv_options": [],
                    }
@@ -172,8 +179,8 @@ def test_different_versions(self):
                 mock_create.return_value = ('env_path', 'env_bin_path', 'pip_installed')
                 mock_mgr_c.return_value = fake_manager = self.FakeManager()
                 fake_manager.really_installed = {'dep1': 'vX', 'dep2': 'v2'}
-                _, installed = envbuilder.create_venv(requested, interpreter, is_current, options,
-                                                      pip_options)
+                _, installed = envbuilder.create_venv(
+                    requested, interpreter, is_current, options, pip_options, avoid_pip_upgrade)
 
         self.assertEqual(installed, {
             REPO_PYPI: {

tests/test_pipmanager.py

@@ -76,6 +76,19 @@ def test_install(mocker):
     pip_path = os.path.join(BIN_PATH, "pip")
     mock = mocker.patch.object(helpers, "logged_exec")
     mgr.install("foo")
+
+    # check it always upgrades pip, and then the proper install
+    python_path = os.path.join(BIN_PATH, "python")
+    c1 = mocker.call([python_path, "-m", "pip", "install", "pip", "--upgrade"])
+    c2 = mocker.call([pip_path, "install", "foo"])
+    assert mock.call_args_list == [c1, c2]
+
+
+def test_install_without_pip_upgrade(mocker):
+    mgr = PipManager(BIN_PATH, pip_installed=True, avoid_pip_upgrade=True)
+    pip_path = os.path.join(BIN_PATH, "pip")
+    mock = mocker.patch.object(helpers, "logged_exec")
+    mgr.install("foo")
     mock.assert_called_with([pip_path, "install", "foo"])
 
 
@@ -105,7 +118,7 @@ def test_install_with_options_using_equal(mocker):
 
 def test_install_raise_error(mocker, logged):
     mgr = PipManager(BIN_PATH, pip_installed=True)
-    mocker.patch.object(helpers, "logged_exec", side_effect=Exception("Kapow!"))
+    mocker.patch.object(helpers, "logged_exec", side_effect=['ok', Exception("Kapow!")])
     with pytest.raises(Exception):
         mgr.install("foo")