]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go: stamp VCS information in test binaries when -buildvcs=true
authorBryan C. Mills <bcmills@google.com>
Fri, 27 May 2022 20:33:36 +0000 (16:33 -0400)
committerGopher Robot <gobot@golang.org>
Tue, 16 Aug 2022 17:18:19 +0000 (17:18 +0000)
(But still not when -buildvcs=auto, the default.)

Fixes #52648.

Change-Id: I87a87d4ea84e8bf9635a4f7c8674c9311c3e21be
Reviewed-on: https://go-review.googlesource.com/c/go/+/409177
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>

src/cmd/go/internal/list/list.go
src/cmd/go/internal/load/pkg.go
src/cmd/go/internal/work/build.go
src/cmd/go/testdata/script/test_buildvcs.txt

index 5f8be6e3c9d9ee3ca310890d5d75e1c5544b211d..2e3614d317c76412721e851a24118c41b5a05f57 100644 (file)
@@ -592,7 +592,7 @@ func runList(ctx context.Context, cmd *base.Command, args []string) {
        pkgOpts := load.PackageOpts{
                IgnoreImports:   *listFind,
                ModResolveTests: *listTest,
-               LoadVCS:         true,
+               AutoVCS:         true,
                // SuppressDeps is set if the user opts to explicitly ask for the json fields they
                // need, don't ask for Deps or DepsErrors. It's not set when using a template string,
                // even if *listFmt doesn't contain .Deps because Deps are used to find import cycles
index 046f508545499d95bd187001bcb30a0376f47950..2cd61b9dcb09d62f4560cb1f1ded183ee713c4e8 100644 (file)
@@ -1974,7 +1974,7 @@ func (p *Package) load(ctx context.Context, opts PackageOpts, path string, stk *
                // Consider starting this as a background goroutine and retrieving the result
                // asynchronously when we're actually ready to build the package, or when we
                // actually need to evaluate whether the package's metadata is stale.
-               p.setBuildInfo(opts.LoadVCS)
+               p.setBuildInfo(opts.AutoVCS)
        }
 
        // unsafe is a fake package.
@@ -2264,7 +2264,7 @@ var vcsStatusCache par.Cache
 //
 // Note that the GoVersion field is not set here to avoid encoding it twice.
 // It is stored separately in the binary, mostly for historical reasons.
-func (p *Package) setBuildInfo(includeVCS bool) {
+func (p *Package) setBuildInfo(autoVCS bool) {
        setPkgErrorf := func(format string, args ...any) {
                if p.Error == nil {
                        p.Error = &PackageError{Err: fmt.Errorf(format, args...)}
@@ -2420,7 +2420,19 @@ func (p *Package) setBuildInfo(includeVCS bool) {
        var vcsCmd *vcs.Cmd
        var err error
        const allowNesting = true
-       if includeVCS && cfg.BuildBuildvcs != "false" && p.Module != nil && p.Module.Version == "" && !p.Standard && !p.IsTestOnly() {
+
+       wantVCS := false
+       switch cfg.BuildBuildvcs {
+       case "true":
+               wantVCS = true // Include VCS metadata even for tests if requested explicitly; see https://go.dev/issue/52648.
+       case "auto":
+               wantVCS = autoVCS && !p.IsTestOnly()
+       case "false":
+       default:
+               panic(fmt.Sprintf("unexpected value for cfg.BuildBuildvcs: %q", cfg.BuildBuildvcs))
+       }
+
+       if wantVCS && p.Module != nil && p.Module.Version == "" && !p.Standard {
                repoDir, vcsCmd, err = vcs.FromDir(base.Cwd(), "", allowNesting)
                if err != nil && !errors.Is(err, os.ErrNotExist) {
                        setVCSError(err)
@@ -2724,8 +2736,9 @@ type PackageOpts struct {
        // may be printed for non-literal arguments that match no main packages.
        MainOnly bool
 
-       // LoadVCS controls whether we also load version-control metadata for main packages.
-       LoadVCS bool
+       // AutoVCS controls whether we also load version-control metadata for main packages
+       // when -buildvcs=auto (the default).
+       AutoVCS bool
 
        // SuppressDeps is true if the caller does not need Deps and DepsErrors to be populated
        // on the package. TestPackagesAndErrors examines the  Deps field to determine if the test
index 42745d992814c918c8b7a8a023d932b88d0f2d98..352e46b48f9955414e8ccf2571534339e8508a98 100644 (file)
@@ -406,7 +406,7 @@ func runBuild(ctx context.Context, cmd *base.Command, args []string) {
        var b Builder
        b.Init()
 
-       pkgs := load.PackagesAndErrors(ctx, load.PackageOpts{LoadVCS: true}, args)
+       pkgs := load.PackagesAndErrors(ctx, load.PackageOpts{AutoVCS: true}, args)
        load.CheckPackageErrors(pkgs)
 
        explicitO := len(cfg.BuildO) > 0
@@ -636,7 +636,7 @@ func runInstall(ctx context.Context, cmd *base.Command, args []string) {
 
        modload.InitWorkfile()
        BuildInit()
-       pkgs := load.PackagesAndErrors(ctx, load.PackageOpts{LoadVCS: true}, args)
+       pkgs := load.PackagesAndErrors(ctx, load.PackageOpts{AutoVCS: true}, args)
        if cfg.ModulesEnabled && !modload.HasModRoot() {
                haveErrors := false
                allMissingErrors := true
index a6699660364581033d2ca8bd3d3ad412db02857e..965f76bf0d05df965607cf7fb4581f89a99ac2b5 100644 (file)
@@ -5,14 +5,18 @@
 [short] skip
 [!exec:git] skip
 
-env GOFLAGS=-buildvcs  # override default -buildvcs=auto in GOFLAGS, as a user might
-
 exec git init
 
-# The test binaries should not have VCS settings stamped.
+# The test binaries should not have VCS settings stamped by default.
 # (The test itself verifies that.)
 go test . ./testonly
 
+# However, setting -buildvcs explicitly should override that and
+# stamp anyway (https://go.dev/issue/52648).
+go test -buildvcs -c -o ./testonly.exe ./testonly
+! exec ./testonly.exe
+stdout 'unexpected VCS setting: vcs\.modified=true'
+
 
 # Remove 'git' from $PATH. The test should still build.
 # This ensures that we aren't loading VCS metadata that
@@ -26,11 +30,11 @@ go test -c -o $devnull .
 
 # When listing a main package, in general we need its VCS metadata to determine
 # the .Stale and .StaleReason fields.
-! go list .
+! go list -buildvcs=true .
 stderr '^go: missing Git command\. See https://golang\.org/s/gogetcmd\nerror obtaining VCS status: .*\n\tUse -buildvcs=false to disable VCS stamping.'
 
 # Adding the -test flag should be strictly additive — it should not suppress the error.
-! go list -test .
+! go list -buildvcs=true -test .
 stderr '^go: missing Git command\. See https://golang\.org/s/gogetcmd\nerror obtaining VCS status: .*\n\tUse -buildvcs=false to disable VCS stamping.'
 
 # Adding the suggested flag should suppress the error.
@@ -38,13 +42,19 @@ go list -test -buildvcs=false .
 ! stderr .
 
 
-# Since the ./testonly package can't produce an actual binary, we shouldn't
-# invoke a VCS tool to compute a build stamp when listing it.
+# Since the ./testonly package doesn't itself produce an actual binary, we shouldn't
+# invoke a VCS tool to compute a build stamp by default when listing it.
 go list ./testonly
 ! stderr .
 go list -test ./testonly
 ! stderr .
 
+# Again, setting -buildvcs explicitly should force the use of the VCS tool.
+! go list -buildvcs ./testonly
+stderr '^go: missing Git command\. See https://golang\.org/s/gogetcmd\nerror obtaining VCS status: .*\n\tUse -buildvcs=false to disable VCS stamping.'
+! go list -buildvcs -test ./testonly
+stderr '^go: missing Git command\. See https://golang\.org/s/gogetcmd\nerror obtaining VCS status: .*\n\tUse -buildvcs=false to disable VCS stamping.'
+
 
 -- go.mod --
 module example