diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 698594a6..5977440b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,13 +6,7 @@ jobs: strategy: fail-fast: false matrix: - go-version: - - stable - - 1.21.x - - 1.20.x - - 1.19.x - - 1.18.x - - 1.17.x + go-version: [ stable, 1.18.x ] # min and max versions should be sufficient os: [ubuntu-latest, macos-latest, windows-latest] steps: - name: Checkout code diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 3813744f..e828ed00 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -15,7 +15,7 @@ jobs: golangci: strategy: matrix: - go: [stable] + go: [ stable, 1.18.x ] # min and max versions should be sufficient os: [ubuntu-latest, macos-latest, windows-latest] name: lint runs-on: ${{ matrix.os }} diff --git a/.golangci.toml b/.golangci.toml index c7f46b6b..46b17be8 100644 --- a/.golangci.toml +++ b/.golangci.toml @@ -116,6 +116,9 @@ disable = [ ] enable-all = true +[linters.settings.modernize] +disable = ["any"] + [linters.settings.nestif] min-complexity = 9 diff --git a/.goreleaser.yml b/.goreleaser.yml index f60f117e..5e90d302 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -10,7 +10,7 @@ release: builds: - binary: mage main: . - ldflags: -s -w -X github.com/magefile/mage/mage.timestamp={{.Date}} -X github.com/magefile/mage/mage.commitHash={{.Commit}} -X github.com/magefile/mage/mage.gitTag={{.Version}} + ldflags: -s -w goos: - darwin - linux diff --git a/go.mod b/go.mod index 7491a95c..83daf2a8 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,3 @@ module github.com/magefile/mage -go 1.17 +go 1.18 diff --git a/internal/run_test.go b/internal/run_test.go new file mode 100644 index 00000000..f771af03 --- /dev/null +++ b/internal/run_test.go @@ -0,0 +1,136 @@ +package internal + +import ( + "runtime" + "strings" + "testing" +) + +func TestSplitEnv(t *testing.T) { + tests := []struct { + name string + env []string + wantLen int + wantErr bool + }{ + { + name: "normal env vars", + env: []string{"FOO=bar", "BAZ=qux"}, + wantLen: 2, + }, + { + name: "empty value", + env: []string{"FOO="}, + wantLen: 1, + }, + { + name: "value with equals sign", + env: []string{"FOO=bar=baz"}, + wantLen: 1, + }, + { + name: "empty input", + env: nil, + wantLen: 0, + }, + { + name: "malformed entry", + env: []string{"NO_EQUALS"}, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := SplitEnv(tt.env) + if (err != nil) != tt.wantErr { + t.Errorf("SplitEnv() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !tt.wantErr && len(got) != tt.wantLen { + t.Errorf("SplitEnv() returned %d entries, want %d", len(got), tt.wantLen) + } + }) + } +} + +func TestSplitEnvValues(t *testing.T) { + env := []string{"FOO=bar", "BAZ=qux=quux"} + got, err := SplitEnv(env) + if err != nil { + t.Fatal(err) + } + if got["FOO"] != "bar" { + t.Errorf("expected FOO=bar, got FOO=%s", got["FOO"]) + } + if got["BAZ"] != "qux=quux" { + t.Errorf("expected BAZ=qux=quux, got BAZ=%s", got["BAZ"]) + } +} + +func TestEnvWithCurrentGOOS(t *testing.T) { + env, err := EnvWithCurrentGOOS() + if err != nil { + t.Fatal(err) + } + + var foundGOOS, foundGOARCH bool + for _, e := range env { + parts := strings.SplitN(e, "=", 2) + if len(parts) != 2 { + continue + } + switch parts[0] { + case "GOOS": + foundGOOS = true + if parts[1] != runtime.GOOS { + t.Errorf("expected GOOS=%s, got %s", runtime.GOOS, parts[1]) + } + case "GOARCH": + foundGOARCH = true + if parts[1] != runtime.GOARCH { + t.Errorf("expected GOARCH=%s, got %s", runtime.GOARCH, parts[1]) + } + default: + // ignore other env vars + continue + } + } + if !foundGOOS { + t.Error("GOOS not found in env") + } + if !foundGOARCH { + t.Error("GOARCH not found in env") + } +} + +func TestRunDebug(t *testing.T) { + // Test successful command + err := RunDebug("echo", "hello") + if err != nil { + t.Fatalf("RunDebug with valid command failed: %v", err) + } + + // Test failed command + err = RunDebug("false") + if err == nil { + t.Fatal("RunDebug with failing command should return error") + } +} + +func TestOutputDebug(t *testing.T) { + // Test successful command + out, err := OutputDebug("echo", "hello") + if err != nil { + t.Fatalf("OutputDebug with valid command failed: %v", err) + } + if out != "hello" { + t.Errorf("expected 'hello', got %q", out) + } + + // Test failed command + _, err = OutputDebug("false") + if err == nil { + t.Fatal("OutputDebug with failing command should return error") + } +} diff --git a/mage/main.go b/mage/main.go index 8ff26aa5..ea5cd721 100644 --- a/mage/main.go +++ b/mage/main.go @@ -16,6 +16,7 @@ import ( "path/filepath" "regexp" "runtime" + dbg "runtime/debug" "sort" "strings" "syscall" @@ -69,13 +70,6 @@ const ( var debug = log.New(io.Discard, "DEBUG: ", log.Ltime|log.Lmicroseconds) -// set by ldflags when you "mage build". -var ( - commitHash = "" - timestamp = "" - gitTag = "" -) - //go:generate stringer -type=Command // Command tracks invocations of mage that run without targets or other flags. @@ -149,10 +143,7 @@ func ParseAndRun(stdout, stderr io.Writer, stdin io.Reader, args []string) int { switch cmd { case Version: - out.Println("Mage Build Tool", gitTag) - out.Println("Build Date:", timestamp) - out.Println("Commit:", commitHash) - out.Println("built with:", runtime.Version()) + doVersion(out) return 0 case Init: if err := generateInit(inv.Dir); err != nil { @@ -171,8 +162,38 @@ func ParseAndRun(stdout, stderr io.Writer, stdin io.Reader, args []string) int { case CompileStatic, None: return Invoke(inv) default: - panic(fmt.Errorf("unknown command type: %v", cmd)) + errlog.Printf("unknown command type: %v", cmd) + return 1 + } +} + +func doVersion(out *log.Logger) { + var ( + commitHash = "" + timestamp = "" + gitTag = "" + ) + + info, ok := dbg.ReadBuildInfo() + if ok { + if info.Main.Version != "" { + gitTag = info.Main.Version + } + for _, kv := range info.Settings { + switch kv.Key { + case "vcs.revision": + commitHash = kv.Value + case "vcs.time": + timestamp = kv.Value + default: + continue + } + } } + out.Println("Mage Build Tool", gitTag) + out.Println("Build Date:", timestamp) + out.Println("Commit:", commitHash) + out.Println("built with:", runtime.Version()) } // Parse parses the given args and returns structured data. If parse returns diff --git a/magefiles/targets/targets.go b/magefiles/targets/targets.go index 553fb785..eff99197 100644 --- a/magefiles/targets/targets.go +++ b/magefiles/targets/targets.go @@ -9,7 +9,6 @@ import ( "regexp" "runtime" "strings" - "time" "github.com/magefile/mage/mg" "github.com/magefile/mage/sh" @@ -17,6 +16,7 @@ import ( // Install runs "go install" for mage. This generates the version info the binary. func Install() error { + fmt.Println("`mage install` is deprecated. Just use `go install` now.") name := "mage" if runtime.GOOS == "windows" { name += ".exe" @@ -48,7 +48,7 @@ func Install() error { // install` turns into a no-op, and `go install -a` fails on people's // machines that have go installed in a non-writeable directory (such as // normal OS installs in /usr/bin) - return sh.RunV(gocmd, "build", "-o", path, "-ldflags="+flags(), "github.com/magefile/mage") + return sh.RunV(gocmd, "build", "-o", path, "github.com/magefile/mage") } var releaseTag = regexp.MustCompile(`^v1\.\d+\.\d+$`) @@ -81,28 +81,6 @@ func Clean() error { return sh.Rm("dist") } -func flags() string { - timestamp := time.Now().Format(time.RFC3339) - hash := hash() - tag := tag() - if tag == "" { - tag = "dev" - } - return fmt.Sprintf(`-X "github.com/magefile/mage/mage.timestamp=%s" -X "github.com/magefile/mage/mage.commitHash=%s" -X "github.com/magefile/mage/mage.gitTag=%s"`, timestamp, hash, tag) -} - -// tag returns the git tag for the current branch or "" if none. -func tag() string { - s, _ := sh.Output("git", "describe", "--tags") - return s -} - -// hash returns the git hash for the current repo or "" if none. -func hash() string { - hash, _ := sh.Output("git", "rev-parse", "--short", "HEAD") - return hash -} - var goTools = []string{ "github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.11.2", "github.com/goreleaser/goreleaser/v2@v2.14.3", diff --git a/mg/color.go b/mg/color.go index 7251a598..8cf48ccb 100644 --- a/mg/color.go +++ b/mg/color.go @@ -1,6 +1,8 @@ // Package mg provides support functions for running mage targets. package mg +import "strings" + // Color is ANSI color type. type Color int @@ -53,26 +55,10 @@ const AnsiColorReset = "\033[0m" // It is set to Cyan as an arbitrary color, because it has a neutral meaning. var DefaultTargetAnsiColor = ansiColor[Cyan] -func toLowerCase(s string) string { - // this is a naive implementation - // borrowed from https://golang.org/src/strings/strings.go - // and only considers alphabetical characters [a-zA-Z] - // so that we don't depend on the "strings" package - buf := make([]byte, len(s)) - for i := 0; i < len(s); i++ { - c := s[i] - if 'A' <= c && c <= 'Z' { - c += 'a' - 'A' - } - buf[i] = c - } - return string(buf) -} - func getAnsiColor(color string) (string, bool) { - colorLower := toLowerCase(color) + colorLower := strings.ToLower(color) for k, v := range ansiColor { - colorConstLower := toLowerCase(k.String()) + colorConstLower := strings.ToLower(k.String()) if colorConstLower == colorLower { return v, true } diff --git a/mg/deps_test.go b/mg/deps_test.go index 79b76c15..fe39bbb2 100644 --- a/mg/deps_test.go +++ b/mg/deps_test.go @@ -1,6 +1,7 @@ package mg import ( + "context" "errors" "fmt" "sync/atomic" @@ -196,3 +197,89 @@ func TestDepsErrors(t *testing.T) { }() f() } + +func TestDepPanicNonError(t *testing.T) { + f := func() { + panic("string panic value") + } + defer func() { + v := recover() + if v == nil { + t.Fatal("expected panic, but didn't get one") + } + actual := fmt.Sprint(v) + if actual != "string panic value" { + t.Fatalf(`expected "string panic value" but got %q`, actual) + } + err, ok := v.(error) + if !ok { + t.Fatalf("expected recovered val to be error but was %T", v) + } + code := ExitStatus(err) + if code != 1 { + t.Fatalf("Expected exit status 1 for non-error panic, but got %v", code) + } + }() + Deps(f) +} + +func TestCtxDepsPassesContext(t *testing.T) { + type ctxKey string + found := false + f := func(ctx context.Context) { + if ctx.Value(ctxKey("key")) != "value" { + t.Fatal("context value was not propagated") + } + found = true + } + ctx := context.WithValue(context.Background(), ctxKey("key"), "value") + CtxDeps(ctx, f) + if !found { + t.Fatal("context was not passed to dependency") + } +} + +func TestChangeExit(t *testing.T) { + tests := []struct { + name string + old int + nw int + want int + }{ + {"both zero", 0, 0, 0}, + {"old zero new nonzero", 0, 5, 5}, + {"old nonzero new zero", 5, 0, 5}, + {"same nonzero", 5, 5, 5}, + {"different nonzero", 5, 3, 1}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := changeExit(tt.old, tt.nw) + if got != tt.want { + t.Errorf("changeExit(%d, %d) = %d, want %d", tt.old, tt.nw, got, tt.want) + } + }) + } +} + +func TestSerialCtxDeps(t *testing.T) { + type ctxKey string + ch := make(chan string, 2) + f := func(ctx context.Context) { + if ctx.Value(ctxKey("key")) != "value" { + panic("missing context value") + } + ch <- "f" + } + g := func(_ context.Context) { + ch <- "g" + } + ctx := context.WithValue(context.Background(), ctxKey("key"), "value") + SerialCtxDeps(ctx, f, g) + + first := <-ch + second := <-ch + if first != "f" || second != "g" { + t.Fatalf("expected serial execution f then g, got %s then %s", first, second) + } +} diff --git a/mg/errors.go b/mg/errors.go index 36e664db..fc363291 100644 --- a/mg/errors.go +++ b/mg/errors.go @@ -39,14 +39,14 @@ func Fatalf(code int, format string, args ...interface{}) error { // ExitStatus queries the error for an exit status. If the error is nil, it // returns 0. If the error does not implement ExitStatus() int, it returns 1. -// Otherwise it retiurns the value from ExitStatus(). +// Otherwise it returns the value from ExitStatus(). func ExitStatus(err error) int { if err == nil { return 0 } - exit, ok := err.(exitStatus) - if !ok { - return 1 + var exit exitStatus + if errors.As(err, &exit) { + return exit.ExitStatus() } - return exit.ExitStatus() + return 1 } diff --git a/mg/errors_test.go b/mg/errors_test.go index ac5e68f6..634925dd 100644 --- a/mg/errors_test.go +++ b/mg/errors_test.go @@ -1,6 +1,10 @@ package mg -import "testing" +import ( + "errors" + "fmt" + "testing" +) func TestFatalExit(t *testing.T) { expected := 99 @@ -17,3 +21,64 @@ func TestFatalfExit(t *testing.T) { t.Fatalf("Expected code %v but got %v", expected, code) } } + +func TestExitStatusNil(t *testing.T) { + code := ExitStatus(nil) + if code != 0 { + t.Fatalf("expected 0 for nil error, got %d", code) + } +} + +func TestExitStatusGenericError(t *testing.T) { + code := ExitStatus(errors.New("generic")) + if code != 1 { + t.Fatalf("expected 1 for generic error, got %d", code) + } +} + +func TestExitStatusFatal(t *testing.T) { + code := ExitStatus(Fatal(42)) + if code != 42 { + t.Fatalf("expected 42, got %d", code) + } +} + +func TestFatalErrorMessage(t *testing.T) { + err := Fatal(1, "hello", " ", "world") + if err.Error() != "hello world" { + t.Fatalf("expected 'hello world', got %q", err.Error()) + } +} + +func TestFatalfErrorMessage(t *testing.T) { + err := Fatalf(1, "code %d", 42) + if err.Error() != "code 42" { + t.Fatalf("expected 'code 42', got %q", err.Error()) + } +} + +func TestFatalImplementsExitStatus(t *testing.T) { + err := Fatal(7, "test") + var es exitStatus + if !errors.As(err, &es) { + // Fatal error implements exitStatus directly via type assertion, not via errors.As, + // since fatalError doesn't support unwrap. Verify it implements the interface. + fe, ok := err.(exitStatus) + if !ok { + t.Fatal("Fatal error should implement exitStatus interface") + } + if fe.ExitStatus() != 7 { + t.Fatalf("expected exit status 7, got %d", fe.ExitStatus()) + } + } +} + +func TestWrappedFatalExitStatus(t *testing.T) { + inner := Fatal(42, "inner") + wrapped := fmt.Errorf("outer: %w", inner) + // With errors.As, wrapped errors now correctly propagate exit status + code := ExitStatus(wrapped) + if code != 42 { + t.Fatalf("expected 42 for wrapped Fatal, got %d", code) + } +} diff --git a/mg/fn_test.go b/mg/fn_test.go index 6ce48a2b..95f2b58c 100644 --- a/mg/fn_test.go +++ b/mg/fn_test.go @@ -2,6 +2,7 @@ package mg import ( "context" + "errors" "fmt" "reflect" "sync/atomic" @@ -267,3 +268,91 @@ func (Foo) CtxError(context.Context) error { return nil } func (Foo) CtxErrorArgs(_ context.Context, _ int, _ string, _ bool, _ time.Duration) error { return nil } + +func TestFNonFunction(t *testing.T) { + defer func() { + r := recover() + if r == nil { + t.Fatal("expected panic for non-function") + } + }() + F("not a function") +} + +func TestFTooManyArgs(t *testing.T) { + defer func() { + r := recover() + if r == nil { + t.Fatal("expected panic for too many args") + } + }() + F(func(int) {}, 1, 2) +} + +func TestFWrongArgType(t *testing.T) { + defer func() { + r := recover() + if r == nil { + t.Fatal("expected panic for wrong arg type") + } + }() + F(func(int) {}, "not an int") +} + +func TestFUnsupportedArgType(t *testing.T) { + defer func() { + r := recover() + if r == nil { + t.Fatal("expected panic for unsupported arg type") + } + }() + F(func(*int) {}, (*int)(nil)) +} + +func TestFTooManyReturns(t *testing.T) { + defer func() { + r := recover() + if r == nil { + t.Fatal("expected panic for too many return values") + } + }() + F(func() (int, error) { return 0, nil }) +} + +func TestFNonErrorReturn(t *testing.T) { + defer func() { + r := recover() + if r == nil { + t.Fatal("expected panic for non-error return") + } + }() + F(func() int { return 0 }) +} + +func TestFReturnsError(t *testing.T) { + origErr := errors.New("boom") + fn := F(func() error { return origErr }) + err := fn.Run(context.Background()) + if !errors.Is(err, origErr) { + t.Fatalf("expected 'boom' error, got %v", err) + } +} + +func TestFnID(t *testing.T) { + fn1 := F(func(int) {}, 1) + fn2 := F(func(int) {}, 2) + fn3 := F(func(int) {}, 1) + if fn1.ID() == fn2.ID() { + t.Error("different args should produce different IDs") + } + if fn1.ID() != fn3.ID() { + t.Error("same args should produce same IDs") + } +} + +func TestFnName(t *testing.T) { + fn := F(func() {}) + if fn.Name() == "" { + t.Error("expected non-empty function name") + } +} diff --git a/parse/parse.go b/parse/parse.go index 1a492e57..cd5c3ab0 100644 --- a/parse/parse.go +++ b/parse/parse.go @@ -27,7 +27,7 @@ func EnableDebug() { debug.SetOutput(os.Stderr) } -// PkgInfo contains inforamtion about a package of files according to mage's +// PkgInfo contains information about a package of files according to mage's // parsing rules. type PkgInfo struct { AstPkg *ast.Package @@ -657,7 +657,7 @@ func getImportPath(imp *ast.ImportSpec) (path, alias string, ok bool) { } func getImportPathFromCommentGroup(comments *ast.CommentGroup) []string { - if comments == nil || len(comments.List) == 9 { + if comments == nil || len(comments.List) == 0 { return nil } // import is always the last comment @@ -745,7 +745,7 @@ func setDefault(pi *PkgInfo) { f, err := getFunction(spec.Values[0], pi) if err != nil { - log.Println("warning, default declaration malformed:", err) + log.Println("warning: default declaration malformed:", err) return } pi.DefaultFunc = f diff --git a/parse/parse_test.go b/parse/parse_test.go index df9b12a7..e64fe020 100644 --- a/parse/parse_test.go +++ b/parse/parse_test.go @@ -1,6 +1,8 @@ package parse import ( + "go/ast" + "go/doc" "log" "os" "reflect" @@ -102,6 +104,162 @@ func TestParse(t *testing.T) { } } +func TestGetImportPathFromCommentGroupNil(t *testing.T) { + // nil comments should return nil + result := getImportPathFromCommentGroup(nil) + if result != nil { + t.Fatalf("expected nil for nil comments, got %v", result) + } +} + +func TestGetImportPathFromCommentGroupEmpty(t *testing.T) { + // empty comment list should return nil + cg := &ast.CommentGroup{List: []*ast.Comment{}} + result := getImportPathFromCommentGroup(cg) + if result != nil { + t.Fatalf("expected nil for empty comments, got %v", result) + } +} + +func TestGetImportPathFromCommentGroupNoImportTag(t *testing.T) { + cg := &ast.CommentGroup{List: []*ast.Comment{ + {Text: "// just a regular comment"}, + }} + result := getImportPathFromCommentGroup(cg) + if result != nil { + t.Fatalf("expected nil for non-import comment, got %v", result) + } +} + +func TestGetImportPathFromCommentGroupWithImportTag(t *testing.T) { + cg := &ast.CommentGroup{List: []*ast.Comment{ + {Text: "// mage:import"}, + }} + result := getImportPathFromCommentGroup(cg) + if result == nil { + t.Fatal("expected non-nil result for mage:import comment") + } + if len(result) != 1 || result[0] != "mage:import" { + t.Fatalf("expected [mage:import], got %v", result) + } +} + +func TestGetImportPathFromCommentGroupWithAlias(t *testing.T) { + cg := &ast.CommentGroup{List: []*ast.Comment{ + {Text: "// mage:import foo"}, + }} + result := getImportPathFromCommentGroup(cg) + if result == nil { + t.Fatal("expected non-nil result") + } + if len(result) != 2 || result[0] != "mage:import" || result[1] != "foo" { + t.Fatalf("expected [mage:import foo], got %v", result) + } +} + +func TestGetImportPathFromCommentGroupMultipleComments(t *testing.T) { + // import tag should be read from the last comment + cg := &ast.CommentGroup{List: []*ast.Comment{ + {Text: "// a preceding comment"}, + {Text: "// another comment"}, + {Text: "// mage:import myalias"}, + }} + result := getImportPathFromCommentGroup(cg) + if result == nil { + t.Fatal("expected non-nil result for mage:import in last comment") + } + if len(result) != 2 || result[0] != "mage:import" || result[1] != "myalias" { + t.Fatalf("expected [mage:import myalias], got %v", result) + } +} + +func TestCheckDupeTargetsNoDupes(t *testing.T) { + info := &PkgInfo{ + Funcs: Functions{ + {Name: "Build"}, + {Name: "Test"}, + {Name: "Clean"}, + }, + } + hasDupes, _ := checkDupeTargets(info) + if hasDupes { + t.Fatal("expected no duplicates") + } +} + +func TestCheckDupeTargetsCaseInsensitive(t *testing.T) { + info := &PkgInfo{ + Funcs: Functions{ + {Name: "Build"}, + {Name: "build"}, + }, + } + hasDupes, names := checkDupeTargets(info) + if !hasDupes { + t.Fatal("expected duplicates for case-insensitive match") + } + if len(names["build"]) != 2 { + t.Fatalf("expected 2 entries for 'build', got %d", len(names["build"])) + } +} + +func TestCheckDupeTargetsWithReceiver(t *testing.T) { + info := &PkgInfo{ + Funcs: Functions{ + {Name: "Build", Receiver: "Deploy"}, + {Name: "Build", Receiver: "Test"}, + }, + } + hasDupes, _ := checkDupeTargets(info) + if hasDupes { + t.Fatal("expected no duplicates - different receivers") + } +} + +func TestSanitizeSynopsis(t *testing.T) { + tests := []struct { + name string + funcName string + doc string + want string + }{ + { + name: "removes function name prefix", + funcName: "Clean", + doc: "Clean removes all generated files.", + want: "removes all generated files.", + }, + { + name: "case insensitive prefix removal", + funcName: "Build", + doc: "build compiles the project.", + want: "compiles the project.", + }, + { + name: "no prefix to remove", + funcName: "Build", + doc: "Compiles the project.", + want: "Compiles the project.", + }, + { + name: "empty doc", + funcName: "Build", + doc: "", + want: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := &doc.Func{Name: tt.funcName, Doc: tt.doc} + got := sanitizeSynopsis(f) + if got != tt.want { + t.Errorf("sanitizeSynopsis() = %q, want %q", got, tt.want) + } + }) + } +} + func TestGetImportSelf(t *testing.T) { imp, err := getImport("go", "github.com/magefile/mage/parse/testdata/importself", "") if err != nil { diff --git a/sh/cmd_test.go b/sh/cmd_test.go index 208955aa..7e7d993f 100644 --- a/sh/cmd_test.go +++ b/sh/cmd_test.go @@ -2,6 +2,7 @@ package sh import ( "bytes" + "errors" "os" "testing" ) @@ -67,3 +68,57 @@ func TestAutoExpand(t *testing.T) { t.Fatalf(`Expected "baz" but got %q`, s) } } + +func TestCmdRanNilErr(t *testing.T) { + if !CmdRan(nil) { + t.Fatal("CmdRan(nil) should return true") + } +} + +func TestCmdRanNotFound(t *testing.T) { + _, err := Exec(nil, nil, nil, "thiswontwork") + if CmdRan(err) { + t.Fatal("CmdRan should return false for not-found command") + } +} + +func TestExitStatusNil(t *testing.T) { + code := ExitStatus(nil) + if code != 0 { + t.Fatalf("expected 0 for nil error, got %d", code) + } +} + +func TestExitStatusNonExecError(t *testing.T) { + code := ExitStatus(errors.New("generic error")) + if code != 1 { + t.Fatalf("expected 1 for generic error, got %d", code) + } +} + +func TestExitStatusFromExec(t *testing.T) { + _, err := Exec(nil, nil, nil, os.Args[0], "-helper", "-exit", "42") + code := ExitStatus(err) + if code != 42 { + t.Fatalf("expected exit status 42, got %d", code) + } +} + +func TestRunCmd(t *testing.T) { + echoHello := RunCmd("echo", "hello") + err := echoHello("world") + // RunWith directs output based on verbose, so just check no error + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestOutputWith(t *testing.T) { + out, err := OutputWith(map[string]string{"MY_TEST_VAR": "xyz"}, os.Args[0], "-printVar", "MY_TEST_VAR") + if err != nil { + t.Fatal(err) + } + if out != "xyz" { + t.Fatalf("expected 'xyz', got %q", out) + } +} diff --git a/sh/helpers.go b/sh/helpers.go index 18be6dcb..11be360c 100644 --- a/sh/helpers.go +++ b/sh/helpers.go @@ -31,10 +31,13 @@ func Copy(dst, src string) error { if err != nil { return fmt.Errorf(`can't copy to %s: %w`, dst, err) } - defer to.Close() _, err = io.Copy(to, from) if err != nil { + _ = to.Close() return fmt.Errorf(`error copying %s to %s: %w`, src, dst, err) } + if err := to.Close(); err != nil { + return fmt.Errorf(`error closing %s: %w`, dst, err) + } return nil } diff --git a/sh/helpers_test.go b/sh/helpers_test.go index 9e20bc98..4a12d4b0 100644 --- a/sh/helpers_test.go +++ b/sh/helpers_test.go @@ -113,3 +113,65 @@ func TestHelpers(t *testing.T) { } }) } + +func TestCopyNonExistentSource(t *testing.T) { + dir := t.TempDir() + dst := filepath.Join(dir, "dst.txt") + err := sh.Copy(dst, filepath.Join(dir, "nonexistent.txt")) + if err == nil { + t.Fatal("expected error copying from non-existent source") + } +} + +func TestCopyReadOnlyDir(t *testing.T) { + dir := t.TempDir() + src := filepath.Join(dir, "src.txt") + if err := os.WriteFile(src, []byte("data"), 0o600); err != nil { + t.Fatal(err) + } + // Try to copy to a path inside a non-existent subdirectory + dst := filepath.Join(dir, "nodir", "dst.txt") + err := sh.Copy(dst, src) + if err == nil { + t.Fatal("expected error copying to non-existent directory") + } +} + +func TestCopyPreservesPermissions(t *testing.T) { + dir := t.TempDir() + src := filepath.Join(dir, "src.txt") + if err := os.WriteFile(src, []byte("data"), 0o600); err != nil { + t.Fatal(err) + } + dst := filepath.Join(dir, "dst.txt") + if err := sh.Copy(dst, src); err != nil { + t.Fatal(err) + } + srcInfo, _ := os.Stat(src) + dstInfo, _ := os.Stat(dst) + if srcInfo.Mode() != dstInfo.Mode() { + t.Errorf("permissions differ: src=%v dst=%v", srcInfo.Mode(), dstInfo.Mode()) + } +} + +func TestCopyOverwrite(t *testing.T) { + dir := t.TempDir() + src := filepath.Join(dir, "src.txt") + dst := filepath.Join(dir, "dst.txt") + if err := os.WriteFile(src, []byte("new content"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(dst, []byte("old content"), 0o600); err != nil { + t.Fatal(err) + } + if err := sh.Copy(dst, src); err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(dst) + if err != nil { + t.Fatal(err) + } + if string(data) != "new content" { + t.Fatalf("expected 'new content', got %q", string(data)) + } +} diff --git a/site/content/index.md b/site/content/index.md index 5e0fba26..b60898e6 100644 --- a/site/content/index.md +++ b/site/content/index.md @@ -11,18 +11,10 @@ and Mage automatically uses them as Makefile-like runnable targets. ### From GitHub source (any OS) -Mage has no dependencies outside the Go standard library, and builds with Go 1.7 -and above (possibly even lower versions, but they're not regularly tested). +Mage has no dependencies outside the Go standard library, and builds with Go 1.18 +and above. -#### Using Go Modules (With go version < 1.17) - -```plain -git clone https://github.com/magefile/mage -cd mage -go run bootstrap.go -``` - -#### Using Go Install (With go version >= [1.17](https://go.dev/doc/go-get-install-deprecation)) +#### Using Go Install ```plain go install github.com/magefile/mage@latest @@ -31,27 +23,9 @@ mage -init Instead of the @latest tag, you can specify the desired version, for example: ```plain -go install github.com/magefile/mage@v1.15.0 -``` - -#### Using GOPATH - -```plain -go get -u -d github.com/magefile/mage -cd $GOPATH/src/github.com/magefile/mage -go run bootstrap.go +go install github.com/magefile/mage@v1.16.0 ``` -This will download the code into your GOPATH, and then run the bootstrap script -to build mage with version information embedded in it. A normal `go get` -(without -d) will build the binary correctly, but no version info will be -embedded. If you've done this, no worries, just go to -`$GOPATH/src/github.com/magefile/mage` and run `mage install` or `go run -bootstrap.go` and a new binary will be created with the correct version -information. - -The mage binary will be created in your $GOPATH/bin directory. - ### From GitHub releases (any OS) You may also install a binary release from our diff --git a/target/newer.go b/target/newer.go index 7c958305..e055545c 100644 --- a/target/newer.go +++ b/target/newer.go @@ -3,13 +3,14 @@ package target import ( "errors" "fmt" + "io/fs" "os" "path/filepath" "time" ) var ( - // errNewer is an ugly sentinel error to cause filepath.Walk to abort + // errNewer is a sentinel error to cause filepath.WalkDir to abort // as soon as a newer file is encountered. errNewer = errors.New("newer item encountered") ) @@ -18,7 +19,11 @@ var ( // Sources are searched recursively and searching stops as soon as any entry // is newer than the target. func DirNewer(target time.Time, sources ...string) (bool, error) { - walkFn := func(_ string, info os.FileInfo, err error) error { + walkFn := func(_ string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + info, err := d.Info() if err != nil { return err } @@ -29,7 +34,7 @@ func DirNewer(target time.Time, sources ...string) (bool, error) { } for _, source := range sources { source = os.ExpandEnv(source) - err := filepath.Walk(source, walkFn) + err := filepath.WalkDir(source, walkFn) if err == nil { continue } @@ -84,9 +89,13 @@ func PathNewer(target time.Time, sources ...string) (bool, error) { // OldestModTime recurses a list of target filesystem objects and finds the // the oldest ModTime among them. func OldestModTime(targets ...string) (time.Time, error) { - t := time.Now().Add(time.Hour * 100000) + t := time.Date(9999, 1, 1, 0, 0, 0, 0, time.UTC) for _, target := range targets { - walkFn := func(_ string, info os.FileInfo, err error) error { + walkFn := func(_ string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + info, err := d.Info() if err != nil { return err } @@ -96,7 +105,7 @@ func OldestModTime(targets ...string) (time.Time, error) { } return nil } - if err := filepath.Walk(target, walkFn); err != nil { + if err := filepath.WalkDir(target, walkFn); err != nil { return t, err } } @@ -108,7 +117,11 @@ func OldestModTime(targets ...string) (time.Time, error) { func NewestModTime(targets ...string) (time.Time, error) { t := time.Time{} for _, target := range targets { - walkFn := func(_ string, info os.FileInfo, err error) error { + walkFn := func(_ string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + info, err := d.Info() if err != nil { return err } @@ -118,7 +131,7 @@ func NewestModTime(targets ...string) (time.Time, error) { } return nil } - if err := filepath.Walk(target, walkFn); err != nil { + if err := filepath.WalkDir(target, walkFn); err != nil { return t, err } } diff --git a/target/newer_test.go b/target/newer_test.go index b60f4106..7f043b3e 100644 --- a/target/newer_test.go +++ b/target/newer_test.go @@ -97,3 +97,74 @@ func TestOldestModTime(t *testing.T) { t.Fatal("expected newest mod time to match c") } } + +func TestDirNewerWithEmptyDir(t *testing.T) { + t.Parallel() + dir := t.TempDir() + // No files inside dir — nothing newer than any target time + newer, err := DirNewer(time.Now(), dir) + if err != nil { + t.Fatal(err) + } + // The directory itself was just created, its modtime is very recent + // so it should be newer than time.Time{} but we pass time.Now() + // The dir's modtime should be ≤ now, so it should not be newer + if newer { + t.Fatal("expected empty dir to not be newer than now") + } +} + +func TestDirNewerMissingSource(t *testing.T) { + t.Parallel() + dir := t.TempDir() + _, err := DirNewer(time.Now(), filepath.Join(dir, "nonexistent")) + if err == nil { + t.Fatal("expected error for missing source") + } +} + +func TestNewestModTimeEmptyDir(t *testing.T) { + t.Parallel() + dir := t.TempDir() + // An empty directory still has the directory entry itself + newest, err := NewestModTime(dir) + if err != nil { + t.Fatal(err) + } + // Should return the directory's own modtime + info, _ := os.Stat(dir) + if !newest.Equal(info.ModTime()) { + t.Fatalf("expected dir modtime, got %v vs %v", newest, info.ModTime()) + } +} + +func TestOldestModTimeEmptyDir(t *testing.T) { + t.Parallel() + dir := t.TempDir() + oldest, err := OldestModTime(dir) + if err != nil { + t.Fatal(err) + } + info, _ := os.Stat(dir) + if !oldest.Equal(info.ModTime()) { + t.Fatalf("expected dir modtime, got %v vs %v", oldest, info.ModTime()) + } +} + +func TestPathNewerMissingSource(t *testing.T) { + t.Parallel() + dir := t.TempDir() + _, err := PathNewer(time.Now(), filepath.Join(dir, "nonexistent")) + if err == nil { + t.Fatal("expected error for missing source") + } +} + +func TestGlobNewerNoMatch(t *testing.T) { + t.Parallel() + dir := t.TempDir() + _, err := GlobNewer(time.Now(), filepath.Join(dir, "*.nonexistent")) + if err == nil { + t.Fatal("expected error for glob with no matches") + } +} diff --git a/target/target.go b/target/target.go index 2bb00ae8..bdc223de 100644 --- a/target/target.go +++ b/target/target.go @@ -1,4 +1,4 @@ -// Package target provodes functions for checking if a file or directory is +// Package target provides functions for checking if a file or directory is // newer than another file or directory. This is useful for determining whether // a target needs to be rebuilt based on the modification times of its // dependencies.