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

cmd/go: go env output is unstable/non-deterministic #42628

Open
myitcv opened this issue Nov 16, 2020 · 8 comments
Open

cmd/go: go env output is unstable/non-deterministic #42628

myitcv opened this issue Nov 16, 2020 · 8 comments
Milestone

Comments

@myitcv
Copy link
Member

@myitcv myitcv commented Nov 16, 2020

What version of Go are you using (go version)?

$ go version
go version devel +7307e86afd Sun Nov 8 12:19:55 2020 +0000 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/myitcv/.cache/go-build"
GOENV="/home/myitcv/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/myitcv/gostuff/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/myitcv/gos"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build332437426=/tmp/go-build -gno-record-gcc-switches"

What did you do?

As you can see from the go env output above, GOGCCFLAGS includes a temporary build directory path that changes with each invocation of go env.

Whilst putting together a Play with Go guide on installing Go (https://play-with-go.dev/installing-go_go115_en/) I noticed this unstable output. Unfortunately, it makes writing guides that have automated checks to ensure they still work impossible without a custom sanitisation step (c.f. conversation with @bcmills on Slack about the output of go get being non-deterministic).

Can the output of go env be made deterministic in the same way?

cc @bcmills @jayconrod

FYI @mvdan

@mvdan
Copy link
Member

@mvdan mvdan commented Nov 16, 2020

I can't help but wonder, is that -fdebug-prefix-map=/tmp/go-build332437426=/tmp/go-build flag ever going to do anything useful as part of go env?

Option 1 is that it's a temporary directory created for a following build, which does not happen with go env as it just prints the environment and exits. So the temporary directory is deleted, and the flag in go env makes no sense.

Option 2 is that the temporary directory is created and left behind, to perhaps allow the flag to be useful at a later time. I find this almost worse than option 1, because this would mean that go env invocations leave temporary directories behind.

I imagine it's option 1, in which case go env should not print the flag.

@jayconrod
Copy link
Contributor

@jayconrod jayconrod commented Nov 16, 2020

/tmp/go-build332437426 is a temporary work directory. We don't necessarily need to include the -fdebug-prefix-map flag in GOGCCFLAGS. There are other cases, especially when -trimpath is used, when we add a lot of similar flags that aren't included in go env.

I wonder if it would make more sense to match the output with a regular expression though, like we do in the cmd/go script tests? The script tests seem like a very similar use case, though I guess the difference is that in tests, we can add lots of extra non-user-friendly commands to make the output more machine-readable (like go list -f with a complicated template).

@jayconrod
Copy link
Contributor

@jayconrod jayconrod commented Nov 16, 2020

@mvdan go env creates (and deletes) this directory to test which flags the C toolchain supports. I think in 1.15, go env does that unconditionally. In 1.16, it will only check the C toolchain when run without arguments or with a C-specific argument (like go env GOGCCFLAGS).

@myitcv
Copy link
Member Author

@myitcv myitcv commented Nov 16, 2020

Thanks, @jayconrod

I wonder if it would make more sense to match the output with a regular expression though, like we do in the cmd/go script tests?

That's exactly how we are sanitising the output from various commands (we need to do the same for go test). I'm certainly not averse to doing that, just flagging that I was surprised that go env needed this treatment too, because from one invocation to the next, my environment hasn't changed.

The script tests seem like a very similar use case, though I guess the difference is that in tests, we can add lots of extra non-user-friendly commands to make the output more machine-readable (like go list -f with a complicated template).

Exactly. The guides themselves aim to present a very simple sequence of steps for the user; they see and run those commands. So we have to keep things as straightforward and as little magic as possible.

@bcmills
Copy link
Member

@bcmills bcmills commented Nov 16, 2020

I think we should make sure that go env is deterministic and reproducible. We could either drop the -fdebug-prefix-map argument from the printed GOGCCFLAGS, or perhaps replace the /tmp/go-build* portion with some environment variable (similar to $WORK).

@bcmills
Copy link
Member

@bcmills bcmills commented Nov 16, 2020

In particular, it seems very reasonable to me for wrappers around the go command to invalidate their own caches based on changes in the output of go env, especially the compiler-argument variables.

@mvdan
Copy link
Member

@mvdan mvdan commented Nov 16, 2020

Care to clarify if this would be fine to fix during the current freeze? It almost seems simple enough that I might give it a try.

I'd personally go for removing the flag argument entirely, because I can't think how it would ever be useful as part of the go env output. It's really an internal detail that shouldn't be surfaced.

@bcmills
Copy link
Member

@bcmills bcmills commented Nov 16, 2020

Care to clarify if this would be fine to fix during the current freeze? It almost seems simple enough that I might give it a try.

I think it depends on how invasive the fix is? I did a bit of investigating and this seems to be reproducible all the way back to Go 1.10 (that is, the bug is as old as the build cache).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.