diff --git a/CLAUDE.md b/CLAUDE.md index e89e138e..b495036c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -18,6 +18,16 @@ When writing or running any Rust test, follow the functions and hand-rolled loops, `proptest` for domain-wide properties. Run with plain `cargo test -p ` (no nextest/just wrapper). +## Documentation + +When writing or editing any docblock, doc comment, or module header (`///`, +`//!`) — including on code you just changed — follow the +[`writing-interface-docblocks`](.claude/skills/writing-interface-docblocks/SKILL.md) +skill: a docblock is a contract, not a changelog. Terse, present-tense, no +prompt or diff leakage (`rather than`, `now returns`, `as requested`); document +the *when* of errors/panics, not the *why*; module docs name the domain, not the +one item currently inside them. + ## DSL The `harmont` Python package (pipeline DSL) lives inside `crates/hm-dsl-engine/harmont-py/` so it ships with the crate. diff --git a/Cargo.lock b/Cargo.lock index 4f8972bb..70d816c6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1132,7 +1132,6 @@ dependencies = [ "hm-plugin-cloud", "hm-plugin-protocol", "hm-render", - "hm-util", "hm-vm", "human-units", "indicatif", @@ -1259,6 +1258,7 @@ dependencies = [ "bstr", "chrono", "derive_more", + "dirs", "include_dir", "num-traits", "proptest", @@ -1272,6 +1272,7 @@ dependencies = [ "unicode-segmentation", "unicode-width", "which", + "windows", ] [[package]] @@ -1281,7 +1282,7 @@ dependencies = [ "anyhow", "derive_more", "figment", - "hm-util", + "hm-common", "rstest", "serde", "tempfile", @@ -1321,7 +1322,6 @@ dependencies = [ "hm-common", "hm-pipeline-ir", "hm-plugin-protocol", - "hm-util", "hm-vm", "human-units", "ignore", @@ -1407,19 +1407,6 @@ dependencies = [ "uuid", ] -[[package]] -name = "hm-util" -version = "0.0.0-dev" -dependencies = [ - "anyhow", - "dirs", - "rstest", - "tempfile", - "tokio", - "tracing", - "windows", -] - [[package]] name = "hm-vm" version = "0.0.0-dev" diff --git a/Cargo.toml b/Cargo.toml index 307070a5..c57a8a2a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,7 +6,6 @@ members = [ "crates/hm-config", "crates/hm-plugin-protocol", "crates/hm-pipeline-ir", - "crates/hm-util", "crates/hm-plugin-cloud", "crates/hm-dsl-engine", "crates/hm-render", @@ -19,7 +18,6 @@ default-members = [ "crates/hm-config", "crates/hm-plugin-protocol", "crates/hm-pipeline-ir", - "crates/hm-util", "crates/hm-plugin-cloud", "crates/hm-dsl-engine", "crates/hm-render", @@ -37,7 +35,6 @@ hm-exec = { path = "crates/hm-exec", version = "0.0.0-dev" hm-plugin-protocol = { path = "crates/hm-plugin-protocol", version = "0.0.0-dev" } hm-plugin-cloud = { path = "crates/hm-plugin-cloud", version = "0.0.0-dev" } hm-pipeline-ir = { path = "crates/hm-pipeline-ir", version = "0.0.0-dev" } -hm-util = { path = "crates/hm-util", version = "0.0.0-dev" } hm-config = { path = "crates/hm-config", version = "0.0.0-dev" } hm-dsl-engine = { path = "crates/hm-dsl-engine", version = "0.0.0-dev" } hm-render = { path = "crates/hm-render", version = "0.0.0-dev" } diff --git a/crates/hm-common/Cargo.toml b/crates/hm-common/Cargo.toml index 618b1f57..e1900042 100644 --- a/crates/hm-common/Cargo.toml +++ b/crates/hm-common/Cargo.toml @@ -12,13 +12,14 @@ categories = ["command-line-utilities"] path = "src/lib.rs" [features] -app-runtime = [] +sys-runtime = [] [dependencies] anyhow = { workspace = true } async-trait = { workspace = true } chrono = { workspace = true } derive_more = { workspace = true } +dirs = "6" serde = { workspace = true } serde_json = { workspace = true } thiserror = { workspace = true } @@ -26,12 +27,19 @@ bstr = "1" include_dir = "0.7" num-traits = { workspace = true } tempfile = "3" -tokio = { version = "1", features = ["process", "fs"] } +tokio = { version = "1", features = ["process", "fs", "rt", "rt-multi-thread", "io-util"] } tracing = "0.1" unicode-width = { workspace = true } unicode-segmentation = { workspace = true } which = "7" +[target.'cfg(windows)'.dependencies.windows] +version = "0.62" +features = [ + "Win32_Foundation", + "Win32_Storage_FileSystem", +] + [dev-dependencies] tempfile = "3" tokio = { version = "1", features = ["macros", "rt-multi-thread"] } diff --git a/crates/hm-common/src/dir_provider.rs b/crates/hm-common/src/dir_provider.rs new file mode 100644 index 00000000..477bf113 --- /dev/null +++ b/crates/hm-common/src/dir_provider.rs @@ -0,0 +1,76 @@ +//! Platform user directories. +//! +//! Exposes the operating system's per-user directory roots. Application- +//! agnostic: it knows nothing of Harmont's own `hm/` subdirectory — callers +//! join that (and any file name) onto these roots. +//! +//! On non-Windows the roots are `~/.config` and `~/.cache`; the `$XDG_*` env +//! vars are intentionally not honored, keeping paths predictable. + +use std::path::{Path, PathBuf}; + +/// The platform per-user directory roots, resolved once at construction and +/// read as borrowed paths. +/// +/// Build with [`DirProvider::new`], then read the `&Path` accessors. Attach a +/// process-wide instance to the system runtime and reach it via its `dirs()` +/// accessor instead of reconstructing one per call. +#[derive(Debug, Clone)] +pub struct DirProvider { + config: PathBuf, + cache: PathBuf, +} + +impl DirProvider { + /// Resolve the platform directory roots, once. + /// + /// Returns `None` if a root cannot be determined — e.g. there is no home + /// directory. + #[must_use] + pub fn new() -> Option { + #[cfg(windows)] + let (config, cache) = (dirs::config_dir()?, dirs::cache_dir()?); + #[cfg(not(windows))] + let (config, cache) = { + let home = dirs::home_dir()?; + (home.join(".config"), home.join(".cache")) + }; + + Some(Self { config, cache }) + } + + /// The user configuration root (`~/.config` on non-Windows). + #[must_use] + pub fn config(&self) -> &Path { + &self.config + } + + /// The user cache root (`~/.cache` on non-Windows). + #[must_use] + pub fn cache(&self) -> &Path { + &self.cache + } +} + +#[cfg(test)] +#[allow(clippy::unwrap_used, reason = "test setup and assertions")] +mod tests { + use super::*; + use rstest::rstest; + + #[rstest] + fn config_is_the_platform_config_root() { + let dirs = DirProvider::new().unwrap(); + assert!(dirs.config().is_absolute(), "got {:?}", dirs.config()); + #[cfg(not(windows))] + assert!(dirs.config().ends_with(".config"), "got {:?}", dirs.config()); + } + + #[rstest] + fn cache_is_the_platform_cache_root() { + let dirs = DirProvider::new().unwrap(); + assert!(dirs.cache().is_absolute(), "got {:?}", dirs.cache()); + #[cfg(not(windows))] + assert!(dirs.cache().ends_with(".cache"), "got {:?}", dirs.cache()); + } +} diff --git a/crates/hm-common/src/fs.rs b/crates/hm-common/src/fs.rs index 319bac16..bcc6a643 100644 --- a/crates/hm-common/src/fs.rs +++ b/crates/hm-common/src/fs.rs @@ -3,21 +3,214 @@ use std::io; use std::path::Path; +/// Who may read a file (or directory) that hm writes. +/// +/// On Windows this is not enforced nor respected. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Privacy { + /// Owner-only. Unix file `0o600`, directory `0o700`. Use for secrets (e.g. credentials). + Private, + /// World-readable, owner-writable. Unix file `0o644`, directory `0o755`. + Public, +} + +impl Privacy { + /// Unix permission bits for a *file* at this privacy level. + #[cfg(unix)] + const fn file_mode(self) -> u32 { + match self { + Self::Private => 0o600, + Self::Public => 0o644, + } + } + + /// Unix permission bits for a *directory* at this privacy level. + #[cfg(unix)] + const fn dir_mode(self) -> u32 { + match self { + Self::Private => 0o700, + Self::Public => 0o755, + } + } +} + /// Write `contents` to `path`, creating any missing parent directories first. /// -/// Joins [`std::fs::create_dir_all`] on the parent and [`std::fs::write`] into -/// one call, so callers scaffolding a file into a not-yet-existing directory -/// don't repeat the parent-creation dance. An existing file is overwritten. +/// This makes no permission guarantees — reach for [`write_atomic`] when the result must be +/// owner-only or written atomically. /// /// # Errors -/// Returns the underlying [`io::Error`] if a parent directory cannot be created -/// or the file cannot be written. -pub fn write_create_all(path: impl AsRef, contents: impl AsRef<[u8]>) -> io::Result<()> { +/// Returns the underlying [`io::Error`] if a parent directory cannot be created or the file cannot +/// be written. +pub async fn write_create_all( + path: impl AsRef, + contents: impl AsRef<[u8]>, +) -> io::Result<()> { let path = path.as_ref(); if let Some(parent) = path.parent().filter(|p| !p.as_os_str().is_empty()) { - std::fs::create_dir_all(parent)?; + tokio::fs::create_dir_all(parent).await?; + } + tokio::fs::write(path, contents).await +} + +/// Atomically write `contents` to `path` at the given [`Privacy`], ensuring the parent directory +/// exists at that same privacy. +/// +/// The parent directory is created (and, if it already exists, chmod'd) to match `file`: `0o700` +/// for [`Privacy::Private`], `0o755` for [`Privacy::Public`] on Unix. +pub async fn write_atomic( + path: impl AsRef, + contents: impl AsRef<[u8]>, + file: Privacy, +) -> io::Result<()> { + let path = path.as_ref().to_owned(); + let contents = contents.as_ref().to_vec(); + + let parent = path + .parent() + .ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + format!("{} has no parent directory", path.display()), + ) + })? + .to_owned(); + + create_dir_at(&parent, file).await?; + + let file_name = path + .file_name() + .ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + format!("{} has no file name", path.display()), + ) + })? + .to_os_string(); + let mut tmp_name = file_name; + tmp_name.push(format!(".tmp.{}", std::process::id())); + let tmp_path = parent.join(&tmp_name); + + write_file_private(&tmp_path, &contents, file).await?; + + let rename_result = atomic_rename_over(&tmp_path, &path).await; + if rename_result.is_err() { + let _ = tokio::fs::remove_file(&tmp_path).await; + } + rename_result +} + +/// Atomically replace `to` with `from`. +pub async fn atomic_rename_over(from: impl AsRef, to: impl AsRef) -> io::Result<()> { + #[cfg(unix)] + { + tokio::fs::rename(from.as_ref(), to.as_ref()).await + } + #[cfg(windows)] + { + fn atomic_rename_over_impl(from: &Path, to: &Path) -> io::Result<()> { + use windows::Win32::Storage::FileSystem::{ + MOVEFILE_REPLACE_EXISTING, MOVEFILE_WRITE_THROUGH, MoveFileExW, + REPLACEFILE_IGNORE_MERGE_ERRORS, ReplaceFileW, + }; + use windows::core::HSTRING; + + let from_w = HSTRING::from(from.as_os_str()); + let to_w = HSTRING::from(to.as_os_str()); + + if to.exists() { + let result = unsafe { + ReplaceFileW( + &to_w, + &from_w, + windows::core::PCWSTR::null(), + REPLACEFILE_IGNORE_MERGE_ERRORS, + None, + None, + ) + }; + return result.map_err(|e| io::Error::new(io::ErrorKind::Other, e)); + } + + let result = unsafe { + MoveFileExW( + &from_w, + &to_w, + MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH, + ) + }; + result.map_err(|e| io::Error::new(io::ErrorKind::Other, e)) + } + + let from = from.as_ref().to_owned(); + let to = to.as_ref().to_owned(); + tokio::task::spawn_blocking(move || atomic_rename_over_impl(&from, &to)) + .await + .map_err(io::Error::other)? + } +} + +/// Remove a file if it exists; silently return `Ok(())` if it does not. +pub async fn remove_file_if_exists(path: impl AsRef) -> io::Result<()> { + match tokio::fs::remove_file(path).await { + Ok(()) => Ok(()), + Err(e) if e.kind() == io::ErrorKind::NotFound => Ok(()), + Err(e) => Err(e), + } +} + +/// Create `dir` (and any missing parents) at `privacy`'s directory mode. +async fn create_dir_at(dir: &Path, privacy: Privacy) -> io::Result<()> { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mode = privacy.dir_mode(); + match tokio::fs::metadata(dir).await { + Ok(meta) => { + let current = meta.permissions().mode() & 0o777; + if current != mode { + tokio::fs::set_permissions(dir, std::fs::Permissions::from_mode(mode)).await?; + } + } + Err(e) if e.kind() == io::ErrorKind::NotFound => { + let mut builder = tokio::fs::DirBuilder::new(); + builder.recursive(true).mode(mode); + builder.create(dir).await?; + } + Err(e) => return Err(e), + } + Ok(()) + } + + #[cfg(windows)] + { + let _ = privacy; + tokio::fs::create_dir_all(dir).await } - std::fs::write(path, contents) +} + +/// Write `contents` to `path` at the file mode implied by `privacy`. +async fn write_file_private(path: &Path, contents: &[u8], privacy: Privacy) -> io::Result<()> { + #[cfg(unix)] + { + use tokio::io::AsyncWriteExt; + let mut opts = tokio::fs::OpenOptions::new(); + opts.write(true) + .create(true) + .truncate(true) + .mode(privacy.file_mode()); + let mut f = opts.open(path).await?; + f.write_all(contents).await?; + f.sync_all().await?; + } + + #[cfg(windows)] + { + let _ = privacy; + tokio::fs::write(path, contents).await?; + } + + Ok(()) } #[cfg(test)] @@ -30,28 +223,31 @@ mod tests { #[case::flat("file.txt")] #[case::one_level("sub/file.txt")] #[case::deeply_nested("a/b/c/file.txt")] - fn writes_file_creating_missing_parents(#[case] rel: &str) { + #[tokio::test] + async fn writes_file_creating_missing_parents(#[case] rel: &str) { let tmp = tempfile::tempdir().unwrap(); let path = tmp.path().join(rel); - write_create_all(&path, b"hello").unwrap(); + write_create_all(&path, b"hello").await.unwrap(); assert_eq!(std::fs::read_to_string(&path).unwrap(), "hello"); } #[rstest] - fn overwrites_an_existing_file() { + #[tokio::test] + async fn overwrites_an_existing_file() { let tmp = tempfile::tempdir().unwrap(); let path = tmp.path().join("file.txt"); - write_create_all(&path, b"first").unwrap(); - write_create_all(&path, b"second").unwrap(); + write_create_all(&path, b"first").await.unwrap(); + write_create_all(&path, b"second").await.unwrap(); assert_eq!(std::fs::read_to_string(&path).unwrap(), "second"); } #[rstest] - fn propagates_io_error_when_parent_is_a_file() { + #[tokio::test] + async fn propagates_io_error_when_parent_is_a_file() { let tmp = tempfile::tempdir().unwrap(); // A *file* sits where the target's parent directory would go, so // creating the parent must fail with an OS error. @@ -59,6 +255,58 @@ mod tests { std::fs::write(&blocker, b"x").unwrap(); let target = blocker.join("child.txt"); - assert!(write_create_all(&target, b"data").is_err()); + assert!(write_create_all(&target, b"data").await.is_err()); + } +} + +#[cfg(all(test, unix))] +#[allow(clippy::unwrap_used, reason = "test setup and assertions")] +mod unix_privacy_tests { + use super::*; + use std::os::unix::fs::PermissionsExt; + + fn mode_of(path: &Path) -> u32 { + std::fs::metadata(path).unwrap().permissions().mode() & 0o777 + } + + /// A `Private` file must land at exactly 0o600, in a 0o700 dir. + #[tokio::test] + async fn private_file_is_0600_in_dir_0700() { + let tmp = tempfile::tempdir().unwrap(); + let dir = tmp.path().join("hm"); + let file = dir.join("credentials.toml"); + + write_atomic(&file, b"token = \"hunter2\"\n", Privacy::Private) + .await + .unwrap(); + + assert_eq!(mode_of(&file), 0o600, "file mode must be 0o600"); + assert_eq!(mode_of(&dir), 0o700, "dir mode must be 0o700"); + } + + /// A `Public` file lands at 0o644, in a matching 0o755 dir. + #[tokio::test] + async fn public_file_is_0644_in_dir_0755() { + let tmp = tempfile::tempdir().unwrap(); + let dir = tmp.path().join("hm"); + let file = dir.join("config.toml"); + + write_atomic(&file, b"key = 1\n", Privacy::Public) + .await + .unwrap(); + + assert_eq!(mode_of(&file), 0o644, "file mode must be 0o644"); + assert_eq!(mode_of(&dir), 0o755, "dir mode must be 0o755"); + } + + /// Overwriting a secret must preserve 0o600 (guards the tempfile + + /// atomic-rename path against perm drift). + #[tokio::test] + async fn rewrite_preserves_private_mode() { + let tmp = tempfile::tempdir().unwrap(); + let file = tmp.path().join("credentials.toml"); + write_atomic(&file, b"a", Privacy::Private).await.unwrap(); + write_atomic(&file, b"bb", Privacy::Private).await.unwrap(); + assert_eq!(mode_of(&file), 0o600, "file mode must stay 0o600"); } } diff --git a/crates/hm-common/src/git.rs b/crates/hm-common/src/git.rs index d4f021e4..7149b765 100644 --- a/crates/hm-common/src/git.rs +++ b/crates/hm-common/src/git.rs @@ -21,7 +21,7 @@ pub struct Git<'bin> { } impl<'bin> Git<'bin> { - /// Wrap a `git` executable, typically via `AppRuntime::git()`. + /// Wrap a `git` executable, typically via `SysRuntime::git()`. #[must_use] pub const fn new(bin: &'bin Path) -> Self { Self { bin } diff --git a/crates/hm-common/src/lib.rs b/crates/hm-common/src/lib.rs index b0c5fb15..556bfa89 100644 --- a/crates/hm-common/src/lib.rs +++ b/crates/hm-common/src/lib.rs @@ -1,7 +1,8 @@ //! Harmont common utilities shared across the `hm` workspace. -#[cfg(feature = "app-runtime")] -pub mod app_runtime; +#[cfg(feature = "sys-runtime")] +pub mod sys_runtime; +pub mod dir_provider; pub mod format; pub mod fs; pub mod git; diff --git a/crates/hm-common/src/python.rs b/crates/hm-common/src/python.rs index f0b6b9a8..a79897ba 100644 --- a/crates/hm-common/src/python.rs +++ b/crates/hm-common/src/python.rs @@ -15,7 +15,7 @@ pub struct Python<'bin> { } impl<'bin> Python<'bin> { - /// Wrap a `python3` executable, typically via `AppRuntime::python()`. + /// Wrap a `python3` executable, typically via `SysRuntime::python()`. #[must_use] pub const fn new(bin: &'bin Path) -> Self { Self { bin } diff --git a/crates/hm-common/src/app_runtime.rs b/crates/hm-common/src/sys_runtime.rs similarity index 76% rename from crates/hm-common/src/app_runtime.rs rename to crates/hm-common/src/sys_runtime.rs index 8844ff4f..617f4261 100644 --- a/crates/hm-common/src/app_runtime.rs +++ b/crates/hm-common/src/sys_runtime.rs @@ -1,13 +1,14 @@ -//! Application runtime context. +//! System runtime context. use std::path::{Path, PathBuf}; use std::sync::OnceLock; +use crate::dir_provider::DirProvider; use crate::git::Git; use crate::process::{ExecutableNotFound, pathbin}; use crate::python::Python; -/// Failure to initialize the [`AppRuntime`]. +/// Failure to initialize the [`SysRuntime`]. #[derive(Debug, thiserror::Error)] pub enum InitError { /// A required executable was missing from `PATH`. @@ -16,26 +17,29 @@ pub enum InitError { /// The current directory could not be read. #[error("reading the current directory")] Cwd(#[source] std::io::Error), + /// The Harmont directory layout could not be resolved (no home directory?). + #[error("could not resolve the Harmont directory layout")] + Dirs, } // TODO: This is a process-wide singleton (a global `OnceLock`), which is // convenient but couples every caller to hidden global state. Consider -// threading an explicit `AppRuntime` handle through the application instead, +// threading an explicit `SysRuntime` handle through the application instead, // so dependencies are visible in signatures and tests can inject their own. -/// Process-wide runtime context, resolved once at startup. /// /// Install it with [`init`](Self::init), then read it from anywhere through the /// associated accessors — no threading required. #[derive(Debug)] -pub struct AppRuntime { +pub struct SysRuntime { git: PathBuf, python3: PathBuf, cwd: PathBuf, + dirs: DirProvider, } -static RUNTIME: OnceLock = OnceLock::new(); +static RUNTIME: OnceLock = OnceLock::new(); -impl AppRuntime { +impl SysRuntime { /// Resolve the runtime and install it as the process-wide singleton. /// /// Call once, early in `main`, before any accessor. A later call is ignored. @@ -54,6 +58,7 @@ impl AppRuntime { git: pathbin("git")?, python3: pathbin("python3")?, cwd: std::env::current_dir().map_err(InitError::Cwd)?, + dirs: DirProvider::new().ok_or(InitError::Dirs)?, }) } @@ -64,7 +69,7 @@ impl AppRuntime { fn get() -> &'static Self { RUNTIME .get() - .expect("AppRuntime::init must be called before the runtime is accessed") + .expect("SysRuntime::init must be called before the runtime is accessed") } /// The absolute working directory captured at initialization. @@ -76,6 +81,15 @@ impl AppRuntime { &Self::get().cwd } + /// The platform user directory roots, resolved once at initialization. + /// + /// # Panics + /// If [`init`](Self::init) has not been called. + #[must_use] + pub fn dirs() -> &'static DirProvider { + &Self::get().dirs + } + /// The system `git`, bound to a [`Git`] handle. /// /// # Panics @@ -108,14 +122,14 @@ mod tests { #[rstest] fn resolve_reports_toolchain_availability() { assert_eq!( - AppRuntime::resolve().is_ok(), + SysRuntime::resolve().is_ok(), pathbin("python3").is_ok() && pathbin("git").is_ok() ); } #[rstest] fn resolve_captures_an_absolute_cwd() { - let Ok(runtime) = AppRuntime::resolve() else { + let Ok(runtime) = SysRuntime::resolve() else { eprintln!("skipping: toolchain unavailable"); return; }; @@ -124,15 +138,15 @@ mod tests { #[rstest] fn init_installs_a_globally_accessible_runtime() { - if AppRuntime::resolve().is_err() { + if SysRuntime::resolve().is_err() { eprintln!("skipping: toolchain unavailable"); return; } - AppRuntime::init().unwrap(); - assert!(AppRuntime::cwd().is_absolute()); + SysRuntime::init().unwrap(); + assert!(SysRuntime::cwd().is_absolute()); // git() binds the resolved git; a fresh temp dir is not a repo. let dir = tempfile::tempdir().unwrap(); - assert!(AppRuntime::git().repo(dir.path()).is_err()); + assert!(SysRuntime::git().repo(dir.path()).is_err()); } #[rstest] diff --git a/crates/hm-config/Cargo.toml b/crates/hm-config/Cargo.toml index 2dd182c1..5f635577 100644 --- a/crates/hm-config/Cargo.toml +++ b/crates/hm-config/Cargo.toml @@ -7,7 +7,7 @@ repository.workspace = true description = "Layered (project/user/env) configuration + credential storage for the hm CLI." [dependencies] -hm-util = { workspace = true } +hm-common = { workspace = true } derive_more = { workspace = true } figment = { workspace = true } serde = { workspace = true } diff --git a/crates/hm-config/src/creds.rs b/crates/hm-config/src/creds.rs index b8efcfa8..5a1ab705 100644 --- a/crates/hm-config/src/creds.rs +++ b/crates/hm-config/src/creds.rs @@ -1,8 +1,7 @@ //! File-backed credential store at `~/.config/hm/credentials.toml`. //! -//! The file is written with mode 0o600 (parent dir 0o700) via -//! [`hm_util::os::fs::blocking::write_atomic_restricted`], keyed by -//! `(service, account)`. +//! The file is written with [`Privacy::Private`] (0o600, parent dir 0o700) +//! via [`hm_common::fs::write_atomic`], keyed by `(service, account)`. use anyhow::{Context, Result}; use serde::{Deserialize, Serialize}; @@ -16,8 +15,8 @@ struct CredentialFile { } fn path() -> Result { - let dir = hm_util::dirs::hm_config_dir().context("could not determine config directory")?; - Ok(dir.join("credentials.toml")) + let dirs = hm_common::dir_provider::DirProvider::new().context("could not determine config directory")?; + Ok(dirs.config().join("hm").join("credentials.toml")) } fn load() -> CredentialFile { @@ -30,16 +29,12 @@ fn load() -> CredentialFile { toml::from_str(&contents).unwrap_or_default() } -fn save(file: &CredentialFile) -> Result<()> { +async fn save(file: &CredentialFile) -> Result<()> { let p = path()?; let serialized = toml::to_string_pretty(file).context("serializing credentials")?; - hm_util::os::fs::blocking::write_atomic_restricted( - &p, - serialized.as_bytes(), - hm_util::os::fs::FileMode(0o600), - hm_util::os::fs::DirMode(0o700), - ) - .with_context(|| format!("writing {}", p.display()))?; + hm_common::fs::write_atomic(&p, serialized.as_bytes(), hm_common::fs::Privacy::Private) + .await + .with_context(|| format!("writing {}", p.display()))?; Ok(()) } @@ -51,13 +46,13 @@ pub fn get(service: &str, account: &str) -> Option { } /// Write a credential. Silently no-ops on I/O failure (best-effort). -pub fn set(service: &str, account: &str, secret: &str) { +pub async fn set(service: &str, account: &str, secret: &str) { let mut f = load(); f.entries .entry(service.to_string()) .or_default() .insert(account.to_string(), secret.to_string()); - let _ = save(&f); + let _ = save(&f).await; } /// Credential `service` name for the cloud bearer token (account = API base URL). @@ -82,20 +77,20 @@ pub fn cloud_token(api_base: &str) -> Option { /// /// Silently no-ops on I/O failure (matches the best-effort semantics of /// the underlying [`set`] call). -pub fn set_cloud_token(api_base: &str, token: &str) { - set(CLOUD_SERVICE, api_base, token); +pub async fn set_cloud_token(api_base: &str, token: &str) { + set(CLOUD_SERVICE, api_base, token).await; } /// Remove any stored cloud bearer token for `api_base`. /// /// Silently no-ops if the entry is absent or the write fails. -pub fn forget_cloud_token(api_base: &str) { - delete(CLOUD_SERVICE, api_base); +pub async fn forget_cloud_token(api_base: &str) { + delete(CLOUD_SERVICE, api_base).await; } /// Remove a credential. Silently no-ops if the entry is absent or the /// underlying write fails. -pub fn delete(service: &str, account: &str) { +pub async fn delete(service: &str, account: &str) { let mut f = load(); let now_empty = f.entries.get_mut(service).is_some_and(|svc| { svc.remove(account); @@ -104,22 +99,31 @@ pub fn delete(service: &str, account: &str) { if now_empty { f.entries.remove(service); } - let _ = save(&f); + let _ = save(&f).await; } #[cfg(test)] #[allow(clippy::unwrap_used, unsafe_code)] mod tests { use super::*; + use rstest::rstest; - fn with_home(f: F) { + #[rstest] + #[tokio::test] + async fn round_trip() { let tmp = tempfile::tempdir().unwrap(); let prev = std::env::var_os("HOME"); // SAFETY: tests are single-threaded for env mutation by Cargo. unsafe { std::env::set_var("HOME", tmp.path()); } - f(); + + assert_eq!(get("svc", "acct"), None); + set("svc", "acct", "shh").await; + assert_eq!(get("svc", "acct").as_deref(), Some("shh")); + delete("svc", "acct").await; + assert_eq!(get("svc", "acct"), None); + unsafe { if let Some(v) = prev { std::env::set_var("HOME", v); @@ -128,15 +132,4 @@ mod tests { } } } - - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] - async fn round_trip() { - with_home(|| { - assert_eq!(get("svc", "acct"), None); - set("svc", "acct", "shh"); - assert_eq!(get("svc", "acct").as_deref(), Some("shh")); - delete("svc", "acct"); - assert_eq!(get("svc", "acct"), None); - }); - } } diff --git a/crates/hm-config/src/lib.rs b/crates/hm-config/src/lib.rs index a916f92e..d9bd57e8 100644 --- a/crates/hm-config/src/lib.rs +++ b/crates/hm-config/src/lib.rs @@ -119,8 +119,8 @@ impl Config { /// /// Returns an error if the platform config directory cannot be determined. pub fn user_config_path() -> Result { - let dir = hm_util::dirs::hm_config_dir().context("could not determine config directory")?; - Ok(dir.join("config.toml")) + let dirs = hm_common::dir_provider::DirProvider::new().context("could not determine config directory")?; + Ok(dirs.config().join("hm").join("config.toml")) } /// Project-level config path: `/.hm/config.toml`. @@ -177,15 +177,11 @@ impl Config { /// # Errors /// /// Returns an error if TOML serialization fails or the atomic write fails. - pub fn save_to(&self, path: &Path) -> Result<()> { + pub async fn save_to(&self, path: &Path) -> Result<()> { let serialized = toml::to_string_pretty(self).context("serializing config")?; - hm_util::os::fs::blocking::write_atomic_restricted( - path, - serialized.as_bytes(), - hm_util::os::fs::FileMode(0o644), - hm_util::os::fs::DirMode(0o700), - ) - .with_context(|| format!("writing {}", path.display())) + hm_common::fs::write_atomic(path, serialized.as_bytes(), hm_common::fs::Privacy::Public) + .await + .with_context(|| format!("writing {}", path.display())) } /// Save to user-level config path (`~/.config/hm/config.toml`). @@ -193,8 +189,8 @@ impl Config { /// # Errors /// /// Returns an error if the path cannot be determined or the write fails. - pub fn save_user(&self) -> Result<()> { - self.save_to(&Self::user_config_path()?) + pub async fn save_user(&self) -> Result<()> { + self.save_to(&Self::user_config_path()?).await } } @@ -390,7 +386,7 @@ org = "project-org" }, ..Config::default() }; - cfg.save_to(&path).unwrap(); + cfg.save_to(&path).await.unwrap(); let loaded = Config::load_from_paths(Some(&path), None).unwrap(); assert_eq!(loaded.cloud.org.as_deref(), Some("saved-org")); diff --git a/crates/hm-dsl-engine/Cargo.toml b/crates/hm-dsl-engine/Cargo.toml index db718a23..261b7651 100644 --- a/crates/hm-dsl-engine/Cargo.toml +++ b/crates/hm-dsl-engine/Cargo.toml @@ -12,7 +12,7 @@ categories = ["command-line-utilities"] path = "src/lib.rs" [dependencies] -hm-common = { workspace = true, features = ["app-runtime"] } +hm-common = { workspace = true, features = ["sys-runtime"] } anyhow = { workspace = true } async-trait = { workspace = true } derive_more = { workspace = true } diff --git a/crates/hm-dsl-engine/src/python_engine.rs b/crates/hm-dsl-engine/src/python_engine.rs index 9980898a..b8e2dea9 100644 --- a/crates/hm-dsl-engine/src/python_engine.rs +++ b/crates/hm-dsl-engine/src/python_engine.rs @@ -2,7 +2,7 @@ use std::path::Path; use anyhow::{Context, Result}; use async_trait::async_trait; -use hm_common::app_runtime::AppRuntime; +use hm_common::sys_runtime::SysRuntime; use hm_common::process::CapturedStreams as _; use tracing::debug; @@ -63,8 +63,8 @@ print(json.dumps(match['definition'])) pub struct SubprocessPythonEngine; impl SubprocessPythonEngine { - /// Create the engine. It runs `python3` through [`AppRuntime::python`], so - /// [`AppRuntime::init`] must have been called first. + /// Create the engine. It runs `python3` through [`SysRuntime::python`], so + /// [`SysRuntime::init`] must have been called first. #[must_use] pub const fn new() -> Self { Self @@ -80,7 +80,7 @@ impl SubprocessPythonEngine { let harmont_pkg = tmp.path().join("harmont"); bundled_sources::extract_to(&bundled_sources::HARMONT_PY, &harmont_pkg)?; - let mut py = AppRuntime::python().program(script); + let mut py = SysRuntime::python().program(script); py.args(extra_args).current_dir(project_dir); py.pythonpath(tmp.path()); diff --git a/crates/hm-dsl-engine/tests/python_engine_test.rs b/crates/hm-dsl-engine/tests/python_engine_test.rs index 76b37cc5..ca8ba081 100644 --- a/crates/hm-dsl-engine/tests/python_engine_test.rs +++ b/crates/hm-dsl-engine/tests/python_engine_test.rs @@ -10,7 +10,7 @@ use hm_dsl_engine::DslEngine; #[tokio::test] async fn python_roundtrip() { // Skip if the toolchain (python3 + git) is unavailable. - if hm_common::app_runtime::AppRuntime::init().is_err() { + if hm_common::sys_runtime::SysRuntime::init().is_err() { eprintln!("skipping: build toolchain unavailable"); return; } @@ -43,7 +43,7 @@ def ci() -> hm.Step: #[tokio::test] async fn python_registry_json_carries_triggers_and_allow_manual() { - if hm_common::app_runtime::AppRuntime::init().is_err() { + if hm_common::sys_runtime::SysRuntime::init().is_err() { eprintln!("skipping: build toolchain unavailable"); return; } diff --git a/crates/hm-exec/Cargo.toml b/crates/hm-exec/Cargo.toml index 6afc9d28..90056d24 100644 --- a/crates/hm-exec/Cargo.toml +++ b/crates/hm-exec/Cargo.toml @@ -10,7 +10,6 @@ description = "Pluggable CI execution backends (local VM + cloud) for the hm CLI hm-common = { workspace = true } hm-plugin-protocol = { workspace = true } hm-pipeline-ir = { workspace = true } -hm-util = { workspace = true } hm-vm = { workspace = true, features = ["docker-backend"] } harmont-cloud = { workspace = true } async-trait = { workspace = true } diff --git a/crates/hm-exec/src/local/backend.rs b/crates/hm-exec/src/local/backend.rs index 0713100c..0f3b3b9d 100644 --- a/crates/hm-exec/src/local/backend.rs +++ b/crates/hm-exec/src/local/backend.rs @@ -50,10 +50,11 @@ impl LocalBackend { /// (VM backend + snapshot registry) and registering the [`VmRunner`] as /// the default runner. fn build_registry(&self) -> Result { - let cache_dir = hm_util::dirs::hm_cache_dir().ok_or_else(|| { + let dirs = hm_common::dir_provider::DirProvider::new().ok_or_else(|| { BackendError::Local("cannot resolve the Harmont cache directory".into()) })?; - let registry = ImageRegistry::open(&cache_dir.join("registry.db"), REGISTRY_CAPACITY) + let registry = + ImageRegistry::open(&dirs.cache().join("hm").join("registry.db"), REGISTRY_CAPACITY) .map_err(|e| BackendError::Local(format!("opening snapshot registry: {e:#}")))?; let config = VmConfig { diff --git a/crates/hm-plugin-cloud/src/auth/login.rs b/crates/hm-plugin-cloud/src/auth/login.rs index 97b18423..d5bc7b85 100644 --- a/crates/hm-plugin-cloud/src/auth/login.rs +++ b/crates/hm-plugin-cloud/src/auth/login.rs @@ -28,7 +28,7 @@ pub(crate) async fn run(env: &BTreeMap, paste: bool) -> Result<( login_loopback(&client, &app).await? }; - hm_config::creds::set_cloud_token(&api, &token); + hm_config::creds::set_cloud_token(&api, &token).await; // Confirm by reading back the authenticated user. let authed = HarmontClient::with_base_url(token, &api); diff --git a/crates/hm-plugin-cloud/src/auth/logout.rs b/crates/hm-plugin-cloud/src/auth/logout.rs index 57478847..9461dc6b 100644 --- a/crates/hm-plugin-cloud/src/auth/logout.rs +++ b/crates/hm-plugin-cloud/src/auth/logout.rs @@ -6,13 +6,9 @@ use anyhow::Result; use crate::settings; -#[allow( - clippy::unused_async, - reason = "async to match the uniform await arm in cli::dispatch_command" -)] pub(crate) async fn run(_env: &BTreeMap) -> Result<()> { let (_client, api) = settings::anon_client()?; - hm_config::creds::forget_cloud_token(&api); + hm_config::creds::forget_cloud_token(&api).await; tracing::info!("logged out of {api}"); Ok(()) } diff --git a/crates/hm-plugin-cloud/src/verbs/org.rs b/crates/hm-plugin-cloud/src/verbs/org.rs index 6d9f76fe..00aefbbf 100644 --- a/crates/hm-plugin-cloud/src/verbs/org.rs +++ b/crates/hm-plugin-cloud/src/verbs/org.rs @@ -29,7 +29,7 @@ async fn switch(client: &harmont_cloud::HarmontClient, slug: &str) -> Result<()> .ok_or_else(|| anyhow::anyhow!("no organization with slug '{slug}'"))?; let mut cfg = hm_config::Config::load(None)?; cfg.cloud.org = Some(found.slug.clone()); - cfg.save_user().context("saving config")?; + cfg.save_user().await.context("saving config")?; tracing::info!("active organization: {} ({})", found.name, found.slug); Ok(()) } diff --git a/crates/hm-util/Cargo.toml b/crates/hm-util/Cargo.toml deleted file mode 100644 index e5738a6a..00000000 --- a/crates/hm-util/Cargo.toml +++ /dev/null @@ -1,29 +0,0 @@ -[package] -name = "hm-util" -version = "0.0.0-dev" -edition.workspace = true -license.workspace = true -repository.workspace = true -description = "Shared OS and filesystem utilities for Harmont crates." - -[dependencies] -anyhow = { workspace = true } -dirs = "6" -tempfile = "3" -tokio = { version = "1", features = ["rt", "rt-multi-thread", "fs", "io-util"] } -tracing = { workspace = true } - -[target.'cfg(windows)'.dependencies.windows] -version = "0.62" -features = [ - "Win32_Foundation", - "Win32_Storage_FileSystem", -] - -[dev-dependencies] -rstest = { workspace = true } -tempfile = "3" -tokio = { version = "1", features = ["full", "test-util"] } - -[lints] -workspace = true diff --git a/crates/hm-util/src/dirs.rs b/crates/hm-util/src/dirs.rs deleted file mode 100644 index d9e9b172..00000000 --- a/crates/hm-util/src/dirs.rs +++ /dev/null @@ -1,97 +0,0 @@ -//! Harmont-specific directory resolution. -//! -//! Every directory accessor in this module returns an `hm`-namespaced path -//! under an XDG-correct root: configuration in `~/.config/hm/`, regenerable -//! cache in `~/.cache/hm/`. Raw platform primitives (`home_dir`, `config_dir`, -//! `cache_dir`) live in `os::dirs` and are **not** re-exported — callers -//! outside `hm-util` should never need them. - -#![allow(clippy::must_use_candidate)] - -use std::path::PathBuf; - -use crate::os::dirs as platform; - -/// `~/.config/hm/` — user config root (`config.toml`, `credentials.toml`). -pub fn hm_config_dir() -> Option { - platform::config_dir().map(|c| c.join("hm")) -} - -/// `~/.cache/hm/` — local build cache root (regenerable). -pub fn hm_cache_dir() -> Option { - platform::cache_dir().map(|c| c.join("hm")) -} - -/// `~/.cache/hm/workspaces/` — COW workspace cache root. -pub fn hm_workspace_cache_dir() -> Option { - hm_cache_dir().map(|c| c.join("workspaces")) -} - -/// Walk up from `start` looking for a directory containing `.hm/`. -/// Returns the project root (the directory *containing* `.hm/`), -/// or `None` if the filesystem root is reached without finding one. -pub fn find_project_root(start: &std::path::Path) -> Option { - let mut current = start; - loop { - if current.join(".hm").is_dir() { - return Some(current.to_path_buf()); - } - current = current.parent()?; - } -} - -#[cfg(test)] -#[allow(clippy::unwrap_used, reason = "test setup and assertions")] -mod tests { - use rstest::rstest; - - use super::*; - - #[rstest] - fn hm_config_dir_under_config() { - let p = hm_config_dir().unwrap(); - assert!(p.ends_with("hm"), "expected path ending in 'hm', got {p:?}"); - let parent = p.parent().unwrap(); - assert!( - parent.ends_with(".config") || parent.ends_with("AppData/Roaming"), - "unexpected parent: {parent:?}" - ); - } - - #[rstest] - fn hm_cache_dir_under_cache() { - let p = hm_cache_dir().unwrap(); - assert!(p.ends_with("hm"), "expected path ending in 'hm', got {p:?}"); - } - - #[rstest] - fn hm_workspace_cache_dir_resolves() { - let p = hm_workspace_cache_dir().unwrap(); - assert!(p.ends_with("hm/workspaces"), "got {p:?}"); - } - - #[rstest] - fn find_project_root_at_current_dir() { - let tmp = tempfile::tempdir().unwrap(); - std::fs::create_dir(tmp.path().join(".hm")).unwrap(); - let found = find_project_root(tmp.path()); - assert_eq!(found, Some(tmp.path().to_path_buf())); - } - - #[rstest] - fn find_project_root_walks_up() { - let tmp = tempfile::tempdir().unwrap(); - std::fs::create_dir(tmp.path().join(".hm")).unwrap(); - let nested = tmp.path().join("src").join("deep"); - std::fs::create_dir_all(&nested).unwrap(); - let found = find_project_root(&nested); - assert_eq!(found, Some(tmp.path().to_path_buf())); - } - - #[rstest] - fn find_project_root_returns_none_when_missing() { - let tmp = tempfile::tempdir().unwrap(); - let found = find_project_root(tmp.path()); - assert_eq!(found, None); - } -} diff --git a/crates/hm-util/src/lib.rs b/crates/hm-util/src/lib.rs deleted file mode 100644 index c5284c52..00000000 --- a/crates/hm-util/src/lib.rs +++ /dev/null @@ -1,2 +0,0 @@ -pub mod dirs; -pub mod os; diff --git a/crates/hm-util/src/os/dirs.rs b/crates/hm-util/src/os/dirs.rs deleted file mode 100644 index 9027ad3a..00000000 --- a/crates/hm-util/src/os/dirs.rs +++ /dev/null @@ -1,31 +0,0 @@ -//! Raw platform directory primitives. -//! -//! This module is `pub(crate)` — external callers must use -//! [`crate::dirs`] which provides Harmont-specific accessors. -//! -//! On non-Windows we intentionally hardcode `~/.config` and `~/.cache` rather -//! than reading `$XDG_CONFIG_HOME` / `$XDG_CACHE_HOME`. This keeps both -//! primitives consistent and our paths predictable; it is deliberate, not an -//! oversight. Revisit only if honoring the XDG env vars becomes a real need. - -use std::path::PathBuf; - -pub(crate) fn home_dir() -> Option { - dirs::home_dir() -} - -pub(crate) fn config_dir() -> Option { - if cfg!(windows) { - dirs::config_dir() - } else { - home_dir().map(|h| h.join(".config")) - } -} - -pub(crate) fn cache_dir() -> Option { - if cfg!(windows) { - dirs::cache_dir() - } else { - home_dir().map(|h| h.join(".cache")) - } -} diff --git a/crates/hm-util/src/os/fs.rs b/crates/hm-util/src/os/fs.rs deleted file mode 100644 index 19441ca4..00000000 --- a/crates/hm-util/src/os/fs.rs +++ /dev/null @@ -1,287 +0,0 @@ -//! Filesystem helpers. -//! -//! The main entry point is [`write_atomic_restricted`]. A synchronous -//! wrapper is available at [`blocking::write_atomic_restricted`] for -//! callers that run inside a tokio runtime but cannot use async. -//! -//! Both guarantee that readers observe either the full old contents or -//! the full new contents — never a truncated file — and that Unix -//! file/directory modes are set atomically with creation. - -use std::io; -use std::path::Path; - -/// Unix mode bits for a file (e.g. `0o600`). -/// -/// A distinct newtype from [`DirMode`] so the file- and directory-mode -/// arguments of [`write_atomic_restricted`] cannot be transposed: passing -/// them in the wrong order is a compile error rather than a silent -/// security regression (a secrets file landing at `0o700`, say). -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct FileMode(pub u32); - -/// Unix mode bits for a directory (e.g. `0o700`). -/// -/// See [`FileMode`] for why this is a distinct newtype. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct DirMode(pub u32); - -/// Write `contents` to `path` atomically with `file`, ensuring the -/// parent directory exists and is set to `dir`. -/// -/// # Errors -/// -/// Returns an error if `path` has no parent or no file-name component, -/// the parent directory cannot be created or chmod'd to `dir`, the -/// tempfile cannot be opened with `file` or written, or the final -/// `rename` over `path` fails. -pub async fn write_atomic_restricted( - path: impl AsRef, - contents: impl AsRef<[u8]>, - file: FileMode, - dir: DirMode, -) -> io::Result<()> { - let path = path.as_ref().to_owned(); - let contents = contents.as_ref().to_vec(); - - let parent = path - .parent() - .ok_or_else(|| { - io::Error::new( - io::ErrorKind::InvalidInput, - format!("{} has no parent directory", path.display()), - ) - })? - .to_owned(); - - create_dir_with_mode(&parent, dir.0).await?; - - let file_name = path - .file_name() - .ok_or_else(|| { - io::Error::new( - io::ErrorKind::InvalidInput, - format!("{} has no file name", path.display()), - ) - })? - .to_os_string(); - let mut tmp_name = file_name; - tmp_name.push(format!(".tmp.{}", std::process::id())); - let tmp_path = parent.join(&tmp_name); - - write_file_with_mode(&tmp_path, &contents, file.0).await?; - - let rename_result = atomic_rename_over(&tmp_path, &path).await; - if rename_result.is_err() { - let _ = tokio::fs::remove_file(&tmp_path).await; - } - rename_result -} - -/// Atomically replace `to` with `from`. -/// -/// On Unix this delegates to [`tokio::fs::rename`] (`rename(2)` — atomic -/// by POSIX guarantee). On Windows this uses `ReplaceFileW` (preserves -/// ACLs and alternate data streams) when the target exists, falling back -/// to `MoveFileExW` with `MOVEFILE_REPLACE_EXISTING` for first-write. -/// -/// # Errors -/// -/// Returns an error if the rename fails (permission denied, cross-device, -/// source missing, etc.). -pub async fn atomic_rename_over(from: impl AsRef, to: impl AsRef) -> io::Result<()> { - #[cfg(unix)] - { - tokio::fs::rename(from.as_ref(), to.as_ref()).await - } - #[cfg(windows)] - { - fn atomic_rename_over_impl(from: &Path, to: &Path) -> io::Result<()> { - use windows::Win32::Storage::FileSystem::{ - MOVEFILE_REPLACE_EXISTING, MOVEFILE_WRITE_THROUGH, MoveFileExW, - REPLACEFILE_IGNORE_MERGE_ERRORS, ReplaceFileW, - }; - use windows::core::HSTRING; - - let from_w = HSTRING::from(from.as_os_str()); - let to_w = HSTRING::from(to.as_os_str()); - - if to.exists() { - let result = unsafe { - ReplaceFileW( - &to_w, - &from_w, - windows::core::PCWSTR::null(), - REPLACEFILE_IGNORE_MERGE_ERRORS, - None, - None, - ) - }; - return result.map_err(|e| io::Error::new(io::ErrorKind::Other, e)); - } - - let result = unsafe { - MoveFileExW( - &from_w, - &to_w, - MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH, - ) - }; - result.map_err(|e| io::Error::new(io::ErrorKind::Other, e)) - } - - let from = from.as_ref().to_owned(); - let to = to.as_ref().to_owned(); - tokio::task::spawn_blocking(move || atomic_rename_over_impl(&from, &to)) - .await - .map_err(io::Error::other)? - } -} - -/// Remove a file if it exists; silently return `Ok(())` if it does not. -/// -/// # Errors -/// -/// Returns an error if `remove_file` fails for any reason other than -/// `NotFound`. -pub async fn remove_file_if_exists(path: impl AsRef) -> io::Result<()> { - match tokio::fs::remove_file(path).await { - Ok(()) => Ok(()), - Err(e) if e.kind() == io::ErrorKind::NotFound => Ok(()), - Err(e) => Err(e), - } -} - -#[cfg(unix)] -async fn create_dir_with_mode(dir: &Path, mode: u32) -> io::Result<()> { - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - match tokio::fs::metadata(dir).await { - Ok(meta) => { - let current = meta.permissions().mode() & 0o777; - if current != mode { - tokio::fs::set_permissions(dir, std::fs::Permissions::from_mode(mode)).await?; - } - } - Err(e) if e.kind() == io::ErrorKind::NotFound => { - let mut builder = tokio::fs::DirBuilder::new(); - builder.recursive(true).mode(mode); - builder.create(dir).await?; - } - Err(e) => return Err(e), - } - } - - #[cfg(windows)] - { - tokio::fs::create_dir_all(dir).await - } - Ok(()) -} - -async fn write_file_with_mode(path: &Path, contents: &[u8], mode: u32) -> io::Result<()> { - #[cfg(unix)] - { - use tokio::io::AsyncWriteExt; - let mut opts = tokio::fs::OpenOptions::new(); - opts.write(true).create(true).truncate(true).mode(mode); - let mut f = opts.open(path).await?; - f.write_all(contents).await?; - f.sync_all().await?; - } - - #[cfg(windows)] - { - tokio::fs::write(path, contents).await - } - - Ok(()) -} - -/// Synchronous wrappers that shell out to the async API via -/// `tokio::task::block_in_place`. Safe to call from sync contexts -/// that run inside a tokio runtime. -pub mod blocking { - use super::{DirMode, FileMode}; - use std::io; - use std::path::Path; - - fn block_on>>(f: F) -> io::Result<()> { - tokio::task::block_in_place(|| tokio::runtime::Handle::current().block_on(f)) - } - - /// Blocking counterpart of [`super::write_atomic_restricted`]. - /// - /// See the [module-level documentation](super) for semantics. - /// - /// # Errors - /// - /// Returns an error if `path` has no parent or no file-name component, - /// the parent directory cannot be created or chmod'd to `dir`, the - /// tempfile cannot be opened with `file` or written, or the final - /// `rename` over `path` fails. - pub fn write_atomic_restricted( - path: impl AsRef, - contents: impl AsRef<[u8]>, - file: FileMode, - dir: DirMode, - ) -> io::Result<()> { - block_on(super::write_atomic_restricted(path, contents, file, dir)) - } - - /// Blocking counterpart of [`super::remove_file_if_exists`]. - /// - /// # Errors - /// - /// Returns an error if `remove_file` fails for any reason other than - /// `NotFound`. - pub fn remove_if_exists(path: impl AsRef) -> io::Result<()> { - block_on(super::remove_file_if_exists(path)) - } -} - -#[cfg(all(test, unix))] -#[allow(clippy::unwrap_used)] -mod tests { - use super::*; - use std::os::unix::fs::PermissionsExt; - - /// Credentials (and any secret) must land at exactly 0o600, in a 0o700 dir. - #[tokio::test] - async fn writes_file_0600_in_dir_0700() { - let tmp = tempfile::tempdir().unwrap(); - let dir = tmp.path().join("hm"); - let file = dir.join("credentials.toml"); - - write_atomic_restricted( - &file, - b"token = \"hunter2\"\n", - FileMode(0o600), - DirMode(0o700), - ) - .await - .unwrap(); - - let fmode = std::fs::metadata(&file).unwrap().permissions().mode() & 0o777; - assert_eq!(fmode, 0o600, "file mode must be 0o600, got {fmode:o}"); - let dmode = std::fs::metadata(&dir).unwrap().permissions().mode() & 0o777; - assert_eq!(dmode, 0o700, "dir mode must be 0o700, got {dmode:o}"); - } - - /// Overwriting an existing secret must preserve 0o600 (guards the - /// temp-file + atomic-rename path against perm drift). - #[tokio::test] - async fn rewrite_preserves_0600() { - let tmp = tempfile::tempdir().unwrap(); - let file = tmp.path().join("credentials.toml"); - write_atomic_restricted(&file, b"a", FileMode(0o600), DirMode(0o700)) - .await - .unwrap(); - write_atomic_restricted(&file, b"bb", FileMode(0o600), DirMode(0o700)) - .await - .unwrap(); - let fmode = std::fs::metadata(&file).unwrap().permissions().mode() & 0o777; - assert_eq!(fmode, 0o600, "file mode must stay 0o600, got {fmode:o}"); - } -} diff --git a/crates/hm-util/src/os/mod.rs b/crates/hm-util/src/os/mod.rs deleted file mode 100644 index 0bc7829b..00000000 --- a/crates/hm-util/src/os/mod.rs +++ /dev/null @@ -1,2 +0,0 @@ -pub(crate) mod dirs; -pub mod fs; diff --git a/crates/hm/Cargo.toml b/crates/hm/Cargo.toml index a54b868d..4187392a 100644 --- a/crates/hm/Cargo.toml +++ b/crates/hm/Cargo.toml @@ -59,8 +59,7 @@ futures = "0.3" futures-util = "0.3" hm-pipeline-ir = { workspace = true } hm-plugin-protocol = { workspace = true } -hm-common = { workspace = true, features = ["app-runtime"] } -hm-util = { workspace = true } +hm-common = { workspace = true, features = ["sys-runtime"] } hm-config = { workspace = true } hm-plugin-cloud = { workspace = true } harmont-cloud = { workspace = true } diff --git a/crates/hm/src/commands/cache/clean.rs b/crates/hm/src/commands/cache/clean.rs index 0b7fe184..1fe68280 100644 --- a/crates/hm/src/commands/cache/clean.rs +++ b/crates/hm/src/commands/cache/clean.rs @@ -1,13 +1,15 @@ use anyhow::Result; +use hm_common::sys_runtime::SysRuntime; use hm_vm::VmBackend as _; use human_units::FormatSize as _; /// # Errors /// Returns an error if workspace cache removal fails. pub async fn handle_clean() -> Result { - let ws_cleaned = if let Some(ws_cache) = hm_util::dirs::hm_workspace_cache_dir() - && ws_cache.exists() - { + let hm_cache = SysRuntime::dirs().cache().join("hm"); + + let ws_cache = hm_cache.join("workspaces"); + let ws_cleaned = if ws_cache.exists() { let size = dir_size(&ws_cache); std::fs::remove_dir_all(&ws_cache)?; tracing::info!( @@ -20,24 +22,20 @@ pub async fn handle_clean() -> Result { false }; - let db_cleaned = if let Some(cache_dir) = hm_util::dirs::hm_cache_dir() { - let db_path = cache_dir.join("registry.db"); - if db_path.exists() { - // Remove the backing Docker images BEFORE deleting registry.db. - // The registry is the only index from a cache key to its tagged - // image (`forever-*`, etc.); once the DB is gone the images can't - // be located by key, and `docker image prune` only reclaims - // *dangling* images, so a tagged snapshot survives it. So we - // enumerate the registry, remove each image via the Docker - // backend (best-effort), then drop the DB. - remove_registered_images(&db_path).await; + let db_path = hm_cache.join("registry.db"); + let db_cleaned = if db_path.exists() { + // Remove the backing Docker images BEFORE deleting registry.db. + // The registry is the only index from a cache key to its tagged + // image (`forever-*`, etc.); once the DB is gone the images can't + // be located by key, and `docker image prune` only reclaims + // *dangling* images, so a tagged snapshot survives it. So we + // enumerate the registry, remove each image via the Docker + // backend (best-effort), then drop the DB. + remove_registered_images(&db_path).await; - std::fs::remove_file(&db_path)?; - tracing::info!(path = %db_path.display(), "removed VM image registry"); - true - } else { - false - } + std::fs::remove_file(&db_path)?; + tracing::info!(path = %db_path.display(), "removed VM image registry"); + true } else { false }; diff --git a/crates/hm/src/commands/init.rs b/crates/hm/src/commands/init.rs index 846e7a38..40e32b87 100644 --- a/crates/hm/src/commands/init.rs +++ b/crates/hm/src/commands/init.rs @@ -168,7 +168,7 @@ fn write_cloud_project_config(dir: &std::path::Path, org_slug: &str) -> Result<( Ok(()) } -fn write_template(dir: &Path, tmpl: &Template, force: bool) -> Result { +async fn write_template(dir: &Path, tmpl: &Template, force: bool) -> Result { let harmont_dir = dir.join(".hm"); let already_has_pipeline = detect::has_pipeline_files(dir); @@ -187,13 +187,14 @@ fn write_template(dir: &Path, tmpl: &Template, force: bool) -> Result { // pipeline.py and deploy.py). `std::fs::write` clobbers just the target. let dest = harmont_dir.join(tmpl.filename); hm_common::fs::write_create_all(&dest, tmpl.content) + .await .with_context(|| format!("writing {}", dest.display()))?; ensure_gitignore_entry(&harmont_dir, "node_modules/")?; ensure_gitignore_entry(&harmont_dir, "__pycache__/")?; Ok(true) } -fn write_skills(dir: &Path, force: bool) -> Result<()> { +async fn write_skills(dir: &Path, force: bool) -> Result<()> { let skills: &[(&str, &str)] = &[ ("validate-ci", SKILL_VALIDATE_CI), ("write-pipeline", SKILL_WRITE_PIPELINE), @@ -220,6 +221,7 @@ fn write_skills(dir: &Path, force: bool) -> Result<()> { let updated = dest.exists(); hm_common::fs::write_create_all(&dest, content) + .await .with_context(|| format!("writing {}", dest.display()))?; if updated { tracing::info!("overwrote Claude Code skill: .claude/skills/{slug}/SKILL.md"); @@ -287,7 +289,7 @@ pub async fn handle(args: InitArgs) -> Result<()> { pick_interactive()? }; let tmpl = kind.meta(); - let wrote_pipeline = write_template(&args.dir, &tmpl, args.force)?; + let wrote_pipeline = write_template(&args.dir, &tmpl, args.force).await?; if wrote_pipeline { let dsl = match kind { TemplateKind::Nextjs | TemplateKind::Js | TemplateKind::Zig => "TypeScript", @@ -314,7 +316,7 @@ pub async fn handle(args: InitArgs) -> Result<()> { // Skills are offered whenever a terminal is present, independent of // whether a template flag was passed. if tty && prompt_skills()? { - write_skills(&args.dir, args.force)?; + write_skills(&args.dir, args.force).await?; } let project_config = hm_config::Config::project_config_path(&args.dir); @@ -352,7 +354,8 @@ mod tests { #[case::customized_no_force(Some("# my local edits"), false, "# my local edits")] #[case::customized_force(Some("# my local edits"), true, SKILL_VALIDATE_CI)] #[case::unchanged_idempotent(Some(SKILL_VALIDATE_CI), false, SKILL_VALIDATE_CI)] - fn write_skills_behaves( + #[tokio::test] + async fn write_skills_behaves( #[case] preexisting: Option<&str>, #[case] force: bool, #[case] expected: &str, @@ -364,15 +367,16 @@ mod tests { std::fs::write(&dest, content).unwrap(); } - write_skills(dir.path(), force).unwrap(); + write_skills(dir.path(), force).await.unwrap(); assert_eq!(std::fs::read_to_string(&dest).unwrap(), expected); } #[rstest] - fn write_skills_installs_sibling_skills_when_absent() { + #[tokio::test] + async fn write_skills_installs_sibling_skills_when_absent() { let dir = tempfile::tempdir().unwrap(); - write_skills(dir.path(), false).unwrap(); + write_skills(dir.path(), false).await.unwrap(); // Skills other than the one under test are installed too. assert!(skill_path(dir.path(), "write-pipeline").exists()); diff --git a/crates/hm/src/commands/run/mod.rs b/crates/hm/src/commands/run/mod.rs index ce2ef283..82510567 100644 --- a/crates/hm/src/commands/run/mod.rs +++ b/crates/hm/src/commands/run/mod.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use anyhow::{Context, Result}; use bstr::ByteSlice as _; -use hm_common::app_runtime::AppRuntime; +use hm_common::sys_runtime::SysRuntime; use hm_common::git::{GitBranch, GitRemote, GitRepo}; use hm_dsl_engine::{DslEngine, detect}; use human_units::FormatSize as _; @@ -137,7 +137,7 @@ pub async fn handle(args: RunArgs, ctx: RunContext) -> Result { // strings so `repo_root` can move into the request, and the cloud paths // below reuse `remote_url` / `default_branch` instead of re-shelling to git. let (branch, commit, repo_name, remote_url, default_branch) = { - let git = AppRuntime::git(); + let git = SysRuntime::git(); let repo = git.repo(&repo_root).ok(); let head = repo.as_ref().and_then(GitRepo::current_branch); let remote = repo.as_ref().and_then(|r| r.remote("origin")); @@ -296,7 +296,7 @@ async fn render_pipeline( ) -> Result<(std::path::PathBuf, String, String)> { let repo_root = match args.dir.clone() { Some(p) => p, - None => AppRuntime::cwd().to_path_buf(), + None => SysRuntime::cwd().to_path_buf(), }; detect::check_python(&repo_root).map_err(|e| HmError::DslEngine(format!("{e:#}")))?; @@ -382,7 +382,7 @@ fn build_create_pipeline_request( /// `.hm/config.toml`, preserving any other keys already in the file. Creates /// the file (and `.hm/`) when absent. Used after registering a remoteless /// directory so later runs submit by the persisted slug without prompting. -fn persist_project_pipeline(dir: &std::path::Path, org: &str, slug: &str) -> Result<()> { +async fn persist_project_pipeline(dir: &std::path::Path, org: &str, slug: &str) -> Result<()> { let path = dir.join(".hm/config.toml"); let mut doc: toml::Table = std::fs::read_to_string(&path) .ok() @@ -398,6 +398,7 @@ fn persist_project_pipeline(dir: &std::path::Path, org: &str, slug: &str) -> Res } let serialized = toml::to_string_pretty(&doc).context("serializing .hm/config.toml")?; hm_common::fs::write_create_all(&path, serialized) + .await .with_context(|| format!("writing {}", path.display()))?; Ok(()) } @@ -467,7 +468,9 @@ async fn register_remoteless_pipeline( } }; - persist_project_pipeline(dir, org, &slug).context("saving .hm/config.toml")?; + persist_project_pipeline(dir, org, &slug) + .await + .context("saving .hm/config.toml")?; tracing::info!("registered pipeline '{slug}' — submitting build"); Ok(slug) } @@ -798,7 +801,8 @@ mod tests { "web", Some("https://example.test") )] - fn persist_project_pipeline_writes_config( + #[tokio::test] + async fn persist_project_pipeline_writes_config( #[case] preexisting: Option<&str>, #[case] slug: &str, #[case] expected_api_url: Option<&str>, @@ -809,7 +813,7 @@ mod tests { std::fs::write(dir.path().join(".hm/config.toml"), content).unwrap(); } - persist_project_pipeline(dir.path(), "acme", slug).unwrap(); + persist_project_pipeline(dir.path(), "acme", slug).await.unwrap(); let raw = std::fs::read_to_string(dir.path().join(".hm/config.toml")).unwrap(); let doc: toml::Table = toml::from_str(&raw).unwrap(); diff --git a/crates/hm/src/context.rs b/crates/hm/src/context.rs index eaf4c1e7..d64c7b27 100644 --- a/crates/hm/src/context.rs +++ b/crates/hm/src/context.rs @@ -28,7 +28,7 @@ impl RunContext { /// Returns an error if the config file is unreadable or malformed. pub fn from_cli(cli: &Cli) -> Result { let start_dir = std::env::current_dir().context("cannot determine current directory")?; - let project_root = hm_util::dirs::find_project_root(&start_dir); + let project_root = crate::project::find_project_root(&start_dir); let config = Config::load(project_root.as_deref())?; let output = OutputMode::Human { diff --git a/crates/hm/src/lib.rs b/crates/hm/src/lib.rs index 846b85da..2219d437 100644 --- a/crates/hm/src/lib.rs +++ b/crates/hm/src/lib.rs @@ -3,10 +3,9 @@ reason = "transitive dependency version conflicts in rand/windows-sys/thiserror chains; not fixable without upstream updates" )] // The `dirs` crate must NOT be added as a direct dependency of this -// crate. All directory resolution goes through `hm_util::dirs`, which -// owns the `dirs` dependency and provides both platform primitives and -// Harmont-specific discovery. Adding `dirs` here would bypass that -// single source of truth. +// crate. Platform-directory resolution goes through +// `hm_common::DirProvider`, which owns the `dirs` dependency. Adding +// `dirs` here would bypass that single source of truth. #[allow( clippy::print_stdout, @@ -24,5 +23,6 @@ pub use hm_config as config; /// `harmont_cli::creds_store` path. pub use hm_config::creds as creds_store; pub mod context; +pub mod project; pub mod error; pub(crate) mod signal; diff --git a/crates/hm/src/main.rs b/crates/hm/src/main.rs index dae1a46e..c33675f7 100644 --- a/crates/hm/src/main.rs +++ b/crates/hm/src/main.rs @@ -5,7 +5,7 @@ use anyhow::Context as _; use clap::Parser; -use hm_common::app_runtime::AppRuntime; +use hm_common::sys_runtime::SysRuntime; use tracing_subscriber::EnvFilter; use tracing_subscriber::Layer; use tracing_subscriber::filter::Targets; @@ -93,7 +93,7 @@ async fn main() { async fn run(args: Cli) -> Result { // Every command runs against the build toolchain (git + python3); resolve it // once up front and fail fast with a clear error if anything is missing. - AppRuntime::init().context("resolving the build toolchain")?; + SysRuntime::init().context("resolving the build toolchain")?; let command = args.command.clone(); let ctx = RunContext::from_cli(&args)?; diff --git a/crates/hm/src/project.rs b/crates/hm/src/project.rs new file mode 100644 index 00000000..8a580bf9 --- /dev/null +++ b/crates/hm/src/project.rs @@ -0,0 +1,54 @@ +//! The current Harmont project on disk. +//! +//! A project is any directory tree rooted at a directory that contains `.hm/`; +//! this module locates that root. + +#![allow(clippy::must_use_candidate)] + +use std::path::PathBuf; + +/// Walk up from `start` looking for a directory containing `.hm/`. +/// Returns the project root (the directory *containing* `.hm/`), +/// or `None` if the filesystem root is reached without finding one. +pub fn find_project_root(start: &std::path::Path) -> Option { + let mut current = start; + loop { + if current.join(".hm").is_dir() { + return Some(current.to_path_buf()); + } + current = current.parent()?; + } +} + +#[cfg(test)] +#[allow(clippy::unwrap_used, reason = "test setup and assertions")] +mod tests { + use rstest::rstest; + + use super::*; + + #[rstest] + fn find_project_root_at_current_dir() { + let tmp = tempfile::tempdir().unwrap(); + std::fs::create_dir(tmp.path().join(".hm")).unwrap(); + let found = find_project_root(tmp.path()); + assert_eq!(found, Some(tmp.path().to_path_buf())); + } + + #[rstest] + fn find_project_root_walks_up() { + let tmp = tempfile::tempdir().unwrap(); + std::fs::create_dir(tmp.path().join(".hm")).unwrap(); + let nested = tmp.path().join("src").join("deep"); + std::fs::create_dir_all(&nested).unwrap(); + let found = find_project_root(&nested); + assert_eq!(found, Some(tmp.path().to_path_buf())); + } + + #[rstest] + fn find_project_root_returns_none_when_missing() { + let tmp = tempfile::tempdir().unwrap(); + let found = find_project_root(tmp.path()); + assert_eq!(found, None); + } +} diff --git a/crates/hm/tests/config_layered.rs b/crates/hm/tests/config_layered.rs index 074e27ae..c4d7076a 100644 --- a/crates/hm/tests/config_layered.rs +++ b/crates/hm/tests/config_layered.rs @@ -85,7 +85,7 @@ fn load_resolves_project_root() { ) .unwrap(); - let found = hm_util::dirs::find_project_root(project_dir.path()); + let found = harmont_cli::project::find_project_root(project_dir.path()); assert_eq!(found, Some(project_dir.path().to_path_buf())); let config_path = harmont_cli::config::Config::project_config_path(project_dir.path());