From aee841a0f0cc7fef131e4dbd108fe463b0c44463 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Wed, 15 Jan 2025 14:51:48 +0100 Subject: [PATCH] hack: backport apidiff.sh This makes the script identical to current master (f3cbd79db7f0c86a2d3602fdff6b174543d2cf1c). This is needed because pull-kubernetes-apidiff-client-go is the same for all branches and assumes that the script automatically determines the diff based on Prow env variables. --- hack/apidiff.sh | 235 ++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 187 insertions(+), 48 deletions(-) diff --git a/hack/apidiff.sh b/hack/apidiff.sh index f00c19d40ad..57b47614590 100755 --- a/hack/apidiff.sh +++ b/hack/apidiff.sh @@ -14,10 +14,9 @@ # See the License for the specific language governing permissions and # limitations under the License. -# This script checks the coding style for the Go language files using -# golangci-lint. Which checks are enabled depends on command line flags. The -# default is a minimal set of checks that all existing code passes without -# issues. +# This script analyzes API changes between specified revisions this repository. +# It uses the apidiff tool to detect differences, reports incompatible changes, and optionally +# builds downstream projects to assess the impact of those changes. usage () { cat <&2 @@ -26,6 +25,11 @@ Usage: $0 [-r ] [directory ...]" Default is the current working tree instead of a revision. -r : Report change in code added since this revision. Default is the common base of origin/master and HEAD. + -b Build all packages in that directory after replacing + Kubernetes dependencies with the current content of the + staging repo. May be given more than once. Must be an + absolute path. + WARNING: this will modify the go.mod in that directory. [directory]: Check one or more specific directory instead of everything. EOF exit 1 @@ -40,7 +44,8 @@ source "${KUBE_ROOT}/hack/lib/init.sh" base= target= -while getopts "r:t:" o; do +builds=() +while getopts "r:t:b:" o; do case "${o}" in r) base="${OPTARG}" @@ -58,6 +63,14 @@ while getopts "r:t:" o; do usage fi ;; + b) + if [ ! "${OPTARG}" ]; then + echo "ERROR: -${o} needs a non-empty parameter" >&2 + echo >&2 + usage + fi + builds+=("${OPTARG}") + ;; *) usage ;; @@ -65,36 +78,26 @@ while getopts "r:t:" o; do done shift $((OPTIND - 1)) -# Check specific directory or everything. -targets=("$@") -if [ ${#targets[@]} -eq 0 ]; then - # This lists all entries in the go.work file as absolute directory paths. - kube::util::read-array targets < <(go list -f '{{.Dir}}' -m) +# default from prow env if unset from args +# https://docs.prow.k8s.io/docs/jobs/#job-environment-variables +# TODO: handle batch PR testing + +if [[ -z "${target:-}" && -n "${PULL_PULL_SHA:-}" ]]; then + target="${PULL_PULL_SHA}" fi - -# Sanitize paths: -# - We need relative paths because we will invoke apidiff in -# different work trees. -# - Must start with a dot. -for (( i=0; i<${#targets[@]}; i++ )); do - d="${targets[i]}" - d=$(realpath -s --relative-to="$(pwd)" "${d}") - if [ "${d}" != "." ]; then - # sub-directories have to have a leading dot. - d="./${d}" - fi - targets[i]="${d}" -done - -# Must be a something that git can resolve to a commit. +# target must be a something that git can resolve to a commit. # "git rev-parse --verify" checks that and prints a detailed # error. -if [ -n "${target}" ]; then +if [[ -n "${target}" ]]; then target="$(git rev-parse --verify "${target}")" fi -# Determine defaults. -if [ -z "${base}" ]; then +if [[ -z "${base}" && -n "${PULL_BASE_SHA:-}" && -n "${PULL_PULL_SHA:-}" ]]; then + if ! base="$(git merge-base "${PULL_BASE_SHA}" "${PULL_PULL_SHA}")"; then + echo >&2 "Failed to detect base revision correctly with prow environment variables." + exit 1 + fi +elif [[ -z "${base}" ]]; then if ! base="$(git merge-base origin/master "${target:-HEAD}")"; then echo >&2 "Could not determine default base revision. -r must be used explicitly." exit 1 @@ -102,6 +105,20 @@ if [ -z "${base}" ]; then fi base="$(git rev-parse --verify "${base}")" +# Check specific directory or everything. +targets=("$@") +if [ ${#targets[@]} -eq 0 ]; then + shopt -s globstar + # Modules are discovered by looking for go.mod rather than asking go + # to ensure that modules that aren't part of the workspace and/or are + # not dependencies are checked too. + # . and staging are listed explicitly here to avoid _output + for module in ./go.mod ./staging/**/go.mod; do + module="${module%/go.mod}" + targets+=("$module") + done +fi + # Give some information about what's happening. Failures from "git describe" are ignored # silently, that's optional information. describe () { @@ -136,51 +153,80 @@ output_name () { # run invokes apidiff once per target and stores the output # file(s) in the given directory. +# +# shellcheck disable=SC2317 # "Command appears to be unreachable" - gets called indirectly. run () { out="$1" mkdir -p "$out" for d in "${targets[@]}"; do - apidiff -m -w "${out}/$(output_name "${d}")" "${d}" + if ! [ -d "${d}" ]; then + echo "module ${d} does not exist, skipping ..." + continue + fi + # cd to the path for modules that are intree but not part of the go workspace + # per example staging/src/k8s.io/code-generator/examples + ( + cd "${d}" + apidiff -m -w "${out}/$(output_name "${d}")" . + ) & done + wait } -# runWorktree checks out a specific revision, then invokes run there. -runWorktree () { - local out="$1" - local worktree="$2" - local rev="$3" +# inWorktree checks out a specific revision, then invokes the given +# command there. +# +# shellcheck disable=SC2317 # "Command appears to be unreachable" - gets called indirectly. +inWorktree () { + local worktree="$1" + shift + local rev="$1" + shift # Create a copy of the repo with the specific revision checked out. - git worktree add -f -d "${worktree}" "${rev}" - # Clean up the copy on exit. - kube::util::trap_add "git worktree remove -f ${worktree}" EXIT + # Might already have been done before. + if ! [ -d "${worktree}" ]; then + git worktree add -f -d "${worktree}" "${rev}" + # Clean up the copy on exit. + kube::util::trap_add "git worktree remove -f ${worktree}" EXIT + fi # Ready for apidiff. ( cd "${worktree}" - run "${out}" + "$@" ) } +# inTarget runs the given command in the target revision of Kubernetes, +# checking it out in a work tree if necessary. +inTarget () { + if [ -z "${target}" ]; then + "$@" + else + inWorktree "${KUBE_TEMP}/target" "${target}" "$@" + fi +} + # Dump old and new api state. -if [ -z "${target}" ]; then - run "${KUBE_TEMP}/after" -else - runWorktree "${KUBE_TEMP}/after" "${KUBE_TEMP}/target" "${target}" -fi -runWorktree "${KUBE_TEMP}/before" "${KUBE_TEMP}/base" "${base}" +inTarget run "${KUBE_TEMP}/after" +inWorktree "${KUBE_TEMP}/base" "${base}" run "${KUBE_TEMP}/before" # Now produce a report. All changes get reported because exporting some API # unnecessarily might also be good to know, but the final exit code will only # be non-zero if there are incompatible changes. # # The report is Markdown-formatted and can be copied into a PR comment verbatim. -res=0 +failures=() echo compare () { what="$1" before="$2" after="$3" + if [ ! -f "${before}" ] || [ ! -f "${after}" ]; then + echo "can not compare changes, module didn't exist before or after" + return + fi changes=$(apidiff -m "${before}" "${after}" 2>&1 | grep -v -e "^Ignoring internal package") || true echo "## ${what}" if [ -z "$changes" ]; then @@ -189,9 +235,9 @@ compare () { echo "$changes" echo fi - incompatible=$(apidiff -incompatible -m "${before}" "${after}" 2>&1) || true + incompatible=$(apidiff -incompatible -m "${before}" "${after}" 2>&1 | grep -v -e "^Ignoring internal package") || true if [ -n "$incompatible" ]; then - res=1 + failures+=("${what}") fi } @@ -199,4 +245,97 @@ for d in "${targets[@]}"; do compare "${d}" "${KUBE_TEMP}/before/$(output_name "${d}")" "${KUBE_TEMP}/after/$(output_name "${d}")" done +# tryBuild checks whether some other project builds with the staging repos +# of the current Kubernetes directory. +# +# shellcheck disable=SC2317 # "Command appears to be unreachable" - gets called indirectly. +tryBuild () { + local build="$1" + + # Replace all staging repos, whether the project uses them or not (playing it safe...). + local repo + for repo in $(cd staging/src; find k8s.io -name go.mod); do + local path + repo=$(dirname "${repo}") + path="$(pwd)/staging/src/${repo}" + ( + cd "$build" + go mod edit -replace "${repo}"="${path}" + ) + done + + # We only care about building. Breaking compilation of unit tests is also + # annoying, but does not affect downstream consumers. + ( + cd "$build" + rm -rf vendor + go mod tidy + go build ./... + ) +} + +res=0 +if [ ${#failures[@]} -gt 0 ]; then + res=1 + echo "Detected incompatible changes on modules:" + printf '%s\n' "${failures[@]}" + cat <