-
-
Notifications
You must be signed in to change notification settings - Fork 63
ADFA-4128 (7/11): quickbuild:core — provisioning and the daemon client #1719
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
fryanpan
wants to merge
26
commits into
feature/ADFA-4128-qb-06-core-deploy
from
feature/ADFA-4128-qb-07-core-provisioning
Open
Changes from all commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
b4cd32f
ADFA-4128: qb 07/12 core-provisioning — Core slice 3: proxy-app insta…
fryanpan 481b699
ADFA-4128: qb 07 review fixes — parseDiagnostics no-throw contract + …
fryanpan 681a7ee
ADFA-4128 (7/11): address CodeRabbit review
fryanpan ac2ff7c
ADFA-4128: qb-07 review fixes - per-spawn daemon state, IO-confined i…
fryanpan 494982a
ADFA-4128: 0902 review round on quickbuild:core provisioning
fryanpan 11d8846
style: spotless reformat of DaemonProcessClient, no functional change
fryanpan b7cabc0
ADFA-4128: shutdown takes the start mutex, and the polite stop runs u…
fryanpan 674a227
ADFA-4128: guard every installed-package read, and state why OTHER is…
fryanpan 2416064
ADFA-4128: drop blank classpath entries; deadline before the isRunnin…
fryanpan a7531a9
ADFA-4128: pin the pump drain with a daemon that replies and exits in…
fryanpan 6c1c38a
ADFA-4128: let the start, not the watcher's timing, decide whether an…
fryanpan 5cc9df5
ADFA-4128: carry a successful compile's warnings on CompileOutput
fryanpan 83a5156
ADFA-4128: read the daemon's own op duration into daemonMillis
fryanpan cfd470a
ADFA-4128: log the swallowed install-launch and stderr-drain exceptions
fryanpan c0486c2
ADFA-4128: keep a failed stamp read distinct from an absent package
fryanpan d83beb0
ADFA-4128: make pendingUserActionSeen an AtomicBoolean
fryanpan 592df01
ADFA-4128: expose quickbuild:protocol as an api dependency of core
fryanpan b7c50df
ADFA-4128: suspend the generation store and scratch tree on an inject…
fryanpan 6727ef9
ADFA-4128: name the layout walkers and the scratch residue a failed p…
fryanpan 041e22c
ADFA-4128: keep the daemon's configured state on its Spawn and kill a…
fryanpan f96facf
ADFA-4128: read only the generation counter's first line and stage ea…
fryanpan b053b0f
ADFA-4128: make the clobber check ask when PackageManager cannot answer
fryanpan 71d979e
ADFA-4128: round 5 doc and comment fixes on quickbuild/core (PR 7)
fryanpan 158d354
style: spotless reformat, no functional change
fryanpan 6613317
ADFA-4128: keep the cancel-mid-configure test inside ktlint's if-wrap…
fryanpan cfc7abf
style: spotless reformat, no functional change
fryanpan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
737 changes: 737 additions & 0 deletions
737
quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/data/DaemonProcessClient.kt
Large diffs are not rendered by default.
Oops, something went wrong.
102 changes: 102 additions & 0 deletions
102
quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/data/FileGenerationStore.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| package org.appdevforall.cotg.quickbuild.data | ||
|
|
||
| import kotlinx.coroutines.CoroutineDispatcher | ||
| import kotlinx.coroutines.Dispatchers | ||
| import kotlinx.coroutines.withContext | ||
| import org.appdevforall.cotg.quickbuild.domain.reload.GenerationStore | ||
| import org.slf4j.LoggerFactory | ||
| import java.io.File | ||
| import java.io.IOException | ||
|
|
||
| /** | ||
| * Keeps the generation counter in `<project>/.androidide/quickbuild/generation`. | ||
| * | ||
| * Lives with the project rather than in the app-private [QuickBuildScratch] tree because | ||
| * scratch is deleted on session teardown while this counter must outlive sessions: an | ||
| * installed proxy app keys its payloads by generation, so only a surviving counter lets a | ||
| * later session stay strictly newer. A corrupt or unreadable file loads as null (fresh | ||
| * session), so a broken state file cannot take quick build down. | ||
| * | ||
| * Every read and write runs under [ioDispatcher]: the file sits under the project root on | ||
| * FUSE-backed storage, and the callers are on the session thread that concurrency.md says | ||
| * must not block. | ||
| * | ||
| * @property file the counter file; it need not exist yet, its parent directory is created on | ||
| * first [save], and each save stages through its own uniquely named sibling `.tmp`. | ||
| * @property ioDispatcher where the file I/O runs; injectable so tests can pin the hop. | ||
| */ | ||
| class FileGenerationStore( | ||
| private val file: File, | ||
| private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO, | ||
| ) : GenerationStore { | ||
| /** | ||
| * Reads the persisted counter. | ||
| * | ||
| * @return the stored generation, or null when the file is missing, unreadable, or its | ||
| * first line does not parse as a Long - all of which the caller treats as a fresh | ||
| * session. Only the first line is read, so a torn or appended-to file still yields the | ||
| * counter it starts with rather than nothing. | ||
| */ | ||
| override suspend fun load(): Long? = | ||
| withContext(ioDispatcher) { | ||
| try { | ||
| if (file.isFile) file.useLines { it.firstOrNull() }?.trim()?.toLongOrNull() else null | ||
| } catch (e: IOException) { | ||
| log.warn("Failed to read generation from {}; starting fresh", file, e) | ||
| null | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Persists the counter atomically via temp file plus rename. | ||
| * | ||
| * @param generation the value to store; the caller guarantees it is strictly greater than | ||
| * any previously saved one, since the installed proxy app keys its payloads by it. | ||
| * @throws IOException when the value could not be persisted: the staged write failed | ||
| * before any rename was tried, or both renames AND the direct-write fallback failed. | ||
| * Unlike [load] this is never swallowed, since losing it would let a later session | ||
| * reuse a generation. | ||
| */ | ||
| override suspend fun save(generation: Long) = | ||
| withContext(ioDispatcher) { | ||
| file.parentFile?.mkdirs() | ||
| // A unique staging name per save: two stores on the same path (or two saves racing on | ||
| // one) would otherwise stage into a single file and rename each other's bytes. | ||
| val tmp = File.createTempFile(file.name + ".", ".tmp", file.parentFile) | ||
| tmp.writeText(generation.toString()) | ||
| if (!tmp.renameTo(file)) { | ||
| // Windows-style rename-over-existing failure path; harmless on device but | ||
| // keeps the store correct wherever the JVM tests run. | ||
| file.delete() | ||
| if (!tmp.renameTo(file)) { | ||
| // The old value is already deleted, so a bare throw here would leave NO | ||
| // counter at all - the next load() would restart the sequence, the exact | ||
| // reuse the class exists to rule out. Non-atomic beats lost. | ||
| try { | ||
| file.writeText(generation.toString()) | ||
| } catch (e: IOException) { | ||
| throw IOException("Unable to persist generation $generation to $file", e) | ||
| } finally { | ||
| tmp.delete() | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| companion object { | ||
| private val log = LoggerFactory.getLogger("QB-GenerationStore") | ||
|
|
||
| /** | ||
| * Builds a store at the canonical per-project location of the generation file. | ||
| * | ||
| * @param projectRoot the user project's root directory; the file lands at | ||
| * `.androidide/quickbuild/generation` beneath it, and neither need exist yet. | ||
| * @param ioDispatcher where the file I/O runs; see the class KDoc. | ||
| * @return a store for that path; no filesystem access happens until [load] or [save]. | ||
| */ | ||
| fun forProject( | ||
| projectRoot: File, | ||
| ioDispatcher: CoroutineDispatcher = Dispatchers.IO, | ||
| ): FileGenerationStore = FileGenerationStore(File(projectRoot, ".androidide/quickbuild/generation"), ioDispatcher) | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MINOR:
load()catchesIOExceptiononly, so a read that fails any other way does not give the documented "fresh session" - it fails the whole provision.readText()isreadBytes().toString(charset), andreadBytes()raisesOutOfMemoryError- anError, not anException- on an oversized file. This path is<project>/.androidide/quickbuild/generationunder the project root on shared storage, so its size is not app-controlled. The class KDoc promises "a broken state file cannot take quick build down"; what actually happens at the stack tip is that both live call sites (ProxyAppBuildRunner:186,GradleQuickBuildProvisioner:378) sit under an outercatch (Throwable)and report a failed provision instead. ASecurityExceptiontakes the same route.Unreachable while only CoGo writes the store. Bound the read -
useLines { it.firstOrNull() }- rather than widening the catch.The construct, verbatim at
12bd5b4so the anchor survives line drift:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Real, fixed in ac1cbed38:
load()reads the first line only (FileGenerationStore.kt:43). Testonly the first line is the counterfails without it withexpected: 42 but was : null.