Skip to content

Commit

Permalink
Server-like make build and ensuring builds are clean in CI (cadence…
Browse files Browse the repository at this point in the history
…-workflow#1329)

There's more work to be done, but it's a major step towards forcing stability.

Oddly, missing the safe.directory git setting leads to Go failing to get version info, leading to errors building, but for some reason it doesn't show the error that git's reporting (missing safe directory).  Just stuff like this with no other output:
```
error obtaining VCS status:
	Use -buildvcs=false to disable VCS stamping.
```

Once I fixed that, I learned that `go test -exec nonexistent ./... >/dev/null` errors, but the "FAIL: command nonexistent not found" error is.... reported on stdout, so it's hidden.
And so is the "FAIL the/package/name" message.
But if you have a failing build, the failure *is* printed to stderr:
```
❯ go test -exec true ./... >/dev/null
# go.uber.org/cadence/evictiontest
evictiontest/workflow_cache_eviction_test.go:58:4: missing ',' in composite literal
```
I have no idea why `-exec nonexistent` isn't on stderr, but I left a comment in the makefile anyway.

Quite a large amount of weird stuff condensed into a seemingly simple goal.
  • Loading branch information
Groxx authored Apr 1, 2024
1 parent a55fc13 commit 68afcb9
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 48 deletions.
14 changes: 14 additions & 0 deletions .buildkite/pipeline.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,20 @@ steps:
run: unit-test
config: docker/buildkite/docker-compose.yml

- label: ":golangci-lint: validate code is clean"
agents:
queue: "workers"
docker: "*"
command: "./scripts/golint.sh"
artifact_paths: [ ]
retry:
automatic:
limit: 1
plugins:
- docker-compose#v3.0.0:
run: unit-test
config: docker/buildkite/docker-compose.yml

- label: ":golang: integration-test-sticky-off"
agents:
queue: "workers"
Expand Down
19 changes: 11 additions & 8 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,7 @@ endif
# ====================================

# note that vars that do not yet exist are empty, so stick to BUILD/BIN and probably nothing else.
$(BUILD)/lint: $(BUILD)/fmt $(BUILD)/dummy # lint will fail if fmt or dummy fails, so run them first
$(BUILD)/dummy: $(BUILD)/fmt # do a build after fmt-ing
$(BUILD)/lint: $(BUILD)/fmt # lint will fail if fmt (more generally, build) fails, so run it first
$(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
$(BUILD)/codegen: $(BUILD)/thrift $(BUILD)/generate
Expand Down Expand Up @@ -165,10 +164,6 @@ $(BIN)/mockery: internal/tools/go.mod
$(BIN)/copyright: internal/cmd/tools/copyright/licensegen.go
go build -mod=readonly -o $@ ./internal/cmd/tools/copyright/licensegen.go

# dummy binary that ensures most/all packages build, without needing to wait for tests.
$(BUILD)/dummy: $(ALL_SRC) $(BUILD)/go_mod_check
go build -mod=readonly -o $@ internal/cmd/dummy/dummy.go

# ensures mod files are in sync for critical packages
$(BUILD)/go_mod_check: go.mod internal/tools/go.mod
$Q # ensure both have the same apache/thrift replacement
Expand Down Expand Up @@ -278,7 +273,10 @@ $Q +$(MAKE) --no-print-directory $(addprefix $(BUILD)/,$(1))
endef

.PHONY: build
build: $(BUILD)/dummy ## ensure all packages build
build: $(BUILD)/fmt ## ensure all packages build
go build ./...
$Q # caution: some errors are reported on stdout for some reason
go test -exec true ./... >/dev/null

.PHONY: lint
# useful to actually re-run to get output again.
Expand Down Expand Up @@ -307,8 +305,13 @@ errcheck: $(BIN)/errcheck $(BUILD)/fmt ## (re)run errcheck
.PHONY: generate
generate: $(BUILD)/generate ## run go-generate

.PHONY: tidy
tidy:
go mod tidy
cd internal/tools; go mod tidy

.PHONY: all
all: $(BUILD)/lint ## refresh codegen, lint, and ensure the dummy binary builds, if necessary
all: $(BUILD)/lint ## refresh codegen, lint, and ensure everything builds, whatever is necessary

.PHONY: clean
clean:
Expand Down
3 changes: 3 additions & 0 deletions docker/buildkite/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,7 @@ RUN mkdir -p /go/src/go.uber.org/cadence
WORKDIR /go/src/go.uber.org/cadence

ADD go.mod go.sum /go/src/go.uber.org/cadence/

# allow git-status and similar to work
RUN git config --global --add safe.directory /go/src/go.uber.org/cadence
RUN GO111MODULE=on go mod download
6 changes: 1 addition & 5 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMyw
github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
github.com/google/go-cmp v0.5.8 h1:e6P7q2lk1O+qJJb4BtCQXlK8vWEO8V1ZeuEdJNOqZyg=
github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI=
github.com/jessevdk/go-flags v1.4.0 h1:4IU2WS7AumrZ/40jfhf4QVDMsQwqA7VEHozFRrGARJA=
Expand Down Expand Up @@ -206,8 +206,6 @@ github.com/uber-go/mapdecode v1.0.0/go.mod h1:b5nP15FwXTgpjTjeA9A2uTHXV5UJCl4arw
github.com/uber-go/tally v3.3.12+incompatible/go.mod h1:YDTIBxdXyOU/sCWilKB4bgyufu1cEi0jdVnRdxvjnmU=
github.com/uber-go/tally v3.3.15+incompatible h1:9hLSgNBP28CjIaDmAuRTq9qV+UZY+9PcvAkXO4nNMwg=
github.com/uber-go/tally v3.3.15+incompatible/go.mod h1:YDTIBxdXyOU/sCWilKB4bgyufu1cEi0jdVnRdxvjnmU=
github.com/uber/cadence-idl v0.0.0-20230905165949-03586319b849 h1:j3bfADG1t35Lt4rNRpc9AuQ3l2cGw2Ao25Qt6rDamgc=
github.com/uber/cadence-idl v0.0.0-20230905165949-03586319b849/go.mod h1:oyUK7GCNCRHCCyWyzifSzXpVrRYVBbAMHAzF5dXiKws=
github.com/uber/cadence-idl v0.0.0-20240212223805-34b4519b2709 h1:1u+kMB6p8P9fjK6jk3QHAl8PxLyNjO9/TMXoPOVr1O8=
github.com/uber/cadence-idl v0.0.0-20240212223805-34b4519b2709/go.mod h1:oyUK7GCNCRHCCyWyzifSzXpVrRYVBbAMHAzF5dXiKws=
github.com/uber/jaeger-client-go v2.22.1+incompatible h1:NHcubEkVbahf9t3p75TOCR83gdUHXjRJvjoBh1yACsM=
Expand Down Expand Up @@ -299,8 +297,6 @@ golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAG
golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
golang.org/x/oauth2 v0.1.0 h1:isLCZuhj4v+tYv7eskaN4v/TM+A1begWWgyVJDdl1+Y=
golang.org/x/oauth2 v0.1.0/go.mod h1:G9FE4dLTsbXUu90h/Pf85g4w1D+SSAgR+q46nJZ8M4A=
golang.org/x/oauth2 v0.15.0 h1:s8pnnxNVzjWyrvYdFUQq5llS1PX2zhPXmccZv99h7uQ=
golang.org/x/oauth2 v0.15.0/go.mod h1:q48ptWNTY5XWf+JNten23lcvHpLJ0ZSxF5ttTHKVCAM=
golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
Expand Down
35 changes: 0 additions & 35 deletions internal/cmd/dummy/dummy.go

This file was deleted.

18 changes: 18 additions & 0 deletions scripts/golint.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#!/bin/sh

# refresh deps
make tidy
# regenerate, format, and make sure everything builds
make build

# intentionally capture stderr, so status-errors are also PR-failing.
# in particular this catches "dubious ownership" failures, which otherwise
# do not fail this check and the $() hides the exit code.
if [ -n "$(git status --porcelain 2>&1)" ]; then
echo "There file changes after applying your diff and performing a build."
echo "Please run this command and commit the changes:"
echo "\tmake tidy && make build"
git status --porcelain
git --no-pager diff
exit 1
fi

0 comments on commit 68afcb9

Please sign in to comment.