Reconcile computation of isolated build environment paths

[dnicolodi]

Use the same code to determine isolated environment paths at
dependency install time and at environment setup time. We do not care
about the exact paths but the paths needs to be consistent at package
installation time and environment setup.

This should fix all issues observed on platforms that customize the
installation schemes, such as Debian and Homebrew, where dependency
installation and isolated build environment setup resolved to
different paths.

See #11598 and #11623.

[dnicolodi]

Fixes the failure reported in #11623 (comment)

[uranusjr]

You removed way too much logic. This will break pip on Xcode-installed Python, and also suppress the path mismatch logic that we put in place for distutils-sysconfig migration.

[dnicolodi]

I thought about it and I didn't find a situation in which the Xcode warning could be issued: the comment says it can trigger only on Python 3.10, but for Python 3.10 and later we use sysconfig and in this case the function exits early without evaluating that logic.

Similarly for the distutils vs sysconfig paths: we don't care what the exact paths are, as long as pip install --prefix and the build environment setup logic agree. Unless there are instances in which pip install --prefix is known to be broken, the current code should work everywhere.

[dnicolodi]

Also, get_scheme() has extensive checks for distutils vs sysconfig paths, much more detailed than the ones that I've removed. I don't see a good reason to have two set of slightly different checks.

[dnicolodi]

Force pushed to restart the Windows tests which failed on setup.

[stefanor]

Tested on Debian, and it seems to do the right thing.

[dnicolodi]

Is there any way to check that this does not break on Xcode installed Python and that the warnings that are supposed to be emitted are still emitted? The test suite does not seem to test this aspects. I do have a macOS system but system Python here is still 2.7.

[dnicolodi]

I spent some time setting up an environment in which I could test the generation of the warnings. All the warnings that where generated before are still generated withing get_scheme() except for the Apple's Python one because get_scheme() has code in place to detect the situation and do the right thing. I think that doing the right thing is much better than emitting a warning that is hardly actionable by the user.

@uranusjr The warnings are still generated here

pip/src/pip/_internal/locations/__init__.py

Lines 260 to 405 in 729032d

	warning_contexts = []
	for k in SCHEME_KEYS:
	old_v = pathlib.Path(getattr(old, k))
	new_v = pathlib.Path(getattr(new, k))
	if old_v == new_v:
	continue
	# distutils incorrectly put PyPy packages under ``site-packages/python``
	# in the ``posix_home`` scheme, but PyPy devs said they expect the
	# directory name to be ``pypy`` instead. So we treat this as a bug fix
	# and not warn about it. See bpo-43307 and python/cpython#24628.
	skip_pypy_special_case = (
	sys.implementation.name == "pypy"
	and home is not None
	and k in ("platlib", "purelib")
	and old_v.parent == new_v.parent
	and old_v.name.startswith("python")
	and new_v.name.startswith("pypy")
	)
	if skip_pypy_special_case:
	continue
	# sysconfig's ``osx_framework_user`` does not include ``pythonX.Y`` in
	# the ``include`` value, but distutils's ``headers`` does. We'll let
	# CPython decide whether this is a bug or feature. See bpo-43948.
	skip_osx_framework_user_special_case = (
	user
	and is_osx_framework()
	and k == "headers"
	and old_v.parent.parent == new_v.parent
	and old_v.parent.name.startswith("python")
	)
	if skip_osx_framework_user_special_case:
	continue
	# On Red Hat and derived Linux distributions, distutils is patched to
	# use "lib64" instead of "lib" for platlib.
	if k == "platlib" and _looks_like_red_hat_lib():
	continue
	# On Python 3.9+, sysconfig's posix_user scheme sets platlib against
	# sys.platlibdir, but distutils's unix_user incorrectly coninutes
	# using the same $usersite for both platlib and purelib. This creates a
	# mismatch when sys.platlibdir is not "lib".
	skip_bpo_44860 = (
	user
	and k == "platlib"
	and not WINDOWS
	and sys.version_info >= (3, 9)
	and _PLATLIBDIR != "lib"
	and _looks_like_bpo_44860()
	)
	if skip_bpo_44860:
	continue
	# Slackware incorrectly patches posix_user to use lib64 instead of lib,
	# but not usersite to match the location.
	skip_slackware_user_scheme = (
	user
	and k in ("platlib", "purelib")
	and not WINDOWS
	and _looks_like_slackware_scheme()
	)
	if skip_slackware_user_scheme:
	continue
	# Both Debian and Red Hat patch Python to place the system site under
	# /usr/local instead of /usr. Debian also places lib in dist-packages
	# instead of site-packages, but the /usr/local check should cover it.
	skip_linux_system_special_case = (
	not (user or home or prefix or running_under_virtualenv())
	and old_v.parts[1:3] == ("usr", "local")
	and len(new_v.parts) > 1
	and new_v.parts[1] == "usr"
	and (len(new_v.parts) < 3 or new_v.parts[2] != "local")
	and (_looks_like_red_hat_scheme() or _looks_like_debian_scheme())
	)
	if skip_linux_system_special_case:
	continue
	# On Python 3.7 and earlier, sysconfig does not include sys.abiflags in
	# the "pythonX.Y" part of the path, but distutils does.
	skip_sysconfig_abiflag_bug = (
	sys.version_info < (3, 8)
	and not WINDOWS
	and k in ("headers", "platlib", "purelib")
	and tuple(_fix_abiflags(old_v.parts)) == new_v.parts
	)
	if skip_sysconfig_abiflag_bug:
	continue
	# MSYS2 MINGW's sysconfig patch does not include the "site-packages"
	# part of the path. This is incorrect and will be fixed in MSYS.
	skip_msys2_mingw_bug = (
	WINDOWS and k in ("platlib", "purelib") and _looks_like_msys2_mingw_scheme()
	)
	if skip_msys2_mingw_bug:
	continue
	# CPython's POSIX install script invokes pip (via ensurepip) against the
	# interpreter located in the source tree, not the install site. This
	# triggers special logic in sysconfig that's not present in distutils.
	# https://github.com/python/cpython/blob/8c21941ddaf/Lib/sysconfig.py#L178-L194
	skip_cpython_build = (
	sysconfig.is_python_build(check_home=True)
	and not WINDOWS
	and k in ("headers", "include", "platinclude")
	)
	if skip_cpython_build:
	continue
	warning_contexts.append((old_v, new_v, f"scheme.{k}"))
	if not warning_contexts:
	return old
	# Check if this path mismatch is caused by distutils config files. Those
	# files will no longer work once we switch to sysconfig, so this raises a
	# deprecation message for them.
	default_old = _distutils.distutils_scheme(
	dist_name,
	user,
	home,
	root,
	isolated,
	prefix,
	ignore_config_files=True,
	)
	if any(default_old[k] != getattr(old, k) for k in SCHEME_KEYS):
	deprecated(
	reason=(
	"Configuring installation scheme with distutils config files "
	"is deprecated and will no longer work in the near future. If you "
	"are using a Homebrew or Linuxbrew Python, please see discussion "
	"at https://github.com/Homebrew/homebrew-core/issues/76621"
	),
	replacement=None,
	gone_in=None,
	)
	return old
	# Post warnings about this mismatch so user can report them back.
	for old_v, new_v, key in warning_contexts:
	_warn_mismatched(old_v, new_v, key=key)
	_log_context(user=user, home=home, root=root, prefix=prefix)

in get_scheme(). Handling of Apple's Python (and to an extent also Homebrew's) is here

pip/src/pip/_internal/locations/_sysconfig.py

Lines 55 to 83 in 729032d

	def _infer_prefix() -> str:
	"""Try to find a prefix scheme for the current platform.
	This tries:
	* A special ``osx_framework_library`` for Python distributed by Apple's
	Command Line Tools, when not running in a virtual environment.
	* Implementation + OS, used by PyPy on Windows (``pypy_nt``).
	* Implementation without OS, used by PyPy on POSIX (``pypy``).
	* OS + "prefix", used by CPython on POSIX (``posix_prefix``).
	* Just the OS name, used by CPython on Windows (``nt``).
	If none of the above works, fall back to ``posix_prefix``.
	"""
	if _PREFERRED_SCHEME_API:
	return _PREFERRED_SCHEME_API("prefix")
	if _should_use_osx_framework_prefix():
	return "osx_framework_library"
	implementation_suffixed = f"{sys.implementation.name}_{os.name}"
	if implementation_suffixed in _AVAILABLE_SCHEMES:
	return implementation_suffixed
	if sys.implementation.name in _AVAILABLE_SCHEMES:
	return sys.implementation.name
	suffixed = f"{os.name}_prefix"
	if suffixed in _AVAILABLE_SCHEMES:
	return suffixed
	if os.name in _AVAILABLE_SCHEMES: # On Windows, prefx is just called "nt".
	return os.name
	return "posix_prefix"

I think that this implementation is better than the existing one. So much that I cannot figure out why the paths lookup was implemented twice in slightly different ways.

[pradyunsg]

pradyunsg deleted a comment from dnicolodi Jan 22, 2023

I have no idea how this happened. :/

[dnicolodi]

Just for the records, the deleted comment pointed out that the condition for emitting a warning is obviously wrong. The fix is in #11741. However, this PR removes the affected code and is a much better solution.

[dnicolodi]

@pradyunsg Can you please review this?

[dnicolodi]

Can we assign this to the 23.0 milestone, so it is not forgotten? Thanks!

[pradyunsg]

LGTM, with one non-blocking concern.

[uranusjr]

Is there any way to check that this does not break on Xcode installed Python and that the warnings that are supposed to be emitted are still emitted?

Unfortunately there is no way to check in CI, all you can do is to find an actual machine (and it can’t be too new either). But thi would not actually “break” Xcode Python, in the sense that it is not actually fixable—all the old code does is emit a warning to tell users things are totally broken beyond repair. So what you should do is to add that Apple check back, and emit a warning when the function is called for build isolation purposes. This would require a flag, or (better IMO) a wrapper function instead of calling get_scheme directly in the build isolation code path.

[dnicolodi]

It drops the _looks_like_apple_library thing…

get_scheme() has his own handling for the same case, see the _infer_prefix() function I linked to above.

[pradyunsg]

I'm gonna say that @uranusjr is the expert in this area of pip, so I'll defer to him on whether this is a blocker for 23.0.

[pradyunsg]

OK, had a chat with him over a different channel, and he reckons this isn't a blocker and I'm hapyp to trust his judgement on this.

[dnicolodi]

It is very unfortunate that you don't want to resolve this issue for the next release. It makes pip unsuitable to build wheels on Debian and thus incapable of installing any package that does not distribute wheels. It seems to me that the remaining objections to have this patch merged are based on a some misunderstanding ans cursory review. I'm willing to provide more explanation. I've tried to explain why the removed code is redundant, but apparently I failed.

get_scheme() is central in how pip works. Therefore, if any warning related to broken Python setups should be emitted, it should be emitted in all circumstances, not only when setting up an isolated build environment. However, get_scheme() handles the case where a warning is emitted just fine, thus the reason to emit a warning does not exist anymore.

If this is not the case, I would like to have a precise description of a setup where the current code fails to do what is expected. I've tested it on Debian, on Homebrew, and with the Python distributed by Apple. In all cases it works just fine.

@uranusjr can you please formulate you objections more precisely, so that they are actionable?

[dnicolodi]

Can you then consider #11741 for the release? @uranusjr apparently really wants a warning to be emitted, but the code that is supposedly emitting this warning has the condition written backward.

[dnicolodi]

Unfortunately there is no way to check in CI, all you can do is to find an actual machine

I've done it. And the warning triggers also for Homebrew's Python, when the condition for emitting the warning is written properly, see below.

all the old code does is emit a warning to tell users things are totally broken beyond repair. So what you should do is to add that Apple check back, and emit a warning when the function is called for build isolation purposes.

The code does not emit any warning, see #11741. Also, isn't it better to handle the situation (as is done in existing code when pip install --prefix is run) than to emit a warning and hope for the best?

[pfmoore]

So having pip broken on Debian is not a blocker for a new release?

Please moderate your statements here. Saying that "pip is broken on Debian" is over-stating the issue. It's perfectly possible for this particular breakage to be sufficiently limited in its effect that it doesn't warrant holding up the release, and I assume that's what we're saying.

Over-exaggerated statements tend to simply polarise discussions. It's extremely tempting to respond to your statement by saying instead that Debian shouldn't customise things in a way that "breaks pip", and that's just as (in)valid, and just as exaggerated, as your claim. Doing this sort of thing won't help users, it will simply push people away from helping address this issue.

I appreciate that you've worked hard on this, and want it to go in. And your work is valuable. But we have to balance this one change against other priorities and issues, and make sometimes hard judgements on when to draw the line. Please don't make that decision making process harder than it already is.

[dnicolodi]

@pfmoore Thank you. I rewrote that comment before you calling me out on this. I tried to explain better the implications of the bug this PR tries to fix and why I don't understand the objections that are being made. I hope this to be a more constructive way to approach the situation. Is there any reason why the release cannot be delayed a bit to take the proper time to evaluate this PR?

[pfmoore]

@dnicolodi Thanks. Sorry our posts passed each other in transit.

For my own understanding, can you confirm precisely which variants of Debian are affected? I just successfully built a dummy project on WSL (Ubuntu 22.04.1 LTS), so it clearly isn't every Debian variant.

Although, to be honest, I don't really care that much about the details. I trust the other pip maintainers on this. I'm really just interested in avoiding having misleadingly broad statements appear "in print", where we know from experience people will end up quoting them out of context at us.

As regards delaying the release, I know from my experience as RM, cutting a release is something you plan in, and "just delaying it" is non-trivial, especially with family and work commitments to work around. And we have a policy of "release what is currently on main" with pre-published release dates, to allow us to do that while still giving contributors clear expectations. In this case, there still seems to be a fair amount of misunderstanding, so it's not at all clear how much time it will take for this to be ready (especially given that we're all volunteers). That makes "delaying the release" a fairly imprecise prospect - and we probably have people waiting on other features of the release for whom delays are equally frustrating.

[stefanor]

For my own understanding, can you confirm precisely which variants of Debian are affected? I just successfully built a dummy project on WSL (Ubuntu 22.04.1 LTS), so it clearly isn't every Debian variant.

Not my PR, but I came to this thread, because I was looking at https://bugs.debian.org/1019293 and this looks like the correct upstream solution to the problem.
That bug is about installing things with a system pip (not in a virtualenv), that needs to build dependencies in isolated environments.

I've only been looking at Debian unstable/testing, here. I haven't checked what stable releases were affected by that bug.

[dnicolodi]

For my own understanding, can you confirm precisely which variants of Debian are affected? I just successfully built a dummy project on WSL (Ubuntu 22.04.1 LTS), so it clearly isn't every Debian variant.

I've done some testing and indeed the impact is less severe than what I thought. Debian stable is not affects. On Debian unstable, the problem is encountered when building wheel (or installing packages from source distribution) outside a virtual environment and only in projects that run a binary provided by one of the build dependencies. This is done, for example, by Python packages that use meson-python and Meson as build system (SciPy is a good example, or siphash24, if you want something small to test with).

I still very much like @uranusjr to indicate which platforms or systems may be adversely affected by the proposed change.

As I already wrote, I'm unable to find any system where the warning gated by _looks_like_apple_library() is emitted. This is because #11741. Once the condition gating the warning is fixed, the warning is emitted also on Homebrew's Python. However, Homebrew distributes also Python 3.11 and nothing breaks there. Thus, even in the status quo, the warning warns about future breakage that does not happen.

[pradyunsg]

Thanks for the rebase @dnicolodi, it's appreciated!

[dnicolodi]

@uranusjr ?

[pradyunsg]

I'll land this, given the discussion so far. Let's re-add the Apple-related warning in a separate follow up.