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

Seems to still drop some markers #1

Closed
uranusjr opened this issue Aug 16, 2018 · 5 comments
Closed

Seems to still drop some markers #1

uranusjr opened this issue Aug 16, 2018 · 5 comments

Comments

@uranusjr
Copy link
Member

uranusjr commented Aug 16, 2018

Experiementing on requirementslib, various markers disappeared. apipkg, for example, got the following from PIpenv:

python_version >= '2.7' and python_version != '3.1.*' and python_version != '3.2.*' and
python_version != '3.3.*' and python_version != '3.0.*'

Did it get them from requires_python? Because not all python_version markers are lost. pathlib2, for example, keeps it fine. (Passa even gets more because it correctly merges markers!)

@techalchemy
Copy link
Member

Give me the source requirements for the resolution here

@uranusjr
Copy link
Member Author

Confirmed those are all from python_requires. apipkg got it via

apipkg ← execnet ← pytest-xdist ← '-e .[tests]'

pytest-xdist provided those values.

I think the theoretically best option is to add an extra field to the lockfile format (and requirementslib.Requirement). Otherwise we’ll need to merge them into the markers. It’s not difficult, just messy.

@techalchemy
Copy link
Member

I also still like this idea because it allows us to keep records of otherwise unresolvable dependencies together by just adding the extra marker. I still wonder if it makes sense to treat this special or to allow uniqueness in lockfiles the same way we do in the dep cache

As for this specific case, I assume you looked at the various ways pipenv handles that marker?

@techalchemy
Copy link
Member

(This is related to not picking up requires_python attributes from the requirementset after resolution -- see https://github.com/pypa/pipenv/blob/master/pipenv/patched/piptools/repositories/pypi.py#L396 for info)

@uranusjr
Copy link
Member Author

Implemented requires_python pickup in d339019 (with a cache), and merging them into markers in 2d90438. This should do it for now. (The logic can use some improvement, the markers are a bit ugly now.)

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

No branches or pull requests

2 participants