[nejdetkadir]

Problem

gea_app_entry.cpp declares gea_embedded_assets_prepare with a plain __attribute__((weak)). Since 81cbe9f the generator gives every non-bundle app a weak definition of it. On COFF a weak reference meeting a weak definition is a duplicate, as win32_hook_defaults.cpp in geastack/windows describes. Linked with clang-cl and lld-link 23.1.3 on macOS (not a Windows build), the exact line 44 and the generator's line 254 give:

lld-link: error: duplicate symbol: gea_embedded_assets_prepare

The review on this PR also asked for a test of the configuration that broke on Apple, a log when file-backed assets fail to prepare, and removal of a stale deployment manifest.

Change

• gea_app_entry.cpp: declare the hook with GEA_PROGRAM_HOOK, like the cycle-collection hooks, so a target with GEA_LINKED_HOOK_DEFAULTS references it as an ordinary symbol. Link a default for the embedded-assets prepare hook windows#7 adds its /alternatename default; the two should land together.
• asset_bundle.h: log when storage is not mounted, the loader's one silent failure.
• gea-file-asset-bundle.mjs: a build without file storage removes the .assets.json an earlier storage-backed build left.
• test_embedded_assets_prepare.mjs: links the real gea_app_entry.cpp object (-r) with generated units for apps with no storage, no assets and file-backed assets, and checks that the hook resolves. Skips on Windows.
• test_embedded_assets_manifest.mjs and test_embedded_weak_symbols.mjs: the manifest removal and the GEA_PROGRAM_HOOK spelling.

The branch is on 4fd69f1. Its diff applies cleanly on 81cbe9f, and the new prepare test needs 81cbe9f.

Target matrix

Target	Evidence
macOS (Apple silicon)	tested on device: the blank starter built and ran; a file-backed build links one strong definition, called from Application::init (lldb)
iOS, iPhone 11	tested on device: the blank starter built, installed and ran
Linux (raspberry-pi-os)	ran in a Debian 13 arm64 VM under Xvfb
Windows	not run on Windows. COFF link checked with clang-cl and lld-link 23.1.3 on macOS: with this PR and windows#7, the non-bundle, file-backed and no-assets-unit cases link and resolve as expected
ESP32, RP2350	source-only

Tests

Run on 81cbe9f plus this PR:

$ node test/test_embedded_assets_prepare.mjs
Embedded assets prepare: entry links against no-storage, no-asset and file-backed generated units
$ node test/test_embedded_assets_manifest.mjs
Embedded assets manifest: a build without file storage removes the stale deployment manifest
$ node test/test_file_asset_bundle.mjs
File asset bundles: validated content, rollback identity, corrupt/truncated/oversized/missing files, ASan/UBSan passed

test_embedded_weak_symbols.mjs exits 0. test_embedded_assets_prepare.mjs fails with the generator from before 81cbe9f (U _gea_embedded_assets_prepare with ld64, w with GNU ld), and the manifest test fails with main's gea-file-asset-bundle.mjs. The 13 steps of bouncing-balls-regressions.yml pass locally.

Notes for review

• Without the generated assets unit, a macOS link still fails on _gea_embedded_asset_lookup from host/image.cpp. A weak default there would compete with the generator's weak lookup, since ld64 and GNU ld take the first weak definition in link order. I left it alone.
• The fonts generator gets the manifest cleanup when the app has fonts. With no fonts at all, it returns before that.

Summary by CodeRabbit

• Bug Fixes

  • Asset bundle failures now report the affected asset path.
  • Stale asset metadata is removed when storage is unavailable or contains no entries.
  • Embedded asset preparation uses consistent optional-linkage behavior.

• Tests

  • Added coverage for metadata cleanup and embedded asset preparation across storage configurations.
  • Added validation for embedded asset linkage and hook declarations.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: aaa59c8a-cd6b-4619-b5ad-fb72a4d573a1

📥 Commits

Reviewing files that changed from the base of the PR and between 9eba555 and b23bcae.

📒 Files selected for processing: 6

• packages/core/gea_app_entry.cpp
• packages/core/include/platform/asset_bundle.h
• packages/core/scripts/gea-file-asset-bundle.mjs
• packages/core/test/test_embedded_assets_manifest.mjs
• packages/core/test/test_embedded_assets_prepare.mjs
• packages/core/test/test_embedded_weak_symbols.mjs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The changes update embedded-assets hook linkage, remove stale asset metadata when file storage is unavailable, and add an error log when asset bundle storage mounting fails. Integration tests cover hook linking and manifest cleanup.

Changes: Embedded asset handling

Layer / File(s)	Summary
Embedded-assets hook linkage packages/core/gea_app_entry.cpp, packages/core/test/test_embedded_weak_symbols.mjs, packages/core/test/test_embedded_assets_prepare.mjs	The entry declaration uses GEA_PROGRAM_HOOK. Tests check the declaration and verify that the generated hook symbol is defined and resolved across no-storage, no-assets, and file-backed configurations.
Asset metadata cleanup packages/core/scripts/gea-file-asset-bundle.mjs, packages/core/test/test_embedded_assets_manifest.mjs	Asset generation removes an existing manifest when storage is absent or the asset entries are empty. The integration test checks manifest creation and stale-manifest removal.
Bundle mount diagnostics packages/core/include/platform/asset_bundle.h	loadAssetBundle logs the requested path when storage mounting fails, then returns false.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix

Suggested reviewers: dashersw

Merge Risk: ⚪ Minimal · up to b23bc

The change adjusts how the embedded-assets hook is declared, removes stale asset manifests and adds a mount-failure log. No actionable merge-blocking risk remains. It is intended to land alongside the related Windows change that supplies the linker default.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 6 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly summarizes the two primary changes: using GEA_PROGRAM_HOOK for the prepare hook and removing stale asset manifests.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[dashersw]

Thanks for this. The diagnosis is right, and the generator change fixes the reported case: an Apple app with no embeddedAssets.storage links again. Notes, most important first:

1. The fix depends on every link including the generated assets unit. gea_app_entry.cpp still has an undefined weak reference whenever gea_embedded_assets_generated.cpp is not linked beside it. apple build-macos.sh adds that file only [[ -f "$GENERATED_ASSETS_CPP" ]], and the no-program smoke path (macos_smoke_app.cpp) may not produce one. Either case gives the same Undefined symbols _gea_embedded_assets_prepare. A weak default in gea_app_entry.cpp itself, in the !GEA_LINKED_HOOK_DEFAULTS branch, would fix every non-COFF link at the root. You mention this as an alternative, and I think it is the sturdier one. The generator's weak default can stay as well.

2. Merge order with geastack/windows#7. With GEA_PROGRAM_HOOK, the Windows reference becomes ordinary, and no /alternatename default exists until windows#7 lands. These two must merge together, and the Windows side has never been built or linked. Under the ordinary reference, if (gea_embedded_assets_prepare) is also always true, so clang-cl will emit -Wpointer-bool-conversion. The cycle-collection hooks already do the same, so this is only a nit.

3. The prepare() result is ignored (gea_app_entry.cpp:107). This predates the PR, but the PR now runs the hook for every app. If a file-backed bundle is missing or corrupt, prepare returns false, init carries on, and images built straight from the zero-filled GEA_FILE_ASSET_BUFFER arrays render blank, with no log line. It should at least log.

4. Multi-app binaries. The new comment justifies weak because a bundled binary links several apps' assets. The file-backed branch (generator line 303) still emits a strong definition, though, so two storage-backed apps in one resident build fail with duplicate symbol. Making it weak would let the linker silently pick one app's loader instead. Resident mode also drops gea_app_entry.cpp (build-macos.sh:1058), so no app's prepare runs there at all. A single global hook can't serve per-app bundles. This is out of scope here, but the comment should not imply the weak default solves multi-app.

5. Stale manifest. When an app drops storage, the generator writes the no-bundle .cpp but leaves the old ${outCpp}.assets.json behind, so deploy tooling can still push the old bundle. This predates the PR, but it is cheap to fix here: remove the file when bundle is null.

6. Test coverage. The new check links the generated unit against a plain strong extern declaration, not against gea_app_entry.cpp's actual hook reference. The configuration that actually broke (entry linked, generated unit absent or lacking the symbol) is still untested. The check also sits inside the file-bundle test after the ASan runs, and the final success line doesn't mention it.

7. Nit. The if (!bundle) block sits about 30 lines from the strong definition it pairs with. Writing it as the else of the if (bundle) at line 301, plus the no-assets branch, keeps both definitions of the hook together.

