From 8eeee223041a973642bb646d37ccfe0e6ff0a8f5 Mon Sep 17 00:00:00 2001
From: Pradyun Gedam <pradyunsg@users.noreply.github.com>
Date: Fri, 10 Dec 2021 15:54:09 +0000
Subject: [PATCH 1/3] Abort immediately on metadata generation failure instead
of backtracking
This behaviour is more forgiving when a source distribution cannot be
installed (eg: due to missing build dependencies or platform
incompatibility) and favours early eager failures instead of trying to
ensure that a package is installed regardless of the amount of effort it
takes.
---
news/10655.bugfix.rst | 4 ++++
.../resolution/resolvelib/factory.py | 5 ++---
tests/functional/test_new_resolver.py | 19 ++++++++++++++++++
tests/lib/__init__.py | 20 ++++++++++++++++---
4 files changed, 42 insertions(+), 6 deletions(-)
create mode 100644 news/10655.bugfix.rst
diff --git a/news/10655.bugfix.rst b/news/10655.bugfix.rst
new file mode 100644
index 00000000000..5e867d8246a
--- /dev/null
+++ b/news/10655.bugfix.rst
@@ -0,0 +1,4 @@
+Stop backtracking on build failures, by instead surfacing them to the
+user and failing immediately. This behaviour is more forgiving when
+a package cannot be built due to missing build dependencies or platform
+incompatibility.
diff --git a/src/pip/_internal/resolution/resolvelib/factory.py b/src/pip/_internal/resolution/resolvelib/factory.py
index 8c28d0014dc..20d779e880f 100644
--- a/src/pip/_internal/resolution/resolvelib/factory.py
+++ b/src/pip/_internal/resolution/resolvelib/factory.py
@@ -27,7 +27,6 @@
from pip._internal.exceptions import (
DistributionNotFound,
InstallationError,
- InstallationSubprocessError,
MetadataInconsistent,
UnsupportedPythonVersion,
UnsupportedWheel,
@@ -190,7 +189,7 @@ def _make_candidate_from_link(
name=name,
version=version,
)
- except (InstallationSubprocessError, MetadataInconsistent) as e:
+ except MetadataInconsistent as e:
logger.info(
"Discarding [blue underline]%s[/]: [yellow]%s[reset]",
link,
@@ -210,7 +209,7 @@ def _make_candidate_from_link(
name=name,
version=version,
)
- except (InstallationSubprocessError, MetadataInconsistent) as e:
+ except MetadataInconsistent as e:
logger.info(
"Discarding [blue underline]%s[/]: [yellow]%s[reset]",
link,
diff --git a/tests/functional/test_new_resolver.py b/tests/functional/test_new_resolver.py
index b3f523788ea..5e64f1665c8 100644
--- a/tests/functional/test_new_resolver.py
+++ b/tests/functional/test_new_resolver.py
@@ -2308,3 +2308,22 @@ def test_new_resolver_respect_user_requested_if_extra_is_installed(
"pkg2",
)
script.assert_installed(pkg3="1.0", pkg2="2.0", pkg1="1.0")
+
+
+def test_new_resolver_do_not_backtrack_on_build_failure(
+ script: PipTestEnvironment,
+) -> None:
+ create_basic_sdist_for_package(script, "pkg1", "2.0", fails_egg_info=True)
+ create_basic_wheel_for_package(script, "pkg1", "1.0")
+
+ result = script.pip(
+ "install",
+ "--no-cache-dir",
+ "--no-index",
+ "--find-links",
+ script.scratch_path,
+ "pkg1",
+ expect_error=True,
+ )
+
+ assert "egg_info" in result.stderr
diff --git a/tests/lib/__init__.py b/tests/lib/__init__.py
index 9c1b4f442a9..06849d2d705 100644
--- a/tests/lib/__init__.py
+++ b/tests/lib/__init__.py
@@ -1183,22 +1183,36 @@ def create_basic_sdist_for_package(
name: str,
version: str,
extra_files: Optional[Dict[str, str]] = None,
+ *,
+ fails_egg_info: bool = False,
+ fails_bdist_wheel: bool = False,
) -> Path:
files = {
- "setup.py": """
+ "setup.py": f"""\
+ import sys
from setuptools import find_packages, setup
+
+ fails_bdist_wheel = {fails_bdist_wheel!r}
+ fails_egg_info = {fails_egg_info!r}
+
+ if fails_egg_info and "egg_info" in sys.argv:
+ raise Exception("Simulated failure for generating metadata.")
+
+ if fails_bdist_wheel and "bdist_wheel" in sys.argv:
+ raise Exception("Simulated failure for building a wheel.")
+
setup(name={name!r}, version={version!r})
""",
}
# Some useful shorthands
- archive_name = "{name}-{version}.tar.gz".format(name=name, version=version)
+ archive_name = f"{name}-{version}.tar.gz"
# Replace key-values with formatted values
for key, value in list(files.items()):
del files[key]
key = key.format(name=name)
- files[key] = textwrap.dedent(value).format(name=name, version=version).strip()
+ files[key] = textwrap.dedent(value)
# Add new files after formatting
if extra_files:
From ca78aba4565d95b1f70039da205ea2f8ddfe9134 Mon Sep 17 00:00:00 2001
From: Pradyun Gedam <pradyunsg@users.noreply.github.com>
Date: Sun, 12 Dec 2021 08:18:28 +0000
Subject: [PATCH 2/3] Test that the resolver skips packages, as instructed by
constraints
---
tests/functional/test_new_resolver.py | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/tests/functional/test_new_resolver.py b/tests/functional/test_new_resolver.py
index 5e64f1665c8..1f04e4ab253 100644
--- a/tests/functional/test_new_resolver.py
+++ b/tests/functional/test_new_resolver.py
@@ -2327,3 +2327,26 @@ def test_new_resolver_do_not_backtrack_on_build_failure(
)
assert "egg_info" in result.stderr
+
+
+def test_new_resolver_works_when_failing_package_builds_are_disallowed(
+ script: PipTestEnvironment,
+) -> None:
+ create_basic_wheel_for_package(script, "pkg2", "1.0", depends=["pkg1"])
+ create_basic_sdist_for_package(script, "pkg1", "2.0", fails_egg_info=True)
+ create_basic_wheel_for_package(script, "pkg1", "1.0")
+ constraints_file = script.scratch_path / "constraints.txt"
+ constraints_file.write_text("pkg1 != 2.0")
+
+ script.pip(
+ "install",
+ "--no-cache-dir",
+ "--no-index",
+ "--find-links",
+ script.scratch_path,
+ "-c",
+ constraints_file,
+ "pkg2",
+ )
+
+ script.assert_installed(pkg2="1.0", pkg1="1.0")
From 9d0db8839ffc3c7750410f22a85bfc424a7a58f5 Mon Sep 17 00:00:00 2001
From: Pradyun Gedam <pradyunsg@users.noreply.github.com>
Date: Sun, 12 Dec 2021 13:00:51 +0000
Subject: [PATCH 3/3] Add `--use-deprecated=backtrack-on-build-failures`
This serves as an opt-out from build failures causing the entire
installation to abort.
---
src/pip/_internal/cli/cmdoptions.py | 2 +-
src/pip/_internal/cli/req_command.py | 27 +++++++++++++++++++
.../resolution/resolvelib/factory.py | 16 +++++++++++
.../resolution/resolvelib/resolver.py | 2 ++
tests/functional/test_new_resolver.py | 20 ++++++++++++++
tests/unit/resolution_resolvelib/conftest.py | 1 +
.../resolution_resolvelib/test_resolver.py | 1 +
7 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/src/pip/_internal/cli/cmdoptions.py b/src/pip/_internal/cli/cmdoptions.py
index 6e43928f6ee..e9806fd79d0 100644
--- a/src/pip/_internal/cli/cmdoptions.py
+++ b/src/pip/_internal/cli/cmdoptions.py
@@ -964,7 +964,7 @@ def check_list_path_option(options: Values) -> None:
metavar="feature",
action="append",
default=[],
- choices=["legacy-resolver", "out-of-tree-build"],
+ choices=["legacy-resolver", "out-of-tree-build", "backtrack-on-build-failures"],
help=("Enable deprecated functionality, that will be removed in the future."),
)
diff --git a/src/pip/_internal/cli/req_command.py b/src/pip/_internal/cli/req_command.py
index 177b3dbc24d..8dc00e32826 100644
--- a/src/pip/_internal/cli/req_command.py
+++ b/src/pip/_internal/cli/req_command.py
@@ -227,6 +227,31 @@ def determine_resolver_variant(options: Values) -> str:
return "2020-resolver"
+ @staticmethod
+ def determine_build_failure_suppression(options: Values) -> bool:
+ """Determines whether build failures should be suppressed and backtracked on."""
+ if "backtrack-on-build-failures" not in options.deprecated_features_enabled:
+ return False
+
+ if "legacy-resolver" in options.deprecated_features_enabled:
+ raise CommandError("Cannot backtrack with legacy resolver.")
+
+ deprecated(
+ reason=(
+ "Backtracking on build failures can mask issues related to how "
+ "a package generates metadata or builds a wheel. This flag will "
+ "be removed in pip 22.2."
+ ),
+ gone_in=None,
+ replacement=(
+ "avoiding known-bad versions by explicitly telling pip to ignore them "
+ "(either directly as requirements, or via a constraints file)"
+ ),
+ feature_flag=None,
+ issue=10655,
+ )
+ return True
+
@classmethod
def make_requirement_preparer(
cls,
@@ -323,6 +348,7 @@ def make_resolver(
isolated=options.isolated_mode,
use_pep517=use_pep517,
)
+ suppress_build_failures = cls.determine_build_failure_suppression(options)
resolver_variant = cls.determine_resolver_variant(options)
# The long import name and duplicated invocation is needed to convince
# Mypy into correctly typechecking. Otherwise it would complain the
@@ -342,6 +368,7 @@ def make_resolver(
force_reinstall=force_reinstall,
upgrade_strategy=upgrade_strategy,
py_version_info=py_version_info,
+ suppress_build_failures=suppress_build_failures,
)
import pip._internal.resolution.legacy.resolver
diff --git a/src/pip/_internal/resolution/resolvelib/factory.py b/src/pip/_internal/resolution/resolvelib/factory.py
index 20d779e880f..5c4d3f7cd81 100644
--- a/src/pip/_internal/resolution/resolvelib/factory.py
+++ b/src/pip/_internal/resolution/resolvelib/factory.py
@@ -27,6 +27,7 @@
from pip._internal.exceptions import (
DistributionNotFound,
InstallationError,
+ InstallationSubprocessError,
MetadataInconsistent,
UnsupportedPythonVersion,
UnsupportedWheel,
@@ -96,6 +97,7 @@ def __init__(
force_reinstall: bool,
ignore_installed: bool,
ignore_requires_python: bool,
+ suppress_build_failures: bool,
py_version_info: Optional[Tuple[int, ...]] = None,
) -> None:
self._finder = finder
@@ -106,6 +108,7 @@ def __init__(
self._use_user_site = use_user_site
self._force_reinstall = force_reinstall
self._ignore_requires_python = ignore_requires_python
+ self._suppress_build_failures = suppress_build_failures
self._build_failures: Cache[InstallationError] = {}
self._link_candidate_cache: Cache[LinkCandidate] = {}
@@ -198,6 +201,13 @@ def _make_candidate_from_link(
)
self._build_failures[link] = e
return None
+ except InstallationSubprocessError as e:
+ if not self._suppress_build_failures:
+ raise
+ logger.warning("Discarding %s due to build failure: %s", link, e)
+ self._build_failures[link] = e
+ return None
+
base: BaseCandidate = self._editable_candidate_cache[link]
else:
if link not in self._link_candidate_cache:
@@ -218,6 +228,12 @@ def _make_candidate_from_link(
)
self._build_failures[link] = e
return None
+ except InstallationSubprocessError as e:
+ if not self._suppress_build_failures:
+ raise
+ logger.warning("Discarding %s due to build failure: %s", link, e)
+ self._build_failures[link] = e
+ return None
base = self._link_candidate_cache[link]
if not extras:
diff --git a/src/pip/_internal/resolution/resolvelib/resolver.py b/src/pip/_internal/resolution/resolvelib/resolver.py
index 8ee36d377d8..618f1e1aead 100644
--- a/src/pip/_internal/resolution/resolvelib/resolver.py
+++ b/src/pip/_internal/resolution/resolvelib/resolver.py
@@ -47,6 +47,7 @@ def __init__(
ignore_requires_python: bool,
force_reinstall: bool,
upgrade_strategy: str,
+ suppress_build_failures: bool,
py_version_info: Optional[Tuple[int, ...]] = None,
):
super().__init__()
@@ -61,6 +62,7 @@ def __init__(
force_reinstall=force_reinstall,
ignore_installed=ignore_installed,
ignore_requires_python=ignore_requires_python,
+ suppress_build_failures=suppress_build_failures,
py_version_info=py_version_info,
)
self.ignore_dependencies = ignore_dependencies
diff --git a/tests/functional/test_new_resolver.py b/tests/functional/test_new_resolver.py
index 1f04e4ab253..21b9e0b7fa9 100644
--- a/tests/functional/test_new_resolver.py
+++ b/tests/functional/test_new_resolver.py
@@ -2329,6 +2329,26 @@ def test_new_resolver_do_not_backtrack_on_build_failure(
assert "egg_info" in result.stderr
+def test_new_resolver_flag_permits_backtracking_on_build_failure(
+ script: PipTestEnvironment,
+) -> None:
+ create_basic_sdist_for_package(script, "pkg1", "2.0", fails_egg_info=True)
+ create_basic_wheel_for_package(script, "pkg1", "1.0")
+
+ script.pip(
+ "install",
+ "--use-deprecated=backtrack-on-build-failures",
+ "--no-cache-dir",
+ "--no-index",
+ "--find-links",
+ script.scratch_path,
+ "pkg1",
+ allow_stderr_warning=True,
+ )
+
+ script.assert_installed(pkg1="1.0")
+
+
def test_new_resolver_works_when_failing_package_builds_are_disallowed(
script: PipTestEnvironment,
) -> None:
diff --git a/tests/unit/resolution_resolvelib/conftest.py b/tests/unit/resolution_resolvelib/conftest.py
index 8f3bba5016e..cfd440570e6 100644
--- a/tests/unit/resolution_resolvelib/conftest.py
+++ b/tests/unit/resolution_resolvelib/conftest.py
@@ -63,6 +63,7 @@ def factory(finder: PackageFinder, preparer: RequirementPreparer) -> Iterator[Fa
force_reinstall=False,
ignore_installed=False,
ignore_requires_python=False,
+ suppress_build_failures=False,
py_version_info=None,
)
diff --git a/tests/unit/resolution_resolvelib/test_resolver.py b/tests/unit/resolution_resolvelib/test_resolver.py
index 579195b55ea..d1af696b7a2 100644
--- a/tests/unit/resolution_resolvelib/test_resolver.py
+++ b/tests/unit/resolution_resolvelib/test_resolver.py
@@ -29,6 +29,7 @@ def resolver(preparer: RequirementPreparer, finder: PackageFinder) -> Resolver:
ignore_requires_python=False,
force_reinstall=False,
upgrade_strategy="to-satisfy-only",
+ suppress_build_failures=False,
)
return resolver