fix(build): stamp the version into the deb packages instead of shipping placeholders

An xCAT installed from these debs could report no version at all: `lsxcatd -v`
printed a bare "Version" and there was no /opt/xcat/share/xcat/version to
consult.

xCAT::Version ships with #XCATVERSIONSUBHERE / #XCATSVNBUILDSUBHERE
placeholders that perl-xCAT/debian/rules fills by calling modifyUtils with the
version and the commit. It takes the commit from ../Gitinfo, falling back to
`git log`. Neither Debian builder wrote Gitinfo -- buildrpms.pl does, which is
why only the deb side was affected -- so a tree without a readable .git left
that argument empty. modifyUtils then hit `if [ -z "$2" ]` and returned 0
WITHOUT substituting, and neither caller checks the status, so the placeholders
shipped.

Two independent things had to be wrong for this to be silent, and both are
fixed:

modifyUtils now exits non-zero on a missing argument, so a build that cannot
stamp a version fails instead of producing one that cannot identify itself. Its
message was also wrong -- it checked $2 and complained about the version, which
is $1 -- so each argument now names itself.

builddebs.pl writes Gitinfo, as buildrpms.pl already does. BuildUtils::
git_revision prefers the checkout, falls back to an existing Gitinfo (a source
export carries the real revision that way, and overwriting it with a
placeholder would discard the only provenance the tree has), and only then
returns "unknown" -- never the empty string modifyUtils ignores.

"unknown" is now warned about rather than stamped quietly. The cause is usually
not a missing .git but git refusing one it considers dubiously owned: the tree
belongs to another user and the safe.directory exception sits in a config that
the build's own HOME override hides. That is exactly how this went unnoticed.

Not a regression -- build-ubunturepo produces byte-identical unsubstituted
placeholders in the same .git-less tree, verified by running it there. It is a
pre-existing hole that CD masks by writing Gitinfo in the pipeline.

Verified on xcat-master-ub: with a readable checkout the deb carries
'Version ' . '2.19.0' . ' (git commit 0030133c89)';
without one it carries '(git commit unknown)' and the build prints the warning
naming the tree, the command to run and the HOME to run it under. Nine
assertions drive modifyUtils itself -- stamping both forms, and each missing
argument -- and dropping either sed, or restoring the silent exit, reddens it.

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

BuildUtils.pm

@@ -27,7 +27,7 @@ our @EXPORT_OK = qw(
     pin_control_version rewrite_changelog_header
     reprepro_distributions reprepro_options
     lock_id_for take_build_lock
-    sh_quote clean_debian_residue
+    sh_quote clean_debian_residue git_revision
     backup_file restore_file
 );
 
@@ -77,6 +77,36 @@ sub default_dists { return @DEFAULT_DISTS; }
 # 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.
