From ff07e75a12882e9ed1539e6caa8081ddea39fd0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=83=9E=E3=82=A4=E3=83=8E=E3=83=AB?= <97069334+MY-RV@users.noreply.github.com> Date: Sun, 20 Sep 2026 23:12:43 -0600 Subject: [PATCH 1/3] =?UTF-8?q?fix:=20windows=20=E2=80=94=20find=20the=20s?= =?UTF-8?q?hell=20up=20the=20process=20tree,=20not=20one=20level=20up?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported from a real Windows host running v0.3.0-preview.1, which is what the preview was for. The roadmap said Windows detection was written from the documented behavior of those shells and unverified; it was also wrong. Every PowerShell user was told they were in cmd. Detection read the parent process, and something is usually in between: scoop installs godo as shims\godo.exe, which starts the real godo.exe as a child, so the parent of the process asking the question was *godo*. Not a shell, so it fell through to %ComSpec%. It now walks up to the nearest shell, which also covers npm, bun and make wrappers and an editor's terminal. The walk is a pure function over a process table, so it is tested off Windows too. Also from the same session: - A decorator at the end of a line was dropped in silence. YAML files it as a line comment on the value, and nothing reads it, so the catalog ran with a dialect its author believed they had written and failed somewhere else. Same for a decorator after the last key. Both are errors now, naming where the decorator belongs. - fs.slink on Windows returned "A required privilege is not held by the client" — true, and useless unless you already know Developer Mode grants it. - godo -e runners says how the shell was chosen. A wrong answer is only diagnosable if you can tell a stale GODO_SHELL from a fallback. --- CHANGELOG.md | 30 ++++++++++ docs/contract.md | 23 ++++++-- docs/guide/runners.md | 13 ++++- internal/catalog/decorator_test.go | 67 ++++++++++++++++++++++ internal/catalog/load.go | 55 ++++++++++++++++++ internal/cli/app.go | 4 +- internal/cli/e2e_contract_test.go | 5 +- internal/execshell/ancestor.go | 51 +++++++++++++++++ internal/execshell/ancestor_test.go | 85 ++++++++++++++++++++++++++++ internal/execshell/inherit.go | 23 ++++++-- internal/execshell/parent_windows.go | 41 ++++++++------ internal/plugin/invoke.go | 3 +- internal/plugin/symlink_other.go | 7 +++ internal/plugin/symlink_windows.go | 28 +++++++++ 14 files changed, 400 insertions(+), 35 deletions(-) create mode 100644 internal/catalog/decorator_test.go create mode 100644 internal/execshell/ancestor.go create mode 100644 internal/execshell/ancestor_test.go create mode 100644 internal/plugin/symlink_other.go create mode 100644 internal/plugin/symlink_windows.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 46d3649..9e1acb7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,36 @@ ## [Unreleased] +Found by running `v0.3.0-preview.1` on a real Windows host, which is what the +preview was for. The roadmap said Windows shell detection was written from +documentation and unverified; it was also wrong. + +### Fixed +- **Every PowerShell user on Windows was told they were in `cmd`.** Detection + read the parent process, and something is usually in between: scoop installs + godo as `shims\godo.exe`, which starts the real `godo.exe` as a child, so the + parent of the process asking the question was *godo*. Not a shell, so the + answer fell through to `%ComSpec%`. It now walks up the process tree to the + nearest shell, which also covers npm, bun and make wrappers and an editor's + terminal. +- **A decorator at the end of a line was dropped in silence.** + `ins: bun install # @dialect matcher` is a YAML line comment on the value, + and nothing reads it, so the catalog ran with a dialect its author believed + they had written — and failed somewhere else. Same for a decorator after the + last key, which YAML files as that key's foot comment. Both are errors now, + naming the decorator and where it belongs. Prose that merely mentions `@deps` + is still a comment. +- **`fs.slink` on Windows now says what to do.** Creating a symlink there is a + privilege, not a file operation, so an ordinary user got "A required + privilege is not held by the client" — true, and useless unless you already + know Developer Mode is what grants it. The error says so. + +### Changed +- `godo -e runners` says **how** the default shell was chosen — `your $SHELL`, + `found in the process tree`, `set by GODO_SHELL`, `%ComSpec%, because no + shell is in the process tree`. A wrong answer is only diagnosable if you can + tell a stale `GODO_SHELL` from a fallback, and a path alone cannot. + ## [0.3.0-preview.1] — 2026-09-20 **A preview.** It is a GitHub pre-release, so `godo -e update` does not offer diff --git a/docs/contract.md b/docs/contract.md index 61679db..48720a7 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -195,16 +195,16 @@ Selection, in order: |--|--| | `GODO_SHELL` | Always wins | | Unix | `$SHELL`, else `/bin/sh` | -| Windows | the parent process when it is a shell, else `%ComSpec%` | +| Windows | the nearest shell in the process tree, else `%ComSpec%` | -godo answers "which shell am I in" from the parent process, which is the only -thing that knows. No command run *inside* a shell can report it — it would only +godo answers "which shell am I in" by walking up the process tree, which is +the only thing that knows. No command run *inside* a shell can report it — it would only describe the shell godo just started. To confirm it yourself, in your own terminal: `echo $0` (sh, bash, zsh, dash, ksh), `echo $version` (fish), `$PSVersionTable.PSVersion` (PowerShell), `echo %COMSPEC%` (cmd). `godo -e runners` prints these too. -Windows reads the parent process because the environment cannot answer: +Windows reads the process tree because the environment cannot answer: PowerShell sets `PSModulePath` and everything it starts inherits it, so a `cmd.exe` opened from PowerShell would look like PowerShell. On Unix, `$SHELL` is the login shell rather than the one running right now — bash started inside @@ -330,6 +330,21 @@ that runner's bodies are not expanded either way. YAML comment block **immediately above** the script key. Apply to scripts only. +Above the key is the only placement that works, and godo refuses the two that +look like they should: + +```yaml + ins: bun install # @dialect matcher <- error + # @deps ins <- error, if nothing follows it +``` + +YAML files a trailing comment on the value and a comment after the last key on +that key, neither of which decorates anything. Silently dropping them meant a +catalog ran with a dialect or a dependency list its author believed they had +written, and the symptom arrived far away — a script matching nothing, a +dependency that never ran. A comment that merely mentions `@deps` in prose is +still a comment; only one that *starts* with a decorator is refused. + | Line | | |------|--| | `# text` (no `@`) | Doc | diff --git a/docs/guide/runners.md b/docs/guide/runners.md index 3056711..efd811e 100644 --- a/docs/guide/runners.md +++ b/docs/guide/runners.md @@ -83,7 +83,8 @@ A shell that is not installed fails when the plan is built, so nothing runs. ``` $ godo -e runners -inherit /bin/zsh [default] +inherit /bin/zsh + the default runner, your $SHELL shells found here: sh /bin/sh @@ -103,12 +104,18 @@ Selection order: |--|--| | `GODO_SHELL` | Always wins | | Unix | `$SHELL`, else `/bin/sh` | -| Windows | the parent process when it is a shell, else `%ComSpec%` | +| Windows | the nearest shell in the process tree, else `%ComSpec%` | -Windows reads the parent process because the environment cannot answer: +Windows reads the process tree because the environment cannot answer: PowerShell sets `PSModulePath` and everything it starts inherits it, so a `cmd.exe` opened from PowerShell would look like PowerShell. +It walks the tree rather than reading the parent alone because something is +usually in between. A package manager's shim is the common one: scoop installs +godo as `shims\godo.exe`, which starts the real `godo.exe` as a child — so the +parent of the process asking the question is *godo*. The same shape appears +with npm, bun and make wrappers, and inside an editor's terminal. + On Unix, `$SHELL` is your *login* shell, not necessarily the one running right now — bash started inside zsh still reports zsh. Every other tool follows that convention; `GODO_SHELL` is how you disagree with it. diff --git a/internal/catalog/decorator_test.go b/internal/catalog/decorator_test.go new file mode 100644 index 0000000..8a14d90 --- /dev/null +++ b/internal/catalog/decorator_test.go @@ -0,0 +1,67 @@ +package catalog_test + +import ( + "strings" + "testing" + + "github.com/my-rv/godo/internal/catalog" +) + +// A decorator at the end of the line is the failure a Windows user hit on the +// 0.3.0 preview: YAML files it as a line comment, godo read nobody's comment, +// and the catalog ran with a dialect its author believed they had written. +func TestDecorator_trailingOnTheLineIsAnError(t *testing.T) { + dir := t.TempDir() + path := writeCat(t, dir, "version: \"0.1\"\nscripts:\n ins: bun install # @dialect matcher\n b: echo b\n") + + _, err := catalog.LoadFile(path) + if err == nil { + t.Fatal("want an error, got a catalog that quietly ignored the decorator") + } + for _, want := range []string{"@dialect", "not read", "own line above the key"} { + if !strings.Contains(err.Error(), want) { + t.Fatalf("error %q does not say %q", err, want) + } + } +} + +// After the last key there is no next key to decorate, so YAML files it as +// that key's foot comment. Silently dropped before. +func TestDecorator_belowTheLastKeyIsAnError(t *testing.T) { + dir := t.TempDir() + path := writeCat(t, dir, "version: \"0.1\"\nscripts:\n a: echo a\n # @deps a\n") + + _, err := catalog.LoadFile(path) + if err == nil || !strings.Contains(err.Error(), "@deps") { + t.Fatalf("err=%v", err) + } +} + +// The check must not turn ordinary comments into errors. +func TestDecorator_proseMentioningOneIsStillAComment(t *testing.T) { + dir := t.TempDir() + path := writeCat(t, dir, "version: \"0.1\"\nscripts:\n a: echo a # like @deps but not\n b: echo b\n") + + if _, err := catalog.LoadFile(path); err != nil { + t.Fatalf("a comment that merely names a decorator is a comment: %v", err) + } +} + +// The placement that works has to keep working, blank line or not. +func TestDecorator_aboveTheKeyIsRead(t *testing.T) { + for name, doc := range map[string]string{ + "no blank line": "version: \"0.1\"\nscripts:\n a: echo a\n # @dialect matcher\n ${x}: echo ${godo:argv[x]}\n", + "blank line": "version: \"0.1\"\nscripts:\n a: echo a\n\n # @dialect matcher\n ${x}: echo ${godo:argv[x]}\n", + } { + t.Run(name, func(t *testing.T) { + dir := t.TempDir() + cat, err := catalog.LoadFile(writeCat(t, dir, doc)) + if err != nil { + t.Fatal(err) + } + if cat.Scripts[1].Dialect != "matcher" { + t.Fatalf("dialect=%q, want matcher", cat.Scripts[1].Dialect) + } + }) + } +} diff --git a/internal/catalog/load.go b/internal/catalog/load.go index 030d871..1e7c913 100644 --- a/internal/catalog/load.go +++ b/internal/catalog/load.go @@ -115,6 +115,9 @@ func Parse(data []byte, path string, dialects ...*DialectRegistry) (*Catalog, er func scriptFromNodes(key, val *yaml.Node, fileDialect DialectName, reg *DialectRegistry, path string) (Script, error) { s := Script{Key: key.Value} + if err := rejectMisplacedDecorators(key, val); err != nil { + return Script{}, fmt.Errorf("%w: script %q: %v", ErrInvalidCatalog, s.Key, err) + } dec, err := parseDecorators(key.HeadComment) if err != nil { return Script{}, fmt.Errorf("%w: script %q: %v", ErrInvalidCatalog, s.Key, err) @@ -183,6 +186,58 @@ func validateMatcherKey(key string) error { return nil } +// decoratorNames are the @-words parseDecorators answers to. +var decoratorNames = []string{"@deps", "@dependencies", "@dialect", "@runner"} + +// rejectMisplacedDecorators fails on a decorator YAML puts somewhere godo does +// not read. +// +// Only the comment block *above* a key decorates it. A decorator written at +// the end of the line lands on the value as a line comment, and one written +// after the last key lands on that key as a foot comment; both were read by +// nobody and dropped in silence. The catalog then ran with a dialect or a +// dependency list its author believed they had written, which surfaces far +// from the line that caused it — a script matching nothing, or a dependency +// that never runs. +// +// So it is an error, and the error says where the decorator belongs. A comment +// only trips this if it starts with a decorator godo knows: prose mentioning +// "@deps" is a comment, and stays one. +func rejectMisplacedDecorators(key, val *yaml.Node) error { + places := []struct { + comment string + where string + }{ + {key.LineComment, "at the end of the line"}, + {val.LineComment, "at the end of the line"}, + {key.FootComment, "below the key"}, + {val.FootComment, "below the key"}, + } + for _, p := range places { + if name := leadingDecorator(p.comment); name != "" { + return fmt.Errorf("%s is %s, where it is not read; a decorator goes on its own line above the key", name, p.where) + } + } + return nil +} + +// leadingDecorator returns the decorator a comment opens with, or "". +func leadingDecorator(comment string) string { + for _, line := range strings.Split(comment, "\n") { + line = strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(line), "#")) + fields := strings.Fields(line) + if len(fields) == 0 { + continue + } + for _, name := range decoratorNames { + if fields[0] == name { + return name + } + } + } + return "" +} + // decorators is the parsed @-block above a script key. type decorators struct { doc string diff --git a/internal/cli/app.go b/internal/cli/app.go index de1a275..5f0c32a 100644 --- a/internal/cli/app.go +++ b/internal/cli/app.go @@ -396,7 +396,9 @@ func (a *App) nearestCatalog() *catalog.Catalog { } func (a *App) listRunners(cat *catalog.Catalog) error { - fmt.Fprintf(a.Stdout, "%-10s %s [default]\n", string(catalog.RunnerInherit), execshell.DetectShell()) + shell, why := execshell.DetectedShell() + fmt.Fprintf(a.Stdout, "%-10s %s\n", string(catalog.RunnerInherit), shell) + fmt.Fprintf(a.Stdout, "%-10s the default runner, %s\n", "", why) fmt.Fprintln(a.Stdout) fmt.Fprintln(a.Stdout, "shells found here:") found := false diff --git a/internal/cli/e2e_contract_test.go b/internal/cli/e2e_contract_test.go index a6ba2b2..975f17e 100644 --- a/internal/cli/e2e_contract_test.go +++ b/internal/cli/e2e_contract_test.go @@ -165,7 +165,10 @@ func TestE2E_engineRunnersLists(t *testing.T) { t.Fatal(err) } got := out.String() - for _, want := range []string{"inherit", "[default]", "shells found here", "GODO_SHELL", "Not the shell you expected"} { + // The listing says which shell, and how it was chosen: a wrong shell is + // only diagnosable if the reader can tell a stale GODO_SHELL from a + // fallback. + for _, want := range []string{"inherit", "the default runner", "shells found here", "GODO_SHELL", "Not the shell you expected"} { if !strings.Contains(got, want) { t.Fatalf("missing %q in:\n%s", want, got) } diff --git a/internal/execshell/ancestor.go b/internal/execshell/ancestor.go new file mode 100644 index 0000000..6cbd569 --- /dev/null +++ b/internal/execshell/ancestor.go @@ -0,0 +1,51 @@ +package execshell + +// proc is one row of the process table: who started it, and what it is. +type proc struct { + parent uint32 + name string +} + +// maxAncestry bounds the walk. A shell is a handful of hops away at most; a +// longer chain means the answer is not up there, and the bound is also what +// keeps a corrupt table from spinning. +const maxAncestry = 16 + +// shellAncestor returns the nearest ancestor of pid that is a shell godo +// knows, or "". +// +// It walks rather than reading the parent alone because something is usually +// in between. A package manager's shim is the common one: scoop installs +// godo as shims\godo.exe, which starts the real godo.exe as a child, so the +// parent of the process asking this question is *godo* — and godo is not a +// shell, so reading one level up answered "no shell here" and fell back to +// %ComSpec%, telling every PowerShell user they were in cmd. The same shape +// appears with npm, bun and make wrappers, and inside an editor's terminal. +// +// Walking up is a wider question than "who started me" — it is "what shell am +// I under". That is the question the runner is trying to answer, and it is +// the same looseness $SHELL already has on Unix, where the login shell answers +// even when another shell is running right now. +// +// A pid is visited once. Windows reuses pids, so a table can point a process +// at a "parent" that is really its own descendant, and a cycle would otherwise +// never end. +func shellAncestor(table map[uint32]proc, pid uint32) string { + seen := make(map[uint32]bool, maxAncestry) + for i := 0; i < maxAncestry; i++ { + p, ok := table[pid] + if !ok || p.parent == 0 || seen[p.parent] { + return "" + } + seen[pid] = true + parent, ok := table[p.parent] + if !ok { + return "" + } + if isKnownShell(shellBase(parent.name)) { + return parent.name + } + pid = p.parent + } + return "" +} diff --git a/internal/execshell/ancestor_test.go b/internal/execshell/ancestor_test.go new file mode 100644 index 0000000..e4514f9 --- /dev/null +++ b/internal/execshell/ancestor_test.go @@ -0,0 +1,85 @@ +package execshell + +import "testing" + +func TestShellAncestor(t *testing.T) { + // explorer → WindowsTerminal → powershell → godo-shim → godo + table := map[uint32]proc{ + 4: {parent: 0, name: "System"}, + 100: {parent: 4, name: "explorer.exe"}, + 200: {parent: 100, name: "WindowsTerminal.exe"}, + 300: {parent: 200, name: "powershell.exe"}, + 400: {parent: 300, name: "godo.exe"}, // the scoop shim + 500: {parent: 400, name: "godo.exe"}, // godo itself + } + + t.Run("through a package manager shim", func(t *testing.T) { + // The bug this fixes: reading only the parent found godo.exe, decided + // no shell was there, and reported cmd.exe to a PowerShell user. + if got := shellAncestor(table, 500); got != "powershell.exe" { + t.Fatalf("got %q, want powershell.exe", got) + } + }) + + t.Run("directly from the shell", func(t *testing.T) { + if got := shellAncestor(table, 400); got != "powershell.exe" { + t.Fatalf("got %q, want powershell.exe", got) + } + }) + + t.Run("the nearest shell wins", func(t *testing.T) { + // A cmd started from PowerShell is the shell you are in. + table := map[uint32]proc{ + 300: {parent: 0, name: "powershell.exe"}, + 310: {parent: 300, name: "cmd.exe"}, + 320: {parent: 310, name: "godo.exe"}, + } + if got := shellAncestor(table, 320); got != "cmd.exe" { + t.Fatalf("got %q, want cmd.exe", got) + } + }) + + t.Run("no shell up there", func(t *testing.T) { + table := map[uint32]proc{ + 4: {parent: 0, name: "System"}, + 600: {parent: 4, name: "services.exe"}, + 610: {parent: 600, name: "godo.exe"}, + } + if got := shellAncestor(table, 610); got != "" { + t.Fatalf("got %q, want the caller to fall back", got) + } + }) + + t.Run("a pid missing from the table", func(t *testing.T) { + if got := shellAncestor(table, 9999); got != "" { + t.Fatalf("got %q, want empty", got) + } + }) +} + +// Windows reuses pids, so a table can point a process at a "parent" that is +// really its own descendant. The walk has to end anyway. +func TestShellAncestorTerminatesOnACycle(t *testing.T) { + table := map[uint32]proc{ + 10: {parent: 20, name: "a.exe"}, + 20: {parent: 10, name: "b.exe"}, + } + if got := shellAncestor(table, 10); got != "" { + t.Fatalf("got %q, want empty", got) + } +} + +// A chain of wrappers deeper than the bound is not worth walking, but it must +// not hang either. +func TestShellAncestorStopsAtTheBound(t *testing.T) { + table := map[uint32]proc{1: {parent: 0, name: "powershell.exe"}} + var pid uint32 = 1 + for i := 0; i < maxAncestry+5; i++ { + next := pid + 1 + table[next] = proc{parent: pid, name: "wrapper.exe"} + pid = next + } + if got := shellAncestor(table, pid); got != "" { + t.Fatalf("got %q, want empty past the bound", got) + } +} diff --git a/internal/execshell/inherit.go b/internal/execshell/inherit.go index feb4ec5..8f04fe8 100644 --- a/internal/execshell/inherit.go +++ b/internal/execshell/inherit.go @@ -87,22 +87,33 @@ func (r InheritRunner) Run(command string) error { // now: bash started inside zsh still reports zsh. That is the convention every // other tool follows, and GODO_SHELL is the way to disagree with it too. func DetectShell() string { + shell, _ := DetectedShell() + return shell +} + +// DetectedShell is DetectShell with the reason it landed there. +// +// The reason is not decoration. When the detected shell is wrong, the useful +// question is which rule answered — a stale GODO_SHELL reads nothing like a +// fallback to %ComSpec% because no shell was found — and a reader cannot tell +// those apart from a path alone. +func DetectedShell() (shell, why string) { if s := strings.TrimSpace(os.Getenv("GODO_SHELL")); s != "" { - return s + return s, "set by GODO_SHELL" } if runtime.GOOS == "windows" { if s := callerShell(); s != "" { - return s + return s, "found in the process tree" } if c := strings.TrimSpace(os.Getenv("ComSpec")); c != "" { - return c + return c, "%ComSpec%, because no shell is in the process tree" } - return "cmd.exe" + return "cmd.exe", "the last resort; no shell in the process tree, no %ComSpec%" } if s := strings.TrimSpace(os.Getenv("SHELL")); s != "" { - return s + return s, "your $SHELL" } - return "/bin/sh" + return "/bin/sh", "the last resort; $SHELL is unset" } // shellBase is the shell's name, lowercased and without a .exe suffix. diff --git a/internal/execshell/parent_windows.go b/internal/execshell/parent_windows.go index 43c8457..3dd2e85 100644 --- a/internal/execshell/parent_windows.go +++ b/internal/execshell/parent_windows.go @@ -8,45 +8,50 @@ import ( "unsafe" ) -// callerShell returns the shell that started this process, or "". +// callerShell returns the shell godo is running under, or "". // // Windows has no $SHELL, and the environment cannot answer the question: // PSModulePath is set by PowerShell but inherited by everything it starts, so -// a cmd.exe opened from PowerShell looks exactly like PowerShell. The parent -// process is the only honest answer, so that is what this reads. +// a cmd.exe opened from PowerShell looks exactly like PowerShell. The process +// tree is the only honest answer, so that is what this reads — see +// shellAncestor for why it is the tree and not just the parent. // -// Empty when the parent is not a shell — a build tool, an editor, a CI runner — -// which is the common case and why the caller falls back to %ComSpec%. +// Empty when no shell is up there — a service, a CI runner — which is why the +// caller falls back to %ComSpec%. func callerShell() string { - name, err := processName(uint32(os.Getppid())) - if err != nil || name == "" { - return "" - } - if !isKnownShell(shellBase(name)) { + table, err := processTable() + if err != nil { return "" } - return name + return shellAncestor(table, uint32(os.Getpid())) } -// processName returns the executable name of pid via the process snapshot. -func processName(pid uint32) (string, error) { +// processTable snapshots every running process once. +// +// One snapshot rather than one per hop: walking the tree would otherwise take +// a fresh snapshot at every level, and the tree could change underneath the +// walk between them. +func processTable() (map[uint32]proc, error) { snap, err := syscall.CreateToolhelp32Snapshot(syscall.TH32CS_SNAPPROCESS, 0) if err != nil { - return "", err + return nil, err } defer syscall.CloseHandle(snap) var e syscall.ProcessEntry32 e.Size = uint32(unsafe.Sizeof(e)) if err := syscall.Process32First(snap, &e); err != nil { - return "", err + return nil, err } + table := make(map[uint32]proc, 256) for { - if e.ProcessID == pid { - return syscall.UTF16ToString(e.ExeFile[:]), nil + table[e.ProcessID] = proc{ + parent: e.ParentProcessID, + name: syscall.UTF16ToString(e.ExeFile[:]), } if err := syscall.Process32Next(snap, &e); err != nil { - return "", err + // ERROR_NO_MORE_FILES ends the walk; the table is complete. + return table, nil } } } diff --git a/internal/plugin/invoke.go b/internal/plugin/invoke.go index 063dc51..c6e0b6c 100644 --- a/internal/plugin/invoke.go +++ b/internal/plugin/invoke.go @@ -320,9 +320,8 @@ func (p *Plugin) slinkOp(op Op, host Host) Result { return Result{Error: fmt.Sprintf("slink: dst: %v", err)} } - // Windows support for os.Symlink is unverified on a real Windows host. if err := os.Symlink(src, dst); err != nil { - return Result{Error: fmt.Sprintf("slink: %v", err)} + return Result{Error: fmt.Sprintf("slink: %v%s", err, symlinkHint(err))} } return Result{Code: 0, OK: true} } diff --git a/internal/plugin/symlink_other.go b/internal/plugin/symlink_other.go new file mode 100644 index 0000000..cd85749 --- /dev/null +++ b/internal/plugin/symlink_other.go @@ -0,0 +1,7 @@ +//go:build !windows + +package plugin + +// symlinkHint has nothing to add off Windows: creating a symlink there is an +// ordinary file operation, and the OS error already says what went wrong. +func symlinkHint(error) string { return "" } diff --git a/internal/plugin/symlink_windows.go b/internal/plugin/symlink_windows.go new file mode 100644 index 0000000..794e41c --- /dev/null +++ b/internal/plugin/symlink_windows.go @@ -0,0 +1,28 @@ +//go:build windows + +package plugin + +import ( + "errors" + "syscall" +) + +// errPrivilegeNotHeld is ERROR_PRIVILEGE_NOT_HELD, what Windows answers when a +// process without SeCreateSymbolicLinkPrivilege tries to create a symlink. +// +// Spelled out rather than imported: the stdlib syscall package does not name +// this one, and Windows error numbers are a stable ABI. +const errPrivilegeNotHeld = syscall.Errno(1314) + +// symlinkHint turns an OS refusal into something the reader can act on. +// +// On Windows creating a symlink is a privilege, not a file operation, so an +// ordinary user gets "A required privilege is not held by the client" — true, +// and useless unless you already know that Developer Mode is what grants it. +func symlinkHint(err error) string { + if !errors.Is(err, errPrivilegeNotHeld) { + return "" + } + return " (Windows grants this to an ordinary user only with Developer Mode on: " + + "Settings > System > For developers > Developer Mode. Otherwise run as Administrator)" +} From cae78b406cd6a838b3608aecfd70d8737c551f99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=83=9E=E3=82=A4=E3=83=8E=E3=83=AB?= <97069334+MY-RV@users.noreply.github.com> Date: Sun, 20 Sep 2026 23:27:07 -0600 Subject: [PATCH 2/3] fix: a CRLF catalog keeps its decorators, and slink links on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two corrections to the previous commit, both mine. A catalog written on Windows has CRLF, and with it the YAML parser files the comment above a key as the *previous* key's foot comment — so the decorator decorated nothing, unless a blank line happened to sit above it. That is the "no space between the command and the comment" report, and the misplaced- decorator error added in the last commit would have fired on a file where the decorator was placed correctly. Line endings are normalized before parsing, which YAML already calls the same line break; it also keeps a \r out of a block scalar, where it was reaching the shell as part of the command. slink is symlinks on Unix, junctions and links on Windows. It was calling os.Symlink everywhere, and the last commit answered the resulting failure with advice about Developer Mode — which is not the op working, it is the op explaining why it does not. It now creates a junction for a directory and a hard link for a file, neither of which needs a privilege. The reparse buffer is built in portable code so the layout is tested off Windows; the syscall around it is not. --- CHANGELOG.md | 30 +++++---- docs/dev/plugin-protocol.md | 7 +++ go.mod | 7 +-- go.sum | 2 - internal/catalog/decorator_test.go | 35 +++++++++++ internal/catalog/load.go | 21 ++++++- internal/plugin/invoke.go | 6 +- internal/plugin/reparse.go | 68 +++++++++++++++++++++ internal/plugin/reparse_test.go | 76 +++++++++++++++++++++++ internal/plugin/symlink_other.go | 10 ++- internal/plugin/symlink_windows.go | 97 ++++++++++++++++++++++++------ 11 files changed, 319 insertions(+), 40 deletions(-) create mode 100644 internal/plugin/reparse.go create mode 100644 internal/plugin/reparse_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e1acb7..22e4627 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,17 +14,25 @@ documentation and unverified; it was also wrong. answer fell through to `%ComSpec%`. It now walks up the process tree to the nearest shell, which also covers npm, bun and make wrappers and an editor's terminal. -- **A decorator at the end of a line was dropped in silence.** - `ins: bun install # @dialect matcher` is a YAML line comment on the value, - and nothing reads it, so the catalog ran with a dialect its author believed - they had written — and failed somewhere else. Same for a decorator after the - last key, which YAML files as that key's foot comment. Both are errors now, - naming the decorator and where it belongs. Prose that merely mentions `@deps` - is still a comment. -- **`fs.slink` on Windows now says what to do.** Creating a symlink there is a - privilege, not a file operation, so an ordinary user got "A required - privilege is not held by the client" — true, and useless unless you already - know Developer Mode is what grants it. The error says so. +- **A catalog written on Windows lost its decorators.** With CRLF the YAML + parser files the comment above a key as the *previous* key's foot comment, so + `# @dialect matcher` decorated nothing — unless a blank line happened to sit + above it, which is why it read as "no space between the command and the + comment breaks it". Line endings are normalized before parsing, which YAML + already calls the same line break. It also keeps a `\r` out of a block + scalar, where it was being handed to the shell as part of the command. +- **`fs.slink` did nothing on Windows.** It called `os.Symlink`, and a symlink + there is a privilege rather than a file operation, so an ordinary user got + "A required privilege is not held by the client". It now uses the link + Windows actually offers — a **junction** for a directory, a **hard link** for + a file — neither of which needs a privilege. The op is the same op; the + mechanism is the platform's. + +### Added +- A decorator YAML puts somewhere godo does not read is now an error naming + where it belongs, instead of being dropped: `ins: x # @deps y` files the + comment on the value, and a decorator after the last key files as that key's + foot comment. Prose that merely mentions `@deps` is still a comment. ### Changed - `godo -e runners` says **how** the default shell was chosen — `your $SHELL`, diff --git a/docs/dev/plugin-protocol.md b/docs/dev/plugin-protocol.md index 78c72cc..68ec9b0 100644 --- a/docs/dev/plugin-protocol.md +++ b/docs/dev/plugin-protocol.md @@ -111,6 +111,13 @@ unbuffered, so a plugin that spoke first would deadlock. {"code":0,"ok":true,"stdout":"","stderr":"","error":""} ``` +`slink` makes the link the platform offers: a **symlink** on Unix, and on +Windows a **junction** for a directory or a **hard link** for a file. Windows +treats a symlink as a privilege rather than a file operation, so asking for one +fails for an ordinary user; a junction and a hard link do not. Consequence: on +Windows the target must exist, because which link to make depends on what it +is, and a hard link cannot cross volumes. + `error` is godo refusing — an unknown op, or a capability the catalog did not grant. A command that ran and failed is `code`, not `error`. diff --git a/go.mod b/go.mod index 1354bf4..5b5ad64 100644 --- a/go.mod +++ b/go.mod @@ -2,9 +2,8 @@ module github.com/my-rv/godo go 1.26.0 -require gopkg.in/yaml.v3 v3.0.1 - require ( - github.com/tetratelabs/wazero v1.12.0 // indirect - golang.org/x/sys v0.44.0 // indirect + github.com/tetratelabs/wazero v1.12.0 + golang.org/x/sys v0.44.0 + gopkg.in/yaml.v3 v3.0.1 ) diff --git a/go.sum b/go.sum index d385a7e..42e511f 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,3 @@ -github.com/tetratelabs/wazero v1.9.0 h1:IcZ56OuxrtaEz8UYNRHBrUa9bYeX9oVY93KspZZBf/I= -github.com/tetratelabs/wazero v1.9.0/go.mod h1:TSbcXCfFP0L2FGkRPxHphadXPjo1T6W+CseNNY7EkjM= github.com/tetratelabs/wazero v1.12.0 h1:DuWcpNu/FzgEXgGBDp8J1Spc+CWOvvtvVyjKlaZopYU= github.com/tetratelabs/wazero v1.12.0/go.mod h1:LvKtzl2RqO4gyF27BiXU+nKAjcV8f38U+kP/q2vgxh0= golang.org/x/sys v0.44.0 h1:ildZl3J4uzeKP07r2F++Op7E9B29JRUy+a27EibtBTQ= diff --git a/internal/catalog/decorator_test.go b/internal/catalog/decorator_test.go index 8a14d90..4b52994 100644 --- a/internal/catalog/decorator_test.go +++ b/internal/catalog/decorator_test.go @@ -65,3 +65,38 @@ func TestDecorator_aboveTheKeyIsRead(t *testing.T) { }) } } + +// A catalog written on Windows has CRLF, and the YAML parser files comments +// differently for it: the decorator directly above its key became the previous +// key's foot comment and decorated nothing. A blank line above the comment +// happened to hide it, which is why the report was "no space between the +// command and the comment breaks it". +func TestDecorator_crlfCatalogReadsTheDecorator(t *testing.T) { + doc := "version: \"0.1\"\nscripts:\n a: echo a\n # @dialect matcher\n ${x}: echo ${godo:argv[x]}\n" + dir := t.TempDir() + path := writeCat(t, dir, strings.ReplaceAll(doc, "\n", "\r\n")) + + cat, err := catalog.LoadFile(path) + if err != nil { + t.Fatalf("a CRLF catalog is a catalog: %v", err) + } + if cat.Scripts[1].Dialect != "matcher" { + t.Fatalf("dialect=%q, want matcher", cat.Scripts[1].Dialect) + } +} + +// The same normalization keeps \r out of a block scalar, where it would reach +// the shell as part of the command. +func TestDecorator_crlfDoesNotLeakIntoABody(t *testing.T) { + doc := "version: \"0.1\"\nscripts:\n a: |\n echo one\n echo two\n" + dir := t.TempDir() + path := writeCat(t, dir, strings.ReplaceAll(doc, "\n", "\r\n")) + + cat, err := catalog.LoadFile(path) + if err != nil { + t.Fatal(err) + } + if strings.Contains(cat.Scripts[0].Commands[0], "\r") { + t.Fatalf("body carries a carriage return: %q", cat.Scripts[0].Commands[0]) + } +} diff --git a/internal/catalog/load.go b/internal/catalog/load.go index 1e7c913..21cd8ea 100644 --- a/internal/catalog/load.go +++ b/internal/catalog/load.go @@ -1,6 +1,7 @@ package catalog import ( + "bytes" "fmt" "os" "path/filepath" @@ -28,7 +29,7 @@ func Parse(data []byte, path string, dialects ...*DialectRegistry) (*Catalog, er } var root yaml.Node - if err := yaml.Unmarshal(data, &root); err != nil { + if err := yaml.Unmarshal(normalizeBreaks(data), &root); err != nil { return nil, fmt.Errorf("%w: parse yaml: %v", ErrInvalidCatalog, err) } doc := &root @@ -113,6 +114,24 @@ func Parse(data []byte, path string, dialects ...*DialectRegistry) (*Catalog, er return cat, nil } +// normalizeBreaks turns CRLF into LF before the YAML parser sees it. +// +// A catalog written on Windows has CRLF, and the parser files a comment +// differently for it: a decorator directly above its key became the *previous* +// key's foot comment instead of that key's head comment, so it decorated +// nothing. A blank line above the comment happened to hide it, which is why it +// read as "no space between the command and the comment breaks it". +// +// YAML treats CRLF as a line break, so normalizing is what the format already +// says. It also keeps a \r out of block scalars, where it would otherwise be +// handed to the shell as part of the command. +func normalizeBreaks(data []byte) []byte { + if !bytes.Contains(data, []byte("\r\n")) { + return data + } + return bytes.ReplaceAll(data, []byte("\r\n"), []byte("\n")) +} + func scriptFromNodes(key, val *yaml.Node, fileDialect DialectName, reg *DialectRegistry, path string) (Script, error) { s := Script{Key: key.Value} if err := rejectMisplacedDecorators(key, val); err != nil { diff --git a/internal/plugin/invoke.go b/internal/plugin/invoke.go index c6e0b6c..87bfec9 100644 --- a/internal/plugin/invoke.go +++ b/internal/plugin/invoke.go @@ -320,8 +320,10 @@ func (p *Plugin) slinkOp(op Op, host Host) Result { return Result{Error: fmt.Sprintf("slink: dst: %v", err)} } - if err := os.Symlink(src, dst); err != nil { - return Result{Error: fmt.Sprintf("slink: %v%s", err, symlinkHint(err))} + // The mechanism is the platform's: a symlink on Unix, a junction or a hard + // link on Windows. See linkPath. + if err := linkPath(src, dst); err != nil { + return Result{Error: fmt.Sprintf("slink: %v", err)} } return Result{Code: 0, OK: true} } diff --git a/internal/plugin/reparse.go b/internal/plugin/reparse.go new file mode 100644 index 0000000..8c35ceb --- /dev/null +++ b/internal/plugin/reparse.go @@ -0,0 +1,68 @@ +package plugin + +import ( + "encoding/binary" + "fmt" + "unicode/utf16" +) + +// Sizes of REPARSE_DATA_BUFFER, which Windows defines as: +// +// ULONG ReparseTag; 4 +// USHORT ReparseDataLength; 2 +// USHORT Reserved; 2 +// USHORT SubstituteNameOffset; 2 <- the mount-point body starts here +// USHORT SubstituteNameLength; 2 +// USHORT PrintNameOffset; 2 +// USHORT PrintNameLength; 2 +// WCHAR PathBuffer[]; +const ( + reparseHeader = 8 + mountPointFields = 8 + reparseTagMountPt = 0xA0000003 // IO_REPARSE_TAG_MOUNT_POINT + maxReparseData = 16384 // MAXIMUM_REPARSE_DATA_BUFFER_SIZE +) + +// mountPointBuffer builds the reparse data that turns a directory into a +// junction pointing at target. +// +// It lives here, away from the syscall and without a build tag, because the +// layout is the part that is easy to get wrong and the part worth testing on +// whatever machine you happen to be on. The syscall around it is three lines. +// +// target must be an absolute path. PathBuffer holds both names, each +// NUL-terminated: the substitute name is the NT object path (\??\C:\x), which +// is what the filesystem follows, and the print name is the plain path, which +// is what Explorer shows. +func mountPointBuffer(target string) ([]byte, error) { + sub := utf16.Encode([]rune(`\??\` + target)) + print16 := utf16.Encode([]rune(target)) + + path := make([]byte, 0, (len(sub)+len(print16)+2)*2) + path = appendUTF16(path, sub) + printOffset := len(path) + path = appendUTF16(path, print16) + + if reparseHeader+mountPointFields+len(path) > maxReparseData { + return nil, fmt.Errorf("junction target is too long: %s", target) + } + + buf := make([]byte, 0, reparseHeader+mountPointFields+len(path)) + buf = binary.LittleEndian.AppendUint32(buf, reparseTagMountPt) + buf = binary.LittleEndian.AppendUint16(buf, uint16(mountPointFields+len(path))) + buf = binary.LittleEndian.AppendUint16(buf, 0) // Reserved + buf = binary.LittleEndian.AppendUint16(buf, 0) // SubstituteNameOffset + buf = binary.LittleEndian.AppendUint16(buf, uint16(len(sub)*2)) + buf = binary.LittleEndian.AppendUint16(buf, uint16(printOffset)) + buf = binary.LittleEndian.AppendUint16(buf, uint16(len(print16)*2)) + return append(buf, path...), nil +} + +// appendUTF16 writes code units little-endian and terminates them with a NUL, +// which the lengths above exclude and the offsets count. +func appendUTF16(dst []byte, s []uint16) []byte { + for _, u := range s { + dst = binary.LittleEndian.AppendUint16(dst, u) + } + return binary.LittleEndian.AppendUint16(dst, 0) +} diff --git a/internal/plugin/reparse_test.go b/internal/plugin/reparse_test.go new file mode 100644 index 0000000..6502d6c --- /dev/null +++ b/internal/plugin/reparse_test.go @@ -0,0 +1,76 @@ +package plugin + +import ( + "encoding/binary" + "strings" + "testing" + "unicode/utf16" +) + +// The layout is what Windows reads byte by byte, so it is checked field by +// field. Getting an offset wrong produces a directory that looks like a +// junction and points nowhere. +func TestMountPointBuffer(t *testing.T) { + const target = `C:\Users\me\src\repo` + buf, err := mountPointBuffer(target) + if err != nil { + t.Fatal(err) + } + + tag := binary.LittleEndian.Uint32(buf[0:4]) + if tag != reparseTagMountPt { + t.Fatalf("tag=%#x, want IO_REPARSE_TAG_MOUNT_POINT", tag) + } + dataLen := binary.LittleEndian.Uint16(buf[4:6]) + if int(dataLen) != len(buf)-reparseHeader { + t.Fatalf("ReparseDataLength=%d, want %d (everything after the header)", dataLen, len(buf)-reparseHeader) + } + if r := binary.LittleEndian.Uint16(buf[6:8]); r != 0 { + t.Fatalf("Reserved=%d, want 0", r) + } + + subOff := binary.LittleEndian.Uint16(buf[8:10]) + subLen := binary.LittleEndian.Uint16(buf[10:12]) + printOff := binary.LittleEndian.Uint16(buf[12:14]) + printLen := binary.LittleEndian.Uint16(buf[14:16]) + path := buf[reparseHeader+mountPointFields:] + + if subOff != 0 { + t.Fatalf("SubstituteNameOffset=%d, want 0", subOff) + } + // The substitute name is the NT object path: that prefix is what makes the + // junction resolve at all. + if got := decodeUTF16(path[subOff : subOff+subLen]); got != `\??\`+target { + t.Fatalf("substitute name=%q", got) + } + if got := decodeUTF16(path[printOff : printOff+printLen]); got != target { + t.Fatalf("print name=%q", got) + } + // Each name is NUL-terminated, and the lengths exclude the terminator — + // which is why the print name starts two bytes past the end of the first. + if int(printOff) != int(subLen)+2 { + t.Fatalf("PrintNameOffset=%d, want %d (past the substitute name and its NUL)", printOff, subLen+2) + } + if u := binary.LittleEndian.Uint16(path[subLen : subLen+2]); u != 0 { + t.Fatalf("substitute name is not NUL-terminated") + } + if u := binary.LittleEndian.Uint16(path[printOff+printLen:][:2]); u != 0 { + t.Fatalf("print name is not NUL-terminated") + } +} + +// Windows caps reparse data at 16 KiB, and a refusal here is better than one +// from DeviceIoControl after the directory has already been created. +func TestMountPointBufferRefusesAnEnormousTarget(t *testing.T) { + if _, err := mountPointBuffer(`C:\` + strings.Repeat("x", maxReparseData)); err == nil { + t.Fatal("want an error") + } +} + +func decodeUTF16(b []byte) string { + u := make([]uint16, len(b)/2) + for i := range u { + u[i] = binary.LittleEndian.Uint16(b[i*2:]) + } + return string(utf16.Decode(u)) +} diff --git a/internal/plugin/symlink_other.go b/internal/plugin/symlink_other.go index cd85749..85556ee 100644 --- a/internal/plugin/symlink_other.go +++ b/internal/plugin/symlink_other.go @@ -2,6 +2,10 @@ package plugin -// symlinkHint has nothing to add off Windows: creating a symlink there is an -// ordinary file operation, and the OS error already says what went wrong. -func symlinkHint(error) string { return "" } +import "os" + +// linkPath makes dst point at src. +// +// A symlink is an ordinary file operation here, needs no privilege, and may +// dangle. Windows has none of those three, which is why it has its own. +func linkPath(src, dst string) error { return os.Symlink(src, dst) } diff --git a/internal/plugin/symlink_windows.go b/internal/plugin/symlink_windows.go index 794e41c..3177746 100644 --- a/internal/plugin/symlink_windows.go +++ b/internal/plugin/symlink_windows.go @@ -3,26 +3,89 @@ package plugin import ( - "errors" - "syscall" + "fmt" + "os" + "path/filepath" + + "golang.org/x/sys/windows" ) -// errPrivilegeNotHeld is ERROR_PRIVILEGE_NOT_HELD, what Windows answers when a -// process without SeCreateSymbolicLinkPrivilege tries to create a symlink. +// linkPath makes dst point at src using the link Windows actually offers. +// +// A symlink on Windows is a privilege, not a file operation: without Developer +// Mode or an elevated terminal the OS refuses one, so a plugin that links +// something worked on Unix and failed here. Windows has two links that need no +// privilege, and between them they cover what a symlink is used for: +// +// a directory -> a junction (a mount-point reparse point) +// a file -> a hard link // -// Spelled out rather than imported: the stdlib syscall package does not name -// this one, and Windows error numbers are a stable ABI. -const errPrivilegeNotHeld = syscall.Errno(1314) +// So that is what this uses. The mechanism is the platform's; the op is the +// same op. +// +// src must exist, because which link to make depends on what it is. That is +// the one place this cannot match Unix, where a link may dangle. +func linkPath(src, dst string) error { + info, err := os.Stat(src) + if err != nil { + if os.IsNotExist(err) { + return fmt.Errorf("%s does not exist; on Windows the link is a junction for a directory "+ + "and a hard link for a file, so the target has to be there to say which", src) + } + return err + } + if info.IsDir() { + return createJunction(src, dst) + } + if err := os.Link(src, dst); err != nil { + return fmt.Errorf("%w (a hard link is how a file is linked on Windows; "+ + "it cannot cross volumes, so src and dst must be on the same drive)", err) + } + return nil +} -// symlinkHint turns an OS refusal into something the reader can act on. +// createJunction makes dst a junction pointing at the directory src. // -// On Windows creating a symlink is a privilege, not a file operation, so an -// ordinary user gets "A required privilege is not held by the client" — true, -// and useless unless you already know that Developer Mode is what grants it. -func symlinkHint(err error) string { - if !errors.Is(err, errPrivilegeNotHeld) { - return "" - } - return " (Windows grants this to an ordinary user only with Developer Mode on: " + - "Settings > System > For developers > Developer Mode. Otherwise run as Administrator)" +// A junction is a directory carrying a mount-point reparse point, so the +// directory is created first and the reparse point written into it. If writing +// it fails the empty directory is removed, because a bare directory where a +// link was asked for is worse than nothing. +func createJunction(src, dst string) error { + target, err := filepath.Abs(src) + if err != nil { + return err + } + if err := os.Mkdir(dst, 0o755); err != nil { + return err + } + if err := writeMountPoint(dst, target); err != nil { + os.Remove(dst) + return err + } + return nil +} + +// writeMountPoint sets the mount-point reparse point on an existing directory. +func writeMountPoint(dir, target string) error { + buf, err := mountPointBuffer(target) + if err != nil { + return err + } + name, err := windows.UTF16PtrFromString(dir) + if err != nil { + return err + } + // BACKUP_SEMANTICS is what lets a directory be opened at all; + // OPEN_REPARSE_POINT opens the directory itself rather than following a + // reparse point that is already on it. + h, err := windows.CreateFile(name, windows.GENERIC_WRITE, 0, nil, windows.OPEN_EXISTING, + windows.FILE_FLAG_BACKUP_SEMANTICS|windows.FILE_FLAG_OPEN_REPARSE_POINT, 0) + if err != nil { + return err + } + defer windows.CloseHandle(h) + + var returned uint32 + return windows.DeviceIoControl(h, windows.FSCTL_SET_REPARSE_POINT, + &buf[0], uint32(len(buf)), nil, 0, &returned, nil) } From 188654884521cb24850997270e3afbeabcaf9c33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=83=9E=E3=82=A4=E3=83=8E=E3=83=AB?= <97069334+MY-RV@users.noreply.github.com> Date: Mon, 21 Sep 2026 13:25:07 -0600 Subject: [PATCH 3/3] chore: release v0.3.0-preview.2 Everything in it came from running preview.1 on a real Windows host. The junction syscall behind slink is the reason this is preview.2 and not 0.3.0: its buffer is tested, the call itself has been run by nobody. --- CHANGELOG.md | 17 ++++++++++++++--- docs/install.md | 4 ++-- docs/roadmap.md | 8 +++++--- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 22e4627..fb6047f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,20 @@ ## [Unreleased] -Found by running `v0.3.0-preview.1` on a real Windows host, which is what the -preview was for. The roadmap said Windows shell detection was written from -documentation and unverified; it was also wrong. +## [0.3.0-preview.2] — 2026-09-21 + +**A preview**, on the same terms as the one before it: a GitHub pre-release, so +`godo -e update` does not offer it and neither Homebrew nor Scoop carries it. + +All of it was found by running `v0.3.0-preview.1` on a real Windows host, which +is what the preview was for. The roadmap said Windows shell detection was +written from documentation and unverified; it was also wrong, and so were two +other things. + +Still unverified, and the reason this is preview.2 rather than 0.3.0: the +junction syscall runs only on Windows. Its reparse buffer is built in portable +code and tested field by field, but `DeviceIoControl` itself has been executed +by nobody. ### Fixed - **Every PowerShell user on Windows was told they were in `cmd`.** Detection diff --git a/docs/install.md b/docs/install.md index 9b91bad..48e0ad2 100644 --- a/docs/install.md +++ b/docs/install.md @@ -41,10 +41,10 @@ A preview is published as a GitHub **pre-release**. `godo -e update` reads the arrives on its own — you go and get it: ```bash -go install github.com/my-rv/godo/cmd/godo@v0.3.0-preview.1 +go install github.com/my-rv/godo/cmd/godo@v0.3.0-preview.2 ``` -or download the `v0.3.0-preview.1` assets from +or download the `v0.3.0-preview.2` assets from [Releases](https://github.com/MY-RV/godo/releases). On a preview binary `godo -e update check` reports no update available: the diff --git a/docs/roadmap.md b/docs/roadmap.md index 923bb45..13dd41a 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -16,7 +16,7 @@ This is what we **commit to communicate**. Pre-1.0 APIs can still change within **Not promised in v0.1:** Homebrew/Scoop installs, dialects `nscript`/`matchns`, stable Go API. -## v0.3 — previewed as `v0.3.0-preview.1` +## v0.3 — previewed as `v0.3.0-preview.2` Breaking: the shell that runs your scripts changes, and file fields move into `engine:`. It was held until plugin loading worked — `engine:` without a loader @@ -25,7 +25,7 @@ is half a promise, and this is the release where the promise gets made. A preview is a GitHub pre-release: `godo -e update` does not offer it, and neither Homebrew nor Scoop carries it. It is on [Releases](https://github.com/MY-RV/godo/releases) and on -`go install github.com/my-rv/godo/cmd/godo@v0.3.0-preview.1`. +`go install github.com/my-rv/godo/cmd/godo@v0.3.0-preview.2`. | Promise | | |---------|--| @@ -46,7 +46,9 @@ Everything in the table above. ### Before v0.3.0 ships - Preview feedback. Plugin loading is new, and the preview is where it gets - found out. + found out. preview.1 on Windows already produced three fixes. +- **Windows verification.** preview.2 fixes shell detection, CRLF catalogs and + `fs.slink`; the junction syscall behind `slink` has still been run by nobody. - The plugin protocol is **not** frozen by this preview. A plugin is pinned by digest, so a protocol change cannot silently break a catalog — it fails by naming the plugin.