Skip to content

Commit 68afcb9

Browse files
authored
Server-like make build and ensuring builds are clean in CI (#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.
1 parent a55fc13 commit 68afcb9

File tree

6 files changed

+47
-48
lines changed

6 files changed

+47
-48
lines changed

.buildkite/pipeline.yml

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,20 @@ steps:
2121
run: unit-test
2222
config: docker/buildkite/docker-compose.yml
2323

24+
- label: ":golangci-lint: validate code is clean"
25+
agents:
26+
queue: "workers"
27+
docker: "*"
28+
command: "./scripts/golint.sh"
29+
artifact_paths: [ ]
30+
retry:
31+
automatic:
32+
limit: 1
33+
plugins:
34+
- docker-compose#v3.0.0:
35+
run: unit-test
36+
config: docker/buildkite/docker-compose.yml
37+
2438
- label: ":golang: integration-test-sticky-off"
2539
agents:
2640
queue: "workers"

Makefile

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,7 @@ endif
5050
# ====================================
5151

5252
# note that vars that do not yet exist are empty, so stick to BUILD/BIN and probably nothing else.
53-
$(BUILD)/lint: $(BUILD)/fmt $(BUILD)/dummy # lint will fail if fmt or dummy fails, so run them first
54-
$(BUILD)/dummy: $(BUILD)/fmt # do a build after fmt-ing
53+
$(BUILD)/lint: $(BUILD)/fmt # lint will fail if fmt (more generally, build) fails, so run it first
5554
$(BUILD)/fmt: $(BUILD)/copyright # formatting must occur only after all other go-file-modifications are done
5655
$(BUILD)/copyright: $(BUILD)/codegen # must add copyright to generated code
5756
$(BUILD)/codegen: $(BUILD)/thrift $(BUILD)/generate
@@ -165,10 +164,6 @@ $(BIN)/mockery: internal/tools/go.mod
165164
$(BIN)/copyright: internal/cmd/tools/copyright/licensegen.go
166165
go build -mod=readonly -o $@ ./internal/cmd/tools/copyright/licensegen.go
167166

168-
# dummy binary that ensures most/all packages build, without needing to wait for tests.
169-
$(BUILD)/dummy: $(ALL_SRC) $(BUILD)/go_mod_check
170-
go build -mod=readonly -o $@ internal/cmd/dummy/dummy.go
171-
172167
# ensures mod files are in sync for critical packages
173168
$(BUILD)/go_mod_check: go.mod internal/tools/go.mod
174169
$Q # ensure both have the same apache/thrift replacement
@@ -278,7 +273,10 @@ $Q +$(MAKE) --no-print-directory $(addprefix $(BUILD)/,$(1))
278273
endef
279274

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

283281
.PHONY: lint
284282
# useful to actually re-run to get output again.
@@ -307,8 +305,13 @@ errcheck: $(BIN)/errcheck $(BUILD)/fmt ## (re)run errcheck
307305
.PHONY: generate
308306
generate: $(BUILD)/generate ## run go-generate
309307

308+
.PHONY: tidy
309+
tidy:
310+
go mod tidy
311+
cd internal/tools; go mod tidy
312+
310313
.PHONY: all
311-
all: $(BUILD)/lint ## refresh codegen, lint, and ensure the dummy binary builds, if necessary
314+
all: $(BUILD)/lint ## refresh codegen, lint, and ensure everything builds, whatever is necessary
312315

313316
.PHONY: clean
314317
clean:

docker/buildkite/Dockerfile

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,7 @@ RUN mkdir -p /go/src/go.uber.org/cadence
44
WORKDIR /go/src/go.uber.org/cadence
55

66
ADD go.mod go.sum /go/src/go.uber.org/cadence/
7+
8+
# allow git-status and similar to work
9+
RUN git config --global --add safe.directory /go/src/go.uber.org/cadence
710
RUN GO111MODULE=on go mod download

go.sum

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMyw
9191
github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
9292
github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
9393
github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
94-
github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
94+
github.com/google/go-cmp v0.5.8 h1:e6P7q2lk1O+qJJb4BtCQXlK8vWEO8V1ZeuEdJNOqZyg=
9595
github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
9696
github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI=
9797
github.com/jessevdk/go-flags v1.4.0 h1:4IU2WS7AumrZ/40jfhf4QVDMsQwqA7VEHozFRrGARJA=
@@ -206,8 +206,6 @@ github.com/uber-go/mapdecode v1.0.0/go.mod h1:b5nP15FwXTgpjTjeA9A2uTHXV5UJCl4arw
206206
github.com/uber-go/tally v3.3.12+incompatible/go.mod h1:YDTIBxdXyOU/sCWilKB4bgyufu1cEi0jdVnRdxvjnmU=
207207
github.com/uber-go/tally v3.3.15+incompatible h1:9hLSgNBP28CjIaDmAuRTq9qV+UZY+9PcvAkXO4nNMwg=
208208
github.com/uber-go/tally v3.3.15+incompatible/go.mod h1:YDTIBxdXyOU/sCWilKB4bgyufu1cEi0jdVnRdxvjnmU=
209-
github.com/uber/cadence-idl v0.0.0-20230905165949-03586319b849 h1:j3bfADG1t35Lt4rNRpc9AuQ3l2cGw2Ao25Qt6rDamgc=
210-
github.com/uber/cadence-idl v0.0.0-20230905165949-03586319b849/go.mod h1:oyUK7GCNCRHCCyWyzifSzXpVrRYVBbAMHAzF5dXiKws=
211209
github.com/uber/cadence-idl v0.0.0-20240212223805-34b4519b2709 h1:1u+kMB6p8P9fjK6jk3QHAl8PxLyNjO9/TMXoPOVr1O8=
212210
github.com/uber/cadence-idl v0.0.0-20240212223805-34b4519b2709/go.mod h1:oyUK7GCNCRHCCyWyzifSzXpVrRYVBbAMHAzF5dXiKws=
213211
github.com/uber/jaeger-client-go v2.22.1+incompatible h1:NHcubEkVbahf9t3p75TOCR83gdUHXjRJvjoBh1yACsM=
@@ -299,8 +297,6 @@ golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAG
299297
golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
300298
golang.org/x/oauth2 v0.1.0 h1:isLCZuhj4v+tYv7eskaN4v/TM+A1begWWgyVJDdl1+Y=
301299
golang.org/x/oauth2 v0.1.0/go.mod h1:G9FE4dLTsbXUu90h/Pf85g4w1D+SSAgR+q46nJZ8M4A=
302-
golang.org/x/oauth2 v0.15.0 h1:s8pnnxNVzjWyrvYdFUQq5llS1PX2zhPXmccZv99h7uQ=
303-
golang.org/x/oauth2 v0.15.0/go.mod h1:q48ptWNTY5XWf+JNten23lcvHpLJ0ZSxF5ttTHKVCAM=
304300
golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
305301
golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
306302
golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=

internal/cmd/dummy/dummy.go

Lines changed: 0 additions & 35 deletions
This file was deleted.

scripts/golint.sh

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
#!/bin/sh
2+
3+
# refresh deps
4+
make tidy
5+
# regenerate, format, and make sure everything builds
6+
make build
7+
8+
# intentionally capture stderr, so status-errors are also PR-failing.
9+
# in particular this catches "dubious ownership" failures, which otherwise
10+
# do not fail this check and the $() hides the exit code.
11+
if [ -n "$(git status --porcelain 2>&1)" ]; then
12+
echo "There file changes after applying your diff and performing a build."
13+
echo "Please run this command and commit the changes:"
14+
echo "\tmake tidy && make build"
15+
git status --porcelain
16+
git --no-pager diff
17+
exit 1
18+
fi

0 commit comments

Comments
 (0)