From d34548e0b6acc14a99bc6ffc225eedbb56e03d60 Mon Sep 17 00:00:00 2001 From: Jay Conrod Date: Mon, 18 Mar 2019 18:35:44 -0400 Subject: [PATCH] cmd/go: avoid link error when -coverpkg covers main packages (more) This fixes two problems missed in CL 164877. First, p.Internal.BuildInfo is now part of the cache key. This is important since p.Internal.BuildInfo causes the build action to synthesize a new source file, which affects the output. Second, recompileForTest is always called for test packages. Previously, it was only called when there were internal test sources, so the fix in CL 164877 did not apply to packages that only had external tests. Fixes #30374 Change-Id: Iac2d7e8914f0313f9ab4222299a866f67889eb2e Reviewed-on: https://go-review.googlesource.com/c/go/+/168200 Run-TryBot: Jay Conrod TryBot-Result: Gobot Gobot Reviewed-by: Ian Lance Taylor --- src/cmd/go/internal/load/test.go | 23 ++++++++------- src/cmd/go/internal/work/exec.go | 1 + .../script/cover_pkgall_multiple_mains.txt | 28 +++++++++++-------- 3 files changed, 29 insertions(+), 23 deletions(-) diff --git a/src/cmd/go/internal/load/test.go b/src/cmd/go/internal/load/test.go index 5142b16e06..c8e0b3f5f6 100644 --- a/src/cmd/go/internal/load/test.go +++ b/src/cmd/go/internal/load/test.go @@ -292,17 +292,8 @@ func TestPackagesAndErrors(p *Package, cover *TestCover) (pmain, ptest, pxtest * pmain.Imports = pmain.Imports[:w] pmain.Internal.RawImports = str.StringList(pmain.Imports) - if ptest != p { - // We have made modifications to the package p being tested - // and are rebuilding p (as ptest). - // Arrange to rebuild all packages q such that - // the test depends on q and q depends on p. - // This makes sure that q sees the modifications to p. - // Strictly speaking, the rebuild is only necessary if the - // modifications to p change its export metadata, but - // determining that is a bit tricky, so we rebuild always. - recompileForTest(pmain, p, ptest, pxtest) - } + // Replace pmain's transitive dependencies with test copies, as necessary. + recompileForTest(pmain, p, ptest, pxtest) // Should we apply coverage analysis locally, // only for this package and only for this test? @@ -351,6 +342,14 @@ Search: return stk } +// recompileForTest copies and replaces certain packages in pmain's dependency +// graph. This is necessary for two reasons. First, if ptest is different than +// preal, packages that import the package under test should get ptest instead +// of preal. This is particularly important if pxtest depends on functionality +// exposed in test sources in ptest. Second, if there is a main package +// (other than pmain) anywhere, we need to clear p.Internal.BuildInfo in +// the test copy to prevent link conflicts. This may happen if both -coverpkg +// and the command line patterns include multiple main packages. func recompileForTest(pmain, preal, ptest, pxtest *Package) { // The "test copy" of preal is ptest. // For each package that depends on preal, make a "test copy" @@ -393,7 +392,7 @@ func recompileForTest(pmain, preal, ptest, pxtest *Package) { // Don't compile build info from a main package. This can happen // if -coverpkg patterns include main packages, since those packages - // are imported by pmain. + // are imported by pmain. See golang.org/issue/30907. if p.Internal.BuildInfo != "" && p != pmain { split() } diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go index 0e10f2c926..14d13f83d3 100644 --- a/src/cmd/go/internal/work/exec.go +++ b/src/cmd/go/internal/work/exec.go @@ -213,6 +213,7 @@ func (b *Builder) buildActionID(a *Action) cache.ActionID { if p.Internal.CoverMode != "" { fmt.Fprintf(h, "cover %q %q\n", p.Internal.CoverMode, b.toolID("cover")) } + fmt.Fprintf(h, "modinfo %q\n", p.Internal.BuildInfo) // Configuration specific to compiler toolchain. switch cfg.BuildToolchainName { diff --git a/src/cmd/go/testdata/script/cover_pkgall_multiple_mains.txt b/src/cmd/go/testdata/script/cover_pkgall_multiple_mains.txt index 8ee4848d0a..ab7cd66949 100644 --- a/src/cmd/go/testdata/script/cover_pkgall_multiple_mains.txt +++ b/src/cmd/go/testdata/script/cover_pkgall_multiple_mains.txt @@ -6,32 +6,38 @@ env GO111MODULE=on [short] skip -go test -coverpkg=all ./main1 ./main2 +go test -coverpkg=all ./... -- go.mod -- module example.com/cov --- main1/main1.go -- +-- mainonly/mainonly.go -- package main func main() {} --- main1/main1_test.go -- +-- mainwithtest/mainwithtest.go -- package main -import "testing" +func main() {} -func TestMain1(t *testing.T) {} +func Foo() {} --- main2/main2.go -- +-- mainwithtest/mainwithtest_test.go -- package main -func main() {} +import "testing" --- main2/main2_test.go -- -package main +func TestFoo(t *testing.T) { + Foo() +} -import "testing" +-- xtest/x.go -- +package x -func TestMain2(t *testing.T) {} +-- xtest/x_test.go -- +package x_test + +import "testing" +func TestX(t *testing.T) {} -- 2.50.0