diff --git a/mage/main.go b/mage/main.go index 0062bd35..6f731733 100644 --- a/mage/main.go +++ b/mage/main.go @@ -119,6 +119,10 @@ type Invocation struct { GoCmd string // the go binary command to run CacheDir string // the directory where we should store compiled binaries HashFast bool // don't rely on GOCACHE, just hash the magefiles + + // all the arguments that came after --, we do not do any kind of parsing on them as + // they are intended for a subprocess and we know nothing about them + ExtraArgs []string } // MagefilesDirName is the name of the default folder to look for if no directory was specified, @@ -296,7 +300,23 @@ Options: return inv, cmd, errors.New("-goos and -goarch only apply when running with -compile") } - inv.Args = fs.Args() + var extraArgs []string + extraArgsFound := false + for _, arg := range fs.Args() { + // we do not really care about args before this point, this is where extra args begin + if arg == "--" { + extraArgsFound = true + continue + } + // all args before -- are the positional args that go to mage + if !extraArgsFound { + inv.Args = append(inv.Args, arg) + continue + } + // now we parse all that comes after -- + extraArgs = append(extraArgs, arg) + } + inv.ExtraArgs = extraArgs if inv.Help && len(inv.Args) > 1 { return inv, cmd, errors.New("-h can only show help for a single target") } @@ -704,7 +724,13 @@ func generateInit(dir string) error { // RunCompiled runs an already-compiled mage command with the given args, func RunCompiled(inv Invocation, exePath string, errlog *log.Logger) int { debug.Println("running binary", exePath) - c := exec.Command(exePath, inv.Args...) + args := make([]string, len(inv.Args), len(inv.Args)+len(inv.ExtraArgs)+1) + copy(args, inv.Args) + if len(inv.ExtraArgs) > 0 { + args = append(append(args, "--"), inv.ExtraArgs...) + } + + c := exec.Command(exePath, args...) c.Stderr = inv.Stderr c.Stdout = inv.Stdout c.Stdin = inv.Stdin diff --git a/mage/main_test.go b/mage/main_test.go index 8b21fb4b..22c7c5c1 100644 --- a/mage/main_test.go +++ b/mage/main_test.go @@ -1877,6 +1877,268 @@ func TestWrongDependency(t *testing.T) { } } +func TestExtraArgs(t *testing.T) { + stdout := &bytes.Buffer{} + inv := Invocation{ + Dir: "./testdata/extra_args", + Stderr: ioutil.Discard, + Stdout: stdout, + Args: []string{"targetOne"}, + ExtraArgs: []string{"--", "-baz", "foo", "bar"}, + } + + code := Invoke(inv) + if code != 0 { + t.Fatalf("expected 0, but got %v", code) + } + expected := "mg.ExtraArgs{\"-baz\", \"foo\", \"bar\"}\n" + if stdout.String() != expected { + t.Fatalf("expected %q, but got %q", expected, stdout.String()) + } +} + +func TestExtraArgsParsing(t *testing.T) { + os.Setenv("MAGEFILE_VERBOSE", "1") + defer os.Unsetenv("MAGEFILE_VERBOSE") + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + code := ParseAndRun(stderr, stdout, nil, + []string{"-d", "./testdata/extra_args", "targetOne", "--", "-baz", "foo", "bar"}) + if code != 0 { + t.Fatal("unexpected code", code) + } + + targetOneExpectedOut := "Running target: TargetOne\n" + if stdout.String() != targetOneExpectedOut { + t.Fatalf("expected output %q, but got %q", targetOneExpectedOut, stdout.String()) + } + targetOneExpectedErr := `mg.ExtraArgs{"-baz", "foo", "bar"} +` + if stderr.String() != targetOneExpectedErr { + t.Fatalf("expected stderr output %q, but got %q", targetOneExpectedErr, stderr.String()) + } +} + +func TestExtraArgsWithContext(t *testing.T) { + stdout := &bytes.Buffer{} + inv := Invocation{ + Dir: "./testdata/extra_args", + Stderr: ioutil.Discard, + Stdout: stdout, + Args: []string{"targetTwo"}, + ExtraArgs: []string{"--", "-baz", "foo", "bar"}, + } + + code := Invoke(inv) + if code != 0 { + t.Fatalf("expected 0, but got %v", code) + } + expected := "Context is nil: false\nmg.ExtraArgs{\"-baz\", \"foo\", \"bar\"}\n" + if stdout.String() != expected { + t.Fatalf("expected %q, but got %q", expected, stdout.String()) + } +} + +func TestExtraArgsWithContextParsing(t *testing.T) { + os.Setenv("MAGEFILE_VERBOSE", "1") + defer os.Unsetenv("MAGEFILE_VERBOSE") + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + code := ParseAndRun(stderr, stdout, nil, + []string{"-d", "./testdata/extra_args", "targetTwo", "--", "-baz", "foo", "bar"}) + if code != 0 { + t.Fatal("unexpected code", code) + } + + targetOneExpectedOut := "Running target: TargetTwo\n" + if stdout.String() != targetOneExpectedOut { + t.Fatalf("expected output %q, but got %q", targetOneExpectedOut, stdout.String()) + } + targetOneExpectedErr := `Context is nil: false +mg.ExtraArgs{"-baz", "foo", "bar"} +` + if stderr.String() != targetOneExpectedErr { + t.Fatalf("expected stderr output %q, but got %q", targetOneExpectedErr, stderr.String()) + } +} + +func TestExtraArgsWithContextAndString(t *testing.T) { + stdout := &bytes.Buffer{} + inv := Invocation{ + Dir: "./testdata/extra_args", + Stderr: ioutil.Discard, + Stdout: stdout, + Args: []string{"targetThree", "stringvalue"}, + ExtraArgs: []string{"--", "-baz", "foo", "bar"}, + } + + code := Invoke(inv) + if code != 0 { + t.Fatalf("expected 0, but got %v", code) + } + expected := "Context is nil: false\nmg.ExtraArgs{\"-baz\", \"foo\", \"bar\"}\nstringvalue\n" + if stdout.String() != expected { + t.Fatalf("expected %q, but got %q", expected, stdout.String()) + } +} + +func TestExtraArgsWithContextAndStringParsing(t *testing.T) { + os.Setenv("MAGEFILE_VERBOSE", "1") + defer os.Unsetenv("MAGEFILE_VERBOSE") + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + code := ParseAndRun(stderr, stdout, nil, + []string{"-d", "./testdata/extra_args", "targetThree", "stringvalue", "--", "-baz", "foo", "bar"}) + if code != 0 { + t.Fatal("unexpected code", code) + } + + targetOneExpectedOut := "Running target: TargetThree\n" + if stdout.String() != targetOneExpectedOut { + t.Fatalf("expected output %q, but got %q", targetOneExpectedOut, stdout.String()) + } + targetOneExpectedErr := `Context is nil: false +mg.ExtraArgs{"-baz", "foo", "bar"} +stringvalue +` + if stderr.String() != targetOneExpectedErr { + t.Fatalf("expected stderr output %q, but got %q", targetOneExpectedErr, stderr.String()) + } +} + +func TestExtraArgsWithContextAndStringAndInt(t *testing.T) { + stdout := &bytes.Buffer{} + inv := Invocation{ + Dir: "./testdata/extra_args", + Stderr: ioutil.Discard, + Stdout: stdout, + Args: []string{"targetFour", "stringvalue", "2"}, + ExtraArgs: []string{"--", "-baz", "foo", "bar"}, + } + + code := Invoke(inv) + if code != 0 { + t.Fatalf("expected 0, but got %v", code) + } + expected := "Context is nil: false\nmg.ExtraArgs{\"-baz\", \"foo\", \"bar\"}\nstringvalue\n2\n" + if stdout.String() != expected { + t.Fatalf("expected %q, but got %q", expected, stdout.String()) + } +} + +func TestExtraArgsWithContextAndStringAndIntParsing(t *testing.T) { + os.Setenv("MAGEFILE_VERBOSE", "1") + defer os.Unsetenv("MAGEFILE_VERBOSE") + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + code := ParseAndRun(stderr, stdout, nil, + []string{"-d", "./testdata/extra_args", "targetFour", "stringvalue", "2", "--", "-baz", "foo", "bar"}) + if code != 0 { + t.Fatal("unexpected code", code) + } + + targetOneExpectedOut := "Running target: TargetFour\n" + if stdout.String() != targetOneExpectedOut { + t.Fatalf("expected output %q, but got %q", targetOneExpectedOut, stdout.String()) + } + targetOneExpectedErr := `Context is nil: false +mg.ExtraArgs{"-baz", "foo", "bar"} +stringvalue +2 +` + if stderr.String() != targetOneExpectedErr { + t.Fatalf("expected stderr output %q, but got %q", targetOneExpectedErr, stderr.String()) + } +} + +func TestExtraArgsWithoutContextAndStringAndInt(t *testing.T) { + stdout := &bytes.Buffer{} + inv := Invocation{ + Dir: "./testdata/extra_args", + Stderr: ioutil.Discard, + Stdout: stdout, + Args: []string{"targetFive", "stringvalue", "2"}, + ExtraArgs: []string{"--", "-baz", "foo", "bar"}, + } + + code := Invoke(inv) + if code != 0 { + t.Fatalf("expected 0, but got %v", code) + } + expected := "mg.ExtraArgs{\"-baz\", \"foo\", \"bar\"}\nstringvalue\n2\n" + if stdout.String() != expected { + t.Fatalf("expected %q, but got %q", expected, stdout.String()) + } +} + +func TestExtraArgsWithoutContextAndStringAndIntParsing(t *testing.T) { + os.Setenv("MAGEFILE_VERBOSE", "1") + defer os.Unsetenv("MAGEFILE_VERBOSE") + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + code := ParseAndRun(stderr, stdout, nil, + []string{"-d", "./testdata/extra_args", "targetFive", "stringvalue", "2", "--", "-baz", "foo", "bar"}) + if code != 0 { + t.Fatal("unexpected code", code) + } + + targetOneExpectedOut := "Running target: TargetFive\n" + if stdout.String() != targetOneExpectedOut { + t.Fatalf("expected output %q, but got %q", targetOneExpectedOut, stdout.String()) + } + targetOneExpectedErr := `mg.ExtraArgs{"-baz", "foo", "bar"} +stringvalue +2 +` + if stderr.String() != targetOneExpectedErr { + t.Fatalf("expected stderr output %q, but got %q", targetOneExpectedErr, stderr.String()) + } +} + +func TestExtraArgsWithoutContextAndStringAndIntNonOrdered(t *testing.T) { + stdout := &bytes.Buffer{} + inv := Invocation{ + Dir: "./testdata/extra_args", + Stderr: ioutil.Discard, + Stdout: stdout, + Args: []string{"targetSix", "stringvalue", "2"}, + ExtraArgs: []string{"--", "-baz", "foo", "bar"}, + } + + code := Invoke(inv) + if code != 0 { + t.Fatalf("expected 0, but got %v", code) + } + expected := "mg.ExtraArgs{\"-baz\", \"foo\", \"bar\"}\nstringvalue\n2\n" + if stdout.String() != expected { + t.Fatalf("expected %q, but got %q", expected, stdout.String()) + } +} + +func TestExtraArgsWithoutContextAndStringAndIntNonOrderedParsing(t *testing.T) { + os.Setenv("MAGEFILE_VERBOSE", "1") + defer os.Unsetenv("MAGEFILE_VERBOSE") + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + code := ParseAndRun(stderr, stdout, nil, + []string{"-d", "./testdata/extra_args", "targetSix", "stringvalue", "2", "--", "-baz", "foo", "bar"}) + if code != 0 { + t.Fatal("unexpected code", code) + } + + targetOneExpectedOut := "Running target: TargetSix\n" + if stdout.String() != targetOneExpectedOut { + t.Fatalf("expected output %q, but got %q", targetOneExpectedOut, stdout.String()) + } + targetOneExpectedErr := `mg.ExtraArgs{"-baz", "foo", "bar"} +stringvalue +2 +` + if stderr.String() != targetOneExpectedErr { + t.Fatalf("expected stderr output %q, but got %q", targetOneExpectedErr, stderr.String()) + } +} + // Regression tests, add tests to ensure we do not regress on known issues. // TestBug508 is a regression test for: Bug: using Default with imports selects first matching func by name @@ -1894,6 +2156,7 @@ func TestBug508(t *testing.T) { t.Fatalf("expected 0, but got %v", code) } expected := "test\n" + if stdout.String() != expected { t.Fatalf("expected %q, but got %q", expected, stdout.String()) } diff --git a/mage/template.go b/mage/template.go index e6dc1306..b10cc5c7 100644 --- a/mage/template.go +++ b/mage/template.go @@ -35,6 +35,7 @@ func main() { Help bool // print out help for a specific target Timeout time.Duration // set a timeout to running the targets Args []string // args contain the non-flag command-line arguments + ExtraArgs []string // extraArgs contain the command-line arguments after the "--" separator } parseBool := func(env string) bool { @@ -90,7 +91,23 @@ Options: // flag will have printed out an error already. return } - args.Args = fs.Args() + var extraArgs []string + extraArgsFound := false + for _, arg := range fs.Args() { + // we do not really care about args before this point, this is where extra args begin + if arg == "--" { + extraArgsFound = true + continue + } + // all args before -- are the positional args that go to mage + if !extraArgsFound { + args.Args = append(args.Args, arg) + continue + } + // now we parse all that comes after -- + extraArgs = append(extraArgs, arg) + } + args.ExtraArgs = extraArgs if args.Help && len(args.Args) == 0 { fs.Usage() return @@ -445,7 +462,7 @@ Options: switch _strings.ToLower(target) { {{range .Funcs }} case "{{lower .TargetName}}": - expected := x + {{len .Args}} + expected := x + {{len .Args}} - {{.ExtraArgsPresent}} if expected > len(args.Args) { // note that expected and args at this point include the arg for the target itself // so we subtract 1 here to show the number of args without the target. @@ -462,7 +479,7 @@ Options: {{$imp := .}} {{range .Info.Funcs }} case "{{lower .TargetName}}": - expected := x + {{len .Args}} + expected := x + {{len .Args}} - {{.ExtraArgsPresent}} if expected > len(args.Args) { // note that expected and args at this point include the arg for the target itself // so we subtract 1 here to show the number of args without the target. diff --git a/mage/testdata/extra_args/extra.go b/mage/testdata/extra_args/extra.go new file mode 100644 index 00000000..84910bbe --- /dev/null +++ b/mage/testdata/extra_args/extra.go @@ -0,0 +1,49 @@ +//go:build mage +// +build mage + +package main + +import ( + "context" + "fmt" + + "github.com/magefile/mage/mg" +) + +func TargetOne(extra mg.ExtraArgs) { + fmt.Printf("%#v\n", extra) +} + +func TargetTwo(ctx context.Context, extra mg.ExtraArgs) { + fmt.Printf("Context is nil: %t\n", ctx == nil) + fmt.Printf("%#v\n", extra) +} + +func TargetThree(ctx context.Context, extra mg.ExtraArgs, aString string) error { + fmt.Printf("Context is nil: %t\n", ctx == nil) + fmt.Printf("%#v\n", extra) + fmt.Printf("%s\n", aString) + return nil +} + +func TargetFour(ctx context.Context, extra mg.ExtraArgs, aString string, anInt int) error { + fmt.Printf("Context is nil: %t\n", ctx == nil) + fmt.Printf("%#v\n", extra) + fmt.Printf("%s\n", aString) + fmt.Printf("%d\n", anInt) + return nil +} + +func TargetFive(extra mg.ExtraArgs, aString string, anInt int) error { + fmt.Printf("%#v\n", extra) + fmt.Printf("%s\n", aString) + fmt.Printf("%d\n", anInt) + return nil +} + +func TargetSix(aString string, anInt int, extra mg.ExtraArgs) error { + fmt.Printf("%#v\n", extra) + fmt.Printf("%s\n", aString) + fmt.Printf("%d\n", anInt) + return nil +} diff --git a/mg/extra_args.go b/mg/extra_args.go new file mode 100644 index 00000000..a5729d19 --- /dev/null +++ b/mg/extra_args.go @@ -0,0 +1,3 @@ +package mg + +type ExtraArgs []string diff --git a/parse/parse.go b/parse/parse.go index e68703bd..619ffce5 100644 --- a/parse/parse.go +++ b/parse/parse.go @@ -40,16 +40,17 @@ type PkgInfo struct { // Function represents a job function from a mage file type Function struct { - PkgAlias string - Package string - ImportPath string - Name string - Receiver string - IsError bool - IsContext bool - Synopsis string - Comment string - Args []Arg + PkgAlias string + Package string + ImportPath string + Name string + Receiver string + IsError bool + IsContext bool + Synopsis string + Comment string + Args []Arg + ExtraArgsPresent int } var _ sort.Interface = (Functions)(nil) @@ -116,6 +117,9 @@ func (f Function) ExecCode() string { var parseargs string for x, arg := range f.Args { switch arg.Type { + case "mg.ExtraArgs": + parseargs += fmt.Sprintf(` + arg%d := args.ExtraArgs`, x) // do not advance x index, this is not part of .Args case "string": parseargs += fmt.Sprintf(` arg%d := args.Args[x] @@ -755,8 +759,27 @@ func getPackage(path string, files []string, fset *token.FileSet) (*ast.Package, } } -// hasContextParams returns whether or not the first parameter is a context.Context. If it -// determines that hte first parameter makes this function invalid for mage, it'll return a non-nil +func isExtraArgsField(field *ast.Field) bool { + sel, ok := field.Type.(*ast.SelectorExpr) + if !ok { + return false + } + pkg, ok := sel.X.(*ast.Ident) + if !ok { + return false + } + + if pkg.Name != "mg" { + return false + } + if sel.Sel.Name == "ExtraArgs" { + return true + } + return false +} + +// hasContextParams returns whether the first parameter is a context.Context. If it +// determines that the first parameter makes this function invalid for mage, it'll return a non-nil // error. func hasContextParam(ft *ast.FuncType) (bool, error) { if ft.Params.NumFields() < 1 { @@ -771,12 +794,14 @@ func hasContextParam(ft *ast.FuncType) (bool, error) { if !ok { return false, nil } + if pkg.Name != "context" { return false, nil } if sel.Sel.Name != "Context" { return false, nil } + if len(param.Names) > 1 { // something like foo, bar context.Context return false, errors.New("ETOOMANYCONTEXTS") @@ -832,6 +857,9 @@ func funcType(ft *ast.FuncType) (*Function, error) { } // support for foo, bar string for _, name := range param.Names { + if isExtraArgsField(param) { + f.ExtraArgsPresent++ + } f.Args = append(f.Args, Arg{Name: name.Name, Type: typ}) } } @@ -847,4 +875,5 @@ var argTypes = map[string]string{ "int": "int", "&{time Duration}": "time.Duration", "bool": "bool", + "&{mg ExtraArgs}": "mg.ExtraArgs", }