Merge pull request #10722 from pradyunsg/stop-backtracking-on-build-failures

Author: pradyunsg

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.

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."),
 )
 

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
 

src/pip/_internal/resolution/resolvelib/factory.py

@@ -97,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
@@ -107,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] = {}
@@ -190,7 +192,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,
@@ -199,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:
@@ -210,7 +219,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,
@@ -219,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:

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

tests/functional/test_new_resolver.py

@@ -2308,3 +2308,65 @@ 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
+
+
+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:
+    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")

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:

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,
     )
 

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