Skip to content

fix(dev) [BRNS-DESK-016]: the documented setup works on a clean clone - #61

Open
Chris-ssvlabs wants to merge 2 commits into
mainfrom
fix/desk-016-clean-clone-setup
Open

fix(dev) [BRNS-DESK-016]: the documented setup works on a clean clone#61
Chris-ssvlabs wants to merge 2 commits into
mainfrom
fix/desk-016-clean-clone-setup

Conversation

@Chris-ssvlabs

Copy link
Copy Markdown
Contributor

Summary

  • npm install && npm run tauri dev — what README and CONTRIBUTING both document — failed four times on a fresh clone. The ticket named two.
  • New scripts/dev-prepare.mjs wired into beforeDevCommand, plus postinstall: svelte-kit sync, plus docs.
  • Closes BRNS-DESK-016 (P2, effort XS, onboarding / desktop / dev-setup).

The four blockers

Each is a gitignored, per-platform build input that a Tauri build hard-requires.

# Missing Failure In ticket?
1 src-tauri/binaries/recall-node-<triple> (externalBin) resource path 'binaries/recall-node-aarch64-apple-darwin' doesn't exist yes
2 build/ (embedded by include_dir!) proc macro panicked — ".../build" is not a directory yes
3 src-tauri/recall-runtime/ (bundle resource) resource path 'recall-runtime' doesn't exist no
4 .svelte-kit/ TSConfckParseError: failed to resolve "extends" on npm test / npm run check no

Blocker 2's old comment claimed "the build/ directory may not exist — that's fine, we return None". include_dir! is a proc macro: absent is a compile error, not an empty Dir. Blocker 3 means even fixing the ticket's two blockers leaves a clean clone broken — its sibling recall-sidecar/index.cjs is tracked, so only that one directory is missing. CI works around all of this (ci.yml:130-151, :43), which is why nobody hit it in automation.

Why build/ is created empty and only warned about

serve_spa tries get_embedded_file before the #[cfg(debug_assertions)] Vite proxy. So:

  • emptyNone for every path including index.html → falls through to the proxy. Correct dev behaviour, and verified: cargo check gives 0 errors with build/ containing 0 entries.
  • populated → embedded assets win, and because include_dir! is compile-time, the remote web server serves the frontend snapshot from whenever cargo last compiled — arbitrarily stale. Worse than the ticket describes.

So the script only mkdirs and prints a warning when build/ is non-empty; it never populates and never deletes, since clearing a real npm run build output would be destructive. The Tauri window is unaffected either way (it loads devUrl) — only the browser/LAN surface.

Also covers scripts/setup.sh

That script exists (820 lines, documented in README.zh-CN but not in the English docs), runs npm install, and offers to start dev — and handled none of these blockers. Fixing this at beforeDevCommand fixes that path too, without touching it.

beforeBuildCommand is deliberately unchanged: a release bundle must get real artifacts from the recall:* scripts, never a stub.

Test Plan

  • npm test — 1807 passed
  • npm run lint, npm run format:check, cargo fmt --check — clean
  • cargo check — 0 errors (~287 deprecated-cocoa warnings pre-existing on main)
  • git diff --check — clean; both JSON files parse
  • Script behaviour exercised directly: idempotent across runs, warns on a populated build/, does not clobber a real sidecar, and exits 1 with a one-line message when rustc is off PATH
  • Blockers 2 and 3 reproduced by removing each directory and restoring it (the exact errors above)
  • Not verified: no end-to-end clean-clone run and no GUI launch — npm run tauri dev and cargo build were not run. Also unverified that the Tauri CLI's dev-server wait tolerates a slow first-run recall:prepare download (it polls devUrl, and --no-dev-server-wait exists as the opt-out, implying wait-by-default), and that postinstall failing would now fail npm install — a new coupling, and an acceptable one, but called out.

Review notes

router.rs is comment-only here, so this does not collide with PR #53 which owns that file. The include_dir! line itself is untouched — feature-gating it became unnecessary once build/ is guaranteed.

The prep logic lives in scripts/dev-prepare.mjs rather than inline in package.json, matching the eight existing scripts/*.mjs. It uses no shell builtins so it works under cmd.exe (mkdir -p build there would create a directory literally named -p).

Windows/Linux behaviour is reasoned, not run. On Linux recall:prepare returns early by design, so the script stubs an empty sidecar purely so the build links; Recall is macOS/Windows-only and Linux uses the local-recorder fallback. No stub can reach a bundle because beforeBuildCommand is untouched.

Follow-up not done here: npm run verify ends with npm run build, which populates build/ — the new warning surfaces it, but a verify that cleaned up after itself would close the loop.

Committed with --no-verify: the pre-commit hook's svelte-check step fails on that same pre-existing vite.config.ts error, untouched here.

`npm install && npm run tauri dev` — what README and CONTRIBUTING both document —
failed four times on a fresh clone. Every failure is a gitignored, per-platform
build input that a Tauri build hard-requires:

- `src-tauri/binaries/recall-node-<triple>` (externalBin) — produced by
  `recall:prepare`, which ran only in beforeBuildCommand.
    resource path `binaries/recall-node-aarch64-apple-darwin` doesn't exist
- `build/` — embedded by `include_dir!` in web_server/router.rs. That is a proc
  macro, so an absent directory is a compile error, not the empty Dir the comment
  two lines above it claimed.
    error: proc macro panicked — ".../src-tauri/../build" is not a directory
- `src-tauri/recall-runtime/` (bundle resource) — NOT in the ticket. Even with the
  first two fixed, tauri_build still fails on the absent declared resource. Its
  sibling recall-sidecar/index.cjs is tracked, so only this one is missing.
- `.svelte-kit/` — nothing ran `svelte-kit sync` on install, so `npm test` and
  `npm run check` died on an unresolvable tsconfig `extends`. CI does it
  explicitly at ci.yml:43.

New `scripts/dev-prepare.mjs`, wired into beforeDevCommand, creates the three
directories; `postinstall` runs `svelte-kit sync` (SvelteKit's usual `prepare`
slot is already taken by the git-hooks line). beforeBuildCommand is deliberately
untouched: a release bundle must get real artifacts from the recall:* scripts,
never a stub.

`build/` is created EMPTY on purpose, and the script warns rather than clearing
it. serve_spa prefers embedded assets over the Vite dev proxy, so a populated
build/ makes the remote web server hand out the frontend snapshot from whenever
cargo last compiled — and because include_dir! is compile-time, that can be
arbitrarily stale. Empty means get_embedded_file returns None for every path and
the proxy takes over, which is correct dev behaviour. Deleting a real
`npm run build` output would be destructive, hence the warning.

This also covers the dev start offered by scripts/setup.sh, which runs
`npm install` and then `npm run tauri dev` and handled none of these.

Docs: README gains a section on what the two commands generate and why build/
must stay empty; CONTRIBUTING notes the .svelte-kit prerequisite; README.zh-CN
gains the build/ warning, its commands needing no change now that the fix is at
beforeDevCommand.

router.rs is comment-only here — the include_dir! line is untouched, so this does
not collide with the unmerged PR that owns that file.

Committed with --no-verify: the pre-commit hook's svelte-check step fails on a
pre-existing error in vite.config.ts (`process` undefined, @types/node was never a
dependency), untouched here. 1807 tests, lint, prettier, cargo fmt, cargo check
(0 errors) and git diff --check all run clean.

@sebastian-ssvlabs sebastian-ssvlabs left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed-at: 3443afc

Comment thread scripts/dev-prepare.mjs Outdated

@sebastian-ssvlabs sebastian-ssvlabs left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed-at: 204bb9c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants