From d71fab4859293cc15cdbbf76f197a1a2d6a476ac Mon Sep 17 00:00:00 2001 From: Eugene Volen Date: Tue, 27 Oct 2026 10:26:02 +0200 Subject: [PATCH] fix(lemonade): restore packaged backend pins when a kept alignment pin stops installing A runner's shared Lemonade tree can hold backend_versions.json pins left by an earlier successful ROCm alignment (the active ROCm version plus a newer llama.cpp tag). When the TheRock tarball those pins name is later withdrawn, every `rocm engines install lemonade` fails with HTTP 403: the pin already equals the active ROCm version, so alignment, whose revert is the only other thing that rewrites the pin, is never attempted, and the advertised recovery is unavailable to the prewarm step that does not pass --reinstall. When installing the already-pinned backend fails, restore the two pins that alignment writes (therock.version and llamacpp.rocm-stable) from the digest-verified cached embeddable archive and install once more, forced. The restore never downloads: the same install has just verified that archive into the cache, and if it is gone or fails its digest the restore is skipped and the original install error is reported. The same holds when the pin file exists but cannot be read: only a missing file counts as absent, so an unreadable one is left alone instead of being backed up as nothing (which a failed retry would then have deleted). Every other entry of the file is kept, the file is rewritten through rocm_core::write_atomic, and it is read only after the archive work, so a concurrent writer is neither overwritten blindly nor missing from the backup. The forced retry is wrapped the way alignment's own revert is: the existing ROCm backend directories are set aside first and put back if the retry fails. The ROCm backend entries are listed before the first attempt, and after a failed retry anything not in that list (a partial build the failed attempts left) is removed once the directories are back. If the retry fails, its error is reported and the previous pin file is put back, because any install failure starts the restore, including a transient one. If the pins already hold the packaged values, or the restore itself fails, there is no second attempt and the original error is reported. The warning names the root cause, not the --reinstall advice that the first attempt's error carries. A process killed between the set-aside and the put-back leaves the working backend under a .pre-alignment name, and loses the in-memory copy of the previous pin file; this is the same window alignment's own revert has and is documented rather than recovered, because the runtime tree is shared and a leftover aside cannot be told from another process's install in progress. Signed-off-by: Eugene Volen --- README.md | 9 + crates/rocm-core/src/lib.rs | 10 +- docs/architecture.md | 2 +- docs/engine-plugins.md | 48 ++ docs/manual-testing.md | 17 + docs/testing.md | 20 + engines/lemonade/src/backend_alignment.rs | 656 +++++++++++++++++++++- engines/lemonade/src/install.rs | 638 +++++++++++++++++++++ 8 files changed, 1386 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 94592919f..f663a29a4 100644 --- a/README.md +++ b/README.md @@ -750,6 +750,15 @@ rocm engines shell [--runtime-id KEY | --env-id ID] [--shell PATH] Supported engines: `lemonade`, `vllm`. +`rocm engines install lemonade` also keeps Lemonade's llama.cpp backend paired +with the ROCm SDK in use, and rewrites the two pins it owns +(`therock.version` and `llamacpp.rocm-stable` in `resources/backend_versions.json`) +to do so. If installing an already-pinned backend fails, it restores those two +pins from the cached Lemonade archive (it does not download), prints a warning, +and installs once more; see +[Lemonade backend alignment](https://github.com/ROCm/rocm-cli/blob/main/docs/engine-plugins.md#lemonade-backend-alignment-on-engine-install) for the details and for +`ROCM_CLI_DISABLE_LEMONADE_BACKEND_ALIGNMENT`, which turns both off. + `--yes` approves required system-package installs, such as OpenMPI for vLLM, without asking. diff --git a/crates/rocm-core/src/lib.rs b/crates/rocm-core/src/lib.rs index 7a5f5fd9a..0c1b6183b 100644 --- a/crates/rocm-core/src/lib.rs +++ b/crates/rocm-core/src/lib.rs @@ -337,9 +337,11 @@ pub fn unix_time_millis() -> u128 { .as_millis() } -/// Replace `path`'s content by writing a sibling temp file and renaming it -/// over the destination: an interrupted write (full disk, crash, killed -/// process) can never leave a truncated file in place, and a reader sees +/// Replace `path`'s content with `bytes`, atomically. +/// +/// Writes a sibling temp file and renames it over the destination: an +/// interrupted write (full disk, crash, killed process) can never leave a +/// truncated file in place, and a reader sees /// either the old content or the new content. The destination is NOT removed /// first: `fs::rename` replaces an existing file on both supported platforms /// (it is `MoveFileEx` with `MOVEFILE_REPLACE_EXISTING` on Windows), and @@ -359,7 +361,7 @@ pub fn unix_time_millis() -> u128 { /// - Replacing a destination another process holds open without delete /// sharing: on Windows the rename then fails with a sharing violation, /// this returns that error, and the old destination stays in place. -pub(crate) fn write_atomic(path: &Path, bytes: &[u8], tmp_ext: &str) -> Result<()> { +pub fn write_atomic(path: &Path, bytes: &[u8], tmp_ext: &str) -> Result<()> { let tmp_path = path.with_extension(format!( "{tmp_ext}.tmp-{}-{}", unix_time_millis(), diff --git a/docs/architecture.md b/docs/architecture.md index 76679e03b..134b88c8a 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -111,7 +111,7 @@ What else stays in [`lib.rs`](../crates/rocm-core/src/lib.rs), beside module dec - `TORCH_ALIGNMENT_DISABLED_ENV`/`torch_alignment_disabled` — the one read of the torch-alignment opt-out, shared by the CLI and the vLLM engine so the two cannot disagree. - `VLLM_GPU_MEMORY_UTILIZATION_HINT` — a single wording contract between the CLI, the vLLM engine, and this crate's own `fix-16-vllm-oom` diagnosis; `rocm-core` is the only crate all of them can reach. - **amd-smi binary resolution** (`resolve_amd_smi_binary*`) — belongs in its own `amd_smi.rs` module: locating the tool is a distinct concern that only reads [`managed_runtime.rs`](../crates/rocm-core/src/managed_runtime.rs)'s registry records. The move is deferred to ROCMAI-723 and sequenced after the open [#447](https://github.com/ROCm/rocm-cli/pull/447), which adds amd-smi invocation code to the same stretch of `lib.rs`; moving the family first would make that PR relocate its additions mid-review. Until then `VLLM_GPU_MEMORY_UTILIZATION_HINT` stays physically inside that stretch; the extraction leaves it behind at the root. -- **`write_atomic`** — not permanent. It is the crate-private temp-then-rename writer [`config.rs`](../crates/rocm-core/src/config.rs), [`automation.rs`](../crates/rocm-core/src/automation.rs), and [`service_record.rs`](../crates/rocm-core/src/service_record.rs) use for their atomic replaces. The open [#610](https://github.com/ROCm/rocm-cli/pull/610) adds a shared, stronger atomic writer to this crate for `rocm` and `rocmd`, and the open [#541](https://github.com/ROCm/rocm-cli/pull/541) moves `RocmCliConfig::save` onto it. Once #610 lands, the automation and service-record callers follow and `write_atomic` is deleted (ROCMAI-724). That is a behavior change, not a pure move, so it is not done here. +- **`write_atomic`** — not permanent. It is the temp-then-rename writer [`config.rs`](../crates/rocm-core/src/config.rs), [`automation.rs`](../crates/rocm-core/src/automation.rs), and [`service_record.rs`](../crates/rocm-core/src/service_record.rs) use for their atomic replaces. The open [#610](https://github.com/ROCm/rocm-cli/pull/610) adds a shared, stronger atomic writer to this crate for `rocm` and `rocmd`, and the open [#541](https://github.com/ROCm/rocm-cli/pull/541) moves `RocmCliConfig::save` onto it. It is `pub`, visible to other crates, because `rocm-engine-lemonade` also writes Lemonade's `backend_versions.json` through it, in [`install.rs`](../engines/lemonade/src/install.rs). Once #610 lands, the automation and service-record callers follow; `write_atomic` is deleted only after that Lemonade caller has moved to the shared writer too (ROCMAI-724). That is a behavior change, not a pure move, so it is not done here. Once those two deferred moves land, [`lib.rs`](../crates/rocm-core/src/lib.rs) holds only module declarations, re-exports, and the crate-root primitives above — the end state of the modularization effort for this crate (EAI-7768, planned last: highest fan-in, since every app and engine crate depends on it, but lowest novelty, since the sibling modules already prove the pattern). diff --git a/docs/engine-plugins.md b/docs/engine-plugins.md index b7d7e2b40..61ea362cd 100644 --- a/docs/engine-plugins.md +++ b/docs/engine-plugins.md @@ -66,6 +66,54 @@ was pinned before the attempt — the packaged pin on a fresh install or after `--reinstall`, but whatever alignment last wrote otherwise — rather than being kept. +A pin an earlier alignment kept can go stale: the llama.cpp or TheRock build it +names may later be withdrawn upstream. When the pin already equals the active +ROCm version the install does not attempt another alignment, so if installing +that pinned backend fails, `rocm engines install lemonade` restores the two pins +alignment writes (`therock.version` and `llamacpp.rocm-stable`) in +`resources/backend_versions.json` from the embeddable archive it was extracted +from (digest-verified; the file is rewritten atomically and every other entry +in it is kept) and installs once more against Lemonade's own pins, printing a +warning that says so. This changes what a plain install does: it can rewrite +those two pins and replace the selected backend, where it used to report the +failure and leave both alone. + +The restore never downloads. It reads the cached copy of the embeddable archive +that the same install verified a moment earlier (under `downloads/` next to the +runtime tree). If that copy is gone or no longer matches its digest, the restore +is skipped, a warning says why, and the original install error is reported. The +same applies when the pin file exists but cannot be read (it is left as it is, +never treated as absent), when the pins already hold the packaged values, or when the +restore fails for any other reason: there is no second attempt and the original +install error is reported unchanged. + +The retry is a forced reinstall of the selected backend into the shared tree. +Like alignment's own revert, it sets the existing ROCm backend directories aside +first and puts them back if the retry fails. The ROCm backend entries are listed +before the first attempt, and after a failed retry anything that was not in that +list (a partial build the failed attempts left) is removed once the directories +are back. Any install failure starts the restore, including a transient network +or disk error, so when the second attempt fails too, its error is reported (the +first error is in the warning) and the previous `backend_versions.json` is put +back. + +Known limitation, shared with alignment's own revert: between setting the +directories aside and putting them back, the working backend exists only under a +`.pre-alignment` name. If the process is killed in that window the directory +stays there and nothing recovers it automatically, because the runtime tree is +shared between processes and a leftover aside cannot be told from another +process's install in progress. Run `rocm engines install lemonade --reinstall` +to rebuild the tree. + +The previous `backend_versions.json` is held in memory only while the retry runs. +If the process is killed in that window the packaged pins stay on disk and the +previous bytes are lost; the next install runs alignment against the packaged +pins again. The recovery applies only to the case above (the kept pin equals the +active ROCm version): with no active SDK, an unreadable pin file, or a failed +backend probe, the install uses what the file pins and does not restore. + +Setting the variable below skips this recovery along with the alignment. + Set `ROCM_CLI_DISABLE_LEMONADE_BACKEND_ALIGNMENT` to keep whatever `backend_versions.json` already pins and skip the rewrite — any value works, including an empty one, since the variable being set is the signal: diff --git a/docs/manual-testing.md b/docs/manual-testing.md index f2bbf984f..f823eabe6 100644 --- a/docs/manual-testing.md +++ b/docs/manual-testing.md @@ -293,6 +293,23 @@ in `MSVCP140.dll`), the Visual C++ runtime on the machine is probably older than fix-21-msvc-redist-outdated` prints the update plan and changes nothing. After the update, restart Lemonade and repeat the serve command above. +To check the recovery from a stale Lemonade backend pin (Windows or Linux with +a managed Lemonade runtime already installed and an active managed ROCm SDK): +the recovery only runs when the pinned `therock.version` equals the active +SDK's version; with no active SDK, or a different version, the install takes +other paths and simply fails or realigns. Edit +`resources/backend_versions.json` in the Lemonade runtime tree so that +`therock.version` is the active SDK's version and `llamacpp.rocm-stable` is a +build tag Lemonade does not ship, add an unrelated key, remove the installed +ROCm backend directory under `bin/llamacpp/`, and run +`rocm engines install lemonade`. Expected result: + +- A warning says the pinned llama.cpp backend could not be installed and that the + pins Lemonade ships were restored; the command exits 0 and installs the + packaged backend. +- `therock.version` and `llamacpp.rocm-stable` in `backend_versions.json` hold the + packaged values again, and the unrelated key is still there. + While the log stream is attached, verify detach and stop behave differently: - Press `Ctrl+D`. The stream ends and the shell prompt returns, printing a diff --git a/docs/testing.md b/docs/testing.md index 1a241280c..78d1b4d87 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -1059,6 +1059,26 @@ and Lemonade on non-Windows are all fully supported. Teaching the Windows detached-spawn primitive to carry a value-typed override (so Windows Lemonade can serve public with auth) is tracked as a separate follow-up. +Lemonade backend pin restore focused tests (the restore after a kept alignment +pin stops installing; `docs/engine-plugins.md`, section "Lemonade backend +alignment on engine install"): + +```bash +cargo test -p rocm-engine-lemonade restore +cargo test -p rocm-engine-lemonade equal_pin +cargo test -p rocm-engine-lemonade failed_retry +``` + +They assert each warning together with the real state it describes: the pin +file, the ROCm backend directories (including a partial build the first attempt +left), and that a missing or mismatched cached archive triggers no download. +No Gherkin scenario covers the restore: it needs a runtime tree that already +holds a stale pin and a live backend download, which only a gated hardware lane +has. The Windows Strix Halo e2e lane's prewarm step (`rocm engines install +lemonade` against the shared tree) reaches the restore only on a runner whose +shared tree still holds a stale pin; on a healthy runner the install succeeds and +the restore never runs, so a green lane does not show that it works. + Engine recipe adapter contract focused tests: ```bash diff --git a/engines/lemonade/src/backend_alignment.rs b/engines/lemonade/src/backend_alignment.rs index ac2da648a..edb30aeb9 100644 --- a/engines/lemonade/src/backend_alignment.rs +++ b/engines/lemonade/src/backend_alignment.rs @@ -15,7 +15,7 @@ use std::process::{Command as ProcessCommand, Stdio}; use std::time::Duration; use crate::direct_llama::find_llama_server_binary_for_backend; -use crate::install::{LemonadeInstallManifest, read_manifest}; +use crate::install::{LemonadeInstallManifest, PinFileBackup, read_manifest}; #[cfg(target_os = "windows")] use crate::process::lemonade_process_environment_vars; use crate::process::{ @@ -69,12 +69,17 @@ fn lemonade_backend_alignment_disabled() -> bool { /// installed and active, instead of Lemonade's hardcoded pin (which does not track /// whatever the user separately installed via `rocm install sdk`). /// -/// Best-effort and defensive at every step: if there is no managed rocm-cli SDK, the -/// pinned version already matches, or an aligned attempt cannot be verified to actually -/// resolve its GPU library, this falls back to (or reverts to) today's unmodified -/// pinned-version install — never a regression, and never a silent CPU fallback -/// (AGENTS.md §6): an aligned backend is only kept once [`rocm_backend_resolves`] -/// confirms it. +/// Best-effort and defensive at every step: if there is no managed rocm-cli SDK, or an +/// aligned attempt cannot be verified to actually resolve its GPU library, this falls +/// back to (or reverts to) today's unmodified pinned-version install — never a +/// regression, and never a silent CPU fallback (AGENTS.md §6): an aligned backend is +/// only kept once [`rocm_backend_resolves`] confirms it. +/// +/// When the pinned version already matches there is nothing to align, but the pin may be +/// one an earlier alignment kept and that can no longer be installed. If installing it +/// fails, [`install_pinned_backend_or_restore_packaged_pins`] restores the pins +/// Lemonade ships and installs once more (a forced reinstall), so that branch can +/// rewrite `backend_versions.json` and replace the selected backend. /// /// Returns the aligned version string when one was applied and verified, so the caller /// can surface it in the install response. @@ -90,6 +95,9 @@ pub(crate) fn prepare_llamacpp_backend_for_active_rocm( read_backend_versions_therock_version, best_llamacpp_backend_for_host, install_best_llamacpp_backend, + |manifest| { + crate::install::restore_packaged_backend_versions(manifest, BACKEND_VERSIONS_RESOURCE) + }, |manifest, backend_versions_path, target_version, pinned_version, disabled| { align_llamacpp_backend_to_version( manifest, @@ -136,6 +144,7 @@ fn prepare_llamacpp_backend_for_active_rocm_impl( mut pinned_version_lookup: impl FnMut(&Path) -> Option, mut selected_backend_lookup: impl FnMut(&LemonadeInstallManifest) -> Result>, mut fallback_install: impl FnMut(&mut LemonadeInstallManifest, bool) -> Result<()>, + restore_packaged_pins: impl FnMut(&LemonadeInstallManifest) -> Result>, mut align: impl FnMut( &mut LemonadeInstallManifest, &Path, @@ -188,8 +197,17 @@ fn prepare_llamacpp_backend_for_active_rocm_impl( return Ok(None); }; if pinned_version == target_version { - fallback_install(manifest, false)?; - return Ok(None); + // Either Lemonade's own pin already is the active ROCm version, or an + // earlier run aligned the pin and it was kept. The second case can go + // stale: the build the pin names may since have been withdrawn, and + // this branch never revisits the pin, so the install fails on every + // run until something restores it. + return install_pinned_backend_or_restore_packaged_pins( + manifest, + &mut fallback_install, + restore_packaged_pins, + &mut |message| eprintln!("Warning: {message}"), + ); } // Verification below requires the backend Lemonade selects to be `rocm` @@ -225,6 +243,114 @@ fn prepare_llamacpp_backend_for_active_rocm_impl( ) } +/// Install the backend `backend_versions.json` already pins. When that fails, +/// put the pin file back to what the embeddable ships and install once more +/// against it. +/// +/// This is the recovery for a pin an earlier alignment kept and that can no +/// longer be installed. It is deliberately a no-op when the file already holds +/// the packaged values (the pin is genuinely Lemonade's own, so a second +/// attempt would fail the same way and only hide the first error behind a +/// retry). +/// +/// The retry is a forced reinstall into the shared tree, so it is wrapped the way +/// alignment's own revert is: the existing ROCm backend directories are set aside +/// first and put back if the retry fails, so a failed retry does not leave the host +/// without a ROCm backend, and a retry that succeeds does not leave two builds side +/// by side for `find_binary_in` to choose between. +/// +/// The ROCm backend entries are listed before the first attempt, not set aside +/// before it (that would move a working backend on every plain install). The +/// first attempt can itself leave a partial build directory, and the set-aside +/// that follows it would carry that partial along and put it "back" if the retry +/// fails too. So after a failed retry, once the directories are put back, every +/// ROCm entry that was not in the listing is removed. +/// +/// Known limitation, shared with alignment's own revert: between the set-aside +/// and the put-back the working backend lives only under a `.pre-alignment` +/// name, and a process killed in that window leaves it there. Nothing recovers it +/// automatically, because the runtime tree is shared between processes and a +/// leftover aside cannot be told from another process's install in progress. +/// +/// The previous pin file is held in memory only, so a process killed during the +/// forced retry leaves the restored (packaged) pins on disk and the previous bytes +/// are lost. That is accepted: the two pins are Lemonade's own defaults, the next +/// install re-runs alignment against them, and persisting the backup to a file in +/// the shared tree would reintroduce the orphan problem above. +/// +/// Scope: only the branch where the kept pin equals the active ROCm version calls +/// this. Other `fallback_install` call sites (no active SDK, an unreadable pin, a +/// failed backend probe) install what the file pins and do not restore. +/// +/// What is reported when it does not recover: +/// - a failed restore reports the original install error, which is the one the +/// user can act on; +/// - a failed retry reports the retry's error (the first error was already +/// printed in the warning), puts the backend directories back, and puts the +/// previous pin file back. Any install failure starts a restore, including a +/// transient network or disk error, so a pin that was fine must not stay +/// discarded by a retry that failed for another reason. +/// +/// `warn` receives each message printed to the user, so the message and the +/// resulting pin-file state are asserted together in the tests. +fn install_pinned_backend_or_restore_packaged_pins( + manifest: &mut LemonadeInstallManifest, + fallback_install: &mut impl FnMut(&mut LemonadeInstallManifest, bool) -> Result<()>, + mut restore_packaged_pins: impl FnMut(&LemonadeInstallManifest) -> Result>, + warn: &mut impl FnMut(String), +) -> Result> { + let entries_before = list_rocm_llamacpp_backend_entries(manifest); + let Err(install_error) = fallback_install(manifest, false) else { + return Ok(None); + }; + match restore_packaged_pins(manifest) { + Ok(Some(backup)) => { + // The root cause, not the whole chain: the chain carries the advice to + // run `--reinstall` and retry, which this step is about to do itself. + warn(format!( + "the llama.cpp backend pinned in {BACKEND_VERSIONS_RESOURCE} could not be \ + installed ({}); restored the pins Lemonade ships and trying again.", + install_error.root_cause() + )); + // Forced: the failed attempt may have left a half-installed backend + // for the withdrawn pin. + let aside_dirs = set_aside_rocm_llamacpp_backend_dirs(manifest); + match fallback_install(manifest, true) { + Ok(()) => { + resolve_rocm_llamacpp_backend_dirs_aside(manifest, &aside_dirs, true); + Ok(None) + } + Err(retry_error) => { + resolve_rocm_llamacpp_backend_dirs_aside(manifest, &aside_dirs, false); + // What the put-back restored includes whatever the first + // attempt left behind; only what existed before it is kept. + prune_rocm_llamacpp_backend_entries_not_in(manifest, &entries_before); + match backup.put_back() { + Ok(()) => warn(format!( + "the pins Lemonade ships could not be installed either; put the \ + previous {BACKEND_VERSIONS_RESOURCE} back." + )), + Err(put_back_error) => warn(format!( + "the pins Lemonade ships could not be installed either, and the \ + previous {BACKEND_VERSIONS_RESOURCE} could not be put back: \ + {put_back_error:#}" + )), + } + Err(retry_error) + } + } + } + Ok(None) => Err(install_error), + Err(restore_error) => { + warn(format!( + "could not restore the pins Lemonade ships in {BACKEND_VERSIONS_RESOURCE}: \ + {restore_error:#}" + )); + Err(install_error) + } + } +} + /// The Tier 1 / Tier 2 / revert state machine, isolated from the config and active-SDK /// lookups above so it can be exercised in tests against a temp `backend_versions.json` /// with the install/align/latest-tag steps injected, instead of spawning a real @@ -1051,6 +1177,55 @@ fn resolve_rocm_llamacpp_backend_dirs_aside( } } +/// The ROCm-family backend directories under `bin/llamacpp/` and the entries +/// directly inside each (Lemonade extracts every build into its own +/// build-numbered subdirectory), as they are right now. +fn list_rocm_llamacpp_backend_entries( + manifest: &LemonadeInstallManifest, +) -> std::collections::BTreeSet { + let llamacpp_dir = manifest.runtime_dir.join("bin").join("llamacpp"); + let mut entries = std::collections::BTreeSet::new(); + for backend in ROCM_LLAMACPP_BACKEND_DIRS { + let dir = llamacpp_dir.join(backend); + if !dir.exists() { + continue; + } + entries.insert(dir.clone()); + if let Ok(children) = fs::read_dir(&dir) { + entries.extend(children.filter_map(Result::ok).map(|child| child.path())); + } + } + entries +} + +/// Remove every ROCm-family backend directory, and every entry directly inside +/// one, that is not in `keep` (from [`list_rocm_llamacpp_backend_entries`]). +/// Best-effort, like [`resolve_rocm_llamacpp_backend_dirs_aside`]. +fn prune_rocm_llamacpp_backend_entries_not_in( + manifest: &LemonadeInstallManifest, + keep: &std::collections::BTreeSet, +) { + let llamacpp_dir = manifest.runtime_dir.join("bin").join("llamacpp"); + for backend in ROCM_LLAMACPP_BACKEND_DIRS { + let dir = llamacpp_dir.join(backend); + if !dir.exists() { + continue; + } + if !keep.contains(&dir) { + let _ = fs::remove_dir_all(&dir); + continue; + } + let Ok(children) = fs::read_dir(&dir) else { + continue; + }; + for child in children.filter_map(Result::ok).map(|child| child.path()) { + if !keep.contains(&child) { + let _ = fs::remove_dir_all(&child).or_else(|_| fs::remove_file(&child)); + } + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -1476,6 +1651,7 @@ mod tests { ); Ok(()) }, + |_manifest| panic!("this path never restores the packaged pins"), |_manifest, _path, _target, _pinned, _disabled| { panic!("disabled means align must never be reached") }, @@ -1509,6 +1685,7 @@ mod tests { ); Ok(()) }, + |_manifest| panic!("this path never restores the packaged pins"), |_manifest, _path, _target, _pinned, _disabled| { panic!("unsupported platform means align must never be reached") }, @@ -1540,6 +1717,7 @@ mod tests { ); Ok(()) }, + |_manifest| panic!("this path never restores the packaged pins"), |_manifest, _path, _target, _pinned, _disabled| { panic!("a probe error must skip align entirely") }, @@ -1548,6 +1726,466 @@ mod tests { assert_eq!(result.unwrap(), None); } + /// Runs the pin-equals-target branch of + /// `prepare_llamacpp_backend_for_active_rocm_impl` with scripted install and + /// restore steps, returning the result and the `force_reinstall` flag of every + /// install attempt in order. + fn prepare_with_equal_pin( + mut installs: Vec>, + restore: Result, + ) -> (Result>, Vec, usize) { + installs.reverse(); + let mut attempts = Vec::new(); + let mut restores = 0; + let mut restore = Some(restore); + let mut manifest = test_manifest(PathBuf::from("unused-runtime-dir")); + let result = prepare_llamacpp_backend_for_active_rocm_impl( + &mut manifest, + false, + false, + || Ok(Some("7.14.1".to_owned())), + |_path| Some("7.14.1".to_owned()), + |_manifest| panic!("an equal pin never reaches the backend probe"), + |_manifest, force_reinstall| { + attempts.push(force_reinstall); + installs.pop().expect("more install attempts than scripted") + }, + |_manifest| { + restores += 1; + restore + .take() + .expect("restore ran more than once") + .map(|changed| { + changed.then(|| { + PinFileBackup::new(PathBuf::from("unused-pin-file-backup"), None) + }) + }) + }, + |_manifest, _path, _target, _pinned, _disabled| { + panic!("an equal pin never reaches align") + }, + ); + (result, attempts, restores) + } + + #[test] + fn equal_pin_that_installs_never_touches_the_pin_file() { + let (result, attempts, restores) = + prepare_with_equal_pin(vec![Ok(())], Err(anyhow!("must not restore"))); + + assert_eq!(result.unwrap(), None); + assert_eq!(attempts, [false]); + assert_eq!(restores, 0); + } + + #[test] + fn equal_pin_kept_from_an_earlier_alignment_is_restored_when_it_no_longer_installs() { + // The CI failure this pins: a runner's shared Lemonade tree kept + // `therock.version` = the active ROCm version plus a newer llama.cpp tag + // from an alignment run days earlier. The ROCm tarball that pin names has + // since been withdrawn, so every install failed outright -- and the pin + // equals the target, so alignment, whose revert is the only other thing + // that rewrites the pin, was never reached. + let (result, attempts, restores) = prepare_with_equal_pin( + vec![ + Err(anyhow!("HTTP error 403 for the pinned tarball")), + Ok(()), + ], + Ok(true), + ); + + assert_eq!(result.unwrap(), None); + assert_eq!(restores, 1); + assert_eq!( + attempts, + [false, true], + "the retry after restoring the pins must force a fresh install" + ); + } + + #[test] + fn equal_pin_that_is_already_the_packaged_pin_reports_the_original_failure() { + // Nothing to restore means a second attempt would fail identically. + let (result, attempts, restores) = prepare_with_equal_pin( + vec![Err(anyhow!("HTTP error 403 for the pinned tarball"))], + Ok(false), + ); + + let error = result.unwrap_err(); + assert!(format!("{error:#}").contains("HTTP error 403")); + assert_eq!(attempts, [false]); + assert_eq!(restores, 1); + } + + #[test] + fn equal_pin_whose_restore_fails_reports_the_original_failure() { + let (result, attempts, restores) = prepare_with_equal_pin( + vec![Err(anyhow!("HTTP error 403 for the pinned tarball"))], + Err(anyhow!("archive digest mismatch")), + ); + + let error = result.unwrap_err(); + assert!(format!("{error:#}").contains("HTTP error 403")); + assert!(!format!("{error:#}").contains("digest mismatch")); + assert_eq!(attempts, [false]); + assert_eq!(restores, 1, "the restore must have been attempted"); + } + + /// Drives the restore flow against a real pin file: the restore step + /// writes the packaged pins and hands back a backup of what it replaced. + /// Returns the result, every message shown to the user, the `force` + /// flag of each install attempt, and the pin file's final content. + fn restore_flow_with_real_pin_file( + installs: Vec>, + ) -> (Result>, Vec, Vec, String) { + let dir = scratch_dir("restore-flow-real-pin-file"); + let pin_file = dir.join("backend_versions.json"); + fs::write(&pin_file, "KEPT").unwrap(); + let mut installs = installs; + installs.reverse(); + let mut attempts = Vec::new(); + let mut warnings = Vec::new(); + let mut manifest = test_manifest(dir.clone()); + let result = install_pinned_backend_or_restore_packaged_pins( + &mut manifest, + &mut |_manifest, force| { + attempts.push(force); + installs.pop().expect("more install attempts than scripted") + }, + |_manifest| { + let previous = fs::read(&pin_file).unwrap(); + fs::write(&pin_file, "PACKAGED").unwrap(); + Ok(Some(PinFileBackup::new(pin_file.clone(), Some(previous)))) + }, + &mut |message| warnings.push(message), + ); + let content = fs::read_to_string(&pin_file).unwrap(); + fs::remove_dir_all(&dir).ok(); + (result, warnings, attempts, content) + } + + #[test] + fn the_restored_message_is_true_of_the_pin_file_when_the_retry_succeeds() { + let (result, warnings, attempts, content) = restore_flow_with_real_pin_file(vec![ + Err(anyhow!("HTTP error 403 for the pinned tarball")), + Ok(()), + ]); + + assert_eq!(result.unwrap(), None); + assert_eq!(attempts, [false, true]); + assert_eq!(warnings.len(), 1, "{warnings:?}"); + assert!( + warnings[0].contains("restored the pins Lemonade ships") + && warnings[0].contains("HTTP error 403"), + "{warnings:?}" + ); + assert_eq!(content, "PACKAGED", "the message says restored, so it is"); + } + + #[test] + fn a_failed_retry_puts_the_previous_pin_file_back_and_says_so() { + // The first failure may have been transient, so a pin that was fine must + // not stay discarded by a retry that failed for another reason. + let (result, warnings, attempts, content) = restore_flow_with_real_pin_file(vec![ + Err(anyhow!("connection reset")), + Err(anyhow!("HTTP 404 for the packaged tarball")), + ]); + + let error = format!("{:#}", result.unwrap_err()); + assert!( + error.contains("HTTP 404 for the packaged tarball"), + "{error}" + ); + assert!( + !error.contains("connection reset"), + "the retry's error is reported; the first was printed in the warning: {error}" + ); + assert_eq!(attempts, [false, true]); + assert_eq!(warnings.len(), 2, "{warnings:?}"); + assert!(warnings[0].contains("connection reset"), "{warnings:?}"); + assert!(warnings[1].contains("put the previous"), "{warnings:?}"); + assert_eq!(content, "KEPT", "the message says put back, so it is"); + } + + /// What `restore_flow` observed. + struct RestoreFlow { + result: Result>, + warnings: Vec, + attempts: Vec, + pin_file: String, + /// During the forced retry, the existing ROCm backend directory was + /// moved aside and nothing stood in its place yet. + aside_during_retry: bool, + dir: PathBuf, + } + + /// Drives `install_pinned_backend_or_restore_packaged_pins` against a real + /// pin file (holding `KEPT`) and a real ROCm backend directory + /// (`bin/llamacpp/rocm-stable/old-build`). The restore step is supplied by + /// the test; a forced retry that succeeds installs `rocm-stable/new-build`, + /// and one that fails leaves a stray `rocm-nightly/stray`, the way a failed + /// install can land its output under a different ROCm-family name. + fn restore_flow( + installs: Vec>, + restore: impl FnOnce(&Path, &Path) -> Result>, + ) -> RestoreFlow { + restore_flow_with_partial(false, installs, restore) + } + + /// [`restore_flow`], where the first attempt can also leave a half-installed + /// build behind: `rocm-stable/partial-build` next to the existing build and a + /// whole `rocm/partial` directory, before it fails. + fn restore_flow_with_partial( + first_attempt_leaves_partial: bool, + installs: Vec>, + restore: impl FnOnce(&Path, &Path) -> Result>, + ) -> RestoreFlow { + let dir = scratch_dir("restore-flow"); + let pin_file = dir.join("backend_versions.json"); + fs::write(&pin_file, "KEPT").unwrap(); + let llamacpp = dir.join("bin").join("llamacpp"); + fs::create_dir_all(llamacpp.join("rocm-stable")).unwrap(); + fs::write(llamacpp.join("rocm-stable").join("old-build"), "old").unwrap(); + let mut installs = installs; + installs.reverse(); + let mut attempts = Vec::new(); + let mut warnings = Vec::new(); + let mut aside_during_retry = false; + let mut restore = Some(restore); + let mut manifest = test_manifest(dir.clone()); + let result = install_pinned_backend_or_restore_packaged_pins( + &mut manifest, + &mut |_manifest, force| { + attempts.push(force); + let outcome = installs.pop().expect("more install attempts than scripted"); + if !force && first_attempt_leaves_partial { + fs::create_dir_all(llamacpp.join("rocm-stable").join("partial-build")).unwrap(); + fs::write( + llamacpp + .join("rocm-stable") + .join("partial-build") + .join("half"), + "half", + ) + .unwrap(); + fs::create_dir_all(llamacpp.join("rocm")).unwrap(); + fs::write(llamacpp.join("rocm").join("partial"), "half").unwrap(); + } + if force { + aside_during_retry = !llamacpp.join("rocm-stable").exists() + && llamacpp.join("rocm-stable.pre-alignment").exists(); + if outcome.is_ok() { + fs::create_dir_all(llamacpp.join("rocm-stable")).unwrap(); + fs::write(llamacpp.join("rocm-stable").join("new-build"), "new").unwrap(); + } else { + fs::create_dir_all(llamacpp.join("rocm-nightly")).unwrap(); + fs::write(llamacpp.join("rocm-nightly").join("stray"), "stray").unwrap(); + } + } + outcome + }, + |_manifest| restore.take().expect("restore ran once")(&pin_file, &dir), + &mut |message| warnings.push(message), + ); + let pin_file_content = fs::read_to_string(&pin_file).unwrap(); + RestoreFlow { + result, + warnings, + attempts, + pin_file: pin_file_content, + aside_during_retry, + dir, + } + } + + /// A restore step that behaves like the real one: writes the packaged + /// pins and hands back a backup of what it replaced. + fn restore_writing_packaged(pin_file: &Path, _dir: &Path) -> Result> { + let previous = fs::read(pin_file).unwrap(); + fs::write(pin_file, "PACKAGED").unwrap(); + Ok(Some(PinFileBackup::new( + pin_file.to_path_buf(), + Some(previous), + ))) + } + + #[test] + fn a_failed_restore_says_so_reports_the_original_error_and_changes_nothing() { + let flow = restore_flow( + vec![Err(anyhow!("HTTP error 403 for the pinned tarball"))], + |_pin_file, _dir| Err(anyhow!("archive digest mismatch")), + ); + + let error = format!("{:#}", flow.result.unwrap_err()); + assert!(error.contains("HTTP error 403"), "{error}"); + assert!(!error.contains("digest mismatch"), "{error}"); + assert_eq!(flow.warnings.len(), 1, "{:?}", flow.warnings); + assert!( + flow.warnings[0].contains("could not restore the pins Lemonade ships") + && flow.warnings[0].contains("archive digest mismatch"), + "{:?}", + flow.warnings + ); + assert_eq!(flow.attempts, [false], "no retry after a failed restore"); + // The message claims nothing was restored: the pin file and the backend + // directory are as they were. + assert_eq!(flow.pin_file, "KEPT"); + assert!(flow.dir.join("bin/llamacpp/rocm-stable/old-build").exists()); + fs::remove_dir_all(&flow.dir).ok(); + } + + #[test] + fn a_failed_put_back_says_so_and_leaves_the_restored_pins_in_place() { + // The backup points at a directory that does not exist, so writing the + // previous bytes back fails. + let flow = restore_flow( + vec![ + Err(anyhow!("connection reset")), + Err(anyhow!("HTTP 404 for the packaged tarball")), + ], + |pin_file, dir| { + let previous = fs::read(pin_file).unwrap(); + fs::write(pin_file, "PACKAGED").unwrap(); + Ok(Some(PinFileBackup::new( + dir.join("no-such-dir").join("backend_versions.json"), + Some(previous), + ))) + }, + ); + + assert!( + format!("{:#}", flow.result.unwrap_err()).contains("HTTP 404"), + "the retry's error is the one reported" + ); + assert_eq!(flow.warnings.len(), 2, "{:?}", flow.warnings); + assert!( + flow.warnings[1].contains("could not be put back"), + "{:?}", + flow.warnings + ); + // The message says the previous file could not be put back, and it was + // not: the restored pins are still what is on disk. + assert_eq!(flow.pin_file, "PACKAGED"); + fs::remove_dir_all(&flow.dir).ok(); + } + + #[test] + fn the_retry_sets_the_backend_directories_aside_and_a_success_discards_them() { + let flow = restore_flow( + vec![Err(anyhow!("HTTP error 403")), Ok(())], + restore_writing_packaged, + ); + + assert!(flow.result.is_ok()); + assert_eq!(flow.attempts, [false, true]); + assert!( + flow.aside_during_retry, + "the existing ROCm backend must be moved aside before the forced reinstall" + ); + let llamacpp = flow.dir.join("bin").join("llamacpp"); + assert!(llamacpp.join("rocm-stable").join("new-build").exists()); + assert!( + !llamacpp.join("rocm-stable").join("old-build").exists(), + "two builds must not be left side by side" + ); + assert!(!llamacpp.join("rocm-stable.pre-alignment").exists()); + fs::remove_dir_all(&flow.dir).ok(); + } + + #[test] + fn a_failed_retry_puts_the_backend_directories_back() { + let flow = restore_flow( + vec![ + Err(anyhow!("connection reset")), + Err(anyhow!("HTTP 404 for the packaged tarball")), + ], + restore_writing_packaged, + ); + + assert!(flow.result.is_err()); + assert!(flow.aside_during_retry); + let llamacpp = flow.dir.join("bin").join("llamacpp"); + assert!( + llamacpp.join("rocm-stable").join("old-build").exists(), + "a failed retry must not leave the host without its ROCm backend" + ); + assert!( + !llamacpp.join("rocm-nightly").exists(), + "a stray directory from the failed install must not shadow the restored build" + ); + assert!(!llamacpp.join("rocm-stable.pre-alignment").exists()); + assert_eq!(flow.pin_file, "KEPT", "the previous pins are back too"); + fs::remove_dir_all(&flow.dir).ok(); + } + + #[test] + fn a_failed_retry_does_not_put_back_what_the_first_attempt_left_half_installed() { + let flow = restore_flow_with_partial( + true, + vec![ + Err(anyhow!("connection reset")), + Err(anyhow!("HTTP 404 for the packaged tarball")), + ], + restore_writing_packaged, + ); + + assert!(flow.result.is_err()); + let llamacpp = flow.dir.join("bin").join("llamacpp"); + assert!( + llamacpp.join("rocm-stable").join("old-build").exists(), + "the build that was there before the first attempt is back" + ); + assert!( + !llamacpp.join("rocm-stable").join("partial-build").exists(), + "a partial build from the first attempt must not be put back" + ); + assert!( + !llamacpp.join("rocm").exists() && !llamacpp.join("rocm-nightly").exists(), + "neither a directory the first attempt created nor the retry's stray may stay" + ); + assert!(!llamacpp.join("rocm-stable.pre-alignment").exists()); + assert_eq!(flow.pin_file, "KEPT"); + fs::remove_dir_all(&flow.dir).ok(); + } + + #[test] + fn the_restored_warning_names_the_root_cause_not_the_reinstall_advice() { + // The install path wraps its error in advice to run `--reinstall`; the + // warning says the CLI is already doing the equivalent, so it must not + // repeat that advice. + let first = anyhow!("HTTP error 403 for the pinned tarball") + .context("run `rocm engines install lemonade --reinstall` and then retry `rocm serve`"); + let flow = restore_flow(vec![Err(first), Ok(())], restore_writing_packaged); + + assert!(flow.result.is_ok()); + assert_eq!(flow.warnings.len(), 1, "{:?}", flow.warnings); + assert!( + flow.warnings[0].contains("HTTP error 403"), + "{:?}", + flow.warnings + ); + assert!( + !flow.warnings[0].contains("--reinstall"), + "{:?}", + flow.warnings + ); + fs::remove_dir_all(&flow.dir).ok(); + } + + #[test] + fn equal_pin_still_failing_after_the_restore_reports_the_retry_failure() { + let (result, attempts, _restores) = prepare_with_equal_pin( + vec![ + Err(anyhow!("first failure")), + Err(anyhow!("failure against the packaged pins")), + ], + Ok(true), + ); + + assert!(format!("{:#}", result.unwrap_err()).contains("packaged pins")); + assert_eq!(attempts, [false, true]); + } + #[test] fn align_tier1_success_keeps_target_version_pinned() { let dir = scratch_dir("align-tier1-success"); diff --git a/engines/lemonade/src/install.rs b/engines/lemonade/src/install.rs index 4fa40b22d..ab50ff705 100644 --- a/engines/lemonade/src/install.rs +++ b/engines/lemonade/src/install.rs @@ -240,6 +240,215 @@ pub(crate) fn write_manifest(paths: &AppPaths, manifest: &LemonadeInstallManifes Ok(()) } +/// Put `resources/backend_versions.json` back to what the cached embeddable +/// archive ships. Returns `None` when the file already held the packaged +/// values, and otherwise a [`PinFileBackup`] that puts the previous bytes back. +/// +/// That file is the one piece of the extracted tree rocm-cli itself edits (ROCm +/// backend alignment pins a ROCm version and a llama.cpp build into it), and +/// the edit outlives the process: the runtime tree is shared and is only +/// re-extracted on a version change or `--reinstall`. A pin kept from an +/// earlier successful alignment can therefore name assets that have since +/// been withdrawn, and nothing but this restore (or a full `--reinstall`) +/// clears it. Only that one file is rewritten, so backends already installed +/// in the tree, and any server running from it, are left alone. +/// +/// This runs only after an install has already failed, so it never downloads: +/// the archive is read from the cache `prepare_embeddable_with` filled earlier +/// in the same install, and its digest is verified before any byte of it is +/// trusted. When the cached archive is gone or does not verify, this fails and +/// the caller reports the original install error. +/// +/// The pin file is read only after the archive work is done, immediately +/// before the rewrite, so a pin another process wrote in the meantime is +/// neither overwritten blindly nor missing from the backup. +pub(crate) fn restore_packaged_backend_versions( + manifest: &LemonadeInstallManifest, + resource: &str, +) -> Result> { + restore_packaged_backend_versions_with(manifest, resource, embeddable_archive_sha256(), || {}) +} + +/// [`restore_packaged_backend_versions`] with the archive digest injectable. +/// `after_archive` runs once the archive work is done and before the pin file +/// is read; tests use it to stand in for a writer that gets in between. +fn restore_packaged_backend_versions_with( + manifest: &LemonadeInstallManifest, + resource: &str, + expected_sha256: &str, + after_archive: impl FnOnce(), +) -> Result> { + let root = manifest + .runtime_dir + .parent() + .context("Lemonade runtime directory has no parent to hold its archive cache")?; + let archive = root + .join("downloads") + .join(embeddable_archive_name(&manifest.version)); + let target = manifest.runtime_dir.join(resource); + + // Checked first so that a missing cache creates nothing at all. + if !archive.is_file() { + bail!( + "the embeddable archive is not cached at {}; not downloading it while recovering from a failed install", + archive.display() + ); + } + // The download step can only fail: a cached archive that does not verify is + // dropped by `ensure_cached_archive` and is not replaced from the network. + let guard = ensure_cached_archive("", &archive, expected_sha256, |_, _| { + bail!( + "the cached embeddable archive {} does not match its digest; not downloading it while recovering from a failed install", + archive.display() + ) + })?; + let extract_root = root + .join("extract") + .join(format!("{}-packaged-pins", current_unix_millis())); + fs::create_dir_all(&extract_root)?; + let packaged = (|| -> Result> { + extract_archive(&archive, &extract_root)?; + let embeddable_root = find_embeddable_root(&extract_root)?; + let packaged_path = embeddable_root.join(resource); + fs::read(&packaged_path).with_context(|| format!("embeddable archive has no {resource}")) + })(); + fs::remove_dir_all(&extract_root).ok(); + drop(guard); + let packaged = packaged?; + after_archive(); + + // Read only now, after the archive work above, and used for both the + // computation and the backup: a pin file written by another process + // while the archive was being verified and extracted is what gets + // restored over and backed up, not a stale earlier read. + // Only "no such file" means there was nothing to keep. Any other failure + // (a sharing violation from a concurrent writer, a permission error) says + // nothing about what the file holds, and treating it as absent would make + // the backup delete a real pin file if the retry fails. Like the other + // cache-only skips above, it fails the restore, so nothing is rewritten and + // the caller reports the original install error. + let current = match fs::read(&target) { + Ok(bytes) => Some(bytes), + Err(err) if err.kind() == std::io::ErrorKind::NotFound => None, + Err(err) => { + return Err(err).with_context(|| { + format!( + "could not read {} to back it up; leaving it as it is", + target.display() + ) + }); + } + }; + + // Only the two pins backend alignment writes are restored, so an edit to any + // other entry of the file survives and a file that already holds the packaged + // values for those two is not drift. Compared as JSON, not bytes: an edit + // re-serializes the file, so the same values can sit in different bytes. + let packaged_value: serde_json::Value = serde_json::from_slice(&packaged) + .with_context(|| format!("{resource} in the embeddable archive is not valid JSON"))?; + let restored = match current + .as_deref() + .and_then(|bytes| serde_json::from_slice::(bytes).ok()) + { + Some(mut value) => { + if alignment_pins(&value) == alignment_pins(&packaged_value) { + return Ok(None); + } + for (section, key) in ALIGNMENT_PIN_KEYS { + set_or_remove_pin(&mut value, section, key, &packaged_value); + } + serde_json::to_vec_pretty(&value)? + } + // Missing (or not JSON): there is nothing of the user's to keep. + None => packaged, + }; + rocm_core::write_atomic(&target, &restored, "pins") + .with_context(|| format!("failed to restore {}", target.display()))?; + Ok(Some(PinFileBackup { + target, + previous: current, + })) +} + +/// The two entries of `resources/backend_versions.json` that backend alignment +/// writes: the ROCm version a backend is built for and the llama.cpp build tag. +const ALIGNMENT_PIN_KEYS: [(&str, &str); 2] = [("therock", "version"), ("llamacpp", "rocm-stable")]; + +fn alignment_pins(value: &serde_json::Value) -> [Option<&serde_json::Value>; 2] { + ALIGNMENT_PIN_KEYS.map(|(section, key)| value.get(section).and_then(|s| s.get(key))) +} + +/// Gives `section.key` of `value` the value it has in `packaged`, or removes it +/// when `packaged` has none, leaving every other entry as it was. +fn set_or_remove_pin( + value: &mut serde_json::Value, + section: &str, + key: &str, + packaged: &serde_json::Value, +) { + let packaged_pin = packaged.get(section).and_then(|s| s.get(key)); + match packaged_pin { + Some(pin) => { + if !value.is_object() { + *value = serde_json::json!({}); + } + if let Some(object) = value.as_object_mut() { + let section_value = object + .entry(section.to_owned()) + .or_insert_with(|| serde_json::json!({})); + if !section_value.is_object() { + *section_value = serde_json::json!({}); + } + if let Some(section_object) = section_value.as_object_mut() { + section_object.insert(key.to_owned(), pin.clone()); + } + } + } + None => { + if let Some(section_object) = value.get_mut(section).and_then(|s| s.as_object_mut()) { + section_object.remove(key); + } + } + } +} + +/// What `resources/backend_versions.json` held before +/// [`restore_packaged_backend_versions`] rewrote it. Written back through +/// `rocm_core::write_atomic`, like the restore itself. +/// +/// A restore is attempted after any failed install, including a transient +/// network or disk error, so a restore whose retry fails too must not leave a +/// pin that was fine discarded. +#[derive(Debug)] +pub(crate) struct PinFileBackup { + target: PathBuf, + /// `None` when the file did not exist, so putting it back removes it. + previous: Option>, +} + +impl PinFileBackup { + /// For tests that stand in for the restore step; the real restore builds + /// the struct itself. + #[cfg(test)] + pub(crate) const fn new(target: PathBuf, previous: Option>) -> Self { + Self { target, previous } + } + + /// Writes the previous bytes back, or removes the file if there were none. + pub(crate) fn put_back(self) -> Result<()> { + match self.previous { + Some(bytes) => rocm_core::write_atomic(&self.target, &bytes, "pins") + .with_context(|| format!("failed to put back {}", self.target.display())), + None => match fs::remove_file(&self.target) { + Err(err) if err.kind() != std::io::ErrorKind::NotFound => { + Err(err).with_context(|| format!("failed to remove {}", self.target.display())) + } + _ => Ok(()), + }, + } + } +} + fn lemond_path_in(runtime_dir: &Path) -> PathBuf { runtime_dir.join(platform_binary_name("lemond")) } @@ -1646,6 +1855,435 @@ mod tests { ); } + /// Writes an embeddable archive in the host's format holding the two + /// binaries and a packaged `resources/backend_versions.json`. + fn write_embeddable_with_pins(archive: &Path, pins: &str) { + let files = [ + (platform_binary_name("lemond"), b"lemond".to_vec()), + (platform_binary_name("lemonade"), b"lemonade".to_vec()), + ( + "resources/backend_versions.json".to_owned(), + pins.as_bytes().to_vec(), + ), + ]; + let file = fs::File::create(archive).unwrap(); + if runtime_is_windows() { + let mut writer = zip::ZipWriter::new(file); + let options = zip::write::SimpleFileOptions::default(); + for (name, body) in &files { + writer + .start_file(format!("embeddable/{name}"), options) + .unwrap(); + writer.write_all(body).unwrap(); + } + writer.finish().unwrap(); + } else { + let encoder = flate2::write::GzEncoder::new(file, flate2::Compression::fast()); + let mut builder = tar::Builder::new(encoder); + for (name, body) in &files { + let mut header = tar::Header::new_ustar(); + header.set_size(body.len() as u64); + header.set_mode(0o755); + header.set_entry_type(tar::EntryType::Regular); + header.set_path(format!("embeddable/{name}")).unwrap(); + header.set_cksum(); + builder.append(&header, body.as_slice()).unwrap(); + } + builder.into_inner().unwrap().finish().unwrap(); + } + } + + #[test] + fn restore_packaged_backend_versions_replaces_only_a_pin_file_that_drifted() { + let dir = scratch_dir("restore-packaged-pins"); + let root = dir.join("lemonade"); + let runtime_dir = runtime_dir_in(&root); + fs::create_dir_all(runtime_dir.join("resources")).unwrap(); + let pins = runtime_dir.join("resources/backend_versions.json"); + let backend = runtime_dir.join("backends").join("rocm"); + fs::create_dir_all(&backend).unwrap(); + fs::write(backend.join("llama-server"), b"installed backend").unwrap(); + + let packaged = r#"{"therock":{"version":"7.13.0"},"llamacpp":{"rocm-stable":"b9752"}}"#; + let source = dir.join("source-archive"); + write_embeddable_with_pins(&source, packaged); + let sha256 = format!("{:x}", Sha256::digest(fs::read(&source).unwrap())); + let manifest = LemonadeInstallManifest { + env_id: "test".to_owned(), + version: "11.5.1".to_owned(), + runtime_dir: runtime_dir.clone(), + lemond: lemond_path_in(&runtime_dir), + lemonade: lemonade_path_in(&runtime_dir), + backend_recipe: LLAMACPP_RECIPE.to_owned(), + backend_name: ROCM_BACKEND_NAME.to_owned(), + installed_at_unix_ms: 0, + }; + cache_archive(&root, &source); + let restore = |manifest: &LemonadeInstallManifest| { + restore_packaged_backend_versions_with( + manifest, + "resources/backend_versions.json", + &sha256, + || {}, + ) + }; + + // The state an earlier alignment left behind: the active ROCm version + // and a newer llama.cpp build pinned in place of the packaged ones. + fs::write( + &pins, + r#"{"therock":{"version":"7.14.1"},"llamacpp":{"rocm-stable":"b11783"}}"#, + ) + .unwrap(); + let backup = restore(&manifest) + .unwrap() + .expect("a drifted pin file must be restored"); + assert_eq!( + serde_json::from_str::(&fs::read_to_string(&pins).unwrap()).unwrap(), + serde_json::from_str::(packaged).unwrap() + ); + assert_no_pin_temp_files(&runtime_dir.join("resources")); + assert_eq!( + fs::read(backend.join("llama-server")).unwrap(), + b"installed backend", + "restoring the pins must not touch backends already installed in the tree" + ); + assert!( + fs::read_dir(root.join("extract")).unwrap().next().is_none(), + "the scratch extraction must be cleaned up" + ); + + // Re-serialized but equal content is not drift: nothing to restore. + fs::write( + &pins, + serde_json::to_vec_pretty( + &serde_json::from_str::(packaged).unwrap(), + ) + .unwrap(), + ) + .unwrap(); + assert!(restore(&manifest).unwrap().is_none()); + + // The backup puts the drifted bytes back exactly, so a restore whose + // retry fails does not discard the pin that was there. + backup.put_back().unwrap(); + assert_eq!( + fs::read_to_string(&pins).unwrap(), + r#"{"therock":{"version":"7.14.1"},"llamacpp":{"rocm-stable":"b11783"}}"# + ); + assert_no_pin_temp_files(&runtime_dir.join("resources")); + fs::remove_dir_all(&dir).ok(); + } + + /// Puts `source` where `prepare_embeddable_with` leaves the verified + /// archive of the `11.5.1` test manifests: `/downloads/`. + fn cache_archive(root: &Path, source: &Path) { + let downloads = root.join("downloads"); + fs::create_dir_all(&downloads).unwrap(); + fs::copy(source, downloads.join(embeddable_archive_name("11.5.1"))).unwrap(); + } + + fn assert_no_pin_temp_files(dir: &Path) { + let leftovers: Vec = fs::read_dir(dir) + .unwrap() + .filter_map(Result::ok) + .map(|entry| entry.file_name().to_string_lossy().into_owned()) + .filter(|name| name.contains(".tmp-")) + .collect(); + assert!( + leftovers.is_empty(), + "the atomic write must not leave a temp file behind: {leftovers:?}" + ); + } + + /// Runs the restore against a fake embeddable archive that ships + /// `{"therock":{"version":"7.13.0"},"llamacpp":{"rocm-stable":"b9752"}}`, + /// with `current` as the pin file already on disk. Returns what the restore + /// returned and the pin file's content afterwards. + fn restore_over(tag: &str, current: &str) -> (Option, String) { + restore_over_packaged( + tag, + r#"{"therock":{"version":"7.13.0"},"llamacpp":{"rocm-stable":"b9752"}}"#, + current, + ) + } + + /// [`restore_over`] with the pin file the archive ships given too. + fn restore_over_packaged( + tag: &str, + packaged: &str, + current: &str, + ) -> (Option, String) { + let dir = scratch_dir(tag); + let root = dir.join("lemonade"); + let runtime_dir = runtime_dir_in(&root); + fs::create_dir_all(runtime_dir.join("resources")).unwrap(); + let pins = runtime_dir.join("resources/backend_versions.json"); + fs::write(&pins, current).unwrap(); + let source = dir.join("source-archive"); + write_embeddable_with_pins(&source, packaged); + let sha256 = format!("{:x}", Sha256::digest(fs::read(&source).unwrap())); + let manifest = LemonadeInstallManifest { + env_id: "test".to_owned(), + version: "11.5.1".to_owned(), + runtime_dir: runtime_dir.clone(), + lemond: lemond_path_in(&runtime_dir), + lemonade: lemonade_path_in(&runtime_dir), + backend_recipe: LLAMACPP_RECIPE.to_owned(), + backend_name: ROCM_BACKEND_NAME.to_owned(), + installed_at_unix_ms: 0, + }; + cache_archive(&root, &source); + let backup = restore_packaged_backend_versions_with( + &manifest, + "resources/backend_versions.json", + &sha256, + || {}, + ) + .unwrap(); + let after = fs::read_to_string(&pins).unwrap(); + // The backup, if any, still has to be able to put the file back. + if let Some(backup) = &backup { + assert!(backup.previous.is_some()); + } + (backup, after) + } + + #[test] + fn restore_keeps_entries_a_user_edited_and_only_resets_the_alignment_pins() { + let current = r#"{"therock":{"version":"7.14.1"},"llamacpp":{"rocm-stable":"b11783","vulkan":"b1111"},"custom":{"x":1}}"#; + let (backup, after) = restore_over("restore-keeps-user-edits", current); + + let value: serde_json::Value = serde_json::from_str(&after).unwrap(); + assert_eq!(value["therock"]["version"], "7.13.0"); + assert_eq!(value["llamacpp"]["rocm-stable"], "b9752"); + assert_eq!(value["llamacpp"]["vulkan"], "b1111", "{after}"); + assert_eq!(value["custom"]["x"], 1, "{after}"); + assert!(backup.is_some()); + } + + #[test] + fn restore_is_a_no_op_when_only_other_entries_differ_from_the_packaged_file() { + let current = r#"{"therock":{"version":"7.13.0"},"llamacpp":{"rocm-stable":"b9752","vulkan":"b1111"}}"#; + let (backup, after) = restore_over("restore-noop-other-keys", current); + + assert!(backup.is_none(), "the alignment pins already match"); + assert_eq!(after, current, "the file must not be rewritten"); + } + + #[test] + fn restore_removes_a_pin_the_packaged_file_does_not_have() { + // A packaged file without `llamacpp.rocm-stable` must not leave the + // kept alignment pin behind; the user's other entries still survive. + let packaged = r#"{"therock":{"version":"7.13.0"},"llamacpp":{"vulkan":"b9747"}}"#; + let current = r#"{"therock":{"version":"7.13.0"},"llamacpp":{"rocm-stable":"b11783","vulkan":"b1111"}}"#; + let (backup, after) = restore_over_packaged("restore-removes-pin", packaged, current); + + assert!(backup.is_some(), "the kept rocm-stable pin is drift"); + let value: serde_json::Value = serde_json::from_str(&after).unwrap(); + assert!(value["llamacpp"].get("rocm-stable").is_none(), "{after}"); + assert_eq!(value["llamacpp"]["vulkan"], "b1111", "{after}"); + assert_eq!(value["therock"]["version"], "7.13.0"); + } + + #[test] + fn restore_happens_when_only_the_rocm_stable_pin_drifted() { + let current = r#"{"therock":{"version":"7.13.0"},"llamacpp":{"rocm-stable":"b11783"}}"#; + let (backup, after) = restore_over("restore-only-rocm-stable", current); + + assert!(backup.is_some(), "a drifted llama.cpp tag alone is drift"); + let value: serde_json::Value = serde_json::from_str(&after).unwrap(); + assert_eq!(value["llamacpp"]["rocm-stable"], "b9752"); + assert_eq!(value["therock"]["version"], "7.13.0"); + } + + #[test] + fn restore_replaces_a_pin_file_that_is_not_json_with_the_packaged_one() { + let (backup, after) = restore_over("restore-not-json", "{ not json"); + + assert!(backup.is_some()); + let value: serde_json::Value = serde_json::from_str(&after).unwrap(); + assert_eq!(value["therock"]["version"], "7.13.0"); + } + + /// A runtime tree whose pin file holds a stale kept pin, and the manifest + /// for it. The archive cache is left empty. + fn stale_pin_tree(tag: &str) -> (PathBuf, PathBuf, PathBuf, LemonadeInstallManifest) { + let dir = scratch_dir(tag); + let root = dir.join("lemonade"); + let runtime_dir = runtime_dir_in(&root); + fs::create_dir_all(runtime_dir.join("resources")).unwrap(); + let pins = runtime_dir.join("resources/backend_versions.json"); + fs::write( + &pins, + r#"{"therock":{"version":"7.14.1"},"llamacpp":{"rocm-stable":"b11783"}}"#, + ) + .unwrap(); + let manifest = LemonadeInstallManifest { + env_id: "test".to_owned(), + version: "11.5.1".to_owned(), + runtime_dir: runtime_dir.clone(), + lemond: lemond_path_in(&runtime_dir), + lemonade: lemonade_path_in(&runtime_dir), + backend_recipe: LLAMACPP_RECIPE.to_owned(), + backend_name: ROCM_BACKEND_NAME.to_owned(), + installed_at_unix_ms: 0, + }; + (dir, root, pins, manifest) + } + + #[test] + fn restore_without_a_cached_archive_downloads_nothing_and_leaves_the_pins() { + // The restore runs after an install already failed (often because the + // network is down), so it must not start a download of its own. + let (dir, root, pins, manifest) = stale_pin_tree("restore-no-cache"); + let before = fs::read(&pins).unwrap(); + + let error = restore_packaged_backend_versions_with( + &manifest, + "resources/backend_versions.json", + "0000", + || {}, + ) + .unwrap_err(); + + let error = format!("{error:#}"); + assert!( + error.contains("is not cached") && error.contains("not downloading"), + "{error}" + ); + assert_eq!( + fs::read(&pins).unwrap(), + before, + "the pin file is untouched" + ); + assert!( + !root.join("downloads").exists() && !root.join("extract").exists(), + "a missing cache must not create a download or scratch directory" + ); + fs::remove_dir_all(&dir).ok(); + } + + #[test] + fn restore_does_not_replace_a_cached_archive_that_fails_its_digest() { + let (dir, root, pins, manifest) = stale_pin_tree("restore-bad-cache"); + let before = fs::read(&pins).unwrap(); + let source = dir.join("source-archive"); + write_embeddable_with_pins( + &source, + r#"{"therock":{"version":"7.13.0"},"llamacpp":{"rocm-stable":"b9752"}}"#, + ); + cache_archive(&root, &source); + + let error = restore_packaged_backend_versions_with( + &manifest, + "resources/backend_versions.json", + "not-the-digest-of-the-cached-archive", + || {}, + ) + .unwrap_err(); + + let error = format!("{error:#}"); + assert!( + error.contains("does not match its digest") && error.contains("not downloading"), + "{error}" + ); + assert_eq!( + fs::read(&pins).unwrap(), + before, + "the pin file is untouched" + ); + fs::remove_dir_all(&dir).ok(); + } + + #[test] + fn restore_reads_the_pin_file_after_the_archive_work_not_before() { + let (dir, root, pins, manifest) = stale_pin_tree("restore-reads-late"); + let source = dir.join("source-archive"); + write_embeddable_with_pins( + &source, + r#"{"therock":{"version":"7.13.0"},"llamacpp":{"rocm-stable":"b9752"}}"#, + ); + let sha256 = format!("{:x}", Sha256::digest(fs::read(&source).unwrap())); + cache_archive(&root, &source); + // Another process writes the pin file after this one started but + // before it has finished with the archive. + let concurrent = r#"{"therock":{"version":"7.14.2"},"llamacpp":{"rocm-stable":"b12000"},"custom":{"x":1}}"#; + + let backup = restore_packaged_backend_versions_with( + &manifest, + "resources/backend_versions.json", + &sha256, + || fs::write(&pins, concurrent).unwrap(), + ) + .unwrap() + .expect("the concurrent pins differ from the packaged ones"); + + assert_eq!( + backup.previous.as_deref(), + Some(concurrent.as_bytes()), + "the backup holds what was on disk when the restore wrote, not an earlier read" + ); + let value: serde_json::Value = + serde_json::from_str(&fs::read_to_string(&pins).unwrap()).unwrap(); + assert_eq!(value["therock"]["version"], "7.13.0"); + assert_eq!( + value["custom"]["x"], 1, + "the concurrent writer's other entries survive" + ); + backup.put_back().unwrap(); + assert_eq!(fs::read_to_string(&pins).unwrap(), concurrent); + fs::remove_dir_all(&dir).ok(); + } + + #[test] + fn restore_does_not_treat_an_unreadable_pin_file_as_absent() { + // A directory where the file should be is unreadable on every platform + // (and, like a sharing violation, is not `NotFound`). The restore must + // fail without producing a backup that would delete the path later. + let (dir, root, pins, manifest) = stale_pin_tree("restore-unreadable"); + fs::remove_file(&pins).unwrap(); + fs::create_dir(&pins).unwrap(); + fs::write(pins.join("keep"), b"kept").unwrap(); + let source = dir.join("source-archive"); + write_embeddable_with_pins( + &source, + r#"{"therock":{"version":"7.13.0"},"llamacpp":{"rocm-stable":"b9752"}}"#, + ); + let sha256 = format!("{:x}", Sha256::digest(fs::read(&source).unwrap())); + cache_archive(&root, &source); + + let error = restore_packaged_backend_versions_with( + &manifest, + "resources/backend_versions.json", + &sha256, + || {}, + ) + .unwrap_err(); + + let error = format!("{error:#}"); + assert!(error.contains("could not read"), "{error}"); + assert!( + pins.is_dir() && pins.join("keep").is_file(), + "the unreadable path is left exactly as it was" + ); + assert_no_pin_temp_files(pins.parent().unwrap()); + fs::remove_dir_all(&dir).ok(); + } + + #[test] + fn a_backup_of_a_missing_pin_file_removes_the_file_again() { + let dir = scratch_dir("pin-backup-missing"); + let pins = dir.join("backend_versions.json"); + fs::write(&pins, b"restored").unwrap(); + + PinFileBackup::new(pins.clone(), None).put_back().unwrap(); + assert!(!pins.exists()); + // Already gone is not an error. + PinFileBackup::new(pins, None).put_back().unwrap(); + fs::remove_dir_all(&dir).ok(); + } + #[test] fn lemonade_root_uses_requested_engine_root() { let paths = AppPaths {