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

Should we workaround pip's build isolation bugs? #993

Open
henryiii opened this issue Feb 17, 2025 · 7 comments
Open

Should we workaround pip's build isolation bugs? #993

henryiii opened this issue Feb 17, 2025 · 7 comments

Comments

@henryiii
Copy link
Collaborator

henryiii commented Feb 17, 2025

Pip's build isolation doesn't actually set up normal virtual environments, but instead hacks together one with environment variables and other hacks. This causes a bunch of issues, such as site-packages not being reported correctly (which we already work around, see comments in #880), and any tools with Python wrappers being broken, like cmake, ninja, uv, etc. uv has a built-in workaround for this. Should we add a workaround when calling cmake and ninja? We've already seen this with #973, but we could fix it so that it works, I think. The workaround is at https://github.com/astral-sh/ruff/pull/13591/files, we could similarly strip PYTHONPATH when calling cmake/ninja.

I'm probably first going to see if we can workaround it in each package. See scikit-build/cmake-python-distributions#586.

@LecrisUT
Copy link
Collaborator

Oh boy that pip-build-env- approach looks fragile. Is there any movement on pip side to see if they can find a cleverer approach. Wouldn't copying the _vendor files that they need from the original environment to the isolated environment work?

@henryiii
Copy link
Collaborator Author

pypa/pip#13222:

IMO "use a proper venv" is the right solution here1. Especially if it turns out that it's what uv does. But switching to venv is not trivial (as the links @ichard26 provided show) so we can't give any realistic timescale for when it will get done. A 3rd party PR might help, but I wouldn't necessarily expect an external contributor to be willing to navigate the complexities of our compatibility policies, so maintainer time could still be the bottleneck.

  1. It might not actually solve all the problems we're seeing reports of, although if uv is using this approach, that would give us good evidence that it does. But at a minimum, it gives us the option of saying "blame venv" rather than having to patch up our own approach 🙂

@XuehaiPan
Copy link

XuehaiPan commented Feb 18, 2025

The workaround is at https://github.com/astral-sh/ruff/pull/13591/files

Currently, cmake works fine with pip's build environment pseudo-isolation if users put it in build-system.requires. The patch above solves ruff discovery for ruff in build-system.requires as the repro code shows: gaborbernat/ruff-find-bin-during-pip-build@fd532e8.

In issue scikit-build/cmake-python-distributions#586,

The cmake PyPI package is not present in build-system.requires because users complain it shadows the system CMake during build_ext. If cmake is not a setup_requires, it will not be installed in pip's build env overlay prefix.

@LecrisUT
Copy link
Collaborator

LecrisUT commented Feb 18, 2025

  • If cmake is not a setup_requires but installed in a parent venv

Wait, this is problematic. It means that the build isolation is escaped and it added a PATH from the original environment. The issue is, does it come from venv environment or does it come from other PATH manipulations like

$ PATH=$PATH:/path/to/cmake pip install

We want the later case to be respected while the first one stripped. Could you check this scenario in your patch?

@XuehaiPan
Copy link

XuehaiPan commented Feb 18, 2025

It means that the build isolation is escaped and it added a PATH from the original environment.

The pip build isolation manipulates PYTHONPATH but inherits PATH.

python3 -m venv parent-venv
source parent-venv/bin/activate  # prepends parent-venv/bin to PATH
pip3 install cmake               # creates a console script parent-venv/bin/cmake
pip3 install .                   # the local package needs cmake

The cmake executable to be found in PATH is parent-venv/bin/cmake. However, PYTHONPATH is modified by pip pseudo-isolation, the parent-venv/bin/cmake console script breaks because it failed to find the cmake Python module. The module is installed in parent-venv, not the build env overlay prefix (because cmake is not present in build-system.requires).

@LecrisUT
Copy link
Collaborator

That should be intended, although I can see why we should also allow it to be broken with some overloads. I.e.

$ python3 -m venv parent-venv
$ source parent-venv/bin/activate  # prepends parent-venv/bin to PATH
$ pip3 install cmake               # creates a console script parent-venv/bin/cmake
$ pip3 install .                   # the local package needs cmake
(should fail)
$ PATH=/path/to/a/weird/place/bin/cmake pip3 install .
(should pass)
$ python3 -m venv parent-venv
$ source parent-venv/bin/activate  # prepends parent-venv/bin to PATH
$ pip3 install cmake               # creates a console script parent-venv/bin/cmake
$ PATH=$(pwd)/parent-venv/bin/cmake pip3 install .
(should pass)

However pip is sanitizing its path for the build isolation, I don't think there is a good way to make the third situation work consistently.

@XuehaiPan
Copy link

The first and third ones are identical because parent-venv/bin is present in PATH. Both cases will fail.

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

No branches or pull requests

3 participants