ReviewOS

also looking at this

craft-native/craft

fix(build): three test artifacts reach Carbon and none of them linked it

#139
Merged chris wants to merge fix/tray-tests-link-carbon into main
1 file +23 -0
10 days ago

zig-core (macos-latest) has been failing on main, and on every PR opened against it. Five undefined symbols, all Carbon:

error: undefined symbol: _GetApplicationEventTarget
error: undefined symbol: _GetEventParameter
error: undefined symbol: _InstallEventHandler
error: undefined symbol: _RegisterEventHotKey
error: undefined symbol: _UnregisterEventHotKey

The dependency is four files from anything that looks like a hotkey

tray.zig → macos.zig → bridge_shortcuts.zig → macos_hotkey.zig → Carbon

Nothing in tray.zig names a hotkey. Nothing in the iOS surface does either. So three artifacts that hand-roll their framework lists — system_tray_tests, system_tray_benchmark, ios_surface_test — each named Cocoa, WebKit and CoreMIDI and missed the one framework they could not link without.

ios_surface_test is the clearest case, because its own comment already explains the cause without drawing the conclusion. It says the reply path "drags in the AppKit/CoreFoundation surface behind it". That surface is exactly what reaches macos_hotkey.zig.

Why they drifted

linkPlatformLibraries has had Carbon since it was written, with a comment explaining why (RegisterEventHotKey is the only route to a global hotkey that needs neither an Objective-C block nor Accessibility permission). These three don't call it — it also compiles the vendored SQLite amalgamation, which a tray test has no use for — so they duplicated the list by hand, and the duplicate drifted.

Each now names Carbon with the import chain written down, so the next reader doesn't have to re-derive it from a linker error.

A correction about how I verified

On a warm cache this does not reproduce. The earlier objects link and the build reports success.

I asserted twice in this session that zig build test was clean apart from the known injected_js_test failure. Both times I measured against a warm cache, and both times I was wrong — a pruned cache shows all three of these. Worth recording because it's the same class of mistake as trusting test:android alone: a narrow or stale signal read as a broad one.

After this change

zig build test from cold:

Build Summary: 156/159 steps succeeded (1 failed); 5/5 tests passed
remaining failure: -Mroot=test/injected_js_test.zig

That one is injected_js_test against a local incompatible zig-js checkout — absent in CI, and it fails identically on main.

Build configuration only; no source changes.

1 changed file on the files tab, with 0 review threads.

Sign in to comment on this pull request.