]> Cypherpunks repositories - gostls13.git/commitdiff
[release-branch.go1.18] cmd/go: allow '-buildvcs=auto' and treat it as the default
authorBryan C. Mills <bcmills@google.com>
Thu, 7 Apr 2022 21:25:23 +0000 (17:25 -0400)
committerDmitri Shuralyov <dmitshur@golang.org>
Wed, 20 Apr 2022 16:33:28 +0000 (16:33 +0000)
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>
src/cmd/go/alldocs.go
src/cmd/go/internal/cfg/cfg.go
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/build_buildvcs_auto.txt [new file with mode: 0644]
src/cmd/go/testdata/script/test_buildvcs.txt
src/cmd/go/testdata/script/version_buildvcs_nested.txt

index 420529b1a96ac7e6fa86c8d59ccefed29ef25a2f..1c5dbfc7584decb36bc7f1584e5f93c0f6afa766 100644 (file)
 //     -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'
index deab3dddd0dbeaa94e136422d750a8a42bd2c03e..7d1d1f379f0ea845127be86eba7eaeae78ffcb9b 100644 (file)
@@ -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
index eabf73ca4348c217ed2c0098ff97f56512c990ba..62c18866c4c65287939d5e08ac65116e382a8b98 100644 (file)
@@ -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 {
index 1a628475ca668c6ea074195463696e3e544db44a..438cc350859feed8201b6a902c3f28a6241a231e 100644 (file)
@@ -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()
 }
index 6e5e155cb955bf2befbbb943d5b43d4d6cb50fd8..394fe9105003c7b149439b19b0f5188b1bfddfc1 100644 (file)
@@ -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 (file)
index 0000000..9eac568
--- /dev/null
@@ -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() {}
index a0689195e8111fc975db7007815d2a671601a353..a6699660364581033d2ca8bd3d3ad412db02857e 100644 (file)
@@ -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.
index 08d4c92bafb697ef45f8435cc4e53b75f99b8ad1..a0c69f9c12acfdf36bdf40898dba0f668eb81025 100644 (file)
@@ -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