Improve error message for failed imports (#298)

* internal.OutputDebug: Improve returned error
* Documentation: Clarify that files with build tags aren't imported

internal/run.go

@@ -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
 }

mage/import_test.go

@@ -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)
+	}
+}

mage/main.go

@@ -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

mage/testdata/mageimport/tagged/magefile.go

@@ -0,0 +1,8 @@
+//+build mage
+
+package main
+
+import (
+	// mage:import
+	_ "github.com/magefile/mage/mage/testdata/mageimport/tagged/pkg"
+)

mage/testdata/mageimport/tagged/pkg/mage.go

@@ -0,0 +1,7 @@
+//+build mage
+
+package pkg
+
+// Build builds stuff.
+func Build() {
+}

site/content/importing/_index.en.md

@@ -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.