Skip to content

Commit 24fcbb9

Browse files
Bryan C. Millsdmitshur
Bryan C. Mills
authored andcommitted
[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 4569fe6) 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>
1 parent 0b0d2fe commit 24fcbb9

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
@@ -24,9 +24,9 @@ import (
2424

2525
// These are general "build flags" used by build and other commands.
2626
var (
27-
BuildA bool // -a flag
28-
BuildBuildmode string // -buildmode flag
29-
BuildBuildvcs bool // -buildvcs flag
27+
BuildA bool // -a flag
28+
BuildBuildmode string // -buildmode flag
29+
BuildBuildvcs = "auto" // -buildvcs flag: "true", "false", or "auto"
3030
BuildContext = defaultContext()
3131
BuildMod string // -mod flag
3232
BuildModExplicit bool // whether -mod was set explicitly

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,7 @@ func runList(ctx context.Context, cmd *base.Command, args []string) {
503503
pkgOpts := load.PackageOpts{
504504
IgnoreImports: *listFind,
505505
ModResolveTests: *listTest,
506-
LoadVCS: cfg.BuildBuildvcs,
506+
LoadVCS: true,
507507
}
508508
pkgs := load.PackagesAndErrors(ctx, pkgOpts, args)
509509
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 ||
@@ -2375,7 +2376,7 @@ func (p *Package) setBuildInfo(includeVCS bool) {
23752376
var vcsCmd *vcs.Cmd
23762377
var err error
23772378
const allowNesting = true
2378-
if includeVCS && p.Module != nil && p.Module.Version == "" && !p.Standard && !p.IsTestOnly() {
2379+
if includeVCS && cfg.BuildBuildvcs != "false" && p.Module != nil && p.Module.Version == "" && !p.Standard && !p.IsTestOnly() {
23792380
repoDir, vcsCmd, err = vcs.FromDir(base.Cwd(), "", allowNesting)
23802381
if err != nil && !errors.Is(err, os.ErrNotExist) {
23812382
setVCSError(err)
@@ -2387,7 +2388,14 @@ func (p *Package) setBuildInfo(includeVCS bool) {
23872388
// repository containing the working directory. Don't include VCS info.
23882389
// If the repo contains the module or vice versa, but they are not
23892390
// the same directory, it's likely an error (see below).
2390-
repoDir, vcsCmd = "", nil
2391+
goto omitVCS
2392+
}
2393+
if cfg.BuildBuildvcs == "auto" && vcsCmd != nil && vcsCmd.Cmd != "" {
2394+
if _, err := exec.LookPath(vcsCmd.Cmd); err != nil {
2395+
// We fould a repository, but the required VCS tool is not present.
2396+
// "-buildvcs=auto" means that we should silently drop the VCS metadata.
2397+
goto omitVCS
2398+
}
23912399
}
23922400
}
23932401
if repoDir != "" && vcsCmd.Status != nil {
@@ -2401,17 +2409,23 @@ func (p *Package) setBuildInfo(includeVCS bool) {
24012409
return
24022410
}
24032411
if pkgRepoDir != repoDir {
2404-
setVCSError(fmt.Errorf("main package is in repository %q but current directory is in repository %q", pkgRepoDir, repoDir))
2405-
return
2412+
if cfg.BuildBuildvcs != "auto" {
2413+
setVCSError(fmt.Errorf("main package is in repository %q but current directory is in repository %q", pkgRepoDir, repoDir))
2414+
return
2415+
}
2416+
goto omitVCS
24062417
}
24072418
modRepoDir, _, err := vcs.FromDir(p.Module.Dir, "", allowNesting)
24082419
if err != nil {
24092420
setVCSError(err)
24102421
return
24112422
}
24122423
if modRepoDir != repoDir {
2413-
setVCSError(fmt.Errorf("main module is in repository %q but current directory is in repository %q", modRepoDir, repoDir))
2414-
return
2424+
if cfg.BuildBuildvcs != "auto" {
2425+
setVCSError(fmt.Errorf("main module is in repository %q but current directory is in repository %q", modRepoDir, repoDir))
2426+
return
2427+
}
2428+
goto omitVCS
24152429
}
24162430

24172431
type vcsStatusError struct {
@@ -2438,6 +2452,7 @@ func (p *Package) setBuildInfo(includeVCS bool) {
24382452
}
24392453
appendSetting("vcs.modified", strconv.FormatBool(st.Uncommitted))
24402454
}
2455+
omitVCS:
24412456

24422457
p.Internal.BuildInfo = info.String()
24432458
}

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
@@ -603,7 +629,7 @@ func runInstall(ctx context.Context, cmd *base.Command, args []string) {
603629

604630
modload.InitWorkfile()
605631
BuildInit()
606-
pkgs := load.PackagesAndErrors(ctx, load.PackageOpts{LoadVCS: cfg.BuildBuildvcs}, args)
632+
pkgs := load.PackagesAndErrors(ctx, load.PackageOpts{LoadVCS: true}, args)
607633
if cfg.ModulesEnabled && !modload.HasModRoot() {
608634
haveErrors := false
609635
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)