fix(mockbuild-all): address PR #62 review (build-number, finalize, run-state, skip-build, goconserver, docs)

Reviewed by @viniciusferrao. Each numbered point below is his; the code changes verify + fix it.

1. --build-number over-reach / dry-run / double-stamp
   - The bump now runs ONLY on a real build: `--dry-run --build-number N` prints what it would
     stamp and writes nothing (previously it rewrote every spec on disk during a dry run).
   - Re-stamping is idempotent AND replacing: a re-run in a reused tree with a different
     --build-number strips the prior .snap<ts>.<n> before applying the new one, instead of
     accumulating a second stamp (…snap57.snap58). Extracted the per-line logic into the
     unit-testable MockBuildUtils::restamp_release_line and covered it in t/mockbuild-all.t.
   (The headline "rewrites xcat-core / xCAT-genesis-base.spec" does not occur in the real layout:
   xcat-core is a sibling of $repo_root, and there is no genesis spec under the dep tree. The
   legacy nested xcat-source-code case remains a non-CD layout; left as a follow-up.)

3. --finalize-xcat-dep idempotency
   - cross_copy_genesis compared only SIGMD5 (content), which is blind to signature + index state.
     It now also treats a same-content-but-UNSIGNED destination rpm as not-up-to-date (new
     rpm_is_signed helper) so a crash between copy and sign heals on re-run.
   - finalize_xcat_dep now re-indexes+signs BOTH repos of a touched pair every run, not only when
     an rpm was copied, so a crash after copy+sign but before createrepo (rpm on disk, absent from
     repomd) also heals.

4. Stale run-state can mask a failed build
   - A real build now wipes its per-target $run_root first (run_id is derived from the deterministic
     commit time, so re-runs reused the same tree). --skip-build keeps the tree; --dry-run writes nothing.
   - mockbuild-perl-packages.pl clears each package's stale status.txt/error.txt BEFORE building, and
     the aggregate now treats the child worker's exit code as authoritative: a package is PASS only if
     its worker exited 0 AND wrote a PASS this run (a stale PASS in a reused log dir no longer counts).

5. --skip-build can publish the wrong artifacts
   - --skip-build now REQUIRES an explicit --target (without it, all three EL targets collected the
     same EL-agnostic roots and cross-published them).
   - Collection is scoped to this target's own per-target $build_root (the same tree a normal build
     populates), not the legacy build-output/list3/list5/list6 dirs.
   - The manifest version-pin validation (and the "no manifest section" guard) now also run under
     --skip-build, so a collection-only publish is validated exactly like a fresh build.

2. goconserver bypassed the CD bump (minimal fix; hermetic rebuild deferred)
   - goconserver/mockbuild.pl gains --release-suffix, appended to its generated `Release: 4.elN`,
     and mockbuild-all.pl passes the CD suffix down -- so goconserver's NVR advances per run like
     every other dep package (an additive publish is no longer a silent no-op on a frozen NVR).
   - Pinned the clone to an immutable upstream commit instead of the moving `master` (0.3.3 is
     unreleased -- newest tag is v0.3.2 -- so a SHA pin is required; clone now fetches by ref).
   - The host build + `go mod tidy` hermeticity concern is a tracked follow-up, not in this change.

7. Docs
   - BUILD.md: --target is a single value, not repeatable; conserver-xcat is built for every target
     (not "not required"). POD: --parallel-targets default is 1 = serial, not "auto".
   - Added a manifest<->docs consistency test (conserver-xcat present in every target section).

(6, --max-parallel not a true global cap, is a documented nice-to-have and is left as a follow-up.)

Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>

BUILD.md

@@ -60,9 +60,10 @@ Each build path uses `mock` for chroot isolation. Top-level steps are paralleliz
 `packages-manifest.conf` (repo root) declares, per target, exactly which packages are required —
 one `[<target>]` section (matching `--target`, e.g. `[alma+epel-10-x86_64]`) of
 `<package>=<version|*>` lines. For each target, `mockbuild-all.pl` builds **only** the packages
