diff --git a/BuildUtils.pm b/BuildUtils.pm index 9e5a1ec7a..69e98dc51 100644 --- a/BuildUtils.pm +++ b/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; diff --git a/builddebs.pl b/builddebs.pl index b1aa61114..57b952292 100755 --- a/builddebs.pl +++ b/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'; diff --git a/perl-xCAT/modifyUtils b/perl-xCAT/modifyUtils index 1d7128265..448010f9b 100755 --- a/perl-xCAT/modifyUtils +++ b/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 diff --git a/xCAT-test/unit/build_utils.t b/xCAT-test/unit/build_utils.t index 652b90f7f..00f975308 100644 --- a/xCAT-test/unit/build_utils.t +++ b/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(); diff --git a/xCAT-test/unit/modify_utils_version.t b/xCAT-test/unit/modify_utils_version.t new file mode 100644 index 000000000..bfd77a230 --- /dev/null +++ b/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();