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

Installing directory poetry package with dependencies in secondary source fails #1689

Closed
3 tasks done
wakemaster39 opened this issue Dec 7, 2019 · 14 comments · Fixed by #1796 or #1875
Closed
3 tasks done

Installing directory poetry package with dependencies in secondary source fails #1689

wakemaster39 opened this issue Dec 7, 2019 · 14 comments · Fixed by #1796 or #1875
Assignees
Labels
kind/bug Something isn't working as expected

Comments

@wakemaster39
Copy link

  • I am on the latest Poetry version.
  • I have searched the issues of this repo and believe that this is not a duplicate.
  • If an exception occurs when executing a command, I executed it again in debug mode (-vvv option).
  • MacOS 10.14:
  • 1.0.0b8:

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.

@wakemaster39 wakemaster39 added the kind/bug Something isn't working as expected label Dec 7, 2019
wakemaster39 pushed a commit to wakemaster39/poetry that referenced this issue Dec 7, 2019
@wakemaster39
Copy link
Author

wakemaster39 commented Dec 7, 2019

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

https://github.com/sdispater/poetry/blob/e943a4e127b0a658e76386006c60b79954a8fa57/poetry/masonry/api.py#L21-L29

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 get_requires_for_build_wheel(config_settings=None) should be modified to return an empty list. That is all that is required to build the wheel, poetry and nothing else.

@finswimmer
Copy link
Member

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.

@finswimmer
Copy link
Member

I put some code here for testing.

my_package has dependency1 as path dependency and dependency1 has dependency2 as path dependency.

Trying to install my_packageresults in:

$ poetry install
Creating virtualenv my-package in /home/finswimmer/tmp/poetry/1689/my_package/.venv
Updating dependencies
Resolving dependencies... (0.1s)

Writing lock file


Package operations: 2 installs, 0 updates, 0 removals

  - Installing dependency2 (0.1.0 ../dependency1/../dependency2)
  - Installing dependency1 (0.1.0 ../dependency1)

[EnvCommandError]
Command ['/home/finswimmer/tmp/poetry/1689/my_package/.venv/bin/pip', 'install', '--no-deps', '-U', '-e', '/home/finswimmer/tmp/poetry/1689/my_package/../dependency1'] errored with the following return code 1, and output: 
Obtaining file:///home/finswimmer/tmp/poetry/1689/dependency1
  Installing build dependencies: started
  Installing build dependencies: finished with status 'done'
  Getting requirements to build wheel: started
  Getting requirements to build wheel: finished with status 'done'
  Installing backend dependencies: started
  Installing backend dependencies: finished with status 'error'
  ERROR: Command errored out with exit status 1:
   command: /home/finswimmer/tmp/poetry/1689/my_package/.venv/bin/python /home/finswimmer/tmp/poetry/1689/my_package/.venv/lib/python3.8/site-packages/pip install --ignore-installed --no-user --prefix /tmp/pip-build-env-d7gnkeai/normal --no-warn-script-location --no-binary :none: --only-binary :none: -i https://pypi.org/simple -- dependency2
       cwd: None
  Complete output (5 lines):
  Collecting dependency2
    ERROR: Could not find a version that satisfies the requirement dependency2 (from versions: none)
  ERROR: No matching distribution found for dependency2
  WARNING: You are using pip version 19.2.3, however version 19.3.1 is available.
  You should consider upgrading via the 'pip install --upgrade pip' command.
  ----------------------------------------
ERROR: Command errored out with exit status 1: /home/finswimmer/tmp/poetry/1689/my_package/.venv/bin/python /home/finswimmer/tmp/poetry/1689/my_package/.venv/lib/python3.8/site-packages/pip install --ignore-installed --no-user --prefix /tmp/pip-build-env-d7gnkeai/normal --no-warn-script-location --no-binary :none: --only-binary :none: -i https://pypi.org/simple -- dependency2 Check the logs for full command output.
WARNING: You are using pip version 19.2.3, however version 19.3.1 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.

poetry version: 1.0.0b8
python version: 3.8
pip (system): 19.3.1
pip (in created venv): 19.2.3

@wakemaster39
Copy link
Author

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 get_requires_for_build_wheel as well.

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.

@finswimmer
Copy link
Member

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 --no-deps parameter is given, pip shouldn't call get_requires_for_build_wheel or it should ignore the returned values. Maybe @chrahunt could help us understanding this?

fin swimmer

@wakemaster39
Copy link
Author

I could also be terribly wrong here but from the PEP:

get_requires_for_build_wheel
This hook MUST return an additional list of strings containing PEP 508 dependency specifications, above and beyond those specified in the pyproject.toml file, to be installed when calling the build_wheel or prepare_metadata_for_build_wheel hooks.

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.

        # 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.

get_requires_for_build_wheel are build dependencies which are not skipped from being installed with the --no-deps only the runtime dependencies are skipped as I learnt in the linked issue in pip.

@finswimmer
Copy link
Member

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:

  • If pip < 19 is installed, we don't have the problem. Because the pyproject.toml and therefor the backend declaration, is ignored and the setup.py is used.
  • Starting with pip version 19 the pyproject.toml is recognized by pip.
    • So a new virtual environment is created for building the package.
    • In this venv poetry is installed, because it is listed in the requires field. This poetry version is the one, pip can find on pypi. Which is in the moment 0.12.17 and not the one installed locally!
    • "Someone" is ignoring the --no-deps parameter. I/We don't know by now if this should be part of the build-backend or pip. Or this parameter isn't intended to really ignore dependencies. The help page says "Don't install package dependencies." So checking, which fails in the build env, would be fine 🤔

Solution I have thought about and reject:

ERROR: Disabling PEP 517 processing is invalid: project specifies a build backend of poetry.masonry.api in pyproject.toml
  • Because pip uses the poetry version, it can find on pypi, when creating the build env, we are not able to test/start the build against our local poetry version. This might change with pip version 20, when backend-path is supported (Add a build system (intreehooks) to pyproject.toml #1684 (comment)). But I don't know, if then all dependencies for poetry are automatically installed into the build env or we have to define them first in the build-system section.

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 setup.py. That's why I think your PR should be implemented until we found a better solution.

@finswimmer
Copy link
Member

Ok, we are coming a step closer.

get_requires_for_build_wheel "MUST return an additional list of strings containing PEP 508 dependency specifications". At the moment, poetry returns for directory dependencies just the package name, which is wrong.

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)?

@kalyangvs
Copy link

Does this solve the following install hierarchy,

poetry new aa
poetry new ab
poetry new ac
cd ac/ && poetry add ../aa && cd ../
cd ab && poetry add ../ac

gives the following error even with the merged part(locally)
Complete output (2 lines): ERROR: Could not find a version that satisfies the requirement aa (from versions: none) ERROR: No matching distribution found for aa

Tried this hack too.
@finswimmer Any suggestions to try out?

@finswimmer
Copy link
Member

Hello @wakemaster39 ,

it took some time for me, but now I fully agree with you that get_requires_for_build_wheel should not return package dependencies but build dependencies. A fix (#1874) is on the way and I could test it successfully.

Thanks a lot! 👍

@finswimmer finswimmer self-assigned this Jan 12, 2020
@wakemaster39
Copy link
Author

@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 get_requires_for_build_wheel is anything additional that is required to build the wheel. So lets say poetry required the library random_wheel_builder to build wheels but it wasn't installed when you install poetry via pip then it would need to be returned by this function.

@finswimmer
Copy link
Member

finswimmer commented Jan 13, 2020

Hey @wakemaster39 ,

you are right. PEP517 writes "additional list of strings containing PEP 508
dependency specifications, above and beyond those specified in the pyproject.toml file". I've read it a hundred times now, be it seems to be not enough. The important word here, seems to be "additional". My PR works, because - as you said - pip already installed the dependencies listed in the pyproject.toml.

I've changed the PR (now #1875) once more.

Thanks again!

@wakemaster39
Copy link
Author

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.

Copy link

github-actions bot commented Mar 3, 2024

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.