Skip to content

cmd/go: env -w can set (but not remove) an invalid GOOS and GOARCH value #34194

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.

Sign up for GitHub

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

Jump to bottom
Closed
jsign opened this issue Sep 9, 2019 · 4 comments
Closed

cmd/go: env -w can set (but not remove) an invalid GOOS and GOARCH value #34194

jsign opened this issue Sep 9, 2019 · 4 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Backlog

Comments

@jsign
Copy link
Contributor

jsign commented Sep 9, 2019 •
edited
Loading

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 smasher164 added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Sep 9, 2019
@smasher164
Copy link
Member

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
Copy link
Contributor

bcmills commented Sep 10, 2019 •
edited
Loading

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 The path to resolution is known, but the work has not been done. label Sep 10, 2019
@bcmills bcmills added this to the Go1.14 milestone Sep 10, 2019
@gopherbot gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Sep 10, 2019
@jsign
Copy link
Contributor Author

jsign commented Sep 10, 2019 •
edited
Loading

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 mentioned this issue Sep 10, 2019
Closed
@gopherbot
Copy link
Contributor

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

@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@mvdan mvdan mentioned this issue Oct 13, 2019
Closed
@ALTree ALTree mentioned this issue Nov 4, 2019
Closed
@golang golang locked and limited conversation to collaborators Nov 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Backlog
Development

Successfully merging a pull request may close this issue.

cmd/go: make env -w and -u validate GOOS and GOARCH values jsign/go
5 participants
@rsc @bcmills @jsign @gopherbot @smasher164