From a71a9e956b060d957fa7072ab9e8ede176437b67 Mon Sep 17 00:00:00 2001 From: Snider Date: Sat, 8 Aug 2026 11:36:05 +0100 Subject: [PATCH] =?UTF-8?q?fix(app):=20Find=20resolves=20programs=20on=20W?= =?UTF-8?q?indows=20=E2=80=94=20PATHEXT=20candidates,=20no=20mode=20bit?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects, one function, all Windows-fatal and all invisible from a POSIX runner: 1. isExecutable asked mode&0111. Windows has no execute bit — Stat synthesises 0666, or 0444 for read-only — so the test rejected EVERY file on the platform, git.exe included, and Find could never succeed there. This is the 'Program.Find: "git": not found in PATH' arm of the go-inference Windows failures (3 packages), and the same defect go-process fixed in v0.16.2. 2. No extension candidates: PathJoin(dir, "git") never tries git.exe, so fixing the mode test alone changes nothing. 3. The path-vs-name test matched only the platform separator, so "bin/tool" — which Go accepts on Windows — was hunted on PATH instead of checked directly. Shape mirrors go-process's proven fix: findWith/isExecutableWith take PATH and PATHEXT as arguments, and the extension list IS the platform switch (empty = POSIX mode-bit semantics, non-empty = Windows PATHEXT semantics), so every Windows rule is exercised by fixture from any host. Find passes the real environment, defaulting PATHEXT to Go's own .COM;.EXE;.BAT;.CMD when unset on Windows. Receipt: TestApp_isExecutableWith_Good is a 0644 git.exe accepted under a listed extension — the exact file the old logic rejected. Full module: ok, vet + gofmt clean. POSIX behaviour byte-identical (existing Find/isExecutable triplets pass unmodified). Co-Authored-By: Virgil --- app.go | 116 +++++++++++++++++++++++++++++++++++++++---- app_internal_test.go | 87 ++++++++++++++++++++++++++++++++ 2 files changed, 193 insertions(+), 10 deletions(-) diff --git a/app.go b/app.go index 869131cb..fd748710 100644 --- a/app.go +++ b/app.go @@ -47,26 +47,45 @@ func (a App) New(opts Options) App { // r := core.App{}.Find("node", "Node.js") // if r.OK { app := r.Value.(*App) } func (a App) Find(filename, name string) Result { - // If filename contains a separator, check it directly - if Contains(filename, string(PathSeparator)) { + pathExt := Env("PATHEXT") + if pathExt == "" && OS() == "windows" { + // Go's own default when the variable is unset. + pathExt = ".COM;.EXE;.BAT;.CMD" + } + return a.findWith(filename, name, Env("PATH"), pathExt) +} + +// findWith is Find with the environment as arguments, so tests can drive +// the Windows resolution rules from any platform against a fixture PATH. +// An empty extension list means POSIX semantics; a non-empty one means +// Windows PATHEXT semantics — the list IS the platform switch. +func (a App) findWith(filename, name, pathEnv, pathExt string) Result { + // A path rather than a bare name is checked directly. Go accepts "/" + // as a separator on Windows too, so both spellings mean "path" — + // matching only the platform separator sent "bin/tool" on a PATH + // hunt there instead of probing it. + if Contains(filename, string(PathSeparator)) || Contains(filename, "/") { abs := PathAbs(filename) if !abs.OK { return abs } - if isExecutable(abs.Value.(string)) { - return Result{&App{Name: name, Filename: filename, Path: abs.Value.(string)}, true} + for _, candidate := range executableCandidates(abs.Value.(string), pathExt) { + if isExecutableWith(candidate, pathExt) { + return Result{&App{Name: name, Filename: filename, Path: candidate}, true} + } } return Result{E("app.Find", Concat(filename, " not found"), nil), false} } // Search PATH - pathEnv := Env("PATH") if pathEnv == "" { return Result{E("app.Find", "PATH is empty", nil), false} } for _, dir := range Split(pathEnv, string(PathListSeparator)) { - candidate := PathJoin(dir, filename) - if isExecutable(candidate) { + for _, candidate := range executableCandidates(PathJoin(dir, filename), pathExt) { + if !isExecutableWith(candidate, pathExt) { + continue + } abs := PathAbs(candidate) if !abs.OK { continue @@ -77,8 +96,78 @@ func (a App) Find(filename, name string) Result { return Result{E("app.Find", Concat(filename, " not found on PATH"), nil), false} } -// isExecutable checks if a path exists and is executable. +// executableCandidates lists the filenames a program name may resolve to. +// With no extension list (POSIX) the path stands alone. With one (Windows +// PATHEXT) the name is tried as itself when it already carries a listed +// extension, then with each listed extension appended — "git" on disk is +// "git.exe", and the bare join alone would never find it. +func executableCandidates(path, pathExt string) []string { + exts := splitPathExt(pathExt) + if len(exts) == 0 { + return []string{path} + } + out := make([]string, 0, len(exts)+1) + if hasListedExt(path, exts) { + out = append(out, path) + } + for _, ext := range exts { + out = append(out, Concat(path, ext)) + } + return out +} + +// splitPathExt parses a PATHEXT value (";"-separated, case-insensitive) +// into normalised lower-case ".ext" entries. Empty input yields nil — +// the POSIX arm. +func splitPathExt(pathExt string) []string { + if pathExt == "" { + return nil + } + var out []string + for _, e := range Split(pathExt, ";") { + e = Lower(Trim(e)) + if e == "" { + continue + } + if !HasPrefix(e, ".") { + e = Concat(".", e) + } + out = append(out, e) + } + return out +} + +// hasListedExt reports whether path's extension appears in exts +// (already lower-cased ".ext" entries). +func hasListedExt(path string, exts []string) bool { + got := Lower(PathExt(path)) + if got == "" { + return false + } + for _, e := range exts { + if got == e { + return true + } + } + return false +} + +// isExecutable checks if a path exists and is executable on this platform. func isExecutable(path string) bool { + pathExt := Env("PATHEXT") + if pathExt == "" && OS() == "windows" { + pathExt = ".COM;.EXE;.BAT;.CMD" + } + return isExecutableWith(path, pathExt) +} + +// isExecutableWith is isExecutable with the extension list as an argument. +// POSIX (empty list) asks the mode bits. Windows has no execute bit — +// Stat synthesises 0666, or 0444 for a read-only file — so the old +// mode&0111 test rejected EVERY file on the platform, git.exe included, +// and Find could never succeed there. With a list present the answer is +// "a regular file whose extension is listed", and the mode is not asked. +func isExecutableWith(path, pathExt string) bool { r := Stat(path) if !r.OK { return false @@ -87,6 +176,13 @@ func isExecutable(path string) bool { IsDir() bool Mode() FileMode }) - // Regular file with at least one execute bit - return !info.IsDir() && info.Mode()&0111 != 0 + if info.IsDir() { + return false + } + exts := splitPathExt(pathExt) + if len(exts) == 0 { + // Regular file with at least one execute bit + return info.Mode()&0111 != 0 + } + return hasListedExt(path, exts) } diff --git a/app_internal_test.go b/app_internal_test.go index 72519ca9..c8942c82 100644 --- a/app_internal_test.go +++ b/app_internal_test.go @@ -14,3 +14,90 @@ func TestApp_isExecutable_Bad(t *T) { func TestApp_isExecutable_Ugly(t *T) { AssertFalse(t, isExecutable(t.TempDir())) } + +func TestApp_isExecutableWith_Good(t *T) { + // The Windows receipt: a 0644 file — no execute bit anywhere — IS + // executable when its extension is listed. The old mode&0111 test + // rejected exactly this file, which is why Find never succeeded on + // the platform. + path := Path(t.TempDir(), "git.exe") + RequireTrue(t, WriteFile(path, []byte("MZ"), 0o644).OK) + + AssertTrue(t, isExecutableWith(path, ".COM;.EXE;.BAT;.CMD")) +} +func TestApp_isExecutableWith_Bad(t *T) { + // An unlisted extension is not executable under PATHEXT semantics, + // however runnable its mode bits claim it is. + path := Path(t.TempDir(), "notes.txt") + RequireTrue(t, WriteFile(path, []byte("x"), 0o755).OK) + + AssertFalse(t, isExecutableWith(path, ".COM;.EXE")) +} +func TestApp_isExecutableWith_Ugly(t *T) { + // An empty list is the POSIX arm: mode bits decide, so a 0644 file + // stays non-executable and a directory never qualifies either way. + path := Path(t.TempDir(), "plain") + RequireTrue(t, WriteFile(path, []byte("x"), 0o644).OK) + + AssertFalse(t, isExecutableWith(path, "")) + AssertFalse(t, isExecutableWith(t.TempDir(), ".EXE")) +} + +func TestApp_findWith_Good(t *T) { + // Bare name "git" resolves to git.exe on a fixture PATH under + // PATHEXT semantics — candidate generation plus the extension + // check working together, from any host platform. + dir := t.TempDir() + RequireTrue(t, WriteFile(Path(dir, "git.exe"), []byte("MZ"), 0o644).OK) + + r := App{}.findWith("git", "Git", dir, ".COM;.EXE;.BAT;.CMD") + RequireTrue(t, r.OK) + AssertEqual(t, Path(dir, "git.exe"), r.Value.(*App).Path) +} +func TestApp_findWith_Bad(t *T) { + // Nothing on the fixture PATH satisfies the name; and an empty + // PATH is its own distinct failure. + AssertFalse(t, App{}.findWith("git", "Git", t.TempDir(), ".EXE").OK) + AssertFalse(t, App{}.findWith("git", "Git", "", ".EXE").OK) +} +func TestApp_findWith_Ugly(t *T) { + // A forward-slash path is a direct check on every platform — Go + // accepts "/" on Windows, so "sub/tool" must never be hunted on + // PATH. The candidate resolves via extension there too. + dir := t.TempDir() + RequireTrue(t, MkdirAll(Path(dir, "sub"), 0o755).OK) + RequireTrue(t, WriteFile(Path(dir, "sub", "tool.exe"), []byte("MZ"), 0o644).OK) + + r := App{}.findWith(Concat(dir, "/sub/tool"), "Tool", "", ".EXE") + RequireTrue(t, r.OK) + AssertEqual(t, Path(dir, "sub", "tool.exe"), r.Value.(*App).Path) +} + +func TestApp_splitPathExt_Good(t *T) { + exts := splitPathExt(".COM;.EXE;bat; .Cmd ;") + AssertEqual(t, 4, len(exts)) + AssertEqual(t, ".com", exts[0]) + AssertEqual(t, ".exe", exts[1]) + AssertEqual(t, ".bat", exts[2]) + AssertEqual(t, ".cmd", exts[3]) +} +func TestApp_splitPathExt_Bad(t *T) { + AssertEqual(t, 0, len(splitPathExt(""))) +} +func TestApp_executableCandidates_Good(t *T) { + // A name already carrying a listed extension is tried as itself + // first, then with each extension appended; a bare name only with + // the extensions; POSIX (empty list) is the path alone. + withExt := executableCandidates("dir/git.exe", ".COM;.EXE") + AssertEqual(t, 3, len(withExt)) + AssertEqual(t, "dir/git.exe", withExt[0]) + + bare := executableCandidates("dir/git", ".COM;.EXE") + AssertEqual(t, 2, len(bare)) + AssertEqual(t, "dir/git.com", bare[0]) + AssertEqual(t, "dir/git.exe", bare[1]) + + posix := executableCandidates("dir/git", "") + AssertEqual(t, 1, len(posix)) + AssertEqual(t, "dir/git", posix[0]) +}