cmd/go: do not install dependencies during "go install"

This CL makes "go install" behave the way many users expect:
install only the things named on the command line.
Future builds still run as fast, thanks to the new build cache (CL 75473).
To install dependencies as well (the old behavior), use "go install -i".

Actual definitions aside, what most users know and expect of "go install"
is that (1) it installs what you asked, and (2) it's fast, unlike "go build".
It was fast because it installed dependencies, but installing dependencies
confused users repeatedly (see for example #5065, #6424, #10998, #12329,
"go build" and "go test" so that they could be "fast" too, but that only
created new opportunities for confusion. We also had to add -installsuffix
and then -pkgdir, to allow "fast" even when dependencies could not be
installed in the usual place.

The recent introduction of precise content-based staleness logic means that
the go command detects the need for rebuilding packages more often than it
used to, with the consequence that "go install" rebuilds and reinstalls
dependencies more than it used to. This will create more new opportunities
for confusion and will certainly lead to more issues filed like the ones
listed above.

CL 75743 introduced a build cache, separate from the install locations.
That cache makes all operations equally incremental and fast, whether or
not the operation is "install" or "build", and whether or not "-i" is used.

Installing dependencies is no longer necessary for speed, it has confused
users in the past, and the more accurate rebuilds mean that it will confuse
users even more often in the future. This CL aims to end all that confusion
by not installing dependencies by default.

By analogy with "go build -i" and "go test -i", which still install
dependencies, this CL introduces "go install -i", which installs
dependencies in addition to the things named on the command line.

Fixes #5065.
Fixes #6424.
Fixes #10998.
Fixes #12329.
Fixes #18981.
Fixes #22469.

Another step toward #4719.

Change-Id: I3d7bc145c3a680e2f26416e182fa0dcf1e2a15e5
Reviewed-on: https://go-review.googlesource.com/75850
Run-TryBot: Russ Cox <[email protected]>
Reviewed-by: David Crawshaw <[email protected]>

Author: rsc

misc/cgo/testcarchive/carchive_test.go

@@ -174,7 +174,7 @@ func TestInstall(t *testing.T) {
 	testInstall(t, "./testp1"+exeSuffix,
 		filepath.Join("pkg", libgodir, "libgo.a"),
 		filepath.Join("pkg", libgodir, "libgo.h"),
-		"go", "install", "-buildmode=c-archive", "libgo")
+		"go", "install", "-i", "-buildmode=c-archive", "libgo")
 
 	// Test building libgo other than installing it.
 	// Header files are now present.
@@ -491,7 +491,7 @@ func TestPIE(t *testing.T) {
 		os.RemoveAll("pkg")
 	}()
 
-	cmd := exec.Command("go", "install", "-buildmode=c-archive", "libgo")
+	cmd := exec.Command("go", "install", "-i", "-buildmode=c-archive", "libgo")
 	cmd.Env = gopathEnv
 	if out, err := cmd.CombinedOutput(); err != nil {
 		t.Logf("%s", out)

misc/cgo/testcshared/cshared_test.go

@@ -216,7 +216,7 @@ func runCC(t *testing.T, args ...string) string {
 }
 
 func createHeaders() error {
-	args := []string{"go", "install", "-buildmode=c-shared",
+	args := []string{"go", "install", "-i", "-buildmode=c-shared",
 		"-installsuffix", "testcshared", "libgo"}
 	cmd := exec.Command(args[0], args[1:]...)
 	cmd.Env = gopathEnv

src/cmd/dist/build.go

@@ -1157,7 +1157,7 @@ func cmdbootstrap() {
 		// chosen $CC_FOR_TARGET in this case.
 		os.Setenv("CC", defaultcctarget)
 	}
-	goInstall(goBootstrap, toolchain...)
+	goInstall(goBootstrap, append([]string{"-i"}, toolchain...)...)
 	if debug {
 		run("", ShowOutput|CheckExit, pathf("%s/compile", tooldir), "-V=full")
 		run("", ShowOutput|CheckExit, pathf("%s/buildid", tooldir), pathf("%s/pkg/%s_%s/runtime/internal/sys.a", goroot, goos, goarch))
@@ -1185,7 +1185,7 @@ func cmdbootstrap() {
 		xprintf("\n")
 	}
 	xprintf("Building Go toolchain3 using go_bootstrap and Go toolchain2.\n")
-	goInstall(goBootstrap, append([]string{"-a"}, toolchain...)...)
+	goInstall(goBootstrap, append([]string{"-a", "-i"}, toolchain...)...)
 	if debug {
 		run("", ShowOutput|CheckExit, pathf("%s/compile", tooldir), "-V=full")
 		run("", ShowOutput|CheckExit, pathf("%s/buildid", tooldir), pathf("%s/pkg/%s_%s/runtime/internal/sys.a", goroot, goos, goarch))

src/cmd/dist/test.go

@@ -116,10 +116,25 @@ func (t *tester) run() {
 
 	if t.rebuild {
 		t.out("Building packages and commands.")
-		// Rebuilding is a shortened bootstrap.
-		// See cmdbootstrap for a description of the overall process.
-		goInstall("go", toolchain...)
-		goInstall("go", toolchain...)
+		// Force rebuild the whole toolchain.
+		goInstall("go", append([]string{"-a", "-i"}, toolchain...)...)
+	}
+
+	// Complete rebuild bootstrap, even with -no-rebuild.
+	// If everything is up-to-date, this is a no-op.
+	// If everything is not up-to-date, the first checkNotStale
+	// during the test process will kill the tests, so we might
+	// as well install the world.
+	// Now that for example "go install cmd/compile" does not
+	// also install runtime (you need "go install -i cmd/compile"
+	// for that), it's easy for previous workflows like
+	// "rebuild the compiler and then run run.bash"
+	// to break if we don't automatically refresh things here.
+	// Rebuilding is a shortened bootstrap.
+	// See cmdbootstrap for a description of the overall process.
+	if !t.listMode {
+		goInstall("go", append([]string{"-i"}, toolchain...)...)
+		goInstall("go", append([]string{"-i"}, toolchain...)...)
 		goInstall("go", "std", "cmd")
 		checkNotStale("go", "std", "cmd")
 	}

src/cmd/go/go_test.go

@@ -960,7 +960,7 @@ func TestGoInstallRebuildsStalePackagesInOtherGOPATH(t *testing.T) {
 		func F() {}`)
 	sep := string(filepath.ListSeparator)
 	tg.setenv("GOPATH", tg.path("d1")+sep+tg.path("d2"))
-	tg.run("install", "p1")
+	tg.run("install", "-i", "p1")
 	tg.wantNotStale("p1", "", "./testgo list claims p1 is stale, incorrectly")
 	tg.wantNotStale("p2", "", "./testgo list claims p2 is stale, incorrectly")
 	tg.sleep()
@@ -974,7 +974,7 @@ func TestGoInstallRebuildsStalePackagesInOtherGOPATH(t *testing.T) {
 	tg.wantStale("p2", "build ID mismatch", "./testgo list claims p2 is NOT stale, incorrectly")
 	tg.wantStale("p1", "stale dependency: p2", "./testgo list claims p1 is NOT stale, incorrectly")
 
-	tg.run("install", "p1")
+	tg.run("install", "-i", "p1")
 	tg.wantNotStale("p2", "", "./testgo list claims p2 is stale after reinstall, incorrectly")
 	tg.wantNotStale("p1", "", "./testgo list claims p1 is stale after reinstall, incorrectly")
 }
@@ -3296,10 +3296,11 @@ func TestGoInstallPkgdir(t *testing.T) {
 	tg.makeTempdir()
 	pkg := tg.path(".")
 	tg.run("install", "-pkgdir", pkg, "errors")
-	_, err := os.Stat(filepath.Join(pkg, "errors.a"))
-	tg.must(err)
-	_, err = os.Stat(filepath.Join(pkg, "runtime.a"))
-	tg.must(err)
+	tg.mustExist(filepath.Join(pkg, "errors.a"))
+	tg.mustNotExist(filepath.Join(pkg, "runtime.a"))
+	tg.run("install", "-i", "-pkgdir", pkg, "errors")
+	tg.mustExist(filepath.Join(pkg, "errors.a"))
+	tg.mustExist(filepath.Join(pkg, "runtime.a"))
 }
 
 func TestGoTestRaceInstallCgo(t *testing.T) {
@@ -4869,3 +4870,39 @@ func TestTestVet(t *testing.T) {
 	tg.run("test", "-vet=off", filepath.Join(tg.tempdir, "p1.go"))
 	tg.grepStdout(`\[no test files\]`, "did not print test summary")
 }
+
+func TestInstallDeps(t *testing.T) {
+	tg := testgo(t)
+	defer tg.cleanup()
+	tg.parallel()
+	tg.makeTempdir()
+	tg.setenv("GOPATH", tg.tempdir)
+
+	tg.tempFile("src/p1/p1.go", "package p1\nvar X =  1\n")
+	tg.tempFile("src/p2/p2.go", "package p2\nimport _ \"p1\"\n")
+	tg.tempFile("src/main1/main.go", "package main\nimport _ \"p2\"\nfunc main() {}\n")
+
+	tg.run("list", "-f={{.Target}}", "p1")
+	p1 := strings.TrimSpace(tg.getStdout())
+	tg.run("list", "-f={{.Target}}", "p2")
+	p2 := strings.TrimSpace(tg.getStdout())
+	tg.run("list", "-f={{.Target}}", "main1")
+	main1 := strings.TrimSpace(tg.getStdout())
+
+	tg.run("install", "main1")
+
+	tg.mustExist(main1)
+	tg.mustNotExist(p2)
+	tg.mustNotExist(p1)
+
+	tg.run("install", "p2")
+	tg.mustExist(p2)
+	tg.mustNotExist(p1)
+
+	tg.run("install", "-i", "main1")
+	tg.mustExist(p1)
+	tg.must(os.Remove(p1))
+
+	tg.run("install", "-i", "p2")
+	tg.mustExist(p1)
+}

src/cmd/go/internal/work/action.go

@@ -390,17 +390,6 @@ func (b *Builder) VetAction(mode, depMode BuildMode, p *load.Package) *Action {
 		a1.needVet = true
 		a.Func = (*Builder).vet
 
-		// If there might be an install action, make it depend on vet,
-		// so that the temporary files generated by the build step
-		// are not deleted before vet can use them.
-		// If nothing was going to install p, calling b.CompileAction with
-		// ModeInstall here creates the action, but nothing links it into the
-		// graph, so it will still not be installed.
-		install := b.CompileAction(ModeInstall, depMode, p)
-		if install != a1 {
-			install.Deps = append(install.Deps, a)
-		}
-
 		return a
 	})
 	return a
@@ -466,6 +455,13 @@ func (b *Builder) LinkAction(mode, depMode BuildMode, p *load.Package) *Action {
 
 // installAction returns the action for installing the result of a1.
 func (b *Builder) installAction(a1 *Action) *Action {
+	// Because we overwrite the build action with the install action below,
+	// a1 may already be an install action fetched from the "build" cache key,
+	// and the caller just doesn't realize.
+	if strings.HasSuffix(a1.Mode, "-install") {
+		return a1
+	}
+
 	// If there's no actual action to build a1,
 	// there's nothing to install either.
 	// This happens if a1 corresponds to reusing an already-built object.
@@ -475,18 +471,36 @@ func (b *Builder) installAction(a1 *Action) *Action {
 
 	p := a1.Package
 	return b.cacheAction(a1.Mode+"-install", p, func() *Action {
-		a := &Action{
-			Mode:    a1.Mode + "-install",
+		// The install deletes the temporary build result,
+		// so we need all other actions, both past and future,
+		// that attempt to depend on the build to depend instead
+		// on the install.
+
+		// Make a private copy of a1 (the build action),
+		// no longer accessible to any other rules.
+		buildAction := new(Action)
+		*buildAction = *a1
+
+		// Overwrite a1 with the install action.
+		// This takes care of updating past actions that
+		// point at a1 for the build action; now they will
+		// point at a1 and get the install action.
+		// We also leave a1 in the action cache as the result
+		// for "build", so that actions not yet created that
+		// try to depend on the build will instead depend
+		// on the install.
+		*a1 = Action{
+			Mode:    buildAction.Mode + "-install",
 			Func:    BuildInstallFunc,
 			Package: p,
-			Objdir:  a1.Objdir,
-			Deps:    []*Action{a1},
+			Objdir:  buildAction.Objdir,
+			Deps:    []*Action{buildAction},
 			Target:  p.Target,
 			built:   p.Target,
 		}
 
-		b.addInstallHeaderAction(a)
-		return a
+		b.addInstallHeaderAction(a1)
+		return a1
 	})
 }
 
@@ -514,7 +528,7 @@ func (b *Builder) addTransitiveLinkDeps(a, a1 *Action, shlib string) {
 		a1 := workq[i]
 		for _, a2 := range a1.Deps {
 			// TODO(rsc): Find a better discriminator than the Mode strings, once the dust settles.
-			if a2.Package == nil || (a2.Mode != "build-install" && a2.Mode != "build" && a2.Mode != "use installed") || haveDep[a2.Package.ImportPath] {
+			if a2.Package == nil || (a2.Mode != "build-install" && a2.Mode != "build") || haveDep[a2.Package.ImportPath] {
 				continue
 			}
 			haveDep[a2.Package.ImportPath] = true

src/cmd/go/internal/work/build.go

@@ -141,6 +141,8 @@ func init() {
 	CmdBuild.Flag.BoolVar(&cfg.BuildI, "i", false, "")
 	CmdBuild.Flag.StringVar(&cfg.BuildO, "o", "", "output file")
 
+	CmdInstall.Flag.BoolVar(&cfg.BuildI, "i", false, "")
+
 	AddBuildFlags(CmdBuild)
 	AddBuildFlags(CmdInstall)
 }
@@ -464,11 +466,12 @@ func runBuild(cmd *base.Command, args []string) {
 }
 
 var CmdInstall = &base.Command{
-	UsageLine: "install [build flags] [packages]",
+	UsageLine: "install [-i] [build flags] [packages]",
 	Short:     "compile and install packages and dependencies",
 	Long: `
-Install compiles and installs the packages named by the import paths,
-along with their dependencies.
+Install compiles and installs the packages named by the import paths.
+
+The -i flag installs the dependencies of the named packages as well.
 
 For more about the build flags, see 'go help build'.
 For more about specifying packages, see 'go help packages'.
@@ -565,6 +568,10 @@ func InstallPackages(args []string, forGet bool) {
 
 	var b Builder
 	b.Init()
+	depMode := ModeBuild
+	if cfg.BuildI {
+		depMode = ModeInstall
+	}
 	a := &Action{Mode: "go install"}
 	var tools []*Action
 	for _, p := range pkgs {
@@ -576,7 +583,7 @@ func InstallPackages(args []string, forGet bool) {
 		// If p is a tool, delay the installation until the end of the build.
 		// This avoids installing assemblers/compilers that are being executed
 		// by other steps in the build.
-		a1 := b.AutoAction(ModeInstall, ModeInstall, p)
+		a1 := b.AutoAction(ModeInstall, depMode, p)
 		if load.InstallTargetDir(p) == load.ToTool {
 			a.Deps = append(a.Deps, a1.Deps...)
 			a1.Deps = append(a1.Deps, a)