diff --git a/src/fromager/bootstrapper.py b/src/fromager/bootstrapper.py index b680b217..2516fdca 100644 --- a/src/fromager/bootstrapper.py +++ b/src/fromager/bootstrapper.py @@ -224,59 +224,38 @@ def _processing_build_requirement(self, current_req_type: RequirementType) -> bo def bootstrap(self, req: Requirement, req_type: RequirementType) -> None: """Bootstrap a package and its dependencies. + Handles setup, validation, and error handling. Delegates actual build + work to _bootstrap_impl(). + In test mode, catches build exceptions, records package name, and continues. In normal mode, raises exceptions immediately (fail-fast). """ + logger.info(f"bootstrapping {req} as {req_type} dependency of {self.why[-1:]}") + + # Resolve version first so we have it for error reporting. + # In test mode, record resolution failures and continue. try: - self._bootstrap_impl(req, req_type) + source_url, resolved_version = self.resolve_version( + req=req, + req_type=req_type, + ) except Exception as err: if not self.test_mode: raise - # Get version from cache if available - cached = self._resolved_requirements.get(str(req)) - if cached: - _source_url, resolved_version = cached - version = str(resolved_version) - else: - version = None - self._record_test_mode_failure(req, version, err, "bootstrap") + self._record_test_mode_failure(req, None, err, "resolution") + return - def _bootstrap_impl(self, req: Requirement, req_type: RequirementType) -> None: - """Internal implementation of bootstrap logic. + # Capture parent before _track_why pushes current package onto the stack + parent: tuple[Requirement, Version] | None = None + if self.why: + _, parent_req, parent_version = self.why[-1] + parent = (parent_req, parent_version) - Error Handling: - Fatal errors (version resolution, source build, prebuilt download) - raise exceptions for bootstrap() to catch and record. + # Update dependency graph unconditionally (before seen check to capture all edges) + self._add_to_graph(req, req_type, resolved_version, source_url, parent) - Non-fatal errors (post-hook, dependency extraction) are recorded - locally and processing continues. These are recorded here because - the package build succeeded - only optional post-processing failed. - """ - logger.info(f"bootstrapping {req} as {req_type} dependency of {self.why[-1:]}") - constraint = self.ctx.constraints.get_constraint(req.name) - if constraint: - logger.info( - f"incoming requirement {req} matches constraint {constraint}. Will apply both." - ) - - source_url, resolved_version = self.resolve_version( - req=req, - req_type=req_type, - ) - pbi = self.ctx.package_build_info(req) - - self._add_to_graph(req, req_type, resolved_version, source_url) - - # Is bootstrap going to create a wheel or just an sdist? - # - # Use fast sdist-only if flag is set and requirement is not a build - # requirement. - # - # An install requirement on a pre-built wheel treats the wheel as - # sdist-only in order to build its installation requirements sdist-only. - # - # When bootstrap encounters another package with a *build* requirement - # on a pre-built wheel, its installation dependencies are materialized. + # Build sdist-only (no wheel) if flag is set, unless this is a build + # requirement which always needs a full wheel. build_sdist_only = self.sdist_only and not self._processing_build_requirement( req_type ) @@ -292,109 +271,153 @@ def _bootstrap_impl(self, req: Requirement, req_type: RequirementType) -> None: logger.info(f"new {req_type} dependency {req} resolves to {resolved_version}") - # Track dependency chain for error messages - context manager ensures cleanup + # Track dependency chain - context manager ensures cleanup even on exception with self._track_why(req_type, req, resolved_version): - cached_wheel_filename: pathlib.Path | None = None - unpacked_cached_wheel: pathlib.Path | None = None - - if pbi.pre_built: - wheel_filename, unpack_dir = self._download_prebuilt( - req=req, - req_type=req_type, - resolved_version=resolved_version, - wheel_url=source_url, - ) - build_result = SourceBuildResult( - wheel_filename=wheel_filename, - sdist_filename=None, - unpack_dir=unpack_dir, - sdist_root_dir=None, - build_env=None, - source_type=SourceType.PREBUILT, - ) - else: - # Look for an existing wheel in caches before building - cached_wheel_filename, unpacked_cached_wheel = self._find_cached_wheel( - req, resolved_version - ) - - # Build from source (handles test-mode fallback internally) - 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, - ) - - # Run post-bootstrap hooks (non-fatal in test mode) try: - hooks.run_post_bootstrap_hooks( - ctx=self.ctx, - req=req, - dist_name=canonicalize_name(req.name), - dist_version=str(resolved_version), - sdist_filename=build_result.sdist_filename, - wheel_filename=build_result.wheel_filename, + self._bootstrap_impl( + req, req_type, source_url, resolved_version, build_sdist_only ) - except Exception as hook_error: + except Exception as err: if not self.test_mode: raise self._record_test_mode_failure( - req, str(resolved_version), hook_error, "hook", "warning" + req, str(resolved_version), err, "bootstrap" ) - # Extract install dependencies (non-fatal in test mode) - try: - install_dependencies = self._get_install_dependencies( - req=req, - resolved_version=resolved_version, - wheel_filename=build_result.wheel_filename, - sdist_filename=build_result.sdist_filename, - sdist_root_dir=build_result.sdist_root_dir, - build_env=build_result.build_env, - unpack_dir=build_result.unpack_dir, - ) - except Exception as dep_error: - if not self.test_mode: - raise - self._record_test_mode_failure( - req, - str(resolved_version), - dep_error, - "dependency_extraction", - "warning", - ) - install_dependencies = [] + def _bootstrap_impl( + self, + req: Requirement, + req_type: RequirementType, + source_url: str, + resolved_version: Version, + build_sdist_only: bool, + ) -> None: + """Internal implementation - performs the actual bootstrap work. - logger.debug( - "install dependencies: %s", - ", ".join(sorted(str(r) for r in install_dependencies)), + Called by bootstrap() after setup, validation, and seen-checking. + + Args: + req: The requirement to bootstrap. + req_type: The type of requirement. + source_url: The resolved source URL. + resolved_version: The resolved version. + build_sdist_only: Whether to build only sdist (no wheel). + + Error Handling: + Fatal errors (source build, prebuilt download) raise exceptions + for bootstrap() to catch and record. + + Non-fatal errors (post-hook, dependency extraction) are recorded + locally and processing continues. These are recorded here because + the package build succeeded - only optional post-processing failed. + """ + constraint = self.ctx.constraints.get_constraint(req.name) + if constraint: + logger.info( + f"incoming requirement {req} matches constraint {constraint}. Will apply both." + ) + + pbi = self.ctx.package_build_info(req) + + cached_wheel_filename: pathlib.Path | None = None + unpacked_cached_wheel: pathlib.Path | None = None + + if pbi.pre_built: + wheel_filename, unpack_dir = self._download_prebuilt( + req=req, + req_type=req_type, + resolved_version=resolved_version, + wheel_url=source_url, + ) + build_result = SourceBuildResult( + wheel_filename=wheel_filename, + sdist_filename=None, + unpack_dir=unpack_dir, + sdist_root_dir=None, + build_env=None, + source_type=SourceType.PREBUILT, + ) + else: + # Look for an existing wheel in caches before building + cached_wheel_filename, unpacked_cached_wheel = self._find_cached_wheel( + req, resolved_version ) - self._add_to_build_order( + # Build from source (handles test-mode fallback internally) + build_result = self._build_from_source( req=req, - version=resolved_version, + resolved_version=resolved_version, source_url=source_url, - source_type=build_result.source_type, - prebuilt=pbi.pre_built, - constraint=constraint, + req_type=req_type, + build_sdist_only=build_sdist_only, + cached_wheel_filename=cached_wheel_filename, + unpacked_cached_wheel=unpacked_cached_wheel, ) - self.progressbar.update_total(len(install_dependencies)) - for dep in self._sort_requirements(install_dependencies): - with req_ctxvar_context(dep): - # In test mode, bootstrap() catches and records failures internally. - # In normal mode, it raises immediately which we propagate. - self.bootstrap(req=dep, req_type=RequirementType.INSTALL) - self.progressbar.update() - - # Clean up build directories (why stack cleanup handled by context manager) - self.ctx.clean_build_dirs( - build_result.sdist_root_dir, build_result.build_env + # Run post-bootstrap hooks (non-fatal in test mode) + try: + hooks.run_post_bootstrap_hooks( + ctx=self.ctx, + req=req, + dist_name=canonicalize_name(req.name), + dist_version=str(resolved_version), + sdist_filename=build_result.sdist_filename, + wheel_filename=build_result.wheel_filename, + ) + except Exception as hook_error: + if not self.test_mode: + raise + self._record_test_mode_failure( + req, str(resolved_version), hook_error, "hook", "warning" + ) + + # Extract install dependencies (non-fatal in test mode) + try: + install_dependencies = self._get_install_dependencies( + req=req, + resolved_version=resolved_version, + wheel_filename=build_result.wheel_filename, + sdist_filename=build_result.sdist_filename, + sdist_root_dir=build_result.sdist_root_dir, + build_env=build_result.build_env, + unpack_dir=build_result.unpack_dir, ) + except Exception as dep_error: + if not self.test_mode: + raise + self._record_test_mode_failure( + req, + str(resolved_version), + dep_error, + "dependency_extraction", + "warning", + ) + install_dependencies = [] + + logger.debug( + "install dependencies: %s", + ", ".join(sorted(str(r) for r in install_dependencies)), + ) + + self._add_to_build_order( + req=req, + version=resolved_version, + source_url=source_url, + source_type=build_result.source_type, + prebuilt=pbi.pre_built, + constraint=constraint, + ) + + self.progressbar.update_total(len(install_dependencies)) + for dep in self._sort_requirements(install_dependencies): + with req_ctxvar_context(dep): + # In test mode, bootstrap() catches and records failures internally. + # In normal mode, it raises immediately which we propagate. + self.bootstrap(req=dep, req_type=RequirementType.INSTALL) + self.progressbar.update() + + # Clean up build directories + self.ctx.clean_build_dirs(build_result.sdist_root_dir, build_result.build_env) @contextlib.contextmanager def _track_why( @@ -1340,11 +1363,12 @@ def _add_to_graph( req_type: RequirementType, req_version: Version, download_url: str, + parent: tuple[Requirement, Version] | None, ) -> None: if req_type == RequirementType.TOP_LEVEL: return - _, parent_req, parent_version = self.why[-1] if self.why else (None, None, None) + parent_req, parent_version = parent if parent else (None, None) pbi = self.ctx.package_build_info(req) # Update the dependency graph after we determine that this requirement is # useful but before we determine if it is redundant so that we capture all diff --git a/tests/test_bootstrap_test_mode.py b/tests/test_bootstrap_test_mode.py index 862daeb9..84b8bc60 100644 --- a/tests/test_bootstrap_test_mode.py +++ b/tests/test_bootstrap_test_mode.py @@ -10,84 +10,55 @@ import json import pathlib -import tempfile -import typing -from unittest.mock import Mock +from unittest import mock import pytest +from packaging.requirements import Requirement from fromager import bootstrapper, context - - -@pytest.fixture -def mock_context() -> typing.Generator[context.WorkContext, None, None]: - """Create a mock WorkContext for testing.""" - with tempfile.TemporaryDirectory() as tmpdir: - work_dir = pathlib.Path(tmpdir) - - mock_ctx = Mock(spec=context.WorkContext) - mock_ctx.work_dir = work_dir - mock_ctx.wheels_build = work_dir / "wheels-build" - mock_ctx.wheels_downloads = work_dir / "wheels-downloads" - mock_ctx.wheels_prebuilt = work_dir / "wheels-prebuilt" - mock_ctx.sdists_builds = work_dir / "sdists-builds" - mock_ctx.wheel_server_url = None - mock_ctx.constraints = Mock() - mock_ctx.constraints.get_constraint = Mock(return_value=None) - mock_ctx.settings = Mock() - mock_ctx.variant = "test" - - for d in [ - mock_ctx.wheels_build, - mock_ctx.wheels_downloads, - mock_ctx.wheels_prebuilt, - mock_ctx.sdists_builds, - ]: - d.mkdir(parents=True, exist_ok=True) - - yield mock_ctx +from fromager.requirements_file import RequirementType class TestBootstrapperInitialization: """Test Bootstrapper initialization with test_mode parameter.""" - def test_test_mode_enabled(self, mock_context: context.WorkContext) -> None: + def test_test_mode_enabled(self, tmp_context: context.WorkContext) -> None: """Test Bootstrapper with test_mode=True.""" - bt = bootstrapper.Bootstrapper(ctx=mock_context, test_mode=True) + bt = bootstrapper.Bootstrapper(ctx=tmp_context, test_mode=True) assert bt.test_mode is True assert isinstance(bt.failed_packages, list) assert len(bt.failed_packages) == 0 def test_test_mode_disabled_by_default( - self, mock_context: context.WorkContext + self, tmp_context: context.WorkContext ) -> None: """Test Bootstrapper with test_mode=False (default).""" - bt = bootstrapper.Bootstrapper(ctx=mock_context) + bt = bootstrapper.Bootstrapper(ctx=tmp_context) assert bt.test_mode is False def test_test_mode_incompatible_with_sdist_only( - self, mock_context: context.WorkContext + self, tmp_context: context.WorkContext ) -> None: """Test that test_mode and sdist_only are mutually exclusive.""" with pytest.raises(ValueError, match="--test-mode requires full wheel builds"): - bootstrapper.Bootstrapper(ctx=mock_context, test_mode=True, sdist_only=True) + bootstrapper.Bootstrapper(ctx=tmp_context, test_mode=True, sdist_only=True) class TestFinalizeExitCodes: """Test finalize() returns correct exit codes.""" def test_finalize_no_failures_returns_zero( - self, mock_context: context.WorkContext + self, tmp_context: context.WorkContext ) -> None: """Test finalize returns 0 when no failures in test mode.""" - bt = bootstrapper.Bootstrapper(ctx=mock_context, test_mode=True) + bt = bootstrapper.Bootstrapper(ctx=tmp_context, test_mode=True) assert bt.finalize() == 0 def test_finalize_with_failures_returns_one( - self, mock_context: context.WorkContext + self, tmp_context: context.WorkContext ) -> None: """Test finalize returns 1 when there are failures in test mode.""" - bt = bootstrapper.Bootstrapper(ctx=mock_context, test_mode=True) + bt = bootstrapper.Bootstrapper(ctx=tmp_context, test_mode=True) bt.failed_packages.append( { "package": "failing-pkg", @@ -100,10 +71,10 @@ def test_finalize_with_failures_returns_one( assert bt.finalize() == 1 def test_finalize_not_in_test_mode_returns_zero( - self, mock_context: context.WorkContext + self, tmp_context: context.WorkContext ) -> None: """Test finalize returns 0 when not in test mode (regardless of failures).""" - bt = bootstrapper.Bootstrapper(ctx=mock_context, test_mode=False) + bt = bootstrapper.Bootstrapper(ctx=tmp_context, test_mode=False) # Even if we manually add failures (shouldn't happen), it returns 0 bt.failed_packages.append( { @@ -117,10 +88,10 @@ def test_finalize_not_in_test_mode_returns_zero( assert bt.finalize() == 0 def test_finalize_logs_failed_packages( - self, mock_context: context.WorkContext, caplog: pytest.LogCaptureFixture + self, tmp_context: context.WorkContext, caplog: pytest.LogCaptureFixture ) -> None: """Test finalize logs the list of failed packages.""" - bt = bootstrapper.Bootstrapper(ctx=mock_context, test_mode=True) + bt = bootstrapper.Bootstrapper(ctx=tmp_context, test_mode=True) bt.failed_packages.extend( [ { @@ -166,10 +137,10 @@ class TestJsonFailureReport: """Test JSON failure report generation.""" def test_finalize_writes_json_report( - self, mock_context: context.WorkContext + self, tmp_context: context.WorkContext ) -> None: """Test finalize writes test-mode-failures-.json with failure details.""" - bt = bootstrapper.Bootstrapper(ctx=mock_context, test_mode=True) + bt = bootstrapper.Bootstrapper(ctx=tmp_context, test_mode=True) bt.failed_packages.append( { "package": "failing-pkg", @@ -182,7 +153,7 @@ def test_finalize_writes_json_report( bt.finalize() - report_path = _find_failure_report(mock_context.work_dir) + report_path = _find_failure_report(tmp_context.work_dir) assert report_path is not None assert report_path.name.startswith("test-mode-failures-") assert report_path.name.endswith(".json") @@ -199,21 +170,21 @@ def test_finalize_writes_json_report( assert report["failures"][0]["failure_type"] == "bootstrap" def test_finalize_no_report_when_no_failures( - self, mock_context: context.WorkContext + self, tmp_context: context.WorkContext ) -> None: """Test finalize does not write report when there are no failures.""" - bt = bootstrapper.Bootstrapper(ctx=mock_context, test_mode=True) + bt = bootstrapper.Bootstrapper(ctx=tmp_context, test_mode=True) bt.finalize() - report_path = _find_failure_report(mock_context.work_dir) + report_path = _find_failure_report(tmp_context.work_dir) assert report_path is None def test_finalize_report_with_null_version( - self, mock_context: context.WorkContext + self, tmp_context: context.WorkContext ) -> None: """Test finalize handles failures where version is None (resolution failure).""" - bt = bootstrapper.Bootstrapper(ctx=mock_context, test_mode=True) + bt = bootstrapper.Bootstrapper(ctx=tmp_context, test_mode=True) bt.failed_packages.append( { "package": "failed-to-resolve", @@ -226,7 +197,7 @@ def test_finalize_report_with_null_version( bt.finalize() - report_path = _find_failure_report(mock_context.work_dir) + report_path = _find_failure_report(tmp_context.work_dir) assert report_path is not None with open(report_path) as f: report = json.load(f) @@ -235,10 +206,10 @@ def test_finalize_report_with_null_version( assert report["failures"][0]["failure_type"] == "resolution" def test_finalize_report_multiple_failure_types( - self, mock_context: context.WorkContext + self, tmp_context: context.WorkContext ) -> None: """Test finalize correctly reports multiple failures with different types.""" - bt = bootstrapper.Bootstrapper(ctx=mock_context, test_mode=True) + bt = bootstrapper.Bootstrapper(ctx=tmp_context, test_mode=True) bt.failed_packages.extend( [ { @@ -267,7 +238,7 @@ def test_finalize_report_multiple_failure_types( bt.finalize() - report_path = _find_failure_report(mock_context.work_dir) + report_path = _find_failure_report(tmp_context.work_dir) assert report_path is not None with open(report_path) as f: report = json.load(f) @@ -277,3 +248,45 @@ def test_finalize_report_multiple_failure_types( assert "bootstrap" in failure_types assert "hook" in failure_types assert "dependency_extraction" in failure_types + + +class TestBootstrapExceptionHandling: + """Test bootstrap() catches and records exceptions in test mode.""" + + def test_resolution_failure_recorded_in_test_mode( + self, tmp_context: context.WorkContext + ) -> None: + """Test that resolve_version failures are recorded in test mode.""" + bt = bootstrapper.Bootstrapper(ctx=tmp_context, test_mode=True) + req = Requirement("nonexistent-package>=1.0") + + # Mock resolve_version to raise an exception + with mock.patch.object( + bt, "resolve_version", side_effect=RuntimeError("Version resolution failed") + ): + # Should not raise in test mode + bt.bootstrap(req=req, req_type=RequirementType.TOP_LEVEL) + + # Verify failure was recorded + assert len(bt.failed_packages) == 1 + failure = bt.failed_packages[0] + assert failure["package"] == "nonexistent-package" + assert ( + failure["version"] is None + ) # No version available for resolution failures + assert failure["failure_type"] == "resolution" + assert "Version resolution failed" in failure["exception_message"] + + def test_resolution_failure_raises_in_normal_mode( + self, tmp_context: context.WorkContext + ) -> None: + """Test that resolve_version failures raise in normal mode.""" + bt = bootstrapper.Bootstrapper(ctx=tmp_context, test_mode=False) + req = Requirement("nonexistent-package>=1.0") + + # Mock resolve_version to raise an exception + with mock.patch.object( + bt, "resolve_version", side_effect=RuntimeError("Version resolution failed") + ): + with pytest.raises(RuntimeError, match="Version resolution failed"): + bt.bootstrap(req=req, req_type=RequirementType.TOP_LEVEL)