]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go: revamp and simplify logic in PrepareForCoverageBuild
authorThan McIntosh <thanm@google.com>
Wed, 26 Oct 2022 20:56:48 +0000 (16:56 -0400)
committerThan McIntosh <thanm@google.com>
Tue, 1 Nov 2022 14:12:26 +0000 (14:12 +0000)
Change the 'PrepareForCoverageBuild' helper function to provide more
sensible defaults in the case where Go packages are listed on the
command line (e.g. cases such as "go run -cover mumble.go").

With the old implementation, if module mode was enabled, we would only
instrument packages in the main module, meaning that if you did
something like this:

  $ ls go.mod
  go.mod
  $ GOCOVERDATA=/tmp/cov go run -cover testdata/somefile.go
  $

no coverage profiles would be generated at all. This is due to the
fact that the pseudo-package created by the Go command internally when
building "somefile.go" is not considered part of the main module.

This patch moves the default to "packages explicitly mentioned on the
command line, plus packages in the main module", which will make more
sense to users passing specific packages and *.go files on the command
line. Examples:

  // Here cmd/compile is part the Go standard library + commands
  // (which we exclude from instrumentation by default), but since
  // 'cmd/compile' is mentioned on the command line, we will instrument
  // just that single package (not any of its deps).
  $ cd $GOROOT/src
  $ go build -o gc.exe -cover cmd/compile
  $ GOCOVERDATA=/tmp/cov ./gc.exe ...
  ...
  $

  // Here we're running a Go file named on the command line, where
  // the pseudo-package for the command line is not part of the
  // main module, but it makes sense to instrument it anyhow.
  $ cd ~/go/k8s.io/kubernetes
  $ GOCOVERDATA=/tmp/cov go run -cover test/typecheck/testdata/bad/bad.go
  ...
  $

This patch also simplifies the logic and improves flow/comments in
in the helper function PrepareForCoverageBuild.

Change-Id: Id8fc8571157dac8c09e44cc73baa05aeba1640ca
Reviewed-on: https://go-review.googlesource.com/c/go/+/445918
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>

src/cmd/go/internal/load/pkg.go
src/cmd/go/testdata/script/cover_build_cmdline_pkgs.txt [new file with mode: 0644]

index 4a6414016ade4068593e20a26fc017b73556f0de..0e1a632d7a7a2557237eae43e20d0aa2de6085a1 100644 (file)
@@ -3240,52 +3240,36 @@ func EnsureImport(p *Package, pkg string) {
        p.Internal.Imports = append(p.Internal.Imports, p1)
 }
 
