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 · 10 comments
Open

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

henryiii opened this issue Feb 17, 2025 · 10 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.

@ichard26
Copy link

Puts on my pip maintainer hat.

Is there any movement on pip side to see if they can find a cleverer approach.

No? We recognize that our build isolation implementation is not great. We used this approach initially because PEP 517 and the build isolation logic was added when pip still supported Python 2 which didn't have venv. We were going to switch over to venv, but there are a lot of finicky details to figure out and that effort has been stalled for a few years(?) I believe.

@XuehaiPan
Copy link

XuehaiPan commented Feb 22, 2025

We were going to switch over to venv, but there are a lot of finicky details to figure out and that effort has been stalled for a few years(?) I believe.

Note that venv might not always ship with the Python package on some platforms. It needs to be installed separately (package python3-venv on Ubuntu).

$ docker run -it --rm --pull=always -h ubuntu ubuntu:latest /bin/bash
latest: Pulling from library/ubuntu
Digest: sha256:72297848456d5d37d1262630108ab308d3e9ec7ed1c3286a32fe09856619a782
Status: Image is up to date for ubuntu:latest

root@ubuntu:/# apt update -qq && apt install python3-dev python3-pip -yqq
18 packages can be upgraded. Run 'apt list --upgradable' to see them.
The following additional packages will be installed:
  adduser binutils binutils-aarch64-linux-gnu binutils-common build-essential bzip2 ca-certificates cpp cpp-13 cpp-13-aarch64-linux-gnu cpp-aarch64-linux-gnu dirmngr dpkg-dev fakeroot
  fontconfig-config fonts-dejavu-core fonts-dejavu-mono g++ g++-13 g++-13-aarch64-linux-gnu g++-aarch64-linux-gnu gcc gcc-13 gcc-13-aarch64-linux-gnu gcc-13-base gcc-aarch64-linux-gnu
  gnupg gnupg-l10n gnupg-utils gpg gpg-agent gpg-wks-client gpgconf gpgsm javascript-common keyboxd libalgorithm-diff-perl libalgorithm-diff-xs-perl libalgorithm-merge-perl libaom3
  libasan8 libatomic1 libbinutils libbrotli1 libbsd0 libc-bin libc-dev-bin libc-devtools libc6 libc6-dev libcc1-0 libcrypt-dev libctf-nobfd0 libctf0 libde265-0 libdeflate0 libdpkg-perl
  libexpat1 libexpat1-dev libfakeroot libfile-fcntllock-perl libfontconfig1 libfreetype6 libgcc-13-dev libgd3 libgdbm-compat4t64 libgdbm6t64 libgomp1 libgprofng0 libheif-plugin-aomdec
  libheif-plugin-aomenc libheif-plugin-libde265 libheif1 libhwasan0 libisl23 libitm1 libjansson4 libjbig0 libjpeg-turbo8 libjpeg8 libjs-jquery libjs-sphinxdoc libjs-underscore libksba8
  libldap-common libldap2 liblerc4 liblocale-gettext-perl liblsan0 libmpc3 libmpfr6 libperl5.38t64 libpng16-16t64 libpython3-dev libpython3-stdlib libpython3.12-dev
  libpython3.12-minimal libpython3.12-stdlib libpython3.12t64 libreadline8t64 libsasl2-2 libsasl2-modules libsasl2-modules-db libsframe1 libsharpyuv0 libsqlite3-0 libssl3t64
  libstdc++-13-dev libtiff6 libtsan2 libubsan1 libwebp7 libx11-6 libx11-data libxau6 libxcb1 libxdmcp6 libxpm4 linux-libc-dev lto-disabled-list make manpages manpages-dev media-types
  netbase openssl patch perl perl-base perl-modules-5.38 pinentry-curses python3 python3-minimal python3-pkg-resources python3-setuptools python3-wheel python3.12 python3.12-dev
  python3.12-minimal readline-common rpcsvc-proto tzdata xz-utils zlib1g-dev
