From: Bryan C. Mills Date: Fri, 27 May 2022 20:33:36 +0000 (-0400) Subject: cmd/go: stamp VCS information in test binaries when -buildvcs=true X-Git-Tag: go1.20rc1~1616 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=805305e24f762887a10eb5c410683aa541d1b3c3;p=gostls13.git cmd/go: stamp VCS information in test binaries when -buildvcs=true (But still not when -buildvcs=auto, the default.) Fixes #52648. Change-Id: I87a87d4ea84e8bf9635a4f7c8674c9311c3e21be Reviewed-on: https://go-review.googlesource.com/c/go/+/409177 TryBot-Result: Gopher Robot Reviewed-by: Ian Lance Taylor Auto-Submit: Bryan Mills Run-TryBot: Bryan Mills --- diff --git a/src/cmd/go/internal/list/list.go b/src/cmd/go/internal/list/list.go index 5f8be6e3c9..2e3614d317 100644 --- a/src/cmd/go/internal/list/list.go +++ b/src/cmd/go/internal/list/list.go @@ -592,7 +592,7 @@ func runList(ctx context.Context, cmd *base.Command, args []string) { pkgOpts := load.PackageOpts{ IgnoreImports: *listFind, ModResolveTests: *listTest, - LoadVCS: true, + AutoVCS: true, // SuppressDeps is set if the user opts to explicitly ask for the json fields they // need, don't ask for Deps or DepsErrors. It's not set when using a template string, // even if *listFmt doesn't contain .Deps because Deps are used to find import cycles diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go index 046f508545..2cd61b9dcb 100644 --- a/src/cmd/go/internal/load/pkg.go +++ b/src/cmd/go/internal/load/pkg.go @@ -1974,7 +1974,7 @@ func (p *Package) load(ctx context.Context, opts PackageOpts, path string, stk * // Consider starting this as a background goroutine and retrieving the result // asynchronously when we're actually ready to build the package, or when we // actually need to evaluate whether the package's metadata is stale. - p.setBuildInfo(opts.LoadVCS) + p.setBuildInfo(opts.AutoVCS) } // unsafe is a fake package. @@ -2264,7 +2264,7 @@ var vcsStatusCache par.Cache // // Note that the GoVersion field is not set here to avoid encoding it twice. // It is stored separately in the binary, mostly for historical reasons. -func (p *Package) setBuildInfo(includeVCS bool) { +func (p *Package) setBuildInfo(autoVCS bool) { setPkgErrorf := func(format string, args ...any) { if p.Error == nil { p.Error = &PackageError{Err: fmt.Errorf(format, args...)} @@ -2420,7 +2420,19 @@ func (p *Package) setBuildInfo(includeVCS bool) { var vcsCmd *vcs.Cmd var err error const allowNesting = true - if includeVCS && cfg.BuildBuildvcs != "false" && p.Module != nil && p.Module.Version == "" && !p.Standard && !p.IsTestOnly() { + + wantVCS := false + switch cfg.BuildBuildvcs { + case "true": + wantVCS = true // Include VCS metadata even for tests if requested explicitly; see https://go.dev/issue/52648. + case "auto": + wantVCS = autoVCS && !p.IsTestOnly() + case "false": + default: + panic(fmt.Sprintf("unexpected value for cfg.BuildBuildvcs: %q", cfg.BuildBuildvcs)) + } + + if wantVCS && p.Module != nil && p.Module.Version == "" && !p.Standard { repoDir, vcsCmd, err = vcs.FromDir(base.Cwd(), "", allowNesting) if err != nil && !errors.Is(err, os.ErrNotExist) { setVCSError(err) @@ -2724,8 +2736,9 @@ type PackageOpts struct { // may be printed for non-literal arguments that match no main packages. MainOnly bool - // LoadVCS controls whether we also load version-control metadata for main packages. - LoadVCS bool + // AutoVCS controls whether we also load version-control metadata for main packages + // when -buildvcs=auto (the default). + AutoVCS bool // SuppressDeps is true if the caller does not need Deps and DepsErrors to be populated // on the package. TestPackagesAndErrors examines the Deps field to determine if the test diff --git a/src/cmd/go/internal/work/build.go b/src/cmd/go/internal/work/build.go index 42745d9928..352e46b48f 100644 --- a/src/cmd/go/internal/work/build.go +++ b/src/cmd/go/internal/work/build.go @@ -406,7 +406,7 @@ func runBuild(ctx context.Context, cmd *base.Command, args []string) { var b Builder b.Init() - pkgs := load.PackagesAndErrors(ctx, load.PackageOpts{LoadVCS: true}, args) + pkgs := load.PackagesAndErrors(ctx, load.PackageOpts{AutoVCS: true}, args) load.CheckPackageErrors(pkgs) explicitO := len(cfg.BuildO) > 0 @@ -636,7 +636,7 @@ func runInstall(ctx context.Context, cmd *base.Command, args []string) { modload.InitWorkfile() BuildInit() - pkgs := load.PackagesAndErrors(ctx, load.PackageOpts{LoadVCS: true}, args) + pkgs := load.PackagesAndErrors(ctx, load.PackageOpts{AutoVCS: true}, args) if cfg.ModulesEnabled && !modload.HasModRoot() { haveErrors := false allMissingErrors := true diff --git a/src/cmd/go/testdata/script/test_buildvcs.txt b/src/cmd/go/testdata/script/test_buildvcs.txt index a669966036..965f76bf0d 100644 --- a/src/cmd/go/testdata/script/test_buildvcs.txt +++ b/src/cmd/go/testdata/script/test_buildvcs.txt @@ -5,14 +5,18 @@ [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. +# The test binaries should not have VCS settings stamped by default. # (The test itself verifies that.) go test . ./testonly +# However, setting -buildvcs explicitly should override that and +# stamp anyway (https://go.dev/issue/52648). +go test -buildvcs -c -o ./testonly.exe ./testonly +! exec ./testonly.exe +stdout 'unexpected VCS setting: vcs\.modified=true' + # Remove 'git' from $PATH. The test should still build. # This ensures that we aren't loading VCS metadata that @@ -26,11 +30,11 @@ go test -c -o $devnull . # When listing a main package, in general we need its VCS metadata to determine # the .Stale and .StaleReason fields. -! go list . +! go list -buildvcs=true . stderr '^go: missing Git command\. See https://golang\.org/s/gogetcmd\nerror obtaining VCS status: .*\n\tUse -buildvcs=false to disable VCS stamping.' # Adding the -test flag should be strictly additive — it should not suppress the error. -! go list -test . +! go list -buildvcs=true -test . stderr '^go: missing Git command\. See https://golang\.org/s/gogetcmd\nerror obtaining VCS status: .*\n\tUse -buildvcs=false to disable VCS stamping.' # Adding the suggested flag should suppress the error. @@ -38,13 +42,19 @@ go list -test -buildvcs=false . ! stderr . -# Since the ./testonly package can't produce an actual binary, we shouldn't -# invoke a VCS tool to compute a build stamp when listing it. +# Since the ./testonly package doesn't itself produce an actual binary, we shouldn't +# invoke a VCS tool to compute a build stamp by default when listing it. go list ./testonly ! stderr . go list -test ./testonly ! stderr . +# Again, setting -buildvcs explicitly should force the use of the VCS tool. +! go list -buildvcs ./testonly +stderr '^go: missing Git command\. See https://golang\.org/s/gogetcmd\nerror obtaining VCS status: .*\n\tUse -buildvcs=false to disable VCS stamping.' +! go list -buildvcs -test ./testonly +stderr '^go: missing Git command\. See https://golang\.org/s/gogetcmd\nerror obtaining VCS status: .*\n\tUse -buildvcs=false to disable VCS stamping.' + -- go.mod -- module example