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

Add support for dependencies (such as six) #229

Closed
bb-migration opened this Issue Jul 5, 2014 · 23 comments

Comments

Projects
None yet
1 participant
@bb-migration

bb-migration commented Jul 5, 2014

Originally reported by: jaraco (Bitbucket: jaraco, GitHub: jaraco)


I'd like to use the six library for Python 2 compatibility.

As it currently stands, setuptools does not have any non-extra dependencies, so introduction of any dependency has the potentially to be disruptive, so this ticket is created primarily around that discussion.

Here's what I have in mind:

  1. Vendor six in the sdist to be used when setuptools installs itself (but never install from that verison).
  2. Declare those dependencies as proper dependencies in install_requires.
  3. Have the code reference those libraries naturally.

This change would require bootstrap installers to either:

  • Have the dependencies installed in advance.
  • Have network access to install the dependencies from PyPI.
  • Configure an index-url from which to install the dependencies.

What I want to avoid is for setuptools to vendor the dependencies for any purpose other than bootstrapping. I'd also like to avoid having conditional imports in the code. I especially want to avoid creating complicated vendoring scenarios that violate the expectations of platform package maintainers (namely Debian and Fedora).


@bb-migration

This comment has been minimized.

bb-migration commented Jul 5, 2014

Original comment by arfrever (Bitbucket: arfrever, GitHub: arfrever):


Recent versions of six (>=1.7.0) have a regression with Jython: https://bitbucket.org/gutworth/six/issue/76

@bb-migration

This comment has been minimized.

bb-migration commented Jul 5, 2014

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


Add six 1.7.3 for bootstrapping purposes. Ref #229

1 similar comment
@bb-migration

This comment has been minimized.

bb-migration commented Jul 6, 2014

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


Add six 1.7.3 for bootstrapping purposes. Ref #229

@bb-migration

This comment has been minimized.

bb-migration commented Jul 8, 2014

Original comment by koobs (Bitbucket: koobs, GitHub: koobs):


Of note, this will require a global run dependency on six for all FreeBSD Python packages in the Ports tree (in addition to setuptools) as well as add QA complexity and regression risks for what is already a very difficult target to track.

Coupled with the challenge of getting regressions and other changes committed/addressed upstream, it de-prioritizes our resources and focus away from user-facing functionality and features.

The above will be true for all other OS's that wrap all modules in setuptools functionality to attain a consistent packaging experience for users that use OS-maintained packages. While this may not be an obvious issue right now, the long term robustness and quality of packaging tools is of critical importance, especially downstream.

While everyone is all for actively maintained packaging modules (distutils, setuptools, pip, etc), it seems as if the proliferation of individual per-project decision-making moving forward without much in the way of a external consultation process, or a coordinated evolution plan, risks moving us back to the dark-days of packaging.

Among other questions:

  • What is the vendoring:reward ratio of requiring six for setuptools?
  • What alternative methods or mechanisms may be possible to achieve similar results?
  • To what extent is the vendoring of setuptools pkg_resources in pip a consideration now? In the future?
@bb-migration

This comment has been minimized.

bb-migration commented Jul 11, 2014

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


@koobs All good points. At PyCon, I consulted with Debian and Fedora maintainers about the impacts of this kind of change. At the time, my take-away was that it couldn't be done, and I may come to that conclusion again. I do respect the impact of system package maintainers and will make points to get specific feedback from relevant groups prior to releasing this.

What is the vendoring:reward ratio of requiring six for setuptools?

The reward is relatively small to require six. Vendoring would be cheap, easy, and convenient, but it violates system packager preferences. The reason I'm choosing six for consideration now is because it has obvious benefit, but also is a trivially simple and non-controversial package, so it provides a good proving ground for the concept (of requiring other packages), which is the real win. If setuptools could require arbitrary packages, it would enable flexibility that other packages enjoy, including the ability to factor out functionality like pkg_resources.

What alternative methods or mechanisms may be possible to achieve similar results?

