diff --git a/mage/main.go b/mage/main.go index f57fdc69..a41e7435 100644 --- a/mage/main.go +++ b/mage/main.go @@ -118,6 +118,15 @@ type Invocation struct { HashFast bool // don't rely on GOCACHE, just hash the magefiles } +// MagefilesDirName is the name of the default folder to look for if no directory was specified, +// if this folder exists it will be assumed mage package lives inside it. +const MagefilesDirName = "magefiles" + +// UsesMagefiles returns true if we are getting our mage files from a magefiles directory. +func (i Invocation) UsesMagefiles() bool { + return i.Dir == MagefilesDirName +} + // ParseAndRun parses the command line, and then compiles and runs the mage // files in the given directory with the given args (do not include the command // name in the args). @@ -180,7 +189,7 @@ func Parse(stderr, stdout io.Writer, args []string) (inv Invocation, cmd Command fs.BoolVar(&inv.Help, "h", false, "show this help") fs.DurationVar(&inv.Timeout, "t", 0, "timeout in duration parsable format (e.g. 5m30s)") fs.BoolVar(&inv.Keep, "keep", false, "keep intermediate mage files around after running") - fs.StringVar(&inv.Dir, "d", ".", "directory to read magefiles from") + fs.StringVar(&inv.Dir, "d", "", "directory to read magefiles from") fs.StringVar(&inv.WorkDir, "w", "", "working directory where magefiles will run") fs.StringVar(&inv.GoCmd, "gocmd", mg.GoCmd(), "use the given go binary to compile the output") fs.StringVar(&inv.GOOS, "goos", "", "set GOOS for binary produced with -compile") @@ -216,7 +225,7 @@ Commands: Options: -d - directory to read magefiles from (default ".") + directory to read magefiles from (default "." or "magefiles" if exists) -debug turn on debug messages -f force recreation of compiled magefile -goarch sets the GOARCH for the binary created by -compile (default: current arch) @@ -296,23 +305,50 @@ Options: return inv, cmd, err } +const dotDirectory = "." + // Invoke runs Mage with the given arguments. func Invoke(inv Invocation) int { errlog := log.New(inv.Stderr, "", 0) if inv.GoCmd == "" { inv.GoCmd = "go" } + var noDir bool if inv.Dir == "" { - inv.Dir = "." + noDir = true + inv.Dir = dotDirectory + // . will be default unless we find a mage folder. + mfSt, err := os.Stat(MagefilesDirName) + if err == nil { + if mfSt.IsDir() { + stderrBuf := &bytes.Buffer{} + inv.Dir = MagefilesDirName // preemptive assignment + // TODO: Remove this fallback and the above Magefiles invocation when the bw compatibility is removed. + files, err := Magefiles(dotDirectory, inv.GOOS, inv.GOARCH, inv.GoCmd, stderrBuf, false, inv.Debug) + if err == nil { + if len(files) != 0 { + errlog.Println("[WARNING] You have both a magefiles directory and mage files in the " + + "current directory, in future versions the files will be ignored in favor of the directory") + inv.Dir = dotDirectory + } + } + } + } } + if inv.WorkDir == "" { - inv.WorkDir = inv.Dir + if noDir { + inv.WorkDir = dotDirectory + } else { + inv.WorkDir = inv.Dir + } } + if inv.CacheDir == "" { inv.CacheDir = mg.CacheDir() } - files, err := Magefiles(inv.Dir, inv.GOOS, inv.GOARCH, inv.GoCmd, inv.Stderr, inv.Debug) + files, err := Magefiles(inv.Dir, inv.GOOS, inv.GOARCH, inv.GoCmd, inv.Stderr, inv.UsesMagefiles(), inv.Debug) if err != nil { errlog.Println("Error determining list of magefiles:", err) return 1 @@ -432,69 +468,87 @@ 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() + 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, fmt.Errorf("failed to list gofiles tagged with %q: %v: %s", tags, err, stderr) + } + } + out := strings.TrimSpace(string(b)) + list := strings.Split(out, "||") + for i := range list { + list[i] = filepath.Join(magePath, list[i]) + } + return list, nil +} + // Magefiles returns the list of magefiles in dir. -func Magefiles(magePath, goos, goarch, goCmd string, stderr io.Writer, isDebug bool) ([]string, error) { +func Magefiles(magePath, goos, goarch, goCmd string, stderr io.Writer, isMagefilesDirectory, isDebug bool) ([]string, error) { start := time.Now() defer func() { debug.Println("time to scan for Magefiles:", time.Since(start)) }() - fail := func(err error) ([]string, error) { - return nil, err - } env, err := internal.EnvWithGOOS(goos, goarch) if err != nil { return nil, err } - debug.Println("getting all non-mage files in", magePath) + debug.Println("getting all files including those with mage tag in", magePath) + mageFiles, err := listGoFiles(magePath, goCmd, "mage", env) + if err != nil { + return nil, fmt.Errorf("listing mage files: %v", err) + } - // // first, grab all the files with no build tags specified.. this is actually - // // our exclude list of things without the mage build tag. - cmd := exec.Command(goCmd, "list", "-e", "-f", `{{join .GoFiles "||"}}`) - cmd.Env = env - buf := &bytes.Buffer{} - cmd.Stderr = buf - cmd.Dir = magePath - b, err := cmd.Output() + if isMagefilesDirectory { + // For the magefiles directory, we always use all go files, both with + // and without the mage tag, as per normal go build tag rules. + debug.Println("using all go files in magefiles directory", magePath) + return mageFiles, nil + } + + // For folders other than the magefiles directory, we only consider files + // that have the mage build tag and ignore those that don't. + + debug.Println("getting all files without mage tag in", magePath) + nonMageFiles, err := listGoFiles(magePath, goCmd, "", env) 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") { - return fail(fmt.Errorf("failed to list non-mage gofiles: %v: %s", err, stderr)) - } + return nil, fmt.Errorf("listing non-mage files: %v", err) } - list := strings.TrimSpace(string(b)) - debug.Println("found non-mage files", list) + + // convert non-Mage list to a map of files to exclude. exclude := map[string]bool{} - for _, f := range strings.Split(list, "||") { + for _, f := range nonMageFiles { if f != "" { debug.Printf("marked file as non-mage: %q", f) exclude[f] = true } } - debug.Println("getting all files plus mage files") - cmd = exec.Command(goCmd, "list", "-tags=mage", "-e", "-f", `{{join .GoFiles "||"}}`) - cmd.Env = env - - buf.Reset() - cmd.Dir = magePath - b, err = cmd.Output() - if err != nil { - return fail(fmt.Errorf("failed to list mage gofiles: %v: %s", err, buf.Bytes())) - } - list = strings.TrimSpace(string(b)) - files := []string{} - for _, f := range strings.Split(list, "||") { + // filter out the non-mage files from the mage files. + var files []string + for _, f := range mageFiles { if f != "" && !exclude[f] { files = append(files, f) } } - for i := range files { - files[i] = filepath.Join(magePath, files[i]) - } return files, nil } diff --git a/mage/main_test.go b/mage/main_test.go index d0b43d84..2074c64a 100644 --- a/mage/main_test.go +++ b/mage/main_test.go @@ -189,7 +189,7 @@ func TestTransitiveHashFast(t *testing.T) { func TestListMagefilesMain(t *testing.T) { buf := &bytes.Buffer{} - files, err := Magefiles("testdata/mixed_main_files", "", "", "go", buf, false) + files, err := Magefiles("testdata/mixed_main_files", "", "", "go", buf, false, false) if err != nil { t.Errorf("error from magefile list: %v: %s", err, buf) } @@ -210,7 +210,7 @@ func TestListMagefilesIgnoresGOOS(t *testing.T) { os.Setenv("GOOS", "windows") } defer os.Setenv("GOOS", runtime.GOOS) - files, err := Magefiles("testdata/goos_magefiles", "", "", "go", buf, false) + files, err := Magefiles("testdata/goos_magefiles", "", "", "go", buf, false, false) if err != nil { t.Errorf("error from magefile list: %v: %s", err, buf) } @@ -234,7 +234,7 @@ func TestListMagefilesIgnoresRespectsGOOSArg(t *testing.T) { goos = "windows" } // Set GOARCH as amd64 because windows is not on all non-x86 architectures. - files, err := Magefiles("testdata/goos_magefiles", goos, "amd64", "go", buf, false) + files, err := Magefiles("testdata/goos_magefiles", goos, "amd64", "go", buf, false, false) if err != nil { t.Errorf("error from magefile list: %v: %s", err, buf) } @@ -308,7 +308,7 @@ func TestCompileDiffGoosGoarch(t *testing.T) { func TestListMagefilesLib(t *testing.T) { buf := &bytes.Buffer{} - files, err := Magefiles("testdata/mixed_lib_files", "", "", "go", buf, false) + files, err := Magefiles("testdata/mixed_lib_files", "", "", "go", buf, false, false) if err != nil { t.Errorf("error from magefile list: %v: %s", err, buf) } @@ -342,6 +342,140 @@ func TestMixedMageImports(t *testing.T) { } } +func TestMagefilesFolder(t *testing.T) { + resetTerm() + wd, err := os.Getwd() + t.Log(wd) + if err != nil { + t.Fatalf("finding current working directory: %v", err) + } + if err := os.Chdir("testdata/with_magefiles_folder"); err != nil { + t.Fatalf("changing to magefolders tests data: %v", err) + } + // restore previous state + defer os.Chdir(wd) + + stderr := &bytes.Buffer{} + stdout := &bytes.Buffer{} + inv := Invocation{ + Dir: "", + Stdout: stdout, + Stderr: stderr, + List: true, + } + code := Invoke(inv) + if code != 0 { + t.Errorf("expected to exit with code 0, but got %v, stderr: %s", code, stderr) + } + expected := "Targets:\n build \n" + actual := stdout.String() + if actual != expected { + t.Fatalf("expected %q but got %q", expected, actual) + } +} + +func TestMagefilesFolderMixedWithMagefiles(t *testing.T) { + resetTerm() + wd, err := os.Getwd() + t.Log(wd) + if err != nil { + t.Fatalf("finding current working directory: %v", err) + } + if err := os.Chdir("testdata/with_magefiles_folder_and_mage_files_in_dot"); err != nil { + t.Fatalf("changing to magefolders tests data: %v", err) + } + // restore previous state + defer os.Chdir(wd) + + stderr := &bytes.Buffer{} + stdout := &bytes.Buffer{} + inv := Invocation{ + Dir: "", + Stdout: stdout, + Stderr: stderr, + List: true, + } + code := Invoke(inv) + if code != 0 { + t.Errorf("expected to exit with code 0, but got %v, stderr: %s", code, stderr) + } + expected := "Targets:\n build \n" + actual := stdout.String() + if actual != expected { + t.Fatalf("expected %q but got %q", expected, actual) + } + + expectedErr := "[WARNING] You have both a magefiles directory and mage files in the current directory, in future versions the files will be ignored in favor of the directory\n" + actualErr := stderr.String() + if actualErr != expectedErr { + t.Fatalf("expected Warning %q but got %q", expectedErr, actualErr) + } +} + +func TestUntaggedMagefilesFolder(t *testing.T) { + resetTerm() + wd, err := os.Getwd() + t.Log(wd) + if err != nil { + t.Fatalf("finding current working directory: %v", err) + } + if err := os.Chdir("testdata/with_untagged_magefiles_folder"); err != nil { + t.Fatalf("changing to magefolders tests data: %v", err) + } + // restore previous state + defer os.Chdir(wd) + + stderr := &bytes.Buffer{} + stdout := &bytes.Buffer{} + inv := Invocation{ + Dir: "", + Stdout: stdout, + Stderr: stderr, + List: true, + } + code := Invoke(inv) + if code != 0 { + t.Errorf("expected to exit with code 0, but got %v, stderr: %s", code, stderr) + } + expected := "Targets:\n build \n" + actual := stdout.String() + if actual != expected { + t.Fatalf("expected %q but got %q", expected, actual) + } +} + +func TestMixedTaggingMagefilesFolder(t *testing.T) { + resetTerm() + wd, err := os.Getwd() + t.Log(wd) + if err != nil { + t.Fatalf("finding current working directory: %v", err) + } + if err := os.Chdir("testdata/with_mixtagged_magefiles_folder"); err != nil { + t.Fatalf("changing to magefolders tests data: %v", err) + } + // restore previous state + defer os.Chdir(wd) + + stderr := &bytes.Buffer{} + stdout := &bytes.Buffer{} + inv := Invocation{ + Dir: "", + Stdout: stdout, + Stderr: stderr, + List: true, + } + code := Invoke(inv) + if code != 0 { + t.Errorf("expected to exit with code 0, but got %v, stderr: %s", code, stderr) + } + expected := "Targets:\n build \n untaggedBuild \n" + actual := stdout.String() + if actual != expected { + t.Fatalf("expected %q but got %q", expected, actual) + } +} + func TestGoRun(t *testing.T) { c := exec.Command("go", "run", "main.go") c.Dir = "./testdata" @@ -1615,7 +1749,7 @@ func TestWrongDependency(t *testing.T) { } } -/// This code liberally borrowed from https://github.com/rsc/goversion/blob/master/version/exe.go +// / This code liberally borrowed from https://github.com/rsc/goversion/blob/master/version/exe.go type ( exeType int diff --git a/mage/testdata/with_magefiles_folder/magefiles/mage_helpers.go b/mage/testdata/with_magefiles_folder/magefiles/mage_helpers.go new file mode 100644 index 00000000..e4b8872f --- /dev/null +++ b/mage/testdata/with_magefiles_folder/magefiles/mage_helpers.go @@ -0,0 +1,6 @@ +//go:build mage +// +build mage + +package main + +func foo() {} diff --git a/mage/testdata/with_magefiles_folder/magefiles/magefile.go b/mage/testdata/with_magefiles_folder/magefiles/magefile.go new file mode 100644 index 00000000..63584509 --- /dev/null +++ b/mage/testdata/with_magefiles_folder/magefiles/magefile.go @@ -0,0 +1,6 @@ +//go:build mage +// +build mage + +package main + +func Build() {} diff --git a/mage/testdata/with_magefiles_folder_and_mage_files_in_dot/magefile.go b/mage/testdata/with_magefiles_folder_and_mage_files_in_dot/magefile.go new file mode 100644 index 00000000..63584509 --- /dev/null +++ b/mage/testdata/with_magefiles_folder_and_mage_files_in_dot/magefile.go @@ -0,0 +1,6 @@ +//go:build mage +// +build mage + +package main + +func Build() {} diff --git a/mage/testdata/with_magefiles_folder_and_mage_files_in_dot/magefiles/mage_helpers.go b/mage/testdata/with_magefiles_folder_and_mage_files_in_dot/magefiles/mage_helpers.go new file mode 100644 index 00000000..e4b8872f --- /dev/null +++ b/mage/testdata/with_magefiles_folder_and_mage_files_in_dot/magefiles/mage_helpers.go @@ -0,0 +1,6 @@ +//go:build mage +// +build mage + +package main + +func foo() {} diff --git a/mage/testdata/with_magefiles_folder_and_mage_files_in_dot/magefiles/magefile.go b/mage/testdata/with_magefiles_folder_and_mage_files_in_dot/magefiles/magefile.go new file mode 100644 index 00000000..a465aafd --- /dev/null +++ b/mage/testdata/with_magefiles_folder_and_mage_files_in_dot/magefiles/magefile.go @@ -0,0 +1,6 @@ +//go:build mage +// +build mage + +package main + +func FolderBuild() {} diff --git a/mage/testdata/with_mixtagged_magefiles_folder/magefiles/mage_helpers.go b/mage/testdata/with_mixtagged_magefiles_folder/magefiles/mage_helpers.go new file mode 100644 index 00000000..e4b8872f --- /dev/null +++ b/mage/testdata/with_mixtagged_magefiles_folder/magefiles/mage_helpers.go @@ -0,0 +1,6 @@ +//go:build mage +// +build mage + +package main + +func foo() {} diff --git a/mage/testdata/with_mixtagged_magefiles_folder/magefiles/magefile.go b/mage/testdata/with_mixtagged_magefiles_folder/magefiles/magefile.go new file mode 100644 index 00000000..63584509 --- /dev/null +++ b/mage/testdata/with_mixtagged_magefiles_folder/magefiles/magefile.go @@ -0,0 +1,6 @@ +//go:build mage +// +build mage + +package main + +func Build() {} diff --git a/mage/testdata/with_mixtagged_magefiles_folder/magefiles/main.go b/mage/testdata/with_mixtagged_magefiles_folder/magefiles/main.go new file mode 100644 index 00000000..4c65d866 --- /dev/null +++ b/mage/testdata/with_mixtagged_magefiles_folder/magefiles/main.go @@ -0,0 +1,7 @@ +// some points here: this is the main pacakge just like the magefiles. This one +// has a main function, which could conflict with the generated main function we +// make (but clearly shouldn't cause problems). Finally, there's a duplicate function name here. + +package main + +func UntaggedBuild() {} diff --git a/mage/testdata/with_untagged_magefiles_folder/magefiles/mage_helpers.go b/mage/testdata/with_untagged_magefiles_folder/magefiles/mage_helpers.go new file mode 100644 index 00000000..3de84a3d --- /dev/null +++ b/mage/testdata/with_untagged_magefiles_folder/magefiles/mage_helpers.go @@ -0,0 +1,3 @@ +package main + +func foo() {} diff --git a/mage/testdata/with_untagged_magefiles_folder/magefiles/magefile.go b/mage/testdata/with_untagged_magefiles_folder/magefiles/magefile.go new file mode 100644 index 00000000..5660fe17 --- /dev/null +++ b/mage/testdata/with_untagged_magefiles_folder/magefiles/magefile.go @@ -0,0 +1,3 @@ +package main + +func Build() {} diff --git a/parse/parse.go b/parse/parse.go index 37df0317..14c5bcd9 100644 --- a/parse/parse.go +++ b/parse/parse.go @@ -225,8 +225,10 @@ func checkDupes(info *PkgInfo, imports []*Import) error { for _, f := range funcs[d] { ids = append(ids, f.ID()) } + sort.Strings(ids) errs = append(errs, fmt.Sprintf("%q target has multiple definitions: %s\n", d, strings.Join(ids, ", "))) } + sort.Strings(errs) return errors.New(strings.Join(errs, "\n")) } diff --git a/site/content/index.md b/site/content/index.md index 25bfe14a..ac0f860a 100644 --- a/site/content/index.md +++ b/site/content/index.md @@ -89,6 +89,13 @@ func Build() error { Run the above `Build` target by simply running `mage build` in the same directory as the magefile. +## Magefiles directory + +If you create your Magefile or files within a directory named `magefiles` And there is no Magefile in your current directory, +`mage` will default to the directory as the source for your targets while keeping the current directory as working one. + +The result is the equivalent of running `mage -d magefiles -w .` + ## Demo {{< youtube Hoga60EF_1U >}} diff --git a/site/content/magefiles/_index.en.md b/site/content/magefiles/_index.en.md index 6f908172..dcfb4bda 100644 --- a/site/content/magefiles/_index.en.md +++ b/site/content/magefiles/_index.en.md @@ -89,3 +89,17 @@ mage target: The first sentence in the comment will be the short help text shown with mage -l. The rest of the comment is long help text that will be shown with mage -h ``` + +### Magefiles directory + +Some development IDEs might work erratically with magefiles sharing location with other go files which very well might +have a different package name. + +To simplify development, if you place your Magefiles inside a `magefiles` directory, and none remains in your +main folder, the `mage` command will pick the directory as the source of your targets while keeping the current directory +as the working one. + +The `magefiles` directory does not require for you to add build tags to the files in it, it is optional, however we +encourage you to keep it consistent, either all or none. + +Effectively this is the equivalent of running: `mage -d magefiles -w .` \ No newline at end of file