Commit 3445ece2 authored by Russ Cox's avatar Russ Cox

cmd/go: be more precise when a directory cannot be built

Maybe there are no Go files at all.
Maybe they are all excluded by build constraints.
Maybe there are only test Go files.
Be specific.

Fixes #17008.
Fixes parts of #20760.

Change-Id: If6ac82ba0ed437772e76e06763263747d3bc4f65
Reviewed-on: https://go-review.googlesource.com/46427
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
parent f3e82e04
...@@ -1386,7 +1386,7 @@ func TestInstallFailsWithNoBuildableFiles(t *testing.T) { ...@@ -1386,7 +1386,7 @@ func TestInstallFailsWithNoBuildableFiles(t *testing.T) {
tg.setenv("GOPATH", filepath.Join(tg.pwd(), "testdata")) tg.setenv("GOPATH", filepath.Join(tg.pwd(), "testdata"))
tg.setenv("CGO_ENABLED", "0") tg.setenv("CGO_ENABLED", "0")
tg.runFail("install", "cgotest") tg.runFail("install", "cgotest")
tg.grepStderr("no buildable Go source files", "go install cgotest did not report 'no buildable Go Source files'") tg.grepStderr("build constraints exclude all Go files", "go install cgotest did not report 'build constraints exclude all Go files'")
} }
func TestRelativeGOBINFail(t *testing.T) { func TestRelativeGOBINFail(t *testing.T) {
...@@ -1514,11 +1514,11 @@ func TestGoGetNonPkg(t *testing.T) { ...@@ -1514,11 +1514,11 @@ func TestGoGetNonPkg(t *testing.T) {
tg.setenv("GOPATH", tg.path(".")) tg.setenv("GOPATH", tg.path("."))
tg.setenv("GOBIN", tg.path("gobin")) tg.setenv("GOBIN", tg.path("gobin"))
tg.runFail("get", "-d", "golang.org/x/tools") tg.runFail("get", "-d", "golang.org/x/tools")
tg.grepStderr("golang.org/x/tools: no buildable Go source files", "missing error") tg.grepStderr("golang.org/x/tools: no Go files", "missing error")
tg.runFail("get", "-d", "-u", "golang.org/x/tools") tg.runFail("get", "-d", "-u", "golang.org/x/tools")
tg.grepStderr("golang.org/x/tools: no buildable Go source files", "missing error") tg.grepStderr("golang.org/x/tools: no Go files", "missing error")
tg.runFail("get", "-d", "golang.org/x/tools") tg.runFail("get", "-d", "golang.org/x/tools")
tg.grepStderr("golang.org/x/tools: no buildable Go source files", "missing error") tg.grepStderr("golang.org/x/tools: no Go files", "missing error")
} }
func TestGoGetTestOnlyPkg(t *testing.T) { func TestGoGetTestOnlyPkg(t *testing.T) {
...@@ -2269,7 +2269,6 @@ func TestTestEmpty(t *testing.T) { ...@@ -2269,7 +2269,6 @@ func TestTestEmpty(t *testing.T) {
wd, _ := os.Getwd() wd, _ := os.Getwd()
testdata := filepath.Join(wd, "testdata") testdata := filepath.Join(wd, "testdata")
for _, dir := range []string{"pkg", "test", "xtest", "pkgtest", "pkgxtest", "pkgtestxtest", "testxtest"} { for _, dir := range []string{"pkg", "test", "xtest", "pkgtest", "pkgxtest", "pkgtestxtest", "testxtest"} {
t.Run(dir, func(t *testing.T) { t.Run(dir, func(t *testing.T) {
tg := testgo(t) tg := testgo(t)
...@@ -2284,6 +2283,29 @@ func TestTestEmpty(t *testing.T) { ...@@ -2284,6 +2283,29 @@ func TestTestEmpty(t *testing.T) {
} }
} }
func TestNoGoError(t *testing.T) {
wd, _ := os.Getwd()
testdata := filepath.Join(wd, "testdata")
for _, dir := range []string{"empty/test", "empty/xtest", "empty/testxtest", "exclude", "exclude/ignore", "exclude/empty"} {
t.Run(dir, func(t *testing.T) {
tg := testgo(t)
defer tg.cleanup()
tg.setenv("GOPATH", testdata)
tg.cd(filepath.Join(testdata, "src"))
tg.runFail("build", "./"+dir)
var want string
if strings.Contains(dir, "test") {
want = "no non-test Go files in "
} else if dir == "exclude" {
want = "build constraints exclude all Go files in "
} else {
want = "no Go files in "
}
tg.grepStderr(want, "wrong reason for failure")
})
}
}
func TestTestRaceInstall(t *testing.T) { func TestTestRaceInstall(t *testing.T) {
if !canRace { if !canRace {
t.Skip("no race detector") t.Skip("no race detector")
...@@ -2584,7 +2606,7 @@ func TestGoBuildInTestOnlyDirectoryFailsWithAGoodError(t *testing.T) { ...@@ -2584,7 +2606,7 @@ func TestGoBuildInTestOnlyDirectoryFailsWithAGoodError(t *testing.T) {
tg := testgo(t) tg := testgo(t)
defer tg.cleanup() defer tg.cleanup()
tg.runFail("build", "./testdata/testonly") tg.runFail("build", "./testdata/testonly")
tg.grepStderr("no buildable Go", "go build ./testdata/testonly produced unexpected error") tg.grepStderr("no non-test Go files in", "go build ./testdata/testonly produced unexpected error")
} }
func TestGoTestDetectsTestOnlyImportCycles(t *testing.T) { func TestGoTestDetectsTestOnlyImportCycles(t *testing.T) {
...@@ -3165,7 +3187,7 @@ func TestGoTestRaceFailures(t *testing.T) { ...@@ -3165,7 +3187,7 @@ func TestGoTestRaceFailures(t *testing.T) {
func TestGoTestImportErrorStack(t *testing.T) { func TestGoTestImportErrorStack(t *testing.T) {
const out = `package testdep/p1 (test) const out = `package testdep/p1 (test)
imports testdep/p2 imports testdep/p2
imports testdep/p3: no buildable Go source files` imports testdep/p3: build constraints exclude all Go files `
tg := testgo(t) tg := testgo(t)
defer tg.cleanup() defer tg.cleanup()
...@@ -3554,7 +3576,7 @@ func TestBinaryOnlyPackages(t *testing.T) { ...@@ -3554,7 +3576,7 @@ func TestBinaryOnlyPackages(t *testing.T) {
func F() { p1.F(true) } func F() { p1.F(true) }
`) `)
tg.runFail("install", "p2") tg.runFail("install", "p2")
tg.grepStderr("no buildable Go source files", "did not complain about missing sources") tg.grepStderr("no Go files", "did not complain about missing sources")
tg.tempFile("src/p1/missing.go", `//go:binary-only-package tg.tempFile("src/p1/missing.go", `//go:binary-only-package
......
...@@ -114,6 +114,32 @@ type PackageInternal struct { ...@@ -114,6 +114,32 @@ type PackageInternal struct {
GobinSubdir bool // install target would be subdir of GOBIN GobinSubdir bool // install target would be subdir of GOBIN
} }
type NoGoError struct {
Package *Package
}
func (e *NoGoError) Error() string {
// Count files beginning with _ and ., which we will pretend don't exist at all.
dummy := 0
for _, name := range e.Package.IgnoredGoFiles {
if strings.HasPrefix(name, "_") || strings.HasPrefix(name, ".") {
dummy++
}
}
if len(e.Package.IgnoredGoFiles) > dummy {
// Go files exist, but they were ignored due to build constraints.
return "build constraints exclude all Go files in " + e.Package.Dir
}
if len(e.Package.TestGoFiles)+len(e.Package.XTestGoFiles) > 0 {
// Test Go files exist, but we're not interested in them.
// The double-negative is unfortunate but we want e.Package.Dir
// to appear at the end of error message.
return "no non-test Go files in " + e.Package.Dir
}
return "no Go files in " + e.Package.Dir
}
// Vendored returns the vendor-resolved version of imports, // Vendored returns the vendor-resolved version of imports,
// which should be p.TestImports or p.XTestImports, NOT p.Imports. // which should be p.TestImports or p.XTestImports, NOT p.Imports.
// The imports in p.TestImports and p.XTestImports are not recursively // The imports in p.TestImports and p.XTestImports are not recursively
...@@ -840,6 +866,9 @@ func (p *Package) load(stk *ImportStack, bp *build.Package, err error) *Package ...@@ -840,6 +866,9 @@ func (p *Package) load(stk *ImportStack, bp *build.Package, err error) *Package
p.Internal.LocalPrefix = dirToImportPath(p.Dir) p.Internal.LocalPrefix = dirToImportPath(p.Dir)
if err != nil { if err != nil {
if _, ok := err.(*build.NoGoError); ok {
err = &NoGoError{Package: p}
}
p.Incomplete = true p.Incomplete = true
err = base.ExpandScanner(err) err = base.ExpandScanner(err)
p.Error = &PackageError{ p.Error = &PackageError{
......
...@@ -576,8 +576,6 @@ func InstallPackages(args []string, forGet bool) { ...@@ -576,8 +576,6 @@ func InstallPackages(args []string, forGet bool) {
var b Builder var b Builder
b.Init() b.Init()
// Set the behavior for `go get` to not error on packages with test files only.
b.testFilesOnlyOK = forGet
var a *Action var a *Action
if cfg.BuildBuildmode == "shared" { if cfg.BuildBuildmode == "shared" {
if libName, err := libname(args, pkgs); err != nil { if libName, err := libname(args, pkgs); err != nil {
...@@ -589,6 +587,11 @@ func InstallPackages(args []string, forGet bool) { ...@@ -589,6 +587,11 @@ func InstallPackages(args []string, forGet bool) {
a = &Action{} a = &Action{}
var tools []*Action var tools []*Action
for _, p := range pkgs { for _, p := range pkgs {
// During 'go get', don't attempt (and fail) to install packages with only tests.
// TODO(rsc): It's not clear why 'go get' should be different from 'go install' here. See #20760.
if forGet && len(p.GoFiles)+len(p.CgoFiles) == 0 && len(p.TestGoFiles)+len(p.XTestGoFiles) > 0 {
continue
}
// If p is a tool, delay the installation until the end of the build. // If p is a tool, delay the installation until the end of the build.
// This avoids installing assemblers/compilers that are being executed // This avoids installing assemblers/compilers that are being executed
// by other steps in the build. // by other steps in the build.
...@@ -660,8 +663,6 @@ type Builder struct { ...@@ -660,8 +663,6 @@ type Builder struct {
flagCache map[string]bool // a cache of supported compiler flags flagCache map[string]bool // a cache of supported compiler flags
Print func(args ...interface{}) (int, error) Print func(args ...interface{}) (int, error)
testFilesOnlyOK bool // do not error if the packages only have test files
output sync.Mutex output sync.Mutex
scriptDir string // current directory in printed script scriptDir string // current directory in printed script
...@@ -1165,8 +1166,6 @@ func (b *Builder) Do(root *Action) { ...@@ -1165,8 +1166,6 @@ func (b *Builder) Do(root *Action) {
if err != nil { if err != nil {
if err == errPrintedOutput { if err == errPrintedOutput {
base.SetExitStatus(2) base.SetExitStatus(2)
} else if _, ok := err.(*build.NoGoError); ok && len(a.Package.TestGoFiles) > 0 && b.testFilesOnlyOK {
// Ignore the "no buildable Go source files" error for a package with only test files.
} else { } else {
base.Errorf("%s", err) base.Errorf("%s", err)
} }
...@@ -1253,7 +1252,7 @@ func (b *Builder) build(a *Action) (err error) { ...@@ -1253,7 +1252,7 @@ func (b *Builder) build(a *Action) (err error) {
} }
defer func() { defer func() {
if _, ok := err.(*build.NoGoError); err != nil && err != errPrintedOutput && !(ok && b.testFilesOnlyOK && len(a.Package.TestGoFiles) > 0) { if err != nil && err != errPrintedOutput {
err = fmt.Errorf("go build %s: %v", a.Package.ImportPath, err) err = fmt.Errorf("go build %s: %v", a.Package.ImportPath, err)
} }
}() }()
...@@ -1365,7 +1364,7 @@ func (b *Builder) build(a *Action) (err error) { ...@@ -1365,7 +1364,7 @@ func (b *Builder) build(a *Action) (err error) {
} }
if len(gofiles) == 0 { if len(gofiles) == 0 {
return &build.NoGoError{Dir: a.Package.Dir} return &load.NoGoError{Package: a.Package}
} }
// If we're doing coverage, preprocess the .go files and put them in the work directory // If we're doing coverage, preprocess the .go files and put them in the work directory
......
// +build linux,!linux
package x
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment