feat(lg): extend -strip to -w bundles - #800
Conversation
Self-reviewI found two path-handling issues that should be fixed before merging:
Focused validation passed for the root package, |
`-strip` split debug info out of `-c` and `-b` artifacts (#624) but rejected `-w`, so a wasm bundle had no way to ship without its source maps and local-variable tables. It now composes with `-w`. The companion is written beside the bundle directory, not inside it. Every file in the output directory is served; a debug sidecar is build output, not something to hand to each visitor. The output directory is canonicalized first, so `dist/`, `dist/.` and `.` all derive the same sibling instead of a dotfile inside the bundle. `-debug-output` overrides the path and is refused when it names the bundle directory or anything under it: an override such as `dist/index.html` would pass a same-path check and then replace a generated asset with binary debug data. The companion is written last, after the bundle succeeds, so a failed build leaves no sidecar claiming to describe an artifact that was never produced; these are digest-bound, and a stale one is worse than none. Sizes, so the scale is not oversold. On a real program bundle (xsofy's main.lg compiled to .lgb) stripping takes 709,129 B to 583,963 B, and the gzipped artifact from 232,854 B to 164,882 B: 17.7% raw but 29.2% compressed, because source maps and local-variable tables are structured numeric data that compresses worse than bytecode and strings. Against a whole wasm bundle the program is a small term: a three-line app shrank by 48 bytes on 8.1 MB, and xsofy's saving is roughly 1.5% of its bundle. Worth having and nearly free; the larger footprint argument belongs to the embedded core. The strip step lives in pkg/rt/wasm so its tests run without a wasm build. They cover the companion landing outside the served directory, every spelling of the directory deriving the same sibling, `-w .` resolving to the cwd's sibling, overrides inside the bundle (including the generated asset names) being rejected while siblings are allowed, and the stripped bundle plus companion still decoding together. Each was checked against a reintroduced bug. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
18d7db3 to
994e03b
Compare
|
Both addressed at
Verified end to end with the rebuilt Observed and left alone: an override whose parent directory does not exist ( |
Review summaryRan a multi-angle review (correctness, reuse/simplification/efficiency, altitude, conventions) plus hands-on repro of the wasm build path. Found two real path-handling bugs in the original
Both were independently caught in your self-review and fixed at 994e03b ( Left open, non-blocking:
The missing-parent-directory case you flagged yourself as "observed and left alone" checks out — same pre-existing convention as LGTM — approving. |
Second batch off [#28](#28). Every number was re-counted against `origin/main` at `36b13f79`, which changed two of them relative to the sweep that scheduled the work. ## `concepts/stack-vm.md` "The ~37 opcodes" is **47**, and not the 48 an enum count suggests: `OP_COUNT` is a sentinel (`// keep last; must equal len(opcodeNames)`), `len(opcodeNames)` is 47, `init()` panics if they disagree, and `lg -v` on 1.13.0 prints `opcodes: 47 (signature 7cf8f862cf5ba1e5)`. Three sources agreeing seemed worth the trouble, since this is the number the `.lgb` capability check compares. The roles list gained the unchecked trio ([#811](nooga/let-go#811)) and a bitwise bullet it had always omitted, which is eight opcodes the page never grouped. The `Frame` listing was seven fields stale, missing `argbuf`, `prepArgs`, `argc`, `constsc`, `debug`, `prevOp`, `profileOn` and, load-bearingly, `parent`. That last one is what [#645](nooga/let-go#645) hangs on, so the page's flat claim that "cross-frame propagation *does* unwind Go frames" is now qualified: a contiguous span of direct bytecode calls runs as child frames inside one dispatch loop, and `releaseFailedFrames` walks the `parent` chain offering the error to each suspended handler. It is still true once the call leaves bytecode through `ec.Invoke`, so the caveat is narrowed rather than dropped. The overflow paragraph gained the `*unchecked-math*` caveat ([#839](nooga/let-go#839)). ## `concepts/op-catalog.md` — the page was right "39 at `0911118`" was **correct**, and is kept as the prior value. Flagging it because the miscount is easy to repeat: a grep for op rows in `pkg/ir/ir_ops.lg` misses the `Invalid` row, which is written on a `'[[` line rather than a `["` line. Counting it gives 39 then and **42** now, the three new ones being #811's. ## `concepts/debug-info.md` and `concepts/lgb-bytecode-format.md` Both said "the `-w`/WASI paths are not stripped". [#800](nooga/let-go#800) made `-strip` compose with `-w`; `cli.go` now only refuses `-strip` when none of `-c`, `-b` or `-w` is given. `lgb-bytecode-format` also carried an illustrative "runtime has 44" pinned to `0911118`, now the real 47 and signature. Its capability section names #811 as the second deliberate opcode-set break after the one that added `OP_DIV`, which is the thing most likely to reach someone as an unexplained rejection after upgrading. ## `sources/let-go-source-code.md` "`/pkg` holds thirteen main packages" is **18**. The five newer ones are named rather than described, with [#773](nooga/let-go#773) called out: moving the CLI into `pkg/cli` is the one that changes an import for anyone building their own `lg`. `check_wiki.py` passes.
3 participants
What changes
-stripsplit debug info out of-cand-bartifacts in #624, but rejected-w. A wasm bundle therefore had no way to ship without its source maps and local-variable tables. This makes the flag compose with-w, closing half of #631.Where the companion goes
Beside the bundle directory, not inside it. Every file in the output directory is served; a debug sidecar is build output, not something to hand to each visitor.
-debug-outputoverrides the path.Three details that are easy to get wrong:
dist/,dist/., and.all name the directory itself. Without that, a trailing separator gives a companion named.debuginside the bundle, anddist/.givesdist/..debug.-debug-output dist/index.htmlthrough, and the build then replaces a generated asset with binary debug data while still reporting success. The same holds forcoi-serviceworker.jsand, in external mode,main.wasm, so the check is "inside the served directory" rather than a list of filenames.Sizes, not oversold
On a real program bundle, xsofy's
main.lgcompiled to.lgb:The compressed saving is the larger percentage because source maps and local-variable tables are structured numeric data that compresses worse than bytecode and strings, so removing them takes a disproportionate share of the shipped bytes.
Against a whole wasm bundle, though, the program is a small term. A three-line app shrank by 48 bytes on an 8.1 MB bundle, and xsofy's saving is roughly 1.5% of its bundle. This is worth having and nearly free, but the footprint argument in #631 rests on the embedded core and on
.lgbartifacts shipped directly, not on the wasm bundle. The embedded-core half is a separate change, the way #501 and #502 were split.Tests
The strip step lives in
pkg/rt/wasmso its tests run without a full wasm build, which would have put minutes and a module fetch into the suite. They cover: the companion landing outside the served directory; every spelling of the directory deriving the same sibling;-w .resolving to the current directory's sibling under its real name; overrides inside the bundle, including the generated asset names, being rejected while siblings and unrelated paths are allowed; a filesystem root being refused; and a stripped bundle plus its companion still decoding together.Each rejection was checked against the reintroduced bug: with canonicalization disabled the
-w .case fails, and with only the same-path check the inside-bundle case fails.gofmt,go vet, thepkg/rt/wasm,pkg/cli, andpkg/bytecodesuites,make check-generated, and cross-builds for linux/amd64, js/wasm, plan9/amd64, and wasip1/wasm are clean.