xCAT/xcat-core
2
0
Fork 0
mirror of https://github.com/xcat2/xcat-core.git synced 2026-09-04 12:07:56 +00:00
Code Issues Releases Wiki Activity

refactor(build): run-or-fail through a single sh_or_die

Browse Source 
The two builders wrote the same run-a-command-or-stop step in opposite
polarities. builddebs.pl used sh(...) == 0 or die; buildrpms.pl used
sh(...) and die, and also used the == 0 or die form for its sh_retry calls, so
both directions appeared in one file. The 'and die' spelling reads as though
the die is what happens next rather than what happens on failure, which is a
poor thing to have to re-read at every call site.

Add BuildUtils::sh_or_die and convert the fourteen plain sh() call sites to it.
The failure message now also carries the exit code, which every one of the old
spellings discarded -- a build that failed said only that a command failed, not
what it returned. The sh_retry sites keep their own form: retry is a different
operation and sh_retry is local to buildrpms.pl.

Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
This commit is contained in:
Daniel Hilst
2026-09-01 17:00:33 -03:00
parent 1f376f5dd0
commit 25c3efe6fd
4 changed files with 71 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files
BuildUtils.pm
+15 -1
View File
@@ -32,7 +32,7 @@ our @EXPORT_OK = qw(
lock_id_for take_build_lock
sh_quote clean_debian_residue git_revision
backup_file restore_file
sh usage
sh sh_or_die usage
rewrite_file write_script read_line
buildinfo_text
);
@@ -115,6 +115,20 @@ sub sh {
# pod2usage reads the POD of the running program, so each builder keeps its own
# help text while sharing the way it is printed and the status it exits with.
# Run a command and stop the build when it fails. The same operation was
# spelled in opposite polarities -- `sh(...) == 0 or die` in builddebs.pl,
# `sh(...) and die` in buildrpms.pl, which also used both -- and the `and die`
# form reads as though the die is what happens next rather than what happens on
# failure. One name, one direction, and the exit code lands in the message.
sub sh_or_die {
my ($cmd, $message) = @_;
my $rc = sh($cmd);
return 0 if $rc == 0;
$message = "FATAL: command failed: $cmd" unless defined $message;
$message =~ s/\n\z//;
die "$message (exit $rc)\n";
}
sub usage {
my (%args) = @_;
pod2usage(
builddebs.pl
+9 -9
View File
@@ -37,7 +37,7 @@ use BuildUtils qw(
pin_control_version rewrite_changelog_header
reprepro_distributions reprepro_options
lock_id_for take_build_lock sh_quote
sh usage rewrite_file write_script read_line buildinfo_text
sh sh_or_die usage rewrite_file write_script read_line buildinfo_text
);
# The xcat-core packages that ship as debs. xCAT-openbmc-py, xCAT-rmc and xCAT-release
@@ -223,17 +223,17 @@ sub build_package {
if ($text =~ /3\.0 \(quilt\)/) {
my $tar = "$ROOT/" . orig_tarball_name($pkg, $PKGVER);
unless (-f $tar) {
sh(sprintf('tar czf %s --exclude debian -C %s .',
sh_quote($tar), sh_quote($dir))) == 0
or die "FATAL: could not create $tar\n";
sh_or_die(sprintf('tar czf %s --exclude debian -C %s .',
sh_quote($tar), sh_quote($dir)),
"FATAL: could not create $tar\n");
}
}
}
my $arch_flag = $arch eq 'all' ? '' : " -a$arch";
my $quiet = $opts{verbose} ? '' : ' >/dev/null';
sh("cd " . sh_quote($dir) . " && dpkg-buildpackage -rfakeroot -uc -us$arch_flag$quiet") == 0
or die "FATAL: dpkg-buildpackage failed for $pkg ($arch)\n";
sh_or_die("cd " . sh_quote($dir) . " && dpkg-buildpackage -rfakeroot -uc -us$arch_flag$quiet",
"FATAL: dpkg-buildpackage failed for $pkg ($arch)\n");
});
return;
@@ -301,9 +301,9 @@ sub assemble_repo {
for my $deb (@debs) {
# A release that predates an architecture must not be handed its packages.
next if basename($deb) =~ /_(\w+)\.deb\z/ && $1 ne 'all' && !$ok{$1};
sh("cd " . sh_quote($repodir) . " && reprepro -b ./ includedeb "
. sh_quote($dist) . ' ' . sh_quote($deb)) == 0
or die "FATAL: reprepro could not add $deb to $dist\n";
sh_or_die("cd " . sh_quote($repodir) . " && reprepro -b ./ includedeb "
. sh_quote($dist) . ' ' . sh_quote($deb),
"FATAL: reprepro could not add $deb to $dist\n");
}
}
return scalar @debs;
buildrpms.pl
+27 -26
View File
@@ -43,8 +43,8 @@ use File::Slurper qw(read_text write_text);
use File::Temp qw(tempdir tempfile);
use FindBin qw($Bin);
use lib $Bin;
use BuildUtils qw(git_revision source_date_epoch sh usage buildinfo_text write_script
read_line);
use BuildUtils qw(git_revision source_date_epoch sh sh_or_die usage buildinfo_text
write_script read_line);
use Fcntl qw(:flock); # per-target build lock (concurrency guard; see main())
use Getopt::Long qw(GetOptions);
use POSIX qw(strftime);
@@ -342,14 +342,14 @@ sub buildsources_genesis_base($) {
remove_tree($staging_parent) if -e $staging_parent;
make_path("$staging_root/dracut_105");
sh(qq(cp -a "xCAT-genesis-builder/dracut_105" "$staging_root/"))
and die "Error copying dracut_105 sources";
sh_or_die(qq(cp -a "xCAT-genesis-builder/dracut_105" "$staging_root/"),
"Error copying dracut_105 sources");
cp "xCAT-genesis-builder/80-net-name-slot.rules",
"$staging_root/80-net-name-slot.rules";
unlink $support_tarball if -f $support_tarball;
sh(qq(tar --sort=name --owner=0 --group=0 --mtime="\@$SOURCE_DATE_EPOCH" -cjf "$support_tarball" -C "$staging_parent" xCAT-genesis-base-build-support))
and die "Error creating $support_tarball";
sh_or_die(qq(tar --sort=name --owner=0 --group=0 --mtime="\@$SOURCE_DATE_EPOCH" -cjf "$support_tarball" -C "$staging_parent" xCAT-genesis-base-build-support),
"Error creating $support_tarball");
remove_tree($staging_parent);
}
@@ -360,8 +360,8 @@ sub prepare_xcat_probe_source_tar {
my $helper_dir = "$staging_root/lib/perl/xCAT";
my $source_tarball = "$SOURCES/xCAT-probe-$VERSION.tar.gz";
sh(qq(cp -a "xCAT-probe" "$staging_root"))
and die "Error staging xCAT-probe sources";
sh_or_die(qq(cp -a "xCAT-probe" "$staging_root"),
"Error staging xCAT-probe sources");
remove_tree($helper_dir) if -e $helper_dir;
make_path($helper_dir);
@@ -379,8 +379,8 @@ sub prepare_xcat_probe_source_tar {
);
close $archive_fh;
sh(qq(tar --sort=name --owner=0 --group=0 --numeric-owner --mtime="\@$SOURCE_DATE_EPOCH" --use-compress-program="gzip -n" -cf "$archive_path" -C "$staging_parent" xCAT-probe))
and die "Error creating $source_tarball";
sh_or_die(qq(tar --sort=name --owner=0 --group=0 --numeric-owner --mtime="\@$SOURCE_DATE_EPOCH" --use-compress-program="gzip -n" -cf "$archive_path" -C "$staging_parent" xCAT-probe),
"Error creating $source_tarball");
chmod 0644, $archive_path;
rename $archive_path, $source_tarball;
@@ -661,9 +661,9 @@ sub setup_local_repos {
sub createrepo_dir {
my ($dir, $extra) = @_;
$extra //= '';
sh(qq(createrepo_c --update --database )
. qq(--revision "$SOURCE_DATE_EPOCH" --set-timestamp-to-revision $extra "$dir"))
and die "Failed to createrepo_c $dir\n";
sh_or_die(qq(createrepo_c --update --database )
. qq(--revision "$SOURCE_DATE_EPOCH" --set-timestamp-to-revision $extra "$dir"),
"Failed to createrepo_c $dir\n");
}
# A core repo dir holds binaries flat plus a SRPMS/ subdir carrying its own
@@ -727,15 +727,15 @@ sub sign_repo_dir {
say "Signing RPMs in $repodir";
my @bin = glob("$repodir/*.rpm");
if (@bin) {
sh(qq(rpmsign --define "%_gpg_name $key_name" --addsign )
. join(" ", map { qq("$_") } @bin))
and die "Failed to sign RPMs in $repodir";
sh_or_die(qq(rpmsign --define "%_gpg_name $key_name" --addsign )
. join(" ", map { qq("$_") } @bin),
"Failed to sign RPMs in $repodir");
}
my @src = glob("$repodir/SRPMS/*.src.rpm");
if (@src) {
sh(qq(rpmsign --define "%_gpg_name $key_name" --addsign )
. join(" ", map { qq("$_") } @src))
and die "Failed to sign SRPMs in $repodir/SRPMS";
sh_or_die(qq(rpmsign --define "%_gpg_name $key_name" --addsign )
. join(" ", map { qq("$_") } @src),
"Failed to sign SRPMs in $repodir/SRPMS");
}
# Regenerate both indexes (binary + SRPMS) after signing, before signing repomd.
@@ -747,10 +747,10 @@ sub sign_repo_dir {
next unless -f $repomd;
say "Signing $repomd";
unlink "$repomd.asc" if -f "$repomd.asc";
sh(qq(gpg -a --detach-sign --default-key "$key_name" "$repomd"))
and die "Failed to sign $repomd";
sh(qq(gpg -a --export "$key_name" > "$rd/repomd.xml.key"))
and die "Failed to export public key to $rd";
sh_or_die(qq(gpg -a --detach-sign --default-key "$key_name" "$repomd"),
"Failed to sign $repomd");
sh_or_die(qq(gpg -a --export "$key_name" > "$rd/repomd.xml.key"),
"Failed to export public key to $rd");
}
}
@@ -832,11 +832,12 @@ sub merge_core_repos {
die "FATAL: --merge-core-repos requires at least one --input-core-repos dir\n" unless @ins;
-d $_ or die "FATAL: --input-core-repos dir '$_' does not exist\n" for @ins;
sh(qq(rm -rf "$out")) and die "Failed to clean output dir '$out'\n";
sh_or_die(qq(rm -rf "$out"),
"Failed to clean output dir '$out'\n");
make_path($out);
for my $in (@ins) {
sh(qq(rsync -a --exclude 'repodata/' "$in/" "$out/"))
and die "Failed to rsync '$in' into '$out'\n";
sh_or_die(qq(rsync -a --exclude 'repodata/' "$in/" "$out/"),
"Failed to rsync '$in' into '$out'\n");
}
# Index, sign (when --gpg-sign), write the final repository metadata, then create the
xCAT-test/unit/build_utils.t
+20
View File
@@ -427,4 +427,24 @@ isnt( git_revision( git => sub { '' }, read_file => sub { '' } ), '',
like( $out, qr/\ARunning: true/, 'a verbose run echoes the command' );
}
# ---------------------------------------------------------- sh_or_die() --
# The same run-or-fail step was written `sh(...) == 0 or die` in one builder
# and `sh(...) and die` in the other -- which also used both spellings itself.
# Reversed polarities for one operation are easy to misread, so there is now a
# single name with a single direction.
{
is( BuildUtils::sh_or_die('true'), 0,
'a command that succeeds returns 0 and does not die' );
my $err = eval { BuildUtils::sh_or_die('sh -c "exit 4"', 'FATAL: it failed'); 1 }
? '' : $@;
like( $err, qr/FATAL: it failed/, 'a failure dies with the caller\'s message' );
like( $err, qr/exit 4/,
'and names the exit code, which the old spellings threw away' );
my $bare = eval { BuildUtils::sh_or_die('sh -c "exit 5"'); 1 } ? '' : $@;
like( $bare, qr/\Qsh -c "exit 5"\E/,
'a caller with no message still gets the command that failed' );
}
done_testing();