Skip to content

Test mode post merge cleanups #893

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

Merged
mergify merged 3 commits into from
Dec 19, 2025
Merged

Test mode post merge cleanups #893

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8e56380
refactor(tests): use existing tmp_context fixture in test-mode tests
LalatenduMohanty Dec 19, 2025
400544a
refactor(bootstrapper): move resolution and tracking to bootstrap()
LalatenduMohanty Dec 19, 2025
5bb0b55
refactor(bootstrapper): move setup logic from _bootstrap_impl to boot…
LalatenduMohanty Dec 19, 2025
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
286 changes: 155 additions & 131 deletions src/fromager/bootstrapper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,59 +224,38 @@ 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.

Handles setup, validation, and error handling. Delegates actual build
work to _bootstrap_impl().

In test mode, catches build exceptions, records package name, and continues.
In normal mode, raises exceptions immediately (fail-fast).
"""
logger.info(f"bootstrapping {req} as {req_type} dependency of {self.why[-1:]}")
Copy link
Member

@dhellmann dhellmann Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The resolver logic needs to be inside the try/except, since it is still a common source of failures.

Copy link
Member Author

@LalatenduMohanty LalatenduMohanty Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good suggestion. Will fix it.

Copy link
Member Author

@LalatenduMohanty LalatenduMohanty Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed it in 400544a


# Resolve version first so we have it for error reporting.
# In test mode, record resolution failures and continue.
try:
self._bootstrap_impl(req, req_type)
source_url, resolved_version = self.resolve_version(
req=req,
req_type=req_type,
)
except Exception as err:
if not self.test_mode:
raise
# Get version from cache if available
cached = self._resolved_requirements.get(str(req))
if cached:
_source_url, resolved_version = cached
version = str(resolved_version)
else:
version = None
self._record_test_mode_failure(req, version, err, "bootstrap")
self._record_test_mode_failure(req, None, err, "resolution")
return

def _bootstrap_impl(self, req: Requirement, req_type: RequirementType) -> None:
"""Internal implementation of bootstrap logic.
# Capture parent before _track_why pushes current package onto the stack
parent: tuple[Requirement, Version] | None = None
if self.why:
_, parent_req, parent_version = self.why[-1]
parent = (parent_req, parent_version)

Error Handling:
Fatal errors (version resolution, source build, prebuilt download)
raise exceptions for bootstrap() to catch and record.
# Update dependency graph unconditionally (before seen check to capture all edges)
self._add_to_graph(req, req_type, resolved_version, source_url, parent)

Non-fatal errors (post-hook, dependency extraction) are recorded
locally and processing continues. These are recorded here because
the package build succeeded - only optional post-processing failed.
"""
logger.info(f"bootstrapping {req} as {req_type} dependency of {self.why[-1:]}")
constraint = self.ctx.constraints.get_constraint(req.name)
if constraint:
logger.info(
f"incoming requirement {req} matches constraint {constraint}. Will apply both."
)

source_url, resolved_version = self.resolve_version(
req=req,
req_type=req_type,
)
pbi = self.ctx.package_build_info(req)

self._add_to_graph(req, req_type, resolved_version, source_url)

# Is bootstrap going to create a wheel or just an sdist?
#
# Use fast sdist-only if flag is set and requirement is not a build
# requirement.
#
# An install requirement on a pre-built wheel treats the wheel as
# sdist-only in order to build its installation requirements sdist-only.
#
# When bootstrap encounters another package with a *build* requirement
# on a pre-built wheel, its installation dependencies are materialized.
# Build sdist-only (no wheel) if flag is set, unless this is a build
# requirement which always needs a full wheel.
build_sdist_only = self.sdist_only and not self._processing_build_requirement(
req_type
)
Expand All @@ -292,109 +271,153 @@ def _bootstrap_impl(self, req: Requirement, req_type: RequirementType) -> None:

logger.info(f"new {req_type} dependency {req} resolves to {resolved_version}")

# Track dependency chain for error messages - context manager ensures cleanup
# Track dependency chain - context manager ensures cleanup even on exception
with self._track_why(req_type, req, resolved_version):
cached_wheel_filename: pathlib.Path | None = None
unpacked_cached_wheel: pathlib.Path | None = None

if pbi.pre_built:
wheel_filename, unpack_dir = self._download_prebuilt(
req=req,
req_type=req_type,
resolved_version=resolved_version,
wheel_url=source_url,
)
build_result = SourceBuildResult(
wheel_filename=wheel_filename,
sdist_filename=None,
unpack_dir=unpack_dir,
sdist_root_dir=None,
build_env=None,
source_type=SourceType.PREBUILT,
)
else:
# Look for an existing wheel in caches before building
cached_wheel_filename, unpacked_cached_wheel = self._find_cached_wheel(
req, resolved_version
)

# Build from source (handles test-mode fallback internally)
build_result = self._build_from_source(
req=req,
resolved_version=resolved_version,
source_url=source_url,
req_type=req_type,
build_sdist_only=build_sdist_only,
cached_wheel_filename=cached_wheel_filename,
unpacked_cached_wheel=unpacked_cached_wheel,
)

# Run post-bootstrap hooks (non-fatal in test mode)
try:
hooks.run_post_bootstrap_hooks(
ctx=self.ctx,
req=req,
dist_name=canonicalize_name(req.name),
dist_version=str(resolved_version),
sdist_filename=build_result.sdist_filename,
wheel_filename=build_result.wheel_filename,
self._bootstrap_impl(
req, req_type, source_url, resolved_version, build_sdist_only
)
except Exception as hook_error:
except Exception as err:
if not self.test_mode:
raise
self._record_test_mode_failure(
req, str(resolved_version), hook_error, "hook", "warning"
req, str(resolved_version), err, "bootstrap"
)

# Extract install dependencies (non-fatal in test mode)
try:
install_dependencies = self._get_install_dependencies(
req=req,
resolved_version=resolved_version,
wheel_filename=build_result.wheel_filename,
sdist_filename=build_result.sdist_filename,
sdist_root_dir=build_result.sdist_root_dir,
build_env=build_result.build_env,
unpack_dir=build_result.unpack_dir,
)
except Exception as dep_error:
if not self.test_mode:
raise
self._record_test_mode_failure(
req,
str(resolved_version),
dep_error,
"dependency_extraction",
"warning",
)
install_dependencies = []
def _bootstrap_impl(
self,
req: Requirement,
req_type: RequirementType,
source_url: str,
resolved_version: Version,
build_sdist_only: bool,
) -> None:
"""Internal implementation - performs the actual bootstrap work.

logger.debug(
"install dependencies: %s",
", ".join(sorted(str(r) for r in install_dependencies)),
Called by bootstrap() after setup, validation, and seen-checking.

Args:
req: The requirement to bootstrap.
req_type: The type of requirement.
source_url: The resolved source URL.
resolved_version: The resolved version.
build_sdist_only: Whether to build only sdist (no wheel).

Error Handling:
Fatal errors (source build, prebuilt download) raise exceptions
for bootstrap() to catch and record.

Non-fatal errors (post-hook, dependency extraction) are recorded
locally and processing continues. These are recorded here because
the package build succeeded - only optional post-processing failed.
"""
constraint = self.ctx.constraints.get_constraint(req.name)
if constraint:
logger.info(
f"incoming requirement {req} matches constraint {constraint}. Will apply both."
)

pbi = self.ctx.package_build_info(req)

cached_wheel_filename: pathlib.Path | None = None
unpacked_cached_wheel: pathlib.Path | None = None

if pbi.pre_built:
wheel_filename, unpack_dir = self._download_prebuilt(
req=req,
req_type=req_type,
resolved_version=resolved_version,
wheel_url=source_url,
)
build_result = SourceBuildResult(
wheel_filename=wheel_filename,
sdist_filename=None,
unpack_dir=unpack_dir,
sdist_root_dir=None,
build_env=None,
source_type=SourceType.PREBUILT,
)
else:
# Look for an existing wheel in caches before building
cached_wheel_filename, unpacked_cached_wheel = self._find_cached_wheel(
req, resolved_version
)

self._add_to_build_order(
# Build from source (handles test-mode fallback internally)
build_result = self._build_from_source(
req=req,
version=resolved_version,
resolved_version=resolved_version,
source_url=source_url,
source_type=build_result.source_type,
prebuilt=pbi.pre_built,
constraint=constraint,
req_type=req_type,
build_sdist_only=build_sdist_only,
cached_wheel_filename=cached_wheel_filename,
unpacked_cached_wheel=unpacked_cached_wheel,
)

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)
self.progressbar.update()

# Clean up build directories (why stack cleanup handled by context manager)
self.ctx.clean_build_dirs(
build_result.sdist_root_dir, build_result.build_env
# Run post-bootstrap hooks (non-fatal in test mode)
try:
hooks.run_post_bootstrap_hooks(
ctx=self.ctx,
req=req,
dist_name=canonicalize_name(req.name),
dist_version=str(resolved_version),
sdist_filename=build_result.sdist_filename,
wheel_filename=build_result.wheel_filename,
)
except Exception as hook_error:
if not self.test_mode:
raise
self._record_test_mode_failure(
req, str(resolved_version), hook_error, "hook", "warning"
)

# Extract install dependencies (non-fatal in test mode)
try:
install_dependencies = self._get_install_dependencies(
req=req,
resolved_version=resolved_version,
wheel_filename=build_result.wheel_filename,
sdist_filename=build_result.sdist_filename,
sdist_root_dir=build_result.sdist_root_dir,
build_env=build_result.build_env,
unpack_dir=build_result.unpack_dir,
)
except Exception as dep_error:
if not self.test_mode:
raise
self._record_test_mode_failure(
req,
str(resolved_version),
dep_error,
"dependency_extraction",
"warning",
)
install_dependencies = []

logger.debug(
"install dependencies: %s",
", ".join(sorted(str(r) for r in install_dependencies)),
)

self._add_to_build_order(
req=req,
version=resolved_version,
source_url=source_url,
source_type=build_result.source_type,
prebuilt=pbi.pre_built,
constraint=constraint,
)

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)
self.progressbar.update()

# Clean up build directories
self.ctx.clean_build_dirs(build_result.sdist_root_dir, build_result.build_env)

@contextlib.contextmanager
def _track_why(
Expand Down Expand Up @@ -1340,11 +1363,12 @@ def _add_to_graph(
req_type: RequirementType,
req_version: Version,
download_url: str,
parent: tuple[Requirement, Version] | None,
) -> None:
if req_type == RequirementType.TOP_LEVEL:
return

_, parent_req, parent_version = self.why[-1] if self.why else (None, None, None)
parent_req, parent_version = parent if parent else (None, None)
pbi = self.ctx.package_build_info(req)
# Update the dependency graph after we determine that this requirement is
# useful but before we determine if it is redundant so that we capture all
Expand Down
Loading
Loading