Skip to content

refactor(bootstrapper): replace _handle_test_mode_failure with phase re-entry #894

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
LalatenduMohanty wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

refactor(bootstrapper): replace _handle_test_mode_failure with phase re-entry #894

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
137 changes: 53 additions & 84 deletions src/fromager/bootstrapper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ class WorkItem:
cached_wheel_filename: pathlib.Path | None = None
build_result: SourceBuildResult | None = None
pbi_pre_built: bool = False
is_test_mode_fallback: bool = False
build_system_deps: set[Requirement] = dataclasses.field(default_factory=set)
build_backend_deps: set[Requirement] = dataclasses.field(default_factory=set)
build_sdist_deps: set[Requirement] = dataclasses.field(default_factory=set)
Expand Down Expand Up @@ -789,82 +790,6 @@ def _do_build(
)
return self._build_wheel(req, resolved_version, sdist_root_dir, build_env)

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.

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.

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,
)

try:
parent_req = self.why[-1][1] if self.why else None
results = self._resolver.resolve(
req=req,
req_type=req_type,
parent_req=parent_req,
pre_built=True, # Force prebuilt for test mode fallback
)
wheel_url, fallback_version = results[0]

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

return SourceBuildResult(
wheel_filename=wheel_filename,
sdist_filename=None,
unpack_dir=unpack_dir,
sdist_root_dir=None,
build_env=None,
source_type=SourceType.PREBUILT,
)

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

def _look_for_existing_wheel(
self,
req: Requirement,
Expand Down Expand Up @@ -1464,6 +1389,12 @@ def _phase_prepare_source(self, item: WorkItem) -> list[WorkItem]:
resolved_version=item.resolved_version,
wheel_url=item.source_url,
)
if item.is_test_mode_fallback:
logger.info(
"test mode: successfully used pre-built wheel for %s==%s",
item.req.name,
item.resolved_version,
)
item.build_result = SourceBuildResult(
wheel_filename=wheel_filename,
sdist_filename=None,
Expand Down Expand Up @@ -1754,18 +1685,56 @@ def _handle_phase_error(
BootstrapPhase.BUILD,
)
and not item.pbi_pre_built
and not item.is_test_mode_fallback
):
assert item.resolved_version is not None
fallback = self._handle_test_mode_failure(
req=item.req,
resolved_version=item.resolved_version,
req_type=item.req_type,
build_error=err,
logger.warning(
"test mode: build failed for %s==%s, attempting pre-built fallback: %s",
item.req.name,
item.resolved_version,
err,
)
if fallback is not None:
item.build_result = fallback
item.phase = BootstrapPhase.PROCESS_INSTALL_DEPS
try:
# why[-1] is the current item (pushed by _track_why),
# so the real parent is why[-2].
parent_req = self.why[-2][1] if len(self.why) > 1 else None
results = self._resolver.resolve(
req=item.req,
req_type=item.req_type,
parent_req=parent_req,
pre_built=True,
)
wheel_url, fallback_version = results[0]
if fallback_version != item.resolved_version:
logger.warning(
"test mode: version mismatch for %s"
" - requested %s, fallback %s",
item.req.name,
item.resolved_version,
fallback_version,
)
item.source_url = wheel_url
item.resolved_version = fallback_version
item.pbi_pre_built = True
item.is_test_mode_fallback = True
item.phase = BootstrapPhase.PREPARE_SOURCE
item.build_env = None
item.sdist_root_dir = None
item.unpack_dir = None
item.cached_wheel_filename = None
item.build_result = None
return [item]
except Exception as fallback_error:
logger.error(
"test mode: pre-built fallback also failed for %s: %s",
item.req.name,
fallback_error,
exc_info=True,
)
self._record_test_mode_failure(
item.req, str(item.resolved_version), err, "bootstrap"
)
return []
self._record_test_mode_failure(
item.req, str(item.resolved_version), err, "bootstrap"
)
Expand Down
39 changes: 34 additions & 5 deletions tests/test_bootstrapper_iterative.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -543,30 +543,43 @@ def test_resolve_error_in_multiple_versions_mode_raises(
def test_build_phase_test_mode_fallback_success(
self, tmp_context: WorkContext
) -> None:
"""Fallback re-enters PREPARE_SOURCE as prebuilt."""
bt = bootstrapper.Bootstrapper(tmp_context, test_mode=True)
item = _make_build_item(phase=BootstrapPhase.PREPARE_SOURCE)
item.pbi_pre_built = False
err = RuntimeError("build failed")

mock_fallback = Mock(spec=SourceBuildResult)
with patch.object(bt, "_handle_test_mode_failure", return_value=mock_fallback):
fallback_url = "https://pypi.org/testpkg-1.0-py3-none-any.whl"
with patch.object(
bt._resolver,
"resolve",
return_value=[(fallback_url, Version("1.0"))],
):
result = bt._handle_phase_error(item, err)

assert len(result) == 1
assert result[0] is item
assert item.build_result is mock_fallback
assert item.phase == BootstrapPhase.PROCESS_INSTALL_DEPS
assert item.phase == BootstrapPhase.PREPARE_SOURCE
assert item.pbi_pre_built is True
assert item.is_test_mode_fallback is True
assert item.source_url == fallback_url
assert item.build_result is None
assert len(bt.failed_packages) == 0

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

with patch.object(bt, "_handle_test_mode_failure", return_value=None):
with patch.object(
bt._resolver,
"resolve",
side_effect=RuntimeError("no prebuilt available"),
):
result = bt._handle_phase_error(item, err)

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

def test_build_phase_test_mode_fallback_item_skips_second_fallback(
self, tmp_context: WorkContext
) -> None:
"""A fallback item that fails does not attempt another fallback."""
bt = bootstrapper.Bootstrapper(tmp_context, test_mode=True)
item = _make_build_item(phase=BootstrapPhase.PREPARE_SOURCE)
item.pbi_pre_built = False
item.is_test_mode_fallback = True
err = RuntimeError("fallback download failed")

result = bt._handle_phase_error(item, err)

assert result == []
assert len(bt.failed_packages) == 1
assert bt.failed_packages[0]["failure_type"] == "bootstrap"

def test_non_build_phase_test_mode_records_failure(
self, tmp_context: WorkContext
) -> None:
Expand Down
Loading