cmd/go: decide whether to install .a based on number of CgoFiles

Instead of hardcoding the set of five packages that depend on cgo to
decide whether a package should have an install target, make the
decision based on whether the package has any CgoFiles. This means that
in nocgo configurations, there will be no installed packages, and that
if an GOOS/GOARCH combination doesn't have cgo files we don't
unnecessarily install a .a.

Because the determination of whether a file is a CgoFile is made later
in the Import functions, the choice of whether to add a PkgObj for teh
case there are CgoFiles is moved later. One concern here is that in some
cases, PkgObj may be set differently in the case of the FindOnly mode,
since the determination is moved across the boundary. We might want
to always set PkgObj after the FindOnly boundary for consistency? cmd/go
doesn't seem to use it when calling Import with FindOnly.

Also remove internal/buildinternal/needs_install.go because we will be
checking whether to install based on the number of cgo files and it
might be overkill to make the NeedsInstalledDotA function be the
equivalent of len(input) > 0.

For #47257

Change-Id: I5f7f2137dc99aaeb2e2695c14e0222093a6b2407
Reviewed-on: https://go-review.googlesource.com/c/go/+/448803
Reviewed-by: Bryan Mills <[email protected]>
Reviewed-by: Michael Matloob <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Michael Matloob <[email protected]>

Author: matloob

src/cmd/go/internal/modindex/read.go

