A project can produce more than one APK
tdep-survey is one checkout with a backend and two independent Android clients, so a project has to be able to declare two Apk components. It could not: APK_PATTERNS was anchored at the project root, which reached the first client and stopped, and package, strip, the variant list and the download all came from AppEntry::apk_component()'s first match -- correct only while a project produced one APK. Each component's builds are now found under its own cwd, which is what cwd already meant everywhere else: the directory the build command runs in, the subtree staleness is scoped to, a server's WorkingDirectory. A component that declares none sits at the project root, so nothing about the single-APK case changes. The alternative -- naming the file on the component -- would have made a component a file path, and an app being a project path rather than a file path is this project's central invariant. Everything derived from an APK follows it onto the component: package, previousPackage, strip, size, mtime, variants and the rename note, in a nested `apk` block that a Server simply doesn't have. Two clients install over different packages, so first-wins would have checked the installed state of one and reported it as the other's -- which looks exactly like a correct answer. For the same reason a download naming no component is refused rather than guessed; naming none still answers for a project with one, which is what lets the frozen /self/apk keep working. On the phone the per-device state is keyed by project and component, so the variant picker moved inside the component's own card, beside the build it picks, and the installed state, size and icon are each their own component's. A project building two clients shows no single icon of its own rather than borrowing the first one's. Measured against the real thing: pointed at tdep-survey, the two clients resolve to their own builds (15 MB and 153 MB), strip applies only to the one that asked for it (153 MB served as 52 MB), pressing Install on one row installed that row's package and left the other row offering Install, and a download with no component named answers 400 saying which flag to pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
db47972a25
commit
c4e19c2e2b
9 files changed
+760
-309
No files matched your search
+189
-82
@@ -127,6 +127,14 @@ enum ApiError {
|
||||
UnknownApp(String),
|
||||
#[error("{0} has no build yet")]
|
||||
NotBuilt(String),
|
||||
/// A project that produces more than one APK, asked for "the" APK.
|
||||
/// Refused rather than answered with the first: which of two clients
|
||||
/// somebody meant is not something to guess, and a guess here would
|
||||
/// install the wrong app while looking like it worked.
|
||||
#[error("{0} builds more than one app -- say which with ?component=")]
|
||||
AmbiguousApk(String),
|
||||
#[error("{0} has no component named {1}")]
|
||||
UnknownComponent(String, String),
|
||||
#[error("{0} has no on-demand build step configured")]
|
||||
NoBuildStep(String),
|
||||
#[error("{0}")]
|
||||
@@ -140,7 +148,11 @@ enum ApiError {
|
||||
impl IntoResponse for ApiError {
|
||||
fn into_response(self) -> Response {
|
||||
let status = match self {
|
||||
Self::UnknownApp(_) | Self::NotBuilt(_) | Self::NoBuildStep(_) => StatusCode::NOT_FOUND,
|
||||
Self::UnknownApp(_)
|
||||
| Self::NotBuilt(_)
|
||||
| Self::NoBuildStep(_)
|
||||
| Self::UnknownComponent(..) => StatusCode::NOT_FOUND,
|
||||
Self::AmbiguousApk(_) => StatusCode::BAD_REQUEST,
|
||||
Self::BadRequest(_) => StatusCode::BAD_REQUEST,
|
||||
Self::RangeNotSatisfiable => StatusCode::RANGE_NOT_SATISFIABLE,
|
||||
Self::Internal(err) => {
|
||||
@@ -162,6 +174,56 @@ fn bad_request(err: anyhow::Error) -> ApiError {
|
||||
ApiError::BadRequest(format!("{err:#}"))
|
||||
}
|
||||
|
||||
/// The installable half of a component, absent for a `Server`.
|
||||
///
|
||||
/// Nested rather than flattened onto the component with every field
|
||||
/// optional, because "this component has an APK" is one fact rather than
|
||||
/// six: a server has no build to install, no package to replace and no
|
||||
/// variants to choose between, and saying that once is what stops the
|
||||
/// phone having to work it out from a size of zero.
|
||||
///
|
||||
/// Per component and not per project. Two clients built from one checkout
|
||||
/// install over different packages and are worth stripping to different
|
||||
/// degrees, so a project-level answer would be the first component's,
|
||||
/// presented as the project's.
|
||||
#[derive(Serialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
struct ManifestApk {
|
||||
/// What the download is saved as on the device.
|
||||
filename: String,
|
||||
/// Absent until a build has been read for it -- a project can be added
|
||||
/// before it has ever been built, and claiming a package before then
|
||||
/// would be inventing one.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
package: Option<String>,
|
||||
/// What this component used to install over, when it has been renamed
|
||||
/// since this server started.
|
||||
///
|
||||
/// Android treats a renamed `applicationId` as an unrelated app, so
|
||||
/// the old one is still installed and nothing will ever replace it.
|
||||
/// The phone offers to remove it -- and knows whether it is still
|
||||
/// there, which is why nothing here has to be cleared.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
previous_package: Option<String>,
|
||||
/// False when this component has no APK yet (never built, or cleaned).
|
||||
/// It is still drawn -- it was declared deliberately, and a row saying
|
||||
/// so is a better answer than one that silently vanished -- with
|
||||
/// `mtime`/`size` at zero and nothing to download.
|
||||
built: bool,
|
||||
/// Epoch seconds of the raw build's mtime. Always the *raw* build's,
|
||||
/// even when a stripped copy is what's served: that's the number that
|
||||
/// actually moves when something is rebuilt, which is what the app
|
||||
/// compares against the installed copy.
|
||||
mtime: f64,
|
||||
/// Of the file that would be served as things stand -- the slim copy
|
||||
/// where one has already been produced. Close to the bytes about to be
|
||||
/// downloaded rather than exactly them, because finding out exactly
|
||||
/// would mean running the strip pipeline here; see
|
||||
/// `strip::serveable_now`.
|
||||
size: u64,
|
||||
variants: Vec<ManifestVariant>,
|
||||
}
|
||||
|
||||
/// One component, as the card needs it: what it is called and which kind
|
||||
/// it is. Nothing else -- what it *does* is the build step, which is not
|
||||
/// the phone's business, and how far along it is arrives on the status.
|
||||
@@ -246,15 +308,22 @@ struct ManifestComponent {
|
||||
/// project that keeps nothing.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
resources_error: Option<String>,
|
||||
/// What there is to install, for a component that produces an APK.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
apk: Option<ManifestApk>,
|
||||
}
|
||||
|
||||
impl ManifestComponent {
|
||||
fn read(
|
||||
/// Async only for the APK's size, which is a `stat` of whatever is on
|
||||
/// disk -- never a strip run to find out what the slim copy would
|
||||
/// weigh, because this path is fetched on every open, resume and
|
||||
/// Refresh.
|
||||
async fn read(
|
||||
state: &AppState,
|
||||
key: &str,
|
||||
entry: &AppEntry,
|
||||
component: &crate::config::Component,
|
||||
) -> Self {
|
||||
) -> Result<Self, ApiError> {
|
||||
let name = component.name().to_string();
|
||||
let is_server = matches!(component, crate::config::Component::Server { .. });
|
||||
// A build log is a file this server wrote, so its existence is a
|
||||
@@ -271,7 +340,7 @@ impl ManifestComponent {
|
||||
.then(|| state.resource_checks.facts(key))
|
||||
.flatten()
|
||||
.map(|facts| crate::purge::paths(&facts, &entry.project_path));
|
||||
Self {
|
||||
Ok(Self {
|
||||
kind: if is_server { "server" } else { "apk" },
|
||||
state: is_server
|
||||
.then(|| state.service_checks.state(key, &name))
|
||||
@@ -317,8 +386,56 @@ impl ManifestComponent {
|
||||
resources_error: is_server
|
||||
.then(|| state.resource_checks.error(key))
|
||||
.flatten(),
|
||||
apk: match is_server {
|
||||
true => None,
|
||||
false => Some(ManifestApk::read(state, key, entry, component).await?),
|
||||
},
|
||||
name,
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
impl ManifestApk {
|
||||
async fn read(
|
||||
state: &AppState,
|
||||
key: &str,
|
||||
entry: &AppEntry,
|
||||
component: &crate::config::Component,
|
||||
) -> Result<Self, ApiError> {
|
||||
let name = component.name();
|
||||
let newest = entry.resolve_apk(component, None);
|
||||
let size = match &newest {
|
||||
Some(apk) => {
|
||||
tokio::fs::metadata(crate::strip::serveable_now(&apk.path, component.strip()))
|
||||
.await
|
||||
.context("stat the apk to be served")?
|
||||
.len()
|
||||
}
|
||||
None => 0,
|
||||
};
|
||||
Ok(Self {
|
||||
filename: newest
|
||||
.as_ref()
|
||||
.map(|apk| entry.filename(&apk.path))
|
||||
.unwrap_or_else(|| format!("{key}-{name}.apk")),
|
||||
package: component.package().map(str::to_string),
|
||||
previous_package: state.previous_package(key, name),
|
||||
built: newest.is_some(),
|
||||
mtime: newest
|
||||
.as_ref()
|
||||
.map(|apk| epoch_secs(apk.modified))
|
||||
.unwrap_or(0.0),
|
||||
size,
|
||||
variants: entry
|
||||
.variants(component)
|
||||
.into_iter()
|
||||
.map(|candidate| ManifestVariant {
|
||||
path: candidate.path.to_string_lossy().into_owned(),
|
||||
variant: candidate.variant,
|
||||
mtime: epoch_secs(candidate.modified),
|
||||
})
|
||||
.collect(),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -341,44 +458,22 @@ struct ManifestResponse {
|
||||
struct ManifestApp {
|
||||
key: String,
|
||||
label: String,
|
||||
filename: String,
|
||||
/// Where this project's APKs are fetched from. Which of them is said
|
||||
/// with the request (`?component=`), so this stays one route per
|
||||
/// project rather than one string per component that differs only in
|
||||
/// its query.
|
||||
route: String,
|
||||
/// Absent until a build has been read for it -- a project can be added
|
||||
/// before it has ever been built, and claiming a package before then
|
||||
/// would be inventing one.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
package: Option<String>,
|
||||
/// What this project's APK used to install over, when it has been
|
||||
/// renamed since this server started.
|
||||
///
|
||||
/// Android treats a renamed `applicationId` as an unrelated app, so
|
||||
/// the old one is still installed and nothing will ever replace it.
|
||||
/// The phone offers to remove it -- and knows whether it is still
|
||||
/// there, which is why nothing here has to be cleared.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
previous_package: Option<String>,
|
||||
project_path: String,
|
||||
/// Epoch seconds of the raw build's mtime. Always the *raw* build's,
|
||||
/// even when a stripped copy is what's served: that's the number that
|
||||
/// actually moves when something is rebuilt, which is what the app
|
||||
/// compares against the installed copy.
|
||||
mtime: f64,
|
||||
/// Of the file that would be served as things stand -- the slim copy
|
||||
/// where one has already been produced. Close to the bytes about to be
|
||||
/// downloaded rather than exactly them, because finding out exactly
|
||||
/// would mean running the strip pipeline here; see
|
||||
/// `strip::serveable_now`.
|
||||
size: u64,
|
||||
needs_build: bool,
|
||||
/// This machine's preferences for the project, so the card's settings
|
||||
/// can show what is currently set rather than a guess at it.
|
||||
git_ipv4: bool,
|
||||
/// True for this server's own app, which has no Remove button.
|
||||
built_in: bool,
|
||||
/// False when the project has no APK yet (never built, or cleaned).
|
||||
/// Such an app is still listed -- it was added deliberately, and a card
|
||||
/// saying so is a better answer than one that silently vanished -- with
|
||||
/// `mtime`/`size` at zero and nothing to download.
|
||||
/// Whether *anything* this project produces has been built. Per
|
||||
/// component is on the component (`ManifestApk::built`); this is what
|
||||
/// the card's own "nothing here yet" line reads, and what keeps a
|
||||
/// project with one built client out of it.
|
||||
built: bool,
|
||||
/// Present when the project is in a git repository at all: the branch,
|
||||
/// how far behind it is as of the last fetch, and whether the tree is
|
||||
@@ -422,7 +517,6 @@ struct ManifestApp {
|
||||
/// something the keys don't say.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pending_declaration: Option<String>,
|
||||
variants: Vec<ManifestVariant>,
|
||||
}
|
||||
|
||||
/// One build found under a project. Which of them a device wants is that
|
||||
@@ -514,27 +608,13 @@ async fn app(
|
||||
/// whole list and a single card cannot come to describe the same app
|
||||
/// differently.
|
||||
async fn describe(state: &Arc<AppState>, entry: &AppEntry) -> Result<ManifestApp, ApiError> {
|
||||
let apk = entry.resolve_apk(None);
|
||||
// Whatever is on disk right now, never a strip run to find out --
|
||||
// see `strip::serveable_now`. This path is fetched on every open,
|
||||
// resume and Refresh.
|
||||
let size = match &apk {
|
||||
Some(apk) => tokio::fs::metadata(crate::strip::serveable_now(&apk.path, entry.strip()))
|
||||
.await
|
||||
.context("stat the apk to be served")?
|
||||
.len(),
|
||||
None => 0,
|
||||
};
|
||||
|
||||
let variants = entry
|
||||
.variants()
|
||||
.into_iter()
|
||||
.map(|candidate| ManifestVariant {
|
||||
path: candidate.path.to_string_lossy().into_owned(),
|
||||
variant: candidate.variant,
|
||||
mtime: epoch_secs(candidate.modified),
|
||||
})
|
||||
.collect();
|
||||
// A loop rather than a map because each component's APK is a `stat`,
|
||||
// and they are described in declaration order -- which is build order,
|
||||
// and the order the card draws them in.
|
||||
let mut components = Vec::with_capacity(entry.components.len());
|
||||
for component in &entry.components {
|
||||
components.push(ManifestComponent::read(state, &entry.key, entry, component).await?);
|
||||
}
|
||||
|
||||
let git = crate::git::status(&entry.project_path);
|
||||
// An upstream is part of it: a branch that tracks nothing has nothing
|
||||
@@ -550,34 +630,22 @@ async fn describe(state: &Arc<AppState>, entry: &AppEntry) -> Result<ManifestApp
|
||||
// Read after `new_commits`, so a check that lands between the
|
||||
// two reports its answer rather than that it is still coming.
|
||||
check_pending: can_pull && state.remote_checks.is_checking(&entry.project_path),
|
||||
components: entry
|
||||
.components
|
||||
built: components
|
||||
.iter()
|
||||
.map(|component| ManifestComponent::read(state, &entry.key, entry, component))
|
||||
.collect(),
|
||||
.any(|component| component.apk.as_ref().is_some_and(|apk| apk.built)),
|
||||
components,
|
||||
check_error: can_pull
|
||||
.then(|| state.remote_checks.error(&entry.project_path))
|
||||
.flatten(),
|
||||
can_pull,
|
||||
git,
|
||||
route: format!("/apps/{}/apk", entry.key),
|
||||
filename: apk
|
||||
.as_ref()
|
||||
.map(|apk| entry.filename(&apk.path))
|
||||
.unwrap_or_else(|| format!("{}.apk", entry.key)),
|
||||
key: entry.key.clone(),
|
||||
label: entry.label.clone(),
|
||||
package: entry.package().map(str::to_string),
|
||||
previous_package: state.previous_package(&entry.key),
|
||||
// Shown on a phone, where the home prefix is the least
|
||||
// interesting part of a long path. expand_tilde accepts this
|
||||
// form back, so it stays copy-pasteable into "add by path".
|
||||
project_path: crate::config::contract_tilde(&entry.project_path),
|
||||
mtime: apk
|
||||
.as_ref()
|
||||
.map(|apk| epoch_secs(apk.modified))
|
||||
.unwrap_or(0.0),
|
||||
size,
|
||||
// False while something is waiting to be accepted: the command
|
||||
// won't run until it is, so offering to build would be a
|
||||
// button that does nothing.
|
||||
@@ -588,11 +656,9 @@ async fn describe(state: &Arc<AppState>, entry: &AppEntry) -> Result<ManifestApp
|
||||
.is_some_and(|build| build.has_command()),
|
||||
git_ipv4: entry.git_ipv4,
|
||||
built_in: entry.built_in,
|
||||
built: apk.is_some(),
|
||||
pending_declaration: pending
|
||||
.as_ref()
|
||||
.and_then(|components| crate::config::render_request(components).ok()),
|
||||
variants,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -747,7 +813,10 @@ async fn add_app(
|
||||
tracing::info!(
|
||||
"added {} ({}) from {}",
|
||||
entry.key,
|
||||
entry.package().unwrap_or("package not read yet"),
|
||||
entry
|
||||
.apk_component(None)
|
||||
.and_then(crate::config::Component::package)
|
||||
.unwrap_or("package not read yet"),
|
||||
entry.project_path.display()
|
||||
);
|
||||
Ok(Json(AddedResponse {
|
||||
@@ -1038,10 +1107,19 @@ struct PullQuery {
|
||||
force: bool,
|
||||
}
|
||||
|
||||
/// Which build a download wants. Absent means the newest, which is what
|
||||
/// every phone gets until it says otherwise.
|
||||
/// Which build a download wants, and whose.
|
||||
///
|
||||
/// `component` names the APK on a project that produces more than one;
|
||||
/// absent is the only APK, which is every project with one and the frozen
|
||||
/// `/self/apk`, which cannot say a component. A project with two and a
|
||||
/// request that names neither is refused rather than served the first --
|
||||
/// see [`ApiError::AmbiguousApk`].
|
||||
///
|
||||
/// `variant` absent means the newest, which is what every phone gets until
|
||||
/// it says otherwise.
|
||||
#[derive(Deserialize)]
|
||||
struct VariantQuery {
|
||||
component: Option<String>,
|
||||
variant: Option<String>,
|
||||
}
|
||||
|
||||
@@ -1334,8 +1412,12 @@ struct SelfBuild {
|
||||
/// the file and answers two numbers.
|
||||
async fn self_build(State(state): State<Arc<AppState>>) -> Result<Json<SelfBuild>, ApiError> {
|
||||
let entry = lookup(&state, None)?;
|
||||
// No component named, because this route is frozen and cannot carry
|
||||
// one -- and this server's own project produces exactly one APK,
|
||||
// which is what makes that an answer rather than a guess.
|
||||
let component = apk_component(&entry, None)?;
|
||||
let apk = entry
|
||||
.resolve_apk(None)
|
||||
.resolve_apk(component, None)
|
||||
.ok_or_else(|| ApiError::NotBuilt(entry.label.clone()))?;
|
||||
let size = tokio::fs::metadata(&apk.path)
|
||||
.await
|
||||
@@ -1347,6 +1429,28 @@ async fn self_build(State(state): State<Arc<AppState>>) -> Result<Json<SelfBuild
|
||||
}))
|
||||
}
|
||||
|
||||
/// The APK component a request is about, or why there isn't one.
|
||||
///
|
||||
/// The single place a name from a phone becomes a component, so the
|
||||
/// download and anything that follows it cannot disagree about which APK
|
||||
/// was meant.
|
||||
fn apk_component<'a>(
|
||||
entry: &'a AppEntry,
|
||||
named: Option<&str>,
|
||||
) -> Result<&'a crate::config::Component, ApiError> {
|
||||
if let Some(component) = entry.apk_component(named) {
|
||||
return Ok(component);
|
||||
}
|
||||
Err(match named {
|
||||
Some(name) => ApiError::UnknownComponent(entry.label.clone(), name.to_string()),
|
||||
// Nothing named, and not one obvious answer: either the project
|
||||
// builds no APK at all, or it builds several and the request has
|
||||
// to say which.
|
||||
None if entry.apk_components().next().is_none() => ApiError::NotBuilt(entry.label.clone()),
|
||||
None => ApiError::AmbiguousApk(entry.label.clone()),
|
||||
})
|
||||
}
|
||||
|
||||
async fn serve_apk(
|
||||
State(state): State<Arc<AppState>>,
|
||||
key: Option<UrlPath<String>>,
|
||||
@@ -1355,20 +1459,23 @@ async fn serve_apk(
|
||||
Query(query): Query<VariantQuery>,
|
||||
) -> Result<Response, ApiError> {
|
||||
let entry = lookup(&state, key)?;
|
||||
let component = apk_component(&entry, query.component.as_deref())?;
|
||||
// Which build this device wants, if it has a preference. Its own
|
||||
// preference, travelling with the request: two phones enrolled against
|
||||
// one server must not change what the other gets.
|
||||
// one server must not change what the other gets. Validated against
|
||||
// *this component's* builds, so naming another one's path is a
|
||||
// fallback to this one's newest rather than a way to be served it.
|
||||
let requested = query.variant.map(PathBuf::from);
|
||||
let apk = entry
|
||||
.resolve_apk(requested.as_deref())
|
||||
.resolve_apk(component, requested.as_deref())
|
||||
.ok_or_else(|| ApiError::NotBuilt(entry.label.clone()))?;
|
||||
// A fresh download, not a range continuation: the one moment this
|
||||
// server can notice that a local rebuild changed what the APK installs
|
||||
// over. Off the request, so it costs the download nothing.
|
||||
if headers.get(header::RANGE).is_none() {
|
||||
state.refresh_package(&entry.key, apk.path.clone());
|
||||
state.refresh_package(&entry.key, component.name(), apk.path.clone());
|
||||
}
|
||||
let resolved = resolve_serveable_path(&apk.path, entry.strip()).await?;
|
||||
let resolved = resolve_serveable_path(&apk.path, component.strip()).await?;
|
||||
let size = tokio::fs::metadata(&resolved)
|
||||
.await
|
||||
.context("stat resolved apk")?
|
||||
|
||||
Reference in new issue
Block a user