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: env -w can set (but not remove) an invalid GOOS and GOARCH value #34194

Open
jsign opened this issue Sep 9, 2019 · 4 comments · May be fixed by #34221

Comments

@jsign
Copy link
Contributor

commented Sep 9, 2019

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

~ % go version
go version go1.13 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/ignacio/.cache/go-build"
GOENV="/home/ignacio/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/ignacio/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
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-build527315638=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Write config file regarding GOOS with a typo:
~ % go env -w GOOS=linuxx

What did you expect to see?

Some error indicating the value being incorrect.

What did you see instead?

Command ran successfully and written to config file, then doing go env will result in an error:

~ % go env
cmd/go: unsupported GOOS/GOARCH pair linuxx/amd64

Despite this mean setting an invalid GOOS value, it can't be fixed with -w again:

~ % go env -w GOOS=linux
cmd/go: unsupported GOOS/GOARCH pair linuxx/amd64

So basically I had to edit $GOENV file manually to get unstuck.

The error output is the result of calling MkEnv() in main.go which eventually detects the invalid value and exits. This means that any -w attempt to fix the invalid value won't be reached to make it.

An option can be to do the same control setting a valid value with -w to avoid being invalid.
A similar thing happens if an invalid value is set with export GOOS=linuxx, but that's somehow OK since external env GOOS set isn't in go command control.

Similarly, the same situation happens with an invalid env -w GOARCH.

If checking these situations in env -w makes sense, I can make a PR adding this validation in checkEnvWrite() which I think would the correct place: one extra case "GOOS", "GOARCH":.
Some tests can also be added to cover this.

@smasher164

This comment has been minimized.

Copy link
Member

commented Sep 9, 2019

I think it makes sense to check for a valid GOOS/GOARCH before writing in the first place, so checkEnvWrite seems like the right way to do this. /cc @rsc @bcmills

@bcmills bcmills changed the title cmd/go: env -w allows an invalid GOOS and GOARCH value cmd/go: env -w can set (but not remove) an invalid GOOS and GOARCH value Sep 10, 2019

@bcmills

This comment has been minimized.

Copy link
Member

commented Sep 10, 2019

Certainly go env should be made consistent one way or the other: if it accepts a value without error, then subsequent invocations of go env should not reject that value.

CC @jayconrod

@bcmills bcmills added the NeedsFix label Sep 10, 2019

@bcmills bcmills added this to the Go1.14 milestone Sep 10, 2019

@jsign

This comment has been minimized.

Copy link
Contributor Author

commented Sep 10, 2019

Pushing things a bit, proper checking in go env -w isn't enough. Doing go env -u could lead to an incorrect combination too, e.g (my machine):

$ go env GOOS GOARCH
linux
amd64
$ go env -w GOOS=nacl GOARCH=amd64p32
$ go env 
(... works as expected since nacl/amd64p32 is a valid combination ...)
$ go env -u GOOS
(... works, which isn't correct)
$ go env 
cmd/go: unsupported GOOS/GOARCH pair linux/amd64p32

Basically, the go env -u should do a similar check if GOOS or GOARCH are unset.

If you decide to go into the check when change option, looking again checkEnvWrite() only does checks for independent variables changes, so I think it can't be leveraged.
In this case, GOOS or GOARCH can't be validated independently but in combination. Making checks here would work only if GOOS or GOARCH are being touched, but not both in the same go env -w (as the example above).

@jsign jsign referenced a pull request that will close this issue Sep 10, 2019
@gopherbot

This comment has been minimized.

Copy link

commented Sep 10, 2019

Change https://golang.org/cl/194617 mentions this issue: cmd/go: env -w and -u check for invalid GOOS and GOARCH values

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.