From 6456342c1fad650e6271983b3617bb9500a600d9 Mon Sep 17 00:00:00 2001 From: Horacio Duran Date: Wed, 16 Feb 2022 22:32:34 +0100 Subject: [PATCH 01/11] Use magefolder if no directory set and it exists. If no directory was passed by the user as an explicit option and there is a folder named "magefolder" use that. Workdir is kept as it is likely still "." --- mage/main.go | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/mage/main.go b/mage/main.go index f57fdc69..785cd271 100644 --- a/mage/main.go +++ b/mage/main.go @@ -216,7 +216,7 @@ Commands: Options: -d - directory to read magefiles from (default ".") + directory to read magefiles from (default "." or "magefolder" 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,18 +296,38 @@ Options: return inv, cmd, err } +// Folder 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 Folder = "magefolder" +const dotFolder = "." + // 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 = dotFolder + // . will be default unless we find a mage folder. + mfSt, err := os.Stat(Folder) + if err != nil { + if mfSt.IsDir() { + inv.Dir = Folder + } + } } + if inv.WorkDir == "" { - inv.WorkDir = inv.Dir + if noDir { + inv.WorkDir = dotFolder + } else { + inv.WorkDir = inv.Dir + } } + if inv.CacheDir == "" { inv.CacheDir = mg.CacheDir() } From 4408a5489a4af0425c7aa4bef7efa17aeeb20250 Mon Sep 17 00:00:00 2001 From: Horacio Duran Date: Wed, 16 Feb 2022 22:50:45 +0100 Subject: [PATCH 02/11] Remove the default . for -d flag Also correct os.Stat error checking to expect no error --- mage/main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mage/main.go b/mage/main.go index 785cd271..0a53b707 100644 --- a/mage/main.go +++ b/mage/main.go @@ -180,7 +180,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") @@ -313,7 +313,7 @@ func Invoke(inv Invocation) int { inv.Dir = dotFolder // . will be default unless we find a mage folder. mfSt, err := os.Stat(Folder) - if err != nil { + if err == nil { if mfSt.IsDir() { inv.Dir = Folder } From 4385834ede628254fd2470867830d5cf99665b46 Mon Sep 17 00:00:00 2001 From: Horacio Duran Date: Wed, 16 Feb 2022 23:04:58 +0100 Subject: [PATCH 03/11] Add tests and test data for magefolder --- mage/main_test.go | 34 ++++++++++++++++++- .../magefolder/mage_helpers.go | 6 ++++ .../with_magefolder/magefolder/magefile.go | 6 ++++ 3 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 mage/testdata/with_magefolder/magefolder/mage_helpers.go create mode 100644 mage/testdata/with_magefolder/magefolder/magefile.go diff --git a/mage/main_test.go b/mage/main_test.go index d0b43d84..6b222648 100644 --- a/mage/main_test.go +++ b/mage/main_test.go @@ -342,6 +342,38 @@ func TestMixedMageImports(t *testing.T) { } } +func TestMagefolder(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_magefolder"); 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 TestGoRun(t *testing.T) { c := exec.Command("go", "run", "main.go") c.Dir = "./testdata" @@ -1615,7 +1647,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_magefolder/magefolder/mage_helpers.go b/mage/testdata/with_magefolder/magefolder/mage_helpers.go new file mode 100644 index 00000000..e4b8872f --- /dev/null +++ b/mage/testdata/with_magefolder/magefolder/mage_helpers.go @@ -0,0 +1,6 @@ +//go:build mage +// +build mage + +package main + +func foo() {} diff --git a/mage/testdata/with_magefolder/magefolder/magefile.go b/mage/testdata/with_magefolder/magefolder/magefile.go new file mode 100644 index 00000000..63584509 --- /dev/null +++ b/mage/testdata/with_magefolder/magefolder/magefile.go @@ -0,0 +1,6 @@ +//go:build mage +// +build mage + +package main + +func Build() {} From 6a99d937d4169d59d2bd8c1ad0531c51307ea404 Mon Sep 17 00:00:00 2001 From: Horacio Duran Date: Thu, 17 Feb 2022 22:28:38 +0100 Subject: [PATCH 04/11] Rename magefolder and accept untagged files Magefolder was renamed to magefiles We now accept files that are not tagged too when using a magefiles directory --- mage/main.go | 107 ++++++++++-------- mage/main_test.go | 44 ++++++- .../magefiles}/mage_helpers.go | 0 .../magefiles}/magefile.go | 0 .../magefiles/mage_helpers.go | 3 + .../magefiles/magefile.go | 3 + 6 files changed, 106 insertions(+), 51 deletions(-) rename mage/testdata/{with_magefolder/magefolder => with_magefiles_folder/magefiles}/mage_helpers.go (100%) rename mage/testdata/{with_magefolder/magefolder => with_magefiles_folder/magefiles}/magefile.go (100%) create mode 100644 mage/testdata/with_untagged_magefiles_folder/magefiles/mage_helpers.go create mode 100644 mage/testdata/with_untagged_magefiles_folder/magefiles/magefile.go diff --git a/mage/main.go b/mage/main.go index 0a53b707..45297afb 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). @@ -216,7 +225,7 @@ Commands: Options: -d - directory to read magefiles from (default "." or "magefolder" if exists) + 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,10 +305,7 @@ Options: return inv, cmd, err } -// Folder 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 Folder = "magefolder" -const dotFolder = "." +const dotDirectory = "." // Invoke runs Mage with the given arguments. func Invoke(inv Invocation) int { @@ -310,19 +316,19 @@ func Invoke(inv Invocation) int { var noDir bool if inv.Dir == "" { noDir = true - inv.Dir = dotFolder + inv.Dir = dotDirectory // . will be default unless we find a mage folder. - mfSt, err := os.Stat(Folder) + mfSt, err := os.Stat(MagefilesDirName) if err == nil { if mfSt.IsDir() { - inv.Dir = Folder + inv.Dir = MagefilesDirName } } } if inv.WorkDir == "" { if noDir { - inv.WorkDir = dotFolder + inv.WorkDir = dotDirectory } else { inv.WorkDir = inv.Dir } @@ -332,7 +338,7 @@ func Invoke(inv Invocation) int { 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 @@ -452,26 +458,13 @@ type mainfileTemplateData struct { BinaryName string } -// Magefiles returns the list of magefiles in dir. -func Magefiles(magePath, goos, goarch, goCmd string, stderr io.Writer, 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 +func listGoFiles(magePath, goCmd, tags string, env []string) ([]string, error) { + args := []string{"list"} + if tags != "" { + args = append(args, fmt.Sprintf("-tags=%s", tags)) } - - debug.Println("getting all non-mage files in", magePath) - - // // 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 "||"}}`) + args = append(args, "-e", "-f", `{{join .GoFiles "||"}}`) + cmd := exec.Command(goCmd, args...) cmd.Env = env buf := &bytes.Buffer{} cmd.Stderr = buf @@ -482,32 +475,56 @@ func Magefiles(magePath, goos, goarch, goCmd string, stderr io.Writer, isDebug b // 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)) + 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) } } list := strings.TrimSpace(string(b)) - debug.Println("found non-mage files", list) + return strings.Split(list, "||"), nil +} + +// Magefiles returns the list of magefiles in dir. +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)) + }() + + env, err := internal.EnvWithGOOS(goos, goarch) + if err != nil { + return nil, err + } + + if !isMagefilesDirectory { + debug.Println("getting all non-mage files in", magePath) + } + + // // first, grab all the files with no build tags specified.. this is actually + // // our exclude list of things without the mage build tag. exclude := map[string]bool{} - for _, f := range strings.Split(list, "||") { - if f != "" { - debug.Printf("marked file as non-mage: %q", f) - exclude[f] = true + if !isMagefilesDirectory { + goFiles, err := listGoFiles(magePath, goCmd, "", env) + if err != nil { + return nil, fmt.Errorf("listing non-mage files: %w", err) + } + + for _, f := range goFiles { + 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() + goFiles, err := listGoFiles(magePath, goCmd, "mage", env) if err != nil { - return fail(fmt.Errorf("failed to list mage gofiles: %v: %s", err, buf.Bytes())) + return nil, fmt.Errorf("listing mage files: %w", err) } - list = strings.TrimSpace(string(b)) - files := []string{} - for _, f := range strings.Split(list, "||") { + var files []string + for _, f := range goFiles { if f != "" && !exclude[f] { files = append(files, f) } diff --git a/mage/main_test.go b/mage/main_test.go index 6b222648..04d77952 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,14 +342,46 @@ func TestMixedMageImports(t *testing.T) { } } -func TestMagefolder(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_magefolder"); err != nil { + 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 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 diff --git a/mage/testdata/with_magefolder/magefolder/mage_helpers.go b/mage/testdata/with_magefiles_folder/magefiles/mage_helpers.go similarity index 100% rename from mage/testdata/with_magefolder/magefolder/mage_helpers.go rename to mage/testdata/with_magefiles_folder/magefiles/mage_helpers.go diff --git a/mage/testdata/with_magefolder/magefolder/magefile.go b/mage/testdata/with_magefiles_folder/magefiles/magefile.go similarity index 100% rename from mage/testdata/with_magefolder/magefolder/magefile.go rename to mage/testdata/with_magefiles_folder/magefiles/magefile.go 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() {} From 6e39f5481294b0d4bf8fe0c3b863ef22fffe92fa Mon Sep 17 00:00:00 2001 From: Horacio Duran Date: Thu, 17 Feb 2022 22:37:44 +0100 Subject: [PATCH 05/11] Assume tagging when mix tagging is present When using magefiles directory, if there are mixed tagging files assume tagging is used for mage files --- mage/main.go | 26 ++++++++------- mage/main_test.go | 32 +++++++++++++++++++ .../magefiles/mage_helpers.go | 6 ++++ .../magefiles/magefile.go | 6 ++++ .../magefiles/main.go | 9 ++++++ 5 files changed, 68 insertions(+), 11 deletions(-) create mode 100644 mage/testdata/with_mixtagged_magefiles_folder/magefiles/mage_helpers.go create mode 100644 mage/testdata/with_mixtagged_magefiles_folder/magefiles/magefile.go create mode 100644 mage/testdata/with_mixtagged_magefiles_folder/magefiles/main.go diff --git a/mage/main.go b/mage/main.go index 45297afb..a937f2c3 100644 --- a/mage/main.go +++ b/mage/main.go @@ -504,25 +504,29 @@ func Magefiles(magePath, goos, goarch, goCmd string, stderr io.Writer, isMagefil // // first, grab all the files with no build tags specified.. this is actually // // our exclude list of things without the mage build tag. exclude := map[string]bool{} - if !isMagefilesDirectory { - goFiles, err := listGoFiles(magePath, goCmd, "", env) - if err != nil { - return nil, fmt.Errorf("listing non-mage files: %w", err) - } - for _, f := range goFiles { - if f != "" { - debug.Printf("marked file as non-mage: %q", f) - exclude[f] = true - } + goFiles, err := listGoFiles(magePath, goCmd, "", env) + if err != nil { + return nil, fmt.Errorf("listing non-mage files: %w", err) + } + + for _, f := range goFiles { + if f != "" { + debug.Printf("marked file as non-mage: %q", f) + exclude[f] = true } } - goFiles, err := listGoFiles(magePath, goCmd, "mage", env) + goFiles, err = listGoFiles(magePath, goCmd, "mage", env) if err != nil { return nil, fmt.Errorf("listing mage files: %w", err) } + // there are only mage tagged files + if len(exclude) == len(goFiles) && isMagefilesDirectory { + exclude = map[string]bool{} + } + var files []string for _, f := range goFiles { if f != "" && !exclude[f] { diff --git a/mage/main_test.go b/mage/main_test.go index 04d77952..1780fff1 100644 --- a/mage/main_test.go +++ b/mage/main_test.go @@ -406,6 +406,38 @@ func TestUntaggedMagefilesFolder(t *testing.T) { } } +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" + 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" 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..5bcb3b0d --- /dev/null +++ b/mage/testdata/with_mixtagged_magefiles_folder/magefiles/main.go @@ -0,0 +1,9 @@ +// 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 main() {} + +func Build() {} From b68587f92f7746af6813de631ce7d25ce08ff173 Mon Sep 17 00:00:00 2001 From: Horacio Duran Date: Mon, 21 Feb 2022 16:51:26 +0100 Subject: [PATCH 06/11] Update error format to %v We support building for older go versions so error formatting should use %v --- mage/main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mage/main.go b/mage/main.go index a937f2c3..d89c37ea 100644 --- a/mage/main.go +++ b/mage/main.go @@ -507,7 +507,7 @@ func Magefiles(magePath, goos, goarch, goCmd string, stderr io.Writer, isMagefil goFiles, err := listGoFiles(magePath, goCmd, "", env) if err != nil { - return nil, fmt.Errorf("listing non-mage files: %w", err) + return nil, fmt.Errorf("listing non-mage files: %v", err) } for _, f := range goFiles { @@ -519,7 +519,7 @@ func Magefiles(magePath, goos, goarch, goCmd string, stderr io.Writer, isMagefil goFiles, err = listGoFiles(magePath, goCmd, "mage", env) if err != nil { - return nil, fmt.Errorf("listing mage files: %w", err) + return nil, fmt.Errorf("listing mage files: %v", err) } // there are only mage tagged files From 3ab41f0de311812adea61fa91aa8f54fd7e66a34 Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Fri, 11 Mar 2022 12:22:10 -0500 Subject: [PATCH 07/11] sort outputs --- parse/parse.go | 2 ++ 1 file changed, 2 insertions(+) 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")) } From 1c2ef11594119302a276a5a1bead8fc0a9e44dd0 Mon Sep 17 00:00:00 2001 From: Horacio Duran Date: Fri, 11 Mar 2022 18:49:41 +0100 Subject: [PATCH 08/11] Accept mixed tagging in magefiles folder When mixed tagging is found within a magefiles folder, opt to use all files --- mage/main.go | 8 ++++---- mage/main_test.go | 2 +- .../with_mixtagged_magefiles_folder/magefiles/main.go | 4 +--- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/mage/main.go b/mage/main.go index d89c37ea..0fc79967 100644 --- a/mage/main.go +++ b/mage/main.go @@ -501,8 +501,8 @@ func Magefiles(magePath, goos, goarch, goCmd string, stderr io.Writer, isMagefil debug.Println("getting all non-mage files in", magePath) } - // // first, grab all the files with no build tags specified.. this is actually - // // our exclude list of things without the mage build tag. + // first, grab all the files with no build tags specified.. + // this is actually our exclude list of things without the mage build tag. exclude := map[string]bool{} goFiles, err := listGoFiles(magePath, goCmd, "", env) @@ -522,8 +522,8 @@ func Magefiles(magePath, goos, goarch, goCmd string, stderr io.Writer, isMagefil return nil, fmt.Errorf("listing mage files: %v", err) } - // there are only mage tagged files - if len(exclude) == len(goFiles) && isMagefilesDirectory { + // we accept mixed tagging within a magefiles folder, all files are used. + if isMagefilesDirectory { exclude = map[string]bool{} } diff --git a/mage/main_test.go b/mage/main_test.go index 1780fff1..aee37eb6 100644 --- a/mage/main_test.go +++ b/mage/main_test.go @@ -431,7 +431,7 @@ func TestMixedTaggingMagefilesFolder(t *testing.T) { if code != 0 { t.Errorf("expected to exit with code 0, but got %v, stderr: %s", code, stderr) } - expected := "Targets:\n build \n" + expected := "Targets:\n build \n untaggedBuild \n" actual := stdout.String() if actual != expected { t.Fatalf("expected %q but got %q", expected, actual) diff --git a/mage/testdata/with_mixtagged_magefiles_folder/magefiles/main.go b/mage/testdata/with_mixtagged_magefiles_folder/magefiles/main.go index 5bcb3b0d..4c65d866 100644 --- a/mage/testdata/with_mixtagged_magefiles_folder/magefiles/main.go +++ b/mage/testdata/with_mixtagged_magefiles_folder/magefiles/main.go @@ -4,6 +4,4 @@ package main -func main() {} - -func Build() {} +func UntaggedBuild() {} From c9f018de0a0cac8796c6758a1471174c931069ec Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Fri, 11 Mar 2022 13:23:01 -0500 Subject: [PATCH 09/11] little tweak to only do go list once when using magefiles directory --- mage/main.go | 49 ++++++++++++++++++++++++++----------------------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/mage/main.go b/mage/main.go index 0fc79967..fba2f654 100644 --- a/mage/main.go +++ b/mage/main.go @@ -481,8 +481,12 @@ func listGoFiles(magePath, goCmd, tags string, env []string) ([]string, error) { return nil, fmt.Errorf("failed to list gofiles tagged with %q: %v: %s", tags, err, stderr) } } - list := strings.TrimSpace(string(b)) - return strings.Split(list, "||"), nil + 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. @@ -497,45 +501,44 @@ func Magefiles(magePath, goos, goarch, goCmd string, stderr io.Writer, isMagefil return nil, err } - if !isMagefilesDirectory { - 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. - exclude := map[string]bool{} + 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. - goFiles, err := listGoFiles(magePath, goCmd, "", env) + debug.Println("getting all files without mage tag in", magePath) + nonMageFiles, err := listGoFiles(magePath, goCmd, "", env) if err != nil { return nil, fmt.Errorf("listing non-mage files: %v", err) } - for _, f := range goFiles { + // convert non-Mage list to a map of files to exclude. + exclude := map[string]bool{} + for _, f := range nonMageFiles { if f != "" { debug.Printf("marked file as non-mage: %q", f) exclude[f] = true } } - goFiles, err = listGoFiles(magePath, goCmd, "mage", env) - if err != nil { - return nil, fmt.Errorf("listing mage files: %v", err) - } - - // we accept mixed tagging within a magefiles folder, all files are used. - if isMagefilesDirectory { - exclude = map[string]bool{} - } - + // filter out the non-mage files from the mage files. var files []string - for _, f := range goFiles { + 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 } From ec2df47d1f5eb958d07baba8b52c04166cc41fba Mon Sep 17 00:00:00 2001 From: Horacio Duran Date: Fri, 11 Mar 2022 20:19:53 +0100 Subject: [PATCH 10/11] Add magefiles directory information to the website --- site/content/index.md | 7 +++++++ site/content/magefiles/_index.en.md | 14 ++++++++++++++ 2 files changed, 21 insertions(+) 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 From 00c7860f378341137a16c807ebe9982551ba465a Mon Sep 17 00:00:00 2001 From: Horacio Duran Date: Sat, 12 Mar 2022 14:34:57 +0100 Subject: [PATCH 11/11] Add a preference for mage files over directories Add a temporary preference for mage files over magefiles directories and warn users this is a temporary functionality leading to a change where directory will be preferred. --- mage/main.go | 12 +++++- mage/main_test.go | 38 +++++++++++++++++++ .../magefile.go | 6 +++ .../magefiles/mage_helpers.go | 6 +++ .../magefiles/magefile.go | 6 +++ 5 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 mage/testdata/with_magefiles_folder_and_mage_files_in_dot/magefile.go create mode 100644 mage/testdata/with_magefiles_folder_and_mage_files_in_dot/magefiles/mage_helpers.go create mode 100644 mage/testdata/with_magefiles_folder_and_mage_files_in_dot/magefiles/magefile.go diff --git a/mage/main.go b/mage/main.go index fba2f654..a41e7435 100644 --- a/mage/main.go +++ b/mage/main.go @@ -321,7 +321,17 @@ func Invoke(inv Invocation) int { mfSt, err := os.Stat(MagefilesDirName) if err == nil { if mfSt.IsDir() { - inv.Dir = MagefilesDirName + 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 + } + } } } } diff --git a/mage/main_test.go b/mage/main_test.go index aee37eb6..2074c64a 100644 --- a/mage/main_test.go +++ b/mage/main_test.go @@ -374,6 +374,44 @@ func TestMagefilesFolder(t *testing.T) { } } +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() 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() {}