cmd/go: ignore wildcard-matched test-only packages in go build
It makes sense to be able to cd into the top of a directory tree
and run commands like:
go list ./...
go test ./...
go build ./...
go install ./...
Some packages have no tests. Some packages have only tests (no code).
While go list applies to both kinds of packages, and go test correctly
handles packages without tests, go build and go install reject packages
with only tests.
If you run
go install testonlypkg
then it does seem reasonable to fail with an error, since testonlypkg
cannot actually be installed. But if testonlypkg was only picked up
by wildcard expansion, then to keep the examples above working,
we should just silently skip over it during go build and go install.
CL 23314 fixed this same problem for "go get golang.org/x/tour/..."
but special-cased "go get", leaving "go build" and "go install" failing.
This CL fixes the general case and removes the "go get" special case.
Picked this up in module work because I'm using "go build ./..." as part
of evaluating whether repos are compatible with Go modules.
Fixes golang/go#20760.
Fixes golang/go#22409.
Change-Id: I5caaa6a346a5d07a7ad9e2669e89da76b397dc42
Reviewed-on: https://go-review.googlesource.com/121196
Reviewed-by: Bryan C. Mills <bcmills@google.com>
diff --git a/vendor/cmd/go/go_test.go b/vendor/cmd/go/go_test.go
index 4dadd72..ecd37e3 100644
--- a/vendor/cmd/go/go_test.go
+++ b/vendor/cmd/go/go_test.go
@@ -1779,8 +1779,8 @@
defer tg.cleanup()
tg.tempDir("gopath")
tg.setenv("GOPATH", tg.path("gopath"))
- tg.run("get", "golang.org/x/tour/content")
- tg.run("get", "-t", "golang.org/x/tour/content")
+ tg.run("get", "golang.org/x/tour/content...")
+ tg.run("get", "-t", "golang.org/x/tour/content...")
}
func TestInstalls(t *testing.T) {
@@ -3204,11 +3204,25 @@
checkbar("cmd")
}
-func TestGoBuildInTestOnlyDirectoryFailsWithAGoodError(t *testing.T) {
+func TestGoBuildTestOnly(t *testing.T) {
tg := testgo(t)
defer tg.cleanup()
- tg.runFail("build", "./testdata/testonly")
- tg.grepStderr("no non-test Go files in", "go build ./testdata/testonly produced unexpected error")
+ tg.makeTempdir()
+ tg.setenv("GOPATH", tg.path("."))
+ tg.tempFile("src/testonly/t_test.go", `package testonly`)
+ tg.tempFile("src/testonly2/t.go", `package testonly2`)
+ tg.cd(tg.path("src"))
+
+ // Named explicitly, test-only packages should be reported as unbuildable/uninstallable,
+ // even if there is a wildcard also matching.
+ tg.runFail("build", "testonly", "testonly...")
+ tg.grepStderr("no non-test Go files in", "go build ./xtestonly produced unexpected error")
+ tg.runFail("install", "./testonly")
+ tg.grepStderr("no non-test Go files in", "go install ./testonly produced unexpected error")
+
+ // Named through a wildcards, the test-only packages should be silently ignored.
+ tg.run("build", "testonly...")
+ tg.run("install", "./testonly...")
}
func TestGoTestDetectsTestOnlyImportCycles(t *testing.T) {
@@ -5164,6 +5178,8 @@
}
func TestExecBuildX(t *testing.T) {
+ t.Skip("vgo")
+
tooSlow(t)
if !canCgo {
t.Skip("skipping because cgo not enabled")
diff --git a/vendor/cmd/go/internal/get/get.go b/vendor/cmd/go/internal/get/get.go
index eeae4b1..2348693 100644
--- a/vendor/cmd/go/internal/get/get.go
+++ b/vendor/cmd/go/internal/get/get.go
@@ -167,7 +167,7 @@
return
}
- work.InstallPackages(args, true)
+ work.InstallPackages(args)
}
// downloadPaths prepares the list of paths to pass to download.
diff --git a/vendor/cmd/go/internal/load/flag.go b/vendor/cmd/go/internal/load/flag.go
index 7ad4208..d9177b0 100644
--- a/vendor/cmd/go/internal/load/flag.go
+++ b/vendor/cmd/go/internal/load/flag.go
@@ -6,6 +6,7 @@
import (
"cmd/go/internal/base"
+ "cmd/go/internal/search"
"cmd/go/internal/str"
"fmt"
"strings"
@@ -92,7 +93,10 @@
return flags
}
-var cmdlineMatchers []func(*Package) bool
+var (
+ cmdlineMatchers []func(*Package) bool
+ cmdlineMatcherLiterals []func(*Package) bool
+)
// SetCmdlinePatterns records the set of patterns given on the command line,
// for use by the PerPackageFlags.
@@ -105,9 +109,15 @@
args = []string{"."}
}
cmdlineMatchers = nil // allow reset for testing
+ cmdlineMatcherLiterals = nil
for _, arg := range args {
cmdlineMatchers = append(cmdlineMatchers, MatchPackage(arg, cwd))
}
+ for _, arg := range args {
+ if !strings.Contains(arg, "...") && !search.IsMetaPackage(arg) {
+ cmdlineMatcherLiterals = append(cmdlineMatcherLiterals, MatchPackage(arg, cwd))
+ }
+ }
}
// isCmdlinePkg reports whether p is a package listed on the command line.
@@ -119,3 +129,15 @@
}
return false
}
+
+// isCmdlinePkgLiteral reports whether p is a package listed as
+// a literal package argument on the command line
+// (as opposed to being the result of expanding a wildcard).
+func isCmdlinePkgLiteral(p *Package) bool {
+ for _, m := range cmdlineMatcherLiterals {
+ if m(p) {
+ return true
+ }
+ }
+ return false
+}
diff --git a/vendor/cmd/go/internal/load/pkg.go b/vendor/cmd/go/internal/load/pkg.go
index 951c7f4..41ebf89 100644
--- a/vendor/cmd/go/internal/load/pkg.go
+++ b/vendor/cmd/go/internal/load/pkg.go
@@ -149,21 +149,22 @@
type PackageInternal struct {
// Unexported fields are not part of the public API.
- Build *build.Package
- Imports []*Package // this package's direct imports
- RawImports []string // this package's original imports as they appear in the text of the program
- ForceLibrary bool // this package is a library (even if named "main")
- CmdlineFiles bool // package built from files listed on command line
- CmdlinePkg bool // package listed on command line
- Local bool // imported via local path (./ or ../)
- LocalPrefix string // interpret ./ and ../ imports relative to this prefix
- ExeName string // desired name for temporary executable
- CoverMode string // preprocess Go source files with the coverage tool in this mode
- CoverVars map[string]*CoverVar // variables created by coverage analysis
- OmitDebug bool // tell linker not to write debug information
- GobinSubdir bool // install target would be subdir of GOBIN
- BuildInfo string // add this info to package main
- TestmainGo *[]byte // content for _testmain.go
+ Build *build.Package
+ Imports []*Package // this package's direct imports
+ RawImports []string // this package's original imports as they appear in the text of the program
+ ForceLibrary bool // this package is a library (even if named "main")
+ CmdlineFiles bool // package built from files listed on command line
+ CmdlinePkg bool // package listed on command line
+ CmdlinePkgLiteral bool // package listed as literal on command line (not via wildcard)
+ Local bool // imported via local path (./ or ../)
+ LocalPrefix string // interpret ./ and ../ imports relative to this prefix
+ ExeName string // desired name for temporary executable
+ CoverMode string // preprocess Go source files with the coverage tool in this mode
+ CoverVars map[string]*CoverVar // variables created by coverage analysis
+ OmitDebug bool // tell linker not to write debug information
+ GobinSubdir bool // install target would be subdir of GOBIN
+ BuildInfo string // add this info to package main
+ TestmainGo *[]byte // content for _testmain.go
Asmflags []string // -asmflags for this package
Gcflags []string // -gcflags for this package
@@ -1116,8 +1117,9 @@
// of fmt before it attempts to load as a command-line argument.
// Because loads are cached, the later load will be a no-op,
// so it is important that the first load can fill in CmdlinePkg correctly.
- // Hence the call to an explicit matching check here.
+ // Hence the call to a separate matching check here.
p.Internal.CmdlinePkg = isCmdlinePkg(p)
+ p.Internal.CmdlinePkgLiteral = isCmdlinePkgLiteral(p)
p.Internal.Asmflags = BuildAsmflags.For(p)
p.Internal.Gcflags = BuildGcflags.For(p)
diff --git a/vendor/cmd/go/internal/vgo/get.go b/vendor/cmd/go/internal/vgo/get.go
index f19364f..45bf1ce 100644
--- a/vendor/cmd/go/internal/vgo/get.go
+++ b/vendor/cmd/go/internal/vgo/get.go
@@ -176,7 +176,7 @@
}
}
if len(list) > 0 {
- work.InstallPackages(list, false)
+ work.InstallPackages(list)
}
}
}
diff --git a/vendor/cmd/go/internal/work/build.go b/vendor/cmd/go/internal/work/build.go
index c637d76..cc4572b 100644
--- a/vendor/cmd/go/internal/work/build.go
+++ b/vendor/cmd/go/internal/work/build.go
@@ -328,7 +328,7 @@
depMode = ModeInstall
}
- pkgs = pkgsFilter(load.Packages(args))
+ pkgs = omitTestOnly(pkgsFilter(load.Packages(args)))
// Special case -o /dev/null by not writing at all.
if cfg.BuildO == os.DevNull {
@@ -433,15 +433,32 @@
func runInstall(cmd *base.Command, args []string) {
BuildInit()
- InstallPackages(args, false)
+ InstallPackages(args)
}
-func InstallPackages(args []string, forGet bool) {
+// omitTestOnly returns pkgs with test-only packages removed.
+func omitTestOnly(pkgs []*load.Package) []*load.Package {
+ var list []*load.Package
+ for _, p := range pkgs {
+ if len(p.GoFiles)+len(p.CgoFiles) == 0 && !p.Internal.CmdlinePkgLiteral {
+ // Package has no source files,
+ // perhaps due to build tags or perhaps due to only having *_test.go files.
+ // Also, it is only being processed as the result of a wildcard match
+ // like ./..., not because it was listed as a literal path on the command line.
+ // Ignore it.
+ continue
+ }
+ list = append(list, p)
+ }
+ return list
+}
+
+func InstallPackages(args []string) {
if cfg.GOBIN != "" && !filepath.IsAbs(cfg.GOBIN) {
base.Fatalf("cannot install, GOBIN must be an absolute path")
}
- pkgs := pkgsFilter(load.PackagesForBuild(args))
+ pkgs := omitTestOnly(pkgsFilter(load.PackagesForBuild(args)))
for _, p := range pkgs {
if p.Target == "" && (!p.Standard || p.ImportPath != "unsafe") {
@@ -469,11 +486,6 @@
a := &Action{Mode: "go install"}
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.
diff --git a/vendor/cmd/go/testdata/testonly2/t.go b/vendor/cmd/go/testdata/testonly2/t.go
new file mode 100644
index 0000000..82267d3
--- /dev/null
+++ b/vendor/cmd/go/testdata/testonly2/t.go
@@ -0,0 +1,6 @@
+// This package is not a test-only package,
+// but it still matches the pattern ./testdata/testonly... when in cmd/go.
+
+package main
+
+func main() {}
