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 test -race does not set implicit race build tag #54468

Closed
578559967 opened this issue Aug 16, 2022 · 14 comments
Closed

cmd/go: go test -race does not set implicit race build tag #54468

578559967 opened this issue Aug 16, 2022 · 14 comments
Assignees
Labels
FrozenDueToAge GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@578559967
Copy link

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

$ go version
go version go1.19 darwin/arm64

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="arm64"
GOBIN=""
GOCACHE="/Users/r/Library/Caches/go-build"
GOENV="/Users/r/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/r/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/r/go"
GOPRIVATE=""
GOPROXY="https://goproxy.cn,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.19"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/r/workspace/testcode/testrace/tmp/go.mod"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/lr/5ttkwx8x6s9fm23m80546v180000gn/T/go-build4125567734=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

run_test.go:

//go:build race

package tmp

import (
        "testing"

        _ "github.com/gogo/protobuf/proto"
)

func TestRun(t *testing.T) {
}

go.mod:

module testrace/tmp

go 1.19

require github.com/gogo/protobuf v1.3.2

go.sum:

github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q=
github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q=
github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8=
github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU=
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=

compile and run the test:

$ go test -race ./...

What did you expect to see?

expect to see:

ok      testrace/tmp    0.425s

What did you see instead?

# testrace/tmp
run_test.go:8:2: cannot find package
FAIL    testrace/tmp [setup failed]
FAIL

but the go1.17.10 can compile successfully and output the expected result.

@mengzhuo
Copy link
Contributor

This is WAI, as your code shows

//go:build race

You are using build constraints https://pkg.go.dev/go/build#hdr-Build_Constraints

In the meantime go test -race is passing a race argument into compiler (note it's not a build tag)

@mengzhuo mengzhuo closed this as not planned Won't fix, can't repro, duplicate, stale Aug 16, 2022
@578559967
Copy link
Author

This is WAI, as your code shows

//go:build race

You are using build constraints https://pkg.go.dev/go/build#hdr-Build_Constraints

In the meantime go test -race is passing a race argument into compiler (note it's not a build tag)

Won't go add build tag "race" when race enabled?
Why go1.17 works fine?
Why it works fine if not import thirdparty package?
And when I add "-x -a" flags, I see it compiles go/src/runtime/race.go which also has build constraints "go:build race" in its head.

@mengzhuo
Copy link
Contributor

Go1.17 isn't works fine, it's a bug.
The race argument should be command line only, and this is droped at 931d80e (cmd/go/internal/work/init.go), which @rsc stats

  • Change the tags list to match the -tags command-line argument.
  • Add msan and race, echoing the -msan and -race arguments
    (always 'true' when present, omitted when false).

If you insist this is a bug, I can reopen it.

@ianlancetaylor
Copy link
Contributor

I think there is a bug here. We may be enabling the "race" build constraint for some purposes but not others. Changing race to, say, race1 in the test case produces a completely different result (go: warning: "./..." matched no packages).

@ianlancetaylor ianlancetaylor added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go labels Aug 16, 2022
@ianlancetaylor ianlancetaylor added this to the Go1.20 milestone Aug 16, 2022
@578559967
Copy link
Author

I think this is a bug.
If "-race" does not add tag "race", it should not compile the test file "run_test.go" and will output go: warning: "./..." matched no packages instead. But actually it compiles the file and report the import error.

And I find that if we build with the single file instead of "./...", it will pass:

$ go test -race ./run_test.go
ok      command-line-arguments  0.362s

$ go test -race ./...
# testrace/tmp
run_test.go:8:2: cannot find package
FAIL    testrace/tmp [setup failed]
FAIL

@bcmills
Copy link
Contributor

bcmills commented Aug 17, 2022

The implicit race build tag is documented in https://go.dev/doc/articles/race_detector#Excluding_Tests, so I agree that this looks like a cmd/go bug.

@bcmills
Copy link
Contributor

bcmills commented Aug 17, 2022

@578559967, can you identify a Go version at which the test worked as expected (perhaps using // +build instead of //go:build, if it was prior to #41184)? If this is a regression, we can probably bisect it.

@bcmills bcmills added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Aug 17, 2022
@578559967
Copy link
Author

@578559967, can you identify a Go version at which the test worked as expected (perhaps using // +build instead of //go:build, if it was prior to #41184)? If this is a regression, we can probably bisect it.

go1.17.5、go1.17.10、go1.17.13 all worked as expected.
go1.18~go1.18.5、go1.19 all failed.

@gopherbot
Copy link

Change https://go.dev/cl/425154 mentions this issue: cmd/go/internal/imports: loadTags() should merge tags from BuildContext.ToolTags

@ZekeLu
Copy link
Contributor

ZekeLu commented Aug 23, 2022

git bisect shows that 931d80e is the commit that changed the behavior.

diff --git a/src/cmd/go/internal/work/init.go b/src/cmd/go/internal/work/init.go
index dc368de1c1..26192ecaed 100644
--- a/src/cmd/go/internal/work/init.go
+++ b/src/cmd/go/internal/work/init.go
@@ -138,7 +138,7 @@ func instrumentInit() {
        cfg.BuildContext.InstallSuffix += "_"
    }
    cfg.BuildContext.InstallSuffix += mode
-   cfg.BuildContext.BuildTags = append(cfg.BuildContext.BuildTags, mode)
+   cfg.BuildContext.ToolTags = append(cfg.BuildContext.ToolTags, mode)
 }

BuildContext.ToolTags is introduced by dba8928. cmd/go/internal/imports.loadTags() should be updated to merge tags from BuildContext.ToolTags. I have sent a CL to fix that.

@bcmills bcmills self-assigned this Aug 24, 2022
@bcmills bcmills removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Aug 24, 2022
@bcmills
Copy link
Contributor

bcmills commented Aug 24, 2022

@gopherbot, please backport to Go 1.19 and 1.18. This is a regression from prior releases that can cause build failures.

@gopherbot
Copy link

Backport issue(s) opened: #54659 (for 1.18), #54660 (for 1.19).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@bcmills bcmills changed the title cmd/go: go test -race report cannot find package when test has "//go:build race" cmd/go: go test -race does not set implicit race build tag Aug 24, 2022
@bcmills bcmills changed the title cmd/go: go test -race does not set implicit race build tag cmd/go: go test -race does not set implicit race build tag Aug 24, 2022
@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Aug 25, 2022
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 25, 2022
@gopherbot
Copy link

Change https://go.dev/cl/426434 mentions this issue: [release-branch.go1.19] cmd/go/internal/imports: include ToolTags in the Tags map

@gopherbot
Copy link

Change https://go.dev/cl/426435 mentions this issue: [release-branch.go1.18] cmd/go/internal/imports: include ToolTags in the Tags map

gopherbot pushed a commit that referenced this issue Aug 29, 2022
…the Tags map

This fixes a regression introduced when the "race" mode tag was moved to
the ToolTags field in CL 358539.

Fixes #54659
Updates #54468

Change-Id: I107771948a4fe9d743cc13d1c15f324212b08e03
GitHub-Last-Rev: d211e35
GitHub-Pull-Request: #54618
Reviewed-on: https://go-review.googlesource.com/c/go/+/425154
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit 6ba2674)
Reviewed-on: https://go-review.googlesource.com/c/go/+/426435
Reviewed-by: Heschi Kreinick <heschi@google.com>
gopherbot pushed a commit that referenced this issue Aug 29, 2022
…the Tags map

This fixes a regression introduced when the "race" mode tag was moved to
the ToolTags field in CL 358539.

Fixes #54660
Updates #54468

Change-Id: I107771948a4fe9d743cc13d1c15f324212b08e03
GitHub-Last-Rev: d211e35
GitHub-Pull-Request: #54618
Reviewed-on: https://go-review.googlesource.com/c/go/+/425154
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit 6ba2674)
Reviewed-on: https://go-review.googlesource.com/c/go/+/426434
Reviewed-by: Heschi Kreinick <heschi@google.com>
bradfitz pushed a commit to tailscale/go that referenced this issue Sep 8, 2022
…the Tags map

This fixes a regression introduced when the "race" mode tag was moved to
the ToolTags field in CL 358539.

Fixes golang#54660
Updates golang#54468

Change-Id: I107771948a4fe9d743cc13d1c15f324212b08e03
GitHub-Last-Rev: d211e35
GitHub-Pull-Request: golang#54618
Reviewed-on: https://go-review.googlesource.com/c/go/+/425154
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit 6ba2674)
Reviewed-on: https://go-review.googlesource.com/c/go/+/426434
Reviewed-by: Heschi Kreinick <heschi@google.com>
@golang golang locked and limited conversation to collaborators Aug 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants