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 <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>

diff --git a/src/cmd/go/internal/modindex/read.go b/src/cmd/go/internal/modindex/read.go
index f01ca6ec1724a8e922e4c5f367892c4594f462fe..fa0271f6ecfef6ba6053a387414250bbd1e20b51 100644 (file)
--- a/src/cmd/go/internal/modindex/read.go
+++ b/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)

diff --git a/src/cmd/go/internal/work/build.go b/src/cmd/go/internal/work/build.go
index f93fb0bb6af56c3c75111dcf5a405fe12fe69eeb..98babc0024ee135a49a5dbeb5c3f14b3a87dbb39 100644 (file)
--- a/src/cmd/go/internal/work/build.go
+++ b/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:

diff --git a/src/cmd/go/testdata/script/install_goroot_targets.txt b/src/cmd/go/testdata/script/install_goroot_targets.txt
index cc143657c7a9e587f5b4c9f7a1e534dcc2a44eec..4d6ca13e2436f21e365acdf36e3dbefde33281e9 100644 (file)
--- a/src/cmd/go/testdata/script/install_goroot_targets.txt
+++ b/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

diff --git a/src/go/build/build.go b/src/go/build/build.go
index ccdc657e367d20a1fe6b358236dd032aa6373677..4c0388149dbc9b52ecfdaadb4c2700e858f10a9f 100644 (file)
--- a/src/go/build/build.go
+++ b/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)
        }

diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go
index 6fd83f777b9a78bed9699864a1b7d02840dec174..25556ac04c6c55735c42aaa3f9c5b9ec930e6a59 100644 (file)
--- a/src/go/build/deps_test.go
+++ b/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

diff --git a/src/internal/buildinternal/needs_install.go b/src/internal/buildinternal/needs_install.go
deleted file mode 100644 (file)
index b3c17df..0000000
--- a/src/internal/buildinternal/needs_install.go
+++ /dev/null
@@ -1,14 +0,0 @@
-// Copyright 2022 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// Package buildinternal provides internal functions used by go/build
-// that need to be used by other packages too.
-package buildinternal
-
-// NeedsInstalledDotA returns true if the given stdlib package
-// needs an installed .a file in the stdlib.
-func NeedsInstalledDotA(importPath string) bool {
-       return importPath == "net" || importPath == "os/signal" || importPath == "os/user" || importPath == "plugin" ||
-               importPath == "runtime/cgo"
-}