Skip to content

fix(ios): give the generator a runtimeDir option, and run its tests in CI - #108

Merged
glennmichael123 merged 2 commits into
mainfrom
fix/ios-generator-runtime-dir
Sep 3, 2026
Merged

fix(ios): give the generator a runtimeDir option, and run its tests in CI#108
glennmichael123 merged 2 commits into
mainfrom
fix/ios-generator-runtime-dir

Conversation

@glennmichael123

@glennmichael123 glennmichael123 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Four findings from the #103 seam review, each reproduced before being fixed.

The suite depended on the developer's shell

init() read CRAFT_IOS_RUNTIME straight from process.env with no way to override it:

CRAFT_IOS_RUNTIME bun test src/index.test.ts
unset 12 pass, 0 fail
set to an empty dir 8 pass, 4 fail

InitOptions.runtimeDir is the escape hatch CLAUDE.md already documents for the sibling variable — AppConfig.craftPath is to CRAFT_BIN what this is to CRAFT_IOS_RUNTIME — and null means no runtime, whatever the environment says. Every init() in the suite now declares it, and the result is identical either way: 22 pass, 0 fail.

And nothing ran that suite

No CI job touched packages/ios. That is how the above survived. The new ios-builder job runs it with CRAFT_IOS_RUNTIME explicitly empty — so it is the job that notices if the dependency returns — and publish-commit gates on it.

installRuntime destroyed a working install before validating the source

rmSync(Runtime/) ran first; the "none of these archives" check ran after. Pointing at an incomplete runtime directory deleted the archives already in place on the way to throwing, leaving project.yml linking -lcraft-ios against a directory that no longer existed. Every SDK is resolved before anything is removed, and there is a test that installs, then fails an install, then asserts the first one survived.

A single simulator slice was copied silently

RUNTIME_ARCHIVES' own comment calls a single-slice archive the break that "only shows up on someone else's laptop" — and the code then shipped one without a word. It still copies, because a one-architecture dev loop is legitimate, but it now names the missing slice and says the project will not link on the other architecture.

Re-running init without a runtime orphaned the archives

The link settings came out of project.yml and Runtime/ stayed on disk, so the directory and the project disagreed. Whatever a run decides, the tree agrees with it.

build() relinked a stale runtime

installRuntime had one caller: init. build never touched Runtime/, and run just calls build. Because the archives are copied rather than symlinked, the dev loop this option exists for did not work — edit Zig, zig build build-ios-all, craft ios run, and xcodebuild relinks the copy from init. The build succeeds, the app launches, the change is absent, and there is no error to chase.

build now refreshes a runtime the project already links, taking the same runtimeDir override. Refresh only: it never installs into a project that links none (those archives would be unreferenced — that is init's decision) and never removes one when the variable is simply unset. Both pinned by tests.

Not covered

The two-slice lipo path needs genuine Mach-O input and lipo does not exist on the Linux runner, so it is not unit-tested. It is covered by building a real generated app, which is how the fat archive was verified when the seam landed.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

✅ Binary Size Report

Metric Value
Current Size 14933KB (14.58MB)
Change 0KB (0%) unchanged
Size limits
  • Warning: 14.50MB
  • Maximum: 16.00MB

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

✅ Binary load time

rounds:    25 interleaved
base:      p50 25.2ms   p95 26.1ms   (24.6–26.5ms)
head:      p50 24.9ms   p95 26.7ms   (24.1–29.2ms)
delta:     -1.1%  (fails above +20.0%)

No binary load time regression.
What this measures

craft --help: process spawn, dynamic linking and argument parsing.
It never opens a window, so it cannot see a change in window or
webview startup — real startup is benchmarks/startup.bench.ts, which
needs a display.

Both binaries are measured interleaved on this runner and compared by
p50, rather than against a number recorded on another machine. On
byte-identical binaries that method reads within ~3.5%; the old one
swung 45%.

…n CI

Four findings from the seam review, all reproduced before being fixed.

**The suite depended on the developer's shell.** `init()` read
`CRAFT_IOS_RUNTIME` straight from `process.env`, with nothing to override it,
so `bun test src/index.test.ts` gave 12 passing in a clean shell and 8 passing
with 4 failing when the documented dev-loop variable happened to be exported.
`InitOptions.runtimeDir` is the escape hatch CLAUDE.md already describes for
the sibling variable — `AppConfig.craftPath` is to `CRAFT_BIN` what this is to
`CRAFT_IOS_RUNTIME` — and `null` means "no runtime, whatever the environment
says". Every `init()` in the suite now says so, and the result is identical
with the variable set and unset.

**And nothing ran that suite.** No CI job touched `packages/ios`, which is how
the above survived. `ios-builder` runs it with `CRAFT_IOS_RUNTIME` explicitly
empty — the job that would notice if the dependency came back — and
`publish-commit` gates on it.

**`installRuntime` destroyed a working install before validating the source.**
`rmSync(Runtime/)` ran first and the per-SDK "none of these archives" check
ran after, so pointing at an incomplete runtime directory deleted the archives
already in place on the way to throwing, leaving `project.yml` linking
`-lcraft-ios` against a directory that no longer existed. Every SDK is
resolved before anything is removed.

**A single simulator slice was copied silently.** `RUNTIME_ARCHIVES`' own
comment calls a single-slice archive the break that "only shows up on someone
else's laptop", and then the code shipped one without a word. It still copies
— a one-architecture dev loop is legitimate — but it says which slice is
missing and that the project will not link on the other architecture.

**Re-running init without a runtime left the archives behind.** The link
settings came out of `project.yml` and `Runtime/` stayed on disk, so the
directory and the project disagreed. Whatever a run decides, the tree now
agrees with it.

The two-slice `lipo` path is not unit-tested: it needs genuine Mach-O input
and `lipo` does not exist on the Linux runner this job uses. It is covered by
building a real generated app, which is how the fat archive was verified when
the seam landed.
…king a stale copy

`installRuntime` had exactly one caller: `init`. `build` never touched
`Runtime/`, and `run` just calls `build`. Since the archives are copied rather
than symlinked, the monorepo dev loop this whole option exists for did not
work: edit `packages/zig/src`, run `zig build build-ios-all`, then `craft ios
run`, and xcodebuild relinks the archive copied when the project was first
generated. The build succeeds, the app launches, and the change is absent —
the worst shape a stale artefact takes, because there is no error to chase.

`build` now refreshes a runtime the project already links, taking the same
`runtimeDir` override `init` does.

Refresh only: never installs, never removes. A project with no `Runtime/` was
generated without one and its `project.yml` carries no link settings, so
copying archives in would leave them unreferenced — that decision is `init`'s.
A project that does link one keeps the archives it has when no runtime
directory is configured, because a shell that forgot the variable should not
quietly turn the runtime off. Both are pinned by tests.
@chrisbbreuer
chrisbbreuer force-pushed the fix/ios-generator-runtime-dir branch from b59f0b3 to 19d071f Compare September 3, 2026 18:30
@glennmichael123
glennmichael123 merged commit 50badb6 into main Sep 3, 2026
11 checks passed
@glennmichael123
glennmichael123 deleted the fix/ios-generator-runtime-dir branch September 3, 2026 18:44
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.

1 participant