Skip to content

Commit 5e56e96

Browse files
authored
Merge pull request kubernetes#127802 from pohly/apidiff-try-builds
apidiff: support trial builds of other projects
2 parents 3d6c99e + 42e12c3 commit 5e56e96

File tree

1 file changed

+134
-16
lines changed

1 file changed

+134
-16
lines changed

hack/apidiff.sh

Lines changed: 134 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,11 @@ Usage: $0 [-r <revision>] [directory ...]"
2626
Default is the current working tree instead of a revision.
2727
-r <revision>: Report change in code added since this revision. Default is
2828
the common base of origin/master and HEAD.
29+
-b <directory> Build all packages in that directory after replacing
30+
Kubernetes dependencies with the current content of the
31+
staging repo. May be given more than once. Must be an
32+
absolute path.
33+
WARNING: this will modify the go.mod in that directory.
2934
[directory]: Check one or more specific directory instead of everything.
3035
EOF
3136
exit 1
@@ -40,7 +45,8 @@ source "${KUBE_ROOT}/hack/lib/init.sh"
4045

4146
base=
4247
target=
43-
while getopts "r:t:" o; do
48+
builds=()
49+
while getopts "r:t:b:" o; do
4450
case "${o}" in
4551
r)
4652
base="${OPTARG}"
@@ -58,6 +64,14 @@ while getopts "r:t:" o; do
5864
usage
5965
fi
6066
;;
67+
b)
68+
if [ ! "${OPTARG}" ]; then
69+
echo "ERROR: -${o} needs a non-empty parameter" >&2
70+
echo >&2
71+
usage
72+
fi
73+
builds+=("${OPTARG}")
74+
;;
6175
*)
6276
usage
6377
;;
@@ -136,6 +150,8 @@ output_name () {
136150

137151
# run invokes apidiff once per target and stores the output
138152
# file(s) in the given directory.
153+
#
154+
# shellcheck disable=SC2317 # "Command appears to be unreachable" - gets called indirectly.
139155
run () {
140156
out="$1"
141157
mkdir -p "$out"
@@ -144,31 +160,44 @@ run () {
144160
done
145161
}
146162

147-
# runWorktree checks out a specific revision, then invokes run there.
148-
runWorktree () {
149-
local out="$1"
150-
local worktree="$2"
151-
local rev="$3"
163+
# inWorktree checks out a specific revision, then invokes the given
164+
# command there.
165+
#
166+
# shellcheck disable=SC2317 # "Command appears to be unreachable" - gets called indirectly.
167+
inWorktree () {
168+
local worktree="$1"
169+
shift
170+
local rev="$1"
171+
shift
152172

153173
# Create a copy of the repo with the specific revision checked out.
154-
git worktree add -f -d "${worktree}" "${rev}"
155-
# Clean up the copy on exit.
156-
kube::util::trap_add "git worktree remove -f ${worktree}" EXIT
174+
# Might already have been done before.
175+
if ! [ -d "${worktree}" ]; then
176+
git worktree add -f -d "${worktree}" "${rev}"
177+
# Clean up the copy on exit.
178+
kube::util::trap_add "git worktree remove -f ${worktree}" EXIT
179+
fi
157180

158181
# Ready for apidiff.
159182
(
160183
cd "${worktree}"
161-
run "${out}"
184+
"$@"
162185
)
163186
}
164187

188+
# inTarget runs the given command in the target revision of Kubernetes,
189+
# checking it out in a work tree if necessary.
190+
inTarget () {
191+
if [ -z "${target}" ]; then
192+
"$@"
193+
else
194+
inWorktree "${KUBE_TEMP}/target" "${target}" "$@"
195+
fi
196+
}
197+
165198
# Dump old and new api state.
166-
if [ -z "${target}" ]; then
167-
run "${KUBE_TEMP}/after"
168-
else
169-
runWorktree "${KUBE_TEMP}/after" "${KUBE_TEMP}/target" "${target}"
170-
fi
171-
runWorktree "${KUBE_TEMP}/before" "${KUBE_TEMP}/base" "${base}"
199+
inTarget run "${KUBE_TEMP}/after"
200+
inWorktree "${KUBE_TEMP}/base" "${base}" run "${KUBE_TEMP}/before"
172201

173202
# Now produce a report. All changes get reported because exporting some API
174203
# unnecessarily might also be good to know, but the final exit code will only
@@ -199,4 +228,93 @@ for d in "${targets[@]}"; do
199228
compare "${d}" "${KUBE_TEMP}/before/$(output_name "${d}")" "${KUBE_TEMP}/after/$(output_name "${d}")"
200229
done
201230

231+
# tryBuild checks whether some other project builds with the staging repos
232+
# of the current Kubernetes directory.
233+
#
234+
# shellcheck disable=SC2317 # "Command appears to be unreachable" - gets called indirectly.
235+
tryBuild () {
236+
local build="$1"
237+
238+
# Replace all staging repos, whether the project uses them or not (playing it safe...).
239+
local repo
240+
for repo in $(cd staging/src; find k8s.io -name go.mod); do
241+
local path
242+
repo=$(dirname "${repo}")
243+
path="$(pwd)/staging/src/${repo}"
244+
(
245+
cd "$build"
246+
go mod edit -replace "${repo}"="${path}"
247+
)
248+
done
249+
250+
# We only care about building. Breaking compilation of unit tests is also
251+
# annoying, but does not affect downstream consumers.
252+
(
253+
cd "$build"
254+
rm -rf vendor
255+
go mod tidy
256+
go build ./...
257+
)
258+
}
259+
260+
if [ $res -ne 0 ]; then
261+
cat <<EOF
262+
263+
Some notes about API differences:
264+
265+
Changes in internal packages are usually okay.
266+
However, remember that custom schedulers
267+
and scheduler plugins depend on pkg/scheduler/framework.
268+
269+
API changes in staging repos are more critical.
270+
Try to avoid them as much as possible.
271+
But sometimes changing an API is the lesser evil
272+
and/or the impact on downstream consumers is low.
273+
Use common sense and code searches.
274+
EOF
275+
276+
if [ ${#builds[@]} -gt 0 ]; then
277+
278+
cat <<EOF
279+
280+
To help with assessing the real-world impact of an
281+
API change, $0 will now try to build code in
282+
${builds[@]}.
283+
EOF
284+
285+
if [[ "${builds[*]}" =~ controller-runtime ]]; then
286+
cat <<EOF
287+
288+
controller-runtime is used because
289+
- It tends to use advanced client-go functionality.
290+
- Breaking it has additional impact on controller
291+
built on top of it.
292+
293+
This doesn't mean that an API change isn't allowed
294+
if it breaks controller runtime, it just needs additional
295+
scrutiny.
296+
297+
https://github.com/kubernetes-sigs/controller-runtime?tab=readme-ov-file#compatibility
298+
explicitly states that a controller-runtime
299+
release cannot be expected to work with a newer
300+
release of the Kubernetes Go packages.
301+
EOF
302+
fi
303+
304+
for build in "${builds[@]}"; do
305+
echo
306+
echo "vvvvvvvvvvvvvvvv ${build} vvvvvvvvvvvvvvvvvv"
307+
if inTarget tryBuild "${build}"; then
308+
echo "${build} builds without errors."
309+
else
310+
cat <<EOF
311+
312+
WARNING: Building ${build} failed. This may or may not be because of the API changes!
313+
EOF
314+
fi
315+
echo "^^^^^^^^^^^^^^^^ ${build} ^^^^^^^^^^^^^^^^^^"
316+
done
317+
fi
318+
fi
319+
202320
exit "$res"

0 commit comments

Comments
 (0)