From 17d873ab7c50c79f43447950b1f64f1404086aea Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Wed, 2 Sep 2026 06:22:05 -0400 Subject: [PATCH] Give each component build modes and a settings sheet A component now declares its ways of being built in one list -- `modes: ["release", "debug"]`, the first the default -- and every command it runs is handed the mode as its last argument, so a project whose script takes `release` or `debug` names that script once. A field may instead be written per mode, which is the escape hatch for the commands that cannot take the word: cargo takes `--release` or nothing, and its profile for the unoptimised build is called `dev` while the directory it writes is called `debug`, so no single word serves as both the flag and the path. A command written per mode is not handed the word as well; it already is the answer, and a stray argument to a service binary is a process that will not start. One list rather than gathering names from whichever fields happened to mention them is what makes a mode missing from one part unsayable: every per-mode map is checked against it, so a gap is named rather than resolved to some other mode's command. Which mode to build in and whether to strip are the build machine's -- there is one checkout and one set of outputs, so a per-device mode would have two phones rebuilding over each other silently. Which of the finished builds a phone installs stays that phone's. Neither choice is part of the acceptance gate, so both have to be carried across the components list being rewritten, by acceptance and by the self entry's startup reconciliation alike; without the second a mode chosen for this server's own component would not survive the restart that applies it. A mode switch moves no commit, so `builtMode` is recorded beside `builtFrom`. Without it a component built in debug and switched to release reads as current and serves the debug build for ever -- and for an APK nothing else notices, because the "never built at all" check finds any variant under the component's directory. Each component card gains a settings sheet behind a gear at the row's right-hand end, holding the mode, which build to install, strip, and an Enrol button. The variant picker moved into it: on the card the two read as one choice, both saying debug and release, and they are not. The row's text is now bounded and truncates, so a long status can no longer push the log and settings buttons off the edge. `enroll:` is a declared command whose one line of stdout is a URL for the phone to open after installing -- generic on purpose, run per press since such a link is one-shot and carries a credential, and in the acceptance gate because it runs on the build machine. Co-Authored-By: Claude Opus 5 --- .dev-updater.ron | 35 +- AGENTS.md | 106 ++- .../kotlin/com/example/devupdater/AppsApi.kt | 47 + .../com/example/devupdater/UpdateManifest.kt | 34 + .../com/example/devupdater/UpdaterScreen.kt | 590 ++++++++++--- server/src/build_state.rs | 181 +++- server/src/config.rs | 833 +++++++++++++++++- server/src/main.rs | 1 + server/src/registry.rs | 184 +++- server/src/resources.rs | 51 +- server/src/routes.rs | 159 ++++ server/src/script.rs | 76 ++ server/src/service.rs | 101 ++- .../service-and-app/.dev-updater.ron | 7 + test-projects/service-and-app/build-daemon.sh | 21 +- test-projects/service-and-app/serve.sh | 19 +- 16 files changed, 2225 insertions(+), 220 deletions(-) create mode 100644 server/src/script.rs diff --git a/.dev-updater.ron b/.dev-updater.ron index f56f03e..4ec486b 100644 --- a/.dev-updater.ron +++ b/.dev-updater.ron @@ -28,19 +28,50 @@ components: [ // started anywhere else this server finds no checkout of its own, // and its card silently loses the branch line, its commit count // and the Pull button. - build: "cargo build --release --manifest-path server/Cargo.toml", + // The ways this component can be built, declared in one place. + // Release first, because the first is the default: what this + // server serves to a phone should be the optimised build unless + // somebody says otherwise, and debug is here for the times when + // a stack trace matters more than the speed. + modes: ["release", "debug"], + // Written per mode rather than handed the mode as an argument, + // which is the escape hatch and this is what it is for: cargo + // takes `--release` or nothing, and its *profile* for the + // unoptimised build is called `dev` while the directory it + // writes is called `debug`. So no single word can serve as both + // the flag and the path, and a project that wants one would have + // to carry a wrapper script to translate. + build: { + "release": "cargo build --release --manifest-path server/Cargo.toml", + "debug": "cargo build --manifest-path server/Cargo.toml", + }, // Managed, like anything else that just wants its binary kept // running. Nothing about restarting *this* server lives in the // script -- that is `restart.rs`, which defers the hand-over past // the reply and spawns it detached. So there was nothing left for // a script of its own to say. - service: Managed("server/target/release/dev-updater"), + // And the same for the binary, which is the pairing that makes + // per-mode worth having at all: built with `--release` and + // started from `target/debug/`, this server would go on running + // last week's build with nothing anywhere saying so. + service: Managed({ + "release": "server/target/release/dev-updater", + "debug": "server/target/debug/dev-updater", + }), ), Apk( name: "app", // The command resolves against the project root and `cwd` says // where to run it -- two different things, which is why this is // not `./build-apk.sh`. + // + // Deliberately one mode. A release build of *this* app would be + // signed with a different key from the debug one, and Android + // refuses to install a differently-signed APK over an installed + // package -- which for this app means the copy on the phone can + // no longer be updated through the server it updates itself + // from. Every other project can switch freely; this is the one + // that cannot, because it is the way back. build: "app/build-apk.sh", cwd: "app", ), diff --git a/AGENTS.md b/AGENTS.md index 8aef175..958bb91 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -36,7 +36,9 @@ mutable at runtime from the phone. list). `discover.rs` is the scanner, `config.rs` the persisted schema and the RON both config files are in, `apkinfo.rs` the `aapt2` reads, - `strip.rs` the slim-APK pipeline, `sdk.rs` the SDK/NDK tool lookups. + `strip.rs` the slim-APK pipeline, `sdk.rs` the SDK/NDK tool lookups, + `script.rs` the one runner for a project's short declared commands + (`resources:`, `enroll:`) -- stdin closed, a deadline, stdout kept. - `app/` — Kotlin + Compose, a single `:androidApp` module. `UpdaterScreen.kt` is the list, `AddAppScreen.kt` the add/settings screen, `AppsApi.kt` the management calls, `UpdateManifest.kt` the read side, `ApkInstaller.kt` / @@ -618,6 +620,108 @@ mutable at runtime from the phone. the declaration build anyway. Pulling stays allowed while a request is unaccepted, since taking commits runs git rather than the project's command, and it is how the new request arrives to be read. +- **A component declares its build modes in one list, and that list is the + only place a mode is declared.** `modes: ["release", "debug"]` on the + component; the first is the default, which is why a project puts the one + it wants built by default first. Every command the component runs is + then handed the mode **as its last argument** -- so a project whose + script takes `release` or `debug` names that script once and nothing is + written twice. The escape hatch is writing a field *per mode* + (`build: {"release": ..., "debug": ...}`, `service: Managed({...})`), + and it exists because some commands cannot take the word: cargo takes + `--release` or nothing, and its profile for the unoptimised build is + called `dev` while the directory it writes is called `debug`, so no + single word serves as both the flag and the path. Gradle is the same + shape (`assembleRelease` needs capitalising). + **A command written per mode is not handed the mode as well** -- it + already is the answer, and appending the word would pass a stray + argument to a binary that never asked for one, which for a service is a + process that will not start. `ByMode::mode_argument` is the one place + that rule lives. + Having *one* list is what makes "this part has a mode the other part + never heard of" unsayable rather than something to detect: every + per-mode map is checked against it (`config::mode_problem`), so a + missing entry is named as missing instead of resolving to some other + mode's command. A map on a component that declares no modes is the same + mistake from the other side and is refused with a message saying what + to write. A declaration that fails the check is discarded exactly as an + unknown field is, and the card says so. + **`ByMode` holds a `Command` and nothing else, and that is load-bearing + rather than incidental.** Telling the single form from the map needs + `deserialize_any`, and RON reports a struct `(path: "a")` as a *map* -- + so a struct-valued field behind it would read its own field names as + mode names. Worse, measured: RON **discards the variant name** under + `deserialize_any`, so `Cargo("server")` and the array command + `["server"]` arrive as the same one-element sequence. That is why the + by-mode map lives *inside* `Service` rather than around it, and why a + `Cargo(...)`-style shorthand cannot be added to `build:` beside the + bare-string form without a format migration. + +- **Which mode to build in, and whether to strip, are the build + machine's choices; which finished build to install is the phone's.** + There is one checkout and one set of outputs, so a per-device mode + would have two enrolled phones rebuilding over each other with nothing + on either screen to say why -- the settings sheet says as much out + loud. They are stored on the component in `config.ron` (`mode`, + `stripHere`), set through `PUT /apps/{key}/components/{name}/settings`, + and excluded from `Component::same_declaration`: choosing one of the + declared modes is not the project asking for something new, and + including it would make every settings change re-open the acceptance + gate. Which means both halves of the carry-across have to be written -- + `registry::chosen_settings`/`restore_chosen_settings`, called by + `approve_declaration` **and** `reconcile_self`. Without the second, a + mode chosen for this server's own component would last until the next + restart, and restarting is how this server is updated. + `strip` stays declarable by the project and `stripHere` overrides it, + rather than being copied in at acceptance: copied, a later change to + the declaration would be ignored on every machine, silently. + **`BuildState::matches` compares the effective mode as well as the + declaration**, because that state holds a *snapshot* of the components + it builds from -- reused across a mode change it would go on running + the old mode's command from a card reporting the new one. + +- **A mode switch moves no commit, so `builtMode` is recorded beside + `builtFrom`.** Without it a component built in `debug` and switched to + `release` reads as current, offers nothing to press, and serves the + debug build for ever -- and for an `Apk` nothing else would notice, + because the "never built at all" check finds *any* variant under the + component's directory, so the debug APK sitting there is enough to + satisfy it. `Component::built_in_another_mode` answers it and both + `freshness` and `component_is_stale` ask, the first reporting `Behind` + *before* consulting the checkout at all: a clean tree at the very + commit the debug build was made from still does not make that build a + release one. It stays quiet until this server has built the component + once, so a project built by hand is not told it is out of date. + +- **Each component card has a settings sheet, and the variant picker + lives in it.** The gear sits at the row's right-hand end beside the log + button, drawn unconditionally so its presence is never the signal. The + sheet holds the build mode, which of the discovered builds this phone + installs, whether to strip, and Enrol; a section with nothing to offer + says so rather than vanishing, since "this project declares one way of + building" and "we could not tell" must not look alike. The variant + picker moved here from the foot of the card because on the card the two + read as one choice -- both say `debug` and `release` -- and they are + not: one decides what the build machine *builds*, the other which + finished build this phone takes. Everything applies on Save except + Enrol, which is an action and happens on the press. + **The row's text is inside one weighted child so it can never push the + controls off the edge**, and every reading in it truncates with an + ellipsis. A control that leaves because the text grew is one the reader + cannot get back to. + +- **`enroll:` is a command whose one line of stdout is a URL for the + phone to open after installing.** Deliberately "a URL to open" rather + than anything named after enrolment -- a route that knew what enrolling + was would be a special case of itself. Run **per press** + (`POST /apps/{key}/components/{name}/enroll-link`), never cached: the + link a project mints is ordinarily one-shot and carries a credential, + so a stored one would be both stale and a secret sitting in a file. In + the acceptance gate like `build`, because it runs on the build machine. + `crate::script::capture` is the shared runner it and `resources` both + use -- stdin closed, a deadline, stdout kept, and a failure naming the + command and the first line of its stderr. + - **Which build variant to serve is the phone's choice, not the server's.** It arrives as `?variant=` on the download, beside the `?component=` that says whose build it is, and is validated against *that component's* diff --git a/app/androidApp/src/main/kotlin/com/example/devupdater/AppsApi.kt b/app/androidApp/src/main/kotlin/com/example/devupdater/AppsApi.kt index 299843a..beb6b06 100644 --- a/app/androidApp/src/main/kotlin/com/example/devupdater/AppsApi.kt +++ b/app/androidApp/src/main/kotlin/com/example/devupdater/AppsApi.kt @@ -203,6 +203,53 @@ data class ComponentLog( * Given the manifest's timeout rather than the default: a large log is a real request that takes * real time, and timing it out would report the server as unreachable when it is merely reading. */ +/** + * This machine's preferences for one component: which declared build mode to build it in, and + * whether to strip what it serves. + * + * On the build machine rather than on this device, unlike the chosen variant. A mode decides what + * gets *built*, and there is one checkout there -- two phones holding different answers would + * rebuild over each other with nothing on either screen to say why. Which of the finished builds + * this phone installs stays this phone's business, and stays in SharedPreferences. + * + * A null mode means "no choice, take the first the project declared", and a null strip means the + * same about the project's own declaration. Both are real answers rather than missing fields. + */ +fun setComponentSettings(key: String, component: String, mode: String?, strip: Boolean?) { + // Explicit JSON nulls rather than omitted keys, so "put this back to + // what the project says" is something this can express at all. + val body = + JSONObject().put("mode", mode ?: JSONObject.NULL).put("strip", strip ?: JSONObject.NULL) + requestFromServer( + "/apps/$key/components/$component/settings", + method = "PUT", + jsonBody = body.toString(), + ) {} +} + +/** + * Asks the build machine to run this component's `enroll:` command and hand back the URL it + * printed, for this phone to open. + * + * Run on every press rather than fetched once with the manifest: the link a project mints is + * ordinarily one-shot and carries a credential, so a cached one would be both stale and a secret + * sitting in a list this app redraws constantly. + */ +fun enrollmentLink(key: String, component: String): String = + requestFromServer( + "/apps/$key/components/$component/enroll-link", + method = "POST", + readTimeoutMs = ENROLL_TIMEOUT_MS, + ) { connection -> + JSONObject(connection.inputStream.bufferedReader().readText()).getString("url") + } + +// Longer than an ordinary call: the command runs on the build machine and +// may have to touch a keystore or write a file on the way. Timing it out +// early would report "couldn't reach the server" about a server that is +// working. +private const val ENROLL_TIMEOUT_MS = 25000 + fun componentLog( key: String, component: String, 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 6073403..e166b08 100644 --- a/app/androidApp/src/main/kotlin/com/example/devupdater/UpdateManifest.kt +++ b/app/androidApp/src/main/kotlin/com/example/devupdater/UpdateManifest.kt @@ -143,6 +143,24 @@ data class ProjectComponent( // The last is the ordinary case and not a fault. val resourcesChecking: Boolean, val resourcesError: String?, + // Every way this component can be built, in the order the project + // declared them, with the first the default. Empty for a component + // that declares one way of building, which is not a choice -- the + // settings sheet says so rather than drawing a picker holding one + // entry that cannot be changed. + val modes: List, + // Which of them the build machine is set to build it in. Null exactly + // when [modes] is empty. Not a per-device preference like the chosen + // variant: there is one checkout and one set of build outputs on that + // machine, so this is what *everyone* gets, which is why the sheet + // says so out loud. + val mode: String?, + // The project declares a command that prints a link for this phone to + // open after installing -- an enrolment link, for the apps that need + // one. What the link does is the project's business; the button just + // opens it. False while a declaration is waiting to be accepted, when + // the command would not run. + val hasEnrollLink: Boolean, // What there is to install, for a component that produces one. Null // for a server, which builds nothing this phone installs. val apk: ComponentApk?, @@ -197,6 +215,14 @@ data class ComponentApk( // build freshness is the only meaningful signal, not a version code. val mtime: Double, val size: Long, + // Whether the build machine serves a stripped copy: what it settled + // on, which is what a download will actually do. + val strip: Boolean, + // What the *project* asks for, which differs only when somebody + // overrode it on that machine. Both are sent so the sheet can say the + // two disagree, rather than showing a switch that silently contradicts + // the checkout. + val stripDeclared: Boolean, // Every build discovered under this component, so a different one can // be selected without another round trip. val variants: List, @@ -370,6 +396,12 @@ private fun readEntry(entry: JSONObject): ManifestEntry { configPresent = component.optBoolean("configPresent", false), resourcesChecking = component.optBoolean("resourcesChecking", false), resourcesError = component.optString("resourcesError").ifEmpty { null }, + modes = + component.optJSONArray("modes").let { modes -> + (0 until (modes?.length() ?: 0)).map { modes!!.getString(it) } + }, + mode = component.optString("mode").ifEmpty { null }, + hasEnrollLink = component.optBoolean("hasEnrollLink", false), apk = component.optJSONObject("apk")?.let(::readApk), ) }, @@ -386,6 +418,8 @@ private fun readApk(apk: JSONObject): ComponentApk { built = apk.getBoolean("built"), mtime = apk.getDouble("mtime"), size = apk.getLong("size"), + strip = apk.optBoolean("strip", false), + stripDeclared = apk.optBoolean("stripDeclared", false), 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 96855b5..8c5d93d 100644 --- a/app/androidApp/src/main/kotlin/com/example/devupdater/UpdaterScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/devupdater/UpdaterScreen.kt @@ -1,5 +1,7 @@ package com.example.devupdater +import android.content.ActivityNotFoundException +import android.content.Intent import android.content.pm.PackageManager import android.net.Uri import android.os.SystemClock @@ -219,6 +221,16 @@ private sealed class ComponentState { /** [progress] is null when the response gave no length to measure against. */ data class Downloading(val progress: Float?) : ComponentState() + /** + * A short call to the build machine that is neither a build nor a download -- fetching an + * enrolment link is the one so far. + * + * Carries its own words because the card has no other way to say which call it is waiting on, + * and "working" over a bar that could be any of three things is what makes a screen feel like + * it is doing something at random. + */ + data class Busy(val what: String) : ComponentState() + /** Why the last thing this component was asked to do stopped. */ data class Error(val message: String) : ComponentState() } @@ -1013,6 +1025,61 @@ private fun AppListScreen( } } + /** + * The same as [manage], for a call that is about one component rather than the whole project. + * + * A failure belongs where the press happened: reported on the card, one component's settings + * write would blame the project and go quiet on the row somebody was actually looking at. + */ + fun manageComponent(entry: ManifestEntry, component: String, action: () -> Unit) { + scope.launch { + try { + withContext(Dispatchers.IO) { action() } + setComponent(entry.key, component, null) + applyOne(entry.key) + } catch (e: DownloadServerException) { + setComponent(entry.key, component, failure(e)?.let(ComponentState::Error)) + } + } + } + + /** + * Asks the build machine for this component's link and opens it here. + * + * Fetched on every press rather than carried on the manifest: what a project mints is + * ordinarily one-shot and carries a credential, so a link this app had been holding since the + * last refresh would be the wrong one. + * + * Nothing here inspects the URL. Which app answers it is Android's business, and a link the + * phone has nothing to open is reported as such rather than swallowed -- an action whose whole + * effect is elsewhere has to say when it did not happen. + */ + fun openEnrollmentLink(entry: ManifestEntry, component: String) { + scope.launch { + setComponent(entry.key, component, ComponentState.Busy("Getting the link")) + try { + val url = withContext(Dispatchers.IO) { enrollmentLink(entry.key, component) } + try { + context.startActivity( + Intent(Intent.ACTION_VIEW, Uri.parse(url)) + .addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) + ) + setComponent(entry.key, component, null) + } catch (e: ActivityNotFoundException) { + setComponent( + entry.key, + component, + ComponentState.Error( + "Nothing on this phone opens $url" + (e.message?.let { " ($it)" } ?: "") + ), + ) + } + } catch (e: DownloadServerException) { + setComponent(entry.key, component, failure(e)?.let(ComponentState::Error)) + } + } + } + /** * Runs one of a server component's actions on the build machine. * @@ -1356,6 +1423,24 @@ private fun AppListScreen( (component to variant.path) }) }, + // The build machine's, not this + // device's: a mode decides what + // gets built there, so it is a + // round trip and a refetch rather + // than a preference written here. + onComponentSettings = { component, mode, strip -> + manageComponent(entry, component) { + setComponentSettings( + entry.key, + component, + mode, + strip, + ) + } + }, + onEnroll = { component -> + openEnrollmentLink(entry, component) + }, serviceBusy = serviceBusy[entry.key], onServiceAction = { component, action, purge -> runServiceAction(entry, component, action, purge) @@ -1471,6 +1556,10 @@ private fun AppCard( onApprove: () -> Unit, onRemove: () -> Unit, onSelectVariant: (component: String, ApkVariant?) -> Unit, + /** This machine's settings for one component: which mode it builds in, and whether to strip. */ + onComponentSettings: (component: String, mode: String?, strip: Boolean?) -> Unit, + /** Fetch this component's link from the build machine and open it here. */ + onEnroll: (component: String) -> 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. @@ -1692,6 +1781,10 @@ private fun AppCard( }, chosenVariantPath = chosenVariantPath, onSelectVariant = { onSelectVariant(component.name, it) }, + onSettings = { mode, strip -> + onComponentSettings(component.name, mode, strip) + }, + onEnroll = { onEnroll(component.name) }, // This app reaches the server through this server. // Stopping or uninstalling it is the one action // here that cannot be undone from the phone. @@ -2020,6 +2113,12 @@ private fun ApkProgress(state: ComponentState?) { Text("Preparing the download...") } + is ComponentState.Busy -> { + ProgressBar() + Spacer(Modifier.height(4.dp)) + Text("${state.what}...") + } + is ComponentState.Downloading -> { val progress = state.progress if (progress == null) { @@ -2356,6 +2455,10 @@ private fun ComponentCard( /** Which of this component's builds this device is pinned to, if any. */ chosenVariantPath: String? = null, onSelectVariant: (ApkVariant?) -> Unit = {}, + /** Save this machine's settings for this component. */ + onSettings: (mode: String?, strip: Boolean?) -> Unit = { _, _ -> }, + /** Ask the build machine for this component's link and open it. */ + onEnroll: () -> Unit = {}, isOwnServer: Boolean, /** This component's part of a build in progress, if it has one. */ build: ComponentBuild?, @@ -2387,6 +2490,7 @@ private fun ComponentCard( // a second thing to remember to clear. var confirming by remember { mutableStateOf(null) } var showingLog by remember { mutableStateOf(false) } + var showingSettings by remember { mutableStateOf(false) } // What Uninstall has been asked to take away as well. Logs start // ticked and the other two do not: the dialog's defaults, deliberately // different from `Purge()`'s, which is what a caller with no dialog @@ -2407,116 +2511,149 @@ private fun ComponentCard( // same size as a line of this text anyway, so the row comes out // the same height without being told. Row(verticalAlignment = Alignment.CenterVertically) { - // Both kinds get the same square, so a server's glyph and - // an app's icon are the same size as each other -- one - // drawn smaller than the other reads as the row meaning - // less, rather than as a different kind of thing. - Box( - Modifier.size(COMPONENT_ICON_SIZE), - contentAlignment = Alignment.Center, + // Everything the row *says* lives inside one weighted + // child, so it can only ever have the space the controls + // on the right are not using. Without it a long name or a + // long status pushed the log and settings buttons off the + // edge -- and a control that leaves because the text grew + // is one the reader cannot get back to. + Row( + Modifier.weight(1f), + verticalAlignment = Alignment.CenterVertically, ) { - if (component.isServer) { - Text( - SERVER_GLYPH, - fontFamily = NerdIcons, - // Larger than the square in font terms: a glyph - // is drawn well inside its line box, so matching - // the numbers would draw it noticeably smaller - // than the icon beside it. - fontSize = COMPONENT_GLYPH_SIZE, - color = MaterialTheme.colorScheme.onSurfaceVariant, - // Which means its line box is taller than the - // square, and text clips to the height it is - // given. Measured unbounded and drawn centred - // instead: the row keeps the icon's height and - // the glyph keeps all of itself. - modifier = Modifier.wrapContentSize(unbounded = true), - ) - } else { - // The app's own icon, the same one the project card - // shows: this row is about the thing that gets - // installed, and that is what it looks like. - AppIcon( - packageName, - size = COMPONENT_ICON_SIZE, - glyphSize = COMPONENT_GLYPH_SIZE, - ) + // Both kinds get the same square, so a server's glyph and + // an app's icon are the same size as each other -- one + // drawn smaller than the other reads as the row meaning + // less, rather than as a different kind of thing. + Box( + Modifier.size(COMPONENT_ICON_SIZE), + contentAlignment = Alignment.Center, + ) { + if (component.isServer) { + Text( + SERVER_GLYPH, + fontFamily = NerdIcons, + // Larger than the square in font terms: a glyph + // is drawn well inside its line box, so matching + // the numbers would draw it noticeably smaller + // than the icon beside it. + fontSize = COMPONENT_GLYPH_SIZE, + color = MaterialTheme.colorScheme.onSurfaceVariant, + // Which means its line box is taller than the + // square, and text clips to the height it is + // given. Measured unbounded and drawn centred + // instead: the row keeps the icon's height and + // the glyph keeps all of itself. + modifier = Modifier.wrapContentSize(unbounded = true), + ) + } else { + // The app's own icon, the same one the project card + // shows: this row is about the thing that gets + // installed, and that is what it looks like. + AppIcon( + packageName, + size = COMPONENT_ICON_SIZE, + glyphSize = COMPONENT_GLYPH_SIZE, + ) + } } - } - Spacer(Modifier.width(6.dp)) - Text( - component.name, - style = MaterialTheme.typography.bodyMedium, - color = MaterialTheme.colorScheme.onSurfaceVariant, - ) - // Nothing at all until its script has been asked -- an - // unknown state is not a state, and a dot introducing - // nothing is worse than no dot. - // Said in words, not by colour alone: "behind the - // checkout" is a difference in kind from "running", and a - // reader has no way to learn a colour that means it. - val behind = component.isBehind - val status = - when { - !component.isServer -> null - component.error != null -> "couldn't check" - component.state == null -> null - component.isRunning -> "running" - // Not folded into "stopped": stopped is a state - // somebody chose, and calling a crash that sends - // the reader looking for who chose it. - component.isFailed -> "failed" - component.isInstalled -> "stopped" - // Nothing for a service that isn't installed: the row - // offers Install and nothing else, which says it more - // plainly than a state would, and saying both makes the - // absence of a thing look like a condition it is in. - else -> null - } - status?.let { - Separator() + Spacer(Modifier.width(6.dp)) Text( - it, - style = MaterialTheme.typography.bodyMedium, - color = - when { - component.isRunning -> runningColor - component.isFailed -> failedColor - else -> MaterialTheme.colorScheme.onSurfaceVariant - }, - ) - } - if (behind) { - Separator() - Text( - "out of date", - style = MaterialTheme.typography.bodyMedium, - // The colour Pull & Build wears, because that is - // the button this is telling you to press. - color = ActionTone.Primary.color, - ) - } - sizeText?.let { - Separator() - Text( - it, + component.name, style = MaterialTheme.typography.bodyMedium, color = MaterialTheme.colorScheme.onSurfaceVariant, + // One line and an ellipsis, on this and on every + // reading beside it: a row that wraps grows the + // card, and one that overflows silently loses its + // right-hand end without saying it was cut. + maxLines = 1, + overflow = TextOverflow.Ellipsis, ) + // Nothing at all until its script has been asked -- an + // unknown state is not a state, and a dot introducing + // nothing is worse than no dot. + // Said in words, not by colour alone: "behind the + // checkout" is a difference in kind from "running", and a + // reader has no way to learn a colour that means it. + val behind = component.isBehind + val status = + when { + !component.isServer -> null + component.error != null -> "couldn't check" + component.state == null -> null + component.isRunning -> "running" + // Not folded into "stopped": stopped is a state + // somebody chose, and calling a crash that sends + // the reader looking for who chose it. + component.isFailed -> "failed" + component.isInstalled -> "stopped" + // Nothing for a service that isn't installed: the row + // offers Install and nothing else, which says it more + // plainly than a state would, and saying both makes the + // absence of a thing look like a condition it is in. + else -> null + } + status?.let { + Separator() + Text( + it, + style = MaterialTheme.typography.bodyMedium, + color = + when { + component.isRunning -> runningColor + component.isFailed -> failedColor + else -> MaterialTheme.colorScheme.onSurfaceVariant + }, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + ) + } + if (behind) { + Separator() + Text( + "out of date", + style = MaterialTheme.typography.bodyMedium, + // The colour Pull & Build wears, because that is + // the button this is telling you to press. + color = ActionTone.Primary.color, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + ) + } + sizeText?.let { + Separator() + Text( + it, + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + ) + } } + // Outside the weighted text, so the spinner is never the + // thing that gets truncated: it is the one mark on the row + // that says something is happening right now. if (working || busy || component.checking) { Spacer(Modifier.width(6.dp)) Working() } + // These belong to the component, not to whatever the row + // happens to say about it, so they hold the same corner + // whatever the text does. Unconditional, so the settings + // button sits in the same place whether or not there is a + // log beside it -- the log's own absence must not move it. if (component.hasLogs) { - // Pushed to the far edge rather than following the - // text: it belongs to the component, not to whatever - // the row happens to say about it, and a control that - // slides about as the state changes is harder to find - // than one always in the same corner. - Spacer(Modifier.weight(1f)) IconGlyphButton(LOG_GLYPH, "Show ${component.name}'s log") { showingLog = true } } + // Always drawn, including for a component with a single + // build mode and nothing to strip: what a component can be + // told is part of what it is, and a control that comes and + // goes makes its own presence the signal. What it opens + // says which of its settings this component has. + IconGlyphButton(SETTINGS_GLYPH, "${component.name} settings") { + showingSettings = true + } } // The service script's own words about why it could not answer, @@ -2663,27 +2800,35 @@ private fun ComponentCard( } } - // 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) - } - } + // The variant picker used to sit here. It moved into the + // settings sheet, where the build mode is: on the card the two + // read as the same choice -- both say "debug" and "release" -- + // and they are not. One decides what the build machine + // *builds*; the other decides which of the finished builds + // this phone installs. } } + if (showingSettings) { + ComponentSettingsDialog( + component = component, + chosenVariantPath = chosenVariantPath, + // A mode changes what the build machine runs, and the server + // replaces the state a running build reports through. So the + // picker goes quiet exactly while this component cannot be + // acted on anyway -- the same pair of conditions that disables + // its Update button and withholds its freshness. + modeSettled = !(busy || working || projectState.busy), + onSelectVariant = onSelectVariant, + onEnroll = onEnroll, + onApply = { mode, strip -> + showingSettings = false + onSettings(mode, strip) + }, + onDismiss = { showingSettings = false }, + ) + } + if (showingLog) { ComponentLogDialog( entryKey = entryKey, @@ -2764,6 +2909,194 @@ private fun ComponentCard( } } +/** + * Everything one component can be told, in one sheet: how the build machine builds it, which of the + * finished builds this phone installs, whether it is stripped on the way, and the link a project + * hands over after an install. + * + * The two "debug or release" choices in here are deliberately separated and labelled, because they + * are not the same question and they used to sit on the card looking as though they were. **Build + * mode** is the build machine's: there is one checkout and one set of outputs there, so choosing it + * changes what *every* enrolled phone is offered, which is why the sheet says so rather than + * leaving it to be discovered. **Install** is this device's alone, and picks among builds that + * already exist. + * + * Applied on Save rather than as each control moves, matching the project's own settings sheet: a + * settings write is a round trip that rebuilds the entry on the server, and one per control touched + * while making up your mind is a lot of them. Enrol is the exception and is not a setting -- it is + * an action, it happens on the press, and it is drawn apart from the rest for that reason. + * + * A component with nothing to choose still gets its sections, saying so. Removing them would make + * the sheet's shape the signal, and "this project declares one way of building" and "we could not + * tell" would then look identical -- which is the whole reason the empty cases are written out here + * rather than skipped. + */ +@Composable +private fun ComponentSettingsDialog( + component: ProjectComponent, + chosenVariantPath: String?, + /** False while a build is in flight, when the mode must not be changed underneath it. */ + modeSettled: Boolean, + onSelectVariant: (ApkVariant?) -> Unit, + onEnroll: () -> Unit, + onApply: (mode: String?, strip: Boolean?) -> Unit, + onDismiss: () -> Unit, +) { + // Keyed on what the server last said, so reopening after a save shows + // the saved values rather than stale local ones. + var mode by remember(component.mode) { mutableStateOf(component.mode) } + var strip by remember(component.apk?.strip) { mutableStateOf(component.apk?.strip ?: false) } + // The variant is this device's, so it is applied locally on Save + // rather than sent anywhere; held here so Cancel really cancels. + var variantPath by remember(chosenVariantPath) { mutableStateOf(chosenVariantPath) } + + AlertDialog( + onDismissRequest = onDismiss, + title = { Text(component.name) }, + text = { + Column(Modifier.verticalScroll(rememberScrollState())) { + SettingsHeading("Build mode") + when { + component.modes.isEmpty() -> + SettingsNote( + "This project declares one way of building ${component.name}, so there " + + "is nothing to choose." + ) + else -> { + SettingsNote( + "Chosen on the build machine, so it is what every phone here is " + + "offered -- not just this one." + ) + // A dropdown rather than a row of radios, matching + // the build picker below it: they are the same + // shape of question -- one of a short list -- and + // two different controls for that in one sheet + // makes them look like different kinds of choice. + ModePicker(component.modes, mode, modeSettled) { mode = it } + if (!modeSettled) { + SettingsNote( + "Not while this component is busy: changing it now would leave " + + "the build that is running building the other one." + ) + } + } + } + + // Only an APK has anything below this. A server is not + // installed here, so there is no build for this phone to + // pick and nothing to strip on the way to it. + component.apk?.let { apk -> + Spacer(Modifier.height(16.dp)) + SettingsHeading("Install") + when { + apk.variants.isEmpty() -> + SettingsNote("Nothing is built yet, so there is nothing to install.") + apk.variants.size == 1 -> + SettingsNote( + "One build so far (${apk.variants.first().variant}), so there is " + + "nothing to choose between." + ) + else -> { + SettingsNote( + "Which of the builds on the machine this phone installs. Only " + + "this phone's." + ) + VariantPicker(apk.variants, variantPath) { variantPath = it?.path } + } + } + + Spacer(Modifier.height(16.dp)) + SettingsHeading("Transfer") + Row( + verticalAlignment = Alignment.CenterVertically, + modifier = Modifier.fillMaxWidth(), + ) { + Text( + "Strip debug symbols", + style = MaterialTheme.typography.bodyLarge, + modifier = Modifier.weight(1f), + ) + Spacer(Modifier.width(12.dp)) + Switch(checked = strip, onCheckedChange = { strip = it }) + } + // Said only when the two disagree. A machine that + // follows the checkout has nothing to report, and a + // line under every switch saying "as declared" is one + // more thing to read on every card. + if (strip != apk.stripDeclared) { + SettingsNote( + if (apk.stripDeclared) { + "The project asks for stripping; this machine is set not to." + } else { + "The project does not ask for stripping; this machine is set to." + } + ) + } + } + + if (component.hasEnrollLink) { + Spacer(Modifier.height(16.dp)) + SettingsHeading("Enrolment") + SettingsNote( + "Asks the build machine for a fresh link and opens it here. Do this after " + + "installing, or after reinstalling over a different signing key -- " + + "either loses whatever enrolment the app had." + ) + // Outside the Save/Cancel bargain the rest of the + // sheet makes, because it is not a setting: pressing + // it does the thing, now, and there is nothing about + // it left to save. + TextButton( + onClick = { + onDismiss() + onEnroll() + } + ) { + Text("Enrol this app") + } + } + } + }, + confirmButton = { + TextButton( + onClick = { + // The variant is this device's, so it is written here + // and not sent; the other two are the build machine's. + if (variantPath != chosenVariantPath) { + onSelectVariant( + component.apk?.variants?.firstOrNull { it.path == variantPath } + ) + } + onApply(mode, strip) + } + ) { + Text("Save") + } + }, + dismissButton = { TextButton(onClick = onDismiss) { Text("Cancel") } }, + ) +} + +@Composable +private fun SettingsHeading(text: String) { + Text(text, style = MaterialTheme.typography.titleSmall) +} + +/** + * A line of explanation under a control, or in place of one that has nothing to offer. + * + * Its own composable so that "there is nothing to choose here" looks the same wherever it is said + * -- four sections each phrasing their empty case differently is how a reader learns to skip them. + */ +@Composable +private fun SettingsNote(text: String) { + Text( + text, + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) +} + /** * The three things Uninstall can take away besides the service. * @@ -2903,6 +3236,35 @@ private fun PurgeToggle( } } +@Composable +private fun ModePicker( + modes: List, + chosen: String?, + enabled: Boolean, + onSelect: (String) -> Unit, +) { + var expanded by remember { mutableStateOf(false) } + Box { + TextButton(onClick = { expanded = true }, enabled = enabled) { + // Never "none": unlike the build picker there is always an + // answer here, because a component with modes is always being + // built in one of them -- the first, when nobody has chosen. + Text(chosen ?: modes.firstOrNull().orEmpty()) + } + DropdownMenu(expanded = expanded, onDismissRequest = { expanded = false }) { + modes.forEach { candidate -> + DropdownMenuItem( + text = { Text(if (candidate == chosen) "$candidate ✓" else candidate) }, + onClick = { + expanded = false + onSelect(candidate) + }, + ) + } + } + } +} + @Composable private fun VariantPicker( variants: List, diff --git a/server/src/build_state.rs b/server/src/build_state.rs index 3f0f35f..1ffb7ab 100644 --- a/server/src/build_state.rs +++ b/server/src/build_state.rs @@ -416,8 +416,18 @@ impl BuildState { self.git_pull == git_pull && self.git_ipv4 == git_ipv4 && self.components.len() == components.len() - && std::iter::zip(&self.components, components) - .all(|(mine, theirs)| mine.same_declaration(theirs)) + && std::iter::zip(&self.components, components).all(|(mine, theirs)| { + // The declaration, plus the one choice that changes what + // this state would actually run. `same_declaration` + // deliberately ignores the chosen mode -- picking one is + // not the project asking for something new -- but this + // state holds a *snapshot* of the components it builds + // from, so a mode changed underneath it would go on + // running the old mode's command from a card reporting + // the new one. Strip is not here: it is decided at + // download, and nothing this state does depends on it. + mine.same_declaration(theirs) && mine.effective_mode() == theirs.effective_mode() + }) } /// Idempotent: kicks off the build in the background if this app is @@ -546,6 +556,15 @@ impl BuildState { /// uncommitted work and sending it to a phone would also be a /// surprising thing to do with work its author has not committed. pub fn freshness(&self, component: &Component) -> Freshness { + // Behind for a reason no commit can express: what is on disk was + // built some other way than this component is set to build now. + // Reported before the checkout is consulted at all, because it is + // a fact rather than a comparison -- a clean tree at the very + // commit the debug build was made from still does not make that + // build a release one. + if component.built_in_another_mode() { + return Freshness::Behind; + } let built = self .inner .lock() @@ -575,6 +594,16 @@ impl BuildState { if component.build().is_empty() { return false; } + // The same fact `freshness` reports as Behind, and it has to be + // asked here too: for an `Apk`, the "never built at all" check + // below finds *any* variant under the component's directory, so a + // debug build sitting there is enough to make a component + // switched to release look built. Nothing else would notice -- + // the commit has not moved -- so Update would do nothing and the + // phone would install the debug APK from a card saying release. + if component.built_in_another_mode() { + return true; + } // Nothing built at all is as far behind as an output gets, and it // is checked before any rule rather than after: a `staleWhen` // compares two files *inside* a build, so a component that has @@ -960,11 +989,18 @@ impl BuildState { // wants to read afterwards. let log = crate::logs::open_build_log(&self.key, component.name()) .map(|file| Arc::new(Mutex::new(file))); + // The mode, for a build command written once -- which is how a + // project with a script that takes `release` or `debug` says it, + // and why nothing has to be written twice in that case. Nothing + // for a command written per mode: that command already *is* the + // answer, and handing the word over as well would pass a stray + // argument to a build that never asked for one. + let mode = component.build_mode_argument(); self.run_streaming_command( component.name(), component.build(), component.cwd(), - &[], + mode.as_slice(), log, ) } @@ -1323,22 +1359,30 @@ mod tests { vec![ Component::Server { name: "backend".to_string(), - build: crate::config::Command::from_line("true"), + modes: Vec::new(), + build: crate::config::ByMode::One(crate::config::Command::from_line("true")), cwd: Some(PathBuf::from("server")), stale_when: None, also_watch: Vec::new(), service: None, + mode: None, built_from: None, + built_mode: None, }, Component::Apk { name: "app".to_string(), - build: crate::config::Command::from_line("true"), + modes: Vec::new(), + build: crate::config::ByMode::One(crate::config::Command::from_line("true")), cwd: Some(PathBuf::from("app")), stale_when: None, also_watch: Vec::new(), strip: false, + enroll: crate::config::Command::default(), + strip_here: None, + mode: None, package: None, built_from: None, + built_mode: None, }, ] } @@ -1389,13 +1433,20 @@ mod tests { // pressed, and only the pull closes it. let accepted = vec![Component::Apk { name: "app".to_string(), - build: crate::config::Command::from_line("touch built-marker"), + modes: Vec::new(), + build: crate::config::ByMode::One(crate::config::Command::from_line( + "touch built-marker", + )), cwd: None, stale_when: None, also_watch: Vec::new(), strip: false, + enroll: crate::config::Command::default(), + strip_here: None, package: None, + mode: None, built_from: None, + built_mode: None, }]; let state = state_for(&clone, accepted.clone()); @@ -1447,22 +1498,34 @@ mod tests { let components = vec![ Component::Server { name: "backend".to_string(), - build: crate::config::Command::from_line("touch backend-built"), + modes: Vec::new(), + build: crate::config::ByMode::One(crate::config::Command::from_line( + "touch backend-built", + )), cwd: Some(PathBuf::from("server")), stale_when: None, also_watch: Vec::new(), service: None, + mode: None, built_from: None, + built_mode: None, }, Component::Apk { name: "app".to_string(), - build: crate::config::Command::from_line("touch app-built"), + modes: Vec::new(), + build: crate::config::ByMode::One(crate::config::Command::from_line( + "touch app-built", + )), cwd: Some(PathBuf::from("app")), stale_when: None, also_watch: Vec::new(), strip: false, + enroll: crate::config::Command::default(), + strip_here: None, + mode: None, package: None, built_from: None, + built_mode: None, }, ]; let state = state_for(root, components); @@ -1527,16 +1590,21 @@ mod tests { let components = ["slow", "quick"] .map(|name| Component::Apk { name: name.to_string(), - build: crate::config::Command::from_line(match name { + modes: Vec::new(), + build: crate::config::ByMode::One(crate::config::Command::from_line(match name { "slow" => "./slow.sh", _ => "touch quick-built", - }), + })), cwd: Some(PathBuf::from(name)), stale_when: None, also_watch: Vec::new(), strip: false, + enroll: crate::config::Command::default(), + strip_here: None, + mode: None, package: None, built_from: None, + built_mode: None, }) .to_vec(); let state = state_for(root, components); @@ -1598,16 +1666,21 @@ mod tests { let components = ["broken", "fine"] .map(|name| Component::Apk { name: name.to_string(), - build: crate::config::Command::from_line(match name { + modes: Vec::new(), + build: crate::config::ByMode::One(crate::config::Command::from_line(match name { "broken" => "false", _ => "true", - }), + })), cwd: Some(PathBuf::from(name)), stale_when: None, also_watch: Vec::new(), strip: false, + enroll: crate::config::Command::default(), + strip_here: None, + mode: None, package: None, built_from: None, + built_mode: None, }) .to_vec(); let state = state_for(root, components); @@ -1671,23 +1744,37 @@ mod tests { let components = vec![ Component::Apk { name: "a".to_string(), - build: crate::config::Command::from_line("touch a-built"), + modes: Vec::new(), + build: crate::config::ByMode::One(crate::config::Command::from_line( + "touch a-built", + )), cwd: Some(PathBuf::from("a")), stale_when: None, also_watch: Vec::new(), strip: false, + enroll: crate::config::Command::default(), + strip_here: None, + mode: None, package: None, built_from: None, + built_mode: None, }, Component::Apk { name: "b".to_string(), - build: crate::config::Command::from_line("touch b-built"), + modes: Vec::new(), + build: crate::config::ByMode::One(crate::config::Command::from_line( + "touch b-built", + )), cwd: Some(PathBuf::from("b")), stale_when: None, also_watch: Vec::new(), strip: false, + enroll: crate::config::Command::default(), + strip_here: None, + mode: None, package: None, built_from: None, + built_mode: None, }, ]; let state = state_for(root, components); @@ -1742,6 +1829,65 @@ mod tests { state.component_is_stale(component, &state.inner.lock().unwrap().built_from.clone()) } + /// The trap a per-component staleness check walks straight into once + /// modes exist. An APK's "never built at all" test finds *any* build + /// under the component's directory, so a debug APK sitting there is + /// enough to make a component switched to release look built -- and + /// switching modes moves no commit, so nothing else has an opinion. + /// Update would then do nothing and the phone would install the debug + /// build from a card saying release. + /// + /// Asserted through `component_is_stale` rather than through + /// `built_in_another_mode` alone, because the rule being right is not + /// the same as the check that decides whether to build asking it. + #[test] + fn a_component_switched_to_another_mode_is_stale_even_with_a_build_on_disk() { + let dir = tempfile::tempdir().expect("tempdir"); + let root = dir.path(); + // A build output where an APK actually lands, so the "nothing + // built here" branch is satisfied and cannot be what answers. + let built = root.join("build/outputs/apk/debug"); + std::fs::create_dir_all(&built).expect("mkdir"); + std::fs::write(built.join("app-debug.apk"), b"not really an apk").expect("write"); + + let moded = |mode: Option<&str>| { + vec![Component::Apk { + name: "app".to_string(), + modes: vec!["release".to_string(), "debug".to_string()], + build: crate::config::ByMode::One(crate::config::Command::from_line("true")), + cwd: None, + stale_when: None, + also_watch: Vec::new(), + strip: false, + enroll: crate::config::Command::default(), + strip_here: None, + package: None, + mode: mode.map(str::to_string), + // Built here once, in debug -- without which there is no + // build of ours for the comparison to be about. + built_from: Some("abc123".to_string()), + built_mode: Some("debug".to_string()), + }] + }; + + let matching = state_for(root, moded(Some("debug"))); + assert!( + !stale(&matching, "app"), + "the build on disk is this mode's, so there is nothing to do", + ); + + let switched = state_for(root, moded(Some("release"))); + assert!( + stale(&switched, "app"), + "a debug build must not satisfy a component set to build release", + ); + assert_eq!( + switched.freshness(&switched.components[0]), + Freshness::Behind, + "and the card has to say so rather than reading as current", + ); + } + /// The shape of bug a unit test that only calls `component_is_stale` /// cannot see: `status()` and `trigger_if_needed` hold the lock that /// the staleness check also wants. Taking it twice on one thread is a @@ -1893,13 +2039,18 @@ mod tests { root, vec![Component::Apk { name: "app".to_string(), - build: crate::config::Command::from_line("true"), + modes: Vec::new(), + build: crate::config::ByMode::One(crate::config::Command::from_line("true")), cwd: None, stale_when: None, also_watch: Vec::new(), strip: false, + enroll: crate::config::Command::default(), + strip_here: None, package: None, + mode: None, built_from: Some("0000000000000000000000000000000000000000".to_string()), + built_mode: None, }], ); assert!(!stale(&state, "app"), "no checkout means no opinion"); diff --git a/server/src/config.rs b/server/src/config.rs index 50a9b79..42adbf6 100644 --- a/server/src/config.rs +++ b/server/src/config.rs @@ -124,6 +124,15 @@ pub struct ProjectConfig { /// "neither set" are states the file can express and nothing means, which /// then need a check to reject rather than being unsayable. /// +/// The command inside either variant is a [`ByMode`], because which +/// binary a service runs is exactly what a build mode changes: a server +/// built with `--release` and started from `target/debug/` is the one +/// mistake this whole feature would otherwise introduce, and it would +/// look like a build that silently did nothing. The map goes *inside* the +/// variant rather than around it -- `service: {"debug": Managed(...)}` +/// cannot be told from a single value, since RON reports both a map and a +/// struct the same way; see [`ByMode`]. +/// /// Whichever it is, what comes out the other side is the same thing -- a /// command run as ` `, answering the contract in /// `crate::service`. `Managed` is not a second mechanism; it is the @@ -136,12 +145,30 @@ pub enum Service { /// something the built-in one does not -- dev-updater's own service is /// the example, since restarting it means restarting the process doing /// the asking. - Script(Command), + Script(ByMode), /// dev-updater supervises this command with its built-in script, so a /// project that just wants its binary kept running does not carry 150 /// lines of init-system detection to keep in step with everyone /// else's copy. - Managed(Command), + Managed(ByMode), +} + +impl Service { + /// The command as written, before a mode is resolved. Public for the + /// declaration check, which asks about the writing rather than the + /// result. + pub fn by_mode(&self) -> &ByMode { + match self { + Self::Script(command) | Self::Managed(command) => command, + } + } + + /// The command for `mode`, keeping the variant -- which decides + /// whether the built-in script wraps it, and is not something a mode + /// changes. + pub fn command(&self, mode: Option<&str>) -> &Command { + self.by_mode().get(mode) + } } /// Where a project's own resources are declared. @@ -231,8 +258,13 @@ pub enum Component { /// An APK to install on the phone. Apk { name: String, - #[serde(default, skip_serializing_if = "Command::is_empty")] - build: Command, + /// The ways this component can be built, in order, the first the + /// default. See [`ByMode`] for what a mode is and how a command + /// learns which one it is being run in. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + modes: Vec, + #[serde(default, skip_serializing_if = "ByMode::is_empty")] + build: ByMode, #[serde(default, skip_serializing_if = "Option::is_none")] cwd: Option, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -249,6 +281,36 @@ pub enum Component { /// a project built without dev-updater has no reason to strip. #[serde(default, skip_serializing_if = "std::ops::Not::not")] strip: bool, + /// A command whose single line of stdout is a URL for the phone + /// to open once this app is installed. + /// + /// What that URL *means* is the project's business and none of + /// this server's: ai-app mints a one-shot enrolment link with it, + /// and anything else that needs the phone to be handed a link + /// after an install fits the same shape. Which is why it is a URL + /// rather than anything named after enrolment -- a route that + /// knew what enrolling was would be a special case of itself. + /// + /// Run **per press**, never cached: a link that carries a + /// credential is minted fresh each time, and one captured at + /// build time would be a stale secret sitting in a config file. + /// + /// Part of the acceptance gate, because it runs on the build + /// machine exactly as `build` does. + #[serde(default, skip_serializing_if = "Command::is_empty")] + enroll: Command, + /// This machine's answer instead, when somebody gave one from the + /// phone. `None` -- the ordinary case -- follows whatever the + /// project declares above, so a project that changes its mind + /// still takes effect on every machine that never overrode it. + /// + /// An override rather than copying the declared value in at + /// acceptance time: copied, a later change to the declaration + /// would be ignored on every machine silently, which is the + /// divergence this file has already been bitten by. Chosen, not + /// measured -- see [`Self::same_declaration`]. + #[serde(default, skip_serializing_if = "Option::is_none")] + strip_here: Option, /// The installed package this replaces, read out of the APK by /// `crate::apkinfo` and cached here so `aapt2` never runs on the /// manifest path. Measured, not declared: absent until there has @@ -269,14 +331,42 @@ pub enum Component { /// has built it once, which reads as *unknown* rather than as /// stale -- a project built by hand before this existed must not /// announce that it needs rebuilding. + /// Which of the modes `build` declares this machine builds in. + /// + /// Chosen here rather than declared, and chosen here rather than + /// on the phone: there is one checkout and one set of build + /// outputs, so two enrolled devices holding different answers + /// would rebuild over each other with nothing on either screen to + /// say why. `None` means the first declared mode, which is why a + /// project puts the one it wants by default first. + /// + /// May name a mode that no longer exists -- a pull rewrites the + /// declaration and nothing rewrites this -- which [`ByMode::get`] + /// resolves by falling back rather than failing. + #[serde(default, skip_serializing_if = "Option::is_none")] + mode: Option, + /// The mode the recorded build was made in, beside the commit it + /// was made from. + /// + /// Measured, like `built_from`, and needed for the same reason it + /// is: switching modes moves no commit, so without this a + /// component built in `debug` and switched to `release` reads as + /// current, offers nothing to press, and serves the debug build + /// for ever. The card would look right the whole time, which is + /// the expensive kind of wrong. + #[serde(default, skip_serializing_if = "Option::is_none")] + built_mode: Option, #[serde(default, skip_serializing_if = "Option::is_none")] built_from: Option, }, /// A server on the build machine, delivered by restarting it. Server { name: String, - #[serde(default, skip_serializing_if = "Command::is_empty")] - build: Command, + /// The ways this component can be built. See [`ByMode`]. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + modes: Vec, + #[serde(default, skip_serializing_if = "ByMode::is_empty")] + build: ByMode, #[serde(default, skip_serializing_if = "Option::is_none")] cwd: Option, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -296,11 +386,44 @@ pub enum Component { /// has built it once, which reads as *unknown* rather than as /// stale -- a project built by hand before this existed must not /// announce that it needs rebuilding. + /// Which of the modes `build` declares this machine builds in. + /// + /// Chosen here rather than declared, and chosen here rather than + /// on the phone: there is one checkout and one set of build + /// outputs, so two enrolled devices holding different answers + /// would rebuild over each other with nothing on either screen to + /// say why. `None` means the first declared mode, which is why a + /// project puts the one it wants by default first. + /// + /// May name a mode that no longer exists -- a pull rewrites the + /// declaration and nothing rewrites this -- which [`ByMode::get`] + /// resolves by falling back rather than failing. + #[serde(default, skip_serializing_if = "Option::is_none")] + mode: Option, + /// The mode the recorded build was made in, beside the commit it + /// was made from. + /// + /// Measured, like `built_from`, and needed for the same reason it + /// is: switching modes moves no commit, so without this a + /// component built in `debug` and switched to `release` reads as + /// current, offers nothing to press, and serves the debug build + /// for ever. The card would look right the whole time, which is + /// the expensive kind of wrong. + #[serde(default, skip_serializing_if = "Option::is_none")] + built_mode: Option, #[serde(default, skip_serializing_if = "Option::is_none")] built_from: Option, }, } +/// What this machine decided about one component, as opposed to what the +/// project asked for. See [`Component::choices`]. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct Choices { + pub mode: Option, + pub strip: Option, +} + impl Component { pub fn name(&self) -> &str { match self { @@ -308,16 +431,118 @@ impl Component { } } - pub fn build(&self) -> &Command { + /// What this component's `build` says as written, before a mode is + /// resolved -- which the declaration check needs, since it is asking + /// about the writing rather than the result. + pub fn build_declaration(&self) -> &ByMode { + self.build_modes() + } + + /// What this component's `build` says, whichever mode it may say it + /// in. + fn build_modes(&self) -> &ByMode { match self { Self::Apk { build, .. } | Self::Server { build, .. } => build, } } - /// 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. + /// Every mode this component offers, in declaration order, with the + /// first the default. Empty means one way of building, which is not + /// a choice and gets no picker. + /// + /// One list per component, declared in one place, rather than + /// gathered from whichever fields happened to name modes. That is + /// what makes "this part has a mode the other part has not heard of" + /// unsayable rather than a thing to detect: every per-mode map is + /// checked against *this*, so a missing entry is named as missing + /// instead of silently resolving to some other mode's command. + pub fn modes(&self) -> &[String] { + match self { + Self::Apk { modes, .. } | Self::Server { modes, .. } => modes, + } + } + + /// The mode this machine chose, exactly as stored -- which may name + /// one the checkout no longer declares. [`Self::effective_mode`] is + /// what to ask when the answer has to be a mode that exists. + pub fn chosen_mode(&self) -> Option<&str> { + match self { + Self::Apk { mode, .. } | Self::Server { mode, .. } => mode.as_deref(), + } + } + + /// The mode this component is actually built and run in: the one + /// chosen here if it is still declared, else the first declared, else + /// none at all for a component that offers no choice. + /// + /// One definition, because three things have to agree about it -- + /// which command runs, which service command the unit gets, and what + /// the card says the mode is. Two of them disagreeing is a build in + /// one mode reported as the other. + pub fn effective_mode(&self) -> Option<&str> { + let modes = self.modes(); + match self.chosen_mode() { + // A choice that survived the pull that could have removed it. + Some(chosen) if modes.iter().any(|name| name == chosen) => Some(chosen), + // Nobody chose, or what they chose is gone: the first + // declared, which is what the declaration order is for. + _ => modes.first().map(String::as_str), + } + } + + /// The extra arguments this component's build command needs in order + /// to know which mode it is being run in. + /// + /// The mode name, for a command written once -- which is how a + /// project with a script that takes `release` or `debug` declares it, + /// and it is why nothing has to be written twice in the ordinary + /// case. Nothing at all for a command written per mode: that command + /// *is* the answer, and appending the word as well would hand a + /// stray argument to a binary that never asked for one. + pub fn build_mode_argument(&self) -> Option<&str> { + self.build_modes().mode_argument(self.effective_mode()) + } + + /// The command that builds this component, in the mode it is set to. + /// + /// Every caller of this is unchanged from before modes existed: a + /// component that declares one command still answers with it, and one + /// that declares several answers with the selected one. Which is the + /// point of resolving it here rather than handing a mode down through + /// the build, the staleness check and the service in three separate + /// arguments that could each be passed the wrong thing. + pub fn build(&self) -> &Command { + self.build_modes().get(self.effective_mode()) + } + + /// Whether to serve a stripped copy of this component's build: what + /// this machine chose, or what the project declared when nobody here + /// has said otherwise. 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 { + match self { + Self::Apk { + strip, strip_here, .. + } => strip_here.unwrap_or(*strip), + Self::Server { .. } => false, + } + } + + /// The command that prints a URL for the phone to open after + /// installing this component, if the project declares one. Only an + /// `Apk` can: it is about the app that was just installed. + pub fn enroll(&self) -> Option<&Command> { + match self { + Self::Apk { enroll, .. } if !enroll.is_empty() => Some(enroll), + Self::Apk { .. } | Self::Server { .. } => None, + } + } + + /// What the *project* asks for, as opposed to what this machine + /// settled on. Shown beside the switch so a machine that differs from + /// the checkout says so rather than quietly disagreeing with it. + pub fn strip_declared(&self) -> bool { matches!(self, Self::Apk { strip: true, .. }) } @@ -382,7 +607,14 @@ impl Component { } /// Whether these two say the same thing about what to *do*, ignoring - /// what this server measured for itself. + /// what this server measured and what this machine chose. + /// + /// Both kinds of exclusion are the same rule from two directions: the + /// gate is about what a *project* asked for, so a package read out of + /// a build and a mode picked on the phone are equally not it. A + /// chosen mode in particular must stay out -- it names one of the + /// declared modes, so including it would make choosing one look like + /// the declaration changing and ask for the acceptance again. /// /// This is the acceptance comparison: a project's file against what was /// accepted from it, so a pull that changes a command shows up as @@ -398,53 +630,76 @@ impl Component { ( Self::Apk { name, + modes, build, cwd, stale_when, also_watch, strip, + enroll, + strip_here: _, + mode: _, package: _, built_from: _, + built_mode: _, }, Self::Apk { name: other_name, + modes: other_modes, build: other_build, cwd: other_cwd, stale_when: other_stale_when, also_watch: other_also_watch, strip: other_strip, + enroll: other_enroll, + strip_here: _, + mode: _, package: _, built_from: _, + built_mode: _, }, ) => { name == other_name + && modes == other_modes && build == other_build && cwd == other_cwd && stale_when == other_stale_when && also_watch == other_also_watch && strip == other_strip + // In the gate because it runs on the build machine, + // which is the whole of what the gate is about. A + // pull must not be able to introduce a command the + // Enroll button then runs unasked. + && enroll == other_enroll } ( Self::Server { name, + modes, build, cwd, stale_when, also_watch, service, + mode: _, built_from: _, + built_mode: _, }, Self::Server { name: other_name, + modes: other_modes, build: other_build, cwd: other_cwd, stale_when: other_stale_when, also_watch: other_also_watch, service: other_service, + mode: _, built_from: _, + built_mode: _, }, ) => { name == other_name + && modes == other_modes && build == other_build && cwd == other_cwd && stale_when == other_stale_when @@ -462,6 +717,13 @@ impl Component { /// something this machine runs by hand, and its card offers no service /// controls. pub fn service(&self) -> Option<&Service> { + self.declared_service() + } + + /// The `service:` as written, without resolving a mode. Separate from + /// [`Self::service`] only because [`Self::modes`] needs it while + /// computing the very mode a resolution would use. + fn declared_service(&self) -> Option<&Service> { match self { Self::Server { service, .. } => service.as_ref(), Self::Apk { .. } => None, @@ -485,6 +747,28 @@ impl Component { } } + /// The mode the recorded build was made in. `None` both for a + /// component never built here and for one built before modes existed + /// -- the same thing to every caller, since neither is evidence that + /// what is on disk is a build of the mode now selected. + pub fn built_mode(&self) -> Option<&str> { + match self { + Self::Apk { built_mode, .. } | Self::Server { built_mode, .. } => built_mode.as_deref(), + } + } + + /// Whether what was last built here was built some other way than + /// this component is set to build now. + /// + /// Only ever true once this server has built it: with no recorded + /// commit there is no build of ours to be about, and announcing that + /// a project built by hand needs rebuilding is the false staleness + /// `built_from` is careful to avoid. The two are read together for + /// that reason. + pub fn built_in_another_mode(&self) -> bool { + self.built_from().is_some() && self.built_mode() != self.effective_mode() + } + /// Records the commit a successful build was made from. Unlike /// [`Self::set_package`] this applies to either kind, because either /// kind can fall behind the checkout. @@ -496,6 +780,60 @@ impl Component { } } + /// Records which mode a successful build was made in, alongside the + /// commit. Written from the component's own resolved mode rather than + /// from anything the caller worked out, so the record cannot describe + /// a different build from the one that ran. + pub fn set_built_mode(&mut self) { + let built = self.effective_mode().map(str::to_string); + match self { + Self::Apk { built_mode, .. } | Self::Server { built_mode, .. } => *built_mode = built, + } + } + + /// Records what somebody chose on this machine: which mode to build + /// in, and whether to strip. `None` for either leaves that one alone. + /// + /// Both at once because they arrive together -- the phone has just + /// shown a person every setting a component has, so what comes back + /// is the whole answer. + pub fn choose(&mut self, chosen_mode: Option, chosen_strip: Option) { + match self { + Self::Apk { + mode, strip_here, .. + } => { + *mode = chosen_mode; + *strip_here = chosen_strip; + } + // A server has nothing to strip, so a `strip` arriving for + // one is dropped rather than stored somewhere nothing reads. + Self::Server { mode, .. } => *mode = chosen_mode, + } + } + + /// Everything this machine decided about this component, for carrying + /// across a rewrite of the components list. + /// + /// Acceptance and the self entry's startup reconciliation both + /// replace `components` wholesale with what the checkout declares, + /// which is right for everything a project asked for and wrong for + /// everything somebody chose here. Returned as one value so a field + /// added to the set is carried by both callers or by neither. + pub fn choices(&self) -> Choices { + Choices { + mode: self.chosen_mode().map(str::to_string), + strip: match self { + Self::Apk { strip_here, .. } => *strip_here, + Self::Server { .. } => None, + }, + } + } + + /// Puts back what [`Self::choices`] took. + pub fn restore(&mut self, choices: Choices) { + self.choose(choices.mode, choices.strip); + } + /// Records what a build was measured to produce, leaving everything a /// project declared alone. The inverse of [`Self::same_declaration`]'s /// exclusion: exactly the fields that comparison ignores are the ones @@ -604,6 +942,12 @@ impl Command { Self(words) } + /// The words, for a caller that has one more to add. Cloned rather + /// than borrowed because every caller is building a new command. + pub fn to_words(&self) -> Vec { + self.0.clone() + } + /// The program and its arguments, or `None` for an empty command -- /// which is a configuration that says "no build step", not an error. pub fn split_first(&self) -> Option<(&String, &[String])> { @@ -649,6 +993,172 @@ impl Serialize for Command { } } +/// A command that may differ per **build mode**. +/// +/// A mode is a way of building the same component -- `debug` and +/// `release` are the ones every toolchain has, but the names are the +/// project's, because what a second way of building means is the +/// project's business and not this server's. Which one to use is chosen +/// on the build machine and stored beside the component; see +/// [`Component::chosen_mode`]. +/// +/// Two forms rather than an optional list, because a component either +/// offers a choice or does not: +/// +/// ```text +/// build: "cargo build" // one way, no name +/// build: { // two, named +/// "release": "cargo build --release", +/// "debug": "cargo build", +/// } +/// ``` +/// +/// The single form is what every project wrote before modes existed and +/// still means exactly what it did, which is the point of accepting both +/// here rather than making everyone grow a mode called something. +/// +/// **Order is the declaration's.** The first mode is the default, so a +/// project puts the one it wants built by default first -- ordinarily +/// `release`. A map preserves it because this stores pairs rather than a +/// `HashMap`; RON hands them over in file order and they are written back +/// in the same. +/// +/// Only [`Command`] goes in here, and that is load-bearing rather than +/// incidental: telling the map form apart from the single form is done +/// with `deserialize_any`, and RON reports a struct (`(path: "a")`) as a +/// *map* -- so a struct-valued field behind this would read its own +/// fields as mode names. `Command` is a string or a sequence, neither of +/// which collides. A field that wants modes and is not a command puts +/// this inside itself instead, which is what [`Service`] does. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ByMode { + /// One command, whatever the mode. + One(Command), + /// One per named mode, in declaration order. + Modes(Vec<(String, Command)>), +} + +/// What [`ByMode::get`] answers for a mode a component does not declare +/// and has no default for -- the same as declaring nothing, which every +/// caller already handles as "no build step". +static NO_COMMAND: Command = Command(Vec::new()); + +impl Default for ByMode { + fn default() -> Self { + Self::One(Command::default()) + } +} + +impl ByMode { + /// The mode names, in declaration order. Empty for a single command: + /// one way of building is not a choice, and a picker offering one + /// entry is a control that cannot do anything. + pub fn modes(&self) -> impl Iterator { + let named: &[(String, Command)] = match self { + Self::One(_) => &[], + Self::Modes(modes) => modes, + }; + named.iter().map(|(name, _)| name.as_str()) + } + + /// The command for `mode`, falling back to the first declared one. + /// + /// Total on purpose. A mode is chosen on this machine and the modes + /// are declared in the checkout, so a pull can rename or remove the + /// one that was chosen -- and this is read on the manifest path, + /// where the alternative to falling back is a card that cannot be + /// drawn. The fallback is the same answer as before anybody chose, + /// which is what a phone's stale variant choice already does. + pub fn get(&self, mode: Option<&str>) -> &Command { + match self { + Self::One(command) => command, + Self::Modes(modes) => mode + .and_then(|mode| modes.iter().find(|(name, _)| name == mode)) + .or_else(|| modes.first()) + .map(|(_, command)| command) + .unwrap_or(&NO_COMMAND), + } + } + + /// The mode name, when this command has to be *told* which mode it + /// is being run in, and nothing when it does not. + /// + /// The rule in one line: a command written per mode already says + /// which mode it is by being the one that was chosen, so it is not + /// told again. Written once, it is told, which is what lets a project + /// with a script that takes `release` or `debug` name that script a + /// single time. + pub fn mode_argument<'a>(&self, mode: Option<&'a str>) -> Option<&'a str> { + match self { + Self::One(_) => mode, + Self::Modes(_) => None, + } + } + + /// Nothing to run in any mode. An empty map counts, since a project + /// that wrote `{}` declared no way of building this either. + pub fn is_empty(&self) -> bool { + match self { + Self::One(command) => command.is_empty(), + Self::Modes(modes) => modes.iter().all(|(_, command)| command.is_empty()), + } + } +} + +impl<'de> Deserialize<'de> for ByMode { + fn deserialize>(deserializer: D) -> Result { + use serde::de; + + struct Either; + impl<'de> de::Visitor<'de> for Either { + type Value = ByMode; + fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + f.write_str(r#"a command, or {"mode": "command"} for one per mode"#) + } + // The two single-command forms, forwarded rather than + // reimplemented so there is still one definition of where a + // command's argument boundaries are. + fn visit_str(self, line: &str) -> Result { + Ok(ByMode::One(Command::from_line(line))) + } + fn visit_seq>(self, seq: A) -> Result { + Command::deserialize(de::value::SeqAccessDeserializer::new(seq)).map(ByMode::One) + } + fn visit_map>(self, mut map: A) -> Result { + let mut modes = Vec::new(); + while let Some((name, command)) = map.next_entry::()? { + if modes.iter().any(|(seen, _)| seen == &name) { + return Err(de::Error::custom(format!( + "the mode {name} is declared twice" + ))); + } + modes.push((name, command)); + } + Ok(ByMode::Modes(modes)) + } + } + deserializer.deserialize_any(Either) + } +} + +impl Serialize for ByMode { + fn serialize(&self, serializer: S) -> Result { + match self { + // Written back as the bare command, so a component that + // declares no modes round-trips to the file it came from. + Self::One(command) => command.serialize(serializer), + Self::Modes(modes) => { + use serde::ser::SerializeMap; + let mut map = serializer.serialize_map(Some(modes.len()))?; + for (name, command) in modes { + map.serialize_entry(name, command)?; + } + map.end() + } + } + } +} + /// "`path` is out of date with respect to `older_than`" -- the whole /// staleness vocabulary, deliberately. /// @@ -773,6 +1283,78 @@ pub struct ProjectFile { pub error: Option, } +/// Why this component's modes do not add up, if they do not. +/// +/// With one declared list per component there is only one thing left to +/// get wrong: a field written *per mode* naming a different set from the +/// list. Checked rather than resolved, because there is no right guess -- +/// [`ByMode::get`] answers an unknown name by falling back to the first, +/// which for a server means building `release` and starting `debug` with +/// nothing anywhere saying so. +/// +/// A per-mode map on a component that declares no modes is the same +/// mistake seen from the other side, and is refused with the same +/// sentence: the list is where modes are declared, and a map is only ever +/// a way of answering for the modes already there. +fn mode_problem(component: &Component) -> Option { + let declared = component.modes(); + let mut fields = vec![("build", component.build_declaration())]; + if let Some(service) = component.service() { + fields.push(("service", service.by_mode())); + } + for (field, by_mode) in fields { + let named: Vec<&str> = by_mode.modes().collect(); + if named.is_empty() { + // Written once, which covers every mode -- including none. + continue; + } + if declared.is_empty() { + return Some(format!( + "the component {}'s {field} is written per mode ({}), but the component declares \ + no modes -- add `modes: [{}]` beside it, which is the one place a mode is \ + declared", + component.name(), + named.join(", "), + named + .iter() + .map(|name| format!("\"{name}\"")) + .collect::>() + .join(", "), + )); + } + let missing: Vec<&str> = declared + .iter() + .map(String::as_str) + .filter(|mode| !named.contains(mode)) + .collect(); + let extra: Vec<&str> = named + .iter() + .copied() + .filter(|mode| !declared.iter().any(|declared| declared == mode)) + .collect(); + if !missing.is_empty() || !extra.is_empty() { + return Some(format!( + "the component {}'s {field} answers for [{}] but the component declares modes \ + [{}]: {} has no {field}, and {} is not a mode", + component.name(), + named.join(", "), + declared.join(", "), + list_or_nothing(&missing), + list_or_nothing(&extra), + )); + } + } + None +} + +fn list_or_nothing(names: &[&str]) -> String { + if names.is_empty() { + "nothing".to_string() + } else { + names.join(", ") + } +} + /// What `project` says about itself, or an empty declaration for a project /// that says nothing -- which is the normal case. /// @@ -790,10 +1372,24 @@ pub fn project_config(project: &Path) -> ProjectFile { let Ok(text) = std::fs::read_to_string(&path) else { return ProjectFile::default(); }; - match wg_app_link::format::parse(&text) { - Ok(declaration) => ProjectFile { - declaration, - error: None, + match wg_app_link::format::parse::(&text) { + // Parsing is not the whole of being readable. A mode set that + // does not add up is discarded exactly as an unknown field is, + // and for the same reason: what would otherwise happen is that + // the project builds in one mode and runs another, from a card + // that looks entirely ordinary. + Ok(declaration) => match declaration.components.iter().find_map(mode_problem) { + None => ProjectFile { + declaration, + error: None, + }, + Some(problem) => { + tracing::warn!("ignoring {}: {problem}", path.display()); + ProjectFile { + declaration: Declaration::default(), + error: Some(format!("{}: {problem}", path.display())), + } + } }, Err(err) => { tracing::warn!("ignoring {}: {err}", path.display()); @@ -945,11 +1541,13 @@ mod tests { for (text, expected) in [ ( r#"service: Script("server/service")"#, - Service::Script(Command::from_line("server/service")), + Service::Script(ByMode::One(Command::from_line("server/service"))), ), ( r#"service: Managed("target/release/ai-server --port 8080")"#, - Service::Managed(Command::from_line("target/release/ai-server --port 8080")), + Service::Managed(ByMode::One(Command::from_line( + "target/release/ai-server --port 8080", + ))), ), ] { let parsed = component(&format!(r#"Server(name: "s", build: "b", {text})"#)) @@ -960,6 +1558,207 @@ mod tests { } } + /// The form every project wrote before modes existed still means + /// what it did, and still comes back out of a render unchanged. This + /// is the whole reason `build:` accepts two shapes rather than every + /// project growing a mode called something. + #[test] + fn one_command_is_still_one_command() { + let parsed = component(r#"Apk(name: "app", build: "./build.sh")"#).expect("parse"); + assert!( + parsed.modes().is_empty(), + "one way of building is no choice" + ); + assert_eq!(parsed.effective_mode(), None); + assert_eq!(parsed.build().to_line(), "./build.sh"); + let written = wg_app_link::format::render(&parsed).expect("render"); + assert!( + written.contains(r#"build: "./build.sh""#), + "the bare command did not survive: {written}" + ); + } + + /// Order is the declaration's, and the first is what gets built when + /// nobody has chosen -- which is why a project puts `release` first. + #[test] + fn the_first_declared_mode_is_the_default() { + let parsed = component( + r#"Apk(name: "app", modes: ["release", "debug"], build: { + "release": "./build.sh release", + "debug": "./build.sh debug", + })"#, + ) + .expect("parse"); + assert_eq!(parsed.modes(), vec!["release", "debug"]); + assert_eq!(parsed.effective_mode(), Some("release")); + assert_eq!(parsed.build().to_line(), "./build.sh release"); + } + + /// Choosing one picks its command, and choosing one that a pull has + /// since removed falls back to the first rather than failing -- this + /// is read on the manifest path, where the alternative to a fallback + /// is a card that cannot be drawn at all. + #[test] + fn a_chosen_mode_picks_its_command_and_a_stale_one_falls_back() { + let mut parsed = + component(r#"Apk(name: "app", modes: ["release", "debug"], build: {"release": "r", "debug": "d"})"#).expect("parse"); + parsed.choose(Some("debug".to_string()), None); + assert_eq!(parsed.effective_mode(), Some("debug")); + assert_eq!(parsed.build().to_line(), "d"); + + parsed.choose(Some("profile".to_string()), None); + assert_eq!(parsed.effective_mode(), Some("release")); + assert_eq!(parsed.build().to_line(), "r"); + } + + /// A mode map round-trips as a map, so a config written back by this + /// server is still the file it was read from. The two halves only + /// round-trip together, which is what this is here to hold. + #[test] + fn a_mode_map_is_written_back_as_a_map() { + let text = r#"Apk(name: "app", modes: ["release", "debug"], build: {"release": "r", "debug": "d"})"#; + let parsed = component(text).expect("parse"); + let written = wg_app_link::format::render(&parsed).expect("render"); + let again = component(&written).expect("re-parse what was written"); + assert_eq!(parsed, again, "did not survive a round trip: {written}"); + } + + /// Picking a mode is not the project asking for something new, so it + /// must not re-open the acceptance gate -- a card that asked to be + /// accepted again every time somebody changed a setting would teach + /// people to press it without reading. + #[test] + fn choosing_a_mode_or_strip_is_not_a_changed_declaration() { + let declared = + component(r#"Apk(name: "app", modes: ["release", "debug"], build: {"release": "r", "debug": "d"}, strip: true)"#) + .expect("parse"); + let mut chosen = declared.clone(); + chosen.choose(Some("debug".to_string()), Some(false)); + assert!(declared.same_declaration(&chosen)); + assert!(chosen.same_declaration(&declared)); + // ...while a changed command still is one. + let other = + component(r#"Apk(name: "app", modes: ["release", "debug"], build: {"release": "x", "debug": "d"})"#).expect("parse"); + assert!(!declared.same_declaration(&other)); + } + + /// This machine's answer wins where it has one, and the project's + /// stands where it does not -- so a project that changes its mind + /// still reaches every machine that never overrode it. + #[test] + fn strip_follows_the_declaration_until_this_machine_says_otherwise() { + let mut parsed = component(r#"Apk(name: "app", build: "b", strip: true)"#).expect("parse"); + assert!(parsed.strip()); + assert!(parsed.strip_declared()); + parsed.choose(None, Some(false)); + assert!(!parsed.strip()); + assert!(parsed.strip_declared(), "the project still asks for it"); + } + + /// A server names its modes on `service:` alone when both modes build + /// the same way, and the union is what the component offers. + #[test] + fn a_service_may_be_where_the_modes_are_named() { + let parsed = component( + r#"Server(name: "s", modes: ["release", "debug"], build: "cargo build --all", service: Managed({ + "release": "target/release/s", + "debug": "target/debug/s", + }))"#, + ) + .expect("parse"); + assert_eq!(parsed.modes(), vec!["release", "debug"]); + assert_eq!(parsed.build().to_line(), "cargo build --all"); + } + + /// With one declared list per component there is one thing left to + /// get wrong: a field written per mode that answers for a different + /// set. Refused with both sets named, because resolving it would + /// build one mode and run another with nothing on the card saying so. + #[test] + fn a_per_mode_field_must_answer_for_the_modes_that_were_declared() { + let missing = component( + r#"Server(name: "s", modes: ["release", "debug"], build: "b", service: Managed({ + "release": "target/release/s", + }))"#, + ) + .expect("parse"); + let problem = mode_problem(&missing).expect("this should not add up"); + assert!(problem.contains("debug has no service"), "{problem}"); + + let unknown = component( + r#"Server(name: "s", modes: ["release"], build: {"release": "r", "quick": "q"})"#, + ) + .expect("parse"); + let problem = mode_problem(&unknown).expect("quick is not a declared mode"); + assert!(problem.contains("quick is not a mode"), "{problem}"); + + // The list is the one place modes are declared, so a map without + // one is the same mistake from the other side -- and the refusal + // says what to write rather than only what is wrong. + let undeclared = + component(r#"Apk(name: "app", build: {"release": "r", "debug": "d"})"#).expect("parse"); + let problem = mode_problem(&undeclared).expect("no modes are declared"); + assert!( + problem.contains(r#"modes: ["release", "debug"]"#), + "{problem}" + ); + + // Answering for exactly the declared set says nothing, and so + // does a command written once -- which covers every mode. + let fine = component( + r#"Server(name: "s", modes: ["release", "debug"], build: "./build.sh", service: Managed({ + "debug": "target/debug/s", "release": "target/release/s", + }))"#, + ) + .expect("parse"); + assert_eq!(mode_problem(&fine), None); + } + + /// A command written once is handed the mode; a command written per + /// mode is not handed it again. Getting this the other way round + /// passes a stray word to a binary that never asked for one, which + /// for a service is a process that will not start. + #[test] + fn only_a_command_written_once_is_told_which_mode_it_is() { + let script = + component(r#"Apk(name: "app", modes: ["release", "debug"], build: "./build.sh")"#) + .expect("parse"); + assert_eq!(script.build().to_line(), "./build.sh"); + assert_eq!(script.build_mode_argument(), Some("release")); + + let per_mode = component( + r#"Apk(name: "app", modes: ["release", "debug"], build: {"release": "r", "debug": "d"})"#, + ) + .expect("parse"); + assert_eq!(per_mode.build().to_line(), "r"); + assert_eq!(per_mode.build_mode_argument(), None); + + // And a component with no modes at all is told nothing, which is + // every project that existed before modes did. + let plain = component(r#"Apk(name: "app", build: "./build.sh")"#).expect("parse"); + assert_eq!(plain.build_mode_argument(), None); + } + + /// Switching modes moves no commit, so nothing about the checkout can + /// say that what is on disk is a build of something else. This is the + /// only thing that can, and it stays quiet for a component this + /// server has never built -- a project built by hand must not be told + /// it is out of date. + #[test] + fn a_build_from_another_mode_is_not_this_mode_s_build() { + let mut parsed = + component(r#"Apk(name: "app", modes: ["release", "debug"], build: {"release": "r", "debug": "d"})"#).expect("parse"); + assert!( + !parsed.built_in_another_mode(), + "never built here, so there is no build of ours to be about" + ); + parsed.set_built_from("abc123".to_string()); + parsed.set_built_mode(); + assert!(!parsed.built_in_another_mode()); + parsed.choose(Some("debug".to_string()), None); + assert!(parsed.built_in_another_mode()); + } + /// A server that declares no service at all is a real state -- it /// builds something a person runs by hand -- and must not become an /// error or a service with an empty command. diff --git a/server/src/main.rs b/server/src/main.rs index 58081a3..e8e8b48 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -41,6 +41,7 @@ mod registry; mod resources; mod restart; mod routes; +mod script; mod sdk; mod service; mod shipped; diff --git a/server/src/registry.rs b/server/src/registry.rs index b77fa8f..7cf197f 100644 --- a/server/src/registry.rs +++ b/server/src/registry.rs @@ -304,6 +304,39 @@ fn component_id(component: &Component) -> ComponentId { ) } +/// What this machine chose about each component, for carrying across a +/// rewrite of the components list. +/// +/// The mirror of [`measured_packages`], and here for the same reason: +/// both acceptance and the self entry's startup reconciliation replace +/// `components` wholesale with what the checkout declares, which is right +/// for everything the project asked for and wrong for everything somebody +/// chose here. Without it, choosing to build this server in `debug` +/// would last exactly until the next restart -- and restarting is how +/// this server is updated, so it would never last at all. +/// +/// Keyed the same way for the same reason: once a component's directory +/// decides which builds are its own, a reused name is a different +/// component, and handing it the old one's mode would build something +/// nobody asked for. +fn chosen_settings(components: &[Component]) -> HashMap { + components + .iter() + .map(|component| (component_id(component), component.choices())) + .collect() +} + +fn restore_chosen_settings( + components: &mut [Component], + chosen: &HashMap, +) { + for component in components { + if let Some(choices) = chosen.get(&component_id(component)) { + component.restore(choices.clone()); + } + } +} + fn measured_packages(components: &[Component]) -> HashMap { components .iter() @@ -492,6 +525,11 @@ impl AppState { .find(|candidate| candidate.name() == component) { built.set_built_from(sha); + // Beside the commit and in the same write: the mode is + // half of "what is on disk", and a record of one without + // the other is a component that looks current in a mode + // it was never built in. + built.set_built_mode(); } Ok(()) }); @@ -604,13 +642,18 @@ impl AppState { // way to start running its commands. components: vec![Component::Apk { name: "app".to_string(), - build: crate::config::Command::default(), + modes: Vec::new(), + build: crate::config::ByMode::default(), cwd: None, stale_when: None, also_watch: Vec::new(), strip: false, + enroll: crate::config::Command::default(), + strip_here: None, + mode: None, package, built_from: None, + built_mode: None, }], }); Ok(key) @@ -661,6 +704,46 @@ impl AppState { }) } + /// Records what somebody chose for one component from the phone: + /// which declared mode to build it in, and whether to strip it. + /// + /// A whole-component write rather than a field at a time, because the + /// phone has just shown a person every setting the component has and + /// what comes back is the complete answer. + /// + /// The mode is checked against what the component actually declares. + /// An unknown one is refused rather than stored: `ByMode::get` would + /// fall back to the first mode, so storing it would leave the card + /// naming a mode nothing builds in -- which reads exactly like a + /// setting that took effect. + pub fn set_component_choices( + &self, + key: &str, + component: &str, + mode: Option, + strip: Option, + ) -> Result<()> { + self.update(|config| { + let project = config + .projects + .iter_mut() + .find(|project| project.key == key) + .with_context(|| format!("no app named {key}"))?; + let target = project + .components + .iter_mut() + .find(|candidate| candidate.name() == component) + .with_context(|| format!("{key} has no component named {component}"))?; + if let Some(mode) = &mode + && !target.modes().iter().any(|name| name == mode) + { + bail!("{component} has no build mode called {mode}"); + } + target.choose(mode, strip); + Ok(()) + }) + } + /// Accepts what this project asks to have run, after a person has read /// it on the phone. From here on those components are ordinary /// configuration, indistinguishable from hand-written ones. @@ -695,6 +778,7 @@ impl AppState { .find(|project| project.key == key) .with_context(|| format!("{key} is built in and has nothing to accept"))?; let measured = measured_packages(&project.components); + let chosen = chosen_settings(&project.components); project.git_pull = declared.git_pull; // Everything `matches_accepted` compares has to be written // here, or accepting cannot clear the gate. `resources` joined @@ -710,6 +794,12 @@ impl AppState { component.set_package(package.clone()); } } + // Accepting is about what the project asks to have run. It is + // not somebody withdrawing the mode they picked or the strip + // they turned off, so those survive it -- exactly as the + // measured package does, and for the same reason: neither is + // part of what was being agreed to. + restore_chosen_settings(&mut project.components, &chosen); Ok(()) }) } @@ -775,13 +865,18 @@ fn reconcile_self(config: &mut Config, self_project: &Path) -> bool { None => { components.push(Component::Apk { name: "app".to_string(), - build: crate::config::Command::default(), + modes: Vec::new(), + build: crate::config::ByMode::default(), cwd: None, stale_when: None, also_watch: Vec::new(), strip: false, + enroll: crate::config::Command::default(), + strip_here: None, + mode: None, package: None, built_from: None, + built_mode: None, }); "app".to_string() } @@ -791,6 +886,14 @@ fn reconcile_self(config: &mut Config, self_project: &Path) -> bool { .projects .iter_mut() .find(|project| project.key == SELF_KEY); + // Carried across the row being rebuilt, like `gitIpv4` below: what is + // *derived* here is what the checkout declares, and a mode chosen for + // this server's own component is not that. Restarting is how this + // server takes an update, so a choice that did not survive a restart + // would not survive being acted on. + if let Some(existing) = existing.as_ref() { + restore_chosen_settings(&mut components, &chosen_settings(&existing.components)); + } let derived = ProjectConfig { key: SELF_KEY.to_string(), label: declared.label.unwrap_or_else(|| SELF_LABEL.to_string()), @@ -946,13 +1049,18 @@ mod tests { fn asking_for(command: &str) -> Vec { vec![Component::Apk { name: "app".to_string(), - build: crate::config::Command::from_line(command), + modes: Vec::new(), + build: crate::config::ByMode::One(crate::config::Command::from_line(command)), cwd: None, stale_when: None, also_watch: Vec::new(), strip: false, + enroll: crate::config::Command::default(), + strip_here: None, package: None, + mode: None, built_from: None, + built_mode: None, }] } @@ -964,6 +1072,16 @@ mod tests { .expect("write"); } + /// A component's declaration in the project file, with modes. + fn write_moded_request(project: &Path) { + std::fs::write( + project.join(crate::config::PROJECT_CONFIG_FILE), + "components: [Apk(name: \"app\", modes: [\"release\", \"debug\"], \ + build: {\"release\": \"r\", \"debug\": \"d\"})],\n", + ) + .expect("write"); + } + /// A live `AppState` holding one configured app pointed at `project`, /// with nothing accepted yet. /// @@ -1027,13 +1145,18 @@ mod tests { fn apk_named(name: &str, cwd: Option<&str>) -> Component { Component::Apk { name: name.to_string(), - build: crate::config::Command::default(), + modes: Vec::new(), + build: crate::config::ByMode::One(crate::config::Command::default()), cwd: cwd.map(PathBuf::from), stale_when: None, also_watch: Vec::new(), strip: false, + enroll: crate::config::Command::default(), + strip_here: None, package: None, + mode: None, built_from: None, + built_mode: None, } } @@ -1178,6 +1301,59 @@ mod tests { ); } + /// Accepting is about what the project asks to have *run*. It is not + /// somebody withdrawing the mode they picked, so the choice survives + /// -- exactly as the measured package does, and it has to be written + /// here rather than asserted from a hand-built config, because what + /// "accepted" means is whatever `approve_declaration` writes. + #[test] + fn a_chosen_mode_survives_the_declaration_being_accepted_again() { + let dir = tempfile::tempdir().expect("tempdir"); + write_moded_request(dir.path()); + let (_home, state) = state_with(dir.path()); + state.approve_declaration("demo").expect("approve"); + state + .set_component_choices("demo", "app", Some("debug".to_string()), None) + .expect("choose the debug mode"); + assert_eq!( + state.entry("demo").expect("entry").components[0].effective_mode(), + Some("debug"), + ); + + state.approve_declaration("demo").expect("approve again"); + let component = &state.entry("demo").expect("entry").components[0]; + assert_eq!( + component.effective_mode(), + Some("debug"), + "accepting must not silently put the build back to the default mode", + ); + assert_eq!(component.build().to_line(), "d"); + } + + /// A mode this component does not declare is refused rather than + /// stored: `ByMode::get` falls back to the first, so storing it would + /// leave the card naming a mode nothing is built in -- which reads + /// exactly like a setting that took effect. + #[test] + fn a_mode_the_component_does_not_declare_is_refused() { + let dir = tempfile::tempdir().expect("tempdir"); + write_moded_request(dir.path()); + let (_home, state) = state_with(dir.path()); + state.approve_declaration("demo").expect("approve"); + let refused = state + .set_component_choices("demo", "app", Some("profile".to_string()), None) + .expect_err("there is no such mode"); + assert!( + refused.to_string().contains("profile"), + "the refusal should name what was asked for: {refused}", + ); + assert_eq!( + state.entry("demo").expect("entry").components[0].effective_mode(), + Some("release"), + "and it must leave the component where it was", + ); + } + /// The case the gate exists for, and the one a cached answer got /// wrong: the project file changes on a *pull*, which writes no config /// and so rebuilds no entries. An entry built while the request diff --git a/server/src/resources.rs b/server/src/resources.rs index d2c6527..b782af8 100644 --- a/server/src/resources.rs +++ b/server/src/resources.rs @@ -58,7 +58,7 @@ pub fn read(project: &Path, declaration: &Resources) -> Result { - let text = run(project, command)?; + let text = crate::script::capture(project, None, command, SCRIPT_TIMEOUT)?; parse(&text).map_err(|err| format!("in the output of {}: {err}", command.to_line())) } } @@ -74,55 +74,6 @@ fn parse(text: &str) -> Result { wg_app_link::format::parse(text).map_err(|err| err.to_string()) } -fn run(project: &Path, command: &crate::config::Command) -> Result { - use std::io::Read; - - let mut child = command - .to_process(project, None, &[])? - // Closed, because a script that asks a question would otherwise - // wait for an answer nobody is there to give -- the same rule the - // service scripts run under. - .stdin(std::process::Stdio::null()) - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::piped()) - .spawn() - .map_err(|err| format!("starting {}: {err}", command.to_line()))?; - - let deadline = std::time::Instant::now() + SCRIPT_TIMEOUT; - loop { - match child.try_wait() { - Err(err) => return Err(format!("waiting on {}: {err}", command.to_line())), - Ok(Some(status)) => { - let mut out = String::new(); - if let Some(mut pipe) = child.stdout.take() { - let _ = pipe.read_to_string(&mut out); - } - if status.success() { - return Ok(out); - } - let mut err = String::new(); - if let Some(mut pipe) = child.stderr.take() { - let _ = pipe.read_to_string(&mut err); - } - return Err(format!( - "{} failed ({status}): {}", - command.to_line(), - err.lines().next().unwrap_or("no output").trim() - )); - } - Ok(None) if std::time::Instant::now() >= deadline => { - let _ = child.kill(); - return Err(format!( - "{} did not answer within {}s", - command.to_line(), - SCRIPT_TIMEOUT.as_secs() - )); - } - Ok(None) => std::thread::sleep(std::time::Duration::from_millis(20)), - } - } -} - /// One project to read, and where from. pub struct Target { pub key: String, diff --git a/server/src/routes.rs b/server/src/routes.rs index 8bcf695..971d52e 100644 --- a/server/src/routes.rs +++ b/server/src/routes.rs @@ -22,6 +22,12 @@ //! services again //! PUT /apps/{key}/settings {gitIpv4} //! this machine's preferences for it +//! PUT /apps/{key}/components/{name}/settings {mode, strip} +//! this machine's preferences for one +//! component +//! POST /apps/{key}/components/{name}/enroll-link +//! run that component's `enroll:` and +//! answer the URL it printed //! GET /apps/{key}/components/{name}/logs[?lines=&generation=] //! what that component wrote //! POST /apps/{key}/components/{name}/{action} @@ -98,6 +104,21 @@ pub fn tls_router(state: Arc) -> Router { // action -- axum matches a literal segment ahead of a capture, // but declaring it first says so to a reader too. .route("/apps/{key}/components/{name}/logs", get(component_logs)) + // Beside `logs` and ahead of `{action}` for the same reason, even + // though the methods already keep them apart: a reader should not + // have to check that `settings` is not an action. + .route( + "/apps/{key}/components/{name}/settings", + put(set_component_settings), + ) + // Its own route rather than one more `{action}`: those drive a + // server's service and answer with what the service is doing, + // where this belongs to an APK and answers with a URL. Folding it + // in would have made the action list mean two things. + .route( + "/apps/{key}/components/{name}/enroll-link", + post(enrollment_link), + ) .route( "/apps/{key}/components/{name}/{action}", post(service_action), @@ -236,6 +257,14 @@ struct ManifestApk { /// would mean running the strip pipeline here; see /// `strip::serveable_now`. size: u64, + /// Whether a debug-symbol-stripped copy is what gets served -- what + /// this machine settled on, which is what the download will actually + /// do. + strip: bool, + /// What the *project* asks for, which is only different when somebody + /// overrode it here. Sent so the sheet can say the two differ rather + /// than showing a switch that silently disagrees with the checkout. + strip_declared: bool, variants: Vec, } @@ -332,6 +361,21 @@ struct ManifestComponent { /// project that keeps nothing. #[serde(skip_serializing_if = "Option::is_none")] resources_error: Option, + /// This component can hand the phone a link to open after it is + /// installed, because the project declares a command that prints one. + /// What the link does is the project's business; the button just + /// opens it. + has_enroll_link: bool, + /// Every way this component can be built, in declaration order, with + /// the first the default. Empty for a component that declares one + /// way, which is not a choice -- the settings sheet says so rather + /// than drawing a picker with one entry in it. + modes: Vec, + /// Which of them it is set to build in. Absent exactly when `modes` + /// is empty; when it is not, this is always one of them, including + /// the case where nobody has chosen and it is the first. + #[serde(skip_serializing_if = "Option::is_none")] + mode: Option, /// What there is to install, for a component that produces an APK. #[serde(skip_serializing_if = "Option::is_none")] apk: Option, @@ -415,6 +459,12 @@ impl ManifestComponent { resources_error: is_server .then(|| state.resource_checks.error(key)) .flatten(), + // Gated the same way `has_build` is: while a declaration is + // waiting to be accepted the command will not run, so + // offering the button would be offering one that refuses. + has_enroll_link: may_build && component.enroll().is_some(), + modes: component.modes().to_vec(), + mode: component.effective_mode().map(str::to_string), apk: match is_server { true => None, false => Some(ManifestApk::read(state, key, entry, component).await?), @@ -455,6 +505,8 @@ impl ManifestApk { .map(|apk| epoch_secs(apk.modified)) .unwrap_or(0.0), size, + strip: component.strip(), + strip_declared: component.strip_declared(), variants: entry .variants(component) .into_iter() @@ -1220,6 +1272,113 @@ async fn set_settings( Ok(StatusCode::NO_CONTENT) } +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +struct EnrollmentLink { + url: String, +} + +/// How long a project's `enroll:` command is given before it is given up +/// on. Somebody is holding a phone waiting for it, so shorter than a +/// service action's minute -- but long enough for a command that has to +/// touch a keystore or a config file on the way. +const ENROLL_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(20); + +/// Runs one component's `enroll:` and answers the URL it printed, for the +/// phone to open. +/// +/// Run on every press rather than cached anywhere. The link a project +/// mints is ordinarily one-shot and carries a credential, so a stored one +/// would be both stale and a secret sitting in a file -- and a project +/// that mints a fresh token each time is exactly the shape this is for. +/// +/// One line, and the first: the contract is a single URL on stdout, which +/// makes a project's diagnostics stderr's business. Trimmed rather than +/// parsed -- what a URL means is the phone's business and not this +/// server's, so nothing here inspects the scheme. +async fn enrollment_link( + State(state): State>, + UrlPath((key, name)): UrlPath<(String, String)>, +) -> Result, ApiError> { + let entry = state.entry(&key).ok_or(ApiError::UnknownApp(key.clone()))?; + // The same gate the build runs behind, asked here rather than + // inherited: this is a command the project declared, so a pull must + // not be able to introduce one that a press then runs. + if entry.pending_declaration().is_some() { + return Err(ApiError::BadRequest(format!( + "{} is asking to have a command accepted before anything of its own runs", + entry.label + ))); + } + let component = entry + .components + .iter() + .find(|component| component.name() == name) + .ok_or_else(|| ApiError::UnknownComponent(key.clone(), name.clone()))?; + let command = component + .enroll() + .ok_or_else(|| { + ApiError::BadRequest(format!("{name} does not say how to get an enrolment link")) + })? + .clone(); + let project = entry.project_path.clone(); + let cwd = component.cwd().map(std::path::Path::to_path_buf); + let printed = tokio::task::spawn_blocking(move || { + crate::script::capture(&project, cwd.as_deref(), &command, ENROLL_TIMEOUT) + }) + .await + .context("the enrolment command panicked")? + .map_err(|err| ApiError::Internal(anyhow::anyhow!(err)))?; + let url = printed + .lines() + .next() + .unwrap_or_default() + .trim() + .to_string(); + if url.is_empty() { + return Err(ApiError::Internal(anyhow::anyhow!( + "{name}'s enrolment command printed nothing on stdout" + ))); + } + Ok(Json(EnrollmentLink { url })) +} + +/// What somebody chose for one component on the settings sheet. +/// +/// `mode` absent means "no choice, take the first declared one", which is +/// a real answer rather than a missing field -- it is what a component +/// reverts to and what every component starts as. `strip` absent means +/// the same about the project's own declaration. +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct ComponentSettingsBody { + #[serde(default)] + mode: Option, + #[serde(default)] + strip: Option, +} + +/// This machine's preferences for one component: which declared mode to +/// build it in, and whether to strip what it serves. +/// +/// On the build machine rather than on the device, unlike the variant a +/// download names: a mode decides what gets *built*, and there is one +/// checkout and one set of outputs, so two enrolled phones holding +/// different answers would rebuild over each other with nothing on either +/// screen to say why. Which of the finished builds a phone installs stays +/// that phone's business. +async fn set_component_settings( + State(state): State>, + UrlPath((key, name)): UrlPath<(String, String)>, + Json(body): Json, +) -> Result { + mutate(state, move |state| { + state.set_component_choices(&key, &name, body.mode, body.strip) + }) + .await?; + Ok(StatusCode::NO_CONTENT) +} + async fn set_roots( State(state): State>, Json(body): Json, diff --git a/server/src/script.rs b/server/src/script.rs new file mode 100644 index 0000000..75b588c --- /dev/null +++ b/server/src/script.rs @@ -0,0 +1,76 @@ +//! Running a command a project declared and keeping what it printed. +//! +//! Two things here ask a project to answer a question by running +//! something: `resources`, which reads where a project keeps its state, +//! and the enrolment link on an `Apk` component. Both want the same +//! bargain -- stdin closed, a deadline, stdout kept, and a failure that +//! names the command and the first line of its stderr -- so the bargain +//! is written once rather than twice with the second copy quietly +//! drifting. +//! +//! Deliberately *not* what a build runs through. A build's output is +//! streamed as it arrives, because it is long and somebody is watching a +//! progress bar; these are short and their whole value is the string at +//! the end. + +use std::path::Path; +use std::time::{Duration, Instant}; + +use crate::config::Command; + +/// Runs `command` in the project and answers its stdout. +/// +/// Stdin is closed rather than inherited. A script that prompts for a +/// password gets end-of-file and fails, instead of hanging until the +/// deadline with a card stuck on whatever it was doing -- the same +/// bargain the service scripts make. +pub fn capture( + project: &Path, + cwd: Option<&Path>, + command: &Command, + timeout: Duration, +) -> Result { + use std::io::Read; + + let mut child = command + .to_process(project, cwd, &[])? + .stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .map_err(|err| format!("starting {}: {err}", command.to_line()))?; + + let deadline = Instant::now() + timeout; + loop { + match child.try_wait() { + Err(err) => return Err(format!("waiting on {}: {err}", command.to_line())), + Ok(Some(status)) => { + let mut out = String::new(); + if let Some(mut pipe) = child.stdout.take() { + let _ = pipe.read_to_string(&mut out); + } + if status.success() { + return Ok(out); + } + let mut err = String::new(); + if let Some(mut pipe) = child.stderr.take() { + let _ = pipe.read_to_string(&mut err); + } + return Err(format!( + "{} failed ({status}): {}", + command.to_line(), + err.lines().next().unwrap_or("no output").trim() + )); + } + Ok(None) if Instant::now() >= deadline => { + let _ = child.kill(); + return Err(format!( + "{} did not answer within {}s", + command.to_line(), + timeout.as_secs() + )); + } + Ok(None) => std::thread::sleep(Duration::from_millis(20)), + } + } +} diff --git a/server/src/service.rs b/server/src/service.rs index 20540c3..e873832 100644 --- a/server/src/service.rs +++ b/server/src/service.rs @@ -80,19 +80,42 @@ impl ServiceState { /// declares no service, which are the same thing to every caller: there /// is nothing to drive. pub fn driver(key: &str, component: &Component) -> Option { - match component.service()? { - Service::Script(script) if !script.is_empty() => Some(script.clone()), - Service::Managed(run) if !run.is_empty() => Some(Command::from_words(vec![ + let service = component.service()?; + // Resolved here, once, from the component's own mode. Which binary a + // managed service runs is exactly what a build mode changes, so a + // second place deciding the mode is a service started from the other + // mode's output -- which looks like a build that did nothing. + let mode = component.effective_mode(); + let command = service.command(mode); + if command.is_empty() { + // Declared empty, which says the same as not declaring it. + return None; + } + // The same rule the build follows: a command written once is told + // which mode it is running in, a command written per mode is not told + // twice. It lands *before* the subcommand the caller appends, which + // is what keeps ` ` the contract every service + // script is written against. + let mode = service.by_mode().mode_argument(mode); + let with_mode = |command: &Command| match mode { + None => command.clone(), + Some(mode) => { + let mut words: Vec = command.to_words(); + words.push(mode.to_string()); + Command::from_words(words) + } + }; + match service { + Service::Script(_) => Some(with_mode(command)), + Service::Managed(_) => Some(Command::from_words(vec![ crate::shipped::service_default() .to_string_lossy() .into_owned(), "--name".to_string(), unit_name(key, component.name()), "--exec".to_string(), - run.to_line(), + with_mode(command).to_line(), ])), - // Declared empty, which says the same as not declaring it. - Service::Script(_) | Service::Managed(_) => None, } } @@ -391,19 +414,64 @@ mod tests { use std::os::unix::fs::PermissionsExt; use super::*; + use crate::config::ByMode; fn server(service: Option) -> Component { + server_with_modes(Vec::new(), service) + } + + fn server_with_modes(modes: Vec, service: Option) -> Component { Component::Server { name: "backend".to_string(), - build: Command::default(), + modes, + build: ByMode::One(Command::default()), cwd: None, stale_when: None, also_watch: Vec::new(), service, + mode: None, built_from: None, + built_mode: None, } } + /// Which binary a managed service runs is exactly what a mode + /// changes, and this is the single place that gets decided -- a + /// second one would build `release` and start `debug`, from a card + /// reporting the mode it was asked for. + #[test] + fn the_mode_decides_which_binary_the_service_runs() { + let mut component = server_with_modes( + vec!["release".to_string(), "debug".to_string()], + Some(Service::Managed(ByMode::Modes(vec![ + ( + "release".to_string(), + Command::from_line("target/release/backend"), + ), + ( + "debug".to_string(), + Command::from_line("target/debug/backend"), + ), + ]))), + ); + + let exec_of = |component: &Component| { + let words = driver("app", component).expect("a command to drive it"); + let (_, args) = words.split_first().expect("the script and its arguments"); + let at = args + .iter() + .position(|arg| arg == "--exec") + .expect("--exec is how the built-in script is told what to run"); + args[at + 1].clone() + }; + + // Nobody has chosen, so it is the first declared -- which is what + // makes declaration order worth getting right. + assert_eq!(exec_of(&component), "target/release/backend"); + component.choose(Some("debug".to_string()), None); + assert_eq!(exec_of(&component), "target/debug/backend"); + } + /// The one place the two variants become the same thing, so this is /// where it is worth pinning down what each turns into. #[test] @@ -412,7 +480,10 @@ mod tests { // already the thing the contract describes. let own = Command::from_line("server/service"); assert_eq!( - driver("app", &server(Some(Service::Script(own.clone())))), + driver( + "app", + &server(Some(Service::Script(ByMode::One(own.clone())))) + ), Some(own) ); @@ -421,9 +492,9 @@ mod tests { // appends still lands last. let managed = driver( "app", - &server(Some(Service::Managed(Command::from_line( + &server(Some(Service::Managed(ByMode::One(Command::from_line( "target/release/ai-server --port 8080", - )))), + ))))), ) .expect("a managed component has a driver"); let (program, arguments) = managed.split_first().expect("a program"); @@ -452,11 +523,17 @@ mod tests { fn nothing_to_drive_is_none_however_it_was_said() { assert_eq!(driver("app", &server(None)), None); assert_eq!( - driver("app", &server(Some(Service::Script(Command::default())))), + driver( + "app", + &server(Some(Service::Script(ByMode::One(Command::default())))) + ), None ); assert_eq!( - driver("app", &server(Some(Service::Managed(Command::default())))), + driver( + "app", + &server(Some(Service::Managed(ByMode::One(Command::default())))) + ), None ); } diff --git a/test-projects/service-and-app/.dev-updater.ron b/test-projects/service-and-app/.dev-updater.ron index c58b3ab..cca5e64 100644 --- a/test-projects/service-and-app/.dev-updater.ron +++ b/test-projects/service-and-app/.dev-updater.ron @@ -20,6 +20,13 @@ resources: Ron("resources.ron"), components: [ Server( name: "daemon", + // Two modes, declared once. Both commands are written *once* + // and handed the mode as their last argument, which is the + // ordinary way -- Dev Updater's own declaration covers the other + // way, where a command cannot take the word and is written per + // mode instead. Between the two fixtures both paths are + // exercised. `release` first, since the first is the default. + modes: ["release", "debug"], build: "./build-daemon.sh", // Managed, so Dev Updater's own service script drives it and this // project needs no systemd or OpenRC knowledge of its own. The diff --git a/test-projects/service-and-app/build-daemon.sh b/test-projects/service-and-app/build-daemon.sh index d60a3e3..c797d27 100755 --- a/test-projects/service-and-app/build-daemon.sh +++ b/test-projects/service-and-app/build-daemon.sh @@ -6,16 +6,31 @@ # seconds. A card with one instant component and one slow one is how you see # whether the per-component timings and the concurrent build really are per # component. +# +# It takes the build mode as its one argument, so that this fixture +# exercises a component with more than one way of being built. `release` +# is deliberately the slow one, which is both what a real optimised build +# is and what makes a mode switch visible on the card: a bar that takes +# eight seconds and one that takes two. set -eu -STEPS=8 +MODE="${1:-release}" +case "$MODE" in +release) STEPS=8 ;; +debug) STEPS=2 ;; +*) + echo "unknown build mode: $MODE (expected release or debug)" >&2 + exit 2 + ;; +esac + i=0 while [ "$i" -lt "$STEPS" ]; do i=$((i + 1)) echo "@@progress $i/$STEPS" - echo "==> Pretending to compile part $i of $STEPS" + echo "==> Pretending to compile part $i of $STEPS ($MODE)" sleep 1 done chmod +x ./serve.sh -echo "==> Daemon ready" +echo "==> Daemon ready ($MODE)" diff --git a/test-projects/service-and-app/serve.sh b/test-projects/service-and-app/serve.sh index 1b70a4d..9c02c64 100755 --- a/test-projects/service-and-app/serve.sh +++ b/test-projects/service-and-app/serve.sh @@ -7,12 +7,27 @@ # that the directory exists to be shown in the dialog. Its *log* is not # written here: a managed service's output is captured by Dev Updater's # service script, which is what the runtime log tab reads. +# +# Takes the build mode as its one argument, ahead of whatever subcommand +# the service contract appends -- which is how a command written once +# learns which mode it is running in. It ticks at a different rate in +# each, so that a mode switch is checkable from a phone: the runtime log +# is the only place the difference shows. set -eu +MODE="${1:-release}" +case "$MODE" in +release) INTERVAL=10 ;; +debug) INTERVAL=2 ;; +*) + echo "unknown mode: $MODE (expected release or debug)" >&2 + exit 2 + ;; +esac DATA="${XDG_DATA_HOME:-$HOME/.local/share}/dutest-service" mkdir -p "$DATA" -echo "started at $(date -Is), writing to $DATA" +echo "started at $(date -Is), writing to $DATA, ticking every ${INTERVAL}s in $MODE" count=0 while true; do count=$((count + 1)) @@ -20,5 +35,5 @@ while true; do # Colour, so the runtime log tab has escapes to render too -- the same # reason breakable/build.sh writes them. printf 'tick \033[1;32m%s\033[0m -- still here\n' "$count" - sleep 10 + sleep "$INTERVAL" done