From 2da5e345b90acc6eabfcebe004b4753aec3d003b Mon Sep 17 00:00:00 2001 From: Chris Koch Date: Sat, 5 Aug 2023 18:01:23 -0700 Subject: [PATCH] API: improve golang.Default Instead of the clunky env := golang.Default() env.CgoEnabled = false env.GOARCH = ... Then using &env somewhere like in u-root, we now have a nicer With* API that should allow all this in one line: env := golang.Default(golang.DisableCGO(), golang.WithGOARCH("arm")) Signed-off-by: Chris Koch --- src/pkg/bb/bb.go | 6 +++-- src/pkg/bb/findpkg/bb.go | 26 ++++++++++++-------- src/pkg/bb/findpkg/bb_test.go | 40 +++++++++++++----------------- src/pkg/golang/build.go | 46 +++++++++++++++++++++++++++++++++-- 4 files changed, 81 insertions(+), 37 deletions(-) diff --git a/src/pkg/bb/bb.go b/src/pkg/bb/bb.go index bc9f039b..224595bc 100644 --- a/src/pkg/bb/bb.go +++ b/src/pkg/bb/bb.go @@ -68,7 +68,7 @@ func checkDuplicate(cmds []*bbinternal.Package) error { type Opts struct { // Env are the environment variables used in Go compilation and package // discovery. - Env golang.Environ + Env *golang.Environ // LookupEnv is the environment for looking up and resolving command // paths. @@ -116,6 +116,8 @@ type Opts struct { func BuildBusybox(l ulog.Logger, opts *Opts) (nerr error) { if opts == nil { return fmt.Errorf("no options given for busybox build") + } else if opts.Env == nil { + return fmt.Errorf("Go build environment unspecified for busybox build") } else if err := opts.Env.Valid(); err != nil { return err } @@ -586,7 +588,7 @@ func moduleVersionIdentifier(m *packages.Module) string { // // Then, in the generated tree's main module, we create a go.mod file with // replace directives for all the local modules we just copied over. -func copyLocalDeps(l ulog.Logger, env golang.Environ, bbDir, tmpDir, pkgDir string, mainPkgs []*bbinternal.Package) error { +func copyLocalDeps(l ulog.Logger, env *golang.Environ, bbDir, tmpDir, pkgDir string, mainPkgs []*bbinternal.Package) error { localModules, err := findLocalModules(l, mainPkgs) if err != nil { return err diff --git a/src/pkg/bb/findpkg/bb.go b/src/pkg/bb/findpkg/bb.go index 63a29473..7565853d 100644 --- a/src/pkg/bb/findpkg/bb.go +++ b/src/pkg/bb/findpkg/bb.go @@ -111,7 +111,7 @@ func batchFSPackages(l ulog.Logger, absPaths []string, loadFunc func(moduleDir s // .), however doing that N times is very expensive -- takes several minutes // for 30 packages. So here, we figure out every module involved and do one // query per module and one query for everything that isn't in a module. -func batchLoadFSPackages(l ulog.Logger, env golang.Environ, absPaths []string) ([]*packages.Package, error) { +func batchLoadFSPackages(l ulog.Logger, env *golang.Environ, absPaths []string) ([]*packages.Package, error) { var allps []*packages.Package err := batchFSPackages(l, absPaths, func(moduleDir string, packageDirs []string) error { @@ -146,7 +146,7 @@ func addPkg(l ulog.Logger, plist []*packages.Package, p *packages.Package) ([]*p return plist, nil } -func newPackages(l ulog.Logger, genv golang.Environ, env Env, patterns ...string) ([]*packages.Package, error) { +func newPackages(l ulog.Logger, genv *golang.Environ, env Env, patterns ...string) ([]*packages.Package, error) { var goImportPaths []string var filesystemPaths []string @@ -246,7 +246,10 @@ func newPackages(l ulog.Logger, genv golang.Environ, env Env, patterns ...string // - GBB_PATH=$HOME/u-root:$HOME/yourproject cmds/core/* cmd/foobar // // - UROOT_SOURCE=$HOME/u-root github.com/u-root/u-root/cmds/core/ip -func NewPackages(l ulog.Logger, genv golang.Environ, env Env, names ...string) ([]*bbinternal.Package, error) { +func NewPackages(l ulog.Logger, genv *golang.Environ, env Env, names ...string) ([]*bbinternal.Package, error) { + if genv == nil { + return nil, fmt.Errorf("Go build environment must be specified") + } ps, err := newPackages(l, genv, env, names...) if err != nil { return nil, err @@ -259,7 +262,7 @@ func NewPackages(l ulog.Logger, genv golang.Environ, env Env, names ...string) ( return ips, nil } -func loadPkgs(env golang.Environ, dir string, patterns ...string) ([]*packages.Package, error) { +func loadPkgs(env *golang.Environ, dir string, patterns ...string) ([]*packages.Package, error) { cfg := &packages.Config{ Mode: packages.NeedName | packages.NeedImports | packages.NeedFiles | packages.NeedDeps | packages.NeedTypes | packages.NeedSyntax | packages.NeedTypesInfo | packages.NeedCompiledGoFiles | packages.NeedModule | packages.NeedEmbedFiles, Env: append(os.Environ(), env.Env()...), @@ -272,7 +275,7 @@ func loadPkgs(env golang.Environ, dir string, patterns ...string) ([]*packages.P return packages.Load(cfg, patterns...) } -func filterDirectoryPaths(l ulog.Logger, env golang.Environ, directories []string, excludes []string) ([]string, error) { +func filterDirectoryPaths(l ulog.Logger, env *golang.Environ, directories []string, excludes []string) ([]string, error) { // Eligibility check: does each directory contain files that are // compilable under the current GOROOT/GOPATH/GOOS/GOARCH and build // tags? @@ -354,7 +357,7 @@ func excludePaths(paths []string, exclusions []string) []string { } // Just looking up the stuff that doesn't take forever to parse. -func lookupPkgNameAndFiles(env golang.Environ, dir string, patterns ...string) ([]*packages.Package, error) { +func lookupPkgNameAndFiles(env *golang.Environ, dir string, patterns ...string) ([]*packages.Package, error) { cfg := &packages.Config{ Mode: packages.NeedName | packages.NeedFiles, Env: append(os.Environ(), env.Env()...), @@ -371,7 +374,7 @@ func couldBeGlob(s string) bool { // Go command paths. It may return a list that contains errors. // // Precondition: couldBeGlob(pattern) is true -func lookupPkgsWithGlob(env golang.Environ, wd string, pattern string) ([]*packages.Package, error) { +func lookupPkgsWithGlob(env *golang.Environ, wd string, pattern string) ([]*packages.Package, error) { elems := strings.Split(pattern, "/") globIndex := 0 @@ -404,7 +407,7 @@ func lookupPkgsWithGlob(env golang.Environ, wd string, pattern string) ([]*packa // lookupCompilablePkgsWithGlob resolves Go package path globs to a realized // list of Go command paths. It filters out packages that have no files // matching our build constraints and other errors. -func lookupCompilablePkgsWithGlob(l ulog.Logger, env golang.Environ, wd string, patterns ...string) ([]string, error) { +func lookupCompilablePkgsWithGlob(l ulog.Logger, env *golang.Environ, wd string, patterns ...string) ([]string, error) { var pkgs []*packages.Package // Batching saves time. Patterns with globs cannot be batched. // @@ -442,7 +445,7 @@ func lookupCompilablePkgsWithGlob(l ulog.Logger, env golang.Environ, wd string, return paths, nil } -func filterGoPaths(l ulog.Logger, env golang.Environ, wd string, gopathIncludes, gopathExcludes []string) ([]string, error) { +func filterGoPaths(l ulog.Logger, env *golang.Environ, wd string, gopathIncludes, gopathExcludes []string) ([]string, error) { goInc, err := lookupCompilablePkgsWithGlob(l, env, wd, gopathIncludes...) if err != nil { return nil, err @@ -559,7 +562,10 @@ func DefaultEnv() Env { // normalized Go package paths. The return list may be mixed. // // See NewPackages for allowed formats. -func ResolveGlobs(logger ulog.Logger, genv golang.Environ, env Env, patterns []string) ([]string, error) { +func ResolveGlobs(logger ulog.Logger, genv *golang.Environ, env Env, patterns []string) ([]string, error) { + if genv == nil { + return nil, fmt.Errorf("Go build environment must be specified") + } var dirIncludes []string var dirExcludes []string var gopathIncludes []string diff --git a/src/pkg/bb/findpkg/bb_test.go b/src/pkg/bb/findpkg/bb_test.go index ee7bc711..c91ebd46 100644 --- a/src/pkg/bb/findpkg/bb_test.go +++ b/src/pkg/bb/findpkg/bb_test.go @@ -84,7 +84,7 @@ type testCase struct { // name of the test case name string // envs to try it in (if unset, default will be GO111MODULE=on and off) - envs []golang.Environ + envs []*golang.Environ // wd sets the findpkg.Env.WorkingDirectory // WorkingDirectory is the directory used for module-enabled // `go list` lookups. The go.mod in this directory (or one of @@ -121,14 +121,9 @@ func TestResolve(t *testing.T) { } gbbroot := filepath.Dir(gbbmod) - moduleOffEnv := golang.Default() - moduleOffEnv.GO111MODULE = "off" - - moduleOnEnv := golang.Default() - moduleOnEnv.GO111MODULE = "on" - - noGoToolEnv := golang.Default() - noGoToolEnv.GOROOT = t.TempDir() + moduleOffEnv := golang.Default(golang.WithGO111MODULE("off")) + moduleOnEnv := golang.Default(golang.WithGO111MODULE("on")) + noGoToolEnv := golang.Default(golang.WithGOROOT(t.TempDir())) if err := os.Mkdir("./test/resolvebroken", 0777); err != nil { t.Fatal(err) @@ -307,7 +302,7 @@ func TestResolve(t *testing.T) { // of this test, this is an ON only test. { name: "pkgpath-multi-module", - envs: []golang.Environ{moduleOnEnv}, + envs: []*golang.Environ{moduleOnEnv}, wd: filepath.Join(gbbroot, "test/resolve-modules"), in: []string{ "github.com/u-root/u-root/cmds/core/init", @@ -348,7 +343,7 @@ func TestResolve(t *testing.T) { // of this test, this is an ON only test. { name: "pkgpath-multi-module-exclusion-glob", - envs: []golang.Environ{moduleOnEnv}, + envs: []*golang.Environ{moduleOnEnv}, wd: filepath.Join(gbbroot, "test/resolve-modules"), in: []string{ "github.com/u-root/u-root/cmds/core/init", @@ -370,14 +365,14 @@ func TestResolve(t *testing.T) { }, { name: "fspath-nomodule", - envs: []golang.Environ{moduleOffEnv}, + envs: []*golang.Environ{moduleOffEnv}, in: []string{filepath.Join(gbbroot, "vendortest/cmd/dmesg")}, want: []string{filepath.Join(gbbroot, "vendortest/cmd/dmesg")}, wantPkgPath: []string{"github.com/u-root/gobusybox/vendortest/cmd/dmesg"}, }, { name: "pkgpath-nomodule", - envs: []golang.Environ{moduleOffEnv}, + envs: []*golang.Environ{moduleOffEnv}, in: []string{"github.com/u-root/gobusybox/vendortest/cmd/dmesg"}, want: []string{"github.com/u-root/gobusybox/vendortest/cmd/dmesg"}, wantPkgPath: []string{"github.com/u-root/gobusybox/vendortest/cmd/dmesg"}, @@ -392,19 +387,19 @@ func TestResolve(t *testing.T) { // Some error cases where $GOROOT/bin/go is unavailable, so packages.Load fails. { name: "fspath-load-fails", - envs: []golang.Environ{noGoToolEnv}, + envs: []*golang.Environ{noGoToolEnv}, in: []string{"./test/goglob/*"}, wantErr: true, }, { name: "pkgpath-batched-load-fails", - envs: []golang.Environ{noGoToolEnv}, + envs: []*golang.Environ{noGoToolEnv}, in: []string{"./test/goglob/..."}, wantErr: true, }, { name: "pkgpath-glob-load-fails", - envs: []golang.Environ{noGoToolEnv}, + envs: []*golang.Environ{noGoToolEnv}, in: []string{"github.com/u-root/gobusybox/src/pkg/bb/findpkg/test/goglob/*"}, wantErr: true, }, @@ -504,7 +499,7 @@ func TestResolve(t *testing.T) { } for _, tc := range append(sharedTestCases, externalDepCases...) { - envs := []golang.Environ{moduleOffEnv, moduleOnEnv} + envs := []*golang.Environ{moduleOffEnv, moduleOnEnv} if tc.envs != nil { envs = tc.envs } @@ -529,14 +524,13 @@ func TestResolve(t *testing.T) { } } - noGopathModuleOffEnv := moduleOffEnv - noGopathModuleOffEnv.GOPATH = "" + noGopathModuleOffEnv := golang.Default(golang.WithGO111MODULE("off"), golang.WithGOPATH("")) newPkgTests := append(sharedTestCases, testCase{ // UROOT_SOURCE, file system paths, non-Gobusybox module. // Cannot resolve dependency packages without GOPATH. name: "fspath-uroot-source-no-GOPATH", - envs: []golang.Environ{noGopathModuleOffEnv}, + envs: []*golang.Environ{noGopathModuleOffEnv}, urootSource: urootSrc, in: []string{ "cmds/core/ip", @@ -552,9 +546,9 @@ func TestResolve(t *testing.T) { in: []string{"github.com/u-root/gobusybox/src/pkg/bb/findpkg/test/parsebroken"}, wantErr: true, }) - newPkgTests = append(newPkgTests, testCasesWithEnv([]golang.Environ{moduleOnEnv}, externalDepCases...)...) + newPkgTests = append(newPkgTests, testCasesWithEnv([]*golang.Environ{moduleOnEnv}, externalDepCases...)...) for _, tc := range newPkgTests { - envs := []golang.Environ{moduleOffEnv, moduleOnEnv} + envs := []*golang.Environ{moduleOffEnv, moduleOnEnv} if tc.envs != nil { envs = tc.envs } @@ -587,7 +581,7 @@ func TestResolve(t *testing.T) { } } -func testCasesWithEnv(envs []golang.Environ, tcs ...testCase) []testCase { +func testCasesWithEnv(envs []*golang.Environ, tcs ...testCase) []testCase { var newTCs []testCase for _, tc := range tcs { newTC := tc diff --git a/src/pkg/golang/build.go b/src/pkg/golang/build.go index 98a313c2..929245cc 100644 --- a/src/pkg/golang/build.go +++ b/src/pkg/golang/build.go @@ -58,14 +58,56 @@ func parseBool(s string) bool { return ok } +// Opt is an option function applied to Environ. +type Opt func(*Environ) + +// DisableCGO is an option that disables cgo. +func DisableCGO() Opt { + return func(c *Environ) { + c.CgoEnabled = false + } +} + +// WithGOARCH is an option that overrides GOARCH. +func WithGOARCH(goarch string) Opt { + return func(c *Environ) { + c.GOARCH = goarch + } +} + +// WithGOPATH is an option that overrides GOPATH. +func WithGOPATH(gopath string) Opt { + return func(c *Environ) { + c.GOPATH = gopath + } +} + +// WithGOROOT is an option that overrides GOROOT. +func WithGOROOT(goroot string) Opt { + return func(c *Environ) { + c.GOROOT = goroot + } +} + +// WithGO111MODULE is an option that overrides GO111MODULE. +func WithGO111MODULE(go111module string) Opt { + return func(c *Environ) { + c.GO111MODULE = go111module + } +} + // Default is the default build environment comprised of the default GOPATH, // GOROOT, GOOS, GOARCH, and CGO_ENABLED values. -func Default() Environ { - return Environ{ +func Default(opt ...Opt) *Environ { + env := &Environ{ Context: build.Default, GO111MODULE: os.Getenv("GO111MODULE"), GBBDEBUG: parseBool(os.Getenv("GBBDEBUG")), } + for _, o := range opt { + o(env) + } + return env } // GoCmd runs a go command in the environment.