Skip to content

Improve error message for failed imports #298

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Apr 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions internal/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@ func OutputDebug(cmd string, args ...string) (string, error) {
c.Stderr = errbuf
c.Stdout = buf
if err := c.Run(); err != nil {
debug.Print("error running '", cmd, strings.Join(args, " "), "': ", err, ": ", errbuf)
return "", err
errMsg := strings.TrimSpace(errbuf.String())
debug.Print("error running '", cmd, strings.Join(args, " "), "': ", err, ": ", errMsg)
return "", fmt.Errorf("error running \"%s %s\": %s\n%s", cmd, strings.Join(args, " "), err, errMsg)
}
return strings.TrimSpace(buf.String()), nil
}
Expand Down
27 changes: 27 additions & 0 deletions mage/import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,3 +187,30 @@ func TestMageImportsOneLine(t *testing.T) {
t.Fatalf("expected: %q got: %q", expected, actual)
}
}

func TestMageImportsTaggedPackage(t *testing.T) {
stdout := &bytes.Buffer{}
stderr := &bytes.Buffer{}
inv := Invocation{
Dir: "./testdata/mageimport/tagged",
Stdout: stdout,
Stderr: stderr,
List: true,
}

code := Invoke(inv)
if code != 1 {
t.Fatalf("expected to exit with code 1, but got %v, stdout:\n%s\nstderr:\n%s", code, stdout, stderr)
}

actual := stderr.String()
// Match a shorter version of the error message, since the output from go list differs between versions
expected := `
Error parsing magefiles: error running "go list -f {{.Dir}}||{{.Name}} github.com/magefile/mage/mage/testdata/mageimport/tagged/pkg": exit status 1`[1:]
actualShortened := actual[:len(expected)]
if actualShortened != expected {
t.Logf("expected: %q", expected)
t.Logf("actual: %q", actualShortened)
t.Fatalf("expected:\n%s\n\ngot:\n%s", expected, actualShortened)
}
}
5 changes: 3 additions & 2 deletions mage/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -506,8 +506,9 @@ func Compile(goos, goarch, magePath, goCmd, compileTo string, gofiles []string,
for i := range gofiles {
gofiles[i] = filepath.Base(gofiles[i])
}
debug.Printf("running %s build -o %s %s", goCmd, compileTo, strings.Join(gofiles, " "))
c := exec.Command(goCmd, append([]string{"build", "-o", compileTo}, gofiles...)...)
args := append([]string{"build", "-o", compileTo}, gofiles...)
debug.Printf("running %s %s", goCmd, strings.Join(args, " "))
c := exec.Command(goCmd, args...)
c.Env = environ
c.Stderr = stderr
c.Stdout = stdout
Expand Down
8 changes: 8 additions & 0 deletions mage/testdata/mageimport/tagged/magefile.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
//+build mage

package main

import (
// mage:import
_ "github.com/magefile/mage/mage/testdata/mageimport/tagged/pkg"
)
7 changes: 7 additions & 0 deletions mage/testdata/mageimport/tagged/pkg/mage.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
//+build mage

package pkg

// Build builds stuff.
func Build() {
}
15 changes: 7 additions & 8 deletions site/content/importing/_index.en.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,13 @@ package. i.e. it cannot be `package main`. This is in contrast to a normal
mage package that must be `package main`. The reason is that the go tool won't
let you import a main package.

In addition, all files will be imported, not just those tagged with the
`//+build mage` build tag. Again, this is a restriction of the go tool. Mage
can build only the `.go` files in the current directory that have the `mage`
build tag, but when importing code from other directories, it's simply not
possible. This means that any exported function (in any file) that matches
Mage's allowed formats will be picked up as a target.

Aliases and defaults in the imported package will be ignored.
In addition, all package files will be imported, so long as they don't have a
build tag. If you try to import a package consisting only of files with build
tags (e.g. `//+build mage`), it will cause an error since mage doesn't set any
build tags when importing packages. Any exported function, in imported
packages, that matches Mage's allowed formats will be picked up as a target.

Aliases and defaults in imported packages will be ignored.

Other than these differences, you can write targets in those packages just
like a normal magefile.
Expand Down