From 3afc993266c3f331bc116eeea0fd224a266c64de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Fri, 9 Apr 2021 22:34:41 +0100 Subject: [PATCH] use "go env -json" to collect env info all at once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the worst case scenario, when GOPRIVATE isn't set at all, we would run these three commands: * "go env GOPRIVATE", to fetch GOPRIVATE itself * "go list -m", for GOPRIVATE's fallback * "go version", to check the version of Go being used Now that we support Go 1.16 and later, all these three can be obtained via "go env -json": $ go env -json GOPRIVATE GOMOD GOVERSION { "GOMOD": "/home/mvdan/src/garble/go.mod", "GOPRIVATE": "", "GOVERSION": "go1.16.3" } Note that we don't get the module path directly, but we can use the x/mod/modfile Go API to parse it from the GOMOD file cheaply. Notably, this also simplifies our Go version checking logic, as now we get just the version string without the "go version" prefix and "GOOS/GOARCH" suffix we don't care about. This makes our code a bit more maintainable and robust. When running a short incremental build, we can also see a small speed-up, as saving two "go" invocations can save a few milliseconds: name old time/op new time/op delta Build/Cache-8 168ms ± 0% 166ms ± 1% -1.26% (p=0.009 n=6+6) name old bin-B new bin-B delta Build/Cache-8 6.36M ± 0% 6.36M ± 0% +0.12% (p=0.002 n=6+6) name old sys-time/op new sys-time/op delta Build/Cache-8 222ms ± 2% 219ms ± 3% ~ (p=0.589 n=6+6) name old user-time/op new user-time/op delta Build/Cache-8 857ms ± 1% 846ms ± 1% -1.31% (p=0.041 n=6+6) --- hash.go | 4 +- main.go | 99 ++++++++++++++++++---------------- shared.go | 7 +++ testdata/scripts/goversion.txt | 24 +++++---- 4 files changed, 77 insertions(+), 57 deletions(-) diff --git a/hash.go b/hash.go index be72bc1..8aeaad3 100644 --- a/hash.go +++ b/hash.go @@ -92,8 +92,8 @@ func addGarbleToBuildIDComponent(inputHash []byte) []byte { // We also need to add the selected options to the full version string, // because all of them result in different output. We use spaces to // separate the env vars and flags, to reduce the chances of collisions. - if envGoPrivate != "" { - fmt.Fprintf(h, " GOPRIVATE=%s", envGoPrivate) + if cache.GoEnv.GOPRIVATE != "" { + fmt.Fprintf(h, " GOPRIVATE=%s", cache.GoEnv.GOPRIVATE) } if opts.GarbleLiterals { fmt.Fprintf(h, " -literals") diff --git a/main.go b/main.go index d74551c..7c9025c 100644 --- a/main.go +++ b/main.go @@ -7,6 +7,7 @@ import ( "bytes" "encoding/base64" "encoding/binary" + "encoding/json" "errors" "flag" "fmt" @@ -16,6 +17,7 @@ import ( "go/token" "go/types" "io" + "io/ioutil" "log" mathrand "math/rand" "os" @@ -29,6 +31,7 @@ import ( "unicode" "unicode/utf8" + "golang.org/x/mod/modfile" "golang.org/x/mod/module" "golang.org/x/mod/semver" "golang.org/x/tools/go/ast/astutil" @@ -114,8 +117,6 @@ var ( }).(types.ImporterFrom) opts *flagOptions - - envGoPrivate = os.Getenv("GOPRIVATE") // complemented by 'go env' later ) func obfuscatedTypesPackage(path string) *types.Package { @@ -175,7 +176,7 @@ var errJustExit = errors.New("") func goVersionOK() bool { const ( - minGoVersion = "v1.16.0" + minGoVersionSemver = "v1.16.0" suggestedGoVersion = "1.16.x" gitTimeFormat = "Mon Jan 2 15:04:05 2006 -0700" @@ -183,32 +184,30 @@ func goVersionOK() bool { // Go 1.16 was released on Febuary 16th, 2021. minGoVersionDate := time.Date(2021, 2, 16, 0, 0, 0, 0, time.UTC) - out, err := exec.Command("go", "version").CombinedOutput() - rawVersion := strings.TrimSpace(string(out)) - if err != nil || !strings.HasPrefix(rawVersion, "go version ") { - fmt.Fprintf(os.Stderr, `Can't get Go version: %v - -This is likely due to go not being installed/setup correctly. - -How to install Go: https://golang.org/doc/install -`, err) + version := cache.GoEnv.GOVERSION + if version == "" { + // Go 1.15.x and older do not have GOVERSION yet. + // We could go the extra mile and fetch it via 'go version', + // but we'd have to error anyway. + fmt.Fprintf(os.Stderr, "Go version is too old; please upgrade to Go %s or a newer devel version\n", suggestedGoVersion) return false } - rawVersion = strings.TrimPrefix(rawVersion, "go version ") - - tagIdx := strings.IndexByte(rawVersion, ' ') - tag := rawVersion[:tagIdx] - if tag == "devel" { - commitAndDate := rawVersion[tagIdx+1:] + if strings.HasPrefix(version, "devel ") { + commitAndDate := strings.TrimPrefix(version, "devel ") // Remove commit hash and architecture from version startDateIdx := strings.IndexByte(commitAndDate, ' ') + 1 - endDateIdx := strings.LastIndexByte(commitAndDate, ' ') - if endDateIdx <= 0 || endDateIdx <= startDateIdx { + if startDateIdx < 0 { // Custom version; assume the user knows what they're doing. return true } - date := commitAndDate[startDateIdx:endDateIdx] + + // TODO: once we support Go 1.17 and later, use the major Go + // version included in its devel versions: + // + // go version devel go1.17-8518aac314 ... + + date := commitAndDate[startDateIdx:] versionDate, err := time.Parse(gitTimeFormat, date) if err != nil { @@ -220,13 +219,13 @@ How to install Go: https://golang.org/doc/install return true } - fmt.Fprintf(os.Stderr, "Go version %q is too old; please upgrade to Go %s or a newer devel version\n", rawVersion, suggestedGoVersion) + fmt.Fprintf(os.Stderr, "Go version %q is too old; please upgrade to Go %s or a newer devel version\n", version, suggestedGoVersion) return false } - version := "v" + strings.TrimPrefix(tag, "go") - if semver.Compare(version, minGoVersion) < 0 { - fmt.Fprintf(os.Stderr, "Go version %q is too old; please upgrade to Go %s\n", rawVersion, suggestedGoVersion) + versionSemver := "v" + strings.TrimPrefix(version, "go") + if semver.Compare(versionSemver, minGoVersionSemver) < 0 { + fmt.Fprintf(os.Stderr, "Go version %q is too old; please upgrade to Go %s\n", version, suggestedGoVersion) return false } @@ -346,9 +345,6 @@ func mainErr(args []string) error { // Note that it uses and modifies global state; in general, it should only be // called once from mainErr in the top-level garble process. func toolexecCmd(command string, args []string) (*exec.Cmd, error) { - if !goVersionOK() { - return nil, errJustExit - } // Split the flags from the package arguments, since we'll need // to run 'go list' on the same set of packages. flags, args := splitFlagsFromArgs(args) @@ -374,10 +370,14 @@ func toolexecCmd(command string, args []string) (*exec.Cmd, error) { cache.BuildFlags = append(cache.BuildFlags, "-test") } - if err := setGoPrivate(); err != nil { + if err := fetchGoEnv(); err != nil { return nil, err } + if !goVersionOK() { + return nil, errJustExit + } + var err error cache.ExecPath, err = os.Executable() if err != nil { @@ -834,7 +834,7 @@ func isPrivate(path string) bool { if path == "command-line-arguments" || strings.HasPrefix(path, "plugin/unnamed") { return true } - return module.MatchPrefixPatterns(envGoPrivate, path) + return module.MatchPrefixPatterns(cache.GoEnv.GOPRIVATE, path) } // processImportCfg initializes importCfgEntries via the supplied flags, and @@ -1452,26 +1452,33 @@ func flagSetValue(flags []string, name, value string) []string { return append(flags, name+"="+value) } -func setGoPrivate() error { - if envGoPrivate == "" { - // Try 'go env' too, to query ${CONFIG}/go/env as well. - out, err := exec.Command("go", "env", "GOPRIVATE").CombinedOutput() - if err != nil { - return fmt.Errorf("%v: %s", err, out) - } - envGoPrivate = string(bytes.TrimSpace(out)) +func fetchGoEnv() error { + out, err := exec.Command("go", "env", "-json", + "GOPRIVATE", "GOMOD", "GOVERSION", + ).CombinedOutput() + if err != nil { + fmt.Fprintf(os.Stderr, `Can't find Go toolchain: %v + +This is likely due to go not being installed/setup correctly. + +How to install Go: https://golang.org/doc/install +`, err) + return errJustExit + } + if err := json.Unmarshal(out, &cache.GoEnv); err != nil { + return err } // If GOPRIVATE isn't set and we're in a module, use its module // path as a GOPRIVATE default. Include a _test variant too. - if envGoPrivate == "" { - modpath, err := exec.Command("go", "list", "-m").Output() - if err == nil { - path := string(bytes.TrimSpace(modpath)) - envGoPrivate = path + "," + path + "_test" + // TODO(mvdan): we shouldn't need the _test variant here, + // as the import path should not include it; only the package name. + if cache.GoEnv.GOPRIVATE == "" { + if mod, err := ioutil.ReadFile(cache.GoEnv.GOMOD); err == nil { + modpath := modfile.ModulePath(mod) + if modpath != "" { + cache.GoEnv.GOPRIVATE = modpath + "," + modpath + "_test" + } } } - // Explicitly set GOPRIVATE, since future garble processes won't - // query 'go env' again. - os.Setenv("GOPRIVATE", envGoPrivate) return nil } diff --git a/shared.go b/shared.go index 11661c6..1f97273 100644 --- a/shared.go +++ b/shared.go @@ -37,6 +37,13 @@ type sharedCache struct { // Once https://github.com/golang/go/issues/37475 is fixed, we // can likely just use that. BinaryContentID []byte + + // From "go env", primarily. + GoEnv struct { + GOPRIVATE string // Set to the module path as a fallback. + GOMOD string + GOVERSION string + } } var cache *sharedCache diff --git a/testdata/scripts/goversion.txt b/testdata/scripts/goversion.txt index 176ec15..0e73997 100644 --- a/testdata/scripts/goversion.txt +++ b/testdata/scripts/goversion.txt @@ -5,33 +5,33 @@ go build -o .bin/go$exe ./fakego env PATH=${WORK}/.bin${:}${PATH} # An empty go version. -env GO_VERSION='' +env GOVERSION='' ! garble build -stderr 'Can''t get Go version' +stderr 'Go version is too old' # We should error on a devel version that's too old. -env GO_VERSION='go version devel +afb5fca Sun Aug 07 00:00:00 2020 +0000 linux/amd64' +env GOVERSION='devel +afb5fca Sun Aug 07 00:00:00 2020 +0000' ! garble build stderr 'Go version.*Aug 07.*too old; please upgrade to Go 1.16.x or a newer devel version' # A future devel timestamp should be fine. -env GO_VERSION='go version devel +afb5fca Sun Sep 13 07:54:42 2021 +0000 linux/amd64' +env GOVERSION='devel +afb5fca Sun Sep 13 07:54:42 2021 +0000' ! garble build stderr 'mocking the real build' # We should error on a stable version that's too old. -env GO_VERSION='go version go1.14 windows/amd64' +env GOVERSION='go1.14' ! garble build stderr 'Go version.*go1.14.*too old; please upgrade to Go 1.16.x' ! stderr 'or a newer devel version' # We should accept a future stable version. -env GO_VERSION='go version go1.16.2 windows/amd64' +env GOVERSION='go1.16.2' ! garble build stderr 'mocking the real build' # We should accept custom devel strings. -env GO_VERSION='go version devel somecustomversion linux/amd64' +env GOVERSION='devel somecustomversion' ! garble build stderr 'mocking the real build' @@ -48,13 +48,19 @@ func main() {} package main import ( + "encoding/json" "fmt" "os" ) func main() { - if len(os.Args) > 0 && os.Args[1] == "version" { - fmt.Println(os.Getenv("GO_VERSION")) + if len(os.Args) > 0 && os.Args[1] == "env" { + enc, _ := json.Marshal(struct{ + GOVERSION string + } { + GOVERSION: os.Getenv("GOVERSION"), + }) + fmt.Printf("%s\n", enc) return } fmt.Fprintln(os.Stderr, "mocking the real build")