]> Cypherpunks.ru repositories - gostls13.git/commitdiff
cmd/go: don't install most GOROOT .a files in pkg
authorMichael Matloob <matloob@golang.org>
Wed, 21 Sep 2022 19:51:27 +0000 (15:51 -0400)
committerMichael Matloob <matloob@golang.org>
Fri, 28 Oct 2022 23:35:08 +0000 (23:35 +0000)
Packages in GOROOT that don't use cgo will not be installed in
GOROOT/pkg, and will instead be cached as usual like other Go
packages.

- add a internal/buildinternal package to hold the identities of the
  five packages that use cgo
- update dist's test code to do a go build std cmd before checking
  staleness on builders. Because most of those packages no longer have
  install locations, and have dependencies that don't either, the
  packages need to be cached to not be stale.
- fix index_test to import packages with the path "." when preparing
  the "want" values to compare the indexed data to. (the module index
  matches the behavior of build.ImportDir, which always passes in "."
  as the path.
- In both the index and go/build Importers, don't set
  PkgObj for GOROOT packages which will no longer have install
  targets. PkgTargetRoot will still be set to compute target paths,
  which will still be needed in buildmode=shared.
- "downgrade" all install actions that don't have a target to build
  actions. (The target should already not be set for packages that
  shouldn't be installed).

For #47257

Change-Id: Ia5aee6b3b20b58e028119cf0352a4c4a2f10f6b8
Reviewed-on: https://go-review.googlesource.com/c/go/+/432535
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
18 files changed:
misc/cgo/testshared/shared_test.go
src/cmd/compile/internal/importer/gcimporter.go
src/cmd/dist/test.go
src/cmd/go/go_test.go
src/cmd/go/internal/load/pkg.go
src/cmd/go/internal/modindex/index_test.go
src/cmd/go/internal/modindex/read.go
src/cmd/go/internal/work/action.go
src/cmd/go/internal/work/build.go
src/cmd/go/internal/work/exec.go
src/cmd/go/testdata/script/install_goroot_targets.txt [new file with mode: 0644]
src/cmd/link/link_test.go
src/go/build/build.go
src/go/build/build_test.go
src/go/build/deps_test.go
src/go/internal/gcimporter/gcimporter.go
src/go/types/example_test.go
src/internal/buildinternal/needs_install.go [new file with mode: 0644]

index 92c2166674f40b52915be19ec41a91e400494355..cd8a144d73aa82c54c45e3068ac2aad0877eed89 100644 (file)
@@ -151,17 +151,17 @@ func testMain(m *testing.M) (int, error) {
        myContext := build.Default
        myContext.GOROOT = goroot
        myContext.GOPATH = gopath
-       runtimeP, err := myContext.Import("runtime", ".", build.ImportComment)
-       if err != nil {
-               return 0, fmt.Errorf("import failed: %v", err)
-       }
-       gorootInstallDir = runtimeP.PkgTargetRoot + "_dynlink"
 
        // All tests depend on runtime being built into a shared library. Because
        // that takes a few seconds, do it here and have all tests use the version
        // built here.
        goCmd(nil, append([]string{"install", "-buildmode=shared"}, minpkgs...)...)
 
+       shlib := goCmd(nil, "list", "-linkshared", "-f={{.Shlib}}", "runtime")
+       if shlib != "" {
+               gorootInstallDir = filepath.Dir(shlib)
+       }
+
        myContext.InstallSuffix = "_dynlink"
        depP, err := myContext.Import("./depBase", ".", build.ImportComment)
        if err != nil {
index 0aa779441a7a3a364ed81aa689f752efce6bd6ee..e4d8a86f9a61598fd0edc41fdb47d6786e3a7d75 100644 (file)
@@ -74,8 +74,8 @@ func FindPkg(path, srcDir string) (filename, id string) {
                        }
                } else {
                        noext = strings.TrimSuffix(bp.PkgObj, ".a")
-                       id = bp.ImportPath
                }
+               id = bp.ImportPath
 
        case build.IsLocalImport(path):
                // "./x" -> "/this/directory/x.ext", "/this/directory/x"
index 1663ff8c94868e4d3678338dfb08318d1d63f878..29a886f4561da022899b45fa48b854dc979785ec 100644 (file)
@@ -161,6 +161,10 @@ func (t *tester) run() {
                        // Instead, we can just check that it is not stale, which may be less
                        // expensive (and is also more likely to catch bugs in the builder
                        // implementation).
+                       // The cache used by dist when building is different from that used when
+                       // running dist test, so rebuild (but don't install) std and cmd to make
+                       // sure packages without install targets are cached so they are not stale.
+                       goCmd("go", "build", "std", "cmd") // make sure dependencies of targets are cached
                        checkNotStale("go", "std", "cmd")
                }
        }
index bd1de7d1b9b183a4ae216c5d91567a3549da8d6e..f5f583fbeacfd9a331ea2747202cfe5a76bd7f18 100644 (file)
@@ -1803,16 +1803,23 @@ func TestImportLocal(t *testing.T) {
 
 func TestGoInstallPkgdir(t *testing.T) {
        skipIfGccgo(t, "gccgo has no standard packages")
+       if !canCgo {
+               // Only the stdlib packages that use cgo have install
+               // targets, (we're using net below) so cgo is required
+               // for the install.
+               t.Skip("skipping because cgo not enabled")
+       }
        tooSlow(t)
 
        tg := testgo(t)
        tg.parallel()
+       tg.setenv("GODEBUG", "installgoroot=all")
        defer tg.cleanup()
        tg.makeTempdir()
        pkg := tg.path(".")
-       tg.run("install", "-pkgdir", pkg, "sync")
-       tg.mustExist(filepath.Join(pkg, "sync.a"))
-       tg.mustNotExist(filepath.Join(pkg, "sync/atomic.a"))
+       tg.run("install", "-pkgdir", pkg, "net")
+       tg.mustExist(filepath.Join(pkg, "net.a"))
+       tg.mustNotExist(filepath.Join(pkg, "runtime/cgo.a"))
 }
 
 // For issue 14337.
index 1e50fdc0a5f0bc032049606ec95816891c2e5007..4a6414016ade4068593e20a26fc017b73556f0de 100644 (file)
@@ -374,7 +374,9 @@ func (p *Package) copyBuild(opts PackageOpts, pp *build.Package) {
                old := pp.PkgTargetRoot
                pp.PkgRoot = cfg.BuildPkgdir
                pp.PkgTargetRoot = cfg.BuildPkgdir
-               pp.PkgObj = filepath.Join(cfg.BuildPkgdir, strings.TrimPrefix(pp.PkgObj, old))
+               if pp.PkgObj != "" {
+                       pp.PkgObj = filepath.Join(cfg.BuildPkgdir, strings.TrimPrefix(pp.PkgObj, old))
+               }
        }
 
        p.Dir = pp.Dir
@@ -1814,11 +1816,19 @@ func (p *Package) load(ctx context.Context, opts PackageOpts, path string, stk *
                p.Target = ""
        } else {
                p.Target = p.Internal.Build.PkgObj
-               if cfg.BuildLinkshared && p.Target != "" {
-                       // TODO(bcmills): The reliance on p.Target implies that -linkshared does
-                       // not work for any package that lacks a Target — such as a non-main
+               if cfg.BuildBuildmode == "shared" && p.Internal.Build.PkgTargetRoot != "" {
+                       // TODO(matloob): This shouldn't be necessary, but the misc/cgo/testshared
+                       // test fails without Target set for this condition. Figure out why and
+                       // fix it.
+                       p.Target = filepath.Join(p.Internal.Build.PkgTargetRoot, p.ImportPath+".a")
+               }
+               if cfg.BuildLinkshared && p.Internal.Build.PkgTargetRoot != "" {
+                       // TODO(bcmills): The reliance on PkgTargetRoot implies that -linkshared does
+                       // not work for any package that lacks a PkgTargetRoot — such as a non-main
                        // package in module mode. We should probably fix that.
-                       shlibnamefile := p.Target[:len(p.Target)-2] + ".shlibname"
+                       targetPrefix := filepath.Join(p.Internal.Build.PkgTargetRoot, p.ImportPath)
+                       p.Target = targetPrefix + ".a"
+                       shlibnamefile := targetPrefix + ".shlibname"
                        shlib, err := os.ReadFile(shlibnamefile)
                        if err != nil && !os.IsNotExist(err) {
                                base.Fatalf("reading shlibname: %v", err)
index 1c32973d399f14fe93aa4186bcd920b7277970c7..6bc62f393fad5fd0b20d6577b98d20cb61caace0 100644 (file)
@@ -28,7 +28,7 @@ func TestIndex(t *testing.T) {
                if err != nil {
                        t.Fatal(err)
                }
-               bp1, err := build.Default.Import(pkg, filepath.Join(src, pkg), build.ImportComment)
+               bp1, err := build.Default.Import(".", filepath.Join(src, pkg), build.ImportComment)
                if err != nil {
                        t.Fatal(err)
                }
index d6a04a015636b67968ead92dbadd95e86298c5c6..e9cfbca8aed4b6bb6be921e569872aa0785a69db 100644 (file)
@@ -12,6 +12,7 @@ import (
        "go/build"
        "go/build/constraint"
        "go/token"
+       "internal/buildinternal"
        "internal/godebug"
        "internal/goroot"
        "path"
@@ -431,8 +432,15 @@ func (rp *IndexPackage) Import(bctxt build.Context, mode build.ImportMode) (p *b
                        p.PkgRoot = ctxt.joinPath(p.Root, "pkg")
                        p.BinDir = ctxt.joinPath(p.Root, "bin")
                        if pkga != "" {
+                               // Always set PkgTargetRoot. It might be used when building in shared
+                               // mode.
                                p.PkgTargetRoot = ctxt.joinPath(p.Root, pkgtargetroot)
-                               p.PkgObj = ctxt.joinPath(p.Root, pkga)
+
+                               // Set the install target if applicable.
+                               if strings.ToLower(godebug.Get("installgoroot")) == "all" ||
+                                       !p.Goroot || buildinternal.NeedsInstalledDotA(p.ImportPath) {
+                                       p.PkgObj = ctxt.joinPath(p.Root, pkga)
+                               }
                        }
                }
        }
index 60ab68c65c940f5bf4caf7785659a66af81f9285..d2f32558fa555ad3cd96d534ad17c7c6099f7f4c 100644 (file)
@@ -423,8 +423,8 @@ func (b *Builder) CompileAction(mode, depMode BuildMode, p *load.Package) *Actio
        vetOnly := mode&ModeVetOnly != 0
        mode &^= ModeVetOnly
 
-       if mode != ModeBuild && (p.Internal.Local || p.Module != nil) && p.Target == "" {
-               // Imported via local path or using modules. No permanent target.
+       if mode != ModeBuild && p.Target == "" {
+               // No permanent target.
                mode = ModeBuild
        }
        if mode != ModeBuild && p.Name == "main" {
@@ -872,14 +872,15 @@ func (b *Builder) linkSharedAction(mode, depMode BuildMode, shlib string, a1 *Ac
                        }
                        for _, a2 := range buildAction.Deps[0].Deps {
                                p := a2.Package
-                               if p.Target == "" {
+                               pkgTargetRoot := p.Internal.Build.PkgTargetRoot
+                               if pkgTargetRoot == "" {
                                        continue
                                }
                                a.Deps = append(a.Deps, &Action{
                                        Mode:    "shlibname",
                                        Package: p,
                                        Func:    (*Builder).installShlibname,
-                                       Target:  strings.TrimSuffix(p.Target, ".a") + ".shlibname",
+                                       Target:  filepath.Join(pkgTargetRoot, p.ImportPath+".shlibname"),
                                        Deps:    []*Action{a.Deps[0]},
                                })
                        }
index d27d114d912d6d9b4e47de68adf58d2904b33ba8..d8b78480718fa80ce3135cd564cd615424c6b7dd 100644 (file)
@@ -10,6 +10,7 @@ import (
        "flag"
        "fmt"
        "go/build"
+       "internal/buildinternal"
        "os"
        "os/exec"
        "path/filepath"
@@ -723,8 +724,6 @@ func InstallPackages(ctx context.Context, patterns []string, pkgs []*load.Packag
        for _, p := range pkgs {
                if p.Target == "" {
                        switch {
-                       case p.Standard && p.ImportPath == "unsafe":
-                               // unsafe is a built-in package, has no target
                        case p.Name != "main" && p.Internal.Local && p.ConflictDir == "":
                                // Non-executables outside GOPATH need not have a target:
                                // we can use the cache to hold the built package archive for use in future builds.
@@ -732,6 +731,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):
+                               // 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:
                        case p.Internal.GobinSubdir:
                                base.Errorf("go: cannot install cross-compiled binaries when GOBIN is set")
                        case p.Internal.CmdlineFiles:
index 11f6c7a498d179c89ce8e36cf4f4852c2a14e2f8..29d75001cbb23aea13c0e5ae5941098b1612f3f3 100644 (file)
@@ -1583,6 +1583,9 @@ func (b *Builder) installShlibname(ctx context.Context, a *Action) error {
 
        // TODO: BuildN
        a1 := a.Deps[0]
+       if err := b.Mkdir(filepath.Dir(a.Target)); err != nil {
+               return err
+       }
        err := os.WriteFile(a.Target, []byte(filepath.Base(a1.Target)+"\n"), 0666)
        if err != nil {
                return err
diff --git a/src/cmd/go/testdata/script/install_goroot_targets.txt b/src/cmd/go/testdata/script/install_goroot_targets.txt
new file mode 100644 (file)
index 0000000..cc14365
--- /dev/null
@@ -0,0 +1,21 @@
+[short] skip
+[!cgo] skip
+
+# Most packages in std do not have an install target.
+go list -f '{{.Target}}' fmt
+! stdout .
+go list -export -f '{{.Export}}' fmt
+stdout $GOCACHE
+
+# Packages that use cgo still do.
+go list -f '{{.Target}}' runtime/cgo
+stdout .
+go list -export -f '{{.Export}}' runtime/cgo
+! stdout $GOCACHE
+stdout cgo\.a
+
+# With GODEBUG=installgoroot=all, fmt has a target.
+# (Though we can't try installing it without modifying goroot).
+env GODEBUG=installgoroot=all
+go list -f '{{.Target}}' fmt
+stdout fmt\.a
index ce065721649d30baa25bc8bbbb30064f54ce98c0..406487c7eee4d7be7af61b6bbfeb3971f408f6af 100644 (file)
@@ -1104,6 +1104,8 @@ func TestUnlinkableObj(t *testing.T) {
        xObj := filepath.Join(tmpdir, "x.o")
        pObj := filepath.Join(tmpdir, "p.o")
        exe := filepath.Join(tmpdir, "x.exe")
+       importcfgfile := filepath.Join(tmpdir, "importcfg")
+       testenv.WriteImportcfg(t, importcfgfile, map[string]string{"p": pObj})
        err := os.WriteFile(xSrc, []byte("package main\nimport _ \"p\"\nfunc main() {}\n"), 0666)
        if err != nil {
                t.Fatalf("failed to write source file: %v", err)
@@ -1112,17 +1114,17 @@ func TestUnlinkableObj(t *testing.T) {
        if err != nil {
                t.Fatalf("failed to write source file: %v", err)
        }
-       cmd := exec.Command(testenv.GoToolPath(t), "tool", "compile", "-o", pObj, pSrc) // without -p
+       cmd := exec.Command(testenv.GoToolPath(t), "tool", "compile", "-importcfg="+importcfgfile, "-o", pObj, pSrc) // without -p
        out, err := cmd.CombinedOutput()
        if err != nil {
                t.Fatalf("compile p.go failed: %v. output:\n%s", err, out)
        }
-       cmd = exec.Command(testenv.GoToolPath(t), "tool", "compile", "-I", tmpdir, "-p=main", "-o", xObj, xSrc)
+       cmd = exec.Command(testenv.GoToolPath(t), "tool", "compile", "-importcfg="+importcfgfile, "-p=main", "-o", xObj, xSrc)
        out, err = cmd.CombinedOutput()
        if err != nil {
                t.Fatalf("compile x.go failed: %v. output:\n%s", err, out)
        }
-       cmd = exec.Command(testenv.GoToolPath(t), "tool", "link", "-L", tmpdir, "-o", exe, xObj)
+       cmd = exec.Command(testenv.GoToolPath(t), "tool", "link", "-importcfg="+importcfgfile, "-o", exe, xObj)
        out, err = cmd.CombinedOutput()
        if err == nil {
                t.Fatalf("link did not fail")
@@ -1132,17 +1134,18 @@ func TestUnlinkableObj(t *testing.T) {
        }
 
        // It is okay to omit -p for (only) main package.
-       cmd = exec.Command(testenv.GoToolPath(t), "tool", "compile", "-p=p", "-o", pObj, pSrc)
+       cmd = exec.Command(testenv.GoToolPath(t), "tool", "compile", "-importcfg="+importcfgfile, "-p=p", "-o", pObj, pSrc)
        out, err = cmd.CombinedOutput()
        if err != nil {
                t.Fatalf("compile p.go failed: %v. output:\n%s", err, out)
        }
-       cmd = exec.Command(testenv.GoToolPath(t), "tool", "compile", "-I", tmpdir, "-o", xObj, xSrc) // without -p
+       cmd = exec.Command(testenv.GoToolPath(t), "tool", "compile", "-importcfg="+importcfgfile, "-o", xObj, xSrc) // without -p
        out, err = cmd.CombinedOutput()
        if err != nil {
                t.Fatalf("compile failed: %v. output:\n%s", err, out)
        }
-       cmd = exec.Command(testenv.GoToolPath(t), "tool", "link", "-L", tmpdir, "-o", exe, xObj)
+
+       cmd = exec.Command(testenv.GoToolPath(t), "tool", "link", "-importcfg="+importcfgfile, "-o", exe, xObj)
        out, err = cmd.CombinedOutput()
        if err != nil {
                t.Errorf("link failed: %v. output:\n%s", err, out)
index 4fa07788c9c5182742f61bc71ccccc186b60ad75..28c74455809573fe03794de8836350249a1f451e 100644 (file)
@@ -13,6 +13,8 @@ import (
        "go/doc"
        "go/token"
        "internal/buildcfg"
+       "internal/buildinternal"
+       "internal/godebug"
        "internal/goroot"
        "internal/goversion"
        "io"
@@ -777,8 +779,15 @@ Found:
                p.PkgRoot = ctxt.joinPath(p.Root, "pkg")
                p.BinDir = ctxt.joinPath(p.Root, "bin")
                if pkga != "" {
+                       // Always set PkgTargetRoot. It might be used when building in shared
+                       // mode.
                        p.PkgTargetRoot = ctxt.joinPath(p.Root, pkgtargetroot)
-                       p.PkgObj = ctxt.joinPath(p.Root, pkga)
+
+                       // Set the install target if applicable.
+                       if strings.ToLower(godebug.Get("installgoroot")) == "all" ||
+                               !p.Goroot || buildinternal.NeedsInstalledDotA(p.ImportPath) {
+                               p.PkgObj = ctxt.joinPath(p.Root, pkga)
+                       }
                }
        }
 
index 8fa17c7343e64ae31d7420a08f80ef293a32cb48..1b0a371d67be250f511638847b1793abedfc6b08 100644 (file)
@@ -675,7 +675,9 @@ func TestImportDirTarget(t *testing.T) {
        testenv.MustHaveGoBuild(t) // really must just have source
        ctxt := Default
        ctxt.GOPATH = ""
-       p, err := ctxt.ImportDir(filepath.Join(testenv.GOROOT(t), "src/path"), 0)
+       // In GOROOT only a handful of packages have install targets. Most stdlib packages will
+       // only be built and placed in the build cache.
+       p, err := ctxt.ImportDir(filepath.Join(testenv.GOROOT(t), "src/runtime/cgo"), 0)
        if err != nil {
                t.Fatal(err)
        }
index 25556ac04c6c55735c42aaa3f9c5b9ec930e6a59..6fd83f777b9a78bed9699864a1b7d02840dec174 100644 (file)
@@ -40,6 +40,7 @@ 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,
@@ -285,7 +286,7 @@ var depsRules = `
        FMT, internal/goexperiment
        < internal/buildcfg;
 
-       go/build/constraint, go/doc, go/parser, internal/buildcfg, internal/goroot, internal/goversion
+       go/build/constraint, go/doc, go/parser, internal/buildcfg, internal/goroot, internal/goversion, internal/buildinternal
        < go/build;
 
        # databases
index 0ec464056c0f7bd6eb05e91ab6071275b87b4f88..614fe52caf86f01e24ade5e9508f4c43a86d9c90 100644 (file)
@@ -73,8 +73,8 @@ func FindPkg(path, srcDir string) (filename, id string) {
                        }
                } else {
                        noext = strings.TrimSuffix(bp.PkgObj, ".a")
-                       id = bp.ImportPath
                }
+               id = bp.ImportPath
 
        case build.IsLocalImport(path):
                // "./x" -> "/this/directory/x.ext", "/this/directory/x"
index be70e14610eb6e1f8f6a49dec5f14058ad7f3d2c..605e9874088de78f6a7340cac6cc1a4c5f4494fe 100644 (file)
@@ -5,7 +5,7 @@
 // Only run where builders (build.golang.org) have
 // access to compiled packages for import.
 //
-//go:build !arm && !arm64
+//go:build !android && !ios && !js
 
 package types_test
 
diff --git a/src/internal/buildinternal/needs_install.go b/src/internal/buildinternal/needs_install.go
new file mode 100644 (file)
index 0000000..b3c17df
--- /dev/null
@@ -0,0 +1,14 @@
+// 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"
+}