From e6fccc36d8e7f557a99948bfeb2a852460e3f4c2 Mon Sep 17 00:00:00 2001 From: Antonio Ojea Date: Thu, 28 Nov 2024 15:42:59 +0000 Subject: [PATCH 1/6] apidiff support internal go modules The kubernetes repository contains some internal golang modules that are not part of the golang global workspace. Because apidiff is currently run from the root of the repository, it does not work against this internal modules. Instead of executing apidiff from the root we can just cd into the passed path of the module to avoid this limitation. --- hack/apidiff.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/hack/apidiff.sh b/hack/apidiff.sh index fa70631de38..43fb44f825f 100755 --- a/hack/apidiff.sh +++ b/hack/apidiff.sh @@ -156,7 +156,12 @@ run () { out="$1" mkdir -p "$out" for d in "${targets[@]}"; do - apidiff -m -w "${out}/$(output_name "${d}")" "${d}" + # 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 } From 52386915a83180cb862d686f4b8140e64362bb8a Mon Sep 17 00:00:00 2001 From: Antonio Ojea Date: Thu, 28 Nov 2024 17:39:59 +0000 Subject: [PATCH 2/6] use relative paths and modules that are not visible to golagn workspaces --- hack/apidiff.sh | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/hack/apidiff.sh b/hack/apidiff.sh index 43fb44f825f..3fc6fe0e79a 100755 --- a/hack/apidiff.sh +++ b/hack/apidiff.sh @@ -82,24 +82,17 @@ 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) + 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 -# 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. # "git rev-parse --verify" checks that and prints a detailed # error. From 7de94d43cabc688f538072a9a8dbce726bde1670 Mon Sep 17 00:00:00 2001 From: Antonio Ojea Date: Thu, 28 Nov 2024 17:50:27 +0000 Subject: [PATCH 3/6] handle the case when modules are added or removed don't compare modules that didn't or doesn't exist --- hack/apidiff.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/hack/apidiff.sh b/hack/apidiff.sh index 3fc6fe0e79a..6b67227caea 100755 --- a/hack/apidiff.sh +++ b/hack/apidiff.sh @@ -149,6 +149,10 @@ run () { out="$1" mkdir -p "$out" for d in "${targets[@]}"; do + 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 ( @@ -208,6 +212,10 @@ 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 From affafd906da933fb9cab5daa26f0a4dc76a7eb94 Mon Sep 17 00:00:00 2001 From: Antonio Ojea Date: Thu, 28 Nov 2024 18:05:29 +0000 Subject: [PATCH 4/6] improve report on failures summarizing the modules with incompatible changes --- hack/apidiff.sh | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/hack/apidiff.sh b/hack/apidiff.sh index 6b67227caea..aa95b22c461 100755 --- a/hack/apidiff.sh +++ b/hack/apidiff.sh @@ -206,7 +206,7 @@ inWorktree "${KUBE_TEMP}/base" "${base}" run "${KUBE_TEMP}/before" # 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" @@ -226,7 +226,7 @@ compare () { fi incompatible=$(apidiff -incompatible -m "${before}" "${after}" 2>&1) || true if [ -n "$incompatible" ]; then - res=1 + failures+=("${what}") fi } @@ -263,7 +263,11 @@ tryBuild () { ) } -if [ $res -ne 0 ]; then +res=0 +if [ ${#failures[@]} -gt 0 ]; then + res=1 + echo "Detected incompatible changes on modules:" + printf '%s\n' "${failures[@]}" cat < Date: Thu, 28 Nov 2024 18:11:23 +0000 Subject: [PATCH 5/6] parallalize the apidiff dump --- hack/apidiff.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hack/apidiff.sh b/hack/apidiff.sh index aa95b22c461..b4ced21f61e 100755 --- a/hack/apidiff.sh +++ b/hack/apidiff.sh @@ -158,8 +158,9 @@ run () { ( cd "${d}" apidiff -m -w "${out}/$(output_name "${d}")" . - ) + ) & done + wait } # inWorktree checks out a specific revision, then invokes the given From e5ebbdc3c7f63759906c32da9c2e149329bc3adc Mon Sep 17 00:00:00 2001 From: Antonio Ojea Date: Fri, 29 Nov 2024 07:29:34 +0000 Subject: [PATCH 6/6] Ignore internal packages messages --- hack/apidiff.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hack/apidiff.sh b/hack/apidiff.sh index b4ced21f61e..a24b3d41e4b 100755 --- a/hack/apidiff.sh +++ b/hack/apidiff.sh @@ -225,7 +225,7 @@ 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 failures+=("${what}") fi