Skip to content
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

-e . does not take precedence over pin in requirements file #8307

Closed
davidism opened this issue May 22, 2020 · 26 comments
Closed

-e . does not take precedence over pin in requirements file #8307

davidism opened this issue May 22, 2020 · 26 comments
Labels
C: dependency resolution About choosing which dependencies to install C: error messages Improving error messages state: needs discussion This needs some more discussion type: feature request Request for a new feature UX User experience related

Comments

@davidism
Copy link

I've been experimenting with using pip-compile for Jinja, and have run into an interesting circular dependency between Jinja and Sphinx. Jinja uses an editable install (-e .) for development. Jinja lists Sphinx in requirements.in for building docs. But Sphinx depends on Jinja, so jinja2==2.11.1 gets pinned in requirements.txt. But this pin overwrites the locally installed version of Jinja, which is what should be documented and tested.

I wanted to provide contributors with a command to set up a dev env. Using the current resolver, pip install -e . -r requirements.txt ends up installing jinja2==2.11.1 from requirements.txt instead of -e . The order of -e and -r in the command doesn't matter.

Using the alpha resolver, pip install --unstable-feature=resolver -e . -r requirements.txt (or -r -e) produces an error.

$ pip install --unstable-feature=resolver -r requirements/dev.txt -e .
Obtaining file:///home/david/Projects/jinja
ERROR: Could not find a version that satisfies the requirement ExplicitRequirement(EditableCandidate('file:///home/david/Projects/jinja'))
ERROR: Could not find a version that satisfies the requirement jinja2==2.11.2
ERROR: No matching distribution found for jinja2, jinja2

I would expect a local install to always take precedence over pinned requirements. Otherwise, projects that depend on each other during development will have a more difficult time pinning requirements.

As a workaround, the editable install can be done as a separate command. Tox, Nox, and ReadTheDocs can install the local project after the dependencies, but adding a second command introduces a hard to detect way for new contributors to misconfigure their local environment.

@pfmoore
Copy link
Member

pfmoore commented May 22, 2020

If you have a pinned requirement and a local directory that conflicts (i.e., is a different version) then it's not possible to satisfy both requirements. So the new resolver is behaving correctly here.

@triage-new-issues triage-new-issues bot removed the S: needs triage Issues/PRs that need to be triaged label May 22, 2020
@pradyunsg pradyunsg added this to To do in New Resolver Implementation via automation May 22, 2020
@pradyunsg pradyunsg added this to the Print Better Error Messages milestone May 22, 2020
@pradyunsg
Copy link
Member

We should have a better error message in this case, though. :)

/cc @ei8fdb @nlhkabu

@davidism
Copy link
Author

davidism commented May 22, 2020

Then it's impossible for me to pin dependencies for Jinja (or in general any project with mutual dependencies), which seems wrong. I understand the behavior of the new resolver is logical, but it's not the only correct solution.

@uranusjr
Copy link
Member

One thing I feel needs clarification: Is the local project’s version 2.11.1? The new resolver will be able[1] to resolve to the local project in that case. If the local project is not 2.11.1, I don’t think accepting it is a correct solution.

[1]: It does not resolve currently yet, but I believe we intend to support this.

@davidism
Copy link
Author

It's the next dev version, currently 3.0.0a1.

@davidism
Copy link
Author

There's never a time that the local development version would match the released/pinned version. I feel like the -e flag should be removing any pin for that name from the requirements. Presumably if you're developing against those requirements you'd be opting in to possibly breaking (and then fixing) compatibility with them. Unfortunately, that's exactly what an automatically generated pin of a sub-dependency prevents right now. I could manually remove the pin, but then I lose the benefits of tools like pip-compile and Dependabot, they'll just add the pin back in.

@uranusjr
Copy link
Member

I disagree. A requirement is a requirement, there’s no reason to treat editable installs specially. Indeed there is a genuine use case here, but it would be wrong IMO to make editables a special case for it. There are other ways to achieve this byt introducing workflow tools.

@davidism
Copy link
Author

Note that I can essentially "break" the dependency tree anyway by doing pip install --unstable-feature=resolver -e . afterwards. All I'm really asking is that the single command pip install -e . -r requirements.txt be equivalent to that. I can't think of a case where that wouldn't make sense.

@pradyunsg
Copy link
Member

pradyunsg commented May 22, 2020

@davidism I think understand your perspective on this. I do agree that there should be some way to say "I want this, ignoring all dependency conflicts that this choice would cause".

OTOH, pip really shouldn't be ignoring dependency requirements declared by the projects (that's the whole point of a proper dependency resolver!), unless very clearly told to do so by the user. And even if we provide a mechanism like this, we'd need to be very careful that we're not designing a footgun for our end users.

While you understand the implications of such a mechanism, I do think it's very difficult to explain it to someone who's not familiar with the nuances of a broken dependency graph.

I don't think that because --editable <something> was passed, that the <something> should result in that choice getting precedence. That sounds like a very sharp edge for something to run their finger against (A simple example is a hypothetical Jinja 4.0.0 that's under development, making backward incompatible changes, that are not compatible with the current sphinx version and... the docs build breaking because of this behavior in pip) and giving the user with a painful experience. That said, I do think that pip should have an escape hatch for users who really do want to be able to do stuff like this (eg: if tox wants to do something like this by default, if users really want to do this to override an improper dependency constraint etc) -- with the clear expectation that the responsibility for dependency breakages is no longer on pip.


That "break" in the second command is permitted today only because we've actually not decided on how we want to report/behave in cases like that (#7744) so there's no guarantees that that'll keep working.

@uranusjr
Copy link
Member

Note that I can essentially "break" the dependency tree anyway by doing pip install --unstable-feature=resolver -e . afterwards.

And I’m going to tell you there’s a group of people telling us we should also make this impossible because they can’t think of a case where it makes sense 🙂

The problem with pip here is there are so many workflows out there, it’s impossible to accommodate all of them. The new resolver guarantees dependency robustness within the packages you installed together in a given install command, while providing an escape hatch if you explicitly want to break the guarantee (running another install command to override the result from the previous one). I feel the this behaviour is likely the best pip can do (or close to it).

@davidism
Copy link
Author

davidism commented May 23, 2020

If installing in multiple commands stops working, it really will be impossible for Jinja to pin dependencies for testing and docs. I feel that -e . is telling pip that you're opting into that.

Your example with hypothetical Jinja 4 wouldn't happen. Either Jinja 4 dev wouldn't break Sphinx (likely), or it would and that's exactly what the tests should show so we can fix it, or it would and we would choose a different docs engine anyway, in which case Sphinx would no longer be a dependency.

Jinja (and all of Pallets) seems to be about as vanilla as you can get. We only require venv and pip to get a development environment. Adding another flag for this just makes the command more complicated for new contributors.

@davidism
Copy link
Author

This is also causing issues in Click, because pip-tools depends on Click, but pip-tools is a dev requirement for Click.

I didn't notice until a new contributor was confused because they couldn't run pytest even though it looked like all the dependencies were installed.

@pfmoore
Copy link
Member

pfmoore commented Jun 18, 2020

This sounds like it's a variation on the issue being discussed in #8076. If it is, can I suggest that we link this issue to that one, and move further discussion over to that issue?

@pradyunsg pradyunsg added C: dependency resolution About choosing which dependencies to install state: needs discussion This needs some more discussion type: feature request Request for a new feature and removed C: new resolver labels Jun 18, 2020
@pradyunsg
Copy link
Member

pradyunsg commented Jun 18, 2020

For clarity, I've removed the new resolver label from this, and added the appropriate labels here.

This issue is requesting for a change in the behavior of pip's handling of editables, when it conflicts with other requirements. The current resolver is giving an undesirable result, due to how it handles the conflicting requirements, and the new resolver is rejecting this as conflicting requirements have been fed into the resolver and its capable of detecting that.

@pradyunsg
Copy link
Member

