Skip to content

Commit 4569fe6

Browse files
Bryan C. Millsgopherbot
Bryan C. Mills
authored andcommitted
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. Fixes #51748. Updates #51999. Change-Id: Iebc955c2af0abca9b7517f62ca48b1d944eb2df4 Reviewed-on: https://go-review.googlesource.com/c/go/+/398855 Run-TryBot: Bryan Mills <[email protected]> Reviewed-by: Russ Cox <[email protected]> Reviewed-by: Michael Matloob <[email protected]> Auto-Submit: Bryan Mills <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
1 parent 6f5590e commit 4569fe6

File tree

8 files changed

+159
-27
lines changed

8 files changed

+159
-27
lines changed

src/cmd/go/alldocs.go

+7-5
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/cmd/go/internal/cfg/cfg.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@ func exeSuffix() string {
4444

4545
// These are general "build flags" used by build and other commands.
4646
var (
47-
BuildA bool // -a flag
48-
BuildBuildmode string // -buildmode flag
49-
BuildBuildvcs bool // -buildvcs flag
47+
BuildA bool // -a flag
48+
BuildBuildmode string // -buildmode flag
49+
BuildBuildvcs = "auto" // -buildvcs flag: "true", "false", or "auto"
5050
BuildContext = defaultContext()
5151
BuildMod string // -mod flag
5252
BuildModExplicit bool // whether -mod was set explicitly

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,7 @@ func runList(ctx context.Context, cmd *base.Command, args []string) {
567567
pkgOpts := load.PackageOpts{
568568
IgnoreImports: *listFind,
569569
ModResolveTests: *listTest,
570-
LoadVCS: cfg.BuildBuildvcs,
570+
LoadVCS: true,
571571
}
572572
pkgs := load.PackagesAndErrors(ctx, pkgOpts, args)
573573
if !*listE {

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

+24-9
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"internal/goroot"
1818
"io/fs"
1919
"os"
20+
"os/exec"
2021
"path"
2122
pathpkg "path"
2223
"path/filepath"
@@ -196,9 +197,9 @@ func (p *Package) Desc() string {
196197
// IsTestOnly reports whether p is a test-only package.
197198
//
198199
// A “test-only” package is one that:
199-
// - is a test-only variant of an ordinary package, or
200-
// - is a synthesized "main" package for a test binary, or
201-
// - contains only _test.go files.
200+
// - is a test-only variant of an ordinary package, or
201+
// - is a synthesized "main" package for a test binary, or
202+
// - contains only _test.go files.
202203
func (p *Package) IsTestOnly() bool {
203204
return p.ForTest != "" ||
204205
p.Internal.TestmainGo != nil ||
@@ -2372,7 +2373,7 @@ func (p *Package) setBuildInfo(includeVCS bool) {
23722373
var vcsCmd *vcs.Cmd
23732374
var err error
23742375
const allowNesting = true
2375-
if includeVCS && p.Module != nil && p.Module.Version == "" && !p.Standard && !p.IsTestOnly() {
2376+
if includeVCS && cfg.BuildBuildvcs != "false" && p.Module != nil && p.Module.Version == "" && !p.Standard && !p.IsTestOnly() {
23762377
repoDir, vcsCmd, err = vcs.FromDir(base.Cwd(), "", allowNesting)
23772378
if err != nil && !errors.Is(err, os.ErrNotExist) {
23782379
setVCSError(err)
@@ -2384,7 +2385,14 @@ func (p *Package) setBuildInfo(includeVCS bool) {
23842385
// repository containing the working directory. Don't include VCS info.
23852386
// If the repo contains the module or vice versa, but they are not
23862387
// the same directory, it's likely an error (see below).
2387-
repoDir, vcsCmd = "", nil
2388+
goto omitVCS
2389+
}
2390+
if cfg.BuildBuildvcs == "auto" && vcsCmd != nil && vcsCmd.Cmd != "" {
2391+
if _, err := exec.LookPath(vcsCmd.Cmd); err != nil {
2392+
// We fould a repository, but the required VCS tool is not present.
2393+
// "-buildvcs=auto" means that we should silently drop the VCS metadata.
2394+
goto omitVCS
2395+
}
23882396
}
23892397
}
23902398
if repoDir != "" && vcsCmd.Status != nil {
@@ -2398,17 +2406,23 @@ func (p *Package) setBuildInfo(includeVCS bool) {
23982406
return
23992407
}
24002408
if pkgRepoDir != repoDir {
2401-
setVCSError(fmt.Errorf("main package is in repository %q but current directory is in repository %q", pkgRepoDir, repoDir))
2402-
return
2409+
if cfg.BuildBuildvcs != "auto" {
2410+
setVCSError(fmt.Errorf("main package is in repository %q but current directory is in repository %q", pkgRepoDir, repoDir))
2411+
return
2412+
}
2413+
goto omitVCS
24032414
}
24042415
modRepoDir, _, err := vcs.FromDir(p.Module.Dir, "", allowNesting)
24052416
if err != nil {
24062417
setVCSError(err)
24072418
return
24082419
}
24092420
if modRepoDir != repoDir {
2410-
setVCSError(fmt.Errorf("main module is in repository %q but current directory is in repository %q", modRepoDir, repoDir))
2411-
return
2421+
if cfg.BuildBuildvcs != "auto" {
2422+
setVCSError(fmt.Errorf("main module is in repository %q but current directory is in repository %q", modRepoDir, repoDir))
2423+
return
2424+
}
2425+
goto omitVCS
24122426
}
24132427

24142428
type vcsStatusError struct {
@@ -2435,6 +2449,7 @@ func (p *Package) setBuildInfo(includeVCS bool) {
24352449
}
24362450
appendSetting("vcs.modified", strconv.FormatBool(st.Uncommitted))
24372451
}
2452+
omitVCS:
24382453

24392454
p.Internal.BuildInfo = info.String()
24402455
}

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

+34-8
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"os"
1414
"path/filepath"
1515
"runtime"
16+
"strconv"
1617
"strings"
1718

1819
"cmd/go/internal/base"
@@ -91,11 +92,13 @@ and test commands:
9192
-buildmode mode
9293
build mode to use. See 'go help buildmode' for more.
9394
-buildvcs
94-
Whether to stamp binaries with version control information. By default,
95-
version control information is stamped into a binary if the main package
96-
and the main module containing it are in the repository containing the
97-
current directory (if there is a repository). Use -buildvcs=false to
98-
omit version control information.
95+
Whether to stamp binaries with version control information
96+
("true", "false", or "auto"). By default ("auto"), version control
97+
information is stamped into a binary if the main package, the main module
98+
containing it, and the current directory are all in the same repository.
99+
Use -buildvcs=false to always omit version control information, or
100+
-buildvcs=true to error out if version control information is available but
101+
cannot be included due to a missing tool or ambiguous directory structure.
99102
-compiler name
100103
name of compiler to use, as in runtime.Compiler (gccgo or gc).
101104
-gccgoflags '[pattern=]arg list'
@@ -302,7 +305,7 @@ func AddBuildFlags(cmd *base.Command, mask BuildFlagMask) {
302305
cmd.Flag.Var((*base.StringsFlag)(&cfg.BuildToolexec), "toolexec", "")
303306
cmd.Flag.BoolVar(&cfg.BuildTrimpath, "trimpath", false, "")
304307
cmd.Flag.BoolVar(&cfg.BuildWork, "work", false, "")
305-
cmd.Flag.BoolVar(&cfg.BuildBuildvcs, "buildvcs", true, "")
308+
cmd.Flag.Var((*buildvcsFlag)(&cfg.BuildBuildvcs), "buildvcs", "")
306309

307310
// Undocumented, unstable debugging flags.
308311
cmd.Flag.StringVar(&cfg.DebugActiongraph, "debug-actiongraph", "", "")
@@ -332,6 +335,29 @@ func (v *tagsFlag) String() string {
332335
return "<TagsFlag>"
333336
}
334337

338+
// buildvcsFlag is the implementation of the -buildvcs flag.
339+
type buildvcsFlag string
340+
341+
func (f *buildvcsFlag) IsBoolFlag() bool { return true } // allow -buildvcs (without arguments)
342+
343+
func (f *buildvcsFlag) Set(s string) error {
344+
// https://go.dev/issue/51748: allow "-buildvcs=auto",
345+
// in addition to the usual "true" and "false".
346+
if s == "" || s == "auto" {
347+
*f = "auto"
348+
return nil
349+
}
350+
351+
b, err := strconv.ParseBool(s)
352+
if err != nil {
353+
return errors.New("value is neither 'auto' nor a valid bool")
354+
}
355+
*f = (buildvcsFlag)(strconv.FormatBool(b)) // convert to canonical "true" or "false"
356+
return nil
357+
}
358+
359+
func (f *buildvcsFlag) String() string { return string(*f) }
360+
335361
// fileExtSplit expects a filename and returns the name
336362
// and ext (without the dot). If the file has no
337363
// extension, ext will be empty.
@@ -379,7 +405,7 @@ func runBuild(ctx context.Context, cmd *base.Command, args []string) {
379405
var b Builder
380406
b.Init()
381407

382-
pkgs := load.PackagesAndErrors(ctx, load.PackageOpts{LoadVCS: cfg.BuildBuildvcs}, args)
408+
pkgs := load.PackagesAndErrors(ctx, load.PackageOpts{LoadVCS: true}, args)
383409
load.CheckPackageErrors(pkgs)
384410

385411
explicitO := len(cfg.BuildO) > 0
@@ -609,7 +635,7 @@ func runInstall(ctx context.Context, cmd *base.Command, args []string) {
609635

610636
modload.InitWorkfile()
611637
BuildInit()
612-
pkgs := load.PackagesAndErrors(ctx, load.PackageOpts{LoadVCS: cfg.BuildBuildvcs}, args)
638+
pkgs := load.PackagesAndErrors(ctx, load.PackageOpts{LoadVCS: true}, args)
613639
if cfg.ModulesEnabled && !modload.HasModRoot() {
614640
haveErrors := false
615641
allMissingErrors := true
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
# Regression test for https://go.dev/issue/51748: by default, 'go build' should
2+
# not attempt to stamp VCS information when the VCS tool is not present.
3+
4+
[short] skip
5+
[!exec:git] skip
6+
7+
cd sub
8+
exec git init .
9+
exec git add sub.go
10+
exec git commit -m 'initial state'
11+
cd ..
12+
13+
exec git init
14+
exec git submodule add ./sub
15+
exec git add go.mod example.go
16+
exec git commit -m 'initial state'
17+
18+
19+
# Control case: with a git binary in $PATH,
20+
# 'go build' on a package in the same git repo
21+
# succeeds and stamps VCS metadata by default.
22+
23+
go build -o example.exe .
24+
go version -m example.exe
25+
stdout '^\tbuild\tvcs=git$'
26+
stdout '^\tbuild\tvcs.modified=false$'
27+
28+
29+
# Building a binary from a different (nested) VCS repo should not stamp VCS
30+
# info. It should be an error if VCS stamps are requested explicitly with
31+
# '-buildvcs' (since we know the VCS metadata exists), but not an error
32+
# with '-buildvcs=auto'.
33+
34+
go build -o sub.exe ./sub
35+
go version -m sub.exe
36+
! stdout '^\tbuild\tvcs'
37+
38+
! go build -buildvcs -o sub.exe ./sub
39+
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'
40+
41+
cd ./sub
42+
go build -o sub.exe .
43+
go version -m sub.exe
44+
! stdout '^\tbuild\tvcs'
45+
46+
! go build -buildvcs -o sub.exe .
47+
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'
48+
cd ..
49+
50+
51+
# After removing 'git' from $PATH, 'go build -buildvcs' should fail...
52+
53+
env PATH=
54+
env path=
55+
! go build -buildvcs -o example.exe .
56+
stderr 'go: missing Git command\. See https://golang\.org/s/gogetcmd$'
57+
58+
# ...but by default we should omit VCS metadata when the tool is missing.
59+
60+
go build -o example.exe .
61+
go version -m example.exe
62+
! stdout '^\tbuild\tvcs'
63+
64+
# The default behavior can be explicitly set with '-buildvcs=auto'.
65+
66+
go build -buildvcs=auto -o example.exe .
67+
go version -m example.exe
68+
! stdout '^\tbuild\tvcs'
69+
70+
# Other flag values should be rejected with a useful error message.
71+
72+
! go build -buildvcs=hg -o example.exe .
73+
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'
74+
75+
76+
-- go.mod --
77+
module example
78+
79+
go 1.18
80+
-- example.go --
81+
package main
82+
83+
func main() {}
84+
-- sub/sub.go --
85+
package main
86+
87+
func main() {}

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

+2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
[short] skip
66
[!exec:git] skip
77

8+
env GOFLAGS=-buildvcs # override default -buildvcs=auto in GOFLAGS, as a user might
9+
810
exec git init
911

1012
# The test binaries should not have VCS settings stamped.

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
[!exec:git] skip
22
[!exec:hg] skip
33
[short] skip
4-
env GOFLAGS=-n
4+
env GOFLAGS='-n -buildvcs'
55

66
# Create a root module in a root Git repository.
77
mkdir root

0 commit comments

Comments
 (0)