From d0875ee1ff6dc04a24980b303fa340c6310e0a4c Mon Sep 17 00:00:00 2001
From: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Date: Thu, 13 Aug 2026 11:05:33 -0300
Subject: [PATCH] fix(xcat-dep): validate_manifest must apply the SAME arch:all
 skip as the builder (ppc build was aborting)

The previous commit added the arch:all boot tools (syslinux-xcat/grub2-xcat/
elilo-xcat/xnba-undi) to the ppc64el manifest and taught build_one_codename to
skip BUILDING them on non-amd64 -- but validate_manifest, which runs per-arch on
every non-dry-run invocation and is NOT gated by --skip-createrepo, still
demanded them. On the ppc64el build stage (--arch ppc64el, no --skip-build) the
builder skipped the four (correct -- their source is x86-only) so they were never
staged, then validate_manifest reported them MISSING and aborted the whole ppc
build before assembly: every BUILD_PPC=true run would have failed.

Fix the drift structurally: extract the skip rule into one pure, tested decider
BuildUtils::skip_arch_all_on() behind a shared sbuild-all.pl helper
pkg_skip_on_arch(), consulted by BOTH build_one_codename and validate_manifest,
so a package the build skips is never demanded by the per-arch validation. The
arch:all debs' presence on ppc is still verified later against the published
index by verify_assembled_repo. Also make control_binary_arch return the full
Architecture value (not just the first token of a multi-arch list) and add a
regression test for the shared skip rule.

Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
---
 BuildUtils.pm  | 25 +++++++++++++++++++------
 sbuild-all.pl  | 32 +++++++++++++++++++++++---------
 t/sbuild-all.t | 16 +++++++++++++++-
 3 files changed, 57 insertions(+), 16 deletions(-)

diff --git a/BuildUtils.pm b/BuildUtils.pm
index d6d52c7..3fbd2f7 100644
--- a/BuildUtils.pm
+++ b/BuildUtils.pm
@@ -25,7 +25,7 @@ our @EXPORT_OK = qw(
     sh_quote print_step
     version_matches required_pkgs read_manifest standard_options
     verify_repo_packages verify_repo_signature parse_packages_index resolve_present_names
-    index_has_native_arch control_binary_arch
+    index_has_native_arch control_binary_arch skip_arch_all_on
     codename_to_version version_to_codename known_codenames
     chroot_name chroot_sources_list
     control_field genesis_deb_control
@@ -240,22 +240,35 @@ sub index_has_native_arch {
 }
 
 # control_binary_arch($control_text, $binpkg): the Architecture field of the BINARY package $binpkg in
-# a debian/control (a source may declare several binary packages). Returns e.g. 'all', 'any',
-# 'ppc64el', or undef if that package/field is absent. Lets the builder tell an arch:all
-# single-producer package (built once on amd64 -- e.g. syslinux-xcat/grub2-xcat, whose source is
-# x86-only) from a genuinely per-arch one, so it is not rebuilt on ppc. Pure: text in, string out.
+# a debian/control (a source may declare several binary packages). Returns the field value verbatim --
+# 'all', 'any', or a space-separated arch list ('i386 amd64 ia64 ppc64el') -- or undef if that
+# package/field is absent. Lets the builder tell an arch:all single-producer package (built once on
+# amd64 -- e.g. syslinux-xcat/grub2-xcat, whose source is x86-only) from a genuinely per-arch one, so
+# it is not rebuilt on ppc. Pure: text in, string out.
 sub control_binary_arch {
     my ($text, $binpkg) = @_;
     return undef unless defined $text && defined $binpkg && $binpkg ne '';
     for my $para (split /\n\n+/, $text) {
         my ($p) = $para =~ /^Package:[ \t]*(\S+)/m;
         next unless defined $p && $p eq $binpkg;
-        my ($a) = $para =~ /^Architecture:[ \t]*(\S+)/m;
+        my ($a) = $para =~ /^Architecture:[ \t]*(.+?)[ \t]*$/m;   # full value (may be a space list)
         return $a;   # undef if this paragraph lacks an Architecture field
     }
     return undef;
 }
 
+# skip_arch_all_on($control_text, $binpkg, $arch): true iff $binpkg is an Architecture:all
+# single-producer package (built ONCE on amd64) and $arch is NOT amd64 -- so it must be neither BUILT
+# nor per-arch VALIDATED on $arch (it arrives from the amd64 producer; its presence on $arch is checked
+# later against the PUBLISHED index by verify_assembled_repo). The single source of truth shared by
+# build_one_codename and validate_manifest, so a package the build skips is never demanded by the
+# per-arch validation. Pure: control text in, boolean out.
+sub skip_arch_all_on {
+    my ($control_text, $binpkg, $arch) = @_;
+    return 0 if !defined $arch || $arch eq 'amd64';
+    return ((control_binary_arch($control_text, $binpkg) // '') eq 'all') ? 1 : 0;
+}
+
 # resolve_present_names(\%parsed, $arch, \@names) -> \%present  (name => upstream version | undef)
 # PURE. Resolves each manifest package NAME to the version actually in the parsed index (\%parsed from
 # parse_packages_index), reducing to the upstream version so it compares against the manifest's
diff --git a/sbuild-all.pl b/sbuild-all.pl
index b900d04..97368b5 100755
--- a/sbuild-all.pl
+++ b/sbuild-all.pl
@@ -38,7 +38,7 @@ use FindBin qw($RealBin);
 use lib $RealBin;
 use BuildUtils qw(sh_quote print_step version_matches required_pkgs read_manifest standard_options
                   verify_repo_packages verify_repo_signature parse_packages_index resolve_present_names
-                  index_has_native_arch control_binary_arch
+                  index_has_native_arch control_binary_arch skip_arch_all_on
                   codename_to_version known_codenames chroot_name chroot_sources_list
                   control_field genesis_deb_control
                   deb_field deb_version deb_upstream_version deb_hash cross_copy_genesis_deb);
@@ -331,6 +331,19 @@ sub ensure_chroots {
 # build_one_codename: build every required (non-genesis) package for ONE codename, serially, each in
 # that codename's <codename>-<arch>-sbuild chroot. Returns 0 on success, non-zero if any package
 # failed. Called either directly (serial mode) or inside a forked child (parallel mode).
+# pkg_skip_on_arch($pkg, $arch): the SINGLE source of truth for "is $pkg an arch:all single-producer
+# that must be neither built nor per-arch-validated on $arch?". Reads the package's debian/control and
+# delegates the decision to the pure BuildUtils::skip_arch_all_on. Used by BOTH build_one_codename and
+# validate_manifest so they can never drift (a package the build skips must not be demanded by the
+# validation). Fail-safe: an unreadable/missing control yields no skip (the package is built/validated).
+sub pkg_skip_on_arch {
+    my ($pkg, $a) = @_;
+    my $dir = $PKG_DIR{$pkg} or return 0;
+    my $ctl = '';
+    if (open my $cf, '<', "$repo_root/$dir/debian/control") { local $/; $ctl = <$cf>; close $cf; }
+    return skip_arch_all_on($ctl, $pkg, $a);
+}
+
 sub build_one_codename {
     my ($cn) = @_;
     my $tgt = "$cn-$arch";
@@ -345,14 +358,11 @@ sub build_one_codename {
         # ONCE on amd64 -- their source is x86-only (syslinux compiles with nasm/gcc-multilib) -- and,
         # being Architecture:all, are assembled into every arch's Packages index. They stay REQUIRED in
         # the ppc64el manifest so the gate verifies the ppc repo actually carries them, but are NOT
-        # rebuilt here (a ppc build would fail). Detect arch:all from the package's own debian/control.
-        if ($arch ne 'amd64') {
-            my $ctl = '';
-            if (open my $cf, '<', "$repo_root/$dir/debian/control") { local $/; $ctl = <$cf>; close $cf; }
-            if ((control_binary_arch($ctl, $pkg) // '') eq 'all') {
-                print "  [$cn] -> $pkg: arch:all single-producer (built on amd64) -- not rebuilt on $arch\n";
-                next;
-            }
+        # rebuilt here. pkg_skip_on_arch() is the SHARED rule -- validate_manifest applies the SAME one
+        # so the per-arch validation never demands a package the build deliberately skipped.
+        if (pkg_skip_on_arch($pkg, $arch)) {
+            print "  [$cn] -> $pkg: arch:all single-producer (built on amd64) -- not rebuilt on $arch\n";
+            next;
         }
         my $builder = "$repo_root/$dir/sbuild.pl";
         die "FATAL: missing builder $builder (required for $pkg on $tgt)\n" unless -f $builder;
@@ -531,6 +541,10 @@ sub validate_manifest {
         my $tgt = "$cn-$arch";
         my $dir = "$staging/$cn/$arch";
         for my $pkg (required_pkgs([sort keys %{$MANIFEST{$tgt}}], $skip_genesis, $skip_xcat_dep)) {
+            # arch:all single-producer packages are built on amd64 and are NOT staged for this arch, so
+            # do not validate them per-arch here (build_one_codename skips them via the SAME rule). Their
+            # presence on this arch is verified later against the PUBLISHED index (verify_assembled_repo).
+            next if pkg_skip_on_arch($pkg, $arch);
             my $want = $MANIFEST{$tgt}{$pkg};
             my $got  = deb_version($dir, $pkg);
             if (!defined $got) { push @fail, "[$tgt] MISSING $pkg"; next; }
diff --git a/t/sbuild-all.t b/t/sbuild-all.t
index fe5c4b0..a618207 100644
--- a/t/sbuild-all.t
+++ b/t/sbuild-all.t
@@ -14,7 +14,7 @@ use File::Path qw(make_path);
 use File::Basename qw(basename);
 use BuildUtils qw(required_pkgs version_matches read_manifest standard_options
                   verify_repo_packages verify_repo_signature parse_packages_index resolve_present_names
-                  index_has_native_arch control_binary_arch
+                  index_has_native_arch control_binary_arch skip_arch_all_on
                   codename_to_version version_to_codename known_codenames
                   chroot_name chroot_sources_list
                   control_field genesis_deb_control
@@ -366,4 +366,18 @@ SKIP: {
         'ppc64el', 'native per-arch value returned verbatim');
 }
 
+# ---- skip_arch_all_on: the SHARED build/validate skip rule -----------------------------------------
+# build_one_codename AND validate_manifest both consult this, so the arch:all boot tools the ppc build
+# skips are ALSO exempt from the per-arch validation (else the ppc build stage would false-fail MISSING
+# on packages it deliberately did not build -- the regression this guards).
+{
+    my $ctl = "Package: syslinux\nArchitecture: any\n\nPackage: syslinux-xcat\nArchitecture: all\n";
+    ok( skip_arch_all_on($ctl, 'syslinux-xcat', 'ppc64el'), 'arch:all pkg skipped on ppc64el (build+validate)');
+    ok(!skip_arch_all_on($ctl, 'syslinux-xcat', 'amd64'),   'arch:all pkg NOT skipped on amd64 (its producer)');
+    ok(!skip_arch_all_on("Package: ipmitool-xcat\nArchitecture: i386 amd64 ppc64el\n", 'ipmitool-xcat', 'ppc64el'),
+        'native multi-arch pkg NOT skipped on ppc64el (it IS built there)');
+    ok(!skip_arch_all_on('',   'x',             'ppc64el'), 'absent control -> not skipped (fail-safe)');
+    ok(!skip_arch_all_on($ctl, 'syslinux-xcat', undef),     'undef arch -> not skipped (no crash)');
+}
+
 done_testing;