Commit Graph

15 Commits

Author SHA1 Message Date
Eric Seidel 8649c75206 perf: mmap libapp.so out of the APK instead of buffering in RAM (#354)
* perf: mmap libapp.so out of the APK instead of buffering in RAM

`open_base_lib` previously read the entire decompressed libapp.so into a
Vec<u8> via `read_to_end` and handed bipatch a Cursor over that buffer.
For large apps libapp.so can be tens of megabytes, and that allocation
happens immediately after the patch download is buffered to disk — a
plausible OOM trigger on memory-constrained devices (we have at least
one customer report on OnePlus where the patch install appears to halt
silently right after the download completes).

Modern AGP (3.6+) defaults to extractNativeLibs=false, which keeps
libapp.so STORED uncompressed inside the APK so the dynamic linker can
mmap it directly. When that's the case, do the same: find the entry's
data offset via the zip crate, drop the archive, reopen the APK, and
mmap the entry's slice. Cursor<Mmap> implements Read + Seek, which is
what bipatch's `Reader::new(patch, base)` requires.

When the entry isn't stored uncompressed (older builds, or builds that
explicitly compress native libs), fall back to the previous buffered
read so we always succeed.

Mmap doesn't change the peak working set when bipatch traverses the
whole base linearly, but file-backed mappings are clean and reclaimable
under memory pressure where an anonymous Vec is not, and we avoid the
~2x transient allocation peak from `read_to_end` growing the buffer.

Tests cover both paths (stored → mmap, deflated → buffered) on the host.

* ci: add mmap, memmap, SIGBUS to cspell dictionary
2026-05-05 17:33:22 -07:00
Eric Seidel 8072ed9ad7 feat: stage 1 integration test harness (#353)
* docs: integration tests design proposal

Adds docs/integration_tests.md proposing a desktop-only integration suite
that drives the Dart `ShorebirdUpdater` API against the real Rust core via
FFI, with a fake HTTP server and per-test tempdir state. Describes a new
`library_test_hooks` cdylib that reaches into `updater` via a `test-hooks`
Cargo feature, so production C API stays clean. Status: design exploration,
not a committed plan.

* feat: stage 1 integration test harness for shorebird_code_push

Lands the test_hooks crate, the Dart-side loader, and one trivial
end-to-end test, per docs/integration_tests.md stage 1.

Pieces:

- New `library_test_hooks` workspace crate (cdylib). Depends on `updater`
  with a new `test-hooks` Cargo feature that widens the visibility of
  internal items (currently `testing_reset_config`) for sibling-crate
  access only — production builds do not enable it. The crate also
  re-exports `updater::c_api::dart::*` and `updater::c_api::engine::*`,
  which keeps the rlib's `#[no_mangle]` symbols out of DCE so the
  resulting cdylib carries both the production C API and the new
  `shorebird_test_*` hooks.
- Workspace `default-members` excludes `library_test_hooks` so plain
  `cargo build` / `cargo test` invocations don't unify the `test-hooks`
  feature into production builds.
- `shorebird_code_push/test/integration/all_test.dart` (single file,
  with a header comment explaining why) loads the cdylib via
  `DynamicLibrary.open`, reassigns the existing `@visibleForTesting`
  `Updater.bindings` setter, and exercises both surfaces. The build
  helper shells out to `cargo build -p library_test_hooks`; if it
  fails, `markTestSkipped` keeps the suite green on machines without
  a working Rust toolchain.
- ffigen config (`ffigen_test_hooks.yaml`) generates the test-only
  Dart bindings under `test/integration/generated/`, not `lib/`.
- CI: `library_test_hooks` is added to the rust_crate matrix, and the
  shorebird_code_push job triggers on `library/**` and
  `library_test_hooks/**` so cdylib changes can't break the Dart-side
  integration suite without CI noticing.

Verified locally: 232 Rust unit tests + 42 Dart tests (41 existing + 1
integration) green; clippy/fmt/cspell clean; production cdylib does
not contain `shorebird_test_reset` (`nm` confirms feature isolation).

Stages 2 (FakePatchServer + golden path) and 3 (adversarial scenarios)
land separately.

* fix(integration test): early-return on skip and bump per-test timeout

Two issues caught by Shorebird CI on PR #353:

1. `markTestSkipped` does not abort test execution — it only flags the
   test as skipped on its way out. The body kept running and crashed on
   the `late testHooks` field when the cdylib build had failed in
   setUpAll. Move the skip check into the test body itself with an
   early return; drop the (no-op) skip in `setUp`.

2. Default per-test timeout (30s) also covers `setUpAll`. A cold
   `cargo build -p library_test_hooks` compiles `updater` and ~100
   transitive deps, which can run minutes on CI. Bump to 10 minutes
   via `@Timeout` on the library.

Verified locally: passes when cargo is on PATH (1 passed), reports a
clean skip and exit 0 when cargo is removed from PATH (1 skipped).

* refactor(integration test): drop ! by promoting testHooks to late final

`testHooks` was nullable so accessing it after the skipReason check
required `!`, and `markTestSkipped(skipReason!)` had the same smell.
Make `testHooks` `late final` (non-nullable, throws if read before
setUpAll assigns) and pull `skipReason` into a non-null local before
use. Same control flow, no bang operators.
2026-05-05 16:32:57 -07:00
Eric Seidel a1f2a9b8bc ci: speed up Rust CI builds (#327)
* ci: speed up Rust CI builds with caching and prebuilt tools

The Windows builder takes ~6min vs ~3min on Ubuntu, largely because
cargo-llvm-cov is compiled from source on every run. This adds
Swatinem/rust-cache for build artifact caching and switches to
taiki-e/install-action for prebuilt cargo-llvm-cov binaries.

Also fixes the undefined `inputs.shell` references by hardcoding `bash`.

* fix: add Swatinem and taiki to cspell dictionary

* ci: remove rust-cache, no measurable benefit for this project

Investigated cache hit/miss across 3 CI runs. Even with warm cache
hits (macOS/Windows), build times were unchanged or slightly slower
due to cache download/extract overhead (~39MB) outweighing the small
dependency compile time savings.

* ci: remove redundant cargo build step

Clippy already compiles all targets, so the explicit cargo build step
was redundant. The test step (cargo llvm-cov) also does its own
instrumented build, so nothing depended on the build step's artifacts.

* ci: try larger Windows runner for faster Rust builds

Switch from windows-latest (4 vCPU) to windows-latest-large (8 vCPU)
to see if the extra cores meaningfully speed up Rust compilation.
Windows builds currently take ~2x longer than macOS/Ubuntu.

* ci: use Namespace Windows runner (8 vCPU) instead of GitHub-hosted

Switch to nscloud-windows-2022-amd64-8x16 to test whether doubling
the cores from 4 to 8 meaningfully speeds up Rust compilation on
Windows, which currently takes ~2x longer than macOS/Ubuntu.

* fix: add nscloud to cspell dictionary
2026-04-02 20:34:11 +00:00
Eric Seidel 07fa11c9d9 perf: add release profile to reduce binary size (#328)
* perf: add release profile to reduce binary size

Add workspace-level release profile with size optimizations:
- opt-level = "z" (optimize for size)
- lto = true (cross-crate link-time optimization)
- codegen-units = 1 (better whole-program optimization)
- strip = "debuginfo" (remove debug info, preserve C API symbols)

Reduces .text section by ~32% (1.9 MiB → 1.3 MiB). The .a file
grows due to LTO bitcode embedding, but the final linked binary
(libflutter.so) will be smaller when the engine consumes it.

* chore: add codegen and debuginfo to spell check dictionary
2026-04-02 13:29:34 -07:00
Eric Seidel 0af491b628 ci: speed up Rust CI builds with prebuilt tools (#325)
* ci: speed up Rust CI builds with caching and prebuilt tools

The Windows builder takes ~6min vs ~3min on Ubuntu, largely because
cargo-llvm-cov is compiled from source on every run. This adds
Swatinem/rust-cache for build artifact caching and switches to
taiki-e/install-action for prebuilt cargo-llvm-cov binaries.

Also fixes the undefined `inputs.shell` references by hardcoding `bash`.

* fix: add Swatinem and taiki to cspell dictionary

* ci: remove rust-cache, no measurable benefit for this project

Investigated cache hit/miss across 3 CI runs. Even with warm cache
hits (macOS/Windows), build times were unchanged or slightly slower
due to cache download/extract overhead (~39MB) outweighing the small
dependency compile time savings.
2026-04-02 03:14:24 +00:00
Eric Seidel 50954da68a style: fix clippy warnings and add clippy check to CI (#321)
* style: fix clippy warnings and add clippy check to CI

Runs `cargo clippy --all-targets -- -D warnings` in the rust_crate CI
action to catch lint issues before they land.

Fixes: redundant field names, needless returns, unnecessary closures,
io_other_error, needless borrows, single_match, unnecessary_unwrap,
and adds missing safety docs.

* chore: add 'clippy' to cspell dictionary
2026-04-02 02:02:05 +00:00
Eric Seidel 463ace0c56 feat: add resumable downloads with streaming to disk (#313)
* feat: add resumable downloads with streaming to disk

Previously, failed patch downloads were lost entirely — the updater
would re-download from scratch on every retry, creating a doom loop for
users on poor networks. Downloads were also fully buffered in memory.

This changes the download abstraction from returning bytes in memory
(`DownloadFileFn`) to streaming directly to disk with resume support
(`DownloadToPathFn`). The new signature accepts a `resume_from` byte
offset and the default implementation uses HTTP Range headers.

Key changes:
- DownloadToPathFn streams to file, sends Range header when resuming
- DownloadResult returns total_bytes and content_length from server
- DownloadState sidecar JSON tracks URL/patch/size for resume decisions
- compute_resume_offset detects valid partial downloads to resume
- Post-download size validation catches truncated downloads
- cleanup_download_artifacts removes compressed files + sidecars after
  successful install (fixes pre-existing leak of download artifacts)

The fn-pointer signature maps directly to a future C callback for
platform-native download backends (iOS NSURLSession, Android
DownloadManager).

* fix: address self-review issues in download implementation

- Parse Content-Range header for 206 responses to get total file size
  (reqwest's content_length() only returns the partial body size)
- Only append to existing file when server actually returns 206, not
  just when resume_from > 0 (handles servers that ignore Range header)
- Remove incorrect resume_from offset addition to expected_size
- Clean up download artifacts on size mismatch before bailing
- Restore parent directory creation in download_to_path wrapper

* refactor: remove expected_hash from DownloadState

The hash stored in the sidecar was the *inflated* file hash from the
server response — it couldn't validate the compressed partial download
and was never used for resume decisions. The URL match already
determines whether to resume, and the real hash check happens after
inflate using the fresh server response. Simplifies the sidecar to
just url, patch_number, and expected_size.

* test: add coverage for resumable downloads + review fixes

- Add 12 new tests covering:
  - compute_resume_offset: no sidecar, matching sidecar, mismatched URL,
    empty file
  - cleanup_download_artifacts: removes file+sidecar, noop when missing
  - Integration: successful update cleans up artifacts
  - Integration: partial download resumes via 206 with mockito
  - Integration: URL change triggers fresh download
  - parse_content_range_total: valid, missing header, unknown size (*)

- Extract parse_content_range_total as testable helper (was inline chain)
- Add expected_hash back to DownloadState — catches the case where a
  patch is deleted and re-added with same number but different content
- Add "rsplit" to cspell config

* feat: add orphan cleanup for download directory + WriteFile comment

Scan the download directory before each download and remove any files
that don't belong to the current patch number. We own this directory
entirely, so anything from a prior patch, a crashed inflate (.full),
or an unrecognized file is safe to delete. This prevents gradual
accumulation of orphaned partial downloads over time.

Also adds a TODO comment explaining the reuse of WriteFile context for
seek operations (FileOperation lacks a SeekFile variant).

* test: add coverage for hash mismatch, patch number mismatch, corrupt sidecar

- compute_resume_offset_mismatched_hash: same URL but hash changed
  (patch deleted and re-added), verifies fresh download
- compute_resume_offset_mismatched_patch_number: sidecar for different
  patch number, verifies fresh download
- compute_resume_offset_corrupt_sidecar: garbage JSON in sidecar,
  verifies graceful fallback to fresh download

* test: cover download size mismatch and unknown content-length paths

- update_fails_on_download_size_mismatch: mock returns content_length
  that doesn't match total_bytes, verifies error + artifact cleanup
- update_succeeds_when_content_length_unknown: verifies the size
  validation is skipped when content_length is None

* fix: replace fake hash strings to pass cspell

* chore: remove unnecessary TODO comment about FileOperation::SeekFile

* refactor: panic in test download mocks that should never be called

Tests where the download is never reached (patch check fails or no
patch available) now panic instead of returning dummy data, making it
explicit that the mock shouldn't be invoked.

* refactor: add UNEXPECTED_DOWNLOAD/UNEXPECTED_REPORT test constants

Shared panicking constants for test mocks that should never be called.
Tests use these by name instead of writing inline panic closures,
making intent clearer and avoiding uncoverable dead code in closures.

* test: add coverage for handle_download_result

Tests for the download-specific HTTP response handler:
- 200 OK: accepted
- 206 Partial Content: accepted (for resumed downloads)
- 500: rejected with error message

* fix: update missed download fn signatures in tests

Two test sites in updater.rs were not updated to the new
DownloadToPathFn 3-argument signature:
- set_noop_network_hooks in multi_engine_tests used old 1-arg closure
- update_starts_fresh_when_url_changes had unused patch_bytes variable

* test: add coverage for handle_download_result error branches

Mirror the existing handle_network_result_no_internet and
handle_network_result_unknown_error tests for the download variant.
These exercise the connection error and builder error paths in
handle_download_result that were previously uncovered.

* fix: adapt resumable downloads to ureq (post-rebase cleanup)

- Replace reqwest with ureq for download_to_path_default
- Remove handle_download_result (ureq's handle_network_result handles 206)
- Fix TempDir::new("prefix") → TempDir::new() for tempfile crate
- Remove unused Read/Write imports
2026-04-01 15:51:16 +00:00
Eric Seidel c6647a2dfe refactor: replace reqwest with ureq to reduce binary size (#317)
* refactor: replace reqwest with ureq to reduce binary size

reqwest's blocking API is built on top of its async implementation,
pulling in tokio, hyper, futures, and ~84 other transitive dependencies
even though we only make simple synchronous HTTP calls.

ureq is a synchronous-only HTTP client that eliminates the async
runtime entirely. This reduces transitive dependencies from 227 to 143
and the linked dylib from 4.5 MB to 3.7 MB (-18%). The .a archive
drops from 28 MB to 26 MB, but real savings will be larger once
linked into libflutter with dead code stripping.

The network API surface is unchanged — three functions (patch check,
file download, event reporting) using POST/GET with JSON.

* chore: add ureq to spell check dictionary

* refactor: use into_body() instead of body_mut() where response is consumed

* fix: simplify network error matching to avoid fragile string checks

Consolidate HostNotFound, ConnectionFailed, and all Io errors into
a single network-error arm instead of pattern-matching on error
message strings that could change across OS versions or locales.

* chore: add TODO for misleading network error message
2026-04-01 08:23:48 -07:00
Eric Seidel 4ff2839cdb feat: add tests and docs for boot state machine (#309)
* test: test api calls

* chore: add docs

* chore: fix cspell

* fix: use no-op network hooks in multi_engine tests

Set no-op network hooks after init_for_testing so the fire-and-forget
thread spawned by report_launch_success completes instantly without
network I/O, preventing leaked threads from interfering with subsequent
serial tests that use mock servers.
2026-03-30 23:01:50 +00:00
Eric Seidel 6d0e4a1193 chore(deps): bump Rust and Dart dependencies (#315)
* chore(deps): bump Rust and Dart dependencies

Bump Rust dependencies in library/ and patch/:
- comde: 0.2.3 → 0.3.1 (library), 0.2.3 → 0.3.0 (patch)
- zip: 0.6.4 → 3.0.0 (breaking: FileOptions → SimpleFileOptions)
- android_logger: 0.13.0 → 0.15.0
- mockall: 0.12.1 → 0.13.1
- serial_test: 2.0.0 → 3.2.0
- cbindgen: 0.24.0 → 0.28.0

Bump Dart dev dependency in shorebird_code_push/:
- ffigen: upper bound <17.0.0 → <19.0.0

Updated zip API usage (FileOptions → SimpleFileOptions) and
adjusted test assertion for changed error message.

Binary size impact (macOS release, arm64):
- libupdater.a: +83 KB (+0.28%)
- libupdater.dylib: +34 KB (+0.75%)

Closes #206, #271, #273.

* chore: add EOCD to cspell dictionary

The zip 3.0 crate changed its error message to reference "EOCD"
(End of Central Directory), which cspell doesn't recognize.
2026-03-30 15:50:44 -07:00
Brandon DeRosier eeec42efb7 feat: add enhanced error messages for file operations (#310)
* feat: add enhanced error messages for file operations

Add a file_errors module that provides context-aware error messages
for file operations. When file operations fail, users now see:
- The specific operation that failed (create, read, write, rename, etc.)
- The full path involved
- Helpful hints about possible causes based on error type
- Android-specific hints for permission errors (SELinux, Work Profile,
  MDM/Knox policies, app cloning features)

This helps diagnose issues like "Permission denied (os error 13)" by
indicating which operation failed and suggesting possible causes.
2026-02-04 12:36:39 -08:00
Felix Angelov b2fbf7c3ee chore: various platforms updates (#295) 2025-09-17 16:48:24 -05:00
Bryan Oltman a6c761f50f fix: build updater with static runtime to prevent flutter engine build linking errors (#254)
* fix: build updater with static runtime to prevent flutter engine build linking errors

* Update library/.cargo/config.toml

Co-authored-by: Eric Seidel <eric@shorebird.dev>

* cspell

---------

Co-authored-by: Eric Seidel <eric@shorebird.dev>
2024-12-17 21:34:20 +00:00
Eric Seidel 6298d37d86 chore: add cspell checking and make pass (#244)
* chore: add cspell checking and make pass

* chore(shorebird_code_push): minor improvements to example (#242)

* chore(shorebird_code_push): v2.0.2 (#243)

* chore: fix cspell

---------

Co-authored-by: Felix Angelov <felix@shorebird.dev>
2024-11-20 13:50:16 -05:00
Eric Seidel 2ae6afbb65 fix: unbreak web build for 2.x (#241)
* fix: unbreak web build

* chore: add cspell config
2024-11-20 11:35:49 -06:00