diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 7e90a6ba..920bf112 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -39,6 +39,9 @@ jobs: - '.gitmodules' - 'cpp/**' - 'android/jni/**' + # The release flag sets test-core-platform-flags.sh mirrors. + - 'react-native-update.podspec' + - 'harmony/pushy/src/main/cpp/CMakeLists.txt' - 'scripts/test-*' - 'scripts/lib/**' # package.json `files` lists the submodule sources that ship. @@ -120,6 +123,11 @@ jobs: - name: Run patch_core unit tests (sanitized) run: SANITIZE=1 ./scripts/test-patch-core.sh + # The tests above build the cores one way; the shipped binaries build + # them with three other flag sets (no RTTI on Android, C++20 on iOS). + - name: Shared cores compile with every platform's release flags + run: ./scripts/test-core-platform-flags.sh + # Short bounded fuzz pass over the two untrusted-input parsers # (flow_json, hbc_transform_wire). Skips with a notice when clang or # libFuzzer is unavailable on the runner. diff --git a/cpp/patch_core/error_codes.h b/cpp/patch_core/error_codes.h index 8edf8f9a..7043a90b 100644 --- a/cpp/patch_core/error_codes.h +++ b/cpp/patch_core/error_codes.h @@ -37,8 +37,10 @@ constexpr const char* kSwitchVersionFailed = "SWITCH_VERSION_FAILED"; constexpr const char* kMarkSuccessFailed = "MARK_SUCCESS_FAILED"; // reloadUpdate / restartApp failed. constexpr const char* kRestartFailed = "RESTART_FAILED"; -// resetToPackagedBundle failed (state wipe or cleanup could not run), or the -// installed native module predates the method (JS-layer detection). +// resetToPackagedBundle could not persist the reset state, or the installed +// native module predates the method (JS-layer detection). Deleting the +// downloaded versions afterwards is best-effort on every platform and never +// reports this code: the reset is complete once the state is. constexpr const char* kResetFailed = "RESET_FAILED"; // Stored or provided hash info is not a valid JSON object. constexpr const char* kInvalidHashInfo = "INVALID_HASH_INFO"; diff --git a/ios/RCTPushy/RCTPushy.mm b/ios/RCTPushy/RCTPushy.mm index 59afbc9c..95ecea7d 100644 --- a/ios/RCTPushy/RCTPushy.mm +++ b/ios/RCTPushy/RCTPushy.mm @@ -457,13 +457,18 @@ - (NSDictionary *)constantsToExport return; } + // The reset is done once the state is: the next launch resolves to the + // packaged bundle. Settle now, as Android and Harmony do; deleting the + // downloaded versions is best-effort below and never delays the caller + // (it queues behind any unzip/patch on the file queue). A directory a + // failed delete leaves behind is no longer referenced by any state, so + // the regular cleanup removes it later. + resolve(@true); + dispatch_async(PushyFileQueue(), ^{ // maxAgeDays=0: remove every downloaded entry except the running // version's directory (cleaned up by the next regular cleanup). The - // state was just wiped, so that is the only name left to keep. The - // promise settles here, after the cleanup, so a failed wipe reaches - // JS as RESET_FAILED (Android parity) instead of a warning nobody - // aggregates. + // state was just wiped, so that is the only name left to keep. pushy::delta::Status status = pushy::delta::Status::Ok(); try { status = pushy::delta::CleanupOldEntries( @@ -476,12 +481,7 @@ - (NSDictionary *)constantsToExport } if (!status.ok) { RCTLogWarn(@"Pushy reset cleanup error: %s", status.message.c_str()); - PushyRejectError(reject, PushyErrorWithCode( - pushy::error_codes::kResetFailed, - [NSString stringWithUTF8String:status.message.c_str()] ?: @"reset cleanup failed")); - return; } - resolve(@true); }); } diff --git a/package.json b/package.json index 86caf06d..7ac5244c 100644 --- a/package.json +++ b/package.json @@ -53,6 +53,7 @@ "test": "bun test src/__tests__", "test:patch-core": "./scripts/test-patch-core.sh", "test:flow-core": "./scripts/test-update-flow-core.sh", + "test:core-flags": "./scripts/test-core-platform-flags.sh", "generate:flow-vectors": "bun scripts/generate-flow-vectors.ts", "build:harmony-har": "node scripts/build-harmony-har.js", "build:harmony-e2e": "bash Example/e2etest/scripts/build-harmony-e2e.sh", diff --git a/scripts/contract.ts b/scripts/contract.ts index 7fc9f290..071ca87d 100644 --- a/scripts/contract.ts +++ b/scripts/contract.ts @@ -64,8 +64,10 @@ export const errorCodes: ErrorCode[] = [ name: 'ResetFailed', value: 'RESET_FAILED', doc: - 'resetToPackagedBundle failed (state wipe or cleanup could not run), or the ' + - 'installed native module predates the method (JS-layer detection).', + 'resetToPackagedBundle could not persist the reset state, or the installed ' + + 'native module predates the method (JS-layer detection). Deleting the ' + + 'downloaded versions afterwards is best-effort on every platform and never ' + + 'reports this code: the reset is complete once the state is.', }, { name: 'InvalidHashInfo', diff --git a/scripts/test-core-platform-flags.sh b/scripts/test-core-platform-flags.sh new file mode 100755 index 00000000..ebcbe8fb --- /dev/null +++ b/scripts/test-core-platform-flags.sh @@ -0,0 +1,55 @@ +#!/bin/sh +# Compiles every shared core source (cpp/patch_core, cpp/update_flow_core) +# with each platform's release flags, warnings as errors, syntax only. The +# unit tests build the cores one way (host c++, C++17, RTTI, asserts on); +# the shipped binaries build them three other ways, so a construct only one +# platform rejects (RTTI under Android's -fno-rtti, C++20 changes under the +# iOS standard RN selects, code that only warns with NDEBUG) would otherwise +# first fail in that platform's build. +# +# Keep each set in step with its build: +# android android/jni/Application.mk (APP_CPPFLAGS) +# ios react-native-update.podspec (Core compiler_flags; the C++ +# standard is React Native's, C++20 since RN 0.73) +# harmony harmony/pushy/src/main/cpp/CMakeLists.txt (release) +# Flag variables must split into compiler arguments. +# shellcheck disable=SC2086 +set -eu + +# Intentional one-command CDPATH assignment. +# shellcheck disable=SC1007 +ROOT_DIR="$(CDPATH= cd -- "$(dirname "$0")/.." && pwd)" +BUILD_DIR="$ROOT_DIR/.tmp/core-platform-flags" +mkdir -p "$BUILD_DIR" + +# CORE_WARNINGS (-Werror). +# shellcheck source-path=SCRIPTDIR source=lib/cpp-test-common.sh +. "$ROOT_DIR/scripts/lib/cpp-test-common.sh" + +# Every shipped binary is built with clang; prefer it over the host c++. +CXX="${CXX:-}" +if [ -z "$CXX" ]; then + if command -v clang++ >/dev/null 2>&1; then CXX=clang++; else CXX=c++; fi +fi + +failed=0 +check() { + platform="$1" + flags="$2" + for source in "$ROOT_DIR"/cpp/patch_core/*.cpp "$ROOT_DIR"/cpp/update_flow_core/*.cpp; do + if ! "$CXX" -fsyntax-only $CORE_WARNINGS $flags "$source"; then + echo "[FAIL] $platform ($flags): ${source#"$ROOT_DIR"/}" + failed=1 + fi + done + echo "[done] $platform: $flags" +} + +check android "-std=c++17 -fexceptions -fno-rtti -DNDEBUG -fvisibility=hidden -fvisibility-inlines-hidden" +check ios "-std=c++20 -DNDEBUG -fvisibility=hidden -fvisibility-inlines-hidden" +check harmony "-std=c++17 -DNDEBUG -fvisibility=hidden -fvisibility-inlines-hidden" + +if [ "$failed" != 0 ]; then + exit 1 +fi +echo "shared cores compile with every platform's release flags ($CXX)" diff --git a/scripts/tests/ios-purge-restore/purge_restore_test.mm b/scripts/tests/ios-purge-restore/purge_restore_test.mm index 878dc9f2..b0d79096 100644 --- a/scripts/tests/ios-purge-restore/purge_restore_test.mm +++ b/scripts/tests/ios-purge-restore/purge_restore_test.mm @@ -70,6 +70,8 @@ static void Await(dispatch_semaphore_t gate, const char *description) { static uint64_t requestGeneration; static BOOL roundCommitted, roundActivated; static BOOL resetResolved; +// Signalled by the reset's (failing) cleanup stub below. +static dispatch_semaphore_t cleanupRan; static int scheduleCount; static int64_t waitBudget; static NSTimeInterval processElapsed; @@ -169,10 +171,14 @@ static void PushyRejectError(RCTPromiseRejectBlock reject, NSError *error) { // Filesystem cleanup is not under test. Keep the real declaration/return type; // the real reset body (including its asynchronous completion) still executes. namespace pushy { namespace delta { +// The reset's best-effort delete fails here on purpose: the reset is complete +// once the state is, so its promise must resolve regardless (a reset that +// still waited for the delete would reject, failing the test's rejecter). Status CleanupOldEntries(const std::string&, const std::vector&, int max_age_days, std::time_t) { Expect(max_age_days == 0, "reset requests a full cleanup"); - return {true, ""}; + Signal(cleanupRan); + return {false, "injected cleanup failure"}; } }} @@ -347,6 +353,7 @@ static void SetUp(Order selected) { workerSettled = dispatch_semaphore_create(0); resetAttempted = dispatch_semaphore_create(0); resetFinished = dispatch_semaphore_create(0); + cleanupRan = dispatch_semaphore_create(0); roundCommitted = NO; roundActivated = YES; // rejected commits must explicitly overwrite this resetResolved = NO; @@ -465,7 +472,7 @@ static void CommitBeforeSignal(void) { static void ResetOrdering(Order selected) { SetUp(selected); NSURL *url = [RCTPushy bundleURL]; - Expect(resetResolved, "reset promise must resolve after its file queue work"); + Expect(resetResolved, "reset promise must resolve once the state is reset"); Expect(pushyResetGeneration.load() == 1 && requestGeneration == 0, "reset must invalidate the captured generation"); if (selected == Order::ResetFirst) { Expect(!roundCommitted && !roundActivated, "stale generation must reject ALL commit writes"); @@ -476,6 +483,9 @@ static void ResetOrdering(Order selected) { Expect(Value(PushyHashInfoKey(@"B")) == nil, "reset must leave no version metadata"); Expect(Value(keyNativeCheckCache) == nil, "reset must leave no response cache"); Expect([Value(keyUuid) isEqual:@"installation-uuid"], "reset must preserve install identity"); + // The failed delete ran after the promise had already resolved, and did + // not turn it into a rejection. + Await(cleanupRan, "reset still deletes the downloaded versions"); Finish(); } diff --git a/src/clientActivation.ts b/src/clientActivation.ts index a6846ba0..288bf6ce 100644 --- a/src/clientActivation.ts +++ b/src/clientActivation.ts @@ -265,10 +265,11 @@ export class Activation { }; /** - * Reset to the bundle packaged in the binary: wipes every downloaded update - * and the whole update state on the native side, so the app loads the - * built-in bundle on the next launch (or immediately with - * `{ restart: true }`). The client uuid is preserved. + * Reset to the bundle packaged in the binary: wipes the whole update state + * on the native side, so the app loads the built-in bundle on the next + * launch (or immediately with `{ restart: true }`), and deletes the + * downloaded updates in the background (best-effort; the promise does not + * wait for it). The client uuid is preserved. * * Returns whether the reset actually happened. Like the other update-flow * APIs it never throws by default — failures land in lastError/onError with