diff --git a/AGENTS.md b/AGENTS.md index cd2efe0..66eb613 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -427,10 +427,46 @@ mutable at runtime from the phone. unaccepted, since taking commits runs git rather than the project's command, and it is how the new request arrives to be read. - **Which build variant to serve is the phone's choice, not the server's.** - It arrives as `?variant=` on the download and is validated against the + It arrives as `?variant=` on the download, beside the `?component=` that + says whose build it is, and is validated against *that component's* discovered builds -- an unvalidated path would let a phone name any file on disk to be served. Storing it server-side meant one enrolled device - silently changing what another was offered. + silently changing what another was offered. The stored choice is keyed + by project *and* component on the device, so pinning one client to a + release build says nothing about the other. + +- **A project can produce more than one APK, and each component's builds + are found under its own `cwd`.** `APK_PATTERNS` is anchored at + `project.join(cwd)` rather than at the project root, which is what + `cwd` already meant everywhere else -- the directory the build command + runs in, the subtree `subtree_head` scopes staleness to, a server's + `WorkingDirectory`. A component that declares none sits at the root, + which is what every single-APK project has always meant, so nothing + about that case changed. tdep-survey is the project that needed it: one + checkout, a backend and *two* independent Android clients, where the + root-anchored patterns reached the first and stopped. + The alternative -- naming the file on the component, `apk: "path"` -- + was rejected because it makes a component a file path, and **an app + being a project path rather than a file path** is the invariant at the + top of this document. + Everything downstream is per component in consequence: `package`, + `strip`, the variant list, the size, the mtime and the rename note. + Those were all `apk_component()`'s *first* match before, which was + correct only while a project had one. Two clients install over + different packages and differ in whether their symbols are worth + carrying to a phone, so first-wins would have checked the installed + state of one app and reported it as the other's -- the expensive kind + of wrong, because it looks exactly like an answer. + **A download that names no component is refused, not guessed** + (`ApiError::AmbiguousApk`), for that reason. Naming none still answers + for a project with one, which is nearly all of them and is what lets + the frozen `/self/apk` keep working -- it cannot carry a component, and + the project it describes has a single APK. + The measurement carried across an acceptance is keyed by name *and* + `cwd` (`registry::component_id`): once the directory decides which + builds a component has, a reused name is not the same APK, and handing + it the old one's package would be wrong until that component happened + to be downloaded. - **The self entry's project is the working directory itself**, so this server has to be started from the root of its own checkout -- which is where everything else here is driven from, and what the service unit diff --git a/app/androidApp/src/main/kotlin/com/example/devupdater/ApkInstaller.kt b/app/androidApp/src/main/kotlin/com/example/devupdater/ApkInstaller.kt index 5ae3b7c..710fc61 100644 --- a/app/androidApp/src/main/kotlin/com/example/devupdater/ApkInstaller.kt +++ b/app/androidApp/src/main/kotlin/com/example/devupdater/ApkInstaller.kt @@ -23,21 +23,30 @@ private const val DOWNLOAD_READ_TIMEOUT_MS = 15000 fun downloadApk( context: Context, entry: ManifestEntry, + component: String, onProgress: (bytesRead: Long, total: Long) -> Unit, ): File { - // The chosen build travels with the request rather than being + // Which component's APK, always said rather than left to the server: + // a project can build two clients, and the server refuses to guess + // between them rather than serving the first, which would install the + // wrong app while looking like it worked. + // + // The chosen build travels with the request too rather than being // stored on the server: it is this device's preference, and a // second phone must not have its download changed by it. The - // server checks the path against the builds it can see, so a stale - // one falls back to the newest rather than naming a file. - val route = - when (val variant = chosenVariant(context, entry.key)) { - null -> entry.route - else -> "${entry.route}?variant=${URLEncoder.encode(variant, "UTF-8")}" - } - return downloadFromRoute(context, route, entry.key, onProgress) + // server checks the path against that component's builds, so a stale + // one falls back to its newest rather than naming a file. + val query = StringBuilder("?component=").append(encode(component)) + chosenVariant(context, entry.key, component)?.let { + query.append("&variant=").append(encode(it)) + } + // Named for the component as well, so two clients of one project do + // not overwrite each other's download on the way to the installer. + return downloadFromRoute(context, entry.route + query, "${entry.key}-$component", onProgress) } +private fun encode(value: String): String = URLEncoder.encode(value, "UTF-8") + /** * Fetches one APK from [route] into private storage, named [name], and returns the file. * diff --git a/app/androidApp/src/main/kotlin/com/example/devupdater/UpdateManifest.kt b/app/androidApp/src/main/kotlin/com/example/devupdater/UpdateManifest.kt index a4a2495..66adeb0 100644 --- a/app/androidApp/src/main/kotlin/com/example/devupdater/UpdateManifest.kt +++ b/app/androidApp/src/main/kotlin/com/example/devupdater/UpdateManifest.kt @@ -9,26 +9,14 @@ data class ManifestEntry( // is what lets the app list be edited at runtime from the Add screen // with no rebuild here. val label: String, - val filename: String, + // Where this project's APKs come from. Which of them is said with the + // request, so one route covers a project that builds two clients -- + // see [ProjectComponent.apk]. val route: String, - // Null until this project has been built at least once -- there is no - // APK to read an identity out of before then, and inventing one would - // make the installed-version check compare against nothing. - val packageName: String?, - // What this project's APK used to install over, when it has been - // renamed. Android treats a renamed applicationId as an unrelated app, - // so that one is still installed and nothing will ever replace it -- - // the card offers to remove it, but only while it is actually there. - val previousPackageName: String?, // The project directory this app was added by. Shown on the card so // it's possible to tell two similarly-named apps apart, and to spot an // entry pointing somewhere unexpected. val projectPath: String, - // Epoch seconds of the raw build's mtime, straight from the server -- - // these are ad hoc local rebuilds with no CI bumping a version, so - // build freshness is the only meaningful signal, not a version code. - val mtime: Double, - val size: Long, // True for an entry with an on-demand build step (see BuildStatus.kt) // -- only then does this app call that entry's prepare/status routes, // which 404 for an entry without one. @@ -39,13 +27,10 @@ data class ManifestEntry( // Force git's remote commands onto IPv4 for this project -- this // machine's choice, editable from the card's settings. val gitIpv4: Boolean, - // False when the project has no APK yet (never built, or cleaned). - // Such an entry is still listed rather than silently dropped -- it was - // added deliberately, so saying so beats it disappearing. + // Whether anything this project produces has been built. Per component + // is on the component; this is what the card's own "nothing here yet" + // line reads. val built: Boolean, - // Every build discovered under the project, so a different one can be - // selected without another round trip. - val variants: List, // What this project produces, in build order. One is the ordinary case // and the card stays flat; more than one is drawn as a nested list, so // a project that also runs a server says so without every single-app @@ -144,6 +129,9 @@ data class ProjectComponent( // The last is the ordinary case and not a fault. val resourcesChecking: Boolean, val resourcesError: String?, + // What there is to install, for a component that produces one. Null + // for a server, which builds nothing this phone installs. + val apk: ComponentApk?, ) { // Only "behind" is worth saying. "Current" is what a card already // implies, and "unknown" said out loud would be on most rows most of @@ -167,6 +155,39 @@ data class ProjectComponent( get() = state == "failed" } +// The installable half of a component. +// +// Per component rather than per project, because a project can build two +// clients: they install over different packages, are different sizes, and +// have their own builds to choose between. Read off the project, the +// answer would be the first component's wearing the project's name -- and +// nothing on screen would say so. +data class ComponentApk( + // What the download is saved as on this device. + val filename: String, + // Null until this component has been built at least once -- there is + // no APK to read an identity out of before then, and inventing one + // would make the installed-version check compare against nothing. + val packageName: String?, + // What this component used to install over, when it has been renamed. + // Android treats a renamed applicationId as an unrelated app, so that + // one is still installed and nothing will ever replace it -- the card + // offers to remove it, but only while it is actually there. + val previousPackageName: String?, + // False when this component has no APK yet (never built, or cleaned). + // Its row is still drawn rather than silently dropped -- it was + // declared deliberately, so saying so beats it disappearing. + val built: Boolean, + // Epoch seconds of the raw build's mtime, straight from the server -- + // these are ad hoc local rebuilds with no CI bumping a version, so + // build freshness is the only meaningful signal, not a version code. + val mtime: Double, + val size: Long, + // Every build discovered under this component, so a different one can + // be selected without another round trip. + val variants: List, +) + // One discovered build of an app. `variant` is the Gradle-style build // variant name ("debug", "freeRelease") taken from the output directory. data class ApkVariant( @@ -191,14 +212,14 @@ data class Manifest( // In the units PackageInfo.lastUpdateTime reports, which is what this is // ever compared against (see InstalledBuilds.kt). -fun ManifestEntry.mtimeMillis(): Long = (mtime * 1000).toLong() +fun ComponentApk.mtimeMillis(): Long = (mtime * 1000).toLong() // When this device has pinned a build, that build's timestamp is the one // freshness is about -- the newest build being newer than the installed // copy says nothing when the newest is not what would be installed. Falls -// back to the entry's own when the pinned one is gone, which is the same -// build the server would fall back to serving. -fun ManifestEntry.mtimeMillisFor(chosenVariantPath: String?): Long = +// back to this component's own when the pinned one is gone, which is the +// same build the server would fall back to serving. +fun ComponentApk.mtimeMillisFor(chosenVariantPath: String?): Long = variants.firstOrNull { it.path == chosenVariantPath }?.let { (it.mtime * 1000).toLong() } ?: mtimeMillis() @@ -290,17 +311,11 @@ fun fetchApp(key: String): ManifestEntry = /** One app as the server describes it, shared by both reads above. */ private fun readEntry(entry: JSONObject): ManifestEntry { val components = entry.optJSONArray("components") - val variants = entry.getJSONArray("variants") return ManifestEntry( key = entry.getString("key"), label = entry.getString("label"), - filename = entry.getString("filename"), route = entry.getString("route"), - packageName = entry.optString("package").ifEmpty { null }, - previousPackageName = entry.optString("previousPackage").ifEmpty { null }, projectPath = entry.getString("projectPath"), - mtime = entry.getDouble("mtime"), - size = entry.getLong("size"), needsBuild = entry.getBoolean("needsBuild"), builtIn = entry.getBoolean("builtIn"), gitIpv4 = entry.getBoolean("gitIpv4"), @@ -339,8 +354,22 @@ private fun readEntry(entry: JSONObject): ManifestEntry { configPresent = component.optBoolean("configPresent", false), resourcesChecking = component.optBoolean("resourcesChecking", false), resourcesError = component.optString("resourcesError").ifEmpty { null }, + apk = component.optJSONObject("apk")?.let(::readApk), ) }, + ) +} + +/** The installable half of one component, absent for a server. */ +private fun readApk(apk: JSONObject): ComponentApk { + val variants = apk.getJSONArray("variants") + return ComponentApk( + filename = apk.getString("filename"), + packageName = apk.optString("package").ifEmpty { null }, + previousPackageName = apk.optString("previousPackage").ifEmpty { null }, + built = apk.getBoolean("built"), + mtime = apk.getDouble("mtime"), + size = apk.getLong("size"), variants = (0 until variants.length()).map { j -> val variant = variants.getJSONObject(j) diff --git a/app/androidApp/src/main/kotlin/com/example/devupdater/UpdaterScreen.kt b/app/androidApp/src/main/kotlin/com/example/devupdater/UpdaterScreen.kt index e9da428..0cf933b 100644 --- a/app/androidApp/src/main/kotlin/com/example/devupdater/UpdaterScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/devupdater/UpdaterScreen.kt @@ -422,12 +422,17 @@ private fun AppListScreen( // two effects that follow -- so a fresh manifest, a package-change // broadcast, and a return from the system installer all go through the // same one path rather than each refreshing these their own way. - var installedTimes by remember { mutableStateOf>(emptyMap()) } - var installedSizes by remember { mutableStateOf>(emptyMap()) } - // Which build each project is pinned to on *this* device, read from + // + // Two levels: a project, then a component of it. A project can build + // two clients, and they install over different packages -- one map + // keyed by project alone would answer for whichever was asked about + // last, on both rows. + var installedTimes by remember { mutableStateOf>>(emptyMap()) } + var installedSizes by remember { mutableStateOf>>(emptyMap()) } + // Which build each component is pinned to on *this* device, read from // local storage rather than the manifest. Held as state so picking one // redraws the card without a round trip. - var chosenVariants by remember { mutableStateOf>(emptyMap()) } + var chosenVariants by remember { mutableStateOf>>(emptyMap()) } // The project whose pull came back saying its checkout and its remote // share no history, waiting on an answer about throwing that history // away. One slot rather than one per card: it is a modal, so only one @@ -710,7 +715,7 @@ private fun AppListScreen( } } - fun startUpdate(entry: ManifestEntry) { + fun startUpdate(entry: ManifestEntry, component: String) { scope.launch { if (entry.needsBuild) { cardStates = cardStates + (entry.key to CardState.Preparing(null)) @@ -741,7 +746,7 @@ private fun AppListScreen( val file = try { withContext(Dispatchers.IO) { - downloadApk(context, entry) { read, total -> + downloadApk(context, entry, component) { read, total -> val progress = if (total > 0) read.toFloat() / total else null cardStates = cardStates + (entry.key to CardState.Downloading(progress)) } @@ -814,19 +819,31 @@ private fun AppListScreen( } fun updateInstalledState(entries: List) { - // A project with no build yet has no package, so there is nothing - // installed to ask about -- which is exactly what null already - // means to the callers of these two maps. - installedTimes = entries.associate { entry -> - entry.key to entry.packageName?.let { installedLastUpdateTimeMillis(context, it) } + // A component with no build yet has no package, so there is + // nothing installed to ask about -- which is exactly what null + // already means to the callers of these two maps. + fun byComponent(read: (String) -> T?): Map> = + entries.associate { entry -> + entry.key to + entry.components + .filter { !it.isServer } + .associate { component -> + component.name to component.apk?.packageName?.let(read) + } + } + installedTimes = byComponent { installedLastUpdateTimeMillis(context, it) } + installedSizes = byComponent { installedApkSizeBytes(context, it) } + chosenVariants = entries.associate { entry -> + entry.key to + entry.components + .filter { !it.isServer } + .mapNotNull { component -> + chosenVariant(context, entry.key, component.name)?.let { + component.name to it + } + } + .toMap() } - installedSizes = entries.associate { entry -> - entry.key to entry.packageName?.let { installedApkSizeBytes(context, it) } - } - chosenVariants = - entries - .mapNotNull { entry -> chosenVariant(context, entry.key)?.let { entry.key to it } } - .toMap() } LaunchedEffect(Unit) { refresh() } @@ -902,7 +919,11 @@ private fun AppListScreen( val receiver = registerPackageChangeReceiver(context) { packageName -> currentEntries.value - ?.takeIf { entries -> entries.any { it.packageName == packageName } } + ?.takeIf { entries -> + entries.any { entry -> + entry.components.any { it.apk?.packageName == packageName } + } + } ?.let(::updateInstalledState) } onDispose { context.unregisterReceiver(receiver) } @@ -1002,11 +1023,19 @@ private fun AppListScreen( cardStates[entry.key].isBuilding() || !entry.built || entry.newCommits || - !isUpToDate( - entry, - installedTimes[entry.key], - chosenVariants[entry.key], - ) + // Any client of the project being + // behind is the project being + // behind: a card with one of two + // apps waiting has something + // waiting. + entry.components.any { component -> + val apk = component.apk ?: return@any false + !isUpToDate( + apk, + installedTimes[entry.key]?.get(component.name), + chosenVariants[entry.key]?.get(component.name), + ) + } } // The two groups render identically; only the up-to-date one @@ -1022,11 +1051,13 @@ private fun AppListScreen( group.forEach { entry -> AppCard( entry = entry, - installedLastUpdateTimeMillis = installedTimes[entry.key], - chosenVariantPath = chosenVariants[entry.key], - installedSizeBytes = installedSizes[entry.key], + installedTimes = installedTimes[entry.key] ?: emptyMap(), + chosenVariants = chosenVariants[entry.key] ?: emptyMap(), + installedSizes = installedSizes[entry.key] ?: emptyMap(), cardState = cardStates[entry.key], - onUpdate = { startUpdate(it) }, + onUpdate = { updated, component -> + startUpdate(updated, component) + }, onPull = { startPull(entry) }, onRebuild = { startRebuild(entry) }, onRefresh = { refreshOne(entry) }, @@ -1037,7 +1068,7 @@ private fun AppListScreen( manage(entry) { approveDeclaration(entry.key) } }, onRemove = { - forgetVariant(context, entry.key) + forgetVariants(context, entry.key) manage(entry, removes = true) { removeApp(entry.key) } }, // Written here rather than sent to the server: @@ -1045,14 +1076,23 @@ private fun AppListScreen( // reload token is what redraws the card with // the new choice and the mtime that goes with // it. - onSelectVariant = { variant -> - chooseVariant(context, entry.key, variant?.path) + onSelectVariant = { component, variant -> + chooseVariant( + context, + entry.key, + component, + variant?.path, + ) + val forProject = chosenVariants[entry.key] ?: emptyMap() chosenVariants = - when (variant) { - null -> chosenVariants - entry.key - else -> - chosenVariants + (entry.key to variant.path) - } + chosenVariants + + (entry.key to + when (variant) { + null -> forProject - component + else -> + forProject + + (component to variant.path) + }) }, serviceBusy = serviceBusy[entry.key], onServiceAction = { component, action, purge -> @@ -1151,25 +1191,25 @@ private fun ForcePullDialog(entry: ManifestEntry, onDismiss: () -> Unit, onForce @Composable private fun AppCard( entry: ManifestEntry, - installedLastUpdateTimeMillis: Long?, - installedSizeBytes: Long?, + /** By component name, for the project's own APKs. */ + installedTimes: Map, + installedSizes: Map, + chosenVariants: Map, cardState: CardState?, - onUpdate: (ManifestEntry) -> Unit, + onUpdate: (ManifestEntry, component: String) -> Unit, onPull: () -> Unit, onRebuild: () -> Unit, onRefresh: () -> Unit, onSettings: (gitIpv4: Boolean) -> Unit, onApprove: () -> Unit, onRemove: () -> Unit, - chosenVariantPath: String?, - onSelectVariant: (ApkVariant?) -> Unit, + onSelectVariant: (component: String, ApkVariant?) -> Unit, // Which component this card is running a service action for, if any -- // so the one being acted on is the one that shows it, rather than // every row going quiet together. serviceBusy: String?, onServiceAction: (component: String, action: String, purge: Purge) -> Unit, ) { - val upToDate = isUpToDate(entry, installedLastUpdateTimeMillis, chosenVariantPath) var settingsOpen by remember { mutableStateOf(false) } // Until the build step this project asks for has been accepted, the // card is about that request and nothing else: no size, no components, @@ -1178,18 +1218,6 @@ private fun AppCard( // decline by getting rid of the card. val awaitingApproval = entry.pendingDeclaration != null - // What the download would cost, and what it replaces. Belongs to the - // APK the same way the Update button does, and travels with it. - val sizeText = - if (awaitingApproval) null - else - when { - !entry.built -> null - !upToDate && installedSizeBytes != null -> - "${formatSize(installedSizeBytes)} \u2192 ${formatSize(entry.size)}" - else -> formatSize(entry.size) - } - Card(Modifier.fillMaxWidth()) { Column(Modifier.padding(16.dp)) { // The two corner controls belong to the card, not to its title, @@ -1207,7 +1235,18 @@ private fun AppCard( // edge with nothing in the way. Box(Modifier.fillMaxWidth()) { Row(verticalAlignment = Alignment.Top, modifier = Modifier.fillMaxWidth()) { - AppIcon(entry.packageName, Modifier.align(Alignment.CenterVertically)) + // The project's icon is its client's, and only when + // it has exactly one: a project building two of them + // has no single answer, and showing the first would + // label the whole card with one of its two apps. + // AppIcon draws its own placeholder for null, which is + // what a project with two gets -- the same mark a + // project with nothing built yet gets, because in both + // cases there is no one icon to show. + AppIcon( + entry.components.mapNotNull { it.apk?.packageName }.singleOrNull(), + Modifier.align(Alignment.CenterVertically), + ) Spacer(Modifier.width(10.dp)) Column(Modifier.weight(1f)) { // The one line that makes room for them: they are beside @@ -1335,14 +1374,34 @@ private fun AppCard( entry.components .sortedBy { it.isServer } .forEach { component -> + val installed = installedTimes[component.name] + val installedSize = installedSizes[component.name] + val chosenVariantPath = chosenVariants[component.name] + val upToDate = + component.apk?.let { + isUpToDate(it, installed, chosenVariantPath) + } == true ComponentCard( entryKey = entry.key, component = component, - packageName = entry.packageName, - // An APK's size sits where a server's state - // does: the one thing worth knowing about it - // besides its name. - sizeText = sizeText.takeIf { !component.isServer }, + packageName = component.apk?.packageName, + // What the download would cost, and what it + // replaces. An APK's size sits where a + // server's state does: the one thing worth + // knowing about it besides its name. + sizeText = + component.apk + ?.takeIf { it.built } + ?.let { apk -> + when { + !upToDate && installedSize != null -> + "${formatSize(installedSize)} \u2192 " + + formatSize(apk.size) + else -> formatSize(apk.size) + } + }, + chosenVariantPath = chosenVariantPath, + onSelectVariant = { onSelectVariant(component.name, it) }, // This app reaches the server through this server. // Stopping or uninstalling it is the one action // here that cannot be undone from the phone. @@ -1370,12 +1429,12 @@ private fun AppCard( // how far along it is: a bar reports on // the control above it. UpdateButton( - built = entry.built, + built = component.apk?.built == true, needsBuild = entry.needsBuild, - installed = installedLastUpdateTimeMillis != null, + installed = installed != null, upToDate = upToDate, cardState = cardState, - onUpdate = { onUpdate(entry) }, + onUpdate = { onUpdate(entry, component.name) }, onPull = onPull, ) ApkProgress(cardState) @@ -1483,26 +1542,6 @@ private fun AppCard( BuildProgress("Building", cardState.status) } - // Only while the old app is actually still there. The server - // remembers the name it was renamed from; whether anything - // answers to it is this device's question, and asking it here - // is what saves the server needing to be told when it stops - // being true. - val cardContext = LocalContext.current - val orphan = entry.previousPackageName?.takeIf { isInstalled(cardContext, it) } - if (orphan != null) { - Spacer(Modifier.height(4.dp)) - Text( - "Renamed from $orphan. Android treats that as a different " + - "app, so it is still installed and nothing will replace it.", - style = MaterialTheme.typography.bodySmall, - color = MaterialTheme.colorScheme.onSurfaceVariant, - ) - TextButton(onClick = { cardContext.startActivity(uninstallIntent(orphan)) }) { - Text("Remove the old app") - } - } - // What is left to say about the card once its components have // said their own part: a failure, or that there is no build to // talk about yet. Progress is not here -- it belongs beside the @@ -1545,23 +1584,6 @@ private fun AppCard( ) } } - - // Only worth a row when there is actually a choice, which is - // rare -- the usual case is a single debug build, and an empty - // row here was leaving a band of space at the foot of every - // card for a control almost none of them have. - // - // And not while a declaration is waiting either: this picks - // which build gets installed, on a card that is offering no - // way to install one. - if (entry.variants.size > 1 && !awaitingApproval) { - Row( - horizontalArrangement = Arrangement.End, - modifier = Modifier.fillMaxWidth(), - ) { - VariantPicker(entry.variants, chosenVariantPath, onSelectVariant) - } - } } } @@ -1918,6 +1940,9 @@ private fun ComponentCard( /** The package an APK component installs, for its icon. */ packageName: String?, sizeText: String?, + /** Which of this component's builds this device is pinned to, if any. */ + chosenVariantPath: String? = null, + onSelectVariant: (ApkVariant?) -> Unit = {}, isOwnServer: Boolean, /** This component's part of a build in progress, if it has one. */ build: ComponentBuild?, @@ -2148,6 +2173,49 @@ private fun ComponentCard( Spacer(Modifier.height(6.dp)) ComponentBuildProgress(it) } + + // Only while the old app is actually still there. The build + // machine remembers the name this component was renamed from; + // whether anything answers to it is this device's question, + // and asking it here is what saves the server needing to be + // told when it stops being true. + // + // In this component's card rather than the project's: with two + // clients, only one of them was renamed, and the offer has to + // sit with the one it is about. + val cardContext = LocalContext.current + val orphan = component.apk?.previousPackageName?.takeIf { isInstalled(cardContext, it) } + if (orphan != null) { + Spacer(Modifier.height(4.dp)) + Text( + "Renamed from $orphan. Android treats that as a different " + + "app, so it is still installed and nothing will replace it.", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + TextButton(onClick = { cardContext.startActivity(uninstallIntent(orphan)) }) { + Text("Remove the old app") + } + } + + // Only worth a row when there is actually a choice, which is + // rare -- the usual case is a single debug build, and an empty + // row here was leaving a band of space at the foot of every + // card for a control almost none of them have. + // + // Beside the build it picks, which is what makes it answerable + // for a project with two clients: the choice is this + // component's, and a picker at the foot of the card could only + // have been the project's. + val variants = component.apk?.variants.orEmpty() + if (variants.size > 1) { + Row( + horizontalArrangement = Arrangement.End, + modifier = Modifier.fillMaxWidth(), + ) { + VariantPicker(variants, chosenVariantPath, onSelectVariant) + } + } } } @@ -2515,12 +2583,12 @@ private fun CardState?.isBuilding(): Boolean = this is CardState.Pulling || this is CardState.Preparing || this is CardState.Rebuilding private fun isUpToDate( - entry: ManifestEntry, + apk: ComponentApk, installedLastUpdateTimeMillis: Long?, chosenVariantPath: String?, ): Boolean = installedLastUpdateTimeMillis != null && - installedLastUpdateTimeMillis >= entry.mtimeMillisFor(chosenVariantPath) + installedLastUpdateTimeMillis >= apk.mtimeMillisFor(chosenVariantPath) internal fun formatSize(bytes: Long): String { val mb = bytes / 1024.0 / 1024.0 diff --git a/app/androidApp/src/main/kotlin/com/example/devupdater/VariantChoice.kt b/app/androidApp/src/main/kotlin/com/example/devupdater/VariantChoice.kt index 43b9dee..daa7e9b 100644 --- a/app/androidApp/src/main/kotlin/com/example/devupdater/VariantChoice.kt +++ b/app/androidApp/src/main/kotlin/com/example/devupdater/VariantChoice.kt @@ -11,8 +11,14 @@ import android.content.Context * the choice lives here and travels with the download request, rather than * being written into the server's config. * - * Keyed by the project key, which the server promises never to change: it - * is the same identifier the downloaded file is named after. + * Keyed by the project *and the component*, because a project can build + * two clients: pinning one of them to a release build has nothing to say + * about the other, and a key with only the project in it would have made + * the second component inherit the first's choice. + * + * The project key is the identifier the server promises never to change, + * and a component's name is fixed by the declaration this machine + * accepted, so a stored choice keeps meaning what it meant. * * The path is the server's, not this device's, and is checked there against * the builds it can actually see. Nothing here can name a file into @@ -22,22 +28,38 @@ import android.content.Context private const val PREFS_NAME = "variants" -/** The build [key] is pinned to on this device, or null for "the newest". */ -fun chosenVariant(context: Context, key: String): String? = - context.getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE).getString(key, null) +/** + * One stored preference's name. A component's name cannot contain a slash -- it is a RON identifier + * -- so nothing else can collide with a project key that contains one. + */ +private fun slot(key: String, component: String) = "$key/$component" + +/** The build one component is pinned to on this device, or null for "the newest". */ +fun chosenVariant(context: Context, key: String, component: String): String? = + context + .getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE) + .getString(slot(key, component), null) /** Passing null goes back to "whatever is newest", which is the default. */ -fun chooseVariant(context: Context, key: String, path: String?) { +fun chooseVariant(context: Context, key: String, component: String, path: String?) { val prefs = context.getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE) if (path == null) { - prefs.edit().remove(key).apply() + prefs.edit().remove(slot(key, component)).apply() } else { - prefs.edit().putString(key, path).apply() + prefs.edit().putString(slot(key, component), path).apply() } } /** - * Forgets a project's choice, for one being removed -- otherwise a key reused by a later project - * would inherit a preference nobody set. + * Forgets every choice made for a project being removed -- otherwise a key reused by a later + * project would inherit preferences nobody set. + * + * Every component at once, because the card that is going away is the only thing that knew which + * components it had. */ -fun forgetVariant(context: Context, key: String) = chooseVariant(context, key, null) +fun forgetVariants(context: Context, key: String) { + val prefs = context.getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE) + val editor = prefs.edit() + prefs.all.keys.filter { it == key || it.startsWith("$key/") }.forEach(editor::remove) + editor.apply() +} diff --git a/server/src/config.rs b/server/src/config.rs index 48f0ed7..3a4d371 100644 --- a/server/src/config.rs +++ b/server/src/config.rs @@ -310,6 +310,13 @@ impl Component { } } + /// Whether to serve a stripped copy of this component's build. Only + /// an `Apk` has anything to strip, so a `Server` is always false + /// rather than this being an option only one variant carries. + pub fn strip(&self) -> bool { + matches!(self, Self::Apk { strip: true, .. }) + } + pub fn cwd(&self) -> Option<&Path> { match self { Self::Apk { cwd, .. } | Self::Server { cwd, .. } => cwd.as_deref(), diff --git a/server/src/main.rs b/server/src/main.rs index 00f5630..4e4a7ce 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -333,10 +333,15 @@ async fn main() -> Result<()> { // out from a bare 404 in a phone browser -- which is where this // flag is used and where there is least to go on. let self_entry = state.entry(registry::SELF_KEY); - match self_entry - .as_ref() - .and_then(|entry| entry.resolve_apk(None)) - { + // Named as no component, because this listener is half of the + // frozen rescue contract and cannot say one -- and this server's + // own project produces a single APK, which is what makes that + // safe. Two would answer `None` here rather than pick. + match self_entry.as_ref().and_then(|entry| { + entry + .apk_component(None) + .and_then(|component| entry.resolve_apk(component, None)) + }) { // The age is here because this listener serves the file already // on disk and builds nothing, so an old APK installs in silence. // That one is unusually expensive to land on: a fresh install is @@ -386,14 +391,25 @@ async fn main() -> Result<()> { tracing::info!("scanning for projects under {}", root.display()); } } + // One line per APK, not per project: a project producing two clients + // has two answers here, and folding them into one would hide exactly + // the case where a component's builds are not where somebody expected. for entry in state.entries() { - match entry.resolve_apk(None) { - Some(apk) => tracing::info!(" {} -> {}", entry.key, apk.path.display()), - None => tracing::warn!( - " {} -> no build found under {} (it will show as not built)", - entry.key, - entry.project_path.display(), - ), + for component in entry.apk_components() { + match entry.resolve_apk(component, None) { + Some(apk) => tracing::info!( + " {}/{} -> {}", + entry.key, + component.name(), + apk.path.display() + ), + None => tracing::warn!( + " {}/{} -> no build found under {} (it will show as not built)", + entry.key, + component.name(), + entry.component_dir(component).display(), + ), + } } } diff --git a/server/src/registry.rs b/server/src/registry.rs index bdb370b..6d37bc7 100644 --- a/server/src/registry.rs +++ b/server/src/registry.rs @@ -61,30 +61,60 @@ pub struct AppEntry { } impl AppEntry { - /// The APK component this entry serves, if it has one. - /// - /// One for now, and the first wins if a project ever declares two: the - /// download route serves a project, and which of two APKs it meant - /// would need saying. That is a question for whoever adds the second. - fn apk_component(&self) -> Option<&Component> { + /// Every APK this project produces, in declaration order. + pub fn apk_components(&self) -> impl Iterator { self.components .iter() - .find(|component| matches!(component, Component::Apk { .. })) + .filter(|component| matches!(component, Component::Apk { .. })) } - /// The package this project's APK installs over, once a build has been - /// read. `None` until there has been one -- a project can be added - /// before it has ever been built. - pub fn package(&self) -> Option<&str> { - self.apk_component().and_then(Component::package) + /// The APK component a request means: the one it names, or the only + /// one when it names none. + /// + /// Deliberately not first-wins for a project with two. Two APKs differ + /// in the package they install over and in whether their symbols are + /// worth carrying to a phone, so picking one for a request that didn't + /// say would answer a question nobody asked -- and it would look + /// exactly like a correct answer, which is the expensive kind of + /// wrong. `None` is the caller's cue to say which. + /// + /// Naming none stays right for the projects that produce one, which is + /// nearly all of them, and for the frozen `/self` contract: it cannot + /// carry a component name, and the project it describes has a single + /// APK. + pub fn apk_component(&self, named: Option<&str>) -> Option<&Component> { + match named { + Some(name) => self + .apk_components() + .find(|component| component.name() == name), + None => { + let mut components = self.apk_components(); + let only = components.next()?; + components.next().is_none().then_some(only) + } + } } - /// Whether to serve a stripped copy. Declared, not detected. - pub fn strip(&self) -> bool { - matches!( - self.apk_component(), - Some(Component::Apk { strip: true, .. }) - ) + /// Where one component's builds are. + /// + /// Its own directory, which is what `cwd` already means everywhere + /// else: the directory its build command runs in, the subtree its + /// staleness and its recorded commit are scoped to, and a server's + /// working directory. A component that doesn't say one sits at the + /// project root, which is what a project with a single APK has always + /// meant -- so nothing about the one-APK case changes. + /// + /// This is what lets one project produce two APKs: the patterns are + /// anchored per component rather than at the root, so the second + /// client's build is reachable and each component's builds are its + /// own. + pub fn component_dir(&self, component: &Component) -> PathBuf { + match component.cwd() { + // `join` on an absolute path yields that path, as everywhere + // else a cwd is resolved. + Some(cwd) => self.project_path.join(cwd), + None => self.project_path.clone(), + } } /// The APK to serve: `requested` if it is still one of this project's @@ -101,8 +131,12 @@ impl AppEntry { /// rather than failing -- a `./gradlew clean` shouldn't take an app out /// of the list, and the fallback is the answer this would have given /// before any variant was chosen. - pub fn resolve_apk(&self, requested: Option<&Path>) -> Option { - let variants = self.variants(); + pub fn resolve_apk( + &self, + component: &Component, + requested: Option<&Path>, + ) -> Option { + let variants = self.variants(component); if let Some(requested) = requested && let Some(found) = variants .iter() @@ -115,8 +149,8 @@ impl AppEntry { /// Every build found under this project, newest first -- what the app /// offers when letting the user switch variants. - pub fn variants(&self) -> Vec { - discover::find_apks(&self.project_path) + pub fn variants(&self, component: &Component) -> Vec { + discover::find_apks(&self.component_dir(component)) } /// The build step this project is asking for that nobody has accepted @@ -223,25 +257,37 @@ impl AppEntry { } } -/// The packages already read for each component, by name -- what an -/// acceptance has to carry across. -fn measured_packages(components: &[Component]) -> HashMap { +/// The packages already read for each component -- what an acceptance has +/// to carry across. +/// +/// Keyed by name *and* directory, because a name alone stopped +/// identifying an APK once each component's builds came from its own +/// `cwd`: a declared component that happens to reuse a name while +/// pointing somewhere else is a different app, and handing it the package +/// read from the old one would make the card check the installed state of +/// something else -- silently, and looking like an answer, until that +/// component was downloaded once and re-read. +type ComponentId = (String, Option); + +fn component_id(component: &Component) -> ComponentId { + ( + component.name().to_string(), + component.cwd().map(Path::to_path_buf), + ) +} + +fn measured_packages(components: &[Component]) -> HashMap { components .iter() - .filter_map(|component| { - Some(( - component.name().to_string(), - component.package()?.to_string(), - )) - }) + .filter_map(|component| Some((component_id(component), component.package()?.to_string()))) .collect() } -/// Records a freshly read package on whichever component is the APK. -fn set_measured_package(components: &mut [Component], package: String) { +/// Records a freshly read package on the APK component that produced it. +fn set_measured_package(components: &mut [Component], name: &str, package: String) { if let Some(component) = components .iter_mut() - .find(|component| matches!(component, Component::Apk { .. })) + .find(|component| matches!(component, Component::Apk { .. }) && component.name() == name) { component.set_package(package); } @@ -291,7 +337,10 @@ pub struct AppState { /// It needs no clearing protocol: the phone knows what is installed, /// so it only shows the offer while the old package is actually there. /// Dropped when the project is, so this can't outlive it. - previous_packages: Mutex>, + /// Keyed by project *and* component: a project with two clients can + /// rename either of them, and one card would otherwise offer to + /// remove the other's old package. + previous_packages: Mutex>, config_path: PathBuf, registry: RwLock, } @@ -336,10 +385,14 @@ impl AppState { .cloned() } - /// What this project's APK used to install over, if it has been + /// What one component's APK used to install over, if it has been /// renamed since this server started. See [`Self::previous_packages`]. - pub fn previous_package(&self, key: &str) -> Option { - self.previous_packages.lock().unwrap().get(key).cloned() + pub fn previous_package(&self, key: &str, component: &str) -> Option { + self.previous_packages + .lock() + .unwrap() + .get(&(key.to_string(), component.to_string())) + .cloned() } /// Re-reads what `apk` installs over and records it if it has changed. @@ -350,9 +403,10 @@ impl AppState { /// bytes are asked for. Runs off the request so the download is not /// held up by a process spawn -- the answer is wanted by the *next* /// manifest, not this one. - pub fn refresh_package(self: &Arc, key: &str, apk: PathBuf) { + pub fn refresh_package(self: &Arc, key: &str, component: &str, apk: PathBuf) { let state = Arc::clone(self); let key = key.to_string(); + let component = component.to_string(); tokio::task::spawn_blocking(move || { let Ok(info) = crate::apkinfo::read(&apk) else { return; @@ -360,12 +414,16 @@ impl AppState { let Some(entry) = state.entry(&key) else { return; }; - if entry.package() == Some(info.package.as_str()) { + let known = entry + .apk_component(Some(&component)) + .and_then(Component::package) + .map(str::to_string); + if known.as_deref() == Some(info.package.as_str()) { return; } - if let Some(previous) = entry.package() { + if let Some(previous) = known { tracing::info!( - "{} now installs {} rather than {previous}", + "{}/{component} now installs {} rather than {previous}", entry.label, info.package, ); @@ -373,11 +431,11 @@ impl AppState { .previous_packages .lock() .unwrap() - .insert(key.clone(), previous.to_string()); + .insert((key.clone(), component.clone()), previous); } let update = state.update(|config| { if let Some(project) = config.projects.iter_mut().find(|p| p.key == key) { - set_measured_package(&mut project.components, info.package); + set_measured_package(&mut project.components, &component, info.package); } Ok(()) }); @@ -547,7 +605,10 @@ impl AppState { Ok(()) })?; // Nothing left for either to be about. - self.previous_packages.lock().unwrap().remove(key); + self.previous_packages + .lock() + .unwrap() + .retain(|(project, _), _| project != key); self.service_checks.forget(key); self.resource_checks.forget(key); Ok(()) @@ -616,7 +677,7 @@ impl AppState { project.resources = declared.resources; project.components = declared.components; for component in &mut project.components { - if let Some(package) = measured.get(component.name()) { + if let Some(package) = measured.get(&component_id(component)) { component.set_package(package.clone()); } } @@ -674,21 +735,28 @@ fn reconcile_self(config: &mut Config, self_project: &Path) -> bool { } else { Vec::new() }; - if !components + // Named rather than assumed: which component is the APK is the + // declaration's business, and only the fallback below gets to pick a + // name for it. + let apk_name = match components .iter() - .any(|component| matches!(component, Component::Apk { .. })) + .find(|component| matches!(component, Component::Apk { .. })) { - components.push(Component::Apk { - name: "app".to_string(), - build: crate::config::Command::default(), - cwd: None, - stale_when: None, - strip: false, - package: None, - built_from: None, - }); - } - set_measured_package(&mut components, SELF_PACKAGE.to_string()); + Some(component) => component.name().to_string(), + None => { + components.push(Component::Apk { + name: "app".to_string(), + build: crate::config::Command::default(), + cwd: None, + stale_when: None, + strip: false, + package: None, + built_from: None, + }); + "app".to_string() + } + }; + set_measured_package(&mut components, &apk_name, SELF_PACKAGE.to_string()); let existing = config .projects .iter_mut() @@ -925,6 +993,89 @@ mod tests { .expect("the configured app") } + fn apk_named(name: &str, cwd: Option<&str>) -> Component { + Component::Apk { + name: name.to_string(), + build: crate::config::Command::default(), + cwd: cwd.map(PathBuf::from), + stale_when: None, + strip: false, + package: None, + built_from: None, + } + } + + fn write_apk(dir: &Path, relative: &str) -> PathBuf { + let path = dir.join(relative); + std::fs::create_dir_all(path.parent().expect("a parent")).expect("mkdir"); + std::fs::write(&path, "not really an apk").expect("write"); + path + } + + /// A project with two clients: each component's builds are its own, + /// found under the directory the component already says it lives in. + /// + /// The second APK is the case this exists for -- anchored at the + /// project root, the patterns reach the first client and stop, so the + /// second was invisible while the first looked like the project's + /// answer. + #[test] + fn each_apk_component_finds_its_own_builds() { + let dir = tempfile::tempdir().expect("tempdir"); + let first = write_apk( + dir.path(), + "app/androidApp/build/outputs/apk/debug/androidApp-debug.apk", + ); + let second = write_apk( + dir.path(), + "app-dioxus/target/dx/app-dioxus/debug/android/app/app/build/outputs/apk/debug/app-debug.apk", + ); + + let entry = entry_for( + dir.path(), + vec![ + apk_named("app", Some("app")), + apk_named("app-dioxus", Some("app-dioxus")), + ], + ); + let paths = |name: &str| { + let component = entry.apk_component(Some(name)).expect("the component"); + entry + .variants(component) + .into_iter() + .map(|candidate| candidate.path) + .collect::>() + }; + assert_eq!(paths("app"), vec![first]); + assert_eq!(paths("app-dioxus"), vec![second]); + } + + /// Which of two APKs a request means is a question, not something to + /// answer with whichever was declared first -- two clients install + /// over different packages, so a guess reads as a correct answer while + /// putting the wrong app on the phone. + #[test] + fn naming_no_component_answers_only_for_a_project_with_one_apk() { + let dir = tempfile::tempdir().expect("tempdir"); + + let one = entry_for(dir.path(), vec![apk_named("app", None)]); + assert_eq!(one.apk_component(None).map(Component::name), Some("app")); + + let two = entry_for( + dir.path(), + vec![ + apk_named("app", Some("app")), + apk_named("app-dioxus", Some("app-dioxus")), + ], + ); + assert!(two.apk_component(None).is_none()); + assert_eq!( + two.apk_component(Some("app-dioxus")).map(Component::name), + Some("app-dioxus") + ); + assert!(two.apk_component(Some("nothing-by-that-name")).is_none()); + } + /// The point of the whole mechanism: a project asking for a command /// does not thereby get to run one. #[test] @@ -1091,7 +1242,10 @@ mod tests { assert_eq!(entry.label, "Declared Name"); // Its own package is this server's, not something a file may // claim -- it is the one entry that is this program. - assert_eq!(entry.package(), Some(SELF_PACKAGE)); + assert_eq!( + entry.apk_component(None).and_then(Component::package), + Some(SELF_PACKAGE) + ); assert!( entry .build @@ -1114,7 +1268,10 @@ mod tests { let entry = self_entry(&app); assert_eq!(entry.label, SELF_LABEL); - assert_eq!(entry.package(), Some(SELF_PACKAGE)); + assert_eq!( + entry.apk_component(None).and_then(Component::package), + Some(SELF_PACKAGE) + ); assert!(entry.built_in); // Nothing declares how to build it, so nothing is guessed. assert!(entry.build.is_none()); diff --git a/server/src/routes.rs b/server/src/routes.rs index dc598a8..fce6fc1 100644 --- a/server/src/routes.rs +++ b/server/src/routes.rs @@ -127,6 +127,14 @@ enum ApiError { UnknownApp(String), #[error("{0} has no build yet")] NotBuilt(String), + /// A project that produces more than one APK, asked for "the" APK. + /// Refused rather than answered with the first: which of two clients + /// somebody meant is not something to guess, and a guess here would + /// install the wrong app while looking like it worked. + #[error("{0} builds more than one app -- say which with ?component=")] + AmbiguousApk(String), + #[error("{0} has no component named {1}")] + UnknownComponent(String, String), #[error("{0} has no on-demand build step configured")] NoBuildStep(String), #[error("{0}")] @@ -140,7 +148,11 @@ enum ApiError { impl IntoResponse for ApiError { fn into_response(self) -> Response { let status = match self { - Self::UnknownApp(_) | Self::NotBuilt(_) | Self::NoBuildStep(_) => StatusCode::NOT_FOUND, + Self::UnknownApp(_) + | Self::NotBuilt(_) + | Self::NoBuildStep(_) + | Self::UnknownComponent(..) => StatusCode::NOT_FOUND, + Self::AmbiguousApk(_) => StatusCode::BAD_REQUEST, Self::BadRequest(_) => StatusCode::BAD_REQUEST, Self::RangeNotSatisfiable => StatusCode::RANGE_NOT_SATISFIABLE, Self::Internal(err) => { @@ -162,6 +174,56 @@ fn bad_request(err: anyhow::Error) -> ApiError { ApiError::BadRequest(format!("{err:#}")) } +/// The installable half of a component, absent for a `Server`. +/// +/// Nested rather than flattened onto the component with every field +/// optional, because "this component has an APK" is one fact rather than +/// six: a server has no build to install, no package to replace and no +/// variants to choose between, and saying that once is what stops the +/// phone having to work it out from a size of zero. +/// +/// Per component and not per project. Two clients built from one checkout +/// install over different packages and are worth stripping to different +/// degrees, so a project-level answer would be the first component's, +/// presented as the project's. +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +struct ManifestApk { + /// What the download is saved as on the device. + filename: String, + /// Absent until a build has been read for it -- a project can be added + /// before it has ever been built, and claiming a package before then + /// would be inventing one. + #[serde(skip_serializing_if = "Option::is_none")] + package: Option, + /// What this component used to install over, when it has been renamed + /// since this server started. + /// + /// Android treats a renamed `applicationId` as an unrelated app, so + /// the old one is still installed and nothing will ever replace it. + /// The phone offers to remove it -- and knows whether it is still + /// there, which is why nothing here has to be cleared. + #[serde(skip_serializing_if = "Option::is_none")] + previous_package: Option, + /// False when this component has no APK yet (never built, or cleaned). + /// It is still drawn -- it was declared deliberately, and a row saying + /// so is a better answer than one that silently vanished -- with + /// `mtime`/`size` at zero and nothing to download. + built: bool, + /// Epoch seconds of the raw build's mtime. Always the *raw* build's, + /// even when a stripped copy is what's served: that's the number that + /// actually moves when something is rebuilt, which is what the app + /// compares against the installed copy. + mtime: f64, + /// Of the file that would be served as things stand -- the slim copy + /// where one has already been produced. Close to the bytes about to be + /// downloaded rather than exactly them, because finding out exactly + /// would mean running the strip pipeline here; see + /// `strip::serveable_now`. + size: u64, + variants: Vec, +} + /// One component, as the card needs it: what it is called and which kind /// it is. Nothing else -- what it *does* is the build step, which is not /// the phone's business, and how far along it is arrives on the status. @@ -246,15 +308,22 @@ struct ManifestComponent { /// project that keeps nothing. #[serde(skip_serializing_if = "Option::is_none")] resources_error: Option, + /// What there is to install, for a component that produces an APK. + #[serde(skip_serializing_if = "Option::is_none")] + apk: Option, } impl ManifestComponent { - fn read( + /// Async only for the APK's size, which is a `stat` of whatever is on + /// disk -- never a strip run to find out what the slim copy would + /// weigh, because this path is fetched on every open, resume and + /// Refresh. + async fn read( state: &AppState, key: &str, entry: &AppEntry, component: &crate::config::Component, - ) -> Self { + ) -> Result { let name = component.name().to_string(); let is_server = matches!(component, crate::config::Component::Server { .. }); // A build log is a file this server wrote, so its existence is a @@ -271,7 +340,7 @@ impl ManifestComponent { .then(|| state.resource_checks.facts(key)) .flatten() .map(|facts| crate::purge::paths(&facts, &entry.project_path)); - Self { + Ok(Self { kind: if is_server { "server" } else { "apk" }, state: is_server .then(|| state.service_checks.state(key, &name)) @@ -317,8 +386,56 @@ impl ManifestComponent { resources_error: is_server .then(|| state.resource_checks.error(key)) .flatten(), + apk: match is_server { + true => None, + false => Some(ManifestApk::read(state, key, entry, component).await?), + }, name, - } + }) + } +} + +impl ManifestApk { + async fn read( + state: &AppState, + key: &str, + entry: &AppEntry, + component: &crate::config::Component, + ) -> Result { + let name = component.name(); + let newest = entry.resolve_apk(component, None); + let size = match &newest { + Some(apk) => { + tokio::fs::metadata(crate::strip::serveable_now(&apk.path, component.strip())) + .await + .context("stat the apk to be served")? + .len() + } + None => 0, + }; + Ok(Self { + filename: newest + .as_ref() + .map(|apk| entry.filename(&apk.path)) + .unwrap_or_else(|| format!("{key}-{name}.apk")), + package: component.package().map(str::to_string), + previous_package: state.previous_package(key, name), + built: newest.is_some(), + mtime: newest + .as_ref() + .map(|apk| epoch_secs(apk.modified)) + .unwrap_or(0.0), + size, + variants: entry + .variants(component) + .into_iter() + .map(|candidate| ManifestVariant { + path: candidate.path.to_string_lossy().into_owned(), + variant: candidate.variant, + mtime: epoch_secs(candidate.modified), + }) + .collect(), + }) } } @@ -341,44 +458,22 @@ struct ManifestResponse { struct ManifestApp { key: String, label: String, - filename: String, + /// Where this project's APKs are fetched from. Which of them is said + /// with the request (`?component=`), so this stays one route per + /// project rather than one string per component that differs only in + /// its query. route: String, - /// Absent until a build has been read for it -- a project can be added - /// before it has ever been built, and claiming a package before then - /// would be inventing one. - #[serde(skip_serializing_if = "Option::is_none")] - package: Option, - /// What this project's APK used to install over, when it has been - /// renamed since this server started. - /// - /// Android treats a renamed `applicationId` as an unrelated app, so - /// the old one is still installed and nothing will ever replace it. - /// The phone offers to remove it -- and knows whether it is still - /// there, which is why nothing here has to be cleared. - #[serde(skip_serializing_if = "Option::is_none")] - previous_package: Option, project_path: String, - /// Epoch seconds of the raw build's mtime. Always the *raw* build's, - /// even when a stripped copy is what's served: that's the number that - /// actually moves when something is rebuilt, which is what the app - /// compares against the installed copy. - mtime: f64, - /// Of the file that would be served as things stand -- the slim copy - /// where one has already been produced. Close to the bytes about to be - /// downloaded rather than exactly them, because finding out exactly - /// would mean running the strip pipeline here; see - /// `strip::serveable_now`. - size: u64, needs_build: bool, /// This machine's preferences for the project, so the card's settings /// can show what is currently set rather than a guess at it. git_ipv4: bool, /// True for this server's own app, which has no Remove button. built_in: bool, - /// False when the project has no APK yet (never built, or cleaned). - /// Such an app is still listed -- it was added deliberately, and a card - /// saying so is a better answer than one that silently vanished -- with - /// `mtime`/`size` at zero and nothing to download. + /// Whether *anything* this project produces has been built. Per + /// component is on the component (`ManifestApk::built`); this is what + /// the card's own "nothing here yet" line reads, and what keeps a + /// project with one built client out of it. built: bool, /// Present when the project is in a git repository at all: the branch, /// how far behind it is as of the last fetch, and whether the tree is @@ -422,7 +517,6 @@ struct ManifestApp { /// something the keys don't say. #[serde(skip_serializing_if = "Option::is_none")] pending_declaration: Option, - variants: Vec, } /// One build found under a project. Which of them a device wants is that @@ -514,27 +608,13 @@ async fn app( /// whole list and a single card cannot come to describe the same app /// differently. async fn describe(state: &Arc, entry: &AppEntry) -> Result { - let apk = entry.resolve_apk(None); - // Whatever is on disk right now, never a strip run to find out -- - // see `strip::serveable_now`. This path is fetched on every open, - // resume and Refresh. - let size = match &apk { - Some(apk) => tokio::fs::metadata(crate::strip::serveable_now(&apk.path, entry.strip())) - .await - .context("stat the apk to be served")? - .len(), - None => 0, - }; - - let variants = entry - .variants() - .into_iter() - .map(|candidate| ManifestVariant { - path: candidate.path.to_string_lossy().into_owned(), - variant: candidate.variant, - mtime: epoch_secs(candidate.modified), - }) - .collect(); + // A loop rather than a map because each component's APK is a `stat`, + // and they are described in declaration order -- which is build order, + // and the order the card draws them in. + let mut components = Vec::with_capacity(entry.components.len()); + for component in &entry.components { + components.push(ManifestComponent::read(state, &entry.key, entry, component).await?); + } let git = crate::git::status(&entry.project_path); // An upstream is part of it: a branch that tracks nothing has nothing @@ -550,34 +630,22 @@ async fn describe(state: &Arc, entry: &AppEntry) -> Result, entry: &AppEntry) -> Result, variant: Option, } @@ -1334,8 +1412,12 @@ struct SelfBuild { /// the file and answers two numbers. async fn self_build(State(state): State>) -> Result, ApiError> { let entry = lookup(&state, None)?; + // No component named, because this route is frozen and cannot carry + // one -- and this server's own project produces exactly one APK, + // which is what makes that an answer rather than a guess. + let component = apk_component(&entry, None)?; let apk = entry - .resolve_apk(None) + .resolve_apk(component, None) .ok_or_else(|| ApiError::NotBuilt(entry.label.clone()))?; let size = tokio::fs::metadata(&apk.path) .await @@ -1347,6 +1429,28 @@ async fn self_build(State(state): State>) -> Result( + entry: &'a AppEntry, + named: Option<&str>, +) -> Result<&'a crate::config::Component, ApiError> { + if let Some(component) = entry.apk_component(named) { + return Ok(component); + } + Err(match named { + Some(name) => ApiError::UnknownComponent(entry.label.clone(), name.to_string()), + // Nothing named, and not one obvious answer: either the project + // builds no APK at all, or it builds several and the request has + // to say which. + None if entry.apk_components().next().is_none() => ApiError::NotBuilt(entry.label.clone()), + None => ApiError::AmbiguousApk(entry.label.clone()), + }) +} + async fn serve_apk( State(state): State>, key: Option>, @@ -1355,20 +1459,23 @@ async fn serve_apk( Query(query): Query, ) -> Result { let entry = lookup(&state, key)?; + let component = apk_component(&entry, query.component.as_deref())?; // Which build this device wants, if it has a preference. Its own // preference, travelling with the request: two phones enrolled against - // one server must not change what the other gets. + // one server must not change what the other gets. Validated against + // *this component's* builds, so naming another one's path is a + // fallback to this one's newest rather than a way to be served it. let requested = query.variant.map(PathBuf::from); let apk = entry - .resolve_apk(requested.as_deref()) + .resolve_apk(component, requested.as_deref()) .ok_or_else(|| ApiError::NotBuilt(entry.label.clone()))?; // A fresh download, not a range continuation: the one moment this // server can notice that a local rebuild changed what the APK installs // over. Off the request, so it costs the download nothing. if headers.get(header::RANGE).is_none() { - state.refresh_package(&entry.key, apk.path.clone()); + state.refresh_package(&entry.key, component.name(), apk.path.clone()); } - let resolved = resolve_serveable_path(&apk.path, entry.strip()).await?; + let resolved = resolve_serveable_path(&apk.path, component.strip()).await?; let size = tokio::fs::metadata(&resolved) .await .context("stat resolved apk")?