Skip to content
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

dynamic install_requires breaks universal wheels #36

Closed
mgedmin opened this issue May 25, 2015 · 10 comments · Fixed by #37
Closed

dynamic install_requires breaks universal wheels #36

mgedmin opened this issue May 25, 2015 · 10 comments · Fixed by #37

Comments

@mgedmin
Copy link
Member

mgedmin commented May 25, 2015

pip 7 builds and caches wheels, which speeds up repeated installs.

Unfortunately, if you build a wheel of ZODB on Python 2.6, it will compute install_requires that works on 2.6 but fails on 2.7 (because no 'zodbpickle'). This wheel will be named ZODB-4.2.0b1-py2-none-any.whl, and so pip will try to use it on 2.7. This breaks people's buildbots.

STEPS TO REPRODUCE:

$ virtualenv --version
13.0.1

$ cat tox.ini
[tox]
envlist = py26,py27
skipsdist = true

[testenv]
skip_install = true
deps = ZODB
commands =
  python -c 'import ZODB'

$ rm ~/.cache/pip/wheels/*/*/*/*/ZODB-*.whl

$ tox --pre
py26 installed: argparse==1.3.0,BTrees==4.1.3,persistent==4.1.0,six==1.9.0,transaction==1.4.4,wheel==0.24.0,zc.lockfile==1.1.0,ZConfig==3.0.4,zdaemon==4.1.0,ZODB==4.2.0b1,zope.interface==4.1.2
py26 runtests: PYTHONHASHSEED='2859500558'
py26 runtests: commands[0] | python -c import ZODB
py27 recreate: /tmp/bork/.tox/py27
py27 installdeps: ZODB
py27 installed: BTrees==4.1.3,persistent==4.1.0,six==1.9.0,transaction==1.4.4,wheel==0.24.0,zc.lockfile==1.1.0,ZConfig==3.0.4,zdaemon==4.1.0,ZODB==4.2.0b1,zope.interface==4.1.2
py27 runtests: PYTHONHASHSEED='2859500558'
py27 runtests: commands[0] | python -c import ZODB
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/tmp/bork/.tox/py27/local/lib/python2.7/site-packages/ZODB/__init__.py", line 28, in <module>
    from ZODB.DB import DB, connection
  File "/tmp/bork/.tox/py27/local/lib/python2.7/site-packages/ZODB/DB.py", line 23, in <module>
    from ZODB.broken import find_global
  File "/tmp/bork/.tox/py27/local/lib/python2.7/site-packages/ZODB/broken.py", line 23, in <module>
    from ZODB._compat import IMPORT_MAPPING
  File "/tmp/bork/.tox/py27/local/lib/python2.7/site-packages/ZODB/_compat.py", line 42, in <module>
    import zodbpickle.pickle
ImportError: No module named zodbpickle.pickle
ERROR: InvocationError: '/tmp/bork/.tox/py27/bin/python -c import ZODB'
___________________________________ summary ___________________________________
  py26: commands succeeded
ERROR:   py27: commands failed

$ ls ~/.cache/pip/wheels/*/*/*/*/ZODB*
/home/mg/.cache/pip/wheels/98/e4/85/1b42bbea2eb9f3d1bd2043843104a15121cdeceaa24ad659d1/ZODB-4.2.0b1-py2-none-any.whl

I can see three possible solutions:

@mgedmin
Copy link
Member Author

mgedmin commented May 25, 2015

The third solution is probably not much of a solution, unless everyone who makes releases remembers to
build and upload py26, py27 wheels to PyPI at release time, passing --python-tag=py26/py27 as appropriate for each python bdist_wheel command.

mgedmin added a commit that referenced this issue May 25, 2015
@jamadden
Copy link
Member

I'm responsible for the use of zodbpickle under 2.7 (it was already dynamically computed and used under Py3), so I'm happy to help resolve this. I guess I should finally try to read and understand PEP 426 :)

I added it for Python 2.7 because cPickle is broken in 2.7. cPickle works fine under 2.6, though (which is why jimfulton hadn't encountered the issue before). Now, zodbpickle claims to be an implementation of the Python 2.7 pickle modules, but it is also tested on Python 2.6. When the use of zodpickle under 2.7 was merged, I did a quick review of the differences between the 2.6 and 2.7 pickle modules, and didn't find anything that looked like a compatibility issue to me.

If that holds true, then I would support option one, using zodbpickle under Python 2.6. Although it's an additional dependency (for that one implementation), it would simplify the test matrix, and it seems reasonable and consistent to use the same pickle implementation under all platforms.

@mgedmin
Copy link
Member Author

mgedmin commented Jun 2, 2015

Oh snap I don't have PyPI rights for ZODB (and discovered this in the middle of ./fullrelease for 4.2.0b2). @jimfulton, @tseaver: please?

@tseaver
Copy link
Member

tseaver commented Jun 2, 2015

@mgedmin
Copy link
Member Author

mgedmin commented Jun 3, 2015

Welp, 4.2.0 doesn't fix this :(

Back to the drawing board (I want to understand why before attempting a different fix).

@mgedmin mgedmin reopened this Jun 3, 2015
@mgedmin
Copy link
Member Author

mgedmin commented Jun 3, 2015

I cannot reproduce using the STEPS TO REPRODUCE anymore, but I can reproduce using zodbbrowser's tox.ini on Linux.

@jamadden
Copy link
Member

jamadden commented Jun 3, 2015

RE: using zodbpickle everywhere: Although the pickle format wouldn't change, the exceptions that get raised would (from cPickle.PicklingError to zodbpickle.fastpickle.PicklingError, represented as _pickle.PicklingError) . This could be a minor compatibility concern (we ran into this in one of our test suites after jumping to ZODB 4.2.0).

@mgedmin
Copy link
Member Author

mgedmin commented Jun 4, 2015

I did a tox -r and now can't reproduce on Linux any more.

I can reproduce on Windows, even after wiping the Jenkins workspace.

@mgedmin
Copy link
Member Author

mgedmin commented Jun 4, 2015

Filed a pip bug: pypa/pip#2870

@mgedmin
Copy link
Member Author

mgedmin commented Jun 26, 2015

My problem was that I was building ZODB wheels with wheel 0.23.0, which did not support environment markers and thus created wrong metadata in the .whl file.

Upgrading to wheel 0.24.0 fixed everything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants