feat: Add android build - #1357
Conversation
|
No concrete correctness, security, or merge-blocking issues found in the PR changes. |
|
|
Two real bugs from automated review: - `run_apex` called `nix build --out-link target/android-apex/build-apex` before creating target/android-apex/ - nix can't create that symlink without its parent existing, so this failed outright on any clean checkout (no prior `target/` dir). Moved the create_dir_all before the nix build, and made it independent of the user-configurable `--out-dir` rather than conflating the two. - `run_payload` never cleaned a package's payload directory between runs, so a binary/init script removed since the last run (or a crate that just became unsupported) would linger in `content/` and get bundled into the next APEX, inconsistent with the freshly generated canned_fs_config. Now removes each package's output dir before restaging it. (A third comment, about crate names with hyphens producing invalid Android package names, was already fixed in f785345e.) Verified: reproduced the clean-checkout scenario (removed target/android-apex and target/android-apex-payloads first) and confirmed `cargo x android-apex` now succeeds, producing all 14 .apex files.
Adds an Android NDK toolchain to the dev shell (nixpkgs androidenv, NDK-only, license accepted, API level 24) and wires it into cargo via env vars, following the same pattern as the existing OP-TEE cross-toolchain setup. cargo-zigbuild can't be reused here since zig doesn't bundle Android's bionic sysroot. Adds `cargo x android-sweep` to attempt building every workspace crate (or a given list) for aarch64-linux-android and report pass/fail - not every crate is expected to build. Verified end-to-end: orb-const-concat, orb-arc-pool, orb-header-parsing, and orb-attest (with its full zbus/zenoh/dbus dependency stack) all compile and link for aarch64-linux-android with this toolchain, no source changes required.
Marks 24 crates as unsupported on aarch64-linux-android via the existing `[package.metadata.orb] unsupported_targets` mechanism (already used by ci/rust_ci_helper.py to skip Darwin-incompatible crates in `cargo nextest run --workspace`), grouped by root cause: - Bionic libc gaps: can-rs, orb-mcu-interface, orb-mcu-util, orb-update-agent, orb-update-verifier, orb-uart (no SocketCAN, no termios c_ispeed/c_ospeed) - No EFI on Android: efivar, orb-slot-ctrl, orb-update-agent-core - Vendor/native libs with no Android build: orb-gst-jpeg, orb-sound, orb-ui, orb-cone, orb-hil, seek-camera(-sys), orb-thermal-cam-ctrl, orb-connd, orb-secure-storage-ca - squashfs toolchain not cross-built for Android: orb-bidiff-squashfs(-shim), orb-bidiff-cli, orb-tools - Genuine bionic incompatibility: orb-update-agent-loader (the `nix` crate calls `fexecve`, which bionic doesn't implement) Adds `cargo x android-build`, which reads that same metadata to compute the exclude list and runs a single `cargo build --workspace --target aarch64-linux-android --exclude ...` (verified locally, exit 0). This is a CI gate, unlike the existing `android-sweep` discovery command: `--exclude` only removes packages as build roots, so anything depending on an excluded crate (e.g. orb-tools -> orb-bidiff-squashfs-shim) needed its own entry too. Wires this into a new `Android Build` job in rust-ci.yaml, matching the existing job pattern. Note: agentwire-macros failed when built in isolation (missing syn/full feature) but was deliberately left off the exclude list - verified it builds fine as part of the full `--workspace` build, once feature unification pulls in syn/full from elsewhere in the graph.
Adds `cargo x android-apex-payload`: for every Android-supported binary crate, stages its own payload directory (bin/<binary>, apex_manifest.json, a placeholder etc/init/<binary>.rc) - one per project, not a single shared APEX. Reuses android-build's exclude logic and rebuilds first so staged binaries are current. Verified end-to-end: staged 14 crates (orbd, orb-attest, orb-jobs-agent, etc.), confirmed correct directory layout and that staged binaries are real aarch64 ELF Android binaries. This stops short of invoking `apexer` to produce a real signed .apex - that needs a cluster of AOSP-built host tools (avbtool, mkfs.erofs, aapt2, apexer's own protobuf-generated manifest parser) that this workspace doesn't have a sourcing story for yet. Manifest naming and init.rc contents are marked TODO pending real answers on APEX namespace and SELinux domain.
run_payload already authors every path in the payload (bin/<binary>, etc/init/<binary>.rc); it can emit the canned_fs_config apexer needs directly as it writes those paths, rather than re-deriving the same information by walking the tree after the fact with a separate tool (gen-canned-fs-config.nix). One source of truth for what's in the payload instead of two. Verified: re-ran `cargo x android-apex-payload`, inspected the generated canned_fs_config for orbd - correct paths/uid/gid/mode.
Adds nix/packages/android-apex.nix, building `apexer` from AOSP source (system/apex at android-16.0.0_r4, one repo - manifest.py and mke2fs.conf are single-file fetches, not full extra repo clones) and substituting nixpkgs/android-tools for the rest of its usual AOSP-build-only toolchain: android-tools' mke2fs.android/e2fsdroid for the ext4 payload plus avbtool, e2fsprogs' resize2fs, aapt's aapt2. Payload filesystem is ext4, not erofs - matches apexer-for-clanker.nix. Confirmed nixpkgs' erofs-utils can't produce a canned_fs_config-capable mkfs.erofs even with selinuxSupport=true: --file-contexts needs only libselinux, but --canned_fs_config needs a second, separate -DWITH_ANDROID build plus AOSP's libcutils (system/core) for load_canned_fs_config, which isn't implemented anywhere else and has no standalone build path. android-tools' e2fsdroid already has this built in as a working prebuilt, so ext4 avoids that whole dependency chain. Restructures run_payload's output: content actually going into the APEX (bin/, etc/) now lives under a content/ subdirectory, sibling to apex_manifest.json/canned_fs_config/file_contexts - apexer's e2fsdroid scans whatever directory it's given as literal payload content, so the previous flat layout made it choke trying to find our own sidecar files (e.g. apex_manifest.json) in canned_fs_config. Verified end-to-end: built a real signed orbd.apex from the staged orbd payload. Confirmed with zipfile (AndroidManifest.xml, resources.arsc, apex_manifest.pb, apex_payload.img all present) and debugfs (ext4 image contains /bin/orbd at mode 755, uid/gid 1000, plus apex_manifest.pb at the image root) and avbtool info_image (valid SHA256_RSA4096 hashtree footer). apex_manifest.pb round-trips to name=com.worldcoin.orb.orbd, version=1 as expected. Signing key is a throwaway generated per build-apex invocation - fine for local testing, needs a real managed key before this ships anywhere near a device.
Switches the APEX payload filesystem back to erofs, using a real AOSP-built mkfs.erofs binary fetched directly from kernel/prebuilts/build-tools (android-16.0.0_r0.4) rather than trying to build one ourselves. Patched via autoPatchelfHook to run under nixpkgs' glibc; uses AOSP's own libc++.so (also fetched from the same prebuilts repo) rather than nixpkgs' to avoid any ABI mismatch with a binary Google built and tested against that exact one. Confirmed via readelf that mkfs.erofs only NEEDS generic libc/libc++ (libcutils/libbase/liblog are statically linked in for the host build), so no separate AOSP shared libs are needed beyond libc++.so. This drops all the ext4-specific tooling from the previous commit (mke2fs.android, e2fsdroid, resize2fs, the sefcontext_compile shim, AOSP's mke2fs.conf) - erofs needs none of it, mkfs.erofs takes --file-contexts/--fs-config-file directly. Verified end-to-end: rebuilt orbd.apex using this toolchain. Confirmed with zipfile (same structure as before) and, this time, actually extracted the erofs payload with AOSP's own fsck.erofs and confirmed apex_manifest.pb/bin/orbd/etc/init/orbd.rc are all present and orbd is still executable. Image is ~46MB vs ext4's ~135MB, thanks to lz4hc compression.
Exposes the `build-apex` tool from nix/packages/android-apex.nix as a real flake package (packages.x86_64-linux.build-apex), and adds `cargo x android-apex`: stages payloads (same as android-apex-payload), nix builds build-apex once, then packages each staged crate into target/android-apex/<crate>.apex. android-apex-payload's run_payload now returns the list of staged package names (Result<Vec<String>> instead of Result<()>) so run_apex can reuse it directly instead of re-deriving the same crate list a second time. Also fixes a real bug this surfaced: the placeholder APEX name (com.worldcoin.orb.<crate>) breaks for any crate with a hyphen in its name (e.g. orb-attest) since Android package names are dot-joined Java identifiers and aapt2 rejects hyphens outright. Sanitizes to `_`. Verified end-to-end: `cargo x android-apex` packaged all 14 Android-supported binary crates into individual signed .apex files (orb-attest, orbd, orb-supervisor, zorb, etc.), each confirmed with zipfile to have the expected structure.
Replaces the Android Build job's plain `cargo x android-build` step with `cargo x android-apex`, which is a strict superset (it calls the same workspace build internally, then additionally packages every Android-supported binary crate into a signed .apex) - running both would just rebuild the same already-warm workspace a second time for no benefit. Uploads target/android-apex/*.apex as a build artifact (android-apex), matching the existing `build` job's upload-artifact pattern. Note: this makes CI depend on network access to android.googlesource.com (to fetch the AOSP apexer source and Google's prebuilt mkfs.erofs) on a cache miss - a new external dependency this job didn't have before.
Two real bugs from automated review: - `run_apex` called `nix build --out-link target/android-apex/build-apex` before creating target/android-apex/ - nix can't create that symlink without its parent existing, so this failed outright on any clean checkout (no prior `target/` dir). Moved the create_dir_all before the nix build, and made it independent of the user-configurable `--out-dir` rather than conflating the two. - `run_payload` never cleaned a package's payload directory between runs, so a binary/init script removed since the last run (or a crate that just became unsupported) would linger in `content/` and get bundled into the next APEX, inconsistent with the freshly generated canned_fs_config. Now removes each package's output dir before restaging it. (A third comment, about crate names with hyphens producing invalid Android package names, was already fixed in f785345e.) Verified: reproduced the clean-checkout scenario (removed target/android-apex and target/android-apex-payloads first) and confirmed `cargo x android-apex` now succeeds, producing all 14 .apex files.
These two crates were left out when every other workspace crate's unsupported_targets list was updated to exclude the new Android build target, so the android-build CI gate would attempt to compile them.
The build-apex flake package is intentionally x86_64-linux-only (the prebuilt mkfs.erofs binary and its flake exposure in nix/shells/flake-outputs.nix only target that system, since erofs-utils has no working canned_fs_config-capable build for other platforms). Without this check, running `cargo x android-apex` on e.g. aarch64-linux or Darwin surfaced as an opaque `nix build` failure instead of an actionable message.
The "generate key on first use" check in build-apex was dead code: $work is a directory `mktemp -d` just created, so the file-exists check could never be true, and a fresh throwaway key was silently generated on every single invocation instead of ever being reused. Replace it with an explicit $APEX_SIGNING_KEY override, and wire the CI job to supply a real key from a GitHub Environment secret (scoped separately from ordinary repo/org secrets so it can get its own protection rules), falling back to a throwaway key for local/dev use when unset.
run_payload's binary staging and run_apex's `nix build .#build-apex` used paths/refs relative to the process's current working directory instead of the workspace root, so invoking `cargo x android-apex` (or android-apex-payload) from any subdirectory misresolved the built binaries and failed to locate flake.nix. Use cargo_metadata's target_directory/workspace_root, which are always absolute, instead.
run_apex's packaging loop used `?` on the first per-crate failure, aborting the whole command and skipping every crate after it in the list - unlike android-sweep's discovery loop, which already reports a succeeded/failed summary. Package as many crates as possible, print the same kind of summary, and only return an error afterward if any crate failed.
The nix build --out-link target was a fixed "target/android-apex/ build-apex" shared by every invocation regardless of --out-dir. Since --out-link atomically replaces whatever symlink is at that path, two overlapping invocations sharing a checkout could race on it. Use a fresh tempdir per invocation instead.
run_apex re-typed the "target/android-apex-payloads" string as its own literal instead of referencing PayloadArgs::out_dir's default, so editing either one alone would silently desync them - not currently user-triggerable since ApexArgs exposes no override for this intermediate path, but a real maintenance hazard. Share one constant.
Every android-build run previously used the same android-apex-signing environment/key regardless of trigger, so a PR build and a real release build shared the same secret - maximizing the blast radius of that key. Pick android-apex-signing-prod only for pushes to `prod` or tag builds, android-apex-signing-dev otherwise. Also clean up the materialized key file with an EXIT trap instead of leaving it on disk for the rest of the job.
Replace the whole $APEX_SIGNING_KEY / GitHub Environment / dev-vs-prod plumbing with something simpler: always sign with AOSP's published AVB test key (test/data/testkey_rsa4096.pem from external/avb), fetched and pinned the same way this file already pins other AOSP artifacts. This is deterministic across machines/runs (unlike a throwaway key generated fresh per invocation) and unambiguously recognizable as a test signature - useful for local development, but there is no path to a real signing key here at all, so CI no longer needs any secret or environment scoping for this job.
Code review flagged the uncapped thread-per-crate spawn in run_apex as a potential CPU oversubscription risk. In practice each build-apex invocation mostly waits on its own apexer/aapt2/avbtool/mkfs.erofs subprocesses rather than burning CPU directly, and the expected crate count is in the dozens - affordable without a worker-pool cap.
run_apex fetched workspace_root directly, then called run_payload (which fetched metadata again directly, plus once more via run_build's call to unsupported_packages, plus once more via its own call to unsupported_packages) - 4 cargo metadata shell-outs per cargo x android-apex invocation for data that can't change mid-run. Fetch it once in run_apex and thread it through run_payload, run_build_with (a new metadata-taking sibling of the still-public, self-fetching run_build used by the standalone android-build command), and unsupported_packages, which now takes &Metadata instead of fetching its own.
apexer, compileApexManifest, and androidJar were exported but never consumed outside this file (verified via repo-wide grep) - only androidApex.buildApex is referenced, from nix/shells/flake-outputs.nix. Drop the unused public surface; they remain available as let-bindings for buildApex to use internally.
xtask/src/cmd/android.rs's flake_ref and flake-outputs.nix's exposed attribute name are tied together only by a shared "build-apex" string literal across two languages, with nothing checking they stay in sync - a rename on one side would only surface at CI runtime as an opaque "flake does not provide attribute" error. Add a comment on each side pointing at the other, so an editor changing one is prompted to check the other.
Both functions built a Command from args via the identical
args.split_first().ok_or_else(|| eyre!("empty cmd"))? line before
diverging on stdio handling. Factor that into new_command, returning
(program, Command) for each to finish configuring.
CI's `cargo x android-apex` never passes `--release`, so binaries were built with the default dev profile: full per-variable debug info, unstripped. That alone bloated total APEX size to ~900MB. Scoping line-tables-only debuginfo plus a debuginfo strip to just this target gets it under 200MB, without touching debug info for host builds.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 39 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
xtask/src/cmd/android.rs:247
- [P0]
out_diris caller-controlled, so--out-dir .(or/tmp,/, etc.) recursively deletes unrelated data here. Preserve the directory and remove only stale generated.apexfiles, or reject unsafe/non-dedicated paths before cleanup.
xtask/src/cmd/android.rs:173 - [P1] Every uploaded APEX contains this explicitly non-working init definition: the placeholder class/user/group and nonexistent SELinux domain prevent Android init from starting the service. The catch-all placeholder
file_contextsbelow also lacks a real policy type. Do not publish these as build artifacts until package-specific init configuration and matching device sepolicy are supplied, or make the CI output clearly non-deployable payload-only test data.
| for pkg in md.workspace_packages() { | ||
| if excluded.iter().any(|e| e == pkg.name.as_str()) { | ||
| continue; | ||
| } | ||
| let binaries: Vec<&str> = pkg |
cmd() accepts only '&str', but sometimes we want to pass a String or Path and we need to convert them in &str. That is not convenient. Create a macro to convert various types (&str, Path, PathBuf) into &OsStr and feed that in Command. Example: // before cmd(&["cargo", "zigbuild", "--target", target.as_str(), "--release", "-p", pkg.as_str()])?; // after cmd(&args!["cargo", "zigbuild", "--target", &target, "--release", "-p", &pkg])?;
apexer's apex_manifest.py rejects a `version` of 0 ("version field is
required"). xtask/src/cmd/android.rs derives that field from each crate's
own Cargo.toml semver, so any crate still at the 0.0.0 placeholder failed
`cargo x android-apex`. Bump orb-jwk-util, orb-speed-test,
orb-ota-backend, orb-se050, orb-jobs-agent, orb-zbus-proxies-cli,
orb-backend-status, and orb-se050-reprovision to 0.0.1.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
9431a83 to
191d6b0
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 52 out of 53 changed files in this pull request and generated 4 comments.
Suppressed comments (3)
xtask/src/cmd/android.rs:470
- [P1]
adb installcommonly emitsFailure [INSTALL_FAILED_*]on stdout, but this branch examines only stderr. The expected marker can therefore be missed and the function returns before callingseed_apex_on_device; combine both streams asdisable_verityalready does.
xtask/src/cmd/android.rs:428 - [P2] This second
adb rootcan also restart adbd (notably after the preceding reboot), whileremountruns immediately. Wait for reconnection first or fresh-device seeding will be timing-dependent.
xtask/src/cmd/android.rs:431 - [P1]
wait-for-deviceonly waits for adbd, not for Android boot completion or the Package Manager service. Returning here makes the immediate retry at line 480 race boot and fail with a missing package service on first-time installs. Add a bounded wait forsys.boot_completed=1(and package service readiness) before returning.
| serde_json::to_vec_pretty(&json!({ | ||
| "name": apex_name, | ||
| "version": version_code, | ||
| "versionName": version.to_string(), | ||
| }))?, |
| /// itself, so no separate check is needed. Needs a userdebug/eng build. | ||
| #[cfg(all(target_os = "linux", target_arch = "x86_64"))] | ||
| fn disable_verity() -> Result<()> { | ||
| cmd(&["adb", "root"])?; |
| Runs `android-apex` under the hood, then `adb install -t -r -g --stage`s | ||
| each resulting `.apex`, so it's usable immediately - no reboot required. |
| } | ||
| }, | ||
| "initializeCommand": "mkdir -p ${localEnv:HOME}/.config/nix ${localEnv:HOME}/.aws", | ||
| "initializeCommand": "mkdir -p ${localEnv:HOME}/.config/nix ${localEnv:HOME}/.aws && { adb kill-server; adb -a start-server; true; }", |
Build the projects for 🤖