Skip to content

feat(bootstrap): parallelize RESOLVE and PREPARE_SOURCE I/O with background threads #1199

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

Open
dhellmann wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(bootstrap): parallelize RESOLVE and PREPARE_SOURCE I/O with background threads #1199

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
4 changes: 2 additions & 2 deletions e2e/test_bootstrap_build_tags.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ fromager \
--settings-file="$SCRIPTDIR/bootstrap_settings.yaml" \
bootstrap --cache-wheel-server-url=$WHEEL_SERVER_URL 'stevedore==5.2.0'

if ! grep -q "stevedore: found built wheel on cache server" "$LOGFILE"; then
if ! grep -q "stevedore-5.2.0: found built wheel on cache server" "$LOGFILE"; then
echo "FAIL: Did not find log message found built wheel on cache server in $LOGFILE" 1>&2
pass=false
fi
Expand All @@ -77,7 +77,7 @@ fromager \
--settings-file="$SCRIPTDIR/bootstrap_settings.yaml" \
bootstrap --cache-wheel-server-url=$WHEEL_SERVER_URL 'stevedore==5.2.0'

if ! grep -q "stevedore: found existing wheel " "$LOGFILE"; then
if ! grep -q "stevedore-5.2.0: found existing wheel " "$LOGFILE"; then
echo "FAIL: Did not find log message found existing wheel in $LOGFILE" 1>&2
pass=false
fi
Expand Down
12 changes: 6 additions & 6 deletions e2e/test_bootstrap_cache.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ fromager \
bootstrap --cache-wheel-server-url="https://pypi.org/simple" "$DIST==$VER"

EXPECTED_LOG_MESSAGES=(
"$DIST: looking for existing wheel for version $VER with build tag () in"
"$DIST: found existing wheel"
"$DIST-$VER: looking for existing wheel for version $VER with build tag () in"
"$DIST-$VER: found existing wheel"
Comment on lines +45 to +46

@coderabbitai coderabbitai Bot Jun 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use fixed-string grep for versioned log assertions.

These patterns now include dotted versions (for example 78.1.0), and regex grep -q can match unintended strings. Switch assertion checks to fixed-string mode to avoid false positives.

Suggested patch 
-for pattern in "${EXPECTED_LOG_MESSAGES[@]}"; do
-  if ! grep -q "$pattern" "$OUTDIR/bootstrap.log"; then
+for pattern in "${EXPECTED_LOG_MESSAGES[@]}"; do
+  if ! grep -Fq "$pattern" "$OUTDIR/bootstrap.log"; then
@@
-for pattern in "${UNEXPECTED_LOG_MESSAGES[@]}"; do
-  if grep -q "$pattern" "$OUTDIR/bootstrap.log"; then
+for pattern in "${UNEXPECTED_LOG_MESSAGES[@]}"; do
+  if grep -Fq "$pattern" "$OUTDIR/bootstrap.log"; then
@@
-for pattern in "${UNEXPECTED_LOG_MESSAGES[@]}"; do
+for pattern in "${UNEXPECTED_LOG_MESSAGES[@]}"; do
   echo $pattern
-  if grep -q "$pattern" "$OUTDIR/bootstrap.log"; then
+  if grep -Fq "$pattern" "$OUTDIR/bootstrap.log"; then

Also applies to: 57-57, 108-110

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@e2e/test_bootstrap_cache.sh` around lines 45 - 46, The grep assertions in the
bootstrap cache test script use regex mode which can match unintended strings
when patterns contain dotted versions like "78.1.0" where dots are interpreted
as wildcards. Switch all grep commands checking log assertions (at lines 45-46,
57, and 108-110) from standard grep mode to fixed-string mode by adding the -F
flag to each grep -q invocation, so that version strings with dots are matched
literally rather than as regex patterns.

)
for pattern in "${EXPECTED_LOG_MESSAGES[@]}"; do
if ! grep -q "$pattern" "$OUTDIR/bootstrap.log"; then
Expand All @@ -54,7 +54,7 @@ done
$pass

UNEXPECTED_LOG_MESSAGES=(
"$DIST: checking if wheel was already uploaded to https://pypi.org/simple"
"$DIST-$VER: checking if wheel was already uploaded to https://pypi.org/simple"
)

for pattern in "${UNEXPECTED_LOG_MESSAGES[@]}"; do
Expand Down Expand Up @@ -105,9 +105,9 @@ done
$pass

UNEXPECTED_LOG_MESSAGES=(
"$DIST: loading build sdist dependencies from build-sdist-requirements.txt"
"$DIST: loading build backend dependencies from build-backend-requirements.txt"
"$DIST: loading build system dependencies from build-system-requirements.txt"
"$DIST-$VER: loading build sdist dependencies from build-sdist-requirements.txt"
"$DIST-$VER: loading build backend dependencies from build-backend-requirements.txt"
"$DIST-$VER: loading build system dependencies from build-system-requirements.txt"
)

for pattern in "${UNEXPECTED_LOG_MESSAGES[@]}"; do
Expand Down
2 changes: 1 addition & 1 deletion e2e/test_bootstrap_cooldown_prebuilt.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ if ! grep -q "new toplevel dependency stevedore resolves to 5.3.0" "$OUTDIR/boot
fi

# The wheel must have been downloaded as a pre-built (not built from source).
if ! grep -q "uses a pre-built wheel" "$OUTDIR/bootstrap.log"; then
if ! grep -q "using pre-built wheel" "$OUTDIR/bootstrap.log"; then
echo "FAIL: stevedore was not downloaded as a pre-built wheel" 1>&2
pass=false
fi
Expand Down
4 changes: 2 additions & 2 deletions e2e/test_bootstrap_prerelease.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ fromager \
pass=true

# Check for log message that the override is loaded
if ! grep -q "flit_core: new toplevel dependency flit_core<2.0.1 resolves to 2.0" "$OUTDIR/bootstrap.log"; then
if ! grep -q "flit_core-2.0: new toplevel dependency flit_core<2.0.1 resolves to 2.0" "$OUTDIR/bootstrap.log"; then

@coderabbitai coderabbitai Bot Jun 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Make these grep checks fixed-string matches.

Line 23 and Line 44 include dotted versions in the search text; regex grep -q can match unintended variants. Use grep -Fq for deterministic assertions.

Suggested patch 
-if ! grep -q "flit_core-2.0: new toplevel dependency flit_core<2.0.1 resolves to 2.0" "$OUTDIR/bootstrap.log"; then
+if ! grep -Fq "flit_core-2.0: new toplevel dependency flit_core<2.0.1 resolves to 2.0" "$OUTDIR/bootstrap.log"; then
@@
-if ! grep -q "flit_core-2.0rc3: new toplevel dependency flit_core<2.0.1 resolves to 2.0rc3" "$OUTDIR/bootstrap.log"; then
+if ! grep -Fq "flit_core-2.0rc3: new toplevel dependency flit_core<2.0.1 resolves to 2.0rc3" "$OUTDIR/bootstrap.log"; then

Also applies to: 44-44

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@e2e/test_bootstrap_prerelease.sh` at line 23, The grep command uses regex
pattern matching which treats dots as metacharacters that match any character,
causing unintended matches in version strings. Replace `grep -q` with `grep -Fq`
on both occurrences (the line checking for "flit_core-2.0: new toplevel
dependency flit_core<2.0.1 resolves to 2.0" and the other similar check at line
44) to use fixed-string literal matching instead, ensuring the grep assertions
match only the exact expected version strings.

echo "FAIL: flit_core did not resolve to 2.0 $OUTDIR/bootstrap.log" 1>&2
pass=false
fi
Expand All @@ -41,7 +41,7 @@ DEBUG_RESOLVER=true fromager \


# Check for log message that the override is loaded
if ! grep -q "flit_core: new toplevel dependency flit_core<2.0.1 resolves to 2.0rc3" "$OUTDIR/bootstrap.log"; then
if ! grep -q "flit_core-2.0rc3: new toplevel dependency flit_core<2.0.1 resolves to 2.0rc3" "$OUTDIR/bootstrap.log"; then
echo "FAIL: flit_core did not resolve to 2.0rc3 $OUTDIR/bootstrap.log" 1>&2
pass=false
fi
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ features = ["docs"]
build = "sphinx-build -M html docs docs/_build -j auto --keep-going {args:--fail-on-warning --fresh-env -n}"

[tool.pytest.ini_options]
testpaths = ["tests"]
markers = [
"network: test that need network access",
]
35 changes: 35 additions & 0 deletions src/fromager/bootstrap_requirement_resolver.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,15 @@ def resolve(
pbi = self.ctx.package_build_info(req)
pre_built = pbi.pre_built

# Check session cache BEFORE the git URL guard so that background
# threads can retrieve pre-cached git URL resolutions (populated by
# Bootstrapper.resolve_versions() on the main thread before bootstrap()
# is called) without hitting the ValueError.
cached_result = self.get_cached_resolution(req, pre_built)
if cached_result is not None:
logger.debug(f"resolved {req} from cache")
return list(cached_result) if return_all_versions else [cached_result[0]]

Comment on lines +121 to +129

@coderabbitai coderabbitai Bot Jun 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

IndexError when cached resolution returns empty list.

get_cached_resolution() returns [] when a rule was previously resolved but no versions match the specifier. Line 128 then does [cached_result[0]] which raises IndexError.

The non-cached path handles this at lines 152-156 with if not matching: return [], but this cached early-return doesn't.

Suggested fix 
         cached_result = self.get_cached_resolution(req, pre_built)
         if cached_result is not None:
             logger.debug(f"resolved {req} from cache")
-            return list(cached_result) if return_all_versions else [cached_result[0]]
+            if return_all_versions:
+                return list(cached_result)
+            return [cached_result[0]] if cached_result else []
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/fromager/bootstrap_requirement_resolver.py` around lines 121 - 129, The
cached resolution path does not handle the case where get_cached_resolution()
returns an empty list, causing an IndexError when trying to access
cached_result[0]. Add a check after retrieving the cached_result to verify it is
not empty before attempting to access its first element. If the cached_result is
empty, return an empty list to match the behavior of the non-cached code path
that handles empty matching results.

# Git URL source resolution must be handled by Bootstrapper.
# But git URL prebuilt resolution is allowed - we look for wheels on PyPI
# (test mode fallback uses this path).
Expand Down Expand Up @@ -146,6 +155,32 @@ def resolve(
return matching
return [matching[0]]

def get_cached_resolution(
self,
req: Requirement,
pre_built: bool,
) -> list[tuple[str, Version]] | None:
"""Return cached matching versions if this requirement was already resolved.

Returns ``None`` if the requirement has not been resolved yet, allowing
callers to distinguish between "no matching versions" and "not yet resolved".

Used by background threads to retrieve pre-cached git URL resolutions
populated by the main thread before entering the parallel section.

Args:
req: Package requirement
pre_built: Whether looking for prebuilt or source resolution

Returns:
List of (url, version) tuples if previously resolved, None otherwise.
"""
rule_key = (str(req), pre_built)
with self._lock:
if rule_key in self._resolved_rules:
return self._get_matching_versions(req, pre_built)
return None

def _resolve_and_extend(
self,
req: Requirement,
Expand Down
Loading
Loading