Server-like `make build` and ensuring builds are clean in CI #1329

Groxx · 2024-03-30T00:13:28Z

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.

Probably should've done this a while ago, but the sooner the better.

codecov · 2024-03-30T00:16:45Z

Codecov Report

Merging #1329 (20b4c8d) into master (a55fc13) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a55fc13...20b4c8d. Read the comment docs.

…lt, but does not print error messages it encounters

…/bin/true: no such file or directory

This reverts commit 6ec6aa8.

davidporter-id-au · 2024-03-31T20:36:03Z

Makefile

+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


any reason for >/dev/null here?

Just that it's quite noisy, printing every package at it builds it, and somewhat misleading as it looks like test runs but it didn't run any actual tests.
(same reasons as for the server)

davidporter-id-au · 2024-03-31T20:38:36Z

scripts/golint.sh

+# 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


nit: generally prefer [[ and it's not needed, but also set -o pipefail

I'm pretty sure I do still need to capture output with $(), and bash / [[ don't change this semantic in that sense... but this is more than niche enough that I haven't actually tried it so now I'm curious. let's see what happens.

... and I'm struggling to reproduce exactly what I thought I remembered 🤔
I might be getting it mixed up with how export x=$(...) behaves too (as that also suppresses failures).

gonna just stick with what the server does here since it's reasonably well proven over there, and also seems to work correctly here. and that way they're at least consistent and the path to fix/improve if we do find something conclusive is obvious.

bash + pipefail does seem promising enough to check at another time though.

davidporter-id-au · 2024-03-31T20:38:47Z

scripts/golint.sh

@@ -0,0 +1,18 @@
+#!/bin/sh


shouldn't this be /bin/bash?

In this, the year of 50 of Mechagodzilla's incarnation, is there any reason to still use shell?

this is basically just copying from github.com/uber/cadence so any change we'd apply here we should probably keep in sync.

but: agreed! though this is mostly executed in docker, and docker is less likely to have bash for space reasons. I suspect our buildkite does have bash installed, but it's not quite a practical guarantee like it is on developer machines.

Groxx · 2024-04-01T17:31:44Z

internal/cmd/dummy/dummy.go

@@ -1,35 +0,0 @@
-// Copyright (c) 2017 Uber Technologies, Inc.


no longer necessary at all - go build ./... is much better proof of build-ability.

pipefail

test

last try

sh check

no pipefail til

going back to before, really not sure any more how bash / sh / $() work. so match the server setup so we at least have the same behavior (which is reasonably well tested both there and here), and fixing both will be easier if needed.

Groxx · 2024-04-01T20:15:07Z

Makefile

@@ -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


go build ./... does this same check substantially better

restore safe.directory

Groxx added 6 commits March 29, 2024 17:50

Pin mockery and regenerate everything

dce97e4

makefile

3ec1397

might be finished

9f0c370

minor polish to regen less

63648ac

no need for manual copyright run

b23f6ba

Server-like make build and ensuring builds are clean in CI

02bcbd8

Probably should've done this a while ago, but the sooner the better.

Groxx changed the title ~~tidy~~ Server-like make build and ensuring builds are clean in CI Mar 30, 2024

remove dummy

de988d3

Groxx added 12 commits March 29, 2024 19:21

maybe this works for ci, which seems to have no git set up

46cc722

debugging

4cf7626

debugging

1d0e878

debugging

801dd72

man it is just SO USEFUL that go requires git status to work by defau…

a6d0b28

…lt, but does not print error messages it encounters

moooore

4d7464d

wut

b40b8eb

weird

79f7557

seriously what, why is this not coming through stderr: fork/exec /usr…

11d38a5

…/bin/true: no such file or directory

ok I think I fixed it finally, srsly wtf

aaddb09

testing

6ec6aa8

Revert "testing", it works

47ef716

This reverts commit 6ec6aa8.

Groxx marked this pull request as ready for review March 30, 2024 00:49

Groxx requested review from shijiesheng, agautam478, jakobht, 3vilhamster, dkrotx, taylanisikdemir and demirkayaender as code owners March 30, 2024 00:49

davidporter-id-au reviewed Mar 31, 2024

View reviewed changes

davidporter-id-au approved these changes Mar 31, 2024

View reviewed changes

taylanisikdemir approved these changes Apr 1, 2024

View reviewed changes

Merge branch 'master' into tidy

b598ed2

Groxx commented Apr 1, 2024

View reviewed changes

Groxx added 8 commits April 1, 2024 12:32

Update golint.sh

2ce8efa

pipefail

try bash and [[

7650eb5

Update golint.sh

2b017f8

Update golint.sh

da95f7f

test

Update golint.sh

fd03dce

last try

Update golint.sh

f4c7455

sh check

Update golint.sh

7898cda

no pipefail til

Update golint.sh

1f84cfb

going back to before, really not sure any more how bash / sh / $() work. so match the server setup so we at least have the same behavior (which is reasonably well tested both there and here), and fixing both will be easier if needed.

Groxx commented Apr 1, 2024

View reviewed changes

Update Dockerfile

20b4c8d

restore safe.directory

Groxx merged commit 68afcb9 into cadence-workflow:master Apr 1, 2024
13 checks passed

Groxx deleted the tidy branch April 1, 2024 20:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Server-like `make build` and ensuring builds are clean in CI #1329

Server-like `make build` and ensuring builds are clean in CI #1329

Groxx commented Mar 30, 2024 •

edited

Loading

codecov bot commented Mar 30, 2024 •

edited

Loading

davidporter-id-au Mar 31, 2024

Groxx Mar 31, 2024 •

edited

Loading

davidporter-id-au Mar 31, 2024

Groxx Apr 1, 2024

Groxx Apr 1, 2024 •

edited

Loading

davidporter-id-au Mar 31, 2024 •

edited

Loading

Groxx Apr 1, 2024

Groxx Apr 1, 2024

Groxx Apr 1, 2024

		@@ -1,35 +0,0 @@
		// Copyright (c) 2017 Uber Technologies, Inc.

Server-like make build and ensuring builds are clean in CI #1329

Server-like make build and ensuring builds are clean in CI #1329

Conversation

Groxx commented Mar 30, 2024 • edited Loading

codecov bot commented Mar 30, 2024 • edited Loading

Codecov Report

davidporter-id-au Mar 31, 2024

Choose a reason for hiding this comment

Groxx Mar 31, 2024 • edited Loading

Choose a reason for hiding this comment

davidporter-id-au Mar 31, 2024

Choose a reason for hiding this comment

Groxx Apr 1, 2024

Choose a reason for hiding this comment

Groxx Apr 1, 2024 • edited Loading

Choose a reason for hiding this comment

davidporter-id-au Mar 31, 2024 • edited Loading

Choose a reason for hiding this comment

Groxx Apr 1, 2024

Choose a reason for hiding this comment

Groxx Apr 1, 2024

Choose a reason for hiding this comment

Groxx Apr 1, 2024

Choose a reason for hiding this comment

Server-like `make build` and ensuring builds are clean in CI #1329

Server-like `make build` and ensuring builds are clean in CI #1329

Groxx commented Mar 30, 2024 •

edited

Loading

codecov bot commented Mar 30, 2024 •

edited

Loading

Groxx Mar 31, 2024 •

edited

Loading

Groxx Apr 1, 2024 •

edited

Loading

davidporter-id-au Mar 31, 2024 •

edited

Loading