Skip to content

Fix Android: force-keep TalkBack JNI symbols without whole-archiving pulp-view - #151

Merged
danielraffel merged 3 commits into
mainfrom
fix/android-a11y-jni-direct
Apr 14, 2026
Merged

Fix Android: force-keep TalkBack JNI symbols without whole-archiving pulp-view#151
danielraffel merged 3 commits into
mainfrom
fix/android-a11y-jni-direct

Conversation

@danielraffel

Copy link
Copy Markdown
Collaborator

Summary

Narrower retry of #148 (which was closed because whole-archiving pulp-view pulled in SDL3's JNI_OnLoad). Uses -Wl,--undefined=<symbol> for each of the nine Java_com_pulp_accessibility_* entry points to force-retain them without transitively dragging SDL3 into the JNI closure.

Addresses the P1 from PR #132 that was merged unresolved.

Why the previous approach failed

pulp-view transitively links libSDL3.a, which defines its own JNI_OnLoad at _deps/sdl3-src/src/core/android/SDL_android.c:549. Adding $<TARGET_FILE:pulp-view> to the --whole-archive block in #148 forced the linker to include every object in every archive it touched, producing:

ld.lld: error: duplicate symbol: JNI_OnLoad
>>> defined at jni_bridge.cpp:41
>>> defined at SDL_android.c:549

The narrower fix

target_link_options(pulp-jni PRIVATE -Wl,--undefined=<sym>) for each accessibility JNI export. --undefined tells the linker to treat the symbol as a required external, which (a) keeps the accessibility_android.cpp object file alive inside libpulp-view.a and (b) does not force any other objects in any other archive. SDL3's JNI_OnLoad stays weakly referenced through its normal path.

Test plan

…pulp-view

PR #132's P1 review never actually landed on main. accessibility_android.cpp
defines all Java_com_pulp_accessibility_PulpAccessibilityDelegate_native*
entry points inside pulp-view (static lib). Nothing inside libpulp.so
references them — the only callers are on the Kotlin side
(PulpAccessibility.kt) — so the Android linker is free to strip them under
default dead-code elimination. At runtime TalkBack's first node-tree walk
then hits UnsatisfiedLinkError.

PR #148 tried to fix this by whole-archiving pulp-view. That pulled
SDL3's libSDL3.a in transitively, and SDL3 defines its own JNI_OnLoad,
colliding with core/platform/src/android/jni_bridge.cpp:41. #148 was
closed.

This PR uses the narrower `-Wl,--undefined=<symbol>` approach: force-
reference each of the nine TalkBack JNI exports in pulp-jni's link
options. The linker then keeps the accessibility_android.cpp object file
alive without dragging any other sections of pulp-view (or its SDL3
transitive closure) into libpulp.so.

No behaviour change on non-Android builds (the whole pulp-jni target is
gated on ANDROID). On Android, the nine forced references grow libpulp.so
by the single accessibility_android.cpp object file — same footprint PR
#127 intended when it first landed TalkBack.
@danielraffel
danielraffel force-pushed the fix/android-a11y-jni-direct branch from 438508b to edb8dbb Compare April 14, 2026 00:58
danielraffel added a commit that referenced this pull request Apr 14, 2026
AudioUnitSDK 1.4's own source files (external/AudioUnitSDK/src/*.cpp)
#include <expected>, a C++23 header. Under CMake 3.24's policy, the
root project's CMAKE_CXX_STANDARD=20 is authoritative on every target
regardless of target_compile_features: the existing
'target_compile_features(ausdk PUBLIC cxx_std_23)' declares only a
minimum feature requirement, which is silently satisfied by C++20
without upgrading the standard.

Add 'set_target_properties(ausdk PROPERTIES CXX_STANDARD 23
CXX_STANDARD_REQUIRED ON)' so the ausdk static library's own
translation units compile at C++23 and the std::expected includes
resolve. The PUBLIC cxx_std_23 compile feature stays so downstream
consumers (pulp-format) continue to pick up the minimum requirement
via interface — pulp-format already has the same CXX_STANDARD 23
property pinned at core/format/CMakeLists.txt:83, so this just
plugs the gap between the ausdk target and its own sources.

Affected before this fix: every PR cut after ~2026-04-13 14:00 UTC
failed its macOS (ARM64) github-hosted leg with:
  'error: no template named unexpected in namespace std'
  'error: no template named expected in namespace std'
repeatedly across AUUtility.h and AUBuffer.h until the compiler hit
its error limit. Confirmed affecting PRs #150, #151, the SignalGraph
Phase 0 branch, and #152 (admin-merged) among others.

Closes #155.
ViewBridge Phase 3 (#146) added au_v2_cocoa_view.mm to each plugin's
${target}_AU MODULE target via PulpUtils.cmake's pulp_add_plugin helper.
The .mm compiles inside that AU target, not pulp-format, so it does NOT
inherit pulp-format's `target_compile_features(... PUBLIC cxx_std_23)`
PUBLIC setting — CMake only propagates compile features when the
consumer links against the target, not when its sources are merely
`add_library(... src1 src2 ...)`-ed in alongside.

AUUtility.h in AudioUnitSDK 1.4 uses std::expected / std::unexpected
(C++23). Apple clang only exposes those at -std=c++23. Result on
github-hosted macos-14 runners (clang 17): every plugin AU target fails
to build with 'no template named unexpected in namespace std'.

Pin CXX_STANDARD=23 + CXX_STANDARD_REQUIRED=ON on ${target}_AU directly.
Same rationale documented at core/format/CMakeLists.txt L76 for the
pulp-format target.
danielraffel added a commit that referenced this pull request Apr 14, 2026
…) (#156)

* Fix macOS build break: pin ausdk target to CXX_STANDARD=23 (closes #155)

AudioUnitSDK 1.4's own source files (external/AudioUnitSDK/src/*.cpp)
#include <expected>, a C++23 header. Under CMake 3.24's policy, the
root project's CMAKE_CXX_STANDARD=20 is authoritative on every target
regardless of target_compile_features: the existing
'target_compile_features(ausdk PUBLIC cxx_std_23)' declares only a
minimum feature requirement, which is silently satisfied by C++20
without upgrading the standard.

Add 'set_target_properties(ausdk PROPERTIES CXX_STANDARD 23
CXX_STANDARD_REQUIRED ON)' so the ausdk static library's own
translation units compile at C++23 and the std::expected includes
resolve. The PUBLIC cxx_std_23 compile feature stays so downstream
consumers (pulp-format) continue to pick up the minimum requirement
via interface — pulp-format already has the same CXX_STANDARD 23
property pinned at core/format/CMakeLists.txt:83, so this just
plugs the gap between the ausdk target and its own sources.

Affected before this fix: every PR cut after ~2026-04-13 14:00 UTC
failed its macOS (ARM64) github-hosted leg with:
  'error: no template named unexpected in namespace std'
  'error: no template named expected in namespace std'
repeatedly across AUUtility.h and AUBuffer.h until the compiler hit
its error limit. Confirmed affecting PRs #150, #151, the SignalGraph
Phase 0 branch, and #152 (admin-merged) among others.

Closes #155.

* Pin AudioUnitSDK to 1.3.0 (last C++17-friendly tag)

The CXX_STANDARD=23 pin in the previous commit was necessary but not
sufficient: the ausdk target now compiles at -std=c++23 as intended,
but AppleClang/libc++ on the GitHub-hosted macOS-14 runner doesn't
yet expose std::expected even at C++23. Confirmed on the CI retry —
same errors even with CXX_STANDARD=23 pinned.

AudioUnitSDK-1.4.0 is the release that introduced <expected>
references in its headers (verified via GitHub API: 1.3.0 has 0
std::expected hits in AUUtility.h, 1.4.0 has 4). 1.3.0 compiles
cleanly on C++17+ and is the last tag that works on current macOS
runner libc++.

Changes:
- setup.sh: pin AU_SDK_REF to AudioUnitSDK-1.3.0 (was 1.4.0).
- .github/workflows/sign-and-release.yml: clone --branch
  AudioUnitSDK-1.3.0 explicitly (was unpinned default branch, which
  now resolves to 1.4.x).

The CXX_STANDARD=23 ausdk pin from the prior commit is kept — it's
harmless at 1.3.0 (the code doesn't use C++23) and gives us a
one-flip-back path once AppleClang/libc++ ship std::expected, or
when a new AudioUnitSDK release works around it.

Closes #155 (this time for real).

* Skill bypass for #155 fix

Skill-Update: skip skill=ci reason="Pinning AudioUnitSDK to 1.3.0 via sign-and-release.yml is a dependency-pin change for an upstream incompatibility; no new CI-authoring gotcha beyond what the existing 'Dependency Update Workflow' and #155 capture. The ci skill's 'Versioning & Skill-Sync gates' section is still accurate."
Skill-Update: skip skill=ship reason="Same dep pin; ship's SKILL.md already documents sign-and-release.yml as a tag-triggered workflow and doesn't need to know the upstream AU SDK tag."

* au_v2_instrument: use GetParameter for 1.3.0 compat

AudioUnitSDK 1.4 renamed the RT-safe parameter read to GetParameterRT;
1.3.0 uses GetParameter and is equivalent (inline atomic float load).
Switched the call site to GetParameter so the AU adapter builds
against the 1.3.0 pin from the previous commit. Flip back to
GetParameterRT when we can adopt 1.4+ (requires AppleClang/libc++
with std::expected on the GitHub-hosted macOS runner).

Skill-Update: skip skill=cli-maintenance reason="AU adapter source change, not CLI-surface. cli-maintenance SKILL.md isn't affected."
CXX_STANDARD=23 alone doesn't apply to .mm sources — CMake treats them
as Objective-C++ (OBJCXX), a separate language with its own
OBJCXX_STANDARD property. Previous commit 1e3b4f9 only set the C++
standard, so au_v2_cocoa_view.mm still compiled at the default C++17
and AUUtility.h's std::expected still failed.

Add OBJCXX_STANDARD=23 + OBJCXX_STANDARD_REQUIRED=ON alongside the
existing CXX_STANDARD so both C++ and Objective-C++ TUs in
${target}_AU pick up the required standard.
@danielraffel
danielraffel merged commit 894dced into main Apr 14, 2026
13 checks passed
@danielraffel
danielraffel deleted the fix/android-a11y-jni-direct branch April 14, 2026 08:55
danielraffel added a commit that referenced this pull request Apr 22, 2026
* chore: bump Shipyard pin v0.22.9 → v0.25.0

Pulls in two semantic changes that directly affect pulp's ship flow:

1. v0.24.0 (Shipyard #151) — `shipyard pr` now walks past the
   mechanical "chore: bump versions" commit when composing the
   auto-PR title and body, using the feature commit's subject/body
   instead. Pulp PR #624 was the canonical repro: its title read
   "chore: bump versions" and the body was "Automated by
   `shipyard pr`." — both are gone now. The tool-branding text is
   also scrubbed, so shipped PRs read as first-party.

2. v0.25.0 (Shipyard #152) — `Version-Bump: <surface>=<level>`
   trailers are now authoritative rather than ceiling-raising.
   Previously an author-declared `=patch` could be silently raised
   to `=minor` by the conventional-commit heuristic, defeating the
   point of the trailer. This commit also syncs pulp's bundled
   `tools/scripts/version_bump_check.py` with that behaviour so the
   hook-time, CI-time, and shipyard-time gates all agree.

Third improvement, automatic (no pulp-side code): v0.25.0 also
serves `shipyard ship-state list` from the running daemon via IPC
when one is available (Shipyard #154), bypassing the ~5-6s
PyInstaller cold-start on every call. The macOS GUI polls this
every 7s; `pulp pr` also hits it during preflight.

Verified: new binary installed at ~/.local/bin/shipyard, responds
`shipyard, version 0.25.0`. `version_bump_check.py --mode=report`
and `skill_sync_check.py --mode=report` both clean against
origin/main on this branch.

Version-Bump: sdk=skip reason="shipyard pin + bundled script sync — no SDK API surface moved"
Version-Bump: plugin=skip reason="shipyard pin + bundled script sync — no plugin surface moved"

* docs(ci-skill): note v0.24.0/v0.25.0 behavior changes at new pin

Skill-sync maps tools/shipyard.toml to the `ci` skill. The pin bump
in 3fb011c surfaced three new behaviours agents need to know about:

1. Auto-PR title/body now walks past the bump commit (v0.24.0).
2. Version-Bump trailer is authoritative (v0.25.0).
3. ship-state list goes through the daemon over IPC (v0.25.0).

Updated the ci SKILL.md with a new "Behaviour notes at the current
pin" subsection, and corrected the stale `v0.21.0` pin reference.
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