-listed for it; a package absent from a target's section is not built for that target (e.g.
-`conserver-xcat` is not required by any target, and the per-EL perl set differs because the OS/
-EPEL already provides some modules). The lists were derived empirically — on a clean MN of each
+listed for it; a package absent from a target's section is not built for that target (the per-EL
+perl set differs because the OS/EPEL already provides some modules). Note `conserver-xcat` is **not**
+pulled in by `dnf install xCAT` (goconserver superseded it), yet it is listed in — and therefore
+built for — every target, because some deployments still use it. The lists were derived empirically — on a clean MN of each
 (EL, arch), `dnf install xCAT` from xcat.org latest, and the packages whose `from_repo=xcat-dep`
 are exactly the required set. See the file header for details.
 
@@ -163,7 +164,9 @@ mock -r <TARGET> ...
 ```
 
 By default (no `--target`), `mockbuild-all.pl` builds all three EL releases for the host
-arch: `rh8`, `rh9`, and `rh10`. Pass `--target` (repeatable) to restrict the set.
+arch: `rh8`, `rh9`, and `rh10`. Pass `--target <TARGET>` to build a single target instead; it
+takes **one** value and is not repeatable — run the script once per target to build several, or
+omit it to build all three.
 
 # Build the Dependency Repository
 

MockBuildUtils.pm

@@ -12,7 +12,7 @@ use File::Copy qw(copy);
 our @EXPORT_OK = qw(
     sh_quote print_step
     version_matches required_pkgs have_rpm read_manifest
-    rpm_version rpm_sigmd5
+    rpm_version rpm_sigmd5 rpm_is_signed restamp_release_line
     cross_copy_genesis finalize_xcat_dep
 );
 
@@ -76,6 +76,36 @@ sub rpm_sigmd5 {
     return $v;
 }
 
+# rpm_is_signed: does the rpm carry a PGP/GPG header signature? SIGMD5 (above) is content-only and
+# is identical whether or not the rpm is signed, so a cross-copied genesis that was copied but not
+# yet signed (a crash between the copy and the rpmsign) still matches by SIGMD5. finalize uses this
+# to treat such a rpm as NOT up to date so the copy+sign path re-runs and heals it.
+sub rpm_is_signed {
+    my ($f) = @_;
+    return 0 unless defined $f && -f $f;
+    my $v = `rpm -qp --qf '%{SIGPGP}%{SIGGPG}' ${\ sh_quote($f)} 2>/dev/null`;
+    return 0 if !defined $v;
+    $v =~ s/\(none\)//g;      # unsigned rpms report "(none)" for both tags
+    $v =~ s/\s+//g;
+    return $v ne '' ? 1 : 0;
+}
+
+# restamp_release_line: given a spec `Release: ...` line and a CD suffix (".snap<YYYYMMDDHHMM>.<n>"),
+# return (new_line, changed). Idempotent: a line already ending in exactly $suffix is returned
+# unchanged (changed=0). A line carrying a DIFFERENT prior .snap stamp (or several, from an earlier
+# corrupted run) has it stripped before the new suffix is appended, so a re-run in a reused tree
+# REPLACES the stamp instead of accumulating a second one (…snap...57 -> …snap...58, never
+# …snap...57.snap...58). Only the Release token is touched; a non-Release line is returned as-is.
+sub restamp_release_line {
+    my ($line, $suffix) = @_;
+    return ($line, 0) unless defined $line && $line =~ /^Release:\s*\S/i;
+    my $qs = quotemeta($suffix);
+    return ($line, 0) if $line =~ /$qs\s*$/;                 # already carries THIS suffix
+    (my $new = $line) =~ s/(?:\.snap\d{12}\.\d+)+(\s*)$/$1/; # drop any prior CD stamp(s)
+    $new =~ s/(^Release:\s*\S+)/$1$suffix/i;
+    return ($new, 1);
+}
+
 # rpm_version: %{version} of the built binary rpm named <name> under $dir (undef if absent).
 # Skips src/debug rpms and confirms the rpm's real %{name} matches (glob can over-match).
 # 'xCAT-genesis-base' matches the arch-suffixed rpm name (xCAT-genesis-base-x86_64 / -ppc64).
@@ -149,6 +179,10 @@ sub cross_copy_genesis {
             # An empty SIGMD5 (unreadable rpm) means "cannot confirm identical" -> refresh rather
             # than risk skipping on a false match (two '' would otherwise compare equal).
             if (!-f $dst || $src_sig eq '' || $src_sig ne rpm_sigmd5($dst)) { $up_to_date = 0; last; }
+            # Content matches, but SIGMD5 cannot see the signature: a crash between the copy and the
+            # per-rpm sign leaves a same-content-but-UNSIGNED rpm. When a signer is configured, treat
+            # an unsigned dst as not-up-to-date so the copy+sign path re-runs and signs it.
+            if ($sign && !rpm_is_signed($dst)) { $up_to_date = 0; last; }
         }
         return 0 if $up_to_date;
     }
@@ -202,8 +236,12 @@ sub finalize_xcat_dep {
         # named xCAT-genesis-base-ppc64-*. Cross-copy both directions.
         my $to_x86 = cross_copy_genesis($ppcdir, $x86dir, 'ppc64',  $sign);
         my $to_ppc = cross_copy_genesis($x86dir, $ppcdir, 'x86_64', $sign);
-        $reindex->($x86dir) if $to_x86 && $reindex;
-        $reindex->($ppcdir) if $to_ppc && $reindex;
+        # Re-index+sign BOTH repos of the pair every finalize, not only when an rpm was copied this
+        # run. A crash after a prior run's copy+sign but before its createrepo leaves the genesis rpm
+        # on disk (so cross_copy_genesis now returns 0) yet ABSENT from repomd.xml -- which no
+        # signature gate catches. Re-indexing is cheap (these are tiny repos) and idempotent, and it
+        # heals that partial state; skipped only when no signer/indexer was injected.
+        if ($reindex) { $reindex->($x86dir); $reindex->($ppcdir); }
         printf "[finalize] %s: %d ppc64 genesis -> x86_64, %d x86_64 genesis -> ppc64le\n",
             $osdir, $to_x86, $to_ppc;
         $pairs++;

goconserver/mockbuild.pl

@@ -20,7 +20,11 @@ my $log_dir     = "$repo_root/build-logs/list5/goconserver";
 my $skip_install = 0;
 my $version     = '0.3.3';
 my $go_repo     = 'https://github.com/xcat2/goconserver.git';
-my $go_ref      = 'master';
+# Immutable pin: goconserver 0.3.3 is unreleased (newest tag v0.3.2) so it lives only on master.
+# mockbuild-all.pl passes --go-ref with the canonical pin; this default keeps standalone runs
+# reproducible too. `git clone --branch` cannot take a raw SHA, so the clone below fetches by ref.
+my $go_ref      = '6166fe5ec1c5b3c20475e322a9f0e8e93c87e45f';
+my $release_suffix = '';   # CD Release bump (".snap<YYYYMMDDHHMM>.<n>"); passed by mockbuild-all.pl
 my $build_timestamp;
 
 GetOptions(
@@ -33,6 +37,7 @@ GetOptions(
     'version=s'        => \$version,
     'go-repo=s'        => \$go_repo,
     'go-ref=s'         => \$go_ref,
+    'release-suffix=s' => \$release_suffix,
     'build-timestamp=i' => \$build_timestamp,
 ) or die usage();
 
@@ -100,9 +105,14 @@ for my $d (qw(BUILD BUILDROOT RPMS SOURCES SPECS SRPMS)) {
 
 print_step("Clone goconserver source");
 my $src_dir = "$work_dir/goconserver-src";
-run("git clone --depth 1 --branch " . sh_quote($go_ref) . " " .
-    sh_quote($go_repo) . " " . sh_quote($src_dir) .
-    " >" . sh_quote("$log_dir/git-clone.log") . " 2>&1");
+# Fetch the exact pinned ref (a SHA, or a branch/tag). `git clone --branch` rejects a raw SHA, so
+# init + shallow fetch the one object + checkout it -- reproducible and immutable, never "latest
+# master". (xcat2/goconserver has allowReachableSHA1InWant, so fetching a master-reachable SHA works.)
+my $clone_log = sh_quote("$log_dir/git-clone.log");
+run("git init -q " . sh_quote($src_dir) . " >$clone_log 2>&1");
+run("git -C " . sh_quote($src_dir) . " remote add origin " . sh_quote($go_repo) . " >>$clone_log 2>&1");
+run("git -C " . sh_quote($src_dir) . " fetch --depth 1 origin " . sh_quote($go_ref) . " >>$clone_log 2>&1");
+run("git -C " . sh_quote($src_dir) . " checkout -q FETCH_HEAD >>$clone_log 2>&1");
 
 # etcd storage backend has broken deps with modern Go modules;
 # xCAT only uses file storage, so remove etcd before building.
@@ -198,7 +208,7 @@ print_step("Create spec and build RPM");
 my $spec_content = <<"SPEC";
 Name:           goconserver
 Version:        $version
-Release:        4.el$rel
+Release:        4.el$rel$release_suffix
 Summary:        Console server written in Go for xCAT
 License:        EPL-1.0
 URL:            https://github.com/xcat2/goconserver

mockbuild-all.pl

@@ -7,14 +7,15 @@ use Cwd qw(abs_path cwd);
 use File::Basename qw(dirname basename);
 use File::Copy qw(copy);
 use File::Find qw(find);
-use File::Path qw(make_path);
+use File::Path qw(make_path remove_tree);
 use Getopt::Long qw(GetOptions);
 use Parallel::ForkManager;
 use POSIX qw(strftime);
 use FindBin qw($RealBin);
 use lib $RealBin;
 use MockBuildUtils qw(sh_quote print_step version_matches required_pkgs have_rpm
-                      read_manifest rpm_version rpm_sigmd5 cross_copy_genesis finalize_xcat_dep);
+                      read_manifest rpm_version rpm_sigmd5 restamp_release_line
+                      cross_copy_genesis finalize_xcat_dep);
 
 my $script_dir = abs_path(dirname(__FILE__));
 my $repo_root  = abs_path($script_dir);
@@ -37,6 +38,10 @@ my $build_timestamp;
 # fresh, monotonic NVR (deploy's additive rsync is a no-op otherwise). NOT applied
 # to xCAT-genesis-base (built from xcat-core, kept in lockstep with genesis-scripts).
 my $build_number;
+# Pinned goconserver upstream commit (xcat2/goconserver). goconserver 0.3.3 is unreleased (the
+# newest tag is v0.3.2), so it exists only on master -- pin an immutable SHA instead of the moving
+# branch so the build is reproducible. Bump this deliberately when uptaking a new goconserver.
+my $GOCONSERVER_REF = '6166fe5ec1c5b3c20475e322a9f0e8e93c87e45f';
 my $skip_install = 0;
 my $skip_build = 0;
 my $skip_xcat_dep = 0;
@@ -96,6 +101,11 @@ GetOptions(
 ) or die usage();
 
 die "Run as root (uid=$>)\n" if $> != 0 && !$finalize_xcat_dep;
+# --skip-build collects a prior build's artifacts from that build's per-target tree, so it must
+# know the target. Without --target the default is "all three EL targets", and each would collect
+# the same artifacts and cross-publish them into every repo (foreign-EL / foreign-arch rpms).
+die "--skip-build requires an explicit --target (collection is per-target)\n"
+    if $skip_build && $target eq '';
 die "--parallel-builds must be >= 1\n"
     if defined($parallel_builds) && $parallel_builds < 1;
 
@@ -157,7 +167,13 @@ my $RELEASE_BUMP = '';
 if (defined $build_number) {
     die "--build-number must be a non-negative integer\n" if $build_number < 0;
     $RELEASE_BUMP = strftime('.snap%Y%m%d%H%M', gmtime($SOURCE_DATE_EPOCH)) . ".$build_number";
-    bump_dep_release_suffix($repo_root, $RELEASE_BUMP);
+    # A dry run must not touch the tree. Report what would be stamped and leave the specs alone;
+    # $RELEASE_BUMP is still set so the rest of the (no-op) dry-run plan reflects it.
+    if ($dry_run) {
+        print "[dry-run] would stamp Release suffix '$RELEASE_BUMP' on xcat-dep specs under $repo_root (no files written)\n";
+    } else {
+        bump_dep_release_suffix($repo_root, $RELEASE_BUMP);
+    }
 }
 
 # Single output base for every NFS-shared write. Two hosts build in parallel on one NFS by
@@ -258,7 +274,6 @@ exit 0;
 # does not double-stamp). Preserves any %{?dist}/%{?distver} macro already on the line.
 sub bump_dep_release_suffix {
     my ($root, $suffix) = @_;
-    my $qs = quotemeta($suffix);
     my @specs;
     find(sub { push @specs, $File::Find::name if /\.spec$/ && -f $_ }, $root);
     my ($with_release, $bumped, $already) = (0, 0, 0);
@@ -271,9 +286,11 @@ sub bump_dep_release_suffix {
             # case-insensitive: some specs (e.g. Sys-Virt.spec) use a lowercase `release:`
             next unless $line =~ /^Release:\s*\S/i;
             $has_release = 1;
-            if ($line =~ /$qs\s*$/) { last }     # already stamped (idempotent / concurrent arch)
-            $line =~ s/(^Release:\s*\S+)/$1$suffix/i;
-            $changed = 1;
+            # restamp_release_line is idempotent (no-op if already carrying $suffix) and strips any
+            # prior .snap stamp before applying the new one, so a re-run with a different
+            # --build-number replaces rather than accumulates (unit-tested in t/mockbuild-all.t).
+            my ($new, $ch) = restamp_release_line($line, $suffix);
+            if ($ch) { $line = $new; $changed = 1; }
             last;                                 # only the first Release: line
         }
         $with_release++ if $has_release;
@@ -312,7 +329,7 @@ sub build_one_target {
     my %MANIFEST = read_manifest("$repo_root/packages-manifest.conf");
     my %req = %{ $MANIFEST{$target} // {} };
     die "FATAL: no manifest section for target '$target' in packages-manifest.conf\n"
-        if !$skip_build && !%req;
+        if !%req;
 
 my $run_root     = "$output_root/$run_id";
 my $build_root   = "$run_root/build-results";
@@ -323,6 +340,16 @@ my $tarball      = "$output_root/mockbuild-all-$target-$run_id.tar.gz";
 my $srpm_repo_dir = "$run_root/repo-src";
 my $srpm_tarball  = "$output_root/mockbuild-all-$target-$run_id-srpm.tar.gz";
 
+# Each real build must start from a clean per-target tree. run_id is derived from the deterministic
+# commit timestamp, so re-runs of the same commit resolve to the SAME $run_root -- without a wipe, a
+# stale rpm or a stale perl status.txt from an earlier (possibly failed) run could be reused and mask
+# a failure (see mockbuild-perl-packages.pl, which reads per-package status files back). --skip-build
+# deliberately KEEPS the tree (it collects a prior build's artifacts); --dry-run writes nothing.
+if (!$skip_build && !$dry_run && -d $run_root) {
+    print "Cleaning stale per-target tree before build: $run_root\n";
+    remove_tree($run_root);
+}
+
 # All dep builders run natively on every arch. xnba-undi and grub2-xcat are noarch packagings of
 # committed artifacts (an x86 UNDI ROM / the grub2 resource tarball) with no arch-specific build
 # step, so ppc builds them the same as x86 -- no cross-arch import.
@@ -417,6 +444,14 @@ if (!$skip_build) {
                 '--work-dir', sh_quote("/tmp/mockbuild-all-$run_id/$name"),
                 '--build-timestamp', $SOURCE_DATE_EPOCH,
                 ($skip_install ? '--skip-install' : ()),
+                # goconserver generates its spec at build time (from an upstream clone), so the
+                # in-tree spec Release bump above cannot reach it. Hand the CD suffix down so its
+                # NVR advances per run too, and pin the clone to an immutable commit (not the moving
+                # 'master') so the build is reproducible.
+                ($name eq 'goconserver'
+                    ? ('--go-ref', sh_quote($GOCONSERVER_REF),
+                       ($RELEASE_BUMP ne '' ? ('--release-suffix', sh_quote($RELEASE_BUMP)) : ()))
+                    : ()),
             );
             push @build_steps, {
                 id   => "xcat-dep:$name",
@@ -545,16 +580,12 @@ if (!$skip_build) {
 my $xcat_rpms_dir = "$xcat_src/dist/$target/rpms";
 
 if ($skip_build) {
-    push @collect_roots,
-        "$repo_root/build-output/list3/elilo-xcat",
-        "$repo_root/build-output/list3/grub2-xcat",
-        "$repo_root/build-output/list3/ipmitool-xcat",
-        "$repo_root/build-output/list3/syslinux-xcat",
-        "$repo_root/build-output/list3/xnba-undi",
-        "$repo_root/build-output/list5/goconserver/$arch",
-        "$repo_root/goconserver-build-$arch/results/rpm",
-        "$repo_root/build-output/list6/perl/$arch",
-        "$repo_root/perl-list6/$arch";
+    # Collect THIS target's previously-built artifacts from its own per-target build tree -- the
+    # same $build_root a normal build populates (collect_rpms recurses). NOT the legacy EL-agnostic
+    # build-output/list* dirs: those are scoped only by $arch, so an el8 rpm left there would be
+    # pulled into an el9/el10 repo, and with --target omitted the same rpms would be published into
+    # every EL repo. (--target is now required for --skip-build, see the option check above.)
+    push @collect_roots, $build_root;
 }
 
 push @collect_roots, @extra_collect_dirs;
@@ -590,8 +621,10 @@ if (!$skip_genesis && !$dry_run) {
 # Manifest version pins: every required package must be present at its pinned version. A build
 # that produces a different version (a source version bump not reflected here) fails the run;
 # a manifest value of '*' accepts any version. Only the Version is pinned, not the Release
-# (which carries the per-EL dist tag and the genesis snap timestamp).
-if (!$dry_run && !$skip_build) {
+# (which carries the per-EL dist tag and the genesis snap timestamp). This also runs under
+# --skip-build so a collection-only publish is validated against the target's manifest exactly
+# like a fresh build (a stale/foreign-arch collected rpm fails here instead of shipping).
+if (!$dry_run) {
     my @vmiss;
     # Only validate packages whose builder was NOT skipped -- so a clean --skip-* run does not
     # fail on packages it deliberately did not build.
@@ -856,7 +889,7 @@ Options:
   --nproc N               Parallel jobs for buildrpms.pl (default: 1)
   --parallel-builds N     Max concurrent top-level build steps within one EL target (default: auto)
   --parallel-targets N    Concurrent EL targets (rh8/rh9/rh10). 0/auto = all at once, 1 = serial,
-                          N = cap at N. Each target is fully output-isolated (default: auto)
+                          N = cap at N. Each target is fully output-isolated (default: 1 = serial)
   --max-parallel N        Global cap on concurrent mock builds across ALL targets, to avoid
                           oversubscribing the host. Split evenly across active targets.
                           0/auto = host nproc (default: auto)

mockbuild-perl-packages.pl

@@ -202,11 +202,13 @@ my @summary_lines;
 
 print_step("Build packages");
 print "parallel jobs: $jobs\n";
+my %child_rc;   # pkg => child exit code; the AUTHORITATIVE pass/fail for that build
 my $pm = Parallel::ForkManager->new($jobs);
 $pm->run_on_finish(
     sub {
         my ($pid, $exit_code, $ident) = @_;
         my $label = defined $ident ? $ident : "pid=$pid";
+        $child_rc{$ident} = $exit_code if defined $ident;   # record it; do not trust status.txt alone
         my $state = $exit_code == 0 ? 'PASS' : "FAIL(rc=$exit_code)";
         print "[$label] $state\n";
     }
@@ -264,6 +266,17 @@ unless ($keep_buildroots) {
 
 for my $pkg (@packages) {
     my $status_file = "$log_dir/$pkg/status.txt";
+    # The child exit code is authoritative: a package is PASS only if its worker exited 0 AND wrote
+    # a PASS status this run. A missing/non-zero child result is FAIL regardless of any status.txt
+    # (which could be a stale PASS left in a reused log dir, or unwritten because the worker crashed).
+    my $rc = $child_rc{$pkg};
+    if (!defined $rc || $rc != 0) {
+        push @failed, $pkg;
+        push @summary_lines, defined $rc
+            ? "$pkg FAIL worker exited rc=$rc"
+            : "$pkg FAIL no worker result recorded";
+        next;
+    }
     if (!-f $status_file) {
         push @failed, $pkg;
         push @summary_lines, "$pkg FAIL missing status file ($status_file)";
@@ -328,6 +341,9 @@ sub build_package {
     make_path($pkg_run_dir);
     make_path($pkg_result);
     make_path($pkg_log);
+    # Clear any status/error left by an earlier run in a reused log dir BEFORE building, so a crash
+    # between here and the status write below can never leave a stale PASS the aggregate would trust.
+    unlink $status_file, "$pkg_log/error.txt";
 
     my $det_mock_cfg = create_deterministic_mock_cfg($mock_cfg, $SOURCE_DATE_EPOCH, $pkg_run_dir);
 

t/mockbuild-all.t

@@ -10,8 +10,8 @@ use lib "$RealBin/..";
 use File::Temp qw(tempdir);
 use File::Path qw(make_path);
 use File::Basename qw(basename);
-use MockBuildUtils qw(required_pkgs version_matches rpm_sigmd5 rpm_version
-                      cross_copy_genesis finalize_xcat_dep read_manifest);
+use MockBuildUtils qw(required_pkgs version_matches rpm_sigmd5 rpm_version rpm_is_signed
+                      restamp_release_line cross_copy_genesis finalize_xcat_dep read_manifest);
 
 # Run a printing sub with STDOUT muted so its progress lines do not pollute TAP.
 sub quiet(&) {
@@ -158,4 +158,53 @@ SPEC
     like($@, qr/no ppc64le peer repo/, 'finalize error names the missing peer');
 }
 
+# ---- restamp_release_line: CD --build-number Release stamping (PR #62 review point 1) ----------
+# A fresh stamp is appended after the Release token, preserving any %{?dist} macro.
+{
+    my ($l, $ch) = restamp_release_line("Release:        1%{?dist}\n", '.snap202607161200.57');
+    is($l, "Release:        1%{?dist}.snap202607161200.57\n", 'stamps a fresh Release, macro preserved');
+    is($ch, 1, 'reports changed');
+}
+# Idempotent: the exact same suffix is a no-op (concurrent per-arch build / same-tree re-run).
+{
+    my $line = "Release:        1%{?dist}.snap202607161200.57\n";
+    my ($l, $ch) = restamp_release_line($line, '.snap202607161200.57');
+    is($l, $line, 're-stamping the SAME suffix is a no-op');
+    is($ch, 0, 'reports unchanged');
+}
+# A DIFFERENT build-number REPLACES the prior stamp (does not accumulate) -- the double-stamp bug.
+{
+    my ($l, $ch) = restamp_release_line("Release: 1%{?dist}.snap202607161200.57\n", '.snap202607161200.58');
+    is($l, "Release: 1%{?dist}.snap202607161200.58\n", 'a new build-number replaces the old stamp');
+    is($ch, 1, 'reports changed');
+    unlike($l, qr/\.snap\d{12}\.\d+\.snap/, 'never leaves two stacked .snap stamps');
+}
+# Even an already-corrupted (double-stamped) line is healed back to a single stamp.
+{
+    my ($l) = restamp_release_line("Release: 5.snap202601010000.1.snap202601020000.2\n", '.snap202607161200.9');
+    is($l, "Release: 5.snap202607161200.9\n", 'strips multiple stacked prior stamps before re-stamping');
+}
+# A non-Release line is never touched.
+{
+    my ($l, $ch) = restamp_release_line("Version: 0.3.3\n", '.snap202607161200.57');
+    is($l, "Version: 0.3.3\n", 'non-Release line untouched');
+    is($ch, 0, 'reports unchanged');
+}
+
+# ---- rpm_is_signed: unreadable / missing -> not signed (used by the finalize idempotency fix) ---
+is(rpm_is_signed(undef), 0, 'rpm_is_signed(undef) is 0');
+is(rpm_is_signed("/no/such/file.rpm"), 0, 'rpm_is_signed on a missing file is 0');
+
+# ---- manifest <-> docs consistency: conserver-xcat is in EVERY target section (PR #62 point 7c) --
+# BUILD.md documents conserver-xcat as built for every target; guard that the manifest agrees so the
+# doc and the manifest can never silently drift apart again.
+{
+    my %m = read_manifest("$RealBin/../packages-manifest.conf");
+    my @targets = sort keys %m;
+    cmp_ok(scalar(@targets), '>=', 1, 'packages-manifest.conf has at least one target section');
+    my @missing = grep { !exists $m{$_}{'conserver-xcat'} } @targets;
+    is_deeply(\@missing, [], 'conserver-xcat is present in every manifest target section')
+        or diag("missing conserver-xcat in: @missing");
+}
+
 done_testing;