From 527fc409e59430aecfc02dc2bde2fea9785d6cea Mon Sep 17 00:00:00 2001 From: Chris Evich Date: Fri, 23 Sep 2022 12:12:25 -0400 Subject: Cirrus: Add golang code consistency check script Depends on #15893 Fixes: #15913 Signed-off-by: Chris Evich --- contrib/cirrus/check_go_changes.sh | 53 ++++++++++++++++++++++++++++++++++++++ contrib/cirrus/runner.sh | 20 +------------- 2 files changed, 54 insertions(+), 19 deletions(-) create mode 100755 contrib/cirrus/check_go_changes.sh diff --git a/contrib/cirrus/check_go_changes.sh b/contrib/cirrus/check_go_changes.sh new file mode 100755 index 000000000..3c35ce51a --- /dev/null +++ b/contrib/cirrus/check_go_changes.sh @@ -0,0 +1,53 @@ +#!/bin/bash + +set -eo pipefail + +# This script is intended to confirm new go code conforms to certain +# conventions and/or does not introduce use of old/deprecated packages +# or functions. It needs to run in the Cirrus CI environment, on behalf +# of PRs, via runner.sh. This ensures a consistent and predictable +# environment not easily reproduced by a `Makefile`. + +# shellcheck source=contrib/cirrus/lib.sh +source $(dirname $0)/lib.sh + +check_msg() { + msg "#####" # Cirrus-CI logs automatically squash empty lines + msg "##### $1" # Complains if $1 is empty +} + +# First arg is check description, second is regex to search $diffs for. +check_diffs() { + local check regex + check="$1" + regex="$2" + check_msg "Confirming changes have no $check" + req_env_vars check regex diffs + if egrep -q "$regex"<<<"$diffs"; then + # Show 5 context lines before/after as compromise for script simplicity + die "Found $check: +$(egrep -B 5 -A 5 "$regex"<<<"$diffs")" + fi +} + +if [[ -n "$CIRRUS_TAG" ]] || ! req_env_vars CIRRUS_CHANGE_IN_REPO CIRRUS_PR DEST_BRANCH +then + warn "Skipping: Golang code checks cannot run in this context" + exit 0 +fi + +base=$(git merge-base $DEST_BRANCH $CIRRUS_CHANGE_IN_REPO) +diffs=$(git diff $base $CIRRUS_CHANGE_IN_REPO -- '*.go' ':^vendor/') + +if [[ -z "$diffs" ]]; then + check_msg "There are no golang diffs to check between $base...$CIRRUS_CHANGE_IN_REPO" + exit 0 +fi + +check_diffs \ + "use of deprecated ioutil vs recommended io or os packages." \ + "^(\\+[^#]+io/ioutil)|(\\+.+ioutil\\..+)" + +check_diffs \ + "use of os.IsNotExists(err) vs recommended errors.Is(err, os.ErrNotExist)" \ + "^\\+[^#]*os\\.IsNotExists\\(" diff --git a/contrib/cirrus/runner.sh b/contrib/cirrus/runner.sh index 5b1bc8d5c..c44251e2f 100755 --- a/contrib/cirrus/runner.sh +++ b/contrib/cirrus/runner.sh @@ -233,25 +233,7 @@ function _run_consistency() { SUGGESTION="run 'make generate-bindings' and commit all changes" ./hack/tree_status.sh make completions SUGGESTION="run 'make completions' and commit all changes" ./hack/tree_status.sh - - if [[ -z "$CIRRUS_TAG" ]] && \ - req_env_vars CIRRUS_CHANGE_IN_REPO CIRRUS_PR DEST_BRANCH - then - local base diffs regex i - # Prevent this check from detecting itself - i=i - msg "#####" - msg "Verifying no change adds new calls to ${i}o/${i}outil." - base=$(git merge-base $DEST_BRANCH $CIRRUS_CHANGE_IN_REPO) - diffs=$(git diff $base $CIRRUS_CHANGE_IN_REPO -- '*.go' ':^vendor/') - regex=$(echo -e "^(\\+.+${i}o/${i}outil)|(\\+.+${i}outil\\..+)") - if egrep -q "$regex"<<<"$diffs"; then - die "Found attempted use of deprecated ${i}outils: -$(egrep -B 5 -A 5 "$regex"<<<"$diffs")" - fi - else - msg "Skipping check for ${i}o/${i}outil addition." - fi + $SCRIPT_BASE/check_go_changes.sh } function _run_build() { -- cgit v1.2.3-54-g00ecf