Skip to content

Commit cb4099c

Browse files
committed
fix(build): correct skipped status in build-sequence-summary.json
Fix incorrect logic that marked all non-prebuilt wheels as skipped when using --cache-wheel-server-url. The bug was in the _build() function where an else clause unconditionally set use_exiting_wheel=True for all non-prebuilt packages, regardless of whether they were actually built or reused. Changes: - Only set use_exiting_wheel=True when we actually have an existing wheel - Consolidate duplicate prebuilt wheel checks into single block - Eliminate redundant wheel_filename validation - Add test validation for build-sequence-summary.json content The fix ensures packages built from source show skipped=false and packages using existing wheels show skipped=true in the summary file. Closes: #777 Chat log: https://gist.github.com/dhellmann/55da9cedbc977fe644f2c7584c7d1762 Co-authored-by: Claude 3.5 Sonnet (Anthropic AI Assistant) Signed-off-by: Doug Hellmann <dhellmann@redhat.com>
1 parent 5c22357 commit cb4099c

2 files changed

Lines changed: 48 additions & 15 deletions

File tree

e2e/test_build_order.sh

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,23 @@ for pattern in $EXPECTED_FILES; do
6666
fi
6767
done
6868

69+
# Verify that build-sequence-summary.json has correct skipped status for force builds
70+
# When using --force, all packages should be built (skipped: false)
71+
if [ -f "$OUTDIR/work-dir/build-sequence-summary.json" ]; then
72+
# Check that all entries have skipped: false
73+
not_skipped_count=$(jq '[.[] | select(.skipped == false)] | length' "$OUTDIR/work-dir/build-sequence-summary.json")
74+
total_count=$(jq 'length' "$OUTDIR/work-dir/build-sequence-summary.json")
75+
if [ "$not_skipped_count" != "$total_count" ]; then
76+
echo "Expected all $total_count packages to be built (not skipped), but only $not_skipped_count were marked as not skipped" 1>&2
77+
echo "Contents of build-sequence-summary.json:" 1>&2
78+
jq '.' "$OUTDIR/work-dir/build-sequence-summary.json" 1>&2
79+
pass=false
80+
fi
81+
else
82+
echo "build-sequence-summary.json file not found after force build" 1>&2
83+
pass=false
84+
fi
85+
6986
$pass
7087

7188
# Rebuild everything while reusing existing local wheels
@@ -114,4 +131,21 @@ if ! grep -q "skipping building wheel since ${DIST}-${VERSION}-" "$log"; then
114131
pass=false
115132
fi
116133

134+
# Verify that build-sequence-summary.json has correct skipped status
135+
# When using --cache-wheel-server-url with PyPI, all packages should be skipped
136+
if [ -f "$OUTDIR/work-dir/build-sequence-summary.json" ]; then
137+
# Check that all entries have skipped: true
138+
skipped_count=$(jq '[.[] | select(.skipped == true)] | length' "$OUTDIR/work-dir/build-sequence-summary.json")
139+
total_count=$(jq 'length' "$OUTDIR/work-dir/build-sequence-summary.json")
140+
if [ "$skipped_count" != "$total_count" ]; then
141+
echo "Expected all $total_count packages to be skipped, but only $skipped_count were marked as skipped" 1>&2
142+
echo "Contents of build-sequence-summary.json:" 1>&2
143+
jq '.' "$OUTDIR/work-dir/build-sequence-summary.json" 1>&2
144+
pass=false
145+
fi
146+
else
147+
echo "build-sequence-summary.json file not found" 1>&2
148+
pass=false
149+
fi
150+
117151
$pass

src/fromager/commands/build.py

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -363,22 +363,21 @@ def _build(
363363
logger.info("using existing wheel from %s", wheel_filename)
364364
use_exiting_wheel = True
365365

366-
# See if we can download a prebuilt wheel.
367-
if prebuilt and not wheel_filename:
368-
logger.info("downloading prebuilt wheel")
369-
wheel_filename = wheels.download_wheel(
370-
req=req,
371-
wheel_url=source_download_url,
372-
output_directory=wkctx.wheels_build,
373-
)
374-
else:
375-
# already downloaded
376-
use_exiting_wheel = True
366+
# Handle prebuilt wheels.
367+
if prebuilt:
368+
if not wheel_filename:
369+
logger.info("downloading prebuilt wheel")
370+
wheel_filename = wheels.download_wheel(
371+
req=req,
372+
wheel_url=source_download_url,
373+
output_directory=wkctx.wheels_build,
374+
)
375+
else:
376+
# already downloaded prebuilt wheel
377+
use_exiting_wheel = True
377378

378-
# We may have downloaded the prebuilt wheel in _is_wheel_built or
379-
# via download_wheel(). Regardless, if it was a prebuilt wheel,
380-
# run the hooks.
381-
if prebuilt and wheel_filename:
379+
# Run hooks for prebuilt wheels. At this point wheel_filename should
380+
# be set either from _is_wheel_built() or download_wheel().
382381
hooks.run_prebuilt_wheel_hooks(
383382
ctx=wkctx,
384383
req=req,

0 commit comments

Comments
 (0)