fix(bootstrap): auto-disable constraints and extend e2e test coverage#1057
Merged
mergify[bot] merged 2 commits intopython-wheel-build:mainfrom Apr 15, 2026
Merged
Conversation
📝 WalkthroughWalkthroughThe bootstrap command now automatically disables constraints generation when Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/test_bootstrap_multiple_versions.sh`:
- Line 13: The trap currently expands $constraints_file when the trap is
registered rather than at exit, and won't handle paths with spaces; change the
trap registration for rm -f $constraints_file to use single quotes around the
command and quote the variable inside (i.e., trap 'rm -f "$constraints_file"'
EXIT) so the variable is expanded at runtime and paths with spaces are handled
safely; update the trap line that references constraints_file accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 45560f03-d9fd-44c4-a6d5-fe08dd291471
📒 Files selected for processing (2)
e2e/test_bootstrap_multiple_versions.shsrc/fromager/commands/bootstrap.py
|
FWIW, LGTM @rd4398 |
LalatenduMohanty
requested changes
Apr 15, 2026
rd4398
force-pushed
the
disable-constraints
branch
from
60202a4 to
102c719
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
e2e/test_bootstrap_multiple_versions.sh (1)
13-13:⚠️ Potential issue | 🟡 MinorFix trap quoting so cleanup runs with runtime-expanded path.
At Line [13],
$constraints_fileis expanded when the trap is set, not when it executes on exit.Proposed fix
-trap "rm -f $constraints_file" EXIT +trap 'rm -f "$constraints_file"' EXIT#!/bin/bash # Verify trap form in current file rg -n '^\s*trap "rm -f \$constraints_file" EXIT$|^\s*trap '\''rm -f "\$constraints_file"'\'' EXIT$' e2e/test_bootstrap_multiple_versions.sh # Demonstrate expansion timing difference bash -lc 'constraints_file=before; trap "echo double:$constraints_file" EXIT; constraints_file=after' bash -lc 'constraints_file=before; trap '\''echo single:$constraints_file'\'' EXIT; constraints_file=after'As per coding guidelines:
e2e/**/*.sh: “Check for proper cleanup and teardown (trap handlers)... Verify variables are quoted to prevent word splitting.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/test_bootstrap_multiple_versions.sh` at line 13, The trap currently expands $constraints_file when registered instead of at exit; update the trap registration for the constraints_file cleanup so the variable is expanded at runtime by using a single-quoted handler that contains a quoted variable reference (i.e., change the trap that references constraints_file to use a single-quoted string with "$constraints_file" so the path is evaluated when the EXIT handler runs, and ensure the variable is properly quoted to avoid word-splitting).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/test_bootstrap_multiple_versions.sh`:
- Around line 71-82: Replace the current file-counting logic for FLIT_CORE_COUNT
with a deduplicated version count: find the matching wheel files (the same
pattern used now), extract the normalized version string from each filename
(e.g., strip the package name and extension and ignore build tags), pipe through
sort -u to deduplicate, and then use wc -l to set FLIT_CORE_COUNT; update the
echo/checks to use this new FLIT_CORE_COUNT. Ensure you update the command
substitution that sets FLIT_CORE_COUNT (the variable referenced in this script)
and the subsequent error message logic so it reports distinct flit-core versions
rather than raw wheel file count.
---
Duplicate comments:
In `@e2e/test_bootstrap_multiple_versions.sh`:
- Line 13: The trap currently expands $constraints_file when registered instead
of at exit; update the trap registration for the constraints_file cleanup so the
variable is expanded at runtime by using a single-quoted handler that contains a
quoted variable reference (i.e., change the trap that references
constraints_file to use a single-quoted string with "$constraints_file" so the
path is evaluated when the EXIT handler runs, and ensure the variable is
properly quoted to avoid word-splitting).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7c54c1ab-2e8b-4732-8ccd-de2b588017ff
📒 Files selected for processing (3)
e2e/test_bootstrap_multiple_versions.shsrc/fromager/commands/bootstrap.pytests/test_bootstrap.py
✅ Files skipped from review due to trivial changes (1)
- tests/test_bootstrap.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/fromager/commands/bootstrap.py
LalatenduMohanty
requested changes
Apr 15, 2026
Two related fixes for --multiple-versions flag:
1. **Auto-disable constraints with --multiple-versions**
- When --multiple-versions flag is enabled, automatically set
skip_constraints=True because constraints.txt cannot handle
multiple versions of the same package
- Log informative message: "automatically disabling constraints
generation (incompatible with --multiple-versions)"
2. **Extend e2e test to verify dependency chain handling**
- Use generous constraint range (flit-core>=3.9,<3.12) instead
of pinning to single version
- Verify that multiple versions of flit-core are built (not just
tomli), confirming --multiple-versions works for entire chain
- Check for at least 2 flit-core versions (test found 3)
- Remove `|| true` since constraints are now auto-disabled
The e2e test now validates that --multiple-versions bootstraps
multiple versions of both top-level packages AND their dependencies,
providing better test coverage.
Fixes: python-wheel-build#1044
Fixes: python-wheel-build#1045
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Rohan Devasthale <rdevasth@redhat.com>
rd4398
force-pushed
the
disable-constraints
branch
from
310aa9b to
7b58557
Compare
LalatenduMohanty
approved these changes
Apr 15, 2026
There was a problem hiding this comment.
♻️ Duplicate comments (2)
e2e/test_bootstrap_multiple_versions.sh (2)
13-13:⚠️ Potential issue | 🟡 MinorFix trap quoting to avoid early expansion.
Line [13] expands
$constraints_filewhen registering the trap. Use single quotes so expansion happens at exit, with safe path quoting.Proposed fix
-trap "rm -f $constraints_file" EXIT +trap 'rm -f "$constraints_file"' EXIT#!/bin/bash # Verify trap form in the script (read-only) rg -n '^\s*trap\s+' e2e/test_bootstrap_multiple_versions.sh -n -C1As per coding guidelines, "Verify variables are quoted to prevent word splitting."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/test_bootstrap_multiple_versions.sh` at line 13, The trap currently registers with double quotes causing $constraints_file to expand immediately; change the trap registration for the command that removes constraints_file so the variable is expanded at EXIT instead (use single quotes around the trap command and ensure the variable is quoted inside the command, i.e., register the trap as a single-quoted command that runs rm -f "$constraints_file" on EXIT) so the path is expanded only when the trap runs and is safe from word-splitting.
71-82:⚠️ Potential issue | 🟡 MinorCount distinct flit-core versions, not wheel files.
Line [71] counts artifacts; build-tag variants of the same version can overcount and make this check pass incorrectly.
Proposed fix
-FLIT_CORE_COUNT=$(find "$OUTDIR/wheels-repo/downloads/" -name 'flit_core-3.*.whl' | wc -l) +FLIT_CORE_COUNT=$( + find "$OUTDIR/wheels-repo/downloads/" -name 'flit_core-3.*.whl' \ + | awk -F/ '{print $NF}' \ + | awk -F- '{print $2}' \ + | sort -u \ + | wc -l +) echo "Found $FLIT_CORE_COUNT flit-core 3.x wheel(s)"#!/bin/bash # Verify current counting logic is artifact-count based (read-only) rg -n 'FLIT_CORE_COUNT=.*wc -l|flit_core-3\.\*\.whl' e2e/test_bootstrap_multiple_versions.sh -n -C2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/test_bootstrap_multiple_versions.sh` around lines 71 - 82, The current check uses FLIT_CORE_COUNT computed by counting wheel files which can overcount because build-tag variants of the same version are considered separate; update the computation of FLIT_CORE_COUNT (the variable used in the if check) to count distinct semantic versions only by extracting the version part from filenames matching flit_core-3*.whl (strip any build/local tags and file suffix) and using a uniq/deduplicate step before counting; keep the rest of the logic (error message, listing found wheels) intact so the test fails only when fewer than two distinct flit-core versions are present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@e2e/test_bootstrap_multiple_versions.sh`:
- Line 13: The trap currently registers with double quotes causing
$constraints_file to expand immediately; change the trap registration for the
command that removes constraints_file so the variable is expanded at EXIT
instead (use single quotes around the trap command and ensure the variable is
quoted inside the command, i.e., register the trap as a single-quoted command
that runs rm -f "$constraints_file" on EXIT) so the path is expanded only when
the trap runs and is safe from word-splitting.
- Around line 71-82: The current check uses FLIT_CORE_COUNT computed by counting
wheel files which can overcount because build-tag variants of the same version
are considered separate; update the computation of FLIT_CORE_COUNT (the variable
used in the if check) to count distinct semantic versions only by extracting the
version part from filenames matching flit_core-3*.whl (strip any build/local
tags and file suffix) and using a uniq/deduplicate step before counting; keep
the rest of the logic (error message, listing found wheels) intact so the test
fails only when fewer than two distinct flit-core versions are present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0e60d8b8-d222-483a-858b-89ab86ccbcf7
📒 Files selected for processing (3)
e2e/test_bootstrap_multiple_versions.shsrc/fromager/commands/bootstrap.pytests/test_bootstrap.py
✅ Files skipped from review due to trivial changes (1)
- tests/test_bootstrap.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two related fixes for --multiple-versions flag:
Auto-disable constraints with --multiple-versions
Extend e2e test to verify dependency chain handling
|| truesince constraints are now auto-disabledThe e2e test now validates that --multiple-versions bootstraps multiple versions of both top-level packages AND their dependencies, providing better test coverage.
Fixes: #1044
Fixes: #1045