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

info: fix metadata build error propagation #9870

Merged
merged 2 commits into from
Nov 28, 2024

Conversation

abn
Copy link
Member

@abn abn commented Nov 24, 2024

This change fixes a regression that caused the swallowing of PEP517 build errors when isolated builds fail during metadata build.

Borrowing from another issue, you can use the following pyproject.toml to reproduce/test.

[tool.poetry]
name = "my-package"
version = "0.1.0"
description = ""
authors = ["My Name <my.name@example.com>"]
readme = "README.md"

[tool.poetry.dependencies]
python = "^3.10"
mmagic = {git = "/~https://github.com/open-mmlab/mmagic.git", rev = "0a560bb"}

[build-system]
requires = ["poetry-core"]
build-backend = "poetry.core.masonry.api"
Output w/ Poetry (1.8.4)

Updating dependencies
Resolving dependencies... (3.1s)

Unable to determine package info for path: /tmp/foo/.venv/src/mmagic

Command ['/tmp/tmp_xptxh3p/.venv/bin/python', '-I', '-W', 'ignore', '-c', "import build\nimport build.env\nimport pyproject_hooks\n\nsource = '/tmp/foo/.venv/src/mmagic'\ndest = '/tmp/tmp_xptxh3p/dist'\n\nwith build.env.DefaultIsolatedEnv() as env:\n    builder = build.ProjectBuilder.from_isolated_env(\n        env, source, runner=pyproject_hooks.quiet_subprocess_runner\n    )\n    env.install(builder.build_system_requires)\n    env.install(builder.get_requires_for_build('wheel'))\n    builder.metadata_path(dest)\n"] errored with the following return code 1

Error output:
Traceback (most recent call last):
  File "<string>", line 13, in <module>
    env.install(builder.get_requires_for_build('wheel'))
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^
  File "/tmp/tmp_xptxh3p/.venv/lib/python3.13/site-packages/build/__init__.py", line 239, in get_requires_for_build
    with self._handle_backend(hook_name):
         ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^
  File "/usr/lib64/python3.13/contextlib.py", line 162, in __exit__
    self.gen.throw(value)
    ~~~~~~~~~~~~~~^^^^^^^
  File "/tmp/tmp_xptxh3p/.venv/lib/python3.13/site-packages/build/__init__.py", line 360, in _handle_backend
    raise BuildBackendException(exception, f'Backend subprocess exited when trying to invoke {hook}') from None
build._exceptions.BuildBackendException: Backend subprocess exited when trying to invoke get_requires_for_build_wheel

Fallback egg_info generation failed.

Command ['/tmp/tmp_xptxh3p/.venv/bin/python', 'setup.py', 'egg_info'] errored with the following return code 1

Output:
Traceback (most recent call last):
  File "/tmp/foo/.venv/src/mmagic/setup.py", line 9, in <module>
    import torch
ModuleNotFoundError: No module named 'torch'

Output w/ Poetry (git@main)

Updating dependencies
Resolving dependencies... (0.4s)

Unable to determine package info for path: /tmp/foo/.venv/src/mmagic

Backend subprocess exited when trying to invoke get_requires_for_build_wheel

PEP517 build failed

Output w/ PR

Updating dependencies
Resolving dependencies... (6.7s)

Unable to determine package info for path: /root/.cache/pypoetry/virtualenvs/my-package-lWDpn5M1-py3.13/src/mmagic

PEP517 build of a dependency failed

Backend subprocess exited when trying to invoke get_requires_for_build_wheel

    | Command '['/tmp/tmp34u3w5e2/.venv/bin/python', '/usr/local/lib/python3.13/site-packages/pyproject_hooks/_in_process/_in_process.py', 'get_requires_for_build_wheel', '/tmp/tmp12kuqqsd']' returned non-zero exit status 1.
    | 
    | Traceback (most recent call last):
    |   File "/usr/local/lib/python3.13/site-packages/pyproject_hooks/_in_process/_in_process.py", line 389, in <module>
    |     main()
    |     ~~~~^^
    |   File "/usr/local/lib/python3.13/site-packages/pyproject_hooks/_in_process/_in_process.py", line 373, in main
    |     json_out["return_val"] = hook(**hook_input["kwargs"])
    |                              ~~~~^^^^^^^^^^^^^^^^^^^^^^^^
    |   File "/usr/local/lib/python3.13/site-packages/pyproject_hooks/_in_process/_in_process.py", line 143, in get_requires_for_build_wheel
    |     return hook(config_settings)
    |   File "/tmp/tmp34u3w5e2/.venv/lib/python3.13/site-packages/setuptools/build_meta.py", line 334, in get_requires_for_build_wheel
    |     return self._get_build_requires(config_settings, requirements=[])
    |            ~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |   File "/tmp/tmp34u3w5e2/.venv/lib/python3.13/site-packages/setuptools/build_meta.py", line 304, in _get_build_requires
    |     self.run_setup()
    |     ~~~~~~~~~~~~~~^^
    |   File "/tmp/tmp34u3w5e2/.venv/lib/python3.13/site-packages/setuptools/build_meta.py", line 522, in run_setup
    |     super().run_setup(setup_script=setup_script)
    |     ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |   File "/tmp/tmp34u3w5e2/.venv/lib/python3.13/site-packages/setuptools/build_meta.py", line 320, in run_setup
    |     exec(code, locals())
    |     ~~~~^^^^^^^^^^^^^^^^
    |   File "<string>", line 9, in <module>
    | ModuleNotFoundError: No module named 'torch'

Note: This error originates from the build backend, and is likely not a problem with poetry but one of the following issues with /root/.cache/pypoetry/virtualenvs/my-package-lWDpn5M1-py3.13/src/mmagic

  - not supporting PEP 517 builds
  - not specifying PEP 517 build requirements correctly
  - the build requirements are incompatible with your operating system or Python versions
  - the build requirements are missing system dependencies (eg: compilers, libraries, headers).

You can verify this by running pip wheel --no-cache-dir --use-pep517 "/root/.cache/pypoetry/virtualenvs/my-package-lWDpn5M1-py3.13/src/mmagic".

You can test this pull request using the following command. You can swap podman with docker if you use that.

podman run --rm -it --entrypoint bash docker.io/python:3.13 <<EOF
set -xe
python -m pip install --root-user-action ignore  --disable-pip-version-check -q git+/~https://github.com/python-poetry/poetry.git@refs/pull/9870/head
install -d foobar
pushd foobar
cat > pyproject.toml <<TOML
[tool.poetry]
name = "my-package"
version = "0.1.0"
description = ""
authors = ["My Name <my.name@example.com>"]
readme = "README.md"

[tool.poetry.dependencies]
python = "^3.10"
mmagic = {git = "/~https://github.com/open-mmlab/mmagic.git", rev = "0a560bb"}

[build-system]
requires = ["poetry-core"]
build-backend = "poetry.core.masonry.api"
TOML
poetry install
EOF

You can also preview the changes on asciinema.

@abn abn force-pushed the propagate-build-metadata-error branch from e62c95a to c305c6d Compare November 24, 2024 00:45
@abn abn requested a review from a team November 24, 2024 00:46
@abn abn marked this pull request as ready for review November 24, 2024 00:47
@abn abn added the area/error-handling Bad error messages/insufficient error handling label Nov 24, 2024
Copy link
Member

@radoering radoering left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. Have you thought about adding the indentation of the inner error and/or the listing of possible reasons also at

"<info>"
"Note: This error originates from the build backend,"
" and is likely not a problem with poetry"
f" but with {pkg.pretty_name} ({pkg.full_pretty_version})"
" not supporting PEP 517 builds. You can verify this by"
f" running '{pip_command} \"{requirement}\"'."
"</info>"
?

@abn
Copy link
Member Author

abn commented Nov 24, 2024

Yeah. But figured I'll do that in a separate PR just to keep things clean here. I also wanted to see if we can unify that somehow. Once this is accepted I can work on that, ie. No one has objections to the formatting.

@abn
Copy link
Member Author

abn commented Nov 25, 2024

@radoering @Secrus I have now also unified the changes for executor and info as well here.

@abn abn force-pushed the propagate-build-metadata-error branch from b9518af to 5c825e1 Compare November 25, 2024 17:25
@abn abn requested review from radoering and Secrus November 25, 2024 17:29
@abn abn force-pushed the propagate-build-metadata-error branch 2 times, most recently from 140130c to b27ea61 Compare November 26, 2024 09:10
@abn abn force-pushed the propagate-build-metadata-error branch from b27ea61 to ca4ceb1 Compare November 27, 2024 19:46
@abn abn requested a review from radoering November 28, 2024 16:51
@abn abn force-pushed the propagate-build-metadata-error branch from ca4ceb1 to 6b0d0f6 Compare November 28, 2024 18:49
@abn abn enabled auto-merge (rebase) November 28, 2024 18:51
abn added 2 commits November 28, 2024 19:52
This change fixes a regression that caused the swallowing of PEP517
build errors when isolated builds fail during metadata build.
@abn abn force-pushed the propagate-build-metadata-error branch from 6b0d0f6 to f308e32 Compare November 28, 2024 18:52
@abn abn merged commit 868b655 into python-poetry:main Nov 28, 2024
73 checks passed
@abn abn deleted the propagate-build-metadata-error branch November 28, 2024 19:24
Copy link

This pull request 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 Dec 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/error-handling Bad error messages/insufficient error handling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants