diff --git a/changes/cli-audit.md b/changes/cli-audit.md new file mode 100644 index 00000000..61c001af --- /dev/null +++ b/changes/cli-audit.md @@ -0,0 +1,24 @@ +bump: patch +type: fix +**`ludic new` scaffolded a project that would not compile.** The project name went +straight into the `program ` identifier, so `ludic new my-game` wrote +`program My-Game` — a subtraction — and the first `ludic run` failed with +`expected '{', got '-'`. A name is now turned into a valid identifier +(`my-game` → `MyGame`, `2048` → `Game2048`), and a name that cannot be a +directory or a package is refused with the rule rather than mangled. + +Found while auditing every command's flags and arguments, along with: + +- **Unknown options are errors.** `ludic build --headles` silently built a + windowed binary; `ludic build -o` with no path silently ignored it. Both now + say what is wrong and exit non-zero. +- **`ludic fmt` formats in place**, as its help always claimed — it was printing + the file to stdout and changing nothing. `ludic fmt --check` reports drift + without writing, for a hook or CI. +- **`ludic test nosuch.ludic`** says the file does not exist instead of passing it + to the compiler. +- **`ludic build-lib`** reported failures as a shell syntax error (an + interpolation written in a non-interpolating string), and its no-argument + auto-detection picked up `package.lock.ludic` as a module to compile. +- Error messages that still began with `x:` — the CLI's old name — now say + `ludic:`. diff --git a/tools/ludic-cli/main.ludic b/tools/ludic-cli/main.ludic index ef5ef63b..8436a0c6 100644 --- a/tools/ludic-cli/main.ludic +++ b/tools/ludic-cli/main.ludic @@ -51,7 +51,7 @@ program Ludic { print(" version print the toolchain version") print(" upgrade [version] reinstall from the docs site (the same script that installed it)") print(" doctor check that the install is complete and usable") - print(" fmt [paths...] format Ludic source in place") + print(" fmt [--check] [paths...] format Ludic source in place (--check: report, write nothing)") print(" lsp run the language server on stdio (what editors spawn)") print(" help this message") print("") diff --git a/tools/ludic-cli/pkg.ludic b/tools/ludic-cli/pkg.ludic index 74017c41..a0adf497 100644 --- a/tools/ludic-cli/pkg.ludic +++ b/tools/ludic-cli/pkg.ludic @@ -1,4 +1,5 @@ -# pkg.ludic — the Ludic package manager (issue #63), a set of `x` subcommands. +# pkg.ludic — the Ludic package manager (issue #63), the `ludic add`/`get`/ +# `update`/`verify`/`vendor` commands. # # It realises the v1 direction decided in the RFC: # @@ -223,7 +224,7 @@ function ensure_clone(module: pointer) -> pointer { } run(`mkdir -p {store_root()}cache`) if not shq(`git clone -q {repo_url(module)} {cache} 2>/dev/null`) { - err(`x: cannot fetch {module} (git clone {repo_url(module)} failed)\n`) + err(`ludic: cannot fetch {module} (git clone {repo_url(module)} failed)\n`) return "" } return cache @@ -262,7 +263,7 @@ function fetch_manifest(module: pointer, ver: pointer) -> Manifest { m.ver = ver if cache == "" { return m } if not checkout_ver(cache, ver) { - err(`x: {module} has no version {ver}\n`) + err(`ludic: {module} has no version {ver}\n`) return m } let txt = read_file(`{cache}/package.ludic`) @@ -445,7 +446,7 @@ function do_install(root: Manifest) -> int { return 0 } let clash = collision(sels) - if slen(clash) > 0 { err(`x: namespace collision — {clash}\n`); return 1 } + if slen(clash) > 0 { err(`ludic: namespace collision — {clash}\n`); return 1 } var i = 0 while i < len(sels) { @@ -453,18 +454,18 @@ function do_install(root: Manifest) -> int { if m.kind == "prebuilt" { let t = target_id() if not has_target(m, t) { - err(`x: {m.module}@{m.ver} is a prebuilt lib and ships no artifact for target {t}\n`) + err(`ludic: {m.module}@{m.ver} is a prebuilt lib and ships no artifact for target {t}\n`) return 1 } } let h = snapshot(m.module, m.ver) - if slen(h) == 0 { err(`x: failed to snapshot {m.module}@{m.ver}\n`); return 1 } + if slen(h) == 0 { err(`ludic: failed to snapshot {m.module}@{m.ver}\n`); return 1 } m.hash = `sha256:{h}` link_module(m.module, h) print(` {m.module} {m.ver} ({m.kind}) sha256:{sslice(h, 0, 12)}…`) i += 1 } - if not write_lock(sels) { err("x: cannot write package.lock.ludic\n"); return 1 } + if not write_lock(sels) { err("ludic: cannot write package.lock.ludic\n"); return 1 } print(`resolved {string(len(sels))} package(s) — see package.lock.ludic; linked under ludic_modules/`) return 0 } @@ -545,7 +546,7 @@ function cmd_pkg_add() -> int { var ver = spec[1] if slen(ver) == 0 { ver = latest_version(module) - if slen(ver) == 0 { err(`x: {module} has no published version tags (git tag vX.Y.Z to publish)\n`); return 1 } + if slen(ver) == 0 { err(`ludic: {module} has no published version tags (git tag vX.Y.Z to publish)\n`); return 1 } print(`ludic add: {module} -> latest v{ver}`) } set_require(module, ver) @@ -555,7 +556,7 @@ function cmd_pkg_add() -> int { # ludic get — resolve + fetch + link every dependency in package.ludic, write lock function cmd_pkg_get() -> int { let txt = read_file("package.ludic") - if txt == null { err("x: no package.ludic in the current directory (ludic add to start one)\n"); return 1 } + if txt == null { err("ludic: no package.ludic in the current directory (ludic add to start one)\n"); return 1 } print("resolving dependencies (MVS)…") return do_install(parse_manifest(txt)) } @@ -563,7 +564,7 @@ function cmd_pkg_get() -> int { # ludic update [module] — bump a dep (or all) to its latest published version, relock function cmd_pkg_update() -> int { let root = read_root_manifest() - if len(root.deps) == 0 { err("x: package.ludic declares no dependencies\n"); return 1 } + if len(root.deps) == 0 { err("ludic: package.ludic declares no dependencies\n"); return 1 } let only = argn(2, "") var i = 0 while i < len(root.deps) { @@ -584,7 +585,7 @@ function cmd_pkg_update() -> int { # confirm the project view links to it function cmd_pkg_verify() -> int { let txt = read_file("package.lock.ludic") - if txt == null { err("x: no package.lock.ludic (run ludic get first)\n"); return 1 } + if txt == null { err("ludic: no package.lock.ludic (run ludic get first)\n"); return 1 } let locked = parse_lock(txt) if len(locked) == 0 { print("lockfile lists no packages"); return 0 } var bad_count = 0 @@ -611,7 +612,7 @@ function cmd_pkg_verify() -> int { i += 1 } if bad_count == 0 { print(`verified {string(len(locked))} package(s) against the store`); return 0 } - err(`x: {string(bad_count)} package(s) failed verification\n`) + err(`ludic: {string(bad_count)} package(s) failed verification\n`) return 1 } @@ -619,7 +620,7 @@ function cmd_pkg_verify() -> int { # builds. Build against them with LUDIC_MODULES=vendor. function cmd_pkg_vendor() -> int { let txt = read_file("package.lock.ludic") - if txt == null { err("x: no package.lock.ludic (run ludic get first)\n"); return 1 } + if txt == null { err("ludic: no package.lock.ludic (run ludic get first)\n"); return 1 } let locked = parse_lock(txt) run("rm -rf vendor") var i = 0 @@ -627,7 +628,7 @@ function cmd_pkg_vendor() -> int { let m = locked[i] let raw = strip_prefix(m.hash, "sha256:") let dest = `{store_root()}{raw}` - if not file_exists(dest) { err(`x: {m.module}@{m.ver} not in the store — run ludic get\n`); return 1 } + if not file_exists(dest) { err(`ludic: {m.module}@{m.ver} not in the store — run ludic get\n`); return 1 } let vdir = `vendor/{m.module}` run(`mkdir -p "$(dirname {vdir})"`) run(`cp -R {dest} {vdir}`) @@ -647,7 +648,9 @@ function cmd_pkg_vendor() -> int { # with the flag set, from an installed toolchain. function cmd_pkg_build_lib() -> int { var src = argn(2, "") - if src == "" { src = capture_line("ls *.ludic 2>/dev/null | grep -v package.ludic | head -1") } + # the manifest and the lockfile are not modules: match their names exactly, + # rather than as a regex where `.` also matched package.lock.ludic + if src == "" { src = capture_line("ls *.ludic 2>/dev/null | grep -vxF -e package.ludic -e package.lock.ludic | head -1") } if src == "" or not file_exists(src) { err("usage: ludic build-lib (run in the package directory)\n"); return 1 } let man = read_root_manifest() var name = "" @@ -657,13 +660,16 @@ function cmd_pkg_build_lib() -> int { run(`mkdir -p lib/{t}`) let ludicc = getenv_or("LUDICC", "bin/ludicc") let ll = `{tmp_dir()}/buildlib.ll` - if not shq(`{ludicc} --emit-module {src} -o {ll} 2>{tmp_dir()}/bl.err`) { - err(`x: build-lib compile failed — {capture_line("tail -1 {tmp_dir()}/bl.err")}\n`); return 1 + let blerr = tmp_path("bl.err") + if not shq(`{ludicc} --emit-module {src} -o {ll} 2>{blerr}`) { + let why = capture_line(`tail -1 {blerr}`) + err(`ludic build-lib: compile failed — {why}\n`); return 1 } let out = `lib/{t}/lib{name}.dylib` # @rpath install name so a consumer resolves it via -rpath to the store dir - if not shq(`{cc()} -O2 -Wno-override-module -dynamiclib -undefined dynamic_lookup -Wl,-install_name,@rpath/lib{name}.dylib {ll} -o {out} 2>{tmp_dir()}/bl.err`) { - err(`x: build-lib link failed — {capture_line("tail -1 {tmp_dir()}/bl.err")}\n`); return 1 + if not shq(`{cc()} -O2 -Wno-override-module -dynamiclib -undefined dynamic_lookup -Wl,-install_name,@rpath/lib{name}.dylib {ll} -o {out} 2>{blerr}`) { + let why2 = capture_line(`tail -1 {blerr}`) + err(`ludic build-lib: link failed — {why2}\n`); return 1 } run(`rm -f {ll}`) print(`built {out} (target {t})`) diff --git a/tools/ludic-cli/project.ludic b/tools/ludic-cli/project.ludic index c30742f1..620879c9 100644 --- a/tools/ludic-cli/project.ludic +++ b/tools/ludic-cli/project.ludic @@ -55,6 +55,66 @@ function no_entry() -> int { return 1 } +# ---- ludic new --------------------------------------------------------------- + +# A project name is a directory name and a package identifier, so it is held to +# what both accept: letters, digits, '-', '_' and '.'. Anything else — a space, a +# slash, a quote — either breaks the shell commands that create the tree or +# produces a manifest nobody can depend on, and failing here with the rule beats +# failing later with something obscure. +function valid_project_name(name: pointer) -> bool { + let n = slen(name) + if n == 0 { return false } + var i = 0 + while i < n { + let c = name[i] + let alnum = (c >= 'a' and c <= 'z') or (c >= 'A' and c <= 'Z') or (c >= '0' and c <= '9') + let punct = c == '-' or c == '_' or c == '.' + if not (alnum or punct) { return false } + i += 1 + } + return true +} + +# The `program ` identifier for a project called `name`. +# +# The name is a directory name and the identifier is Ludic source, and they do +# not accept the same characters: `ludic new my-game` wrote `program My-Game`, +# which is a subtraction, and `ludic new 2048` wrote an identifier starting with +# a digit. Both scaffolded a project that would not compile — the first thing the +# user did with it. So: split on anything that is not a letter or digit, +# capitalise each piece, and join. A leading digit gets a `Game` prefix, since an +# identifier cannot start with one. +function ident_of(name: pointer) -> pointer { + let n = slen(name) + let b = bytes(n + 8) + var out = 0 + var at_start = true + var i = 0 + while i < n { + var c = name[i] + let alpha = (c >= 'a' and c <= 'z') or (c >= 'A' and c <= 'Z') + let digit = c >= '0' and c <= '9' + if alpha or digit { + if at_start { + if c >= 'a' and c <= 'z' { c -= 32 } + at_start = false + } else { + if c >= 'A' and c <= 'Z' { c += 32 } + } + b[out] = c + out += 1 + } else { + at_start = true # the next letter starts a new word + } + i += 1 + } + b[out] = 0 + if out == 0 { return "Game" } + if b[0] >= '0' and b[0] <= '9' { return "Game" + sslice(b, 0, out) } + return sslice(b, 0, out) +} + # ---- ludic new -------------------------------------------------------------- # The smallest program worth running: a window, an entity moving under the ECS, @@ -136,6 +196,11 @@ function cmd_new() -> int { return 1 } let name = arg(2) + if not valid_project_name(name) { + err(`ludic new: '{name}' is not a usable project name\n`) + err(" use letters, digits, '-', '_' or '.' (it names a directory and a package)\n") + return 1 + } if file_exists(name) { err(`ludic new: {name} already exists\n`) return 1 @@ -145,8 +210,8 @@ function cmd_new() -> int { err(`ludic new: cannot write {name}/package.ludic\n`) return 1 } - write_file(`{name}/src/main.ludic`, template_main(title_case(name))) - write_file(`{name}/tests/smoke.ludic`, template_test(title_case(name))) + write_file(`{name}/src/main.ludic`, template_main(ident_of(name))) + write_file(`{name}/tests/smoke.ludic`, template_test(ident_of(name))) write_file(`{name}/.gitignore`, template_gitignore()) write_file(`{name}/README.md`, template_readme(name)) print(`created {name}/`) @@ -167,21 +232,38 @@ var g_mode: int = 1 # 1 = windowed, 2 = headless var g_out: pointer = "" # -o var g_save: bool = false # --save-temps +# g_argerr is set when the command line itself was wrong — an unknown flag, or +# -o with nothing after it. Silently ignoring those meant `ludic build --headles` +# quietly produced a windowed binary and `ludic build -o` quietly ignored the +# request, which is the kind of thing you only notice much later. +var g_argerr: bool = false + function parse_build_args(start: int) -> pointer { var src = "" g_mode = 1 g_out = "" g_save = false + g_argerr = false var ai = start while ai < arg_count() { let a = arg(ai) if a == "--headless" { g_mode = 2 } else if a == "--windowed" { g_mode = 1 } else if a == "--save-temps" { g_save = true } - else if a == "-o" { ai += 1; if ai < arg_count() { g_out = arg(ai) } } - else if a[0] != '-' { src = a } + else if a == "-o" { + ai += 1 + if ai < arg_count() { g_out = arg(ai) } + else { err("ludic: -o needs a path\n"); g_argerr = true } + } + else if a[0] == '-' { + err(`ludic: unknown option {a}\n`) + err(" build/run take: [file] [--headless|--windowed] [-o out] [--save-temps]\n") + g_argerr = true + } + else { src = a } ai += 1 } + if g_argerr { return "" } return find_entry(src) } @@ -197,6 +279,7 @@ function output_path(entry: pointer) -> pointer { # ludic build [file] [--headless] [-o out] [--save-temps] function cmd_build() -> int { let entry = parse_build_args(2) + if g_argerr { return 1 } if entry == "" { return no_entry() } let out = output_path(entry) if not compile_app(entry, out, g_mode, g_save) { return 1 } @@ -212,6 +295,7 @@ function cmd_build() -> int { # so assets/ resolves relative to the game. function cmd_run() -> int { let entry = parse_build_args(2) + if g_argerr { return 1 } if entry == "" { return no_entry() } let out = output_path(entry) if not compile_app(entry, out, g_mode, g_save) { return 1 } @@ -221,6 +305,7 @@ function cmd_run() -> int { # `ludic mygame.ludic` — the file is argv[1], so the scan starts there. function cmd_run_file() -> int { let entry = parse_build_args(1) + if g_argerr { return 1 } if entry == "" { return no_entry() } let out = output_path(entry) if not compile_app(entry, out, g_mode, g_save) { return 1 } @@ -251,6 +336,11 @@ function cmd_test() -> int { err("ludic test: no tests found (expected tests/*.ludic or src/**/*_test.ludic)\n") return 1 } + var mi = 0 + while mi < len(files) { + if not file_exists(files[mi]) { err(`ludic test: no such file: {files[mi]}\n`); return 1 } + mi += 1 + } run("mkdir -p build") var failed = 0 var i = 0 @@ -293,12 +383,30 @@ function strip_ext(p: pointer) -> pointer { # ---- ludic fmt / lsp -------------------------------------------------------- -# ludic fmt [paths...] — the formatter over the project (src/ and tests/ by -# default), or over the paths named. +# ludic fmt [--check] [paths...] — format the project's source in place (src/ and +# tests/ by default), or the paths named. --check writes nothing and exits +# non-zero if anything is unformatted, which is what a pre-commit hook or CI +# wants. +# +# In place is the default because that is what `fmt` means everywhere else and +# what this command's own help promised; the underlying ludic-fmt defaults to +# printing to stdout, which as a project-level command would just scroll the file +# past you and change nothing. function cmd_fmt() -> int { + var mode = "-w" var args = "" var ai = 2 - while ai < arg_count() { args = `{args} {arg(ai)}`; ai += 1 } + while ai < arg_count() { + let a = arg(ai) + if a == "--check" { mode = "--check" } + else if a[0] == '-' { + err(`ludic fmt: unknown option {a}\n`) + err(" usage: ludic fmt [--check] [paths...]\n") + return 1 + } + else { args = `{args} {a}` } + ai += 1 + } if args == "" { let found = capture_line("find src tests -name '*.ludic' 2>/dev/null | sort") if found == "" { @@ -307,7 +415,7 @@ function cmd_fmt() -> int { } args = ` {found}` } - return sh(`{tool("ludic-fmt")}{args}`) + return sh(`{tool("ludic-fmt")} {mode}{args}`) } # ludic lsp — the language server on stdio. Editors are configured to run this, @@ -410,7 +518,9 @@ function cmd_upgrade() -> int { err("ludic upgrade: needs curl\n") return 1 } - print(`upgrading from {install_url()}`) + # stderr, not stdout: this line is progress, and stdout is block-buffered when + # piped, which would print it after the installer it introduces + err(`upgrading from {install_url()}\n`) if ver == "" { return sh(`curl -fsSL {install_url()} | sh`) } return sh(`curl -fsSL {install_url()} | sh -s -- --version {ver}`) } diff --git a/tools/ludic-cli/selfhost.ludic b/tools/ludic-cli/selfhost.ludic index d7f0d509..07057eda 100644 --- a/tools/ludic-cli/selfhost.ludic +++ b/tools/ludic-cli/selfhost.ludic @@ -97,7 +97,7 @@ function line_count(path: pointer) -> pointer { function cmd_selfhost_build(lc: pointer, outbin: pointer) -> int { run("mkdir -p build") let src = "build/selfhost.ludic" - if not write_selfhost_src(src) { err("x: cannot write build/selfhost.ludic\n"); return 1 } + if not write_selfhost_src(src) { err("ludic-dev: cannot write build/selfhost.ludic\n"); return 1 } let ll = `{outbin}.ll` if not shq(`{lc} {src} > {ll} 2>/dev/null`) { run(`rm -f {ll}`); return 1 } let rc = shq(`{cc()} {ll} -o {outbin} 2>/dev/null`) diff --git a/tools/ludic-cli/test.ludic b/tools/ludic-cli/test.ludic index 7c957347..a1e17e83 100644 --- a/tools/ludic-cli/test.ludic +++ b/tools/ludic-cli/test.ludic @@ -104,6 +104,42 @@ function install_layout_case() -> void { ok(lbl) } +# `ludic new` must scaffold something that compiles, for any name a user might +# reasonably pick. `my-game` produced `program My-Game`, which is a subtraction, +# and `2048` produced an identifier starting with a digit — both scaffolded a +# project that failed on the very first `ludic run`. +function scaffold_names_case() -> void { + let lbl = "ludic new scaffolds a project that compiles (hyphens, digits, dots)" + let work = `{tmp_dir()}/scaffold` + run(`rm -rf {work} && mkdir -p {work}`) + let root = capture_line("pwd") + let names = new []pointer + push(names, "my-game") + push(names, "2048") + push(names, "a.b.c") + push(names, "UPPER_case") + var i = 0 + while i < len(names) { + let nm = names[i] + i += 1 + if not shq(`cd {work} && {root}/bin/ludic new {nm} > {work}/new.out 2>&1`) { + bad2(lbl, `ludic new {nm} failed`); return + } + if not shq(`cd {work}/{nm} && {root}/bin/ludic build --headless > {work}/build.out 2>&1`) { + let why = capture_line(`tail -1 {work}/build.out`) + bad2(lbl, `{nm}: {why}`); return + } + if not shq(`cd {work}/{nm} && {root}/bin/ludic test > {work}/test.out 2>&1`) { + bad2(lbl, `{nm}: tests failed`); return + } + } + # and a name that cannot be a directory or a package is refused, not mangled + if shq(`cd {work} && {root}/bin/ludic new 'two words' > {work}/bad.out 2>&1`) { + bad2(lbl, "a name with a space was accepted"); return + } + ok(lbl) +} + # a "does it still compile" smoke test (parse -> lower -> link), no run function qsmoke(path: pointer) -> void { let nm = flat(path) @@ -217,7 +253,7 @@ function cmd_test_coverage() -> int { # instrumentation path is exactly what a clean checkout ships. run(`mkdir -p bin build {tmp_dir()}/cov_out`) if (not is_exec("bin/ludicc")) or newer("selfhost/ludicc.seed.ll", "bin/ludicc") { - if not shq(`{cc()} selfhost/ludicc.seed.ll -o bin/ludicc`) { err("x: cannot build bin/ludicc from the seed\n"); return 1 } + if not shq(`{cc()} selfhost/ludicc.seed.ll -o bin/ludicc`) { err("ludic-dev: cannot build bin/ludicc from the seed\n"); return 1 } } COV_COVERED = 0 COV_TOTAL = 0 @@ -453,6 +489,7 @@ function cmd_dev_test() -> int { # ./bin, finding its compiler, the engine runtime and the bundled packages # from its own location. This is the shape `curl … | sh` produces. install_layout_case() + scaffold_names_case() # install.sh is what the landing page tells people to pipe into sh, and it is # published with the docs site — so it is checked here rather than discovered