From 23de3801d64706136a025d8fb7ee40442a40dffa Mon Sep 17 00:00:00 2001 From: Ed Santiago Date: Wed, 27 Apr 2022 06:24:03 -0600 Subject: buildah-vendor-treadmill script: yet more checks More safety checks for the treadmill script: * for --sync: - issue warning if HEAD is not a vendor commit - if run-buildah-bud-tests fails, leave the working dir for user to investigate. And offer a long helpful warning. - tweak .cirrus.yml so buildah-bud tests run early, so we can fail early. (Remember, the top commit will never ever ever ever be merged) * for --pick: - check branch merge-base (of your vendor-update branch), compare against that of the treadmill PR. If treadmill is newer, bail, and suggest rebasing. This would've saved us some time in #14005. Signed-off-by: Ed Santiago --- hack/buildah-vendor-treadmill | 203 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 189 insertions(+), 14 deletions(-) (limited to 'hack') diff --git a/hack/buildah-vendor-treadmill b/hack/buildah-vendor-treadmill index 1feffaa60..dddabef93 100755 --- a/hack/buildah-vendor-treadmill +++ b/hack/buildah-vendor-treadmill @@ -24,6 +24,10 @@ our $VERSION = '0.1'; ############################################################################### # BEGIN user-customizable section +# Page describing this process in much more detail +our $Docs_URL = + 'https://github.com/containers/podman/wiki/Buildah-Vendor-Treadmill'; + # github path to buildah our $Buildah = 'github.com/containers/buildah'; @@ -72,7 +76,7 @@ Call me with one of two options: For latest documentation and best practices, please see: - https://github.com/containers/podman/wiki/Buildah-Vendor-Treadmill + $Docs_URL OPTIONS: @@ -86,7 +90,8 @@ END_USAGE # Command-line options. Note that this operates directly on @ARGV ! our %action; our $debug = 0; -our $force = 0; +our $force_old_main = 0; # in --pick, proceeds even if main is old +our $force_testing = 0; # in --sync, test even no podman/buildah changes our $verbose = 0; our $NOT = ''; # print "blahing the blah$NOT\n" if $debug sub handle_opts { @@ -95,9 +100,11 @@ sub handle_opts { 'sync' => sub { $action{sync}++ }, 'pick' => sub { $action{pick}++ }, + 'force-old-main' => \$force_old_main, + 'force-testing' => \$force_testing, + 'debug!' => \$debug, 'dry-run|n!' => sub { $NOT = ' [NOT]' }, - 'force' => \$force, 'verbose|v' => \$verbose, help => \&usage, @@ -161,6 +168,13 @@ sub do_sync { die "$ME: HEAD is a buildah commit, but HEAD^ is not a treadmill commit! Cannot continue.\n"; } } + else { + # This can happen if a previous script run got interrupted before + # committing a new buildah; or when a new buildah has been vendored + # into podman and our go.mod version == latest on buildah. + warn "$ME: Warning: HEAD is not a buildah vendor commit; will try to continue anyway\n"; + } + # HEAD must now be a treadmill commit is_treadmill_commit('HEAD') or die "$ME: HEAD is not a treadmill commit!\n"; @@ -168,7 +182,7 @@ sub do_sync { # HEAD is now a change to buildah-tests. Now update main and rebase. pull_main(); git('checkout', '-q', $current_branch); - my $forkpoint = git('merge-base', '--fork-point', 'main'); + my $forkpoint = git_forkpoint(); my $main_commit = git('rev-parse', 'main'); my $rebased; if ($forkpoint eq $main_commit) { @@ -195,6 +209,10 @@ sub do_sync { or die "$ME: make vendor failed\n"; my $buildah_new = vendored_buildah(); print "-> buildah new = $buildah_new\n"; + + # Tweak .cirrus.yml so we run bud tests first. Fail fast. + tweak_cirrus_test_order(); + git('commit', '-as', '-m', <<"END_COMMIT_MESSAGE"); [DO NOT MERGE] vendor in buildah \@ $buildah_new @@ -208,10 +226,19 @@ END_COMMIT_MESSAGE my $change_message = ''; if ($buildah_new eq $buildah_old) { if (! $rebased) { - progress("Nothing has changed (same buildah, same podman). Bye!"); - exit 0; + $change_message = "Nothing has changed (same buildah, same podman)."; + if ($force_testing) { + $change_message .= " Testing anyway due to --force-testing."; + } + else { + progress($change_message); + progress("Not much point to testing this, but use --force-testing to continue."); + exit 0; + } + } + else { + $change_message = "Podman has bumped, but Buildah is unchanged. There's probably not much point to testing this."; } - $change_message = "Podman has bumped, but Buildah is unchanged. There's probably not much point to testing this."; } else { my $samenew = ($rebased ? 'new' : 'same'); @@ -243,6 +270,58 @@ sub pull_main { git('pull', '-r', git_upstream(), 'main'); } +############################# +# tweak_cirrus_test_order # Run bud tests first, to fail fast & early +############################# +sub tweak_cirrus_test_order { + my $cirrus_yml = '.cirrus.yml'; + my $tmpfile = "$cirrus_yml.tmp.$$"; + unlink $tmpfile; + + progress("Tweaking test order in $cirrus_yml to run bud tests early"); + open my $in, '<', $cirrus_yml + or do { + warn "$ME: Cannot read $cirrus_yml: $!\n"; + warn "$ME: Will continue anyway\n"; + return; + }; + open my $out, '>'. $tmpfile + or die "$ME: Cannot create $tmpfile: $!\n"; + my $current_task = ''; + my $in_depend; + while (my $line = <$in>) { + chomp $line; + if ($line =~ /^(\S+)_task:$/) { + $current_task = $1; + undef $in_depend; + } + elsif ($line =~ /^(\s+)depends_on:$/) { + $in_depend = $1; + } + elsif ($in_depend && $line =~ /^($in_depend\s+-\s+)(\S+)/) { + if ($current_task eq 'buildah_bud_test') { + # Buildah bud test now depends on validate, so it runs early + $line = "${1}validate"; + } + elsif ($2 eq 'validate' && $current_task ne 'success') { + # Other tests that relied on validate, now rely on bud instead + $line = "${1}buildah_bud_test"; + } + } + else { + undef $in_depend; + } + + print { $out } $line, "\n"; + } + close $in; + close $out + or die "$ME: Error writing $tmpfile: $!\n"; + chmod 0644 => $tmpfile; + rename $tmpfile => $cirrus_yml + or die "$ME: Could not rename $tmpfile: $!\n"; +} + ############################ # build_and_check_podman # Run quick (local) sanity checks before pushing ############################ @@ -262,13 +341,41 @@ sub build_and_check_podman { # the name of the directory created by the bud-tests script. progress("Confirming that buildah-bud-tests patches still apply..."); system('rm -rf test-buildah-*'); - $errs += system('test/buildah-bud/run-buildah-bud-tests', '--no-test'); - # Clean up - system('rm -rf test-buildah-*'); + if (system('test/buildah-bud/run-buildah-bud-tests', '--no-test')) { + # Error + ++$errs; + warn "$ME: Leaving test-buildah- directory for you to investigate\n"; + } + else { + # Patches apply cleanly. Clean up + system('rm -rf test-buildah-*'); + } return if !$errs; - warn "$ME: Errors found. Please address, then add to HEAD^ commit\n"; - die " ...see $ME --help for more information.\n"; + warn <<"END_WARN"; +$ME: Errors found. I have to stop now for you to fix them. + Your best bet now is: + 1) Find and fix whatever needs to be fixed; then + 2) git commit -am'fixme-fixme'; then + 3) git rebase -i main: + a) you are now in an editor window + b) move the new fixme-fixme commit up a line, to between the + 'buildah vendor treadmill' and 'vendor in buildah @ ...' lines + c) change 'pick' to 'squash' (or just 's') + d) save & quit to continue the rebase + e) back to a new editor window + f) change the commit message: remove fixme-fixme, add a description + of what you actually fixed. If possible, reference the PR (buildah + or podman) that introduced the failure + g) save & quit to continue the rebase + + Now, for good measure, rerun this script. + + For full documentation, refer to + + $Docs_URL +END_WARN + exit 1; } # END sync and its helpers @@ -288,7 +395,11 @@ sub do_pick { my $treadmill_pr = treadmill_pr(); my $treadmill_branch = "$ME/pr$treadmill_pr/tmp$$"; progress("Fetching treadmill PR $treadmill_pr into $treadmill_branch"); - git('fetch', git_upstream(), "pull/$treadmill_pr/head:$treadmill_branch"); + git('fetch', '-q', git_upstream(), "pull/$treadmill_pr/head:$treadmill_branch"); + + # Compare merge bases of our branch and the treadmill one + progress("Checking merge bases"); + check_merge_bases($treadmill_pr, $treadmill_branch); # read buildah go.mod from it, and from current tree, and compare my $buildah_on_treadmill = vendored_buildah($treadmill_branch); @@ -309,7 +420,7 @@ sub do_pick { build_and_check_podman(); - progress("Looks good! Please 'git commit --amend' before pushing."); + progress("Looks good! Please 'git commit --amend' and edit commit message before pushing."); } ################## @@ -383,6 +494,51 @@ END_QUERY return $prs[0]{node}{number}; } +####################### +# check_merge_bases # It's OK if our branch is newer than treadmill +####################### +sub check_merge_bases { + my $treadmill_pr = shift; # e.g., 12345 + my $treadmill_branch = shift; # e.g., b-v-p/pr12345/tmpNNN + + # Fetch latest main, for accurate comparison + git('fetch', '-q', git_upstream(), 'main'); + + my $forkpoint_cur = git_forkpoint(); + my $forkpoint_treadmill = git_forkpoint($treadmill_branch); + + print "fork cur: $forkpoint_cur\nfork tm: $forkpoint_treadmill\n" + if $debug; + if ($forkpoint_cur eq $forkpoint_treadmill) { + progress("Nice. This branch is up-to-date wrt treadmill PR $treadmill_pr"); + return; + } + + # They differ. + if (git_is_ancestor($forkpoint_cur, $forkpoint_treadmill)) { + warn <<"END_WARN"; +$ME: treadmill PR $treadmill_pr is based on + a newer main than this branch. This means it might have + more up-to-date patches. + +END_WARN + + if ($force_old_main) { + warn "$ME: Proceeding due to --force-old-main\n"; + return; + } + + # Cannot continue. Clean up side branch, and bail. + git('branch', '-D', $treadmill_branch); + warn "$ME: You might want to consider rebasing on latest main.\n"; + warn "$ME: Aborting. Use --force-old-main to continue without rebasing.\n"; + exit 1; + } + else { + progress("Your branch is based on a newer main than treadmill PR $treadmill_pr. This is usually OK."); + } +} + ################# # cherry_pick # cherry-pick a commit, updating its commit message ################# @@ -474,6 +630,25 @@ sub git_current_branch() { return $b; } +################### +# git_forkpoint # Hash at which branch (default: cur) branched from main +################### +sub git_forkpoint { + return git('merge-base', '--fork-point', 'main', @_); +} + +##################### +# git_is_ancestor # Is hash1 an ancestor of hash2? +##################### +sub git_is_ancestor { + # Use system(), not git(), because we don't want to abort on exit status + my $rc = system('git', 'merge-base', '--is-ancestor', @_); + die "$ME: Cannot continue\n" if $? > 256; # e.g., Not a valid object + + # Translate shell 0/256 status to logical 1/0 + return !$rc; +} + ################## # git_upstream # Name of true github upstream ################## -- cgit v1.2.3-54-g00ecf