pypa / wheel

The official binary distribution format for Python
MIT License
506 stars 150 forks source link

Wheel doesn't support environment markers from requirements.txt #172

Closed agronholm closed 7 years ago

agronholm commented 8 years ago

Originally reported by: vitaly numenta (Bitbucket: vitaly_numenta, GitHub: Unknown)


Both pip (8.1.2) and setuptools (since 20.6.6) support a common format for environment markers, but wheel ignores the environment markers from requirements.txt.

Environment: OS X El Capitan, pip==8.2.1, setuptools==25.2.0, wheel=0.29.0, python 2.7.10.

I put together a tiny package to demonstrate this here: https://github.com/vitaly-krugl/wheel_marker_fail.

requirements.txt contains a single line: pycapnp==0.5.8 ; platform_system=='BestKeptSecret'

Pip correctly ignores pycapnp because platform_system doesn't match:

$ pip install -r requirements.txt --user
Ignoring pycapnp: markers u"platform_system=='BestKeptSecret'" don't match your environment

In the demonstration package, setup.py reads the lines from requirements.txt and passes them to setuptools.setup() as the install_requires arg. When I execute python setup.py install, setuptools correctly ignores pycapnp, too.

Make an egg via python setup.py bdist_egg and install via easy_install --user dist/wheel_marker_fail-0.0.1-py2.7.egg also honors the markers.

But, unlike all of the other tools, python setup.py bdist_wheel followed by pip install dist/wheel_marker_fail-0.0.1-py2-none-any.whl --user ignores the environment makers and installs pycapnp anyway:

$ pip install dist/wheel_marker_fail-0.0.1-py2-none-any.whl --user
Processing ./dist/wheel_marker_fail-0.0.1-py2-none-any.whl
Collecting pycapnp==0.5.8 (from wheel-marker-fail==0.0.1)
Installing collected packages: pycapnp, wheel-marker-fail
Successfully installed pycapnp-0.5.8 wheel-marker-fail-0.0.1

agronholm commented 7 years ago

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


This seems like a pretty big problem. @suutari-ai even contributed a pull request. Is anyone interested in this?

agronholm commented 7 years ago

Original comment by Tuomas Suutari (Bitbucket: suutari-ai, GitHub: suutari-ai):


This is related to issue #181, maybe even duplicate?

agronholm commented 8 years ago

Original comment by vitaly numenta (Bitbucket: vitaly_numenta, GitHub: Unknown):


Gentlemen, I have a follow-up question about building the wheel that references the concrete (pinned down dependencies). So, as recommended, I am changing install_requires to contain the abstract dependencies for developers, while having the concrete versioned dependencies in requirements.txt that we use to test official releases of our package.

What's not clear to me now is how I can possibly build a wheel for deployment to PyPi that records the concrete dependencies, such that when users install my wheel from PyPi, the concrete dependencies would be installed (not the loosely-versioned ones). Users that install my wheel from PyPi will not (and should not need to) have access to my package's requirements.txt, after all. E.g.,

in setup.py:

setup(
    extras_require = {
        ":platform_system=='Linux' or platform_system=='Darwin'": ["pycapnp"]
    },
    install_requires=[
      "psutil>=1.0.1,<2"
    ],
    ... <other setup args>
)

and requirements.txt:

pycapnp==0.5.8 ; platform_system=='Linux' or platform_system=='Darwin'
psutil==1.0.1

However, when I build my wheel via python setup.py bdist_wheel, the resulting wheel will reference only the abstract loosely-versioned dependencies from setup.py. Thus, when users install my wheel from PyPi, the wrong versions of dependencies would be installed instead of the concrete ones that I have in requirements.txt.

I want developers to be able to use the abstract dependencies for experimenting with my package, but installs of my wheel from PyPi need to reference the concrete (pinned-down) dependencies with which my build system actually tested my package.

What am I missing?

Many thanks,

Vitaly

CC @danielholth @mmerickel

P.S., I posted this question on the Wheel-builders list: https://mail.python.org/pipermail/wheel-builders/2016-September/000220.html

agronholm commented 8 years ago

Original comment by Michael Merickel (Bitbucket: mmerickel, GitHub: mmerickel):


If you have a project that versions some of its dependencies then your workflow should be pip install -r requirements.txt and then pip install -e . (any unpinned deps will be installed from pypi, but anything pinned via requirements.txt will already be there and the pip install -e . will happily keep the pinned version installed).

Optionally you could bundle the -e . into your requirements.txt and name it something like requirements-dev.txt. This workflow is basically equivalent to your python setup.py develop above except you run pip install -r requirements-dev.txt.

agronholm commented 8 years ago

Original comment by vitaly numenta (Bitbucket: vitaly_numenta, GitHub: Unknown):


I see; in your workflow, your expectation is that pip install -e . should install just the abstract dependencies instead of the versioned dependencies from pip freeze. Thank you.

agronholm commented 8 years ago

Original comment by Daniel Holth (Bitbucket: dholth, GitHub: dholth):


It will install all the dependencies of the project in the current directory "." and link the project to the virtual environment.

agronholm commented 8 years ago

Original comment by vitaly numenta (Bitbucket: vitaly_numenta, GitHub: Unknown):


@danielholth, what is the significance of pip install -e . in this thread? pip install --help doesn't relate the -e option to dependencies in any way, so I am not sure what you implied.

-e, --editable <path/url>   Install a project in editable mode (i.e. setuptools "develop mode") from a local project path or a VCS url.

Thank you.

agronholm commented 8 years ago

Original comment by Daniel Holth (Bitbucket: dholth, GitHub: dholth):


As for extras_require, the syntax is like so

  extras_require={  ':python_version=="2.6"': ['argparse'],
  'signatures': ['keyring', 'keyrings.alt'],
  'signatures:sys_platform!="win32"': ['pyxdg'],  },

That's an empty extra name followed by a colon : and the marker. install_requires is equivalent to an extra with the empty string as its name: extras_require = { '' : [ "install", "requires" ] }

agronholm commented 8 years ago

Original comment by vitaly numenta (Bitbucket: vitaly_numenta, GitHub: Unknown):


Our internal work-flow is to run python setup.py install (or python setup.py develop) in our projects and expect that this automatically pre-installs the requirements, hence the copying from requirements.txt into the setup() arg install_requires.

agronholm commented 8 years ago

Original comment by vitaly numenta (Bitbucket: vitaly_numenta, GitHub: Unknown):


Thanks everyone for the feedback. I am not sufficiently well-versed in python package best practices and have to consider that it's entirely possible that our projects are using a technique that, while convenient for our own internal work-flow, may be non-standard. I will consult with my colleagues.

agronholm commented 8 years ago

Original comment by Michael Merickel (Bitbucket: mmerickel, GitHub: mmerickel):


Parsing requirements.txt into your setup() function is definitely an anti-pattern that has caught on lately on a few projects. Hopefully it will die as this shakes out.

agronholm commented 8 years ago

Original comment by Daniel Holth (Bitbucket: dholth, GitHub: dholth):


I think it's shocking that people expect to read requires.txt, which can also sometimes contain pip command line arguments, directly into install_requires = []. It is a file format that came up from nowhere but a lot of people seem to like it. Maybe they do not know about 'pip install -e .' or maybe their project doesn't even have setup.py . The way I use requires.txt it would contain the output of 'pip freeze' but install_requires = [] would contain the abstract dependencies (just the names of the direct dependencies).

Wheel has to know how to translate requirements from install_requires to the dist-info format. It is a different code path than what setuptools uses, and the markers syntax for requires.txt is newer than bdist_wheel.

A pull request to update bdist_wheel would be fine.

agronholm commented 8 years ago

Original comment by Michael Merickel (Bitbucket: mmerickel, GitHub: mmerickel):


@vitaly_numenta I think he is suggesting that the environment markers are supported via adding them to extras_require instead of install_requires. Similar to what wheel is doing here: https://bitbucket.org/pypa/wheel/src/cf4e2d98ecb1f168c50a6de496959b4a10c6b122/setup.py?fileviewer=file-view-default#setup.py-41

Whether this is the standard way to do it, I have no idea. I agree it's surprising that it works for everything except wheel when you put it in install_requires.

agronholm commented 8 years ago

Original comment by vitaly numenta (Bitbucket: vitaly_numenta, GitHub: Unknown):


Dear @dholth, thank you for following up with your recommendations.

A great many packages prefer to keep their python dependencies in a single file - requirements.txt. Numenta's packages are no different. Requirements.txt is an easily-readable single source of truth for dependencies.

I considered adding extra logic to the production package's setup.py to parse its requirements.txt, identify the lines that contain environment markers, extract the environment markers, and then pass those entries to setup() via the extras_require arg, while passing the rest via the install_requires arg.

However, it occurred to me that it doesn't make sense for everyone to be complicating the installation code of their packages with such logic. Both pip and setuptools have already made the leap to support this syntax uniformly, and it works beautifully without additional boilerplate. Therefore, it only makes sense for wheel to follow in their footsteps.

I will take a look at the referenced source code. @dholth, are you recommending that I try to submit a patch to the wheel repo to support this feature or that I try patching it in my own package's installation logic?

agronholm commented 8 years ago

Original comment by Daniel Holth (Bitbucket: dholth, GitHub: dholth):


It expects them in extras-require. Do you know how to put them there? Wheel has an example in its own setup.py

You could probably patch this part of the code to add your feature.

https://bitbucket.org/dholth/wheel/commits/e6fe9bb3108487f7ff9819c19f730edb565dd6ad

agronholm commented 7 years ago

The environment marker support has been fixed in setuptools 36.2.0. As for pinning exact versions, you can use the -c option with pip install. It's a bad idea to upload anything to PyPI with exact pinned versions.