From 549c1448f9d7d4cf8e1e3ce6ef52ab4f4f299dd6 Mon Sep 17 00:00:00 2001
From: Rob Kennedy <rob@sweetkennedy.net>
Date: Wed, 1 May 2024 23:02:18 -0500
Subject: [PATCH 1/4] Support wrapped errors

Wrapping errors instead of merely embedding error messages allows
calling code to use `errors.Is` and `errors.As` to more precisely check
the reasons for various errors instead of having to rely only on
substring searches.

We implement our own wrapper error to retain backward compatibility
prior to Go 1.13, where there is no support for the "%w" format string
in `fmt.Errorf`.
---
 internal/run.go      |  4 +++-
 mage/main.go         | 26 +++++++++++++-------------
 mage/main_test.go    | 16 ++++++++--------
 magefile.go          |  8 ++++----
 mg/errors.go         | 35 +++++++++++++++++++++++++++++++++++
 mg/errors113_test.go | 24 ++++++++++++++++++++++++
 mg/errors_test.go    | 17 ++++++++++++++++-
 mg/fn.go             |  2 +-
 parse/parse.go       |  3 ++-
 sh/cmd.go            |  2 +-
 sh/cmd113_test.go    | 23 +++++++++++++++++++++++
 sh/helpers.go        | 12 +++++++-----
 sh/helpers_test.go   |  9 +++++----
 13 files changed, 142 insertions(+), 39 deletions(-)
 create mode 100644 mg/errors113_test.go
 create mode 100644 sh/cmd113_test.go

diff --git a/internal/run.go b/internal/run.go
index 79b4f049..b5d13958 100644
--- a/internal/run.go
+++ b/internal/run.go
@@ -9,6 +9,8 @@ import (
 	"os/exec"
 	"runtime"
 	"strings"
+
+	"github.com/magefile/mage/mg"
 )
 
 var debug *log.Logger = log.New(ioutil.Discard, "", 0)
@@ -52,7 +54,7 @@ func OutputDebug(cmd string, args ...string) (string, error) {
 	if err := c.Run(); err != nil {
 		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 "", mg.WrapError(err, fmt.Errorf("error running \"%s %s\": %v\n%s", cmd, strings.Join(args, " "), err, errMsg))
 	}
 	return strings.TrimSpace(buf.String()), nil
 }
diff --git a/mage/main.go b/mage/main.go
index 0062bd35..31ef28db 100644
--- a/mage/main.go
+++ b/mage/main.go
@@ -473,14 +473,14 @@ func listGoFiles(magePath, goCmd, tag string, envStr []string) ([]string, error)
 	if !filepath.IsAbs(magePath) {
 		cwd, err := os.Getwd()
 		if err != nil {
-			return nil, fmt.Errorf("can't get current working directory: %v", err)
+			return nil, mg.WrapError(err, fmt.Errorf("can't get current working directory: %v", err))
 		}
 		magePath = filepath.Join(cwd, magePath)
 	}
 
 	env, err := internal.SplitEnv(envStr)
 	if err != nil {
-		return nil, fmt.Errorf("error parsing environment variables: %v", err)
+		return nil, mg.WrapError(err, fmt.Errorf("error parsing environment variables: %v", err))
 	}
 
 	bctx := build.Default
@@ -502,7 +502,7 @@ func listGoFiles(magePath, goCmd, tag string, envStr []string) ([]string, error)
 
 		// Allow multiple packages in the same directory
 		if _, ok := err.(*build.MultiplePackageError); !ok {
-			return nil, fmt.Errorf("failed to parse go source files: %v", err)
+			return nil, mg.WrapError(err, fmt.Errorf("failed to parse go source files: %v", err))
 		}
 	}
 
@@ -530,7 +530,7 @@ func Magefiles(magePath, goos, goarch, goCmd string, stderr io.Writer, isMagefil
 	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)
+		return nil, mg.WrapError(err, fmt.Errorf("listing mage files: %v", err))
 	}
 
 	if isMagefilesDirectory {
@@ -546,7 +546,7 @@ func Magefiles(magePath, goos, goarch, goCmd string, stderr io.Writer, isMagefil
 	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)
+		return nil, mg.WrapError(err, fmt.Errorf("listing non-mage files: %v", err))
 	}
 
 	// convert non-Mage list to a map of files to exclude.
@@ -612,7 +612,7 @@ func GenerateMainfile(binaryName, path string, info *parse.PkgInfo) error {
 
 	f, err := os.Create(path)
 	if err != nil {
-		return fmt.Errorf("error creating generated mainfile: %v", err)
+		return mg.WrapError(err, fmt.Errorf("error creating generated mainfile: %v", err))
 	}
 	defer f.Close()
 	data := mainfileTemplateData{
@@ -629,16 +629,16 @@ func GenerateMainfile(binaryName, path string, info *parse.PkgInfo) error {
 
 	debug.Println("writing new file at", path)
 	if err := mainfileTemplate.Execute(f, data); err != nil {
-		return fmt.Errorf("can't execute mainfile template: %v", err)
+		return mg.WrapError(err, fmt.Errorf("can't execute mainfile template: %v", err))
 	}
 	if err := f.Close(); err != nil {
-		return fmt.Errorf("error closing generated mainfile: %v", err)
+		return mg.WrapError(err, fmt.Errorf("error closing generated mainfile: %v", err))
 	}
 	// we set an old modtime on the generated mainfile so that the go tool
 	// won't think it has changed more recently than the compiled binary.
 	longAgo := time.Now().Add(-time.Hour * 24 * 365 * 10)
 	if err := os.Chtimes(path, longAgo, longAgo); err != nil {
-		return fmt.Errorf("error setting old modtime on generated mainfile: %v", err)
+		return mg.WrapError(err, fmt.Errorf("error setting old modtime on generated mainfile: %v", err))
 	}
 	return nil
 }
@@ -675,13 +675,13 @@ func ExeName(goCmd, cacheDir string, files []string) (string, error) {
 func hashFile(fn string) (string, error) {
 	f, err := os.Open(fn)
 	if err != nil {
-		return "", fmt.Errorf("can't open input file for hashing: %#v", err)
+		return "", mg.WrapError(err, fmt.Errorf("can't open input file for hashing: %v", err))
 	}
 	defer f.Close()
 
 	h := sha1.New()
 	if _, err := io.Copy(h, f); err != nil {
-		return "", fmt.Errorf("can't write data to hash: %v", err)
+		return "", mg.WrapError(err, fmt.Errorf("can't write data to hash: %v", err))
 	}
 	return fmt.Sprintf("%x", h.Sum(nil)), nil
 }
@@ -690,12 +690,12 @@ func generateInit(dir string) error {
 	debug.Println("generating default magefile in", dir)
 	f, err := os.Create(filepath.Join(dir, initFile))
 	if err != nil {
-		return fmt.Errorf("could not create mage template: %v", err)
+		return mg.WrapError(err, fmt.Errorf("could not create mage template: %v", err))
 	}
 	defer f.Close()
 
 	if err := initOutput.Execute(f, nil); err != nil {
-		return fmt.Errorf("can't execute magefile template: %v", err)
+		return mg.WrapError(err, fmt.Errorf("can't execute magefile template: %v", err))
 	}
 
 	return nil
diff --git a/mage/main_test.go b/mage/main_test.go
index 07e598e5..68b46a4e 100644
--- a/mage/main_test.go
+++ b/mage/main_test.go
@@ -1270,8 +1270,8 @@ func TestCompiledFlags(t *testing.T) {
 		cmd.Stderr = stderr
 		cmd.Stdout = stdout
 		if err := cmd.Run(); err != nil {
-			return fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
-				filename, strings.Join(args, " "), err, stdout, stderr)
+			return mg.WrapError(err, fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
+				filename, strings.Join(args, " "), err, stdout, stderr))
 		}
 		return nil
 	}
@@ -1357,8 +1357,8 @@ func TestCompiledEnvironmentVars(t *testing.T) {
 		cmd.Stderr = stderr
 		cmd.Stdout = stdout
 		if err := cmd.Run(); err != nil {
-			return fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
-				filename, strings.Join(args, " "), err, stdout, stderr)
+			return mg.WrapError(err, fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
+				filename, strings.Join(args, " "), err, stdout, stderr))
 		}
 		return nil
 	}
@@ -1511,8 +1511,8 @@ func TestSignals(t *testing.T) {
 		cmd.Stderr = stderr
 		cmd.Stdout = stdout
 		if err := cmd.Start(); err != nil {
-			return fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
-				filename, target, err, stdout, stderr)
+			return mg.WrapError(err, fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
+				filename, target, err, stdout, stderr))
 		}
 		pid := cmd.Process.Pid
 		go func() {
@@ -1523,8 +1523,8 @@ func TestSignals(t *testing.T) {
 			}
 		}()
 		if err := cmd.Wait(); err != nil {
-			return fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
-				filename, target, err, stdout, stderr)
+			return mg.WrapError(err, fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
+				filename, target, err, stdout, stderr))
 		}
 		return nil
 	}
diff --git a/magefile.go b/magefile.go
index c08ffa37..108e8e06 100644
--- a/magefile.go
+++ b/magefile.go
@@ -43,12 +43,12 @@ func Install() error {
 	// in GOPATH environment string
 	bin, err := sh.Output(gocmd, "env", "GOBIN")
 	if err != nil {
-		return fmt.Errorf("can't determine GOBIN: %v", err)
+		return mg.WrapError(err, fmt.Errorf("can't determine GOBIN: %v", err))
 	}
 	if bin == "" {
 		gopath, err := sh.Output(gocmd, "env", "GOPATH")
 		if err != nil {
-			return fmt.Errorf("can't determine GOPATH: %v", err)
+			return mg.WrapError(err, fmt.Errorf("can't determine GOPATH: %v", err))
 		}
 		paths := strings.Split(gopath, string([]rune{os.PathListSeparator}))
 		bin = filepath.Join(paths[0], "bin")
@@ -56,7 +56,7 @@ func Install() error {
 	// specifically don't mkdirall, if you have an invalid gopath in the first
 	// place, that's not on us to fix.
 	if err := os.Mkdir(bin, 0700); err != nil && !os.IsExist(err) {
-		return fmt.Errorf("failed to create %q: %v", bin, err)
+		return mg.WrapError(err, fmt.Errorf("failed to create %q: %v", bin, err))
 	}
 	path := filepath.Join(bin, name)
 
@@ -72,7 +72,7 @@ var releaseTag = regexp.MustCompile(`^v1\.[0-9]+\.[0-9]+$`)
 // Generates a new release. Expects a version tag in v1.x.x format.
 func Release(tag string) (err error) {
 	if _, err := exec.LookPath("goreleaser"); err != nil {
-		return fmt.Errorf("can't find goreleaser: %w", err)
+		return mg.WrapError(err, fmt.Errorf("can't find goreleaser: %v", err))
 	}
 	if !releaseTag.MatchString(tag) {
 		return errors.New("TAG environment variable must be in semver v1.x.x format, but was " + tag)
diff --git a/mg/errors.go b/mg/errors.go
index 2dd780fe..9bf7fad4 100644
--- a/mg/errors.go
+++ b/mg/errors.go
@@ -49,3 +49,38 @@ func ExitStatus(err error) int {
 	}
 	return exit.ExitStatus()
 }
+
+// wrappedError is an error that supports the Go 1.13+ mechanism of wrapping
+// errors. It implements Unwrap to return the underlying error, but it still
+// returns the string version of whatever "main" error it represents.
+type wrappedError struct {
+	underlyingError error
+	stringError     error
+}
+
+// WrapError returns an error that implements the Go 1.13 Unwrap interface. The
+// Error function returns the value of the "string" error, and the Unwrap
+// function returns the "underlying" error. Use this wherever one might
+// otherwise use the "%w" format string in [fmt.Errorf].
+//     err := doSomething()
+//     if err != nil {
+//         return WrapError(err, fmt.Errorf("Could not do something: %v", err))
+//     }
+// The premise is that the "string" error is not an interesting one to try to
+// inspect with [errors.Is] or [errors.As] because it has no other wrapped
+// errors of its own, and it will usually be of whatever error type
+// `fmt.Errorf` returns.
+func WrapError(underlying, str error) error {
+	return &wrappedError{
+		underlyingError: underlying,
+		stringError:     str,
+	}
+}
+
+func (e *wrappedError) Error() string {
+	return e.stringError.Error()
+}
+
+func (e *wrappedError) Unwrap() error {
+	return e.underlyingError
+}
diff --git a/mg/errors113_test.go b/mg/errors113_test.go
new file mode 100644
index 00000000..2cdc0ff3
--- /dev/null
+++ b/mg/errors113_test.go
@@ -0,0 +1,24 @@
+// The concept of "wrapping" errors was only introduced in Go 1.13, so we only
+// want to test that our errors behave like wrapped errors on Go versions that
+// support `errors.Is`.
+//go:build go1.13
+// +build go1.13
+
+package mg
+
+import (
+	"errors"
+	"testing"
+)
+
+// TestErrorUnwrap checks that [errors.Is] can detect the underlying error in a
+// wrappedError.
+func TestErrorUnwrap(t *testing.T) {
+	strError := errors.New("main error")
+	underlyingError := errors.New("underlying error")
+	actual := WrapError(underlyingError, strError)
+
+	if !errors.Is(actual, underlyingError) {
+		t.Fatalf("Expected outer error %#v to include %#v", actual, underlyingError)
+	}
+}
diff --git a/mg/errors_test.go b/mg/errors_test.go
index ac5e68f6..880e82d9 100644
--- a/mg/errors_test.go
+++ b/mg/errors_test.go
@@ -1,6 +1,9 @@
 package mg
 
-import "testing"
+import (
+	"errors"
+	"testing"
+)
 
 func TestFatalExit(t *testing.T) {
 	expected := 99
@@ -17,3 +20,15 @@ func TestFatalfExit(t *testing.T) {
 		t.Fatalf("Expected code %v but got %v", expected, code)
 	}
 }
+
+// TestBasicWrappedError confirms that a wrappedError returns the same string
+// as its "str" error (not its "underlying" error).
+func TestBasicWrappedError(t *testing.T) {
+	strError := errors.New("main error")
+	underlyingError := errors.New("underlying error")
+	actual := WrapError(underlyingError, strError)
+
+	if actual.Error() != strError.Error() {
+		t.Fatalf("Expected outer error to have Error() = %q but got %q", strError.Error(), actual.Error())
+	}
+}
diff --git a/mg/fn.go b/mg/fn.go
index 3856857a..b8aa5370 100644
--- a/mg/fn.go
+++ b/mg/fn.go
@@ -37,7 +37,7 @@ func F(target interface{}, args ...interface{}) Fn {
 	}
 	id, err := json.Marshal(args)
 	if err != nil {
-		panic(fmt.Errorf("can't convert args into a mage-compatible id for mg.Deps: %s", err))
+		panic(WrapError(err, fmt.Errorf("can't convert args into a mage-compatible id for mg.Deps: %v", err)))
 	}
 	return fn{
 		name: funcName(target),
diff --git a/parse/parse.go b/parse/parse.go
index 34216690..1ae56bac 100644
--- a/parse/parse.go
+++ b/parse/parse.go
@@ -15,6 +15,7 @@ import (
 	"time"
 
 	"github.com/magefile/mage/internal"
+	"github.com/magefile/mage/mg"
 )
 
 const importTag = "mage:import"
@@ -735,7 +736,7 @@ func getPackage(path string, files []string, fset *token.FileSet) (*ast.Package,
 
 	pkgs, err := parser.ParseDir(fset, path, filter, parser.ParseComments)
 	if err != nil {
-		return nil, fmt.Errorf("failed to parse directory: %v", err)
+		return nil, mg.WrapError(err, fmt.Errorf("failed to parse directory: %v", err))
 	}
 
 	switch len(pkgs) {
diff --git a/sh/cmd.go b/sh/cmd.go
index 312de65a..c2693d93 100644
--- a/sh/cmd.go
+++ b/sh/cmd.go
@@ -120,7 +120,7 @@ func Exec(env map[string]string, stdout, stderr io.Writer, cmd string, args ...s
 	if ran {
 		return ran, mg.Fatalf(code, `running "%s %s" failed with exit code %d`, cmd, strings.Join(args, " "), code)
 	}
-	return ran, fmt.Errorf(`failed to run "%s %s: %v"`, cmd, strings.Join(args, " "), err)
+	return ran, mg.WrapError(err, fmt.Errorf(`failed to run "%s %s: %v"`, cmd, strings.Join(args, " "), err))
 }
 
 func run(env map[string]string, stdout, stderr io.Writer, cmd string, args ...string) (ran bool, code int, err error) {
diff --git a/sh/cmd113_test.go b/sh/cmd113_test.go
new file mode 100644
index 00000000..23a201ce
--- /dev/null
+++ b/sh/cmd113_test.go
@@ -0,0 +1,23 @@
+// The concept of "wrapping" errors was only introduced in Go 1.13, so we only
+// want to test that our errors behave like wrapped errors on Go versions that
+// support `errors.Is`.
+//go:build go1.13
+// +build go1.13
+
+package sh
+
+import (
+	"errors"
+	"os"
+	"testing"
+)
+
+func TestWrappedError(t *testing.T) {
+	_, err := Exec(nil, nil, nil, os.Args[0]+"-doesnotexist", "-printArgs", "foo")
+	if err == nil {
+		t.Fatalf("Expected to fail running %s", os.Args[0]+"-doesnotexist")
+	}
+	if !errors.Is(err, os.ErrNotExist) {
+		t.Fatalf("Expected error to be like ErrNotExist, got %#v", err)
+	}
+}
diff --git a/sh/helpers.go b/sh/helpers.go
index f5d20a27..1e9542b9 100644
--- a/sh/helpers.go
+++ b/sh/helpers.go
@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"io"
 	"os"
+
+	"github.com/magefile/mage/mg"
 )
 
 // Rm removes the given file or directory even if non-empty. It will not return
@@ -13,28 +15,28 @@ func Rm(path string) error {
 	if err == nil || os.IsNotExist(err) {
 		return nil
 	}
-	return fmt.Errorf(`failed to remove %s: %v`, path, err)
+	return mg.WrapError(err, fmt.Errorf(`failed to remove %s: %v`, path, err))
 }
 
 // Copy robustly copies the source file to the destination, overwriting the destination if necessary.
 func Copy(dst string, src string) error {
 	from, err := os.Open(src)
 	if err != nil {
-		return fmt.Errorf(`can't copy %s: %v`, src, err)
+		return mg.WrapError(err, fmt.Errorf(`can't copy %s: %v`, src, err))
 	}
 	defer from.Close()
 	finfo, err := from.Stat()
 	if err != nil {
-		return fmt.Errorf(`can't stat %s: %v`, src, err)
+		return mg.WrapError(err, fmt.Errorf(`can't stat %s: %v`, src, err))
 	}
 	to, err := os.OpenFile(dst, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, finfo.Mode())
 	if err != nil {
-		return fmt.Errorf(`can't copy to %s: %v`, dst, err)
+		return mg.WrapError(err, fmt.Errorf(`can't copy to %s: %v`, dst, err))
 	}
 	defer to.Close()
 	_, err = io.Copy(to, from)
 	if err != nil {
-		return fmt.Errorf(`error copying %s to %s: %v`, src, dst, err)
+		return mg.WrapError(err, fmt.Errorf(`error copying %s to %s: %v`, src, dst, err))
 	}
 	return nil
 }
diff --git a/sh/helpers_test.go b/sh/helpers_test.go
index 54e78aa1..698eba3f 100644
--- a/sh/helpers_test.go
+++ b/sh/helpers_test.go
@@ -8,6 +8,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/magefile/mage/mg"
 	"github.com/magefile/mage/sh"
 )
 
@@ -17,11 +18,11 @@ import (
 func compareFiles(file1 string, file2 string) error {
 	s1, err := os.Stat(file1)
 	if err != nil {
-		return fmt.Errorf("can't stat %s: %v", file1, err)
+		return mg.WrapError(err, fmt.Errorf("can't stat %s: %v", file1, err))
 	}
 	s2, err := os.Stat(file2)
 	if err != nil {
-		return fmt.Errorf("can't stat %s: %v", file2, err)
+		return mg.WrapError(err, fmt.Errorf("can't stat %s: %v", file2, err))
 	}
 	if s1.Size() != s2.Size() {
 		return fmt.Errorf("files %s and %s have different sizes: %d vs %d", file1, file2, s1.Size(), s2.Size())
@@ -31,11 +32,11 @@ func compareFiles(file1 string, file2 string) error {
 	}
 	f1bytes, err := ioutil.ReadFile(file1)
 	if err != nil {
-		return fmt.Errorf("can't read %s: %v", file1, err)
+		return mg.WrapError(err, fmt.Errorf("can't read %s: %v", file1, err))
 	}
 	f2bytes, err := ioutil.ReadFile(file2)
 	if err != nil {
-		return fmt.Errorf("can't read %s: %v", file2, err)
+		return mg.WrapError(err, fmt.Errorf("can't read %s: %v", file2, err))
 	}
 	if !bytes.Equal(f1bytes, f2bytes) {
 		return fmt.Errorf("files %s and %s have different contents", file1, file2)

From d3f41c5f3f9459e30e4e34f800bba06e798083b0 Mon Sep 17 00:00:00 2001
From: Rob Kennedy <rob@sweetkennedy.net>
Date: Sat, 4 May 2024 12:19:00 -0500
Subject: [PATCH 2/4] Move WrapError to internal package

There's no particular reason to make error-wrapping be part of Mage's
public interface, so now it's kept in the internal package. One
consolation was needed to avoid circular imports, which is that when
`mg.F` panics, it does not _wrap_ the JSON-marshalling error.
---
 internal/errors.go                 | 36 ++++++++++++++++++++++++++++++
 {mg => internal}/errors113_test.go |  2 +-
 internal/errors_test.go            | 18 +++++++++++++++
 internal/run.go                    |  4 +---
 mage/main.go                       | 26 ++++++++++-----------
 mage/main_test.go                  |  8 +++----
 magefile.go                        |  8 +++----
 mg/errors.go                       | 35 -----------------------------
 mg/errors_test.go                  | 13 -----------
 mg/fn.go                           |  2 +-
 parse/parse.go                     |  3 +--
 sh/cmd.go                          |  3 ++-
 sh/helpers.go                      | 12 +++++-----
 sh/helpers_test.go                 | 10 ++++-----
 14 files changed, 92 insertions(+), 88 deletions(-)
 create mode 100644 internal/errors.go
 rename {mg => internal}/errors113_test.go (97%)
 create mode 100644 internal/errors_test.go

diff --git a/internal/errors.go b/internal/errors.go
new file mode 100644
index 00000000..f264a51a
--- /dev/null
+++ b/internal/errors.go
@@ -0,0 +1,36 @@
+package internal
+
+// wrappedError is an error that supports the Go 1.13+ mechanism of wrapping
+// errors. It implements Unwrap to return the underlying error, but it still
+// returns the string version of whatever "main" error it represents.
+type wrappedError struct {
+	underlyingError error
+	stringError     error
+}
+
+// WrapError returns an error that implements the Go 1.13 Unwrap interface. The
+// Error function returns the value of the "string" error, and the Unwrap
+// function returns the "underlying" error. Use this wherever one might
+// otherwise use the "%w" format string in [fmt.Errorf].
+//     err := doSomething()
+//     if err != nil {
+//         return WrapError(err, fmt.Errorf("Could not do something: %v", err))
+//     }
+// The premise is that the "string" error is not an interesting one to try to
+// inspect with [errors.Is] or [errors.As] because it has no other wrapped
+// errors of its own, and it will usually be of whatever error type
+// `fmt.Errorf` returns.
+func WrapError(underlying, str error) error {
+	return &wrappedError{
+		underlyingError: underlying,
+		stringError:     str,
+	}
+}
+
+func (e *wrappedError) Error() string {
+	return e.stringError.Error()
+}
+
+func (e *wrappedError) Unwrap() error {
+	return e.underlyingError
+}
diff --git a/mg/errors113_test.go b/internal/errors113_test.go
similarity index 97%
rename from mg/errors113_test.go
rename to internal/errors113_test.go
index 2cdc0ff3..6e0fcc5b 100644
--- a/mg/errors113_test.go
+++ b/internal/errors113_test.go
@@ -4,7 +4,7 @@
 //go:build go1.13
 // +build go1.13
 
-package mg
+package internal
 
 import (
 	"errors"
diff --git a/internal/errors_test.go b/internal/errors_test.go
new file mode 100644
index 00000000..3efca544
--- /dev/null
+++ b/internal/errors_test.go
@@ -0,0 +1,18 @@
+package internal
+
+import (
+	"errors"
+	"testing"
+)
+
+// TestBasicWrappedError confirms that a wrappedError returns the same string
+// as its "str" error (not its "underlying" error).
+func TestBasicWrappedError(t *testing.T) {
+	strError := errors.New("main error")
+	underlyingError := errors.New("underlying error")
+	actual := WrapError(underlyingError, strError)
+
+	if actual.Error() != strError.Error() {
+		t.Fatalf("Expected outer error to have Error() = %q but got %q", strError.Error(), actual.Error())
+	}
+}
diff --git a/internal/run.go b/internal/run.go
index b5d13958..12a5ed21 100644
--- a/internal/run.go
+++ b/internal/run.go
@@ -9,8 +9,6 @@ import (
 	"os/exec"
 	"runtime"
 	"strings"
-
-	"github.com/magefile/mage/mg"
 )
 
 var debug *log.Logger = log.New(ioutil.Discard, "", 0)
@@ -54,7 +52,7 @@ func OutputDebug(cmd string, args ...string) (string, error) {
 	if err := c.Run(); err != nil {
 		errMsg := strings.TrimSpace(errbuf.String())
 		debug.Print("error running '", cmd, strings.Join(args, " "), "': ", err, ": ", errMsg)
-		return "", mg.WrapError(err, fmt.Errorf("error running \"%s %s\": %v\n%s", cmd, strings.Join(args, " "), err, errMsg))
+		return "", WrapError(err, fmt.Errorf("error running \"%s %s\": %v\n%s", cmd, strings.Join(args, " "), err, errMsg))
 	}
 	return strings.TrimSpace(buf.String()), nil
 }
diff --git a/mage/main.go b/mage/main.go
index 31ef28db..4081c925 100644
--- a/mage/main.go
+++ b/mage/main.go
@@ -473,14 +473,14 @@ func listGoFiles(magePath, goCmd, tag string, envStr []string) ([]string, error)
 	if !filepath.IsAbs(magePath) {
 		cwd, err := os.Getwd()
 		if err != nil {
-			return nil, mg.WrapError(err, fmt.Errorf("can't get current working directory: %v", err))
+			return nil, internal.WrapError(err, fmt.Errorf("can't get current working directory: %v", err))
 		}
 		magePath = filepath.Join(cwd, magePath)
 	}
 
 	env, err := internal.SplitEnv(envStr)
 	if err != nil {
-		return nil, mg.WrapError(err, fmt.Errorf("error parsing environment variables: %v", err))
+		return nil, internal.WrapError(err, fmt.Errorf("error parsing environment variables: %v", err))
 	}
 
 	bctx := build.Default
@@ -502,7 +502,7 @@ func listGoFiles(magePath, goCmd, tag string, envStr []string) ([]string, error)
 
 		// Allow multiple packages in the same directory
 		if _, ok := err.(*build.MultiplePackageError); !ok {
-			return nil, mg.WrapError(err, fmt.Errorf("failed to parse go source files: %v", err))
+			return nil, internal.WrapError(err, fmt.Errorf("failed to parse go source files: %v", err))
 		}
 	}
 
@@ -530,7 +530,7 @@ func Magefiles(magePath, goos, goarch, goCmd string, stderr io.Writer, isMagefil
 	debug.Println("getting all files including those with mage tag in", magePath)
 	mageFiles, err := listGoFiles(magePath, goCmd, "mage", env)
 	if err != nil {
-		return nil, mg.WrapError(err, fmt.Errorf("listing mage files: %v", err))
+		return nil, internal.WrapError(err, fmt.Errorf("listing mage files: %v", err))
 	}
 
 	if isMagefilesDirectory {
@@ -546,7 +546,7 @@ func Magefiles(magePath, goos, goarch, goCmd string, stderr io.Writer, isMagefil
 	debug.Println("getting all files without mage tag in", magePath)
 	nonMageFiles, err := listGoFiles(magePath, goCmd, "", env)
 	if err != nil {
-		return nil, mg.WrapError(err, fmt.Errorf("listing non-mage files: %v", err))
+		return nil, internal.WrapError(err, fmt.Errorf("listing non-mage files: %v", err))
 	}
 
 	// convert non-Mage list to a map of files to exclude.
@@ -612,7 +612,7 @@ func GenerateMainfile(binaryName, path string, info *parse.PkgInfo) error {
 
 	f, err := os.Create(path)
 	if err != nil {
-		return mg.WrapError(err, fmt.Errorf("error creating generated mainfile: %v", err))
+		return internal.WrapError(err, fmt.Errorf("error creating generated mainfile: %v", err))
 	}
 	defer f.Close()
 	data := mainfileTemplateData{
@@ -629,16 +629,16 @@ func GenerateMainfile(binaryName, path string, info *parse.PkgInfo) error {
 
 	debug.Println("writing new file at", path)
 	if err := mainfileTemplate.Execute(f, data); err != nil {
-		return mg.WrapError(err, fmt.Errorf("can't execute mainfile template: %v", err))
+		return internal.WrapError(err, fmt.Errorf("can't execute mainfile template: %v", err))
 	}
 	if err := f.Close(); err != nil {
-		return mg.WrapError(err, fmt.Errorf("error closing generated mainfile: %v", err))
+		return internal.WrapError(err, fmt.Errorf("error closing generated mainfile: %v", err))
 	}
 	// we set an old modtime on the generated mainfile so that the go tool
 	// won't think it has changed more recently than the compiled binary.
 	longAgo := time.Now().Add(-time.Hour * 24 * 365 * 10)
 	if err := os.Chtimes(path, longAgo, longAgo); err != nil {
-		return mg.WrapError(err, fmt.Errorf("error setting old modtime on generated mainfile: %v", err))
+		return internal.WrapError(err, fmt.Errorf("error setting old modtime on generated mainfile: %v", err))
 	}
 	return nil
 }
@@ -675,13 +675,13 @@ func ExeName(goCmd, cacheDir string, files []string) (string, error) {
 func hashFile(fn string) (string, error) {
 	f, err := os.Open(fn)
 	if err != nil {
-		return "", mg.WrapError(err, fmt.Errorf("can't open input file for hashing: %v", err))
+		return "", internal.WrapError(err, fmt.Errorf("can't open input file for hashing: %v", err))
 	}
 	defer f.Close()
 
 	h := sha1.New()
 	if _, err := io.Copy(h, f); err != nil {
-		return "", mg.WrapError(err, fmt.Errorf("can't write data to hash: %v", err))
+		return "", internal.WrapError(err, fmt.Errorf("can't write data to hash: %v", err))
 	}
 	return fmt.Sprintf("%x", h.Sum(nil)), nil
 }
@@ -690,12 +690,12 @@ func generateInit(dir string) error {
 	debug.Println("generating default magefile in", dir)
 	f, err := os.Create(filepath.Join(dir, initFile))
 	if err != nil {
-		return mg.WrapError(err, fmt.Errorf("could not create mage template: %v", err))
+		return internal.WrapError(err, fmt.Errorf("could not create mage template: %v", err))
 	}
 	defer f.Close()
 
 	if err := initOutput.Execute(f, nil); err != nil {
-		return mg.WrapError(err, fmt.Errorf("can't execute magefile template: %v", err))
+		return internal.WrapError(err, fmt.Errorf("can't execute magefile template: %v", err))
 	}
 
 	return nil
diff --git a/mage/main_test.go b/mage/main_test.go
index 68b46a4e..1c064d7d 100644
--- a/mage/main_test.go
+++ b/mage/main_test.go
@@ -1270,7 +1270,7 @@ func TestCompiledFlags(t *testing.T) {
 		cmd.Stderr = stderr
 		cmd.Stdout = stdout
 		if err := cmd.Run(); err != nil {
-			return mg.WrapError(err, fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
+			return internal.WrapError(err, fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
 				filename, strings.Join(args, " "), err, stdout, stderr))
 		}
 		return nil
@@ -1357,7 +1357,7 @@ func TestCompiledEnvironmentVars(t *testing.T) {
 		cmd.Stderr = stderr
 		cmd.Stdout = stdout
 		if err := cmd.Run(); err != nil {
-			return mg.WrapError(err, fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
+			return internal.WrapError(err, fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
 				filename, strings.Join(args, " "), err, stdout, stderr))
 		}
 		return nil
@@ -1511,7 +1511,7 @@ func TestSignals(t *testing.T) {
 		cmd.Stderr = stderr
 		cmd.Stdout = stdout
 		if err := cmd.Start(); err != nil {
-			return mg.WrapError(err, fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
+			return internal.WrapError(err, fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
 				filename, target, err, stdout, stderr))
 		}
 		pid := cmd.Process.Pid
@@ -1523,7 +1523,7 @@ func TestSignals(t *testing.T) {
 			}
 		}()
 		if err := cmd.Wait(); err != nil {
-			return mg.WrapError(err, fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
+			return internal.WrapError(err, fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
 				filename, target, err, stdout, stderr))
 		}
 		return nil
diff --git a/magefile.go b/magefile.go
index 108e8e06..83410b2a 100644
--- a/magefile.go
+++ b/magefile.go
@@ -43,12 +43,12 @@ func Install() error {
 	// in GOPATH environment string
 	bin, err := sh.Output(gocmd, "env", "GOBIN")
 	if err != nil {
-		return mg.WrapError(err, fmt.Errorf("can't determine GOBIN: %v", err))
+		return internal.WrapError(err, fmt.Errorf("can't determine GOBIN: %v", err))
 	}
 	if bin == "" {
 		gopath, err := sh.Output(gocmd, "env", "GOPATH")
 		if err != nil {
-			return mg.WrapError(err, fmt.Errorf("can't determine GOPATH: %v", err))
+			return internal.WrapError(err, fmt.Errorf("can't determine GOPATH: %v", err))
 		}
 		paths := strings.Split(gopath, string([]rune{os.PathListSeparator}))
 		bin = filepath.Join(paths[0], "bin")
@@ -56,7 +56,7 @@ func Install() error {
 	// specifically don't mkdirall, if you have an invalid gopath in the first
 	// place, that's not on us to fix.
 	if err := os.Mkdir(bin, 0700); err != nil && !os.IsExist(err) {
-		return mg.WrapError(err, fmt.Errorf("failed to create %q: %v", bin, err))
+		return internal.WrapError(err, fmt.Errorf("failed to create %q: %v", bin, err))
 	}
 	path := filepath.Join(bin, name)
 
@@ -72,7 +72,7 @@ var releaseTag = regexp.MustCompile(`^v1\.[0-9]+\.[0-9]+$`)
 // Generates a new release. Expects a version tag in v1.x.x format.
 func Release(tag string) (err error) {
 	if _, err := exec.LookPath("goreleaser"); err != nil {
-		return mg.WrapError(err, fmt.Errorf("can't find goreleaser: %v", err))
+		return internal.WrapError(err, fmt.Errorf("can't find goreleaser: %v", err))
 	}
 	if !releaseTag.MatchString(tag) {
 		return errors.New("TAG environment variable must be in semver v1.x.x format, but was " + tag)
diff --git a/mg/errors.go b/mg/errors.go
index 9bf7fad4..2dd780fe 100644
--- a/mg/errors.go
+++ b/mg/errors.go
@@ -49,38 +49,3 @@ func ExitStatus(err error) int {
 	}
 	return exit.ExitStatus()
 }
-
-// wrappedError is an error that supports the Go 1.13+ mechanism of wrapping
-// errors. It implements Unwrap to return the underlying error, but it still
-// returns the string version of whatever "main" error it represents.
-type wrappedError struct {
-	underlyingError error
-	stringError     error
-}
-
-// WrapError returns an error that implements the Go 1.13 Unwrap interface. The
-// Error function returns the value of the "string" error, and the Unwrap
-// function returns the "underlying" error. Use this wherever one might
-// otherwise use the "%w" format string in [fmt.Errorf].
-//     err := doSomething()
-//     if err != nil {
-//         return WrapError(err, fmt.Errorf("Could not do something: %v", err))
-//     }
-// The premise is that the "string" error is not an interesting one to try to
-// inspect with [errors.Is] or [errors.As] because it has no other wrapped
-// errors of its own, and it will usually be of whatever error type
-// `fmt.Errorf` returns.
-func WrapError(underlying, str error) error {
-	return &wrappedError{
-		underlyingError: underlying,
-		stringError:     str,
-	}
-}
-
-func (e *wrappedError) Error() string {
-	return e.stringError.Error()
-}
-
-func (e *wrappedError) Unwrap() error {
-	return e.underlyingError
-}
diff --git a/mg/errors_test.go b/mg/errors_test.go
index 880e82d9..21c053e4 100644
--- a/mg/errors_test.go
+++ b/mg/errors_test.go
@@ -1,7 +1,6 @@
 package mg
 
 import (
-	"errors"
 	"testing"
 )
 
@@ -20,15 +19,3 @@ func TestFatalfExit(t *testing.T) {
 		t.Fatalf("Expected code %v but got %v", expected, code)
 	}
 }
-
-// TestBasicWrappedError confirms that a wrappedError returns the same string
-// as its "str" error (not its "underlying" error).
-func TestBasicWrappedError(t *testing.T) {
-	strError := errors.New("main error")
-	underlyingError := errors.New("underlying error")
-	actual := WrapError(underlyingError, strError)
-
-	if actual.Error() != strError.Error() {
-		t.Fatalf("Expected outer error to have Error() = %q but got %q", strError.Error(), actual.Error())
-	}
-}
diff --git a/mg/fn.go b/mg/fn.go
index b8aa5370..8e303080 100644
--- a/mg/fn.go
+++ b/mg/fn.go
@@ -37,7 +37,7 @@ func F(target interface{}, args ...interface{}) Fn {
 	}
 	id, err := json.Marshal(args)
 	if err != nil {
-		panic(WrapError(err, fmt.Errorf("can't convert args into a mage-compatible id for mg.Deps: %v", err)))
+		panic(fmt.Errorf("can't convert args into a mage-compatible id for mg.Deps: %v", err))
 	}
 	return fn{
 		name: funcName(target),
diff --git a/parse/parse.go b/parse/parse.go
index 1ae56bac..69490052 100644
--- a/parse/parse.go
+++ b/parse/parse.go
@@ -15,7 +15,6 @@ import (
 	"time"
 
 	"github.com/magefile/mage/internal"
-	"github.com/magefile/mage/mg"
 )
 
 const importTag = "mage:import"
@@ -736,7 +735,7 @@ func getPackage(path string, files []string, fset *token.FileSet) (*ast.Package,
 
 	pkgs, err := parser.ParseDir(fset, path, filter, parser.ParseComments)
 	if err != nil {
-		return nil, mg.WrapError(err, fmt.Errorf("failed to parse directory: %v", err))
+		return nil, internal.WrapError(err, fmt.Errorf("failed to parse directory: %v", err))
 	}
 
 	switch len(pkgs) {
diff --git a/sh/cmd.go b/sh/cmd.go
index c2693d93..9710f82a 100644
--- a/sh/cmd.go
+++ b/sh/cmd.go
@@ -9,6 +9,7 @@ import (
 	"os/exec"
 	"strings"
 
+	"github.com/magefile/mage/internal"
 	"github.com/magefile/mage/mg"
 )
 
@@ -120,7 +121,7 @@ func Exec(env map[string]string, stdout, stderr io.Writer, cmd string, args ...s
 	if ran {
 		return ran, mg.Fatalf(code, `running "%s %s" failed with exit code %d`, cmd, strings.Join(args, " "), code)
 	}
-	return ran, mg.WrapError(err, fmt.Errorf(`failed to run "%s %s: %v"`, cmd, strings.Join(args, " "), err))
+	return ran, internal.WrapError(err, fmt.Errorf(`failed to run "%s %s: %v"`, cmd, strings.Join(args, " "), err))
 }
 
 func run(env map[string]string, stdout, stderr io.Writer, cmd string, args ...string) (ran bool, code int, err error) {
diff --git a/sh/helpers.go b/sh/helpers.go
index 1e9542b9..fcf06538 100644
--- a/sh/helpers.go
+++ b/sh/helpers.go
@@ -5,7 +5,7 @@ import (
 	"io"
 	"os"
 
-	"github.com/magefile/mage/mg"
+	"github.com/magefile/mage/internal"
 )
 
 // Rm removes the given file or directory even if non-empty. It will not return
@@ -15,28 +15,28 @@ func Rm(path string) error {
 	if err == nil || os.IsNotExist(err) {
 		return nil
 	}
-	return mg.WrapError(err, fmt.Errorf(`failed to remove %s: %v`, path, err))
+	return internal.WrapError(err, fmt.Errorf(`failed to remove %s: %v`, path, err))
 }
 
 // Copy robustly copies the source file to the destination, overwriting the destination if necessary.
 func Copy(dst string, src string) error {
 	from, err := os.Open(src)
 	if err != nil {
-		return mg.WrapError(err, fmt.Errorf(`can't copy %s: %v`, src, err))
+		return internal.WrapError(err, fmt.Errorf(`can't copy %s: %v`, src, err))
 	}
 	defer from.Close()
 	finfo, err := from.Stat()
 	if err != nil {
-		return mg.WrapError(err, fmt.Errorf(`can't stat %s: %v`, src, err))
+		return internal.WrapError(err, fmt.Errorf(`can't stat %s: %v`, src, err))
 	}
 	to, err := os.OpenFile(dst, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, finfo.Mode())
 	if err != nil {
-		return mg.WrapError(err, fmt.Errorf(`can't copy to %s: %v`, dst, err))
+		return internal.WrapError(err, fmt.Errorf(`can't copy to %s: %v`, dst, err))
 	}
 	defer to.Close()
 	_, err = io.Copy(to, from)
 	if err != nil {
-		return mg.WrapError(err, fmt.Errorf(`error copying %s to %s: %v`, src, dst, err))
+		return internal.WrapError(err, fmt.Errorf(`error copying %s to %s: %v`, src, dst, err))
 	}
 	return nil
 }
diff --git a/sh/helpers_test.go b/sh/helpers_test.go
index 698eba3f..ea4484da 100644
--- a/sh/helpers_test.go
+++ b/sh/helpers_test.go
@@ -8,7 +8,7 @@ import (
 	"path/filepath"
 	"testing"
 
-	"github.com/magefile/mage/mg"
+	"github.com/magefile/mage/internal"
 	"github.com/magefile/mage/sh"
 )
 
@@ -18,11 +18,11 @@ import (
 func compareFiles(file1 string, file2 string) error {
 	s1, err := os.Stat(file1)
 	if err != nil {
-		return mg.WrapError(err, fmt.Errorf("can't stat %s: %v", file1, err))
+		return internal.WrapError(err, fmt.Errorf("can't stat %s: %v", file1, err))
 	}
 	s2, err := os.Stat(file2)
 	if err != nil {
-		return mg.WrapError(err, fmt.Errorf("can't stat %s: %v", file2, err))
+		return internal.WrapError(err, fmt.Errorf("can't stat %s: %v", file2, err))
 	}
 	if s1.Size() != s2.Size() {
 		return fmt.Errorf("files %s and %s have different sizes: %d vs %d", file1, file2, s1.Size(), s2.Size())
@@ -32,11 +32,11 @@ func compareFiles(file1 string, file2 string) error {
 	}
 	f1bytes, err := ioutil.ReadFile(file1)
 	if err != nil {
-		return mg.WrapError(err, fmt.Errorf("can't read %s: %v", file1, err))
+		return internal.WrapError(err, fmt.Errorf("can't read %s: %v", file1, err))
 	}
 	f2bytes, err := ioutil.ReadFile(file2)
 	if err != nil {
-		return mg.WrapError(err, fmt.Errorf("can't read %s: %v", file2, err))
+		return internal.WrapError(err, fmt.Errorf("can't read %s: %v", file2, err))
 	}
 	if !bytes.Equal(f1bytes, f2bytes) {
 		return fmt.Errorf("files %s and %s have different contents", file1, file2)

From 383639d3edaaf9a7a0124a89e81109bd9d958d40 Mon Sep 17 00:00:00 2001
From: Rob Kennedy <rob@sweetkennedy.net>
Date: Sun, 5 May 2024 08:30:11 -0500
Subject: [PATCH 3/4] Streamline wrapped error formatting

Instead of having the caller create an error with fmt.Errorf and then
pass it to WrapError, we now have WrapErrorf that wraps the error and
formats a string error at once.
---
 internal/errors.go         | 32 ++++++++++++++++++--------------
 internal/errors113_test.go |  3 +--
 internal/errors_test.go    | 23 +++++++++++++++++------
 internal/run.go            |  2 +-
 mage/main.go               | 26 +++++++++++++-------------
 mage/main_test.go          | 16 ++++++++--------
 magefile.go                |  8 ++++----
 parse/parse.go             |  2 +-
 sh/cmd.go                  |  2 +-
 sh/helpers.go              | 11 +++++------
 sh/helpers_test.go         |  8 ++++----
 11 files changed, 73 insertions(+), 60 deletions(-)

diff --git a/internal/errors.go b/internal/errors.go
index f264a51a..8e2a2283 100644
--- a/internal/errors.go
+++ b/internal/errors.go
@@ -1,5 +1,7 @@
 package internal
 
+import "fmt"
+
 // wrappedError is an error that supports the Go 1.13+ mechanism of wrapping
 // errors. It implements Unwrap to return the underlying error, but it still
 // returns the string version of whatever "main" error it represents.
@@ -8,22 +10,24 @@ type wrappedError struct {
 	stringError     error
 }
 
-// WrapError returns an error that implements the Go 1.13 Unwrap interface. The
-// Error function returns the value of the "string" error, and the Unwrap
-// function returns the "underlying" error. Use this wherever one might
-// otherwise use the "%w" format string in [fmt.Errorf].
-//     err := doSomething()
-//     if err != nil {
-//         return WrapError(err, fmt.Errorf("Could not do something: %v", err))
-//     }
-// The premise is that the "string" error is not an interesting one to try to
-// inspect with [errors.Is] or [errors.As] because it has no other wrapped
-// errors of its own, and it will usually be of whatever error type
-// `fmt.Errorf` returns.
-func WrapError(underlying, str error) error {
+// WrapErrorf returns an error that implements the Go 1.13 Unwrap interface.
+// The Error function returns the message calculated from formatting the
+// arguments, just like [fmt.Errorf], and the Unwrap function returns the
+// "underlying" error. Use this wherever one might otherwise use the "%w"
+// format string in [fmt.Errorf].
+//
+//	err := doSomething()
+//	if err != nil {
+//	    return WrapErrorf(err, "Could not do something: %v", err)
+//	}
+//
+// Although the "%w" format string will be recognized in versions of Go that
+// support it, any error wrapped by it will not be included in this error; only
+// underlying is considered wrapped by this error.
+func WrapErrorf(underlying error, format string, args ...interface{}) error {
 	return &wrappedError{
 		underlyingError: underlying,
-		stringError:     str,
+		stringError:     fmt.Errorf(format, args...),
 	}
 }
 
diff --git a/internal/errors113_test.go b/internal/errors113_test.go
index 6e0fcc5b..8a0c51f3 100644
--- a/internal/errors113_test.go
+++ b/internal/errors113_test.go
@@ -14,9 +14,8 @@ import (
 // TestErrorUnwrap checks that [errors.Is] can detect the underlying error in a
 // wrappedError.
 func TestErrorUnwrap(t *testing.T) {
-	strError := errors.New("main error")
 	underlyingError := errors.New("underlying error")
-	actual := WrapError(underlyingError, strError)
+	actual := WrapErrorf(underlyingError, "main message")
 
 	if !errors.Is(actual, underlyingError) {
 		t.Fatalf("Expected outer error %#v to include %#v", actual, underlyingError)
diff --git a/internal/errors_test.go b/internal/errors_test.go
index 3efca544..1df01912 100644
--- a/internal/errors_test.go
+++ b/internal/errors_test.go
@@ -5,14 +5,25 @@ import (
 	"testing"
 )
 
-// TestBasicWrappedError confirms that a wrappedError returns the same string
-// as its "str" error (not its "underlying" error).
+// TestBasicWrappedError confirms that a wrappedError returns its string
+// message, not that of its "underlying" error.
 func TestBasicWrappedError(t *testing.T) {
-	strError := errors.New("main error")
+	message := "main message"
 	underlyingError := errors.New("underlying error")
-	actual := WrapError(underlyingError, strError)
+	actual := WrapErrorf(underlyingError, message)
 
-	if actual.Error() != strError.Error() {
-		t.Fatalf("Expected outer error to have Error() = %q but got %q", strError.Error(), actual.Error())
+	if actual.Error() != message {
+		t.Fatalf("Expected outer error to have Error() = %q but got %q", message, actual.Error())
+	}
+}
+
+// TestWrapFormat checks that the arguments get formatted, just like
+// [fmt.Sprintf].
+func TestWrapFormat(t *testing.T) {
+	underlyingError := errors.New("underlying error")
+	actual := WrapErrorf(underlyingError, "%s %s", "main", "message")
+
+	if actual.Error() != "main message" {
+		t.Fatalf("Expected outer error to have formatted message, but got %q", actual.Error())
 	}
 }
diff --git a/internal/run.go b/internal/run.go
index 12a5ed21..cad3685c 100644
--- a/internal/run.go
+++ b/internal/run.go
@@ -52,7 +52,7 @@ func OutputDebug(cmd string, args ...string) (string, error) {
 	if err := c.Run(); err != nil {
 		errMsg := strings.TrimSpace(errbuf.String())
 		debug.Print("error running '", cmd, strings.Join(args, " "), "': ", err, ": ", errMsg)
-		return "", WrapError(err, fmt.Errorf("error running \"%s %s\": %v\n%s", cmd, strings.Join(args, " "), err, errMsg))
+		return "", WrapErrorf(err, "error running \"%s %s\": %v\n%s", cmd, strings.Join(args, " "), err, errMsg)
 	}
 	return strings.TrimSpace(buf.String()), nil
 }
diff --git a/mage/main.go b/mage/main.go
index 4081c925..758c3c31 100644
--- a/mage/main.go
+++ b/mage/main.go
@@ -473,14 +473,14 @@ func listGoFiles(magePath, goCmd, tag string, envStr []string) ([]string, error)
 	if !filepath.IsAbs(magePath) {
 		cwd, err := os.Getwd()
 		if err != nil {
-			return nil, internal.WrapError(err, fmt.Errorf("can't get current working directory: %v", err))
+			return nil, internal.WrapErrorf(err, "can't get current working directory: %v", err)
 		}
 		magePath = filepath.Join(cwd, magePath)
 	}
 
 	env, err := internal.SplitEnv(envStr)
 	if err != nil {
-		return nil, internal.WrapError(err, fmt.Errorf("error parsing environment variables: %v", err))
+		return nil, internal.WrapErrorf(err, "error parsing environment variables: %v", err)
 	}
 
 	bctx := build.Default
@@ -502,7 +502,7 @@ func listGoFiles(magePath, goCmd, tag string, envStr []string) ([]string, error)
 
 		// Allow multiple packages in the same directory
 		if _, ok := err.(*build.MultiplePackageError); !ok {
-			return nil, internal.WrapError(err, fmt.Errorf("failed to parse go source files: %v", err))
+			return nil, internal.WrapErrorf(err, "failed to parse go source files: %v", err)
 		}
 	}
 
@@ -530,7 +530,7 @@ func Magefiles(magePath, goos, goarch, goCmd string, stderr io.Writer, isMagefil
 	debug.Println("getting all files including those with mage tag in", magePath)
 	mageFiles, err := listGoFiles(magePath, goCmd, "mage", env)
 	if err != nil {
-		return nil, internal.WrapError(err, fmt.Errorf("listing mage files: %v", err))
+		return nil, internal.WrapErrorf(err, "listing mage files: %v", err)
 	}
 
 	if isMagefilesDirectory {
@@ -546,7 +546,7 @@ func Magefiles(magePath, goos, goarch, goCmd string, stderr io.Writer, isMagefil
 	debug.Println("getting all files without mage tag in", magePath)
 	nonMageFiles, err := listGoFiles(magePath, goCmd, "", env)
 	if err != nil {
-		return nil, internal.WrapError(err, fmt.Errorf("listing non-mage files: %v", err))
+		return nil, internal.WrapErrorf(err, "listing non-mage files: %v", err)
 	}
 
 	// convert non-Mage list to a map of files to exclude.
@@ -612,7 +612,7 @@ func GenerateMainfile(binaryName, path string, info *parse.PkgInfo) error {
 
 	f, err := os.Create(path)
 	if err != nil {
-		return internal.WrapError(err, fmt.Errorf("error creating generated mainfile: %v", err))
+		return internal.WrapErrorf(err, "error creating generated mainfile: %v", err)
 	}
 	defer f.Close()
 	data := mainfileTemplateData{
@@ -629,16 +629,16 @@ func GenerateMainfile(binaryName, path string, info *parse.PkgInfo) error {
 
 	debug.Println("writing new file at", path)
 	if err := mainfileTemplate.Execute(f, data); err != nil {
-		return internal.WrapError(err, fmt.Errorf("can't execute mainfile template: %v", err))
+		return internal.WrapErrorf(err, "can't execute mainfile template: %v", err)
 	}
 	if err := f.Close(); err != nil {
-		return internal.WrapError(err, fmt.Errorf("error closing generated mainfile: %v", err))
+		return internal.WrapErrorf(err, "error closing generated mainfile: %v", err)
 	}
 	// we set an old modtime on the generated mainfile so that the go tool
 	// won't think it has changed more recently than the compiled binary.
 	longAgo := time.Now().Add(-time.Hour * 24 * 365 * 10)
 	if err := os.Chtimes(path, longAgo, longAgo); err != nil {
-		return internal.WrapError(err, fmt.Errorf("error setting old modtime on generated mainfile: %v", err))
+		return internal.WrapErrorf(err, "error setting old modtime on generated mainfile: %v", err)
 	}
 	return nil
 }
@@ -675,13 +675,13 @@ func ExeName(goCmd, cacheDir string, files []string) (string, error) {
 func hashFile(fn string) (string, error) {
 	f, err := os.Open(fn)
 	if err != nil {
-		return "", internal.WrapError(err, fmt.Errorf("can't open input file for hashing: %v", err))
+		return "", internal.WrapErrorf(err, "can't open input file for hashing: %v", err)
 	}
 	defer f.Close()
 
 	h := sha1.New()
 	if _, err := io.Copy(h, f); err != nil {
-		return "", internal.WrapError(err, fmt.Errorf("can't write data to hash: %v", err))
+		return "", internal.WrapErrorf(err, "can't write data to hash: %v", err)
 	}
 	return fmt.Sprintf("%x", h.Sum(nil)), nil
 }
@@ -690,12 +690,12 @@ func generateInit(dir string) error {
 	debug.Println("generating default magefile in", dir)
 	f, err := os.Create(filepath.Join(dir, initFile))
 	if err != nil {
-		return internal.WrapError(err, fmt.Errorf("could not create mage template: %v", err))
+		return internal.WrapErrorf(err, "could not create mage template: %v", err)
 	}
 	defer f.Close()
 
 	if err := initOutput.Execute(f, nil); err != nil {
-		return internal.WrapError(err, fmt.Errorf("can't execute magefile template: %v", err))
+		return internal.WrapErrorf(err, "can't execute magefile template: %v", err)
 	}
 
 	return nil
diff --git a/mage/main_test.go b/mage/main_test.go
index 1c064d7d..d4ddd9de 100644
--- a/mage/main_test.go
+++ b/mage/main_test.go
@@ -1270,8 +1270,8 @@ func TestCompiledFlags(t *testing.T) {
 		cmd.Stderr = stderr
 		cmd.Stdout = stdout
 		if err := cmd.Run(); err != nil {
-			return internal.WrapError(err, fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
-				filename, strings.Join(args, " "), err, stdout, stderr))
+			return internal.WrapErrorf(err, "running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
+				filename, strings.Join(args, " "), err, stdout, stderr)
 		}
 		return nil
 	}
@@ -1357,8 +1357,8 @@ func TestCompiledEnvironmentVars(t *testing.T) {
 		cmd.Stderr = stderr
 		cmd.Stdout = stdout
 		if err := cmd.Run(); err != nil {
-			return internal.WrapError(err, fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
-				filename, strings.Join(args, " "), err, stdout, stderr))
+			return internal.WrapErrorf(err, "running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
+				filename, strings.Join(args, " "), err, stdout, stderr)
 		}
 		return nil
 	}
@@ -1511,8 +1511,8 @@ func TestSignals(t *testing.T) {
 		cmd.Stderr = stderr
 		cmd.Stdout = stdout
 		if err := cmd.Start(); err != nil {
-			return internal.WrapError(err, fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
-				filename, target, err, stdout, stderr))
+			return internal.WrapErrorf(err, "running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
+				filename, target, err, stdout, stderr)
 		}
 		pid := cmd.Process.Pid
 		go func() {
@@ -1523,8 +1523,8 @@ func TestSignals(t *testing.T) {
 			}
 		}()
 		if err := cmd.Wait(); err != nil {
-			return internal.WrapError(err, fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
-				filename, target, err, stdout, stderr))
+			return internal.WrapErrorf(err, "running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
+				filename, target, err, stdout, stderr)
 		}
 		return nil
 	}
diff --git a/magefile.go b/magefile.go
index 83410b2a..873d1d95 100644
--- a/magefile.go
+++ b/magefile.go
@@ -43,12 +43,12 @@ func Install() error {
 	// in GOPATH environment string
 	bin, err := sh.Output(gocmd, "env", "GOBIN")
 	if err != nil {
-		return internal.WrapError(err, fmt.Errorf("can't determine GOBIN: %v", err))
+		return internal.WrapErrorf(err, "can't determine GOBIN: %v", err)
 	}
 	if bin == "" {
 		gopath, err := sh.Output(gocmd, "env", "GOPATH")
 		if err != nil {
-			return internal.WrapError(err, fmt.Errorf("can't determine GOPATH: %v", err))
+			return internal.WrapErrorf(err, "can't determine GOPATH: %v", err)
 		}
 		paths := strings.Split(gopath, string([]rune{os.PathListSeparator}))
 		bin = filepath.Join(paths[0], "bin")
@@ -56,7 +56,7 @@ func Install() error {
 	// specifically don't mkdirall, if you have an invalid gopath in the first
 	// place, that's not on us to fix.
 	if err := os.Mkdir(bin, 0700); err != nil && !os.IsExist(err) {
-		return internal.WrapError(err, fmt.Errorf("failed to create %q: %v", bin, err))
+		return internal.WrapErrorf(err, "failed to create %q: %v", bin, err)
 	}
 	path := filepath.Join(bin, name)
 
@@ -72,7 +72,7 @@ var releaseTag = regexp.MustCompile(`^v1\.[0-9]+\.[0-9]+$`)
 // Generates a new release. Expects a version tag in v1.x.x format.
 func Release(tag string) (err error) {
 	if _, err := exec.LookPath("goreleaser"); err != nil {
-		return internal.WrapError(err, fmt.Errorf("can't find goreleaser: %v", err))
+		return internal.WrapErrorf(err, "can't find goreleaser: %v", err)
 	}
 	if !releaseTag.MatchString(tag) {
 		return errors.New("TAG environment variable must be in semver v1.x.x format, but was " + tag)
diff --git a/parse/parse.go b/parse/parse.go
index 69490052..15eb46b9 100644
--- a/parse/parse.go
+++ b/parse/parse.go
@@ -735,7 +735,7 @@ func getPackage(path string, files []string, fset *token.FileSet) (*ast.Package,
 
 	pkgs, err := parser.ParseDir(fset, path, filter, parser.ParseComments)
 	if err != nil {
-		return nil, internal.WrapError(err, fmt.Errorf("failed to parse directory: %v", err))
+		return nil, internal.WrapErrorf(err, "failed to parse directory: %v", err)
 	}
 
 	switch len(pkgs) {
diff --git a/sh/cmd.go b/sh/cmd.go
index 9710f82a..adc992ac 100644
--- a/sh/cmd.go
+++ b/sh/cmd.go
@@ -121,7 +121,7 @@ func Exec(env map[string]string, stdout, stderr io.Writer, cmd string, args ...s
 	if ran {
 		return ran, mg.Fatalf(code, `running "%s %s" failed with exit code %d`, cmd, strings.Join(args, " "), code)
 	}
-	return ran, internal.WrapError(err, fmt.Errorf(`failed to run "%s %s: %v"`, cmd, strings.Join(args, " "), err))
+	return ran, internal.WrapErrorf(err, `failed to run "%s %s: %v"`, cmd, strings.Join(args, " "), err)
 }
 
 func run(env map[string]string, stdout, stderr io.Writer, cmd string, args ...string) (ran bool, code int, err error) {
diff --git a/sh/helpers.go b/sh/helpers.go
index fcf06538..ef6bf2ca 100644
--- a/sh/helpers.go
+++ b/sh/helpers.go
@@ -1,7 +1,6 @@
 package sh
 
 import (
-	"fmt"
 	"io"
 	"os"
 
@@ -15,28 +14,28 @@ func Rm(path string) error {
 	if err == nil || os.IsNotExist(err) {
 		return nil
 	}
-	return internal.WrapError(err, fmt.Errorf(`failed to remove %s: %v`, path, err))
+	return internal.WrapErrorf(err, `failed to remove %s: %v`, path, err)
 }
 
 // Copy robustly copies the source file to the destination, overwriting the destination if necessary.
 func Copy(dst string, src string) error {
 	from, err := os.Open(src)
 	if err != nil {
-		return internal.WrapError(err, fmt.Errorf(`can't copy %s: %v`, src, err))
+		return internal.WrapErrorf(err, `can't copy %s: %v`, src, err)
 	}
 	defer from.Close()
 	finfo, err := from.Stat()
 	if err != nil {
-		return internal.WrapError(err, fmt.Errorf(`can't stat %s: %v`, src, err))
+		return internal.WrapErrorf(err, `can't stat %s: %v`, src, err)
 	}
 	to, err := os.OpenFile(dst, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, finfo.Mode())
 	if err != nil {
-		return internal.WrapError(err, fmt.Errorf(`can't copy to %s: %v`, dst, err))
+		return internal.WrapErrorf(err, `can't copy to %s: %v`, dst, err)
 	}
 	defer to.Close()
 	_, err = io.Copy(to, from)
 	if err != nil {
-		return internal.WrapError(err, fmt.Errorf(`error copying %s to %s: %v`, src, dst, err))
+		return internal.WrapErrorf(err, `error copying %s to %s: %v`, src, dst, err)
 	}
 	return nil
 }
diff --git a/sh/helpers_test.go b/sh/helpers_test.go
index ea4484da..38ded150 100644
--- a/sh/helpers_test.go
+++ b/sh/helpers_test.go
@@ -18,11 +18,11 @@ import (
 func compareFiles(file1 string, file2 string) error {
 	s1, err := os.Stat(file1)
 	if err != nil {
-		return internal.WrapError(err, fmt.Errorf("can't stat %s: %v", file1, err))
+		return internal.WrapErrorf(err, "can't stat %s: %v", file1, err)
 	}
 	s2, err := os.Stat(file2)
 	if err != nil {
-		return internal.WrapError(err, fmt.Errorf("can't stat %s: %v", file2, err))
+		return internal.WrapErrorf(err, "can't stat %s: %v", file2, err)
 	}
 	if s1.Size() != s2.Size() {
 		return fmt.Errorf("files %s and %s have different sizes: %d vs %d", file1, file2, s1.Size(), s2.Size())
@@ -32,11 +32,11 @@ func compareFiles(file1 string, file2 string) error {
 	}
 	f1bytes, err := ioutil.ReadFile(file1)
 	if err != nil {
-		return internal.WrapError(err, fmt.Errorf("can't read %s: %v", file1, err))
+		return internal.WrapErrorf(err, "can't read %s: %v", file1, err)
 	}
 	f2bytes, err := ioutil.ReadFile(file2)
 	if err != nil {
-		return internal.WrapError(err, fmt.Errorf("can't read %s: %v", file2, err))
+		return internal.WrapErrorf(err, "can't read %s: %v", file2, err)
 	}
 	if !bytes.Equal(f1bytes, f2bytes) {
 		return fmt.Errorf("files %s and %s have different contents", file1, file2)

From 064a13a29795a90323fe300938148653c471a40b Mon Sep 17 00:00:00 2001
From: Rob Kennedy <rob@sweetkennedy.net>
Date: Sun, 5 May 2024 08:52:40 -0500
Subject: [PATCH 4/4] fixup! Move WrapError to internal package

---
 magefile.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/magefile.go b/magefile.go
index 873d1d95..34708dfc 100644
--- a/magefile.go
+++ b/magefile.go
@@ -17,6 +17,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/magefile/mage/internal"
 	"github.com/magefile/mage/mg"
 	"github.com/magefile/mage/sh"
 )