refactor(tower-package): extract pure core crate for bundle building#257
refactor(tower-package): extract pure core crate for bundle building#257
Conversation
Splits tower-package into a pure tower-package-core crate (no tokio, fs, or glob) and a native shell. The core exposes build_package(PackageInputs) which produces a gzipped tar from in-memory bytes, enabling future wasm32 targets. The native crate handles filesystem walking, globbing, and canonicalization, then delegates to the core. Output is now byte-deterministic: entries are sorted by archive name, tar header mtime/uid/gid are zeroed with mode 0644, and the gzip header drops its embedded mtime. The bundle format (ustar+gzip, app/modules layout, MANIFEST, Towerfile) and checksum algorithm are unchanged, so existing server-side unpacking continues to work.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
New crate exposing tower_package_core::build_package to TypeScript via wasm-bindgen. The crate produces an npm-publishable package with typed bindings (Uint8Array in, Uint8Array out). Output matches what the native tower-package produces, so bundles built in the browser or Node deploy through the server's existing unpack path without changes. The flake devshell now carries wasm-pack, wasm-bindgen-cli, and binaryen, and the rust toolchain picks up the wasm32-unknown-unknown target. Build the package with crates/tower-package-wasm/scripts/build.sh, optionally passing bundler (default), web, or nodejs.
Node-based test suite using tsx and node:test. Covers the output shape, byte-determinism, sort order, MANIFEST contents, module file flow, and checksum divergence on different inputs. Parses the tar stream inline so there are no npm deps beyond tsx and type declarations. The workflow builds the nodejs-target wasm package and runs the tests on every PR that touches tower-package-core, tower-package-wasm, or the workspace manifest.
bradhe
left a comment
There was a problem hiding this comment.
Looking good overall, let's actually push hard to get this out--will be a big benefit for folks. Two things to look at:
- Let's just combine the packages together, and split the implementations. No need to have a separate package, feels to me it just confuses things. We could use feature gates for this.
- Let's actually go all the way and add this to the release pipeline too, where we push
tower-package-wasmto NPM. We can then create a wrapper calledtowerthat pulls this in for convenient, but at least we can give people the WASM for now.
| tokio = { workspace = true } | ||
| tokio-stream = { workspace = true } | ||
| tokio-tar = { workspace = true } | ||
| tower-package-core = { workspace = true } |
There was a problem hiding this comment.
Why does this have to be in a separate package? We can keep this all in the same package.
There was a problem hiding this comment.
I thought it would be nice to be more flexible later, but not a must for sure - could either use feature flags (the cargo kind) or target_arch conditionals instead
1fcf6e8 to
99814dd
Compare
Folds tower-package-core and tower-package-wasm back into tower-package, using cargo features instead of separate crates to split the shells. Default features remain compatible with existing Rust callers: cargo build on a workspace member still gets tokio, glob, Package::build, and everything else it used to. - native feature (default): tokio, glob, tmpdir, Package, PackageSpec, FileResolver — the CLI's usual path. - wasm feature: wasm-bindgen + serde-wasm-bindgen + serde_bytes, exposes buildBundle to JavaScript. - Pure core (Entry, Manifest, build_package, sorting, hashing) is always compiled under both shells. Build the wasm package with crates/tower-package/scripts/build.sh, which invokes wasm-pack with --no-default-features --features wasm and renames the npm package to tower-package-wasm so the crate and npm names can diverge. Ten native tests and eight TypeScript tests pass.
99814dd to
987f9c8
Compare
Mirrors the publish-pypi subworkflow: on every release tag, cargo-dist's release.yml calls a new publish-npm.yml that builds the bundler-target wasm package and runs npm publish. Version tracks Cargo.toml, so the npm package stays in lockstep with the Rust crate. First cut publishes the wasm package only; the consumer brings their own bundler.
Drops NPM_TOKEN in favour of OIDC; no long-lived secret to rotate. Requires a trusted publisher to be configured on the package at npmjs.com matching this repo, this workflow filename, and the release environment. Bumps node to 22 and pulls npm@latest to ensure the CLI supports OIDC publishing.
bradhe
left a comment
There was a problem hiding this comment.
Another quick round of feedback. Biggest thing is the invoke, schedule, etc., are all duplicative of the Towerfile!
| return entries; | ||
| } | ||
|
|
||
| function minimalInputs(): BundleInputs { |
There was a problem hiding this comment.
Can we drop the name bundle? We call it package everywhere else--either that, or we need to go update our docs and everything else everywhere 😄 My mind immediately was wondering "what's the difference between a package and a bundle?"
| import { | ||
| buildBundle, | ||
| type BundleInputs, | ||
| } from "../pkg/tower_package.js"; |
There was a problem hiding this comment.
Another vote in the package direction: tower_package exposes a BundleInputs type?
| invoke: String, | ||
| parameters: Vec<Parameter>, | ||
| schedule: Option<String>, | ||
| import_paths: Vec<String>, |
There was a problem hiding this comment.
All these fields are in the Towerfile -- shouldn't we rely on that for them? Also schedule should get jettisoned entirely, heh.
Move Towerfile/App/Parameter from config into a new tower-package::towerfile module and flip the crate dependency — config now re-exports them so existing `use config::Towerfile` call sites keep working. core::build_package parses the bytes directly through Towerfile::from_toml, retiring the private TowerfileSpec shadow struct and unifying Parameter (description: String) across authoring and manifest views.
bradhe
left a comment
There was a problem hiding this comment.
Looks good, fix the build failure and let's ship it!
…e test Gate the Path import out — it's only needed inside save() under the native feature. Also delete test_manifest_contains_schedule; schedule was removed from the manifest in f625834.
Uh oh!
There was an error while loading. Please reload this page.