-
Notifications
You must be signed in to change notification settings - Fork 2.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
Installing directory poetry package with dependencies in secondary source fails #1689
Comments
works around pypa/pip#7444 resolve python-poetry#1689
Ok many many hours later I have sorted out the behaviour of PEP 517 and PEP 518 and what its going on here causing the problem. I don't know the full implications, but here is the issue And here is the snippet from pip # Install any extra build dependencies that the backend requests.
# This must be done in a second pass, as the pyproject.toml
# dependencies must be installed before we can call the backend.
with self.req.build_env:
runner = runner_with_spinner_message(
"Getting requirements to build wheel"
)
backend = self.req.pep517_backend
with backend.subprocess_runner(runner):
reqs = backend.get_requires_for_build_wheel()
conflicting, missing = self.req.build_env.check_requirements(reqs)
if conflicting:
_raise_conflicts("the backend dependencies", conflicting)
self.req.build_env.install_requirements(
finder, missing, 'normal',
"Installing backend dependencies"
) Basically at this point, pip has gone into the build isolation and poetry has been retrieved and installed. pip then queries poetry and asks "what else is required for me to build this wheel". To which poetry is responding with the dependencies of the package. I do not know enough about packaging, but I think returning all the dependencies is a mistake. The build system doesn't need my dependencies to package up my wheel, I only need it to use the wheel. I have been able to test this out hacking at pip to be the following: with backend.subprocess_runner(runner):
reqs = backend.get_requires_for_build_wheel()
reqs = [] By doing this none of my dependencies are installed and the wheel is generated and installed the way I expect. I don't know how this impact C extensions or if those are even supported with poetry right now but I think that |
Hello @wakemaster39 , thanks for your investigations and informations in the issue! I hope @sdispater has also some time to look at this. What works for me in the meantime, is to make sure that within the venv the pip version is <19. |
I put some code here for testing.
Trying to install
poetry version: 1.0.0b8 |
Investigating around inside of pip and PEP517, it could also be considered a bug that pip is even calling poetry for dependencies for editable installs. Editable installs currently today only support setup.py installs so there really ins't a reason to ask poetry what it needs if it is never being used. This might change one day if a new PEP ever comes together around an editable install spec, but we have to live with this for now. To work around this would make the already complicated pip code base even more to handle. It is possible to raise this as an issue now that I have a batter understanding of it. But I think we would also need to address the Since I think we can solve this purely from a poetry side, I am going to skip opening an issue with pip about the editable install dependency and we can revisit that as this discussion evolves. |
I'm still not convinced that this is a poetry problem. But this can be due to my little knowledge about pip and how it handles PEP517. I would thought, if the fin swimmer |
I could also be terribly wrong here but from the PEP:
I also realize I cut out the comment block from PIP that gave me additional context on what it is doing. I also updated the original comment with it added in.
|
Here's a short summary about what I've learned from this issue so far and why I think we should implement your hack for now:
Solution I have thought about and reject:
Saying this, there is no satisfying implementation of PEP517 in poetry and pip at the moment. Furthermore there seems no official way to force pip to use the |
works around pypa/pip#7444 resolve python-poetry#1689
Ok, we are coming a step closer.
The question is how a correct PEP508 formatted string should look like respectively how it is interpreted by pip. There seems to be some discussion over there. Or would it be enough to return the path (absolute? relative)? |
Does this solve the following install hierarchy, poetry new aa gives the following error even with the merged part(locally) Tried this hack too. |
works around pypa/pip#7444 resolve python-poetry#1689
Hello @wakemaster39 , it took some time for me, but now I fully agree with you that Thanks a lot! 👍 |
@finswimmer thanks for the update on this, I am happy to see I wasn't crazy on how I read this should work. Reading over your PR (#1874) I think this is wrong. See the following lines from pip self.req.build_env = BuildEnvironment()
self.req.build_env.install_requirements(
finder, self.req.pyproject_requires, 'overlay',
"Installing build dependencies"
)
conflicting, missing = self.req.build_env.check_requirements(
self.req.requirements_to_check
) Pip is already installing everything that is specified in the requires section, from my understanding the |
Hey @wakemaster39 , you are right. PEP517 writes "additional list of strings containing PEP 508 I've changed the PR (now #1875) once more. Thanks again! |
Looks good now to me as its the same hack I have been running for a month now. Thanks for providing a proper fix for it. I will close our my old hacky PR. |
This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
-vvv
option).Issue
Due to pypa/pip#7444 installing a directory which is managed by poetry or has a pyproject.toml file present will cause the
--no-deps
argument to be ignored.This can go unnoticed as long as you are only working with pypi dependencies but when your package depends on a private pypi repository this causes installs to fail.
The text was updated successfully, but these errors were encountered: