]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go: be more precise when a directory cannot be built
authorRuss Cox <rsc@golang.org>
Thu, 22 Jun 2017 23:02:35 +0000 (19:02 -0400)
committerRuss Cox <rsc@golang.org>
Fri, 23 Jun 2017 15:02:32 +0000 (15:02 +0000)
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: Ian Lance Taylor <iant@golang.org>
src/cmd/go/go_test.go
src/cmd/go/internal/load/pkg.go
src/cmd/go/internal/work/build.go
src/cmd/go/testdata/src/exclude/empty/x.txt [new file with mode: 0644]
src/cmd/go/testdata/src/exclude/ignore/_x.go [new file with mode: 0644]
src/cmd/go/testdata/src/exclude/x.go [new file with mode: 0644]
src/cmd/go/testdata/src/exclude/x_linux.go [new file with mode: 0644]

index 50760b966c7566dd3895784ecfa9f09559ec2a06..239b9c37a47f0d53aed5d9bc931193192ee65bee 100644 (file)
@@ -1386,7 +1386,7 @@ func TestInstallFailsWithNoBuildableFiles(t *testing.T) {
        tg.setenv("GOPATH", filepath.Join(tg.pwd(), "testdata"))
        tg.setenv("CGO_ENABLED", "0")
        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) {
@@ -1514,11 +1514,11 @@ func TestGoGetNonPkg(t *testing.T) {
        tg.setenv("GOPATH", tg.path("."))
        tg.setenv("GOBIN", tg.path("gobin"))
        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.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.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) {
@@ -2269,7 +2269,6 @@ func TestTestEmpty(t *testing.T) {
 
        wd, _ := os.Getwd()
        testdata := filepath.Join(wd, "testdata")
-
        for _, dir := range []string{"pkg", "test", "xtest", "pkgtest", "pkgxtest", "pkgtestxtest", "testxtest"} {
                t.Run(dir, func(t *testing.T) {
                        tg := testgo(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) {
        if !canRace {
                t.Skip("no race detector")
@@ -2584,7 +2606,7 @@ func TestGoBuildInTestOnlyDirectoryFailsWithAGoodError(t *testing.T) {
        tg := testgo(t)
        defer tg.cleanup()
        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) {
@@ -3165,7 +3187,7 @@ func TestGoTestRaceFailures(t *testing.T) {
 func TestGoTestImportErrorStack(t *testing.T) {
        const out = `package testdep/p1 (test)
        imports testdep/p2
-       imports testdep/p3: no buildable Go source files`
+       imports testdep/p3: build constraints exclude all Go files `
 
        tg := testgo(t)
        defer tg.cleanup()
@@ -3554,7 +3576,7 @@ func TestBinaryOnlyPackages(t *testing.T) {
                func F() { p1.F(true) }
        `)
        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
 
index a8a61f0635af40c60fc6ae2d68157e9fcf55b94e..60de6661647be4ea8be09e696d380889e8d3174e 100644 (file)
@@ -114,6 +114,32 @@ type PackageInternal struct {
        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,
 // which should be p.TestImports or p.XTestImports, NOT p.Imports.
 // 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
        p.Internal.LocalPrefix = dirToImportPath(p.Dir)
 
        if err != nil {
+               if _, ok := err.(*build.NoGoError); ok {
+                       err = &NoGoError{Package: p}
+               }
                p.Incomplete = true
                err = base.ExpandScanner(err)
                p.Error = &PackageError{
index d03ad3e139bf0a895bfecbbeee7c882de979df27..3a64af35b1298ddb59f3c22e53d299195fcb471e 100644 (file)
@@ -576,8 +576,6 @@ func InstallPackages(args []string, forGet bool) {
 
        var b Builder
        b.Init()
-       // Set the behavior for `go get` to not error on packages with test files only.
-       b.testFilesOnlyOK = forGet
        var a *Action
        if cfg.BuildBuildmode == "shared" {
                if libName, err := libname(args, pkgs); err != nil {
@@ -589,6 +587,11 @@ func InstallPackages(args []string, forGet bool) {
                a = &Action{}
                var tools []*Action
                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.
                        // This avoids installing assemblers/compilers that are being executed
                        // by other steps in the build.
@@ -660,8 +663,6 @@ type Builder struct {
        flagCache   map[string]bool      // a cache of supported compiler flags
        Print       func(args ...interface{}) (int, error)
 
-       testFilesOnlyOK bool // do not error if the packages only have test files
-
        output    sync.Mutex
        scriptDir string // current directory in printed script
 
@@ -1165,8 +1166,6 @@ func (b *Builder) Do(root *Action) {
                if err != nil {
                        if err == errPrintedOutput {
                                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 {
                                base.Errorf("%s", err)
                        }
@@ -1253,7 +1252,7 @@ func (b *Builder) build(a *Action) (err error) {
        }
 
        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)
                }
        }()
@@ -1365,7 +1364,7 @@ func (b *Builder) build(a *Action) (err error) {
        }
 
        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
diff --git a/src/cmd/go/testdata/src/exclude/empty/x.txt b/src/cmd/go/testdata/src/exclude/empty/x.txt
new file mode 100644 (file)
index 0000000..e69de29
diff --git a/src/cmd/go/testdata/src/exclude/ignore/_x.go b/src/cmd/go/testdata/src/exclude/ignore/_x.go
new file mode 100644 (file)
index 0000000..823aafd
--- /dev/null
@@ -0,0 +1 @@
+package x
diff --git a/src/cmd/go/testdata/src/exclude/x.go b/src/cmd/go/testdata/src/exclude/x.go
new file mode 100644 (file)
index 0000000..9affd21
--- /dev/null
@@ -0,0 +1,3 @@
+// +build linux,!linux
+
+package x
diff --git a/src/cmd/go/testdata/src/exclude/x_linux.go b/src/cmd/go/testdata/src/exclude/x_linux.go
new file mode 100644 (file)
index 0000000..a5bbb61
--- /dev/null
@@ -0,0 +1,4 @@
+// +build windows
+
+package x
+