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

'extras_require' requirements cannot include environment markers #1087

Closed
sashkab opened this Issue Jul 14, 2017 · 17 comments

Comments

Projects
None yet
3 participants
@sashkab
Copy link

sashkab commented Jul 14, 2017

I just upgraded to the latest setuptools by accident, and now get following error:

 'extras_require' requirements cannot include environment markers, in 'notebook': 'enum34==1.1.6; python_version < "3.4"'

Portion of my extras_notebook_requirement file:

enum34==1.1.6; python_version < '3.4'

This is probably related to the #1081.

@benoit-pierre

This comment has been minimized.

Copy link
Member

benoit-pierre commented Jul 14, 2017

Can you give more information about your exact use-case? The thing is, while this was previously allowed, this would result in wheels that don't work as expected.

For example:

> cat setup.py
import sys

from setuptools import setup

setup(
    name='foo',
    extras_require={
        'test': 'barbazquux; "foobar" in sys_platform',
    },
)
> python ./setup.py -q bdist_wheel
> unzip -p dist/foo-0.0.0-py3-none-any.whl foo-0.0.0.dist-info/metadata.json | json_pp
{
   "extensions" : {
      "python.details" : {
         "document_names" : {
            "description" : "DESCRIPTION.rst"
         }
      }
   },
   "version" : "0.0.0",
   "name" : "foo",
   "summary" : "UNKNOWN",
   "metadata_version" : "2.0",
   "run_requires" : [
      {
         "extra" : "test",
         "requires" : [
            "barbazquux"
         ]
      }
   ],
   "extras" : [
      "test"
   ],
   "generator" : "bdist_wheel (0.29.0)"
}
> pip install --user dist/foo-0.0.0-py3-none-any.whl'[test]'
Processing ./dist/foo-0.0.0-py3-none-any.whl
Collecting barbazquux; extra == "test" (from foo==0.0.0)
  Could not find a version that satisfies the requirement barbazquux; extra == "test" (from foo==0.0.0) (from versions: )
No matching distribution found for barbazquux; extra == "test" (from foo==0.0.0)

As you can see, the environment marker was stripped, and barbarzquux will always be installed.

@erikwright

This comment has been minimized.

Copy link

erikwright commented Jul 14, 2017

We use extras_require to define a list of dev requirements, including mypy. mypy does not install on Python 2.

@benoit-pierre

This comment has been minimized.

Copy link
Member

benoit-pierre commented Jul 14, 2017

The valid syntax is:

> cat setup.py
import sys

from setuptools import setup

setup(
    name='foo',
    extras_require={
        'test:"foobar" in sys_platform': ['barbazquux'],
    },
)
> python ./setup.py -q bdist_wheel
> unzip -p dist/foo-0.0.0-py3-none-any.whl foo-0.0.0.dist-info/metadata.json | json_pp
{
   "name" : "foo",
   "metadata_version" : "2.0",
   "generator" : "bdist_wheel (0.29.0)",
   "summary" : "UNKNOWN",
   "extensions" : {
      "python.details" : {
         "document_names" : {
            "description" : "DESCRIPTION.rst"
         }
      }
   },
   "version" : "0.0.0",
   "run_requires" : [
      {
         "extra" : "test",
         "requires" : [
            "barbazquux"
         ],
         "environment" : "\"foobar\" in sys_platform"
      }
   ],
   "extras" : [
      "test"
   ]
}
> pip install --user dist/foo-0.0.0-py3-none-any.whl'[test]'
Processing ./dist/foo-0.0.0-py3-none-any.whl
Installing collected packages: foo
Successfully installed foo-0.0.0
@sashkab

This comment has been minimized.

Copy link
Author

sashkab commented Jul 14, 2017

My problem wasn't with the change -- it is clearly bug fix, but with the fact that breaking change was introduced in the maintenance release.

I'd expect first deprecation warning, and then removed in follow up release.

@benoit-pierre

This comment has been minimized.

Copy link
Member

benoit-pierre commented Jul 14, 2017

But it does not break a valid case. It was already broken, you just did not know it because setuptools was silently allowing it.

@erikwright

This comment has been minimized.

Copy link

erikwright commented Jul 14, 2017

@benoit-pierre So if I understand correctly, we will define two separate extras_require sections, one for Python 2 and one for Python 3. They will both be named test and they will both be complete lists of packages, and one or the other will be installed. Is that right?

@benoit-pierre

This comment has been minimized.

Copy link
Member

benoit-pierre commented Jul 14, 2017

@erikwright: yes, using something like:

    extras_require={
        'dev:python_version <= "2.7"': ['py2dep1', 'py2dep2'],
        'dev:python_version > "2.7"': ['py3dep1', 'py3dep2'],
   }

should work as expected.

@benoit-pierre

This comment has been minimized.

Copy link
Member

benoit-pierre commented Jul 14, 2017

Just to clarify, this would work as expected:

    extras_require={
        'test': ['flake8'],
        'test:python_version >= "3.4"': ['mypy'],
    },

installing both flake8 and mypy when using Python 3.4+.

@benoit-pierre

This comment has been minimized.

Copy link
Member

benoit-pierre commented Jul 14, 2017

@sashkab: honestly, I did not even know a syntax like 'test:python_version >= "3.4"': ['mypy'], was valid when I make the PR (I never saw it documented anywhere). Maybe I can make a new PR to automatically convert something like 'test': ['mypy; python_version >= "3.4"], to 'test:python_version >= "3.4"': ['mypy'],.

@sashkab

This comment has been minimized.

Copy link
Author

sashkab commented Jul 14, 2017

'test:python_version >= "3.4"': ['mypy'],

No, syntax was:

extras_require={ 
   'notebook': ["enum34==1.1.6; python_version < '3.4'"]
}
@benoit-pierre

This comment has been minimized.

Copy link
Member

benoit-pierre commented Jul 14, 2017

OK, but this would not work as expected, enum34 would be installed no matter the Python version. While this would have worked (and still does):

extras_require={ 
   'notebook: python_version < "3.4"': ['enum34==1.1.6']
}
@sashkab

This comment has been minimized.

Copy link
Author

sashkab commented Jul 14, 2017

this would not work as expected

It was working. enum34 wasn't installing in the python >=3.4. Maybe, due to pinned setuptools to 33.1.1.

@benoit-pierre

This comment has been minimized.

Copy link
Member

benoit-pierre commented Jul 14, 2017

I just tested with Python 3.6 and setuptools==33.1.1, and when installing from wheel enum34 definitively get installed too as the environment marker was stripped in the wheel metadata.

@sashkab

This comment has been minimized.

Copy link
Author

sashkab commented Jul 14, 2017

Interesting: I'm positive, that package in question wasn't installed:

.tox/py36-macos/bin (develop) $ pip list --format=columns | grep -E 'setuptools|enum34'
setuptools (33.1.1)
.tox/py36-macos/bin (develop) $ python -V
Python 3.6.1

My tox environment installs all extras.

@sashkab

This comment has been minimized.

Copy link
Author

sashkab commented Jul 14, 2017

I don't want to waist your time -- my main complain was ERROR instead of DeprecationWarning. I fixed my code to work correctly in 20 minutes, and as I did not see this reported -- reported to have a discussion.

@benoit-pierre

This comment has been minimized.

Copy link
Member

benoit-pierre commented Jul 14, 2017

@sashkab: thanks for reporting, see #1088 for a PR that would allow (and correctly handle) both syntax.

@sashkab

This comment has been minimized.

Copy link
Author

sashkab commented Jul 14, 2017

Thank you, @benoit-pierre!

proofit404 added a commit to django/channels that referenced this issue Jul 18, 2017

dimichxp added a commit to dimichxp/weblib that referenced this issue Jul 19, 2017

setup.py: fix extras_require for setuptools 36.2.0
Install on linux fails with setuptools==36.2.0 (pypa/setuptools#1087)

```
Collecting weblib==0.1.24 (from -r test-requirements.txt (line 62))
  Downloading weblib-0.1.24.tar.gz
    Complete output from command python setup.py egg_info:
    error in weblib setup command: 'extras_require' requirements cannot include environment markers, in 'full': 'lxml; platform_system != "Windows"'
```

dimichxp added a commit to dimichxp/weblib that referenced this issue Jul 19, 2017

setup.py: fix extras_require for setuptools 36.2.0
Install on linux fails with setuptools==36.2.0 (pypa/setuptools#1087)

```
Collecting weblib==0.1.24 (from -r test-requirements.txt (line 62))
  Downloading weblib-0.1.24.tar.gz
    Complete output from command python setup.py egg_info:
    error in weblib setup command: 'extras_require' requirements cannot include environment markers, in 'full': 'lxml; platform_system != "Windows"'
```

@jaraco jaraco closed this in #1089 Jul 23, 2017

heilaaks added a commit to heilaaks/snippy that referenced this issue Feb 17, 2019

Add document for required setuptools version
Snippy setup.py uses environment markers for example for
Pylint. There are comments in Internet that these
would not work because wheel would strip these out in
install_requires. These are old comments.

This is fixed in setuptools 36.2.1 [1]. This is noted
incorrectly in previous commit message which contains
a note about the release of setuptools that introduced
the environment markers. But the fix to support these
is in version 36.2.1.

This is already taken into account in Snippy pyproject
toml file. This is more of an documentation about the
needed versions for setup related tools for the Snippy
project.

[1] pypa/setuptools#1087

Signed-off-by: Heikki Laaksonen <laaksonen.heikki.j@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.