diff --git a/BuildUtils.pm b/BuildUtils.pm index b417ef243..9e5a1ec7a 100644 --- a/BuildUtils.pm +++ b/BuildUtils.pm @@ -14,8 +14,9 @@ package BuildUtils; use strict; use warnings; use Exporter 'import'; -use File::Copy qw(copy); -use File::Path qw(make_path); +use File::Copy qw(copy move); +use File::Basename qw(basename); +use File::Path qw(make_path remove_tree); use POSIX qw(strftime); our @EXPORT_OK = qw( @@ -26,7 +27,8 @@ our @EXPORT_OK = qw( pin_control_version rewrite_changelog_header reprepro_distributions reprepro_options lock_id_for take_build_lock - sh_quote + sh_quote clean_debian_residue + backup_file restore_file ); # The xCAT-probe helpers. xcat-probe reuses functions shipped by xCAT; they are COPIED @@ -56,6 +58,75 @@ my @DEFAULT_DISTS = qw(focal jammy noble resolute); sub default_dists { return @DEFAULT_DISTS; } # sh_quote: single-quote a string for safe use in a shell command. +# clean_debian_residue: remove what dpkg-buildpackage leaves inside a package's +# debian/ directory. +# +# debian/files accumulates one line per artifact and is never truncated by +# `dh_clean -d`, which only removes directories. dpkg-genchanges then reads the +# stale entries on the next build and fstats artifacts that are no longer there: +# dpkg-genchanges: error: cannot fstat file ../perl-xcat__amd64.buildinfo +# so a second build in the same checkout dies as soon as the release string moves. +# The staging directories go for the same reason the old shell builder removed +# them -- they are the previous build's payload, not source. +# +# Call this only after a package's LAST architecture: debian/files carries the +# amd64 artifacts that the ppc64el run's dpkg-genchanges still needs. +# backup_file / restore_file: put a file back exactly as it was. +# +# File::Copy::copy does NOT carry permissions, so a naive backup-and-restore returns +# an executable with its exec bit stripped -- the content compares equal and only +# `git diff` notices the mode change. xCAT/postscripts/{bmcsetup,getipmi} are shipped +# executable and are rewritten during the xCAT build, so this is not hypothetical. +sub backup_file { + my ($path) = @_; + return unless defined $path && -f $path; + my $backup = "$path.build.save"; + my $mode = ( stat $path )[2] & 07777; + copy( $path, $backup ) or die "Cannot back up $path: $!\n"; + return [ $backup, $path, $mode ]; +} + +sub restore_file { + my ($entry) = @_; + return 0 unless $entry; + my ( $backup, $path, $mode ) = @{$entry}; + move( $backup, $path ) or do { warn "Could not restore $path: $!\n"; return 0; }; + chmod $mode, $path if defined $mode; + return 1; +} + +sub clean_debian_residue { + my ($package_root) = @_; + return () unless defined $package_root && -d "$package_root/debian"; + + my @removed; + my $files = "$package_root/debian/files"; + if (-e $files) { + unlink $files or die "Cannot remove $files: $!\n"; + push @removed, $files; + } + + my $stem = lc(basename($package_root)); + foreach my $dir (glob("$package_root/debian/$stem*")) { + next unless -d $dir; + remove_tree($dir); + push @removed, $dir; + } + + # debhelper's own bookkeeping. Never tracked, and it accumulates per build. + # glob returns a wildcard-free pattern verbatim whether or not it exists, so + # the -e guard is what makes a second call a no-op rather than a fatal unlink. + foreach my $residue (glob("$package_root/debian/*.debhelper.log"), + "$package_root/debian/.debhelper") { + next unless -e $residue; + if (-d $residue) { remove_tree($residue); } + else { unlink $residue or die "Cannot remove $residue: $!\n"; } + push @removed, $residue; + } + + return @removed; +} + sub sh_quote { my ($s) = @_; $s = '' if !defined $s; diff --git a/builddebs.pl b/builddebs.pl index d7214f015..b1aa61114 100755 --- a/builddebs.pl +++ b/builddebs.pl @@ -31,7 +31,8 @@ use BuildUtils qw( source_date_epoch snap_release deb_version stage_probe_helpers XCAT_PROBE_HELPERS deb_package_arches dist_arches default_dists - orig_tarball_name upstream_version resolve_dest + orig_tarball_name upstream_version resolve_dest clean_debian_residue + backup_file restore_file pin_control_version rewrite_changelog_header reprepro_distributions reprepro_options lock_id_for take_build_lock sh_quote @@ -148,9 +149,7 @@ sub with_prepared_tree { my ($rel) = @_; my $path = "$dir/$rel"; if (-f $path) { - my $backup = "$path.build.save"; - copy($path, $backup) or die "Cannot back up $path: $!\n"; - push @restore, [$backup, $path]; + push @restore, backup_file($path); } else { push @remove, $path; @@ -159,6 +158,8 @@ sub with_prepared_tree { $claim->('debian/control'); $claim->('debian/changelog'); + # dpkg rewrites debian/.substvars in place, and several of them are tracked. + $claim->("debian/" . basename($_)) for glob("$dir/debian/*.substvars"); # Pin the intra-xCAT dependencies to this exact build, so a partial upgrade cannot # mix versions. @@ -191,13 +192,16 @@ sub with_prepared_tree { my $src = "$ROOT/xCAT-genesis-scripts/usr/bin/$f"; next unless -f $src; my $dst = "$dir/postscripts/$f"; + # Both are TRACKED files. Claiming them backs the originals up and puts + # them back; treating them as created would delete them from the + # checkout, which is what happened before. + $claim->("postscripts/$f"); my $text = do { open my $fh, '<', $src or die; local $/; <$fh> }; $text =~ s/xcat\.genesis\.\Q$f\E/$f/g; open my $out, '>', $dst or die "Cannot write $dst: $!\n"; print {$out} $text; close $out; chmod 0755, $dst; - push @added, $dst; } } # xCAT-genesis-scripts keeps a control file per architecture. @@ -214,10 +218,7 @@ sub with_prepared_tree { my $err = $@; unlink @added, @remove; - for my $pair (reverse @restore) { - my ($backup, $path) = @$pair; - move($backup, $path) or warn "Could not restore $path: $!\n"; - } + restore_file($_) for reverse @restore; die $err if $rc; return; } @@ -270,6 +271,11 @@ sub collect_debs { # The rest of the dpkg output is build residue, not an artifact. unlink glob("$ROOT/*.buildinfo"), glob("$ROOT/*.changes"), glob("$ROOT/*.dsc"), glob("$ROOT/*.tar.xz"), glob("$ROOT/*.tar.gz"); + # And the residue dpkg leaves inside the package -- debian/files survives + # `dh_clean -d` and would make the next build with a different release fstat + # artifacts this run already moved away. Safe here: this runs after the last + # architecture, so no dpkg-genchanges still needs it. + clean_debian_residue("$ROOT/$pkg"); return $moved; } diff --git a/xCAT-test/unit/build_utils.t b/xCAT-test/unit/build_utils.t index 12b118ddc..652b90f7f 100644 --- a/xCAT-test/unit/build_utils.t +++ b/xCAT-test/unit/build_utils.t @@ -24,7 +24,8 @@ use BuildUtils qw( deb_package_arches dist_arches orig_tarball_name upstream_version resolve_dest pin_control_version rewrite_changelog_header - reprepro_distributions reprepro_options sh_quote + reprepro_distributions reprepro_options sh_quote clean_debian_residue + backup_file restore_file ); # ------------------------------------------------------------------- versions -- @@ -137,6 +138,14 @@ like( $rewritten, qr/^ -- xCAT Build Sat, 24 Aug 2026 08 'and the deterministic date, so two builds of one commit match' ); like( $rewritten, qr/^xcat \(2\.17\.0\) unstable/m, 'the older stanza is left alone -- the history is not ours to rewrite' ); +# Its trailer too. This is the defect the old shell builder shipped: its sed had no +# line address, so every trailer in the file was restamped and 2023 entries went out +# authored by today's builder. Only the top stanza may move. +like( $rewritten, + qr/^ -- Somebody Else Mon, 01 Jan 2023 00:00:00 \+0000$/m, + 'including its author and date, which this build did not write' ); +is( scalar( () = $rewritten =~ /^ -- xCAT Build /mg ), 1, + 'exactly one trailer is restamped, however many stanzas the file has' ); # ------------------------------------------------------------------ reprepro -- @@ -200,4 +209,81 @@ like( reprepro_options('/some/gnupghome'), qr/^basedir \.$/m, is( sh_quote(q{it's}), q{'it'"'"'s'}, 'a single quote survives shell quoting' ); is( sh_quote(undef), q{''}, 'undef quotes to the empty string' ); +# ------------------------------------------------ dpkg residue inside a package -- +# debian/files accumulates one line per artifact and survives `dh_clean -d`, so the +# next build's dpkg-genchanges fstats artifacts that are no longer on disk and the +# whole build dies. Only the residue goes; the packaging itself must stay. +{ + my $pkgroot = tempdir( CLEANUP => 1 ) . '/perl-xCAT'; + make_path("$pkgroot/debian/perl-xcat/usr/share"); + make_path("$pkgroot/debian/source"); + for my $f (qw(debian/files debian/control debian/rules debian/changelog debian/source/format)) { + open my $fh, '>', "$pkgroot/$f" or die $!; + print {$fh} "stale\n"; + close $fh; + } + + # debhelper bookkeeping, never tracked, accumulates per build. + make_path("$pkgroot/debian/.debhelper/generated"); + open my $dh, '>', "$pkgroot/debian/perl-xcat.debhelper.log" or die $!; + close $dh; + + my @removed = clean_debian_residue($pkgroot); + + ok( !-e "$pkgroot/debian/files", + 'debian/files does not survive into the next build' ); + ok( !-d "$pkgroot/debian/perl-xcat", + 'nor does the staging tree of the build that just finished' ); + ok( !-e "$pkgroot/debian/perl-xcat.debhelper.log", + 'nor debhelper\'s per-build log' ); + ok( !-d "$pkgroot/debian/.debhelper", + 'nor its generated-state directory' ); + is( scalar @removed, 4, 'and every one is reported as removed' ); + + ok( -f "$pkgroot/debian/control", 'debian/control is left alone' ); + ok( -f "$pkgroot/debian/rules", 'debian/rules is left alone' ); + ok( -f "$pkgroot/debian/changelog", 'debian/changelog is left alone' ); + ok( -f "$pkgroot/debian/source/format", 'and so is the rest of debian/' ); + + is_deeply( [ clean_debian_residue($pkgroot) ], [], + 'a second call has nothing left to remove' ); + is_deeply( [ clean_debian_residue("$pkgroot/nonexistent") ], [], + 'and a package that was never built is not an error' ); +} + +# ------------------------------------------------- putting a file back as it was -- +# The build rewrites tracked files and restores them afterwards. A restore that +# loses the mode is invisible in a content diff and strips the exec bit off shipped +# scripts -- xCAT/postscripts/{bmcsetup,getipmi} are executable and are rewritten +# during the xCAT build. +{ + my $dir = tempdir( CLEANUP => 1 ); + my $script = "$dir/postscript"; + open my $fh, '>', $script or die $!; + print {$fh} "#!/bin/sh\noriginal\n"; + close $fh; + chmod 0755, $script or die $!; + + my $entry = backup_file($script); + ok( -f "$script.build.save", 'the original is set aside before the build edits it' ); + + open my $out, '>', $script or die $!; + print {$out} "rewritten by the build\n"; + close $out; + chmod 0644, $script; + + ok( restore_file($entry), 'and is put back afterwards' ); + open my $in, '<', $script or die $!; + my $restored = do { local $/; <$in> }; + close $in; + is( $restored, "#!/bin/sh\noriginal\n", 'with its original content' ); + is( ( stat $script )[2] & 07777, 0755, + 'and its original mode -- an executable must not come back unexecutable' ); + ok( !-e "$script.build.save", 'leaving no backup behind' ); + + is( backup_file("$dir/never-existed"), undef, + 'a file that is not there is not claimed' ); + is( restore_file(undef), 0, 'and restoring nothing is not an error' ); +} + done_testing(); diff --git a/xCAT-test/unit/buildrpms_source_only.t b/xCAT-test/unit/buildrpms_source_only.t index b82f22d9d..14ef4b5e0 100644 --- a/xCAT-test/unit/buildrpms_source_only.t +++ b/xCAT-test/unit/buildrpms_source_only.t @@ -30,7 +30,7 @@ my $source = read_text($builder); # rather than skip: if the extraction stops matching, this file would silently # cover nothing. my %routine; -for my $name (qw(index_repo write_repo_metadata_dir)) { +for my $name (qw(index_repo write_repo_metadata_dir buildall)) { my ($body) = $source =~ /\n(sub \Q$name\E \{.*?\n\})\n/s; BAIL_OUT("could not extract $name from buildrpms.pl") unless $body; $routine{$name} = $body; @@ -38,12 +38,18 @@ for my $name (qw(index_repo write_repo_metadata_dir)) { our @CREATEREPO; our @METADATA_WRITTEN; +our @STAGES; { package Scratch; no warnings 'redefine'; # Collaborators the lifted code calls. Each records instead of acting. sub createrepo_dir { push @main::CREATEREPO, $_[0]; } + # The four stages buildall drives. Each records that it was reached. + sub createmockconfig { push @main::STAGES, 'createmockconfig'; } + sub buildsources { push @main::STAGES, 'buildsources'; } + sub buildspkgs { push @main::STAGES, 'buildspkgs'; } + sub buildpkgs { push @main::STAGES, 'buildpkgs'; } } # %opts lives in the scratch package and is set directly. Aliasing it to a hash in @@ -57,6 +63,7 @@ my $harness = join "\n", # write_repo_metadata_dir does real work past the guard; stop it there so the # test observes the guard and nothing else. $routine{index_repo}, + $routine{buildall}, ($routine{write_repo_metadata_dir} =~ s/(return if \$opts\{source_only\};).*\n\}\z/$1\n push \@main::METADATA_WRITTEN, \$repodir;\n return 1;\n}/sr), '1;'; @@ -101,6 +108,30 @@ ok( !grep({ $_ eq 'BINARY' } @{ run_index(source_only => 1) }), 'a normal run still emits the repository metadata' ); } + +# ------------------------------------------------------------------ the build -- +# The point of the option: the source rpm is built and the binary rebuild is not. +# Without this, removing the guard from buildall leaves every other assertion in +# this file green -- the repository ones only observe what index_repo does. +sub stages_for { + my ($source_only) = @_; + %Scratch::opts = (source_only => $source_only); + local @STAGES = (); + Scratch::buildall('xCAT-vlan', 'alma+epel-9-x86_64'); + return [@STAGES]; +} + +is_deeply( stages_for(0), + [qw(createmockconfig buildsources buildspkgs buildpkgs)], + 'a normal run builds the source rpm and then rebuilds it into binaries' ); + +is_deeply( stages_for(1), + [qw(createmockconfig buildsources buildspkgs)], + 'a source-only run stops once the source rpm exists' ); + +ok( !grep( { $_ eq 'buildpkgs' } @{ stages_for(1) } ), + 'and never enters the binary rebuild, which is the expensive half' ); + # ------------------------------------------------------------------- the CLI -- # Run the real program. --source-only and --merge-core-repos are different modes: # one builds, the other assembles trees that are already built.