From 194289aa8eece02c19f537b26a361932cc0ff73d Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Fri, 4 Oct 2019 14:08:31 +0200 Subject: [PATCH] update Go mod support It turned out that changes like https://github.com/kubernetes-csi/csi-lib-utils/pull/33 should better have been committed after `go mod tidy` because that adds some indirect dependencies in that example. The revised `test-vendor` checks for that and (just in case that this ever becomes desired) allows projects to not have a vendor directory when using `go mod`. How to use `go mod` properly gets documented in the README.md, because there are such pitfalls. --- README.md | 34 +++++++++++++++++++++++++++++++++ build.make | 55 ++++++++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 79 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index bc061aeeb..d5fdb3a0a 100644 --- a/README.md +++ b/README.md @@ -106,3 +106,37 @@ Kubernetes releases: CSI_PROW_KUBERNETES_VERSION=1.13.3 ./.prow.sh CSI_PROW_KUBERNETES_VERSION=latest ./.prow.sh + +Dependencies and vendoring +-------------------------- + +Most projects will (eventually) use `go mod` to manage +dependencies. `dep` is also still supported by `csi-release-tools`, +but not documented here because it's not recommended anymore. + +The usual instructions for using [go +modules](https://github.com/golang/go/wiki/Modules) apply. Here's a cheat sheet +for some of the relevant commands: +- list available updates: `GO111MODULE=on go list -u -m all` +- update or add a single dependency: `GO111MODULE=on go get ` +- update all dependencies to their next minor or patch release: + `GO111MODULE=on go get ./...` (add `-u=patch` to limit to patch + releases) +- lock onto a specific version: `GO111MODULE=on go get @` +- clean up `go.mod`: `GO111MODULE=on go mod tidy` +- update vendor directory: `GO111MODULE=on go mod vendor` + +`GO111MODULE=on` can be left out when using Go >= 1.13 or when the +source is checked out outside of `$GOPATH`. + +`go mod tidy` must be used to ensure that the listed dependencies are +really still needed. Changing import statements or a tentative `go +get` can result in stale dependencies. + +The `test-vendor` verifies that it was used when run locally or in a +pre-merge CI job. If a `vendor` directory is present, it will also +verify that it's content is up-to-date. + +The `vendor` directory is optional. It is still present in projects +because it avoids downloading sources during CI builds. If this is no +longer deemed necessary, then a project can also remove the directory. diff --git a/build.make b/build.make index 142c85780..c8384c162 100644 --- a/build.make +++ b/build.make @@ -125,6 +125,26 @@ test-fmt: # - the fabricated merge commit leaves go.mod, go.sum and vendor dir unchanged # - release-tools also didn't change (changing rules or Go version might lead to # a different result and thus must be tested) +# - import statements not changed (because if they change, go.mod might have to be updated) +# +# "git diff" is intelligent enough to annotate changes inside the "import" block in +# the start of the diff hunk: +# +# diff --git a/rpc/common.go b/rpc/common.go +# index bb4a5c4..5fa4271 100644 +# --- a/rpc/common.go +# +++ b/rpc/common.go +# @@ -21,7 +21,6 @@ import ( +# "fmt" +# "time" +# +# - "google.golang.org/grpc" +# "google.golang.org/grpc/codes" +# "google.golang.org/grpc/status" +# +# We rely on that to find such changes. +# +# Vendoring is optional when using go.mod. .PHONY: test-vendor test: test-vendor test-vendor: @@ -135,22 +155,37 @@ test-vendor: *v0.[56789]*) dep check && echo "vendor up-to-date" || false;; \ *) echo "skipping check, dep >= 0.5 required";; \ esac; \ - else \ - echo "Repo uses 'go mod' for vendoring."; \ + elif [ -f go.mod ]; then \ + echo "Repo uses 'go mod'."; \ if [ "$${JOB_NAME}" ] && \ ( [ "$${JOB_TYPE}" != "presubmit" ] || \ - [ $$(git diff "${PULL_BASE_SHA}..HEAD" -- go.mod go.sum vendor release-tools | wc -l) -eq 0 ] ); then \ - echo "Skipping vendor check because the Prow pre-submit job does not change vendoring."; \ - elif ! GO111MODULE=on go mod vendor; then \ + [ $$( (git diff "${PULL_BASE_SHA}..HEAD" -- go.mod go.sum vendor release-tools; \ + git diff "${PULL_BASE_SHA}..HEAD" | grep -e '^@@.*@@ import (' -e '^[+-]import') | \ + wc -l) -eq 0 ] ); then \ + echo "Skipping vendor check because the Prow pre-submit job does not affect dependencies."; \ + elif ! GO111MODULE=on go mod tidy; then \ echo "ERROR: vendor check failed."; \ false; \ - elif [ $$(git status --porcelain -- vendor | wc -l) -gt 0 ]; then \ - echo "ERROR: vendor directory *not* up-to-date, it did get modified by 'GO111MODULE=on go mod vendor':"; \ - git status -- vendor; \ - git diff -- vendor; \ + elif [ $$(git status --porcelain -- go.mod go.sum | wc -l) -gt 0 ]; then \ + echo "ERROR: go module files *not* up-to-date, they did get modified by 'GO111MODULE=on go mod tidy':"; \ + git diff -- go.mod go.sum; \ false; \ + elif [ -d vendor ]; then \ + if ! GO111MODULE=on go mod vendor; then \ + echo "ERROR: vendor check failed."; \ + false; \ + elif [ $$(git status --porcelain -- vendor | wc -l) -gt 0 ]; then \ + echo "ERROR: vendor directory *not* up-to-date, it did get modified by 'GO111MODULE=on go mod vendor':"; \ + git status -- vendor; \ + git diff -- vendor; \ + false; \ + else \ + echo "Go dependencies and vendor directory up-to-date."; \ + fi; \ + else \ + echo "Go dependencies up-to-date."; \ fi; \ - fi; + fi .PHONY: test-subtree test: test-subtree