From cb4099c5912c92536a6d701b25589b69e4192a28 Mon Sep 17 00:00:00 2001
From: Doug Hellmann <dhellmann@redhat.com>
Date: Tue, 23 Sep 2025 20:14:52 -0400
Subject: [PATCH] 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>
---
 e2e/test_build_order.sh        | 34 ++++++++++++++++++++++++++++++++++
 src/fromager/commands/build.py | 29 ++++++++++++++---------------
 2 files changed, 48 insertions(+), 15 deletions(-)

diff --git a/e2e/test_build_order.sh b/e2e/test_build_order.sh
index e4982c3b..d9a0f184 100755
--- a/e2e/test_build_order.sh
+++ b/e2e/test_build_order.sh
@@ -66,6 +66,23 @@ for pattern in $EXPECTED_FILES; do
   fi
 done
 
+# Verify that build-sequence-summary.json has correct skipped status for force builds
+# When using --force, all packages should be built (skipped: false)
+if [ -f "$OUTDIR/work-dir/build-sequence-summary.json" ]; then
+  # Check that all entries have skipped: false
+  not_skipped_count=$(jq '[.[] | select(.skipped == false)] | length' "$OUTDIR/work-dir/build-sequence-summary.json")
+  total_count=$(jq 'length' "$OUTDIR/work-dir/build-sequence-summary.json")
+  if [ "$not_skipped_count" != "$total_count" ]; then
+    echo "Expected all $total_count packages to be built (not skipped), but only $not_skipped_count were marked as not skipped" 1>&2
+    echo "Contents of build-sequence-summary.json:" 1>&2
+    jq '.' "$OUTDIR/work-dir/build-sequence-summary.json" 1>&2
+    pass=false
+  fi
+else
+  echo "build-sequence-summary.json file not found after force build" 1>&2
+  pass=false
+fi
+
 $pass
 
 # Rebuild everything while reusing existing local wheels
@@ -114,4 +131,21 @@ if ! grep -q "skipping building wheel since ${DIST}-${VERSION}-" "$log"; then
   pass=false
 fi
 
+# Verify that build-sequence-summary.json has correct skipped status
+# When using --cache-wheel-server-url with PyPI, all packages should be skipped
+if [ -f "$OUTDIR/work-dir/build-sequence-summary.json" ]; then
+  # Check that all entries have skipped: true
+  skipped_count=$(jq '[.[] | select(.skipped == true)] | length' "$OUTDIR/work-dir/build-sequence-summary.json")
+  total_count=$(jq 'length' "$OUTDIR/work-dir/build-sequence-summary.json")
+  if [ "$skipped_count" != "$total_count" ]; then
+    echo "Expected all $total_count packages to be skipped, but only $skipped_count were marked as skipped" 1>&2
+    echo "Contents of build-sequence-summary.json:" 1>&2
+    jq '.' "$OUTDIR/work-dir/build-sequence-summary.json" 1>&2
+    pass=false
+  fi
+else
+  echo "build-sequence-summary.json file not found" 1>&2
+  pass=false
+fi
+
 $pass
diff --git a/src/fromager/commands/build.py b/src/fromager/commands/build.py
index c78521b0..00605ecb 100644
--- a/src/fromager/commands/build.py
+++ b/src/fromager/commands/build.py
@@ -363,22 +363,21 @@ def _build(
             logger.info("using existing wheel from %s", wheel_filename)
             use_exiting_wheel = True
 
-    # See if we can download a prebuilt wheel.
-    if prebuilt and not wheel_filename:
-        logger.info("downloading prebuilt wheel")
-        wheel_filename = wheels.download_wheel(
-            req=req,
-            wheel_url=source_download_url,
-            output_directory=wkctx.wheels_build,
-        )
-    else:
-        # already downloaded
-        use_exiting_wheel = True
+    # Handle prebuilt wheels.
+    if prebuilt:
+        if not wheel_filename:
+            logger.info("downloading prebuilt wheel")
+            wheel_filename = wheels.download_wheel(
+                req=req,
+                wheel_url=source_download_url,
+                output_directory=wkctx.wheels_build,
+            )
+        else:
+            # already downloaded prebuilt wheel
+            use_exiting_wheel = True
 
-    # We may have downloaded the prebuilt wheel in _is_wheel_built or
-    # via download_wheel(). Regardless, if it was a prebuilt wheel,
-    # run the hooks.
-    if prebuilt and wheel_filename:
+        # Run hooks for prebuilt wheels. At this point wheel_filename should
+        # be set either from _is_wheel_built() or download_wheel().
         hooks.run_prebuilt_wheel_hooks(
             ctx=wkctx,
             req=req,