cmd/go: fix coverage rebuild corner case

If you have a package p1 with an xtest (package p1_test)
that imports p2, where p2 itself imports p1, then when
trying to do coverage for p1 we need to make sure to
recompile p2. The problem was that the overall package
import graph looked like:

    main -> p1_test -> p2 -> p1

Since we were recompiling p1 with coverage, we correctly
figured out that because p2 depends on a package being
recompiled due to coverage, p2 also needs to be split (forked) to
insert the dependency on the modified p1. But then we used
the same logic to split p1_test and main, with the effect that
the changes to p2 and p1_test and main were lost, since the
caller was still holding on to the original main, not the split version.

Change the code to treat main and p1_test as "already split"
and just update them in place.

Fixes #23314.

Change-Id: If7edeca6e39cdaeb5b9380d00b0c7d8c5891f086
Reviewed-on: https://go-review.googlesource.com/86237
Run-TryBot: Russ Cox <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>

Author: rsc

src/cmd/go/go_test.go

@@ -2471,6 +2471,17 @@ func TestCoverageSyncAtomicImport(t *testing.T) {
 	tg.run("test", "-short", "-cover", "-covermode=atomic", "-coverpkg=coverdep/p1", "coverdep")
 }
 
+func TestCoverageDepLoop(t *testing.T) {
+	tg := testgo(t)
+	defer tg.cleanup()
+	tg.parallel()
+	tg.setenv("GOPATH", filepath.Join(tg.pwd(), "testdata"))
+	// coverdep2/p1's xtest imports coverdep2/p2 which imports coverdep2/p1.
+	// Make sure that coverage on coverdep2/p2 recompiles coverdep2/p2.
+	tg.run("test", "-short", "-cover", "coverdep2/p1")
+	tg.grepStdout("coverage: 100.0% of statements", "expected 100.0% coverage")
+}
+
 func TestCoverageImportMainLoop(t *testing.T) {
 	tg := testgo(t)
 	defer tg.cleanup()

src/cmd/go/internal/test/test.go

@@ -1003,7 +1003,7 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
 		// This will cause extra compilation, so for now we only do it
 		// when testCover is set. The conditions are more general, though,
 		// and we may find that we need to do it always in the future.
-		recompileForTest(pmain, p, ptest)
+		recompileForTest(pmain, p, ptest, pxtest)
 	}
 
 	for _, cp := range pmain.Internal.Imports {
@@ -1159,14 +1159,17 @@ Search:
 	return stk
 }
 
-func recompileForTest(pmain, preal, ptest *load.Package) {
+func recompileForTest(pmain, preal, ptest, pxtest *load.Package) {
 	// The "test copy" of preal is ptest.
 	// For each package that depends on preal, make a "test copy"
 	// that depends on ptest. And so on, up the dependency tree.
 	testCopy := map[*load.Package]*load.Package{preal: ptest}
 	for _, p := range load.PackageList([]*load.Package{pmain}) {
+		if p == preal {
+			continue
+		}
 		// Copy on write.
-		didSplit := false
+		didSplit := p == pmain || p == pxtest
 		split := func() {
 			if didSplit {
 				return

src/cmd/go/testdata/src/coverdep2/p1/p.go

@@ -0,0 +1,3 @@
+package p1
+
+func F() int { return 1 }

src/cmd/go/testdata/src/coverdep2/p1/p_test.go

@@ -0,0 +1,10 @@
+package p1_test
+
+import (
+	"coverdep2/p2"
+	"testing"
+)
+
+func Test(t *testing.T) {
+	p2.F()
+}

src/cmd/go/testdata/src/coverdep2/p2/p2.go

@@ -0,0 +1,7 @@
+package p2
+
+import "coverdep2/p1"
+
+func F() {
+	p1.F()
+}