Skip to content

Commit

Permalink
Adopt Go toolchains for language-version-control (cadence-workflow#6063)
Browse files Browse the repository at this point in the history
Go 1.21 brought us "toolchains":
We can now describe the version we want, and the Go CLI will automagically download, install, and use that version, with no extra effort.  It should give us a MUCH more reliable way to ensure identical builds and formats and whatnot, and all us devs will auto-switch as we jump between git SHAs.

So let's use that!

This does a few things:
- requires Go 1.21+ to be your `go` version to *work on* Cadence.
  - this does not force our *users* to use 1.21+, that's still 1.20, controlled by the `go 1.20` lines.  just development in this workspace.
- sets a `go.work` toolchain (if there's a `go.work` file, toolchains in individual `go.mod` files are ignored)
  - not all IDEs follow this.  we could set it + lint it in the `go.mod` files too if it helps, it just won't have any effect on CLI `go` use so it may be misleading.
- exports `GOTOOLCHAIN=...` inside the Makefile, so ALL makefile-driven builds by everyone use exactly the same version (you can override this with an explicit `GOTOOLCHAIN`)
- updates our dockerfiles (it would just download [wrong version in docker image] and then auto-download the right one, which is a waste of time)
- adds a lint script to ensure ^ all that stays up to date

---

For future upgrades, upgrading the toolchain should be very simple:
- change `go.work` to a new toolchain version
  - everything now uses the new language version, tools and binaries will rebuild, etc.
- `make lint` or `make pr` or CI will tell you to change the Dockerfiles / other locations, if you don't `git grep -F 1.22.3` to find and update those first.

And temporarily trying a different version to check a bug / performance / try the new version is:
- `GOTOOLCHAIN=go1.23.4 make whatever` and it'll do exactly that (except for the version check in `make lint`)
- you may want to `make clean` as well, to rebuild tools if that's relevant
  - they will not automatically rebuild with this env var changing.  they *could*, if we used `.build/go1.23.4/etc` folders, if we want that.
  • Loading branch information
Groxx authored May 28, 2024
1 parent 7a58a5d commit 5f45da5
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 29 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/codecov.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
- name: Set up Go
uses: actions/setup-go@v4
with:
go-version: 1.20.x
go-version: 1.22.3
cache: false

- name: Download dependencies
Expand Down
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ ARG TARGET=server
ARG GOPROXY

# Build Cadence binaries
FROM golang:1.20-alpine3.18 AS builder
FROM golang:1.22.3-alpine3.18 AS builder

ARG RELEASE_VERSION

Expand Down
55 changes: 29 additions & 26 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,14 @@ BIN := $(BUILD)/bin
# usually unnecessary to clean, and may require downloads to restore, so this folder is not automatically cleaned.
STABLE_BIN := .bin


# current (when committed) version of Go used in CI, and ideally also our docker images.
# this generally does not matter, but can impact goimports output.
# for maximum stability, make sure you use the same version as CI uses.
#
# this can _likely_ remain a major version, as fmt output does not tend to change in minor versions,
# which will allow findstring to match any minor version.
EXPECTED_GO_VERSION := go1.20
CURRENT_GO_VERSION := $(shell go version)
ifeq (,$(findstring $(EXPECTED_GO_VERSION),$(CURRENT_GO_VERSION)))
# if you are seeing this warning: consider using https://github.com/travis-ci/gimme to pin your version
$(warning Caution: you are not using CI's go version. Expected: $(EXPECTED_GO_VERSION), current: $(CURRENT_GO_VERSION))
# toolchain version we all use.
# this export ensures it is a precise version rather than a minimum.
# lint step ensures this matches other files.
GOWORK_TOOLCHAIN := $(word 2,$(shell grep 'toolchain' go.work))
export GOTOOLCHAIN ?= $(GOWORK_TOOLCHAIN)
ifneq ($(GOTOOLCHAIN),$(GOWORK_TOOLCHAIN))
# this can be useful for trying new/old versions, so don't block it
$(warning warning: your Go toolchain is explicitly set to GOTOOLCHAIN=$(GOTOOLCHAIN), ignoring go.work version $(GOWORK_TOOLCHAIN)...)
endif

# ====================================
Expand All @@ -64,6 +60,7 @@ endif
$(BUILD)/lint: $(BUILD)/fmt # lint will fail if fmt fails, so fmt first
$(BUILD)/proto-lint:
$(BUILD)/gomod-lint:
$(BUILD)/goversion-lint:
$(BUILD)/fmt: $(BUILD)/copyright # formatting must occur only after all other go-file-modifications are done
$(BUILD)/copyright: $(BUILD)/codegen # must add copyright to generated code, sometimes needs re-formatting
$(BUILD)/codegen: $(BUILD)/thrift $(BUILD)/protoc
Expand Down Expand Up @@ -175,45 +172,45 @@ endef
$(BIN) $(BUILD) $(STABLE_BIN):
$Q mkdir -p $@

$(BIN)/thriftrw: go.mod
$(BIN)/thriftrw: go.mod go.work
$(call go_mod_build_tool,go.uber.org/thriftrw)

$(BIN)/thriftrw-plugin-yarpc: go.mod
$(BIN)/thriftrw-plugin-yarpc: go.mod go.work
$(call go_mod_build_tool,go.uber.org/yarpc/encoding/thrift/thriftrw-plugin-yarpc)

$(BIN)/mockgen: internal/tools/go.mod
$(BIN)/mockgen: internal/tools/go.mod go.work
$(call go_build_tool,github.com/golang/mock/mockgen)

$(BIN)/mockery: internal/tools/go.mod
$(BIN)/mockery: internal/tools/go.mod go.work
$(call go_build_tool,github.com/vektra/mockery/v2,mockery)

$(BIN)/enumer: internal/tools/go.mod
$(BIN)/enumer: internal/tools/go.mod go.work
$(call go_build_tool,github.com/dmarkham/enumer)

# organizes imports and reformats
$(BIN)/gci: internal/tools/go.mod
$(BIN)/gci: internal/tools/go.mod go.work
$(call go_build_tool,github.com/daixiang0/gci)

# removes unused imports and reformats
$(BIN)/goimports: internal/tools/go.mod
$(BIN)/goimports: internal/tools/go.mod go.work
$(call go_build_tool,golang.org/x/tools/cmd/goimports)

$(BIN)/gowrap: go.mod
$(BIN)/gowrap: go.mod go.work
$(call go_build_tool,github.com/hexdigest/gowrap/cmd/gowrap)

$(BIN)/revive: internal/tools/go.mod
$(BIN)/revive: internal/tools/go.mod go.work
$(call go_build_tool,github.com/mgechev/revive)

$(BIN)/protoc-gen-gogofast: go.mod | $(BIN)
$(BIN)/protoc-gen-gogofast: go.mod go.work | $(BIN)
$(call go_mod_build_tool,github.com/gogo/protobuf/protoc-gen-gogofast)

$(BIN)/protoc-gen-yarpc-go: go.mod | $(BIN)
$(BIN)/protoc-gen-yarpc-go: go.mod go.work | $(BIN)
$(call go_mod_build_tool,go.uber.org/yarpc/encoding/protobuf/protoc-gen-yarpc-go)

$(BIN)/goveralls: internal/tools/go.mod
$(BIN)/goveralls: internal/tools/go.mod go.work
$(call go_build_tool,github.com/mattn/goveralls)

$(BUILD)/go_mod_check: go.mod internal/tools/go.mod
$(BUILD)/go_mod_check: go.mod internal/tools/go.mod go.work
$Q # generated == used is occasionally important for gomock / mock libs in general. this is not a definite problem if violated though.
$Q ./scripts/check-gomod-version.sh github.com/golang/mock/gomock $(if $(verbose),-v)
$Q touch $@
Expand Down Expand Up @@ -372,6 +369,12 @@ $(BUILD)/code-lint: $(LINT_SRC) $(BIN)/revive | $(BUILD)
fi
$Q touch $@

$(BUILD)/goversion-lint: go.work Dockerfile docker/buildkite/Dockerfile
$Q echo "checking go version..."
$Q # intentionally using go.work toolchain, as GOTOOLCHAIN is user-overridable
$Q ./scripts/check-go-toolchain.sh $(GOWORK_TOOLCHAIN)
$Q touch $@

# fmt and copyright are mutually cyclic with their inputs, so if a copyright header is modified:
# - copyright -> makes changes
# - fmt sees changes -> makes changes
Expand Down Expand Up @@ -420,7 +423,7 @@ endef
# useful to actually re-run to get output again.
# reuse the intermediates for simplicity and consistency.
lint: ## (re)run the linter
$(call remake,proto-lint gomod-lint code-lint)
$(call remake,proto-lint gomod-lint code-lint goversion-lint)

# intentionally not re-making, it's a bit slow and it's clear when it's unnecessary
fmt: $(BUILD)/fmt ## run gofmt / organize imports / etc
Expand Down
2 changes: 1 addition & 1 deletion docker/buildkite/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM golang:1.20-bullseye
FROM golang:1.22.3-bullseye

# Tried to set Python to ignore warnings due to the instructions at this link:
# https://github.com/yaml/pyyaml/wiki/PyYAML-yaml.load(input)-Deprecation
Expand Down
9 changes: 9 additions & 0 deletions go.work
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,12 @@ use (
// DO NOT include, tools dependencies are intentionally separate.
// ./internal/tools
)

// technically only a minimum version, but forced to be precise in makefile targets.
// must be kept in sync with docker files to avoid double-downloading.
//
// this should be safe to raise any time as it only impacts us, as this affects the
// Go version used to build within this workspace only, it does not affect dependencies.
// but note that it needs to be a version that docker + mac + linux all support, as
// they all must be in sync.
toolchain go1.22.3
49 changes: 49 additions & 0 deletions scripts/check-go-toolchain.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
#!/usr/bin/env bash

set -eo pipefail

# enable double-star
shopt -s globstar

fail=0
bad() {
echo -e "$@" >/dev/stderr
fail=1
}

if [[ $V == 1 ]]; then
set -x
fi

target="${1#go}"
root="$(git rev-parse --show-toplevel)"

# this SHOULD match the dependencies in the goversion-lint check to avoid skipping it

# check dockerfiles
for file in "$root"/**/Dockerfile; do
# find "FROM golang:1.22.3-alpine3.18 ..." lines
line="$(grep -i 'from golang:' "$file")"
# remove "from golang:" prefix
version="${line#*golang:}"
# remove "-alpine..." suffix
version="${version%-*}"
# and make sure it matches
if [[ "$version" != "$target" ]]; then
bad "Wrong Go version in file $file:\n\t$line"
fi
done

# check workflows
codecov_file="$root/.github/workflows/codecov.yml"
codecov_line="$(grep 'go-version:' "$codecov_file")"
codecov_version="${codecov_line#*go-version: }"
if [[ "$codecov_version" != "$target" ]]; then
bad "Wrong Go version in file $codecov_file:\n\t$codecov_line"
fi

if [[ $fail == 1 ]]; then
bad "Makefile pins Go to go${target}, Dockerfiles and GitHub workflows should too."
bad "Non-matching versions lead to pointless double-downloading to get the correct version."
exit 1
fi

0 comments on commit 5f45da5

Please sign in to comment.