I'm unsure. Traditionally, the only functionality that Setuptools could depend on was what's in Python stdlib. Attempts to move pkg_resources into stdlib were stymied (I'm blissfully ignorant of the details). Moving six into stdlib might be possible, but that approach is not generally viable (or is it)?

What pip does is vendor everything (incl. setuptools, urlllib3, requests, ssl, ...), and virtualenv vendors pip. While this approach has worked, it's also untenable in general. In fact, if setuptools were to vendor its dependencies, then pip would have to unravel those (and possibly keep them in sync if there are conflicts). Vendoring sidesteps good packaging practices and decoupling, so only works in limited scenarios. I'm trying to move setuptools away from that paradigm rather than further into it.

To what extent is the vendoring of setuptools pkg_resources in pip a consideration now? In the future?

I'm on board with the goal to move decouple as much of setuptools as possible, but the current constraints (such as those that limit this issue) make any other arrangement a difficult if not impossible proposition to solve without consequences.

@bb-migration

This comment has been minimized.

bb-migration commented Nov 16, 2014

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


I continue to work this issue in the feature/issue-229 branch, but I continue to be stymied by bootstrapping issues. This morning, I created a build and uploaded it to downloads (https://bitbucket.org/pypa/setuptools/downloads/setuptools-7.1dev-20141116.zip). That build has six available as setuptools/_vendor/six-1.7.3.egg. It will locate it during installation, which works to make setuptools available... but then when the requirement to have six is resolved, setuptools creates a link to the temporary copy of six rather than installing it. Here's what appears in easy-install.pth after installation to a system without six:

c:\users\jaraco\appdata\local\temp\tmp_pv8vmbl\setuptools-7.1dev-20141116\setuptools\_vendor\six-1.7.3.egg

Because that temporary directory has been cleaned up, six is no longer available and invocations of setuptools fail (with ImportError on six).

@bb-migration

This comment has been minimized.

bb-migration commented Dec 31, 2015

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


Update vendoring technique to match that used for packaging. Ref #229.

@bb-migration

This comment has been minimized.

bb-migration commented Dec 31, 2015

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


Merge with master. Ref #229.

@bb-migration

This comment has been minimized.

bb-migration commented Dec 31, 2015

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


Based on the precedent set by vendoring of packaging into setuptools, I'm following that technique for the inclusion of six as well (see 1afacd703c6c). That approach violates three of my goals:

  1. The vendored version is preferred.
  2. The dependency on six is not present in the package metadata.
  3. The conditional imports are littered throughout the code.

However, as these limitations haven't caused any issues with packaging and as I seek to have a uniform approach, I'm replicating that approach for the inclusion of six.

Of note, this will require a global run dependency on six for all FreeBSD Python packages in the Ports tree ...

I believe this global run dependency already exists for pip, so I believe this to be a necessary and reasonable approach.

@koobs, @warsaw, @toshio: based on your experience with the change to include a dependency on packaging in setuptools, is it feasible to replicate that change for other dependencies? What can PyPA do by the way of notification or staging to minimize the surprise and flux for a change such as this?

@bb-migration

This comment has been minimized.

bb-migration commented Dec 31, 2015

Original comment by embray (Bitbucket: embray, GitHub: embray):


You might find the way that I packaged six in with astropy helpful. Rather than having try/excepts around every import you have a setuptools.six module or setuptools._vendor.six or whatever you want to call it. That doesn't contain the six module itself, but rather is a wrapper that loads the vendored copy (if present) or the system copy as appropriate.

I haven't touched this code in a while so it's a little outdated, but it should give the general idea:

https://github.com/astropy/astropy/blob/master/astropy/extern/six.py

This first tries to import astropy.extern.bundled.six, which contains the actual vendored module (and can be removed by system packagers). Failing that it just tries to import six. However, it uses imp.load_module to import it as astropy.extern.six. So in all of Astropy's code it can use from astropy.extern import six and it will get some version of the six module regardless. You could use this approach for any other vendored package as well.

There's still a downside in the lack of dependency metadata. Not sure what to do about that.

@bb-migration

This comment has been minimized.

bb-migration commented Dec 31, 2015

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


Modeling after Astropy's technique for bundling libraries, the imports are now much cleaner. Thanks @embray. Ref #229.

@bb-migration

This comment has been minimized.

bb-migration commented Dec 31, 2015

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


I really quite like the effect that technique has. I whittled it down a bit not to do the version number checking and to more generically handle any package. I applied the technique to the vendored packaging package as well in 2e6e2f292966. I like how clean and explicit it makes the imports.

At this point, I'm ready to make a release with these changes, but I'll wait to hear back from our distro friends for comment.

@bb-migration

This comment has been minimized.

bb-migration commented Dec 31, 2015

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


I may have spoken too soon.

@embray: I think there may be a problem with this technique for anything other than a trivial module like six. Running with that latest commit, I find the tests pass on Python 3, but fail on Python 2 with this error:

$ python2.7 setup.py test
Traceback (most recent call last):
  File "setup.py", line 21, in <module>
    exec(init_file.read(), command_ns)
  File "<string>", line 11, in <module>
  File "/Users/jaraco/Dropbox/code/main/setuptools/setuptools/__init__.py", line 14, in <module>
    from setuptools.extension import Extension
  File "/Users/jaraco/Dropbox/code/main/setuptools/setuptools/extension.py", line 8, in <module>
    from .dist import _get_unpatched
  File "/Users/jaraco/Dropbox/code/main/setuptools/setuptools/dist.py", line 17, in <module>
    from pkg_resources.extern import packaging
  File "/Users/jaraco/Dropbox/code/main/setuptools/pkg_resources/__init__.py", line 91, in <module>
    from pkg_resources.extern import packaging
  File "/Users/jaraco/Dropbox/code/main/setuptools/pkg_resources/extern/packaging.py", line 45, in <module>
    _import_in_place()
  File "/Users/jaraco/Dropbox/code/main/setuptools/pkg_resources/extern/packaging.py", line 34, in _import_in_place
    imp.load_module(__name__, *mod_info)
  File "/Users/jaraco/Dropbox/code/main/setuptools/pkg_resources/_vendor/packaging/__init__.py", line 16, in <module>
    from .__about__ import (
ImportError: No module named __about__

It seems that on Python 2, the relative imports will not succeed. I suspect you can see this failed outcome in the failed travis build, although I'm not currently able to load that page due to network errors (probably near my node).

Any suggestions on how to use this technique for anything more than a simple module under Python 3?

@bb-migration

This comment has been minimized.

bb-migration commented Jan 2, 2016

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


Move extern.packaging into a package to enable package-relative imports to resolve propertly. Ref #229.

@bb-migration

This comment has been minimized.

bb-migration commented Jan 2, 2016

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


I found the ImportError was caused because on Python 2, when in packaging/__init__.py, __package__ was set to pkg_resources.extern and not pkg_resources.extern.packaging, as it appears on Python 3. Moving the wrapper module into a packaging package instead corrects for the issue and tests now pass.

However, there's another condition that now fails. When setuptools is installed as a zip egg, the finder is no longer able to find the vendored package:

$ python -c "import setuptools"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/Library/Frameworks/Python.framework/Versions/3.5/lib/python3.5/site-packages/setuptools-19.3.dev0-py3.5.egg/setuptools/__init__.py", line 14, in <module>
  File "/Library/Frameworks/Python.framework/Versions/3.5/lib/python3.5/site-packages/setuptools-19.3.dev0-py3.5.egg/setuptools/extension.py", line 8, in <module>
  File "/Library/Frameworks/Python.framework/Versions/3.5/lib/python3.5/site-packages/setuptools-19.3.dev0-py3.5.egg/setuptools/dist.py", line 17, in <module>
  File "/Library/Frameworks/Python.framework/Versions/3.5/lib/python3.5/site-packages/setuptools-19.3.dev0-py3.5.egg/pkg_resources/__init__.py", line 91, in <module>
  File "/Library/Frameworks/Python.framework/Versions/3.5/lib/python3.5/site-packages/setuptools-19.3.dev0-py3.5.egg/pkg_resources/extern/packaging/__init__.py", line 45, in <module>
  File "/Library/Frameworks/Python.framework/Versions/3.5/lib/python3.5/site-packages/setuptools-19.3.dev0-py3.5.egg/pkg_resources/extern/packaging/__init__.py", line 42, in _import_in_place
ImportError: The 'packaging' package is required; normally this is bundled with this package so if you get this warning, consult the packager of your distribution.
@bb-migration

This comment has been minimized.

bb-migration commented Jan 2, 2016

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


It seems that imp.find_module does not find anything but simple source files on disk when on Python 3.5. It does seem to find the modules when running on Python 2.7.

I've struggled with this one for a while and I'm giving up. I'm not sure what it will take to have an implementation that can find modules on Python 2.6 - 3.5 even if they're in a zip archive.

@bb-migration

This comment has been minimized.

bb-migration commented Jan 2, 2016

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


Create a PEP 302 importer for managing conditional import of vendored packages from the 'extern' namespace. This technique avoids the use of 'imp' and works even when setuptools is installed as a zipped egg. Ref #229.

@bb-migration

This comment has been minimized.

bb-migration commented Jan 2, 2016

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


I've pushed some more commits with a new approach entirely, but it seems even with those changes, tests are failing on Python 2. That error message is indicating that the Version in packaging.specifiers is different from the Version found in pkg_resources.packaging.version.

@bb-migration

This comment has been minimized.

bb-migration commented Jan 2, 2016

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


I have an implementation that appears to be passing all tests. I'm not super-confident in the implementation. It feels brittle, especially the part that selects on sys.version_info. Perhaps someone with a better understanding of the import system (@gutworth, @brettcannon) could do a quick review and advise on the technique. Is there a better selector for whether the module should be pulled from sys.modules? Is there another way to keep the internal, relative imports consistent with those externally (those going through the 'extern' hook)? Setuptools currently targets Python 2.6-2.7 and 3.1-3.5, but if it helps to drop support for 2.6 and 3.2, that's a viable option.

Thanks for any advice you're able to lend.

@bb-migration

This comment has been minimized.

bb-migration commented Jan 4, 2016

Original comment by toshio (Bitbucket: toshio, GitHub: toshio):


On a piece of my own software I had been working on several strategies similar to @embray's approach ( https://anonbadger.wordpress.com/2015/10/15/dear-lazyweb-how-would-you-nicely-bundle-python-code/ https://github.com/ansible/ansible/tree/devel/lib/ansible/compat/six ). I wasn't happy with what I finally arrived at, though, and so was going to experiment with @embray's approach when I had more time. I'm not sure if one of my attempts at making it work would work for your case (none of them use imp -- only my final try worked for six but the others should work for things that also messing with python's import mechanism. I didn't try any of this with zipped eggs or wheels so I don't know if they have problems with that or not.)

I'll be very interested in seeing what other people think of your PEP 302 importer. It looks to me like overall it's the best strategy to use even if there are some pieces of the code you wish could be better.

@bb-migration

This comment has been minimized.

bb-migration commented Jan 4, 2016

Original comment by embray (Bitbucket: embray, GitHub: embray):


Strange that it didn't work for zipped eggs. I feel like I just recently tested that case, but I need to double check that I didn't already have the vendored modules installed when I tested that. The custom importer you wrote looks like a good way to go though. When I first posted this I noted that my existing code is "out of date" and that's mainly because the imp module is deprecated since Python 3.4. Using an import hook looks like a better approach overall.

@bb-migration

This comment has been minimized.

bb-migration commented Jan 5, 2016

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


I've prepared a beta release as 19.3b1.

@dstufft and @jezdez, can you test that release when vendored in pip and virtualenv to ensure that this new technique doesn't break in that environment?

@bb-migration

This comment has been minimized.

bb-migration commented Jan 16, 2016

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


Released as 19.3

jaraco added a commit that referenced this issue Apr 4, 2016

Add six 1.7.3 for bootstrapping purposes. Ref #229
--HG--
branch : feature/issue-229
extra : source : 7e8095f69ea350cd04fabdbeca5c9733a57ec8ff

jaraco added a commit that referenced this issue Apr 4, 2016

Merge with master. Ref #229.
--HG--
branch : feature/issue-229

jaraco added a commit that referenced this issue Apr 4, 2016

jaraco added a commit that referenced this issue Apr 4, 2016

Modeling after Astropy's technique for bundling libraries, the import…
…s are now much cleaner. Thanks @embray. Ref #229.

--HG--
branch : feature/issue-229

jaraco added a commit that referenced this issue Apr 4, 2016

Move extern.packaging into a package to enable package-relative impor…
…ts to resolve propertly. Ref #229.

--HG--
branch : feature/issue-229

jaraco added a commit that referenced this issue Apr 4, 2016

Create a PEP 302 importer for managing conditional import of vendored…
… packages from the 'extern' namespace. This technique avoids the use of 'imp' and works even when setuptools is installed as a zipped egg. Ref #229.

--HG--
branch : feature/issue-229
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment