Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CI: logformatter: link to correct PR base #23081

Merged
merged 1 commit into from
Jun 25, 2024
+20 −30
Merged

CI: logformatter: link to correct PR base #23081

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Makefile
View file
Open in desktop
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)'"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As best I can tell, PRE_COMMIT does not use EPOCH_TEST_COMMIT. This echo was added in #5066, back in February 2020, but I think it was a mistake.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure pre-cmmit doesn't. I know vbatts/gitvalidate does use it though (in case that matters here).

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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

debugging printf. Does anyone mind if I leave it in for a little while?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fine IMO

diffs=$(git diff --name-only $base $head)

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