Install build dependencies in-process#13450
Conversation
|
I should probably be explicit with what I want from people right now. I simply want feedback on the general idea. Is this something we want to pursue given the inherent additional complexity? If so, I'll add tests, write proper commit messages, and break this up into smaller PRs so this is easier to review. For the rational, please see #9081 (TL;DR, it will be faster, fix a bunch of bugs with passing the right arguments down, allow for better error handling, and clear the way to remove some ugly hacks in the codebase) If not, I'll abandon this. |
|
To help put the extra code into context, I've prototyped what the removal of the legacy installer would look like. In the 10 minutes I spent on this, this is the result: $ git sdiff
src/pip/__pip-runner__.py | 50 -------------
src/pip/_internal/build_env.py | 82 +---------------------
src/pip/_internal/commands/install.py | 4 +-
src/pip/_internal/commands/wheel.py | 4 +-
src/pip/_internal/index/package_finder.py | 31 --------
.../_internal/operations/build/build_tracker.py | 75 ++------------------
6 files changed, 10 insertions(+), 236 deletions(-) |
|
+1 from me on the general idea. |
|
FYI, I was planning to get back to working on #13300 soonish, but looking over this PR I see no issue with working on both at the same time, there will only be a small overlap, just at the point of passing the flags to the build dependency installation command. |
|
Talking with @ichard26, out of band, my above comment is completely wrong, and I think we need to make a choice on this. The current behavior: This PRs Now, while I understand that the current behavior was probably not intentional I do think it's advantageous to be able to have build constraints and regular constraints as separate. And I am concerned with this as a breaking change. But if this is going to be a breaking change I have a proposal to clean everything up: Match uv's behavior, add a build constraint flag, and have regular constraints NEVER apply to the build environments, and have build constraints apply to ALL build environments:
From a user's perspective this will be much more clean and obvious as to what each flag is doing, the new I am willing to figure out how to wrangle the constraints vs build constraints and have it as a separate PR to land after the inprocess-build-deps lands, if @ichard26 is not up for that work. Whether this proposal is agreed to or not, I think it's important that a constraint design is agreed upon before merging this. |
This is also not exactly correct. The current behaviour is that neither I agree that this would be a good opportunity to clean up the constraint handling though. |
|
Alrighty. I've broken this up into a handful of smaller PRs. Reviews on the following would be appreciated :) |
|
Do those 3 PRs cover everything in this PR or are intended to be the building steps before actually adding the feature that users can enable? |
|
Building blocks. The actual feature will be added in a separate PR (or more likely, I'll force push to this PR) once all of the prerequisite building blocks have been landed. |
|
Moving this to 26.0 unless there are objections. |
|
No objections. There is no way I'm going to have time to work on this until the holidays. |
b55b0fd to
621d1d6
Compare
621d1d6 to
794bc78
Compare
|
Update: this PR is now ready for review. I've updated the PR description to reflect current reality and the updated error handling. As a reminder, this feature is gated behind |
notatallshaw
left a comment
There was a problem hiding this comment.
I've run this against a lot of real world scenarios and done some contrived tests and not found an issue with it.
With a merge (or rebase) and conflicts fixed I am happy to see this land.
|
Thank you @notatallshaw for the review! I am very happy to see this land. 🎉 |
This patch adds an alternative build environment dependency installer that installs in-process, using a stripped down re-implementation of the install command. It's disabled by default. To use the in-process installer, pass --use-feature inprocess-build-deps. This new mechanism is faster, supports --no-clean and --no-cache-dir reliably, and supports prompting for authentication. Building pip from source with setuptools cached, inprocess-build-deps shaves ~300 ms (1850ms -> 1530ms, before we switched to flit-core). The plan is to flip the default in a future release and eventually remove the legacy subprocess-based installer once the new installer has stabilized. In general, the in-process installer inherits more options. Notably, since the finder is shared, --implementation, --abi, --platform (and possibly --python-version) are inherited. It may be not be a good idea to inherit these, but we'll see if it becomes a problem. Since there is no longer a subprocess, environment variables cannot be used to directly configure the build environment. Environment variables will only affect the build environment if the associated options are inherited in code. Thus, build-constraint is mandated when this feature is enabled. Please note that the in-memory (finder) caches are shared across all build environments, thus if a new version of a build dependency is released in _the middle of_ a long install, it will no longer be picked up. The error handling story leaves much to be desired, but it can be improved in a follow up.
This patch adds an alternative build environment dependency installer that installs in-process, using a stripped down re-implementation of the install command. It's disabled by default. To use the in-process installer, pass --use-feature inprocess-build-deps. This new mechanism is faster, supports --no-clean and --no-cache-dir reliably, and supports prompting for authentication. Building pip from source with setuptools cached, inprocess-build-deps shaves ~300 ms (1850ms -> 1530ms, before we switched to flit-core). The plan is to flip the default in a future release and eventually remove the legacy subprocess-based installer once the new installer has stabilized. In general, the in-process installer inherits more options. Notably, since the finder is shared, --implementation, --abi, --platform (and possibly --python-version) are inherited. It may be not be a good idea to inherit these, but we'll see if it becomes a problem. Since there is no longer a subprocess, environment variables cannot be used to directly configure the build environment. Environment variables will only affect the build environment if the associated options are inherited in code. Thus, build-constraint is mandated when this feature is enabled. Please note that the in-memory (finder) caches are shared across all build environments, thus if a new version of a build dependency is released in _the middle of_ a long install, it will no longer be picked up. The error handling story leaves much to be desired, but it can be improved in a follow up.
Towards #7294, #9081, and its associated issues.
This PR adds an alternative build environment dependency installer that installs in-process, using a stripped down reimplementation of the install command. It's disabled by default. To use the in-process installer, pass
--use-feature inprocess-build-deps.Building pip from source with setuptools cached,
inprocess-build-depsshaves ~300 ms (1850ms -> 1530ms).I'll note that the plan would be to:
Known behavioural differences
--implementation,--abi,--platform(--python-versionas well?) are inherited through reusing the finder. It may be not be a good idea to inherit these though...?--require-hashesand--constraintsare not inherited as planned as they need a proper transition plan. They will be handled as follow-up items.PIP_CONSTRAINTS. This feature will automatically enable--use-feature build-constraintas well.Sample error messages
Important
I am unhappy with the output and error handling. It is messy and could do with improvement. However, it is in a workable and releasable state. Unless you have suggestions for consolidating and rationalizing things here, I will leave this as a follow up item.
Diagnostic error (normal + verbose)
Note: In verbose mode, coloured logs are printed while installing build dependencies. Otherwise, they are printed without colour.
Other pip error (normal VS verbose)
Errors that do not inherit from
PipError