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.

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

cmd/go: non-test race libraries are rebuilt by "go test -i -race" and vice-versa #19151

Closed
FiloSottile opened this issue Feb 17, 2017 · 12 comments
Closed

cmd/go: non-test race libraries are rebuilt by "go test -i -race" and vice-versa #19151

FiloSottile opened this issue Feb 17, 2017 · 12 comments
Labels
FrozenDueToAge
Milestone
Go1.8.1

Comments

@FiloSottile
Copy link
Contributor

Please answer these questions before submitting your issue. Thanks!

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

go version go1.8 linux/amd64

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/root/go"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build041564216=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

What did you do?

$ go install -v -race std
[... lots of libraries installed ...]
$ go test -race -i -v std
[... lots of libraries installed ...]
$ go install -v -race std
[... lots of libraries installed ...]

What did you expect to see?

All commands after the first should not need to install anything.

It works correctly without -race:

$ go install -v std
[... lots of libraries installed ...]
$ go test -i -v std
$ go install -v std

What did you see instead?

Libraries are rebuilt and reinstalled every time the -race flag is added or removed.

This is a problem because we ship our compiler with the race libraries prebuilt, install it system-wide where the builders don't have permission to write, and then always run go test -i -race before our tests to do incremental compilation of GOPATH packages. That now tries to install stdlib packages and fails.

This is a regression from 1.7.
@FiloSottile FiloSottile changed the title Installed race libraries are rebuilt by "go test -i -race" Non-test race libraries are rebuilt by "go test -i -race" and vice-versa Feb 17, 2017
@FiloSottile FiloSottile changed the title Non-test race libraries are rebuilt by "go test -i -race" and vice-versa cmd/go: non-test race libraries are rebuilt by "go test -i -race" and vice-versa Feb 17, 2017
@ianlancetaylor ianlancetaylor added this to the Go1.8.1 milestone Feb 17, 2017
@amenzhinsky
Copy link
Contributor

#19133

@gopherbot
Copy link

CL https://golang.org/cl/37594 mentions this issue.

@gopherbot
Copy link

CL https://golang.org/cl/37598 mentions this issue.

@Bo0mer Bo0mer mentioned this issue Mar 8, 2017
Closed
@aclements
Copy link
Member

Re-opening for cherry-pick to 1.8.1

@ianlancetaylor, do you consider this low enough risk for a point release?

@aclements aclements reopened this Mar 21, 2017
@ianlancetaylor
Copy link
Contributor

Does this really meet the criterion for a minor release? It is an unfortunate regression, but it's also fairly easy to avoid using both -i and -race with go test.

@FiloSottile
Copy link
Contributor Author

FiloSottile commented Mar 21, 2017 •
edited

but it's also fairly easy to avoid using both -i and -race with go test

That means no incremental builds for tests, and race builds are slow.

Worse, it means rebuilding the stdlib every time, assuming you used go install -race at any point.

Also, it broke a number of projects at Cloudflare because it causes writes to GOROOT, which result in a "Permission denied", which I can't fix at the package level.

@ianlancetaylor
Copy link
Contributor

I think that https://golang.org/cl/37598 should be safe enough for the 1.8 release branch. It will need to be tested, though. Unfortunately I didn't think of a way to test that CL safely without modifying GOROOT.

@flowchartsman flowchartsman mentioned this issue Mar 29, 2017
Closed
@rsc
Copy link
Contributor

rsc commented Apr 5, 2017

Sent CL 39592 to add a test that we can cherry-pick at the same time.

@gopherbot
Copy link

CL https://golang.org/cl/39592 mentions this issue.

gopherbot pushed a commit that referenced this issue Apr 5, 2017
@rsc
cmd/go: add test for test -race -i behavior
10991fd 
This was fixed in CL 37598 but the test was (rightly) dropped
because it modified $GOROOT. Here's a variant that does not.

For #19151.

Change-Id: Iccdbbf9ae8ac4c252e52f4f8ff996963573c4682
Reviewed-on: https://go-review.googlesource.com/39592
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@gopherbot
Copy link

CL https://golang.org/cl/39618 mentions this issue.

@gopherbot
Copy link

CL https://golang.org/cl/39617 mentions this issue.

gopherbot pushed a commit that referenced this issue Apr 5, 2017
@rsc
[release-branch.go1.8] cmd/go: do not install broken libraries during…
166f215 
… 'go test -i -race'

Manual port of CL 37598 (submitted for Go 1.9) to Go 1.8.1.

Fixes #19133.
Fixes #19151.

Change-Id: I51707ea35068a393022f554b391ee2638dba16b5
Reviewed-on: https://go-review.googlesource.com/39617
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
gopherbot pushed a commit that referenced this issue Apr 5, 2017
@rsc
[release-branch.go1.8] cmd/go: add test for test -race -i behavior
853d533 
This was fixed in CL 37598 but the test was (rightly) dropped
because it modified $GOROOT. Here's a variant that does not.

For #19151.

Change-Id: Iccdbbf9ae8ac4c252e52f4f8ff996963573c4682
Reviewed-on: https://go-review.googlesource.com/39592
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-on: https://go-review.googlesource.com/39618
Reviewed-by: Austin Clements <austin@google.com>
@rsc
Copy link
Contributor

rsc commented Apr 5, 2017

Cherry-picked.

@rsc rsc closed this as completed Apr 5, 2017
kimh pushed a commit to CircleCI-Archived/image-builder that referenced this issue Apr 7, 2017
@yangkookkim
Reverting Go
04298fb 
Some customers had deployment issue because of
 golang/go#19151
lparth pushed a commit to lparth/go that referenced this issue Apr 13, 2017
@rsc @lparth
cmd/go: add test for test -race -i behavior
9f7f2fc 
This was fixed in CL 37598 but the test was (rightly) dropped
because it modified $GOROOT. Here's a variant that does not.

For golang#19151.

Change-Id: Iccdbbf9ae8ac4c252e52f4f8ff996963573c4682
Reviewed-on: https://go-review.googlesource.com/39592
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators Apr 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.8.1
Development

No branches or pull requests
6 participants
@rsc @FiloSottile @amenzhinsky @aclements @ianlancetaylor @gopherbot