Commit 4fc9d3bc authored by Jay Conrod's avatar Jay Conrod Committed by Brad Fitzpatrick

[release-branch.go1.12] 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 #30937

Change-Id: Iac2d7e8914f0313f9ab4222299a866f67889eb2e
Reviewed-on: https://go-review.googlesource.com/c/go/+/168200
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
(cherry picked from commit d34548e0)
Reviewed-on: https://go-review.googlesource.com/c/go/+/168717
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
parent 92e78f7e
...@@ -268,17 +268,8 @@ func TestPackagesFor(p *Package, cover *TestCover) (pmain, ptest, pxtest *Packag ...@@ -268,17 +268,8 @@ func TestPackagesFor(p *Package, cover *TestCover) (pmain, ptest, pxtest *Packag
pmain.Imports = pmain.Imports[:w] pmain.Imports = pmain.Imports[:w]
pmain.Internal.RawImports = str.StringList(pmain.Imports) pmain.Internal.RawImports = str.StringList(pmain.Imports)
if ptest != p { // Replace pmain's transitive dependencies with test copies, as necessary.
// We have made modifications to the package p being tested recompileForTest(pmain, p, ptest, pxtest)
// 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)
}
// Should we apply coverage analysis locally, // Should we apply coverage analysis locally,
// only for this package and only for this test? // only for this package and only for this test?
...@@ -325,6 +316,14 @@ Search: ...@@ -325,6 +316,14 @@ Search:
return stk 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) { func recompileForTest(pmain, preal, ptest, pxtest *Package) {
// The "test copy" of preal is ptest. // The "test copy" of preal is ptest.
// For each package that depends on preal, make a "test copy" // For each package that depends on preal, make a "test copy"
...@@ -367,7 +366,7 @@ func recompileForTest(pmain, preal, ptest, pxtest *Package) { ...@@ -367,7 +366,7 @@ func recompileForTest(pmain, preal, ptest, pxtest *Package) {
// Don't compile build info from a main package. This can happen // Don't compile build info from a main package. This can happen
// if -coverpkg patterns include main packages, since those packages // 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 { if p.Internal.BuildInfo != "" && p != pmain {
split() split()
} }
......
...@@ -214,6 +214,7 @@ func (b *Builder) buildActionID(a *Action) cache.ActionID { ...@@ -214,6 +214,7 @@ func (b *Builder) buildActionID(a *Action) cache.ActionID {
if p.Internal.CoverMode != "" { if p.Internal.CoverMode != "" {
fmt.Fprintf(h, "cover %q %q\n", p.Internal.CoverMode, b.toolID("cover")) 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. // Configuration specific to compiler toolchain.
switch cfg.BuildToolchainName { switch cfg.BuildToolchainName {
......
...@@ -6,32 +6,38 @@ env GO111MODULE=on ...@@ -6,32 +6,38 @@ env GO111MODULE=on
[short] skip [short] skip
go test -coverpkg=all ./main1 ./main2 go test -coverpkg=all ./...
-- go.mod -- -- go.mod --
module example.com/cov module example.com/cov
-- main1/main1.go -- -- mainonly/mainonly.go --
package main package main
func main() {} func main() {}
-- main1/main1_test.go -- -- mainwithtest/mainwithtest.go --
package main package main
import "testing" func main() {}
func TestMain1(t *testing.T) {} func Foo() {}
-- main2/main2.go -- -- mainwithtest/mainwithtest_test.go --
package main package main
func main() {} import "testing"
-- main2/main2_test.go -- func TestFoo(t *testing.T) {
package main 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) {}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment