-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Join extras from constraints and requests #3105
Conversation
If a dep is mentioned in constraints we just take that version, meaning we will completely ignore any extras requested on the command line or in a requirments file. This takes a union of the two before installing. Closes #3046
Oh cool.. This should mention issue 3189 in its description :). |
result = script.pip_install_local( | ||
'-c', script.scratch_path / 'constraints.txt', 'LocalExtras[baz]') | ||
assert script.site_packages / 'simple' in result.files_created | ||
assert script.site_packages / 'singlemodule.py'in result.files_created |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it worth having a case with a requirements file? Not conceptually different to install... but it might be nice to be explicit that
pip install foo foo[bar] foo[quux]
should no longer error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pip install foo foo[bar] foo[quux]
would indeed be nice but seems unrelated to this PR that focuses on constraints... Am I missing something ?
Or maybe you thought you were commenting on #3198 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I realised that the constraints bug was a special case of the general problem (which itself is a sub-special-case of the resolver problem). I think it would be confusing if extras work with constraints but not requirements or cli - so am arguing that we address them all concurrently, within thebounds of not having a resolver).
This looks fine but could probably be rebased on develop to avoid the test issue due to pytest. |
@@ -263,6 +263,8 @@ def add_requirement(self, install_req, parent_req_name=None): | |||
# If we're now installing a constraint, mark the existing | |||
# object for real installation. | |||
existing_req.constraint = False | |||
existing_req.extras = tuple(set(existing_req.extras).union( | |||
set(install_req.extras))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A logging.debug of the merge could be helpful.
Closed by #3198 |
If a dep is mentioned in constraints we just take that version, meaning we
will completely ignore any extras requested on the command line or in a
requirements file. This takes a union of the two before installing.
Following commits are being handled in PR #2937