Skip to content

Commit

Permalink
CI: logformatter: link to correct PR base
Browse files Browse the repository at this point in the history
Two enormous misunderstandings:

  1) $CIRRUS_BASE_SHA is worthless. I thought it was, you know,
     the BASE SHA of the current commit, but (as best I can tell)
     it seems to be the SHA of the most recent commit on the
     destination branch. Cirrus docs are unhelpful. Anyhow,
     it's clearly not anything useful. Stop using it.

  2) $EPOCH_TEST_COMMIT is closer to what we want. It is
     defined in Makefile as the git merge-base. But for unknown
     reasons it was being clobbered in CI scripts, and it
     doesn't seem to work in all contexts, so, eliminate it
     from CI setup scripts. Leave it only in Makefile.

This leaves us with no option other than defining our own
merge-base variable, PR_BASE_SHA. Do so and pass it along
to rootless jobs.

Signed-off-by: Ed Santiago <[email protected]>
  • Loading branch information
edsantiago committed Jun 25, 2024
1 parent 7b4f6ec commit 1a6a200
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 30 deletions.
1 change: 0 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,6 @@ help: ## (Default) Print listing of key targets with their descriptions

.PHONY: lint
lint: golangci-lint
@echo "Linting vs commit '$(call err_if_empty,EPOCH_TEST_COMMIT)'"
ifeq ($(PRE_COMMIT),)
@echo "FATAL: pre-commit was not found, make .install.pre-commit to installing it." >&2
@exit 2
Expand Down
22 changes: 6 additions & 16 deletions contrib/cirrus/lib.sh
Original file line number Diff line number Diff line change
Expand Up @@ -71,30 +71,20 @@ export CI="${CI:-false}"
CIRRUS_CI="${CIRRUS_CI:-false}"
CONTINUOUS_INTEGRATION="${CONTINUOUS_INTEGRATION:-false}"
CIRRUS_REPO_NAME=${CIRRUS_REPO_NAME:-podman}
# Cirrus only sets $CIRRUS_BASE_SHA properly for PRs, but $EPOCH_TEST_COMMIT
# needs to be set from this value in order for `make validate` to run properly.
# When running get_ci_vm.sh, most $CIRRUS_xyz variables are empty. Attempt
# to accommodate both branch and get_ci_vm.sh testing by discovering the base
# branch SHA value.

# shellcheck disable=SC2154
if [[ -z "$CIRRUS_BASE_SHA" ]] && [[ -z "$CIRRUS_TAG" ]]
then # Operating on a branch, or under `get_ci_vm.sh`
showrun echo "branch or get_ci_vm (CIRRUS_BASE_SHA and CIRRUS_TAG are unset)"
CIRRUS_BASE_SHA=$(git rev-parse ${UPSTREAM_REMOTE:-origin}/$DEST_BRANCH)
elif [[ -z "$CIRRUS_BASE_SHA" ]]
then # Operating on a tag
showrun echo "operating on tag"
CIRRUS_BASE_SHA=$(git rev-parse HEAD)
if [[ -n "$CIRRUS_PR" ]] && [[ -z "$PR_BASE_SHA" ]]; then
# shellcheck disable=SC2154
PR_BASE_SHA=$(git merge-base ${DEST_BRANCH:-main} HEAD)
export PR_BASE_SHA
fi
# The starting place for linting and code validation
EPOCH_TEST_COMMIT="$CIRRUS_BASE_SHA"

# The next three values define regular expressions matching env. vars. necessary
# for all possible testing contexts (rootless, container, etc.). These values
# are consumed by the passthrough_envars() automation library function.
#
# List of envariables which must be EXACT matches
PASSTHROUGH_ENV_EXACT='CGROUP_MANAGER|DEST_BRANCH|DISTRO_NV|GOCACHE|GOPATH|GOSRC|NETWORK_BACKEND|OCI_RUNTIME|ROOTLESS_USER|SCRIPT_BASE|SKIP_USERNS|EC2_INST_TYPE|PODMAN_DB|STORAGE_FS|PODMAN_BATS_LEAK_CHECK'
PASSTHROUGH_ENV_EXACT='CGROUP_MANAGER|DEST_BRANCH|DISTRO_NV|GOCACHE|GOPATH|GOSRC|NETWORK_BACKEND|OCI_RUNTIME|PR_BASE_SHA|ROOTLESS_USER|SCRIPT_BASE|SKIP_USERNS|EC2_INST_TYPE|PODMAN_DB|STORAGE_FS|PODMAN_BATS_LEAK_CHECK'

# List of envariable patterns which must match AT THE BEGINNING of the name.
# Consumed by the passthrough_envars() automation library function.
Expand Down
11 changes: 7 additions & 4 deletions contrib/cirrus/logformatter
Original file line number Diff line number Diff line change
Expand Up @@ -886,10 +886,13 @@ END_SYNOPSIS
}
$s .= _tr("Logs", join(" / ", @logs));

# BASE_SHA can tell us if our parent includes--or doesn't--a purported
# fix for a flake. Note that "commits", plural, links to a git history
# listing; if we used "commit", singular, that would be less useful.
$s .= _tr("Base commit", _a("{CIRRUS_BASE_SHA}", "https://{CIRRUS_REPO_CLONE_HOST}/{CIRRUS_REPO_FULL_NAME}/commits/{CIRRUS_BASE_SHA}"));
# PR_BASE_SHA (set in lib.sh) can tell us if our parent includes--or
# doesn't--a purported fix for a flake. Note about the URL: "commits", plural,
# links to a git history listing; if we used "commit", singular, that would
# be less useful.
if (my $base = $ENV{PR_BASE_SHA}) {
$s .= _tr("Base commit", _a($base, "https://{CIRRUS_REPO_CLONE_HOST}/{CIRRUS_REPO_FULL_NAME}/commits/$base"));
}

$s .= "</table>\n";
return $s;
Expand Down
16 changes: 7 additions & 9 deletions contrib/cirrus/runner.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,8 @@ source $(dirname $0)/lib.sh
showrun echo "starting"

function _run_validate() {
# git-validation tool fails if $EPOCH_TEST_COMMIT is empty
# shellcheck disable=SC2154
if [[ -n "$EPOCH_TEST_COMMIT" ]]; then
showrun make validate
else
warn "Skipping git-validation since \$EPOCH_TEST_COMMIT is empty"
fi
showrun make validate

# make sure PRs have tests
showrun make tests-included
}
Expand Down Expand Up @@ -257,7 +252,8 @@ function _run_altbuild() {
context_dir=$(mktemp -d --tmpdir make-size-check.XXXXXXX)
savedhead=$(git rev-parse HEAD)
# Push to PR base. First run of the script will write size files
pr_base=$(git merge-base origin/$DEST_BRANCH HEAD)
# shellcheck disable=SC2154
pr_base=$PR_BASE_SHA
showrun git checkout $pr_base
showrun hack/make-and-check-size $context_dir
# pop back to PR, and run incremental makes. Subsequent script
Expand Down Expand Up @@ -457,7 +453,9 @@ function _bail_if_test_can_be_skipped() {
# Defined by Cirrus-CI for all tasks
# shellcheck disable=SC2154
head=$CIRRUS_CHANGE_IN_REPO
base=$(git merge-base $DEST_BRANCH $head)
# shellcheck disable=SC2154
base=$PR_BASE_SHA
echo "_bail_if_test_can_be_skipped: head=$head base=$base"
diffs=$(git diff --name-only $base $head)

# If PR touches any files in an argument directory, we cannot skip
Expand Down

0 comments on commit 1a6a200

Please sign in to comment.