Skip to content

Commit d0e0ff9

Browse files
refactor(bootstrapper): replace _handle_test_mode_failure with phase re-entry
Eliminate `_handle_test_mode_failure` by reusing the existing prebuilt download path in `_phase_prepare_source`. When a source build fails in test mode, `_handle_phase_error` now re-resolves with `pre_built=True` and resets the `WorkItem` to `PREPARE_SOURCE` with `pbi_pre_built=True`, letting the normal phase dispatch handle the download. Add `is_test_mode_fallback` flag to `WorkItem` to prevent infinite fallback loops. Fix parent_req lookup in fallback resolution: use `self.why[-2]` instead of `self.why[-1]` since `_track_why` has already pushed the current item onto the why stack. Closes: #892 Co-Authored-By: Claude <claude@anthropic.com> Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
1 parent 38c0758 commit d0e0ff9

2 files changed

Lines changed: 87 additions & 89 deletions

File tree

src/fromager/bootstrapper.py

Lines changed: 53 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ class WorkItem:
139139
cached_wheel_filename: pathlib.Path | None = None
140140
build_result: SourceBuildResult | None = None
141141
pbi_pre_built: bool = False
142+
is_test_mode_fallback: bool = False
142143
build_system_deps: set[Requirement] = dataclasses.field(default_factory=set)
143144
build_backend_deps: set[Requirement] = dataclasses.field(default_factory=set)
144145
build_sdist_deps: set[Requirement] = dataclasses.field(default_factory=set)
@@ -789,82 +790,6 @@ def _do_build(
789790
)
790791
return self._build_wheel(req, resolved_version, sdist_root_dir, build_env)
791792

792-
def _handle_test_mode_failure(
793-
self,
794-
req: Requirement,
795-
resolved_version: Version,
796-
req_type: RequirementType,
797-
build_error: Exception,
798-
) -> SourceBuildResult | None:
799-
"""Handle build failure in test mode by attempting pre-built fallback.
800-
801-
Args:
802-
req: The requirement that failed to build.
803-
resolved_version: The version that was attempted.
804-
req_type: The type of requirement (for fallback resolution).
805-
build_error: The original exception from the build attempt.
806-
807-
Returns:
808-
SourceBuildResult if fallback succeeded, None if fallback also failed.
809-
"""
810-
logger.warning(
811-
"test mode: build failed for %s==%s, attempting pre-built fallback: %s",
812-
req.name,
813-
resolved_version,
814-
build_error,
815-
)
816-
817-
try:
818-
parent_req = self.why[-1][1] if self.why else None
819-
results = self._resolver.resolve(
820-
req=req,
821-
req_type=req_type,
822-
parent_req=parent_req,
823-
pre_built=True, # Force prebuilt for test mode fallback
824-
)
825-
wheel_url, fallback_version = results[0]
826-
827-
if fallback_version != resolved_version:
828-
logger.warning(
829-
"test mode: version mismatch for %s - requested %s, fallback %s",
830-
req.name,
831-
resolved_version,
832-
fallback_version,
833-
)
834-
835-
wheel_filename, unpack_dir = self._download_prebuilt(
836-
req=req,
837-
req_type=req_type,
838-
resolved_version=fallback_version,
839-
wheel_url=wheel_url,
840-
)
841-
842-
logger.info(
843-
"test mode: successfully used pre-built wheel for %s==%s",
844-
req.name,
845-
fallback_version,
846-
)
847-
# Package succeeded via fallback - no failure to record
848-
849-
return SourceBuildResult(
850-
wheel_filename=wheel_filename,
851-
sdist_filename=None,
852-
unpack_dir=unpack_dir,
853-
sdist_root_dir=None,
854-
build_env=None,
855-
source_type=SourceType.PREBUILT,
856-
)
857-
858-
except Exception as fallback_error:
859-
logger.error(
860-
"test mode: pre-built fallback also failed for %s: %s",
861-
req.name,
862-
fallback_error,
863-
exc_info=True,
864-
)
865-
# Return None to signal failure; bootstrap() will record via re-raised exception
866-
return None
867-
868793
def _look_for_existing_wheel(
869794
self,
870795
req: Requirement,
@@ -1464,6 +1389,12 @@ def _phase_prepare_source(self, item: WorkItem) -> list[WorkItem]:
14641389
resolved_version=item.resolved_version,
14651390
wheel_url=item.source_url,
14661391
)
1392+
if item.is_test_mode_fallback:
1393+
logger.info(
1394+
"test mode: successfully used pre-built wheel for %s==%s",
1395+
item.req.name,
1396+
item.resolved_version,
1397+
)
14671398
item.build_result = SourceBuildResult(
14681399
wheel_filename=wheel_filename,
14691400
sdist_filename=None,
@@ -1754,18 +1685,56 @@ def _handle_phase_error(
17541685
BootstrapPhase.BUILD,
17551686
)
17561687
and not item.pbi_pre_built
1688+
and not item.is_test_mode_fallback
17571689
):
17581690
assert item.resolved_version is not None
1759-
fallback = self._handle_test_mode_failure(
1760-
req=item.req,
1761-
resolved_version=item.resolved_version,
1762-
req_type=item.req_type,
1763-
build_error=err,
1691+
logger.warning(
1692+
"test mode: build failed for %s==%s, attempting pre-built fallback: %s",
1693+
item.req.name,
1694+
item.resolved_version,
1695+
err,
17641696
)
1765-
if fallback is not None:
1766-
item.build_result = fallback
1767-
item.phase = BootstrapPhase.PROCESS_INSTALL_DEPS
1697+
try:
1698+
# why[-1] is the current item (pushed by _track_why),
1699+
# so the real parent is why[-2].
1700+
parent_req = self.why[-2][1] if len(self.why) > 1 else None
1701+
results = self._resolver.resolve(
1702+
req=item.req,
1703+
req_type=item.req_type,
1704+
parent_req=parent_req,
1705+
pre_built=True,
1706+
)
1707+
wheel_url, fallback_version = results[0]
1708+
if fallback_version != item.resolved_version:
1709+
logger.warning(
1710+
"test mode: version mismatch for %s"
1711+
" - requested %s, fallback %s",
1712+
item.req.name,
1713+
item.resolved_version,
1714+
fallback_version,
1715+
)
1716+
item.source_url = wheel_url
1717+
item.resolved_version = fallback_version
1718+
item.pbi_pre_built = True
1719+
item.is_test_mode_fallback = True
1720+
item.phase = BootstrapPhase.PREPARE_SOURCE
1721+
item.build_env = None
1722+
item.sdist_root_dir = None
1723+
item.unpack_dir = None
1724+
item.cached_wheel_filename = None
1725+
item.build_result = None
17681726
return [item]
1727+
except Exception as fallback_error:
1728+
logger.error(
1729+
"test mode: pre-built fallback also failed for %s: %s",
1730+
item.req.name,
1731+
fallback_error,
1732+
exc_info=True,
1733+
)
1734+
self._record_test_mode_failure(
1735+
item.req, str(item.resolved_version), err, "bootstrap"
1736+
)
1737+
return []
17691738
self._record_test_mode_failure(
17701739
item.req, str(item.resolved_version), err, "bootstrap"
17711740
)

tests/test_bootstrapper_iterative.py

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -543,30 +543,43 @@ def test_resolve_error_in_multiple_versions_mode_raises(
543543
def test_build_phase_test_mode_fallback_success(
544544
self, tmp_context: WorkContext
545545
) -> None:
546+
"""Fallback re-enters PREPARE_SOURCE as prebuilt."""
546547
bt = bootstrapper.Bootstrapper(tmp_context, test_mode=True)
547548
item = _make_build_item(phase=BootstrapPhase.PREPARE_SOURCE)
548549
item.pbi_pre_built = False
549550
err = RuntimeError("build failed")
550551

551-
mock_fallback = Mock(spec=SourceBuildResult)
552-
with patch.object(bt, "_handle_test_mode_failure", return_value=mock_fallback):
552+
fallback_url = "https://pypi.org/testpkg-1.0-py3-none-any.whl"
553+
with patch.object(
554+
bt._resolver,
555+
"resolve",
556+
return_value=[(fallback_url, Version("1.0"))],
557+
):
553558
result = bt._handle_phase_error(item, err)
554559

555560
assert len(result) == 1
556561
assert result[0] is item
557-
assert item.build_result is mock_fallback
558-
assert item.phase == BootstrapPhase.PROCESS_INSTALL_DEPS
562+
assert item.phase == BootstrapPhase.PREPARE_SOURCE
563+
assert item.pbi_pre_built is True
564+
assert item.is_test_mode_fallback is True
565+
assert item.source_url == fallback_url
566+
assert item.build_result is None
559567
assert len(bt.failed_packages) == 0
560568

561569
def test_build_phase_test_mode_fallback_failure(
562570
self, tmp_context: WorkContext
563571
) -> None:
572+
"""When prebuilt resolution fails, build failure is recorded and item is skipped."""
564573
bt = bootstrapper.Bootstrapper(tmp_context, test_mode=True)
565574
item = _make_build_item(phase=BootstrapPhase.BUILD)
566575
item.pbi_pre_built = False
567576
err = RuntimeError("build failed")
568577

569-
with patch.object(bt, "_handle_test_mode_failure", return_value=None):
578+
with patch.object(
579+
bt._resolver,
580+
"resolve",
581+
side_effect=RuntimeError("no prebuilt available"),
582+
):
570583
result = bt._handle_phase_error(item, err)
571584

572585
assert result == []
@@ -587,6 +600,22 @@ def test_build_phase_test_mode_prebuilt_skips_fallback(
587600
assert len(bt.failed_packages) == 1
588601
assert bt.failed_packages[0]["failure_type"] == "bootstrap"
589602

603+
def test_build_phase_test_mode_fallback_item_skips_second_fallback(
604+
self, tmp_context: WorkContext
605+
) -> None:
606+
"""A fallback item that fails does not attempt another fallback."""
607+
bt = bootstrapper.Bootstrapper(tmp_context, test_mode=True)
608+
item = _make_build_item(phase=BootstrapPhase.PREPARE_SOURCE)
609+
item.pbi_pre_built = False
610+
item.is_test_mode_fallback = True
611+
err = RuntimeError("fallback download failed")
612+
613+
result = bt._handle_phase_error(item, err)
614+
615+
assert result == []
616+
assert len(bt.failed_packages) == 1
617+
assert bt.failed_packages[0]["failure_type"] == "bootstrap"
618+
590619
def test_non_build_phase_test_mode_records_failure(
591620
self, tmp_context: WorkContext
592621
) -> None:

0 commit comments

Comments
 (0)