Make sure it works with py2 wheel for all version. — Fixes #2175 #2177
Conversation
acme/setup.py
Outdated
There was a problem hiding this comment.
Please leave explicit inline comment reference to 2.7.8 as the last one requiring those packages. this comment should also be moved and merge with the one before branching condition
|
FTR We should really be building universal wheels (for both py2 and py3), so this PR doesn't really fix the general problem: #2176. |
|
010a2ef was created on the assumption that Python 2.6 is not supported, which is not true. As far as I understand we cannot use extras and tags for that. |
Natim
force-pushed
the
2175-broken-wheel-dependencies
branch
from
aba1068 to
661087f
Compare
|
This last commit make it possible to use universal build as well as letting older pip version to correctly install the package. |
acme/setup.py
Outdated
There was a problem hiding this comment.
Did you mean to put this in extras_require rather than install_requires? I don't see how it's ever going to get brought in unless somebody requests it manually.
There was a problem hiding this comment.
Strike that; they work locally for me, though the >< logic seems to operate backward...
There was a problem hiding this comment.
though the >< logic seems to operate backward...
We want the extra dependencies for old version of python lower than 2.7.9
There was a problem hiding this comment.
Right. Pip 7.1.2 literally does the wrong thing for me. It's bizarre.
There was a problem hiding this comment.
I have updated by removing the conditional appending to install_requires. Can you try again? (building the wheel from this branch)
There was a problem hiding this comment.
With python 2.7.9 it still install ndg-httpsclient-0.4.0
There was a problem hiding this comment.
For future reference, the trouble turned out to be that setuptools doesn't yet support the comparators < and > in env markers.
|
I'm trying to get this to pass test farm tests, but it's most likely we'll go with #2182 for the 0.2.0 release and then try to land some version of this later... |
|
@pde Yes it cannot harm anyone 👍 |
|
@pde do not forget to add the setup.cfg to build universal wheel :) |
|
Meanwhile, I did just get this to pass test farm tests. So perhaps we can consider it for 0.2.1: |
None of this is ideal, since we're making the dependencies tighter than they theoretically need to be, but the behavior of the old le-auto makes this necessary to make it succeed in practice (when using LE wheels). Once we move to the new le-auto (which pins everything and makes setup.py dependencies irrelevant for auto installs), we should redo this using env markers as in #2177. Note that argparse *does* actually get installed by the old le-auto but only because we luck out and ConfigArgParse requires it. Since we do import argparse directly, we should make sure it's always a direct dependency.
None of this is ideal, since we're making the dependencies tighter than they theoretically need to be, but the behavior of the old le-auto makes this necessary to make it succeed in practice (when using LE wheels). Once we move to the new le-auto (which pins everything and makes setup.py dependencies irrelevant for auto installs), we should redo this using env markers as in #2177. We're too afraid to do it now. Similarly, we're too afraid to change how we handle argparse right now, despite that it should be required directly by us under 2.6. In practice, ConfigArgParse pulls it in for us, so we're okay as long as it continues to do that.
|
Reopening this for consideration for 0.3.0... |
|
Let me rebase that first. |
Natim
force-pushed
the
2175-broken-wheel-dependencies
branch
from
54c0976 to
b8690cd
Compare
|
IMHO, This should be sufficient. |
|
lgtm! (Though this ticket could more accurately be renamed "build Python-3-compatible wheels".) |
Build a Python-3-compatible wheel for acme.
|
Merging this without fixing |
|
setup.py was actually fixed in a previous PR.
|
|
What was wrong with setup.py? |
5 participants
You can try the wheel here with: