Repository: pybuilder/pybuilder
Issue: Recursive dependency upgrades should be subject to global version constraints

Hi,

The changes to the installation of dependencies in the latest version of pybuilder (0.11.8) seem to be causing problems for a couple of our builds.

Our project has a dependency on a specific version of scipy (0.17.0), we are not ready to upgrade to the latest version of scipy (0.18.0). 

We also have a dependency on Theano (any version), which itself also depends on scipy, however it's dependency on scipy is only >= 0.11.1.

Our pybuild script dependencies looks like:
`

```
project.depends_on('numpy')
project.depends_on('Theano')
project.depends_on('scipy', '==0.17.0')
```

`
Up until now this has worked for us because we've set the 'install_dependencies_upgrade' flag to False, so that when Theano is installed, it doesn't try to upgrade scipy.

Having looked through the code for 0.11.8 pybuilder, it looks like that flag is no longer being used, with the upgrade flag being passed through as True by default. This causes pip to try to upgrade scipy to 0.18.0 and leads to build failures due to our code not supporting the latest version of scipy.

It looks like this issue has arisen out of the fix for #340. Is there another way to prevent pip upgrading dependencies?


Changed files:
src/main/python/pybuilder/pip_common.py, src/main/python/pybuilder/plugins/python/install_dependencies_plugin.py, src/unittest/python/plugins/python/install_dependencies_plugin_tests.py, src/unittest/python/pip_utils_tests.py, src/unittest/python/pluginloader_tests.py, src/main/python/pybuilder/pip_utils.py, src/unittest/python/test_utils.py, build.py

diff --git a/build.py b/build.py
index 4979eaf..0b6a6f0 100755
--- a/build.py
+++ b/build.py
@@ -93,7 +93,7 @@ def initialize(project):
         project.build_depends_on("importlib")  # for fluentmock
 
     project.depends_on("tblib")
-    project.depends_on("pip", ">=7.0")
+    project.depends_on("pip", ">=7.1")
     project.depends_on("wheel")
 
     project.set_property("verbose", True)
diff --git a/src/main/python/pybuilder/pip_common.py b/src/main/python/pybuilder/pip_common.py
index 80cce15..2cfad0c 100644
--- a/src/main/python/pybuilder/pip_common.py
+++ b/src/main/python/pybuilder/pip_common.py
@@ -46,3 +46,8 @@ def _pip_disallows_insecure_packages_by_default():
     # nor will it install externally hosted files by default
     # Also pip v1.1 for example has no __version__
     return hasattr(pip, "__version__") and pip.__version__ >= '1.5'
+
+
+def _pip_supports_constraints():
+    import pip
+    return hasattr(pip, "__version__") and pip.__version__ >= '7.1'
diff --git a/src/main/python/pybuilder/pip_utils.py b/src/main/python/pybuilder/pip_utils.py
index 483332c..774d57b 100644
--- a/src/main/python/pybuilder/pip_utils.py
+++ b/src/main/python/pybuilder/pip_utils.py
@@ -25,7 +25,8 @@ from pybuilder.pip_common import (Version,
                                   SpecifierSet,
                                   search_packages_info,
                                   pip_working_set_init,
-                                  _pip_disallows_insecure_packages_by_default)
+                                  _pip_disallows_insecure_packages_by_default,
+                                  _pip_supports_constraints)
 from pybuilder.utils import execute_command, as_list
 
 PIP_EXEC_STANZA = [sys.executable, "-m", "pip.__main__"]
@@ -72,7 +73,8 @@ def pip_install(install_targets, index_url=None, extra_index_url=None, upgrade=F
 
 
 def build_pip_install_options(index_url=None, extra_index_url=None, upgrade=False, insecure_installs=None,
-                              force_reinstall=False, target_dir=None, verbose=False, trusted_host=None):
+                              force_reinstall=False, target_dir=None, verbose=False, trusted_host=None,
+                              constraint_file=None):
     options = []
     if index_url:
         options.append("--index-url")
@@ -103,6 +105,10 @@ def build_pip_install_options(index_url=None, extra_index_url=None, upgrade=Fals
         options.append("-t")
         options.append(target_dir)
 
+    if constraint_file and _pip_supports_constraints():
+        options.append("-c")
+        options.append(constraint_file)
+
     if _pip_disallows_insecure_packages_by_default() and insecure_installs:
         for insecure_install in insecure_installs:
             arguments_for_insecure_installation = ["--allow-unverified", insecure_install,
@@ -112,6 +118,12 @@ def build_pip_install_options(index_url=None, extra_index_url=None, upgrade=Fals
     return options
 
 
+def create_constraint_file(file_name, constraints):
+    with open(file_name, "wt") as fout:
+        for constraint in as_pip_install_target(constraints):
+            fout.write("%s\\n" % constraint)
+
+
 def as_pip_install_target(mixed):
     arguments = []
     targets = as_list(mixed)
diff --git a/src/main/python/pybuilder/plugins/python/install_dependencies_plugin.py b/src/main/python/pybuilder/plugins/python/install_dependencies_plugin.py
index b540dc9..21aad27 100644
--- a/src/main/python/pybuilder/plugins/python/install_dependencies_plugin.py
+++ b/src/main/python/pybuilder/plugins/python/install_dependencies_plugin.py
@@ -36,7 +36,8 @@ from pybuilder.pip_utils import (PIP_EXEC_STANZA,
                                  as_pip_install_target,
                                  get_package_version,
                                  should_update_package,
-                                 version_satisfies_spec)
+                                 version_satisfies_spec,
+                                 create_constraint_file)
 from pybuilder.terminal import print_file_content
 from pybuilder.utils import execute_command, mkdir, as_list, safe_log_file_name
 
@@ -95,10 +96,15 @@ def create_install_log_directory(logger, project):
 
 
 def install_dependency(logger, project, dependencies):
-    dependencies_to_install, orig_installed_pkgs = _filter_dependencies(logger, project, dependencies)
+    dependencies_to_install, orig_installed_pkgs, dependency_constraints = _filter_dependencies(logger, project,
+                                                                                                dependencies)
     batch_dependencies = []
     standalone_dependencies = []
     local_mapping = project.get_property("install_dependencies_local_mapping")
+
+    constraints_file = project.expand_path("$dir_target", "install_dependencies_constraints")
+    create_constraint_file(constraints_file, dependency_constraints)
+
     for dependency in dependencies_to_install:
         url = getattr(dependency, "url", None)
 
@@ -119,14 +125,20 @@ def install_dependency(logger, project, dependencies):
         url = getattr(standalone_dependency, "url", None)
         log_file = project.expand_path("$dir_install_logs", safe_log_file_name(dependency.name))
         _do_install_dependency(logger, project, standalone_dependency,
-                               True,
-                               url,
+                               True,  # upgrade
+                               url,  # force reinstall
+                               constraints_file,
                                local_mapping.get(dependency.name),
                                log_file)
 
     if len(batch_dependencies):
         log_file = project.expand_path("$dir_install_logs", "install_batch")
-        _do_install_dependency(logger, project, batch_dependencies, True, False, None, log_file)
+        _do_install_dependency(logger, project, batch_dependencies,
+                               True,  # upgrade
+                               False,  # force reinstall
+                               constraints_file,
+                               None,
+                               log_file)
 
     __reload_pip_if_updated(logger, dependencies_to_install)
 
@@ -135,6 +147,7 @@ def _filter_dependencies(logger, project, dependencies):
     dependencies = as_list(dependencies)
     installed_packages = get_package_version(dependencies)
     dependencies_to_install = []
+    dependency_constraints = []
 
     for dependency in dependencies:
         logger.debug("Inspecting dependency '%s'" % dependency)
@@ -144,6 +157,11 @@ def _filter_dependencies(logger, project, dependencies):
             dependencies_to_install.append(dependency)
             continue
         elif isinstance(dependency, Dependency):
+            if dependency.version:
+                dependency_constraints.append(dependency)
+                logger.debug(
+                    "Dependency '%s' is added to the list of installation constraints" % dependency)
+
             if dependency.url:
                 # Always add dependency that is url-based
                 logger.debug("Dependency '%s' is URL-based and will be included" % dependency)
@@ -171,10 +189,11 @@ def _filter_dependencies(logger, project, dependencies):
 
         logger.debug("Dependency '%s' is already up-to-date and will be skipped" % dependency)
 
-    return dependencies_to_install, installed_packages
+    return dependencies_to_install, installed_packages, dependency_constraints
 
 
-def _do_install_dependency(logger, project, dependency, upgrade, force_reinstall, target_dir, log_file):
+def _do_install_dependency(logger, project, dependency, upgrade, force_reinstall, constraint_file, target_dir,
+                           log_file):
     batch = isinstance(dependency, collections.Iterable)
 
     pip_command_line = list()
@@ -188,7 +207,8 @@ def _do_install_dependency(logger, project, dependency, upgrade, force_reinstall
                                                       force_reinstall,
                                                       target_dir,
                                                       project.get_property("verbose"),
-                                                      project.get_property("install_dependencies_trusted_host")
+                                                      project.get_property("install_dependencies_trusted_host"),
+                                                      constraint_file
                                                       ))
     pip_command_line.extend(as_pip_install_target(dependency))
     logger.debug("Invoking pip: %s", pip_command_line)
diff --git a/src/unittest/python/pip_utils_tests.py b/src/unittest/python/pip_utils_tests.py
index eefdeba..377ff43 100644
--- a/src/unittest/python/pip_utils_tests.py
+++ b/src/unittest/python/pip_utils_tests.py
@@ -19,10 +19,9 @@
 import os
 import unittest
 
-from test_utils import patch, ANY
-
 from pybuilder import core
 from pybuilder import pip_utils
+from test_utils import patch, ANY
 
 
 class PipVersionTests(unittest.TestCase):
@@ -85,6 +84,8 @@ class PipVersionTests(unittest.TestCase):
         self.assertEquals(pip_utils.build_pip_install_options(force_reinstall=True), ["--force-reinstall"])
         self.assertEquals(pip_utils.build_pip_install_options(target_dir="target dir"), ["-t", "target dir"])
         self.assertEquals(pip_utils.build_pip_install_options(target_dir="target dir"), ["-t", "target dir"])
+        self.assertEquals(pip_utils.build_pip_install_options(constraint_file="constraint file"),
+                          ["-c", "constraint file"])
         self.assertEquals(pip_utils.build_pip_install_options(insecure_installs=["foo", "bar"]), [
             "--allow-unverified", "foo",
             "--allow-external", "foo",
diff --git a/src/unittest/python/pluginloader_tests.py b/src/unittest/python/pluginloader_tests.py
index 0249e5a..738e862 100644
--- a/src/unittest/python/pluginloader_tests.py
+++ b/src/unittest/python/pluginloader_tests.py
@@ -18,15 +18,6 @@
 
 import unittest
 
-try:
-    import __builtin__
-
-    builtin_module = __builtin__
-except ImportError as e:
-    import builtins
-
-    builtin_module = builtins
-
 from test_utils import patch, Mock, ANY
 from pybuilder.pip_utils import PIP_EXEC_STANZA
 from pybuilder.errors import MissingPluginException, IncompatiblePluginException, UnspecifiedPluginNameException
diff --git a/src/unittest/python/plugins/python/install_dependencies_plugin_tests.py b/src/unittest/python/plugins/python/install_dependencies_plugin_tests.py
index 073f3b9..a00e123 100644
--- a/src/unittest/python/plugins/python/install_dependencies_plugin_tests.py
+++ b/src/unittest/python/plugins/python/install_dependencies_plugin_tests.py
@@ -38,21 +38,27 @@ class InstallDependencyTest(unittest.TestCase):
     def setUp(self):
         self.project = Project("unittest", ".")
         self.project.set_property("dir_install_logs", "any_directory")
+        self.project.set_property("dir_target", "/any_target_directory")
         self.logger = Mock(Logger)
         initialize_install_dependencies_plugin(self.project)
 
+    @patch("pybuilder.plugins.python.install_dependencies_plugin.create_constraint_file")
     @patch("pybuilder.plugins.python.install_dependencies_plugin.get_package_version", return_value={})
     @patch("pybuilder.plugins.python.install_dependencies_plugin.execute_command", return_value=0)
-    def test_should_install_dependency_without_version(self, exec_command, get_package_version):
+    def test_should_install_dependency_without_version(self, exec_command, get_package_version, constraint_file):
         dependency = Dependency("spam")
 
         install_dependency(self.logger, self.project, dependency)
 
-        exec_command.assert_called_with(PIP_EXEC_STANZA + ["install", '--upgrade', 'spam'], ANY, env=ANY, shell=False)
+        exec_command.assert_called_with(PIP_EXEC_STANZA +
+                                        ["install", '--upgrade', '-c',
+                                         'unittest/any_target_directory/install_dependencies_constraints', 'spam'],
+                                        ANY, env=ANY, shell=False)
 
+    @patch("pybuilder.plugins.python.install_dependencies_plugin.create_constraint_file")
     @patch("pybuilder.plugins.python.install_dependencies_plugin.get_package_version", return_value={})
     @patch("pybuilder.plugins.python.install_dependencies_plugin.execute_command", return_value=0)
-    def test_should_install_requirements_file_dependency(self, exec_command, get_package_version):
+    def test_should_install_requirements_file_dependency(self, exec_command, get_package_version, constraint_file):
         dependency = RequirementsFile("requirements.txt")
 
         install_dependency(self.logger, self.project, dependency)
@@ -60,9 +66,11 @@ class InstallDependencyTest(unittest.TestCase):
         exec_command(
             PIP_EXEC_STANZA + ["install", '-r', "requirements.txt"], ANY, env=ANY, shell=False)
 
+    @patch("pybuilder.plugins.python.install_dependencies_plugin.create_constraint_file")
     @patch("pybuilder.plugins.python.install_dependencies_plugin.get_package_version", return_value={})
     @patch("pybuilder.plugins.python.install_dependencies_plugin.execute_command", return_value=0)
-    def test_should_install_dependency_without_version_on_windows_derivate(self, exec_command, get_package_version):
+    def test_should_install_dependency_without_version_on_windows_derivate(self, exec_command, get_package_version,
+                                                                           constraint_file):
         dependency = Dependency("spam")
 
         install_dependency(self.logger, self.project, dependency)
@@ -70,10 +78,12 @@ class InstallDependencyTest(unittest.TestCase):
         exec_command(
             PIP_EXEC_STANZA + ["install", "spam"], ANY, env=ANY, shell=False)
 
+    @patch("pybuilder.plugins.python.install_dependencies_plugin.create_constraint_file")
     @patch("pybuilder.plugins.python.install_dependencies_plugin.get_package_version", return_value={})
     @patch("pybuilder.pip_utils._pip_disallows_insecure_packages_by_default", return_value=True)
     @patch("pybuilder.plugins.python.install_dependencies_plugin.execute_command", return_value=0)
-    def test_should_install_dependency_insecurely_when_property_is_set(self, exec_command, _, get_package_version):
+    def test_should_install_dependency_insecurely_when_property_is_set(self, exec_command, _, get_package_version,
+                                                                       constraint_file):
         dependency = Dependency("spam")
         self.project.set_property("install_dependencies_insecure_installation", ["spam"])
 
@@ -83,11 +93,13 @@ class InstallDependencyTest(unittest.TestCase):
             PIP_EXEC_STANZA + ["install", "--allow-unverified", "spam", "--allow-external", "spam", 'spam'],
             ANY, env=ANY, shell=False)
 
+    @patch("pybuilder.plugins.python.install_dependencies_plugin.create_constraint_file")
     @patch("pybuilder.plugins.python.install_dependencies_plugin.get_package_version", return_value={})
     @patch("pybuilder.pip_utils._pip_disallows_insecure_packages_by_default", return_value=True)
     @patch("pybuilder.plugins.python.install_dependencies_plugin.execute_command", return_value=0)
     def test_should_install_dependency_securely_when_property_is_not_set_to_dependency(self, exec_command, _,
-                                                                                       get_package_version):
+                                                                                       get_package_version,
+                                                                                       constraint_file):
         dependency = Dependency("spam")
         self.project.set_property("install_dependencies_insecure_installation", ["some-other-dependency"])
 
@@ -99,10 +111,12 @@ class InstallDependencyTest(unittest.TestCase):
         #  some-other-dependency might be a dependency of 'spam'
         #  so we always have to put the insecure dependencies in the command line :-(
 
+    @patch("pybuilder.plugins.python.install_dependencies_plugin.create_constraint_file")
     @patch("pybuilder.plugins.python.install_dependencies_plugin.get_package_version", return_value={})
     @patch("pybuilder.pip_utils._pip_disallows_insecure_packages_by_default", return_value=False)
     @patch("pybuilder.plugins.python.install_dependencies_plugin.execute_command", return_value=0)
-    def test_should_not_use_insecure_flags_when_pip_version_is_too_low(self, exec_command, _, get_package_version):
+    def test_should_not_use_insecure_flags_when_pip_version_is_too_low(self, exec_command, _, get_package_version,
+                                                                       constraint_file):
         dependency = Dependency("spam")
         self.project.set_property("install_dependencies_insecure_installation", ["spam"])
 
@@ -111,9 +125,11 @@ class InstallDependencyTest(unittest.TestCase):
         exec_command(
             PIP_EXEC_STANZA + ["install", 'spam'], ANY, env=ANY, shell=False)
 
+    @patch("pybuilder.plugins.python.install_dependencies_plugin.create_constraint_file")
     @patch("pybuilder.plugins.python.install_dependencies_plugin.get_package_version", return_value={})
     @patch("pybuilder.plugins.python.install_dependencies_plugin.execute_command", return_value=0)
-    def test_should_install_dependency_using_custom_index_url(self, exec_command, get_package_version):
+    def test_should_install_dependency_using_custom_index_url(self, exec_command, get_package_version,
+                                                              constraint_file):
         self.project.set_property("install_dependencies_index_url", "some_index_url")
         dependency = Dependency("spam")
 
@@ -123,9 +139,11 @@ class InstallDependencyTest(unittest.TestCase):
             PIP_EXEC_STANZA + ["install", "--index-url", "some_index_url", 'spam'], ANY, env=ANY,
             shell=False)
 
+    @patch("pybuilder.plugins.python.install_dependencies_plugin.create_constraint_file")
     @patch("pybuilder.plugins.python.install_dependencies_plugin.get_package_version", return_value={})
     @patch("pybuilder.plugins.python.install_dependencies_plugin.execute_command", return_value=0)
-    def test_should_use_extra_index_url_when_index_url_is_not_set(self, exec_command, get_package_version):
+    def test_should_use_extra_index_url_when_index_url_is_not_set(self, exec_command, get_package_version,
+                                                                  constraint_file):
         self.project.set_property("install_dependencies_extra_index_url", "some_extra_index_url")
         dependency = Dependency("spam")
 
@@ -135,10 +153,12 @@ class InstallDependencyTest(unittest.TestCase):
             PIP_EXEC_STANZA + ["install", "--extra-index-url", "some_extra_index_url", 'spam'], ANY,
             env=ANY, shell=False)
 
+    @patch("pybuilder.plugins.python.install_dependencies_plugin.create_constraint_file")
     @patch("pybuilder.plugins.python.install_dependencies_plugin.get_package_version", return_value={})
     @patch("pybuilder.plugins.python.install_dependencies_plugin.execute_command", return_value=0)
     def test_should_use_index_and_extra_index_url_when_index_and_extra_index_url_are_set(self, exec_command,
-                                                                                         get_package_version):
+                                                                                         get_package_version,
+                                                                                         constraint_file):
         self.project.set_property("install_dependencies_index_url", "some_index_url")
         self.project.set_property("install_dependencies_extra_index_url", "some_extra_index_url")
         dependency = Dependency("spam")
@@ -149,9 +169,10 @@ class InstallDependencyTest(unittest.TestCase):
             PIP_EXEC_STANZA + ["install", "--index-url", "some_index_url", "--extra-index-url",
                                "some_extra_index_url", 'spam'], ANY, env=ANY, shell=False)
 
+    @patch("pybuilder.plugins.python.install_dependencies_plugin.create_constraint_file")
     @patch("pybuilder.plugins.python.install_dependencies_plugin.get_package_version", return_value={})
     @patch("pybuilder.plugins.python.install_dependencies_plugin.execute_command", return_value=0)
-    def test_should_upgrade_dependencies(self, exec_command, get_package_version):
+    def test_should_upgrade_dependencies(self, exec_command, get_package_version, constraint_file):
         self.project.set_property("install_dependencies_upgrade", True)
         dependency = Dependency("spam")
 
@@ -160,9 +181,10 @@ class InstallDependencyTest(unittest.TestCase):
         exec_command(
             PIP_EXEC_STANZA + ["install", "--upgrade", 'spam'], ANY, env=ANY, shell=False)
 
+    @patch("pybuilder.plugins.python.install_dependencies_plugin.create_constraint_file")
     @patch("pybuilder.plugins.python.install_dependencies_plugin.get_package_version", return_value={})
     @patch("pybuilder.plugins.python.install_dependencies_plugin.execute_command", return_value=0)
-    def test_should_install_dependency_with_version(self, exec_command, get_package_version):
+    def test_should_install_dependency_with_version(self, exec_command, get_package_version, constraint_file):
         dependency = Dependency("spam", "0.1.2")
 
         install_dependency(self.logger, self.project, dependency)
@@ -170,9 +192,11 @@ class InstallDependencyTest(unittest.TestCase):
         exec_command(
             PIP_EXEC_STANZA + ["install", 'spam>=0.1.2'], ANY, env=ANY, shell=False)
 
+    @patch("pybuilder.plugins.python.install_dependencies_plugin.create_constraint_file")
     @patch("pybuilder.plugins.python.install_dependencies_plugin.get_package_version", return_value={})
     @patch("pybuilder.plugins.python.install_dependencies_plugin.execute_command", return_value=0)
-    def test_should_install_dependency_with_version_and_operator(self, exec_command, get_package_version):
+    def test_should_install_dependency_with_version_and_operator(self, exec_command, get_package_version,
+                                                                 constraint_file):
         dependency = Dependency("spam", "==0.1.2")
 
         install_dependency(self.logger, self.project, dependency)
@@ -180,9 +204,10 @@ class InstallDependencyTest(unittest.TestCase):
         exec_command(
             PIP_EXEC_STANZA + ["install", 'spam==0.1.2'], ANY, env=ANY, shell=False)
 
+    @patch("pybuilder.plugins.python.install_dependencies_plugin.create_constraint_file")
     @patch("pybuilder.plugins.python.install_dependencies_plugin.get_package_version", return_value={})
     @patch("pybuilder.plugins.python.install_dependencies_plugin.execute_command", return_value=0)
-    def test_should_install_dependency_with_url(self, exec_command, get_package_version):
+    def test_should_install_dependency_with_url(self, exec_command, get_package_version, constraint_file):
         dependency = Dependency("spam", url="some_url")
 
         install_dependency(self.logger, self.project, dependency)
@@ -190,9 +215,11 @@ class InstallDependencyTest(unittest.TestCase):
         exec_command(
             PIP_EXEC_STANZA + ["install", "--force-reinstall", 'some_url'], ANY, env=ANY, shell=False)
 
+    @patch("pybuilder.plugins.python.install_dependencies_plugin.create_constraint_file")
     @patch("pybuilder.plugins.python.install_dependencies_plugin.get_package_version", return_value={})
     @patch("pybuilder.plugins.python.install_dependencies_plugin.execute_command", return_value=0)
-    def test_should_install_dependency_with_url_even_if_version_is_given(self, exec_command, get_package_version):
+    def test_should_install_dependency_with_url_even_if_version_is_given(self, exec_command, get_package_version,
+                                                                         constraint_file):
         dependency = Dependency("spam", version="0.1.2", url="some_url")
 
         install_dependency(self.logger, self.project, dependency)
diff --git a/src/unittest/python/test_utils.py b/src/unittest/python/test_utils.py
index 022941b..1a6a583 100644
--- a/src/unittest/python/test_utils.py
+++ b/src/unittest/python/test_utils.py
@@ -82,6 +82,7 @@ patch.object = partial(mock.patch.object, new_callable=PyBuilderMagicMock)
 patch.dict = mock.patch.dict
 patch.multiple = partial(mock.patch.multiple, new_callable=PyBuilderMagicMock)
 patch.stopall = mock.patch.stopall
+mock_open = mock.mock_open
 patch.TEST_PREFIX = 'test'
 DEFAULT = mock.DEFAULT
 call = mock.call