@@ -12,7 +12,6 @@ import (
 	"go/build"
 	"go/build/constraint"
 	"go/token"
-	"internal/buildinternal"
 	"internal/godebug"
 	"internal/goroot"
 	"path"
@@ -396,6 +395,7 @@ func (rp *IndexPackage) Import(bctxt build.Context, mode build.ImportMode) (p *b
 	inTestdata := func(sub string) bool {
 		return strings.Contains(sub, "/testdata/") || strings.HasSuffix(sub, "/testdata") || str.HasPathPrefix(sub, "testdata")
 	}
+	var pkga string
 	if !inTestdata(rp.dir) {
 		// In build.go, p.Root should only be set in the non-local-import case, or in
 		// GOROOT or GOPATH. Since module mode only calls Import with path set to "."
@@ -414,7 +414,6 @@ func (rp *IndexPackage) Import(bctxt build.Context, mode build.ImportMode) (p *b
 			// The fields set below (SrcRoot, PkgRoot, BinDir, PkgTargetRoot, and PkgObj)
 			// are only set in build.Import if p.Root != "".
 			var pkgtargetroot string
-			var pkga string
 			suffix := ""
 			if ctxt.InstallSuffix != "" {
 				suffix = "_" + ctxt.InstallSuffix
@@ -437,8 +436,7 @@ func (rp *IndexPackage) Import(bctxt build.Context, mode build.ImportMode) (p *b
 				p.PkgTargetRoot = ctxt.joinPath(p.Root, pkgtargetroot)
 
 				// Set the install target if applicable.
-				if strings.ToLower(godebug.Get("installgoroot")) == "all" ||
-					!p.Goroot || buildinternal.NeedsInstalledDotA(p.ImportPath) {
+				if strings.ToLower(godebug.Get("installgoroot")) == "all" || !p.Goroot {
 					p.PkgObj = ctxt.joinPath(p.Root, pkga)
 				}
 			}
@@ -629,6 +627,12 @@ func (rp *IndexPackage) Import(bctxt build.Context, mode build.ImportMode) (p *b
 		}
 	}
 
+	// Now that p.CgoFiles has been set, use it to determine whether
+	// a package in GOROOT gets an install target:
+	if len(p.CgoFiles) != 0 && p.Root != "" && p.Goroot && pkga != "" {
+		p.PkgObj = ctxt.joinPath(p.Root, pkga)
+	}
+
 	p.EmbedPatterns, p.EmbedPatternPos = cleanDecls(embedPos)
 	p.TestEmbedPatterns, p.TestEmbedPatternPos = cleanDecls(testEmbedPos)
 	p.XTestEmbedPatterns, p.XTestEmbedPatternPos = cleanDecls(xTestEmbedPos)

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

@@ -10,7 +10,6 @@ import (
 	"flag"
 	"fmt"
 	"go/build"
-	"internal/buildinternal"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -740,11 +739,11 @@ func InstallPackages(ctx context.Context, patterns []string, pkgs []*load.Packag
 				// or else something is wrong and worth reporting (like a ConflictDir).
 			case p.Name != "main" && p.Module != nil:
 				// Non-executables have no target (except the cache) when building with modules.
-			case p.Name != "main" && p.Standard && !buildinternal.NeedsInstalledDotA(p.ImportPath):
+			case p.Name != "main" && p.Standard && p.Internal.Build.PkgObj == "":
 				// Most packages in std do not need an installed .a, because they can be
 				// rebuilt and used directly from the build cache.
 				// A few targets (notably those using cgo) still do need to be installed
-				// in case the user's environment lacks a C compiler.			case p.Internal.GobinSubdir:
+				// in case the user's environment lacks a C compiler.
 			case p.Internal.GobinSubdir:
 				base.Errorf("go: cannot install cross-compiled binaries when GOBIN is set")
 			case p.Internal.CmdlineFiles:

src/cmd/go/testdata/script/install_goroot_targets.txt

@@ -19,3 +19,12 @@ stdout cgo\.a
 env GODEBUG=installgoroot=all
 go list -f '{{.Target}}' fmt
 stdout fmt\.a
+
+# With CGO_ENABLED=0, packages that would have
+# an install target with cgo on no longer do.
+env GODEBUG=
+env CGO_ENABLED=0
+go list -f '{{.Target}}' runtime/cgo
+! stdout .
+go list -export -f '{{.Export}}' runtime/cgo
+stdout $GOCACHE

src/go/build/build.go

@@ -13,7 +13,6 @@ import (
 	"go/doc"
 	"go/token"
 	"internal/buildcfg"
-	"internal/buildinternal"
 	"internal/godebug"
 	"internal/goroot"
 	"internal/goversion"
@@ -784,8 +783,7 @@ Found:
 			p.PkgTargetRoot = ctxt.joinPath(p.Root, pkgtargetroot)
 
 			// Set the install target if applicable.
-			if strings.ToLower(godebug.Get("installgoroot")) == "all" ||
-				!p.Goroot || buildinternal.NeedsInstalledDotA(p.ImportPath) {
+			if strings.ToLower(godebug.Get("installgoroot")) == "all" || !p.Goroot {
 				p.PkgObj = ctxt.joinPath(p.Root, pkga)
 			}
 		}
@@ -1003,6 +1001,12 @@ Found:
 		}
 	}
 
+	// Now that p.CgoFiles has been set, use it to determine whether
+	// a package in GOROOT gets an install target:
+	if len(p.CgoFiles) != 0 && p.Root != "" && p.Goroot && pkga != "" {
+		p.PkgObj = ctxt.joinPath(p.Root, pkga)
+	}
+
 	for tag := range allTags {
 		p.AllTags = append(p.AllTags, tag)
 	}

src/go/build/deps_test.go

@@ -40,7 +40,6 @@ var depsRules = `
 	# No dependencies allowed for any of these packages.
 	NONE
 	< constraints, container/list, container/ring,
-	  internal/buildinternal,
 	  internal/cfg, internal/coverage, internal/coverage/rtcov,
 	  internal/coverage/uleb128, internal/coverage/calloc,
 	  internal/cpu, internal/goarch,
@@ -286,7 +285,7 @@ var depsRules = `
 	FMT, internal/goexperiment
 	< internal/buildcfg;
 
-	go/build/constraint, go/doc, go/parser, internal/buildcfg, internal/goroot, internal/goversion, internal/buildinternal
+	go/build/constraint, go/doc, go/parser, internal/buildcfg, internal/goroot, internal/goversion
 	< go/build;
 
 	# databases