-
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
pip silently fails to install extras and returns error code #7122
Comments
Not sure what you're asking for. Could you provide a clear reproducer of this issue? |
@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. |
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. |
@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 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). |
If you could provide a reproduction that shows that behavior it would help, I could not reproduce it locally:
results in
Note the 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. |
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. |
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. |
What do you mean by "pass extras down to dependencies"? |
Does installing |
My immediate answer would be "no, obviously not". What makes you think it might do? (I'm not super familiar with extras, so I could well have missed something, but I don't recall ever having heard of anything like this). |
Yea, it doesn't. I seem to be mixing concepts. Nvm me. :) |
I'm running into this as well. What's the status of adding a |
I don’t think anyone has done anything on it yet, likely since behavioural deprecation is quite messy to deal with and non of the active contributors care enough. I personally think it is perfectly fine for pip to silently drop non-existent extras. One chance to introduce the breaking change without deprecation would be in the new resolver. We already handle extras slightly differently there, and this would offer a good gauge how many are actually affected by it. |
Is there a good reason to keep the existing behavior? If not, I'm perfectly fine w/ deprecating it and making this an error. 🤷🏽 |
I definitely don't consider this to be a "critical bug", considering how long it's been around before anyone mentioned it. If there were any movement, it would be in the form of a PR, which would have been linked here - so no, there's been no action on this. Someone will need to create a PR if they want to move this forward - it seems unlikely from the comments here that any of the pip developers are particularly interested in implementing it (I certainly won't be doing so). Such a PR would likely need some discussion around the deprecation process, but it seems pointless to get into that before there's a PR. |
My impression is that most of us are fine with deprecating the current behaviour and changing it to an error, but no-one really cares enough to implement it. @chrahunt covered what's needed for deprecation here. |
It’s more like there’s no convincing reason to change the existing behaviour for me. I first learnt about extras from setuptools (like most people, I persume), and having non-existent extras to only emit non-critical warning feels like |
That's a good idea!!! Are there any strong objections? |
Does the existence of the "waiting PR" tag imply that there is consensus here and a PR would be accepted if it:
...or would such a PR just produce 5 more years of discussion? |
Digs like this aren't particularly productive. If you would like to create a PR, please do so. To answer your question, there's no consensus here yet, and there are some difficult questions that need to be answered, but the only way to really understand what the best approach would be is if someone creates a PR, so there's no real point simply discussing this further without a proposed solution. One question you might want to consider: If package And a second question, based on your suggestion that we "list the valid extras". In a package that has 50 released versions (not impossible) with different extras across those versions (perfectly possible) do we download and check all of those versions, just to list the valid extras? These are the sorts of things that writing a PR would make more obvious, and which need to be decided before anything can be accepted. Personally, I have no strong opinions on what the "right" behaviour is here, so my preference is the solution that has the least impact on pip's overall behaviour. But what that involves won't be obvious until there's a PR. |
Sorry for the snark. I have tried to start such a PR a couple times (whenever I get extra-bitten by this) but it's pretty de-motivating to consider it simply sitting here, and I don't really know what the tag means. I'd rather write a PR that has the functionality actually desired by the project. I do like the slogan "rough consensus and running code" but I'm not sure that "rough consensus" has been achieved here..? My answer to the above questions would be: let the resolver resolve whatever it believes is the right thing, and then it's an error if that extra doesn't exist. The "right" list of extras is the list that exists in whatever version the resolver found. (If you wanted an older version, you'd ask for an older version ... right?) I actually wouldn't want the behavior you describe: if I say "install X with extra foo" and for whatever reason the version found is 24.4.0, and there is no extra "foo" in that version then I want an error. So I wouldn't want to "succeed anyway" without the extra. I suppose it could be the case that some users might want the "older version that does have the extra" .. I don't think I would? (Even if you do, "explicit is better than implicit" right? So you have to ask by saying "install X version 19.1.0 with extra foo" instead.) Maybe this indicates that a more philosophical description of "what even is an extra" needs to exist? Your answers above seem to indicate that an extra is "some bonus functionality that we can more-or-less ignore". My experience is that packages do not follow this; it is often very critical to the correct functioning. For example, switching between "use asyncio" or "use twisted" (but often more than just Some of the 5-year-old comments are about CI: if I ask for "foo with extra dev" it's probably because I want all those tools installed to my CI runs. So again, I want an error. Maybe the package re-named the extra at some point from "dev" to "development" at some point -- again I feel the right behavior is "error" (so I can fix my extra string). I guess my answer is that it's closer to being "part of the package name" and/or "part of the version" but I don't have any deep understanding of the resolver etc. I am definitely not in a position to know what is "the least impact on pip's behavior" here, and if that's the desired metric it would be extremely helpful if you could write down what you believe those changes consist of (I personally would not use such a metric, but I'm just one user). The "ask" here is a change of behavior, so 🤷♀️ does that mean the "least" is to change "warning" to "error" and exit there...? |
the key problem is that its impossible to off-hand know the difference between a extra that has existed before vs a extra that is misstyped/wrong - and its quite a a pain if pipelines suddenly break this had created quite some problems for example for setuptools_scm back when i had dropped the now empty toml extra a number of builds/tools suddenly started to fail, and there was simply no sidechannel to actually warn downstreams so i just added the extra back to stop breaking downstreams its generally not well received to break thousands of downstream builds in a way that requires a noncompatible workaround and its common practice that people don't update certain things unless "forced" |
Thanks for the comments. Your answers sound like reasonable choices to me - I have little or no personal stake in this functionality, so "whatever people want it to do" is fine with me. We've had a few people commenting that the current behaviour is not suitable for them, so hopefully they will say whether your proposals would suit their use cases.
What I was trying to say is that I'd prefer a change that only altered a small part of pip's code, over one that reworked big chunks of the core logic. My main motivation here is to have a PR that stands a realistic chance of getting reviewed (large PRs tend to sit unreviewed because the maintainers don't have the time to work through the impact of the change) and has less risk of unexpected side effects. The only way to know what approach will do that is to write a PR. So you actually are in the best position, if you plan on writing a PR - simply avoid anything that needs major changes to the code.
There's certainly an argument for that, yes. I tend to find extras are badly understood by many people, and used in very different ways by different projects. I don't personally have the appetite for a philosophical discussion of that nature, but if you do, by all means start a thread on https://discuss.python.org/c/packaging/14 and see if you can get a community consensus. |
I'd like to apologize again for the unhelpful comment. |
I would love some guidance on the above draft PR; if a solution approximately like that proposed works, I can add tests and whatever is required for merging. Thanks |
Instead of burying more discussion in the draft PR, there seems to be disagreement about the behavior. Let use consider installing "example_package" with extra "some_extra". There are two versions of "example_package":
To leave with a bikeshed: a developer on the draft PR asked for this to be behind an option. How do we spell that option? Maybe |
I think there are two important things to understand:
For "1." if For "2.", a transitive dependency of a user's requirement may require "foo[extra]" and so the requirement is out of control of the user. If it is a requirement that they must put transitive dependencies in their requirements file for pip not to throw an exception, e.g. "foo[extra]==1.0 # This is required for pip not to throw an error", this is a bad user experience. |
Hmm, okay. So does that mean kind of "moving extras up" into the resolver process? (Currently it's basically a post-hoc test: you decide on something, and then check if there's a matching extra or not). Maybe |
I'm not quite sure what you mean, if a user wants an error for foo 2.0 not having an extra, they should require As for how to implement this? I think you need to catch this exception in factory.py and skip it, similar to #10625 or the reverse of #10655 (there are other PRs that have implemented skips based on certain conditions, but I couldn't quickly find them). I would just like to say, I'm personally very supportive of this behavior change, I've always disliked that pip throws a warning but installs anyway. I'd always assumed this was a standard, but reading through this issue and carefully through the standards, it seems this was a mistaken assumption on my side. |
Okay, so I've hacked together something that ostensibly works -- by catching I think at least part of the issue is that the question "what extras does So if I try to |
Yeah,
Yeah, well at least the metadata, for indexes that support PEP 658 then for wheels it will just be the small metadata file.
This is already true of packages where requirements can not be met, we encourage users to put reasonable lower bounds on their requirements. But yes, this is one of the reasons why this change will be disruptive. Also, optimizations can be made, I think if extras are understood to be a strict requirement it’s possible to do some additional static optimizations while traversing the resolution graph. |
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 (" 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 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 |
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 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 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. |
@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 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. |
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 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 |
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 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
Hang on, that's a bit of an extreme statement. If you install |
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 |
There's no standard for how installers should resolve requirements, so it's up to pip to follow its own internal consistency. For example:
But pip has chosen to treat extras as special, and if it can't immediately find |
Okay, to me, a reasonable compromise would be, if
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. |
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 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 |
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). |
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
|
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. |
Just so I'm clear, "top-level requirement" means all the requirements collected before we enter the resolver? That is, for |
That is what I meant by this, yes. |
There's an attribute |
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.
The text was updated successfully, but these errors were encountered: