Repository: magefile/mage
Pull request #97 (2017-12-28 21:47:26)

Teach mage clean
Addresses https://github.com/magefile/mage/issues/19 

Starts the refactor of `-init` and `-version` into commands.

natefinch: Thanks for this!  Is it ready for review, or are you still working?  Let me know.  
Zanadar: It should be ready for a long look. Thanks!
On Fri, Dec 29, 2017 at 7:56 AM Nate Finch <notifications@github.com> wrote:

> Thanks for this! Is it ready for review, or are you still working? Let me
> know.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/magefile/mage/pull/97#issuecomment-354442709>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AFpp4zW5vX5Yy5kUcOVW2Tw9cAJ6lu6Iks5tFOFngaJpZM4ROrLY>
> .
>

Zanadar: @natefinch Updated per your comments.  I wonder if I might leave the updating of `list` to a command to a separate PR to keep this PR focussed? 
Zanadar: @natefinch Anything I should do here to see this get to master?
Zanadar: Cool! Made some changes per your feedback. LMK what you think. Thanks for your dutiful attention to this humble PR!
natefinch: LGTM, I'll merge once the tests pass

Patch:
diff --git a/mage/command_string.go b/mage/command_string.go
new file mode 100644
index 0000000..14dbd26
--- /dev/null
+++ b/mage/command_string.go
@@ -0,0 +1,16 @@
+// Code generated by "stringer -type=Command"; DO NOT EDIT.
+
+package mage
+
+import "fmt"
+
+const _Command_name = "NoneVersionInitClean"
+
+var _Command_index = [...]uint8{0, 4, 11, 15, 20}
+
+func (i Command) String() string {
+	if i < 0 || i >= Command(len(_Command_index)-1) {
+		return fmt.Sprintf("Command(%d)", i)
+	}
+	return _Command_name[_Command_index[i]:_Command_index[i+1]]
+}
diff --git a/mage/main.go b/mage/main.go
index 1021c6e..735bb21 100644
--- a/mage/main.go
+++ b/mage/main.go
@@ -6,6 +6,7 @@ import (
 	"flag"
 	"fmt"
 	"io"
+	"io/ioutil"
 	"log"
 	"os"
 	"os/exec"
@@ -49,6 +50,18 @@ var (
 	gitTag     = "v2"
 )
 
+//go:generate stringer -type=Command
+
+// Command tracks invocations of mage that run without targets or other flags.
+type Command int
+
+const (
+	None    Command = iota
+	Version         // report the current version of mage
+	Init            // create a starting template for mage
+	Clean           // clean out old compiled mage binaries from the cache
+)
+
 // Main is the entrypoint for running mage.  It exists external to mage's main
 // function to allow it to be used from other programs, specifically so you can
 // go run a simple file that run's mage's Main.
@@ -76,7 +89,7 @@ type Invocation struct {
 // name in the args).
 func ParseAndRun(dir string, stdout, stderr io.Writer, stdin io.Reader, args []string) int {
 	log := log.New(stderr, "", 0)
-	inv, mageInit, showVersion, err := Parse(stdout, args)
+	inv, cmd, err := Parse(stdout, args)
 	inv.Dir = dir
 	inv.Stderr = stderr
 	inv.Stdin = stdin
@@ -88,7 +101,8 @@ func ParseAndRun(dir string, stdout, stderr io.Writer, stdin io.Reader, args []s
 		return 2
 	}
 
-	if showVersion {
+	switch cmd {
+	case Version:
 		if timestamp == "" {
 			timestamp = "<not set>"
 		}
@@ -99,22 +113,31 @@ func ParseAndRun(dir string, stdout, stderr io.Writer, stdin io.Reader, args []s
 		log.Println("Build Date:", timestamp)
 		log.Println("Commit:", commitHash)
 		return 0
-	}
-	if mageInit {
+	case Init:
 		if err := generateInit(dir); err != nil {
 			log.Println("Error:", err)
 			return 1
 		}
 		log.Println(initFile, "created")
 		return 0
+	case Clean:
+		dir := mg.CacheDir()
+		if err := removeContents(dir); err != nil {
+			log.Println("Error:", err)
+			return 1
+		}
+		log.Println(dir, "cleaned")
+		return 0
+	case None:
+		return Invoke(inv)
+	default:
+		panic(fmt.Errorf("Unknown command type: %v", cmd))
 	}
-
-	return Invoke(inv)
 }
 
 // Parse parses the given args and returns structured data.  If parse returns
 // flag.ErrHelp, the calling process should exit with code 0.
-func Parse(stdout io.Writer, args []string) (inv Invocation, mageInit, showVersion bool, err error) {
+func Parse(stdout io.Writer, args []string) (inv Invocation, cmd Command, err error) {
 	inv.Stdout = stdout
 	fs := flag.FlagSet{}
 	fs.SetOutput(stdout)
@@ -122,10 +145,15 @@ func Parse(stdout io.Writer, args []string) (inv Invocation, mageInit, showVersi
 	fs.BoolVar(&inv.Verbose, "v", false, "show verbose output when running mage targets")
 	fs.BoolVar(&inv.List, "l", false, "list mage targets in this directory")
 	fs.BoolVar(&inv.Help, "h", false, "show this help")
-	fs.BoolVar(&mageInit, "init", false, "create a starting template if no mage files exist")
 	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")
+	var showVersion bool
 	fs.BoolVar(&showVersion, "version", false, "show version info for the mage binary")
+	var mageInit bool
+	fs.BoolVar(&mageInit, "init", false, "create a starting template if no mage files exist")
+	var clean bool
+	fs.BoolVar(&clean, "clean", false, "clean out old generated binaries from CACHE_DIR")
+
 	fs.Usage = func() {
 		fmt.Fprintln(stdout, "mage [options] [target]")
 		fmt.Fprintln(stdout, "Options:")
@@ -134,12 +162,33 @@ func Parse(stdout io.Writer, args []string) (inv Invocation, mageInit, showVersi
 	err = fs.Parse(args)
 	if err == flag.ErrHelp {
 		// parse will have already called fs.Usage()
-		return inv, mageInit, showVersion, err
+		return inv, cmd, err
 	}
 	if err == nil && inv.Help && len(fs.Args()) == 0 {
 		fs.Usage()
 		// tell upstream, to just exit
-		return inv, mageInit, showVersion, flag.ErrHelp
+		return inv, cmd, flag.ErrHelp
+	}
+
+	numFlags := 0
+	switch {
+	case mageInit:
+		numFlags++
+		cmd = Init
+	case showVersion:
+		numFlags++
+		cmd = Version
+	case clean:
+		numFlags++
+		cmd = Clean
+		if fs.NArg() > 0 || fs.NFlag() > 1 {
+			// Temporary dupe of below check until we refactor the other commands to use this check
+			return inv, cmd, errors.New("-h, -init, -clean, and -version cannot be used simultaneously")
+
+		}
+	}
+	if inv.Help {
+		numFlags++
 	}
 
 	// If verbose is still false, we're going to peek at the environment variable to see if
@@ -150,27 +199,17 @@ func Parse(stdout io.Writer, args []string) (inv Invocation, mageInit, showVersi
 			inv.Verbose = envVerbose
 		}
 	}
-	numFlags := 0
-	if inv.Help {
-		numFlags++
-	}
-	if mageInit {
-		numFlags++
-	}
-	if showVersion {
-		numFlags++
-	}
 
 	if numFlags > 1 {
-		return inv, mageInit, showVersion, errors.New("-h, -init, and -version cannot be used simultaneously")
+		return inv, cmd, errors.New("-h, -init, -clean, and -version cannot be used simultaneously")
 	}
 
 	inv.Args = fs.Args()
 	if inv.Help && len(inv.Args) > 1 {
-		return inv, mageInit, showVersion, errors.New("-h can only show help for a single target")
+		return inv, cmd, errors.New("-h can only show help for a single target")
 	}
 
-	return inv, mageInit, showVersion, err
+	return inv, cmd, err
 }
 
 // Invoke runs Mage with the given arguments.
@@ -401,3 +440,20 @@ func RunCompiled(inv Invocation, exePath string) int {
 	}
 	return sh.ExitStatus(c.Run())
 }
+
+func removeContents(dir string) error {
+	files, err := ioutil.ReadDir(dir)
+	if err != nil {
+		return err
+	}
+	for _, f := range files {
+		if f.IsDir() {
+			continue
+		}
+		err = os.Remove(filepath.Join(dir, f.Name()))
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}


Test patch:
diff --git a/mage/main_test.go b/mage/main_test.go
index bdea160..d5f4f12 100644
--- a/mage/main_test.go
+++ b/mage/main_test.go
@@ -98,7 +98,7 @@ func TestVerboseEnv(t *testing.T) {
 	os.Setenv("MAGE_VERBOSE", "true")
 
 	stdout := &bytes.Buffer{}
-	inv, _, _, err := Parse(stdout, []string{})
+	inv, _, err := Parse(stdout, []string{})
 	if err != nil {
 		t.Fatal("unexpected error", err)
 	}
@@ -420,14 +420,14 @@ func TestBadSecondTargets(t *testing.T) {
 
 func TestParse(t *testing.T) {
 	buf := &bytes.Buffer{}
-	inv, init, showVer, err := Parse(buf, []string{"-v", "build"})
+	inv, cmd, err := Parse(buf, []string{"-v", "build"})
 	if err != nil {
 		t.Fatal("unexpected error", err)
 	}
-	if init {
+	if cmd == Init {
 		t.Fatal("init should be false but was true")
 	}
-	if showVer {
+	if cmd == Version {
 		t.Fatal("showVersion should be false but was true")
 	}
 	if len(inv.Args) != 1 && inv.Args[0] != "build" {
@@ -462,12 +462,12 @@ func TestTimeout(t *testing.T) {
 }
 func TestParseHelp(t *testing.T) {
 	buf := &bytes.Buffer{}
-	_, _, _, err := Parse(buf, []string{"-h"})
+	_, _, err := Parse(buf, []string{"-h"})
 	if err != flag.ErrHelp {
 		t.Fatal("unexpected error", err)
 	}
 	buf2 := &bytes.Buffer{}
-	_, _, _, err = Parse(buf2, []string{"--help"})
+	_, _, err = Parse(buf2, []string{"--help"})
 	if err != flag.ErrHelp {
 		t.Fatal("unexpected error", err)
 	}
@@ -582,3 +582,40 @@ func TestInvalidAlias(t *testing.T) {
 		t.Fatalf("expected %q, but got %q", expected, actual)
 	}
 }
+
+func TestClean(t *testing.T) {
+	TestGoRun(t) // make sure we've got something in the CACHE_DIR
+	dir := "./testing"
+	abs, err := filepath.Abs(dir)
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	files, err := ioutil.ReadDir(abs)
+	if err != nil {
+		t.Error("issue reading file:", err)
+	}
+
+	if len(files) < 1 {
+		t.Error("Need at least 1 cached binaries to test --clean")
+	}
+
+	buf := &bytes.Buffer{}
+	_, cmd, err := Parse(buf, []string{"-clean"})
+	if cmd != Clean {
+		t.Errorf("Expected 'clean' command but got %v", cmd)
+	}
+	code := ParseAndRun(dir, os.Stdin, os.Stderr, buf, []string{"-clean"})
+	if code != 0 {
+		t.Errorf("expected 0, but got %v", code)
+	}
+
+	files, err = ioutil.ReadDir(abs)
+	if err != nil {
+		t.Error("issue reading file:", err)
+	}
+
+	if len(files) != 0 {
+		t.Errorf("expected '-clean' to remove files from CACHE_DIR, but still have %v", files)
+	}
+}