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

runtime: support GOFLAGS=-race everywhere #35324

Open
eliasnaur opened this issue Nov 3, 2019 · 6 comments
Labels

Comments

@eliasnaur
Copy link
Contributor

@eliasnaur eliasnaur commented Nov 3, 2019

I'd like to set GOFLAGS=-race on a CI builder. However, some tests invoke the go tool to build for GOOS=js where -race is not supported, leading to this error:

$ GOFLAGS=-race GOOS=js GOARCH=wasm go build std
go build: -race is only supported on linux/amd64, linux/ppc64le, linux/arm64, freebsd/amd64, netbsd/amd64, darwin/amd64 and windows/amd64

I believe unsupported flags should be ignored (unlike unknown flags).

@ALTree

This comment has been minimized.

Copy link
Member

@ALTree ALTree commented Nov 3, 2019

I believe unsupported flags should be ignored

But this way the user would be misled into thinking they are testing with the race detector activated, when they're not.

@ALTree ALTree added the NeedsDecision label Nov 3, 2019
@mvdan

This comment has been minimized.

Copy link
Member

@mvdan mvdan commented Nov 3, 2019

Perhaps we could show a warning instead of an error? There's a precedent for non-fatal warning messages in the go tool.

@mvdan

This comment has been minimized.

Copy link
Member

@mvdan mvdan commented Nov 3, 2019

Thinking outloud some more - what we want is a best-effort GOFLAGS. It kind of is like that already, since commands ignore flags they don't know about, like @eliasnaur said. We want to go a bit further and go past "unsupported" errors. If a user really wants to force the use of a flag, they could use go build -race, which will get the current behavior.

@ALTree

This comment has been minimized.

Copy link
Member

@ALTree ALTree commented Nov 3, 2019

IMO there is a fundamental difference between

  • This flag is not known to us
  • It does not make sense to enable this flag -which yes, we do support- in this specific configuration

Consider the following (which currently errors):

$ GOFLAGS="-buildmode=pie -race" go build test.go

-buildmode=pie not supported when -race is enabled

Which one do we ignore? We build with pie and ignore -race? Or we build with -race and ignore pie? Or we ignore both and we build a non-race, non-pie binary?

It seems to me that this could get confusing quickly, from a UX point of view.

@mvdan

This comment has been minimized.

Copy link
Member

@mvdan mvdan commented Nov 3, 2019

You raise a good point. Treating -race especially within GOFLAGS could add complexity too.

@mvdan

This comment has been minimized.

Copy link
Member

@mvdan mvdan commented Nov 3, 2019

For what it's worth, we made it so that the test that builds with GOOS=js skips itself when the race tag is enabled. Detecting the build tag requires a global bool and an extra file, but the setup is simple enough.

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