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 {