+# git_revision: the commit the packages are built from.
+#
+# This is not cosmetic. perl-xCAT/debian/rules and perl-xCAT.spec both pass it to
+# modifyUtils, which substitutes it and the version into xCAT::Version. Hand
+# modifyUtils an empty string and it does nothing, and the built package reports no
+# version at all -- `lsxcatd -v` prints a bare "Version". So a revision is always
+# produced: the git checkout when there is one, an existing Gitinfo when there is
+# not (a source export carries the real revision that way, and clobbering it with
+# a placeholder would throw away the only provenance the tree has), and only then
+# the "unknown" placeholder.
+sub git_revision {
+    my (%args) = @_;
+    my $run       = $args{git}       || sub { `git rev-parse HEAD 2>/dev/null` };
+    my $read_file = $args{read_file} || sub {
+        return unless -f 'Gitinfo';
+        open my $fh, '<', 'Gitinfo' or return;
+        my $line = <$fh>;
+        close $fh;
+        return $line;
+    };
+
+    for my $source ($run, $read_file) {
+        my $rev = $source->();
+        next unless defined $rev;
+        $rev =~ s/\s+\z//;
+        return $rev if length $rev;
+    }
+    return 'unknown';
+}
+
 sub backup_file {
     my ($path) = @_;
     return unless defined $path && -f $path;

builddebs.pl

@@ -32,7 +32,7 @@ use BuildUtils qw(
     stage_probe_helpers XCAT_PROBE_HELPERS
     deb_package_arches dist_arches default_dists
     orig_tarball_name upstream_version resolve_dest clean_debian_residue
-    backup_file restore_file
+    backup_file restore_file git_revision
     pin_control_version rewrite_changelog_header
     reprepro_distributions reprepro_options
     lock_id_for take_build_lock sh_quote
@@ -118,6 +118,28 @@ my $RELEASE = $opts{release} || $FILE_RELEASE || snap_release($EPOCH);
 my $PKGVER  = deb_version($VERSION, $RELEASE);
 $ENV{SOURCE_DATE_EPOCH} = $EPOCH;
 
+# Gitinfo is what perl-xCAT/debian/rules hands modifyUtils as the commit. Without it
+# modifyUtils does nothing at all and the built xCAT reports no version -- `lsxcatd -v`
+# prints a bare "Version" -- so it is written here rather than left to debian/rules'
+# `git log` fallback, which produces nothing when the tree has no .git (a source
+# export, or a checkout that was copied rather than cloned).
+my $GITINFO = git_revision();
+if ($GITINFO eq 'unknown') {
+    # Say so. The usual cause is not a missing .git but git refusing to read one it
+    # considers dubiously owned -- the tree belongs to another user, and the
+    # safe.directory exception lives in a config that the build's own HOME hides.
+    # Silence here is how packages end up stamped with a provenance nobody notices.
+    warn "WARNING: no git revision for $ROOT; packages will be stamped "
+       . "'(git commit unknown)'.\n"
+       . "         If $ROOT is a git checkout, check `git -C $ROOT rev-parse HEAD` "
+       . "as the build user (HOME=$ENV{HOME}).\n";
+}
+{
+    open my $g, '>', "$ROOT/Gitinfo" or die "Cannot write Gitinfo: $!\n";
+    print {$g} "$GITINFO\n";
+    close $g;
+}
+
 # dpkg reads these for the changelog trailer. Fixed, so the packages do not carry
 # whoever happened to run the build.
 $ENV{DEBFULLNAME} = 'xCAT Build';

perl-xCAT/modifyUtils

@@ -1,10 +1,21 @@
 #!/bin/sh
 # Put the version and git commit into the Version function in Version.pm
 
+# Both arguments are required, and a missing one is fatal. Returning 0 here meant a
+# build with no git information silently produced an xCAT that cannot report its own
+# version -- the substitution below never ran, xCAT::Version kept its placeholders,
+# and `lsxcatd -v` printed a bare "Version". Neither caller checked the status, so
+# the package shipped that way. Fail instead, and say which argument is missing.
+if [ -z "$1" ]
+	then
+	echo "modifyUtils:  Error:  must specify the xCAT version as the first argument" >&2
+	exit 1
+fi
+
 if [ -z "$2" ]
 	then
-	echo "modifyUtils:  Error:  must specify the xCAT version as an argument" >&2
-	exit
+	echo "modifyUtils:  Error:  must specify the git commit as the second argument" >&2
+	exit 1
 fi
 
 VER=$1

xCAT-test/unit/build_utils.t

@@ -25,7 +25,7 @@ use BuildUtils qw(
     orig_tarball_name upstream_version resolve_dest
     pin_control_version rewrite_changelog_header
     reprepro_distributions reprepro_options sh_quote clean_debian_residue
-    backup_file restore_file
+    backup_file restore_file git_revision
 );
 
 # ------------------------------------------------------------------- versions --
@@ -286,4 +286,26 @@ is( sh_quote(undef), q{''}, 'undef quotes to the empty string' );
     is( restore_file(undef), 0, 'and restoring nothing is not an error' );
 }
 
+# ------------------------------------------------------- the commit being built --
+# modifyUtils does nothing when handed an empty commit, and the package then reports
+# no version at all, so a revision must always come out of here.
+is( git_revision( git => sub { "deadbeefcafe\n" }, read_file => sub { 'from-gitinfo' } ),
+    'deadbeefcafe',
+    'the checkout is asked first, and its answer is trimmed' );
+
+is( git_revision( git => sub { '' }, read_file => sub { "from-gitinfo\n" } ),
+    'from-gitinfo',
+    'a tree with no .git falls back to the Gitinfo the export carries' );
+
+is( git_revision( git => sub { undef }, read_file => sub { undef } ),
+    'unknown',
+    'and with neither, a placeholder -- never the empty string modifyUtils ignores' );
+
+is( git_revision( git => sub { "\n" }, read_file => sub { "  \n" } ),
+    'unknown',
+    'whitespace-only answers count as no answer' );
+
+isnt( git_revision( git => sub { '' }, read_file => sub { '' } ), '',
+    'the one thing it must never return is empty' );
+
 done_testing();

xCAT-test/unit/modify_utils_version.t

@@ -0,0 +1,95 @@
+#!/usr/bin/env perl
+# modifyUtils stamps the version and commit into xCAT::Version.
+#
+# It is run, not read: the script is copied into a scratch tree with a stand-in
+# Version.pm carrying the real placeholders, and the assertions are about the file
+# that comes out. The behaviour that matters is the failure path -- modifyUtils used
+# to return 0 when handed no commit, doing nothing, and neither caller
+# (perl-XCAT/debian/rules, perl-xCAT.spec) checks the status, so the package shipped
+# with its placeholders intact and `lsxcatd -v` printed a bare "Version".
+use strict;
+use warnings;
+
+use File::Copy qw(copy);
+use File::Path qw(make_path);
+use File::Spec;
+use File::Temp qw(tempdir);
+use FindBin;
+use lib "$FindBin::Bin/../lib";
+use Test::More;
+
+use XCAT::Test::File qw(repo_path);
+
+my $script = repo_path('perl-xCAT/modifyUtils');
+plan skip_all => 'modifyUtils not found' unless -r $script;
+
+# The real placeholders, as xCAT::Version ships them.
+my $TEMPLATE = <<'PM';
+sub Version
+{
+    my $version = shift;
+    if ($version eq 'short')
+    {
+        $version = ''    #XCATVERSIONSUBHERE ;
+    }
+    else
+    {
+        $version = 'Version '    #XCATVERSIONSUBHERE #XCATSVNBUILDSUBHERE ;
+    }
+    return $version;
+}
+PM
+
+# modifyUtils picks its target from /etc/debian_version, which differs between the
+# build hosts and CI. Stage BOTH candidates so the test asserts the same behaviour
+# wherever it runs, and read back whichever one it chose.
+sub run_modify {
+    my (@args) = @_;
+    my $dir = tempdir(CLEANUP => 1);
+    copy($script, "$dir/modifyUtils") or die "cannot stage modifyUtils: $!";
+    chmod 0755, "$dir/modifyUtils";
+    for my $rel ('xCAT', 'debian/perl-xcat/opt/xcat/lib/perl/xCAT') {
+        make_path("$dir/$rel");
+        open my $fh, '>', "$dir/$rel/Version.pm" or die $!;
+        print {$fh} $TEMPLATE;
+        close $fh;
+    }
+
+    my $out = qx(cd \Q$dir\E && ./modifyUtils @{[ join ' ', map { "'$_'" } @args ]} 2>&1);
+    my $rc  = $? >> 8;
+
+    my $stamped = '';
+    for my $rel ('xCAT', 'debian/perl-xcat/opt/xcat/lib/perl/xCAT') {
+        open my $fh, '<', "$dir/$rel/Version.pm" or next;
+        my $text = do { local $/; <$fh> };
+        close $fh;
+        $stamped = $text if $text !~ /XCATVERSIONSUBHERE/;
+    }
+    return { rc => $rc, out => $out, stamped => $stamped };
+}
+
+# ------------------------------------------------------------------ the happy path --
+my $ok = run_modify('2.19.0', 'abc123def456');
+is( $ok->{rc}, 0, 'a version and a commit are stamped without error' );
+like( $ok->{stamped}, qr/\Q'Version '\E\s*\. '2\.19\.0' \. ' \(git commit abc123def456\)'/,
+    'the long form carries the version and the commit it was built from' );
+like( $ok->{stamped}, qr/\$version = ''\s*\. '2\.19\.0'/,
+    "and the 'short' form carries the bare version" );
+unlike( $ok->{stamped}, qr/XCATVERSIONSUBHERE|XCATSVNBUILDSUBHERE/,
+    'no placeholder survives a successful stamp' );
+
+# -------------------------------------------------------------- the failure path --
+# The whole point: a missing argument must stop the build, not pass silently.
+my $no_commit = run_modify('2.19.0', '');
+isnt( $no_commit->{rc}, 0,
+    'a missing commit fails instead of shipping an unstamped package' );
+like( $no_commit->{out}, qr/git commit/,
+    'and says which argument is missing' );
+is( $no_commit->{stamped}, '',
+    'and stamps nothing, so the failure cannot be mistaken for a partial write' );
+
+my $no_version = run_modify('', 'abc123def456');
+isnt( $no_version->{rc}, 0, 'a missing version fails too' );
+like( $no_version->{out}, qr/version/, 'and says so' );
+
+done_testing();