From 99dc2a1859f15fafc5950ad7ef6026dfbde826c6 Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Wed, 26 Oct 2022 16:56:48 -0400 Subject: [PATCH] cmd/go: revamp and simplify logic in PrepareForCoverageBuild 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 Reviewed-by: Bryan Mills Reviewed-by: Cherry Mui Run-TryBot: Than McIntosh --- src/cmd/go/internal/load/pkg.go | 58 ++++++--------- .../script/cover_build_cmdline_pkgs.txt | 72 +++++++++++++++++++ 2 files changed, 93 insertions(+), 37 deletions(-) create mode 100644 src/cmd/go/testdata/script/cover_build_cmdline_pkgs.txt diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go index 4a6414016a..0e1a632d7a 100644 --- a/src/cmd/go/internal/load/pkg.go +++ b/src/cmd/go/internal/load/pkg.go @@ -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 index 0000000000..4748a85f5e --- /dev/null +++ b/src/cmd/go/testdata/script/cover_build_cmdline_pkgs.txt @@ -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 + -- 2.50.0