fix(build): stop a stale or missing keepsake viewer from shipping silently
Three ways the compiled-in viewer could be wrong, none of which anything would have reported. Found by mutation-testing the guard added below — it failed when it should have passed, and the reason was the second bullet. * `include_dir!` registers NO rebuild dependency. Run `npm run build` in frontend/export-viewer, then `cargo build`, and cargo sees no source change and reuses the cached binary — carrying the PREVIOUS index.html. The file on disk and the file in the binary disagree, git is clean, every check passes, and Memories.zip ships a stale viewer. Confirmed empirically: after replacing the artifact the compiled-in copy did not change until a source file was touched. A build.rs now declares `rerun-if-changed` for `static/export-viewer` AND `migrations` — sqlx::migrate!() embeds its directory the same way, and there the stale snapshot is worse still: the binary boots against a database that already ran a newer migration and crash-loops with VersionMissing. * `emptyOutDir: true` deleted the committed artifact BEFORE generating. That was safe while the build could not fail; it no longer is, because `inlineThemeFonts` now calls `this.error` on a keepsake that is not self-contained. A failed build left the directory empty — and include_dir! over an empty directory compiles fine, while `write_viewer_with_data` iterates zero files and returns Ok. The result is a valid archive with every photo and no viewer. The output is one overwritten file, so nothing accumulates without the wipe. * Nothing asserted the viewer was there at all. Now asserted at the point of use (bail rather than write a viewer-less keepsake) and in a test that checks presence, plausible size, and that no `url(/...)` survived inlining — the three ways it can be present but useless. The Dockerfile copies build.rs with the sources rather than with Cargo.toml, so the dependency-cache layer stays byte-identical and the dummy build does not run it.
This commit is contained in:
@@ -13,6 +13,13 @@ RUN mkdir src && echo "fn main(){}" > src/main.rs && \
|
|||||||
COPY src ./src
|
COPY src ./src
|
||||||
COPY static ./static
|
COPY static ./static
|
||||||
COPY migrations ./migrations
|
COPY migrations ./migrations
|
||||||
|
# Copied WITH the sources, not with Cargo.toml above: cargo auto-detects `build.rs` by presence, so
|
||||||
|
# putting it in the dependency-cache layer would make the dummy build run it too and invalidate a
|
||||||
|
# layer that is otherwise stable. Copied at all because without it the image builds a subtly
|
||||||
|
# DIFFERENT package from the one developers build — no build script, hence none of the
|
||||||
|
# rerun-if-changed tracking for `static/export-viewer` and `migrations`. Harmless here (every image
|
||||||
|
# build is clean, so there is no stale cache to reuse) and confusing everywhere else.
|
||||||
|
COPY build.rs ./
|
||||||
RUN touch src/main.rs && cargo build --release
|
RUN touch src/main.rs && cargo build --release
|
||||||
|
|
||||||
# --- Runtime stage ---
|
# --- Runtime stage ---
|
||||||
|
|||||||
25
backend/build.rs
Normal file
25
backend/build.rs
Normal file
@@ -0,0 +1,25 @@
|
|||||||
|
//! Tell cargo which non-Rust inputs are baked into the binary.
|
||||||
|
//!
|
||||||
|
//! `include_dir!` and `sqlx::migrate!()` both embed directory contents at COMPILE time, and neither
|
||||||
|
//! registers a rebuild dependency on its own. Cargo therefore reuses a cached binary when only
|
||||||
|
//! those directories changed — the source files are untouched, so as far as cargo is concerned
|
||||||
|
//! nothing happened.
|
||||||
|
//!
|
||||||
|
//! For the keepsake viewer that is a silent, shippable defect: run `npm run build` in
|
||||||
|
//! `frontend/export-viewer`, then `cargo build`, and the resulting binary still carries the
|
||||||
|
//! PREVIOUS `static/export-viewer/index.html`. The artifact on disk and the artifact in the binary
|
||||||
|
//! disagree, `git status` is clean, and every check passes — while `Memories.zip` ships a stale
|
||||||
|
//! viewer. Confirmed empirically: after replacing the file, the compiled-in copy did not change
|
||||||
|
//! until a source file was touched.
|
||||||
|
//!
|
||||||
|
//! Production is mostly insulated because images are built from a clean context (no cache to
|
||||||
|
//! reuse), but every incremental build — i.e. all local development and any test run that follows
|
||||||
|
//! a viewer rebuild — hits it, and that includes the test that asserts the viewer is present.
|
||||||
|
fn main() {
|
||||||
|
// The compiled-in keepsake viewer (services/export.rs: `include_dir!`).
|
||||||
|
println!("cargo:rerun-if-changed=static/export-viewer");
|
||||||
|
// The embedded migration set (db.rs: `sqlx::migrate!()`). Same mechanism, and the failure is
|
||||||
|
// worse: a binary built from a stale snapshot boots against a database that has already run a
|
||||||
|
// newer migration and crash-loops with VersionMissing.
|
||||||
|
println!("cargo:rerun-if-changed=migrations");
|
||||||
|
}
|
||||||
@@ -1359,6 +1359,18 @@ async fn run_html_export_inner(
|
|||||||
// `window.__EXPORT_DATA__` global into index.html. Guests double-click
|
// `window.__EXPORT_DATA__` global into index.html. Guests double-click
|
||||||
// index.html (file://), where a cross-origin fetch() of a sibling file is
|
// index.html (file://), where a cross-origin fetch() of a sibling file is
|
||||||
// blocked — so the data must be inlined rather than fetched from data.json.
|
// blocked — so the data must be inlined rather than fetched from data.json.
|
||||||
|
// The viewer IS the keepsake — Memories.zip without it is a folder of files with no way to
|
||||||
|
// look at them. `write_viewer_with_data` walks `dir.files()`, which iterates nothing at all
|
||||||
|
// when the compiled-in directory is empty, so a viewer build that failed after Vite emptied
|
||||||
|
// its output directory used to produce a perfectly valid archive with no viewer in it,
|
||||||
|
// silently. Asserted here rather than trusted: this costs one lookup per export.
|
||||||
|
if VIEWER_DIR.get_file("index.html").is_none() {
|
||||||
|
anyhow::bail!(
|
||||||
|
"the keepsake viewer is missing from this binary (static/export-viewer/index.html \
|
||||||
|
was not compiled in). Run `npm run build` in frontend/export-viewer and rebuild — \
|
||||||
|
an archive without the viewer is not a keepsake."
|
||||||
|
);
|
||||||
|
}
|
||||||
write_viewer_with_data(&VIEWER_DIR, &mut zip, &data_json, theme_css.as_deref()).await?;
|
write_viewer_with_data(&VIEWER_DIR, &mut zip, &data_json, theme_css.as_deref()).await?;
|
||||||
|
|
||||||
let _ = update_progress(pool, event_id, "html", epoch, 75).await;
|
let _ = update_progress(pool, event_id, "html", epoch, 75).await;
|
||||||
@@ -2323,6 +2335,34 @@ Viel Freude mit den Erinnerungen!\n";
|
|||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
|
/// The viewer must actually be compiled into the binary.
|
||||||
|
///
|
||||||
|
/// `include_dir!` over an empty directory is not an error, and `write_viewer_with_data`
|
||||||
|
/// iterates `dir.files()` — zero files, zero writes, `Ok(())`. So a viewer build that failed
|
||||||
|
/// after Vite emptied its output directory produced a binary whose Memories.zip contains every
|
||||||
|
/// photo and no way to view them, with nothing anywhere reporting it. This is the cheapest
|
||||||
|
/// place to notice, and it runs on every `cargo test`.
|
||||||
|
#[test]
|
||||||
|
fn the_keepsake_viewer_is_compiled_into_this_binary() {
|
||||||
|
let index = super::VIEWER_DIR
|
||||||
|
.get_file("index.html")
|
||||||
|
.expect("static/export-viewer/index.html must be compiled in — run `npm run build` in frontend/export-viewer");
|
||||||
|
// Not just present: substantial. An empty or truncated file would satisfy `get_file` and
|
||||||
|
// still ship a blank keepsake. The real artifact is ~235 KB with the fonts inlined.
|
||||||
|
assert!(
|
||||||
|
index.contents().len() > 50_000,
|
||||||
|
"the compiled-in viewer is only {} bytes — that is not a complete keepsake viewer",
|
||||||
|
index.contents().len()
|
||||||
|
);
|
||||||
|
// And it must be self-contained: the whole point of the inlining is that it opens from
|
||||||
|
// file:// with no network. A `/fonts/...` reference here is the bug shipping again.
|
||||||
|
let html = std::str::from_utf8(index.contents()).expect("viewer is valid UTF-8");
|
||||||
|
assert!(
|
||||||
|
!html.contains("url(/"),
|
||||||
|
"the compiled-in viewer references an external asset — it will 404 silently from file://"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
use super::*;
|
use super::*;
|
||||||
|
|
||||||
const EVT: &str = "11111111-1111-1111-1111-111111111111";
|
const EVT: &str = "11111111-1111-1111-1111-111111111111";
|
||||||
|
|||||||
@@ -107,7 +107,17 @@ export default defineConfig({
|
|||||||
},
|
},
|
||||||
build: {
|
build: {
|
||||||
outDir: fileURLToPath(new URL('../../backend/static/export-viewer', import.meta.url)),
|
outDir: fileURLToPath(new URL('../../backend/static/export-viewer', import.meta.url)),
|
||||||
emptyOutDir: true,
|
// NOT `true`. Vite empties outDir BEFORE generating, so a build that fails late — which is
|
||||||
|
// now a real possibility, since `inlineThemeFonts` calls `this.error` on a keepsake that is
|
||||||
|
// not self-contained — left the directory EMPTY. `include_dir!` over an empty directory
|
||||||
|
// compiles perfectly happily, and `write_viewer_with_data` iterates zero files and returns
|
||||||
|
// Ok, so the next `cargo build` produced a binary whose Memories.zip has the photos and no
|
||||||
|
// viewer at all. Before the guard existed the build could not fail, so neither could this.
|
||||||
|
//
|
||||||
|
// The only output is a single `index.html`, overwritten on every successful build, so
|
||||||
|
// there is nothing to accumulate — and a failed build now leaves the last good artifact in
|
||||||
|
// place instead of deleting it.
|
||||||
|
emptyOutDir: false,
|
||||||
target: 'es2020'
|
target: 'es2020'
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user