Suggested packages:
  cron quota ecryptfs-utils binutils-doc gprofng-gui bzip2-doc cpp-doc gcc-13-locales cpp-13-doc dbus-user-session libpam-systemd pinentry-gnome3 tor debian-keyring gcc-13-doc
  gcc-multilib autoconf automake libtool flex bison gdb gcc-doc gdb-aarch64-linux-gnu parcimonie xloadimage gpg-wks-server scdaemon apache2 | lighttpd | httpd glibc-doc locales
  libnss-nis libnss-nisplus git bzr libgd-tools gdbm-l10n libheif-plugin-x265 libheif-plugin-ffmpegdec libheif-plugin-jpegdec libheif-plugin-jpegenc libheif-plugin-j2kdec
  libheif-plugin-j2kenc libheif-plugin-rav1e libheif-plugin-svtenc libsasl2-modules-gssapi-mit | libsasl2-modules-gssapi-heimdal libsasl2-modules-ldap libsasl2-modules-otp
  libsasl2-modules-sql libstdc++-13-doc make-doc man-browser ed diffutils-doc perl-doc libterm-readline-gnu-perl | libterm-readline-perl-perl libtap-harness-archive-perl pinentry-doc
  python3-doc python3-tk python3-venv python-setuptools-doc python3.12-venv python3.12-doc binfmt-support readline-doc
The following NEW packages will be installed:
  adduser binutils binutils-aarch64-linux-gnu binutils-common build-essential bzip2 ca-certificates cpp cpp-13 cpp-13-aarch64-linux-gnu cpp-aarch64-linux-gnu dirmngr dpkg-dev fakeroot
  fontconfig-config fonts-dejavu-core fonts-dejavu-mono g++ g++-13 g++-13-aarch64-linux-gnu g++-aarch64-linux-gnu gcc gcc-13 gcc-13-aarch64-linux-gnu gcc-13-base gcc-aarch64-linux-gnu
  gnupg gnupg-l10n gnupg-utils gpg gpg-agent gpg-wks-client gpgconf gpgsm javascript-common keyboxd libalgorithm-diff-perl libalgorithm-diff-xs-perl libalgorithm-merge-perl libaom3
  libasan8 libatomic1 libbinutils libbrotli1 libbsd0 libc-dev-bin libc-devtools libc6-dev libcc1-0 libcrypt-dev libctf-nobfd0 libctf0 libde265-0 libdeflate0 libdpkg-perl libexpat1
  libexpat1-dev libfakeroot libfile-fcntllock-perl libfontconfig1 libfreetype6 libgcc-13-dev libgd3 libgdbm-compat4t64 libgdbm6t64 libgomp1 libgprofng0 libheif-plugin-aomdec
  libheif-plugin-aomenc libheif-plugin-libde265 libheif1 libhwasan0 libisl23 libitm1 libjansson4 libjbig0 libjpeg-turbo8 libjpeg8 libjs-jquery libjs-sphinxdoc libjs-underscore libksba8
  libldap-common libldap2 liblerc4 liblocale-gettext-perl liblsan0 libmpc3 libmpfr6 libperl5.38t64 libpng16-16t64 libpython3-dev libpython3-stdlib libpython3.12-dev
  libpython3.12-minimal libpython3.12-stdlib libpython3.12t64 libreadline8t64 libsasl2-2 libsasl2-modules libsasl2-modules-db libsframe1 libsharpyuv0 libsqlite3-0 libstdc++-13-dev
  libtiff6 libtsan2 libubsan1 libwebp7 libx11-6 libx11-data libxau6 libxcb1 libxdmcp6 libxpm4 linux-libc-dev lto-disabled-list make manpages manpages-dev media-types netbase openssl
  patch perl perl-modules-5.38 pinentry-curses python3 python3-dev python3-minimal python3-pip python3-pkg-resources python3-setuptools python3-wheel python3.12 python3.12-dev
  python3.12-minimal readline-common rpcsvc-proto tzdata xz-utils zlib1g-dev
The following packages will be upgraded:
  libc-bin libc6 libssl3t64 perl-base
4 upgraded, 142 newly installed, 0 to remove and 14 not upgraded.

root@ubuntu:/# which -a python3
/usr/bin/python3
/bin/python3

root@ubuntu:/# python3 -m venv /venv
The virtual environment was not created successfully because ensurepip is not
available.  On Debian/Ubuntu systems, you need to install the python3-venv
package using the following command.

    apt install python3.12-venv

You may need to use sudo with that command.  After installing the python3-venv
package, recreate your virtual environment.

Failing command: /venv/bin/python3

@LecrisUT
Copy link
Collaborator

Note that venv might not always ship with the Python package on some platforms. It needs to be installed separately.

That would not be an issue. It will be packager's responsibility for adding the venv dependency to the pip package. If you upgrade pip from pip, or you get it from somewhere else, that could be problematic, but a simple try import and message would suffice.

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

4 participants