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

'parse_requirements' does not consider environment markers #61

Closed
hexagonrecursion opened this issue Nov 15, 2020 · 4 comments · Fixed by #99
Closed

'parse_requirements' does not consider environment markers #61

hexagonrecursion opened this issue Nov 15, 2020 · 4 comments · Fixed by #99
Labels
enhancement New feature or request

Comments

@hexagonrecursion
Copy link

Trying to parse the following file

pytest>=4.6.10,<5 ; python_version < '3'
pytest>=5.4.2,<6 ; python_version >= '3'

Throws an exception

PipError: Double requirement given: pytest<6,>=5.4.2 (already in pytest<5,>=4.6.10, name='pytest')
@di di changed the title ; python_version < '3' is not honored 'parse_requirements' does not consider environment markers Nov 16, 2020
@di
Copy link
Owner

di commented Nov 16, 2020

This is because parse_requirements is not considering environment markers. This library would need go do something similar to pip here: https://github.com/pypa/pip/blob/faee60baea2b850427b5fceb22383883fa11290b/src/pip/_internal/req/req_set.py#L90-L95

@woodruffw
Copy link
Collaborator

woodruffw commented Sep 13, 2021

cc @tetsuo-cpp could you take a look at this? I think the Requirements class gives us everything we need, we just need to dedupe the requirements based on which one(s) satisfy the environment marker(s):

class Requirement(object):

Edit: this PEP seems to exhaustively list all of the markers we need to support and their basic expression language: https://www.python.org/dev/peps/pep-0496/. But perhaps there's a (vendored) dependency we could reuse here.

Edit 2: Looks like we already have a marker evaluator vendored under packaging.markers, so we should probably use that!

@di
Copy link
Owner

di commented Sep 13, 2021

Yes, packaging.markers should be sufficient here (and also what pip uses).

@tetsuo-cpp
Copy link
Contributor

I think the code to use the marker is relatively straight forward (we just check marker.evaluate() prior to inserting the requirement into the dict). The problem I'm seeing is that the marker isn't being parsed properly and is being set to None when we construct the packaging.Requirement.

I'm looking at the parsing code to figure out what's going wrong here.

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

Successfully merging a pull request may close this issue.

4 participants