diff --git a/CHANGELOG.md b/CHANGELOG.md index 333b08a..942771a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,12 +2,73 @@ ## [Unreleased] +Breaking. The shell that runs your scripts changed. + +### Changed +- **`dialect` moved to `engine.dialect`.** A top-level `dialect:` keeps + working — it shipped in 0.1 and 0.2 — and `engine.dialect` wins if both are + present. `runner` only ever existed as `engine.runner`. +- **The default runner is the shell you are already in**, not `sh -c` / + `cmd /C`. A zsh user gets zsh, a PowerShell user gets PowerShell. The shell + someone uses is their own business; godo proxying to a different one was a + choice that was not godo's to make. + *Consequence, and it is deliberate:* a catalog is read by the shell of + whoever runs it, so zsh syntax behaves differently for a teammate on bash. + godo is a proxy and promises neither cross-OS nor cross-shell. To pin one + shell for everyone, name it (`runner: sh`, `runner: bash`). + *Library:* an unset runner means the `Runner` injected into the engine, so + `NewEngine(cat, myRunner)` is unaffected. + +### Added +- **Runner axis.** `{file}.runner` and `# @runner` say *how* a script's body + becomes a process, the way `dialect` says *which* script answers the tokens. + It exists so a plugin's body — which is not a shell line at all — has a way + to say so; see the [design note](./docs/dev/runners-and-plugins.md). +- **`inherit`** (the default): the shell you are in. `GODO_SHELL` overrides + detection; otherwise `$SHELL` on Unix, and on Windows the parent process when + it is a shell, else `%ComSpec%`. Windows reads the parent 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. +- **A shell by name:** `# @runner sh` / `bash` / `zsh` / `dash` / `ksh` / `ash` + / `fish` / `nu` / `cmd` / `pwsh` / `powershell`. godo does not manage these — + it resolves the name on `PATH` and hands the line over. The name is logical, + never a path: `cmd`, not `cmd.exe`; `pwsh`, not `ps1`. A name outside the + list is refused rather than run, so `# @runner git` cannot quietly become + `git -c `. +- **`engine:` block** — every dial godo turns while reading and running a + catalog, kept apart from `scripts:`, which is the data. Holds `version` + (minimum binary, enforced before anything runs), `dialect`, `runner`, and + `plugins`. `engine.plugins` takes `source`, a **required** `sha256`, + `provides: [runner:name]`, and an optional `config` that is entirely the + plugin's — godo carries it without reading it. Nothing loads plugins yet; + they are parsed and validated so the shape is settled, and a script asking + for a runner a plugin provides fails by naming that plugin instead of reading + as a typo. +- **`godo -e runners`** lists what is usable on the machine you are on, and how + to confirm which shell you are in when the detected one looks wrong. +- `--ls ` prints `@runner` beside `@dialect`. +- `ArgsAwareRunner`: whether a script takes the tokens left over after the + match is the runner's question. A shell keeps the `${godo:args…}` rule, + message included; a runner whose body is a program answers for itself. +- Public `RunnerName`, `RunnerInherit`, `RunnerRegistry`, `NewRunnerRegistry`, + `DefaultRunners`, `EffectiveRunner`, `WithRunners`, `ErrUnknownRunner`. + ### Fixed - A value shaped like `--flag=…` is quoted from the `=` onward, so `--preview` shows `git commit --am='two words'` instead of `git commit '--am=two words'`, which read as though the flag name were part of the message. Identical single argument to the shell — rendering only. +### Notes +- An unknown runner fails when the plan is built (`--preview` included), not at + load: a dialect must resolve before a script can be matched at all, a runner + only to execute. Nothing executes either way. +- A shell is started non-interactively and without a profile, so you get your + shell's grammar, not your shell's setup — aliases and functions are not there. +- The Windows detection cross-compiles and vets but is unverified on a real + Windows host. + ## [0.2.0] — 2026-09-19 Breaking. Placeholder syntax inside script bodies changed; catalogs need editing. @@ -44,6 +105,12 @@ Full rules in [docs/contract.md](./docs/contract.md). body. Both existed only to rescue host environment variables from being read as captures. +### Fixed +- A value shaped like `--flag=…` is quoted from the `=` onward, so `--preview` + shows `git commit --am='two words'` instead of `git commit '--am=two words'`, + which read as though the flag name were part of the message. Identical single + argument to the shell — rendering only. + ### Notes - The Windows quoting path is **untested**: CI runs Linux only, and the runner tests skip on Windows. `cmd.exe` also expands `%VAR%` before a command sees @@ -62,6 +129,12 @@ Full rules in [docs/contract.md](./docs/contract.md). - Product docs (overview, getting started, guides, reference, distribution, roadmap). - Origin story (EN/ES), SECURITY.md, CONTRIBUTING, GitHub issue/PR templates, CODEOWNERS. +### Fixed +- A value shaped like `--flag=…` is quoted from the `=` onward, so `--preview` + shows `git commit --am='two words'` instead of `git commit '--am=two words'`, + which read as though the flag name were part of the message. Identical single + argument to the shell — rendering only. + ### Notes - Product display name: **GoDo**; identifiers remain lowercase `godo`. - Pre-1.0: APIs and CLI may still change. Treat `v0.x` as evolving. diff --git a/docs/README.md b/docs/README.md index 6e67a58..795aded 100644 --- a/docs/README.md +++ b/docs/README.md @@ -25,6 +25,7 @@ One topic per file (copy-paste examples): | [Preview and ls](./guide/preview-and-ls.md) | `--preview`, `--ls` | | [Package dialect](./guide/package-dialect.md) | Exact names (default) | | [Matcher dialect](./guide/matcher-dialect.md) | Pattern keys + captures | +| [Runners](./guide/runners.md) | Which shell runs your line; `runner:` / `# @runner` | | [Deps](./guide/deps.md) | `# @deps` | | [Placeholders](./guide/placeholders.md) | `${…}` / `${godo:args…}` | | [Working directory](./guide/working-directory.md) | Catalog root, walk-up | diff --git a/docs/contract.md b/docs/contract.md index 9becfee..4d9ea65 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -56,16 +56,177 @@ version: "0.1" Contract version of the file (not the `godo` binary). +## Engine + +File field: `{file}.engine`. Optional. + +```yaml +engine: + version: ">=0.3.0" + dialect: matcher + runner: bash + plugins: + - source: https://github.com/my-rv/godo-micropy@v1.2.0 + sha256: "661471…" + provides: [runner:micropy] + config: + proc: {exec: true, spawn: false} + fs: {slink: true} +``` + +`scripts:` is the catalog — the data. `engine:` is every dial godo turns while +reading and running it: the binary it expects, the dialect, the runner, the +plugins. Keeping them apart is what lets the toolchain side grow without the +script side growing with it. + +| Field | | +|-------|--| +| `version` | Minimum `godo` binary | +| `dialect` | How keys match tokens. Default `package` | +| `runner` | How a body becomes a process. Default: the shell you are in | +| `plugins` | Declared plugins (nothing loads them yet) | + +### `engine.version` + +The minimum `godo` binary, as `"0.3.0"` or `">=0.3.0"`. Only a minimum — no +ranges, no `^`, no `~`. A binary below it refuses the catalog before running +anything: + +``` +godo: ./godo.yaml needs godo 0.3.0 or newer; this is 0.2.0 (godo -e update) +``` + +Comparison drops any pre-release suffix, so a `-dev` build is judged by its +numbers. + +### `engine.plugins` + +**No build loads plugins yet.** Entries are parsed and validated so the shape +is settled and a catalog can already declare what it expects. + +| Field | | +|-------|--| +| `source` | Required. Where the plugin comes from | +| `sha256` | **Required.** A plugin is third-party code that runs when someone types `godo test`; without a digest there is nothing to verify it is the code that was reviewed | +| `provides` | Required. `":"` entries, kind being `runner` or `dialect`. Two plugins may not provide the same one | +| `config` | Optional, and entirely the plugin's: its keys, its meaning, its defaults. godo carries it across without reading it | + +A script asking for a runner a plugin provides fails by naming that plugin: + +``` +godo: unknown runner: script "wt" asks for runner "micropy", provided by +plugin https://github.com/my-rv/godo-micropy@v1.2.0 — this build cannot load +plugins +``` + +`godo -e runners` lists what a catalog declares, beside what the machine has. + ## Dialect -File field: `{file}.dialect`. Optional; default `package`. +File field: `{file}.engine.dialect`. Optional; default `package`. ```yaml -dialect: package # omit → package +engine: + dialect: package # omit → package ``` File default. Scripts may override with `# @dialect`. +A top-level `dialect:` is still read — it shipped in 0.1 and 0.2 — and +`engine.dialect` wins if both are present. + +## Runner + +File field: `{file}.engine.runner`. Optional; default `inherit`. + +```yaml +engine: + runner: inherit # omit → inherit +``` + +File default. Scripts may override with `# @runner`. + +`dialect` answers *which script responds to these tokens*; `runner` answers +*how the resolved body becomes a process*. They are independent: any dialect +may be paired with any runner. + +| Runner | | +|--------|--| +| `inherit` | The shell you are already in. Default | +| *a shell name* | `sh`, `bash`, `zsh`, `dash`, `ksh`, `ash`, `fish`, `nu`, `cmd`, `pwsh`, `powershell` | + +`godo -e runners` lists what is usable on the machine you are on. + +godo does not manage these shells — it resolves the name on `PATH` and hands +the line over. The line is yours and the shell is yours; godo is the proxy. + +### `inherit` + +The default. Your shell runs your line: + +```yaml + arr: "arr=(a b c); echo ${arr[1]}" +``` + +``` +zsh → a (zsh indexes arrays from 1) +sh → b (sh indexes arrays from 0) +``` + +Selection, in order: + +| | | +|--|--| +| `GODO_SHELL` | Always wins | +| Unix | `$SHELL`, else `/bin/sh` | +| Windows | the parent process when it is a shell, 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 +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: +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 +zsh still reports zsh. `GODO_SHELL` is how you disagree with either. + +The shell is started non-interactively and without a profile (`-c`, `/C`, or +`-NoProfile -Command`), so this gives you your shell's **grammar**, not your +shell's **setup** — your aliases and functions are not there. + +**Consequence:** a catalog is read by the shell of whoever runs it, so a script +written in zsh syntax behaves differently for a teammate on bash. That is +deliberate — godo is a proxy and promises neither cross-OS nor cross-shell. + +### A shell by name + +To pin one shell for everyone, name it: + +```yaml + # @runner bash + ci: shopt -s globstar && echo **/*.go +``` + +The name is **logical, never a path**: write `cmd`, not `cmd.exe`; `pwsh`, not +`pwsh.exe` or `ps1` (`.ps1` is a script extension, not the program). The +platform's extension is `PATH`'s business, so the same `godo.yaml` reads the +same everywhere. + +A name outside the list above is refused rather than run — otherwise +`# @runner git` would quietly become `git -c `. For a shell not on the +list, set `GODO_SHELL` and use `inherit`. + +A shell that is not installed here fails when the plan is built, so nothing +executes. + +Whether a script accepts the tokens left over after the match is also the +runner's question. A shell accepts them only when the body references +`${godo:args…}`; a runner whose body is a program answers for itself. + ## Bind Godo expands placeholders **in-process** before `exec` / preview, in two spaces: @@ -91,13 +252,17 @@ no escape syntax. `${godo:argv[i]}` with a number is an error pointing at `${godo:args[i]}`. `${godo:…}` inside a matcher key or a `@deps` entry is rejected. -Expanded values are **shell-quoted** for the host shell (`sh` / `cmd`): one -argument in is one argument out. `:raw` opts a single placeholder out. +Expanded values are **shell-quoted** (`sh` rules on POSIX, `cmd` rules on +Windows): one argument in is one argument out. `:raw` opts a single placeholder +out. A value shaped like `--flag=…` is quoted from the `=` onward, so a preview reads `--am='two words'` rather than `'--am=two words'`. Same single argument to the shell; the flag name is not part of the value. +Those rules also hold for bash, zsh, dash, ksh and fish. PowerShell quotes +differently, so a value containing a backtick or `$` may not survive there. + Windows caveat: `cmd.exe` expands `%VAR%` and `!VAR!` before a command sees its arguments, and no quoting on the command line fully suppresses that. @@ -118,6 +283,7 @@ YAML comment block **immediately above** the script key. Apply to scripts only. |------|--| | `# text` (no `@`) | Doc | | `# @dialect ` | Match this script with another dialect (override of `{file}.dialect`) | +| `# @runner ` | Run this script with another runner (override of `{file}.runner`) | | `# @deps a, b` | Run `a`, then `b`, then the value | | `# @dependencies a, b` | Alias of `@deps` | @@ -134,10 +300,12 @@ test ${MODULE}: go test ./${godo:argv[MODULE]}/... ``` - `@dialect` — per-script override; without it, uses `{file}.dialect` +- `@runner` — per-script override; without it, uses `{file}.runner` - `@deps` / `@dependencies` — invocation like `godo …` (space-separated tokens; entries separated by `,`) - literals and bare `${NAME}` from captures **already bound** by the match (godo space) - no `${godo:…}` in `@deps`; entries resolve to tokens, so a capture holding a space stays one token - order = list order; stop on first failure +- each step keeps **its own** runner: a dep declaring `@runner` runs under that runner, not the caller's - cycle → error (on the **expanded** invocation) - caller args are **not** forwarded to deps - diamond (A→B,C and B→C): C runs **once**, at its first (deepest-first) position @@ -155,7 +323,8 @@ scripts.: string | string[] ```yaml version: "0.1" -dialect: package +engine: + dialect: package scripts: # Unit tests @@ -178,7 +347,7 @@ scripts: - `string` — one command - `string[]` — in order; stop on first failure - `${godo:args}` / `${godo:args[i]}` / `${godo:args[i..j]}` — optional in the value -- without `${godo:args}` → extra tokens error +- without `${godo:args}` → extra tokens error (see **Runner**) - no captures in keys (that is `matcher`) ## Dialect `matcher` @@ -192,7 +361,8 @@ scripts.: string | string[] ```yaml version: "0.1" -dialect: matcher +engine: + dialect: matcher scripts: "${GRP} ${SCR}": go run -C scripts/${GRP}/${SCR} . ${godo:args} @@ -222,7 +392,7 @@ Reserved: cannot be used as `{file}.dialect` / `@dialect` until implemented and ## Common semantics 1. Resolve `godo.yaml` from cwd upward through parents. -2. Read `{file}.version` and `{file}.dialect` (omit dialect → `package`; dialect must be implemented). -3. Match → bind captures → expand `@deps` → run deps → expand value → execute (or `--preview` / `--ls`). +2. Read `{file}.version` and `{file}.engine` (omit `engine.dialect` → `package`, must be implemented; omit `engine.runner` → `inherit`). +3. Match → bind captures → expand `@deps` → run deps → expand value → execute with the step's runner (or `--preview` / `--ls`). 4. Exit code = of the command (or the first failure in a list / deps); catalog/match errors → `1`. 5. Exec cwd = directory of the `godo.yaml` (see **Exec cwd**). diff --git a/docs/dev/README.md b/docs/dev/README.md index 982786c..f30e276 100644 --- a/docs/dev/README.md +++ b/docs/dev/README.md @@ -7,6 +7,7 @@ Internals and contributor conventions. Product docs live in [`docs/`](../README. | [Architecture](./architecture.md) | Facade / internal / CLI | | [Standards](./standards.md) | Coding bar | | [Versioning](./versioning.md) | SemVer + binary vs file `version:` | +| [Runners and plugins](./runners-and-plugins.md) | Premise: godo is a proxy; cross-OS is a plugin's job | | [Archive](./archive/README.md) | Brainstorm notes (historical) | Start with root [`CONTRIBUTING.md`](../../CONTRIBUTING.md). diff --git a/docs/dev/runners-and-plugins.md b/docs/dev/runners-and-plugins.md new file mode 100644 index 0000000..e7f4661 --- /dev/null +++ b/docs/dev/runners-and-plugins.md @@ -0,0 +1,93 @@ +# Runners and plugins — design note + +Not the contract. [`contract.md`](../contract.md) is what we promise; nothing +here is in [`roadmap.md`](../roadmap.md). + +## Premise + +**godo is a proxy.** Same idea as `package.json` scripts: it names the chores +and hands the line to the shell. It does **not** promise to solve cross-OS. If +you are on Linux you write Linux (`&&`); on Windows you write Windows. Your +shell is your responsibility, and a catalog author who wants portability picks +portable commands. + +The first layer is that proxy plus flavored water: dialects, `@deps`, +`--preview`, `--ls`. Native, harmless, no ambition beyond naming things well. + +**Cross-OS is a plugin's job**, not the core's. A plugin like `micropy` — +MicroPython compiled to WASM — carries a body that runs the same everywhere +because it is not a shell line at all. + +## What follows from that + +Two axes, and only one of them was open: + +| Axis | Question | | +|------|----------|--| +| Dialect | which script answers these tokens? | already open | +| Runner | how does the body become a process? | hardcoded to `sh -c` / `cmd /C` | + +A MicroPython body has no shell that can run it. For `micropy` to exist at all, +godo needs a way for a script to say *this one is not a shell line*. That is +the whole reason the runner axis exists — `runner:` / `# @runner` and a registry, +with `shell` as the default. + +It is a socket, not a feature. What was added alongside it is not portability +work either: `shell` hardcoded `sh` / `cmd`, so a zsh or PowerShell user ran +their catalog under a shell they did not pick. `inherit` — now the default — uses +the one they are in, and a catalog can name a shell outright (`# @runner +bash`). Your shell is your responsibility; godo just stops lying about which +one it is, and proxies to whatever `PATH` has. + +A `shell` runner meaning "always `sh` / `cmd`" existed briefly and was removed: +it was a third thing between "your shell" and "this shell", it kept +`internal/execshell` carrying two runners, and almost nobody writes a line that +means the same in `sh` and in `cmd`. `# @runner sh` says it better. + +## Deliberately not done + +Things an earlier draft of this note argued for, dropped because they +contradict the premise: + +- **A built-in `exec` runner** (argv, no shell). Its only real gain was a + Windows `%VAR%` edge case; the quoting added in v0.2.0 already covers the + rest. And it moves the body from the shell's ownership to godo's, which is + how it ended up needing its own quote parser — a mini-shell inside godo, with + rules that are neither `sh`'s nor PowerShell's. Wrong direction. +- **A built-in POSIX interpreter** (`mvdan.cc/sh`). Same reason, plus a + dependency, and it is cross-OS work that belongs in a plugin. +- **A structured `Line` / `Part` expansion.** Only existed to serve `exec`. + +The reserved dialect names `nscript` / `matchns` stay reserved and unplanned. + +## Where a plugin is declared + +`engine:` — kept apart from the script fields on purpose. `dialect:` and +`runner:` say how to read and run the scripts; `engine:` says what godo itself +needs to do it. That line is what lets the toolchain side grow — a lockfile, a +package manager — without the script side growing with it. + +The block parses and validates today; nothing loads from it. `sha256` is +required from the start rather than added later, because a plugin is +third-party code that runs on `godo test` and a digest is the only thing that +says it is the code that was reviewed. + +## What is actually left for plugins + +None of this is built: + +1. Load a `.wasm` at runtime (`wazero`: pure Go, no cgo, one artifact for every + platform, sandboxed by default). +2. Ship MicroPython as that `.wasm`, with the host API from the prototype + (`godo.argv`, `godo.args`, `godo.proc`, `godo.fs`). +3. Fetch and pin it: `sha256` required, no auto-update, capabilities granted + deny-all through the plugin's own `config:`. + +Two constraints worth keeping when that work starts: + +- **Every effect goes through host functions.** Not because sandboxing is + fashionable, but because it is the only way `--preview` keeps working on a + body that is a program: a recording host can show what the script would run + without running it. +- **The plugin cannot police itself.** It declares which host function each + `config:` key unlocks; godo does the gating. diff --git a/docs/getting-started.md b/docs/getting-started.md index 799df6b..dda2674 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -31,7 +31,7 @@ scripts: check: go build -o app . ``` -`version` is required. `dialect` is optional and defaults to `package`. Script docs and `@deps` / `@dialect` are comments immediately above each key — start with [guide/scripts.md](./guide/scripts.md). +`version` is required. `engine.dialect` is optional and defaults to `package`. Script docs and `@deps` / `@dialect` are comments immediately above each key — start with [guide/scripts.md](./guide/scripts.md). ## 3. Run diff --git a/docs/guide/matcher-dialect.md b/docs/guide/matcher-dialect.md index f560803..312bf72 100644 --- a/docs/guide/matcher-dialect.md +++ b/docs/guide/matcher-dialect.md @@ -6,7 +6,8 @@ Keys are token patterns. First matching key (definition order) wins. ```yaml version: "0.1" -dialect: matcher +engine: + dialect: matcher scripts: test ${MODULE}: go test ./${godo:argv[MODULE]}/... diff --git a/docs/guide/placeholders.md b/docs/guide/placeholders.md index bafcdfb..ea99bb1 100644 --- a/docs/guide/placeholders.md +++ b/docs/guide/placeholders.md @@ -13,7 +13,8 @@ Because a body is shell text, godo claims **only** the `${godo:…}` namespace t and leaves every other `${…}` alone. A collision is not resolved — it cannot occur. ```yaml -dialect: matcher +engine: + dialect: matcher scripts: # @deps lint ${MODULE} # godo space @@ -27,7 +28,8 @@ scripts: Consumes a capture bound by the matcher key. ```yaml -dialect: matcher +engine: + dialect: matcher scripts: build ${name}: echo building ${godo:argv[name]} -- ${godo:args} diff --git a/docs/guide/runners.md b/docs/guide/runners.md new file mode 100644 index 0000000..3056711 --- /dev/null +++ b/docs/guide/runners.md @@ -0,0 +1,132 @@ +# Runners + +A **dialect** decides *which* script answers your tokens. A **runner** decides +*how* that script's body becomes a process. They are independent — any dialect +pairs with any runner. + +```yaml +version: "0.1" +engine: + runner: inherit # omit → inherit + +scripts: + test: go test ./... + + # @runner bash + ci: shopt -s globstar && echo **/*.go +``` + +| Runner | | +|--------|--| +| `inherit` | The shell you are already in. **Default** | +| a shell name | `sh`, `bash`, `zsh`, `dash`, `ksh`, `ash`, `fish`, `nu`, `cmd`, `pwsh`, `powershell` | + +godo does not manage these shells. It resolves the name on `PATH` and hands the +line over — the line is yours, the shell is yours, godo is the proxy. + +## `inherit` — your shell + +The default. Whatever shell you are in runs the line: + +```yaml +scripts: + arr: "arr=(a b c); echo ${arr[1]}" +``` + +``` +$ godo arr # from zsh +a +$ godo arr # from bash +b +``` + +Not a bug: zsh indexes arrays from 1, bash from 0. Same line, two shells, two +answers — and that is the point. Before this, godo always used `sh`, so a zsh +user got `b` without being told why. + +**Consequence, and it is deliberate:** a catalog is read by the shell of +whoever runs it. A script written in zsh syntax behaves differently for a +teammate on bash. godo is a proxy and promises neither cross-OS nor +cross-shell. If that matters for a script, name a shell. + +## Naming a shell + +Pins it for everyone, whatever shell they are in: + +```yaml +scripts: + # @runner bash + ci: shopt -s globstar && echo **/*.go + + # @runner pwsh + sign: Get-AuthenticodeSignature .\dist\godo.exe +``` + +The name is **logical, never a path**. Write `cmd`, not `cmd.exe`; `pwsh`, not +`pwsh.exe` or `ps1` — `.ps1` is a script extension, the program is `pwsh`. The +platform's extension is `PATH`'s business, so the same `godo.yaml` reads the +same on every machine. + +A name outside the list is refused rather than run: + +``` +$ godo deploy +godo: script "deploy" asks for runner "git": unknown runner: "git" is not a +known shell (sh, bash, zsh, …); for another one set GODO_SHELL +``` + +Otherwise `# @runner git` would quietly become `git -c `. + +A shell that is not installed fails when the plan is built, so nothing runs. + +## What godo picked, and how to disagree + +``` +$ godo -e runners +inherit /bin/zsh [default] + +shells found here: + sh /bin/sh + bash /bin/bash + zsh /bin/zsh + +Not the shell you expected? Run this in your terminal: + echo $0 (sh, bash, zsh, dash, ksh) + echo $version (fish) + +Then: GODO_SHELL=/path/to/shell +``` + +Selection order: + +| | | +|--|--| +| `GODO_SHELL` | Always wins | +| Unix | `$SHELL`, else `/bin/sh` | +| Windows | the parent process when it is a shell, else `%ComSpec%` | + +Windows reads the parent process 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 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. + +## What you do not get + +Your shell's **grammar**, not your shell's **setup**. The shell starts +non-interactively and without a profile (`-c`, `/C`, `-NoProfile -Command`), so +your aliases and functions are not there — exactly as `sh -c` always behaved. + +```yaml +scripts: + # your alias "gs" does not exist here + status: git status +``` + +## Plugins + +`# @runner` is also how a body that is **not a shell line** says so — a plugin +carrying its own interpreter. Nothing ships yet; see +[runners and plugins](../dev/runners-and-plugins.md). diff --git a/docs/reference/api.md b/docs/reference/api.md index 82be78e..4e27901 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -15,7 +15,15 @@ var Version string // default 0.1.0-dev; override with ldflags | `FileName` | `"godo.yaml"` | | `FindFile` | Walk cwd parents for catalog | | `LoadFile` / `Parse` | Load catalog | -| `NewEngine` / `WithDialects` | Runner | +| `NewEngine` / `WithDialects` / `WithRunners` | Engine wiring | +| `NewRunnerRegistry` / `DefaultRunners` / `EffectiveRunner` | Named runners (`RunnerInherit`) | + +A catalog that names no runner uses the `Runner` injected into the engine, so +`NewEngine(cat, myRunner)` keeps working with nothing registered. The CLI +injects the caller's own shell there, which is why `inherit` is the default for +a `godo.yaml`. +| `ArgsAwareRunner` | Runner decides whether a body takes leftover tokens | +| `EngineSpec` / `Plugin` | The `engine:` block: binary minimum + declared plugins | | `Engine.Run` / `PreviewLines` | Execute / expand | | `ExitCode` | Map errors to process codes | @@ -25,6 +33,6 @@ var Version string // default 0.1.0-dev; override with ldflags ## Errors -`ErrNoMatch`, `ErrNoTokens`, `ErrUnexpectedArgs`, `ErrDependencyCycle`, `ErrInvalidCatalog`, `ErrInvalidCapture`, … +`ErrNoMatch`, `ErrNoTokens`, `ErrUnexpectedArgs`, `ErrDependencyCycle`, `ErrInvalidCatalog`, `ErrInvalidCapture`, `ErrUnknownRunner`, … Pre-1.0: symbols may move; prefer pinning modules. [versioning.md](../dev/versioning.md). diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 53281ec..2d654fd 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -18,6 +18,7 @@ Normative source: [contract.md](../contract.md). | | | |--|--| +| `-e runners` | List runners usable on this machine | | `-e version` | Print binary version | | `-e update` | Install newer Release asset | | `-e update check` | Check only | diff --git a/docs/roadmap.md b/docs/roadmap.md index 545ab2a..bf675dc 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -16,12 +16,44 @@ 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 — in progress, unreleased + +Breaking: the shell that runs your scripts changes, and file fields move into +`engine:`. **Held until plugin loading works** — `engine:` without a loader is +half a promise, and this release is where the promise gets made. + +| Promise | | +|---------|--| +| Runner axis | `engine.runner` / `# @runner` pick how a body becomes a process | +| Default runner | The shell **you are in**, not `sh` / `cmd` | +| Shell by name | `sh`, `bash`, `zsh`, `dash`, `ksh`, `ash`, `fish`, `nu`, `cmd`, `pwsh`, `powershell` | +| `engine:` block | `version`, `dialect`, `runner`, `plugins` — what godo needs, apart from what the scripts are | +| `engine.version` | Minimum binary, enforced before anything runs | +| `godo -e runners` | What is usable here, and how to check which shell you are in | +| Compatibility | Top-level `dialect:` keeps working | + +### Landed + +Everything in the table above. + +### Still required before v0.3 ships + +- **Plugin loading.** `engine.plugins` parses and validates; nothing reads the + artifact yet. WASM via `wazero`, digest-pinned — see + [runners and plugins](./dev/runners-and-plugins.md). + +**Not promised in v0.3:** Windows shell detection is written from the +documented behavior of those shells; it compiles and vets for `windows/amd64` +but is unverified on a real Windows host. `GODO_SHELL` overrides it. + ## Post-v0.1 — intended (not promised dates) - Shared family packaging: `MY-RV/homebrew-tap` (`brew install --cask MY-RV/tap/godo`), `MY-RV/scoop-bucket` - Optional: winget (`MY-RV.Godo`), later choco / AUR / Nix as demand appears - Engine command registry polish; more e2e - Dialects backlog only if explicitly promoted here +- Plugin loading (WASM via wazero, digest-pinned) — see + [runners and plugins](./dev/runners-and-plugins.md) ## v1.0 — future promise diff --git a/godo.yaml b/godo.yaml index 7117aac..ec8f03f 100644 --- a/godo.yaml +++ b/godo.yaml @@ -1,5 +1,10 @@ version: "0.1" +# No runner: named, so every script below runs under the shell you are in. +# These are plain commands, so that is the same everywhere — see +# docs/guide/runners.md. A script needing one specific shell says so with +# "# @runner ". + scripts: test: go test ./... vet: go vet ./... diff --git a/internal/catalog/engine.go b/internal/catalog/engine.go index 7c6d8e4..c1d4094 100644 --- a/internal/catalog/engine.go +++ b/internal/catalog/engine.go @@ -13,16 +13,41 @@ type Runner interface { Run(command string) error } +// ArgsAwareRunner is a Runner that decides for itself whether a body takes the +// tokens left over after the match. +// +// The default policy reads the body for ${godo:args…} and rejects leftover +// tokens when it finds none. That is a fact about shell templates, not about +// godo: a plugin's body is a program, has no such placeholder, and would have +// every extra token rejected. "Does this body accept leftover args?" is the +// runner's question, and a runner that implements this answers it. +type ArgsAwareRunner interface { + Runner + AcceptsArgs(commands []string) bool +} + // Engine resolves matches, deps, expansion, and run/preview. type Engine struct { Catalog *Catalog Dialects *DialectRegistry - Runner Runner + Runners *RunnerRegistry + // Runner is the default: what a script that names no runner resolves to. + // Named runners live in Runners. + Runner Runner } // EngineOption configures NewEngine. type EngineOption func(*Engine) +// WithRunners overrides the named-runner registry. +func WithRunners(r *RunnerRegistry) EngineOption { + return func(e *Engine) { + if r != nil { + e.Runners = r + } + } +} + // WithDialects overrides the dialect registry. func WithDialects(r *DialectRegistry) EngineOption { return func(e *Engine) { @@ -37,6 +62,7 @@ func NewEngine(cat *Catalog, runner Runner, opts ...EngineOption) *Engine { e := &Engine{ Catalog: cat, Dialects: DefaultDialects(), + Runners: DefaultRunners(), Runner: runner, } for _, opt := range opts { @@ -78,6 +104,47 @@ func (e *Engine) Resolve(tokens []string) (*Match, error) { return nil, fmt.Errorf("%w: %v", ErrNoMatch, tokens) } +// runnerFor resolves the Runner that executes one step. +// +// The registry wins. Otherwise the unset name means the Runner injected into +// the Engine. +func (e *Engine) runnerFor(name RunnerName) (Runner, error) { + if r, err := e.Runners.Lookup(name); err == nil { + return r, nil + } + if name == "" { + if e.Runner == nil { + return nil, fmt.Errorf("nil runner") + } + return e.Runner, nil + } + return nil, fmt.Errorf("%w: %q", ErrUnknownRunner, name) +} + +// stepRunner returns the effective runner name for a script, rejecting names +// nothing answers to. +// +// This is a name check, not an instance check: BuildPlan must stay usable with +// a nil Runner, which is how PreviewLines expands without anything to execute. +func (e *Engine) stepRunner(s Script) (RunnerName, error) { + name := EffectiveRunner(s, e.Catalog.Runner) + if name == "" { + return name, nil + } + if _, err := e.Runners.Lookup(name); err != nil { + // A runner the catalog declared a plugin for is a different failure + // from a typo, and saying so saves the reader the hunt. + if p, ok := e.Catalog.Engine.ProviderOf("runner", string(name)); ok { + return "", fmt.Errorf("%w: script %q asks for runner %q, provided by plugin %s — this build cannot load plugins", + ErrUnknownRunner, s.Key, name, p.Source) + } + // Otherwise the registry's resolver knows why (unknown name, shell not + // installed); its message is the one worth reading. + return "", fmt.Errorf("script %q asks for runner %q: %w", s.Key, name, err) + } + return name, nil +} + // Plan is the expanded execution plan (deps + body). type Plan struct { Match *Match @@ -89,6 +156,7 @@ type PlanStep struct { Kind string // "dep" | "body" Source string Command string + Runner RunnerName // effective runner for this step } // BuildPlan expands deps + body without executing. @@ -102,7 +170,11 @@ func (e *Engine) BuildPlan(tokens []string) (*Plan, error) { if err != nil { return nil, err } - if err := validateArgs(m); err != nil { + runner, err := e.stepRunner(m.Script) + if err != nil { + return nil, err + } + if err := e.validateArgs(m, runner); err != nil { return nil, err } plan := &Plan{Match: m} @@ -116,17 +188,44 @@ func (e *Engine) BuildPlan(tokens []string) (*Plan, error) { return nil, err } for _, c := range cmds { - plan.Steps = append(plan.Steps, PlanStep{Kind: "body", Source: m.Script.Key, Command: c}) + plan.Steps = append(plan.Steps, PlanStep{Kind: "body", Source: m.Script.Key, Command: c, Runner: runner}) } return plan, nil } -func validateArgs(m *Match) error { - need := expand.CommandsNeedArgs(m.Script.Commands) - if !need && len(m.Args) > 0 { - return fmt.Errorf("%w: %v (script %q has no ${godo:args})", ErrUnexpectedArgs, m.Args, m.Script.Key) +// validateArgs rejects leftover tokens a script cannot take. +// +// Which tokens a script can take depends on the runner, so the check asks the +// runner when it has an opinion and falls back to the placeholder policy +// otherwise. +func (e *Engine) validateArgs(m *Match, runner RunnerName) error { + if len(m.Args) == 0 { + return nil } - return nil + if ar, ok := e.argsAwareRunner(runner); ok { + if ar.AcceptsArgs(m.Script.Commands) { + return nil + } + return fmt.Errorf("%w: %v (script %q takes no args under runner %q)", ErrUnexpectedArgs, m.Args, m.Script.Key, runner) + } + if expand.CommandsNeedArgs(m.Script.Commands) { + return nil + } + return fmt.Errorf("%w: %v (script %q has no ${godo:args})", ErrUnexpectedArgs, m.Args, m.Script.Key) +} + +// argsAwareRunner reports whether the runner for this step answers the args +// question itself. Registry first, then the injected default, like runnerFor. +func (e *Engine) argsAwareRunner(name RunnerName) (ArgsAwareRunner, bool) { + if r, err := e.Runners.Lookup(name); err == nil { + ar, ok := r.(ArgsAwareRunner) + return ar, ok + } + if name == "" { + ar, ok := e.Runner.(ArgsAwareRunner) + return ar, ok + } + return nil, false } // appendDeps walks m's dependencies depth-first. @@ -158,7 +257,11 @@ func (e *Engine) appendDeps(plan *Plan, m *Match, stack, done map[string]bool) e if err != nil { return fmt.Errorf("deps %q: %w", invKey, err) } - if err := validateArgs(depMatch); err != nil { + depRunner, err := e.stepRunner(depMatch.Script) + if err != nil { + return err + } + if err := e.validateArgs(depMatch, depRunner); err != nil { return err } if err := e.appendDeps(plan, depMatch, stack, done); err != nil { @@ -169,7 +272,7 @@ func (e *Engine) appendDeps(plan *Plan, m *Match, stack, done map[string]bool) e return err } for _, c := range cmds { - plan.Steps = append(plan.Steps, PlanStep{Kind: "dep", Source: invKey, Command: c}) + plan.Steps = append(plan.Steps, PlanStep{Kind: "dep", Source: invKey, Command: c, Runner: depRunner}) } delete(stack, invKey) done[invKey] = true @@ -183,11 +286,12 @@ func (e *Engine) Run(tokens []string) error { if err != nil { return err } - if e.Runner == nil { - return fmt.Errorf("nil runner") - } for _, step := range plan.Steps { - if err := e.Runner.Run(step.Command); err != nil { + runner, err := e.runnerFor(step.Runner) + if err != nil { + return err + } + if err := runner.Run(step.Command); err != nil { return err } } diff --git a/internal/catalog/engine_block.go b/internal/catalog/engine_block.go new file mode 100644 index 0000000..88b7af9 --- /dev/null +++ b/internal/catalog/engine_block.go @@ -0,0 +1,110 @@ +package catalog + +import ( + "fmt" + "strings" +) + +// EngineSpec is the `engine:` block: what godo itself needs in order to run +// this catalog. +// +// Everything else in the file describes the scripts — which one answers your +// tokens, how its body runs. This describes the tool: the binary a catalog +// expects, and the plugins it wants loaded. Keeping them apart is what lets +// the toolchain side grow (a lockfile, a package manager) without the script +// side growing with it. +// +// Named EngineSpec, not Engine, because Engine is already the thing that +// resolves and runs a catalog. +type EngineSpec struct { + // Version is the minimum godo binary, "0.3.0" or ">=0.3.0". + Version string `yaml:"version"` + // Dialect and Runner configure how godo reads and runs this file. They are + // here rather than at the top level because they are settings for the tool, + // not content of the catalog: scripts: is the data, engine: is the dial. + Dialect DialectName `yaml:"dialect"` + Runner RunnerName `yaml:"runner"` + Plugins []Plugin `yaml:"plugins"` +} + +// Plugin is one entry of `engine.plugins`. +// +// Nothing loads these yet. They are parsed and validated so the shape is +// settled and a catalog can already declare what it expects; a script asking +// for a runner a plugin provides fails with that plugin named, rather than +// with "unknown runner". +type Plugin struct { + Source string `yaml:"source"` + // SHA256 is required. A plugin is third-party code that runs when someone + // types `godo test`; without a digest there is nothing to verify it is the + // code that was reviewed. + SHA256 string `yaml:"sha256"` + // Provides entries are ":", kind being runner or dialect. + Provides []string `yaml:"provides"` + // Config is optional and entirely the plugin's: its keys, its meaning, its + // defaults. godo carries it across and does not read it. What a plugin + // treats as deny-all, or ignores, is the plugin's contract, not godo's. + Config map[string]any `yaml:"config"` +} + +// Minimum returns the minimum binary version the catalog asks for, and whether +// it asked at all. +func (e EngineSpec) Minimum() (string, bool) { + v := strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(e.Version), ">=")) + return v, v != "" +} + +// ProviderOf returns the plugin that says it provides ":". +func (e EngineSpec) ProviderOf(kind, name string) (Plugin, bool) { + want := kind + ":" + name + for _, p := range e.Plugins { + for _, got := range p.Provides { + if got == want { + return p, true + } + } + } + return Plugin{}, false +} + +// validateEngine checks the block at load time. +func validateEngine(e EngineSpec) error { + if v := strings.TrimSpace(e.Version); v != "" { + rest := strings.TrimSpace(strings.TrimPrefix(v, ">=")) + if rest == "" { + return fmt.Errorf("engine.version %q: want a version, as \"0.3.0\" or \">=0.3.0\"", e.Version) + } + if strings.ContainsAny(rest, "<>=~^ ") { + return fmt.Errorf("engine.version %q: only a minimum is supported, as \"0.3.0\" or \">=0.3.0\"", e.Version) + } + } + seen := map[string]string{} + for i, p := range e.Plugins { + where := fmt.Sprintf("engine.plugins[%d]", i) + if strings.TrimSpace(p.Source) == "" { + return fmt.Errorf("%s: source is required", where) + } + if strings.TrimSpace(p.SHA256) == "" { + return fmt.Errorf("%s (%s): sha256 is required", where, p.Source) + } + if len(p.Provides) == 0 { + return fmt.Errorf("%s (%s): provides is required, as [runner:name]", where, p.Source) + } + for _, entry := range p.Provides { + kind, name, ok := strings.Cut(entry, ":") + if !ok || name == "" { + return fmt.Errorf("%s (%s): provides %q: want \":\"", where, p.Source, entry) + } + switch kind { + case "runner", "dialect": + default: + return fmt.Errorf("%s (%s): provides %q: kind must be runner or dialect", where, p.Source, entry) + } + if prev, dup := seen[entry]; dup { + return fmt.Errorf("%s (%s): provides %q already provided by %s", where, p.Source, entry, prev) + } + seen[entry] = p.Source + } + } + return nil +} diff --git a/internal/catalog/engine_block_test.go b/internal/catalog/engine_block_test.go new file mode 100644 index 0000000..7195245 --- /dev/null +++ b/internal/catalog/engine_block_test.go @@ -0,0 +1,186 @@ +package catalog_test + +import ( + "errors" + "strings" + "testing" + + "github.com/my-rv/godo/internal/catalog" +) + +const pluginBlock = `version: "0.1" + +engine: + version: ">=0.3.0" + plugins: + - source: https://example.test/godo-micropy@v1.2.0 + sha256: deadbeef + provides: [runner:micropy] + config: + proc: {exec: true, spawn: false} + stdlib: [os, os.path] + +scripts: + # @runner micropy + t: print("hi") +` + +func TestParse_engineBlock(t *testing.T) { + cat, err := catalog.Parse([]byte(pluginBlock), "x") + if err != nil { + t.Fatal(err) + } + min, ok := cat.Engine.Minimum() + if !ok || min != "0.3.0" { + t.Fatalf("minimum=%q ok=%v", min, ok) + } + if len(cat.Engine.Plugins) != 1 { + t.Fatalf("plugins=%+v", cat.Engine.Plugins) + } + p := cat.Engine.Plugins[0] + if p.SHA256 != "deadbeef" || p.Source == "" { + t.Fatalf("plugin=%+v", p) + } + // Config is the plugin's own; godo carries it without interpreting it. + if _, ok := p.Config["proc"]; !ok { + t.Fatalf("config=%+v", p.Config) + } + got, ok := cat.Engine.ProviderOf("runner", "micropy") + if !ok || got.Source != p.Source { + t.Fatalf("ProviderOf=%+v %v", got, ok) + } + if _, ok := cat.Engine.ProviderOf("runner", "nope"); ok { + t.Fatal("ProviderOf matched a name nothing provides") + } +} + +func TestParse_engineMinimumAcceptsBareVersion(t *testing.T) { + cat, err := catalog.Parse([]byte("version: \"0.1\"\nengine:\n version: 0.3.0\nscripts:\n t: echo hi\n"), "x") + if err != nil { + t.Fatal(err) + } + if min, ok := cat.Engine.Minimum(); !ok || min != "0.3.0" { + t.Fatalf("minimum=%q ok=%v", min, ok) + } +} + +func TestParse_noEngineBlockAsksForNothing(t *testing.T) { + cat, err := catalog.Parse([]byte("version: \"0.1\"\nscripts:\n t: echo hi\n"), "x") + if err != nil { + t.Fatal(err) + } + if _, ok := cat.Engine.Minimum(); ok { + t.Fatal("a catalog with no engine block must ask for no version") + } + if len(cat.Engine.Plugins) != 0 { + t.Fatalf("plugins=%+v", cat.Engine.Plugins) + } +} + +func TestParse_engineRejects(t *testing.T) { + cases := []struct{ name, src, want string }{ + {"no source", "engine:\n plugins:\n - sha256: a\n provides: [runner:x]\n", "source is required"}, + {"no digest", "engine:\n plugins:\n - source: s\n provides: [runner:x]\n", "sha256 is required"}, + {"no provides", "engine:\n plugins:\n - source: s\n sha256: a\n", "provides is required"}, + {"bad kind", "engine:\n plugins:\n - source: s\n sha256: a\n provides: [wat:x]\n", "kind must be runner or dialect"}, + {"no name", "engine:\n plugins:\n - source: s\n sha256: a\n provides: [runner]\n", `want ":"`}, + {"range syntax", "engine:\n version: \"^1.2\"\n", "only a minimum is supported"}, + {"empty constraint", "engine:\n version: \">=\"\n", "want a version"}, + { + "duplicate provides", + "engine:\n plugins:\n - source: a\n sha256: x\n provides: [runner:m]\n" + + " - source: b\n sha256: y\n provides: [runner:m]\n", + "already provided by a", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := catalog.Parse([]byte("version: \"0.1\"\n"+tc.src+"scripts:\n t: echo hi\n"), "x") + if err == nil || !errors.Is(err, catalog.ErrInvalidCatalog) { + t.Fatalf("err=%v", err) + } + if !strings.Contains(err.Error(), tc.want) { + t.Fatalf("err=%v, want %q", err, tc.want) + } + }) + } +} + +// A runner a plugin declares fails by naming the plugin, not as a typo. +func TestBuildPlan_pluginRunnerNamesItsPlugin(t *testing.T) { + cat, err := catalog.Parse([]byte(pluginBlock), "x") + if err != nil { + t.Fatal(err) + } + eng := catalog.NewEngine(cat, runnerFunc(func(string) error { return nil })) + _, err = eng.BuildPlan([]string{"t"}) + if !errors.Is(err, catalog.ErrUnknownRunner) { + t.Fatalf("err=%v", err) + } + for _, want := range []string{"micropy", "godo-micropy", "cannot load plugins"} { + if !strings.Contains(err.Error(), want) { + t.Fatalf("err=%v, missing %q", err, want) + } + } +} + +// dialect and runner live in engine:. +func TestParse_engineDialectAndRunner(t *testing.T) { + src := "version: \"0.1\"\nengine:\n dialect: matcher\n runner: bash\nscripts:\n t ${M}: echo ${godo:argv[M]}\n" + cat, err := catalog.Parse([]byte(src), "x") + if err != nil { + t.Fatal(err) + } + if cat.Dialect != catalog.DialectMatcher { + t.Fatalf("dialect=%q", cat.Dialect) + } + if cat.Runner != "bash" { + t.Fatalf("runner=%q", cat.Runner) + } +} + +// Top-level dialect: shipped in 0.1 and 0.2, so it keeps working. +func TestParse_legacyTopLevelDialect(t *testing.T) { + cat, err := catalog.Parse([]byte("version: \"0.1\"\ndialect: matcher\nscripts:\n t ${M}: echo hi\n"), "x") + if err != nil { + t.Fatal(err) + } + if cat.Dialect != catalog.DialectMatcher { + t.Fatalf("dialect=%q", cat.Dialect) + } +} + +// engine.dialect wins when both are present. +func TestParse_engineDialectBeatsLegacy(t *testing.T) { + src := "version: \"0.1\"\ndialect: package\nengine:\n dialect: matcher\nscripts:\n t ${M}: echo hi\n" + cat, err := catalog.Parse([]byte(src), "x") + if err != nil { + t.Fatal(err) + } + if cat.Dialect != catalog.DialectMatcher { + t.Fatalf("dialect=%q", cat.Dialect) + } +} + +// config is the plugin's: optional, and godo carries it without reading it. +func TestParse_pluginConfigIsOptionalAndUninterpreted(t *testing.T) { + src := "version: \"0.1\"\nengine:\n plugins:\n" + + " - source: s\n sha256: a\n provides: [runner:x]\n" + + " - source: t\n sha256: b\n provides: [runner:y]\n" + + " config: {anything: [1, 2], nested: {deep: true}}\n" + + "scripts:\n t: echo hi\n" + cat, err := catalog.Parse([]byte(src), "x") + if err != nil { + t.Fatal(err) + } + if cat.Engine.Plugins[0].Config != nil { + t.Fatalf("absent config should stay nil, got %+v", cat.Engine.Plugins[0].Config) + } + cfg := cat.Engine.Plugins[1].Config + if _, ok := cfg["anything"]; !ok { + t.Fatalf("config=%+v", cfg) + } + if _, ok := cfg["nested"]; !ok { + t.Fatalf("config=%+v", cfg) + } +} diff --git a/internal/catalog/engine_test.go b/internal/catalog/engine_test.go index 018cb26..84c61c9 100644 --- a/internal/catalog/engine_test.go +++ b/internal/catalog/engine_test.go @@ -392,3 +392,294 @@ func TestEngine_argsAreQuotedIntoTheShellLine(t *testing.T) { t.Fatalf("%q", ran) } } + +// --- runner axis ------------------------------------------------------------ + +// An unset runner stays unset and means "the Runner this engine was given", +// so an embedder's injected runner keeps working without naming anything. +func TestParse_unsetRunnerMeansTheInjectedOne(t *testing.T) { + cat, err := catalog.Parse([]byte("version: \"0.1\"\nscripts:\n t: echo ok\n"), "x") + if err != nil { + t.Fatal(err) + } + if cat.Runner != "" { + t.Fatalf("file runner=%q, want empty", cat.Runner) + } + if got := catalog.EffectiveRunner(cat.Scripts[0], cat.Runner); got != "" { + t.Fatalf("effective=%q, want empty", got) + } + var ran []string + eng := catalog.NewEngine(cat, runnerFunc(func(c string) error { ran = append(ran, c); return nil })) + if err := eng.Run([]string{"t"}); err != nil { + t.Fatal(err) + } + if len(ran) != 1 || ran[0] != "echo ok" { + t.Fatalf("ran=%q", ran) + } +} + +func TestParse_fileRunnerAndDecoratorOverride(t *testing.T) { + src := "version: \"0.1\"\nengine:\n runner: micropy\nscripts:\n" + + " inherits: echo a\n" + + " # @runner bash\n" + + " overrides: echo b\n" + cat, err := catalog.Parse([]byte(src), "x") + if err != nil { + t.Fatal(err) + } + if cat.Runner != "micropy" { + t.Fatalf("file runner=%q", cat.Runner) + } + if got := catalog.EffectiveRunner(cat.Scripts[0], cat.Runner); got != "micropy" { + t.Fatalf("inherits=%q", got) + } + if got := catalog.EffectiveRunner(cat.Scripts[1], cat.Runner); got != "bash" { + t.Fatalf("overrides=%q", got) + } +} + +func TestParse_runnerDecoratorRequiresName(t *testing.T) { + src := "version: \"0.1\"\nscripts:\n # @runner\n t: echo ok\n" + _, err := catalog.Parse([]byte(src), "x") + if err == nil || !errors.Is(err, catalog.ErrInvalidCatalog) { + t.Fatalf("err=%v", err) + } + if !strings.Contains(err.Error(), "@runner requires a name") { + t.Fatalf("err=%v", err) + } +} + +// An older binary has no @runner, and its unknown-decorator rejection is what +// stops a body meant for another runner from reaching the host shell. +func TestParse_unknownDecoratorStillRejected(t *testing.T) { + src := "version: \"0.1\"\nscripts:\n # @executor exec\n t: echo ok\n" + _, err := catalog.Parse([]byte(src), "x") + if err == nil || !strings.Contains(err.Error(), "unknown decorator @executor") { + t.Fatalf("err=%v", err) + } +} + +func TestBuildPlan_unknownRunnerFailsClosed(t *testing.T) { + src := "version: \"0.1\"\nscripts:\n # @runner nope\n t: echo ok\n" + cat, err := catalog.Parse([]byte(src), "x") + if err != nil { + t.Fatal(err) + } + eng := catalog.NewEngine(cat, runnerFunc(func(string) error { return nil })) + if _, err := eng.BuildPlan([]string{"t"}); !errors.Is(err, catalog.ErrUnknownRunner) { + t.Fatalf("err=%v", err) + } + // Nothing must execute, so --preview has to fail the same way. + if _, err := eng.PreviewLines([]string{"t"}); !errors.Is(err, catalog.ErrUnknownRunner) { + t.Fatalf("preview err=%v", err) + } +} + +func TestRun_dispatchesPerStepRunner(t *testing.T) { + src := "version: \"0.1\"\nscripts:\n" + + " # @runner other\n" + + " dep: echo dep\n" + + " # @deps dep\n" + + " body: echo body\n" + cat, err := catalog.Parse([]byte(src), "x") + if err != nil { + t.Fatal(err) + } + var shell, other []string + reg := catalog.NewRunnerRegistry() + if err := reg.Register("other", runnerFunc(func(c string) error { + other = append(other, c) + return nil + })); err != nil { + t.Fatal(err) + } + eng := catalog.NewEngine(cat, runnerFunc(func(c string) error { + shell = append(shell, c) + return nil + }), catalog.WithRunners(reg)) + + if err := eng.Run([]string{"body"}); err != nil { + t.Fatal(err) + } + if len(other) != 1 || other[0] != "echo dep" { + t.Fatalf("other=%v", other) + } + if len(shell) != 1 || shell[0] != "echo body" { + t.Fatalf("shell=%v", shell) + } +} + +func TestBuildPlan_stepsRecordTheirRunner(t *testing.T) { + src := "version: \"0.1\"\nscripts:\n" + + " # @runner other\n" + + " dep: echo dep\n" + + " # @deps dep\n" + + " body: echo body\n" + cat, err := catalog.Parse([]byte(src), "x") + if err != nil { + t.Fatal(err) + } + reg := catalog.NewRunnerRegistry() + _ = reg.Register("other", runnerFunc(func(string) error { return nil })) + eng := catalog.NewEngine(cat, nil, catalog.WithRunners(reg)) + + plan, err := eng.BuildPlan([]string{"body"}) + if err != nil { + t.Fatal(err) + } + if len(plan.Steps) != 2 { + t.Fatalf("steps=%+v", plan.Steps) + } + // The dep named one; the body named none, so it is the injected default. + if plan.Steps[0].Runner != "other" || plan.Steps[1].Runner != "" { + t.Fatalf("runners=%q %q", plan.Steps[0].Runner, plan.Steps[1].Runner) + } +} + +// A named runner comes from the registry, never from the injected default. +func TestRun_namedRunnerBeatsTheInjectedDefault(t *testing.T) { + cat, err := catalog.Parse([]byte("version: \"0.1\"\nengine:\n runner: inherit\nscripts:\n t: echo ok\n"), "x") + if err != nil { + t.Fatal(err) + } + var injected, registered int + reg := catalog.NewRunnerRegistry() + _ = reg.Register(catalog.RunnerInherit, runnerFunc(func(string) error { registered++; return nil })) + eng := catalog.NewEngine(cat, runnerFunc(func(string) error { injected++; return nil }), catalog.WithRunners(reg)) + if err := eng.Run([]string{"t"}); err != nil { + t.Fatal(err) + } + if registered != 1 || injected != 0 { + t.Fatalf("registered=%d injected=%d", registered, injected) + } +} + +func TestRun_nilShellRunnerStillErrors(t *testing.T) { + cat, err := catalog.Parse([]byte("version: \"0.1\"\nscripts:\n t: echo ok\n"), "x") + if err != nil { + t.Fatal(err) + } + if err := catalog.NewEngine(cat, nil).Run([]string{"t"}); err == nil || + !strings.Contains(err.Error(), "nil runner") { + t.Fatalf("err=%v", err) + } +} + +func TestRunnerRegistry_rejectsEmptyNameAndNilRunner(t *testing.T) { + reg := catalog.NewRunnerRegistry() + if err := reg.Register("", runnerFunc(func(string) error { return nil })); err == nil { + t.Fatal("want error for empty name") + } + if err := reg.Register("x", nil); err == nil { + t.Fatal("want error for nil runner") + } + if _, err := reg.Lookup("x"); !errors.Is(err, catalog.ErrUnknownRunner) { + t.Fatalf("err=%v", err) + } +} + +// --- args are the runner's question ----------------------------------------- + +// pluginRunner stands in for a plugin whose body is a program rather than a +// shell template: no ${godo:args} to find, and it takes args regardless. +type pluginRunner struct{ ran []string } + +func (r *pluginRunner) Run(c string) error { r.ran = append(r.ran, c); return nil } +func (r *pluginRunner) AcceptsArgs([]string) bool { return true } + +type strictRunner struct{} + +func (strictRunner) Run(string) error { return nil } +func (strictRunner) AcceptsArgs([]string) bool { return false } + +// Default policy, unchanged: no ${godo:args} in the body → extra tokens error. +func TestBuildPlan_defaultPolicyStillRejectsUnexpectedArgs(t *testing.T) { + cat, err := catalog.Parse([]byte("version: \"0.1\"\nscripts:\n t: echo hi\n"), "x") + if err != nil { + t.Fatal(err) + } + eng := catalog.NewEngine(cat, runnerFunc(func(string) error { return nil })) + _, err = eng.BuildPlan([]string{"t", "extra"}) + if !errors.Is(err, catalog.ErrUnexpectedArgs) { + t.Fatalf("err=%v", err) + } + if !strings.Contains(err.Error(), "${godo:args}") { + t.Fatalf("err=%v", err) + } +} + +// A plugin body has no placeholder, and still has to be able to take args. +func TestBuildPlan_argsAwareRunnerOverridesThePlaceholderPolicy(t *testing.T) { + src := "version: \"0.1\"\nscripts:\n # @runner micropy\n t: print(godo.args)\n" + cat, err := catalog.Parse([]byte(src), "x") + if err != nil { + t.Fatal(err) + } + reg := catalog.NewRunnerRegistry() + pr := &pluginRunner{} + if err := reg.Register("micropy", pr); err != nil { + t.Fatal(err) + } + eng := catalog.NewEngine(cat, nil, catalog.WithRunners(reg)) + if err := eng.Run([]string{"t", "--flag", "a b"}); err != nil { + t.Fatalf("run: %v", err) + } + if len(pr.ran) != 1 || pr.ran[0] != "print(godo.args)" { + t.Fatalf("ran=%q", pr.ran) + } +} + +func TestBuildPlan_argsAwareRunnerCanRefuse(t *testing.T) { + src := "version: \"0.1\"\nscripts:\n # @runner strict\n t: echo ${godo:args}\n" + cat, err := catalog.Parse([]byte(src), "x") + if err != nil { + t.Fatal(err) + } + reg := catalog.NewRunnerRegistry() + _ = reg.Register("strict", strictRunner{}) + eng := catalog.NewEngine(cat, nil, catalog.WithRunners(reg)) + _, err = eng.BuildPlan([]string{"t", "extra"}) + if !errors.Is(err, catalog.ErrUnexpectedArgs) { + t.Fatalf("err=%v", err) + } + if !strings.Contains(err.Error(), `runner "strict"`) { + t.Fatalf("err=%v", err) + } +} + +// Deps are matched on their own, so the check uses the dep's runner. +func TestBuildPlan_depArgsUseTheDepRunner(t *testing.T) { + src := "version: \"0.1\"\ndialect: matcher\nscripts:\n" + + " # @runner micropy\n" + + " seed ${ENV}: print(${godo:argv[ENV]})\n" + + " # @deps seed dev\n" + + " boot: echo up\n" + cat, err := catalog.Parse([]byte(src), "x") + if err != nil { + t.Fatal(err) + } + reg := catalog.NewRunnerRegistry() + _ = reg.Register("micropy", &pluginRunner{}) + eng := catalog.NewEngine(cat, runnerFunc(func(string) error { return nil }), catalog.WithRunners(reg)) + plan, err := eng.BuildPlan([]string{"boot"}) + if err != nil { + t.Fatal(err) + } + if len(plan.Steps) != 2 || plan.Steps[0].Runner != "micropy" { + t.Fatalf("steps=%+v", plan.Steps) + } +} + +// An unknown runner is still caught before the args question is asked. +func TestBuildPlan_unknownRunnerBeatsUnexpectedArgs(t *testing.T) { + src := "version: \"0.1\"\nscripts:\n # @runner nope\n t: echo hi\n" + cat, err := catalog.Parse([]byte(src), "x") + if err != nil { + t.Fatal(err) + } + eng := catalog.NewEngine(cat, runnerFunc(func(string) error { return nil })) + _, err = eng.BuildPlan([]string{"t", "extra"}) + if !errors.Is(err, catalog.ErrUnknownRunner) { + t.Fatalf("err=%v", err) + } +} diff --git a/internal/catalog/errors.go b/internal/catalog/errors.go index 4e52f15..1bfbf2f 100644 --- a/internal/catalog/errors.go +++ b/internal/catalog/errors.go @@ -13,6 +13,7 @@ var ( ErrDependencyCycle = errors.New("dependency cycle") ErrInvalidCatalog = errors.New("invalid catalog") ErrInvalidCapture = errors.New("invalid capture name") + ErrUnknownRunner = errors.New("unknown runner") ) // ExitError carries a process exit code from a failed command. diff --git a/internal/catalog/load.go b/internal/catalog/load.go index 5bfa5ba..a9173bc 100644 --- a/internal/catalog/load.go +++ b/internal/catalog/load.go @@ -42,6 +42,7 @@ func Parse(data []byte, path string, dialects ...*DialectRegistry) (*Catalog, er } cat := &Catalog{Path: path} + var legacyDialect DialectName var scriptsNode *yaml.Node for i := 0; i < len(doc.Content); i += 2 { key := doc.Content[i] @@ -50,7 +51,13 @@ func Parse(data []byte, path string, dialects ...*DialectRegistry) (*Catalog, er case "version": cat.Version = strings.TrimSpace(scalarString(val)) case "dialect": - cat.Dialect = DialectName(strings.TrimSpace(scalarString(val))) + // Legacy position, kept because it shipped in 0.1 and 0.2. + // engine.dialect is the current one and wins. + legacyDialect = DialectName(strings.TrimSpace(scalarString(val))) + case "engine": + if err := val.Decode(&cat.Engine); err != nil { + return nil, fmt.Errorf("%w: engine: %v", ErrInvalidCatalog, err) + } case "scripts": scriptsNode = val } @@ -58,9 +65,26 @@ func Parse(data []byte, path string, dialects ...*DialectRegistry) (*Catalog, er if cat.Version == "" { return nil, fmt.Errorf("%w: version is required", ErrInvalidCatalog) } + if err := validateEngine(cat.Engine); err != nil { + return nil, fmt.Errorf("%w: %v", ErrInvalidCatalog, err) + } + cat.Dialect = cat.Engine.Dialect + if cat.Dialect == "" { + cat.Dialect = legacyDialect + } + cat.Runner = cat.Engine.Runner if cat.Dialect == "" { cat.Dialect = DialectPackage } + // An unset runner stays unset: it means "whatever this engine runs with", + // which is the Runner injected into it. Naming a default here would bake a + // choice that belongs to the caller — godo the CLI runs your own shell, + // while an embedder runs what it wired up. + // + // Runner names are not validated here either. A dialect has to resolve + // before a script can be matched at all, so an unknown one is a load error; + // a runner is only needed to execute, and the registry that could answer + // for it belongs to the Engine. An unknown runner fails at plan time. if _, err := reg.Lookup(cat.Dialect); err != nil { return nil, fmt.Errorf("%w: dialect %q not implemented", ErrInvalidCatalog, cat.Dialect) } @@ -90,13 +114,14 @@ func Parse(data []byte, path string, dialects ...*DialectRegistry) (*Catalog, er func scriptFromNodes(key, val *yaml.Node, fileDialect DialectName, reg *DialectRegistry) (Script, error) { s := Script{Key: key.Value} - doc, deps, dialect, err := parseDecorators(key.HeadComment) + dec, err := parseDecorators(key.HeadComment) if err != nil { return Script{}, fmt.Errorf("%w: script %q: %v", ErrInvalidCatalog, s.Key, err) } - s.Doc = doc - s.Deps = deps - s.Dialect = dialect + s.Doc = dec.doc + s.Deps = dec.deps + s.Dialect = dec.dialect + s.Runner = dec.runner if s.Dialect != "" { if _, err := reg.Lookup(DialectName(s.Dialect)); err != nil { return Script{}, fmt.Errorf("%w: script %q: @dialect %q not implemented", ErrInvalidCatalog, s.Key, s.Dialect) @@ -153,9 +178,18 @@ func validateMatcherKey(key string) error { return nil } -func parseDecorators(headComment string) (doc string, deps []string, dialect string, err error) { +// decorators is the parsed @-block above a script key. +type decorators struct { + doc string + deps []string + dialect string + runner string +} + +func parseDecorators(headComment string) (decorators, error) { + var dec decorators if headComment == "" { - return "", nil, "", nil + return dec, nil } var docLines []string for _, line := range strings.Split(headComment, "\n") { @@ -173,20 +207,26 @@ func parseDecorators(headComment string) (doc string, deps []string, dialect str switch fields[0] { case "@deps", "@dependencies": rest := strings.TrimSpace(strings.TrimPrefix(line, fields[0])) - deps = append(deps, splitDepsList(rest)...) + dec.deps = append(dec.deps, splitDepsList(rest)...) case "@dialect": if len(fields) < 2 || fields[1] == "" { - return "", nil, "", fmt.Errorf("@dialect requires a name") + return decorators{}, fmt.Errorf("@dialect requires a name") + } + dec.dialect = fields[1] + case "@runner": + if len(fields) < 2 || fields[1] == "" { + return decorators{}, fmt.Errorf("@runner requires a name") } - dialect = fields[1] + dec.runner = fields[1] default: - return "", nil, "", fmt.Errorf("unknown decorator %s", fields[0]) + return decorators{}, fmt.Errorf("unknown decorator %s", fields[0]) } continue } docLines = append(docLines, line) } - return strings.Join(docLines, "\n"), deps, dialect, nil + dec.doc = strings.Join(docLines, "\n") + return dec, nil } func splitDepsList(s string) []string { diff --git a/internal/catalog/runner.go b/internal/catalog/runner.go new file mode 100644 index 0000000..756a6cc --- /dev/null +++ b/internal/catalog/runner.go @@ -0,0 +1,102 @@ +package catalog + +import "fmt" + +// RunnerName identifies an execution strategy. +// +// Dialect answers "which script responds to these tokens"; runner answers +// "how does the resolved body become a process". The two are independent: a +// script may be matched by any dialect and run by any runner. +type RunnerName string + +const ( + // RunnerInherit runs the line through the shell the caller is already in. + // + // The CLI injects it as the default: the shell someone is using is their + // own business, and godo proxying to a different one was a choice that is + // not godo's to make. + // + // It is deliberately absent from DefaultRunners: internal/execshell imports + // this package, so it cannot be a default registry entry without an import + // cycle. An unset runner resolves to the Runner injected into the Engine, + // which is also what keeps NewEngine(cat, runner) working unchanged. + RunnerInherit RunnerName = "inherit" +) + +// RunnerRegistry maps runner names to implementations (Open/Closed). +// +// Unlike Dialect, a Runner carries per-invocation state (working directory, +// stdio), so the name is supplied at registration rather than being a method +// on the type: one implementation can be registered twice with different +// configuration. +type RunnerRegistry struct { + byName map[RunnerName]Runner + + // Resolve answers for names that were not registered ahead of time. + // + // It is how "# @runner bash" works without godo keeping a list of every + // shell anyone might have: the name is looked up when a catalog asks for + // it. godo does not manage those shells — it proxies to them. + Resolve func(RunnerName) (Runner, error) +} + +// NewRunnerRegistry returns an empty registry. +func NewRunnerRegistry() *RunnerRegistry { + return &RunnerRegistry{byName: make(map[RunnerName]Runner)} +} + +// DefaultRunners returns the registry a stock engine starts with. +// +// It is empty: an unset runner is the injected Runner. The function exists so +// engine wiring reads the same for both axes. +func DefaultRunners() *RunnerRegistry { return NewRunnerRegistry() } + +// Register adds a runner under name. +func (r *RunnerRegistry) Register(name RunnerName, run Runner) error { + if r.byName == nil { + r.byName = make(map[RunnerName]Runner) + } + if name == "" { + return fmt.Errorf("empty runner name") + } + if run == nil { + return fmt.Errorf("nil runner for %q", name) + } + r.byName[name] = run + return nil +} + +// Lookup returns a runner by name. +func (r *RunnerRegistry) Lookup(name RunnerName) (Runner, error) { + if r == nil { + return nil, fmt.Errorf("%w: %q", ErrUnknownRunner, name) + } + if r.byName == nil { + r.byName = make(map[RunnerName]Runner) + } + if run, ok := r.byName[name]; ok { + return run, nil + } + if r.Resolve == nil { + return nil, fmt.Errorf("%w: %q", ErrUnknownRunner, name) + } + run, err := r.Resolve(name) + if err != nil { + return nil, err + } + // Memoised: Lookup is called more than once per step (validate, then run). + r.byName[name] = run + return run, nil +} + +// EffectiveRunner resolves the @runner override or the file default. +// +// Empty means "the Runner this engine was given". godo the CLI injects the +// caller's own shell there, so a catalog that names nothing runs under the +// shell you are in; an embedder injects whatever it wants and gets that. +func EffectiveRunner(script Script, fileDefault RunnerName) RunnerName { + if script.Runner != "" { + return RunnerName(script.Runner) + } + return fileDefault +} diff --git a/internal/catalog/types.go b/internal/catalog/types.go index 98f4943..ccad344 100644 --- a/internal/catalog/types.go +++ b/internal/catalog/types.go @@ -21,13 +21,16 @@ type Script struct { Doc string // joined non-@ comment lines Deps []string // @deps / @dependencies entries (raw, before expand) Dialect string // @dialect override; empty → file dialect + Runner string // @runner override; empty → file runner } // Catalog is a loaded godo.yaml. type Catalog struct { Path string Version string + Engine EngineSpec Dialect DialectName + Runner RunnerName Scripts []Script // definition order } diff --git a/internal/cli/app.go b/internal/cli/app.go index 8b2684f..cc8ac18 100644 --- a/internal/cli/app.go +++ b/internal/cli/app.go @@ -4,7 +4,9 @@ import ( "fmt" "io" "os" + "os/exec" "path/filepath" + "runtime" "strings" "github.com/my-rv/godo" @@ -56,6 +58,9 @@ func (a *App) Run(args []string) error { fmt.Fprintln(a.Stdout, godo.Version) return nil } + if mode == modeRunners { + return a.listRunners(a.nearestCatalog()) + } if mode == modeUpdate || mode == modeUpdateCheck { return a.runUpdate(mode == modeUpdateCheck) } @@ -72,11 +77,32 @@ func (a *App) Run(args []string) error { if err != nil { return err } + if err := requireEngineVersion(cat, godo.Version); err != nil { + return err + } + root := filepath.Dir(path) + // The default is the shell you are in. A catalog that names no runner gets + // this one, which is why godo stops running zsh users under sh. runner := a.Runner if runner == nil { - runner = execshell.Runner{Dir: filepath.Dir(path)} + runner = execshell.InheritRunner{Dir: root} } - eng := catalog.NewEngine(cat, runner) + // Registered here rather than in internal/catalog: internal/execshell + // imports it, so the engine cannot hold it without an import cycle. + runners := catalog.NewRunnerRegistry() + if err := runners.Register(catalog.RunnerInherit, execshell.InheritRunner{Dir: root}); err != nil { + return err + } + // Any other name is a shell the catalog asked for by name. godo does not + // manage those — it proxies to whatever is on PATH. + runners.Resolve = func(name catalog.RunnerName) (catalog.Runner, error) { + r, err := execshell.NativeShell(string(name), root) + if err != nil { + return nil, fmt.Errorf("%w: %v", catalog.ErrUnknownRunner, err) + } + return r, nil + } + eng := catalog.NewEngine(cat, runner, catalog.WithRunners(runners)) switch mode { case modeList: @@ -95,6 +121,27 @@ func (a *App) Run(args []string) error { } } +// requireEngineVersion enforces engine.version against the running binary. +// +// A catalog using something a older godo does not have should say so once, +// clearly, instead of failing later in whatever way that feature happens to +// break. A -dev build compares by its numeric part, so working on godo itself +// is not blocked by its own catalog. +func requireEngineVersion(cat *catalog.Catalog, binary string) error { + want, ok := cat.Engine.Minimum() + if !ok { + return nil + } + older, err := update.Newer(binary, want) + if err != nil { + return fmt.Errorf("engine.version: %w", err) + } + if older { + return fmt.Errorf("%s needs godo %s or newer; this is %s (godo -e update)", cat.Path, want, binary) + } + return nil +} + func (a *App) runUpdate(checkOnly bool) error { client := a.UpdateClient if client == nil { @@ -140,6 +187,84 @@ func (a *App) runUpdate(checkOnly bool) error { return nil } +// listRunners prints what a catalog may put in runner: / # @runner here. +// +// "here" is the point: the native shells are whatever this machine has, so the +// list is a fact about the machine, not about godo. +// nearestCatalog loads the catalog for -e runners, or nil. +// +// The listing is useful outside a repo, so a missing or broken catalog is not +// an error here — it only means there is nothing extra to say about plugins. +func (a *App) nearestCatalog() *catalog.Catalog { + cwd, err := a.cwd() + if err != nil { + return nil + } + path, err := catalog.FindFile(cwd) + if err != nil { + return nil + } + cat, err := catalog.LoadFile(path) + if err != nil { + return nil + } + return cat +} + +func (a *App) listRunners(cat *catalog.Catalog) error { + fmt.Fprintf(a.Stdout, "%-10s %s [default]\n", string(catalog.RunnerInherit), execshell.DetectShell()) + fmt.Fprintln(a.Stdout) + fmt.Fprintln(a.Stdout, "shells found here:") + found := false + for _, name := range execshell.KnownShells() { + path, err := exec.LookPath(name) + if err != nil { + continue + } + found = true + fmt.Fprintf(a.Stdout, " %-10s %s\n", name, path) + } + if !found { + fmt.Fprintln(a.Stdout, " (none)") + } + if cat != nil { + a.printDeclaredPlugins(cat) + } + a.printShellCheck() + return nil +} + +// printDeclaredPlugins names runners a catalog expects from plugins, so the +// listing does not read as though those names simply do not exist. +func (a *App) printDeclaredPlugins(cat *catalog.Catalog) { + if len(cat.Engine.Plugins) == 0 { + return + } + fmt.Fprintln(a.Stdout, "\ndeclared by this catalog (no plugin loader in this build):") + for _, p := range cat.Engine.Plugins { + fmt.Fprintf(a.Stdout, " %-10s %s\n", strings.Join(p.Provides, " "), p.Source) + } +} + +// printShellCheck tells the reader how to confirm which shell they are in. +// +// godo answers that from the parent process, which no command run inside a +// shell can report — running one would only describe the shell godo just +// started. So when the detected shell looks wrong, the check has to happen in +// the reader's own terminal, and this says how. +func (a *App) printShellCheck() { + fmt.Fprintln(a.Stdout, "\nNot the shell you expected? Run this in your terminal:") + if runtime.GOOS == "windows" { + fmt.Fprintln(a.Stdout, " PowerShell $PSVersionTable.PSVersion") + fmt.Fprintln(a.Stdout, " cmd echo %COMSPEC%") + fmt.Fprintln(a.Stdout, "\nThen: set GODO_SHELL=C:\\path\\to\\shell.exe") + return + } + fmt.Fprintln(a.Stdout, " echo $0 (sh, bash, zsh, dash, ksh)") + fmt.Fprintln(a.Stdout, " echo $version (fish)") + fmt.Fprintln(a.Stdout, "\nThen: GODO_SHELL=/path/to/shell") +} + func (a *App) list(eng *catalog.Engine, tokens []string) error { if len(tokens) == 0 { for _, s := range eng.ListAll() { @@ -162,6 +287,9 @@ func (a *App) list(eng *catalog.Engine, tokens []string) error { if s.Dialect != "" { fmt.Fprintf(a.Stdout, "@dialect %s\n", s.Dialect) } + if s.Runner != "" { + fmt.Fprintf(a.Stdout, "@runner %s\n", s.Runner) + } for _, d := range s.Deps { fmt.Fprintf(a.Stdout, "@deps %s\n", d) } @@ -190,6 +318,7 @@ const ( modePreview modeUpdate modeUpdateCheck + modeRunners ) // ParseFlags parses context flags before script tokens. @@ -241,6 +370,11 @@ func parseEngineCommand(tokens []string) (mode mode, rest []string, err error) { return 0, nil, fmt.Errorf("-e version: unexpected arguments %v", tokens[1:]) } return modeVersion, nil, nil + case "runners": + if len(tokens) > 1 { + return 0, nil, fmt.Errorf("-e runners: unexpected arguments %v", tokens[1:]) + } + return modeRunners, nil, nil case "update": switch { case len(tokens) == 1: @@ -276,10 +410,12 @@ Examples: godo test # script "test" from godo.yaml godo update # script "update" if defined godo -e version # binary version + godo -e runners # runners usable on this machine godo -e update # self-update from GitHub Releases godo -e update check # check only File: godo.yaml (walk-up). Dialects: package | matcher. +Runners: inherit (default) | a shell by name (see -e runners). Env: GODO_RELEASES_API overrides GitHub API base for update.`) } @@ -289,6 +425,7 @@ func EngineUsage(w io.Writer) { Built-in commands (not godo.yaml scripts): version print binary version + runners list runners usable here update download latest GitHub Release for this OS/arch update check report whether an update is available help this help diff --git a/internal/cli/e2e_contract_test.go b/internal/cli/e2e_contract_test.go index 77bd61e..a6ba2b2 100644 --- a/internal/cli/e2e_contract_test.go +++ b/internal/cli/e2e_contract_test.go @@ -2,8 +2,10 @@ package cli_test import ( "bytes" + "errors" "os" "path/filepath" + "runtime" "strings" "testing" @@ -120,3 +122,165 @@ scripts: t.Fatal("expected expand error") } } + +// Contract: "runner" — a catalog may name a native shell directly, and godo +// proxies to whatever is on PATH instead of managing a list of them. +func TestE2E_nativeShellRunnerByName(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("posix shells") + } + cwd := t.TempDir() + // BASH_VERSION only exists in bash, so the marker proves which shell ran. + writeGodoYAML(t, cwd, "version: \"0.1\"\nscripts:\n"+ + " # @runner bash\n"+ + " t: printf %s \"${BASH_VERSION:+is-bash}\" > marker\n") + app, _, _ := e2eApp(t, cwd) + // The injected runner covers "shell"; a named shell must not reach it. + app.Runner = runnerFunc(func(string) error { return errors.New("shell runner must not be used") }) + if err := app.Run([]string{"t"}); err != nil { + t.Fatalf("run: %v", err) + } + got, err := os.ReadFile(filepath.Join(cwd, "marker")) + if err != nil { + t.Fatal(err) + } + if string(got) != "is-bash" { + t.Fatalf("marker=%q — bash did not run the line", got) + } +} + +func TestE2E_unknownShellNameIsRefused(t *testing.T) { + cwd := t.TempDir() + writeGodoYAML(t, cwd, "version: \"0.1\"\nscripts:\n # @runner git\n t: echo hi\n") + app, _, _ := e2eApp(t, cwd) + err := app.Run([]string{"t"}) + if err == nil || !strings.Contains(err.Error(), "not a known shell") { + t.Fatalf("err=%v", err) + } +} + +func TestE2E_engineRunnersLists(t *testing.T) { + app, out, _ := e2eApp(t, t.TempDir()) + if err := app.Run([]string{"-e", "runners"}); err != nil { + t.Fatal(err) + } + got := out.String() + for _, want := range []string{"inherit", "[default]", "shells found here", "GODO_SHELL", "Not the shell you expected"} { + if !strings.Contains(got, want) { + t.Fatalf("missing %q in:\n%s", want, got) + } + } +} + +func TestE2E_engineRunnersRejectsArguments(t *testing.T) { + app, _, _ := e2eApp(t, t.TempDir()) + if err := app.Run([]string{"-e", "runners", "extra"}); err == nil { + t.Fatal("want error") + } +} + +// --ls shows the runner a script asked for, next to its dialect. +func TestE2E_lsShowsRunner(t *testing.T) { + cwd := t.TempDir() + writeGodoYAML(t, cwd, "version: \"0.1\"\nscripts:\n # @runner inherit\n t: echo hi\n") + app, out, _ := e2eApp(t, cwd) + if err := app.Run([]string{"--ls", "t"}); err != nil { + t.Fatal(err) + } + if !strings.Contains(out.String(), "@runner inherit") { + t.Fatalf("ls=%q", out.String()) + } +} + +// Contract: a catalog that names no runner runs under the shell you are in. +func TestE2E_defaultRunnerIsTheCallersShell(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("posix shells") + } + if _, err := os.Stat("/bin/zsh"); err != nil { + t.Skip("no zsh on this machine") + } + t.Setenv("GODO_SHELL", "/bin/zsh") + cwd := t.TempDir() + // $0 is the shell running the line, and every POSIX shell reports it. + // (Arrays would be a sharper example but dash has none, and /bin/sh is + // dash on Debian and bash on macOS.) + writeGodoYAML(t, cwd, "version: \"0.1\"\nscripts:\n"+ + " t: \"printf %s \\\"$0\\\" > marker\"\n") + app := cli.New() + app.Getwd = func() (string, error) { return cwd, nil } + if err := app.Run([]string{"t"}); err != nil { + t.Fatal(err) + } + got, err := os.ReadFile(filepath.Join(cwd, "marker")) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(got), "zsh") { + t.Fatalf("marker=%q — default did not use the caller's shell", got) + } +} + +// Naming a shell pins it, whatever shell the caller is in. +func TestE2E_namedShellPinsItRegardlessOfCaller(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("posix shells") + } + t.Setenv("GODO_SHELL", "/bin/zsh") + cwd := t.TempDir() + writeGodoYAML(t, cwd, "version: \"0.1\"\nengine:\n runner: sh\nscripts:\n"+ + " t: \"printf %s \\\"$0\\\" > marker\"\n") + app := cli.New() + app.Getwd = func() (string, error) { return cwd, nil } + if err := app.Run([]string{"t"}); err != nil { + t.Fatal(err) + } + got, err := os.ReadFile(filepath.Join(cwd, "marker")) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(got), "zsh") { + t.Fatalf("marker=%q — runner: sh did not pin sh", got) + } +} + +// Contract: "engine" — a catalog that needs a newer binary says so once, +// clearly, instead of failing later in whatever way the missing feature breaks. +func TestE2E_engineVersionGate(t *testing.T) { + cwd := t.TempDir() + writeGodoYAML(t, cwd, "version: \"0.1\"\nengine:\n version: \">=99.0.0\"\nscripts:\n t: echo hi\n") + app, _, _ := e2eApp(t, cwd) + err := app.Run([]string{"t"}) + if err == nil || !strings.Contains(err.Error(), "needs godo 99.0.0 or newer") { + t.Fatalf("err=%v", err) + } +} + +func TestE2E_engineVersionSatisfied(t *testing.T) { + cwd := t.TempDir() + writeGodoYAML(t, cwd, "version: \"0.1\"\nengine:\n version: \">=0.0.1\"\nscripts:\n t: echo hi\n") + app, _, _ := e2eApp(t, cwd) + if err := app.Run([]string{"t"}); err != nil { + t.Fatalf("err=%v", err) + } +} + +// -e runners names what the catalog expects from plugins, so those runners do +// not read as though they simply did not exist. +func TestE2E_runnersListsDeclaredPlugins(t *testing.T) { + cwd := t.TempDir() + writeGodoYAML(t, cwd, "version: \"0.1\"\nengine:\n plugins:\n"+ + " - source: https://example.test/godo-micropy@v1\n"+ + " sha256: deadbeef\n"+ + " provides: [runner:micropy]\n"+ + "scripts:\n t: echo hi\n") + app, out, _ := e2eApp(t, cwd) + if err := app.Run([]string{"-e", "runners"}); err != nil { + t.Fatal(err) + } + for _, want := range []string{"declared by this catalog", "runner:micropy", "godo-micropy"} { + if !strings.Contains(out.String(), want) { + t.Fatalf("missing %q in:\n%s", want, out.String()) + } + } +} diff --git a/internal/execshell/inherit.go b/internal/execshell/inherit.go new file mode 100644 index 0000000..feb4ec5 --- /dev/null +++ b/internal/execshell/inherit.go @@ -0,0 +1,176 @@ +package execshell + +import ( + "fmt" + "os" + "os/exec" + "runtime" + "strings" + + "github.com/my-rv/godo/internal/catalog" +) + +// InheritRunner runs command lines through the shell the caller is already in. +// +// godo used to always use sh -c / cmd /C, which meant a zsh, fish or PowerShell +// user ran their catalog under a shell they did not choose — and the divergence +// was silent, not an error: "arr=(a b c); echo ${arr[1]}" prints a in zsh and b +// in sh, because zsh indexes arrays from 1. +// +// This does not make anything portable and is not meant to: if you are in +// PowerShell you write PowerShell. It makes godo honest about who runs the line. +// +// What it does not give you is your shell's *configuration*. Like sh -c, the +// shell is started non-interactively and does not read your rc file, so your +// aliases and functions are not there. It is your shell's grammar, not your +// shell's setup. +type InheritRunner struct { + Dir string + Stdout *os.File + Stderr *os.File + Stdin *os.File + + // Shell overrides detection. Empty → detect (see DetectShell). + Shell string +} + +// Run implements catalog.Runner. +func (r InheritRunner) Run(command string) error { + shell := r.Shell + if shell == "" { + shell = DetectShell() + } + name, args := shellCommand(shell, command) + cmd := exec.Command(name, args...) + if r.Dir != "" { + cmd.Dir = r.Dir + } + // Avoid typed-nil *os.File in io.Writer: interface != nil but Write fails. + if r.Stdout != nil { + cmd.Stdout = r.Stdout + } else { + cmd.Stdout = os.Stdout + } + if r.Stderr != nil { + cmd.Stderr = r.Stderr + } else { + cmd.Stderr = os.Stderr + } + if r.Stdin != nil { + cmd.Stdin = r.Stdin + } else { + cmd.Stdin = os.Stdin + } + err := cmd.Run() + if err == nil { + return nil + } + if ee, ok := err.(*exec.ExitError); ok { + return &catalog.ExitError{Code: ee.ExitCode(), Message: fmt.Sprintf("command failed: %s", command)} + } + return err +} + +// DetectShell returns the shell to run a catalog line with. +// +// GODO_SHELL wins, always. Otherwise $SHELL on Unix; on Windows, the parent +// process when it is a shell, else %ComSpec%. +// +// Windows gets the parent process rather than an environment variable because +// the environment cannot answer: PowerShell sets PSModulePath and everything it +// starts inherits it, so a cmd.exe opened from PowerShell looks like +// PowerShell. A parent that is not a shell — a build tool, an editor, CI — +// falls through to %ComSpec%, which is also what a user who disagrees with any +// of this overrides with GODO_SHELL. +// +// $SHELL on Unix is the login shell, not necessarily the one running right +// 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 { + if s := strings.TrimSpace(os.Getenv("GODO_SHELL")); s != "" { + return s + } + if runtime.GOOS == "windows" { + if s := callerShell(); s != "" { + return s + } + if c := strings.TrimSpace(os.Getenv("ComSpec")); c != "" { + return c + } + return "cmd.exe" + } + if s := strings.TrimSpace(os.Getenv("SHELL")); s != "" { + return s + } + return "/bin/sh" +} + +// shellBase is the shell's name, lowercased and without a .exe suffix. +// +// filepath.Base is separator-aware per platform, and this has to recognise a +// Windows path even when the detection runs somewhere else (a test, a catalog +// pinning GODO_SHELL), so it splits on both separators itself. +func shellBase(shell string) string { + s := shell + if i := strings.LastIndexAny(s, `/\`); i >= 0 { + s = s[i+1:] + } + return strings.TrimSuffix(strings.ToLower(s), ".exe") +} + +// shellCommand returns the process and arguments that make shell run one +// command line, non-interactively and without reading a profile. +func shellCommand(shell, command string) (string, []string) { + switch shellBase(shell) { + case "cmd", "command": + return shell, []string{"/C", command} + case "powershell", "pwsh": + // -NoProfile matches what -c does on Unix: the rc file is not read. + return shell, []string{"-NoProfile", "-Command", command} + default: + // sh, bash, zsh, dash, ksh, fish, and anything else that follows the + // convention. -c is close to universal; a shell that does not take it + // is the case GODO_SHELL exists for. + return shell, []string{"-c", command} + } +} + +// KnownShells are the shell names a catalog may name directly with +// "# @runner ". +// +// It is a list rather than "anything on PATH" because a runner name is not an +// arbitrary command: "# @runner git" would otherwise run "git -c " and +// fail in a way nobody could read. A shell that is not here is reachable with +// GODO_SHELL and the default runner. +func KnownShells() []string { + return []string{ + "sh", "bash", "zsh", "dash", "ksh", "ash", "fish", "nu", + "cmd", "pwsh", "powershell", + } +} + +// NativeShell returns a runner for a shell named directly by a catalog. +// +// The name is logical, never a path: a catalog says "cmd" or "pwsh", not +// "cmd.exe" or a drive letter, so the same godo.yaml reads the same on every +// machine. The extension, if the platform wants one, is PATH's business. +func NativeShell(name, dir string) (InheritRunner, error) { + if !isKnownShell(name) { + return InheritRunner{}, fmt.Errorf("%q is not a known shell (%s); for another one set GODO_SHELL", + name, strings.Join(KnownShells(), ", ")) + } + path, err := exec.LookPath(name) + if err != nil { + return InheritRunner{}, fmt.Errorf("shell %q is not installed here", name) + } + return InheritRunner{Dir: dir, Shell: path}, nil +} + +func isKnownShell(name string) bool { + for _, s := range KnownShells() { + if s == name { + return true + } + } + return false +} diff --git a/internal/execshell/inherit_test.go b/internal/execshell/inherit_test.go new file mode 100644 index 0000000..5960e79 --- /dev/null +++ b/internal/execshell/inherit_test.go @@ -0,0 +1,140 @@ +package execshell + +import ( + "bytes" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/my-rv/godo/internal/catalog" +) + +func TestShellCommand_flagsPerShellFamily(t *testing.T) { + cases := []struct { + shell string + want []string + }{ + {"/bin/sh", []string{"-c"}}, + {"/bin/bash", []string{"-c"}}, + {"/bin/zsh", []string{"-c"}}, + {"/usr/local/bin/fish", []string{"-c"}}, + {"/opt/weird/myshell", []string{"-c"}}, + {`C:\Windows\System32\cmd.exe`, []string{"/C"}}, + {"cmd", []string{"/C"}}, + {"powershell.exe", []string{"-NoProfile", "-Command"}}, + {"pwsh", []string{"-NoProfile", "-Command"}}, + {`C:\Program Files\PowerShell\7\pwsh.exe`, []string{"-NoProfile", "-Command"}}, + } + for _, tc := range cases { + name, args := shellCommand(tc.shell, "echo hi") + if name != tc.shell { + t.Fatalf("%s: name=%q", tc.shell, name) + } + if len(args) != len(tc.want)+1 { + t.Fatalf("%s: args=%q", tc.shell, args) + } + for i, w := range tc.want { + if args[i] != w { + t.Fatalf("%s: args=%q want prefix %q", tc.shell, args, tc.want) + } + } + if args[len(args)-1] != "echo hi" { + t.Fatalf("%s: command not last: %q", tc.shell, args) + } + } +} + +// Detection is a heuristic; GODO_SHELL is the documented way out of it. +func TestDetectShell_godoShellWins(t *testing.T) { + t.Setenv("GODO_SHELL", "/opt/mine/sh") + t.Setenv("SHELL", "/bin/zsh") + if got := DetectShell(); got != "/opt/mine/sh" { + t.Fatalf("got %q", got) + } +} + +func TestDetectShell_usesSHELLOnUnix(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("unix branch") + } + t.Setenv("GODO_SHELL", "") + t.Setenv("SHELL", "/bin/zsh") + if got := DetectShell(); got != "/bin/zsh" { + t.Fatalf("got %q", got) + } + t.Setenv("SHELL", "") + if got := DetectShell(); got != "/bin/sh" { + t.Fatalf("fallback: got %q", got) + } +} + +// The point of the runner, as a test: the same line means different things in +// different shells, and godo must stop picking for you. +func TestInheritRunner_runsUnderTheCallersShell(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("posix shells") + } + if _, err := os.Stat("/bin/zsh"); err != nil { + t.Skip("no zsh on this machine") + } + // $0 is the shell running the line. Arrays would be a sharper example but + // dash has none, and /bin/sh is dash on Debian and bash on macOS. + const line = `printf %s "$0"` + + got := captureRun(t, InheritRunner{Shell: "/bin/zsh"}, line) + if !strings.Contains(got, "zsh") { + t.Fatalf("zsh ran the line but $0 was %q", got) + } + got = captureRun(t, InheritRunner{Shell: "/bin/sh"}, line) + if strings.Contains(got, "zsh") { + t.Fatalf("sh ran the line but $0 was %q", got) + } +} + +func TestInheritRunner_propagatesTheChildExitCode(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("posix shells") + } + err := InheritRunner{Shell: "/bin/sh"}.Run("exit 42") + if err == nil { + t.Fatal("want failure") + } + if got := catalog.ExitCode(err); got != 42 { + t.Fatalf("exit=%d (%v)", got, err) + } +} + +func TestInheritRunner_runsInTheCatalogDirectory(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("posix shells") + } + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "marker"), nil, 0o644); err != nil { + t.Fatal(err) + } + got := captureRun(t, InheritRunner{Dir: dir, Shell: "/bin/sh"}, "ls marker") + if got != "marker" { + t.Fatalf("got %q", got) + } +} + +// captureRun runs one line and returns its trimmed stdout. +func captureRun(t *testing.T, r InheritRunner, line string) string { + t.Helper() + f, err := os.CreateTemp(t.TempDir(), "out") + if err != nil { + t.Fatal(err) + } + defer f.Close() + r.Stdout = f + if err := r.Run(line); err != nil { + t.Fatalf("run %q: %v", line, err) + } + b, err := os.ReadFile(f.Name()) + if err != nil { + t.Fatal(err) + } + return strings.TrimSpace(string(bytes.TrimSpace(b))) +} diff --git a/internal/execshell/parent_other.go b/internal/execshell/parent_other.go new file mode 100644 index 0000000..a8b0426 --- /dev/null +++ b/internal/execshell/parent_other.go @@ -0,0 +1,7 @@ +//go:build !windows + +package execshell + +// callerShell is unused off Windows: $SHELL answers the question there, and +// reading the parent process would need a different syscall on every Unix. +func callerShell() string { return "" } diff --git a/internal/execshell/parent_windows.go b/internal/execshell/parent_windows.go new file mode 100644 index 0000000..43c8457 --- /dev/null +++ b/internal/execshell/parent_windows.go @@ -0,0 +1,52 @@ +//go:build windows + +package execshell + +import ( + "os" + "syscall" + "unsafe" +) + +// callerShell returns the shell that started this process, 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. +// +// 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%. +func callerShell() string { + name, err := processName(uint32(os.Getppid())) + if err != nil || name == "" { + return "" + } + if !isKnownShell(shellBase(name)) { + return "" + } + return name +} + +// processName returns the executable name of pid via the process snapshot. +func processName(pid uint32) (string, error) { + snap, err := syscall.CreateToolhelp32Snapshot(syscall.TH32CS_SNAPPROCESS, 0) + if err != nil { + return "", 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 + } + for { + if e.ProcessID == pid { + return syscall.UTF16ToString(e.ExeFile[:]), nil + } + if err := syscall.Process32Next(snap, &e); err != nil { + return "", err + } + } +} diff --git a/internal/execshell/runner.go b/internal/execshell/runner.go deleted file mode 100644 index 07009aa..0000000 --- a/internal/execshell/runner.go +++ /dev/null @@ -1,56 +0,0 @@ -package execshell - -import ( - "fmt" - "os" - "os/exec" - "runtime" - - "github.com/my-rv/godo/internal/catalog" -) - -// Runner runs command lines via the system shell. -type Runner struct { - Dir string - Stdout *os.File - Stderr *os.File - Stdin *os.File -} - -// Run implements catalog.Runner. -func (r Runner) Run(command string) error { - var cmd *exec.Cmd - if runtime.GOOS == "windows" { - cmd = exec.Command("cmd", "/C", command) - } else { - cmd = exec.Command("sh", "-c", command) - } - if r.Dir != "" { - cmd.Dir = r.Dir - } - // Avoid typed-nil *os.File in io.Writer: interface != nil but Write panics/fails. - if r.Stdout != nil { - cmd.Stdout = r.Stdout - } else { - cmd.Stdout = os.Stdout - } - if r.Stderr != nil { - cmd.Stderr = r.Stderr - } else { - cmd.Stderr = os.Stderr - } - if r.Stdin != nil { - cmd.Stdin = r.Stdin - } else { - cmd.Stdin = os.Stdin - } - err := cmd.Run() - if err == nil { - return nil - } - if ee, ok := err.(*exec.ExitError); ok { - code := ee.ExitCode() - return &catalog.ExitError{Code: code, Message: fmt.Sprintf("command failed: %s", command)} - } - return err -} diff --git a/internal/execshell/runner_test.go b/internal/execshell/runner_test.go deleted file mode 100644 index 4f879db..0000000 --- a/internal/execshell/runner_test.go +++ /dev/null @@ -1,51 +0,0 @@ -package execshell_test - -import ( - "errors" - "os" - "path/filepath" - "runtime" - "testing" - - "github.com/my-rv/godo/internal/catalog" - "github.com/my-rv/godo/internal/execshell" -) - -func TestRunner_exitCode(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("shell exit differs") - } - dir := t.TempDir() - r := execshell.Runner{Dir: dir, Stdout: os.Stdout, Stderr: os.Stderr} - err := r.Run("exit 42") - var ee *catalog.ExitError - if !errors.As(err, &ee) || ee.Code != 42 { - t.Fatalf("%v", err) - } -} - -func TestRunner_usesDir(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("pwd") - } - dir := t.TempDir() - marker := filepath.Join(dir, "marker") - r := execshell.Runner{Dir: dir} - if err := r.Run("touch marker"); err != nil { - t.Fatal(err) - } - if _, err := os.Stat(marker); err != nil { - t.Fatal(err) - } -} - -func TestRunner_zeroValueStdioEcho(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("sh") - } - // Mirrors CLI: Runner{Dir: ...} with nil File fields (typed-nil trap). - r := execshell.Runner{Dir: t.TempDir()} - if err := r.Run(`echo "Hello, World!"`); err != nil { - t.Fatal(err) - } -} diff --git a/pkg.go b/pkg.go index d008cc0..d4a649d 100644 --- a/pkg.go +++ b/pkg.go @@ -20,13 +20,18 @@ const FileName = catalog.FileName type ( DialectName = catalog.DialectName + RunnerName = catalog.RunnerName + RunnerRegistry = catalog.RunnerRegistry Script = catalog.Script Catalog = catalog.Catalog + EngineSpec = catalog.EngineSpec + Plugin = catalog.Plugin Match = catalog.Match ExitError = catalog.ExitError Dialect = catalog.Dialect DialectRegistry = catalog.DialectRegistry Runner = catalog.Runner + ArgsAwareRunner = catalog.ArgsAwareRunner Engine = catalog.Engine EngineOption = catalog.EngineOption Plan = catalog.Plan @@ -36,6 +41,8 @@ type ( ) const ( + RunnerInherit = catalog.RunnerInherit + DialectPackage = catalog.DialectPackage DialectMatcher = catalog.DialectMatcher DialectNscript = catalog.DialectNscript @@ -49,6 +56,7 @@ var ( ErrDependencyCycle = catalog.ErrDependencyCycle ErrInvalidCatalog = catalog.ErrInvalidCatalog ErrInvalidCapture = catalog.ErrInvalidCapture + ErrUnknownRunner = catalog.ErrUnknownRunner ) var ( @@ -58,8 +66,12 @@ var ( NewDialectRegistry = catalog.NewDialectRegistry DefaultDialects = catalog.DefaultDialects EffectiveDialect = catalog.EffectiveDialect + NewRunnerRegistry = catalog.NewRunnerRegistry + DefaultRunners = catalog.DefaultRunners + EffectiveRunner = catalog.EffectiveRunner NewEngine = catalog.NewEngine WithDialects = catalog.WithDialects + WithRunners = catalog.WithRunners ExitCode = catalog.ExitCode IsCaptureName = catalog.IsCaptureName )