diff --git a/Cargo.lock b/Cargo.lock index f760ebf10..a46a744a0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3665,6 +3665,7 @@ dependencies = [ "rocm-engine-vllm", "rocmd", "rpassword", + "self-replace", "serde", "serde_json", "sha2", @@ -4169,6 +4170,17 @@ dependencies = [ "libc", ] +[[package]] +name = "self-replace" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "03ec815b5eab420ab893f63393878d89c90fdd94c0bcc44c07abb8ad95552fb7" +dependencies = [ + "fastrand", + "tempfile", + "windows-sys 0.52.0", +] + [[package]] name = "semver" version = "1.0.28" diff --git a/MANIFEST.md b/MANIFEST.md index 0942746ab..5f962e498 100644 --- a/MANIFEST.md +++ b/MANIFEST.md @@ -414,6 +414,7 @@ repository. | security-framework | 2.11.1 | MIT OR Apache-2.0 | | security-framework | 3.7.0 | MIT OR Apache-2.0 | | security-framework-sys | 2.17.0 | MIT OR Apache-2.0 | +| self-replace | 1.5.0 | Apache-2.0 | | semver | 1.0.28 | MIT OR Apache-2.0 | | serde | 1.0.228 | MIT OR Apache-2.0 | | serde_core | 1.0.228 | MIT OR Apache-2.0 | diff --git a/README.md b/README.md index 861591ae8..6a714e2c1 100644 --- a/README.md +++ b/README.md @@ -80,7 +80,9 @@ add a demo. Until the demo-gifs workflow has run once, these links 404. The installer downloads a prebuilt bundle, verifies its SHA-256 checksum, installs the `rocm` and `rocmd` binaries into `~/.local/bin`, and adds that -directory to your shell `PATH`. Rerun it any time to upgrade. +directory to your shell `PATH`. Rerun it any time to upgrade. `rocm uninstall` +removes the binaries, and undoes the `PATH` change too when no other program +lives in that directory. ### Linux and WSL (x86_64) @@ -946,7 +948,20 @@ rocm logs [--service ] [--search TERM ...] rocm uninstall [--yes] [--dry-run] [--keep-binaries] [--keep-config] [--keep-data] [--keep-cache] -``` + [--force-dev-binaries] +``` + +`rocm uninstall` lists what it will do and asks before doing it (`--yes` skips +the question, `--dry-run` only lists). It removes the config, data, and cache +directories and the installed binaries, including the running `rocm.exe` on +Windows. It also undoes the `PATH` change the installer made: the marked block +in your shell profile, or the user `PATH` entry the Windows installer recorded +adding. That change stays when other programs live in the same folder, since +they may rely on it (the review names them), and with `--keep-binaries`. A +`PATH` line you wrote yourself is never touched. The `PATH` change is undone +first: if it cannot be, nothing is removed, so you can fix the cause and run +`rocm uninstall` again. Each step is reported as it finishes, so if a later +step fails, the output still shows what was already removed. On Linux, if a config, data, or cache folder is a link to somewhere else (for example a cache moved to a bigger disk), `rocm uninstall` removes the link and diff --git a/THIRD_PARTY_NOTICES.txt b/THIRD_PARTY_NOTICES.txt index 8c8e6efb2..5da394269 100644 --- a/THIRD_PARTY_NOTICES.txt +++ b/THIRD_PARTY_NOTICES.txt @@ -5846,6 +5846,7 @@ The following component(s) are licensed under Apache License 2.0: - security-framework-sys 2.17.0 - security-framework 2.11.1 - security-framework 3.7.0 + - self-replace 1.5.0 - serde_with 3.21.0 - serde_with_macros 3.21.0 - signal-hook-mio 0.2.5 diff --git a/apps/rocm/Cargo.toml b/apps/rocm/Cargo.toml index 6d08193f5..a319b11c6 100644 --- a/apps/rocm/Cargo.toml +++ b/apps/rocm/Cargo.toml @@ -54,6 +54,7 @@ ureq = { version = "2.12", features = ["native-certs"] } libc.workspace = true [target.'cfg(windows)'.dependencies] +self-replace = "1.5" windows-native-keyring-store = "1.1" windows-sys = { version = "0.61", features = ["Win32_Storage_FileSystem"] } diff --git a/apps/rocm/src/main.rs b/apps/rocm/src/main.rs index 85f258fa9..39e5bcac6 100644 --- a/apps/rocm/src/main.rs +++ b/apps/rocm/src/main.rs @@ -648,6 +648,12 @@ rocm logs --search error timeout")] command: BenchCommand, }, /// Remove ROCm CLI-managed files from this computer. + /// + /// Removes the config, data, and cache directories and the installed + /// binaries, including the running rocm.exe on Windows. It also undoes the + /// PATH change the installer made (its marked shell-profile block, or the + /// user PATH entry it recorded adding on Windows), unless other programs + /// share the install folder. Uninstall { /// Do not ask for interactive confirmation. #[arg(long)] @@ -655,7 +661,7 @@ rocm logs --search error timeout")] /// Show what would be removed without deleting anything. #[arg(long)] dry_run: bool, - /// Keep installed rocm-cli binaries. + /// Keep installed rocm-cli binaries, and the installer's PATH change. #[arg(long)] keep_binaries: bool, /// Keep saved settings. @@ -13595,38 +13601,24 @@ struct UninstallPlan { actions: Vec, skipped: Vec, warnings: Vec, + /// Installer PATH changes `rocm uninstall` undoes. + path_edits: Vec, + /// The installer's record of its PATH change, removed once that change is + /// undone. + path_record: Option, + /// The running executable, on Windows, which is deleted as the command exits. + self_delete: Option, } fn build_uninstall_plan(paths: &AppPaths, options: &UninstallOptions) -> Result { let mut plan = UninstallPlan::default(); - if options.keep_binaries { - plan.skipped - .push("binary removal disabled by --keep-binaries".to_owned()); - } else { - let current_exe = - daemon_binary_path().context("failed to discover current rocm executable")?; - if is_dev_binary_layout(¤t_exe) && !options.force_dev_binaries { - plan.skipped.push(format!( - "binary removal skipped because {} looks like a cargo target build; pass --force-dev-binaries to remove sibling debug/release binaries", - current_exe.display() - )); - } else { - for path in collect_installed_binary_candidates(¤t_exe)? { - if rocm_core::runtime_is_windows() && path == current_exe { - plan.skipped.push(format!( - "skipping running executable on Windows: {}", - path.display() - )); - continue; - } - plan.actions.push(UninstallPlanEntry { - kind: "binary", - path, - }); - } - } - } + plan_installed_binaries( + &mut plan, + options, + || daemon_binary_path().context("failed to discover current rocm executable"), + rocm_core::runtime_home_dir().as_deref(), + )?; for (keep, kind, path) in [ (options.keep_config, "config", paths.config_dir.clone()), @@ -13713,6 +13705,76 @@ fn build_uninstall_plan(paths: &AppPaths, options: &UninstallOptions) -> Result< Ok(plan) } +/// Plan removing the installed binaries beside the running one, which +/// `current_exe` names, and undoing the installer's PATH change for their +/// folder. `--keep-binaries` keeps both, and so does a cargo build's folder. +fn plan_installed_binaries( + plan: &mut UninstallPlan, + options: &UninstallOptions, + current_exe: impl FnOnce() -> Result, + home: Option<&Path>, +) -> Result<()> { + if options.keep_binaries { + // The PATH change only serves the binaries, so it stays with them. + plan.skipped.push( + "binary removal disabled by --keep-binaries, so any PATH change the installer made \ + stays too" + .to_owned(), + ); + return Ok(()); + } + let current_exe = current_exe()?; + if is_dev_binary_layout(¤t_exe) && !options.force_dev_binaries { + plan.skipped.push(format!( + "binary removal skipped because {} looks like a cargo target build; pass --force-dev-binaries to remove sibling debug/release binaries", + current_exe.display() + )); + return Ok(()); + } + let binaries = collect_installed_binary_candidates(¤t_exe)?; + for path in &binaries { + // Windows will not unlink a running image; it is deleted as the + // command exits instead. + if is_running_executable(path, ¤t_exe, cfg!(windows)) { + plan.self_delete = Some(path.clone()); + continue; + } + // The PATH record goes only once its change is undone; see + // `uninstall::plan_path_edits`. + if path.file_name().and_then(|name| name.to_str()) == Some(uninstall::PATH_EDIT_RECORD) { + continue; + } + plan.actions.push(UninstallPlanEntry { + kind: "binary", + path: path.clone(), + }); + } + if let Some(bin_dir) = current_exe.parent() { + uninstall::plan_path_edits(plan, bin_dir, &binaries, home); + } + Ok(()) +} + +/// Whether `path`, an entry of the running executable's folder, is the running +/// executable itself, which only Windows has to treat specially. Windows names +/// are case-insensitive, and the running image may report its own name in +/// another case, or in its 8.3 short form (`ROCM.EXE`), than the folder listing +/// does; missing it would plan an ordinary removal that Windows refuses. +fn is_running_executable(path: &Path, current_exe: &Path, windows: bool) -> bool { + if !windows { + return false; + } + path == current_exe + || (path.parent() == current_exe.parent() + && path + .file_name() + .zip(current_exe.file_name()) + .is_some_and(|(name, exe)| { + name.to_string_lossy() + .eq_ignore_ascii_case(&exe.to_string_lossy()) + })) +} + pub(crate) fn render_uninstall_dry_run(paths: &AppPaths) -> Result { let options = UninstallOptions { dry_run: true, @@ -13726,14 +13788,16 @@ fn render_uninstall_plan(plan: &UninstallPlan, options: &UninstallOptions) -> St let mut output = String::new(); let _ = writeln!(output, "Uninstall review"); let _ = writeln!(output); - if plan.actions.is_empty() { + let removals = uninstall::removal_count(plan); + if uninstall::plan_is_empty(plan) { let _ = writeln!(output, "Nothing managed by rocm-cli would be removed."); - } else { - let _ = writeln!(output, "{} item(s) would be removed:", plan.actions.len()); + } else if removals > 0 { + let _ = writeln!(output, "{removals} item(s) would be removed:"); for entry in &plan.actions { let _ = writeln!(output, " - {}: {}", entry.kind, entry.path.display()); } } + uninstall::render_extra_plan(&mut output, plan); if !plan.warnings.is_empty() { let _ = writeln!(output); let _ = writeln!(output, "Please review:"); @@ -13863,7 +13927,7 @@ fn collect_installed_binary_candidates(current_exe: &Path) -> Result bool { - if name == ".rocm-cli-manifest" { + if name == ".rocm-cli-manifest" || name == uninstall::PATH_EDIT_RECORD { return true; } let normalized = name.strip_suffix(".exe").unwrap_or(name); diff --git a/apps/rocm/src/uninstall.rs b/apps/rocm/src/uninstall.rs index 42ab83a8b..7b8c7352d 100644 --- a/apps/rocm/src/uninstall.rs +++ b/apps/rocm/src/uninstall.rs @@ -4,25 +4,71 @@ //! `rocm uninstall` command handler. //! -//! Mechanically relocated from `main.rs` with no behavior change — the -//! `dispatch()` call site stays `uninstall(UninstallOptions { .. })` (re-imported -//! via `use crate::uninstall::uninstall;`). The `UninstallOptions`/`UninstallPlan` -//! types and the plan/render/remove helpers remain in the crate root and are -//! reached through `crate::` (root items are visible to this descendant module). +//! The plan itself is built by `build_uninstall_plan` in the crate root, which +//! `rocm storage` shares the plan and removal shape with. This module owns what +//! only uninstall does on top of removing files: +//! +//! - undoing the PATH change the installer recorded making; +//! - deleting the running `rocm.exe` on Windows, which cannot unlink itself. + +use std::ffi::OsString; +use std::fmt::Write as _; +use std::fs; +use std::io::Write as _; +use std::path::{Path, PathBuf}; use anyhow::{Context, Result, bail}; use rocm_core::{AppPaths, interactive_terminal}; use crate::{ - UninstallOptions, build_uninstall_plan, confirm_uninstall, remove_path, render_uninstall_plan, + UninstallOptions, UninstallPlan, build_uninstall_plan, confirm_uninstall, + is_rocm_install_entry_name, remove_path, render_uninstall_plan, }; +/// The file, next to the installed binaries, in which `install.sh` and +/// `install.ps1` record the PATH change they made, one per line: +/// `profile ` and `path-expr ` +/// from `install.sh`, `user-path ` from `install.ps1`. +pub(crate) const PATH_EDIT_RECORD: &str = ".rocm-cli-path-edit"; + +const PROFILE_BLOCK_START: &str = "# >>> rocm-cli path >>>"; +const PROFILE_BLOCK_END: &str = "# <<< rocm-cli path <<<"; + +/// A PATH change the installer made that uninstall will undo. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum PathEdit { + /// Remove the marked block `install.sh` appended to a shell profile. + ShellProfile { + profile: PathBuf, + /// The spellings of the install folder the block may use: the ones + /// the installer recorded, the literal path, and the `$HOME/...` form. + /// Kept as the OS spells them, since a path need not be UTF-8. + path_exprs: Vec, + }, + /// Remove the entry `install.ps1` added to the user `Path`. + UserPath { dir: PathBuf }, +} + +impl PathEdit { + pub(crate) fn describe(&self) -> String { + match self { + Self::ShellProfile { profile, .. } => { + format!( + "shell profile: the rocm-cli PATH lines in {}", + profile.display() + ) + } + Self::UserPath { dir } => format!("user PATH: {}", dir.display()), + } + } +} + pub(crate) fn uninstall(options: UninstallOptions) -> Result<()> { let paths = AppPaths::discover()?; let plan = build_uninstall_plan(&paths, &options)?; print!("{}", render_uninstall_plan(&plan, &options)); - if plan.actions.is_empty() || options.dry_run { + if plan_is_empty(&plan) || options.dry_run { return Ok(()); } @@ -36,11 +82,1248 @@ pub(crate) fn uninstall(options: UninstallOptions) -> Result<()> { } } - for entry in &plan.actions { + execute_plan(&plan, &mut |line| println!("{line}"))?; + println!("uninstall complete"); + Ok(()) +} + +/// Carry out `plan`, handing `report` a line for each step as soon as it is +/// done, so a failure partway still leaves the user told what already changed. +/// +/// The PATH changes are undone first, so a change that cannot be undone stops +/// the uninstall before anything is removed: `rocm`, the user's settings, and +/// the record of the change are all still there to retry with. Then the +/// config, data, and cache go, while `rocm` is still there to retry a failure +/// with; then the binaries, and the record only once every change is undone. +fn execute_plan(plan: &UninstallPlan, report: &mut dyn FnMut(String)) -> Result<()> { + for edit in &plan.path_edits { + match apply_path_edit(edit) { + Ok(line) => report(line), + Err(error) => bail!( + "could not undo the installer's PATH change ({}): {error:#}\n\ + Nothing else was removed, so you can fix this and run `rocm uninstall` again.", + edit.describe() + ), + } + } + + let is_binary = |kind: &str| kind == "binary"; + let non_binaries = plan.actions.iter().filter(|entry| !is_binary(entry.kind)); + let binaries = plan.actions.iter().filter(|entry| is_binary(entry.kind)); + for entry in non_binaries.chain(binaries) { remove_path(&entry.path) .with_context(|| format!("failed to remove {}", entry.path.display()))?; - println!("removed {} {}", entry.kind, entry.path.display()); + report(format!("removed {} {}", entry.kind, entry.path.display())); + } + if let Some(record) = &plan.path_record { + remove_path(record).with_context(|| format!("failed to remove {}", record.display()))?; + report(format!("removed install record {}", record.display())); + } + + if let Some(exe) = &plan.self_delete { + report(delete_running_executable(exe)?); + } + Ok(()) +} + +pub(crate) const fn plan_is_empty(plan: &UninstallPlan) -> bool { + plan.actions.is_empty() + && plan.path_edits.is_empty() + && plan.path_record.is_none() + && plan.self_delete.is_none() +} + +/// How many entries the review lists as removed. +pub(crate) fn removal_count(plan: &UninstallPlan) -> usize { + plan.actions.len() + + usize::from(plan.path_record.is_some()) + + usize::from(plan.self_delete.is_some()) +} + +/// Append the parts of the review only uninstall has: the install record, the +/// running executable, and the PATH changes it undoes. +pub(crate) fn render_extra_plan(output: &mut String, plan: &UninstallPlan) { + if let Some(record) = &plan.path_record { + let _ = writeln!(output, " - install record: {}", record.display()); + } + if let Some(exe) = &plan.self_delete { + let _ = writeln!( + output, + " - binary: {} (the running program; deleted as this command exits)", + exe.display() + ); + } + if !plan.path_edits.is_empty() { + let _ = writeln!(output); + let _ = writeln!( + output, + "PATH changes from the installer that would be undone:" + ); + for edit in &plan.path_edits { + let _ = writeln!(output, " - {}", edit.describe()); + } + } +} + +// --------------------------------------------------------------------------- +// PATH changes +// --------------------------------------------------------------------------- + +/// Plan undoing the PATH change the installer made for `bin_dir`, the folder +/// whose ROCm CLI entries `removed` lists. +/// +/// Only a change the installer recorded (or the shell-profile block it marks as +/// its own) is undone, never a PATH line the user wrote. And it is left in place +/// when other programs live in the same folder, since they may depend on it +/// being on PATH; the record then stays too, so a later uninstall still knows. +pub(crate) fn plan_path_edits( + plan: &mut UninstallPlan, + bin_dir: &Path, + removed: &[PathBuf], + home: Option<&Path>, +) { + let record_path = bin_dir.join(PATH_EDIT_RECORD); + // Read as bytes: the installer writes paths as the shell spells them, and + // a home folder need not be UTF-8. + let record = match fs::read(&record_path) { + Ok(record) => Some(record), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => None, + Err(error) => { + plan.warnings.push(format!( + "could not read {}, so the installer's PATH change is left in place: {error}", + record_path.display() + )); + return; + } + }; + let entries = record + .as_deref() + .map(parse_path_edit_record) + .unwrap_or_default(); + + let mut edits = Vec::new(); + // A profile that could not be read may still hold a block, so the record + // that names it must outlive this uninstall. + let mut unreadable = false; + if cfg!(windows) { + for (kind, target) in &entries { + if *kind == "user-path" { + edits.push(PathEdit::UserPath { + dir: PathBuf::from(target), + }); + } + } + let on_path = std::env::var("PATH").is_ok_and(|path| { + rocm_core::user_path::remove_path_list_entry(&path, bin_dir).is_some() + }); + if edits.is_empty() && on_path { + plan.skipped.push(format!( + "{} is on PATH, but this installation did not record adding it, so it is left in place", + bin_dir.display() + )); + } + } else { + let mut path_exprs = profile_path_exprs(bin_dir, home); + for (kind, target) in &entries { + if *kind == "path-expr" && !path_exprs.contains(target) { + path_exprs.push(target.clone()); + } + } + let recorded: Vec = entries + .iter() + .filter(|(kind, _)| *kind == "profile") + .map(|(_, target)| PathBuf::from(target)) + .collect(); + // The record names the profile of the latest install, but an earlier + // install from another shell may have marked a different one, and + // installs from before the record named none. The marked block is the + // installer's own wherever it is, so look everywhere it writes one. + let mut candidates = recorded.clone(); + for profile in home.map(legacy_profile_candidates).unwrap_or_default() { + if !candidates.contains(&profile) { + candidates.push(profile); + } + } + for profile in candidates { + match profile_has_block(&profile, &path_exprs) { + Ok(true) => edits.push(PathEdit::ShellProfile { + profile, + path_exprs: path_exprs.clone(), + }), + Ok(false) if recorded.contains(&profile) => plan.skipped.push(format!( + "{} no longer has the rocm-cli PATH lines, so there is nothing to undo there", + profile.display() + )), + Ok(false) => {} + // The installer recorded writing its block here, so it is + // most likely still there: undoing it is attempted like any + // other, and if the profile still cannot be read then, the + // uninstall stops before removing anything, as for any PATH + // change it cannot undo. + Err(error) if recorded.contains(&profile) => { + plan.warnings.push(format!( + "could not read {}, which the installer recorded adding its PATH lines to: {error:#}\n\ + Unless it can be read when the uninstall runs, nothing will be removed.", + profile.display() + )); + edits.push(PathEdit::ShellProfile { + profile, + path_exprs: path_exprs.clone(), + }); + } + Err(error) => { + unreadable = true; + plan.warnings.push(format!( + "could not read {}, so any rocm-cli PATH lines in it are left in place: {error:#}", + profile.display() + )); + } + } + } + } + + if edits.is_empty() { + if record.is_some() && !unreadable { + plan.path_record = Some(record_path); + } + return; + } + match other_entries(bin_dir, removed) { + Ok(others) if others.is_empty() => { + plan.path_edits = edits; + if record.is_some() && !unreadable { + plan.path_record = Some(record_path); + } + } + Ok(others) => { + let kept_record = if record.is_some() { + format!( + "; its record {} stays with it, so a later uninstall can still undo it", + record_path.display() + ) + } else { + String::new() + }; + plan.skipped.push(format!( + "PATH change left in place: {} also holds {}, which may rely on it being on PATH{kept_record}", + bin_dir.display(), + others.join(", ") + )); + } + Err(error) => plan.warnings.push(format!( + "could not list {} to see whether other programs rely on it being on PATH, so the \ + installer's PATH change is left in place: {error}", + bin_dir.display() + )), + } +} + +/// The `kind target` lines of a PATH-edit record. A target is kept as the +/// bytes the installer wrote, so a path that is not UTF-8 still names its file. +fn parse_path_edit_record(record: &[u8]) -> Vec<(&str, OsString)> { + record + .split(|byte| *byte == b'\n') + .filter_map(|line| { + let line = line.strip_suffix(b"\r").unwrap_or(line); + let space = line.iter().position(|byte| *byte == b' ')?; + let (kind, target) = (&line[..space], &line[space + 1..]); + let kind = std::str::from_utf8(kind).ok()?; + (!target.trim_ascii().is_empty()).then(|| (kind, os_string_from_bytes(target))) + }) + .collect() +} + +#[cfg(unix)] +fn os_string_from_bytes(bytes: &[u8]) -> OsString { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt as _; + OsStr::from_bytes(bytes).to_owned() +} + +/// `install.ps1` writes the record as UTF-8. +#[cfg(not(unix))] +fn os_string_from_bytes(bytes: &[u8]) -> OsString { + OsString::from(String::from_utf8_lossy(bytes).into_owned()) +} + +/// How `install.sh` may have spelled `bin_dir` in the block: `$HOME/...` when it +/// is under the home directory, and always the literal path. Each is tried as +/// given and with links resolved, since the running binary reports its folder +/// resolved while the installer wrote it as the user's `$HOME` named it. +fn profile_path_exprs(bin_dir: &Path, home: Option<&Path>) -> Vec { + let resolve = |path: &Path| fs::canonicalize(path).unwrap_or_else(|_| path.to_path_buf()); + let mut bins = vec![bin_dir.to_path_buf()]; + let resolved_bin = resolve(bin_dir); + if resolved_bin != bin_dir { + bins.push(resolved_bin); + } + let mut homes: Vec = home.map(Path::to_path_buf).into_iter().collect(); + if let Some(home) = home { + let resolved_home = resolve(home); + if resolved_home != home { + homes.push(resolved_home); + } + } + + let mut exprs = Vec::new(); + let mut push = |expr: OsString| { + if !exprs.contains(&expr) { + exprs.push(expr); + } + }; + for bin in &bins { + push(bin.as_os_str().to_owned()); + for home in &homes { + if bin == home { + push(OsString::from("$HOME")); + } else if let Ok(relative) = bin.strip_prefix(home) { + let mut expr = OsString::from("$HOME/"); + expr.push(relative.as_os_str()); + push(expr); + } + } + } + exprs +} + +/// Every profile `install.sh` can pick, by shell, under `home`. +fn legacy_profile_candidates(home: &Path) -> Vec { + vec![ + home.join(".bashrc"), + home.join(".zshrc"), + home.join(".config").join("fish").join("config.fish"), + home.join(".kshrc"), + home.join(".profile"), + ] +} + +/// Whether `profile` holds the installer's block for one of `path_exprs`. A +/// profile that does not exist has none; one that cannot be read is an error, +/// not an absence. +fn profile_has_block(profile: &Path, path_exprs: &[OsString]) -> Result { + match fs::read(profile) { + Ok(bytes) => Ok(remove_profile_block_bytes(&bytes, path_exprs).is_some()), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(false), + Err(error) => Err(error.into()), + } +} + +/// `text` without the installer's marked PATH block for one of `path_exprs`, or +/// `None` when it has none. +/// +/// A block naming a different folder belongs to another install and stays. The +/// blank line the installer writes in front of the block goes with it, so the +/// profile reads back as it did before the install. Works on bytes: a profile +/// is not required to be UTF-8, and every byte outside the block is kept. +fn remove_profile_block_bytes(text: &[u8], path_exprs: &[OsString]) -> Option> { + let lines: Vec<&[u8]> = text.split_inclusive(|byte| *byte == b'\n').collect(); + let contains = |haystack: &[u8], needle: &[u8]| { + haystack + .windows(needle.len()) + .any(|window| window == needle) + }; + // The block quotes the folder the way `install.sh`'s + // `escape_for_double_quotes` does. + let needles: Vec<[Vec; 2]> = path_exprs + .iter() + .map(|expr| { + let mut escaped = Vec::new(); + for byte in expr.as_encoded_bytes() { + if matches!(byte, b'\\' | b'"') { + escaped.push(b'\\'); + } + escaped.push(*byte); + } + let quoted = [&b"\""[..], &escaped, b"\""].concat(); + let listed = [&b"\""[..], &escaped, b":"].concat(); + [quoted, listed] + }) + .collect(); + let names_dir = |body: &[u8]| { + needles + .iter() + .flatten() + .any(|needle| contains(body, needle)) + }; + let mut kept: Vec<&[u8]> = Vec::with_capacity(lines.len()); + let mut removed = false; + let mut index = 0; + while index < lines.len() { + if lines[index].trim_ascii_end() == PROFILE_BLOCK_START.as_bytes() { + let end = (index + 1..lines.len()).find(|&candidate| { + lines[candidate].trim_ascii_end() == PROFILE_BLOCK_END.as_bytes() + }); + if let Some(end) = end + && names_dir(&lines[index + 1..end].concat()) + { + if kept.last().is_some_and(|line| line.trim_ascii().is_empty()) { + kept.pop(); + } + removed = true; + index = end + 1; + continue; + } + } + kept.push(lines[index]); + index += 1; + } + removed.then(|| kept.concat()) +} + +/// [`remove_profile_block_bytes`] for text, so tests read naturally. +#[cfg(test)] +fn remove_profile_block(text: &str, path_exprs: &[OsString]) -> Option { + remove_profile_block_bytes(text.as_bytes(), path_exprs) + .map(|bytes| String::from_utf8(bytes).expect("removing whole lines keeps UTF-8")) +} + +/// Names of entries in `bin_dir` that are not ROCm CLI's and are not being +/// removed: anything there that PATH may still be serving. +/// +/// A folder or entry that cannot be read is an error, not an absence: the +/// caller then keeps the PATH change, since a program it cannot see may still +/// rely on it. A name that is not UTF-8 is not ROCm CLI's, so it counts. +fn other_entries(bin_dir: &Path, removed: &[PathBuf]) -> std::io::Result> { + let mut others = Vec::new(); + for entry in fs::read_dir(bin_dir)? { + let entry = entry?; + if removed.contains(&entry.path()) { + continue; + } + let name = entry.file_name().to_string_lossy().into_owned(); + if !is_rocm_install_entry_name(&name) { + others.push(name); + } + } + others.sort(); + Ok(others) +} + +fn apply_path_edit(edit: &PathEdit) -> Result { + match edit { + PathEdit::ShellProfile { + profile, + path_exprs, + } => { + let text = fs::read(profile) + .with_context(|| format!("failed to read {}", profile.display()))?; + let Some(updated) = remove_profile_block_bytes(&text, path_exprs) else { + return Ok(format!( + "{} no longer had the rocm-cli PATH lines", + profile.display() + )); + }; + // A profile kept as a link (a dotfiles manager's, say) is edited + // where it lives, so the link stays a link. + let target = fs::canonicalize(profile) + .with_context(|| format!("failed to resolve {}", profile.display()))?; + write_replacing(&target, &updated)?; + Ok(format!( + "removed the rocm-cli PATH lines from {}; new terminals will not have them", + profile.display() + )) + } + PathEdit::UserPath { dir } => remove_user_path(dir), + } +} + +#[cfg(windows)] +fn remove_user_path(dir: &Path) -> Result { + Ok(if rocm_core::user_path::remove_user_path_entry(dir)? { + format!( + "removed {} from your user PATH; new terminals will not have it", + dir.display() + ) + } else { + format!("{} was no longer on your user PATH", dir.display()) + }) +} + +#[cfg(not(windows))] +fn remove_user_path(dir: &Path) -> Result { + // Only planned on Windows; see `plan_path_edits`. + bail!("{} is a Windows user PATH entry", dir.display()) +} + +/// Replace `path`'s contents in one step, keeping its permissions, so an +/// interrupted write never leaves a truncated shell profile behind. +/// +/// The replacement is written beside `path` as `.rocm-uninstall.tmp`. It +/// never has wider permissions than `path`, even while it is being written, so +/// a profile only its owner can read never briefly becomes readable to others. +fn write_replacing(path: &Path, contents: &[u8]) -> Result<()> { + let file_name = path + .file_name() + .with_context(|| format!("{} has no file name", path.display()))?; + let mut temp_name = file_name.to_owned(); + temp_name.push(".rocm-uninstall.tmp"); + let temp = path.with_file_name(temp_name); + let permissions = fs::metadata(path) + .with_context(|| format!("failed to read the permissions of {}", path.display()))? + .permissions(); + // A file at that name is one an interrupted uninstall left; anything else + // stays, and fails the write below. + if fs::symlink_metadata(&temp).is_ok_and(|metadata| !metadata.is_dir()) { + let _ = fs::remove_file(&temp); + } + let mut file = create_replacement(&temp, &permissions) + .with_context(|| format!("failed to create {}", temp.display()))?; + let result = file + .write_all(contents) + .and_then(|()| file.sync_all()) + .and_then(|()| { + drop(file); + // The umask may have taken bits off at creation; put them back. + fs::set_permissions(&temp, permissions) + }) + .and_then(|()| fs::rename(&temp, path)); + if let Err(error) = result { + let _ = fs::remove_file(&temp); + return Err(error).with_context(|| format!("failed to replace {}", path.display())); } - println!("uninstall complete"); Ok(()) } + +/// Create `temp` as a new file (never through a link left at that name) that +/// is no more readable than `permissions` allow. +fn create_replacement(temp: &Path, permissions: &fs::Permissions) -> std::io::Result { + let mut options = fs::OpenOptions::new(); + options.write(true).create_new(true); + #[cfg(unix)] + { + use std::os::unix::fs::{OpenOptionsExt as _, PermissionsExt as _}; + options.mode(permissions.mode() & 0o777); + } + #[cfg(not(unix))] + let _ = permissions; + options.open(temp) +} + +// --------------------------------------------------------------------------- +// The running executable +// --------------------------------------------------------------------------- + +/// Delete `exe`, the program running this command. Windows will not unlink a +/// running image, so `self_replace` moves it out of the install folder (into +/// the temp folder, or failing that, beside itself under a hidden name) and +/// deletes it once this process exits. +#[cfg(windows)] +fn delete_running_executable(exe: &Path) -> Result { + self_replace::self_delete_at(exe) + .with_context(|| format!("failed to remove the running program {}", exe.display()))?; + Ok(if exe.exists() { + format!("removed binary {} as this command exits", exe.display()) + } else { + format!("removed binary {}", exe.display()) + }) +} + +/// Only Windows plans one; elsewhere the running binary is unlinked like any +/// other file. +#[cfg(not(windows))] +fn delete_running_executable(exe: &Path) -> Result { + remove_path(exe)?; + Ok(format!("removed binary {}", exe.display())) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A fresh directory under the system temp dir, removed on drop. + struct Scratch(PathBuf); + + impl Scratch { + fn new(label: &str) -> Result { + static NEXT: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(0); + let unique = NEXT.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + let dir = std::env::temp_dir().join(format!( + "rocm-uninstall-{label}-{}-{unique}", + std::process::id() + )); + let _ = fs::remove_dir_all(&dir); + fs::create_dir_all(&dir)?; + Ok(Self(dir)) + } + + fn path(&self) -> &Path { + &self.0 + } + } + + impl Drop for Scratch { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.0); + } + } + + fn exprs() -> Vec { + profile_path_exprs( + Path::new("/home/me/.local/bin"), + Some(Path::new("/home/me")), + ) + } + + fn os(texts: &[&str]) -> Vec { + texts.iter().map(OsString::from).collect() + } + + /// Run `plan`, collecting the lines it reports, including those reported + /// before a failure. + #[cfg(unix)] + fn run(plan: &UninstallPlan) -> (Vec, Result<()>) { + let mut lines = Vec::new(); + let result = execute_plan(plan, &mut |line| lines.push(line)); + (lines, result) + } + + const POSIX_BLOCK: &str = "\n# >>> rocm-cli path >>>\ncase \":$PATH:\" in\n *:\"$HOME/.local/bin\":*) ;;\n *) export PATH=\"$HOME/.local/bin:$PATH\" ;;\nesac\n# <<< rocm-cli path <<<\n"; + + #[test] + fn profile_exprs_cover_the_literal_and_home_relative_forms() { + assert_eq!(exprs(), os(&["/home/me/.local/bin", "$HOME/.local/bin"])); + assert_eq!( + profile_path_exprs(Path::new("/home/me"), Some(Path::new("/home/me"))), + os(&["/home/me", "$HOME"]) + ); + assert_eq!( + profile_path_exprs(Path::new("/opt/rocm-cli"), Some(Path::new("/home/me"))), + os(&["/opt/rocm-cli"]) + ); + } + + #[test] + fn removing_the_block_restores_the_profile_as_it_was() { + let before = "alias ll='ls -l'\nexport EDITOR=vim\n"; + let after_install = format!("{before}{POSIX_BLOCK}"); + assert_eq!( + remove_profile_block(&after_install, &exprs()).as_deref(), + Some(before) + ); + } + + #[test] + fn a_block_in_a_profile_the_installer_created_leaves_it_empty() { + assert_eq!( + remove_profile_block(POSIX_BLOCK, &exprs()).as_deref(), + Some("") + ); + } + + #[test] + fn the_fish_block_and_the_literal_path_form_are_removed() { + let profile = "set -gx EDITOR vim\n\n# >>> rocm-cli path >>>\nif not contains -- \"/home/me/.local/bin\" $PATH\n set -gx PATH \"/home/me/.local/bin\" $PATH\nend\n# <<< rocm-cli path <<<\n"; + assert_eq!( + remove_profile_block(profile, &exprs()).as_deref(), + Some("set -gx EDITOR vim\n") + ); + } + + #[test] + fn crlf_profiles_are_handled() { + let profile = POSIX_BLOCK.replace('\n', "\r\n"); + let profile = format!("export A=1\r\n{profile}export B=2\r\n"); + assert_eq!( + remove_profile_block(&profile, &exprs()).as_deref(), + Some("export A=1\r\nexport B=2\r\n") + ); + } + + #[test] + fn a_users_own_path_line_is_never_touched() { + let profile = "export PATH=\"$HOME/.local/bin:$PATH\"\n"; + assert_eq!(remove_profile_block(profile, &exprs()), None); + } + + #[test] + fn a_block_for_another_install_folder_stays() { + let other = POSIX_BLOCK.replace("$HOME/.local/bin", "/opt/other/bin"); + assert_eq!(remove_profile_block(&other, &exprs()), None); + } + + #[test] + fn an_unterminated_block_is_left_alone() { + let profile = "# >>> rocm-cli path >>>\nexport PATH=\"$HOME/.local/bin:$PATH\"\n"; + assert_eq!(remove_profile_block(profile, &exprs()), None); + } + + #[test] + fn the_record_parses_each_kind_and_skips_blank_or_malformed_lines() { + let record = b"profile /home/me/my profile/.bashrc\r\n\nuser-path C:\\Users\\me\\.local\\bin\ngarbage\nprofile \n"; + assert_eq!( + parse_path_edit_record(record), + vec![ + ("profile", OsString::from("/home/me/my profile/.bashrc")), + ("user-path", OsString::from("C:\\Users\\me\\.local\\bin")), + ] + ); + } + + #[test] + fn other_entries_ignore_rocm_cli_files_and_what_is_being_removed() -> Result<()> { + let dir = Scratch::new("others")?; + for name in ["rocm", "rocmd", PATH_EDIT_RECORD, "uv", "notes.txt"] { + fs::write(dir.path().join(name), "")?; + } + let removed = vec![dir.path().join("notes.txt")]; + assert_eq!(other_entries(dir.path(), &removed)?, vec!["uv".to_owned()]); + Ok(()) + } + + /// A folder that cannot be listed may hold programs that rely on it being + /// on PATH, so it is an error the caller keeps the change for. + #[test] + fn a_folder_that_cannot_be_listed_is_an_error_not_an_empty_one() -> Result<()> { + let dir = Scratch::new("unlistable")?; + let not_a_folder = dir.path().join("rocm"); + fs::write(¬_a_folder, "")?; + assert!(other_entries(¬_a_folder, &[]).is_err()); + Ok(()) + } + + #[cfg(unix)] + #[test] + fn a_program_whose_name_is_not_utf8_still_counts_as_another_program() -> Result<()> { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt as _; + let dir = Scratch::new("non-utf8-other")?; + fs::write(dir.path().join("rocm"), "")?; + fs::write(dir.path().join(OsStr::from_bytes(b"tool-\xff")), "")?; + assert_eq!( + other_entries(dir.path(), &[dir.path().join("rocm")])?, + vec!["tool-\u{fffd}".to_owned()] + ); + Ok(()) + } + + #[cfg(unix)] + #[test] + fn the_replacement_profile_is_never_more_readable_than_the_original() -> Result<()> { + use std::os::unix::fs::PermissionsExt as _; + let dir = Scratch::new("replacement-mode")?; + let temp = dir.path().join("profile.rocm-uninstall.tmp"); + // Created here at 0600 whatever the umask (which only ever takes bits + // away), rather than at the umask default and narrowed afterwards. + let file = create_replacement(&temp, &fs::Permissions::from_mode(0o600))?; + assert_eq!(file.metadata()?.permissions().mode() & 0o777, 0o600); + Ok(()) + } + + #[cfg(unix)] + #[test] + fn a_replacement_left_by_an_interrupted_uninstall_does_not_block_the_next() -> Result<()> { + use std::os::unix::fs::PermissionsExt as _; + let dir = Scratch::new("stale-replacement")?; + let profile = dir.path().join("profile"); + fs::write(&profile, "old\n")?; + fs::set_permissions(&profile, fs::Permissions::from_mode(0o600))?; + fs::write(dir.path().join("profile.rocm-uninstall.tmp"), "partial")?; + write_replacing(&profile, b"new\n")?; + assert_eq!(fs::read_to_string(&profile)?, "new\n"); + assert_eq!(fs::metadata(&profile)?.permissions().mode() & 0o777, 0o600); + assert!(!dir.path().join("profile.rocm-uninstall.tmp").exists()); + Ok(()) + } + + /// Pins only the review's wording. The behavior each line promises is + /// proven end to end: the running exe by + /// `@id:lifecycle-windows-installed-binary-smoke`, the PATH undo by + /// `@id:lifecycle-linux-uninstall-full-purge` and + /// `@id:lifecycle-windows-uninstall-restores-user-path`. + #[test] + fn the_review_lists_the_record_the_running_exe_and_the_path_changes() { + let plan = UninstallPlan { + path_edits: vec![PathEdit::ShellProfile { + profile: PathBuf::from("/home/me/.bashrc"), + path_exprs: Vec::new(), + }], + path_record: Some(PathBuf::from("/opt/bin/.rocm-cli-path-edit")), + self_delete: Some(PathBuf::from("/opt/bin/rocm.exe")), + ..UninstallPlan::default() + }; + let output = render_uninstall_plan(&plan, &UninstallOptions::default()); + for expected in [ + "2 item(s) would be removed:", + " - install record: /opt/bin/.rocm-cli-path-edit", + " - binary: /opt/bin/rocm.exe (the running program; deleted as this command exits)", + "PATH changes from the installer that would be undone:\n - shell profile: the rocm-cli PATH lines in /home/me/.bashrc", + ] { + assert!( + output.contains(expected), + "missing {expected:?} in:\n{output}" + ); + } + assert!(!plan_is_empty(&plan)); + assert!(plan_is_empty(&UninstallPlan::default())); + } + + #[test] + fn the_running_executable_is_matched_whatever_case_windows_reports_it_in() { + let exe = Path::new(r"C:\Users\me\.local\bin\rocm.exe"); + let listed = Path::new(r"C:\Users\me\.local\bin\ROCM.EXE"); + assert!(crate::is_running_executable(listed, exe, true)); + assert!(crate::is_running_executable(exe, exe, true)); + let rocmd = Path::new(r"C:\Users\me\.local\bin\rocmd.exe"); + assert!(!crate::is_running_executable(rocmd, exe, true)); + // Elsewhere the running binary is unlinked like any other. + assert!(!crate::is_running_executable(exe, exe, false)); + } + + #[cfg(unix)] + mod plan { + use super::*; + + struct Layout { + _root: Scratch, + home: PathBuf, + bin: PathBuf, + removed: Vec, + } + + fn layout() -> Result { + let root = Scratch::new("plan")?; + let home = root.path().join("home"); + let bin = home.join(".local").join("bin"); + fs::create_dir_all(&bin)?; + let mut removed = Vec::new(); + for name in ["rocm", "rocmd"] { + let path = bin.join(name); + fs::write(&path, "")?; + removed.push(path); + } + Ok(Layout { + _root: root, + home, + bin, + removed, + }) + } + + fn record(layout: &Layout, lines: &str) -> Result { + let path = layout.bin.join(PATH_EDIT_RECORD); + fs::write(&path, lines)?; + Ok(path) + } + + fn plan_for(layout: &Layout) -> UninstallPlan { + let mut plan = UninstallPlan::default(); + plan_path_edits(&mut plan, &layout.bin, &layout.removed, Some(&layout.home)); + plan + } + + fn profiles(plan: &UninstallPlan) -> Vec { + plan.path_edits + .iter() + .filter_map(|edit| match edit { + PathEdit::ShellProfile { profile, .. } => Some(profile.clone()), + PathEdit::UserPath { .. } => None, + }) + .collect() + } + + /// A plan that removes `layout`'s binaries after undoing `plan`'s PATH + /// changes, as `build_uninstall_plan` lays it out. + fn with_binaries(layout: &Layout, mut plan: UninstallPlan) -> UninstallPlan { + plan.actions = layout + .removed + .iter() + .map(|path| crate::UninstallPlanEntry { + kind: "binary", + path: path.clone(), + }) + .collect(); + plan + } + + /// `plan` also removing a config folder holding the user's settings, + /// as a plain `rocm uninstall` does. + fn with_config( + layout: &Layout, + mut plan: UninstallPlan, + ) -> Result<(UninstallPlan, PathBuf)> { + let config = layout.home.join("config"); + fs::create_dir_all(&config)?; + fs::write(config.join("config.json"), "{}")?; + plan.actions.insert( + 0, + crate::UninstallPlanEntry { + kind: "config", + path: config.clone(), + }, + ); + Ok((plan, config)) + } + + #[test] + fn a_recorded_profile_block_is_undone_and_the_record_goes_last() -> Result<()> { + let layout = layout()?; + let profile = layout.home.join("my-profile"); + fs::write(&profile, format!("export A=1\n{POSIX_BLOCK}"))?; + let record = record(&layout, &format!("profile {}\n", profile.display()))?; + let plan = plan_for(&layout); + assert_eq!(profiles(&plan), vec![profile.clone()], "{plan:?}"); + assert_eq!(plan.path_record.as_ref(), Some(&record)); + + let (lines, result) = run(&with_binaries(&layout, plan)); + result?; + assert_eq!(fs::read_to_string(&profile)?, "export A=1\n"); + assert!(!record.exists() && !layout.removed[0].exists()); + let undo = lines + .iter() + .position(|line| line.starts_with("removed the rocm-cli PATH lines")); + let binary = lines + .iter() + .position(|line| line.starts_with("removed binary")); + assert!(undo < binary, "the PATH change is undone first: {lines:?}"); + Ok(()) + } + + #[test] + fn a_path_change_that_cannot_be_undone_stops_before_the_binaries_go() -> Result<()> { + let layout = layout()?; + let profile = layout.home.join(".bashrc"); + let contents = format!("export A=1\n{POSIX_BLOCK}"); + fs::write(&profile, &contents)?; + let record = record(&layout, &format!("profile {}\n", profile.display()))?; + // The edit writes its replacement beside the profile first; a folder + // in that spot makes the write fail even for root. + fs::create_dir(layout.home.join(".bashrc.rocm-uninstall.tmp"))?; + let (plan, config) = with_config(&layout, with_binaries(&layout, plan_for(&layout)))?; + + let (lines, result) = run(&plan); + let message = format!("{:#}", result.expect_err("the PATH undo fails")); + // The message claims nothing else went; the state below proves it. + assert!( + message.contains("could not undo the installer's PATH change") + && message.contains("Nothing else was removed") + && message.contains("`rocm uninstall` again"), + "{message}" + ); + assert!(lines.is_empty(), "nothing was reported removed: {lines:?}"); + assert_eq!(fs::read_to_string(&profile)?, contents); + assert!(layout.removed.iter().all(|path| path.exists())); + assert!(config.join("config.json").exists(), "the settings stay"); + assert!(record.exists(), "the record stays for the retry"); + + // And the retry the message names does clear it. + fs::remove_dir(layout.home.join(".bashrc.rocm-uninstall.tmp"))?; + let (plan, _) = with_config(&layout, with_binaries(&layout, plan_for(&layout)))?; + run(&plan).1?; + assert_eq!(fs::read_to_string(&profile)?, "export A=1\n"); + assert!(!config.exists() && !record.exists()); + Ok(()) + } + + /// A step that fails after the PATH undo must not hide what was + /// already done: each line is reported as its step finishes. + #[test] + fn a_failure_after_the_path_undo_still_reports_what_was_done() -> Result<()> { + let layout = layout()?; + let profile = layout.home.join(".bashrc"); + fs::write(&profile, format!("export A=1\n{POSIX_BLOCK}"))?; + let (mut plan, config) = + with_config(&layout, with_binaries(&layout, plan_for(&layout)))?; + // A name too long for any filesystem fails to remove, even for root. + plan.actions.push(crate::UninstallPlanEntry { + kind: "binary", + path: layout.bin.join("x".repeat(300)), + }); + + let (lines, result) = run(&plan); + assert!(result.is_err(), "the last removal fails: {lines:?}"); + // Each reported line matches what really happened. + assert_eq!(fs::read_to_string(&profile)?, "export A=1\n"); + assert!(!config.exists() && !layout.removed[0].exists()); + for expected in [ + format!("removed the rocm-cli PATH lines from {}", profile.display()), + format!("removed config {}", config.display()), + format!("removed binary {}", layout.removed[0].display()), + ] { + assert!( + lines.iter().any(|line| line.starts_with(&expected)), + "missing {expected:?} in {lines:?}" + ); + } + Ok(()) + } + + #[test] + fn without_a_record_the_marked_block_in_a_known_profile_is_found() -> Result<()> { + let layout = layout()?; + fs::write(layout.home.join(".bashrc"), POSIX_BLOCK)?; + fs::write( + layout.home.join(".zshrc"), + "export PATH=\"$HOME/.local/bin:$PATH\"\n", + )?; + let plan = plan_for(&layout); + assert_eq!(profiles(&plan), vec![layout.home.join(".bashrc")]); + assert_eq!(plan.path_record, None); + Ok(()) + } + + #[test] + fn a_block_from_an_earlier_install_in_another_shell_is_also_undone() -> Result<()> { + let layout = layout()?; + fs::write(layout.home.join(".bashrc"), POSIX_BLOCK)?; + fs::write(layout.home.join(".zshrc"), POSIX_BLOCK)?; + record( + &layout, + &format!("profile {}\n", layout.home.join(".zshrc").display()), + )?; + let plan = plan_for(&layout); + assert_eq!( + profiles(&plan), + vec![layout.home.join(".zshrc"), layout.home.join(".bashrc")] + ); + Ok(()) + } + + #[test] + fn another_program_in_the_folder_keeps_the_change_and_its_record() -> Result<()> { + let layout = layout()?; + fs::write(layout.home.join(".bashrc"), POSIX_BLOCK)?; + record( + &layout, + &format!("profile {}\n", layout.home.join(".bashrc").display()), + )?; + fs::write(layout.bin.join("uv"), "")?; + let plan = plan_for(&layout); + assert!(plan.path_edits.is_empty(), "{plan:?}"); + assert_eq!(plan.path_record, None, "the record stays with the change"); + let record = layout.bin.join(PATH_EDIT_RECORD); + assert!( + plan.skipped + .iter() + .any(|note| note.starts_with("PATH change left in place") + && note.contains("uv") + && note.contains(&format!("its record {} stays", record.display()))), + "{plan:?}" + ); + Ok(()) + } + + #[test] + fn a_recorded_profile_without_the_block_is_reported_not_edited() -> Result<()> { + let layout = layout()?; + let profile = layout.home.join(".bashrc"); + fs::write(&profile, "export A=1\n")?; + let record = record(&layout, &format!("profile {}\n", profile.display()))?; + let plan = plan_for(&layout); + assert!(plan.path_edits.is_empty()); + assert!( + plan.skipped + .iter() + .any(|note| note.contains("nothing to undo")) + ); + assert_eq!(plan.path_record, Some(record), "a spent record goes"); + Ok(()) + } + + /// A profile the installer recorded writing to most likely still holds + /// its block, so one that cannot be read stops the uninstall like any + /// change that cannot be undone: before anything is removed. + #[test] + fn a_recorded_profile_that_cannot_be_read_stops_before_anything_goes() -> Result<()> { + let layout = layout()?; + // A folder where the profile should be cannot be read as one, even + // by root. + let profile = layout.home.join("my-profile"); + fs::create_dir(&profile)?; + let record = record(&layout, &format!("profile {}\n", profile.display()))?; + let plan = plan_for(&layout); + assert_eq!(profiles(&plan), vec![profile], "{plan:?}"); + assert!( + plan.warnings + .iter() + .any(|note| note.starts_with("could not read") + && note.contains("my-profile") + && note.contains("nothing will be removed")), + "{plan:?}" + ); + + let (plan, config) = with_config(&layout, with_binaries(&layout, plan))?; + let message = format!("{:#}", run(&plan).1.expect_err("the profile is unreadable")); + assert!(message.contains("could not undo"), "{message}"); + assert!(layout.removed.iter().all(|path| path.exists())); + assert!(config.exists() && record.exists()); + Ok(()) + } + + /// A profile only scanned for, not recorded, may not hold a block at + /// all, so one that cannot be read is reported and does not stop the + /// uninstall. + #[test] + fn an_unrecorded_profile_that_cannot_be_read_is_a_warning_not_an_absence() -> Result<()> { + let layout = layout()?; + fs::create_dir(layout.home.join(".bashrc"))?; + let plan = plan_for(&layout); + assert!(plan.path_edits.is_empty(), "{plan:?}"); + assert!( + plan.warnings + .iter() + .any(|note| note.starts_with("could not read") + && note.contains(".bashrc") + && note.contains("left in place")), + "{plan:?}" + ); + Ok(()) + } + + #[test] + fn a_record_naming_a_profile_that_is_not_utf8_is_still_followed() -> Result<()> { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt as _; + let layout = layout()?; + let profile = layout.home.join(OsStr::from_bytes(b"caf\xe9-profile")); + fs::write(&profile, format!("export A=1\n{POSIX_BLOCK}"))?; + let mut record_bytes = b"profile ".to_vec(); + record_bytes.extend_from_slice(profile.as_os_str().as_bytes()); + record_bytes.push(b'\n'); + fs::write(layout.bin.join(PATH_EDIT_RECORD), record_bytes)?; + let plan = plan_for(&layout); + assert!(plan.warnings.is_empty(), "{plan:?}"); + assert_eq!(profiles(&plan), vec![profile.clone()]); + apply_path_edit(&plan.path_edits[0])?; + assert_eq!(fs::read(&profile)?, b"export A=1\n"); + Ok(()) + } + + #[test] + fn an_install_folder_whose_path_is_not_utf8_is_still_found() -> Result<()> { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt as _; + let layout = layout()?; + // Outside the home folder, so the block spells it literally. + let bin = layout.home.with_file_name(OsStr::from_bytes(b"bin-\xff")); + fs::create_dir_all(&bin)?; + let mut block = b"\n# >>> rocm-cli path >>>\ncase \":$PATH:\" in\n *:\"".to_vec(); + block.extend_from_slice(bin.as_os_str().as_bytes()); + block.extend_from_slice(b"\":*) ;;\nesac\n# <<< rocm-cli path <<<\n"); + fs::write(layout.home.join(".bashrc"), &block)?; + let mut plan = UninstallPlan::default(); + plan_path_edits(&mut plan, &bin, &[], Some(&layout.home)); + assert_eq!( + profiles(&plan), + vec![layout.home.join(".bashrc")], + "{plan:?}" + ); + Ok(()) + } + + #[test] + fn a_profile_that_is_not_utf8_is_still_edited_byte_for_byte() -> Result<()> { + let layout = layout()?; + let profile = layout.home.join(".bashrc"); + let before = b"# caf\xe9 latin-1\nexport A=1\n".to_vec(); + let mut installed = before.clone(); + installed.extend_from_slice(POSIX_BLOCK.as_bytes()); + fs::write(&profile, &installed)?; + fs::write(layout.home.join(".zshrc"), b"# \xff not ours\n")?; + let plan = plan_for(&layout); + assert!(plan.warnings.is_empty(), "{plan:?}"); + assert_eq!(profiles(&plan), vec![profile.clone()]); + apply_path_edit(&plan.path_edits[0])?; + assert_eq!(fs::read(&profile)?, before); + Ok(()) + } + + #[test] + fn a_block_written_through_a_linked_home_is_still_found() -> Result<()> { + let layout = layout()?; + // The installer saw HOME as a link; the binary reports its folder + // resolved, under the real home. + let link = layout.home.with_file_name("home-link"); + std::os::unix::fs::symlink(&layout.home, &link)?; + fs::write(layout.home.join(".bashrc"), POSIX_BLOCK)?; + let mut plan = UninstallPlan::default(); + plan_path_edits(&mut plan, &layout.bin, &layout.removed, Some(&link)); + assert_eq!(profiles(&plan), vec![link.join(".bashrc")], "{plan:?}"); + Ok(()) + } + + #[test] + fn a_recorded_spelling_is_matched_even_when_no_derived_one_is() -> Result<()> { + let layout = layout()?; + let block = POSIX_BLOCK.replace("$HOME/.local/bin", "/elsewhere/bin/"); + fs::write(layout.home.join(".bashrc"), &block)?; + record(&layout, "path-expr /elsewhere/bin/\n")?; + assert_eq!( + profiles(&plan_for(&layout)), + vec![layout.home.join(".bashrc")] + ); + Ok(()) + } + + /// `--keep-binaries` keeps the installer's PATH change and its record + /// with the binaries, and says so. Run against an installed folder that + /// holds both, so the same plan without the flag would undo them. + #[test] + fn keeping_the_binaries_keeps_the_path_change_and_says_so() -> Result<()> { + let layout = layout()?; + let profile = layout.home.join(".bashrc"); + fs::write(&profile, POSIX_BLOCK)?; + let record = record(&layout, &format!("profile {}\n", profile.display()))?; + let plan_with = |keep_binaries| -> Result { + let mut plan = UninstallPlan::default(); + let options = UninstallOptions { + keep_binaries, + ..UninstallOptions::default() + }; + crate::plan_installed_binaries( + &mut plan, + &options, + || Ok(layout.removed[0].clone()), + Some(&layout.home), + )?; + Ok(plan) + }; + + let plan = plan_with(false)?; + assert_eq!(profiles(&plan), vec![profile.clone()], "{plan:?}"); + assert_eq!(plan.path_record.as_ref(), Some(&record)); + + let plan = plan_with(true)?; + assert!( + plan.actions.is_empty() && plan.path_edits.is_empty(), + "{plan:?}" + ); + assert_eq!(plan.path_record, None, "{plan:?}"); + assert!( + plan.skipped + .iter() + .any(|note| note.contains("--keep-binaries") + && note.contains("PATH change the installer made stays too")), + "{plan:?}" + ); + // Carrying the plan out leaves the change, its record, and the + // binaries where they are. + run(&plan).1?; + assert_eq!(fs::read_to_string(&profile)?, POSIX_BLOCK); + assert!(record.exists() && layout.removed.iter().all(|path| path.exists())); + Ok(()) + } + + #[test] + fn a_linked_profile_is_edited_where_it_lives_and_stays_a_link() -> Result<()> { + use std::os::unix::fs::PermissionsExt as _; + let layout = layout()?; + let dotfiles = layout.home.join("dotfiles"); + fs::create_dir(&dotfiles)?; + let real = dotfiles.join("bashrc"); + fs::write(&real, format!("export A=1\n{POSIX_BLOCK}"))?; + fs::set_permissions(&real, fs::Permissions::from_mode(0o640))?; + let profile = layout.home.join(".bashrc"); + std::os::unix::fs::symlink(&real, &profile)?; + let plan = plan_for(&layout); + let line = apply_path_edit(&plan.path_edits[0])?; + assert!( + line.starts_with("removed the rocm-cli PATH lines"), + "{line}" + ); + assert!(fs::symlink_metadata(&profile)?.is_symlink()); + assert_eq!(fs::read_to_string(&real)?, "export A=1\n"); + assert_eq!(fs::metadata(&real)?.permissions().mode() & 0o777, 0o640); + Ok(()) + } + } +} diff --git a/crates/rocm-core/Cargo.toml b/crates/rocm-core/Cargo.toml index 1fd0dc4fe..8dc0d950b 100644 --- a/crates/rocm-core/Cargo.toml +++ b/crates/rocm-core/Cargo.toml @@ -52,4 +52,4 @@ tar = "0.4" tempfile = "3" [target.'cfg(target_os = "windows")'.dependencies] -windows-sys = { version = "0.61", features = ["Win32_Foundation", "Win32_Security", "Win32_Storage_FileSystem", "Win32_System_Registry", "Win32_System_SystemInformation", "Win32_System_Threading"] } +windows-sys = { version = "0.61", features = ["Win32_Foundation", "Win32_Security", "Win32_Storage_FileSystem", "Win32_System_Registry", "Win32_System_SystemInformation", "Win32_System_Threading", "Win32_UI_WindowsAndMessaging"] } diff --git a/crates/rocm-core/src/lib.rs b/crates/rocm-core/src/lib.rs index a23896c3f..790d4da4e 100644 --- a/crates/rocm-core/src/lib.rs +++ b/crates/rocm-core/src/lib.rs @@ -36,6 +36,7 @@ pub mod report_delivery; pub mod rocm_install; pub mod runtime; pub mod terminal; +pub mod user_path; #[cfg(test)] mod test_env; diff --git a/crates/rocm-core/src/user_path.rs b/crates/rocm-core/src/user_path.rs new file mode 100644 index 000000000..92a6bea71 --- /dev/null +++ b/crates/rocm-core/src/user_path.rs @@ -0,0 +1,220 @@ +// Copyright © Advanced Micro Devices, Inc., or its affiliates. +// +// SPDX-License-Identifier: MIT + +//! The per-user `Path` that `install.ps1` adds the install directory to. +//! +//! The installer writes it through `[Environment]::SetEnvironmentVariable(.., +//! "User")`, which stores it under `HKCU\Environment`. `rocm uninstall` takes the +//! entry back out here, so nothing it added is left pointing at a removed folder. + +use std::path::Path; + +/// `list` with every entry naming `dir` removed, or `None` when there is none. +/// +/// Entries compare the way Windows resolves them: case-insensitively and +/// ignoring a trailing separator. Every other entry, including empty ones and +/// ones that use `%VARIABLES%`, is kept byte for byte, so the rest of the +/// user's `Path` reads back exactly as it was. +#[must_use] +pub fn remove_path_list_entry(list: &str, dir: &Path) -> Option { + let target = normalize_entry(&dir.to_string_lossy()); + if target.is_empty() { + return None; + } + let entries: Vec<&str> = list.split(';').collect(); + let kept: Vec<&str> = entries + .iter() + .copied() + .filter(|entry| !normalize_entry(entry).eq_ignore_ascii_case(&target)) + .collect(); + (kept.len() != entries.len()).then(|| kept.join(";")) +} + +fn normalize_entry(entry: &str) -> String { + entry.trim().trim_end_matches(['\\', '/']).to_owned() +} + +/// Remove `dir` from the user `Path` in `HKCU\Environment`. +/// +/// Keeps the value's registry type (`REG_SZ` or `REG_EXPAND_SZ`), then tells +/// running programs the environment changed. Returns whether an entry was +/// removed. +#[cfg(windows)] +#[allow(unsafe_code)] // Win32 FFI +pub fn remove_user_path_entry(dir: &Path) -> anyhow::Result { + use anyhow::{Context, bail}; + use windows_sys::Win32::Foundation::{ERROR_FILE_NOT_FOUND, ERROR_SUCCESS}; + use windows_sys::Win32::System::Registry::{ + HKEY, HKEY_CURRENT_USER, KEY_QUERY_VALUE, KEY_SET_VALUE, REG_EXPAND_SZ, REG_SZ, + RegCloseKey, RegOpenKeyExW, RegQueryValueExW, RegSetValueExW, + }; + use windows_sys::Win32::UI::WindowsAndMessaging::{ + HWND_BROADCAST, SMTO_ABORTIFHUNG, SendMessageTimeoutW, WM_SETTINGCHANGE, + }; + + let wide = |text: &str| text.encode_utf16().chain(Some(0)).collect::>(); + let subkey = wide("Environment"); + let value_name = wide("Path"); + + let mut key: HKEY = std::ptr::null_mut(); + let status = unsafe { + RegOpenKeyExW( + HKEY_CURRENT_USER, + subkey.as_ptr(), + 0, + KEY_QUERY_VALUE | KEY_SET_VALUE, + &raw mut key, + ) + }; + if status == ERROR_FILE_NOT_FOUND { + return Ok(false); + } + if status != ERROR_SUCCESS { + bail!( + "could not open HKCU\\Environment: {}", + std::io::Error::from_raw_os_error(status.cast_signed()) + ); + } + let result = (|| -> anyhow::Result { + let mut value_type = 0u32; + let mut size = 0u32; + let status = unsafe { + RegQueryValueExW( + key, + value_name.as_ptr(), + std::ptr::null(), + &raw mut value_type, + std::ptr::null_mut(), + &raw mut size, + ) + }; + if status == ERROR_FILE_NOT_FOUND { + return Ok(false); + } + if status != ERROR_SUCCESS { + bail!( + "could not read the user Path: {}", + std::io::Error::from_raw_os_error(status.cast_signed()) + ); + } + if value_type != REG_SZ && value_type != REG_EXPAND_SZ { + bail!("the user Path has an unexpected registry type ({value_type})"); + } + let mut buffer = vec![0u16; (size as usize).div_ceil(2)]; + let status = unsafe { + RegQueryValueExW( + key, + value_name.as_ptr(), + std::ptr::null(), + &raw mut value_type, + buffer.as_mut_ptr().cast(), + &raw mut size, + ) + }; + if status != ERROR_SUCCESS { + bail!( + "could not read the user Path: {}", + std::io::Error::from_raw_os_error(status.cast_signed()) + ); + } + buffer.truncate((size as usize) / 2); + while buffer.last() == Some(&0) { + buffer.pop(); + } + let current = String::from_utf16(&buffer).context("the user Path is not valid UTF-16")?; + let Some(updated) = remove_path_list_entry(¤t, dir) else { + return Ok(false); + }; + let data = wide(&updated); + let byte_len = u32::try_from(data.len() * 2).context("the user Path is too long")?; + let status = unsafe { + RegSetValueExW( + key, + value_name.as_ptr(), + 0, + value_type, + data.as_ptr().cast(), + byte_len, + ) + }; + if status != ERROR_SUCCESS { + bail!( + "could not write the user Path: {}", + std::io::Error::from_raw_os_error(status.cast_signed()) + ); + } + Ok(true) + })(); + unsafe { + RegCloseKey(key); + } + let removed = result?; + if removed { + // Explorer rereads the user environment on this broadcast, so terminals + // opened from it afterwards no longer get the folder. Best effort: the + // registry write above is the change itself. + let environment = wide("Environment"); + unsafe { + SendMessageTimeoutW( + HWND_BROADCAST, + WM_SETTINGCHANGE, + 0, + environment.as_ptr() as isize, + SMTO_ABORTIFHUNG, + 5000, + std::ptr::null_mut(), + ); + } + } + Ok(removed) +} + +#[cfg(test)] +mod tests { + use super::remove_path_list_entry; + use std::path::Path; + + const DIR: &str = r"C:\Users\me\.local\bin"; + + #[test] + fn removes_only_the_matching_entry_and_keeps_the_rest_verbatim() { + let list = r"C:\Users\me\.local\bin;%USERPROFILE%\go\bin;;C:\tools"; + assert_eq!( + remove_path_list_entry(list, Path::new(DIR)).as_deref(), + Some(r"%USERPROFILE%\go\bin;;C:\tools") + ); + } + + #[test] + fn matches_case_insensitively_and_ignores_a_trailing_separator() { + let list = r"C:\tools;c:\users\ME\.LOCAL\bin\"; + assert_eq!( + remove_path_list_entry(list, Path::new(DIR)).as_deref(), + Some(r"C:\tools") + ); + } + + #[test] + fn removes_every_duplicate() { + let list = format!("{DIR};C:\\tools;{DIR}"); + assert_eq!( + remove_path_list_entry(&list, Path::new(DIR)).as_deref(), + Some(r"C:\tools") + ); + } + + #[test] + fn returns_none_when_the_dir_is_absent() { + assert_eq!( + remove_path_list_entry(r"C:\tools;C:\Users\me\.local\bin2", Path::new(DIR)), + None + ); + assert_eq!(remove_path_list_entry("", Path::new(DIR)), None); + } + + #[test] + fn an_empty_dir_never_matches_empty_entries() { + assert_eq!(remove_path_list_entry(r"C:\tools;;", Path::new("")), None); + } +} diff --git a/docs/architecture.md b/docs/architecture.md index 8d58c164c..4189e7ac9 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -15,7 +15,7 @@ This is the living module map for rocm-cli. It's a contributor-facing reference New subcommands and subsystems default to their own file from day one — they should not grow inside `main.rs`/`lib.rs` waiting for a future extraction pass. Two extraction patterns already exist in the codebase; use whichever fits: - **Full domain extraction** — a subsystem's domain implementation moves into its own file that owns its own types (structs/enums), not just relocated functions; that ownership is usually what distinguishes this pattern from mechanical relocation below, though the Phase 6a variant described next shows ownership alone doesn't fully tell them apart — every `apps/rocm` module is a private `mod x;` regardless of pattern, so module privacy alone doesn't tell the two apart either. Full-domain-extraction modules are typically reached via qualified paths (e.g. `comfyui::render_status(...)`); mechanically relocated modules with no owned types are reached the same way (e.g. [`endpoint_keys.rs`](../apps/rocm/src/endpoint_keys.rs), [`logging.rs`](../apps/rocm/src/logging.rs)). The exception — reached unqualified through `use crate::x::{...}` re-imports, matching their dispatch call sites' pre-extraction form — is limited to the clusters that moved as a dispatch-adjacent group: [`automations.rs`](../apps/rocm/src/automations.rs), [`uninstall.rs`](../apps/rocm/src/uninstall.rs), and (as of Phase 6a, ROCMAI-82) [`driver_install.rs`](../apps/rocm/src/driver_install.rs), [`engines_cmd.rs`](../apps/rocm/src/engines_cmd.rs), [`serve_cmd.rs`](../apps/rocm/src/serve_cmd.rs). [`service_launch.rs`](../apps/rocm/src/service_launch.rs) (as of Phase 6b, ROCMAI-91) uses the same unqualified re-import form for its cross-module callers, but it is not a dispatch-adjacent cluster in the same sense — one of its callers, `automations.rs`, is not a dispatch call site, and its pre-extraction form was a qualified `crate::` call rather than an unqualified import, so the re-import is new rather than kept. This call-site form, not type ownership or dispatch-fn placement, is what actually separates the Phase 6a variant below from full domain extraction. Where a subsystem has a dedicated clap subcommand, its command *enum* usually stays in `main.rs` as part of the `Command`/`Cli` definition (e.g. `ComfyuiCommand`, `RuntimesCommand`) — but the dispatch function itself does not always stay with it: `comfyui()` does, while `engines()` (Phase 6a) and `runtimes()` (Phase 6c, ROCMAI-89) moved out to `engines_cmd.rs`/`runtimes_cmd.rs` respectively, following the mechanical-relocation re-import form below rather than full domain extraction's qualified-path form. Not every domain-extracted module has a dedicated command enum at all (`providers.rs` has none; it's invoked from the existing chat/config command flows). In library crates (`crates/rocm-core`) the module is `pub mod x;` plus a `pub use x::{...};` re-export, since it's part of the crate's public API. This is the default for new subsystems. Examples: [`apps/rocm/src/therock.rs`](../apps/rocm/src/therock.rs) (`RuntimesCommand`'s domain types), [`comfyui.rs`](../apps/rocm/src/comfyui.rs) (`ComfyuiCommand`), [`providers.rs`](../apps/rocm/src/providers.rs) (no dedicated command enum); `crates/rocm-core`'s [`diagnose.rs`](../crates/rocm-core/src/diagnose.rs)/[`examine.rs`](../crates/rocm-core/src/examine.rs). -- **Mechanical relocation** — a `pub(crate) fn` moves out verbatim, with shared types/config staying at the crate root and reached via `crate::`. Used for dispatch-adjacent clusters where a minimal, easy-to-review diff matters more than full extraction. Examples: [`apps/rocm/src/automations.rs`](../apps/rocm/src/automations.rs), [`uninstall.rs`](../apps/rocm/src/uninstall.rs). Variant: when the cluster is large enough to own private types (part of `main.rs`'s Phase 6a split, ROCMAI-82, `main.rs`'s Phase 6b split, ROCMAI-91, and `main.rs`'s Phase 6c split, ROCMAI-89), those types move with it instead of staying in `main.rs` — [`engines_cmd.rs`](../apps/rocm/src/engines_cmd.rs)'s types are used nowhere else, but [`driver_install.rs`](../apps/rocm/src/driver_install.rs)'s `DriverInstallResult`/`DriverInstallError` are `pub(crate)` and read from `main.rs`'s `install()` (`result.output`/`result.executed`, `error.source`/`error.executed`); [`serve_cmd.rs`](../apps/rocm/src/serve_cmd.rs)'s `ServeEngineSelection`/`ServeArgs` both stay `pub(crate)` and are both used from `main.rs` — `assess_model_for_host` calls `select_serve_engine` and reads the resulting `ServeEngineSelection`'s `.engine`/`.source` fields — and `ServeGenerationDefaults` is private; [`service_launch.rs`](../apps/rocm/src/service_launch.rs)'s `AttachOutcome`/`RawModeGuard`/`ManagedSpawn`/`AutostartClaim` are private, and its `ManagedLaunchReport` is `pub(crate)` and read from [`serve_cmd.rs`](../apps/rocm/src/serve_cmd.rs) for the interactive summary; [`runtimes_cmd.rs`](../apps/rocm/src/runtimes_cmd.rs)'s `RuntimeActivationResult` and `ActiveRuntimeMarker` are `pub(crate)` and read from `main.rs`'s `apply_runtime_update`/`append_update_activate_summary` and from [`storage.rs`](../apps/rocm/src/storage.rs) respectively; its sibling [`runtime_install_finalize.rs`](../apps/rocm/src/runtime_install_finalize.rs)'s `SdkInstallConsents` is `pub(crate)` and read from `main.rs`'s `install()`. The pattern is still "mechanical relocation" rather than full domain extraction because call sites keep their pre-extraction unqualified-re-import form — the exception named above — rather than switching to the qualified paths full-domain-extraction modules use. The top-level dispatch fns (`install()`/`dispatch()`) staying in `main.rs` is not what makes it so: full domain extraction usually keeps its dispatch fn in `main.rs` too (see `comfyui()` above). +- **Mechanical relocation** — a `pub(crate) fn` moves out verbatim, with shared types/config staying at the crate root and reached via `crate::`. Used for dispatch-adjacent clusters where a minimal, easy-to-review diff matters more than full extraction. Example: [`apps/rocm/src/automations.rs`](../apps/rocm/src/automations.rs). Variant: when the cluster is large enough to own private types (part of `main.rs`'s Phase 6a split, ROCMAI-82, `main.rs`'s Phase 6b split, ROCMAI-91, and `main.rs`'s Phase 6c split, ROCMAI-89), those types move with it instead of staying in `main.rs` — [`engines_cmd.rs`](../apps/rocm/src/engines_cmd.rs)'s types are used nowhere else, but [`driver_install.rs`](../apps/rocm/src/driver_install.rs)'s `DriverInstallResult`/`DriverInstallError` are `pub(crate)` and read from `main.rs`'s `install()` (`result.output`/`result.executed`, `error.source`/`error.executed`); [`serve_cmd.rs`](../apps/rocm/src/serve_cmd.rs)'s `ServeEngineSelection`/`ServeArgs` both stay `pub(crate)` and are both used from `main.rs` — `assess_model_for_host` calls `select_serve_engine` and reads the resulting `ServeEngineSelection`'s `.engine`/`.source` fields — and `ServeGenerationDefaults` is private; [`service_launch.rs`](../apps/rocm/src/service_launch.rs)'s `AttachOutcome`/`RawModeGuard`/`ManagedSpawn`/`AutostartClaim` are private, and its `ManagedLaunchReport` is `pub(crate)` and read from [`serve_cmd.rs`](../apps/rocm/src/serve_cmd.rs) for the interactive summary; [`runtimes_cmd.rs`](../apps/rocm/src/runtimes_cmd.rs)'s `RuntimeActivationResult` and `ActiveRuntimeMarker` are `pub(crate)` and read from `main.rs`'s `apply_runtime_update`/`append_update_activate_summary` and from [`storage.rs`](../apps/rocm/src/storage.rs) respectively; its sibling [`runtime_install_finalize.rs`](../apps/rocm/src/runtime_install_finalize.rs)'s `SdkInstallConsents` is `pub(crate)` and read from `main.rs`'s `install()`. The pattern is still "mechanical relocation" rather than full domain extraction because call sites keep their pre-extraction unqualified-re-import form — the exception named above — rather than switching to the qualified paths full-domain-extraction modules use. The top-level dispatch fns (`install()`/`dispatch()`) staying in `main.rs` is not what makes it so: full domain extraction usually keeps its dispatch fn in `main.rs` too (see `comfyui()` above). There is no file-line-count CI gate enforcing this — `too_many_lines = "allow"` in the workspace `Cargo.toml` is a deliberate, function-level choice, not an oversight. This convention is the guardrail instead. @@ -52,7 +52,7 @@ Each per-crate module inventory lists one file per line. Flat inventories (every - [`service_launch.rs`](../apps/rocm/src/service_launch.rs) — mechanical relocation, owns private types (Phase 6b, ROCMAI-91): spawning/attaching to a managed service process and streaming its log, plus the endpoint-key/auth checks that gate a public bind — `AttachOutcome`/`RawModeGuard`/`ManagedSpawn`/`AutostartClaim` are its private types; `ManagedLaunchReport` is `pub(crate)`, since [`serve_cmd.rs`](../apps/rocm/src/serve_cmd.rs) reads its fields directly for the interactive summary. Most of this module's functions are `pub(crate)` for the same reason — [`main.rs`](../apps/rocm/src/main.rs), [`serve_cmd.rs`](../apps/rocm/src/serve_cmd.rs), and [`automations.rs`](../apps/rocm/src/automations.rs) all call into it. `ServicesCommand`/`services()` stayed in [`main.rs`](../apps/rocm/src/main.rs) rather than moving alongside — the service-record-management helpers `services()` dispatches to (stop/restart/remove/prune) are entangled with each other; only stop/restart are also reachable from the chat/LLM sandbox tool-calling path, out of scope for Phase 6b. - [`storage.rs`](../apps/rocm/src/storage.rs) — full domain extraction. - [`therock.rs`](../apps/rocm/src/therock.rs) — full domain extraction; owns TheRock SDK install/update and the runtime-manifest model/loaders. The `runtimes` subcommand's own command enum, dispatch fn, and most of its domain logic — rendering, activation, and manifest import — stay in [`main.rs`](../apps/rocm/src/main.rs). -- [`uninstall.rs`](../apps/rocm/src/uninstall.rs) — mechanical relocation, dispatch-adjacent, no owned types. +- [`uninstall.rs`](../apps/rocm/src/uninstall.rs) — mechanical relocation, owns private types: `uninstall()`, plus the installer `PATH` undo and running-executable deletion. The shared plan type and its file-removal helpers stay in [`main.rs`](../apps/rocm/src/main.rs) because [`storage.rs`](../apps/rocm/src/storage.rs) reuses them. ### `apps/rocmd` — background daemon diff --git a/docs/engine-plugins.md b/docs/engine-plugins.md index 23a3c4e87..dda0efc96 100644 --- a/docs/engine-plugins.md +++ b/docs/engine-plugins.md @@ -91,7 +91,9 @@ The same output is available in the TUI with `/engine`. Installer policy: - `install.sh` and `install.ps1` update only the rocm-cli binary install - directory and its `.rocm-cli-manifest`. + directory (its binaries, `.rocm-cli-manifest`, and the `.rocm-cli-path-edit` + record of the `PATH` change they made) and that `PATH` change itself: a + marked block in the shell profile, or the user `PATH` on Windows. - External plugins under the rocm-cli data directory are not touched by install or upgrade. - `rocm uninstall` removes the data directory by default. Use diff --git a/docs/manual-testing.md b/docs/manual-testing.md index 157c01ad7..dd5db6077 100644 --- a/docs/manual-testing.md +++ b/docs/manual-testing.md @@ -615,3 +615,27 @@ To turn provider-assisted planning back off: ```powershell rocm config clear-planner-provider ``` + +## 8. Uninstall + +Run this last, on a machine installed with the release installer (not a +`cargo` build, whose binaries uninstall leaves alone): + +```powershell +rocm uninstall --dry-run +rocm uninstall +``` + +Expected result: + +- The dry run lists the binaries (on Windows, `rocm.exe` is marked as deleted + as the command exits) and, under + `PATH changes from the installer that would be undone:`, the shell profile + (Linux) or the user `PATH` entry (Windows). It removes nothing. +- After confirming, the install folder holds no `rocm`/`rocmd` files, and a new + terminal no longer gets the install folder on `PATH` from the installer's + change. +- If another program lives in the install folder, the review instead says + `PATH change left in place` and names it, and the `PATH` change stays. +- With `rocm uninstall --keep-binaries`, the review says the `PATH` change + stays too, and it does. diff --git a/docs/testing.md b/docs/testing.md index 1374b0bd5..a523d990a 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -1347,7 +1347,14 @@ generated public-key PEM installer verification; verify first-install PATH setup Linux writes the shell profile); reinstall stale-manifest purge and config preservation; a Windows loopback-HTTP install; isolated installed-binary directory smoke checks (`rocm examine` must read only the isolated -config/data/cache, never the real user `.rocm` state); and full-purge uninstall. +config/data/cache, never the real user `.rocm` state); and uninstall: a full +purge that also restores the shell profile, the Windows user `PATH` taken back +off with its other entries and its registry type kept, the `PATH` change kept while another program shares the install folder or +with `--keep-binaries`, a `PATH` change that cannot be undone removing nothing +so it can be retried, a profile and install-folder spelling found only through +the installer's record, and the running Windows `rocm.exe` removed. The two +Windows scenarios that change the user `PATH` are `@serial`, since it is one +registry value for the whole machine. Each scenario generates its own local keys, package, and install root under a per-scenario temp directory, so they are independent and use generated local keys only — project-owned production signing keys remain an owner-controlled diff --git a/install.ps1 b/install.ps1 index 0b42c2800..e0c8e6d02 100644 --- a/install.ps1 +++ b/install.ps1 @@ -525,6 +525,14 @@ function Add-InstallDirToUserPath { $userPath = [Environment]::GetEnvironmentVariable("Path", "User") $userPathAlreadyConfigured = Test-PathInList $userPath $Path if (-not $userPathAlreadyConfigured) { + # Record the entry so `rocm uninstall` can take it out again. Only an + # entry this installer adds is recorded; one that was already on the + # user PATH stays the user's. Written before the change itself, so an + # installer stopped in between leaves a record of an entry that is not + # there (which uninstall reports and skips), never an unrecorded one. + $recordPath = Join-Path $Path ".rocm-cli-path-edit" + $utf8NoBom = New-Object System.Text.UTF8Encoding $false + [System.IO.File]::WriteAllText($recordPath, "user-path $Path`n", $utf8NoBom) $newUserPath = Add-PathListEntry $userPath $Path [Environment]::SetEnvironmentVariable("Path", $newUserPath, "User") } diff --git a/install.sh b/install.sh index 1bf9d3184..b02953986 100755 --- a/install.sh +++ b/install.sh @@ -294,6 +294,33 @@ EOF printf '%s\n' "updated:${profile}" } +# Whether profile $1 holds the installer's marked block for path expression $2, +# spelled the way append_path_snippet writes it, and closed by its end marker, +# as `rocm uninstall` requires before it removes one. A block for another +# folder, or a PATH line outside the markers, does not count. +profile_has_own_block() { + [ -f "$1" ] || return 1 + escaped="$(escape_for_double_quotes "$2")" + # Passed through the environment: awk -v would undo the escaping above. + ROCM_CLI_BLOCK_QUOTED="\"${escaped}\"" ROCM_CLI_BLOCK_LISTED="\"${escaped}:" awk ' + { sub(/[ \t\r]+$/, "") } + $0 == "# >>> rocm-cli path >>>" { inside = 1; hit = 0; next } + $0 == "# <<< rocm-cli path <<<" { if (inside && hit) found = 1; inside = 0; next } + inside && (index($0, ENVIRON["ROCM_CLI_BLOCK_QUOTED"]) || index($0, ENVIRON["ROCM_CLI_BLOCK_LISTED"])) { hit = 1 } + END { exit found ? 0 : 1 } + ' "$1" +} + +# Add one line to the record of the PATH change this installer made, unless it +# is already there. +record_path_edit() { + record="${INSTALL_DIR}/.rocm-cli-path-edit" + if [ -f "${record}" ] && grep -qxF "$1" "${record}"; then + return 0 + fi + printf '%s\n' "$1" >> "${record}" +} + ensure_installer_process_path() { case ":${PATH}:" in *:"${INSTALL_DIR}":*) @@ -511,6 +538,16 @@ case ":${PATH}:" in profile_path="$(profile_path_for_shell)" path_expr="$(path_expr_for_profile "${INSTALL_DIR}")" profile_result="$(append_path_snippet "${profile_path}" "$(shell_name)" "${path_expr}")" || true + # Record where the marked block lives, and how it spells this folder, so + # `rocm uninstall` can take it out again. Only a marked block for this + # folder is recorded: a PATH line the user wrote themselves, or a block + # another install wrote for a different folder, is not this install's. + # Lines are added, not replaced, so a block an earlier install wrote from + # another shell is still on record. + if profile_has_own_block "${profile_path}" "${path_expr}"; then + record_path_edit "profile ${profile_path}" + record_path_edit "path-expr ${path_expr}" + fi case "${profile_result}" in updated:*) echo "shell profile updated:" diff --git a/tests/e2e-cucumber/features/install_lifecycle.feature b/tests/e2e-cucumber/features/install_lifecycle.feature index 63f5a617b..589db146e 100644 --- a/tests/e2e-cucumber/features/install_lifecycle.feature +++ b/tests/e2e-cucumber/features/install_lifecycle.feature @@ -14,7 +14,9 @@ Feature: Release install lifecycle # Scenarios are independent: each packages its own bundle, generates its own # signing key, and installs into its own directory rooted in the scenario's # temp dir, so ordering never affects the outcome. Cargo's release build cache - # is shared naturally. + # is shared naturally. The exception is the Windows user PATH, one registry + # value for the whole machine: the scenarios that change it are @serial, so + # none captures it while another has it changed. # # CONSTRAINT: no @lifecycle scenario may depend on a scripted failure seam # (anything gated behind the `rocm/e2e-test-hooks` feature). ci.yml's Windows @@ -121,15 +123,18 @@ Feature: Release install lifecycle And the installed config still selects the vllm default engine @id:lifecycle-linux-uninstall-full-purge @lifecycle @requires-os:linux - Scenario: lifecycle-09 - Linux - uninstall removes binaries, manifest, and XDG state + Scenario: lifecycle-09 - Linux - uninstall removes binaries, manifest, XDG state, and the installer's PATH change Given a freshly built release tree And a generated signing keypair + And the shell profile already holds the user's own settings And a signed bundle installed with the shell profile updated And the installed binary has isolated XDG directories with state When the user uninstalls from the installed binary Then the installed rocm and rocmd binaries are gone And the install manifest is gone And the isolated XDG config, data, and cache state is gone + And the shell profile is back to the user's own settings + And the record of the installer's PATH change is gone # ── Windows user-PATH restoration, loopback HTTP install, isolated smoke ─ @@ -163,7 +168,7 @@ Feature: Release install lifecycle Then the install fails reporting signature verification failed And no binaries are activated in the target directory - @id:lifecycle-windows-updates-user-path @lifecycle @requires-os:windows + @id:lifecycle-windows-updates-user-path @lifecycle @requires-os:windows @serial Scenario: lifecycle-13 - Windows - a default install updates the user PATH and restores it afterwards Given a freshly built release tree And a generated signing keypair @@ -255,7 +260,7 @@ Feature: Release install lifecycle And the install manifest is present @id:lifecycle-windows-installed-binary-smoke @lifecycle @requires-os:windows - Scenario: lifecycle-22 - Windows - the installed binary honors isolated directories and keeps the running executable on uninstall + Scenario: lifecycle-22 - Windows - the installed binary honors isolated directories and removes itself on uninstall Given a freshly built release tree And a generated signing keypair And a signed bundle installed with the public key file @@ -269,6 +274,83 @@ Feature: Release install lifecycle When the installed rocm examine runs with the isolated environment Then examine reads only the isolated config, data, and cache directories When the user uninstalls from the installed binary keeping config data and cache - Then uninstall reports skipping the running executable on Windows + Then the installed rocm binary that ran uninstall is gone And the non-running installed rocmd binary is gone And the install manifest is gone + + # ── Uninstall: the installer's PATH change ────────────────────────────── + + @id:lifecycle-linux-uninstall-keeps-shared-path @lifecycle @requires-os:linux + Scenario: lifecycle-23 - Linux - uninstall keeps the shell PATH change while another program shares the install directory + Given a freshly built release tree + And a generated signing keypair + And a signed bundle installed with the shell profile updated + And the installed binary has isolated XDG directories with state + And another program is installed in the install directory + When the user uninstalls from the installed binary + Then the installed rocm and rocmd binaries are gone + And the other program is still in the install directory + And the shell profile still adds the install directory to PATH + And uninstall reports keeping the PATH change for the other program + And the record of the installer's PATH change is still in the install directory + + @id:lifecycle-windows-uninstall-restores-user-path @lifecycle @requires-os:windows @serial + Scenario: lifecycle-24 - Windows - uninstall takes the install directory back off the user PATH and keeps the rest of it + Given a freshly built release tree + And a generated signing keypair + And the user PATH is captured for restoration + And a signed bundle installed updating the user PATH + And the user PATH also holds an entry that uses an environment variable + And the installed binary has isolated directories with state + When the user uninstalls from the installed binary + Then the installed rocm and rocmd binaries are gone + And the install directory is not on the user PATH + And the user PATH still holds that entry and is still stored expandable + And the record of the installer's PATH change is gone + + @id:lifecycle-linux-uninstall-keep-binaries-keeps-path @lifecycle @requires-os:linux + Scenario: lifecycle-25 - Linux - uninstall that keeps the binaries keeps the shell PATH change + Given a freshly built release tree + And a generated signing keypair + And a signed bundle installed with the shell profile updated + And the installed binary has isolated XDG directories with state + When the user uninstalls from the installed binary keeping the binaries + Then the installed command exits successfully + And uninstall reports keeping the PATH change with the binaries + And the installed rocm and rocmd binaries are present + And the shell profile still adds the install directory to PATH + And the record of the installer's PATH change is still in the install directory + + @id:lifecycle-linux-uninstall-path-failure-keeps-binaries @lifecycle @requires-os:linux + Scenario: lifecycle-26 - Linux - uninstall that cannot undo the PATH change removes nothing, so it can be retried + Given a freshly built release tree + And a generated signing keypair + And the shell profile already holds the user's own settings + And a signed bundle installed with the shell profile updated + And the installed binary has isolated XDG directories with state + And the shell profile cannot be rewritten + When the user uninstalls from the installed binary + Then uninstall fails saying the PATH change could not be undone + And the isolated XDG config, data, and cache state is still present + And the installed rocm and rocmd binaries are present + And the shell profile still adds the install directory to PATH + And the record of the installer's PATH change is still in the install directory + When the shell profile can be rewritten again + And the user uninstalls from the installed binary + Then the installed rocm and rocmd binaries are gone + And the shell profile is back to the user's own settings + And the record of the installer's PATH change is gone + + @id:lifecycle-linux-uninstall-follows-the-installer-record @lifecycle @requires-os:linux + Scenario: lifecycle-27 - Linux - uninstall undoes the PATH change in a profile it finds only through the installer's record + Given a freshly built release tree + And a generated signing keypair + And the install directory is reached through a link + And the installer writes its PATH lines to a shell profile the user chose + And the shell profile already holds the user's own settings + And a signed bundle installed with the shell profile updated + And the installed binary has isolated XDG directories with state + When the user uninstalls from the installed binary + Then the installed rocm and rocmd binaries are gone + And the shell profile is back to the user's own settings + And the record of the installer's PATH change is gone diff --git a/tests/e2e-cucumber/tests/e2e/lifecycle_steps.rs b/tests/e2e-cucumber/tests/e2e/lifecycle_steps.rs index cc970d690..f20ab63c5 100644 --- a/tests/e2e-cucumber/tests/e2e/lifecycle_steps.rs +++ b/tests/e2e-cucumber/tests/e2e/lifecycle_steps.rs @@ -49,11 +49,34 @@ pub struct LifecycleState { /// A generated installer fixture with controlled pinned trust roots. installer_fixture: Option, /// A captured Windows user PATH to restore on drop (Windows only). - captured_user_path: Option, + captured_user_path: Option, /// Isolated config/data/cache directories for installed-binary smoke tests. smoke_config: Option, smoke_data: Option, smoke_cache: Option, + /// The shell profile contents seeded before install, which uninstall must + /// restore exactly. + profile_before_install: Option, + /// A shell profile the installer is pointed at through + /// `ROCM_CLI_SHELL_PROFILE`; `None` means the default `~/.bashrc`. + custom_profile: Option, +} + +/// The Windows user `Path` exactly as the registry holds it: its value with +/// `%VARIABLES%` unexpanded, and its registry kind (`String` for `REG_SZ`, +/// `ExpandString` for `REG_EXPAND_SZ`), or `None` when there is no value. +#[derive(Debug, Clone)] +#[cfg_attr(not(windows), allow(dead_code))] // Only Windows has a user PATH to capture. +pub struct CapturedUserPath { + kind: Option, + value: String, +} + +/// The shell profile the installer writes its PATH lines to in this scenario. +fn shell_profile(st: &LifecycleState) -> PathBuf { + st.custom_profile + .clone() + .unwrap_or_else(|| st.home_dir.join(".bashrc")) } impl Drop for LifecycleState { @@ -266,7 +289,8 @@ fn which_ok(name: &str) -> bool { .is_ok_and(|s| s.success()) } -/// Capture the current Windows user PATH (registry `HKCU\Environment\Path`). +/// The current Windows user PATH (registry `HKCU\Environment\Path`), with +/// any `%VARIABLES%` expanded, for checking which folders it names. #[cfg(windows)] fn capture_user_path() -> String { let output = Command::new("powershell") @@ -282,17 +306,58 @@ fn capture_user_path() -> String { .unwrap_or_default() } -/// Restore the Windows user PATH to a previously captured value. +/// The Windows user PATH as the registry stores it, so it can be put back +/// exactly: `[Environment]` would expand its `%VARIABLES%` on the way out and +/// write it back as `REG_SZ`. +#[cfg(windows)] +fn capture_raw_user_path() -> CapturedUserPath { + // UTF-8 out, so a folder name outside the console code page survives the + // round trip back into the registry. + let script = "[Console]::OutputEncoding = [System.Text.Encoding]::UTF8; \ + $key = [Microsoft.Win32.Registry]::CurrentUser.OpenSubKey('Environment'); \ + if ($null -ne $key -and $key.GetValueNames() -contains 'Path') { \ + $key.GetValueKind('Path').ToString(); \ + $key.GetValue('Path', '', [Microsoft.Win32.RegistryValueOptions]::DoNotExpandEnvironmentNames) \ + } else { 'Missing' }"; + let output = Command::new("powershell") + .args(["-NoProfile", "-Command", script]) + .output() + .expect("failed to run PowerShell to read the user PATH"); + assert!( + output.status.success(), + "failed to read the user PATH: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8_lossy(&output.stdout); + let (kind, value) = stdout.split_once('\n').unwrap_or((&stdout, "")); + let kind = kind.trim(); + CapturedUserPath { + kind: (kind != "Missing").then(|| kind.to_owned()), + value: value.trim_end_matches(['\r', '\n']).to_owned(), + } +} + +/// Restore the Windows user PATH to a previously captured value and kind. /// /// Runs during `Drop`, so it must not panic; a failed restore is loud on stderr /// instead, because a botched restore leaves the runner's real user PATH /// polluted with the test's temp install dir and that must be diagnosable. #[cfg(windows)] -fn restore_user_path(value: &str) { - let script = "[Environment]::SetEnvironmentVariable('Path', $env:ROCM_E2E_PREV_PATH, 'User')"; +fn restore_user_path(previous: &CapturedUserPath) { + let script = "$key = [Microsoft.Win32.Registry]::CurrentUser.OpenSubKey('Environment', $true); \ + if ($env:ROCM_E2E_PREV_KIND -eq 'Missing') { $key.DeleteValue('Path', $false) } \ + else { \ + $value = if ($null -eq $env:ROCM_E2E_PREV_PATH) { '' } else { $env:ROCM_E2E_PREV_PATH }; \ + $key.SetValue('Path', $value, [Microsoft.Win32.RegistryValueKind]$env:ROCM_E2E_PREV_KIND) \ + }; \ + $key.Close()"; let status = Command::new("powershell") .args(["-NoProfile", "-Command", script]) - .env("ROCM_E2E_PREV_PATH", value) + .env("ROCM_E2E_PREV_PATH", &previous.value) + .env( + "ROCM_E2E_PREV_KIND", + previous.kind.as_deref().unwrap_or("Missing"), + ) .status(); match status { Ok(status) if status.success() => {} @@ -342,6 +407,16 @@ fn assert_missing(path: &Path) { ); } +/// Assert a path is gone within a few seconds. On Windows the running `rocm.exe` +/// may only be deleted as the uninstalling process exits. +async fn assert_missing_soon(path: &Path) { + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(15); + while path.exists() && std::time::Instant::now() < deadline { + tokio::time::sleep(std::time::Duration::from_millis(200)).await; + } + assert_missing(path); +} + // ── Given: setup ─────────────────────────────────────────────────────── #[given("a freshly built release tree")] @@ -384,6 +459,8 @@ async fn given_release_tree(world: &mut E2eWorld) { smoke_config: None, smoke_data: None, smoke_cache: None, + profile_before_install: None, + custom_profile: None, }); } @@ -429,11 +506,64 @@ async fn given_isolated_dirs(world: &mut E2eWorld) { seed_isolated_dirs(world); } +#[given("the shell profile already holds the user's own settings")] +async fn given_profile_user_settings(world: &mut E2eWorld) { + let profile = shell_profile(state(world)); + if let Some(dir) = profile.parent() { + std::fs::create_dir_all(dir).expect("failed to create the shell profile's folder"); + } + let contents = "# the user's own settings\nexport ROCM_E2E_USER_SETTING=1\n".to_owned(); + std::fs::write(&profile, &contents).expect("failed to seed the shell profile"); + state_mut(world).profile_before_install = Some(contents); +} + +#[given("a signed bundle installed updating the user PATH")] +async fn given_installed_updating_user_path(world: &mut E2eWorld) { + package_with_key_file(world).await; + when_install_update_user_path(world).await; + then_user_path_updated(world).await; +} + +#[given("the installer writes its PATH lines to a shell profile the user chose")] +async fn given_custom_profile(world: &mut E2eWorld) { + // Outside every profile `install.sh` picks by shell, so only the installer's + // record can tell uninstall where the lines went. + let st = state_mut(world); + st.custom_profile = Some(st.home_dir.join("dotfiles").join("rocm-path.sh")); +} + +#[given("the install directory is reached through a link")] +async fn given_linked_install_dir(world: &mut E2eWorld) { + #[cfg(unix)] + { + // The installer spells the folder through the link in the profile, but + // the installed binary sees its folder resolved, so only the + // installer's record of that spelling lets uninstall match the block. + let root = root(world); + let target = root.join("install-target"); + std::fs::create_dir_all(&target).expect("failed to create the install target"); + let link = root.join("install-link"); + std::os::unix::fs::symlink(&target, &link).expect("failed to link the install folder"); + state_mut(world).install_dir = link.join("bin"); + } + #[cfg(not(unix))] + { + let _ = world; + } +} + +#[given("another program is installed in the install directory")] +async fn given_other_program(world: &mut E2eWorld) { + let install_dir = state(world).install_dir.clone(); + std::fs::write(install_dir.join("other-tool"), "#!/bin/sh\n") + .expect("failed to seed another program"); +} + #[given("the user PATH is captured for restoration")] async fn given_capture_user_path(world: &mut E2eWorld) { #[cfg(windows)] { - state_mut(world).captured_user_path = Some(capture_user_path()); + state_mut(world).captured_user_path = Some(capture_raw_user_path()); } #[cfg(not(windows))] { @@ -450,6 +580,8 @@ fn seed_isolated_dirs(world: &mut E2eWorld) { let cache = root.join("smoke-cache"); for dir in [&config, &data, &cache] { std::fs::create_dir_all(dir).expect("failed to create isolated smoke dir"); + std::fs::write(dir.join("state-marker"), "seeded by the scenario\n") + .expect("failed to seed isolated smoke state"); } let st = state_mut(world); st.smoke_config = Some(config); @@ -573,7 +705,13 @@ async fn install_with_key_file_shell_profile(world: &mut E2eWorld) { "ROCM_CLI_SIGNING_PUBLIC_KEY_PATH", st.public_key.to_string_lossy().into_owned(), )]; - let shell_on = [("ROCM_CLI_UPDATE_SHELL_PATH", "1".to_string())]; + let mut shell_on = vec![("ROCM_CLI_UPDATE_SHELL_PATH", "1".to_string())]; + if let Some(profile) = &st.custom_profile { + shell_on.push(( + "ROCM_CLI_SHELL_PROFILE", + profile.to_string_lossy().into_owned(), + )); + } let (out, _ok) = run_installer(world, &install_dir, &download_base, &key_env, &shell_on); state_mut(world).last_output = out; } @@ -904,32 +1042,31 @@ async fn when_examine_isolated(world: &mut E2eWorld) { #[when("the user uninstalls from the installed binary")] async fn when_uninstall(world: &mut E2eWorld) { - let st = state(world); - let rocm = installed_binary(&st.install_dir, "rocm"); - let mut cmd = Command::new(&rocm); - cmd.arg("uninstall").arg("--yes"); - apply_smoke_env(&mut cmd, st); - let output = cmd.output().expect("failed to run uninstall"); - let combined = format!( - "{}{}", - String::from_utf8_lossy(&output.stdout), - String::from_utf8_lossy(&output.stderr) - ); - state_mut(world).last_output = combined; + run_uninstall(world, &[]); } #[when("the user uninstalls from the installed binary keeping config data and cache")] async fn when_uninstall_keep(world: &mut E2eWorld) { + run_uninstall(world, &["--keep-config", "--keep-data", "--keep-cache"]); +} + +#[when("the user uninstalls from the installed binary keeping the binaries")] +async fn when_uninstall_keep_binaries(world: &mut E2eWorld) { + run_uninstall(world, &["--keep-binaries"]); +} + +/// Run `rocm uninstall --yes` from the installed binary with `extra_args`, +/// capturing its output and exit code. +fn run_uninstall(world: &mut E2eWorld, extra_args: &[&str]) { let st = state(world); let rocm = installed_binary(&st.install_dir, "rocm"); let mut cmd = Command::new(&rocm); - cmd.args([ - "uninstall", - "--yes", - "--keep-config", - "--keep-data", - "--keep-cache", - ]); + cmd.args(["uninstall", "--yes"]).args(extra_args); + // Uninstall undoes the installer's shell-profile change, so it must see the + // scenario's HOME, never the runner's real one. It must find a profile the + // installer was pointed at from the installer's record alone. + cmd.env("HOME", &st.home_dir) + .env_remove("ROCM_CLI_SHELL_PROFILE"); apply_smoke_env(&mut cmd, st); let output = cmd.output().expect("failed to run uninstall"); let combined = format!( @@ -937,7 +1074,9 @@ async fn when_uninstall_keep(world: &mut E2eWorld) { String::from_utf8_lossy(&output.stdout), String::from_utf8_lossy(&output.stderr) ); - state_mut(world).last_output = combined; + let st = state_mut(world); + st.last_output = combined; + st.last_rc = output.status.code().unwrap_or(-1); } /// Apply the isolated config/data/cache env for installed-binary invocations. @@ -994,7 +1133,7 @@ async fn then_shell_profile_updated(world: &mut E2eWorld) { #[then("the shell profile lists the install directory")] async fn then_shell_profile_lists_dir(world: &mut E2eWorld) { let st = state(world); - let profile = st.home_dir.join(".bashrc"); + let profile = shell_profile(st); let contents = std::fs::read_to_string(&profile).unwrap_or_default(); let install_dir = st.install_dir.to_string_lossy(); assert!( @@ -1150,12 +1289,20 @@ async fn then_config_preserved(world: &mut E2eWorld) { #[then("the shell profile has exactly one rocm-cli PATH marker")] async fn then_single_path_marker(world: &mut E2eWorld) { - let profile = state(world).home_dir.join(".bashrc"); + assert_single_path_marker(state(world)); +} + +/// The scenario's shell profile holds the installer's marked PATH block exactly +/// once. +fn assert_single_path_marker(st: &LifecycleState) { + let profile = shell_profile(st); let contents = std::fs::read_to_string(&profile).unwrap_or_default(); let count = contents.matches("# >>> rocm-cli path >>>").count(); assert_eq!( - count, 1, - "expected exactly one rocm-cli PATH marker, found {count}:\n{contents}" + count, + 1, + "expected exactly one rocm-cli PATH marker in {}, found {count}:\n{contents}", + profile.display() ); } @@ -1186,22 +1333,224 @@ async fn then_examine_isolated(world: &mut E2eWorld) { } } -#[then("uninstall reports skipping the running executable on Windows")] -async fn then_skip_running_exe(world: &mut E2eWorld) { - let out = &state(world).last_output; +#[then("the installed rocm binary that ran uninstall is gone")] +async fn then_running_rocm_gone(world: &mut E2eWorld) { + let st = state(world); + let out = &st.last_output; + let rocm = installed_binary(&st.install_dir, "rocm"); + // Matched by file name: the program reports its own path, which on Windows + // may be spelled differently (long vs 8.3 short) from the scenario's. + let own_name = format!( + "{}rocm{}", + std::path::MAIN_SEPARATOR, + std::env::consts::EXE_SUFFIX + ); + let reported = out.lines().any(|line| { + line.starts_with("removed binary ") + && (line.ends_with(&own_name) + || line.ends_with(&format!("{own_name} as this command exits"))) + }); assert!( - out.contains("skipping running executable on Windows"), - "uninstall did not report the running-executable skip:\n{out}" + reported && out.contains("uninstall complete"), + "uninstall did not report removing its own binary:\n{out}" ); + assert_missing_soon(&rocm).await; } #[then("the installed rocm and rocmd binaries are gone")] async fn then_binaries_gone(world: &mut E2eWorld) { let st = state(world); - assert_missing(&installed_binary(&st.install_dir, "rocm")); + assert_missing_soon(&installed_binary(&st.install_dir, "rocm")).await; assert_missing(&installed_binary(&st.install_dir, "rocmd")); } +#[then("the shell profile is back to the user's own settings")] +async fn then_profile_restored(world: &mut E2eWorld) { + let st = state(world); + let before = st + .profile_before_install + .as_deref() + .expect("the shell profile was not seeded"); + let profile = shell_profile(st); + let contents = std::fs::read_to_string(&profile).unwrap_or_default(); + assert_eq!( + contents, before, + "uninstall did not restore the shell profile to its pre-install contents" + ); + let out = &st.last_output; + assert!( + out.contains("removed the rocm-cli PATH lines from"), + "uninstall did not report removing the PATH lines:\n{out}" + ); +} + +#[then("the shell profile still adds the install directory to PATH")] +async fn then_profile_kept(world: &mut E2eWorld) { + assert_single_path_marker(state(world)); +} + +#[then("the record of the installer's PATH change is still in the install directory")] +async fn then_path_record_kept(world: &mut E2eWorld) { + // The kept change stays undoable by a later uninstall only while its + // record survives. + assert_exists(&state(world).install_dir.join(".rocm-cli-path-edit")); +} + +#[then("the record of the installer's PATH change is gone")] +async fn then_path_record_gone(world: &mut E2eWorld) { + assert_missing(&state(world).install_dir.join(".rocm-cli-path-edit")); +} + +#[then("the isolated XDG config, data, and cache state is still present")] +async fn then_xdg_kept(world: &mut E2eWorld) { + let st = state(world); + for dir in [&st.smoke_config, &st.smoke_data, &st.smoke_cache] + .into_iter() + .flatten() + { + assert_exists(&dir.join("state-marker")); + } +} + +#[then("uninstall reports keeping the PATH change with the binaries")] +async fn then_reports_path_kept_with_binaries(world: &mut E2eWorld) { + let out = &state(world).last_output; + assert!( + out.contains("binary removal disabled by --keep-binaries") + && out.contains("any PATH change the installer made stays too"), + "uninstall did not say the PATH change stays with the binaries:\n{out}" + ); +} + +/// An entry that names an environment variable no one sets, so it reads back +/// the same expanded or not. +#[cfg(windows)] +const EXPANDABLE_PATH_ENTRY: &str = r"%ROCM_E2E_UNSET_VARIABLE%\rocm-e2e-probe"; + +#[given("the user PATH also holds an entry that uses an environment variable")] +async fn given_expandable_user_path_entry(world: &mut E2eWorld) { + #[cfg(windows)] + { + // Stored as REG_EXPAND_SZ, the way Windows itself stores a Path that + // uses variables. Added after the install, since `install.ps1` writes + // the Path back through `[Environment]`, which stores REG_SZ. + assert!( + state(world).captured_user_path.is_some(), + "capture the user PATH for restoration first" + ); + let script = "$key = [Microsoft.Win32.Registry]::CurrentUser.OpenSubKey('Environment', $true); \ + $raw = $key.GetValue('Path', '', [Microsoft.Win32.RegistryValueOptions]::DoNotExpandEnvironmentNames); \ + $value = if ([string]::IsNullOrEmpty($raw)) { $env:ROCM_E2E_PATH_ENTRY } else { $raw + ';' + $env:ROCM_E2E_PATH_ENTRY }; \ + $key.SetValue('Path', $value, [Microsoft.Win32.RegistryValueKind]::ExpandString); \ + $key.Close()"; + let status = Command::new("powershell") + .args(["-NoProfile", "-Command", script]) + .env("ROCM_E2E_PATH_ENTRY", EXPANDABLE_PATH_ENTRY) + .status() + .expect("failed to run PowerShell to edit the user PATH"); + assert!( + status.success(), + "failed to add the expandable user PATH entry" + ); + } + #[cfg(not(windows))] + { + let _ = world; + } +} + +#[then("the user PATH still holds that entry and is still stored expandable")] +async fn then_expandable_user_path_entry_kept(world: &mut E2eWorld) { + #[cfg(windows)] + { + let _ = world; + let current = capture_raw_user_path(); + assert_eq!( + current.kind.as_deref(), + Some("ExpandString"), + "uninstall changed the user PATH's registry type: {current:?}" + ); + assert!( + current + .value + .split(';') + .any(|entry| entry == EXPANDABLE_PATH_ENTRY), + "uninstall dropped or expanded the user's other PATH entry: {current:?}" + ); + } + #[cfg(not(windows))] + { + let _ = world; + } +} + +#[then("the other program is still in the install directory")] +async fn then_other_program_kept(world: &mut E2eWorld) { + assert_exists(&state(world).install_dir.join("other-tool")); +} + +#[then("uninstall reports keeping the PATH change for the other program")] +async fn then_reports_path_kept(world: &mut E2eWorld) { + let out = &state(world).last_output; + assert!( + out.contains("PATH change left in place") && out.contains("other-tool"), + "uninstall did not explain keeping the PATH change:\n{out}" + ); +} + +#[then("the install directory is not on the user PATH")] +async fn then_install_dir_off_user_path(world: &mut E2eWorld) { + #[cfg(windows)] + { + let st = state(world); + let current = capture_user_path(); + let target = st.install_dir.to_string_lossy().to_lowercase(); + let on_path = current.split(';').any(|entry| { + entry.trim().to_lowercase().trim_end_matches('\\') == target.trim_end_matches('\\') + }); + assert!(!on_path, "install dir is still on the user PATH: {current}"); + let out = &st.last_output; + assert!( + out.contains("from your user PATH"), + "uninstall did not report removing the user PATH entry:\n{out}" + ); + } + #[cfg(not(windows))] + { + let _ = world; + } +} + +#[given("the shell profile cannot be rewritten")] +async fn given_profile_unwritable(world: &mut E2eWorld) { + // Uninstall writes the edited profile beside the original before swapping + // it in; a folder in that spot makes the write fail, even for root. + let home = state(world).home_dir.clone(); + std::fs::create_dir_all(home.join(".bashrc.rocm-uninstall.tmp")) + .expect("failed to block the profile rewrite"); +} + +#[when("the shell profile can be rewritten again")] +async fn when_profile_writable(world: &mut E2eWorld) { + let home = state(world).home_dir.clone(); + std::fs::remove_dir(home.join(".bashrc.rocm-uninstall.tmp")) + .expect("failed to unblock the profile rewrite"); +} + +#[then("uninstall fails saying the PATH change could not be undone")] +async fn then_uninstall_fails_path(world: &mut E2eWorld) { + let st = state(world); + let out = &st.last_output; + assert_ne!(st.last_rc, 0, "uninstall succeeded:\n{out}"); + // The scenario's next steps check that nothing else was removed. + assert!( + out.contains("could not undo the installer's PATH change") + && out.contains("Nothing else was removed") + && out.contains("`rocm uninstall` again"), + "uninstall did not explain the PATH failure:\n{out}" + ); +} + #[then("the install manifest is gone")] async fn then_manifest_gone(world: &mut E2eWorld) { let st = state(world); @@ -1210,8 +1559,8 @@ async fn then_manifest_gone(world: &mut E2eWorld) { #[then("the non-running installed rocmd binary is gone")] async fn then_rocmd_gone(world: &mut E2eWorld) { - // The keep-config uninstall skips the running rocm executable on Windows but - // must still remove the non-running rocmd binary (old acceptance-script check). + // The keep-config uninstall still removes the binaries, rocmd included + // (old acceptance-script check). let st = state(world); assert_missing(&installed_binary(&st.install_dir, "rocmd")); } @@ -1223,9 +1572,8 @@ async fn then_xdg_gone(world: &mut E2eWorld) { .into_iter() .flatten() { - // Uninstall purges the CLI's own state; the top-level isolated dir may - // remain but its rocm-cli subtree must be gone. - let rocm_cli = dir.join("rocm-cli"); - assert_missing(&rocm_cli); + // The isolated dirs are the CLI's config, data, and cache directories + // themselves (`ROCM_CLI_*_DIR`), so uninstall removes each one whole. + assert_missing(dir); } }