cmd/go: coverpkg doesn't ignore directories starting with '.'

[matthewhughes934]

Go version

go version go1.22.0 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/mjh/.cache/go-build'
GOENV='/home/mjh/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/mjh/.local/share/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/mjh/.local/share/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/mjh/sdk/go1.22.0'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/mjh/sdk/go1.22.0/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.22.0'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/mjh/src/personal/go-cov/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build3982892531=/tmp/go-build -gno-record-gcc-switches'

What did you do?

Ran Go tests with -coverpkg=./... in a directory which included GOMODCACHE/GOCACHE in a subdirectory starting with "." (the use case is for caching in GitLab, which requires directories stored in caches exist in the working directory, see their Go example https://docs.gitlab.com/ee/ci/caching/#cache-go-dependencies)

Here's a bash script to reproduce:

#!/usr/bin/env bash

set -o errexit -o pipefail -o nounset

mkdir proj
cd proj
go mod init example-proj

# create a trivial lib with 100% test coverage
cat <<EOF > lib.go
package lib

import (
	// add an unused dependency, just so something is stored in the cache directory
	_ "golang.org/x/tools/cover"
)

func Sum(a int, b int) int { 
	return a + b
}
EOF

cat <<EOF > lib_test.go
package lib

import (
	"testing"
)

func TestSum(t *testing.T) {
	if Sum(1, 2) != 3 {
		t.Error("fail")
	}
}
EOF

# .go contains directories we wish to cache
mkdir .go
export GOMODCACHE="$PWD/.go/mod_cache" 
export GOCACHE="$PWD/.go/cache"

go mod tidy
go test -coverprofile=coverage.out -coverpkg=./... ./...

What did you see happen?

The modules under .go were included in test coverage, output of the above script (note the coverage % in the last line):

go: creating new go.mod: module example-proj
go: finding module for package golang.org/x/tools/cover
go: downloading golang.org/x/tools v0.18.0
go: found golang.org/x/tools/cover in golang.org/x/tools v0.18.0
ok  	example-proj	0.002s	coverage: 0.8% of statements in ./...

inspecting the coverage profile:

$ grep --invert-match "$(go list -m)" coverage.out | head
mode: set
golang.org/x/tools/cover/profile.go:37.41,37.58 1 0
golang.org/x/tools/cover/profile.go:38.41,38.81 1 0
golang.org/x/tools/cover/profile.go:39.41,39.68 1 0
golang.org/x/tools/cover/profile.go:43.57,45.16 2 0
golang.org/x/tools/cover/profile.go:45.16,47.3 1 0
golang.org/x/tools/cover/profile.go:48.2,49.36 2 0
golang.org/x/tools/cover/profile.go:54.64,62.15 4 0
golang.org/x/tools/cover/profile.go:62.15,64.17 2 0
golang.org/x/tools/cover/profile.go:64.17,66.48 2 0

What did you expect to see?

Modules under the .go directory to be ignored, per https://pkg.go.dev/cmd/go#hdr-Package_lists_and_patterns

Directory and file names that begin with "." or "_" are ignored by the go tool, as are directories named "testdata".

EDIT: for anyone looking for a workaround, I just filtered out lines not belonging to my module in the coverage file:

awk -v go_mod="$(go list -m)" 'NR==1 || $0 ~ "^"go_mod' "coverage.in" > coverage.out

[matthewhughes934]

Observation, the behaviour changes if I vendor the dependencies:

$ GOMODCACHE="$PWD/.go/mod_cache" GOCACHE="$PWD/.go/cache" go mod vendor
$ GOMODCACHE="$PWD/.go/mod_cache" GOCACHE="$PWD/.go/cache" go test -coverprofile=coverage.out -coverpkg=./... ./...
ok  	example-proj	0.002s	coverage: 100.0% of statements in ./...

Digging further, here's the first quick-and-dirty (not acceptable, it breaks a test in cmd/go/internal/load) change I found to give the behaviour I expected:

diff --git a/src/cmd/go/internal/load/search.go b/src/cmd/go/internal/load/search.go
index 565996a21f..da533661c6 100644
--- a/src/cmd/go/internal/load/search.go
+++ b/src/cmd/go/internal/load/search.go
@@ -39,7 +39,7 @@ func MatchPackage(pattern, cwd string) func(*Package) bool {
                                return false
                        }
                        rel = filepath.ToSlash(rel)
-                       if rel == ".." || strings.HasPrefix(rel, "../") {
+                       if rel == ".." || strings.HasPrefix(rel, "../") || rel[0] == '.' || rel[0] == '_' {
                                return false
                        }
                        return matchPath(rel)

In the case of vendoring the rel path there is like "vendor/github.com/russross/blackfriday/v2" and matchPath will return false. In the other case the path is e.g. .go/mod_cache/github.com/russross/blackfriday/v2@v2.1.0"

[bcmills]

(attn @thanm)

[matthewhughes934]

Here's an updated patch which doesn't break existing tests, and includes a new PerPackageFlag (just because that was the first place I found that was directly checking the behaviour of the package matching). Happy to submit for review if it is at all reasonable.

diff --git a/src/cmd/go/internal/load/flag_test.go b/src/cmd/go/internal/load/flag_test.go
index d3223e12d5..f5b38f5323 100644
--- a/src/cmd/go/internal/load/flag_test.go
+++ b/src/cmd/go/internal/load/flag_test.go
@@ -92,6 +92,7 @@ type ppfTest struct {
 	ppfDirTest("./...", 3, "/my/test/dir", "/my/test/dir/sub", "/my/test/dir/sub/sub", "/my/test/other", "/my/test/other/sub"),
 	ppfDirTest("../...", 4, "/my/test/dir", "/my/test/other", "/my/test/dir/sub", "/my/test/other/sub", "/my/other/test"),
 	ppfDirTest("../...sub...", 3, "/my/test/dir/sub", "/my/test/othersub", "/my/test/yellowsubmarine", "/my/other/test"),
+	ppfDirTest("./...", 1, "/my/test/dir", "/my/test/dir/.sub", "/test/dir/_sub", "/test/dir/testdata/sub"),
 }
 
 func ppfDirTest(pattern string, nmatch int, dirs ...string) ppfTest {
diff --git a/src/cmd/go/internal/load/search.go b/src/cmd/go/internal/load/search.go
index 565996a21f..3af07a9475 100644
--- a/src/cmd/go/internal/load/search.go
+++ b/src/cmd/go/internal/load/search.go
@@ -38,10 +38,20 @@ func MatchPackage(pattern, cwd string) func(*Package) bool {
 				// Cannot make relative - e.g. different drive letters on Windows.
 				return false
 			}
+
 			rel = filepath.ToSlash(rel)
 			if rel == ".." || strings.HasPrefix(rel, "../") {
 				return false
 			}
+
+			cwdRel, err := filepath.Rel(cwd, p.Dir)
+			if (
+				err == nil && cwdRel != "." && !strings.HasPrefix(cwdRel, "../") &&
+				// Avoid .foo, _foo, and testdata subdirectory trees.
+				(strings.HasPrefix(cwdRel, ".") || strings.HasPrefix(cwdRel, "_") || cwdRel == "testdata")) {
+				return false
+			}
+
 			return matchPath(rel)
 		}
 	case pattern == "all":

[thanm]

Thanks for the report, and thanks for sending a patch.

Happy to submit for review if it is at all reasonable.

That would be fine with me, please do. I tried your fix in my own repo and it looks good to me.

[matthewhughes934]

Thanks for the report, and thanks for sending a patch.

Happy to submit for review if it is at all reasonable.

That would be fine with me, please do. I tried your fix in my own repo and it looks good to me.

Thanks, I've created #66171 for this

[gopherbot]

Change https://go.dev/cl/569895 mentions this issue: cmd/go: fix -coverpkg not ignoring special directories

[thanm]

Looks good. I left some comments on the CL.