@pfmoore I think I agree that this is related closely to #8076. I don't know if merging them is the right thing to do, but I was sure that the labels in this issue were incorrect. :)

@nlhkabu nlhkabu added C: error messages Improving error messages UX User experience related labels Jul 28, 2020
@nlhkabu nlhkabu removed this from the Print Better Error Messages milestone Jul 29, 2020
@brainwane brainwane moved this from To do before prod release to Post-release work in New Resolver Implementation Oct 7, 2020
@brainwane brainwane removed this from Post-release work in New Resolver Implementation Oct 7, 2020
@brainwane
Copy link
Contributor

Per conversation in today's meeting I've removed this from the new resolver implementation project board; editable-handling will be tracked here, and the other issue at #8076.

brainwane added a commit to brainwane/pip that referenced this issue Oct 27, 2020
brainwane added a commit to brainwane/pip that referenced this issue Oct 27, 2020
brainwane added a commit to brainwane/pip that referenced this issue Oct 27, 2020
brainwane added a commit to brainwane/pip that referenced this issue Oct 27, 2020
brainwane added a commit to brainwane/pip that referenced this issue Oct 28, 2020
brainwane added a commit to brainwane/pip that referenced this issue Oct 29, 2020
brainwane added a commit to brainwane/pip that referenced this issue Oct 29, 2020
@davidism
Copy link
Author

davidism commented May 13, 2022

Is there any update to this? It sounds like @pradyunsg above, and @pfmoore here #8076 (comment) understood what I was talking about to some degree, but all the responses here so far don't seem to have any answer. Click, Jinja, and other popular libraries used in development tools, need to know how to develop our own libraries when we are transitively in our own pinned development dependencies.

I've moved on to just telling everyone to do pip install -e . after doing pip install -r requirements/dev.txt. But from the responses above it sounds like that's not guaranteed to continue to work either. If that's the case, before this is completely broken I need an alternative answer to how to develop a new version of a library while also pinning development dependencies that include it.

@pfmoore
Copy link
Member

pfmoore commented May 13, 2022

The comment of mine that you linked to explains the behaviour you're asking for, I think. What still isn't at all clear is whether having the ability to install a known-broken environment is an improvement for pip. It's clearly beneficial to the cases you mentioned, but it's very hard to establish to what extent people might be relying on the current behaviour, simply because no-one tells us when things work 🙂 And from a logical perspective, the current behaviour is actually right.

You need to understand that you are deliberately lying to the resolver here. Sphinx says it only works with jinja2==2.11.1, and yet you're installing jinja 3.0.0a1. There's no way that a resolver that requires dependencies to be met can say that's valid, because bluntly it isn't.

In my opinion, you're to an extent creating this issue for yourself, by trying to put every tool you need into the same (virtual) environment. In doing so, you expose yourself to dependency hell, precisely the issue that independent application environments is intended to address. In reality, you should put development tools like Sphinx into a separate virtual environment, so that they can have their independently managed sets of dependencies without conflicts. I'll note that if you do your docs build using tox or nox, those tools will manage the virtual environment for you, so you don't have to worry about that at all.

@davidism
Copy link
Author

davidism commented May 13, 2022

I can't fix the fact that Sphinx has to be installed in the same environment as the thing it's generating documentation for. And Sphinx doesn't say "exactly 2.11", but pip-tools and every other dependency pinning tool does not have a way to exclude pins for a given dependency.

@davidism
Copy link
Author

davidism commented May 13, 2022

I'm getting really frustrated because people keep telling me "what we're doing with pip is right" but not actually answering my question, which is "with the given plans for pip's resolver, how am I supposed to pin my development environment while also developing a new version of a library that is used by tools in that development environment".

@hmc-cs-mdrissi
Copy link

hmc-cs-mdrissi commented May 13, 2022

There’s an open pr in pip tools (disclaimer from me) that would support excluding a dependency. If that pr were to land you could do,

pip-compile requirements.in —exclude sphinx —output-file requirements.txt

the pr is mostly stuck in slowness to review/land prs in pip tools. The tests are at moment failing but did pass in past and I can update pr and make them pass if needed but even if they pass again I have no clue when that pr would merge.

