Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

all: CI's enforcement of clean go generate is lacking #70954

Open
Jorropo opened this issue Dec 21, 2024 · 6 comments
Open

all: CI's enforcement of clean go generate is lacking #70954

Jorropo opened this issue Dec 21, 2024 · 6 comments
Labels
help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@Jorropo
Copy link
Member

Jorropo commented Dec 21, 2024

Go version

go version devel go1.24-110ab1aaf4 Sat Dec 21 08:22:08 2024 -0800 linux/amd64

Output of go env in your module/workspace:

AR='ar'
CC='gcc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='g++'
GCCGO='gccgo'
GO111MODULE=''
GOAMD64='v3'
GOARCH='amd64'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/tmp/go-build'
GODEBUG=''
GOENV='/home/hugo/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build610608568=/tmp/go-build -gno-record-gcc-switches'
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMOD='/home/hugo/k/go/src/go.mod'
GOMODCACHE='/home/hugo/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/hugo/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/hugo/k/go'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/hugo/.config/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/home/hugo/k/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='devel go1.24-110ab1aaf4 Sat Dec 21 08:22:08 2024 -0800'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

In the go repo:

cd src
./make.bash
cd internal/goexperiment/ && go generate .
git status

What did you see happen?

On branch master
Your branch is up to date with 'origin/master'.

Untracked files:
  (use "git add <file>..." to include in what will be committed)
	exp_synctest_off.go
	exp_synctest_on.go

nothing added to commit but untracked files present (use "git add" to track)

What did you expect to see?

On branch master
Your branch is up to date with 'origin/master'.

nothing to commit, working tree clean
@Jorropo
Copy link
Member Author

Jorropo commented Dec 21, 2024

cc @neild ¿ 76e4efd ?

@Jorropo Jorropo changed the title internal/goexperiement: go generate state is dirty due to synctest internal/goexperiment: go generate state is dirty due to synctest Dec 21, 2024
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Dec 21, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/638355 mentions this issue: internal/goexperiment: run go generate for synctest

@Jorropo
Copy link
Member Author

Jorropo commented Dec 21, 2024

I am opening this issue to question our testing procedure around go generate, this is the second time I was working on something and I first need to fix go generate: https://go-review.googlesource.com/c/go/+/603995

It looks to me like we lack some go generate none diff checks in CI even for slowbots.

@Jorropo Jorropo added NeedsFix The path to resolution is known, but the work has not been done. FixPending Issues that have a fix which has not yet been reviewed or submitted. and removed FixPending Issues that have a fix which has not yet been reviewed or submitted. labels Dec 21, 2024
@Jorropo Jorropo changed the title internal/goexperiment: go generate state is dirty due to synctest all: CI's enforcement of clean go generate is lacking Dec 21, 2024
@Jorropo Jorropo removed NeedsFix The path to resolution is known, but the work has not been done. compiler/runtime Issues related to the Go compiler and/or runtime. labels Dec 21, 2024
gopherbot pushed a commit that referenced this issue Dec 23, 2024
Updates #70954

Change-Id: If5f9c8b8b820b1cc4e41e76b50038c6155b575a8
Reviewed-on: https://go-review.googlesource.com/c/go/+/638355
Reviewed-by: David Chase <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Michael Pratt <[email protected]>
@cherrymui cherrymui added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 23, 2024
@cherrymui cherrymui added this to the Backlog milestone Dec 23, 2024
@dmitshur
Copy link
Contributor

Some packages have their own test that checks that generated files are up to date (for example, see here and here). Generally, tests in GOROOT shouldn't write to GOROOT itself, so these tests take care not to do that, and instead make that functionality optionally available via a -fix/-write flag.

There's also the cmd/internal/moddeps test which copies the entire tree to a temporary directory to make it easy to check for inconsistencies and report them, without writing to GOROOT itself. As the name implies, these tests are intended to check things that need to be regenerated and/or kept in sync after pulling in newer versions of dependencies. It can be easily added there. It'd be a bit odd to check these things there, but the advantage is that it's effortless and wouldn't require copying the tree an additional time.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/638582 mentions this issue: cmd/internal/moddeps: check that goexperiment is generated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

5 participants