-// PrepareForCoverageBuild is a helper invoked for "go install -cover"
-// and "go build -cover"; it walks through the packages being built
-// (and dependencies) and marks them for coverage instrumentation
-// when appropriate, and adding dependencies where needed.
+// PrepareForCoverageBuild is a helper invoked for "go install
+// -cover", "go run -cover", and "go build -cover" (but not used by
+// "go test -cover"). It walks through the packages being built (and
+// dependencies) and marks them for coverage instrumentation when
+// appropriate, and possibly adding additional deps where needed.
 func PrepareForCoverageBuild(pkgs []*Package) {
        var match []func(*Package) bool
 
-       matchMainMod := func(p *Package) bool {
-               return !p.Standard && p.Module != nil && p.Module.Main
-       }
-
-       // The set of packages instrumented by default varies depending on
-       // options and the nature of the build. If "-coverpkg" has been
-       // set, then match packages below using that value; if we're
-       // building with a module in effect, then default to packages in
-       // the main module. If no module is in effect and we're building
-       // in GOPATH mode, instrument the named packages and their
-       // dependencies in GOPATH. Otherwise, for "go run ..." and for the
-       // "go build ..." case, instrument just the packages named on the
-       // command line.
-       if len(cfg.BuildCoverPkg) == 0 {
-               if modload.Enabled() {
-                       // Default is main module.
-                       match = []func(*Package) bool{matchMainMod}
-               } else {
-                       // These matchers below are intended to handle the cases of:
-                       //
-                       // 1. "go run ..." and "go build ..."
-                       // 2. building in gopath mode with GO111MODULE=off
-                       //
-                       // In case 2 above, the assumption here is that (in the
-                       // absence of a -coverpkg flag) we will be instrumenting
-                       // the named packages only.
-                       matchMain := func(p *Package) bool { return p.Internal.CmdlineFiles || p.Internal.CmdlinePkg }
-                       match = []func(*Package) bool{matchMain}
-               }
-       } else {
+       matchMainModAndCommandLine := func(p *Package) bool {
+               // note that p.Standard implies p.Module == nil below.
+               return p.Internal.CmdlineFiles || p.Internal.CmdlinePkg || (p.Module != nil && p.Module.Main)
+       }
+
+       if len(cfg.BuildCoverPkg) != 0 {
+               // If -coverpkg has been specified, then we instrument only
+               // the specific packages selected by the user-specified pattern(s).
                match = make([]func(*Package) bool, len(cfg.BuildCoverPkg))
                for i := range cfg.BuildCoverPkg {
                        match[i] = MatchPackage(cfg.BuildCoverPkg[i], base.Cwd())
                }
+       } else {
+               // Without -coverpkg, instrument only packages in the main module
+               // (if any), as well as packages/files specifically named on the
+               // command line.
+               match = []func(*Package) bool{matchMainModAndCommandLine}
        }
 
-       // Visit the packages being built or installed, along with all
-       // of their dependencies, and mark them to be instrumented,
-       // taking into account the value of -coverpkg.
+       // Visit the packages being built or installed, along with all of
+       // their dependencies, and mark them to be instrumented, taking
+       // into account the matchers we've set up in the sequence above.
        SelectCoverPackages(PackageList(pkgs), match, "build")
 }
 
diff --git a/src/cmd/go/testdata/script/cover_build_cmdline_pkgs.txt b/src/cmd/go/testdata/script/cover_build_cmdline_pkgs.txt
new file mode 100644 (file)
index 0000000..4748a85
--- /dev/null
@@ -0,0 +1,72 @@
+
+# This test is intended to verify that when a user does "go run -cover ..."
+# or "go build -cover ...", packages named on the command line are
+# always instrumented (but not their dependencies). This rule applies
+# inside and outside the standard library.
+
+[short] skip
+
+# Compile an object.
+go tool compile -p tiny tiny/tiny.go tiny/tiny2.go
+
+# Build a stdlib command with coverage.
+go build -o $WORK/nm.exe -cover cmd/nm 
+
+# Save off old GOCOVERDIR setting
+env SAVEGOCOVERDIR=$GOCOVERDIR
+
+# Collect a coverage profile from running 'cmd/nm' on the object.
+mkdir $WORK/covdata
+env GOCOVERDIR=$WORK/covdata
+exec $WORK/nm.exe tiny.o
+
+# Restore previous GOCOVERDIR setting
+env GOCOVERDIR=$SAVEGOCOVERDIR
+
+# Check to make sure we instrumented just the main package, not
+# any dependencies.
+go tool covdata pkglist -i=$WORK/covdata
+stdout main
+! stdout cmd/internal/goobj pkglist.txt
+
+# ... now collect a coverage profile from a Go file
+# listed on the command line.
+go build -cover -o $WORK/another.exe testdata/another.go
+mkdir $WORK/covdata2
+env GOCOVERDIR=$WORK/covdata2
+exec $WORK/another.exe 
+
+# Restore previous GOCOVERDIR setting
+env GOCOVERDIR=$SAVEGOCOVERDIR
+
+# Check to make sure we instrumented just the main package.
+go tool covdata pkglist -i=$WORK/covdata2
+stdout main
+! stdout fmt
+
+-- go.mod --
+
+module example.prog
+
+-- testdata/another.go --
+
+package main
+
+import "fmt"
+
+func main() {
+  fmt.Println("Hi dad")
+}
+
+-- tiny/tiny.go --
+
+package tiny
+
+var Tvar int
+
+-- tiny/tiny2.go --
+
+package tiny
+
+var Tvar2 bool
+