diff --git a/modules/tools/00_mod.mk b/modules/tools/00_mod.mk index c48fda1d..7c19bd7e 100644 --- a/modules/tools/00_mod.mk +++ b/modules/tools/00_mod.mk @@ -250,9 +250,9 @@ $(bin_dir)/scratch/%_VERSION: FORCE | $(bin_dir)/scratch CURL := curl --silent --show-error --fail --location --retry 10 --retry-connrefused # LN is expected to be an atomic action, meaning that two Make processes -# can run the "link $(DOWNLOAD_DIR)/tools/xxx@$(XXX_VERSION)_$(HOST_OS)_$(HOST_ARCH) -# to $(bin_dir)/tools/xxx" operation simultaneously without issues (both -# will perform the action and the second time the link will be overwritten). +# can run the "link $(XXX_DOWNLOAD_PATH) to $(bin_dir)/tools/xxx" operation +# simultaneously without issues (both will perform the action and the second +# time the link will be overwritten). # # -s = Create a symbolic link # -f = Force the creation of the link (replace existing links) @@ -287,22 +287,21 @@ tool_names := # the absolute path should be used when executing the binary # in targets or in scripts, because it is agnostic to the # working directory +# - a $(XXX_DOWNLOAD_PATH) variable is generated +# -> this variable contains the path of the versioned binary in +# $(DOWNLOAD_DIR), which the unversioned target links to. Tools +# that are built from source override it in the go_dependency +# template below # - an unversioned target $(bin_dir)/tools/xxx is generated that # creates a link to the corresponding versioned target: -# $(DOWNLOAD_DIR)/tools/xxx@$(XXX_VERSION)_$(HOST_OS)_$(HOST_ARCH) +# $(XXX_DOWNLOAD_PATH) define tool_defs tool_names += $1 $(call uc,$1)_VERSION ?= $2 NEEDS_$(call uc,$1) := $$(bin_dir)/tools/$1 $(call uc,$1) := $$(CURDIR)/$$(bin_dir)/tools/$1 - -# Create symlink from $(bin_dir)/tools/$1 to the versioned binary in $(DOWNLOAD_DIR) -$$(bin_dir)/tools/$1: $$(bin_dir)/scratch/$(call uc,$1)_VERSION | $$(DOWNLOAD_DIR)/tools/$1@$$($(call uc,$1)_VERSION)_$$(HOST_OS)_$$(HOST_ARCH) $$(bin_dir)/tools - @# cd into tools dir and create relative symlink (e.g., ../downloaded/tools/helm@v4.0.1_darwin_arm64) - @# patsubst converts absolute path to relative by replacing $(bin_dir) with .. - @cd $$(dir $$@) && $$(LN) $$(patsubst $$(bin_dir)/%,../%,$$(word 1,$$|)) $$(notdir $$@) - @touch $$@ # making sure the target of the symlink is newer than *_VERSION +$(call uc,$1)_DOWNLOAD_PATH := $$(DOWNLOAD_DIR)/tools/$1@$$($(call uc,$1)_VERSION)_$$(HOST_OS)_$$(HOST_ARCH) endef # For each tool in the tools list (e.g., "helm=v4.0.1"), split on "=" and call tool_defs @@ -338,12 +337,27 @@ __require-go: endif GO := go NEEDS_GO = __require-go +# The version of the Go toolchain that builds the go_dependencies tools, e.g. +# "go1.27.0". When vendoring is disabled this is the system Go, which may +# differ from VENDORED_GO_VERSION. +# GOTOOLCHAIN=local: never trigger a toolchain download while parsing this +# file, and match the go$(VENDORED_GO_VERSION) form used when Go is vendored. +# The awk pass keeps the value safe to embed in a target name: a devel +# toolchain reports a multi-word GOVERSION, which would word-split the +# generated rules. +GO_TOOLCHAIN_VERSION := $(shell GOTOOLCHAIN=local go env GOVERSION 2>/dev/null | awk '{gsub(/[^A-Za-z0-9._-]/,"-"); print}') +ifeq ($(GO_TOOLCHAIN_VERSION),) +# Non-fatal so that targets which need no Go, e.g. "make help", still work +# with no Go installed. Nothing can be built in that state anyway. +GO_TOOLCHAIN_VERSION := unknown +endif else export GOROOT := $(CURDIR)/$(bin_dir)/tools/goroot export PATH := $(CURDIR)/$(bin_dir)/tools/goroot/bin:$(PATH) GO := $(CURDIR)/$(bin_dir)/tools/go NEEDS_GO := $(bin_dir)/tools/go MAKE := $(MAKE) vendor-go +GO_TOOLCHAIN_VERSION := go$(VENDORED_GO_VERSION) endif .PHONY: vendor-go @@ -459,7 +473,15 @@ go_tool_names := # Template for building Go-based tools from source using "go install" define go_dependency go_tool_names += $1 -$$(DOWNLOAD_DIR)/tools/$1@$($(call uc,$1)_VERSION)_$(HOST_OS)_$(HOST_ARCH): | $$(NEEDS_GO) $$(DOWNLOAD_DIR)/tools + +# The binary is keyed on the Go toolchain version as well as the tool version, +# because a tool built by an older Go cannot always parse a newer standard +# library. Without this, a cached binary is never rebuilt after a Go upgrade: +# the download directory is persisted between CI runs, so the stale binary is +# restored and reused indefinitely. +$(call uc,$1)_DOWNLOAD_PATH := $$(DOWNLOAD_DIR)/tools/$1@$$($(call uc,$1)_VERSION)_$$(GO_TOOLCHAIN_VERSION)_$$(HOST_OS)_$$(HOST_ARCH) + +$$($(call uc,$1)_DOWNLOAD_PATH): | $$(NEEDS_GO) $$(DOWNLOAD_DIR)/tools @# 1. Use lock script to prevent concurrent builds of the same tool @# 2. Install to temp dir using GOBIN, with GOWORK=off to ignore workspace files @# 3. Move the binary to final location @@ -471,6 +493,28 @@ $$(DOWNLOAD_DIR)/tools/$1@$($(call uc,$1)_VERSION)_$(HOST_OS)_$(HOST_ARCH): | $$ endef $(call for_each_kv,go_dependency,$(go_dependencies)) +# Create the symlink from $(bin_dir)/tools/xxx to the versioned binary in +# $(DOWNLOAD_DIR). This runs after the go_dependency template above, so that the +# tools built from source link to their Go-version-specific binary. +# +# The versioned binary is a normal (not order-only) prerequisite: rebuilding it +# makes it newer than the symlink, which forces the symlink to be re-pointed. +# In the steady state the symlink resolves to that same binary, so their +# modification times are equal and nothing is remade. The stamp files catch +# version changes that mtimes cannot, e.g. reverting to an older, already-cached +# tool or Go version. The GO_TOOLCHAIN_VERSION stamp is produced by the generic +# %_VERSION pattern rule above, which stamps the value of the make variable of +# the same name. +define tool_link_defs +$$(bin_dir)/tools/$1: $$(bin_dir)/scratch/$(call uc,$1)_VERSION $(if $(filter $1,$(go_tool_names)),$$(bin_dir)/scratch/GO_TOOLCHAIN_VERSION) $$($(call uc,$1)_DOWNLOAD_PATH) | $$(bin_dir)/tools + @# The link is absolute in practice: DOWNLOAD_DIR defaults to a path outside + @# $(bin_dir). The patsubst makes it relative only when DOWNLOAD_DIR is + @# overridden to live under $(bin_dir). + @cd $$(dir $$@) && $$(LN) $$(patsubst $$(bin_dir)/%,../%,$$($(call uc,$1)_DOWNLOAD_PATH)) $$(notdir $$@) + @touch $$@ # making sure the target of the symlink is newer than *_VERSION +endef +$(foreach tool_name,$(tool_names),$(eval $(call tool_link_defs,$(tool_name)))) + ################## # File downloads # ################## diff --git a/tests/e2e-projects/test-project/test-config.sh b/tests/e2e-projects/test-project/test-config.sh index 22be252f..5219f862 100755 --- a/tests/e2e-projects/test-project/test-config.sh +++ b/tests/e2e-projects/test-project/test-config.sh @@ -36,5 +36,6 @@ targets_to_run+=( "_bin/tools/kind" "_bin/tools/kubectl" "_bin/tools/etcd" + "_bin/tools/gojq" "vendor-go" ) diff --git a/tests/test_e2e.sh b/tests/test_e2e.sh index 2d723346..6e813335 100755 --- a/tests/test_e2e.sh +++ b/tests/test_e2e.sh @@ -74,5 +74,30 @@ for project in "${script_dir}"/e2e-projects/*; do echo "> Running make ${target}" make "${target}" done + + # Assert that a Go toolchain version change rebuilds and re-links a + # go_dependency tool, and that reverting re-links the cached binary + # without rebuilding it. An isolated DOWNLOAD_DIR keeps the fake + # toolchain key out of the shared cache. + if printf '%s\n' "${modules_to_copy[@]}" | grep -qx "tools"; then + echo "> Asserting Go toolchain cache invalidation for gojq" + dl_dir="${tmp_dir}/e2e_download" + + make DOWNLOAD_DIR="${dl_dir}" _bin/tools/gojq + original_target=$(readlink _bin/tools/gojq) + + make DOWNLOAD_DIR="${dl_dir}" GO_TOOLCHAIN_VERSION=go0.0.0-test _bin/tools/gojq + fake_target=$(readlink _bin/tools/gojq) + [[ "${fake_target}" == *"_go0.0.0-test_"* ]] + [[ "${fake_target}" != "${original_target}" ]] + + # A rebuild produces a new file (new inode); the relink recipe + # touches the existing one, so compare inodes, not mtimes. + inode_before=$(stat -c %i "${original_target}" 2>/dev/null || stat -f %i "${original_target}") + make DOWNLOAD_DIR="${dl_dir}" _bin/tools/gojq + [[ "$(readlink _bin/tools/gojq)" == "${original_target}" ]] + inode_after=$(stat -c %i "${original_target}" 2>/dev/null || stat -f %i "${original_target}") + [[ "${inode_before}" == "${inode_after}" ]] + fi popd > /dev/null done