I originally created that pr for a very similar reason. Multiple libraries developed in same repository that needed to use -e and not be pinned for dev environment.

@pradyunsg
Copy link
Member

I think the solution that you have right now, of doing it in two steps, is a reasonable one.

I don't think we'd be breaking that workflow, not unless we have something something else that makes it possible to use the initial workflow that you'd described -- having a single command / some way to say "Yes, this will cause a broken dependency graph in the environment, install it anyway" -- and if the migration/churn costs from that are deemed worthwhile. We want to get there eventually, but I would be surprised if that happens within this year or even the next one. :)

@potiuk
Copy link
Contributor

potiuk commented May 14, 2022

I concur with @pradyunsg pip install -r requirements/dev.txt followed by pip install -e . sounds like precisely how develpment environment I'd imagine to work and one that is easy to reason about and explain to contributors.

There are many more much more cases that you are opening here and there is no reason IMHO to make this particular case simpler by combining the two commands - I think it's better that pip does what it promises - keeps dependencies in check when requirements are specified.

I can imagine for example when you would like to make a change that needs to be implemented in both - jinja AND some fix that needs to be done in sphinx to support it. This is what probably you would have to do (not 100% sure if this is right but that's how I'd approach it):

  • pip install -r requirements/dev.txt in jinja
  • pip install -e . (in checked out sphinx to bring new sphinx and it's dependencies from the source code of sphinx)
  • pip install --no-deps -e . (in checked out jinja )

And there you would not have any way to do it in single command either.

I really like the fact that each of those commands does exactly what you ask it to and I do not have to exercise any more mental effort to figure out what else is happening there and which of those dependencies take precedence).

I don't see why you somehow want to combine the two commands into one, they are logically doing different steps.I believe this is precisely as expected from the development environment.

I think as a developer you should manage the dependencies on your own as you are effectively developing the dependencies. Expecting pip to manage dependencies for those cases is not a good idea as you have to know exactly what you are doing and what your intention it, and it's better to explicitly state your intention by using the right commands - rather than making the tools guess your intentions implicitly.

But maybe I am missing something - Is there any deeper problem except doing it it in two steps rather than one?

@davidism
Copy link
Author

I'm fine with doing it in two steps, it's good to hear that PyPA maintainers/contributors acknowledge it and don't plan on breaking it. I was concerned because I hadn't yet got that direct answer.

@pradyunsg unless you want to leave this open, I'm going to close it, I'm fine with your answer as the resolution to it.

@pfmoore
Copy link
Member

pfmoore commented May 14, 2022

PyPA maintainers/contributors acknowledge it and don't plan on breaking it

There's a proviso here. The pin on jinja is (as I understand it) in your requirements file, not in the Sphinx dependencies themselves. So once the dust settles and the two install commands have been executed, there's no dependency in the installed packages that is unsatisfied. As long as that's the case, the 2-step install is fine.

But if Sphinx had a runtime dependency on (say) jinja<3.0.0, then the resulting environment is inconsistent. In that case, tools (including pip) may well refuse to work with the resulting environment. I believe that at the moment, pip only issues a message saying that the environment is inconsistent, and doesn't refuse to do the install. But that is something we might change.

As long as the pin is only in the requirements file, though, you'll be fine with this approach.

@davidism
Copy link
Author

davidism commented May 14, 2022

Yeah, if you change that make sure there's a way for me to still develop Jinja. If I'm developing Jinja 4, and Sphinx requires Jinja<4, I still want to deliberately break that pin in development. Either Jinja 4 will still be compatible with Sphinx (likely), or I'll find an incompatibility and fix it in either Jinja or Sphinx. But either way that requires me to be able to install them together in development in the first place. Same with Click and tools like pip-tools and black.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C: dependency resolution About choosing which dependencies to install C: error messages Improving error messages state: needs discussion This needs some more discussion type: feature request Request for a new feature UX User experience related
Projects
None yet
Development

No branches or pull requests

8 participants