cmd/go: stamp VCS information in test binaries when -buildvcs=true

(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 <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Auto-Submit: Bryan Mills <[email protected]>
Run-TryBot: Bryan Mills <[email protected]>

Author: Bryan C. Mills

src/cmd/go/internal/list/list.go

@@ -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

src/cmd/go/internal/load/pkg.go

@@ -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

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

@@ -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

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

@@ -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,25 +30,31 @@ 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.
 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