New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transitive dependencies in setup_requires should be able to override system versions #1124

Closed
sitaktif opened this Issue Aug 4, 2017 · 5 comments

Comments

Projects
None yet
3 participants
@sitaktif

sitaktif commented Aug 4, 2017

This is basically the same issue as #223 but for transitive dependencies.

Issue:

When a package my_pkg depends (with install_requires) on my_transitive_dep with a version that conflict with the currently installed version of my_transitive_dep, adding my_pkg as a setup_requires to the setup.py of a project fails to initialise the Distribution object and raises pkg_resources.VersionConflict.

Expected behaviour:

The proper version of my_transitive_dep is installed in .eggs and the currently installed version of my_transitive_dep is ignored.

Minimal steps to reproduce the issue:

(in a virtualenv, python 2.7.13)

setuptools-versionconflict $ pip install 'setuptools<30'
Collecting setuptools<30
  Using cached https://pypi.apple.com/packages/06/4b/86a670fd21f7849adb092e40883c48dcd0d66b8a878fc8d63b7f0ea04213/setuptools-29.0.1-py2.py3-none-any.whl
Installing collected packages: setuptools
Successfully installed setuptools-29.0.1

setuptools-versionconflict $ cat setup.py
from setuptools import setup

setup(
    name='my-test',
    setup_requires=['flake8']  # flake8 has `setuptools>=30` in its `install_requires`
)

setuptools-versionconflict $ python setup.py clean
zip_safe flag not set; analyzing archive contents...

Installed /private/var/folders/cg/b7r4x8m15p59z4glfsy6s_640000gn/T/easy_install-q7dnDn/flake8-3.4.1/.eggs/pytest_runner-2.11.1-py2.7.egg
warning: no previously-included files matching '*.pyc' found anywhere in distribution
no previously-included directories found matching 'docs/build/'
zip_safe flag not set; analyzing archive contents...

Installed /Users/rchossart/tmp/setuptools-versionconflict/.eggs/flake8-3.4.1-py2.7.egg
Traceback (most recent call last):
  File "setup.py", line 5, in <module>
    setup_requires=['flake8']
  File "/usr/local/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/lib/python2.7/distutils/core.py", line 111, in setup
    _setup_distribution = dist = klass(attrs)
  File "/Users/rchossart/tmp/setuptools-versionconflict/.venv/lib/python2.7/site-packages/setuptools/dist.py", line 315, in __init__
    self.fetch_build_eggs(attrs['setup_requires'])
  File "/Users/rchossart/tmp/setuptools-versionconflict/.venv/lib/python2.7/site-packages/setuptools/dist.py", line 361, in fetch_build_eggs
    replace_conflicting=True,
  File "/Users/rchossart/tmp/setuptools-versionconflict/.venv/lib/python2.7/site-packages/pkg_resources/__init__.py", line 850, in resolve
    dist = best[req.key] = env.best_match(req, ws, installer)
  File "/Users/rchossart/tmp/setuptools-versionconflict/.venv/lib/python2.7/site-packages/pkg_resources/__init__.py", line 1115, in best_match
    dist = working_set.find(req)
  File "/Users/rchossart/tmp/setuptools-versionconflict/.venv/lib/python2.7/site-packages/pkg_resources/__init__.py", line 719, in find
    raise VersionConflict(dist, req)
pkg_resources.VersionConflict: (setuptools 29.0.1 (/Users/rchossart/tmp/setuptools-versionconflict/.venv/lib/python2.7/site-packages), Requirement.parse('setuptools>=30'))

EDIT: clarified the problem definition.

@benoit-pierre

This comment has been minimized.

Member

benoit-pierre commented Aug 4, 2017

setup.py:

from setuptools import setup
setup(
    name='my-test',
    dependency_links=['deps/my-dep-0.0.1.tar.gz'],
    setup_requires=['my-dep']  # my-dep has `python-xlib>=0.18` in its `install_requires`
)

This work:

> rm -rf venv .eggs && python -m venv venv && ./venv/bin/pip install -U setuptools && ./venv/bin/python setup.py clean

This doesn't:

rm -rf venv .eggs && python -m venv venv && ./venv/bin/pip install -U setuptools 'python-xlib==0.17' && ./venv/bin/python setup.py clean

Tentative patch:

 pkg_resources/__init__.py | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git i/pkg_resources/__init__.py w/pkg_resources/__init__.py
index f13a69b3..cb6f512c 100644
--- i/pkg_resources/__init__.py
+++ w/pkg_resources/__init__.py
@@ -852,7 +852,10 @@ class WorkingSet(object):
                             # distribution
                             env = Environment([])
                             ws = WorkingSet([])
-                    dist = best[req.key] = env.best_match(req, ws, installer)
+                    dist = best[req.key] = env.best_match(
+                        req, ws, installer,
+                        replace_conflicting=replace_conflicting
+                    )
                     if dist is None:
                         requirers = required_by.get(req, None)
                         raise DistributionNotFound(req, requirers)
@@ -1104,7 +1107,7 @@ class Environment(object):
                 dists.append(dist)
                 dists.sort(key=operator.attrgetter('hashcmp'), reverse=True)
 
-    def best_match(self, req, working_set, installer=None):
+    def best_match(self, req, working_set, installer=None, replace_conflicting=False):
         """Find distribution best matching `req` and usable on `working_set`
 
         This calls the ``find(req)`` method of the `working_set` to see if a
@@ -1117,7 +1120,12 @@ class Environment(object):
         calling the environment's ``obtain(req, installer)`` method will be
         returned.
         """
-        dist = working_set.find(req)
+        try:
+            dist = working_set.find(req)
+        except VersionConflict:
+            if not replace_conflicting:
+                raise
+            dist = None
         if dist is not None:
             return dist
         for dist in self[req.key]:
@jaraco

This comment has been minimized.

Member

jaraco commented Aug 5, 2017

This change looks reasonable, though I do loathe the boolean switch being passed from method call to method call.

@sitaktif

This comment has been minimized.

sitaktif commented Aug 7, 2017

Can this code tell the difference between something that was installed outside of .eggs/ (e.g. in the system python) and something that has been installed in .eggs/?

It seems to be that it is important to differentiate the following two cases when considering the my-pkg >= 2 dependency:

  • system python has my-pkg == 1, .eggs does not have my-pkg (yet). Result should be: my-pkg >= 2 gets installed in .eggs/.
  • system python does not have my-pkg, .eggs already has my-pkg == 1 (due to a dependency installed just before, e.g. from setup_requires). Result should be: VersionConflict exception.

Case 1 is the case we're trying to solve. Case 2 is the case I'm afraid we might be breaking.

benoit-pierre added a commit to benoit-pierre/setuptools that referenced this issue Aug 7, 2017

pkg_resources: improve WorkingSet.resolve(replace_conflicting=True)
Correctly replace conflicting distributions in sub-requirements
if possible (instead of only for top-level requirements passed
as arguments).

Fix pypa#1124.
@benoit-pierre

This comment has been minimized.

Member

benoit-pierre commented Aug 7, 2017

@sitaktif: check the torture tests in #1129, I believe both use cases are handled:

benoit-pierre added a commit to benoit-pierre/setuptools that referenced this issue Aug 29, 2017

pkg_resources: improve WorkingSet.resolve(replace_conflicting=True)
Correctly replace conflicting distributions in sub-requirements
if possible (instead of only for top-level requirements passed
as arguments).

Fix pypa#1124.

@jaraco jaraco closed this in #1129 Sep 3, 2017

@sitaktif

This comment has been minimized.

sitaktif commented Sep 10, 2017

🙏 ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment