[go] cmd/go: allow '-buildvcs=auto' and treat it as the default
855 views
Skip to first unread message
Bryan Mills (Gerrit)
Apr 7, 2022, 5:33:37 PM4/7/22
to goph...@pubsubhelper.golang.org, Bryan Mills, golang-co...@googlegroups.com
Bryan Mills has uploaded this change for review.
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 #46693.
Change-Id: Iebc955c2af0abca9b7517f62ca48b1d944eb2df4
---
M src/cmd/go/internal/cfg/cfg.go
M src/cmd/go/internal/list/list.go
M src/cmd/go/internal/load/pkg.go
M src/cmd/go/internal/work/build.go
A src/cmd/go/testdata/script/build_buildvcs_auto.txt
M src/cmd/go/testdata/script/test_buildvcs.txt
6 files changed, 126 insertions(+), 8 deletions(-)
diff --git a/src/cmd/go/internal/cfg/cfg.go b/src/cmd/go/internal/cfg/cfg.go
index a11a1a7..c6ddfe5 100644
--- a/src/cmd/go/internal/cfg/cfg.go
+++ b/src/cmd/go/internal/cfg/cfg.go
@@ -44,9 +44,9 @@
// 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 e039b9f..17864e1 100644
--- a/src/cmd/go/internal/list/list.go
+++ b/src/cmd/go/internal/list/list.go
@@ -567,7 +567,7 @@
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 2592cf5..866164f 100644
--- a/src/cmd/go/internal/load/pkg.go
+++ b/src/cmd/go/internal/load/pkg.go
@@ -17,6 +17,7 @@
"internal/goroot"
"io/fs"
"os"
+ "os/exec"
"path"
pathpkg "path"
"path/filepath"
@@ -2371,7 +2372,7 @@
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)
@@ -2385,6 +2386,13 @@
// the same directory, it's likely an error (see below).
repoDir, vcsCmd = "", nil
}
+ 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.
+ repoDir, vcsCmd = "", nil
+ }
+ }
}
if repoDir != "" && vcsCmd.Status != nil {
// Check that the current directory, package, and module are in the same
diff --git a/src/cmd/go/internal/work/build.go b/src/cmd/go/internal/work/build.go
index 42f052d..3339ad8 100644
--- a/src/cmd/go/internal/work/build.go
+++ b/src/cmd/go/internal/work/build.go
@@ -13,6 +13,7 @@
"os"
"path/filepath"
"runtime"
+ "strconv"
"strings"
"cmd/go/internal/base"
@@ -302,7 +303,7 @@
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 +333,29 @@
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 +403,7 @@
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 +627,7 @@
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 0000000..20cc3ec
--- /dev/null
+++ b/src/cmd/go/testdata/script/build_buildvcs_auto.txt
@@ -0,0 +1,55 @@
+# 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
+
+exec git init
+exec git add go.mod *.go
+exec git commit -m 'initial state'
+
+# Control case: with a git binary in $PATH,
+# 'go build' 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$'
+
+# 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
+
+import "fmt"
+
+func main() {
+ fmt.Println("Hello, world!")
+}
diff --git a/src/cmd/go/testdata/script/test_buildvcs.txt b/src/cmd/go/testdata/script/test_buildvcs.txt
index a068919..a669966 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.
To view, visit change 398855. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Apr 7, 2022, 5:34:26 PM4/7/22
to Bryan Mills, goph...@pubsubhelper.golang.org, Michael Matloob, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Russ Cox, Michael Matloob.
Patch set 1:Run-TryBot +1Auto-Submit +1
1 comment:
Patchset:
TRY=longtest,windows-amd64-longtest
To view, visit change 398855. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Apr 7, 2022, 5:52:28 PM4/7/22
to Bryan Mills, goph...@pubsubhelper.golang.org, Gopher Robot, Michael Matloob, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Russ Cox, Michael Matloob.
Bryan Mills removed a vote from this change.
Removed Auto-Submit+1 by Bryan Mills <bcm...@google.com>
To view, visit change 398855. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Apr 7, 2022, 5:52:55 PM4/7/22
to Bryan Mills, goph...@pubsubhelper.golang.org, Gopher Robot, Michael Matloob, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Russ Cox, Michael Matloob.
Patch set 1:Auto-Submit +1
1 comment:
Patchset:
This change needs a doc update for the flag.
To view, visit change 398855. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Apr 7, 2022, 5:53:00 PM4/7/22
to Bryan Mills, goph...@pubsubhelper.golang.org, Gopher Robot, Michael Matloob, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Russ Cox, Michael Matloob.
Patch set 1:-Auto-Submit
Bryan Mills (Gerrit)
Apr 8, 2022, 5:09:53 PM4/8/22
to Bryan Mills, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Russ Cox, Michael Matloob.
Bryan Mills uploaded patch set #2 to this change.
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 #46693.
Updates #51999.
Change-Id: Iebc955c2af0abca9b7517f62ca48b1d944eb2df4
---
M src/cmd/go/internal/cfg/cfg.go
M src/cmd/go/internal/list/list.go
M src/cmd/go/internal/load/pkg.go
M src/cmd/go/internal/work/build.go
A src/cmd/go/testdata/script/build_buildvcs_auto.txt
M src/cmd/go/testdata/script/test_buildvcs.txt
6 files changed, 171 insertions(+), 13 deletions(-)
To view, visit change 398855. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Apr 8, 2022, 5:10:01 PM4/8/22
to Bryan Mills, goph...@pubsubhelper.golang.org, Hyang-Ah Hana Kim, Gopher Robot, Michael Matloob, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Russ Cox, Michael Matloob.
Patch set 1:Run-TryBot +1Trust +1
Bryan Mills (Gerrit)
Apr 8, 2022, 5:18:18 PM4/8/22
to Bryan Mills, goph...@pubsubhelper.golang.org, Hyang-Ah Hana Kim, Gopher Robot, Michael Matloob, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Russ Cox, Michael Matloob.
1 comment:
Patchset:
This change needs a doc update for the flag.
Done
To view, visit change 398855. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Apr 8, 2022, 5:18:19 PM4/8/22
to Bryan Mills, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Russ Cox, Michael Matloob.
Bryan Mills uploaded patch set #3 to this change.
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 #46693.
Updates #51999.
Change-Id: Iebc955c2af0abca9b7517f62ca48b1d944eb2df4
---
M src/cmd/go/alldocs.go
M src/cmd/go/internal/cfg/cfg.go
M src/cmd/go/internal/list/list.go
M src/cmd/go/internal/load/pkg.go
M src/cmd/go/internal/work/build.go
A src/cmd/go/testdata/script/build_buildvcs_auto.txt
M src/cmd/go/testdata/script/test_buildvcs.txt
7 files changed, 185 insertions(+), 23 deletions(-)
To view, visit change 398855. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Apr 8, 2022, 5:34:50 PM4/8/22
to Bryan Mills, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Bryan Mills, Russ Cox, Michael Matloob.
Bryan Mills uploaded patch set #4 to this change.
M src/cmd/go/testdata/script/version_buildvcs_nested.txt
8 files changed, 186 insertions(+), 24 deletions(-)
To view, visit change 398855. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Apr 11, 2022, 5:42:34 PM4/11/22
to Bryan Mills, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Bryan Mills, Russ Cox, Michael Matloob.
Bryan Mills uploaded patch set #5 to this change.
Bryan Mills (Gerrit)
Apr 12, 2022, 12:34:20 PM4/12/22
to Bryan Mills, goph...@pubsubhelper.golang.org, Gopher Robot, Hyang-Ah Hana Kim, Michael Matloob, Russ Cox, golang-co...@googlegroups.com
Commit Message:
Fixes #46693.
Updates #51999.These references are swapped.
To view, visit change 398855. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Apr 12, 2022, 12:34:51 PM4/12/22
to Bryan Mills, goph...@pubsubhelper.golang.org, Gopher Robot, Hyang-Ah Hana Kim, Michael Matloob, Russ Cox, golang-co...@googlegroups.com
Commit Message:
Patch Set #5, Line 27:
46693#51748
To view, visit change 398855. To unsubscribe, or for help writing mail filters, visit settings.
Michael Matloob (Gerrit)
Apr 12, 2022, 2:33:37 PM4/12/22
to Bryan Mills, goph...@pubsubhelper.golang.org, Russ Cox, Gopher Robot, Hyang-Ah Hana Kim, golang-co...@googlegroups.com
Attention is currently required from: Bryan Mills.
Patch set 5:Code-Review +2
Bryan Mills (Gerrit)
Apr 12, 2022, 4:09:28 PM4/12/22
to Bryan Mills, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Bryan Mills uploaded patch set #6 to this change.
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
---
M src/cmd/go/alldocs.go
M src/cmd/go/internal/cfg/cfg.go
M src/cmd/go/internal/list/list.go
M src/cmd/go/internal/load/pkg.go
M src/cmd/go/internal/work/build.go
A src/cmd/go/testdata/script/build_buildvcs_auto.txt
M src/cmd/go/testdata/script/test_buildvcs.txt
M src/cmd/go/testdata/script/version_buildvcs_nested.txt
8 files changed, 189 insertions(+), 27 deletions(-)
To view, visit change 398855. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Apr 12, 2022, 4:09:29 PM4/12/22
to Bryan Mills, goph...@pubsubhelper.golang.org, Michael Matloob, Russ Cox, Gopher Robot, Hyang-Ah Hana Kim, golang-co...@googlegroups.com
2 comments:
Commit Message:
Patch Set #5, Line 27:
46693#51748
Done
Fixes #46693.
Updates #51999.These references are swapped.
Done
To view, visit change 398855. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Apr 12, 2022, 4:16:07 PM4/12/22
to Bryan Mills, goph...@pubsubhelper.golang.org, Michael Matloob, Russ Cox, Gopher Robot, Hyang-Ah Hana Kim, golang-co...@googlegroups.com
Patch set 6:Auto-Submit +1
Gopher Robot (Gerrit)
Apr 12, 2022, 4:22:50 PM4/12/22
to Bryan Mills, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Michael Matloob, Russ Cox, Hyang-Ah Hana Kim, golang-co...@googlegroups.com
Gopher Robot submitted this change.
5 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: src/cmd/go/internal/load/pkg.go
Insertions: 3, Deletions: 3.
@@ -197,9 +197,9 @@
// 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 ||
@@ -2057,7 +2057,8 @@
// then there may be other things lying around, like symbolic links or .git directories.)
var list []string
for _, file := range match {
- rel := filepath.ToSlash(file[len(pkgdir)+1:]) // file, relative to p.Dir
+ // relative path to p.Dir which begins without prefix slash
+ rel := filepath.ToSlash(str.TrimFilePathPrefix(file, pkgdir))
what := "file"
info, err := fsys.Lstat(file)
@@ -2107,7 +2108,7 @@
if err != nil {
return err
}
- rel := filepath.ToSlash(path[len(pkgdir)+1:])
+ rel := filepath.ToSlash(str.TrimFilePathPrefix(path, pkgdir))
name := info.Name()
if path != file && (isBadEmbedName(name) || ((name[0] == '.' || name[0] == '_') && !all)) {
// Ignore bad names, assuming they won't go into modules.
```
Approvals:
Russ Cox: Looks good to me, but someone else must approve
Michael Matloob: Looks good to me, approved
Bryan Mills: Run TryBots; Automatically submit change
Gopher Robot: TryBots succeeded
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 <bcm...@google.com>
Reviewed-by: Russ Cox <r...@golang.org>
Reviewed-by: Michael Matloob <mat...@golang.org>
Auto-Submit: Bryan Mills <bcm...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
---
M src/cmd/go/alldocs.go
M src/cmd/go/internal/cfg/cfg.go
M src/cmd/go/internal/list/list.go
M src/cmd/go/internal/load/pkg.go
M src/cmd/go/internal/work/build.go
A src/cmd/go/testdata/script/build_buildvcs_auto.txt
M src/cmd/go/testdata/script/test_buildvcs.txt
M src/cmd/go/testdata/script/version_buildvcs_nested.txt
8 files changed, 195 insertions(+), 27 deletions(-)
diff --git a/src/cmd/go/alldocs.go b/src/cmd/go/alldocs.go
index 586bc1a..6fdb4f9 100644
--- a/src/cmd/go/alldocs.go
+++ b/src/cmd/go/alldocs.go
@@ -136,11 +136,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'
index 10a980f..e43117f 100644
--- a/src/cmd/go/internal/load/pkg.go
+++ b/src/cmd/go/internal/load/pkg.go
@@ -17,6 +17,7 @@
"internal/goroot"
"io/fs"
"os"
+ "os/exec"
"path"
pathpkg "path"
"path/filepath"
@@ -196,9 +197,9 @@
// 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 ||
@@ -2372,7 +2373,7 @@
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)
@@ -2384,7 +2385,14 @@
// 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 {
@@ -2398,8 +2406,11 @@
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 {
@@ -2407,8 +2418,11 @@
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 {
@@ -2435,6 +2449,7 @@
}
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 e63e209..e9a8ee6 100644
--- a/src/cmd/go/internal/work/build.go
+++ b/src/cmd/go/internal/work/build.go
@@ -13,6 +13,7 @@
"os"
"path/filepath"
"runtime"
+ "strconv"
"strings"
"cmd/go/internal/base"
@@ -91,11 +92,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'
@@ -302,7 +305,7 @@
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 @@@@ -379,7 +405,7 @@
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
@@ -609,7 +635,7 @@
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 0000000..9eac568
--- /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 ..
+
+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 a068919..a669966 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 08d4c92..a0c69f9 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
To view, visit change 398855. To unsubscribe, or for help writing mail filters, visit settings.
Dmitri Shuralyov (Gerrit)
Apr 14, 2022, 6:08:05 PM4/14/22
to Bryan Mills, Dmitri Shuralyov, goph...@pubsubhelper.golang.org, Michael Matloob, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Bryan Mills.
Patch set 2:Run-TryBot +1
To view, visit change 400454. To unsubscribe, or for help writing mail filters, visit settings.
Dmitri Shuralyov (Gerrit)
Apr 14, 2022, 6:08:25 PM4/14/22
to Bryan Mills, Dmitri Shuralyov, goph...@pubsubhelper.golang.org, Gopher Robot, Michael Matloob, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Bryan Mills.
Patch set 2:Code-Review +1
Bryan Mills (Gerrit)
Apr 18, 2022, 10:32:12 AM4/18/22
to Bryan Mills, Dmitri Shuralyov, goph...@pubsubhelper.golang.org, Gopher Robot, Dmitri Shuralyov, Michael Matloob, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Dmitri Shuralyov.
Patch set 2:Code-Review +2
Dmitri Shuralyov (Gerrit)
Apr 20, 2022, 12:33:34 PM4/20/22
to Bryan Mills, Dmitri Shuralyov, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Gopher Robot, Dmitri Shuralyov, Michael Matloob, Russ Cox, golang-co...@googlegroups.com
Dmitri Shuralyov submitted this change.
Approvals:
Bryan Mills: Looks good to me, approved
Dmitri Shuralyov: Looks good to me, but someone else must approve
Dmitri Shuralyov: Run TryBots
Gopher Robot: TryBots succeeded
[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 <bcm...@google.com>
Reviewed-by: Russ Cox <r...@golang.org>
Reviewed-by: Michael Matloob <mat...@golang.org>
Auto-Submit: Bryan Mills <bcm...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
(cherry picked from commit 4569fe64101c2209e3429bd1c953b5f4021fc43d)
Reviewed-on: https://go-review.googlesource.com/c/go/+/400454
Run-TryBot: Dmitri Shuralyov <dmit...@golang.org>
Reviewed-by: Dmitri Shuralyov <dmit...@google.com>
Reviewed-by: Bryan Mills <bcm...@google.com>
---
M src/cmd/go/alldocs.go
M src/cmd/go/internal/cfg/cfg.go
M src/cmd/go/internal/list/list.go
M src/cmd/go/internal/load/pkg.go
M src/cmd/go/internal/work/build.go
A src/cmd/go/testdata/script/build_buildvcs_auto.txt
M src/cmd/go/testdata/script/test_buildvcs.txt
M src/cmd/go/testdata/script/version_buildvcs_nested.txt
8 files changed, 201 insertions(+), 27 deletions(-)
diff --git a/src/cmd/go/alldocs.go b/src/cmd/go/alldocs.go
index 420529b..1c5dbfc 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 deab3dd..7d1d1f3 100644
--- a/src/cmd/go/internal/cfg/cfg.go
+++ b/src/cmd/go/internal/cfg/cfg.go
@@ -24,9 +24,9 @@
// 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 eabf73c..62c1886 100644
--- a/src/cmd/go/internal/list/list.go
+++ b/src/cmd/go/internal/list/list.go
@@ -503,7 +503,7 @@
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 1a62847..438cc35 100644
--- a/src/cmd/go/internal/load/pkg.go
+++ b/src/cmd/go/internal/load/pkg.go
@@ -17,6 +17,7 @@
"internal/goroot"
"io/fs"
"os"
+ "os/exec"
"path"
pathpkg "path"
"path/filepath"
@@ -196,9 +197,9 @@
// 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 @@
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 @@
// 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 @@
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 @@
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 @@
}
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 6e5e155..394fe91 100644@@ -603,7 +629,7 @@To view, visit change 400454. To unsubscribe, or for help writing mail filters, visit settings.
Reply all
Reply to author
Forward
0 new messages