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 https://github.com/golang/go/pull/66171 for this

[gopherbot]

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

[thanm]

Looks good. I left some comments on the CL.