[PEP 771] #13170
[PEP 771] #13170
Conversation
…nt default extras if present and if extras not specified
|
I've marked the PR as a draft. Please undraft it when you feel it is ready for review. Thank you! |
DEKHTIARJonathan
force-pushed
the
pep_771
branch
2 times, most recently
from
07d9104 to
2e72b58
Compare
DEKHTIARJonathan
force-pushed
the
pep_771
branch
from
2e72b58 to
3b74d67
Compare
|
I realize that this is a draft and won't be finalized until the PEP is accepted, but I will note that we generally avoid carrying patches for our vendored dependencies unless necessary. The changes should be upstreamed prior to final implementation. If necessary, we do carry a set of git patches that our vendoring infrastructure automatically applies. We do it this way so we can upgrade our dependencies without repatching them manually (in most cases). |
|
@ichard26 thanks for the comment ! There's a lot of work to make this PR ready to be merged once/if PEP 771 gets to be accepted. In any case, I'll definitely reach out once this PR need to move more seriously forward. Namely asking what's the best process to push changes in 3-4 vendored projects and how to update them back into PIP after being merged. |
|
|
||
| with contextlib.suppress(AttributeError): | ||
| requirement._extras = requirement._ireq.extras | ||
|
|
||
| criteria[requirement.name] = criterion |
There was a problem hiding this comment.
FYI, if PEP 771 is accepted, be aware you will not be able to submit this specific patch to resolvelib. Resolvelib is very generic and has no concept of extras, you will to solve this in someway in src/pip/_internal/resolvelib
There was a problem hiding this comment.
@notatallshaw who is the best person to have a chat in the future about this PR ? Getting some directions ?
This PEP is a lot more difficult to implement than I anticipated and I'm not entirely sure I took the best approach.
I used the debugger to track up and down how extras are being managed, though it's far from being trivial and my overall confidence in this PR is not very high.
There was a problem hiding this comment.
I'm not sure, as far as I'm aware all active pip maintainers volunteer personal time to do their maintenance duties and therefore it's extremely limited.
I see pip maintainers @pfmoore, @pradyunsg, and myself have all contributed or are actively involved in the PEP 771 discussion.
For my part I will review anything that touches the resolution code, but I can't currently help with larger architectural / mentoring tasks sorry.
There was a problem hiding this comment.
Fantastic thanks for your comment ;)
For now we don't need to worry about. Once a decision is taken on the PEP, I'll see what needs to be done.
And if I have specific questions I can always ask (and hope for the best ahaha)
There was a problem hiding this comment.
I'm happy to help out - I haven't yet taken a look at this PR mainly as it's in draft and I've had some RL issues to deal with.
@ichard26 is correct about vendoring, we'll need to get the patches in the source project(s) and wait for a release before we pick them up. We can carry the patches in the PR until that happens, though, so work here won't be blocked.
As for resolvelib, @notatallshaw is correct. There's code in the resolvelib "examples" directory that shows how to handle extras in resolvelib, if you want to see a simple example before diving into the full glory of pip's implementation.
The way pip handles this is that when you see foo[xtra], you basically treat that as a completely separate "candidate", and inject an additional dependency that says that foo[xtra] depends on the exact same version of foo. The reason you need to do it like this is that candidates must have fixed dependencies in resolvelib, so foo[xtra] and foo must be different because they don't have the same dependencies. You're going to have to change this model a bit, as with default extras, foo could include the default extra or not, depending on whether foo[actual_extra] is requested as well. So when you see "foo", you don't actually know what candidate you need to tell resolvelib about...
For a tricky example to think about, consider the following.
A depends on B. B depends on C. C depends on A[xtra]. Now, if you ask to install A, will the default extra of A get installed? I assume not, as down in the dependency tree, A[xtra] is requested, which "switches off" the default extra. But you can't know this when you process the initial requirement ("A") to pass it to resolvelib - so what dependencies do you say A has? Remember, you can't change your decision later...
Actually, this makes me wonder about the proposal itself. If A has a default extra of X, and X specifies a dependency of B, and B depends on A[Y], then if I install A, will X be installed? Because if it is, A[Y] will be installed, and the fact that you installed extra Y means that X (the default extra) shouldn't be installed. I don't know what the answer is here, but the PEP needs to specify, as you can't leave it to individual tools to pick something.
Placeholder - Work in progress