Skip to content

Commit 0e53faf

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 0e53faf

2 files changed

Lines changed: 95 additions & 145 deletions

File tree

‎src/fromager/bootstrapper.py‎

Lines changed: 94 additions & 143 deletions
Original file line numberDiff line numberDiff line change
@@ -277,12 +277,55 @@ 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+
# wheel_url passed as source_url; force_prebuilt ensures it's
304+
# treated as a wheel URL, not an sdist URL
305+
self._bootstrap_impl(
306+
req,
307+
req_type,
308+
wheel_url,
309+
fallback_version,
310+
build_sdist_only,
311+
force_prebuilt=True,
312+
)
313+
logger.info(
314+
"test mode: successfully used pre-built wheel for %s==%s",
315+
req.name,
316+
fallback_version,
317+
)
318+
except Exception as fallback_error:
319+
logger.error(
320+
"test mode: pre-built fallback also failed for %s: %s",
321+
req.name,
322+
fallback_error,
323+
exc_info=True,
324+
)
325+
# Record the original build error, not the fallback error
326+
self._record_test_mode_failure(
327+
req, str(resolved_version), build_error, "bootstrap"
328+
)
286329

287330
def _bootstrap_impl(
288331
self,
@@ -291,6 +334,7 @@ def _bootstrap_impl(
291334
source_url: str,
292335
resolved_version: Version,
293336
build_sdist_only: bool,
337+
force_prebuilt: bool = False,
294338
) -> None:
295339
"""Internal implementation - performs the actual bootstrap work.
296340
@@ -299,9 +343,11 @@ def _bootstrap_impl(
299343
Args:
300344
req: The requirement to bootstrap.
301345
req_type: The type of requirement.
302-
source_url: The resolved source URL.
346+
source_url: The resolved source URL (sdist or wheel URL).
303347
resolved_version: The resolved version.
304348
build_sdist_only: Whether to build only sdist (no wheel).
349+
force_prebuilt: If True, treat source_url as a wheel URL and skip
350+
source build. Used for test-mode fallback after build failure.
305351
306352
Error Handling:
307353
Fatal errors (source build, prebuilt download) raise exceptions
@@ -322,7 +368,7 @@ def _bootstrap_impl(
322368
cached_wheel_filename: pathlib.Path | None = None
323369
unpacked_cached_wheel: pathlib.Path | None = None
324370

325-
if pbi.pre_built:
371+
if pbi.pre_built or force_prebuilt:
326372
wheel_filename, unpack_dir = self._download_prebuilt(
327373
req=req,
328374
req_type=req_type,
@@ -343,12 +389,11 @@ def _bootstrap_impl(
343389
req, resolved_version
344390
)
345391

346-
# Build from source (handles test-mode fallback internally)
392+
# Build from source
347393
build_result = self._build_from_source(
348394
req=req,
349395
resolved_version=resolved_version,
350396
source_url=source_url,
351-
req_type=req_type,
352397
build_sdist_only=build_sdist_only,
353398
cached_wheel_filename=cached_wheel_filename,
354399
unpacked_cached_wheel=unpacked_cached_wheel,
@@ -779,166 +824,72 @@ def _build_from_source(
779824
req: Requirement,
780825
resolved_version: Version,
781826
source_url: str,
782-
req_type: RequirementType,
783827
build_sdist_only: bool,
784828
cached_wheel_filename: pathlib.Path | None,
785829
unpacked_cached_wheel: pathlib.Path | None,
786830
) -> SourceBuildResult:
787831
"""Build package from source.
788832
789833
Orchestrates download, preparation, build environment setup, and build.
790-
In test mode, attempts pre-built fallback on failure.
791834
792835
Raises:
793-
Exception: In normal mode, if build fails.
794-
In test mode, only if build fails AND fallback also fails.
836+
Exception: If any step fails. In test mode, bootstrap() handles
837+
fallback to pre-built wheels.
795838
"""
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(
839+
# Download and prepare source (if no cached wheel)
840+
if not unpacked_cached_wheel:
841+
logger.debug("no cached wheel, downloading sources")
842+
source_filename = self._download_source(
823843
req=req,
824844
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,
845+
source_url=source_url,
852846
)
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(
847+
sdist_root_dir = self._prepare_source(
860848
req=req,
861849
resolved_version=resolved_version,
862-
req_type=req_type,
863-
build_error=build_error,
850+
source_filename=source_filename,
864851
)
865-
if fallback_result is None:
866-
# Fallback failed, re-raise for bootstrap() to catch
867-
raise
868-
869-
return fallback_result
852+
else:
853+
logger.debug(f"have cached wheel in {unpacked_cached_wheel}")
854+
sdist_root_dir = unpacked_cached_wheel / unpacked_cached_wheel.stem
870855

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.
856+
assert sdist_root_dir is not None
879857

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.
858+
if sdist_root_dir.parent.parent != self.ctx.work_dir:
859+
raise ValueError(f"'{sdist_root_dir}/../..' should be {self.ctx.work_dir}")
860+
unpack_dir = sdist_root_dir.parent
885861

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,
862+
build_env = self._create_build_env(
863+
req=req,
864+
resolved_version=resolved_version,
865+
parent_dir=sdist_root_dir.parent,
894866
)
895867

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

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
873+
# Build wheel or sdist
874+
wheel_filename, sdist_filename = self._do_build(
875+
req=req,
876+
resolved_version=resolved_version,
877+
sdist_root_dir=sdist_root_dir,
878+
build_env=build_env,
879+
build_sdist_only=build_sdist_only,
880+
cached_wheel_filename=cached_wheel_filename,
881+
)
923882

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-
)
883+
source_type = sources.get_source_type(self.ctx, req)
932884

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
885+
return SourceBuildResult(
886+
wheel_filename=wheel_filename,
887+
sdist_filename=sdist_filename,
888+
unpack_dir=unpack_dir,
889+
sdist_root_dir=sdist_root_dir,
890+
build_env=build_env,
891+
source_type=source_type,
892+
)
942893

943894
def _look_for_existing_wheel(
944895
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)