From 1f7ac061e71b4c4a9b9331149140b881a564fa84 Mon Sep 17 00:00:00 2001 From: Rohan Devasthale Date: Mon, 4 May 2026 14:36:58 -0400 Subject: [PATCH] refactor(bootstrapper): replace recursion with trampoline for iterative execution Convert recursive bootstrap methods to generators using the trampoline library, eliminating Python's recursion depth limit for deep/wide dependency graphs. Co-Authored-By: Claude Signed-off-by: Rohan Devasthale --- pyproject.toml | 3 +- src/fromager/bootstrapper.py | 59 +++++++++++++++++++++--------------- tests/test_bootstrapper.py | 32 +++++++++++++------ 3 files changed, 58 insertions(+), 36 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index df579722..00bde3db 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -48,6 +48,7 @@ dependencies = [ "stevedore", "tomlkit", "tqdm", + "trampoline", "wheel", "uv>=0.8.19", "uvicorn", @@ -207,7 +208,7 @@ exclude = [ [[tool.mypy.overrides]] # packages without typing annotations and stubs -module = ["hatchling", "hatchling.build", "license_expression", "pyproject_hooks", "requests_mock", "resolver", "spdx_tools.*", "stevedore"] +module = ["hatchling", "hatchling.build", "license_expression", "pyproject_hooks", "requests_mock", "resolver", "spdx_tools.*", "stevedore", "trampoline"] ignore_missing_imports = true [tool.basedpyright] diff --git a/src/fromager/bootstrapper.py b/src/fromager/bootstrapper.py index a2eb9a3e..dbe3d924 100644 --- a/src/fromager/bootstrapper.py +++ b/src/fromager/bootstrapper.py @@ -19,6 +19,7 @@ from packaging.utils import NormalizedName, canonicalize_name from packaging.version import Version from resolvelib.resolvers import ResolverException +from trampoline import trampoline from . import ( bootstrap_requirement_resolver, @@ -274,6 +275,16 @@ 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. + Public entry point that drives the generator-based implementation + via trampoline() for iterative (non-recursive) execution. + """ + trampoline(self._bootstrap_gen(req, req_type)) + + def _bootstrap_gen( + self, req: Requirement, req_type: RequirementType + ) -> typing.Generator[typing.Any, typing.Any, None]: + """Generator that bootstraps a package and its dependencies. + Handles setup, validation, and error handling. Delegates actual build work to _bootstrap_impl(). @@ -307,7 +318,9 @@ def bootstrap(self, req: Requirement, req_type: RequirementType) -> None: # Bootstrap each resolved version for source_url, resolved_version in resolved_versions: - self._bootstrap_single_version(req, req_type, source_url, resolved_version) + yield self._bootstrap_single_version( + req, req_type, source_url, resolved_version + ) # In multiple versions mode, report any failures for this requirement if self.multiple_versions and self._failed_versions: @@ -329,7 +342,7 @@ def _bootstrap_single_version( req_type: RequirementType, source_url: str, resolved_version: Version, - ) -> None: + ) -> typing.Generator[typing.Any, typing.Any, None]: """Bootstrap a single version of a package. Extracted from bootstrap() to handle both single and multiple version modes. @@ -365,7 +378,7 @@ def _bootstrap_single_version( # Track dependency chain - context manager ensures cleanup even on exception with self._track_why(req_type, req, resolved_version): try: - self._bootstrap_impl( + yield self._bootstrap_impl( req, req_type, source_url, resolved_version, build_sdist_only ) except Exception as err: @@ -400,7 +413,7 @@ def _bootstrap_impl( source_url: str, resolved_version: Version, build_sdist_only: bool, - ) -> None: + ) -> typing.Generator[typing.Any, typing.Any, None]: """Internal implementation - performs the actual bootstrap work. Called by bootstrap() after setup, validation, and seen-checking. @@ -453,7 +466,7 @@ def _bootstrap_impl( ) # Build from source (handles test-mode fallback internally) - build_result = self._build_from_source( + build_result = yield self._build_from_source( req=req, resolved_version=resolved_version, source_url=source_url, @@ -520,9 +533,7 @@ def _bootstrap_impl( 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) + yield self._bootstrap_gen(req=dep, req_type=RequirementType.INSTALL) self.progressbar.update() # Clean up build directories @@ -643,7 +654,7 @@ def _prepare_build_dependencies( resolved_version: Version | None, sdist_root_dir: pathlib.Path, build_env: build_environment.BuildEnvironment, - ) -> set[Requirement]: + ) -> typing.Generator[typing.Any, typing.Any, set[Requirement]]: # build system build_system_dependencies = dependencies.get_build_system_dependencies( ctx=self.ctx, @@ -651,7 +662,7 @@ def _prepare_build_dependencies( version=resolved_version, sdist_root_dir=sdist_root_dir, ) - self._handle_build_requirements( + yield self._handle_build_requirements( req, RequirementType.BUILD_SYSTEM, build_system_dependencies, @@ -667,7 +678,7 @@ def _prepare_build_dependencies( sdist_root_dir=sdist_root_dir, build_env=build_env, ) - self._handle_build_requirements( + yield self._handle_build_requirements( req, RequirementType.BUILD_BACKEND, build_backend_dependencies, @@ -681,7 +692,7 @@ def _prepare_build_dependencies( sdist_root_dir=sdist_root_dir, build_env=build_env, ) - self._handle_build_requirements( + yield self._handle_build_requirements( req, RequirementType.BUILD_SDIST, build_sdist_dependencies, @@ -702,14 +713,12 @@ def _handle_build_requirements( req: Requirement, build_type: RequirementType, build_dependencies: set[Requirement], - ) -> None: + ) -> typing.Generator[typing.Any, typing.Any, None]: self.progressbar.update_total(len(build_dependencies)) for dep in self._sort_requirements(build_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=build_type) + yield self._bootstrap_gen(req=dep, req_type=build_type) self.progressbar.update() def _download_prebuilt( @@ -896,7 +905,7 @@ def _build_from_source( build_sdist_only: bool, cached_wheel_filename: pathlib.Path | None, unpacked_cached_wheel: pathlib.Path | None, - ) -> SourceBuildResult: + ) -> typing.Generator[typing.Any, typing.Any, SourceBuildResult]: """Build package from source. Orchestrates download, preparation, build environment setup, and build. @@ -939,9 +948,7 @@ def _build_from_source( ) # Prepare build dependencies (always needed) - # Note: This may recursively call bootstrap() for build deps, - # which has its own error handling. - self._prepare_build_dependencies( + yield self._prepare_build_dependencies( req=req, resolved_version=resolved_version, sdist_root_dir=sdist_root_dir, @@ -1334,11 +1341,13 @@ def _get_version_from_package_metadata( ctx=self.ctx, parent_dir=source_dir.parent, ) - build_dependencies = self._prepare_build_dependencies( - req=req, - resolved_version=None, - sdist_root_dir=source_dir, - build_env=build_env, + build_dependencies = trampoline( + self._prepare_build_dependencies( + req=req, + resolved_version=None, + sdist_root_dir=source_dir, + build_env=build_env, + ) ) build_env.install(build_dependencies) diff --git a/tests/test_bootstrapper.py b/tests/test_bootstrapper.py index 4144d418..257610f1 100644 --- a/tests/test_bootstrapper.py +++ b/tests/test_bootstrapper.py @@ -1,5 +1,6 @@ import json import pathlib +import typing from unittest.mock import Mock, patch import pytest @@ -8,6 +9,7 @@ from packaging.utils import canonicalize_name from packaging.version import Version from resolvelib.resolvers import ResolverException +from trampoline import trampoline from fromager import bootstrapper, requirements_file from fromager.context import WorkContext @@ -270,21 +272,29 @@ def test_build_from_source_returns_dataclass(tmp_context: WorkContext) -> None: mock_wheel = tmp_context.work_dir / "package-1.0.0-py3-none-any.whl" expected_unpack_dir = mock_sdist_root.parent + def mock_prepare_gen( + *args: typing.Any, **kwargs: typing.Any + ) -> typing.Generator[typing.Any, typing.Any, set[Requirement]]: + return set() + yield + with ( patch("fromager.sources.download_source", return_value=mock_source_file), patch("fromager.sources.prepare_source", return_value=mock_sdist_root), patch("fromager.sources.get_source_type", return_value=SourceType.SDIST), - patch.object(bt, "_prepare_build_dependencies"), + patch.object(bt, "_prepare_build_dependencies", side_effect=mock_prepare_gen), patch.object(bt, "_build_wheel", return_value=(mock_wheel, None)), ): - result = bt._build_from_source( - 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, + result = trampoline( + bt._build_from_source( + 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, + ) ) # Verify return type is SourceBuildResult @@ -323,12 +333,14 @@ def mock_bootstrap_impl( source_url: str, resolved_version: Version, build_sdist_only: bool, - ) -> None: + ) -> typing.Generator[typing.Any, typing.Any, None]: call_count["count"] += 1 if str(resolved_version) == "1.5": raise ValueError("Simulated failure for version 1.5") # For other versions, just mark as seen to avoid actual build bt._mark_as_seen(req, resolved_version, build_sdist_only) + return + yield with patch.object(bt, "_bootstrap_impl", side_effect=mock_bootstrap_impl): # Mock _has_been_seen to return False so we attempt bootstrap