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/cgo/internal: platform-specific test files are missing build constraints #60164

Closed
findleyr opened this issue May 12, 2023 · 5 comments
Closed
Assignees
Labels
NeedsFix The path to resolution is known, but the work has not been done. release-blocker Soon This needs to be done soon. (regressions, serious bugs, outages) Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone

Comments

@findleyr
Copy link
Contributor

findleyr commented May 12, 2023

package nosuchpkg is not in GOROOT (C:\workdir\go\src\nosuchpkg)
package nosuchpkg is not in GOROOT (C:\workdir\go\src\nosuchpkg)
open C:\workdir\gopath\src\golang.org\x\tools\go\loader\missing.go: The system cannot find the file specified.
open C:\workdir\gopath\src\golang.org\x\tools\go\loader\missing.go: The system cannot find the file specified.
C:\workdir\gopath\src\golang.org\x\tools\go\loader\testdata\badpkgdecl.go:1:34: expected 'package', found 'EOF'
C:\workdir\gopath\src\golang.org\x\tools\go\loader\testdata\badpkgdecl.go:1:34: expected 'package', found 'EOF'
\go\src\b\x.go:1:21: could not import c (cannot find package "c" in any of:
	\go\src\c (from $GOROOT)
	($GOPATH not set. For more details see: 'go help gopath'))
\go\src\b\x.go:1:21: could not import c (cannot find package "c" in any of:
	\go\src\c (from $GOROOT)
	($GOPATH not set. For more details see: 'go help gopath'))
\go\src\b\x.go:1:21: could not import c (\go\src\c\x.go:1:8: expected 'IDENT', found 'EOF')
\go\src\c\x.go:1:20: expected operand, found 'EOF'
cannot find package "two/three" in any of:
	\go\src\two\three (from $GOROOT)
	($GOPATH not set. For more details see: 'go help gopath')
cannot find package "http" in any of:
	\go\src\vendor\http (vendor tree)
	\go\src\http (from $GOROOT)
	($GOPATH not set. For more details see: 'go help gopath')
\go\src\c\x.go:1:31: cannot convert false (untyped bool constant) to type int
C:\workdir\go\src\cmd\cgo\internal\testsanitizers\cc_test.go:556:3: unknown field Pdeathsig in struct literal of type syscall.SysProcAttr
--- FAIL: TestStdlib (80.49s)
    stdlib_test.go:55: Load failed: couldn't load packages due to errors: cmd/cgo/internal/testsanitizers_test
FAIL
FAIL	golang.org/x/tools/go/loader	91.176s

https://build.golang.org/log/68018cc7868317a8acd303829ef65abfbcea625a

CC @bcmills @golang/release @golang/tools-team

@findleyr findleyr changed the title golang.org/x/tools/go/loader: windows-amd64-longtest builder is failing at tip golang.org/x/tools/go/loader: (windows|darwin)-amd64-longtest builder is failing at tip May 12, 2023
@seankhliao seankhliao changed the title golang.org/x/tools/go/loader: (windows|darwin)-amd64-longtest builder is failing at tip x/tools/go/loader: windows-amd64-longtest builder is failing at tip May 12, 2023
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label May 12, 2023
@gopherbot gopherbot added this to the Unreleased milestone May 12, 2023
@findleyr findleyr changed the title x/tools/go/loader: windows-amd64-longtest builder is failing at tip x/tools/go/loader: (darwin|windows)-amd64-longtest builder is failing at tip May 12, 2023
@findleyr
Copy link
Contributor Author

Oh, darwin is failing as well: https://build.golang.org/log/68018cc7868317a8acd303829ef65abfbcea625a

@bcmills
Copy link
Contributor

bcmills commented May 12, 2023

Oh, neat! cc_test.go is relying on cmd/dist to do something that ought to be done by build constraints and testenv guards instead. 🤦‍♂️

Because of the lack of build constraints, the test fails when we attempt to load it as a normal go package.

https://cs.opensource.google/go/go/+/master:src/cmd/dist/test.go;l=835-839;drc=c308e9b54c99d0d1fdc2bc317165aab6c36b1420

(attn @aclements; CC @dmitshur @ianlancetaylor)

@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label May 12, 2023
@bcmills bcmills changed the title x/tools/go/loader: (darwin|windows)-amd64-longtest builder is failing at tip cmd/cgo/internal: platform-specific test files are missing build constraints May 12, 2023
@bcmills bcmills added Soon This needs to be done soon. (regressions, serious bugs, outages) and removed Tools This label describes issues relating to any tools in the x/tools repository. labels May 12, 2023
@bcmills bcmills modified the milestones: Unreleased, Go1.21 May 12, 2023
@dmitshur dmitshur added the Testing An issue that has been verified to require only test changes, not just a test failure. label May 12, 2023
@aclements
Copy link
Member

Darn. I thought I'd checked that, but I guess not.

I have on my cleanup list moving as many conditions in dist as possible down into the tests, but I'll do a more targeted fix for this to unbreak things.

@gopherbot
Copy link

Change https://go.dev/cl/494658 mentions this issue: cmd/cgo/internal/testsanitizers: build on all platforms

@gopherbot
Copy link

Change https://go.dev/cl/494659 mentions this issue: cmd/cgo/internal/testcarchive: build on all platforms

gopherbot pushed a commit that referenced this issue May 12, 2023
This test package uses the Pdeathsig field of syscall.SysProcAttr,
which is only available on a few platforms. Currently, dist test
checks for compatible platforms and only registers it as part of
all.bash on platforms where it can build. But this doesn't help if
you're just trying to type check everything in cmd.

Make this package pass type checking by moving the condition from dist
into build tags on the test package itself.

For #60164.
Updates #37486.

Change-Id: I58b12d547c323cec895320baa5fca1b82e99d1b5
Reviewed-on: https://go-review.googlesource.com/c/go/+/494658
Auto-Submit: Austin Clements <austin@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done. release-blocker Soon This needs to be done soon. (regressions, serious bugs, outages) Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Development

No branches or pull requests

5 participants