Skip to content

Commit 7735109

Browse files
LalatenduMohantyLalatenduMohanty
committed
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 <claude@anthropic.com> Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
1 parent c9097d6 commit 7735109

File tree

2 files changed

+93
-145
lines changed

2 files changed

+93
-145
lines changed

‎src/fromager/bootstrapper.py‎

Lines changed: 92 additions & 143 deletions
Original file line numberDiff line numberDiff line change
@@ -277,12 +277,53 @@ def bootstrap(self, req: Requirement, req_type: RequirementType) -> None:
277277
self._bootstrap_impl(
278278
req, req_type, source_url, resolved_version, build_sdist_only
279279
)
280-
except Exception as err:
280+
except Exception as build_error:
281281
if not self.test_mode:
282282
raise
283-
self._record_test_mode_failure(
284-
req, str(resolved_version), err, "bootstrap"
283+
284+
# Test mode: attempt pre-built fallback (may resolve different version)
285+
logger.warning(
286+
"test mode: build failed for %s==%s, attempting pre-built fallback: %s",
287+
req.name,
288+
resolved_version,
289+
build_error,
285290
)
291+
try:
292+
wheel_url, fallback_version = self._resolve_prebuilt_with_history(
293+
req=req,
294+
req_type=req_type,
295+
)
296+
if fallback_version != resolved_version:
297+
logger.warning(
298+
"test mode: version mismatch for %s - requested %s, fallback %s",
299+
req.name,
300+
resolved_version,
301+
fallback_version,
302+
)
303+
self._bootstrap_impl(
304+
req,
305+
req_type,
306+
wheel_url,
307+
fallback_version,
308+
build_sdist_only,
309+
force_prebuilt=True,
310+
)
311+
logger.info(
312+
"test mode: successfully used pre-built wheel for %s==%s",
313+
req.name,
314+
fallback_version,
315+
)
316+
except Exception as fallback_error:
317+
logger.error(
318+
"test mode: pre-built fallback also failed for %s: %s",
319+
req.name,
320+
fallback_error,
321+
exc_info=True,
322+
)
323+
# Record the original build error, not the fallback error
324+
self._record_test_mode_failure(
325+
req, str(resolved_version), build_error, "bootstrap"
326+
)
286327

287328
def _bootstrap_impl(
288329
self,
@@ -291,6 +332,7 @@ def _bootstrap_impl(
291332
source_url: str,
292333
resolved_version: Version,
293334
build_sdist_only: bool,
335+
force_prebuilt: bool = False,
294336
) -> None:
295337
"""Internal implementation - performs the actual bootstrap work.
296338
@@ -299,9 +341,11 @@ def _bootstrap_impl(
299341
Args:
300342
req: The requirement to bootstrap.
301343
req_type: The type of requirement.
302-
source_url: The resolved source URL.
344+
source_url: The resolved source URL (sdist or wheel URL).
303345
resolved_version: The resolved version.
304346
build_sdist_only: Whether to build only sdist (no wheel).
347+
force_prebuilt: If True, treat source_url as a wheel URL and skip
348+
source build. Used for test-mode fallback after build failure.
305349
306350
Error Handling:
307351
Fatal errors (source build, prebuilt download) raise exceptions
@@ -322,7 +366,7 @@ def _bootstrap_impl(
322366
cached_wheel_filename: pathlib.Path | None = None
323367
unpacked_cached_wheel: pathlib.Path | None = None
324368

325-
if pbi.pre_built:
369+
if pbi.pre_built or force_prebuilt:
326370
wheel_filename, unpack_dir = self._download_prebuilt(
327371
req=req,
328372
req_type=req_type,
@@ -343,12 +387,11 @@ def _bootstrap_impl(
343387
req, resolved_version
344388
)
345389

346-
# Build from source (handles test-mode fallback internally)
390+
# Build from source
347391
build_result = self._build_from_source(
348392
req=req,
349393
resolved_version=resolved_version,
350394
source_url=source_url,
351-
req_type=req_type,
352395
build_sdist_only=build_sdist_only,
353396
cached_wheel_filename=cached_wheel_filename,
354397
unpacked_cached_wheel=unpacked_cached_wheel,
@@ -779,166 +822,72 @@ def _build_from_source(
779822
req: Requirement,
780823
resolved_version: Version,
781824
source_url: str,
782-
req_type: RequirementType,
783825
build_sdist_only: bool,
784826
cached_wheel_filename: pathlib.Path | None,
785827
unpacked_cached_wheel: pathlib.Path | None,
786828
) -> SourceBuildResult:
787829
"""Build package from source.
788830
789831
Orchestrates download, preparation, build environment setup, and build.
790-
In test mode, attempts pre-built fallback on failure.
791832
792833
Raises:
793-
Exception: In normal mode, if build fails.
794-
In test mode, only if build fails AND fallback also fails.
834+
Exception: If any step fails. In test mode, bootstrap() handles
835+
fallback to pre-built wheels.
795836
"""
796-
try:
797-
# Download and prepare source (if no cached wheel)
798-
if not unpacked_cached_wheel:
799-
logger.debug("no cached wheel, downloading sources")
800-
source_filename = self._download_source(
801-
req=req,
802-
resolved_version=resolved_version,
803-
source_url=source_url,
804-
)
805-
sdist_root_dir = self._prepare_source(
806-
req=req,
807-
resolved_version=resolved_version,
808-
source_filename=source_filename,
809-
)
810-
else:
811-
logger.debug(f"have cached wheel in {unpacked_cached_wheel}")
812-
sdist_root_dir = unpacked_cached_wheel / unpacked_cached_wheel.stem
813-
814-
assert sdist_root_dir is not None
815-
816-
if sdist_root_dir.parent.parent != self.ctx.work_dir:
817-
raise ValueError(
818-
f"'{sdist_root_dir}/../..' should be {self.ctx.work_dir}"
819-
)
820-
unpack_dir = sdist_root_dir.parent
821-
822-
build_env = self._create_build_env(
837+
# Download and prepare source (if no cached wheel)
838+
if not unpacked_cached_wheel:
839+
logger.debug("no cached wheel, downloading sources")
840+
source_filename = self._download_source(
823841
req=req,
824842
resolved_version=resolved_version,
825-
parent_dir=sdist_root_dir.parent,
826-
)
827-
828-
# Prepare build dependencies (always needed)
829-
# Note: This may recursively call bootstrap() for build deps,
830-
# which has its own error handling.
831-
self._prepare_build_dependencies(req, sdist_root_dir, build_env)
832-
833-
# Build wheel or sdist
834-
wheel_filename, sdist_filename = self._do_build(
835-
req=req,
836-
resolved_version=resolved_version,
837-
sdist_root_dir=sdist_root_dir,
838-
build_env=build_env,
839-
build_sdist_only=build_sdist_only,
840-
cached_wheel_filename=cached_wheel_filename,
841-
)
842-
843-
source_type = sources.get_source_type(self.ctx, req)
844-
845-
return SourceBuildResult(
846-
wheel_filename=wheel_filename,
847-
sdist_filename=sdist_filename,
848-
unpack_dir=unpack_dir,
849-
sdist_root_dir=sdist_root_dir,
850-
build_env=build_env,
851-
source_type=source_type,
843+
source_url=source_url,
852844
)
853-
854-
except Exception as build_error:
855-
if not self.test_mode:
856-
raise
857-
858-
# Test mode: attempt pre-built fallback
859-
fallback_result = self._handle_test_mode_failure(
845+
sdist_root_dir = self._prepare_source(
860846
req=req,
861847
resolved_version=resolved_version,
862-
req_type=req_type,
863-
build_error=build_error,
848+
source_filename=source_filename,
864849
)
865-
if fallback_result is None:
866-
# Fallback failed, re-raise for bootstrap() to catch
867-
raise
868-
869-
return fallback_result
850+
else:
851+
logger.debug(f"have cached wheel in {unpacked_cached_wheel}")
852+
sdist_root_dir = unpacked_cached_wheel / unpacked_cached_wheel.stem
870853

871-
def _handle_test_mode_failure(
872-
self,
873-
req: Requirement,
874-
resolved_version: Version,
875-
req_type: RequirementType,
876-
build_error: Exception,
877-
) -> SourceBuildResult | None:
878-
"""Handle build failure in test mode by attempting pre-built fallback.
854+
assert sdist_root_dir is not None
879855

880-
Args:
881-
req: The requirement that failed to build.
882-
resolved_version: The version that was attempted.
883-
req_type: The type of requirement (for fallback resolution).
884-
build_error: The original exception from the build attempt.
856+
if sdist_root_dir.parent.parent != self.ctx.work_dir:
857+
raise ValueError(f"'{sdist_root_dir}/../..' should be {self.ctx.work_dir}")
858+
unpack_dir = sdist_root_dir.parent
885859

886-
Returns:
887-
SourceBuildResult if fallback succeeded, None if fallback also failed.
888-
"""
889-
logger.warning(
890-
"test mode: build failed for %s==%s, attempting pre-built fallback: %s",
891-
req.name,
892-
resolved_version,
893-
build_error,
860+
build_env = self._create_build_env(
861+
req=req,
862+
resolved_version=resolved_version,
863+
parent_dir=sdist_root_dir.parent,
894864
)
895865

896-
try:
897-
wheel_url, fallback_version = self._resolve_prebuilt_with_history(
898-
req=req,
899-
req_type=req_type,
900-
)
866+
# Prepare build dependencies (always needed)
867+
# Note: This may recursively call bootstrap() for build deps,
868+
# which has its own error handling.
869+
self._prepare_build_dependencies(req, sdist_root_dir, build_env)
901870

902-
if fallback_version != resolved_version:
903-
logger.warning(
904-
"test mode: version mismatch for %s - requested %s, fallback %s",
905-
req.name,
906-
resolved_version,
907-
fallback_version,
908-
)
909-
910-
wheel_filename, unpack_dir = self._download_prebuilt(
911-
req=req,
912-
req_type=req_type,
913-
resolved_version=fallback_version,
914-
wheel_url=wheel_url,
915-
)
916-
917-
logger.info(
918-
"test mode: successfully used pre-built wheel for %s==%s",
919-
req.name,
920-
fallback_version,
921-
)
922-
# Package succeeded via fallback - no failure to record
871+
# Build wheel or sdist
872+
wheel_filename, sdist_filename = self._do_build(
873+
req=req,
874+
resolved_version=resolved_version,
875+
sdist_root_dir=sdist_root_dir,
876+
build_env=build_env,
877+
build_sdist_only=build_sdist_only,
878+
cached_wheel_filename=cached_wheel_filename,
879+
)
923880

924-
return SourceBuildResult(
925-
wheel_filename=wheel_filename,
926-
sdist_filename=None,
927-
unpack_dir=unpack_dir,
928-
sdist_root_dir=None,
929-
build_env=None,
930-
source_type=SourceType.PREBUILT,
931-
)
881+
source_type = sources.get_source_type(self.ctx, req)
932882

933-
except Exception as fallback_error:
934-
logger.error(
935-
"test mode: pre-built fallback also failed for %s: %s",
936-
req.name,
937-
fallback_error,
938-
exc_info=True,
939-
)
940-
# Return None to signal failure; bootstrap() will record via re-raised exception
941-
return None
883+
return SourceBuildResult(
884+
wheel_filename=wheel_filename,
885+
sdist_filename=sdist_filename,
886+
unpack_dir=unpack_dir,
887+
sdist_root_dir=sdist_root_dir,
888+
build_env=build_env,
889+
source_type=source_type,
890+
)
942891

943892
def _look_for_existing_wheel(
944893
self,

‎tests/test_bootstrapper.py‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from packaging.utils import canonicalize_name
77
from packaging.version import Version
88

9-
from fromager import bootstrapper, requirements_file
9+
from fromager import bootstrapper
1010
from fromager.context import WorkContext
1111
from fromager.dependency_graph import DependencyGraph
1212
from fromager.requirements_file import RequirementType, SourceType
@@ -495,7 +495,6 @@ def test_build_from_source_returns_dataclass(tmp_context: WorkContext) -> None:
495495
req=Requirement("test-package"),
496496
resolved_version=Version("1.0.0"),
497497
source_url="https://pypi.org/simple/test-package",
498-
req_type=requirements_file.RequirementType.TOP_LEVEL,
499498
build_sdist_only=False,
500499
cached_wheel_filename=None,
501500
unpacked_cached_wheel=None,

0 commit comments

Comments
 (0)