[nejdetkadir]

Thanks for the review. I saw 81cbe9f put the generator fallback on main, so I've reduced this PR to what main doesn't have. Pushed 3909378; the branch's cumulative diff applies cleanly on 81cbe9f.

Windows (point 2). With 81cbe9f, gea_app_entry.cpp:44 still declares the hook with a plain __attribute__((weak)), and every non-bundle app now gets a weak definition. I linked that pair with clang-cl and lld-link 23.1.3 on macOS (not a Windows build), using the exact line 44 and the generator's line 254:

lld-link: error: duplicate symbol: gea_embedded_assets_prepare

This PR keeps the GEA_PROGRAM_HOOK spelling, and geastack/windows#7 adds the /alternatename default. With both, the non-bundle case resolves to the generator's weak definition, a strong definition replaces it, and without a generated assets unit the default is used. Without windows#7 the reference is undefined, so the two should land together. The if (gea_embedded_assets_prepare) guard stays, like the cycle hooks', so clang-cl will warn on it as you noted.

1. Since main took the generator route, I dropped the default from gea_app_entry.cpp. While testing it I relinked a real macOS build without the generated assets object: it still fails, on _gea_embedded_asset_lookup from host/image.cpp, with or without a prepare default. A weak default for lookup in image.cpp would not be safe, because the generator's lookup is weak as well, and ld64 and GNU ld both take the first weak definition in link order, which in the macOS link is image.o. A link without the generated assets unit needs a different answer, so I left it alone.
2. loadAssetBundle already logs a missing file (asset_bundle.h:34 on main) and a failed validation (:65). The one silent false was ensureMounted(), which now logs too. I left init unchanged so each failure is reported once, where the cause is known. I can add a line there as well if you prefer.
   4, 7. Gone with 81cbe9f.
3. The stale manifest is removed in fileAssetBundle, which owns the path, so the fonts generator gets it too when the app has fonts. With no fonts at all, it returns before that and a stale fonts manifest stays.
4. New test_embedded_assets_prepare.mjs compiles the real gea_app_entry.cpp, links it (-r) with the generator's output for an app with no storage, one with no assets and one with file-backed assets, and checks that the reference resolves. With the generator from before 81cbe9f it fails with U _gea_embedded_assets_prepare (ld64) and w gea_embedded_assets_prepare (GNU ld). It skips on Windows, where lld-link has no relocatable link; the windows target's hook-defaults.test.mjs covers COFF. The manifest has its own test, test_embedded_assets_manifest.mjs, and test_embedded_weak_symbols.mjs checks the GEA_PROGRAM_HOOK spelling. The branch is still on 4fd69f1, so the new test needs 81cbe9f and passes on the merge.

Run on 81cbe9f plus this PR:

• macOS: the blank starter built and ran. A file-backed build links one strong definition, and lldb shows Application::init calling it.
• iPhone 11: the blank starter built, installed and ran.
• Linux (Debian 13 arm64, raspberry-pi-os target): built and ran.
• The 13 steps of bouncing-balls-regressions.yml pass locally, as do test_file_asset_bundle, test_embedded_weak_symbols and the new tests.

I couldn't run the smoke path from point 1: build-macos.sh for an app without index.tsx stops before linking at audio_buffer_runtime.cpp: 'gea_runtime.h' file not found, because the published compiler 1.0.42 has no dist/targets/cpp/runtime.

[nejdetkadir]

Following up on the Windows point with a real Windows run: https://github.com/nejdetkadir/windows/actions/runs/39495396424 (job core-prepare-on-coff, windows-latest, clang-cl and lld-link from LLVM 20.1.8). The job checks the source lines against core main at 81cbe9f before linking.

• main's gea_app_entry.cpp:44 reference with the generator's weak definition: lld-link: error: duplicate symbol: gea_embedded_assets_prepare.
• With GEA_PROGRAM_HOOK from this PR and the default from Link a default for the embedded-assets prepare hook windows#7: a non-bundle app links to the generator's definition and prints prepare 1, a strong definition wins (prepare 0), and with no generated assets unit the default is used (prepare 1).
• clang-cl warns -Wpointer-bool-conversion on the guard, as you said.

The units are minimal stand-ins for those lines, not a full Windows build of an app.

[dashersw]

Thanks, all points addressed. Merging together with geastack/windows#7.