[release-branch.go1.18] cmd/go: allow '-buildvcs=auto' and treat it as the default

When we added VCS stamping in the Go 1.18 release, we defaulted to
-buildvcs=true, on the theory that most folks will actually want VCS
information stamped.

We also made -buildvcs=true error out if a VCS directory is found and
no VCS tool is available, on the theory that a user who builds with
'-buildvcs=true' will be very surprised if the VCS metadata is
silently missing.

However, that causes a problem for CI environments that don't have the
appropriate VCS tool installed. (And we know that's a common situation
because we're in that situation ourselves — see #46693!)

The new '-buildvcs=auto' setting provides a middle ground: it stamps
VCS information by default when the tool is present (and reports
explicit errors if the tool errors out), but omits the metadata
when the tool isn't present at all.

Updates #51748.
Updates #51999.
Fixes #51798.

Change-Id: Iebc955c2af0abca9b7517f62ca48b1d944eb2df4
Reviewed-on: https://go-review.googlesource.com/c/go/+/398855
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit 4569fe64101c2209e3429bd1c953b5f4021fc43d)
Reviewed-on: https://go-review.googlesource.com/c/go/+/400454
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>

8 files changed, 159 insertions, 27 deletions

diff --git a/src/cmd/go/alldocs.go b/src/cmd/go/alldocs.go
index 420529b1a9..1c5dbfc758 100644
--- a/src/cmd/go/alldocs.go
+++ b/src/cmd/go/alldocs.go
@@ -138,11 +138,13 @@
 // 	-buildmode mode
 // 		build mode to use. See 'go help buildmode' for more.
 // 	-buildvcs
-// 		Whether to stamp binaries with version control information. By default,
-// 		version control information is stamped into a binary if the main package
-// 		and the main module containing it are in the repository containing the
-// 		current directory (if there is a repository). Use -buildvcs=false to
-// 		omit version control information.
+// 		Whether to stamp binaries with version control information
+// 		("true", "false", or "auto"). By default ("auto"), version control
+// 		information is stamped into a binary if the main package, the main module
+// 		containing it, and the current directory are all in the same repository.
+// 		Use -buildvcs=false to always omit version control information, or
+// 		-buildvcs=true to error out if version control information is available but
+// 		cannot be included due to a missing tool or ambiguous directory structure.
 // 	-compiler name
 // 		name of compiler to use, as in runtime.Compiler (gccgo or gc).
 // 	-gccgoflags '[pattern=]arg list'
diff --git a/src/cmd/go/internal/cfg/cfg.go b/src/cmd/go/internal/cfg/cfg.go
index deab3dddd0..7d1d1f379f 100644
--- a/src/cmd/go/internal/cfg/cfg.go
+++ b/src/cmd/go/internal/cfg/cfg.go
@@ -24,9 +24,9 @@ import (
 
 // These are general "build flags" used by build and other commands.
 var (
-	BuildA                 bool   // -a flag
-	BuildBuildmode         string // -buildmode flag
-	BuildBuildvcs          bool   // -buildvcs flag
+	BuildA                 bool     // -a flag
+	BuildBuildmode         string   // -buildmode flag
+	BuildBuildvcs          = "auto" // -buildvcs flag: "true", "false", or "auto"
 	BuildContext           = defaultContext()
 	BuildMod               string                  // -mod flag
 	BuildModExplicit       bool                    // whether -mod was set explicitly
diff --git a/src/cmd/go/internal/list/list.go b/src/cmd/go/internal/list/list.go
index eabf73ca43..62c18866c4 100644
--- a/src/cmd/go/internal/list/list.go
+++ b/src/cmd/go/internal/list/list.go
@@ -503,7 +503,7 @@ func runList(ctx context.Context, cmd *base.Command, args []string) {
 	pkgOpts := load.PackageOpts{
 		IgnoreImports:   *listFind,
 		ModResolveTests: *listTest,
-		LoadVCS:         cfg.BuildBuildvcs,
+		LoadVCS:         true,
 	}
 	pkgs := load.PackagesAndErrors(ctx, pkgOpts, args)
 	if !*listE {
diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go
index 1a628475ca..438cc35085 100644
--- a/src/cmd/go/internal/load/pkg.go
+++ b/src/cmd/go/internal/load/pkg.go
@@ -17,6 +17,7 @@ import (
 	"internal/goroot"
 	"io/fs"
 	"os"
+	"os/exec"
 	"path"
 	pathpkg "path"
 	"path/filepath"
@@ -196,9 +197,9 @@ func (p *Package) Desc() string {
 // IsTestOnly reports whether p is a test-only package.
 //
 // A “test-only” package is one that:
-// 	- is a test-only variant of an ordinary package, or
-// 	- is a synthesized "main" package for a test binary, or
-// 	- contains only _test.go files.
+//   - is a test-only variant of an ordinary package, or
+//   - is a synthesized "main" package for a test binary, or
+//   - contains only _test.go files.
 func (p *Package) IsTestOnly() bool {
 	return p.ForTest != "" ||
 		p.Internal.TestmainGo != nil ||
@@ -2375,7 +2376,7 @@ func (p *Package) setBuildInfo(includeVCS bool) {
 	var vcsCmd *vcs.Cmd
 	var err error
 	const allowNesting = true
-	if includeVCS && p.Module != nil && p.Module.Version == "" && !p.Standard && !p.IsTestOnly() {
+	if includeVCS && cfg.BuildBuildvcs != "false" && p.Module != nil && p.Module.Version == "" && !p.Standard && !p.IsTestOnly() {
 		repoDir, vcsCmd, err = vcs.FromDir(base.Cwd(), "", allowNesting)
 		if err != nil && !errors.Is(err, os.ErrNotExist) {
 			setVCSError(err)
@@ -2387,7 +2388,14 @@ func (p *Package) setBuildInfo(includeVCS bool) {
 			// repository containing the working directory. Don't include VCS info.
 			// If the repo contains the module or vice versa, but they are not
 			// the same directory, it's likely an error (see below).
-			repoDir, vcsCmd = "", nil
+			goto omitVCS
+		}
+		if cfg.BuildBuildvcs == "auto" && vcsCmd != nil && vcsCmd.Cmd != "" {
+			if _, err := exec.LookPath(vcsCmd.Cmd); err != nil {
+				// We fould a repository, but the required VCS tool is not present.
+				// "-buildvcs=auto" means that we should silently drop the VCS metadata.
+				goto omitVCS
+			}
 		}
 	}
 	if repoDir != "" && vcsCmd.Status != nil {
@@ -2401,8 +2409,11 @@ func (p *Package) setBuildInfo(includeVCS bool) {
 			return
 		}
 		if pkgRepoDir != repoDir {
-			setVCSError(fmt.Errorf("main package is in repository %q but current directory is in repository %q", pkgRepoDir, repoDir))
-			return
+			if cfg.BuildBuildvcs != "auto" {
+				setVCSError(fmt.Errorf("main package is in repository %q but current directory is in repository %q", pkgRepoDir, repoDir))
+				return
+			}
+			goto omitVCS
 		}
 		modRepoDir, _, err := vcs.FromDir(p.Module.Dir, "", allowNesting)
 		if err != nil {
@@ -2410,8 +2421,11 @@ func (p *Package) setBuildInfo(includeVCS bool) {
 			return
 		}
 		if modRepoDir != repoDir {
-			setVCSError(fmt.Errorf("main module is in repository %q but current directory is in repository %q", modRepoDir, repoDir))
-			return
+			if cfg.BuildBuildvcs != "auto" {
+				setVCSError(fmt.Errorf("main module is in repository %q but current directory is in repository %q", modRepoDir, repoDir))
+				return
+			}
+			goto omitVCS
 		}
 
 		type vcsStatusError struct {
@@ -2438,6 +2452,7 @@ func (p *Package) setBuildInfo(includeVCS bool) {
 		}
 		appendSetting("vcs.modified", strconv.FormatBool(st.Uncommitted))
 	}
+omitVCS:
 
 	p.Internal.BuildInfo = info.String()
 }
diff --git a/src/cmd/go/internal/work/build.go b/src/cmd/go/internal/work/build.go
index 6e5e155cb9..394fe91050 100644
--- a/src/cmd/go/internal/work/build.go
+++ b/src/cmd/go/internal/work/build.go
@@ -13,6 +13,7 @@ import (
 	"os"
 	"path/filepath"
 	"runtime"
+	"strconv"
 	"strings"
 
 	"cmd/go/internal/base"
@@ -91,11 +92,13 @@ and test commands:
 	-buildmode mode
 		build mode to use. See 'go help buildmode' for more.
 	-buildvcs
-		Whether to stamp binaries with version control information. By default,
-		version control information is stamped into a binary if the main package
-		and the main module containing it are in the repository containing the
-		current directory (if there is a repository). Use -buildvcs=false to
-		omit version control information.
+		Whether to stamp binaries with version control information
+		("true", "false", or "auto"). By default ("auto"), version control
+		information is stamped into a binary if the main package, the main module
+		containing it, and the current directory are all in the same repository.
+		Use -buildvcs=false to always omit version control information, or
+		-buildvcs=true to error out if version control information is available but
+		cannot be included due to a missing tool or ambiguous directory structure.
 	-compiler name
 		name of compiler to use, as in runtime.Compiler (gccgo or gc).
 	-gccgoflags '[pattern=]arg list'
@@ -302,7 +305,7 @@ func AddBuildFlags(cmd *base.Command, mask BuildFlagMask) {
 	cmd.Flag.Var((*base.StringsFlag)(&cfg.BuildToolexec), "toolexec", "")
 	cmd.Flag.BoolVar(&cfg.BuildTrimpath, "trimpath", false, "")
 	cmd.Flag.BoolVar(&cfg.BuildWork, "work", false, "")
-	cmd.Flag.BoolVar(&cfg.BuildBuildvcs, "buildvcs", true, "")
+	cmd.Flag.Var((*buildvcsFlag)(&cfg.BuildBuildvcs), "buildvcs", "")
 
 	// Undocumented, unstable debugging flags.
 	cmd.Flag.StringVar(&cfg.DebugActiongraph, "debug-actiongraph", "", "")
@@ -332,6 +335,29 @@ func (v *tagsFlag) String() string {
 	return "<TagsFlag>"
 }
 
+// buildvcsFlag is the implementation of the -buildvcs flag.
+type buildvcsFlag string
+
+func (f *buildvcsFlag) IsBoolFlag() bool { return true } // allow -buildvcs (without arguments)
+
+func (f *buildvcsFlag) Set(s string) error {
+	// https://go.dev/issue/51748: allow "-buildvcs=auto",
+	// in addition to the usual "true" and "false".
+	if s == "" || s == "auto" {
+		*f = "auto"
+		return nil
+	}
+
+	b, err := strconv.ParseBool(s)
+	if err != nil {
+		return errors.New("value is neither 'auto' nor a valid bool")
+	}
+	*f = (buildvcsFlag)(strconv.FormatBool(b)) // convert to canonical "true" or "false"
+	return nil
+}
+
+func (f *buildvcsFlag) String() string { return string(*f) }
+
 // fileExtSplit expects a filename and returns the name
 // and ext (without the dot). If the file has no
 // extension, ext will be empty.
@@ -379,7 +405,7 @@ func runBuild(ctx context.Context, cmd *base.Command, args []string) {
 	var b Builder
 	b.Init()
 
-	pkgs := load.PackagesAndErrors(ctx, load.PackageOpts{LoadVCS: cfg.BuildBuildvcs}, args)
+	pkgs := load.PackagesAndErrors(ctx, load.PackageOpts{LoadVCS: true}, args)
 	load.CheckPackageErrors(pkgs)
 
 	explicitO := len(cfg.BuildO) > 0
@@ -603,7 +629,7 @@ func runInstall(ctx context.Context, cmd *base.Command, args []string) {
 
 	modload.InitWorkfile()
 	BuildInit()
-	pkgs := load.PackagesAndErrors(ctx, load.PackageOpts{LoadVCS: cfg.BuildBuildvcs}, args)
+	pkgs := load.PackagesAndErrors(ctx, load.PackageOpts{LoadVCS: true}, args)
 	if cfg.ModulesEnabled && !modload.HasModRoot() {
 		haveErrors := false
 		allMissingErrors := true
diff --git a/src/cmd/go/testdata/script/build_buildvcs_auto.txt b/src/cmd/go/testdata/script/build_buildvcs_auto.txt
new file mode 100644
index 0000000000..9eac568045
--- /dev/null
+++ b/src/cmd/go/testdata/script/build_buildvcs_auto.txt
@@ -0,0 +1,87 @@
+# Regression test for https://go.dev/issue/51748: by default, 'go build' should
+# not attempt to stamp VCS information when the VCS tool is not present.
+
+[short] skip
+[!exec:git] skip
+
+cd sub
+exec git init .
+exec git add sub.go
+exec git commit -m 'initial state'
+cd ..
+
+exec git init
+exec git submodule add ./sub
+exec git add go.mod example.go
+exec git commit -m 'initial state'
+
+
+# Control case: with a git binary in $PATH,
+# 'go build' on a package in the same git repo
+# succeeds and stamps VCS metadata by default.
+
+go build -o example.exe .
+go version -m example.exe
+stdout '^\tbuild\tvcs=git$'
+stdout '^\tbuild\tvcs.modified=false$'
+
+
+# Building a binary from a different (nested) VCS repo should not stamp VCS
+# info. It should be an error if VCS stamps are requested explicitly with
+# '-buildvcs' (since we know the VCS metadata exists), but not an error
+# with '-buildvcs=auto'.
+
+go build -o sub.exe ./sub
+go version -m sub.exe
+! stdout '^\tbuild\tvcs'
+
+! go build -buildvcs -o sub.exe ./sub
+stderr '\Aerror obtaining VCS status: main package is in repository ".*" but current directory is in repository ".*"\n\tUse -buildvcs=false to disable VCS stamping.\n\z'
+
+cd ./sub
+go build -o sub.exe .
+go version -m sub.exe
+! stdout '^\tbuild\tvcs'
+
+! go build -buildvcs -o sub.exe .
+stderr '\Aerror obtaining VCS status: main module is in repository ".*" but current directory is in repository ".*"\n\tUse -buildvcs=false to disable VCS stamping.\n\z'
+cd ..
+
+
+# After removing 'git' from $PATH, 'go build -buildvcs' should fail...
+
+env PATH=
+env path=
+! go build -buildvcs -o example.exe .
+stderr 'go: missing Git command\. See https://golang\.org/s/gogetcmd$'
+
+# ...but by default we should omit VCS metadata when the tool is missing.
+
+go build -o example.exe .
+go version -m example.exe
+! stdout '^\tbuild\tvcs'
+
+# The default behavior can be explicitly set with '-buildvcs=auto'.
+
+go build -buildvcs=auto -o example.exe .
+go version -m example.exe
+! stdout '^\tbuild\tvcs'
+
+# Other flag values should be rejected with a useful error message.
+
+! go build -buildvcs=hg -o example.exe .
+stderr '\Ainvalid boolean value "hg" for -buildvcs: value is neither ''auto'' nor a valid bool\nusage: go build .*\nRun ''go help build'' for details.\n\z'
+
+
+-- go.mod --
+module example
+
+go 1.18
+-- example.go --
+package main
+
+func main() {}
+-- sub/sub.go --
+package main
+
+func main() {}
diff --git a/src/cmd/go/testdata/script/test_buildvcs.txt b/src/cmd/go/testdata/script/test_buildvcs.txt
index a0689195e8..a669966036 100644
--- a/src/cmd/go/testdata/script/test_buildvcs.txt
+++ b/src/cmd/go/testdata/script/test_buildvcs.txt
@@ -5,6 +5,8 @@
 [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.
diff --git a/src/cmd/go/testdata/script/version_buildvcs_nested.txt b/src/cmd/go/testdata/script/version_buildvcs_nested.txt
index 08d4c92baf..a0c69f9c12 100644
--- a/src/cmd/go/testdata/script/version_buildvcs_nested.txt
+++ b/src/cmd/go/testdata/script/version_buildvcs_nested.txt
@@ -1,7 +1,7 @@
 [!exec:git] skip
 [!exec:hg] skip
 [short] skip
-env GOFLAGS=-n
+env GOFLAGS='-n -buildvcs'
 
 # Create a root module in a root Git repository.
 mkdir root