-
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
Allow extras in constraints #11599
Comments
The test case above is simplified from a real workflow which uses I do not believe this is a bug in I also do not see any reason why I also do not believe that converting the Or more simply, the suggestion given in the error message is to manually undo the effects of fixing #6628. Since #6628 was considered a bug, Finally, I can find no indication in the current So I believe this is a bug in |
The main reason extras are banned from constraints is it has unclear semantics. Say I have Your input does not contain extras, and pip-tools is generating those entries with extras. It should instead flatten it (with a semantic it chooses) before handing the content to pip. |
A constraint never automatically implies installation. And as I said:
In other words, a constraint file containing I don't think you would say that the following constraint file doesn't have useful semantics, would you?
The reproduction case I provided involves a direct dependency ( I chose this case because it is one likely to trip unwary users -- after all, how was I supposed to know that somewhere lurking in my dependency tree was something that It is especially relevant right now because
No, I believe and explained why I believe it is correct for Or, more simply: as long as And in any case the current behaviors do not make sense; the suggestion to switch to The simplest fix is to fix the underlying bug (and, yes, it really is a bug in Please re-read the report and the reasoning and reconsider, or provide an escalation path for reconsideration by persons other than yourself. |
As another pip maintainer familiar with the resolver, and the person who implemented the constraint implementation for the new resolver, do I count as an “escalation path”? As all of the pip maintainers are volunteers, you’re not going to find anything better than “someone else’s opinion”… I agree with @uranusjr. The semantics are unclear, regardless of whether you assert otherwise, and the current limitations are deliberate. Your claim that the information about extras should be retained makes no sense - the file is generated, and shouldn’t be read by the user - the source Apart from anything else, this is a documented limitation of constraints files - if |
This keeps being asserted without further evidence or argument. My claim is that a constraints file with these contents:
has the well-defined semantics of being exactly equivalent to a constraints file containing the expansion of that extra. Namely, a constraints file with these contents:
What other possible semantics can an extra have, if not its expansion? If the semantics of extras are poorly-defined, why does this only affect constraints files and not requirements files (since as far as I know,
In the case of So your claim that this information "shouldn't be read by the user" does not hold up. I look at generated requirements files quite frequently in order to work out how I ended up with a particular package as a dependency. Which leaves me with... no coherent argument presented for why If you want to convince me that this was a properly-executed deprecation and removal and that a feature request is necessary to restore it, the burden is on you to provide some sort of evidence for that, because so far none has been presented. |
OK, with those semantics, there's big problem: If someone does (I'm not asking as some sort of "you solve the problem", but to demonstrate what others likely meant by unclear semantics) |
As @pradyunsg said, you can't expand the extra without extracting the metadata from the project, as extras can change throughout the lifetime of a project. It's not so much that I have an alternative interpretation in mind, more that I don't even understand what your proposed semantics are.
Fair enough. I shouldn't have tried to suggest how
The replacement of the old resolver with the new one was the change. And that certainly was handled with an extensive deprecation, consultation and transition process. You're free to say what you want about the process, but it was certainly as complete and properly executed as we were able to make it (given the resources we had). The implementation of constraints wasn't independent. The legacy resolver had a constraints feature that was not compatible with the new resolver, so it had to be removed. What we did was re-implement the constraints feature. And we did so by consulting with users of the old constraints feature and understanding (to the best of our ability) the key requirements for the feature. We specifically consulted the original implementers (and users) of the feature, to ensure that we replicated all of the originally required functionality. We explicitly did not try to implement all of the incidental and "consequence of the implementation" features, many of which were simply impossible under the new resolver. I don't think there's any real point in arguing over whether this is a regression, or a bug, or properly deprecated. It won't have any practical effect, and it feels like you're accusing the maintainers of not acting in good faith, which won't help your arguments. If you really want this feature, at this point I think you need to accept that you're going to have to try to write a PR. That will have the benefit of giving you a better understanding the issues around what you're proposing, and as a result you may be better able to argue your case. But be aware that even if you do write a PR, there's no guarantee that it will be accepted. Including good documentation in the PR might help, but ultimately you'll still have to persuade at least one of the maintainers that the feature is worth having. |
I wanna push back on this @pfmoore -- I don't think the only way to get this feature is by filing a PR. I think making a reasonable case for what this feature could actually behave as is also a good way to move this forward. I don't think the proposed semantics work, but there might be an alternative interpretation. |
I'm going to give a longer explanation, but the short explanation is I think there's a circular argument here, basically "
The fact that computing dependency information is non-trivial isn't really a technical argument against this, because
Turn this around: if I have unlisted dependencies and try My understanding is that And if it is well-defined behavior for
Suppose that But nobody has, to my knowledge, used this as an argument for I have not yet seen a consistent argument presented for this, and as I explained further up I don't see how there can be a consistent argument that the behavior is well-defined only for one option and not the other. It seems to me that either it should be well-defined for both
I think this statement is not in good faith, or at least not up to the standards I would hope this project maintains. I would ask that you not make such statements in the future, please. |
Sorry, you're correct. I was thinking more that to present a convincing case would need a good understanding of the current resolver and how the new constraints implementation interacts with it. I tend to conflate getting that sort of understanding with "have a go at writing a PR". |
Let me try a different angle here. Requirements files and constraints files are completely different concepts. Requirements are "here's things I want you to install". Constraints are "here's things to help the dependency resolver give me a result I want when trying to install the requirements". If you need to do dependency resolution to compute the information that was supposed to help with dependency resolution... you have a recursive problem.
FWIW, it certianly can be -- what I was hinting at with that is the implementation complexity -- that is not something that comes for "free" and it's always a call about tradeoffs. In this case, this is forbiddingly complicated (for more flavour on this -- I'll point out the three people who've wrote pip's current resolver + constraints implementation over 6+ months full-time, with a broader group and a lot of input from the community, are all pushing back saying that the value proposition isn't there). It's not that "avoid pip performing a non-trivial computation" that's a problem -- it's a change in the entire design of how pip's dependency resolution model works and how constraints interact with them. Currently, a constraints file is static and doesn't influence the dependency resolution process beyond the requirements as-is. With needing to do dependency exploration for constraints files themselves, that'd mean we need to expand the dependency graph for those files as well as the regular |
There's also an additional problem/concern: Currently, pip's not going to try to fetch a package that is not listed in requirements or the transitive dependencies of it. The change proposed here would break another contract around constraints files -- that they don't affect which packages get fetched/requested, only which versions of the packages get fetched/requested. |
And, because I don't like leaving questions unanswered in discussions...
Only look at the top-level requirements. Which means, your constraints file with extras behaves differently, which may not be what you want. :)
Well, it requires everything to be pinned + hashed, so you'll get an error. :)
I did answer that above.
Because that's literally the point of having extras + requirements -- but not the point of constraints. Again, covered above. |
And, finally:
This sort of usecase is part of why pip-compile has a |
I'll also come at this from a slightly different angle, and go back to the original use case. The use case for So I split them into multiple requirements files. Which I understand to be a supported and encouraged use case. And I also compile them to pinned and hashed dependency trees, so that my builds are as reproducible as can be, which I understand to be a supported and encouraged use case. And as part of that compilation, I feed each compiled requirements file to the "next" one via And I do this because I've been bitten by not doing it. To run with the I understand this to be one of the primary use cases of constraints files and
And I'm saying that This is why I say it's circular: when I look into it, this stops being a technical argument about the expansion being difficult (since
This is getting very far afield from the original argument used to close this issue, which was that the semantics of extras in constraints are unclear or ill-defined. It seems you are agreeing with me that the semantics of an extra in a constraint would be perfectly well-defined (treat an extra as identical to the expansion of that extra), and are instead saying that the semantics require But again this is not really a technical argument. Prior to the resolver change,
As I see it there are two possibilities for I don't see how or why
I still don't see any clear argument for why And I don't think this problem is going to go away. More and more people are adopting workflows that use multiple requirements files and rely on tooling to generate "compiled" requirements, and more and more packages are adopting extras now that the tooling for using them is getting more mature, and constraints are the documented way to ensure a compatibility across multiple requirements files. And so more and more people are going to run into things like this that occur at the intersection of all those things. And telling them to go memorize a bunch more non-default command-line options doesn't feel like a good solution, especially when the semantics for things like extras in constraints do seem to be quite well defined. Or telling tooling authors to have their tools do the full expansion also doesn't feel right; I could see such an author justifiably pushing back and saying "no, dependency resolution is Especially since the tooling, at least in my specific case, appears to actually be using the So for the sake of both users of packaging tools, and authors of packaging tools that work with |
Well, my responses literally started with "OK, with those semantics, there's big problem" -- I'm operating with your semantics and trying to point out the issues with that. I don't think there any good way for this to work that isn't either (a) backwards incompatible or (b) difficult to reason about or (c) forbiddingly complex in the codebase. To be clear, I don't think the semantics you're describing are "perfectly well-defined". And, I agree with the rationale provided for closing this, which was:
You have a fully-locked environment -- passing
Theoretically, I guess you could argue that dependency resolution is NP hard so adding more semantics will still keep it NP hard, so it's within the capabilities of the resolver. In the real world, no, it's not. You're welcome to look through the code -- follow this value (and compare that to
(psst, do it on the
I welcome that -- I'd personally love for more alternative dependency resolvers to exist, especially if they're not direct-derivatives of pip's and we're able to learn from them or reuse their logic even! :)
I hear you, I empathise and -- as someone who dealt with the suboptimal design choices made in the legacy resolver, co-authored the 2020 resolver, was involved in extensive discussions about the handling of constraints within the 2020 resolver -- I don't think so. (see the "thanks" section on this blog post for the number of people involved with that rewrite: https://pyfound.blogspot.com/2020/11/pip-20-3-new-resolver.html) At this point though, I do feel like we're talking past each other and it'd be better for me to step away from this for a while. |
I'll avoid responding on everything here because (1) it's late where I am, and (2) I don't want to escalate things further, I want to spend time properly thinking about what's been said. However:
I don't know if there's any useful discussions I can point you to that you haven't already seen, but this honestly wasn't what happened. The "new resolver project" was a major piece of work with the explicit goal of replacing the old, broken and unmaintainable resolver with a reliable, correct implementation. It was not, and never could be, a like-for-like replacement, and we were funded to do a lot of outreach and transition work alongside the technical aspects of the project. When we came to look at constraints, the way they were tied into the old resolver (which was, as you say, something like a "sort of requirements file") had no equivalent in the new resolver. It's not that it was "hard" to replicate the old functionality, some of the key ideas in the old resolver simply no longer existed. This was a big problem, because constraints files are an important feature. So we had to essentially redesign them from scratch. What we did, was to reach out to the people who originally implemented constraints files, to establish precisely what was the core feature set that motivated the original implementation, and what was essentially an "accident of implementation". On top of that, we looked at which of the "accidental features" were likely to have become important to our users. I will note that the (extensive) user surveys we undertook as part of the project didn't highlight any particularly esoteric features of constraints that we hadn't considered (although in reality, the respondents indicated fairly light usage of constraints, from what I recall). With all of that information, we tried to produce a model of constraints that would (a) fit with the structure of the new resolver, (b) provide the key requirements that motivated the original design, and (c) would be consistent and easy for users to reason about. That model can be summarized with the phrase "constraints limit the versions that the resolver sees". (That's obviously an over-simplification, but it's the most basic characterisation of the model). One implication of that is that constraints work on the list of versions passed to the resolver, and as such are applied before any of the work the resolver does has started. That's in contrast to requirements, which are the input to the resolver, and so are processed by the resolver itself. In particular, the constraints mechanism has no access to the machinery that builds candidate packages and extracts metadata from them. So specifically, it can't extract the definitions of extras. Yes, it's possible to do things differently. But it would be a huge rewrite of a very sensitive part of pip's code. And furthermore, it would require yet another redesign of the model underlying constraints - we can't just "go back to the old model" as I noted above, and the current model doesn't support what you want. So a new model would be needed. One thing I will note, in the course of checking what I was saying I reviewed the constraints documentation, and it didn't really get updated to explain the new design. There's a brief note about the rewrite, but the rest is mostly from before the resolver changes. That's bad, and probably on me (as the person who wrote the bulk of the new constraints model). We ran out of budget, unfortunately, and documentation is always one of the first casualties. I'll try to update that section, but I'm hesitant to do so right now, as it could all too easily look like I'm trying to "rewrite history" to eliminate evidence that pip should work the way you want. So I'll probably leave it a while. So history lesson over. I hope it was useful, and doesn't simply come across as me trying to defend our position. It doesn't really alter the discussion about how or if the requested feature would work, but maybe it helps explain why this isn't as simple as reconsidering the removal of a (relatively isolated) feature. |
Given that pip-tools doesn't even document
And this strikes me as contradictory reasoning, in multiple ways:
And still nobody's come forward with what they think would be any reasonable alternate semantics of an extra in constraints. What other semantics could it have, if not expanding it?
Producing a set of requirements files that are mutually compatible with each other seems to me to be one of the most important use cases for While it's understandable not to want to take on a complex piece of work, I've said multiple times now that I think this will only get worse as time goes on. I think there are a lot more ticking time bombs in people's packaging workflows that are going to go off the instant the new resolver is default/enforced in popular toolchains like pip-tools, and that this current GitHub thread is simply the harbinger. You can choose, now, to get out ahead of that and be able to say when the time comes that "we know this is a problem and here's how we're already tackling it", and I think that's clearly the right thing to do. And I think most of the people whose flows are going to break, to be blunt, simply are not going to care that their use case apparently didn't come up, or wasn't considered important enough, in prior research, or that the decisions are now difficult to change. They're simply going to care that their workflow used to work and now doesn't. It also still is not really a technical argument -- at best it's pointing out that decisions were made which would be a lot of work to undo. But even if it is not a simple thing, I will say once again: I think those past decisions about constraints should be reconsidered. And it doesn't even have to go fully back to the old "anything a requirements file allows" -- just allowing extras would probably avoid a huge percentage of the coming breakage, since extras are the things most likely to blow up on someone unexpectedly due to both the growing popularity of extras as officially-recommended ways to install certain packages, and the ease with which they can sneak into transitive dependencies that even a careful user might not become aware of until suddenly their workflow errors out on them (as mine did the other day). |
@ubernostrum It appears this is already being discussed under jazzband/pip-tools#1613, which you're participating in. I hadn't spotted the link Github added above. From my reading of that issue, the pip-tools developers appear to be having a productive discussion on how to handle this. I suggest we wait for conclusions from that issue, and if the pip-tools developers feel that there's something that pip could do which would be of benefit to them, then we can have a discussion. Given that pip-tools works by using pip's internals, I expect the pip-tools developers to have a good understanding of the implementation details here (they must do, because using pip's internals is unsupported, so they will need to know what they are doing in order to keep pip-tools working!) So we should be able to discuss trade-offs and options productively with them. |
Description
Currently
pip
does not allow extras in constraints. It did, previously. The fact that it does not now is incorrect and should be treated as a bug.Expected behavior
Both compilations given in the steps to reproduce succeed, producing files
app.txt
andtests.txt
.pip version
22.3.1
Python version
3.11.0
OS
macOS 13.0.1
How to Reproduce
Create a new venv:
For me, at time of writing, this yields:
Create the following files with the following contents:
app.in
containstests.in
containsAnd now attempt to compile with pip-tools:
Output
Code of Conduct
The text was updated successfully, but these errors were encountered: