Skip to content

Short-circuit get_requires hooks when there are no extra build dependencies #4973

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ichard26
Copy link
Member

@ichard26 ichard26 commented May 2, 2025

Summary of changes

The PEP 517 get_requires_for_build_(wheel|sdist) hooks function by running the egg_info command with a patched Distribution that raises a special exception when build dependencies (from setup_requires) are about to be installed.

This works great at minimizing the overhead of the requires hooks... except when there are no dynamic build dependencies. In such a situation, the injected install_build_eggs() shim that raises SetupRequirementsError is never invoked, thus the entire egg_info command will execute. This often results in substantial overhead for modern projects that use [build-system].requires and not setup_requires.

With this simple script at the root of the pip project, this change brings the execution time from ~300ms to ~100ms.

import setuptools.build_meta
setuptools.build_meta.get_requires_for_build_wheel()

This helps a fair bit with reducing the overhead with PEP 517 processing.

…encies

The PEP 517 get_requires_for_build_(wheel|sdist) hooks function by
running the egg_info command with a patched Distribution that raises a
special exception when build dependencies (from setup_requires) are
about to be installed.

This works great at minimizing the overhead of the requires hooks...
except when there are no dynamic build dependencies. In such a
situation, the injected install_build_eggs() shim that raises
SetupRequirementsError is never invoked, thus the entire egg_info
command will execute. This often results in substantial overhead
for modern projects that use [build-system].requires and not
setup_requires.
@ichard26
Copy link
Member Author

ichard26 commented May 2, 2025

I have no idea why this is causing the prepare metadata hooks to blow up. If anyone else has any ideas, I'm all ears. I'm utterly stumped.

@ichard26
Copy link
Member Author

ichard26 commented May 2, 2025

Oh, it looks like the dist-info command depends on the egg-info command having ran before it...? That's going to kill this idea if true.

@jaraco
Copy link
Member

jaraco commented May 3, 2025

Oh, it looks like the dist-info command depends on the egg-info command having ran before it...? That's going to kill this idea if true.

I do believe that's the case. Soon, however, the biggest dependencies on egg_info (easy_install) should be removed, which gets us much closer to being able to consolidate the metadata handling behavior in dist_info instead of egg_info, so maybe there's hope.

@abravalheri
Copy link
Contributor

abravalheri commented May 12, 2025

Oh, it looks like the dist-info command depends on the egg-info command having ran before it...?

It is a bit more nuanced, I believe, dist-info will try to to run egg-info if it has not run yet. But there might be some caching happening.


I have no idea why this is causing the prepare metadata hooks to blow up. If anyone else has any ideas, I'm all ears. I'm utterly stumped.

I think that this part of the source code is very fragile... build_meta is patching something that has already being patched by setuptools/__init__.py. It is way too much patching and things are not really obvious.

Ultimately this happens because it is not very straight forward to align PEP 517-style build hooks with existing setup.py scripts. The expectations of who detains the control flow do not line up. So the existing build_meta implementation hacks a goto-like feature to circumvent these limitations.

In https://github.com/pypa/setuptools/compare/debt/refactoring-build_meta-patches?expand=1, I am studying if we can reduce the patching... Let's see if it find the same errors happening...

@ichard26
Copy link
Member Author

That branch looks quite promising! Let me know if you turn it into a PR and I'll be happy to verify whether it improves get_requires_for_build_(wheel|sdist) performance like this PR.

@abravalheri
Copy link
Contributor

That branch looks quite promising! Let me know if you turn it into a PR and I'll be happy to verify whether it improves get_requires_for_build_(wheel|sdist) performance like this PR.

I may do this next week when I have a bit more time. But what I am worried is that the interfacing of the backend and setup.py is quite fragile and subject to the Hyrum's Law (people do all sorts of creative things in setup.py and the sky is the limit). The existing approach kind of works. So I am afraid of introducing untested regressions with a refactor.

I also don't know if the approach in https://github.com/pypa/setuptools/compare/debt/refactoring-build_meta-patches?expand=1 is something that would be suitable for the long term vision in setuptools...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants