Repository: pypa/pip

Issue:
Abort immediately when there's a build failure, instead of backtracking
If you try to install a package that does not have wheels for your platform and you don't have the relevant build dependencies for it, pip will backtrack until it finds a version that it can build OR it has exhausted all the available versions of that package.

This can be especially painful when you're missing the build dependencies (in case of https://discuss.python.org/t/pip-without-setuptools-could-the-experience-be-improved/11810, it is setuptools) but in other cases, it can be a compiler / C library etc.

Would it make sense to fail immediately, instead of backtracking in these cases?


Discussion:
pradyunsg: There's a bunch more to do here and this isn't a complete solution to #10655 on its own.

- Need to improve our error messages on build failures (which I consider a blocker TBH, because the failure output right now sucks; but that's something I'll tackle as part of #10421).
- Should create supporting documentation for this to guide users on how to solve these issues. We learnt this lesson as part of our work on the dependency resolver, that providing documentation that clearly states what the user can do when they see something wierd, is a good thing.

[1]: https://github.com/pypa/pip/issues/10655#issuecomment-968052539

notatallshaw: When the new resolver was introduced one of the immediate problems was that some packages had their version number wrong and it caused the metadata to be failed to be built and pip would exit, see: https://github.com/pypa/pip/issues/9203

This was particularly problematic for packages with large dependencies or where this happened with transitive dependencies, and at the time all the recommended workarounds failed to remedy the situation: https://github.com/pypa/pip/issues/9203#issuecomment-738774792

Things may be better now as the ecosystem has had a year to mature. But I don't know if any of the advised workarounds have been fixed, I know adding top level user requirements won't prevent pip from downloading specific package versions, yanking the package to prevent pip from downloading is better but still somewhat depends on https://github.com/pypa/pip/pull/10625 merging, I don't know what the issue with the constraints file was.

Therefore, assuming the constraints file issue is fixed, I think it's important that the error message to the metadata building failure includes the following information:

1. The failure is with the package, not with pip, and the user should check that they have the requirements installed for the package
2. If the user wants to avoid pip checking a specific version of the package they should add an exclusion to the **constraints** file (with link to the constraints file as I think most users will confuse this with the requirements file)
3. If the user owns the package and the version is problematic they should consider yanking the package (it's clear from some discussions that at least some owners aren't aware of this functionality)

However if it's still possible that pip downloads a specific version even though it's excluded in the constraints file I think there really needs to be an option to override this behavior.

My 2 cents anyway, perhaps all of this is already done in the better error reporting and I'm just repeating things you are already aware of 🙂 
pradyunsg: This still backtracks if a package has inconsistent metadata, which was the problem in #9203.

It doesn't, however, backtrack when a project fails to compile or generate metadata.


notatallshaw: > This still backtracks if a package has inconsistent metadata, which was the problem in #9203.
> 
> It doesn't, however, backtrack when a project fails to compile or generate metadata.

That makes sense, but I think my comments still stand.

Imagining the hypothetical scenario PopularTransativeDependencyPackage v2 was released as source only and didn't compile easily on Windows but PopularTransativeDependencyPackage v1 was still mostly used by Windows users and included a compiled wheel for Windows. This PR would affect that scenario right?
pradyunsg: Yes. Now, let's tweak that case, imagine instead of just v2, it's also goes all the way to v35. What is a good behaviour here? Now imagine even v1 doesn't work on Windows.

The question is not whether the behaviour changes, but rather if it is better to fail eagerly instead of trying really hard to get an answer.

At this point I'm repeating OP from https://github.com/pypa/pip/issues/10655, 
notatallshaw: > Yes. Now, let's tweak that case, imagine instead of just v2, it goes all the way through to v35 (all like the v2 you described). What is a good behaviour here? Now imagine even v1 doesn't work on Windows. What is a good behaviour here?
> 
> The question is not whether the behaviour changes, but rather if it is better to fail eagerly instead of trying really hard to get an answer.
> 
> At this point I'm repeating OP from #10655, so... I'm going to stop.

Well in that scenario the install _eventually_ fails. Where as in the scenario I outlined where adding to constraints fails to block downloading a specific version then there is a valid solution to install but it's impossible to use pip to install that valid solution.

Therefore _if_ the issue of constraints still exists then the good behavior is categorically to try all versions so not to have impossible to install versions, or at least to have an option to keep that behavior.

However if the constraints issue has been solved then I think the error message is very important that includes the above 3 points I mentioned.
pradyunsg: > However if the constraints issue has been solved then I agree fast failing is correct

Cool, added a test to make sure that this indeed the case.
pradyunsg: > Well in that scenario the install _eventually_ fails.

Sure. If it hasn't filled up your disk trying to build those packages. If it hasn't been killed due to being out of memory on a memory-constrainted VM/Docker container. If it hasn't hit a timeout in your CI / documentation build, or hit the limit of a human's patience. 

Then, yes, you will get a failure from pip after failing to build every release of $package, because it doesn't support the platform you're running on or you're missing a required build-time dependency -- something you could tell from the first build failure and some Googling.

pradyunsg: I'm leaning toward deferring the documentation updates to be a part of #10421 and #9475's error messages page.

That's a much bigger effort, and I'd like to defer it to deal with this in that.
pfmoore: I'm happy to defer the docs, as long as they don't get forgotten. Maybe we could have an explicit issue for the docs as a reminder, but simply link it to #10421 and #9475?
pradyunsg: I'll leave #10655 open. :)

Patch:
diff --git a/news/10655.bugfix.rst b/news/10655.bugfix.rst
new file mode 100644
index 000000000..5e867d824
--- /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/cli/cmdoptions.py b/src/pip/_internal/cli/cmdoptions.py
index 6e43928f6..e9806fd79 100644
--- a/src/pip/_internal/cli/cmdoptions.py
+++ b/src/pip/_internal/cli/cmdoptions.py
@@ -964,7 +964,7 @@ use_deprecated_feature: Callable[..., Option] = partial(
     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 177b3dbc2..8dc00e328 100644
--- a/src/pip/_internal/cli/req_command.py
+++ b/src/pip/_internal/cli/req_command.py
@@ -227,6 +227,31 @@ class RequirementCommand(IndexGroupCommand):
 
         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 @@ class RequirementCommand(IndexGroupCommand):
             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 @@ class RequirementCommand(IndexGroupCommand):
                 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 8c28d0014..5c4d3f7cd 100644
--- a/src/pip/_internal/resolution/resolvelib/factory.py
+++ b/src/pip/_internal/resolution/resolvelib/factory.py
@@ -97,6 +97,7 @@ class Factory:
         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 @@ class Factory:
         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 @@ class Factory:
                         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 @@ class Factory:
                     )
                     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 @@ class Factory:
                         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 @@ class Factory:
                     )
                     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 8ee36d377..618f1e1ae 100644
--- a/src/pip/_internal/resolution/resolvelib/resolver.py
+++ b/src/pip/_internal/resolution/resolvelib/resolver.py
@@ -47,6 +47,7 @@ class Resolver(BaseResolver):
         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 @@ class Resolver(BaseResolver):
             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


Test patch:
diff --git a/tests/functional/test_new_resolver.py b/tests/functional/test_new_resolver.py
index b3f523788..21b9e0b7f 100644
--- a/tests/functional/test_new_resolver.py
+++ b/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")
diff --git a/tests/lib/__init__.py b/tests/lib/__init__.py
index 9c1b4f442..06849d2d7 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:
diff --git a/tests/unit/resolution_resolvelib/conftest.py b/tests/unit/resolution_resolvelib/conftest.py
index 8f3bba501..cfd440570 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 579195b55..d1af696b7 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