-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Open
Labels
C: extrasHandling optional dependenciesHandling optional dependencieskind: backwards incompatibleWould be backward incompatibleWould be backward incompatiblestate: awaiting PRFeature discussed, PR is neededFeature discussed, PR is neededtype: enhancementImprovements to functionalityImprovements to functionality
Description
It seems that pip just returns success code on anything related to use of extras, commands like: pip install -e .[non_exiting_extra]
reports success.
Even worse, with existing extras that may fail to install, the final result code is still a success, when in fact pip failed to install dependencies.
This is a critical bug because it directly affect CI usage where we can no longer trust result code from pip, bugs may go in unnoticed.
ffix, Li-ReDBox, VaclavPlajt, Mazyod, SagiPolaczek and 3 more
Metadata
Metadata
Assignees
Labels
C: extrasHandling optional dependenciesHandling optional dependencieskind: backwards incompatibleWould be backward incompatibleWould be backward incompatiblestate: awaiting PRFeature discussed, PR is neededFeature discussed, PR is neededtype: enhancementImprovements to functionalityImprovements to functionality
Projects
Milestone
Relationships
Development
Select code repository
Activity
pradyunsg commentedon Oct 1, 2019
Not sure what you're asking for. Could you provide a clear reproducer of this issue?
ssbarnea commentedon Oct 1, 2019
@pradyunsg Not sure if a more than the first sentence is needed to reproduce pip failure on ANY project, even one without any extras.
Pip should never return success (0) if an extra is missing or did not succeeded installing. Current behavior is to fully ignore any of these and to return success.
chrahunt commentedon Oct 1, 2019
Not fully ignore - I believe we trace a warning.
Returning non-zero would be backwards incompatible. I would be +1 on doing it (even aborting installation if we are not able to find an extra), but it would need to be changed over at least 1-2 releases. The first step would be to add to the existing warning to say that it's unsupported and will fail in a future release. We'd also want to go through pypi and see how many packages make reference to nonexistent extras.
ssbarnea commentedon Oct 1, 2019
@chrahunt I am glad to hear that you agree on what should be the desired behavior. Now we only need to agree on a migration path that does not alienate users.
Maybe if we have a
--strict
switch (PIP_STRICT=1
) in pip we could make it easier to to migrate, especially as we can add this to CI systems.Initially enabling new behavior when is defined and in the future to make it default. The same could happen for other functionalities which are kept only for backwards compatibility.
Regarding current behavior: I think that is visible when it fails to install dependencies from existing extras but when an extra does not exist at all, is totally transparent, I did not see any warning (maybe is only a debug message).
chrahunt commentedon Oct 1, 2019
If you could provide a reproduction that shows that behavior it would help, I could not reproduce it locally:
results in
Note the
WARNING: example 0.0.0 does not provide the extra 'nonexistent_extra'
.Regarding adding a configuration option, we need to keep #6221 in mind. Not that it can't be done that way, but there's probably a way to do it that wouldn't require a deprecation period of its own.
xavfernandez commentedon Oct 2, 2019
Note that this is the behavior of pip since version 6.1 (April 2015):
So calling it a "critical bug" seems a far stretch ;)
Nonetheless it wasn't the behavior of pip 6 that crashed in such case:
According to #2138 and #2142 it might have been to make it consistent with setuptools behavior ?
But I'm on board to progressively deprecate this behavior and make it an error.
pradyunsg commentedon Oct 7, 2019
One concern I have is how exactly does pip pass extras down to dependencies -- if it does not, then I'm on board for making this behavior an error.
If we do pass down extras to dependencies (I'm not sure at this point), then we should stop doing that and that'd be required as a step before changing this behavior.
38 remaining items
meejah commentedon Oct 4, 2024
This won't really lead to the UX I'd like as a user here -- when I fat-finger or otherwise don't know the exact name for an extra ("
pip install twisted[ssl]
" for example, wheretls
is correct) -- I don't want to download years of Twisted releases before learning .. nothing?For the use-case of "a user-typed requirement, on the CLI" in particular (ignoring for now the points above about non-user-controlled requirements, etc) it really feels a lot better to literally turn the existing warning into an error. That is, find the latest release + platform, and if the requested extra is not provided that's an immediate error.
(I say "learning nothing" above because what can I actually output as "available extras" to help the user? The list of every extra I saw with all releases downloaded? This could still be wrong because I'm only on one platform, so won't see "every" release...)
As a concrete example, this is my desired UX (for a human entering CLI commands):
...and this exits with non-zero error-code.
Alternatively, with the "keep looking" proposed UX, I will perhaps notice once the downloader starts getting into non-wheel releases that it is downloading a bunch. At some point, hopefully I notice the messages about "this is taking a long time" and "tighten up your versions". Okay, so I ctrl-c it, and then try like
python -m pip install "twisted[foo]>=24.0.0"
which will compare several metadata files and then tell meResolutionImpossible
. Without teaching_vendor/resolvelib
about extras, I don't see a good way to improve this error-message either -- since those kinds of errors all seem to be "inside"resolvelib
? Of course, I've only looked at this a little bit and so perhaps there is a way to achieve an extras-explicit error that I don't see.This definitely seems at odds with @pfmoore 's request for minimal changes, so I'm not sure how to proceed.
(FWIW, I personally don't want to make "disruptive" changes in
pip
-- I want to give myself some way to configurepip
to turn an already existing warning into an error).notatallshaw commentedon Oct 4, 2024
Similar issues can already happen when you input the wrong requirements. Pip cannot predict what requirements you meant, and it would be a bad philosophy to build a user experience around that. It could however log that it's not selecting a particular version because of a missing extra.
The opposite can be true. What if I follow a slightly old guide to install a package, and it recommends an extra, but an incompatible 2.0 version of that package no longer has that extra? This 2.0 was released after the guide was written. So, if pip errors immediately on 2.0, the user gets no feedback that they need to select a previous version of the package.
This "keep looking" UX is the foundation of how package resolvers work for requirement restrictions.
If I request
'a>1' 'b<10'
, and recent versions ofa
depend on'b>10'
, then pip doesn't just error out. It keeps checking each version ofa
, going back until it finds a version ofa
that matches the user requirement'b<10'
. That might involve thousands of packages and might not exist. This could have been a typo by the user — they might have meant'b<100'
— but pip cannot account for user typos like this. (That said, there are optimizations we can, and I plan to, make to improve this)Having pip throw an error if the latest version of a package doesn't meet the requirement would be a whole new concept for pip and contrary to existing features I'm aware of.
Again, without seeing your code or trying it myself, I don’t know. But
resolvelib
reports back the state of the failure to a reporter object that pip provides. That reporter object can give helpful error messages. This is currently a very underutilized feature and may help here.Any changes to vendor code need to be made in that library, resolvelib is used by more than pip, so its API needs to be highly generic: https://github.com/sarugaku/resolvelib
I think it's possible to implement this without it being a large code change, but I could be wrong.
What's also important is that the change needs to be minimal from a user-disruptive point of view. If pip starts a new behavior of erroring out immediately when extra requirements aren't met and doesn't attempt to resolve them, this will cause packages to become incompatible with each other in novel ways that can not be easily reasoned about by the user, in fact they may have to manually figure out the correct version of a transitive dependency themselves which invalidates having a resolver.
pfmoore commentedon Oct 4, 2024
@meejah as you can see, this is why it’s taken so long to get anything done on this - it’s a lot harder problem than it looks.
I’m not at all comfortable with @notatallshaw’s assertion that backtracking is the right thing to do here - you’ve described the way in which this could result in user-unfriendly behaviour, and I agree it’s far from ideal. And unlike a version mismatch, mistyping an extra seems to me like it would in many cases be a simple typo that doesn’t warrant a search through the full dependency tree.
@notatallshaw I see your point about this being a new behaviour, but do you have any evidence that it would be as disruptive as you say? I guess you are thinking of a case where A depends on B[foo], and B 1.0 has extra foo. When B 2.0 is released without extra foo, what happens to
pip install A
? Both the current behaviour, nor @meejah’s proposed behaviour, could be wrong (the current behaviour if B no longer supports the functionality gated behind extra foo, the proposed behaviour if B merged the behaviour into its default, and dropped the extra as no longer necessary). Your proposal handles this case in the sense that a correct set of packages is installed, but A has no way to specify dependencies that supports both versions of B.So there’s no perfect solution here. All options have problems, and it’s very arguable that “do nothing” is the least disruptive.
What is clear is that somewhere, we should document why we end up with whatever behaviour we choose, so that people know it’s a deliberate decision, not an accident.
Oh, and what should also be clear by now is why I hate extras 🙁 The design sucks, the implementation is a nightmare, and the user experience is confusing.
notatallshaw commentedon Oct 4, 2024
Apologies, this has been intuitively obvious to me, so I've not done a good job explaining. Sleeping on it my point if the following scenario (it's similar to your scenario but I feel highlights how deep the problem is):
Scenario: The user depends on
A
andB
, both have deeply transitive dependencies onfoo
,B
's transitive dependency is (at least initially) also an extrafoo[extra]
, and that the first transitive dependency pip finds fromA
onfoo
does not agree with the first transitive dependency pip finds forB
onfoo[extra]
becausefoo
did not haveextra
for that version.If the behavior is modified that pip errors out immediately then the user is left to manually resolve the dependency graph themselves (and almost certainly some users will report an issue to pip and I will be manually resolving the dependency graph ;)).
IMO the why is "extras should be treated as real requirements", currently if you install
foo[extra]
you have no idea if you're going to get[extra]
, it's a bit like if pip's requirements worked like requestingA B
and you definitely got A but you maybe got B. This would need to be written up more formally / user facing.pfmoore commentedon Oct 4, 2024
Your scenario is the same as mine, but made more complex to emphasise the problem it causes. My main point here is that someone gets inconvenienced no matter what we do, and complex build hierarchies with invalid extras embedded deep within them is a rare case, so I'd rather not optimise for that situation, but instead ensure that the common scenario (which frankly is
pip install A[mistypd_extara]
) is handled well. Downloading the whole history of A only to raise an error, is not (IMO) handling that situation well.The problem with "extras should be treated as real requirements" is that it's not even clear what that statement means. IMO the semantics of extras should be clarified at the standards level. Every tool needs to have a shared understanding of what depending on
foo[extra]
means, and I don't think it helps users at all if (for example) pip and uv choose different answers to this question.Hang on, that's a bit of an extreme statement. If you install
foo[extra]
and foo providesextra
, then it'll be installed. The edge cases only arise when things aren't that simple - either foo doesn't provideextra
, or it sometimes does and sometimes doesn't, depending on version, or there's an installed version offoo
(where we have no way of knowing, because there's no standard for recording it, whetherextra
was installed as well). Those are precisely the cases where I don't think the standards give enough information on whatfoo[extra]
means in the first place.notatallshaw commentedon Oct 4, 2024
Yes, just as pip does now with backtracking, some users are inconvenienced and would prefer pip not to backtrack at all when it can't immediately resolve the requirements, and just error out. Maybe this option should be added and then users can choose?
This is an assumption, that the reason users are installing the vast majority of extras is because they are typing them on the command line, and not receiving them as transitive dependencies. Do you have any evidence for this?
And even if that's the case (which I'm highly skeptical of) why would pip optimize for the user mistyping a requirement? Pip doesn't do that with any other situation, e.g. if a user mistypes requirements on boto3 and urllib3 they can end up downloading thousands of packages.
That's not correct though, if a newer or older version of foo could provide that extra and then the version pip checks you don't get that extra. So even if
foo[extra]
is a valid requirement and a valid extra, you have no idea if you're going to getextra
or not, unless you've already resolved the dependency tree yourself.notatallshaw commentedon Oct 4, 2024
There's no standard for how installers should resolve requirements, so it's up to pip to follow its own internal consistency. For example:
foo-extra
and there is no packagefoo-extra
is available for the user then pip doesn't install anythingfoo extra<1
and every version of foo has dependencyextra>1
then no version of foo is installedBut pip has chosen to treat extras as special, and if it can't immediately find
foo[extra]
but can findfoo
it will installfoo
. Iffoo[extra]
was not treated as special, I think all these edge cases would be resolved like all other requirement edge cases, these choices were already deemed suitable for other requirements, why should extras be special?notatallshaw commentedon Oct 4, 2024
Okay, to me, a reasonable compromise would be, if
foo[extra]{specifier}
is a top level requirement only (i.e. a user requirement) then add special logic that if pip can not findextra
infoo
for the first version it finds immediately error out with something like:This could be added independently of whether pip treats extras as real requirements, and backtracks on them, and would handle the typo scenario.
P.S Sorry for the 3 posts in a row.
meejah commentedon Oct 4, 2024
The last comment above sounds like it would do what I want for my particular use-case.
It seems to me that statements like "extras should be treated as real requirements" are getting towards a philosophical position on "what even is an extra?" Such a document / conclusion would be good to have, IMO, no matter what else happens with this PR.
I can appreciate the complexity of some of these scenarios, but my itch is that I do in fact keep getting bitten by mistyping / misremembering an extra name, and I'd really love a way to configure my
pip
to more-obviously alert me to this scenario. I had thought that--give-me-an-error-not-a-warning
combined with making the warning an error would be "least disruptive". Okay, so it turns out that's not true ;) but also I'm not really in a position to decide on a philosophy for extras and also implement it.If the proposal in the comment above this one isn't acceptable, an even less-invasive thing could be to improve the warning text, and make it appear last in the output? (No opt-in etc needed, because there's no behavior change). Consider the following two examples, of current behavior:
versus
For me personally, I do type extras on the command-line and I do sometimes "guess" (e.g. "maybe it's
example-package[dev]
for the development requirements?). So my itch here is to save future-me the time of figuring out which configuration methodexample-package
uses, read it, etc to find out what extras it has. I realize I'm just one user though.notatallshaw commentedon Oct 4, 2024
Great, I would be +1 on a PR that is limited to erroring out on top level requirements. It may still need to issue a deprecation notice for 6 months before changing the behavior, but I don't think it would need an opt-in flag (the behavior is limited, clear, and user actionable).
I would be strongly -1 on erroring out immediately when pip finds a transitive dependency that doesn't match the extra (as I posted in length, lol).
pfmoore commentedon Oct 4, 2024
No more than anyone does in situations like this. I'm mostly going off the fact that @meejah said that was what they most commonly encountered.
I think that's a mischaracterisation, but I don't have the time right now to debate the history and the various choices made over years of pip's development. Let me just say that if you believe this is a better model, and want to do the work to change pip over to that model, I'll reserve judgement until I see the results. But I'm not convinced that we should be making design decisions based in that model until pip implements it. At the moment, I'm certainly getting confused by the implications of what you're saying, because you're working from a different mental model than I am. And I don't think you're fully understanding my concerns for the same reason.
This feels dangerous to me. You're making a big point that pip treats extras specially, and we shouldn't do that, and yet you're now suggesting we treat top-level requirements as special when that's not something we currently do. I'm nervous about the inconsistency here, and I suspect it'll come back to bite us. For example, is a requirement in a requirements file "top level"? What if it's generated by something like pip-tools?
The only place we currently treat user supplied requirements as special is when it comes to upgrading already-installed requirements. And the behaviour around that is another place where there are a number of problematic edge cases. I don't think we want to repeat that decision - the only reason we have the current behaviour is because it was the least bad way of preserving some level of backward compatibility in a situation where there were no good answers.
Absolutely. This is very much my position, with the qualification that I don't think we can treat that question as something that pip can decide alone. I don't think it's acceptable for different installers (or lockers, or other tools) to behave differently when encountering extras. So we need a common decision on semantics (whether we call that a standard or not - @notatallshaw points out that there's no standard for resolving requirements, but outside of extras, I don't think there's a lot of debate as to what the right answer should be1...)
As a general point, improving the visibility of warnings is something I'd support, so +1 from me on something along those lines.
I'm not comfortable with an error only on top-level requirements - certainly not without a lot more clarification of what that would mean and how various edge cases would work. I don't think the benefits are sufficient to justify the added complexity. (I am convinced by what @notatallshaw said that raising an error in all cases isn't realistic).
Basically, this discussion has left me feeling that we're best sticking with the current behaviour (while improving the visibility of the warning). I'm happy to hear opinions from the other maintainers, though.
Footnotes
Actually, there is. I don't think there are good answers to how to resolve when faced with a non-empty target environment, but let's ignore that for now, as it would take us too far off topic. ↩
notatallshaw commentedon Oct 4, 2024
Yes, I'm now convinced this should be a separate issue. I will make a new issue proposing this new model, and when I have time I will make a PR to prove that model, then the merits can be discussed in that issue/PR, not here.
This is not true, "top level requirements" are also special in a number of ways, e.g.
Pip already does this when top level requirements that are inconsistent. This would be additional special behavior, but top level requirements are special, because pip is able to identify them without needing to take any resolution steps. This is effectively their definition, which in practise is all requirements immediately available to pip via CLI or via requirements files.
meejah commentedon Oct 4, 2024
Just so I'm clear, "top-level requirement" means all the requirements collected before we enter the resolver? That is, for
pip install
, everything collected inreqs
before here? https://github.com/pypa/pip/blob/main/src/pip/_internal/commands/install.py#L378notatallshaw commentedon Oct 4, 2024
That is what I meant by this, yes.
pfmoore commentedon Oct 4, 2024
There's an attribute
user_supplied
on the requirement object. https://github.com/pypa/pip/blob/main/src/pip/_internal/req/req_install.py#L157. That's what I'd assume you should use - we definitely don't want to end up with multiple different concepts of what's a "top level" requirement...