From b9086aa977defef67437da7cc623a9fe9ad571bb Mon Sep 17 00:00:00 2001 From: Jared Allard Date: Sat, 27 Aug 2022 15:14:30 +0200 Subject: [PATCH 1/6] refactor(listGoFiles): remove go list dependency --- mage/main.go | 86 +++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 61 insertions(+), 25 deletions(-) diff --git a/mage/main.go b/mage/main.go index a41e7435..d7e18088 100644 --- a/mage/main.go +++ b/mage/main.go @@ -1,11 +1,13 @@ package mage import ( + "bufio" "bytes" "crypto/sha1" "errors" "flag" "fmt" + "go/build/constraint" "io" "io/ioutil" "log" @@ -468,35 +470,68 @@ type mainfileTemplateData struct { BinaryName string } -func listGoFiles(magePath, goCmd, tags string, env []string) ([]string, error) { - args := []string{"list"} - if tags != "" { - args = append(args, fmt.Sprintf("-tags=%s", tags)) - } - args = append(args, "-e", "-f", `{{join .GoFiles "||"}}`) - cmd := exec.Command(goCmd, args...) - cmd.Env = env - buf := &bytes.Buffer{} - cmd.Stderr = buf - cmd.Dir = magePath - b, err := cmd.Output() +// listGoFiles returns a list of all .go files in a given directory, +// matching the provided tag +func listGoFiles(magePath, goCmd, tag string, env []string) ([]string, error) { + files, err := os.ReadDir(magePath) if err != nil { - stderr := buf.String() - // if the error is "cannot find module", that can mean that there's no - // non-mage files, which is fine, so ignore it. - if !strings.Contains(stderr, "cannot find module for path") { - if tags == "" { - return nil, fmt.Errorf("failed to list un-tagged gofiles: %v: %s", err, stderr) + return nil, err + } + + var goFiles []string + for _, file := range files { + if file.IsDir() { + continue + } + + if filepath.Ext(file.Name()) != ".go" { + continue + } + + f, err := os.Open(filepath.Join(magePath, file.Name())) + if err != nil { + return nil, err + } + + scanner := bufio.NewScanner(f) + satisifiesBuildConstraints := false + hadBuildConstraint := false + + // Scan the first three lines for the build tags + for lines := 0; scanner.Scan() && lines != 3; lines++ { + if lines > 2 { + break + } + + line := scanner.Text() + + // parse the build constraint so we can check if the tag is present + expr, err := constraint.Parse(line) + if err != nil { + continue } - return nil, fmt.Errorf("failed to list gofiles tagged with %q: %v: %s", tags, err, stderr) + hadBuildConstraint = true + + // check if the expression matches the tag we specified, if it doesn't + // then continue + satisifiesBuildConstraints = expr.Eval(func(goBuildTag string) bool { + debug.Printf("checking build tag %s against %s (file: %q, line: %q)", goBuildTag, tag, file.Name(), line) + return goBuildTag == tag + }) + } + + // If we didn't satisfy the build constraints AND we're not looking for a tag, then + // add the file. This ensures we skip files that DO have a build constraint. + // + // However, if we DID satisfy the build constraint AND we're looking for a tag, then + // add the file so we include constrained files. + if (!satisifiesBuildConstraints && tag == "" && !hadBuildConstraint) || (hadBuildConstraint && satisifiesBuildConstraints && tag != "") { + goFiles = append(goFiles, file.Name()) } } - out := strings.TrimSpace(string(b)) - list := strings.Split(out, "||") - for i := range list { - list[i] = filepath.Join(magePath, list[i]) - } - return list, nil + + debug.Printf("found %d go files with build tag %s (files: %v)", len(goFiles), tag, goFiles) + return goFiles, nil } // Magefiles returns the list of magefiles in dir. @@ -549,6 +584,7 @@ func Magefiles(magePath, goos, goarch, goCmd string, stderr io.Writer, isMagefil files = append(files, f) } } + return files, nil } From 91fe107da1b8a2fa48b8e51905e20d75c8eb8a29 Mon Sep 17 00:00:00 2001 From: Jared Allard Date: Sat, 27 Aug 2022 16:56:28 +0200 Subject: [PATCH 2/6] refactor to use go/build package to cover all usecases --- internal/run.go | 8 ++--- mage/main.go | 81 ++++++++++++++++++++----------------------------- 2 files changed, 37 insertions(+), 52 deletions(-) diff --git a/internal/run.go b/internal/run.go index 2c127fb0..79b4f049 100644 --- a/internal/run.go +++ b/internal/run.go @@ -57,9 +57,9 @@ func OutputDebug(cmd string, args ...string) (string, error) { return strings.TrimSpace(buf.String()), nil } -// splitEnv takes the results from os.Environ() (a []string of foo=bar values) +// SplitEnv takes the results from os.Environ() (a []string of foo=bar values) // and makes a map[string]string out of it. -func splitEnv(env []string) (map[string]string, error) { +func SplitEnv(env []string) (map[string]string, error) { out := map[string]string{} for _, s := range env { @@ -85,7 +85,7 @@ func joinEnv(env map[string]string) []string { // EnvWithCurrentGOOS returns a copy of os.Environ with the GOOS and GOARCH set // to runtime.GOOS and runtime.GOARCH. func EnvWithCurrentGOOS() ([]string, error) { - vals, err := splitEnv(os.Environ()) + vals, err := SplitEnv(os.Environ()) if err != nil { return nil, err } @@ -97,7 +97,7 @@ func EnvWithCurrentGOOS() ([]string, error) { // EnvWithGOOS retuns the os.Environ() values with GOOS and/or GOARCH either set // to their runtime value, or the given value if non-empty. func EnvWithGOOS(goos, goarch string) ([]string, error) { - env, err := splitEnv(os.Environ()) + env, err := SplitEnv(os.Environ()) if err != nil { return nil, err } diff --git a/mage/main.go b/mage/main.go index d7e18088..7165500f 100644 --- a/mage/main.go +++ b/mage/main.go @@ -1,13 +1,12 @@ package mage import ( - "bufio" "bytes" "crypto/sha1" "errors" "flag" "fmt" - "go/build/constraint" + "go/build" "io" "io/ioutil" "log" @@ -472,64 +471,50 @@ type mainfileTemplateData struct { // listGoFiles returns a list of all .go files in a given directory, // matching the provided tag -func listGoFiles(magePath, goCmd, tag string, env []string) ([]string, error) { - files, err := os.ReadDir(magePath) - if err != nil { - return nil, err - } - - var goFiles []string - for _, file := range files { - if file.IsDir() { - continue - } - - if filepath.Ext(file.Name()) != ".go" { - continue - } - - f, err := os.Open(filepath.Join(magePath, file.Name())) +func listGoFiles(magePath, goCmd, tag string, envStr []string) ([]string, error) { + origMagePath := magePath + if !filepath.IsAbs(magePath) { + cwd, err := os.Getwd() if err != nil { return nil, err } + magePath = filepath.Join(cwd, magePath) + } - scanner := bufio.NewScanner(f) - satisifiesBuildConstraints := false - hadBuildConstraint := false + env, err := internal.SplitEnv(envStr) + if err != nil { + return nil, err + } - // Scan the first three lines for the build tags - for lines := 0; scanner.Scan() && lines != 3; lines++ { - if lines > 2 { - break - } + bctx := build.Default + bctx.Dir = magePath + bctx.BuildTags = []string{tag} - line := scanner.Text() + if _, ok := env["GOOS"]; ok { + bctx.GOOS = env["GOOS"] + } - // parse the build constraint so we can check if the tag is present - expr, err := constraint.Parse(line) - if err != nil { - continue - } - hadBuildConstraint = true - - // check if the expression matches the tag we specified, if it doesn't - // then continue - satisifiesBuildConstraints = expr.Eval(func(goBuildTag string) bool { - debug.Printf("checking build tag %s against %s (file: %q, line: %q)", goBuildTag, tag, file.Name(), line) - return goBuildTag == tag - }) + if _, ok := env["GOARCH"]; ok { + bctx.GOARCH = env["GOARCH"] + } + + pkg, err := bctx.Import(".", bctx.Dir, 0) + if err != nil { + if _, ok := err.(*build.NoGoError); ok { + return []string{}, nil } - // If we didn't satisfy the build constraints AND we're not looking for a tag, then - // add the file. This ensures we skip files that DO have a build constraint. - // - // However, if we DID satisfy the build constraint AND we're looking for a tag, then - // add the file so we include constrained files. - if (!satisifiesBuildConstraints && tag == "" && !hadBuildConstraint) || (hadBuildConstraint && satisifiesBuildConstraints && tag != "") { - goFiles = append(goFiles, file.Name()) + // Allow multiple packages in the same directory + if _, ok := err.(*build.MultiplePackageError); !ok { + return nil, err } } + goFiles := make([]string, len(pkg.GoFiles)) + for i := range pkg.GoFiles { + goFiles[i] = filepath.Join(origMagePath, pkg.GoFiles[i]) + } + debug.Printf("found %d go files with build tag %s (files: %v)", len(goFiles), tag, goFiles) return goFiles, nil } From 9ab876e52fd46cc1a024df1aabfa947ad8078e3c Mon Sep 17 00:00:00 2001 From: Jared Allard Date: Sat, 10 Sep 2022 07:34:23 -0700 Subject: [PATCH 3/6] Update mage/main.go Co-authored-by: Nate Finch --- mage/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mage/main.go b/mage/main.go index 7165500f..6c3518fc 100644 --- a/mage/main.go +++ b/mage/main.go @@ -476,7 +476,7 @@ func listGoFiles(magePath, goCmd, tag string, envStr []string) ([]string, error) if !filepath.IsAbs(magePath) { cwd, err := os.Getwd() if err != nil { - return nil, err + return nil, fmt.Errorf("can't get current working directory: %v", err) } magePath = filepath.Join(cwd, magePath) } From 0c5076d37ff374d94ee93f7a1ba857554bd28c72 Mon Sep 17 00:00:00 2001 From: Jared Allard Date: Sat, 10 Sep 2022 07:34:27 -0700 Subject: [PATCH 4/6] Update mage/main.go Co-authored-by: Nate Finch --- mage/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mage/main.go b/mage/main.go index 6c3518fc..a22d25dd 100644 --- a/mage/main.go +++ b/mage/main.go @@ -483,7 +483,7 @@ func listGoFiles(magePath, goCmd, tag string, envStr []string) ([]string, error) env, err := internal.SplitEnv(envStr) if err != nil { - return nil, err + return nil, fmt.Errorf("error parsing environment variables: %v", err) } bctx := build.Default From c3450918831276eb6cbb1c269dc49fed73264baa Mon Sep 17 00:00:00 2001 From: Jared Allard Date: Sat, 10 Sep 2022 07:36:08 -0700 Subject: [PATCH 5/6] Update mage/main.go --- mage/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mage/main.go b/mage/main.go index a22d25dd..47cb9e3b 100644 --- a/mage/main.go +++ b/mage/main.go @@ -506,7 +506,7 @@ func listGoFiles(magePath, goCmd, tag string, envStr []string) ([]string, error) // Allow multiple packages in the same directory if _, ok := err.(*build.MultiplePackageError); !ok { - return nil, err + return nil, fmt.Errorf("failed to parse go source files: %v", err) } } From f683ad6f7d003350a4c09c59f78573f4619f19ff Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Thu, 15 Sep 2022 14:14:29 -0400 Subject: [PATCH 6/6] make it work with older versions of go --- mage/main.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mage/main.go b/mage/main.go index 47cb9e3b..65964228 100644 --- a/mage/main.go +++ b/mage/main.go @@ -487,7 +487,6 @@ func listGoFiles(magePath, goCmd, tag string, envStr []string) ([]string, error) } bctx := build.Default - bctx.Dir = magePath bctx.BuildTags = []string{tag} if _, ok := env["GOOS"]; ok { @@ -498,7 +497,7 @@ func listGoFiles(magePath, goCmd, tag string, envStr []string) ([]string, error) bctx.GOARCH = env["GOARCH"] } - pkg, err := bctx.Import(".", bctx.Dir, 0) + pkg, err := bctx.Import(".", magePath, 0) if err != nil { if _, ok := err.(*build.NoGoError); ok { return []string{}, nil