From 0e53faf99cf405593e8f60bfd432531ef304ae9e Mon Sep 17 00:00:00 2001 From: Lalatendu Mohanty Date: Fri, 19 Dec 2025 14:32:54 -0500 Subject: [PATCH 1/2] refactor(bootstrapper): add force_prebuilt flag, remove _handle_test_mode_failure Move test-mode fallback logic from _build_from_source to bootstrap(). Add force_prebuilt parameter to _bootstrap_impl for prebuilt retries. Simplifies _build_from_source by removing the code for fallback handling. Closes #892 Co-Authored-By: Claude Signed-off-by: Lalatendu Mohanty --- src/fromager/bootstrapper.py | 237 ++++++++++++++--------------------- tests/test_bootstrapper.py | 3 +- 2 files changed, 95 insertions(+), 145 deletions(-) diff --git a/src/fromager/bootstrapper.py b/src/fromager/bootstrapper.py index 2516fdca..c6472803 100644 --- a/src/fromager/bootstrapper.py +++ b/src/fromager/bootstrapper.py @@ -277,12 +277,55 @@ def bootstrap(self, req: Requirement, req_type: RequirementType) -> None: self._bootstrap_impl( req, req_type, source_url, resolved_version, build_sdist_only ) - except Exception as err: + except Exception as build_error: if not self.test_mode: raise - self._record_test_mode_failure( - req, str(resolved_version), err, "bootstrap" + + # Test mode: attempt pre-built fallback (may resolve different version) + logger.warning( + "test mode: build failed for %s==%s, attempting pre-built fallback: %s", + req.name, + resolved_version, + build_error, ) + try: + wheel_url, fallback_version = self._resolve_prebuilt_with_history( + req=req, + req_type=req_type, + ) + if fallback_version != resolved_version: + logger.warning( + "test mode: version mismatch for %s - requested %s, fallback %s", + req.name, + resolved_version, + fallback_version, + ) + # wheel_url passed as source_url; force_prebuilt ensures it's + # treated as a wheel URL, not an sdist URL + self._bootstrap_impl( + req, + req_type, + wheel_url, + fallback_version, + build_sdist_only, + force_prebuilt=True, + ) + logger.info( + "test mode: successfully used pre-built wheel for %s==%s", + req.name, + fallback_version, + ) + except Exception as fallback_error: + logger.error( + "test mode: pre-built fallback also failed for %s: %s", + req.name, + fallback_error, + exc_info=True, + ) + # Record the original build error, not the fallback error + self._record_test_mode_failure( + req, str(resolved_version), build_error, "bootstrap" + ) def _bootstrap_impl( self, @@ -291,6 +334,7 @@ def _bootstrap_impl( source_url: str, resolved_version: Version, build_sdist_only: bool, + force_prebuilt: bool = False, ) -> None: """Internal implementation - performs the actual bootstrap work. @@ -299,9 +343,11 @@ def _bootstrap_impl( Args: req: The requirement to bootstrap. req_type: The type of requirement. - source_url: The resolved source URL. + source_url: The resolved source URL (sdist or wheel URL). resolved_version: The resolved version. build_sdist_only: Whether to build only sdist (no wheel). + force_prebuilt: If True, treat source_url as a wheel URL and skip + source build. Used for test-mode fallback after build failure. Error Handling: Fatal errors (source build, prebuilt download) raise exceptions @@ -322,7 +368,7 @@ def _bootstrap_impl( cached_wheel_filename: pathlib.Path | None = None unpacked_cached_wheel: pathlib.Path | None = None - if pbi.pre_built: + if pbi.pre_built or force_prebuilt: wheel_filename, unpack_dir = self._download_prebuilt( req=req, req_type=req_type, @@ -343,12 +389,11 @@ def _bootstrap_impl( req, resolved_version ) - # Build from source (handles test-mode fallback internally) + # Build from source build_result = self._build_from_source( req=req, resolved_version=resolved_version, source_url=source_url, - req_type=req_type, build_sdist_only=build_sdist_only, cached_wheel_filename=cached_wheel_filename, unpacked_cached_wheel=unpacked_cached_wheel, @@ -779,7 +824,6 @@ def _build_from_source( req: Requirement, resolved_version: Version, source_url: str, - req_type: RequirementType, build_sdist_only: bool, cached_wheel_filename: pathlib.Path | None, unpacked_cached_wheel: pathlib.Path | None, @@ -787,158 +831,65 @@ def _build_from_source( """Build package from source. Orchestrates download, preparation, build environment setup, and build. - In test mode, attempts pre-built fallback on failure. Raises: - Exception: In normal mode, if build fails. - In test mode, only if build fails AND fallback also fails. + Exception: If any step fails. In test mode, bootstrap() handles + fallback to pre-built wheels. """ - try: - # Download and prepare source (if no cached wheel) - if not unpacked_cached_wheel: - logger.debug("no cached wheel, downloading sources") - source_filename = self._download_source( - req=req, - resolved_version=resolved_version, - source_url=source_url, - ) - sdist_root_dir = self._prepare_source( - req=req, - resolved_version=resolved_version, - source_filename=source_filename, - ) - else: - logger.debug(f"have cached wheel in {unpacked_cached_wheel}") - sdist_root_dir = unpacked_cached_wheel / unpacked_cached_wheel.stem - - assert sdist_root_dir is not None - - if sdist_root_dir.parent.parent != self.ctx.work_dir: - raise ValueError( - f"'{sdist_root_dir}/../..' should be {self.ctx.work_dir}" - ) - unpack_dir = sdist_root_dir.parent - - build_env = self._create_build_env( + # Download and prepare source (if no cached wheel) + if not unpacked_cached_wheel: + logger.debug("no cached wheel, downloading sources") + source_filename = self._download_source( req=req, resolved_version=resolved_version, - parent_dir=sdist_root_dir.parent, - ) - - # Prepare build dependencies (always needed) - # Note: This may recursively call bootstrap() for build deps, - # which has its own error handling. - self._prepare_build_dependencies(req, sdist_root_dir, build_env) - - # Build wheel or sdist - wheel_filename, sdist_filename = self._do_build( - req=req, - resolved_version=resolved_version, - sdist_root_dir=sdist_root_dir, - build_env=build_env, - build_sdist_only=build_sdist_only, - cached_wheel_filename=cached_wheel_filename, - ) - - source_type = sources.get_source_type(self.ctx, req) - - return SourceBuildResult( - wheel_filename=wheel_filename, - sdist_filename=sdist_filename, - unpack_dir=unpack_dir, - sdist_root_dir=sdist_root_dir, - build_env=build_env, - source_type=source_type, + source_url=source_url, ) - - except Exception as build_error: - if not self.test_mode: - raise - - # Test mode: attempt pre-built fallback - fallback_result = self._handle_test_mode_failure( + sdist_root_dir = self._prepare_source( req=req, resolved_version=resolved_version, - req_type=req_type, - build_error=build_error, + source_filename=source_filename, ) - if fallback_result is None: - # Fallback failed, re-raise for bootstrap() to catch - raise - - return fallback_result + else: + logger.debug(f"have cached wheel in {unpacked_cached_wheel}") + sdist_root_dir = unpacked_cached_wheel / unpacked_cached_wheel.stem - def _handle_test_mode_failure( - self, - req: Requirement, - resolved_version: Version, - req_type: RequirementType, - build_error: Exception, - ) -> SourceBuildResult | None: - """Handle build failure in test mode by attempting pre-built fallback. + assert sdist_root_dir is not None - Args: - req: The requirement that failed to build. - resolved_version: The version that was attempted. - req_type: The type of requirement (for fallback resolution). - build_error: The original exception from the build attempt. + if sdist_root_dir.parent.parent != self.ctx.work_dir: + raise ValueError(f"'{sdist_root_dir}/../..' should be {self.ctx.work_dir}") + unpack_dir = sdist_root_dir.parent - Returns: - SourceBuildResult if fallback succeeded, None if fallback also failed. - """ - logger.warning( - "test mode: build failed for %s==%s, attempting pre-built fallback: %s", - req.name, - resolved_version, - build_error, + build_env = self._create_build_env( + req=req, + resolved_version=resolved_version, + parent_dir=sdist_root_dir.parent, ) - try: - wheel_url, fallback_version = self._resolve_prebuilt_with_history( - req=req, - req_type=req_type, - ) + # Prepare build dependencies (always needed) + # Note: This may recursively call bootstrap() for build deps, + # which has its own error handling. + self._prepare_build_dependencies(req, sdist_root_dir, build_env) - if fallback_version != resolved_version: - logger.warning( - "test mode: version mismatch for %s - requested %s, fallback %s", - req.name, - resolved_version, - fallback_version, - ) - - wheel_filename, unpack_dir = self._download_prebuilt( - req=req, - req_type=req_type, - resolved_version=fallback_version, - wheel_url=wheel_url, - ) - - logger.info( - "test mode: successfully used pre-built wheel for %s==%s", - req.name, - fallback_version, - ) - # Package succeeded via fallback - no failure to record + # Build wheel or sdist + wheel_filename, sdist_filename = self._do_build( + req=req, + resolved_version=resolved_version, + sdist_root_dir=sdist_root_dir, + build_env=build_env, + build_sdist_only=build_sdist_only, + cached_wheel_filename=cached_wheel_filename, + ) - return SourceBuildResult( - wheel_filename=wheel_filename, - sdist_filename=None, - unpack_dir=unpack_dir, - sdist_root_dir=None, - build_env=None, - source_type=SourceType.PREBUILT, - ) + source_type = sources.get_source_type(self.ctx, req) - except Exception as fallback_error: - logger.error( - "test mode: pre-built fallback also failed for %s: %s", - req.name, - fallback_error, - exc_info=True, - ) - # Return None to signal failure; bootstrap() will record via re-raised exception - return None + return SourceBuildResult( + wheel_filename=wheel_filename, + sdist_filename=sdist_filename, + unpack_dir=unpack_dir, + sdist_root_dir=sdist_root_dir, + build_env=build_env, + source_type=source_type, + ) def _look_for_existing_wheel( self, diff --git a/tests/test_bootstrapper.py b/tests/test_bootstrapper.py index f87f1a30..e17991e6 100644 --- a/tests/test_bootstrapper.py +++ b/tests/test_bootstrapper.py @@ -6,7 +6,7 @@ from packaging.utils import canonicalize_name from packaging.version import Version -from fromager import bootstrapper, requirements_file +from fromager import bootstrapper from fromager.context import WorkContext from fromager.dependency_graph import DependencyGraph from fromager.requirements_file import RequirementType, SourceType @@ -495,7 +495,6 @@ def test_build_from_source_returns_dataclass(tmp_context: WorkContext) -> None: req=Requirement("test-package"), resolved_version=Version("1.0.0"), source_url="https://pypi.org/simple/test-package", - req_type=requirements_file.RequirementType.TOP_LEVEL, build_sdist_only=False, cached_wheel_filename=None, unpacked_cached_wheel=None, From f31d17c09e4ce0ce97c5e4b56bede03e5bf5496a Mon Sep 17 00:00:00 2001 From: Lalatendu Mohanty Date: Fri, 19 Dec 2025 15:01:44 -0500 Subject: [PATCH 2/2] test(test-mode): add tests for prebuilt fallback path in bootstrap() Co-Authored-By: Claude Signed-off-by: Lalatendu Mohanty --- tests/test_bootstrap_test_mode.py | 179 ++++++++++++++++++++++++++++++ 1 file changed, 179 insertions(+) diff --git a/tests/test_bootstrap_test_mode.py b/tests/test_bootstrap_test_mode.py index 84b8bc60..ded0f94c 100644 --- a/tests/test_bootstrap_test_mode.py +++ b/tests/test_bootstrap_test_mode.py @@ -14,6 +14,7 @@ import pytest from packaging.requirements import Requirement +from packaging.version import Version from fromager import bootstrapper, context from fromager.requirements_file import RequirementType @@ -290,3 +291,181 @@ def test_resolution_failure_raises_in_normal_mode( ): with pytest.raises(RuntimeError, match="Version resolution failed"): bt.bootstrap(req=req, req_type=RequirementType.TOP_LEVEL) + + +class TestPrebuiltFallback: + """Test prebuilt fallback behavior in test mode when build fails.""" + + def test_fallback_succeeds_no_failure_recorded( + self, tmp_context: context.WorkContext, caplog: pytest.LogCaptureFixture + ) -> None: + """Test that successful fallback to prebuilt doesn't record a failure.""" + bt = bootstrapper.Bootstrapper(ctx=tmp_context, test_mode=True) + req = Requirement("test-package>=1.0") + + with ( + mock.patch.object( + bt, + "resolve_version", + return_value=("https://sdist.url", Version("1.0")), + ), + mock.patch.object(bt, "_add_to_graph"), + mock.patch.object(bt, "_has_been_seen", return_value=False), + mock.patch.object(bt, "_mark_as_seen"), + # First call fails (build), second succeeds (fallback with force_prebuilt) + mock.patch.object( + bt, + "_bootstrap_impl", + side_effect=[RuntimeError("Build failed"), None], + ), + mock.patch.object( + bt, + "_resolve_prebuilt_with_history", + return_value=("https://wheel.url", Version("1.0")), + ), + ): + bt.bootstrap(req=req, req_type=RequirementType.TOP_LEVEL) + + # No failure recorded because fallback succeeded + assert len(bt.failed_packages) == 0 + assert "successfully used pre-built wheel" in caplog.text + + def test_fallback_version_mismatch_logs_warning( + self, tmp_context: context.WorkContext, caplog: pytest.LogCaptureFixture + ) -> None: + """Test that version mismatch during fallback logs a warning.""" + bt = bootstrapper.Bootstrapper(ctx=tmp_context, test_mode=True) + req = Requirement("test-package>=1.0") + + with ( + mock.patch.object( + bt, + "resolve_version", + return_value=("https://sdist.url", Version("1.0")), + ), + mock.patch.object(bt, "_add_to_graph"), + mock.patch.object(bt, "_has_been_seen", return_value=False), + mock.patch.object(bt, "_mark_as_seen"), + mock.patch.object( + bt, + "_bootstrap_impl", + side_effect=[RuntimeError("Build failed"), None], + ), + # Fallback resolves to different version + mock.patch.object( + bt, + "_resolve_prebuilt_with_history", + return_value=("https://wheel.url", Version("1.1")), + ), + ): + bt.bootstrap(req=req, req_type=RequirementType.TOP_LEVEL) + + # No failure recorded because fallback succeeded + assert len(bt.failed_packages) == 0 + assert "version mismatch" in caplog.text + assert "requested 1.0" in caplog.text + assert "fallback 1.1" in caplog.text + + def test_fallback_also_fails_records_original_error( + self, tmp_context: context.WorkContext, caplog: pytest.LogCaptureFixture + ) -> None: + """Test that when fallback also fails, original build error is recorded.""" + bt = bootstrapper.Bootstrapper(ctx=tmp_context, test_mode=True) + req = Requirement("test-package>=1.0") + + with ( + mock.patch.object( + bt, + "resolve_version", + return_value=("https://sdist.url", Version("1.0")), + ), + mock.patch.object(bt, "_add_to_graph"), + mock.patch.object(bt, "_has_been_seen", return_value=False), + mock.patch.object(bt, "_mark_as_seen"), + # Both build and fallback fail + mock.patch.object( + bt, + "_bootstrap_impl", + side_effect=[ + RuntimeError("Original build failed"), + RuntimeError("Fallback also failed"), + ], + ), + mock.patch.object( + bt, + "_resolve_prebuilt_with_history", + return_value=("https://wheel.url", Version("1.0")), + ), + ): + bt.bootstrap(req=req, req_type=RequirementType.TOP_LEVEL) + + # Failure recorded with ORIGINAL error, not fallback error + assert len(bt.failed_packages) == 1 + failure = bt.failed_packages[0] + assert failure["package"] == "test-package" + assert failure["version"] == "1.0" + assert failure["failure_type"] == "bootstrap" + assert "Original build failed" in failure["exception_message"] + assert "pre-built fallback also failed" in caplog.text + + def test_fallback_resolution_fails_records_original_error( + self, tmp_context: context.WorkContext + ) -> None: + """Test that when prebuilt resolution fails, original build error is recorded.""" + bt = bootstrapper.Bootstrapper(ctx=tmp_context, test_mode=True) + req = Requirement("test-package>=1.0") + + with ( + mock.patch.object( + bt, + "resolve_version", + return_value=("https://sdist.url", Version("1.0")), + ), + mock.patch.object(bt, "_add_to_graph"), + mock.patch.object(bt, "_has_been_seen", return_value=False), + mock.patch.object(bt, "_mark_as_seen"), + mock.patch.object( + bt, + "_bootstrap_impl", + side_effect=RuntimeError("Original build failed"), + ), + # Prebuilt resolution fails + mock.patch.object( + bt, + "_resolve_prebuilt_with_history", + side_effect=RuntimeError("No prebuilt available"), + ), + ): + bt.bootstrap(req=req, req_type=RequirementType.TOP_LEVEL) + + # Failure recorded with ORIGINAL error + assert len(bt.failed_packages) == 1 + assert "Original build failed" in bt.failed_packages[0]["exception_message"] + + def test_build_failure_raises_in_normal_mode( + self, tmp_context: context.WorkContext + ) -> None: + """Test that build failures raise immediately in normal mode (no fallback).""" + bt = bootstrapper.Bootstrapper(ctx=tmp_context, test_mode=False) + req = Requirement("test-package>=1.0") + + with ( + mock.patch.object( + bt, + "resolve_version", + return_value=("https://sdist.url", Version("1.0")), + ), + mock.patch.object(bt, "_add_to_graph"), + mock.patch.object(bt, "_has_been_seen", return_value=False), + mock.patch.object(bt, "_mark_as_seen"), + mock.patch.object( + bt, + "_bootstrap_impl", + side_effect=RuntimeError("Build failed"), + ), + ): + with pytest.raises(RuntimeError, match="Build failed"): + bt.bootstrap(req=req, req_type=RequirementType.TOP_LEVEL) + + # No fallback attempted in normal mode + assert len(bt.failed_packages) == 0