From aeb7e3fbe2b26507e0aa6c35f58e154390525753 Mon Sep 17 00:00:00 2001 From: Bryan Chan Date: Fri, 21 Aug 2026 14:13:57 -0700 Subject: [PATCH 1/3] =?UTF-8?q?ADFA-4128:=20qb=2002/12=20plumbing=20?= =?UTF-8?q?=E2=80=94=20Host-side=20groundwork:=20feature=20flag,=20asset?= =?UTF-8?q?=20staging,=20build-service=20hooks,=20shared=20utilities?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Kj9YeCDHGp9DU8LPtfWJ7W --- .github/workflows/analyze.yml | 6 + .gitignore | 2 + ARCHITECTURE.md | 31 +- build-info/build.gradle.kts | 5 +- .../androidide/managers/ToolsManager.java | 58 +++- .../itsaky/androidide/models/SaveResult.java | 26 +- .../itsaky/androidide/utils/FeatureFlags.kt | 97 +++++-- .../androidide/utils/FlashbarActivityUtils.kt | 17 +- .../utils/KeyedDebouncingActionCancelTest.kt | 73 +++-- .../plugins/conf/AndroidModuleConf.kt | 3 + .../plugins/conf/MavenPublishConf.kt | 171 +++++------ .../editor/utils/ContentReadWrite.kt | 272 +++++++++--------- .../tooling/api/GradlePluginConfig.java | 15 + gradle/libs.versions.toml | 9 + .../androidide/idetooltips/TooltipTag.kt | 1 + .../androidide/logging/utils/LogUtilsTest.kt | 49 ++++ .../src/main/res/drawable/ic_quick_build.xml | 12 + .../res/drawable/ic_quick_build_building.xml | 15 + .../drawable/ic_quick_build_building_arc.xml | 15 + .../drawable/ic_quick_build_building_stop.xml | 13 + .../res/drawable/ic_quick_build_error.xml | 31 ++ .../res/drawable/ic_quick_build_outline.xml | 17 ++ resources/src/main/res/values/strings.xml | 60 ++++ .../itsaky/androidide/flashbar/Flashbar.kt | 2 +- .../androidide/projects/ProjectManagerImpl.kt | 40 ++- .../projects/builder/BuildService.kt | 13 + .../JarFsClasspathReaderCorruptJarTest.kt | 162 +++++------ .../app/TermuxServiceShellManagerNpeTest.java | 13 +- 28 files changed, 818 insertions(+), 410 deletions(-) create mode 100644 logger/src/test/java/com/itsaky/androidide/logging/utils/LogUtilsTest.kt create mode 100644 resources/src/main/res/drawable/ic_quick_build.xml create mode 100644 resources/src/main/res/drawable/ic_quick_build_building.xml create mode 100644 resources/src/main/res/drawable/ic_quick_build_building_arc.xml create mode 100644 resources/src/main/res/drawable/ic_quick_build_building_stop.xml create mode 100644 resources/src/main/res/drawable/ic_quick_build_error.xml create mode 100644 resources/src/main/res/drawable/ic_quick_build_outline.xml diff --git a/.github/workflows/analyze.yml b/.github/workflows/analyze.yml index e246c78621..9e6bfb82cc 100644 --- a/.github/workflows/analyze.yml +++ b/.github/workflows/analyze.yml @@ -105,6 +105,12 @@ jobs: # the unit-test compile path. FIREBASE_CONSOLE_URL: ${{ secrets.FIREBASE_CONSOLE_URL }} GLITCHTIP_DSN: ${{ secrets.GLITCHTIP_DSN }} + # The aapt2/d8/Compose regression tests (ADFA-4128 bugs 5/6/8) are + # assumption-guarded, so on a runner without an Android SDK they would skip + # green and take that coverage with them. This turns an absent toolchain + # into a hard failure instead. The runner does have an SDK - Assemble V8 + # Debug above could not run otherwise. + REQUIRE_BUILD_TOOLCHAIN: "1" run: flox activate -d flox/base -- ./gradlew :testing:tooling:assemble :testing:common:assemble sonarqube --info --no-build-cache -x lint --continue - name: Upload JaCoCo report diff --git a/.gitignore b/.gitignore index af44d5bf1c..51d7d76732 100755 --- a/.gitignore +++ b/.gitignore @@ -111,6 +111,8 @@ tests/test-home /composite-builds/build-deps/build/ /app/google-services.json +/app/keystore-debug.jks + # Kotlin build files .kotlin/ diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 59df122920..6373861748 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -59,6 +59,7 @@ Strategy: **layer-and-subsystem based**, not feature-by-feature. The Gradle buil |---|---|---| | Application | `app` | The IDE itself — activities, fragments, services, DI, agent, web server. Wires everything together. | | Build engine | `subprojects:tooling-api*`, `gradle-plugin*`, `subprojects:projects`, `subprojects:builder-model-impl` | Runs a real Gradle build of the user's project out-of-process and streams events back. | +| Quick Build (experimental, ADFA-4128) | `quickbuild:core`, `quickbuild:daemon`, `quickbuild:protocol`, `quickbuild:runtime` | Live-reloads the user's app on every save in seconds, by running it as a generated proxy app instead of doing a full Gradle rebuild. | | Language tooling | `lsp:{api,java,kotlin,xml,indexing,…}`, `lexers`, `editor*`, `editor-treesitter` | Language servers, indexing, the Sora-based editor and highlighting. | | UI design tooling | `layouteditor`, `uidesigner`, `xml-inflater`, `vectormaster`, `compose-preview` | Visual/XML design surfaces for the *user's* app. | | Shell | `termux:{termux-app,termux-shared,termux-view,termux-emulator}` | Embedded Termux shell and terminal. | @@ -68,6 +69,7 @@ Strategy: **layer-and-subsystem based**, not feature-by-feature. The Gradle buil | Testing | `testing:{android,unit,lsp,tooling,common}` | Shared test harnesses, split by what's under test. | **Dependency rules (enforced):** + - **`app` depends inward; libraries never depend on `app`.** Subsystems are consumed by `app`, not vice versa. - **Vendored forks are substituted, not imported ad hoc.** `composite-builds/build-deps` and `build-deps-common` provide forked `javac`/`jdt`/`layoutlib`/etc.; `settings.gradle.kts` substitutes them in for `com.itsaky.androidide.build:*`. Don't add a Maven coordinate for something already substituted. - **All module config flows through `composite-builds/build-logic`.** Every Android module gets the `v7`/`v8` ABI flavors centrally (`AndroidModuleConf.kt`) — there is no flavorless `assembleDebug`. `:plugin-api` is intentionally excluded from flavors. @@ -87,16 +89,16 @@ These structural facts shape every module. Day-to-day build *commands* live in ` ## Technology Stack -| Concern | Library / Approach | -|---|---| -| UI | **Jetpack Compose for all new UI** ([ADR 0009](docs/adr/0009-jetpack-compose-for-new-ui.md)). The existing majority is still Android Views + Fragments + `RecyclerView` (Material Components); those legacy screens stay until reworked, but new IDE UI is Compose-only. | -| Dependency Injection | **Koin** (`org.koin`) — `coreModule`/`pluginModule`, `startKoin` in `IDEApplication`, plus a `ServiceLocator : KoinComponent` for lazy post-startup access. No Hilt/Dagger. | -| Asynchronous work | **Kotlin Coroutines + Flow** (`StateFlow`/`SharedFlow`, `viewModelScope`, app-scoped `CoroutineScope(SupervisorJob() + Dispatchers.IO)`); **GreenRobot EventBus** for cross-subsystem events. | -| Networking | Offline-first; no general REST layer. External I/O is **Google GenAI SDK** (Gemini), **on-device llama.cpp**, and **JGit** (git). Retrofit is in the catalog but effectively unused in app code. | +| Concern | Library / Approach | +| ---------------------- | ------------------------------------------------------------ | +| UI | **Jetpack Compose for all new UI** ([ADR 0009](docs/adr/0009-jetpack-compose-for-new-ui.md)). The existing majority is still Android Views + Fragments + `RecyclerView` (Material Components); those legacy screens stay until reworked, but new IDE UI is Compose-only. | +| Dependency Injection | **Koin** (`org.koin`) — `coreModule`/`pluginModule`, `startKoin` in `IDEApplication`, plus a `ServiceLocator : KoinComponent` for lazy post-startup access. No Hilt/Dagger. | +| Asynchronous work | **Kotlin Coroutines + Flow** (`StateFlow`/`SharedFlow`, `viewModelScope`, app-scoped `CoroutineScope(SupervisorJob() + Dispatchers.IO)`); **GreenRobot EventBus** for cross-subsystem events. | +| Networking | Offline-first; no general REST layer. External I/O is **Google GenAI SDK** (Gemini), **on-device llama.cpp**, and **JGit** (git). Retrofit is in the catalog but effectively unused in app code. | | Database / Persistence | **Room** is the default for relational/queryable data; **filesystem + preferences (DataStore)** for non-relational settings. **Raw SQLite** (`SQLiteDatabase` / `SupportSQLiteOpenHelper`) only for justified exceptions (see policy below). | -| Serialization | `kotlinx.serialization` and Gson. | -| Parceling | Kotlin **`@Parcelize`** (`kotlin-parcelize` plugin) for `Parcelable` data classes — never hand-implement `Parcelable`. Do it manually only if `@Parcelize` genuinely can't express it (custom serialization logic, unsupported member types). | -| AI agent | Google GenAI (cloud) + llama (local), behind `GeminiRepository` / `SwitchableGeminiRepository`, with planner/critic/executor agents in `agent/repository`. | +| Serialization | `kotlinx.serialization` and Gson. | +| Parceling | Kotlin **`@Parcelize`** (`kotlin-parcelize` plugin) for `Parcelable` data classes — never hand-implement `Parcelable`. Do it manually only if `@Parcelize` genuinely can't express it (custom serialization logic, unsupported member types). | +| AI agent | Google GenAI (cloud) + llama (local), behind `GeminiRepository` / `SwitchableGeminiRepository`, with planner/critic/executor agents in `agent/repository`. | > **Persistence policy (authoritative):** new relational/queryable persistence uses **Room** (`@Entity` + DAO + `RoomDatabase` with explicit migrations, provided via Koin). Non-relational settings use the **filesystem/preferences (DataStore)**. **Raw SQLite is the exception, not the default** — see [ADR 0001](docs/adr/0001-prefer-room-for-persistence.md). > @@ -184,13 +186,14 @@ fun onEvent(event: PluginManagerUiEvent) = viewModelScope.launch(Dispatchers.IO) Test code lives both alongside each module and in the shared `testing:{unit,android,lsp,tooling,common}` harnesses. Run with the flox wrapper, e.g. `flox activate -d flox/local -- ./gradlew :testing:unit:test` or a module's `:module:test --tests "…"`. -| Layer | Runner / Tools | What to test | -|---|---|---| -| Unit (pure JVM) | **JUnit Jupiter (5)**, some legacy **JUnit 4**; assertions via **Google Truth**; mocking via **MockK** (primary) and **Mockito-Kotlin** (legacy) | ViewModels (state transitions over a fake repository), repositories, parsers, builder/tooling logic. Keep these off the device. | -| JVM + Android framework | **Robolectric** | Code needing `Context`/resources/`SQLiteOpenHelper` without an emulator. | -| Instrumented / UI | **Espresso** + **AndroidX Test** + **UiAutomator**, run under **Test Orchestrator**; `mockk-android` for on-device mocks | End-to-end IDE flows (create/build/deploy, editor, terminal). | +| Layer | Runner / Tools | What to test | +| ----------------------- | ------------------------------------------------------------ | ------------------------------------------------------------ | +| Unit (pure JVM) | **JUnit Jupiter (5)**, some legacy **JUnit 4**; assertions via **Google Truth**; mocking via **MockK** (primary) and **Mockito-Kotlin** (legacy) | ViewModels (state transitions over a fake repository), repositories, parsers, builder/tooling logic. Keep these off the device. | +| JVM + Android framework | **Robolectric** | Code needing `Context`/resources/`SQLiteOpenHelper` without an emulator. | +| Instrumented / UI | **Espresso** + **AndroidX Test** + **UiAutomator**, run under **Test Orchestrator**; `mockk-android` for on-device mocks | End-to-end IDE flows (create/build/deploy, editor, terminal). | Preferences and conventions: + - **Assertions: Google Truth** (`assertThat(x).isEqualTo(...)`) over raw JUnit asserts. - **Mocking: MockK** for new code; relax it deliberately rather than over-stubbing. - For UDF ViewModels, drive `onEvent(...)`/method calls against a fake or mocked repository and assert the emitted `UiState` sequence (collect the `StateFlow`); assert effects by collecting the effect `SharedFlow`. diff --git a/build-info/build.gradle.kts b/build-info/build.gradle.kts index 79dc003c04..e257497773 100644 --- a/build-info/build.gradle.kts +++ b/build-info/build.gradle.kts @@ -75,7 +75,10 @@ tasks.create("generateBuildInfo") { "AGP_VERSION_LATEST" to libs.versions.agp.tooling .get(), - "AGP_VERSION_GRADLE_LATEST" to "8.6", // From SdkConstants.GRADLE_LATEST_VERSION + // The Gradle version AGP_VERSION_LATEST gets exercised against: the + // distribution the IDE bundles. 8.6 was stale - AGP 8.11 refuses to + // configure on anything older than 8.13. + "AGP_VERSION_GRADLE_LATEST" to "8.14.3", "SNAPSHOTS_REPOSITORY" to VersionUtils.SONATYPE_SNAPSHOTS_REPO, "PUBLIC_REPOSITORY" to VersionUtils.SONATYPE_PUBLIC_REPO, ), diff --git a/common/src/main/java/com/itsaky/androidide/managers/ToolsManager.java b/common/src/main/java/com/itsaky/androidide/managers/ToolsManager.java index 363ad47aff..ebf3c3b6c3 100755 --- a/common/src/main/java/com/itsaky/androidide/managers/ToolsManager.java +++ b/common/src/main/java/com/itsaky/androidide/managers/ToolsManager.java @@ -20,7 +20,6 @@ import static org.adfa.constants.ConstantsKt.V7_KEY; import static org.adfa.constants.ConstantsKt.V8_KEY; -import android.content.res.AssetManager; import android.os.Build; import androidx.annotation.NonNull; import androidx.annotation.WorkerThread; @@ -107,7 +106,7 @@ public static void init(@NonNull BaseApplication app, Runnable onFinish) { // Load installed JDK distributions IJdkDistributionProvider.getInstance().loadDistributions(); - updateToolingJar(app.getAssets()); + updateToolingJar(app); extractLogSender(app); writeNoMediaFile(); @@ -244,11 +243,32 @@ private static String generateRandomPassword(int length) { return sb.toString(); } + /** + * Identity of the installed APK for the extraction stamp: versionName plus the package's lastUpdateTime, which changes on every (re)install - exactly when the bundled jar can change. Null (extract unconditionally) if the lookup fails. + */ + private static String installedApkStamp(BaseApplication app) { + try { + final var info = app.getPackageManager().getPackageInfo(app.getPackageName(), 0); + return info.versionName + ":" + info.lastUpdateTime; + } catch (Throwable err) { + LOG.warn("Could not read package info for tooling jar stamp", err); + return null; + } + } + @NonNull private static String readInitScript() { return ResourceUtils.readAssets2String(getCommonAsset("androidide.init.gradle")); } + private static String readStampFile(File stampFile) { + try { + return stampFile.isFile() ? FileIOUtils.readFile2String(stampFile) : null; + } catch (Throwable err) { + return null; + } + } + private static boolean shouldExtractScheme(final BaseApplication app, final File dir, final String path) throws IOException { @@ -293,10 +313,21 @@ private static boolean shouldExtractScheme(final BaseApplication app, final File } @WorkerThread - private static void updateToolingJar(AssetManager assets) { + private static void updateToolingJar(BaseApplication app) { // Ensure relevant shared libraries are loaded Brotli4jLoader.ensureAvailability(); + final var toolingJarFile = Environment.TOOLING_API_JAR; + final var stampFile = new File(toolingJarFile.getParentFile(), toolingJarFile.getName() + ".stamp"); + final var stamp = installedApkStamp(app); + if (toolingJarFile.isFile() && stamp != null && stamp.equals(readStampFile(stampFile))) { + // The jar from this exact APK install is already extracted; skip the copy. + // The stamp is written only after a complete extraction, so a partial + // copy from a killed process can never satisfy this check. + return; + } + + final var assets = app.getAssets(); final var toolingJarName = "tooling-api-all.jar"; InputStream toolingJarStream; try { @@ -311,15 +342,24 @@ private static void updateToolingJar(AssetManager assets) { } try { - final var toolingJarFile = Environment.TOOLING_API_JAR; - if (toolingJarFile.exists()) { - FileUtils.delete(toolingJarFile); - } - + // Extract to a temp sibling, then rename into place. The tooling server + // starts concurrently with this extraction (both run at app init), and + // launching `java -jar` against a half-written jar kills project init + // ("An unexpected error occurred while trying to open file ..."), so a + // partial jar must never be visible at the final path. rename(2) within + // one directory atomically replaces the target on Linux. + final var tempFile = new File(toolingJarFile.getParentFile(), toolingJarFile.getName() + ".part"); Objects.requireNonNull(toolingJarFile.getParentFile()).mkdirs(); - try (final var fos = new FileOutputStream(toolingJarFile)) { + try (final var fos = new FileOutputStream(tempFile)) { IoUtilsKt.transferToStream(toolingJarStream, fos); } + if (!tempFile.renameTo(toolingJarFile)) { + LOG.error("Failed to move extracted tooling API jar into place"); + return; + } + if (stamp != null) { + FileIOUtils.writeFileFromString(stampFile, stamp); + } } catch (Throwable err) { LOG.error("Failed to copy tooling API jar", err); } finally { diff --git a/common/src/main/java/com/itsaky/androidide/models/SaveResult.java b/common/src/main/java/com/itsaky/androidide/models/SaveResult.java index 779ce62d7f..d13b610c41 100755 --- a/common/src/main/java/com/itsaky/androidide/models/SaveResult.java +++ b/common/src/main/java/com/itsaky/androidide/models/SaveResult.java @@ -20,16 +20,24 @@ /** Result obtained when files are saved */ public final class SaveResult { - /** Were any Gradle files saved? */ - public boolean gradleSaved = false; + /** Were any Gradle files saved? */ + public boolean gradleSaved = false; - /** Were any XML files saved? */ - public boolean xmlSaved = false; + /** Were any XML files saved? */ + public boolean xmlSaved = false; - public SaveResult() {} + /** + * Were any Android resource XML files (files under a module's {@code res/} directory) saved? + * + *

+ * Narrower than {@link #xmlSaved} on purpose: only a resource save can change {@code R}, and the Gradle {@code generateSources()} run that follows a save is load-bearing exactly there. Java resolves {@code R.string.*} from the regenerated {@code R.jar} on the compile classpath (the run posts {@code ProjectInitializedEvent}, which makes {@code JavaLanguageServer} drop its stale jar-FS cache), and with view binding on, only {@code dataBindingGenBaseClasses} writes the accessor for an id just added to a layout. Manifest edits and other non-resource XML cannot change {@code R}, so they skip that run. + */ + public boolean resourceXmlSaved = false; - public SaveResult(boolean gradleSaved, boolean xmlSaved) { - this.gradleSaved = gradleSaved; - this.xmlSaved = xmlSaved; - } + public SaveResult() {} + + public SaveResult(boolean gradleSaved, boolean xmlSaved) { + this.gradleSaved = gradleSaved; + this.xmlSaved = xmlSaved; + } } diff --git a/common/src/main/java/com/itsaky/androidide/utils/FeatureFlags.kt b/common/src/main/java/com/itsaky/androidide/utils/FeatureFlags.kt index f71adce1d0..97955c0f66 100644 --- a/common/src/main/java/com/itsaky/androidide/utils/FeatureFlags.kt +++ b/common/src/main/java/com/itsaky/androidide/utils/FeatureFlags.kt @@ -15,6 +15,8 @@ private data class FlagsCache( val reprieveEnabled: Boolean = false, val pardonEnabled: Boolean = false, val leakCanaryDumpInhibited: Boolean = false, + val quickBuildBenchEnabled: Boolean = false, + val quickBuildWarmCompileDisabled: Boolean = false, ) { companion object { /** @@ -31,17 +33,32 @@ object FeatureFlags { private const val REPRIEVE_FILE_NAME = "CodeOnTheGo.a3s19" private const val PARDON_FILE_NAME = "CodeOnTheGo.a2s2" private const val LEAKCANARY_FILE_NAME = "CodeOnTheGo.lc" + private const val QUICK_BUILD_BENCH_FILE_NAME = "CodeOnTheGo.qbbench" + private const val QUICK_BUILD_NO_SEED_FILE_NAME = "CodeOnTheGo.qbnoseed" private val logger = LoggerFactory.getLogger(FeatureFlags::class.java) private val mutex = Mutex() private var flags = FlagsCache.DEFAULT + /** + * Whether the flag files have been read from disk. Explicit rather than inferred from + * [flags] being non-[FlagsCache.DEFAULT]: a device-protected (direct boot) read sees no + * external storage at all, so it produces the same all-false snapshot as a genuine read + * of a device with no flag files, and an identity check cannot tell the two apart. + */ + private var loaded = false + private val downloadsDir = Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS) /** * Whether Code On the Go experiments are enabled. + * + * Read from the sentinel file once per process and cached, so adding or deleting the file + * changes nothing in an app that is already running - including one that was only + * backgrounded. Toggling a flag needs a force-stop, not a relaunch from Recents, and any + * test step that flips one has to say so. */ val isExperimentsEnabled: Boolean get() = flags.experimentsEnabled @@ -76,35 +93,71 @@ object FeatureFlags { val isLeakCanaryDumpInhibited: Boolean get() = flags.leakCanaryDumpInhibited + /** + * Whether the Quick Build benchmark hooks are enabled (CodeOnTheGo.qbbench present in + * Downloads). Gates the adb-triggerable bench activity and the JSON-lines event file + * (ADFA-4128); always paired with [isExperimentsEnabled]. Off in shipping builds. + */ + val isQuickBuildBenchEnabled: Boolean + get() = flags.quickBuildBenchEnabled + + /** + * Whether the Quick Build background warm compile is disabled (CodeOnTheGo.qbnoseed present in + * Downloads). Bench-only A/B seam (ADFA-4128), inert unless [isQuickBuildBenchEnabled] + * is also on - the DI wiring pairs the two. + */ + val isQuickBuildWarmCompileDisabled: Boolean + get() = flags.quickBuildWarmCompileDisabled + /** * Initialize feature flag values. This is thread-safe and idempotent i.e. - * subsequent calls do not access disk. + * subsequent calls do not access disk. Use [refresh] to re-read. */ suspend fun initialize(): Unit = mutex.withLock { - if (flags !== FlagsCache.DEFAULT) { - // already initialized + if (loaded) { return@withLock } + load() + } + + /** + * Re-read the flag files, replacing the cached snapshot. + * + * The startup read can happen in direct boot mode, where external storage is not + * mounted and every flag therefore reads as absent. That snapshot must not be allowed + * to stick, so the phase that runs once credential-protected storage is available + * re-reads instead of relying on [initialize] being a no-op by then. + */ + suspend fun refresh(): Unit = mutex.withLock { load() } + + /** Reads every flag file. Call under [mutex]. */ + private suspend fun load() { + fun checkFlag(fileName: String) = File(downloadsDir, fileName).exists() - fun checkFlag(fileName: String) = File(downloadsDir, fileName).exists() - - flags = - withContext(Dispatchers.IO) { - runCatching { - logger.info("Loading feature flags...") - FlagsCache( - experimentsEnabled = checkFlag(EXPERIMENTS_FILE_NAME), - debugLoggingEnabled = checkFlag(LOGD_FILE_NAME), - emulatorUseEnabled = checkFlag(EMULATOR_FILE_NAME), - reprieveEnabled = checkFlag(REPRIEVE_FILE_NAME), - pardonEnabled = checkFlag(PARDON_FILE_NAME), - leakCanaryDumpInhibited = checkFlag(LEAKCANARY_FILE_NAME), - ) - }.getOrElse { error -> - logger.error("Failed to load feature flags. Falling back to default values.", error) - FlagsCache.DEFAULT - } + val read = + withContext(Dispatchers.IO) { + runCatching { + logger.info("Loading feature flags...") + FlagsCache( + experimentsEnabled = checkFlag(EXPERIMENTS_FILE_NAME), + debugLoggingEnabled = checkFlag(LOGD_FILE_NAME), + emulatorUseEnabled = checkFlag(EMULATOR_FILE_NAME), + reprieveEnabled = checkFlag(REPRIEVE_FILE_NAME), + pardonEnabled = checkFlag(PARDON_FILE_NAME), + leakCanaryDumpInhibited = checkFlag(LEAKCANARY_FILE_NAME), + quickBuildBenchEnabled = checkFlag(QUICK_BUILD_BENCH_FILE_NAME), + quickBuildWarmCompileDisabled = checkFlag(QUICK_BUILD_NO_SEED_FILE_NAME), + ) } - } + } + // A read that threw keeps the previous snapshot (all-off at startup) and leaves + // `loaded` false, so a later call retries instead of latching the failure. + flags = + read.getOrElse { error -> + logger.error("Failed to load feature flags. Falling back to default values.", error) + return@load + } + loaded = true + } } diff --git a/common/src/main/java/com/itsaky/androidide/utils/FlashbarActivityUtils.kt b/common/src/main/java/com/itsaky/androidide/utils/FlashbarActivityUtils.kt index 67bfcec141..76074c52d4 100644 --- a/common/src/main/java/com/itsaky/androidide/utils/FlashbarActivityUtils.kt +++ b/common/src/main/java/com/itsaky/androidide/utils/FlashbarActivityUtils.kt @@ -62,7 +62,8 @@ private fun Flashbar.Builder.applyIcon(iconType: IconType): Flashbar.Builder = /** * Builds and configures a Flashbar for [msg]/[iconType] (icon, and - for an indefinite error - the - * dismiss button), without showing it yet. Shared by [showFlashBar] and [showFlashBarAwaitShown] + * dismiss button plus tap/swipe dismissal), without showing it yet. Shared by [showFlashBar] and + * [showFlashBarAwaitShown] * so their setup can't silently diverge. Returns `null` for a `null` [msg] (nothing to show). */ private fun Activity.configureFlashbar( @@ -82,6 +83,15 @@ private fun Activity.configureFlashbar( if (duration == DURATION_INDEFINITE && iconType == IconType.ERROR) { builder.positiveActionText(getString(R.string.dismiss)) builder.positiveActionTapListener { it.dismiss() } + + // An indefinite bar is drawn OVER the activity, and the error variant is tall enough + // (message + action row) to cover the editor toolbar. Until it goes away the Run and + // Quick Build buttons cannot be reached at all: a tap on them lands on the bar, so both + // read as dead with nothing on screen saying why. Measured on an a56: the bar occupied + // y 236-371 while the toolbar buttons sat at y 261-383. + // So any touch on the bar, and any swipe, gets rid of it - not just the Dismiss button. + builder.listenBarTaps { it.dismiss() } + builder.enableSwipeToDismiss() } when (msg) { @@ -144,6 +154,11 @@ fun Activity.flashError(msg: String?) = showFlashBar(msg, IconType.ERROR, durati fun Activity.flashInfo(msg: String?) = showFlashBar(msg, IconType.INFO) +// A 1 s bar (the default) is gone before a sentence can be read. For an informational +// message that fires once and explains why something did NOT happen, the longer duration +// is the difference between an explanation and a flicker. +fun Activity.flashInfoLong(msg: String?) = showFlashBar(msg, IconType.INFO, duration = DURATION_LONG) + /** * Like [showFlashBar], but suspends until the bar's entrance animation has actually finished (or * [FLASH_SHOWN_TIMEOUT_MS] elapses) instead of firing-and-forgetting - for callers (e.g. a diff --git a/common/src/test/java/com/itsaky/androidide/utils/KeyedDebouncingActionCancelTest.kt b/common/src/test/java/com/itsaky/androidide/utils/KeyedDebouncingActionCancelTest.kt index 2576d46490..33f7a012ae 100644 --- a/common/src/test/java/com/itsaky/androidide/utils/KeyedDebouncingActionCancelTest.kt +++ b/common/src/test/java/com/itsaky/androidide/utils/KeyedDebouncingActionCancelTest.kt @@ -17,50 +17,49 @@ import kotlin.time.Duration.Companion.milliseconds * parked on `channel.receive()` must NOT let a [ClosedReceiveChannelException] * escape to the scope's uncaught-exception handler. * - * On the pre-fix baseline, `ActionEntry.cancel()` did `channel.close()` BEFORE - * `job.cancel()`. Closing the channel wakes the parked `receive()` with a - * [ClosedReceiveChannelException] (NOT a CancellationException), which propagates - * uncaught to the [CoroutineExceptionHandler] -> the Sentry crash this ticket fixes. - * - * The fix swaps the order (job.cancel() first) AND wraps the worker loop in a - * try/catch that swallows ClosedReceiveChannelException, so no uncaught exception fires. + * `ActionEntry.cancel()` must therefore call `job.cancel()` BEFORE `channel.close()`, and + * the worker loop must swallow [ClosedReceiveChannelException] as well. Closing the channel + * first wakes the parked `receive()` with a [ClosedReceiveChannelException] - NOT a + * CancellationException - which propagates uncaught to the [CoroutineExceptionHandler]. */ class KeyedDebouncingActionCancelTest { + /** Cancelling an entry whose worker is parked on receive() must not surface an uncaught exception. */ + @Test + fun `cancelling a parked worker does not leak a ClosedReceiveChannelException`() = + runBlocking { + val uncaught = AtomicReference(null) + // A plain Job (not Supervisor of the worker) + a handler that records anything + // that escapes the debounce worker coroutine. + val handler = CoroutineExceptionHandler { _, t -> uncaught.set(t) } + val scope = CoroutineScope(SupervisorJob() + handler) - /** Cancelling an entry whose worker is parked on receive() must not surface an uncaught exception. */ - @Test - fun `cancelling a parked worker does not leak a ClosedReceiveChannelException`() = runBlocking { - val uncaught = AtomicReference(null) - // A plain Job (not Supervisor of the worker) + a handler that records anything - // that escapes the debounce worker coroutine. - val handler = CoroutineExceptionHandler { _, t -> uncaught.set(t) } - val scope = CoroutineScope(SupervisorJob() + handler) - - val ctx: CoroutineContext = scope.coroutineContext + val ctx: CoroutineContext = scope.coroutineContext - val debouncer = KeyedDebouncingAction( - scope = scope, - debounceDuration = 50.milliseconds, - actionContext = ctx, - action = { _, _ -> /* never invoked: we cancel while parked on receive */ }, - ) + val debouncer = + KeyedDebouncingAction( + scope = scope, + debounceDuration = 50.milliseconds, + actionContext = ctx, + // Never invoked: the worker is cancelled while parked on receive(). + action = { _, _ -> }, + ) - // schedule() creates the entry + launches the worker. With a CONFLATED channel and - // no further sends, the worker debounces the single key, runs the (empty) action, - // then loops back and parks on channel.receive() waiting for the next key. - debouncer.schedule("k") + // schedule() creates the entry + launches the worker. With a CONFLATED channel and + // no further sends, the worker debounces the single key, runs the (empty) action, + // then loops back and parks on channel.receive() waiting for the next key. + debouncer.schedule("k") - // Give the worker time to: receive "k", run the empty action, loop, and PARK on - // the next channel.receive(). 200ms >> 50ms debounce window. - delay(200) + // Give the worker time to: receive "k", run the empty action, loop, and PARK on + // the next channel.receive(). 200ms >> 50ms debounce window. + delay(200) - // Cancel the entry while the worker is parked on receive(). - debouncer.cancelPending("k") + // Cancel the entry while the worker is parked on receive(). + debouncer.cancelPending("k") - // Let any uncaught exception propagate to the handler. - delay(200) + // Let any uncaught exception propagate to the handler. + delay(200) - val leaked = uncaught.get() - assertThat(leaked).isNull() - } + val leaked = uncaught.get() + assertThat(leaked).isNull() + } } diff --git a/composite-builds/build-logic/plugins/src/main/java/com/itsaky/androidide/plugins/conf/AndroidModuleConf.kt b/composite-builds/build-logic/plugins/src/main/java/com/itsaky/androidide/plugins/conf/AndroidModuleConf.kt index f354555e43..4f75f274ef 100644 --- a/composite-builds/build-logic/plugins/src/main/java/com/itsaky/androidide/plugins/conf/AndroidModuleConf.kt +++ b/composite-builds/build-logic/plugins/src/main/java/com/itsaky/androidide/plugins/conf/AndroidModuleConf.kt @@ -48,6 +48,9 @@ private val disableCoreLibDesugaringForModules = arrayOf( ":logsender", ":logger", + // Like :logsender, the AAR is injected into apps built with CoGo and must + // not force desugaring onto user projects (ADFA-4128). + ":quickbuild:runtime", ) /** diff --git a/composite-builds/build-logic/plugins/src/main/java/com/itsaky/androidide/plugins/conf/MavenPublishConf.kt b/composite-builds/build-logic/plugins/src/main/java/com/itsaky/androidide/plugins/conf/MavenPublishConf.kt index 6a2f79bf2b..26dcf19141 100644 --- a/composite-builds/build-logic/plugins/src/main/java/com/itsaky/androidide/plugins/conf/MavenPublishConf.kt +++ b/composite-builds/build-logic/plugins/src/main/java/com/itsaky/androidide/plugins/conf/MavenPublishConf.kt @@ -18,6 +18,7 @@ package com.itsaky.androidide.plugins.conf import com.itsaky.androidide.build.config.ProjectConfig +import com.itsaky.androidide.build.config.publishingVersion import com.vanniktech.maven.publish.AndroidMultiVariantLibrary import com.vanniktech.maven.publish.GradlePlugin import com.vanniktech.maven.publish.JavaLibrary @@ -27,94 +28,104 @@ import com.vanniktech.maven.publish.SonatypeHost.Companion.S01 import org.gradle.api.GradleException import org.gradle.api.Project import org.gradle.api.publish.PublishingExtension +import org.gradle.api.publish.maven.tasks.PublishToMavenRepository import org.gradle.api.tasks.Delete -import org.gradle.api.tasks.testing.Test import org.gradle.kotlin.dsl.configure -import org.gradle.kotlin.dsl.create +import org.gradle.kotlin.dsl.register import org.gradle.kotlin.dsl.withType -import com.itsaky.androidide.build.config.publishingVersion -import java.io.File +import org.gradle.plugins.signing.Sign private val mavenLocalRepos = hashMapOf() @Suppress("UnstableApiUsage") fun Project.configureMavenPublish() { - assert(plugins.hasPlugin("com.vanniktech.maven.publish.base")) { - "${javaClass.simpleName} can only be applied to maven publish projects." - } - - afterEvaluate { - if (project.description.isNullOrBlank()) { - throw GradleException("Project ${project.path} must have a description") - } - } - - configure { - - project.configureMavenLocal() - - pom { - name.set(project.name) - description.set(project.description) - inceptionYear.set("2021") - url.set(ProjectConfig.REPO_URL) - licenses { - license { - name.set("The GNU General Public License, v3.0") - url.set("https://www.gnu.org/licenses/gpl-3.0.en.html") - distribution.set("https://www.gnu.org/licenses/gpl-3.0.en.html") - } - } - - developers { - developer { - id.set("androidide") - name.set("AndroidIDE") - url.set(ProjectConfig.PROJECT_SITE) - } - } - - scm { - url.set(ProjectConfig.REPO_URL) - connection.set(ProjectConfig.SCM_GIT) - developerConnection.set(ProjectConfig.SCM_SSH) - } - } - - coordinates(project.group.toString(), project.name, project.publishingVersion) - publishToMavenCentral(host = S01) - signAllPublications() - - if (plugins.hasPlugin("com.android.library")) { - configure(AndroidMultiVariantLibrary()) - } else if (plugins.hasPlugin("java-gradle-plugin")) { - configure(GradlePlugin(javadocJar = JavadocJar.Javadoc())) - } else if (plugins.hasPlugin("java-library")) { - configure(JavaLibrary(javadocJar = JavadocJar.Javadoc())) - } - } + assert(plugins.hasPlugin("com.vanniktech.maven.publish.base")) { + "${javaClass.simpleName} can only be applied to maven publish projects." + } + + afterEvaluate { + if (project.description.isNullOrBlank()) { + throw GradleException("Project ${project.path} must have a description") + } + } + + configure { + project.configureMavenLocal() + + pom { + name.set(project.name) + description.set(project.description) + inceptionYear.set("2021") + url.set(ProjectConfig.REPO_URL) + licenses { + license { + name.set("The GNU General Public License, v3.0") + url.set("https://www.gnu.org/licenses/gpl-3.0.en.html") + distribution.set("https://www.gnu.org/licenses/gpl-3.0.en.html") + } + } + + developers { + developer { + id.set("androidide") + name.set("AndroidIDE") + url.set(ProjectConfig.PROJECT_SITE) + } + } + + scm { + url.set(ProjectConfig.REPO_URL) + connection.set(ProjectConfig.SCM_GIT) + developerConnection.set(ProjectConfig.SCM_SSH) + } + } + + coordinates(project.group.toString(), project.name, project.publishingVersion) + publishToMavenCentral(host = S01) + signAllPublications() + + // The signing key only exists on the publishing CI (ORG_GRADLE_PROJECT_signingInMemoryKey). + // Without this, publishing to the build-local repo - which the gradle-plugin functional + // tests depend on - fails anywhere else with "no configured signatory". + val hasSigningKey = project.providers.gradleProperty("signingInMemoryKey").isPresent + project.tasks.withType().configureEach { onlyIf { hasSigningKey } } + + if (plugins.hasPlugin("com.android.library")) { + configure(AndroidMultiVariantLibrary()) + } else if (plugins.hasPlugin("java-gradle-plugin")) { + configure(GradlePlugin(javadocJar = JavadocJar.Javadoc())) + } else if (plugins.hasPlugin("java-library")) { + configure(JavaLibrary(javadocJar = JavadocJar.Javadoc())) + } + } } private fun Project.configureMavenLocal() { - val mavenLocalPath = layout.buildDirectory.dir("maven-local") - mavenLocalRepos[project.path] = mavenLocalPath.get().asFile.absolutePath - - extensions.findByType(PublishingExtension::class.java)?.run { - repositories { - maven { - name = "buildMavenLocal" - url = uri(mavenLocalPath) - } - } - } - - tasks.create("deleteBuildMavenLocal") { - delete(mavenLocalPath) - } - - afterEvaluate { - tasks.getByName("publishAllPublicationsToBuildMavenLocalRepository") { - dependsOn(tasks.getByName("deleteBuildMavenLocal")) - } - } -} \ No newline at end of file + val mavenLocalPath = layout.buildDirectory.dir("maven-local") + mavenLocalRepos[project.path] = mavenLocalPath.get().asFile.absolutePath + + extensions.findByType(PublishingExtension::class.java)?.run { + repositories { + maven { + name = "buildMavenLocal" + url = uri(mavenLocalPath) + } + } + } + + val deleteBuildMavenLocal = + tasks.register("deleteBuildMavenLocal") { + delete(mavenLocalPath) + } + + // The delete must be a dependency of every per-publication publish task writing + // into this repo, not only of the publishAll* aggregate: an aggregate-only edge + // leaves the scheduler free to run the delete after an individual publish under + // parallel execution, wiping freshly staged artifacts before consumers (the + // :gradle-plugin:test functional builds) resolve from them. + tasks.withType().configureEach { + if (name.endsWith("ToBuildMavenLocalRepository")) { + dependsOn(deleteBuildMavenLocal) + } + } +} diff --git a/editor/src/main/java/com/itsaky/androidide/editor/utils/ContentReadWrite.kt b/editor/src/main/java/com/itsaky/androidide/editor/utils/ContentReadWrite.kt index 9cf0177864..74ae5d455e 100644 --- a/editor/src/main/java/com/itsaky/androidide/editor/utils/ContentReadWrite.kt +++ b/editor/src/main/java/com/itsaky/androidide/editor/utils/ContentReadWrite.kt @@ -31,135 +31,143 @@ import kotlin.math.floor * @author Akash Yadav */ object ContentReadWrite { - - /** - * Write this [Content] to the given [File]. - * - * @param progressConsumer A function which is invoked to notify about the write progress. - */ - @JvmStatic - fun Content.writeTo(file: File, progressConsumer: ((Int) -> Unit)? = null) { - val consumer = discreteProgressConsumer(progressConsumer = progressConsumer) - - checkForParentDir(file) - - file.writer().buffered(DEFAULT_BUFFER_SIZE * 2).use { writer -> - val lastLine = lineCount - 1 - val length = length - - ContentLockAccessor.lock(this, false) - var totalWrote = 0.0 - try { - for (lineIdx in 0..lastLine) { - val line = getLine(lineIdx) - writer.write(line.backingCharArray, 0, line.length) - - val separatorChars = line.lineSeparator.chars - writer.write(separatorChars) - - totalWrote += line.length + separatorChars.size - val saveProgress = (totalWrote / length) * 100 - consumer(floor(saveProgress).toInt()) - } - } catch (err: IOException) { - throw RuntimeException("Failed to write editor's content to file: ${file.absolutePath}", - err) - } finally { - ContentLockAccessor.unlock(this, false) - consumer(100) - } - - writer.flush() - } - } - - /** - * Reads this file's content to a new [Content] object. - * - * @param progressConsumer A function to consume the read progress. - */ - @JvmStatic - fun File.readContent(progressConsumer: ((Int) -> Unit)? = null): Content { - val consumer = discreteProgressConsumer(progressConsumer = progressConsumer) - return Content().apply { - isUndoEnabled = false - inputStream().use { input -> - val total = input.available().let { if (it == 0) 1 else it } // avoid divide by 0 - input.reader().use { reader -> - val buffer = CharArray(DEFAULT_BUFFER_SIZE * 2) - val wrapper = CharArrayWrapper(buffer, 0) - var totalRead = 0.0 - var count: Int - while (true) { - count = reader.read(buffer) - if (count == -1) { - break - } - if (count == 0) { - continue - } - - totalRead += count - - val progress = floor((totalRead / total) * 100).toInt() - - if (buffer[count - 1] == '\r') { - val peek = reader.read() - if (peek == '\n'.code) { - wrapper.setDataCount(count - 1) - var line = lineCount - 1 - insert(line, getColumnCount(line), wrapper) - - line = lineCount - 1 - insert(line, getColumnCount(line), "\r\n") - consumer(progress) - continue - - } else if (peek != -1) { - wrapper.setDataCount(count) - var line = lineCount - 1 - insert(line, getColumnCount(line), wrapper) - - line = lineCount - 1 - insert(line, getColumnCount(line), peek.toChar().toString()) - consumer(progress) - continue - } - } - wrapper.setDataCount(count) - - val line = lineCount - 1 - insert(line, getColumnCount(line), wrapper) - - consumer(progress) - } - } - isUndoEnabled = true - } - } - } - - @JvmStatic - private fun discreteProgressConsumer( - stepSize: Int = 5, - progressConsumer: ((Int) -> Unit)? - ) : (Int) -> Unit { - var lastProgress = -1 - val consumer = fun (progress: Int) { - if (lastProgress == -1 || progress >= 100 || progress - lastProgress >= stepSize) { - progressConsumer?.invoke(progress) - lastProgress = progress - } - } - - return consumer - } - - private fun checkForParentDir(file: File) { - val parent = file.parentFile ?: return - - if (!parent.exists() && !parent.mkdirs() && !parent.exists()) { - throw IOException("The parent directory could not be created for: ${file.absolutePath}") - } - } -} \ No newline at end of file + /** + * Write this [Content] to the given [File]. + * + * Writes IN PLACE — opens [file] directly and truncates + writes sequentially; this is + * NOT a temp-file-then-rename swap. A filesystem watcher observing a save from this + * method sees the target path itself change, never a sibling temp file (that pattern + * is specific to EXTERNAL tools like `sed -i` or `git checkout`). + * + * @param progressConsumer A function which is invoked to notify about the write progress. + */ + @JvmStatic + fun Content.writeTo( + file: File, + progressConsumer: ((Int) -> Unit)? = null, + ) { + val consumer = discreteProgressConsumer(progressConsumer = progressConsumer) + + checkForParentDir(file) + + file.writer().buffered(DEFAULT_BUFFER_SIZE * 2).use { writer -> + val lastLine = lineCount - 1 + val length = length + + ContentLockAccessor.lock(this, false) + var totalWrote = 0.0 + try { + for (lineIdx in 0..lastLine) { + val line = getLine(lineIdx) + writer.write(line.backingCharArray, 0, line.length) + + val separatorChars = line.lineSeparator.chars + writer.write(separatorChars) + + totalWrote += line.length + separatorChars.size + val saveProgress = (totalWrote / length) * 100 + consumer(floor(saveProgress).toInt()) + } + } catch (err: IOException) { + throw RuntimeException( + "Failed to write editor's content to file: ${file.absolutePath}", + err, + ) + } finally { + ContentLockAccessor.unlock(this, false) + consumer(100) + } + + writer.flush() + } + } + + /** + * Reads this file's content to a new [Content] object. + * + * @param progressConsumer A function to consume the read progress. + */ + @JvmStatic + fun File.readContent(progressConsumer: ((Int) -> Unit)? = null): Content { + val consumer = discreteProgressConsumer(progressConsumer = progressConsumer) + return Content().apply { + isUndoEnabled = false + inputStream().use { input -> + val total = input.available().let { if (it == 0) 1 else it } // avoid divide by 0 + input.reader().use { reader -> + val buffer = CharArray(DEFAULT_BUFFER_SIZE * 2) + val wrapper = CharArrayWrapper(buffer, 0) + var totalRead = 0.0 + var count: Int + while (true) { + count = reader.read(buffer) + if (count == -1) { + break + } + if (count == 0) { + continue + } + + totalRead += count + + val progress = floor((totalRead / total) * 100).toInt() + + if (buffer[count - 1] == '\r') { + val peek = reader.read() + if (peek == '\n'.code) { + wrapper.setDataCount(count - 1) + var line = lineCount - 1 + insert(line, getColumnCount(line), wrapper) + + line = lineCount - 1 + insert(line, getColumnCount(line), "\r\n") + consumer(progress) + continue + } else if (peek != -1) { + wrapper.setDataCount(count) + var line = lineCount - 1 + insert(line, getColumnCount(line), wrapper) + + line = lineCount - 1 + insert(line, getColumnCount(line), peek.toChar().toString()) + consumer(progress) + continue + } + } + wrapper.setDataCount(count) + + val line = lineCount - 1 + insert(line, getColumnCount(line), wrapper) + + consumer(progress) + } + } + isUndoEnabled = true + } + } + } + + @JvmStatic + private fun discreteProgressConsumer( + stepSize: Int = 5, + progressConsumer: ((Int) -> Unit)?, + ): (Int) -> Unit { + var lastProgress = -1 + val consumer = fun (progress: Int) { + if (lastProgress == -1 || progress >= 100 || progress - lastProgress >= stepSize) { + progressConsumer?.invoke(progress) + lastProgress = progress + } + } + + return consumer + } + + private fun checkForParentDir(file: File) { + val parent = file.parentFile ?: return + + if (!parent.exists() && !parent.mkdirs() && !parent.exists()) { + throw IOException("The parent directory could not be created for: ${file.absolutePath}") + } + } +} diff --git a/gradle-plugin-config/src/main/java/com/itsaky/androidide/tooling/api/GradlePluginConfig.java b/gradle-plugin-config/src/main/java/com/itsaky/androidide/tooling/api/GradlePluginConfig.java index c55a0ea95a..5ef74fc090 100644 --- a/gradle-plugin-config/src/main/java/com/itsaky/androidide/tooling/api/GradlePluginConfig.java +++ b/gradle-plugin-config/src/main/java/com/itsaky/androidide/tooling/api/GradlePluginConfig.java @@ -34,6 +34,21 @@ public final class GradlePluginConfig { */ public static final String PROPERTY_PROFILEABLE_ENABLED = "cotg.profileable.enabled"; + /** + * Property used by the Gradle plugin to determine whether this build is a Quick Build proxy app build (ADFA-4128). When {@code true}, the plugin generates the proxy app shell: proxy activities from the merged manifest, the quick-build runtime dependency, and the class-openability transform. + */ + public static final String PROPERTY_QUICK_BUILD_ENABLED = "cotg.quickbuild.enabled"; + + /** + * The path to the Quick Build runtime AAR file, injected into the proxy app like the LogSender AAR. + */ + public static final String PROPERTY_QUICK_BUILD_RUNTIME_AAR = "cotg.quickbuild.runtimeAar"; + + /** + * The generation the host allocated for the proxy app baseline being built, from the same persistent per-project counter that numbers hot deploys. The plugin stamps it into the APK as an asset next to the baseline payload dex, so the runtime boots at this number instead of a constant 0. Unset means an older host: the plugin then stamps 0, which the runtime treats exactly like its pre-stamp baseline. + */ + public static final String PROPERTY_QUICK_BUILD_BASELINE_GENERATION = "cotg.quickbuild.baselineGeneration"; + /** * Property to enable or disable LogSender in the project. Value can be true or false. */ diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 13124ed35f..38f6c5b12f 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -268,6 +268,15 @@ xml-jb-annotations = { module = "org.jetbrains:annotations", version = "24.1.0" # GIT git-jgit = { module = "org.eclipse.jgit:org.eclipse.jgit", version = "6.8.0.202311291450-r" } +# Quick Build daemon (ADFA-4128): Kotlin Build Tools API incremental engine +kotlin-buildToolsApi = { module = "org.jetbrains.kotlin:kotlin-build-tools-api", version.ref = "kotlin" } +kotlin-buildToolsImpl = { module = "org.jetbrains.kotlin:kotlin-build-tools-impl", version.ref = "kotlin" } +# Compose compiler plugin, version-matched to the daemon's compiler +kotlin-composeCompilerPluginEmbeddable = { module = "org.jetbrains.kotlin:kotlin-compose-compiler-plugin-embeddable", version.ref = "kotlin" } +# Compose runtime for the daemon's compose compile tests (see :quickbuild:daemon) +composeRuntimeDaemonTests = { module = "androidx.compose.runtime:runtime-android", version = "1.7.3" } +ow2-asm = { module = "org.ow2.asm:asm", version = "9.7.1" } + # Tests tests-junit = { module = "junit:junit", version = "4.13.2" } tests-junit-jupiter = { module = "org.junit.jupiter:junit-jupiter", version.ref = "junit-jupiter" } diff --git a/idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.kt b/idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.kt index 4fd823aa82..df56f2bc94 100644 --- a/idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.kt +++ b/idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.kt @@ -127,6 +127,7 @@ object TooltipTag { const val EDITOR_TOOLBAR_PREVIEW_COMPOSE = "editor.compose.preview" const val EDITOR_TOOLBAR_COMPUTER_VISION = "project.layout.vision" const val EDITOR_TOOLBAR_LOG_SENDER = "editor.disconnect.logsenders" + const val EDITOR_TOOLBAR_QUICK_BUILD = "project.quickbuild" // Floating window chrome const val WINDOW_MINIMIZE = "window-min" diff --git a/logger/src/test/java/com/itsaky/androidide/logging/utils/LogUtilsTest.kt b/logger/src/test/java/com/itsaky/androidide/logging/utils/LogUtilsTest.kt new file mode 100644 index 0000000000..7a8977c511 --- /dev/null +++ b/logger/src/test/java/com/itsaky/androidide/logging/utils/LogUtilsTest.kt @@ -0,0 +1,49 @@ +package com.itsaky.androidide.logging.utils + +import com.google.common.truth.Truth.assertThat +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 + +/** + * Pins the two properties Quick Build's `QB-` logcat tag convention depends on: a hyphen + * survives the sanitiser, and a name at or under [LogUtils.MAX_TAG_LENGTH] is not trimmed. + */ +@RunWith(JUnit4::class) +class LogUtilsTest { + @Test + fun `a hyphenated tag at the length limit survives unchanged`() { + val tag = "QB-DaemonController" + + assertThat(tag.length).isAtMost(LogUtils.MAX_TAG_LENGTH) + assertThat(LogUtils.processLogTag(tag)).isEqualTo(tag) + } + + @Test + fun `a tag exactly at the limit survives unchanged`() { + val tag = "x".repeat(LogUtils.MAX_TAG_LENGTH) + + assertThat(LogUtils.processLogTag(tag)).isEqualTo(tag) + } + + @Test + fun `an over-length tag keeps its tail behind a double-dot prefix`() { + val tag = "QuickBuildSessionManager" + + val processed = LogUtils.processLogTag(tag) + + assertThat(tag.length).isGreaterThan(LogUtils.MAX_TAG_LENGTH) + assertThat(processed).hasLength(LogUtils.MAX_TAG_LENGTH) + assertThat(processed).isEqualTo("..ckBuildSessionManager") + } + + @Test + fun `characters outside the allowed set become underscores`() { + assertThat(LogUtils.processLogTag("QB Session!")).isEqualTo("QB_Session_") + } + + @Test + fun `a null tag stays null`() { + assertThat(LogUtils.processLogTag(null)).isNull() + } +} diff --git a/resources/src/main/res/drawable/ic_quick_build.xml b/resources/src/main/res/drawable/ic_quick_build.xml new file mode 100644 index 0000000000..31e6f962b5 --- /dev/null +++ b/resources/src/main/res/drawable/ic_quick_build.xml @@ -0,0 +1,12 @@ + + + + + diff --git a/resources/src/main/res/drawable/ic_quick_build_building.xml b/resources/src/main/res/drawable/ic_quick_build_building.xml new file mode 100644 index 0000000000..f14eb3b869 --- /dev/null +++ b/resources/src/main/res/drawable/ic_quick_build_building.xml @@ -0,0 +1,15 @@ + + + + + + + + diff --git a/resources/src/main/res/drawable/ic_quick_build_building_arc.xml b/resources/src/main/res/drawable/ic_quick_build_building_arc.xml new file mode 100644 index 0000000000..4825d03be9 --- /dev/null +++ b/resources/src/main/res/drawable/ic_quick_build_building_arc.xml @@ -0,0 +1,15 @@ + + + + + diff --git a/resources/src/main/res/drawable/ic_quick_build_building_stop.xml b/resources/src/main/res/drawable/ic_quick_build_building_stop.xml new file mode 100644 index 0000000000..2b3aac2d7e --- /dev/null +++ b/resources/src/main/res/drawable/ic_quick_build_building_stop.xml @@ -0,0 +1,13 @@ + + + + + diff --git a/resources/src/main/res/drawable/ic_quick_build_error.xml b/resources/src/main/res/drawable/ic_quick_build_error.xml new file mode 100644 index 0000000000..ea120260b2 --- /dev/null +++ b/resources/src/main/res/drawable/ic_quick_build_error.xml @@ -0,0 +1,31 @@ + + + + + + + + + diff --git a/resources/src/main/res/drawable/ic_quick_build_outline.xml b/resources/src/main/res/drawable/ic_quick_build_outline.xml new file mode 100644 index 0000000000..4294098e81 --- /dev/null +++ b/resources/src/main/res/drawable/ic_quick_build_outline.xml @@ -0,0 +1,17 @@ + + + + + diff --git a/resources/src/main/res/values/strings.xml b/resources/src/main/res/values/strings.xml index e9517f1051..e989ba98b7 100644 --- a/resources/src/main/res/values/strings.xml +++ b/resources/src/main/res/values/strings.xml @@ -1045,6 +1045,65 @@ Folder icon Close Save + Quick Build + Quick Build + Quick Build: %1$s + Standard build in progress + Restart session + Replace the installed app? + %1$s is a regular APK installed by the standard Run button. Quick Build will replace it with a proxy app designed for live reloads without reinstalling. Use the Run button to restore a normal APK. + Replace the Quick Build proxy app? + %1$s is currently the Quick Build proxy app, which reloads your edits without reinstalling. This Run replaces it with a regular APK. + Replace the app installed for this project? + Code On The Go cannot tell which app is installed for this project - the project may still be syncing. Continuing replaces whatever is installed under this project\'s app ID. + Replace + Your app crashed on the last reload. Fix the crash and save. If it keeps crashing, Quick Build cannot clear a bad reload on its own - long-press Quick Build and choose Restart session. + This resource error is now blocking every save, even code-only ones - Quick Build rebuilds all of your resources on each reload. Fix it and save. If the error names something you cannot change, long-press Quick Build and choose Restart session. + Saved. Quick Build does not deploy test sources - nothing under src/test, src/androidTest or testFixtures is part of the app it builds. Run your tests from a build task instead. + Reloaded. A running service, content provider or Application object can still be calling the previous version of the code you changed, until it restarts - close and reopen your app to be sure. + Quick Build: running initial full build + Quick Build: rebuilding app + Quick Build: restarting session - rebuilding app + Quick Build: compiling… + Quick Build: live reloaded in %1$s + Quick Build: restarted in %1$s + Quick Build: ready + Quick Build: BUILD FAILED - see Build Output + Quick Build: built, but could not be delivered - see Build Output + Quick Build: full build needed - tap Quick Build to rebuild + Quick Build: rebuild failed - save a fix to retry + Quick Build: could not start - tap Quick Build to retry + Quick Build failed. See the Build Output panel for more details. + Quick Build successful + Quick Build: compile daemon restarting + Quick Build: compiler is down - tap Quick Build to retry + Your app is not staying open + Quick Build built your changes, but your app closes before it can receive them - usually a crash while it starts up. Saving again will not help, and neither will reopening the app. Restarting the session rebuilds and reinstalls it from your current code. + Restart session + Not now + Quick Build isn\'t available for plugin projects - the build output is a .cgp package, not a runnable app. Use Run/Debug to build the plugin instead. + Quick Build needs a launchable Activity in this project - none was found. Use Run/Debug to build and inspect it instead. + Quick Build is still waiting for this project to finish syncing. Try again once the sync completes. + Quick Build needs an Android app module, and this project has none. + Quick Build needs a debuggable build variant, and \"%1$s\" is a release variant. Open Build Variants in the sidebar and select a debug variant. + Quick Build could not set up \"%1$s\". If that variant is not debuggable, open Build Variants in the sidebar and select a debug variant. + Quick Build setup failed. Check the Build Output for what went wrong. + Another build is running. Wait for it to finish, then start Quick Build again. + Quick Build could not rebuild your app. Check the Build Output for what went wrong. + Your app needs a reinstall - return to CoGo to confirm. + Your app needs a reinstall - the install prompt was cancelled. Tap Quick Build to try again. + Your app needs a reinstall - the install prompt went unanswered for %1$d seconds. Tap Quick Build to try again. + Waiting for the current Gradle build to finish - your app still needs a reinstall. Tap Quick Build to retry. + Quick Build could not start installing your app. + Quick Build could not install your app. + Quick Build installed %1$s, but Android will not open it. Restarting the session reinstalls it. + %1$s is already installed on this device and was not built here, so Quick Build cannot replace it without deleting its data. Back it up and uninstall it yourself first. + Quick Build could not restart its compiler. Tap Quick Build to try again, or restart the session from the long-press menu. (%1$s) + Quick Build is restarting its compiler - your app keeps running. If it does not come back, long-press Quick Build and choose Restart session. + Quick Build needs about %1$d MB free in app storage, but only %2$d MB is available. Free up space and try again. + Quick Build could not create its build folder at %1$s. + Quick Build\'s compiler refused to start on this project. Restarting the session from the long-press menu sets it up again. + Your app needs a reinstall - return to CoGo and tap Quick Build to try again. Undo Redo Delete @@ -1089,6 +1148,7 @@ No APK found in output listing file. APK file specified does not exist: %1$s Build was cancelled by the user. + Quick Build is setting up the app. Run will work once that finishes. Quick Run failed. Building… Installing plugin… diff --git a/subprojects/flashbar/src/main/java/com/itsaky/androidide/flashbar/Flashbar.kt b/subprojects/flashbar/src/main/java/com/itsaky/androidide/flashbar/Flashbar.kt index 908de370a8..7c8ca2c7ea 100644 --- a/subprojects/flashbar/src/main/java/com/itsaky/androidide/flashbar/Flashbar.kt +++ b/subprojects/flashbar/src/main/java/com/itsaky/androidide/flashbar/Flashbar.kt @@ -668,7 +668,7 @@ class Flashbar private constructor(private var builder: Builder) { fun onDismissed(bar: Flashbar, event: DismissEvent) } - interface OnTapListener { + fun interface OnTapListener { fun onTap(flashbar: Flashbar) } diff --git a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/ProjectManagerImpl.kt b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/ProjectManagerImpl.kt index f836d86112..b278384291 100644 --- a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/ProjectManagerImpl.kt +++ b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/ProjectManagerImpl.kt @@ -77,7 +77,6 @@ import kotlin.io.path.pathString class ProjectManagerImpl : IProjectManager, EventReceiver { - private var _indexingServiceManager: IndexingServiceManager? = null lateinit var projectPath: String @@ -90,8 +89,8 @@ class ProjectManagerImpl : return _indexingServiceManager!! } - @Volatile - internal var pluginProjectCached: Boolean? = null + @Volatile + internal var pluginProjectCached: Boolean? = null override var gradleBuild: GradleModels.GradleBuild? = null override var workspace: Workspace? = null @@ -125,9 +124,10 @@ class ProjectManagerImpl : log.warn("Project path not initialized before setup(); skipping plugin project cache check.") pluginProjectCached = null } else { - pluginProjectCached = withContext(Dispatchers.IO) { - File(projectDir, Environment.PLUGIN_API_JAR_RELATIVE_PATH).exists() - } + pluginProjectCached = + withContext(Dispatchers.IO) { + File(projectDir, Environment.PLUGIN_API_JAR_RELATIVE_PATH).exists() + } } this.gradleBuild = gradleBuild @@ -194,11 +194,12 @@ class ProjectManagerImpl : * offering a recovery path (re-sync) — instead of silently dropping its code-completion symbols. */ private fun reportUnreadableClasspathJars(workspace: Workspace) { - val names = workspace.subProjects - .filterIsInstance() - .flatMap { it.unreadableClasspathJars } - .map { it.name } - .distinct() + val names = + workspace.subProjects + .filterIsInstance() + .flatMap { it.unreadableClasspathJars } + .map { it.name } + .distinct() if (names.isEmpty()) { return } @@ -290,20 +291,28 @@ class ProjectManagerImpl : (this.androidBuildVariants as? MutableMap?)?.clear() } + /** + * Hands the resource/source generation tasks to the tooling server and returns immediately. + * + * @return whether the tasks were actually dispatched. False means the request did nothing: + * no build service, no tooling server, or a Gradle build already in progress. Callers that + * owe the request a retry - [com.itsaky.androidide.quickbuild.GenerateSourcesDeferral] - key + * off this, because the in-progress refusal is silent and transient. + */ @JvmOverloads - fun generateSources(builder: BuildService? = Lookup.getDefault().lookup(BuildService.KEY_BUILD_SERVICE)) { + fun generateSources(builder: BuildService? = Lookup.getDefault().lookup(BuildService.KEY_BUILD_SERVICE)): Boolean { if (builder == null) { log.warn("Cannot generate sources. BuildService is null.") - return + return false } if (!builder.isToolingServerStarted()) { flashError(R.string.msg_tooling_server_unavailable) - return + return false } if (builder.isBuildInProgress) { - return + return false } val tasks = @@ -342,6 +351,7 @@ class ProjectManagerImpl : notifyProjectUpdate() } } + return true } fun notifyProjectUpdate() { diff --git a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/builder/BuildService.kt b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/builder/BuildService.kt index 153f853f32..d0c60f3305 100644 --- a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/builder/BuildService.kt +++ b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/builder/BuildService.kt @@ -45,6 +45,19 @@ interface BuildService { /** Whether a build is in progress or not. */ val isBuildInProgress: Boolean + /** + * Whether a build the USER started is in progress. Differs from [isBuildInProgress] only + * while an INTERNAL build owns the single Gradle slot: Quick Build's proxy-app/prewarm build + * runs through the same [executeTasks] path, but nobody asked for it, so the editor's + * build UI (status line, first-build notice, the Run button's cancel affordance) must not + * present it as the user's build. + * + * Read this from anything the user SEES. Read the raw [isBuildInProgress] from anything + * that guards concurrency - an internal build still occupies the slot. + */ + val isUserVisibleBuildInProgress: Boolean + get() = isBuildInProgress + /** Returns `true` if and only if the tooling API server has been started, `false` otherwise. */ fun isToolingServerStarted(): Boolean diff --git a/subprojects/projects/src/test/java/com/itsaky/androidide/projects/classpath/JarFsClasspathReaderCorruptJarTest.kt b/subprojects/projects/src/test/java/com/itsaky/androidide/projects/classpath/JarFsClasspathReaderCorruptJarTest.kt index 4262aa6bab..c6869fd623 100644 --- a/subprojects/projects/src/test/java/com/itsaky/androidide/projects/classpath/JarFsClasspathReaderCorruptJarTest.kt +++ b/subprojects/projects/src/test/java/com/itsaky/androidide/projects/classpath/JarFsClasspathReaderCorruptJarTest.kt @@ -20,14 +20,14 @@ package com.itsaky.androidide.projects.classpath import com.google.common.truth.Truth.assertThat import com.itsaky.androidide.javac.services.fs.CachingJarFileSystemProvider import com.itsaky.androidide.utils.FileProvider -import java.io.File -import java.nio.file.Files import org.junit.After import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner import org.robolectric.annotation.Config +import java.io.File +import java.nio.file.Files /** * Regression test for ADFA-3364: a corrupt/truncated/zero-byte JAR among the classpath entries @@ -38,83 +38,83 @@ import org.robolectric.annotation.Config @RunWith(RobolectricTestRunner::class) @Config(manifest = Config.DEFAULT_VALUE_STRING) class JarFsClasspathReaderCorruptJarTest { - - private lateinit var tmpDir: File - private lateinit var validJar: File - private lateinit var corruptJar: File - private lateinit var zeroByteJar: File - - @Before - fun setUp() { - // The provider caches by normalized path; start clean so prior runs cannot mask behavior. - CachingJarFileSystemProvider.clearCache() - - tmpDir = Files.createTempDirectory("adfa3364").toFile() - - // A genuinely valid JAR known to contain android.content.Context. - val sourceAndroidJar = - FileProvider.testProjectRoot() - .resolve("app/src/main/resources/android.jar") - .toFile() - assertThat(sourceAndroidJar.exists()).isTrue() - - // Copy it to a unique temp path so the FS-provider cache key is distinct per run. - validJar = File(tmpDir, "valid.jar") - sourceAndroidJar.copyTo(validJar, overwrite = true) - - // A truncated JAR: take the first 64 bytes of a real JAR. It has the local-file-header - // signature but no valid end-of-central-directory record -> ZipException on open/walk. - corruptJar = File(tmpDir, "corrupt.jar") - val head = sourceAndroidJar.inputStream().use { it.readNBytes(64) } - corruptJar.writeBytes(head) - - // A zero-byte file with a .jar extension -> also unreadable as a zip. - zeroByteJar = File(tmpDir, "empty.jar") - zeroByteJar.writeBytes(ByteArray(0)) - } - - @After - fun tearDown() { - CachingJarFileSystemProvider.clearCache() - tmpDir.deleteRecursively() - } - - /** - * On the FIX branch: the corrupt JAR is skipped (ZipException caught) and the valid JAR is still - * indexed. On the pre-fix baseline: the ZipException from the corrupt JAR propagates out of - * [JarFsClasspathReader.listClasses] and this test fails with that exception. - * - * Corrupt JAR is listed FIRST so that, if the exception aborts the loop, the valid JAR after it - * never gets indexed either (stronger assertion that indexing did not abort). - */ - @Test - fun corruptJarDoesNotAbortIndexingOfRemainingEntries() { - val classes = - JarFsClasspathReader() - .listClasses(listOf(corruptJar, zeroByteJar, validJar)) - - // The valid JAR after the corrupt ones must have been fully indexed. - val context = classes.firstOrNull { it.name == "android.content.Context" } - assertThat(context).isNotNull() - assertThat(context!!.packageName).isEqualTo("android.content") - - // Sanity: a non-trivial number of classes were indexed from the valid jar. - assertThat(classes.size).isGreaterThan(100) - } - - /** - * The reader must EXPOSE the skipped JARs (not just log them) so the caller can name the offending - * dependency to the user and offer a recovery path (re-sync) instead of silently dropping symbols. - */ - @Test - fun unreadableJarsAreCollectedForUserReporting() { - val reader = JarFsClasspathReader() - reader.listClasses(listOf(corruptJar, zeroByteJar, validJar)) - - val skipped = reader.unreadableJars.map { it.name }.toSet() - // Both the truncated and the zero-byte JAR are reported... - assertThat(skipped).containsExactly("corrupt.jar", "empty.jar") - // ...and the valid JAR is NOT reported as unreadable. - assertThat(skipped).doesNotContain("valid.jar") - } + private lateinit var tmpDir: File + private lateinit var validJar: File + private lateinit var corruptJar: File + private lateinit var zeroByteJar: File + + @Before + fun setUp() { + // The provider caches by normalized path; start clean so prior runs cannot mask behavior. + CachingJarFileSystemProvider.clearCache() + + tmpDir = Files.createTempDirectory("adfa3364").toFile() + + // A genuinely valid JAR known to contain android.content.Context. + val sourceAndroidJar = + FileProvider + .testProjectRoot() + .resolve("app/src/main/resources/android.jar") + .toFile() + assertThat(sourceAndroidJar.exists()).isTrue() + + // Copy it to a unique temp path so the FS-provider cache key is distinct per run. + validJar = File(tmpDir, "valid.jar") + sourceAndroidJar.copyTo(validJar, overwrite = true) + + // A truncated JAR: take the first 64 bytes of a real JAR. It has the local-file-header + // signature but no valid end-of-central-directory record -> ZipException on open/walk. + corruptJar = File(tmpDir, "corrupt.jar") + val head = sourceAndroidJar.inputStream().use { it.readNBytes(64) } + corruptJar.writeBytes(head) + + // A zero-byte file with a .jar extension -> also unreadable as a zip. + zeroByteJar = File(tmpDir, "empty.jar") + zeroByteJar.writeBytes(ByteArray(0)) + } + + @After + fun tearDown() { + CachingJarFileSystemProvider.clearCache() + tmpDir.deleteRecursively() + } + + /** + * A corrupt JAR must be skipped (its ZipException caught) with the valid JAR still indexed - + * an uncaught ZipException propagates out of [JarFsClasspathReader.listClasses] and aborts + * indexing entirely. + * + * Corrupt JAR is listed FIRST so that, if the exception aborts the loop, the valid JAR after it + * never gets indexed either (stronger assertion that indexing did not abort). + */ + @Test + fun corruptJarDoesNotAbortIndexingOfRemainingEntries() { + val classes = + JarFsClasspathReader() + .listClasses(listOf(corruptJar, zeroByteJar, validJar)) + + // The valid JAR after the corrupt ones must have been fully indexed. + val context = classes.firstOrNull { it.name == "android.content.Context" } + assertThat(context).isNotNull() + assertThat(context!!.packageName).isEqualTo("android.content") + + // Sanity: a non-trivial number of classes were indexed from the valid jar. + assertThat(classes.size).isGreaterThan(100) + } + + /** + * The reader must EXPOSE the skipped JARs (not just log them) so the caller can name the offending + * dependency to the user and offer a recovery path (re-sync) instead of silently dropping symbols. + */ + @Test + fun unreadableJarsAreCollectedForUserReporting() { + val reader = JarFsClasspathReader() + reader.listClasses(listOf(corruptJar, zeroByteJar, validJar)) + + val skipped = reader.unreadableJars.map { it.name }.toSet() + // Both the truncated and the zero-byte JAR are reported... + assertThat(skipped).containsExactly("corrupt.jar", "empty.jar") + // ...and the valid JAR is NOT reported as unreadable. + assertThat(skipped).doesNotContain("valid.jar") + } } diff --git a/termux/termux-app/src/test/java/com/termux/app/TermuxServiceShellManagerNpeTest.java b/termux/termux-app/src/test/java/com/termux/app/TermuxServiceShellManagerNpeTest.java index c9c9ad8ca9..27c90c2ef7 100644 --- a/termux/termux-app/src/test/java/com/termux/app/TermuxServiceShellManagerNpeTest.java +++ b/termux/termux-app/src/test/java/com/termux/app/TermuxServiceShellManagerNpeTest.java @@ -19,14 +19,11 @@ * {@code TermuxApplication.onCreate()} does NOT run, so the static * {@link TermuxShellManager} singleton is still {@code null}. * - *

On the pre-fix baseline, {@code TermuxService.onCreate()} assigned - * {@code mShellManager = TermuxShellManager.getShellManager()} (which returns the null - * singleton), then immediately called {@code runStartForeground() -> buildNotification() -> - * getTermuxSessionsSize()}, dereferencing the null {@code mShellManager} and throwing a - * {@link NullPointerException}. - * - *

The fix changes that assignment to {@code TermuxShellManager.init(applicationContext)}, - * which lazily creates the singleton, so the service starts cleanly. + *

{@code TermuxService.onCreate()} must therefore obtain the manager via + * {@code TermuxShellManager.init(applicationContext)}, which lazily creates the singleton. + * Reading it with {@code TermuxShellManager.getShellManager()} instead hands back the null + * singleton, and the immediately-following {@code runStartForeground() -> buildNotification() + * -> getTermuxSessionsSize()} dereferences it and throws a {@link NullPointerException}. * *

This test simulates the auto-restart by forcing the static singleton back to {@code null} * before creating the service, then asserts the service comes up and From 9da54db286f28da6b3aa237dce9554c02835f087 Mon Sep 17 00:00:00 2001 From: Bryan Chan Date: Fri, 21 Aug 2026 22:49:07 -0700 Subject: [PATCH 2/3] =?UTF-8?q?ADFA-4128:=20qb=2002=20review=20fixes=20?= =?UTF-8?q?=E2=80=94=20flag-off=20gating=20+=20test=20gaps?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Important 1 (flashbar tap/swipe dismissal shipped flag-off): gated behind FeatureFlags.isExperimentsEnabled via indefiniteErrorBarDismissesOnTouch() (new FlashbarDismissGate.kt, JVM-pure so unit tests can load it). The change was Quick-Build-driven (bar occludes the toolbar's Run/Quick Build buttons); for flag-off users an accidental brush must not dismiss an unread error, so they keep Dismiss-button-only until this ships on its own sign-off. Covered by FlashbarDismissGateTest (flag-off test fails without the gate). Important 2 (tooling-jar stamp-skip + atomic rename shipped flag-off): left un-gated, with a code comment saying why — a torn jar kills project init for every user, Quick Build or not, so gating it would leave flag-off users exposed. Marked "ships flag-off — needs Bryan sign-off". Logic extracted into isToolingJarCurrent/extractToolingJar (@VisibleForTesting) so it is JVM-testable; behavior unchanged. Test gap (ToolsManager.updateToolingJar): ToolsManagerToolingJarTest covers stamp-match skip, stamp-mismatch/missing-jar/missing-stamp/null-stamp re-extract, atomic copy leaving no .part and stamping only after the rename, and the rename-failure path writing no stamp (fails if the stamp were written before the rename). Test gap (FeatureFlags semantics): FeatureFlagsTest covers the loaded latch, failed-read-retries-on-next-initialize, and refresh() replacing a latched all-false (direct-boot) snapshot. Enabled by a JVM test seam (flagFileResolver + resetForTest; downloadsDir made lazy) instead of Robolectric — common has no Robolectric dep and none was added. Test gap (generateSources Boolean contract): ProjectManagerImplGenerateSourcesTest pins false on null service / server down / build in progress and true on dispatch, which the later Quick Build deferral keys its retry off. Adjacent minors while editing those lines: log ignored stamp-write failure in ToolsManager; reworded the dangling GenerateSourcesDeferral KDoc link in ProjectManagerImpl.generateSources to prose. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Kj9YeCDHGp9DU8LPtfWJ7W --- .../androidide/managers/ToolsManager.java | 74 ++++++----- .../itsaky/androidide/utils/FeatureFlags.kt | 22 +++- .../androidide/utils/FlashbarActivityUtils.kt | 6 +- .../androidide/utils/FlashbarDismissGate.kt | 30 +++++ .../managers/ToolsManagerToolingJarTest.kt | 117 ++++++++++++++++++ .../androidide/utils/FeatureFlagsTest.kt | 102 +++++++++++++++ .../utils/FlashbarDismissGateTest.kt | 54 ++++++++ subprojects/projects/build.gradle.kts | 9 +- .../androidide/projects/ProjectManagerImpl.kt | 4 +- .../ProjectManagerImplGenerateSourcesTest.kt | 81 ++++++++++++ 10 files changed, 461 insertions(+), 38 deletions(-) create mode 100644 common/src/main/java/com/itsaky/androidide/utils/FlashbarDismissGate.kt create mode 100644 common/src/test/java/com/itsaky/androidide/managers/ToolsManagerToolingJarTest.kt create mode 100644 common/src/test/java/com/itsaky/androidide/utils/FeatureFlagsTest.kt create mode 100644 common/src/test/java/com/itsaky/androidide/utils/FlashbarDismissGateTest.kt create mode 100644 subprojects/projects/src/test/java/com/itsaky/androidide/projects/ProjectManagerImplGenerateSourcesTest.kt diff --git a/common/src/main/java/com/itsaky/androidide/managers/ToolsManager.java b/common/src/main/java/com/itsaky/androidide/managers/ToolsManager.java index ebf3c3b6c3..771d1877e7 100755 --- a/common/src/main/java/com/itsaky/androidide/managers/ToolsManager.java +++ b/common/src/main/java/com/itsaky/androidide/managers/ToolsManager.java @@ -22,6 +22,7 @@ import android.os.Build; import androidx.annotation.NonNull; +import androidx.annotation.VisibleForTesting; import androidx.annotation.WorkerThread; import com.aayushatharva.brotli4j.Brotli4jLoader; import com.aayushatharva.brotli4j.decoder.BrotliInputStream; @@ -128,6 +129,44 @@ public static void init(@NonNull BaseApplication app, Runnable onFinish) { }); } + /** + * Copies the stream to a temp sibling of toolingJarFile, renames it into place, then writes the stamp. The tooling server starts concurrently with this extraction (both run at app init), and launching `java -jar` against a half-written jar kills project init ("An unexpected error occurred while trying to open file ..."), so a partial jar must never be visible at the final path. rename(2) within one directory atomically replaces the target on Linux. The stamp is written only after a successful rename, so a failure at any step leaves the stamp absent and the next launch retries. Always closes the stream. + */ + @VisibleForTesting + static void extractToolingJar(InputStream toolingJarStream, File toolingJarFile, File stampFile, String stamp) { + try { + final var tempFile = new File(toolingJarFile.getParentFile(), toolingJarFile.getName() + ".part"); + Objects.requireNonNull(toolingJarFile.getParentFile()).mkdirs(); + try (final var fos = new FileOutputStream(tempFile)) { + IoUtilsKt.transferToStream(toolingJarStream, fos); + } + if (!tempFile.renameTo(toolingJarFile)) { + LOG.error("Failed to move extracted tooling API jar into place"); + return; + } + if (stamp != null && !FileIOUtils.writeFileFromString(stampFile, stamp)) { + // Fail-safe: a lost stamp just re-extracts next launch, but say so. + LOG.warn("Failed to write tooling jar stamp file {}", stampFile); + } + } catch (Throwable err) { + LOG.error("Failed to copy tooling API jar", err); + } finally { + try { + toolingJarStream.close(); + } catch (IOException e) { + LOG.error("Failed to close tooling API jar stream", e); + } + } + } + + /** + * Whether the jar at the final path was extracted from this exact APK install. True only when the jar exists AND the stamp file holds this install's stamp. The stamp is written only after a complete extraction, so a partial copy from a killed process can never satisfy this check. A null stamp (package lookup failed) always re-extracts. + */ + @VisibleForTesting + static boolean isToolingJarCurrent(File toolingJarFile, File stampFile, String stamp) { + return toolingJarFile.isFile() && stamp != null && stamp.equals(readStampFile(stampFile)); + } + private static void deleteIdeenv() { final var file = new File(Environment.BIN_DIR, "ideenv"); if (file.exists() && !file.delete()) { @@ -317,13 +356,13 @@ private static void updateToolingJar(BaseApplication app) { // Ensure relevant shared libraries are loaded Brotli4jLoader.ensureAvailability(); + // Deliberately NOT gated on FeatureFlags.isExperimentsEnabled: a torn jar kills + // project init for every user, Quick Build or not, so gating would leave flag-off users exposed. final var toolingJarFile = Environment.TOOLING_API_JAR; final var stampFile = new File(toolingJarFile.getParentFile(), toolingJarFile.getName() + ".stamp"); final var stamp = installedApkStamp(app); - if (toolingJarFile.isFile() && stamp != null && stamp.equals(readStampFile(stampFile))) { + if (isToolingJarCurrent(toolingJarFile, stampFile, stamp)) { // The jar from this exact APK install is already extracted; skip the copy. - // The stamp is written only after a complete extraction, so a partial - // copy from a killed process can never satisfy this check. return; } @@ -341,34 +380,7 @@ private static void updateToolingJar(BaseApplication app) { } } - try { - // Extract to a temp sibling, then rename into place. The tooling server - // starts concurrently with this extraction (both run at app init), and - // launching `java -jar` against a half-written jar kills project init - // ("An unexpected error occurred while trying to open file ..."), so a - // partial jar must never be visible at the final path. rename(2) within - // one directory atomically replaces the target on Linux. - final var tempFile = new File(toolingJarFile.getParentFile(), toolingJarFile.getName() + ".part"); - Objects.requireNonNull(toolingJarFile.getParentFile()).mkdirs(); - try (final var fos = new FileOutputStream(tempFile)) { - IoUtilsKt.transferToStream(toolingJarStream, fos); - } - if (!tempFile.renameTo(toolingJarFile)) { - LOG.error("Failed to move extracted tooling API jar into place"); - return; - } - if (stamp != null) { - FileIOUtils.writeFileFromString(stampFile, stamp); - } - } catch (Throwable err) { - LOG.error("Failed to copy tooling API jar", err); - } finally { - try { - toolingJarStream.close(); - } catch (IOException e) { - LOG.error("Failed to close tooling API jar stream", e); - } - } + extractToolingJar(toolingJarStream, toolingJarFile, stampFile, stamp); } private static void writeInitScript() { diff --git a/common/src/main/java/com/itsaky/androidide/utils/FeatureFlags.kt b/common/src/main/java/com/itsaky/androidide/utils/FeatureFlags.kt index 97955c0f66..4ad8e4c26f 100644 --- a/common/src/main/java/com/itsaky/androidide/utils/FeatureFlags.kt +++ b/common/src/main/java/com/itsaky/androidide/utils/FeatureFlags.kt @@ -1,6 +1,7 @@ package com.itsaky.androidide.utils import android.os.Environment +import androidx.annotation.VisibleForTesting import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock @@ -49,8 +50,25 @@ object FeatureFlags { */ private var loaded = false - private val downloadsDir = + // Lazy so JVM unit tests that install a flagFileResolver never touch android.os.Environment. + private val downloadsDir: File by lazy { Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS) + } + + /** + * Resolves a flag sentinel file by name. Test seam: unit tests point this at a temp dir + * (or throw from it to exercise the failed-read path); production resolves against the + * shared Downloads dir, which needs a real Android environment. + */ + @VisibleForTesting + internal var flagFileResolver: (String) -> File = { name -> File(downloadsDir, name) } + + /** Drops the cached snapshot and the [loaded] latch so a test starts from process-fresh state. */ + @VisibleForTesting + internal fun resetForTest() { + flags = FlagsCache.DEFAULT + loaded = false + } /** * Whether Code On the Go experiments are enabled. @@ -133,7 +151,7 @@ object FeatureFlags { /** Reads every flag file. Call under [mutex]. */ private suspend fun load() { - fun checkFlag(fileName: String) = File(downloadsDir, fileName).exists() + fun checkFlag(fileName: String) = flagFileResolver(fileName).exists() val read = withContext(Dispatchers.IO) { diff --git a/common/src/main/java/com/itsaky/androidide/utils/FlashbarActivityUtils.kt b/common/src/main/java/com/itsaky/androidide/utils/FlashbarActivityUtils.kt index 76074c52d4..82e4f3e6d0 100644 --- a/common/src/main/java/com/itsaky/androidide/utils/FlashbarActivityUtils.kt +++ b/common/src/main/java/com/itsaky/androidide/utils/FlashbarActivityUtils.kt @@ -90,8 +90,10 @@ private fun Activity.configureFlashbar( // read as dead with nothing on screen saying why. Measured on an a56: the bar occupied // y 236-371 while the toolbar buttons sat at y 261-383. // So any touch on the bar, and any swipe, gets rid of it - not just the Dismiss button. - builder.listenBarTaps { it.dismiss() } - builder.enableSwipeToDismiss() + if (indefiniteErrorBarDismissesOnTouch()) { + builder.listenBarTaps { it.dismiss() } + builder.enableSwipeToDismiss() + } } when (msg) { diff --git a/common/src/main/java/com/itsaky/androidide/utils/FlashbarDismissGate.kt b/common/src/main/java/com/itsaky/androidide/utils/FlashbarDismissGate.kt new file mode 100644 index 0000000000..b511556707 --- /dev/null +++ b/common/src/main/java/com/itsaky/androidide/utils/FlashbarDismissGate.kt @@ -0,0 +1,30 @@ +/* + * This file is part of AndroidIDE. + * + * AndroidIDE is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * AndroidIDE is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with AndroidIDE. If not, see . + */ + +package com.itsaky.androidide.utils + +/** + * Whether an indefinite error bar also dismisses on any tap or swipe, not only via its + * explicit Dismiss button. Gated on [FeatureFlags.isExperimentsEnabled]: the touch-dismiss + * change was driven by Quick Build (the bar occludes the toolbar's Run/Quick Build buttons), + * and for flag-off users an accidental brush must not dismiss an unread error, so they keep + * the Dismiss-button-only behavior until this ships on its own merits. + * + * Lives in its own file (not FlashbarActivityUtils.kt, whose top-level vals need + * android.graphics.Color) so JVM unit tests can load it. + */ +internal fun indefiniteErrorBarDismissesOnTouch(): Boolean = FeatureFlags.isExperimentsEnabled diff --git a/common/src/test/java/com/itsaky/androidide/managers/ToolsManagerToolingJarTest.kt b/common/src/test/java/com/itsaky/androidide/managers/ToolsManagerToolingJarTest.kt new file mode 100644 index 0000000000..4df4506898 --- /dev/null +++ b/common/src/test/java/com/itsaky/androidide/managers/ToolsManagerToolingJarTest.kt @@ -0,0 +1,117 @@ +package com.itsaky.androidide.managers + +import com.google.common.truth.Truth.assertThat +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import java.io.ByteArrayInputStream +import java.io.File + +/** + * Pins the tooling-jar extraction contract in [ToolsManager]: the stamp-match skip, the + * re-extract triggers (stamp mismatch, missing jar, null stamp), the atomic + * temp-then-rename copy, and the stamp-only-after-successful-rename ordering that makes + * the skip check safe against a killed process. + */ +class ToolsManagerToolingJarTest { + @get:Rule + val tempFolder = TemporaryFolder() + + private val stamp = "1.2.3:1723456789" + + private fun jarFile(): File = File(tempFolder.root, "tooling-api-all.jar") + + private fun stampFile(): File = File(tempFolder.root, "tooling-api-all.jar.stamp") + + @Test + fun `matching stamp with an extracted jar skips re-extraction`() { + jarFile().writeText("jar-bytes") + stampFile().writeText(stamp) + + assertThat(ToolsManager.isToolingJarCurrent(jarFile(), stampFile(), stamp)).isTrue() + } + + @Test + fun `a stale stamp re-extracts`() { + jarFile().writeText("jar-bytes") + stampFile().writeText("1.2.2:1700000000") + + assertThat(ToolsManager.isToolingJarCurrent(jarFile(), stampFile(), stamp)).isFalse() + } + + @Test + fun `a missing jar re-extracts even when the stamp matches`() { + stampFile().writeText(stamp) + + assertThat(ToolsManager.isToolingJarCurrent(jarFile(), stampFile(), stamp)).isFalse() + } + + @Test + fun `a missing stamp file re-extracts`() { + jarFile().writeText("jar-bytes") + + assertThat(ToolsManager.isToolingJarCurrent(jarFile(), stampFile(), stamp)).isFalse() + } + + @Test + fun `a null stamp (package lookup failed) always re-extracts`() { + jarFile().writeText("jar-bytes") + stampFile().writeText(stamp) + + assertThat(ToolsManager.isToolingJarCurrent(jarFile(), stampFile(), null)).isFalse() + } + + @Test + fun `extraction lands the full content, leaves no temp file, and writes the stamp`() { + val content = "the-tooling-jar-bytes".toByteArray() + + ToolsManager.extractToolingJar(ByteArrayInputStream(content), jarFile(), stampFile(), stamp) + + assertThat(jarFile().readBytes()).isEqualTo(content) + assertThat(File(tempFolder.root, "tooling-api-all.jar.part").exists()).isFalse() + assertThat(stampFile().readText()).isEqualTo(stamp) + // The freshly-extracted state must satisfy the next launch's skip check. + assertThat(ToolsManager.isToolingJarCurrent(jarFile(), stampFile(), stamp)).isTrue() + } + + @Test + fun `extraction replaces an existing jar in place`() { + jarFile().writeText("old-install-bytes") + val content = "new-install-bytes".toByteArray() + + ToolsManager.extractToolingJar(ByteArrayInputStream(content), jarFile(), stampFile(), stamp) + + assertThat(jarFile().readBytes()).isEqualTo(content) + } + + @Test + fun `a null stamp still extracts the jar but writes no stamp`() { + val content = "jar-bytes".toByteArray() + + ToolsManager.extractToolingJar(ByteArrayInputStream(content), jarFile(), stampFile(), null) + + assertThat(jarFile().readBytes()).isEqualTo(content) + assertThat(stampFile().exists()).isFalse() + } + + @Test + fun `a failed rename writes no stamp, so the next launch retries`() { + // A non-empty directory at the jar's final path makes File.renameTo fail + // deterministically on POSIX, standing in for EIO/permission oddities. + val blockedTarget = jarFile() + blockedTarget.mkdirs() + File(blockedTarget, "occupant").writeText("x") + + ToolsManager.extractToolingJar( + ByteArrayInputStream("jar-bytes".toByteArray()), + blockedTarget, + stampFile(), + stamp, + ) + + // Stamp absent -> isToolingJarCurrent is false -> the next launch re-extracts + // instead of trusting a jar that never made it into place. + assertThat(stampFile().exists()).isFalse() + assertThat(ToolsManager.isToolingJarCurrent(blockedTarget, stampFile(), stamp)).isFalse() + } +} diff --git a/common/src/test/java/com/itsaky/androidide/utils/FeatureFlagsTest.kt b/common/src/test/java/com/itsaky/androidide/utils/FeatureFlagsTest.kt new file mode 100644 index 0000000000..487b09c67f --- /dev/null +++ b/common/src/test/java/com/itsaky/androidide/utils/FeatureFlagsTest.kt @@ -0,0 +1,102 @@ +package com.itsaky.androidide.utils + +import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.test.runTest +import org.junit.After +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import java.io.File +import java.io.IOException + +/** + * Pins the [FeatureFlags] load semantics: the `loaded` latch (one disk read per process), + * a failed read retrying on the next [FeatureFlags.initialize] instead of latching, and + * [FeatureFlags.refresh] replacing an already-latched snapshot (the direct-boot all-false + * snapshot must not stick). + */ +class FeatureFlagsTest { + @get:Rule + val tempFolder = TemporaryFolder() + + private lateinit var originalResolver: (String) -> File + + @Before + fun setUp() { + originalResolver = FeatureFlags.flagFileResolver + FeatureFlags.resetForTest() + FeatureFlags.flagFileResolver = { name -> File(tempFolder.root, name) } + } + + @After + fun tearDown() { + FeatureFlags.flagFileResolver = originalResolver + FeatureFlags.resetForTest() + } + + @Test + fun `initialize reads the sentinel files`() = + runTest { + tempFolder.newFile("CodeOnTheGo.exp") + + FeatureFlags.initialize() + + assertThat(FeatureFlags.isExperimentsEnabled).isTrue() + assertThat(FeatureFlags.isDebugLoggingEnabled).isFalse() + assertThat(FeatureFlags.isQuickBuildBenchEnabled).isFalse() + } + + @Test + fun `initialize latches - a second call does not re-read disk`() = + runTest { + FeatureFlags.initialize() + assertThat(FeatureFlags.isExperimentsEnabled).isFalse() + + // The file appearing after the first read changes nothing in a running process. + tempFolder.newFile("CodeOnTheGo.exp") + FeatureFlags.initialize() + + assertThat(FeatureFlags.isExperimentsEnabled).isFalse() + } + + @Test + fun `a failed read does not latch - the next initialize retries`() = + runTest { + FeatureFlags.flagFileResolver = { throw IOException("storage unavailable") } + FeatureFlags.initialize() + assertThat(FeatureFlags.isExperimentsEnabled).isFalse() + + tempFolder.newFile("CodeOnTheGo.exp") + FeatureFlags.flagFileResolver = { name -> File(tempFolder.root, name) } + FeatureFlags.initialize() + + assertThat(FeatureFlags.isExperimentsEnabled).isTrue() + } + + @Test + fun `refresh re-reads even after initialize has latched`() = + runTest { + // Direct-boot analog: a genuine read that saw no files latches an all-false snapshot. + FeatureFlags.initialize() + assertThat(FeatureFlags.isExperimentsEnabled).isFalse() + + tempFolder.newFile("CodeOnTheGo.exp") + FeatureFlags.refresh() + + assertThat(FeatureFlags.isExperimentsEnabled).isTrue() + } + + @Test + fun `refresh drops flags whose sentinel file disappeared`() = + runTest { + val sentinel = tempFolder.newFile("CodeOnTheGo.exp") + FeatureFlags.initialize() + assertThat(FeatureFlags.isExperimentsEnabled).isTrue() + + check(sentinel.delete()) + FeatureFlags.refresh() + + assertThat(FeatureFlags.isExperimentsEnabled).isFalse() + } +} diff --git a/common/src/test/java/com/itsaky/androidide/utils/FlashbarDismissGateTest.kt b/common/src/test/java/com/itsaky/androidide/utils/FlashbarDismissGateTest.kt new file mode 100644 index 0000000000..2cfd58bf67 --- /dev/null +++ b/common/src/test/java/com/itsaky/androidide/utils/FlashbarDismissGateTest.kt @@ -0,0 +1,54 @@ +package com.itsaky.androidide.utils + +import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.test.runTest +import org.junit.After +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import java.io.File + +/** + * Pins the flag gate on indefinite-error-bar touch dismissal: flag-off users keep the + * pre-existing Dismiss-button-only behavior (an accidental brush must not dismiss an + * unread error); tap-anywhere/swipe dismissal is experiments-only until it ships on its + * own sign-off. Without the gate in [indefiniteErrorBarDismissesOnTouch], the flag-off + * test goes red. + */ +class FlashbarDismissGateTest { + @get:Rule + val tempFolder = TemporaryFolder() + + private lateinit var originalResolver: (String) -> File + + @Before + fun setUp() { + originalResolver = FeatureFlags.flagFileResolver + FeatureFlags.resetForTest() + FeatureFlags.flagFileResolver = { name -> File(tempFolder.root, name) } + } + + @After + fun tearDown() { + FeatureFlags.flagFileResolver = originalResolver + FeatureFlags.resetForTest() + } + + @Test + fun `flag-off - indefinite error bars do not dismiss on touch`() = + runTest { + FeatureFlags.initialize() + + assertThat(indefiniteErrorBarDismissesOnTouch()).isFalse() + } + + @Test + fun `experiments on - indefinite error bars dismiss on touch`() = + runTest { + tempFolder.newFile("CodeOnTheGo.exp") + FeatureFlags.initialize() + + assertThat(indefiniteErrorBarDismissesOnTouch()).isTrue() + } +} diff --git a/subprojects/projects/build.gradle.kts b/subprojects/projects/build.gradle.kts index 9d2683ae80..99db271066 100644 --- a/subprojects/projects/build.gradle.kts +++ b/subprojects/projects/build.gradle.kts @@ -39,5 +39,12 @@ dependencies { implementation(libs.google.auto.service.annotations) implementation(libs.google.guava) - testImplementation(projects.testing.tooling) + // Test-framework deps are declared directly rather than via projects.testing.tooling: + // that module pulls in :testing:common, which shares Gradle coordinates (group:name) + // with :common and silently replaces it on the unit-test runtime classpath -- every + // :common class then fails to load under Robolectric. + testImplementation(libs.tests.junit) + testImplementation(libs.tests.robolectric) + testImplementation(libs.tests.google.truth) + testImplementation(libs.tests.mockk) } diff --git a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/ProjectManagerImpl.kt b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/ProjectManagerImpl.kt index b278384291..b10623835a 100644 --- a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/ProjectManagerImpl.kt +++ b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/ProjectManagerImpl.kt @@ -296,8 +296,8 @@ class ProjectManagerImpl : * * @return whether the tasks were actually dispatched. False means the request did nothing: * no build service, no tooling server, or a Gradle build already in progress. Callers that - * owe the request a retry - [com.itsaky.androidide.quickbuild.GenerateSourcesDeferral] - key - * off this, because the in-progress refusal is silent and transient. + * owe the request a retry (the Quick Build generate-sources deferral, added later in this + * stack) key off this, because the in-progress refusal is silent and transient. */ @JvmOverloads fun generateSources(builder: BuildService? = Lookup.getDefault().lookup(BuildService.KEY_BUILD_SERVICE)): Boolean { diff --git a/subprojects/projects/src/test/java/com/itsaky/androidide/projects/ProjectManagerImplGenerateSourcesTest.kt b/subprojects/projects/src/test/java/com/itsaky/androidide/projects/ProjectManagerImplGenerateSourcesTest.kt new file mode 100644 index 0000000000..748a04e935 --- /dev/null +++ b/subprojects/projects/src/test/java/com/itsaky/androidide/projects/ProjectManagerImplGenerateSourcesTest.kt @@ -0,0 +1,81 @@ +package com.itsaky.androidide.projects + +import com.google.common.truth.Truth.assertThat +import com.itsaky.androidide.projects.builder.BuildService +import com.itsaky.androidide.tooling.api.messages.result.TaskExecutionResult +import com.itsaky.androidide.utils.flashError +import io.mockk.every +import io.mockk.justRun +import io.mockk.mockk +import io.mockk.mockkStatic +import io.mockk.unmockkStatic +import io.mockk.verify +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import java.util.concurrent.CompletableFuture + +/** + * Pins the [ProjectManagerImpl.generateSources] Boolean contract: false means the request + * did nothing (null build service, tooling server down, or a Gradle build already holding + * the slot), true means the tasks were handed to the tooling server. The Quick Build + * generate-sources deferral (added later in this stack) keys its retry off this value, so + * a silently-flipped refusal would break it without any other test noticing. + */ +@RunWith(RobolectricTestRunner::class) +@Config(manifest = Config.NONE) +class ProjectManagerImplGenerateSourcesTest { + @Before + fun setUp() { + // The server-down branch flashes an error bar; there is no foreground activity in + // this test, so stub the top-level flashError out. + mockkStatic("com.itsaky.androidide.utils.FlashbarUtilsKt") + justRun { flashError(any()) } + } + + @After + fun tearDown() { + unmockkStatic("com.itsaky.androidide.utils.FlashbarUtilsKt") + } + + @Test + fun `a null build service returns false`() { + assertThat(ProjectManagerImpl().generateSources(null)).isFalse() + } + + @Test + fun `a stopped tooling server returns false and dispatches nothing`() { + val service = mockk() + every { service.isToolingServerStarted() } returns false + + assertThat(ProjectManagerImpl().generateSources(service)).isFalse() + verify(exactly = 0) { service.executeTasks(*anyVararg()) } + } + + @Test + fun `a build already in progress returns false and dispatches nothing`() { + val service = mockk() + every { service.isToolingServerStarted() } returns true + every { service.isBuildInProgress } returns true + + assertThat(ProjectManagerImpl().generateSources(service)).isFalse() + verify(exactly = 0) { service.executeTasks(*anyVararg()) } + } + + @Test + fun `an idle tooling server dispatches the tasks and returns true`() { + val service = mockk() + every { service.isToolingServerStarted() } returns true + every { service.isBuildInProgress } returns false + // A fresh manager has no workspace, so the dispatched task list is empty - stub and + // verify that exact call rather than a vararg matcher. + every { service.executeTasks() } returns + CompletableFuture.completedFuture(null) + + assertThat(ProjectManagerImpl().generateSources(service)).isTrue() + verify(exactly = 1) { service.executeTasks() } + } +} From 22446a30fd29ec1725afa8e61c05e0a88fb76440 Mon Sep 17 00:00:00 2001 From: Bryan Chan Date: Wed, 26 Aug 2026 23:43:44 -0700 Subject: [PATCH 3/3] ADFA-4128 (2/11): address CodeRabbit review - F1714-1 publish the FeatureFlags snapshot with @Volatile - F1714-3 assert both Quick Build sentinels turn their own flag on - F1714-4 make the ADFA-4328 repro prove the worker actually parked - F1714-5 replace the em dash in the ContentReadWrite KDoc Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FstXxJ5cwWPcvmhZ9vJgJ7 --- .../itsaky/androidide/utils/FeatureFlags.kt | 5 ++++ .../androidide/utils/FeatureFlagsTest.kt | 26 +++++++++++++++++++ .../utils/KeyedDebouncingActionCancelTest.kt | 26 ++++++++++++++----- .../editor/utils/ContentReadWrite.kt | 2 +- 4 files changed, 51 insertions(+), 8 deletions(-) diff --git a/common/src/main/java/com/itsaky/androidide/utils/FeatureFlags.kt b/common/src/main/java/com/itsaky/androidide/utils/FeatureFlags.kt index 4ad8e4c26f..296e4f204e 100644 --- a/common/src/main/java/com/itsaky/androidide/utils/FeatureFlags.kt +++ b/common/src/main/java/com/itsaky/androidide/utils/FeatureFlags.kt @@ -40,6 +40,11 @@ object FeatureFlags { private val logger = LoggerFactory.getLogger(FeatureFlags::class.java) private val mutex = Mutex() + + // The getters below read this without the mutex the sole writer holds. FlagsCache is + // immutable, so publishing the reference is the whole fix - without it a reader can keep + // seeing the direct-boot all-false snapshot after refresh() has replaced it. + @Volatile private var flags = FlagsCache.DEFAULT /** diff --git a/common/src/test/java/com/itsaky/androidide/utils/FeatureFlagsTest.kt b/common/src/test/java/com/itsaky/androidide/utils/FeatureFlagsTest.kt index 487b09c67f..438c55c597 100644 --- a/common/src/test/java/com/itsaky/androidide/utils/FeatureFlagsTest.kt +++ b/common/src/test/java/com/itsaky/androidide/utils/FeatureFlagsTest.kt @@ -99,4 +99,30 @@ class FeatureFlagsTest { assertThat(FeatureFlags.isExperimentsEnabled).isFalse() } + + // The two Quick Build sentinels get a test each. Nothing else asserts that either + // filename maps to its flag, so a typo or a swapped pair would leave the bench harness + // silently unarmed and this suite green - visible only as a device run with no events. + + @Test + fun `the qbbench sentinel arms the bench hooks and nothing else`() = + runTest { + tempFolder.newFile("CodeOnTheGo.qbbench") + + FeatureFlags.initialize() + + assertThat(FeatureFlags.isQuickBuildBenchEnabled).isTrue() + assertThat(FeatureFlags.isQuickBuildWarmCompileDisabled).isFalse() + } + + @Test + fun `the qbnoseed sentinel disables warm compile and nothing else`() = + runTest { + tempFolder.newFile("CodeOnTheGo.qbnoseed") + + FeatureFlags.initialize() + + assertThat(FeatureFlags.isQuickBuildWarmCompileDisabled).isTrue() + assertThat(FeatureFlags.isQuickBuildBenchEnabled).isFalse() + } } diff --git a/common/src/test/java/com/itsaky/androidide/utils/KeyedDebouncingActionCancelTest.kt b/common/src/test/java/com/itsaky/androidide/utils/KeyedDebouncingActionCancelTest.kt index 33f7a012ae..582b9c63ea 100644 --- a/common/src/test/java/com/itsaky/androidide/utils/KeyedDebouncingActionCancelTest.kt +++ b/common/src/test/java/com/itsaky/androidide/utils/KeyedDebouncingActionCancelTest.kt @@ -1,12 +1,14 @@ package com.itsaky.androidide.utils import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineExceptionHandler import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.channels.ClosedReceiveChannelException import kotlinx.coroutines.delay import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout import org.junit.Test import java.util.concurrent.atomic.AtomicReference import kotlin.coroutines.CoroutineContext @@ -35,23 +37,33 @@ class KeyedDebouncingActionCancelTest { val ctx: CoroutineContext = scope.coroutineContext + // Signalled from inside the action, so the test knows the worker really got that + // far. A fixed delay cannot tell "the worker is parked on receive()" apart from + // "the worker never started" - and in the second case the cancellation raises a + // CancellationException the handler never sees, so the repro silently does not run + // and the test still reports green. + val actionRan = CompletableDeferred() + val debouncer = KeyedDebouncingAction( scope = scope, debounceDuration = 50.milliseconds, actionContext = ctx, - // Never invoked: the worker is cancelled while parked on receive(). - action = { _, _ -> }, + action = { _, _ -> actionRan.complete(Unit) }, ) // schedule() creates the entry + launches the worker. With a CONFLATED channel and - // no further sends, the worker debounces the single key, runs the (empty) action, - // then loops back and parks on channel.receive() waiting for the next key. + // no further sends, the worker debounces the single key, runs the action, then + // loops back and parks on channel.receive() waiting for the next key. debouncer.schedule("k") - // Give the worker time to: receive "k", run the empty action, loop, and PARK on - // the next channel.receive(). 200ms >> 50ms debounce window. - delay(200) + // Fails loudly rather than passing vacuously if the worker never reached the action. + withTimeout(5_000) { actionRan.await() } + assertThat(actionRan.isCompleted).isTrue() + + // The action has returned; the remaining hop is joining the action job and looping + // back to the park, which has no observable signal of its own. + delay(100) // Cancel the entry while the worker is parked on receive(). debouncer.cancelPending("k") diff --git a/editor/src/main/java/com/itsaky/androidide/editor/utils/ContentReadWrite.kt b/editor/src/main/java/com/itsaky/androidide/editor/utils/ContentReadWrite.kt index 74ae5d455e..55c2835099 100644 --- a/editor/src/main/java/com/itsaky/androidide/editor/utils/ContentReadWrite.kt +++ b/editor/src/main/java/com/itsaky/androidide/editor/utils/ContentReadWrite.kt @@ -34,7 +34,7 @@ object ContentReadWrite { /** * Write this [Content] to the given [File]. * - * Writes IN PLACE — opens [file] directly and truncates + writes sequentially; this is + * Writes IN PLACE - opens [file] directly and truncates + writes sequentially; this is * NOT a temp-file-then-rename swap. A filesystem watcher observing a save from this * method sees the target path itself change, never a sibling temp file (that pattern * is specific to EXTERNAL tools like `sed -i` or `git checkout`).