From 3868823ba55f3957407c2786f9cdaa8f4d541998 Mon Sep 17 00:00:00 2001 From: Denys Zadorozhnyi Date: Tue, 4 Aug 2026 11:29:38 +0300 Subject: [PATCH 01/21] fix(session): unique the filesystem package cache by build inputs The filesystem package cache at target/miden/packages persisted across builds and keyed files by package name alone. When build inputs changed (dependency set, versions, pins, or the compiler itself), leftover .masp files from older builds could satisfy the SDK proc-macro lookups through MIDENC_PACKAGE_CACHE and bake stale FPI procedure roots into generated code, failing only at transaction execution (#1302, surfaced by #1300). Derive the cache path as target/miden/packages/, where the fingerprint hashes the compiler version and revision, the build-relevant options, and the project's recursive manifest closure (including resolved dependency schemes and preassembled package contents). Session memoizes the fingerprint, so the package registry and the MIDENC_PACKAGE_CACHE variable handed to nested cargo builds keep agreeing on one derivation. Registry construction now also prunes stale midenc-owned cache entries (fingerprint directories and legacy flat .masp files) next to the current directory. The pruning is load-bearing: the macros track their package reads with include_bytes! dummies, so a surviving stale directory would keep cargo reusing a stale macro expansion. Deleting it forces re-expansion against the fresh cache. The fingerprint intentionally excludes Rust sources and lockfiles: every run rewrites each resolved package into the cache before its consumers expand, and content changes at a stable path already invalidate consumers through the include_bytes! tracking. The cargo-miden integration tests that asserted the flat packages/.masp layout now locate the build's single fingerprint directory instead. --- Cargo.lock | 1 + midenc-compile/src/pipeline/frontends/masm.rs | 3 +- midenc-compile/src/pipeline/frontends/rust.rs | 8 +- midenc-session/Cargo.toml | 3 + midenc-session/src/lib.rs | 36 +- midenc-session/src/package_cache.rs | 465 ++++++++++++++++++ midenc-session/src/registry.rs | 135 +++++ tools/cargo-miden/tests/masm_dependency.rs | 8 +- .../tests/p2id_cargo_miden_build.rs | 25 +- tools/cargo-miden/tests/utils.rs | 26 + 10 files changed, 684 insertions(+), 26 deletions(-) create mode 100644 midenc-session/src/package_cache.rs diff --git a/Cargo.lock b/Cargo.lock index 5808a0904..123087c10 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3983,6 +3983,7 @@ dependencies = [ "parking_lot", "rustc-hash", "smallvec", + "tempfile", "termcolor", ] diff --git a/midenc-compile/src/pipeline/frontends/masm.rs b/midenc-compile/src/pipeline/frontends/masm.rs index 0ff4595d2..dd674320f 100644 --- a/midenc-compile/src/pipeline/frontends/masm.rs +++ b/midenc-compile/src/pipeline/frontends/masm.rs @@ -267,7 +267,8 @@ impl MasmProjectFrontend { /// `load_target_sources`, and so this frontend, only afterwards (`:428-434`). For an /// executable root, `assemble_interruptible` also assembles the project's library target /// before either (`:303-320`). So a lint-only run builds the dependency closure and writes - /// `.masp` files into `target/miden/packages` that it previously never produced. + /// `.masp` files into `target/miden/packages/` that it previously never + /// produced. /// /// That is an accepted consequence of routing project compilation through the assembler, /// not an oversight. The frontend is reachable only as an assembler callback, so any goal diff --git a/midenc-compile/src/pipeline/frontends/rust.rs b/midenc-compile/src/pipeline/frontends/rust.rs index 6fd858603..79cddf351 100644 --- a/midenc-compile/src/pipeline/frontends/rust.rs +++ b/midenc-compile/src/pipeline/frontends/rust.rs @@ -1607,9 +1607,11 @@ pub(crate) mod manifest { /// /// `MIDENC_PACKAGE_CACHE` is the whole reason `filesystem_cache_dir` is threaded down from /// the entry point: it tells the nested `midenc` invocations where to publish the packages - /// they compile and where to look for the ones their own dependencies already produced. - /// Absent a cache directory the variable is *unset* rather than set to an empty path, which - /// is what makes the nested build fall back to its own default. + /// they compile and where to look for the ones their own dependencies already produced. The + /// directory is fingerprinted by the root build's compiler, options, and manifest closure so + /// every participant in that build uses the same isolated cache. Absent a cache directory the + /// variable is *unset* rather than set to an empty path, which is what makes the nested build + /// fall back to its own default. /// /// Named — rather than left inline where it was — so that this can be asserted without /// spawning `cargo -Z build-std` against the SDK. diff --git a/midenc-session/Cargo.toml b/midenc-session/Cargo.toml index d216a50a0..ce0c9c6ad 100644 --- a/midenc-session/Cargo.toml +++ b/midenc-session/Cargo.toml @@ -51,3 +51,6 @@ smallvec.workspace = true parking_lot = { workspace = true, optional = true } termcolor = { version = "1.4.1", optional = true } thiserror.workspace = true + +[dev-dependencies] +tempfile.workspace = true diff --git a/midenc-session/src/lib.rs b/midenc-session/src/lib.rs index d333955fd..943965ee6 100644 --- a/midenc-session/src/lib.rs +++ b/midenc-session/src/lib.rs @@ -27,6 +27,8 @@ mod inputs; mod libs; mod options; mod outputs; +#[cfg(feature = "std")] +mod package_cache; pub mod path; pub mod registry; #[cfg(feature = "std")] @@ -79,6 +81,9 @@ pub struct Session { /// Statistics gathered from the current compiler session #[cfg(feature = "std")] pub statistics: Statistics, + /// The build-input fingerprint used to isolate this session's package cache + #[cfg(feature = "std")] + package_cache_fingerprint: std::sync::OnceLock, } impl fmt::Debug for Session { @@ -311,6 +316,8 @@ impl Session { output_files, #[cfg(feature = "std")] statistics: Default::default(), + #[cfg(feature = "std")] + package_cache_fingerprint: Default::default(), } } @@ -362,10 +369,12 @@ impl Session { /// Where compiled dependency packages of this session's project are published and looked for. /// /// `None` unless this session's input is a project locator: the cache lives under the - /// project's own `target/` directory, and a session compiling a standalone source file has no - /// project directory to put one under. Both readers — this session's package registry and the - /// nested `cargo` builds a Rust project's dependencies run through — must agree on the answer, - /// which is why there is one derivation of it. + /// project's own `target/miden/packages//` directory, and a session compiling a + /// standalone source file has no project directory to put one under. The fingerprint covers + /// the compiler identity, relevant build options, and the project's manifest closure. Both + /// readers — this session's package registry and the nested `cargo` builds a Rust project's + /// dependencies run through — must agree on the answer, which is why there is one derivation + /// of it. /// /// Derived from the input locator rather than from a loaded manifest, which is what /// [`Session::new`] no longer has. That is also a repair: the manifest path was previously @@ -385,7 +394,24 @@ impl Session { let project_dir = project_dir.canonicalize().unwrap_or_else(|_| project_dir.to_path_buf()); #[cfg(not(feature = "std"))] let project_dir = project_dir.to_path_buf(); - Some(project_dir.join("target").join("miden").join("packages")) + let package_cache_dir = project_dir.join("target").join("miden").join("packages"); + #[cfg(feature = "std")] + { + let fingerprint = self.package_cache_fingerprint.get_or_init(|| { + package_cache::fingerprint( + &self.options, + &project_dir, + self.source_manager.as_ref(), + MIDENC_BUILD_VERSION, + MIDENC_BUILD_REV, + ) + }); + Some(package_cache_dir.join(fingerprint)) + } + #[cfg(not(feature = "std"))] + { + Some(package_cache_dir) + } } /// Get the [OutputFile] to write the assembled MAST output to diff --git a/midenc-session/src/package_cache.rs b/midenc-session/src/package_cache.rs new file mode 100644 index 000000000..b1ff527ab --- /dev/null +++ b/midenc-session/src/package_cache.rs @@ -0,0 +1,465 @@ +//! Build-input fingerprints for the filesystem package cache. + +use alloc::{ + format, + string::{String, ToString}, + vec::Vec, +}; +use std::{ + collections::BTreeSet, + path::{Path, PathBuf}, +}; + +use miden_core::crypto::hash::Blake3_256; +use miden_project::{Dependency, DependencyVersionScheme, Project}; + +use crate::{DebugInfo, LinkLibrary, OptLevel, Options, SourceManager}; + +/// Computes the filesystem package cache fingerprint for a project build. +/// +/// Failures while reading or loading manifests are recorded as markers instead of being +/// returned. The normal project-loading path will diagnose those failures later with its full +/// context. +pub(crate) fn fingerprint( + options: &Options, + project_dir: &Path, + source_manager: &dyn SourceManager, + compiler_version: &str, + compiler_revision: &str, +) -> String { + let mut transcript = Transcript::new(); + transcript.field("compiler.version", compiler_version.as_bytes()); + transcript.field("compiler.revision", compiler_revision.as_bytes()); + record_options(&mut transcript, options); + + let mut manifests = ManifestClosure::new(&mut transcript, source_manager); + manifests.visit_project(project_dir, None); + + let digest = Blake3_256::hash(transcript.as_bytes()); + let mut fingerprint = String::with_capacity(16); + for byte in &digest.as_bytes()[..8] { + use core::fmt::Write; + write!(&mut fingerprint, "{byte:02x}").expect("writing to a string cannot fail"); + } + fingerprint +} + +/// A length-prefixed, domain-separated byte transcript. +struct Transcript { + bytes: Vec, +} + +impl Transcript { + /// Creates an empty package-cache fingerprint transcript. + fn new() -> Self { + let mut transcript = Self { bytes: Vec::new() }; + transcript.field("domain", b"midenc-package-cache-v1"); + transcript + } + + /// Appends one named field to this transcript. + fn field(&mut self, name: &str, value: &[u8]) { + self.bytes.extend_from_slice(&(name.len() as u64).to_le_bytes()); + self.bytes.extend_from_slice(name.as_bytes()); + self.bytes.extend_from_slice(&(value.len() as u64).to_le_bytes()); + self.bytes.extend_from_slice(value); + } + + /// Appends an optional named field to this transcript. + fn optional_field(&mut self, name: &str, value: Option<&str>) { + match value { + Some(value) => { + self.field(&format!("{name}.state"), b"present"); + self.field(name, value.as_bytes()); + } + None => self.field(&format!("{name}.state"), b"missing"), + } + } + + /// Returns the encoded transcript. + fn as_bytes(&self) -> &[u8] { + &self.bytes + } +} + +/// Records the build configuration which can affect package identity or selection. +fn record_options(transcript: &mut Transcript, options: &Options) { + transcript.field("options.profile", options.profile.as_bytes()); + transcript.field("options.optimize", opt_level_name(options.optimize).as_bytes()); + transcript.field("options.debug", debug_info_name(options.debug).as_bytes()); + transcript.optional_field("options.target", options.target.as_deref()); + + let target_type = options.target_type.map(|target_type| target_type.to_string()); + transcript.optional_field("options.target_type", target_type.as_deref()); + + let mut packages = options.packages.clone(); + packages.sort(); + transcript.field("options.packages.count", &(packages.len() as u64).to_le_bytes()); + for package in packages { + transcript.field("options.package", package.as_bytes()); + } + + transcript.field("options.workspace", &[u8::from(options.workspace)]); + transcript.optional_field("options.rustflags", options.rustflags.as_deref()); + transcript.optional_field("options.toolchain", options.toolchain.as_deref()); + + let mut link_libraries = options + .link_libraries + .iter() + .map(|library| link_library_input(library, options)) + .collect::>(); + link_libraries.sort(); + transcript.field("options.link_libraries.count", &(link_libraries.len() as u64).to_le_bytes()); + for (name, version) in link_libraries { + transcript.field("options.link_library.name", name.as_bytes()); + transcript.optional_field("options.link_library.version", version.as_deref()); + } + + let sysroot = options.sysroot.as_deref().map(path_string); + transcript.optional_field("options.sysroot", sysroot.as_deref()); +} + +/// Returns the stable transcript name for an optimization level. +fn opt_level_name(level: OptLevel) -> &'static str { + match level { + OptLevel::None => "none", + OptLevel::Basic => "basic", + OptLevel::Balanced => "balanced", + OptLevel::Max => "max", + OptLevel::Size => "size", + OptLevel::SizeMin => "size-min", + } +} + +/// Returns the stable transcript name for a debug-information level. +fn debug_info_name(level: DebugInfo) -> &'static str { + match level { + DebugInfo::None => "none", + DebugInfo::Line => "line", + DebugInfo::Full => "full", + } +} + +/// Resolves the name and package version of a requested link library. +fn link_library_input(library: &LinkLibrary, options: &Options) -> (String, Option) { + let version = library.load(options).ok().map(|package| package.version.to_string()); + (library.name.to_string(), version) +} + +/// Walks and records the manifest closure of one project. +struct ManifestClosure<'a> { + transcript: &'a mut Transcript, + source_manager: &'a dyn SourceManager, + visited_projects: BTreeSet, + visited_packages: BTreeSet, +} + +impl<'a> ManifestClosure<'a> { + /// Creates an empty manifest-closure walk. + fn new(transcript: &'a mut Transcript, source_manager: &'a dyn SourceManager) -> Self { + Self { + transcript, + source_manager, + visited_projects: BTreeSet::new(), + visited_packages: BTreeSet::new(), + } + } + + /// Records a project and recursively visits its local dependencies. + fn visit_project(&mut self, locator: &Path, expected_name: Option<&str>) { + let loaded = match expected_name { + Some(name) => Project::load_project_reference(name, locator, self.source_manager), + None => Project::load(locator, self.source_manager), + }; + let project_dir = loaded + .as_ref() + .ok() + .and_then(|project| project.package().manifest_path().map(Path::to_path_buf)) + .and_then(|manifest| manifest.parent().map(Path::to_path_buf)) + .unwrap_or_else(|| project_dir(locator)); + let project_key = canonical_or_original(&project_dir); + if !self.visited_projects.insert(project_key) { + return; + } + + self.transcript.field("project", b"begin"); + self.record_manifest(&project_dir.join("miden-project.toml")); + self.record_manifest(&project_dir.join("Cargo.toml")); + + let Ok(project) = loaded else { + self.transcript.field("project.load", b"failed"); + self.transcript.field("project", b"end"); + return; + }; + self.transcript.field("project.load", b"succeeded"); + + let package = project.package(); + let manifest_dir = + package.manifest_path().and_then(Path::parent).unwrap_or(project_dir.as_path()); + let workspace_root = match &project { + Project::WorkspacePackage { workspace, .. } => workspace.workspace_root(), + Project::Package(_) => None, + }; + + let mut dependencies = package.dependencies().iter().collect::>(); + dependencies.sort_by_cached_key(|dependency| dependency_sort_key(dependency)); + self.transcript + .field("project.dependencies.count", &(dependencies.len() as u64).to_le_bytes()); + for dependency in dependencies { + self.visit_dependency(dependency, manifest_dir, workspace_root); + } + self.transcript.field("project", b"end"); + } + + /// Records one project manifest, including an explicit marker when it cannot be read. + fn record_manifest(&mut self, path: &Path) { + let name = path.file_name().and_then(|name| name.to_str()).unwrap_or("manifest"); + self.transcript.field("manifest.name", name.as_bytes()); + match std::fs::read(path) { + Ok(bytes) => { + self.transcript.field("manifest.state", b"present"); + self.transcript.field("manifest.bytes", &bytes); + } + Err(_) => self.transcript.field("manifest.state", b"missing"), + } + } + + /// Records one dependency and follows it when it names a local project or package file. + fn visit_dependency( + &mut self, + dependency: &Dependency, + manifest_dir: &Path, + workspace_root: Option<&Path>, + ) { + self.transcript.field("dependency", b"begin"); + self.transcript.field("dependency.name", dependency.name().as_bytes()); + self.transcript + .field("dependency.scheme", dependency_scheme_key(dependency).as_bytes()); + + match dependency.scheme() { + DependencyVersionScheme::Registry(_) => {} + DependencyVersionScheme::Path { path, .. } => { + self.visit_path_dependency(dependency, manifest_dir, path.inner()); + } + DependencyVersionScheme::WorkspacePath { path, .. } => { + if let Some(workspace_root) = workspace_root { + self.visit_path_dependency(dependency, workspace_root, path.inner()); + } else { + self.transcript.field("dependency.path", b"unresolved-workspace"); + } + } + DependencyVersionScheme::Workspace { member, .. } => { + if let Some(workspace_root) = workspace_root { + self.visit_path_dependency(dependency, workspace_root, member.inner()); + } else { + self.transcript.field("dependency.path", b"unresolved-workspace"); + } + } + DependencyVersionScheme::Git { repo, revision, .. } => { + self.transcript.field("dependency.git.repo", repo.inner().as_bytes()); + self.transcript + .field("dependency.git.revision", revision.inner().to_string().as_bytes()); + } + } + self.transcript.field("dependency", b"end"); + } + + /// Resolves and records a filesystem dependency using the manifest scheme's base directory. + fn visit_path_dependency( + &mut self, + dependency: &Dependency, + base_dir: &Path, + uri: &miden_project::Uri, + ) { + if uri.scheme().is_some_and(|scheme| scheme != "file") { + self.transcript.field("dependency.path", b"unsupported-uri"); + return; + } + + let relative = Path::new(uri.path()); + let path = if relative.is_absolute() { + relative.to_path_buf() + } else { + base_dir.join(relative) + }; + if path.extension().is_some_and(|extension| extension == "masp") { + self.record_package_file(&path); + } else { + self.visit_project(&path, Some(dependency.name().as_ref())); + } + } + + /// Records the content hash of a preassembled package dependency. + fn record_package_file(&mut self, path: &Path) { + let key = canonical_or_original(path); + if !self.visited_packages.insert(key) { + return; + } + + self.transcript.field("package.file", b"begin"); + match std::fs::read(path) { + Ok(bytes) => { + self.transcript.field("package.file.state", b"present"); + let digest = Blake3_256::hash(&bytes); + self.transcript.field("package.file.digest", digest.as_bytes()); + } + Err(_) => self.transcript.field("package.file.state", b"missing"), + } + self.transcript.field("package.file", b"end"); + } +} + +/// Returns a deterministic key for dependency traversal order. +fn dependency_sort_key(dependency: &Dependency) -> (String, String) { + (dependency.name().to_string(), dependency_scheme_key(dependency)) +} + +/// Returns a stable textual projection of a dependency's resolved scheme. +fn dependency_scheme_key(dependency: &Dependency) -> String { + match dependency.scheme() { + DependencyVersionScheme::Registry(requirement) => format!("registry:{requirement}"), + DependencyVersionScheme::Path { path, version } => { + format!("path:{}:{}", path.inner().as_str(), optional_display(version.as_ref())) + } + DependencyVersionScheme::WorkspacePath { path, version } => format!( + "workspace-path:{}:{}", + path.inner().as_str(), + optional_display(version.as_ref()) + ), + DependencyVersionScheme::Workspace { member, version } => { + format!("workspace:{}:{}", member.inner().as_str(), optional_display(version.as_ref())) + } + DependencyVersionScheme::Git { + repo, + revision, + version, + } => format!( + "git:{}:{}:{}", + repo.inner().as_str(), + revision.inner(), + optional_display(version.as_ref().map(|version| version.inner())) + ), + } +} + +/// Formats an optional display value without conflating absence with an empty value. +fn optional_display(value: Option<&impl core::fmt::Display>) -> String { + value.map(ToString::to_string).unwrap_or_else(|| "".into()) +} + +/// Returns the directory whose sibling project manifests describe a locator. +fn project_dir(locator: &Path) -> PathBuf { + if locator.file_name().is_some_and(|name| { + name.eq_ignore_ascii_case("miden-project.toml") || name.eq_ignore_ascii_case("Cargo.toml") + }) { + locator.parent().map(Path::to_path_buf).unwrap_or_else(|| locator.to_path_buf()) + } else { + locator.to_path_buf() + } +} + +/// Canonicalizes a path for cycle detection, retaining the original spelling on failure. +fn canonical_or_original(path: &Path) -> PathBuf { + path.canonicalize().unwrap_or_else(|_| path.to_path_buf()) +} + +/// Converts a path into the stable string representation used in the transcript. +fn path_string(path: &Path) -> String { + path.to_string_lossy().into_owned() +} + +#[cfg(test)] +mod tests { + use miden_debug_types::DefaultSourceManager; + use tempfile::TempDir; + + use super::*; + + /// Writes a minimal Miden project and Cargo manifest to `dir`. + fn write_project(dir: &Path, name: &str, dependencies: &str) { + std::fs::create_dir_all(dir).unwrap(); + std::fs::write( + dir.join("miden-project.toml"), + format!( + "[package]\nname = \"{name}\"\nversion = \"1.0.0\"\n\n[lib]\npath = \ + \"src/lib.rs\"\n{dependencies}" + ), + ) + .unwrap(); + std::fs::write( + dir.join("Cargo.toml"), + format!("[package]\nname = \"{name}\"\nversion = \"1.0.0\"\n"), + ) + .unwrap(); + } + + /// Computes a test fingerprint with a fresh source manager. + fn test_fingerprint(options: &Options, project_dir: &Path, version: &str, rev: &str) -> String { + fingerprint(options, project_dir, &DefaultSourceManager::default(), version, rev) + } + + #[test] + fn fingerprint_is_stable_for_unchanged_inputs() { + let temp = TempDir::new().unwrap(); + write_project(temp.path(), "root", ""); + let options = Options::default(); + + let first = test_fingerprint(&options, temp.path(), "1.2.3", "abc123"); + let second = test_fingerprint(&options, temp.path(), "1.2.3", "abc123"); + + assert_eq!(first, second); + assert_eq!(first.len(), 16); + assert!(first.bytes().all(|byte| byte.is_ascii_digit() || (b'a'..=b'f').contains(&byte))); + } + + #[test] + fn fingerprint_changes_with_manifest_closure() { + let temp = TempDir::new().unwrap(); + let root = temp.path().join("root"); + let dependency = temp.path().join("dependency"); + write_project( + &root, + "root", + "\n[dependencies]\ndependency = { path = \"../dependency\" }\n", + ); + write_project(&dependency, "dependency", ""); + let options = Options::default(); + let before = test_fingerprint(&options, &root, "1.2.3", "abc123"); + + std::fs::write( + dependency.join("Cargo.toml"), + "[package]\nname = \"dependency\"\nversion = \"2.0.0\"\n", + ) + .unwrap(); + let after = test_fingerprint(&options, &root, "1.2.3", "abc123"); + + assert_ne!(before, after); + } + + #[test] + fn fingerprint_changes_with_build_options() { + let temp = TempDir::new().unwrap(); + write_project(temp.path(), "root", ""); + let options = Options::default(); + let baseline = test_fingerprint(&options, temp.path(), "1.2.3", "abc123"); + + let mut profile = options.clone(); + profile.profile = "release".into(); + assert_ne!(baseline, test_fingerprint(&profile, temp.path(), "1.2.3", "abc123")); + + let mut optimized = options.clone(); + optimized.optimize = OptLevel::Max; + assert_ne!(baseline, test_fingerprint(&optimized, temp.path(), "1.2.3", "abc123")); + } + + #[test] + fn fingerprint_changes_with_compiler_identity() { + let temp = TempDir::new().unwrap(); + write_project(temp.path(), "root", ""); + let options = Options::default(); + let baseline = test_fingerprint(&options, temp.path(), "1.2.3", "abc123"); + + assert_ne!(baseline, test_fingerprint(&options, temp.path(), "1.2.4", "abc123")); + assert_ne!(baseline, test_fingerprint(&options, temp.path(), "1.2.3", "def456")); + } +} diff --git a/midenc-session/src/registry.rs b/midenc-session/src/registry.rs index 649d06ab1..afe94e499 100644 --- a/midenc-session/src/registry.rs +++ b/midenc-session/src/registry.rs @@ -72,6 +72,10 @@ impl HybridPackageRegistry { ) -> Result { use alloc::string::ToString; + if let Some(filesystem_cache) = filesystem_cache.as_deref() { + prepare_filesystem_cache(filesystem_cache); + } + // Load system libraries let mut registry = if options.sysroot.is_some() { Self::from_local_registry(options)? @@ -217,6 +221,95 @@ impl HybridPackageRegistry { } } +/// Creates the current cache directory and removes stale cache entries owned by `midenc`. +/// +/// Cleanup is best-effort because an inability to prune an old build must not obscure the +/// diagnostic from the current build. Package writes still report their own failures normally. +#[cfg(any(test, feature = "std"))] +fn prepare_filesystem_cache(filesystem_cache: &std::path::Path) { + if let Err(err) = std::fs::create_dir_all(filesystem_cache) { + log::debug!( + target: "package-registry", + "failed to create filesystem package cache '{}': {err}", + filesystem_cache.display() + ); + } + + let Some(parent) = filesystem_cache.parent() else { + return; + }; + let entries = match std::fs::read_dir(parent) { + Ok(entries) => entries, + Err(err) => { + log::debug!( + target: "package-registry", + "failed to inspect filesystem package cache '{}': {err}", + parent.display() + ); + return; + } + }; + + for entry in entries { + let entry = match entry { + Ok(entry) => entry, + Err(err) => { + log::debug!( + target: "package-registry", + "failed to inspect an entry in filesystem package cache '{}': {err}", + parent.display() + ); + continue; + } + }; + let path = entry.path(); + if path == filesystem_cache { + continue; + } + let file_type = match entry.file_type() { + Ok(file_type) => file_type, + Err(err) => { + log::debug!( + target: "package-registry", + "failed to inspect filesystem package cache entry '{}': {err}", + path.display() + ); + continue; + } + }; + + let is_stale_fingerprint = + file_type.is_dir() && is_package_cache_fingerprint(&entry.file_name()); + let is_legacy_package = file_type.is_file() + && path.extension().and_then(|extension| extension.to_str()) == Some("masp"); + if !is_stale_fingerprint && !is_legacy_package { + continue; + } + + let result = if is_stale_fingerprint { + std::fs::remove_dir_all(&path) + } else { + std::fs::remove_file(&path) + }; + if let Err(err) = result { + log::debug!( + target: "package-registry", + "failed to prune stale filesystem package cache entry '{}': {err}", + path.display() + ); + } + } +} + +/// Returns true when `name` has the cache fingerprint format owned by `midenc`. +#[cfg(any(test, feature = "std"))] +fn is_package_cache_fingerprint(name: &std::ffi::OsStr) -> bool { + name.to_str().is_some_and(|name| { + name.len() == 16 + && name.bytes().all(|byte| byte.is_ascii_digit() || (b'a'..=b'f').contains(&byte)) + }) +} + impl HybridPackageRegistry { fn insert_record(&mut self, id: PackageId, record: PackageRecord) { self.packages @@ -288,3 +381,45 @@ impl PackageStore for HybridPackageRegistry { self.install_if_missing(package).map_err(Report::from) } } + +#[cfg(test)] +mod tests { + use tempfile::TempDir; + + use super::*; + + #[test] + fn creating_a_filesystem_cache_prunes_only_stale_owned_entries() { + let temp = TempDir::new().unwrap(); + let parent = temp.path().join("packages"); + let current = parent.join("fedcba9876543210"); + let stale = parent.join("0123456789abcdef"); + let unrelated_directory = parent.join("not-a-midenc-cache"); + let uppercase_directory = parent.join("ABCDEF0123456789"); + let legacy_package = parent.join("legacy.masp"); + let unrelated_file = parent.join("keep.txt"); + + for directory in [¤t, &stale, &unrelated_directory, &uppercase_directory] { + std::fs::create_dir_all(directory).unwrap(); + } + let current_marker = current.join("keep"); + std::fs::write(¤t_marker, b"current").unwrap(); + std::fs::write(stale.join("old.masp"), b"stale").unwrap(); + std::fs::write(&legacy_package, b"legacy").unwrap(); + std::fs::write(&unrelated_file, b"unrelated").unwrap(); + + let registry = HybridPackageRegistry::new_with_filesystem_cache( + &crate::Options::default(), + Some(current.clone()), + ) + .unwrap(); + + assert_eq!(registry.filesystem_cache_dir(), Some(current.as_path())); + assert!(current_marker.exists(), "the current cache must remain intact"); + assert!(!stale.exists(), "a stale fingerprint directory must be removed"); + assert!(!legacy_package.exists(), "a legacy flat package must be removed"); + assert!(unrelated_directory.exists(), "unowned directories must be retained"); + assert!(uppercase_directory.exists(), "non-lowercase directories must be retained"); + assert!(unrelated_file.exists(), "unowned files must be retained"); + } +} diff --git a/tools/cargo-miden/tests/masm_dependency.rs b/tools/cargo-miden/tests/masm_dependency.rs index f3a6a248b..2e74f23fa 100644 --- a/tools/cargo-miden/tests/masm_dependency.rs +++ b/tools/cargo-miden/tests/masm_dependency.rs @@ -172,11 +172,9 @@ fn build_rust_project_with_masm_path_dependency() { // The root's own package building is not by itself evidence that the dependency was built // through a source provider: it would also hold if the dependency had been resolved from a // registry, or skipped. A materialized `.masp` for the dependency is produced only by - // assembling it from its Miden Assembly sources. - let dependency_package = project_path - .join("target") - .join("miden") - .join("packages") + // assembling it from its Miden Assembly sources. The package cache is uniqued by the build + // inputs, so the package lives inside the build's fingerprint directory. + let dependency_package = crate::utils::package_cache_fingerprint_dir(&project_path) .join(format!("{dependency_name}.masp")); assert!( dependency_package.exists(), diff --git a/tools/cargo-miden/tests/p2id_cargo_miden_build.rs b/tools/cargo-miden/tests/p2id_cargo_miden_build.rs index c840dee4c..08c1c5c03 100644 --- a/tools/cargo-miden/tests/p2id_cargo_miden_build.rs +++ b/tools/cargo-miden/tests/p2id_cargo_miden_build.rs @@ -8,8 +8,9 @@ use crate::utils::{current_dir_lock, workspace_root}; /// /// The `p2id-note` example depends on the `basic-wallet` example as a Miden dependency. When /// `cargo miden build` compiles `p2id-note`, it compiles `basic-wallet` as a dependency. The -/// resulting dependency package must be materialized to `basic-wallet/target/miden/release` rather -/// than only living in the in-memory package registry. +/// resulting dependency package must be materialized under p2id-note's +/// `target/miden/packages//` package cache rather than only living in the +/// in-memory package registry. #[test] fn p2id_build_materializes_basic_wallet_dependency() { let _cwd_lock = current_dir_lock(); @@ -29,19 +30,16 @@ fn p2id_build_materializes_basic_wallet_dependency() { let examples = workspace_root().join("examples"); let p2id_note_dir = examples.join("p2id-note"); - // The materialized dependency package we expect `cargo miden build` to produce on disk. - let dep_release_dir = p2id_note_dir.join("target").join("miden").join("packages"); - let dep_package = dep_release_dir.join("basic-wallet.masp"); + // The package cache directory we expect `cargo miden build` to populate on disk. The cache + // is uniqued by the build inputs, so the dependency package lands in a fingerprint + // subdirectory whose name is not known up front. + let package_cache_dir = p2id_note_dir.join("target").join("miden").join("packages"); // Make sure the basic-wallet dependency package is not already materialized on disk, so that we // can attribute its presence after the build to the p2id-note build alone. - if dep_release_dir.exists() { - fs::remove_dir_all(&dep_release_dir).unwrap(); + if package_cache_dir.exists() { + fs::remove_dir_all(&package_cache_dir).unwrap(); } - assert!( - !dep_package.exists(), - "basic-wallet dependency package should not be materialized before the build" - ); // Build the p2id-note project, which pulls in basic-wallet as a Miden dependency. let restore_dir = env::current_dir().unwrap(); @@ -61,7 +59,10 @@ fn p2id_build_materializes_basic_wallet_dependency() { .unwrap_build_output(); assert_eq!(output.len(), 1, "expected a single p2id-note package artifact, got {output:?}"); - // The build must have materialized the basic-wallet dependency package on disk. + // The build must have materialized the basic-wallet dependency package on disk, inside the + // build's single fingerprint directory. + let dep_package = + crate::utils::package_cache_fingerprint_dir(&p2id_note_dir).join("basic-wallet.masp"); assert!( dep_package.exists(), "expected basic-wallet dependency package to be materialized at {}", diff --git a/tools/cargo-miden/tests/utils.rs b/tools/cargo-miden/tests/utils.rs index bf0182841..9ee71c0ac 100644 --- a/tools/cargo-miden/tests/utils.rs +++ b/tools/cargo-miden/tests/utils.rs @@ -44,6 +44,32 @@ pub(crate) fn current_dir_lock() -> CurrentDirGuard { } } +/// Returns the single build-fingerprint directory inside a project's package cache. +/// +/// The package cache under `target/miden/packages` is uniqued by the build inputs, so a build +/// materializes its dependency packages inside one fingerprint subdirectory whose name is not +/// known up front. +pub(crate) fn package_cache_fingerprint_dir(project_dir: &Path) -> PathBuf { + let package_cache_dir = project_dir.join("target").join("miden").join("packages"); + let fingerprint_dirs = fs::read_dir(&package_cache_dir) + .unwrap_or_else(|err| { + panic!( + "expected the package cache directory '{}' to exist after the build: {err}", + package_cache_dir.display() + ) + }) + .map(|entry| entry.unwrap().path()) + .filter(|path| path.is_dir()) + .collect::>(); + assert_eq!( + fingerprint_dirs.len(), + 1, + "expected exactly one fingerprint directory in '{}', got {fingerprint_dirs:?}", + package_cache_dir.display() + ); + fingerprint_dirs.into_iter().next().unwrap() +} + pub(crate) fn project_template_arg(template: &str) -> String { let template = template.trim_start_matches("--"); let templates_path = match env::var("TEST_LOCAL_TEMPLATES_PATH") { From 2cf826b694df3c20b686991cf3148e46db8bc594 Mon Sep 17 00:00:00 2001 From: Denys Zadorozhnyi Date: Tue, 4 Aug 2026 16:19:29 +0300 Subject: [PATCH 02/21] fix(session): protect live package caches from pruning and close fingerprint gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A pre-submit review of the fingerprinted package cache found one race and several hardening gaps. The prune deleted every sibling fingerprint directory unconditionally, so two concurrent builds of one project with different inputs (debug and release builds of the same checked-in example, as the test suite itself arranges) could delete each other's live cache between a package write and the consuming macro's read. Each build now holds an exclusive advisory lock on a .build-lock file inside its fingerprint directory for the registry's lifetime, and the prune deletes a sibling only when its lock is free or absent. A live directory is skipped; a lock that cannot be verified is left in place with a warning, since deleting an unverifiable cache risks reviving the stale-expansion bug the prune exists to prevent. Pruning is also refused entirely when the target path is not fingerprint-named, so external callers of the public constructor cannot sweep an arbitrary parent directory, and failed removals of owned entries are logged at warn with their consequence. The fingerprint gains two inputs that escaped it: the inherited RUSTFLAGS environment (composed into every nested cargo build) and the containing workspace's root manifests (member manifests do not change when workspace-level fields do). Moved git branches remain outside the fingerprint by design, now documented. The fingerprint format is defined once and shared by the producer, the prune recognizer, and their tests; record_options destructures Options exhaustively so a future field must be explicitly classified as fingerprinted or ignored; link libraries contribute their declared identity instead of a redundant package load; and the design rationale that previously lived outside the tree is captured in module and function docs. The FPI macro diagnostic for the cache-lookup branch now names the searched MIDENC_PACKAGE_CACHE directory and the expected package file names instead of an empty candidate list and a profile-directory hint that branch never consults. New tests pin the liveness behavior (a locked sibling survives, an unlocked one is pruned), the misuse guard, the fingerprint walk's cycle guard and degradation markers, and — end to end — the invalidation contract itself: rebuilding after a dependency source change keeps the same cache path but replaces the FPI procedure root baked into the consumer's assembly. --- CHANGELOG.md | 9 + midenc-session/src/lib.rs | 18 +- midenc-session/src/package_cache.rs | 343 +++++++++++++++--- midenc-session/src/registry.rs | 253 +++++++++++-- sdk/base-macros/src/fpi.rs | 71 +++- tests/integration/src/sdk/mod.rs | 219 ++++++++++- tools/cargo-miden/tests/masm_dependency.rs | 5 +- .../tests/p2id_cargo_miden_build.rs | 16 +- tools/cargo-miden/tests/utils.rs | 60 ++- 9 files changed, 874 insertions(+), 120 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b44890d8b..d06e694aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,15 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Rust SDK + +- The FPI macro diagnostic for a dependency package missing from a midenc-driven build now names + the searched `MIDENC_PACKAGE_CACHE` directory and the expected package file names, instead of + an empty candidate list and a `target/miden/` hint that the cache lookup never + consults #1302 + ## [0.10.0-rc.1] ### Compiler and `midenc` diff --git a/midenc-session/src/lib.rs b/midenc-session/src/lib.rs index 943965ee6..300d8af77 100644 --- a/midenc-session/src/lib.rs +++ b/midenc-session/src/lib.rs @@ -27,7 +27,7 @@ mod inputs; mod libs; mod options; mod outputs; -#[cfg(feature = "std")] +#[cfg(any(test, feature = "std"))] mod package_cache; pub mod path; pub mod registry; @@ -81,7 +81,10 @@ pub struct Session { /// Statistics gathered from the current compiler session #[cfg(feature = "std")] pub statistics: Statistics, - /// The build-input fingerprint used to isolate this session's package cache + /// The build-input fingerprint used to isolate this session's package cache. + /// + /// Memoization assumes fingerprint-relevant [`Options`] are not mutated after the first cache + /// path request. #[cfg(feature = "std")] package_cache_fingerprint: std::sync::OnceLock, } @@ -368,13 +371,14 @@ impl Session { /// Where compiled dependency packages of this session's project are published and looked for. /// - /// `None` unless this session's input is a project locator: the cache lives under the - /// project's own `target/miden/packages//` directory, and a session compiling a - /// standalone source file has no project directory to put one under. The fingerprint covers + /// `None` unless this session's input is a project locator: with `std`, the cache lives under + /// the project's own `target/miden/packages//` directory, and a session compiling + /// a standalone source file has no project directory to put one under. The fingerprint covers /// the compiler identity, relevant build options, and the project's manifest closure. Both /// readers — this session's package registry and the nested `cargo` builds a Rust project's /// dependencies run through — must agree on the answer, which is why there is one derivation - /// of it. + /// of it. Without `std`, this returns the existing flat `target/miden/packages/` path without a + /// fingerprint component. /// /// Derived from the input locator rather than from a loaded manifest, which is what /// [`Session::new`] no longer has. That is also a repair: the manifest path was previously @@ -398,10 +402,12 @@ impl Session { #[cfg(feature = "std")] { let fingerprint = self.package_cache_fingerprint.get_or_init(|| { + let inherited_rustflags = std::env::var_os("RUSTFLAGS"); package_cache::fingerprint( &self.options, &project_dir, self.source_manager.as_ref(), + inherited_rustflags.as_deref(), MIDENC_BUILD_VERSION, MIDENC_BUILD_REV, ) diff --git a/midenc-session/src/package_cache.rs b/midenc-session/src/package_cache.rs index b1ff527ab..9a6ece12b 100644 --- a/midenc-session/src/package_cache.rs +++ b/midenc-session/src/package_cache.rs @@ -1,4 +1,15 @@ //! Build-input fingerprints for the filesystem package cache. +//! +//! The fingerprint models inputs that change the *set and identity* of packages visible to a +//! build. Source files and lockfiles are deliberately excluded: every resolved package is +//! rewritten into the current cache before its consumers expand, and the generated +//! `include_bytes!` reference makes Cargo re-expand when that package's contents change. +//! Registry and git dependencies contribute declaration text only; in particular, a git branch +//! moving without a manifest edit is outside this fingerprint by design. +//! +//! A workspace-root locator does not select a package, so `miden_project::Project::load` rejects +//! it. Such a locator contributes its raw manifests plus a load-failure marker and does not walk +//! workspace members; normal workspace compilation is expected to create per-member sessions. use alloc::{ format, @@ -7,6 +18,7 @@ use alloc::{ }; use std::{ collections::BTreeSet, + ffi::OsStr, path::{Path, PathBuf}, }; @@ -15,6 +27,15 @@ use miden_project::{Dependency, DependencyVersionScheme, Project}; use crate::{DebugInfo, LinkLibrary, OptLevel, Options, SourceManager}; +/// The number of lowercase hexadecimal characters in a package-cache fingerprint. +pub(crate) const FINGERPRINT_LEN: usize = 16; + +/// Returns true when `name` satisfies the package-cache fingerprint format. +pub(crate) fn is_fingerprint(name: &str) -> bool { + name.len() == FINGERPRINT_LEN + && name.bytes().all(|byte| byte.is_ascii_digit() || (b'a'..=b'f').contains(&byte)) +} + /// Computes the filesystem package cache fingerprint for a project build. /// /// Failures while reading or loading manifests are recorded as markers instead of being @@ -24,23 +45,25 @@ pub(crate) fn fingerprint( options: &Options, project_dir: &Path, source_manager: &dyn SourceManager, + inherited_rustflags: Option<&OsStr>, compiler_version: &str, compiler_revision: &str, ) -> String { let mut transcript = Transcript::new(); transcript.field("compiler.version", compiler_version.as_bytes()); transcript.field("compiler.revision", compiler_revision.as_bytes()); - record_options(&mut transcript, options); + record_options(&mut transcript, options, inherited_rustflags); let mut manifests = ManifestClosure::new(&mut transcript, source_manager); manifests.visit_project(project_dir, None); let digest = Blake3_256::hash(transcript.as_bytes()); - let mut fingerprint = String::with_capacity(16); - for byte in &digest.as_bytes()[..8] { - use core::fmt::Write; - write!(&mut fingerprint, "{byte:02x}").expect("writing to a string cannot fail"); - } + let fingerprint = miden_core::utils::to_hex(&digest.as_bytes()[..FINGERPRINT_LEN / 2]); + log::debug!( + target: "package-cache", + "filesystem package cache fingerprint for '{}': {fingerprint}", + project_dir.display() + ); fingerprint } @@ -67,10 +90,15 @@ impl Transcript { /// Appends an optional named field to this transcript. fn optional_field(&mut self, name: &str, value: Option<&str>) { + self.optional_bytes_field(name, value.map(str::as_bytes)); + } + + /// Appends an optional named byte field to this transcript. + fn optional_bytes_field(&mut self, name: &str, value: Option<&[u8]>) { match value { Some(value) => { self.field(&format!("{name}.state"), b"present"); - self.field(name, value.as_bytes()); + self.field(name, value); } None => self.field(&format!("{name}.state"), b"missing"), } @@ -83,40 +111,97 @@ impl Transcript { } /// Records the build configuration which can affect package identity or selection. -fn record_options(transcript: &mut Transcript, options: &Options) { - transcript.field("options.profile", options.profile.as_bytes()); - transcript.field("options.optimize", opt_level_name(options.optimize).as_bytes()); - transcript.field("options.debug", debug_info_name(options.debug).as_bytes()); - transcript.optional_field("options.target", options.target.as_deref()); - - let target_type = options.target_type.map(|target_type| target_type.to_string()); +fn record_options( + transcript: &mut Transcript, + options: &Options, + inherited_rustflags: Option<&OsStr>, +) { + let Options { + manifest_path: _, + name: _, + entrypoint: _, + profile, + workspace, + packages, + target, + target_type, + optimize, + debug, + output_types: _, + search_paths: _, + link_libraries, + link_modules: _, + sysroot, + midenup_home: _, + toolchain, + color: _, + diagnostics: _, + current_dir: _, + target_dir: _, + output_dir: _, + output_file: _, + remap_path_prefixes: _, + print_hir_source_locations: _, + stop_after: _, + parse_only: _, + analyze_only: _, + link_only: _, + no_link: _, + lint: _, + print_cfg_after_all: _, + print_cfg_after_pass: _, + print_ir_before_stage: _, + print_ir_after_all: _, + print_ir_after_pass: _, + print_ir_after_modified: _, + print_ir_filters: _, + save_temps: _, + rustflags, + cargo_frontmatter: _, + flags: _, + } = options; + + // Deliberate exclusions are classified here so adding an `Options` field forces a choice. + // Output paths, naming, diagnostics, printing, and stop flags do not select dependency + // packages. Search paths, link modules, remapped paths, custom flags, and similar + // content-affecting controls self-heal through the in-run package rewrite; the package-name + // set itself is driven by the manifest closure recorded below. + transcript.field("options.profile", profile.as_bytes()); + transcript.field("options.optimize", opt_level_name(*optimize).as_bytes()); + transcript.field("options.debug", debug_info_name(*debug).as_bytes()); + transcript.optional_field("options.target", target.as_deref()); + + let target_type = target_type.map(|target_type| target_type.to_string()); transcript.optional_field("options.target_type", target_type.as_deref()); - let mut packages = options.packages.clone(); + let mut packages = packages.clone(); packages.sort(); transcript.field("options.packages.count", &(packages.len() as u64).to_le_bytes()); for package in packages { transcript.field("options.package", package.as_bytes()); } - transcript.field("options.workspace", &[u8::from(options.workspace)]); - transcript.optional_field("options.rustflags", options.rustflags.as_deref()); - transcript.optional_field("options.toolchain", options.toolchain.as_deref()); + transcript.field("options.workspace", &[u8::from(*workspace)]); + transcript.optional_field("options.rustflags", rustflags.as_deref()); + transcript.optional_bytes_field( + "options.inherited_rustflags", + inherited_rustflags.map(OsStr::as_encoded_bytes), + ); + transcript.optional_field("options.toolchain", toolchain.as_deref()); - let mut link_libraries = options - .link_libraries - .iter() - .map(|library| link_library_input(library, options)) - .collect::>(); + let mut link_libraries = link_libraries.iter().map(link_library_input).collect::>(); link_libraries.sort(); transcript.field("options.link_libraries.count", &(link_libraries.len() as u64).to_le_bytes()); - for (name, version) in link_libraries { + for (name, path, linkage) in link_libraries { transcript.field("options.link_library.name", name.as_bytes()); - transcript.optional_field("options.link_library.version", version.as_deref()); + transcript.optional_bytes_field("options.link_library.path", path.as_deref()); + transcript.field("options.link_library.linkage", linkage.as_bytes()); } - let sysroot = options.sysroot.as_deref().map(path_string); - transcript.optional_field("options.sysroot", sysroot.as_deref()); + transcript.optional_bytes_field( + "options.sysroot", + sysroot.as_deref().map(|path| path.as_os_str().as_encoded_bytes()), + ); } /// Returns the stable transcript name for an optimization level. @@ -140,10 +225,15 @@ fn debug_info_name(level: DebugInfo) -> &'static str { } } -/// Resolves the name and package version of a requested link library. -fn link_library_input(library: &LinkLibrary, options: &Options) -> (String, Option) { - let version = library.load(options).ok().map(|package| package.version.to_string()); - (library.name.to_string(), version) +/// Returns the I/O-free identity of a requested link library. +/// +/// Built-in library versions are already pinned by the compiler build version. +fn link_library_input(library: &LinkLibrary) -> (String, Option>, &'static str) { + ( + library.name.to_string(), + library.path.as_deref().map(|path| path.as_os_str().as_encoded_bytes().to_vec()), + library.linkage.as_str(), + ) } /// Walks and records the manifest closure of one project. @@ -152,6 +242,7 @@ struct ManifestClosure<'a> { source_manager: &'a dyn SourceManager, visited_projects: BTreeSet, visited_packages: BTreeSet, + visited_workspace_roots: BTreeSet, } impl<'a> ManifestClosure<'a> { @@ -162,6 +253,7 @@ impl<'a> ManifestClosure<'a> { source_manager, visited_projects: BTreeSet::new(), visited_packages: BTreeSet::new(), + visited_workspace_roots: BTreeSet::new(), } } @@ -176,7 +268,7 @@ impl<'a> ManifestClosure<'a> { .ok() .and_then(|project| project.package().manifest_path().map(Path::to_path_buf)) .and_then(|manifest| manifest.parent().map(Path::to_path_buf)) - .unwrap_or_else(|| project_dir(locator)); + .unwrap_or_else(|| locator_project_dir(locator)); let project_key = canonical_or_original(&project_dir); if !self.visited_projects.insert(project_key) { return; @@ -186,27 +278,40 @@ impl<'a> ManifestClosure<'a> { self.record_manifest(&project_dir.join("miden-project.toml")); self.record_manifest(&project_dir.join("Cargo.toml")); - let Ok(project) = loaded else { - self.transcript.field("project.load", b"failed"); - self.transcript.field("project", b"end"); - return; + let project = match loaded { + Ok(project) => project, + Err(err) => { + log::debug!( + target: "package-cache", + "failed to load project '{}' while fingerprinting its manifest closure: {err}", + locator.display() + ); + self.transcript.field("project.load", b"failed"); + self.transcript.field("project", b"end"); + return; + } }; self.transcript.field("project.load", b"succeeded"); let package = project.package(); - let manifest_dir = - package.manifest_path().and_then(Path::parent).unwrap_or(project_dir.as_path()); let workspace_root = match &project { Project::WorkspacePackage { workspace, .. } => workspace.workspace_root(), Project::Package(_) => None, }; + if let Some(workspace_root) = workspace_root { + let workspace_key = canonical_or_original(workspace_root); + if self.visited_workspace_roots.insert(workspace_key) { + self.record_manifest(&workspace_root.join("miden-project.toml")); + self.record_manifest(&workspace_root.join("Cargo.toml")); + } + } let mut dependencies = package.dependencies().iter().collect::>(); dependencies.sort_by_cached_key(|dependency| dependency_sort_key(dependency)); self.transcript .field("project.dependencies.count", &(dependencies.len() as u64).to_le_bytes()); for dependency in dependencies { - self.visit_dependency(dependency, manifest_dir, workspace_root); + self.visit_dependency(dependency, project_dir.as_path(), workspace_root); } self.transcript.field("project", b"end"); } @@ -220,7 +325,14 @@ impl<'a> ManifestClosure<'a> { self.transcript.field("manifest.state", b"present"); self.transcript.field("manifest.bytes", &bytes); } - Err(_) => self.transcript.field("manifest.state", b"missing"), + Err(err) => { + log::debug!( + target: "package-cache", + "unable to read manifest '{}' while fingerprinting: {err}; recording a missing marker", + path.display() + ); + self.transcript.field("manifest.state", b"missing"); + } } } @@ -245,13 +357,25 @@ impl<'a> ManifestClosure<'a> { if let Some(workspace_root) = workspace_root { self.visit_path_dependency(dependency, workspace_root, path.inner()); } else { + log::debug!( + target: "package-cache", + "cannot resolve workspace path dependency '{}' while fingerprinting outside a workspace", + dependency.name() + ); self.transcript.field("dependency.path", b"unresolved-workspace"); } } DependencyVersionScheme::Workspace { member, .. } => { if let Some(workspace_root) = workspace_root { + // Unlike the canonical resolver, the shared helper below extension-classifies + // a workspace member even though `Workspace` always denotes source there. self.visit_path_dependency(dependency, workspace_root, member.inner()); } else { + log::debug!( + target: "package-cache", + "cannot resolve workspace member dependency '{}' while fingerprinting outside a workspace", + dependency.name() + ); self.transcript.field("dependency.path", b"unresolved-workspace"); } } @@ -272,10 +396,19 @@ impl<'a> ManifestClosure<'a> { uri: &miden_project::Uri, ) { if uri.scheme().is_some_and(|scheme| scheme != "file") { + log::debug!( + target: "package-cache", + "unsupported URI '{}' for path dependency '{}' while fingerprinting", + uri.as_str(), + dependency.name() + ); self.transcript.field("dependency.path", b"unsupported-uri"); return; } + // This mirrors `miden-project`'s `dependencies/graph.rs::resolve_dependency`; keep the + // two in sync. The fingerprint walk checks the extension before canonicalization, so a + // symlink to a `.masp` is classified as a project unlike the canonical resolver. let relative = Path::new(uri.path()); let path = if relative.is_absolute() { relative.to_path_buf() @@ -303,7 +436,14 @@ impl<'a> ManifestClosure<'a> { let digest = Blake3_256::hash(&bytes); self.transcript.field("package.file.digest", digest.as_bytes()); } - Err(_) => self.transcript.field("package.file.state", b"missing"), + Err(err) => { + log::debug!( + target: "package-cache", + "unable to read preassembled package '{}' while fingerprinting: {err}; recording a missing marker", + path.display() + ); + self.transcript.field("package.file.state", b"missing"); + } } self.transcript.field("package.file", b"end"); } @@ -348,7 +488,7 @@ fn optional_display(value: Option<&impl core::fmt::Display>) -> String { } /// Returns the directory whose sibling project manifests describe a locator. -fn project_dir(locator: &Path) -> PathBuf { +fn locator_project_dir(locator: &Path) -> PathBuf { if locator.file_name().is_some_and(|name| { name.eq_ignore_ascii_case("miden-project.toml") || name.eq_ignore_ascii_case("Cargo.toml") }) { @@ -363,11 +503,6 @@ fn canonical_or_original(path: &Path) -> PathBuf { path.canonicalize().unwrap_or_else(|_| path.to_path_buf()) } -/// Converts a path into the stable string representation used in the transcript. -fn path_string(path: &Path) -> String { - path.to_string_lossy().into_owned() -} - #[cfg(test)] mod tests { use miden_debug_types::DefaultSourceManager; @@ -395,7 +530,7 @@ mod tests { /// Computes a test fingerprint with a fresh source manager. fn test_fingerprint(options: &Options, project_dir: &Path, version: &str, rev: &str) -> String { - fingerprint(options, project_dir, &DefaultSourceManager::default(), version, rev) + fingerprint(options, project_dir, &DefaultSourceManager::default(), None, version, rev) } #[test] @@ -408,8 +543,7 @@ mod tests { let second = test_fingerprint(&options, temp.path(), "1.2.3", "abc123"); assert_eq!(first, second); - assert_eq!(first.len(), 16); - assert!(first.bytes().all(|byte| byte.is_ascii_digit() || (b'a'..=b'f').contains(&byte))); + assert!(is_fingerprint(&first)); } #[test] @@ -436,6 +570,64 @@ mod tests { assert_ne!(before, after); } + #[test] + fn fingerprint_walk_terminates_on_dependency_cycles() { + let temp = TempDir::new().unwrap(); + let first_project = temp.path().join("first"); + let second_project = temp.path().join("second"); + write_project( + &first_project, + "first", + "\n[dependencies]\nsecond = { path = \"../second\" }\n", + ); + write_project( + &second_project, + "second", + "\n[dependencies]\nfirst = { path = \"../first\" }\n", + ); + let options = Options::default(); + + let first = test_fingerprint(&options, &first_project, "1.2.3", "abc123"); + let second = test_fingerprint(&options, &first_project, "1.2.3", "abc123"); + + assert_eq!(first, second); + } + + #[test] + fn fingerprint_changes_with_preassembled_package_content() { + let temp = TempDir::new().unwrap(); + let root = temp.path().join("root"); + let package = temp.path().join("dependency.masp"); + write_project( + &root, + "root", + "\n[dependencies]\ndependency = { path = \"../dependency.masp\" }\n", + ); + std::fs::write(&package, b"first package").unwrap(); + let options = Options::default(); + let before = test_fingerprint(&options, &root, "1.2.3", "abc123"); + + std::fs::write(&package, b"different package").unwrap(); + let after = test_fingerprint(&options, &root, "1.2.3", "abc123"); + + assert_ne!(before, after); + } + + #[test] + fn project_load_failure_marker_is_stable_and_distinct() { + let temp = TempDir::new().unwrap(); + let options = Options::default(); + + let first = test_fingerprint(&options, temp.path(), "1.2.3", "abc123"); + let second = test_fingerprint(&options, temp.path(), "1.2.3", "abc123"); + assert_eq!(first, second); + + write_project(temp.path(), "root", ""); + let loadable = test_fingerprint(&options, temp.path(), "1.2.3", "abc123"); + + assert_ne!(first, loadable); + } + #[test] fn fingerprint_changes_with_build_options() { let temp = TempDir::new().unwrap(); @@ -452,6 +644,57 @@ mod tests { assert_ne!(baseline, test_fingerprint(&optimized, temp.path(), "1.2.3", "abc123")); } + #[test] + fn fingerprint_changes_with_inherited_rustflags() { + let temp = TempDir::new().unwrap(); + write_project(temp.path(), "root", ""); + let options = Options::default(); + + let missing = fingerprint( + &options, + temp.path(), + &DefaultSourceManager::default(), + None, + "1.2.3", + "abc123", + ); + let present = fingerprint( + &options, + temp.path(), + &DefaultSourceManager::default(), + Some(OsStr::new("-C target-feature=+bulk-memory")), + "1.2.3", + "abc123", + ); + + assert_ne!(missing, present); + } + + #[test] + fn fingerprint_changes_with_workspace_manifests() { + let temp = TempDir::new().unwrap(); + let member = temp.path().join("member"); + write_project(&member, "member", ""); + std::fs::write( + temp.path().join("miden-project.toml"), + "[workspace]\nmembers = [\"member\"]\n", + ) + .unwrap(); + std::fs::write(temp.path().join("Cargo.toml"), "[workspace]\nmembers = [\"member\"]\n") + .unwrap(); + let options = Options::default(); + let before = test_fingerprint(&options, &member, "1.2.3", "abc123"); + + std::fs::write( + temp.path().join("Cargo.toml"), + "[workspace]\nmembers = [\"member\"]\nresolver = \"3\"\n", + ) + .unwrap(); + let after = test_fingerprint(&options, &member, "1.2.3", "abc123"); + + assert_ne!(before, after); + } + #[test] fn fingerprint_changes_with_compiler_identity() { let temp = TempDir::new().unwrap(); diff --git a/midenc-session/src/registry.rs b/midenc-session/src/registry.rs index afe94e499..de76ae805 100644 --- a/midenc-session/src/registry.rs +++ b/midenc-session/src/registry.rs @@ -40,6 +40,8 @@ pub struct HybridPackageRegistry { artifacts: FxHashMap>>, #[cfg(any(test, feature = "std"))] filesystem_cache: Option, + #[cfg(any(test, feature = "std"))] + filesystem_cache_lock: Option, } impl HybridPackageRegistry { @@ -54,6 +56,7 @@ impl HybridPackageRegistry { packages: Default::default(), artifacts: Default::default(), filesystem_cache: None, + filesystem_cache_lock: None, } } @@ -64,7 +67,12 @@ impl HybridPackageRegistry { } /// Get a new instance of the registry, using the current compiler options and an optional - /// filesystem cache directory + /// filesystem cache directory. + /// + /// A cache path whose final component is a `midenc` fingerprint is created and locked for the + /// registry's lifetime. During construction, dead sibling fingerprint directories and legacy + /// flat `.masp` entries are pruned. A path that does not satisfy the fingerprint format is + /// created but deliberately neither locked nor used to sweep its parent. #[cfg(any(test, feature = "std"))] pub fn new_with_filesystem_cache( options: &crate::Options, @@ -72,9 +80,7 @@ impl HybridPackageRegistry { ) -> Result { use alloc::string::ToString; - if let Some(filesystem_cache) = filesystem_cache.as_deref() { - prepare_filesystem_cache(filesystem_cache); - } + let filesystem_cache_lock = filesystem_cache.as_deref().and_then(prepare_filesystem_cache); // Load system libraries let mut registry = if options.sysroot.is_some() { @@ -83,6 +89,7 @@ impl HybridPackageRegistry { Self::empty() }; registry.filesystem_cache = filesystem_cache; + registry.filesystem_cache_lock = filesystem_cache_lock; // Load link libraries let core = crate::LinkLibrary::core(); @@ -221,12 +228,27 @@ impl HybridPackageRegistry { } } -/// Creates the current cache directory and removes stale cache entries owned by `midenc`. +/// The filename used to keep a fingerprint directory live while its build registry exists. +#[cfg(any(test, feature = "std"))] +const BUILD_LOCK_FILENAME: &str = ".build-lock"; + +/// Creates and locks the current cache directory, then removes dead stale entries owned by +/// `midenc`. +/// +/// Deletion is correctness-critical rather than housekeeping. The FPI macro leaves an +/// `include_bytes!` reference to the package path in its expansion; if an old target survives, +/// Cargo can reuse that expansion and preserve stale procedure roots. Removing the old target +/// forces re-expansion. /// -/// Cleanup is best-effort because an inability to prune an old build must not obscure the -/// diagnostic from the current build. Package writes still report their own failures normally. +/// Each build tries to hold an exclusive [BUILD_LOCK_FILENAME] lock for its registry's lifetime. +/// A sibling fingerprint is dead when its lock file is absent or can be locked, and live when the +/// lock would block. The acquired stale lock is closed before deletion for Windows compatibility, +/// leaving a microscopic accepted race in which another process can re-lock the file before +/// `remove_dir_all`. Legacy flat `.masp` files have no lock and retain the accepted one-time race +/// with a pre-fingerprint compiler. Cleanup remains best-effort so it cannot obscure the current +/// build's own diagnostics; package writes still report their failures normally. #[cfg(any(test, feature = "std"))] -fn prepare_filesystem_cache(filesystem_cache: &std::path::Path) { +fn prepare_filesystem_cache(filesystem_cache: &std::path::Path) -> Option { if let Err(err) = std::fs::create_dir_all(filesystem_cache) { log::debug!( target: "package-registry", @@ -234,9 +256,18 @@ fn prepare_filesystem_cache(filesystem_cache: &std::path::Path) { filesystem_cache.display() ); } + if !filesystem_cache.file_name().is_some_and(is_package_cache_fingerprint) { + log::debug!( + target: "package-registry", + "filesystem package cache '{}' is not fingerprint-named; skipping locking and parent pruning", + filesystem_cache.display() + ); + return None; + } + let filesystem_cache_lock = acquire_filesystem_cache_lock(filesystem_cache); let Some(parent) = filesystem_cache.parent() else { - return; + return filesystem_cache_lock; }; let entries = match std::fs::read_dir(parent) { Ok(entries) => entries, @@ -246,7 +277,7 @@ fn prepare_filesystem_cache(filesystem_cache: &std::path::Path) { "failed to inspect filesystem package cache '{}': {err}", parent.display() ); - return; + return filesystem_cache_lock; } }; @@ -287,27 +318,127 @@ fn prepare_filesystem_cache(filesystem_cache: &std::path::Path) { } let result = if is_stale_fingerprint { + if !stale_fingerprint_can_be_pruned(&path) { + continue; + } std::fs::remove_dir_all(&path) } else { std::fs::remove_file(&path) }; if let Err(err) = result { - log::debug!( + log::warn!( target: "package-registry", - "failed to prune stale filesystem package cache entry '{}': {err}", + "failed to prune stale filesystem package cache entry '{}': {err}; stale macro expansions may survive; delete target/miden/packages manually", path.display() ); } } + + filesystem_cache_lock +} + +/// Opens the current fingerprint's lock file and tries to hold it for the registry lifetime. +#[cfg(any(test, feature = "std"))] +fn acquire_filesystem_cache_lock(filesystem_cache: &std::path::Path) -> Option { + use std::fs::{OpenOptions, TryLockError}; + + let lock_path = filesystem_cache.join(BUILD_LOCK_FILENAME); + let lock = match OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(&lock_path) + { + Ok(lock) => lock, + Err(err) => { + log::warn!( + target: "package-registry", + "failed to open filesystem package cache liveness lock '{}': {err}", + lock_path.display() + ); + return None; + } + }; + + match lock.try_lock() { + Ok(()) => Some(lock), + Err(TryLockError::WouldBlock) => { + log::debug!( + target: "package-registry", + "filesystem package cache '{}' is already protected by an identical-input build", + filesystem_cache.display() + ); + None + } + Err(TryLockError::Error(err)) => { + log::warn!( + target: "package-registry", + "failed to lock filesystem package cache '{}': {err}", + filesystem_cache.display() + ); + None + } + } +} + +/// Returns true when a stale fingerprint directory is not protected by a live build. +#[cfg(any(test, feature = "std"))] +fn stale_fingerprint_can_be_pruned(fingerprint_dir: &std::path::Path) -> bool { + use std::{ + fs::{File, TryLockError}, + io::ErrorKind, + }; + + let lock_path = fingerprint_dir.join(BUILD_LOCK_FILENAME); + let lock = match File::open(&lock_path) { + Ok(lock) => lock, + Err(err) if err.kind() == ErrorKind::NotFound => return true, + Err(err) => { + log::warn!( + target: "package-registry", + "cannot verify liveness of stale filesystem package cache '{}': {err}; skipping deletion", + fingerprint_dir.display() + ); + return false; + } + }; + + match lock.try_lock() { + Ok(()) => { + if let Err(err) = lock.unlock() { + log::debug!( + target: "package-registry", + "failed to explicitly unlock stale filesystem package cache '{}': {err}; closing the lock file", + fingerprint_dir.display() + ); + } + drop(lock); + true + } + Err(TryLockError::WouldBlock) => { + log::debug!( + target: "package-registry", + "skipping live filesystem package cache '{}' during stale-cache pruning", + fingerprint_dir.display() + ); + false + } + Err(TryLockError::Error(err)) => { + log::warn!( + target: "package-registry", + "cannot verify liveness of stale filesystem package cache '{}': {err}; skipping deletion", + fingerprint_dir.display() + ); + false + } + } } /// Returns true when `name` has the cache fingerprint format owned by `midenc`. #[cfg(any(test, feature = "std"))] fn is_package_cache_fingerprint(name: &std::ffi::OsStr) -> bool { - name.to_str().is_some_and(|name| { - name.len() == 16 - && name.bytes().all(|byte| byte.is_ascii_digit() || (b'a'..=b'f').contains(&byte)) - }) + name.to_str().is_some_and(crate::package_cache::is_fingerprint) } impl HybridPackageRegistry { @@ -384,6 +515,8 @@ impl PackageStore for HybridPackageRegistry { #[cfg(test)] mod tests { + use std::fs::OpenOptions; + use tempfile::TempDir; use super::*; @@ -408,18 +541,94 @@ mod tests { std::fs::write(&legacy_package, b"legacy").unwrap(); std::fs::write(&unrelated_file, b"unrelated").unwrap(); - let registry = HybridPackageRegistry::new_with_filesystem_cache( - &crate::Options::default(), - Some(current.clone()), - ) - .unwrap(); + let current_lock = + prepare_filesystem_cache(¤t).expect("current cache must be locked"); - assert_eq!(registry.filesystem_cache_dir(), Some(current.as_path())); assert!(current_marker.exists(), "the current cache must remain intact"); assert!(!stale.exists(), "a stale fingerprint directory must be removed"); assert!(!legacy_package.exists(), "a legacy flat package must be removed"); assert!(unrelated_directory.exists(), "unowned directories must be retained"); assert!(uppercase_directory.exists(), "non-lowercase directories must be retained"); assert!(unrelated_file.exists(), "unowned files must be retained"); + + drop(current_lock); + } + + #[test] + fn constructor_prepares_and_locks_the_filesystem_cache() { + let temp = TempDir::new().unwrap(); + let current = temp.path().join("packages").join("fedcba9876543210"); + + let registry = HybridPackageRegistry::new_with_filesystem_cache( + &crate::Options::default(), + Some(current.clone()), + ) + .unwrap(); + + assert_eq!(registry.filesystem_cache_dir(), Some(current.as_path())); + assert!(current.is_dir()); + assert!(registry.filesystem_cache_lock.is_some()); + let contender = OpenOptions::new() + .read(true) + .write(true) + .open(current.join(BUILD_LOCK_FILENAME)) + .unwrap(); + assert!(matches!(contender.try_lock(), Err(std::fs::TryLockError::WouldBlock))); + } + + #[test] + fn live_stale_fingerprint_survives_until_its_lock_is_released() { + let temp = TempDir::new().unwrap(); + let parent = temp.path().join("packages"); + let current = parent.join("fedcba9876543210"); + let stale = parent.join("0123456789abcdef"); + std::fs::create_dir_all(&stale).unwrap(); + + let stale_lock_path = stale.join(BUILD_LOCK_FILENAME); + let stale_lock = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(&stale_lock_path) + .unwrap(); + stale_lock.try_lock().unwrap(); + + let current_lock = + prepare_filesystem_cache(¤t).expect("current cache must be locked"); + assert!(current.join(BUILD_LOCK_FILENAME).exists()); + let current_contender = OpenOptions::new() + .read(true) + .write(true) + .open(current.join(BUILD_LOCK_FILENAME)) + .unwrap(); + assert!(matches!(current_contender.try_lock(), Err(std::fs::TryLockError::WouldBlock))); + assert!(stale.exists(), "a live sibling cache must not be pruned"); + + drop(stale_lock); + let second_lock = prepare_filesystem_cache(¤t); + assert!(second_lock.is_none(), "the first current-cache lock is still held"); + assert!(!stale.exists(), "the stale cache must be pruned after its build exits"); + + drop(current_lock); + } + + #[test] + fn arbitrary_cache_path_cannot_sweep_its_parent() { + let temp = TempDir::new().unwrap(); + let parent = temp.path().join("arbitrary-parent"); + let current = parent.join("cache"); + let fingerprint_sibling = parent.join("0123456789abcdef"); + let package_sibling = parent.join("unrelated.masp"); + std::fs::create_dir_all(&fingerprint_sibling).unwrap(); + std::fs::write(&package_sibling, b"unrelated").unwrap(); + + let lock = prepare_filesystem_cache(¤t); + + assert!(lock.is_none()); + assert!(current.is_dir(), "an arbitrary cache path is still created"); + assert!(!current.join(BUILD_LOCK_FILENAME).exists()); + assert!(fingerprint_sibling.exists()); + assert!(package_sibling.exists()); } } diff --git a/sdk/base-macros/src/fpi.rs b/sdk/base-macros/src/fpi.rs index ff67779fd..91dae9b2c 100644 --- a/sdk/base-macros/src/fpi.rs +++ b/sdk/base-macros/src/fpi.rs @@ -1579,14 +1579,6 @@ fn resolve_dependency_package_path(dependency: &SelectedDependency) -> syn::Resu } let package_stems = dependency_package_stems(dependency); - let preferred_profile = env::var("PROFILE").unwrap_or_else(|_| "debug".to_string()); - let mut profiles = vec![preferred_profile.clone()]; - if preferred_profile != "release" { - profiles.push("release".to_string()); - } - if preferred_profile != "debug" { - profiles.push("debug".to_string()); - } if let Some(filesystem_cache_dir) = std::env::var_os("MIDENC_PACKAGE_CACHE") { let filesystem_cache_dir = PathBuf::from(filesystem_cache_dir); if let Some(package) = @@ -1597,14 +1589,21 @@ fn resolve_dependency_package_path(dependency: &SelectedDependency) -> syn::Resu Err(Error::new( Span::call_site(), - missing_dependency_package_message( + missing_cached_dependency_package_message( dependency, &package_stems, - &[filesystem_cache_dir], - &[], + &filesystem_cache_dir, ), )) } else { + let preferred_profile = env::var("PROFILE").unwrap_or_else(|_| "debug".to_string()); + let mut profiles = vec![preferred_profile.clone()]; + if preferred_profile != "release" { + profiles.push("release".to_string()); + } + if preferred_profile != "debug" { + profiles.push("debug".to_string()); + } let output_dirs = dependency_output_dirs(dependency, &profiles); for dir in &output_dirs { if let Some(package) = find_dependency_package_in_dir(dir, &package_stems)? { @@ -1618,6 +1617,32 @@ fn resolve_dependency_package_path(dependency: &SelectedDependency) -> syn::Resu } } +/// Formats the diagnostic for a missing dependency in the build-owned package cache. +fn missing_cached_dependency_package_message( + dependency: &SelectedDependency, + package_stems: &[String], + filesystem_cache_dir: &Path, +) -> String { + let expected_files = package_stems + .iter() + .map(|stem| format!("'{stem}.masp'")) + .collect::>() + .join(", "); + + format!( + "miden::generate! could not find a built `.masp` package for FPI dependency '{}' (import \ + '{}', root '{}'). FPI wrappers need the dependency package during Rust macro expansion \ + to read procedure roots. Expected one of these package names: {expected_files}. Searched \ + MIDENC_PACKAGE_CACHE directory '{}'. This cache is populated by the enclosing \ + midenc-driven build; compile this crate as part of that build so its dependency packages \ + are available during macro expansion.", + dependency.name, + dependency.import(), + dependency.root.display(), + filesystem_cache_dir.display(), + ) +} + /// Formats the diagnostic emitted when FPI wrapper generation cannot load a dependency package. fn missing_dependency_package_message( dependency: &SelectedDependency, @@ -2120,6 +2145,30 @@ interface api { std::fs::remove_dir_all(temp_root).unwrap(); } + #[test] + fn missing_cached_dependency_package_message_describes_the_cache_contract() { + let dependency = SelectedDependency { + name: "counter".to_string(), + root: PathBuf::from("/projects/counter"), + interface: DependencyInterface { + name: "counter".to_string(), + import: "miden:counter/counter@0.0.1".to_string(), + types: Vec::new(), + }, + }; + let stems = vec!["counter".to_string(), "counter_component".to_string()]; + let cache_dir = Path::new("/target/miden/packages/0123456789abcdef"); + + let message = missing_cached_dependency_package_message(&dependency, &stems, cache_dir); + + assert!(message.contains("'counter.masp'")); + assert!(message.contains("'counter_component.masp'")); + assert!(message.contains(&cache_dir.display().to_string())); + assert!(message.contains("populated by the enclosing midenc-driven build")); + assert!(!message.contains(" in release")); + assert!(!message.contains("target/miden/")); + } + #[test] fn procedure_root_key_rejects_nested_non_wit_export_path() { let path = MasmPath::validate( diff --git a/tests/integration/src/sdk/mod.rs b/tests/integration/src/sdk/mod.rs index 6867e6bea..a11ae679f 100644 --- a/tests/integration/src/sdk/mod.rs +++ b/tests/integration/src/sdk/mod.rs @@ -1,4 +1,4 @@ -use std::{path::Path, sync::Arc}; +use std::{fs, path::Path, sync::Arc}; use miden_assembly::ast::types::{FunctionType, Type}; use miden_core::serde::Serializable; @@ -135,6 +135,165 @@ fn assert_component_export_signatures_match_wit(package: &miden_mast_package::Pa ); } +/// Creates a generated workspace containing the existing basic-wallet/swapp-note FPI pair. +fn fpi_package_cache_regression_project() -> crate::Project { + let sdk_path = sdk_crate_path(); + let workspace_manifest = r#" +[workspace] +members = ["basic-wallet", "swapp-note"] +resolver = "3" + +[profile.release] +opt-level = "z" +panic = "abort" +debug = false +"#; + let basic_wallet_cargo = format!( + r#" +cargo-features = ["trim-paths"] + +[package] +name = "basic_wallet" +version = "0.1.0" +edition = "2024" + +[lib] +crate-type = ["cdylib"] + +[dependencies] +miden = {{ path = "{}" }} +"#, + sdk_path.display(), + ); + let swapp_note_cargo = format!( + r#" +cargo-features = ["trim-paths"] + +[package] +name = "swapp-note" +version = "0.1.0" +edition = "2024" + +[lib] +crate-type = ["cdylib"] + +[dependencies] +miden = {{ path = "{}" }} + +[package.metadata.miden] +project-kind = "note-script" + +[package.metadata.component] +package = "miden:swapp-note" + +[package.metadata.miden.dependencies] +"miden:basic-wallet" = {{ path = "../basic-wallet" }} + +[package.metadata.component.target.dependencies] +"miden:basic-wallet" = {{ path = "../basic-wallet/target/generated-wit/" }} +"#, + sdk_path.display(), + ); + let swapp_note_miden_manifest = r#" +[package] +name = "swapp-note" +version = "0.1.0" + +[lib] +kind = "note" +namespace = "miden:swapp-note/miden-swapp-note@0.1.0" +path = "src/lib.rs" + +[dependencies] +basic-wallet = { path = "../basic-wallet" } + +[package.metadata.miden.dependencies] +basic-wallet = { wit = "../basic-wallet/target/generated-wit/" } +"#; + let swapp_note_source = include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/../fixtures/components/swapp-note/src/lib.rs" + )) + .replacen( + " let offered_asset = ¬e_assets[0];", + " let offered_asset = ¬e_assets[0];\n let foreign_wallet = \ + Wallet::new(self.creator);\n foreign_wallet.receive_asset(*offered_asset);", + 1, + ); + + project("fpi_package_cache_stale_root") + .file("Cargo.toml", workspace_manifest) + .file( + ".cargo/config.toml", + include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/../../examples/basic-wallet/.cargo/config.toml" + )), + ) + .file("basic-wallet/Cargo.toml", &basic_wallet_cargo) + .file( + "basic-wallet/miden-project.toml", + include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/../../examples/basic-wallet/miden-project.toml" + )), + ) + .file( + "basic-wallet/src/lib.rs", + include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/../../examples/basic-wallet/src/lib.rs" + )), + ) + .file("swapp-note/Cargo.toml", &swapp_note_cargo) + .file("swapp-note/miden-project.toml", swapp_note_miden_manifest) + .file("swapp-note/src/lib.rs", &swapp_note_source) + .build() +} + +/// Reads the named dependency package from a compiled consumer's filesystem cache. +fn read_cached_dependency_package( + test: &CompilerTest, + package_name: &str, +) -> miden_mast_package::Package { + let cache_dir = test + .session + .filesystem_package_cache_dir() + .expect("a Cargo Miden project must have a filesystem package cache"); + let path = cache_dir.join(format!("{package_name}.masp")); + let bytes = fs::read(&path) + .unwrap_or_else(|err| panic!("failed to read cached package '{}': {err}", path.display())); + miden_mast_package::Package::read_from_bytes_unchecked(&bytes) + .unwrap_or_else(|err| panic!("failed to decode cached package '{}': {err}", path.display())) +} + +/// Returns the canonical integer representation of a procedure digest's field elements. +fn procedure_digest_felts(digest: &miden_core::Word) -> [u64; 4] { + let elements = digest.as_elements(); + [ + elements[0].as_canonical_u64(), + elements[1].as_canonical_u64(), + elements[2].as_canonical_u64(), + elements[3].as_canonical_u64(), + ] +} + +/// Returns true when MASM reconstructs every felt in `digest` from its decimal `u32` limbs. +fn masm_contains_procedure_digest(masm: &str, digest: &miden_core::Word) -> bool { + let pattern = procedure_digest_felts(digest) + .into_iter() + .rev() + .map(|felt| { + let low = felt as u32; + let high = (felt >> u32::BITS) as u32; + format!("push.{high} push.{low} swap.1 mul.4294967296 add") + }) + .collect::>() + .join(" "); + let normalized_masm = masm.split_whitespace().collect::>().join(" "); + normalized_masm.contains(&pattern) +} + fn component_namespace(name: &str) -> String { let package = name.replace('_', "-"); format!("miden:{package}/miden-{package}@0.0.1") @@ -411,6 +570,64 @@ fn rust_sdk_account_package_build_is_deterministic() { } } +/// A dependency package rewrite must invalidate the FPI roots embedded by `include_bytes!`. +#[test] +fn rust_sdk_fpi_reexpands_after_dependency_package_changes() { + let project = fpi_package_cache_regression_project(); + let consumer = project.root().join("swapp-note"); + let dependency_source = project.root().join("basic-wallet/src/lib.rs"); + let config = WasmTranslationConfig::default(); + + let mut first_build = CompilerTest::rust_source_cargo_miden(&consumer, config.clone(), []); + let first_masm = first_build.masm_src(); + let first_cache = first_build.session.filesystem_package_cache_dir().unwrap(); + let first_dependency = read_cached_dependency_package(&first_build, "basic-wallet"); + let first_export = + find_manifest_procedure(&first_dependency, "basic-wallet receive-asset export", |path| { + path.ends_with("::\"receive-asset\"") + }); + let first_root = first_export.digest; + assert!( + masm_contains_procedure_digest(&first_masm, &first_root), + "consumer MASM does not contain the first receive-asset root {:?}", + procedure_digest_felts(&first_root), + ); + + let original_source = fs::read_to_string(&dependency_source).unwrap(); + let changed_source = original_source.replacen( + " self.add_asset(asset);", + " self.add_asset(asset);\n self.remove_asset(asset);\n \ + self.add_asset(asset);", + 1, + ); + assert_ne!(original_source, changed_source, "fixture mutation must match exactly once"); + fs::write(&dependency_source, changed_source).unwrap(); + + let mut second_build = CompilerTest::rust_source_cargo_miden(&consumer, config, []); + let second_masm = second_build.masm_src(); + let second_cache = second_build.session.filesystem_package_cache_dir().unwrap(); + let second_dependency = read_cached_dependency_package(&second_build, "basic-wallet"); + let second_export = + find_manifest_procedure(&second_dependency, "basic-wallet receive-asset export", |path| { + path.ends_with("::\"receive-asset\"") + }); + let second_root = second_export.digest; + + assert_eq!(first_cache, second_cache, "both builds must exercise the same cache path"); + assert_ne!(first_root, second_root, "the dependency implementation must change its root"); + assert_ne!(first_masm, second_masm, "the consumer must be recompiled with the new root"); + assert!( + !masm_contains_procedure_digest(&second_masm, &first_root), + "consumer MASM still contains the stale receive-asset root {:?}", + procedure_digest_felts(&first_root), + ); + assert!( + masm_contains_procedure_digest(&second_masm, &second_root), + "consumer MASM does not contain the new receive-asset root {:?}", + procedure_digest_felts(&second_root), + ); +} + #[test] fn rust_sdk_cross_ctx_word_arg_account_and_note() { let config = WasmTranslationConfig::default(); diff --git a/tools/cargo-miden/tests/masm_dependency.rs b/tools/cargo-miden/tests/masm_dependency.rs index 2e74f23fa..a5bd430b2 100644 --- a/tools/cargo-miden/tests/masm_dependency.rs +++ b/tools/cargo-miden/tests/masm_dependency.rs @@ -174,8 +174,9 @@ fn build_rust_project_with_masm_path_dependency() { // registry, or skipped. A materialized `.masp` for the dependency is produced only by // assembling it from its Miden Assembly sources. The package cache is uniqued by the build // inputs, so the package lives inside the build's fingerprint directory. - let dependency_package = crate::utils::package_cache_fingerprint_dir(&project_path) - .join(format!("{dependency_name}.masp")); + let dependency_package = + crate::utils::package_cache_fingerprint_dir(&project_path, "masm-dep.masp") + .join(format!("{dependency_name}.masp")); assert!( dependency_package.exists(), "expected the masm dependency to be assembled and materialized at {}", diff --git a/tools/cargo-miden/tests/p2id_cargo_miden_build.rs b/tools/cargo-miden/tests/p2id_cargo_miden_build.rs index 08c1c5c03..d63c96cdc 100644 --- a/tools/cargo-miden/tests/p2id_cargo_miden_build.rs +++ b/tools/cargo-miden/tests/p2id_cargo_miden_build.rs @@ -1,4 +1,4 @@ -use std::{env, fs}; +use std::env; use cargo_miden::run; @@ -30,17 +30,6 @@ fn p2id_build_materializes_basic_wallet_dependency() { let examples = workspace_root().join("examples"); let p2id_note_dir = examples.join("p2id-note"); - // The package cache directory we expect `cargo miden build` to populate on disk. The cache - // is uniqued by the build inputs, so the dependency package lands in a fingerprint - // subdirectory whose name is not known up front. - let package_cache_dir = p2id_note_dir.join("target").join("miden").join("packages"); - - // Make sure the basic-wallet dependency package is not already materialized on disk, so that we - // can attribute its presence after the build to the p2id-note build alone. - if package_cache_dir.exists() { - fs::remove_dir_all(&package_cache_dir).unwrap(); - } - // Build the p2id-note project, which pulls in basic-wallet as a Miden dependency. let restore_dir = env::current_dir().unwrap(); env::set_current_dir(&p2id_note_dir).unwrap(); @@ -62,7 +51,8 @@ fn p2id_build_materializes_basic_wallet_dependency() { // The build must have materialized the basic-wallet dependency package on disk, inside the // build's single fingerprint directory. let dep_package = - crate::utils::package_cache_fingerprint_dir(&p2id_note_dir).join("basic-wallet.masp"); + crate::utils::package_cache_fingerprint_dir(&p2id_note_dir, "basic-wallet.masp") + .join("basic-wallet.masp"); assert!( dep_package.exists(), "expected basic-wallet dependency package to be materialized at {}", diff --git a/tools/cargo-miden/tests/utils.rs b/tools/cargo-miden/tests/utils.rs index 9ee71c0ac..9a295c20b 100644 --- a/tools/cargo-miden/tests/utils.rs +++ b/tools/cargo-miden/tests/utils.rs @@ -44,30 +44,60 @@ pub(crate) fn current_dir_lock() -> CurrentDirGuard { } } -/// Returns the single build-fingerprint directory inside a project's package cache. +/// Returns the newest build-fingerprint directory containing `expected_package`. /// -/// The package cache under `target/miden/packages` is uniqued by the build inputs, so a build -/// materializes its dependency packages inside one fingerprint subdirectory whose name is not -/// known up front. -pub(crate) fn package_cache_fingerprint_dir(project_dir: &Path) -> PathBuf { +/// Another live build may retain a different fingerprint directory, so the package itself rather +/// than directory cardinality identifies the cache produced by the build under test. +pub(crate) fn package_cache_fingerprint_dir(project_dir: &Path, expected_package: &str) -> PathBuf { let package_cache_dir = project_dir.join("target").join("miden").join("packages"); - let fingerprint_dirs = fs::read_dir(&package_cache_dir) + let entries = fs::read_dir(&package_cache_dir) .unwrap_or_else(|err| { panic!( "expected the package cache directory '{}' to exist after the build: {err}", package_cache_dir.display() ) }) - .map(|entry| entry.unwrap().path()) - .filter(|path| path.is_dir()) .collect::>(); - assert_eq!( - fingerprint_dirs.len(), - 1, - "expected exactly one fingerprint directory in '{}', got {fingerprint_dirs:?}", - package_cache_dir.display() - ); - fingerprint_dirs.into_iter().next().unwrap() + + let mut listing = Vec::new(); + let mut candidates = Vec::new(); + for entry in entries { + let entry = entry.unwrap(); + let path = entry.path(); + if !path.is_dir() { + listing.push(path.display().to_string()); + continue; + } + + let contents = fs::read_dir(&path) + .map(|entries| { + entries + .filter_map(Result::ok) + .map(|entry| entry.file_name().to_string_lossy().into_owned()) + .collect::>() + }) + .unwrap_or_else(|err| vec![format!("")]); + listing.push(format!("{}: {contents:?}", path.display())); + + let expected = path.join(expected_package); + if let Ok(metadata) = expected.metadata() { + let modified = metadata.modified().unwrap_or(std::time::UNIX_EPOCH); + candidates.push((modified, path)); + } + } + + candidates + .into_iter() + .max_by_key(|(modified, _)| *modified) + .map(|(_, path)| path) + .unwrap_or_else(|| { + panic!( + "expected a fingerprint directory in '{}' containing '{expected_package}'; \ + entries:\n{}", + package_cache_dir.display(), + listing.join("\n") + ) + }) } pub(crate) fn project_template_arg(template: &str) -> String { From 53fe8a31576938a49504c42e8d3e9329b9679b6d Mon Sep 17 00:00:00 2001 From: Denys Zadorozhnyi Date: Wed, 5 Aug 2026 11:53:45 +0300 Subject: [PATCH 03/21] fix(base-macros): track the package cache path in FPI expansions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stale-expansion story rested entirely on pruning: a consumer's cached macro expansion was re-expanded only because its include_bytes! target vanished. Pruning is best-effort, so every failure path — a live locked directory that deliberately survives, a failed removal on a restrictive filesystem, a directory recreated by a still-running old-input build — left the original stale-roots bug reachable. Emit const _: Option<&str> = option_env!("MIDENC_PACKAGE_CACHE") into every FPI expansion, next to the existing include_bytes! constants. The variable's value carries the fingerprinted cache path, so rustc records it in the consumer's dep-info and Cargo re-expands the macro whenever the fingerprint rotates, even when a stale directory survives on disk. The same mechanism already invalidates cached expansions for MIDENC_EMIT_WIT. include_bytes! keeps covering content changes at an unchanged path. Pruning and locking thereby demote from correctness-critical to defense in depth: they remove legacy flat files whose expansions predate this tracking, keep the cache parent bounded, and prevent transient mid-build file loss. The prepare_filesystem_cache doc is updated to say so. --- midenc-session/src/registry.rs | 10 ++++++---- sdk/base-macros/src/fpi.rs | 5 +++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/midenc-session/src/registry.rs b/midenc-session/src/registry.rs index de76ae805..9ceb84d4c 100644 --- a/midenc-session/src/registry.rs +++ b/midenc-session/src/registry.rs @@ -235,10 +235,12 @@ const BUILD_LOCK_FILENAME: &str = ".build-lock"; /// Creates and locks the current cache directory, then removes dead stale entries owned by /// `midenc`. /// -/// Deletion is correctness-critical rather than housekeeping. The FPI macro leaves an -/// `include_bytes!` reference to the package path in its expansion; if an old target survives, -/// Cargo can reuse that expansion and preserve stale procedure roots. Removing the old target -/// forces re-expansion. +/// Deletion is defense in depth. The primary invalidation is in the FPI expansion itself: it +/// records `option_env!("MIDENC_PACKAGE_CACHE")`, whose value carries the fingerprinted cache +/// path, so Cargo re-expands consumers whenever the fingerprint rotates — even when a stale +/// directory survives here. Pruning still removes the `include_bytes!` targets of expansions +/// made by pre-fingerprint macro versions (the legacy flat files), keeps the parent directory +/// bounded, and takes dead caches out of circulation promptly. /// /// Each build tries to hold an exclusive [BUILD_LOCK_FILENAME] lock for its registry's lifetime. /// A sibling fingerprint is dead when its lock file is absent or can be locked, and live when the diff --git a/sdk/base-macros/src/fpi.rs b/sdk/base-macros/src/fpi.rs index 91dae9b2c..85240c085 100644 --- a/sdk/base-macros/src/fpi.rs +++ b/sdk/base-macros/src/fpi.rs @@ -1188,6 +1188,11 @@ pub(crate) fn augment_foreign_account_bindings( #(#trait_items)* #active_account_item #(#package_includes)* + // Record the package cache location in the consumer's dep-info. The value carries the + // build-input fingerprint, so Cargo re-expands this macro whenever the fingerprint + // rotates — even when a stale cache directory survives on disk. The `include_bytes!` + // constants above cover content changes at an unchanged path. + const _: Option<&str> = option_env!("MIDENC_PACKAGE_CACHE"); }) } From b117b97c0bf8cf4f47834598aa82b296835e6556 Mon Sep 17 00:00:00 2001 From: Denys Zadorozhnyi Date: Wed, 5 Aug 2026 14:28:53 +0300 Subject: [PATCH 04/21] fix(session): harden the package-cache lock and prune protocol The pre-submit review found four defects around cache preparation. A build that failed to create its own cache directory still swept every sibling; an identical-fingerprint contender ran unprotected after observing WouldBlock, so its directory could be deleted mid-build once the first holder exited; the lock file lived inside the directory it protected, leaving create-before-lock and unlock-before-delete windows; and the public constructor pruned the parent of any 16-hex-named path, so an arbitrary caller-supplied location could have unrelated siblings deleted. Locks now live outside the deletable directory as packages/.lock and are acquired before the directory is created. Builders hold the lock shared, so any number of identical-input builds stay protected at once; pruning demands the exclusive lock and holds it while remove_dir_all runs, then removes the orphaned lock file after verifying no builder acquired it. The residual close-to-unlink race is documented as accepted: option_env!(MIDENC_PACKAGE_CACHE) in FPI expansions is the correctness boundary, and pruning is defense in depth. Preparation now stops before any deletion when the cache or its parent cannot be created, and the destructive sweep runs only for paths in the owned miden/packages/ layout. Registry insertion also detects a same-name, same-version, different-digest conflict before touching disk, so a rejected package no longer overwrites the cache file that the accepted in-memory package no longer matches. Accepting paths still rewrite their file on every run, which the content self-heal relies on. Legacy flat cache entries are matched case-insensitively via Package::EXTENSION, so a Legacy.MASP leftover no longer survives the sweep on case-insensitive filesystems. Prune failures keep their consequence in the message and now name the actual parent directory; the log-only nature of cleanup reporting is documented on the constructor. --- midenc-session/src/registry.rs | 459 +++++++++++++++++++++++++-------- 1 file changed, 348 insertions(+), 111 deletions(-) diff --git a/midenc-session/src/registry.rs b/midenc-session/src/registry.rs index 9ceb84d4c..64300a974 100644 --- a/midenc-session/src/registry.rs +++ b/midenc-session/src/registry.rs @@ -69,10 +69,14 @@ impl HybridPackageRegistry { /// Get a new instance of the registry, using the current compiler options and an optional /// filesystem cache directory. /// - /// A cache path whose final component is a `midenc` fingerprint is created and locked for the - /// registry's lifetime. During construction, dead sibling fingerprint directories and legacy - /// flat `.masp` entries are pruned. A path that does not satisfy the fingerprint format is - /// created but deliberately neither locked nor used to sweep its parent. + /// A cache path in the owned `miden/packages/` layout is created and locked for + /// the registry's lifetime. During construction, dead sibling fingerprint directories and + /// legacy flat `.masp` entries are pruned as defense in depth; FPI expansions track the cache + /// path themselves for correctness. Any other path is created but deliberately neither locked + /// nor used to sweep its parent. + /// + /// Cleanup is best-effort and its failures are reported only through the `package-registry` + /// log target. Package insertion failures are still returned to the caller. #[cfg(any(test, feature = "std"))] pub fn new_with_filesystem_cache( options: &crate::Options, @@ -143,7 +147,7 @@ impl HybridPackageRegistry { continue; }; let path = entry.path(); - if path.extension().is_none_or(|ext| !ext.eq_ignore_ascii_case("masp")) { + if path.extension().is_none_or(|ext| !ext.eq_ignore_ascii_case(Package::EXTENSION)) { continue; } @@ -163,9 +167,22 @@ impl HybridPackageRegistry { &mut self, package: Arc, ) -> Result { - use alloc::collections::btree_map::Entry as BTreeMapEntry; - - use hashbrown::hash_map::Entry; + let version = miden_project::Version::new(package.version.clone(), package.digest()); + log::trace!(target: "package-registry", "preparing to install package {}@{version}", &package.name); + if let Some(previous_digest) = self + .packages + .get(&package.name) + .and_then(|versions| versions.get(&package.version)) + .and_then(PackageRecord::digest) + .copied() + && previous_digest != package.digest() + { + log::trace!(target: "package-registry", "package already installed: {}@{version}", &package.name); + return Err(InstallPackageError::AlreadyInstalledWithDifferentDigest { + package: package.name.clone(), + version, + }); + } #[cfg(any(test, feature = "std"))] if let Some(filesystem_cache) = self.filesystem_cache.as_deref() { @@ -177,8 +194,6 @@ impl HybridPackageRegistry { })?; } - let version = miden_project::Version::new(package.version.clone(), package.digest()); - log::trace!(target: "package-registry", "preparing to install package {}@{version}", &package.name); let record = PackageRecord::new( version.clone(), package.manifest.dependencies().map(|dep| { @@ -191,31 +206,10 @@ impl HybridPackageRegistry { ) }), ); - match self.packages.entry(package.name.clone()) { - Entry::Occupied(mut entry) => { - let versions = entry.get_mut(); - match versions.entry(package.version.clone()) { - BTreeMapEntry::Occupied(mut prev) => { - let prev_digest = prev.get().digest().copied(); - if prev_digest.is_none_or(|prev_digest| prev_digest == package.digest()) { - prev.insert(record); - } else { - log::trace!(target: "package-registry", "package already installed: {}@{version}", &package.name); - return Err(InstallPackageError::AlreadyInstalledWithDifferentDigest { - package: package.name.clone(), - version, - }); - } - } - BTreeMapEntry::Vacant(entry) => { - entry.insert(record); - } - } - } - Entry::Vacant(entry) => { - entry.insert([(package.version.clone(), record)].into_iter().collect()); - } - } + self.packages + .entry(package.name.clone()) + .or_default() + .insert(package.version.clone(), record); log::trace!(target: "package-registry", "installed {}@{version}", &package.name); @@ -228,9 +222,9 @@ impl HybridPackageRegistry { } } -/// The filename used to keep a fingerprint directory live while its build registry exists. +/// The extension of a sibling lock file that keeps a fingerprint directory live. #[cfg(any(test, feature = "std"))] -const BUILD_LOCK_FILENAME: &str = ".build-lock"; +const BUILD_LOCK_EXTENSION: &str = "lock"; /// Creates and locks the current cache directory, then removes dead stale entries owned by /// `midenc`. @@ -242,35 +236,59 @@ const BUILD_LOCK_FILENAME: &str = ".build-lock"; /// made by pre-fingerprint macro versions (the legacy flat files), keeps the parent directory /// bounded, and takes dead caches out of circulation promptly. /// -/// Each build tries to hold an exclusive [BUILD_LOCK_FILENAME] lock for its registry's lifetime. -/// A sibling fingerprint is dead when its lock file is absent or can be locked, and live when the -/// lock would block. The acquired stale lock is closed before deletion for Windows compatibility, -/// leaving a microscopic accepted race in which another process can re-lock the file before -/// `remove_dir_all`. Legacy flat `.masp` files have no lock and retain the accepted one-time race -/// with a pre-fingerprint compiler. Cleanup remains best-effort so it cannot obscure the current -/// build's own diagnostics; package writes still report their failures normally. +/// A build holds a shared `packages/.lock` lock for its registry's lifetime. Pruning +/// requires the corresponding exclusive lock and holds it while deleting the sibling fingerprint +/// directory, so every live same-input build protects that directory. The lock lives outside the +/// deletable directory and is acquired before that directory is created, closing both prior +/// create-before-lock and unlock-before-delete windows. +/// +/// After deletion the pruner closes and removes the now-orphaned lock file. A builder can acquire +/// that file between close and unlink; this residual unlink race is accepted because pruning is +/// hygiene-level defense in depth, while `option_env!("MIDENC_PACKAGE_CACHE")` in FPI expansions +/// provides the correctness boundary. Legacy flat `.masp` files have no lock and retain the +/// accepted one-time race with a pre-fingerprint compiler. Cleanup remains best-effort so it +/// cannot obscure the current build's own diagnostics; package writes still report their failures +/// normally. #[cfg(any(test, feature = "std"))] fn prepare_filesystem_cache(filesystem_cache: &std::path::Path) -> Option { - if let Err(err) = std::fs::create_dir_all(filesystem_cache) { + if !is_owned_filesystem_cache_path(filesystem_cache) { + if let Err(err) = std::fs::create_dir_all(filesystem_cache) { + log::warn!( + target: "package-registry", + "failed to create filesystem package cache '{}': {err}; skipping cache preparation", + filesystem_cache.display() + ); + return None; + } log::debug!( target: "package-registry", - "failed to create filesystem package cache '{}': {err}", + "filesystem package cache '{}' is outside the owned miden/packages/ layout; skipping locking and parent pruning", filesystem_cache.display() ); + return None; } - if !filesystem_cache.file_name().is_some_and(is_package_cache_fingerprint) { - log::debug!( + + let parent = filesystem_cache + .parent() + .expect("an owned filesystem cache path always has a packages parent"); + if let Err(err) = std::fs::create_dir_all(parent) { + log::warn!( target: "package-registry", - "filesystem package cache '{}' is not fingerprint-named; skipping locking and parent pruning", + "failed to create filesystem package cache parent '{}': {err}; skipping cache preparation", + parent.display() + ); + return None; + } + let filesystem_cache_lock = acquire_filesystem_cache_lock(filesystem_cache)?; + if let Err(err) = std::fs::create_dir_all(filesystem_cache) { + log::warn!( + target: "package-registry", + "failed to create filesystem package cache '{}': {err}; skipping cache preparation", filesystem_cache.display() ); return None; } - let filesystem_cache_lock = acquire_filesystem_cache_lock(filesystem_cache); - let Some(parent) = filesystem_cache.parent() else { - return filesystem_cache_lock; - }; let entries = match std::fs::read_dir(parent) { Ok(entries) => entries, Err(err) => { @@ -279,9 +297,10 @@ fn prepare_filesystem_cache(filesystem_cache: &std::path::Path) -> Option Option Option Option { use std::fs::{OpenOptions, TryLockError}; - let lock_path = filesystem_cache.join(BUILD_LOCK_FILENAME); + let lock_path = filesystem_cache_lock_path(filesystem_cache); let lock = match OpenOptions::new() .read(true) .write(true) @@ -363,12 +377,12 @@ fn acquire_filesystem_cache_lock(filesystem_cache: &std::path::Path) -> Option Some(lock), Err(TryLockError::WouldBlock) => { log::debug!( target: "package-registry", - "filesystem package cache '{}' is already protected by an identical-input build", + "filesystem package cache '{}' is being pruned; skipping cache preparation", filesystem_cache.display() ); None @@ -384,59 +398,156 @@ fn acquire_filesystem_cache_lock(filesystem_cache: &std::path::Path) -> Option bool { - use std::{ - fs::{File, TryLockError}, - io::ErrorKind, - }; +fn prune_stale_fingerprint(fingerprint_dir: &std::path::Path, parent: &std::path::Path) { + use std::fs::{OpenOptions, TryLockError}; - let lock_path = fingerprint_dir.join(BUILD_LOCK_FILENAME); - let lock = match File::open(&lock_path) { + let lock_path = filesystem_cache_lock_path(fingerprint_dir); + let lock = match OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(&lock_path) + { Ok(lock) => lock, - Err(err) if err.kind() == ErrorKind::NotFound => return true, Err(err) => { log::warn!( target: "package-registry", "cannot verify liveness of stale filesystem package cache '{}': {err}; skipping deletion", fingerprint_dir.display() ); - return false; + return; } }; match lock.try_lock() { Ok(()) => { - if let Err(err) = lock.unlock() { - log::debug!( - target: "package-registry", - "failed to explicitly unlock stale filesystem package cache '{}': {err}; closing the lock file", - fingerprint_dir.display() - ); + if let Err(err) = std::fs::remove_dir_all(fingerprint_dir) { + warn_prune_failure(fingerprint_dir, parent, &err); + return; } drop(lock); - true + remove_orphaned_lock_file(&lock_path, fingerprint_dir, parent); } Err(TryLockError::WouldBlock) => { log::debug!( target: "package-registry", "skipping live filesystem package cache '{}' during stale-cache pruning", fingerprint_dir.display() - ); - false + ) } Err(TryLockError::Error(err)) => { log::warn!( target: "package-registry", "cannot verify liveness of stale filesystem package cache '{}': {err}; skipping deletion", fingerprint_dir.display() + ) + } + } +} + +/// Deletes an orphaned sibling lock after verifying that no builder holds it. +#[cfg(any(test, feature = "std"))] +fn prune_orphaned_fingerprint_lock(lock_path: &std::path::Path, parent: &std::path::Path) { + use std::{fs::TryLockError, io::ErrorKind}; + + let lock = match std::fs::File::open(lock_path) { + Ok(lock) => lock, + Err(err) if err.kind() == ErrorKind::NotFound => return, + Err(err) => { + log::warn!( + target: "package-registry", + "cannot verify liveness of orphaned filesystem package cache lock '{}': {err}; skipping deletion", + lock_path.display() ); - false + return; + } + }; + match lock.try_lock() { + Ok(()) => { + drop(lock); + let fingerprint_dir = lock_path.with_extension(""); + remove_orphaned_lock_file(lock_path, &fingerprint_dir, parent); + } + Err(TryLockError::WouldBlock) => { + log::debug!( + target: "package-registry", + "skipping live filesystem package cache lock '{}' during orphan cleanup", + lock_path.display() + ) + } + Err(TryLockError::Error(err)) => { + log::warn!( + target: "package-registry", + "cannot verify liveness of orphaned filesystem package cache lock '{}': {err}; skipping deletion", + lock_path.display() + ) } } } +/// Removes an unlocked lock file if its fingerprint directory remains absent. +#[cfg(any(test, feature = "std"))] +fn remove_orphaned_lock_file( + lock_path: &std::path::Path, + fingerprint_dir: &std::path::Path, + parent: &std::path::Path, +) { + use std::io::ErrorKind; + + if fingerprint_dir.exists() { + return; + } + if let Err(err) = std::fs::remove_file(lock_path) + && err.kind() != ErrorKind::NotFound + { + warn_prune_failure(lock_path, parent, &err); + } +} + +/// Logs a best-effort cleanup failure with the exact directory a user can remove. +#[cfg(any(test, feature = "std"))] +fn warn_prune_failure(path: &std::path::Path, parent: &std::path::Path, err: &std::io::Error) { + log::warn!( + target: "package-registry", + "failed to prune stale filesystem package cache entry '{}': {err}; stale cache entries may survive; delete '{}' manually", + path.display(), + parent.display() + ); +} + +/// Returns the sibling lock path associated with a fingerprint directory. +#[cfg(any(test, feature = "std"))] +fn filesystem_cache_lock_path(filesystem_cache: &std::path::Path) -> std::path::PathBuf { + filesystem_cache.with_extension(BUILD_LOCK_EXTENSION) +} + +/// Returns true when a path is owned by the `miden/packages/` cache layout. +#[cfg(any(test, feature = "std"))] +fn is_owned_filesystem_cache_path(filesystem_cache: &std::path::Path) -> bool { + use std::ffi::OsStr; + + filesystem_cache.file_name().is_some_and(is_package_cache_fingerprint) + && filesystem_cache + .parent() + .and_then(std::path::Path::file_name) + .is_some_and(|name| name == OsStr::new("packages")) + && filesystem_cache + .parent() + .and_then(std::path::Path::parent) + .and_then(std::path::Path::file_name) + .is_some_and(|name| name == OsStr::new("miden")) +} + +/// Extracts a fingerprint from an owned sibling `.lock` filename. +#[cfg(any(test, feature = "std"))] +fn package_cache_fingerprint_from_lock(name: &std::ffi::OsStr) -> Option<&str> { + let fingerprint = name.to_str()?.strip_suffix(".lock")?; + crate::package_cache::is_fingerprint(fingerprint).then_some(fingerprint) +} + /// Returns true when `name` has the cache fingerprint format owned by `midenc`. #[cfg(any(test, feature = "std"))] fn is_package_cache_fingerprint(name: &std::ffi::OsStr) -> bool { @@ -523,15 +634,57 @@ mod tests { use super::*; + #[test] + fn install_checks_conflicts_before_writing_and_rewrites_accepted_packages() { + let temp = TempDir::new().unwrap(); + let cache = temp.path().join("cache"); + std::fs::create_dir_all(&cache).unwrap(); + + let options = crate::Options::default(); + let package = crate::LinkLibrary::core().load(&options).unwrap(); + let package_name: &str = &package.name; + let cached_package = cache.join(package_name).with_extension(Package::EXTENSION); + let mut registry = HybridPackageRegistry::empty(); + registry.filesystem_cache = Some(cache); + + registry.install_if_missing(Arc::clone(&package)).unwrap(); + std::fs::write(&cached_package, b"damaged").unwrap(); + registry.install_if_missing(Arc::clone(&package)).unwrap(); + assert_ne!( + std::fs::read(&cached_package).unwrap(), + b"damaged", + "an accepted same-digest install must repair the cached package" + ); + + let mut conflict = (*crate::LinkLibrary::tx_kernel().load(&options).unwrap()).clone(); + conflict.name = package.name.clone(); + conflict.version = package.version.clone(); + assert_ne!(conflict.digest(), package.digest()); + std::fs::write(&cached_package, b"keep-on-conflict").unwrap(); + + assert!(matches!( + registry.install_if_missing(Arc::new(conflict)), + Err(InstallPackageError::AlreadyInstalledWithDifferentDigest { .. }) + )); + assert_eq!( + std::fs::read(cached_package).unwrap(), + b"keep-on-conflict", + "a rejected install must not touch the cached package" + ); + } + #[test] fn creating_a_filesystem_cache_prunes_only_stale_owned_entries() { let temp = TempDir::new().unwrap(); - let parent = temp.path().join("packages"); + let parent = temp.path().join("miden").join("packages"); let current = parent.join("fedcba9876543210"); let stale = parent.join("0123456789abcdef"); let unrelated_directory = parent.join("not-a-midenc-cache"); let uppercase_directory = parent.join("ABCDEF0123456789"); let legacy_package = parent.join("legacy.masp"); + let uppercase_legacy_package = parent.join("uppercase.MASP"); + let orphan_lock = parent.join("1111111111111111.lock"); + let live_precreation_lock_path = parent.join("2222222222222222.lock"); let unrelated_file = parent.join("keep.txt"); for directory in [¤t, &stale, &unrelated_directory, &uppercase_directory] { @@ -541,6 +694,16 @@ mod tests { std::fs::write(¤t_marker, b"current").unwrap(); std::fs::write(stale.join("old.masp"), b"stale").unwrap(); std::fs::write(&legacy_package, b"legacy").unwrap(); + std::fs::write(&uppercase_legacy_package, b"legacy").unwrap(); + std::fs::write(&orphan_lock, b"").unwrap(); + let live_precreation_lock = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(&live_precreation_lock_path) + .unwrap(); + live_precreation_lock.try_lock_shared().unwrap(); std::fs::write(&unrelated_file, b"unrelated").unwrap(); let current_lock = @@ -548,18 +711,29 @@ mod tests { assert!(current_marker.exists(), "the current cache must remain intact"); assert!(!stale.exists(), "a stale fingerprint directory must be removed"); + assert!(!filesystem_cache_lock_path(&stale).exists(), "the stale lock must be removed"); assert!(!legacy_package.exists(), "a legacy flat package must be removed"); + assert!( + !uppercase_legacy_package.exists(), + "legacy package extensions must be matched case-insensitively" + ); + assert!(!orphan_lock.exists(), "an unlocked orphan fingerprint lock must be removed"); + assert!( + live_precreation_lock_path.exists(), + "a lock held before its directory is created must survive orphan cleanup" + ); assert!(unrelated_directory.exists(), "unowned directories must be retained"); assert!(uppercase_directory.exists(), "non-lowercase directories must be retained"); assert!(unrelated_file.exists(), "unowned files must be retained"); + drop(live_precreation_lock); drop(current_lock); } #[test] fn constructor_prepares_and_locks_the_filesystem_cache() { let temp = TempDir::new().unwrap(); - let current = temp.path().join("packages").join("fedcba9876543210"); + let current = temp.path().join("miden").join("packages").join("fedcba9876543210"); let registry = HybridPackageRegistry::new_with_filesystem_cache( &crate::Options::default(), @@ -573,20 +747,26 @@ mod tests { let contender = OpenOptions::new() .read(true) .write(true) - .open(current.join(BUILD_LOCK_FILENAME)) + .open(filesystem_cache_lock_path(¤t)) .unwrap(); - assert!(matches!(contender.try_lock(), Err(std::fs::TryLockError::WouldBlock))); + contender.try_lock_shared().unwrap(); + let stale_checker = OpenOptions::new() + .read(true) + .write(true) + .open(filesystem_cache_lock_path(¤t)) + .unwrap(); + assert!(matches!(stale_checker.try_lock(), Err(std::fs::TryLockError::WouldBlock))); } #[test] fn live_stale_fingerprint_survives_until_its_lock_is_released() { let temp = TempDir::new().unwrap(); - let parent = temp.path().join("packages"); + let parent = temp.path().join("miden").join("packages"); let current = parent.join("fedcba9876543210"); let stale = parent.join("0123456789abcdef"); std::fs::create_dir_all(&stale).unwrap(); - let stale_lock_path = stale.join(BUILD_LOCK_FILENAME); + let stale_lock_path = filesystem_cache_lock_path(&stale); let stale_lock = OpenOptions::new() .read(true) .write(true) @@ -594,27 +774,65 @@ mod tests { .truncate(false) .open(&stale_lock_path) .unwrap(); - stale_lock.try_lock().unwrap(); + stale_lock.try_lock_shared().unwrap(); let current_lock = prepare_filesystem_cache(¤t).expect("current cache must be locked"); - assert!(current.join(BUILD_LOCK_FILENAME).exists()); + assert!(filesystem_cache_lock_path(¤t).exists()); let current_contender = OpenOptions::new() .read(true) .write(true) - .open(current.join(BUILD_LOCK_FILENAME)) + .open(filesystem_cache_lock_path(¤t)) .unwrap(); - assert!(matches!(current_contender.try_lock(), Err(std::fs::TryLockError::WouldBlock))); + current_contender.try_lock_shared().unwrap(); assert!(stale.exists(), "a live sibling cache must not be pruned"); drop(stale_lock); - let second_lock = prepare_filesystem_cache(¤t); - assert!(second_lock.is_none(), "the first current-cache lock is still held"); + let second_lock = + prepare_filesystem_cache(¤t).expect("same-input builders share the lock"); assert!(!stale.exists(), "the stale cache must be pruned after its build exits"); + assert!(!stale_lock_path.exists(), "the stale sibling lock must be removed"); + drop(second_lock); drop(current_lock); } + #[test] + fn same_fingerprint_contender_remains_live_after_first_builder_exits() { + let temp = TempDir::new().unwrap(); + let parent = temp.path().join("miden").join("packages"); + let shared = parent.join("fedcba9876543210"); + let different = parent.join("0123456789abcdef"); + + let first = prepare_filesystem_cache(&shared).expect("first builder must lock the cache"); + let contender = + prepare_filesystem_cache(&shared).expect("same-input contender must share the lock"); + drop(first); + + let different_lock = prepare_filesystem_cache(&different) + .expect("different-input builder must lock its cache"); + + assert!(shared.exists(), "the live contender's cache must not be pruned"); + + drop(different_lock); + drop(contender); + } + + #[test] + fn cache_creation_failure_does_not_sweep_siblings() { + let temp = TempDir::new().unwrap(); + let parent = temp.path().join("miden").join("packages"); + let current = parent.join("fedcba9876543210"); + let stale = parent.join("0123456789abcdef"); + std::fs::create_dir_all(&stale).unwrap(); + std::fs::write(¤t, b"not a directory").unwrap(); + + let lock = prepare_filesystem_cache(¤t); + + assert!(lock.is_none()); + assert!(stale.exists(), "siblings must survive when the current cache cannot be created"); + } + #[test] fn arbitrary_cache_path_cannot_sweep_its_parent() { let temp = TempDir::new().unwrap(); @@ -629,7 +847,26 @@ mod tests { assert!(lock.is_none()); assert!(current.is_dir(), "an arbitrary cache path is still created"); - assert!(!current.join(BUILD_LOCK_FILENAME).exists()); + assert!(!filesystem_cache_lock_path(¤t).exists()); + assert!(fingerprint_sibling.exists()); + assert!(package_sibling.exists()); + } + + #[test] + fn fingerprint_name_outside_owned_layout_cannot_sweep_its_parent() { + let temp = TempDir::new().unwrap(); + let parent = temp.path().join("shared"); + let current = parent.join("fedcba9876543210"); + let fingerprint_sibling = parent.join("0123456789abcdef"); + let package_sibling = parent.join("unrelated.masp"); + std::fs::create_dir_all(&fingerprint_sibling).unwrap(); + std::fs::write(&package_sibling, b"unrelated").unwrap(); + + let lock = prepare_filesystem_cache(¤t); + + assert!(lock.is_none()); + assert!(current.is_dir(), "an out-of-layout cache path is still created"); + assert!(!filesystem_cache_lock_path(¤t).exists()); assert!(fingerprint_sibling.exists()); assert!(package_sibling.exists()); } From e496af3ffeb21020872fe1dbc9ba7e68a908c13d Mon Sep 17 00:00:00 2001 From: Denys Zadorozhnyi Date: Wed, 5 Aug 2026 14:29:11 +0300 Subject: [PATCH 05/21] fix(session): close package-cache fingerprint derivation gaps Three inputs escaped the fingerprint or degraded it silently. A bare relative --manifest-path (e.g. plain Cargo.toml) produced an empty project directory, so the manifest walk recorded a load failure and never visited path dependencies; the derivation now absolutizes the locator against the session's configured working directory. A workspace member dependency was classified by file extension instead of being resolved through the loaded workspace; it now resolves via get_member_by_relative_path like the canonical resolver. RUSTUP_TOOLCHAIN influences the nested build's toolchain selection the same way inherited RUSTFLAGS influences its flags, so it is fingerprinted the same way, as a parameter the session reads from the environment. The walk now uses a private source manager, so computing the cache path no longer interns every closure manifest into the compilation session's source manager as a side effect. The remaining walk-versus-resolver deviations are consolidated into one comment pointing at the closest in-tree sibling (frontend/masm's collect_dependency_metadata_for_scheme): path dependencies are extension-classified before canonicalization, and git declarations are recorded but never recursed. The module docs now cover the degraded cases and their recovery: Cargo-only projects without a miden-project.toml (root manifests hashed, no dependency recursion, reported at debug level), moved unpinned git revisions and transitive git dependencies being outside the closure, dropped-but-cached names lingering until the fingerprint next rotates, and the per-member-session assumption workspace builds rely on, noted where the root session is created. --- midenc-session/src/lib.rs | 43 +++++++- midenc-session/src/package_cache.rs | 139 +++++++++++++++++++----- tools/cargo-miden/src/commands/build.rs | 3 + 3 files changed, 152 insertions(+), 33 deletions(-) diff --git a/midenc-session/src/lib.rs b/midenc-session/src/lib.rs index 300d8af77..cff8e24b7 100644 --- a/midenc-session/src/lib.rs +++ b/midenc-session/src/lib.rs @@ -391,23 +391,27 @@ impl Session { return None; } let project_dir = input.as_path()?.parent()?; + let project_dir = if project_dir.is_absolute() { + project_dir.to_path_buf() + } else { + self.options.current_dir.join(project_dir) + }; // Canonicalized because the loaded manifest path this replaces was: the cache directory // is compared by path across nested builds, so `.`-relative and symlinked spellings of // one directory must not resolve to two caches. #[cfg(feature = "std")] - let project_dir = project_dir.canonicalize().unwrap_or_else(|_| project_dir.to_path_buf()); - #[cfg(not(feature = "std"))] - let project_dir = project_dir.to_path_buf(); + let project_dir = project_dir.canonicalize().unwrap_or(project_dir); let package_cache_dir = project_dir.join("target").join("miden").join("packages"); #[cfg(feature = "std")] { let fingerprint = self.package_cache_fingerprint.get_or_init(|| { let inherited_rustflags = std::env::var_os("RUSTFLAGS"); + let inherited_rustup_toolchain = std::env::var_os("RUSTUP_TOOLCHAIN"); package_cache::fingerprint( &self.options, &project_dir, - self.source_manager.as_ref(), inherited_rustflags.as_deref(), + inherited_rustup_toolchain.as_deref(), MIDENC_BUILD_VERSION, MIDENC_BUILD_REV, ) @@ -756,3 +760,34 @@ fn create_target_dir(path: &Path) { #[cfg(not(feature = "std"))] fn create_target_dir(_path: &Path) {} + +#[cfg(test)] +mod tests { + use alloc::sync::Arc; + + use tempfile::TempDir; + + use super::*; + + #[test] + fn relative_manifest_locator_uses_the_configured_current_directory() { + let temp = TempDir::new().unwrap(); + let options = Options { + current_dir: temp.path().to_path_buf(), + target_dir: temp.path().join("target"), + ..Options::default() + }; + let input = InputFile::new(FileType::Toml, InputType::Real("Cargo.toml".into())); + let session = Session::new_project( + "relative-manifest".into(), + Some(input), + Box::new(options), + None, + Arc::new(diagnostics::DefaultSourceManager::default()), + ); + + let cache_dir = session.filesystem_package_cache_dir().unwrap(); + let expected_parent = temp.path().canonicalize().unwrap().join("target/miden/packages"); + assert_eq!(cache_dir.parent(), Some(expected_parent.as_path())); + } +} diff --git a/midenc-session/src/package_cache.rs b/midenc-session/src/package_cache.rs index 9a6ece12b..ce025da4f 100644 --- a/midenc-session/src/package_cache.rs +++ b/midenc-session/src/package_cache.rs @@ -4,12 +4,21 @@ //! build. Source files and lockfiles are deliberately excluded: every resolved package is //! rewritten into the current cache before its consumers expand, and the generated //! `include_bytes!` reference makes Cargo re-expand when that package's contents change. +//! Expansions also record `MIDENC_PACKAGE_CACHE`, so rotating the fingerprinted path re-expands +//! consumers even if best-effort stale-directory pruning does not complete. //! Registry and git dependencies contribute declaration text only; in particular, a git branch -//! moving without a manifest edit is outside this fingerprint by design. +//! moving without a manifest edit is outside this fingerprint by design, as are a git package's +//! transitive dependencies. Pinning a revision or deleting the cache directory recovers from a +//! moved unpinned revision. The current fingerprint directory is never emptied before packages +//! are rewritten, so names dropped from the dependency set can linger until another fingerprinted +//! input rotates the directory. //! //! A workspace-root locator does not select a package, so `miden_project::Project::load` rejects //! it. Such a locator contributes its raw manifests plus a load-failure marker and does not walk //! workspace members; normal workspace compilation is expected to create per-member sessions. +//! Similarly, a Cargo-only project with no sibling `miden-project.toml` contributes both root +//! manifest slots and a load-failure marker, but its dependencies cannot be discovered and are +//! not recursed. That degraded case is reported at debug level while fingerprinting. use alloc::{ format, @@ -23,9 +32,11 @@ use std::{ }; use miden_core::crypto::hash::Blake3_256; +use miden_debug_types::{DefaultSourceManager, SourceManager}; +use miden_mast_package::Package; use miden_project::{Dependency, DependencyVersionScheme, Project}; -use crate::{DebugInfo, LinkLibrary, OptLevel, Options, SourceManager}; +use crate::{DebugInfo, LinkLibrary, OptLevel, Options}; /// The number of lowercase hexadecimal characters in a package-cache fingerprint. pub(crate) const FINGERPRINT_LEN: usize = 16; @@ -40,21 +51,23 @@ pub(crate) fn is_fingerprint(name: &str) -> bool { /// /// Failures while reading or loading manifests are recorded as markers instead of being /// returned. The normal project-loading path will diagnose those failures later with its full -/// context. +/// context. A private source manager keeps fingerprinting from interning manifests in the +/// compilation session's source manager as a side effect. pub(crate) fn fingerprint( options: &Options, project_dir: &Path, - source_manager: &dyn SourceManager, inherited_rustflags: Option<&OsStr>, + inherited_rustup_toolchain: Option<&OsStr>, compiler_version: &str, compiler_revision: &str, ) -> String { let mut transcript = Transcript::new(); transcript.field("compiler.version", compiler_version.as_bytes()); transcript.field("compiler.revision", compiler_revision.as_bytes()); - record_options(&mut transcript, options, inherited_rustflags); + record_options(&mut transcript, options, inherited_rustflags, inherited_rustup_toolchain); - let mut manifests = ManifestClosure::new(&mut transcript, source_manager); + let source_manager = DefaultSourceManager::default(); + let mut manifests = ManifestClosure::new(&mut transcript, &source_manager); manifests.visit_project(project_dir, None); let digest = Blake3_256::hash(transcript.as_bytes()); @@ -115,6 +128,7 @@ fn record_options( transcript: &mut Transcript, options: &Options, inherited_rustflags: Option<&OsStr>, + inherited_rustup_toolchain: Option<&OsStr>, ) { let Options { manifest_path: _, @@ -187,6 +201,10 @@ fn record_options( "options.inherited_rustflags", inherited_rustflags.map(OsStr::as_encoded_bytes), ); + transcript.optional_bytes_field( + "options.inherited_rustup_toolchain", + inherited_rustup_toolchain.map(OsStr::as_encoded_bytes), + ); transcript.optional_field("options.toolchain", toolchain.as_deref()); let mut link_libraries = link_libraries.iter().map(link_library_input).collect::>(); @@ -275,8 +293,17 @@ impl<'a> ManifestClosure<'a> { } self.transcript.field("project", b"begin"); - self.record_manifest(&project_dir.join("miden-project.toml")); - self.record_manifest(&project_dir.join("Cargo.toml")); + let miden_manifest = project_dir.join("miden-project.toml"); + let cargo_manifest = project_dir.join("Cargo.toml"); + self.record_manifest(&miden_manifest); + self.record_manifest(&cargo_manifest); + if cargo_manifest.is_file() && !miden_manifest.is_file() { + log::debug!( + target: "package-cache", + "Cargo-only project '{}' has no sibling miden-project.toml; fingerprinting records its root manifests but cannot recurse dependencies", + project_dir.display() + ); + } let project = match loaded { Ok(project) => project, @@ -294,10 +321,11 @@ impl<'a> ManifestClosure<'a> { self.transcript.field("project.load", b"succeeded"); let package = project.package(); - let workspace_root = match &project { - Project::WorkspacePackage { workspace, .. } => workspace.workspace_root(), + let workspace = match &project { + Project::WorkspacePackage { workspace, .. } => Some(workspace.as_ref()), Project::Package(_) => None, }; + let workspace_root = workspace.and_then(miden_project::Workspace::workspace_root); if let Some(workspace_root) = workspace_root { let workspace_key = canonical_or_original(workspace_root); if self.visited_workspace_roots.insert(workspace_key) { @@ -311,7 +339,7 @@ impl<'a> ManifestClosure<'a> { self.transcript .field("project.dependencies.count", &(dependencies.len() as u64).to_le_bytes()); for dependency in dependencies { - self.visit_dependency(dependency, project_dir.as_path(), workspace_root); + self.visit_dependency(dependency, project_dir.as_path(), workspace); } self.transcript.field("project", b"end"); } @@ -341,8 +369,13 @@ impl<'a> ManifestClosure<'a> { &mut self, dependency: &Dependency, manifest_dir: &Path, - workspace_root: Option<&Path>, + workspace: Option<&miden_project::Workspace>, ) { + // Keep scheme handling aligned with + // `frontend/masm/src/project.rs::collect_dependency_metadata_for_scheme`. The fingerprint + // walk intentionally differs from resolution in only two ways: path dependencies are + // extension-classified before canonicalization, so a symlink to a `.masp` is treated as + // source; and git declarations are recorded but their checkouts are never recursed. self.transcript.field("dependency", b"begin"); self.transcript.field("dependency.name", dependency.name().as_bytes()); self.transcript @@ -354,7 +387,9 @@ impl<'a> ManifestClosure<'a> { self.visit_path_dependency(dependency, manifest_dir, path.inner()); } DependencyVersionScheme::WorkspacePath { path, .. } => { - if let Some(workspace_root) = workspace_root { + if let Some(workspace_root) = + workspace.and_then(miden_project::Workspace::workspace_root) + { self.visit_path_dependency(dependency, workspace_root, path.inner()); } else { log::debug!( @@ -366,15 +401,19 @@ impl<'a> ManifestClosure<'a> { } } DependencyVersionScheme::Workspace { member, .. } => { - if let Some(workspace_root) = workspace_root { - // Unlike the canonical resolver, the shared helper below extension-classifies - // a workspace member even though `Workspace` always denotes source there. - self.visit_path_dependency(dependency, workspace_root, member.inner()); + if let Some(manifest_path) = workspace + .and_then(|workspace| { + workspace.get_member_by_relative_path(member.inner().path()) + }) + .and_then(|package| package.manifest_path().map(Path::to_path_buf)) + { + self.visit_project(&manifest_path, Some(dependency.name().as_ref())); } else { log::debug!( target: "package-cache", - "cannot resolve workspace member dependency '{}' while fingerprinting outside a workspace", - dependency.name() + "cannot resolve workspace member dependency '{}' at '{}' while fingerprinting", + dependency.name(), + member.inner().path() ); self.transcript.field("dependency.path", b"unresolved-workspace"); } @@ -406,16 +445,16 @@ impl<'a> ManifestClosure<'a> { return; } - // This mirrors `miden-project`'s `dependencies/graph.rs::resolve_dependency`; keep the - // two in sync. The fingerprint walk checks the extension before canonicalization, so a - // symlink to a `.masp` is classified as a project unlike the canonical resolver. let relative = Path::new(uri.path()); let path = if relative.is_absolute() { relative.to_path_buf() } else { base_dir.join(relative) }; - if path.extension().is_some_and(|extension| extension == "masp") { + if path + .extension() + .is_some_and(|extension| extension.eq_ignore_ascii_case(Package::EXTENSION)) + { self.record_package_file(&path); } else { self.visit_project(&path, Some(dependency.name().as_ref())); @@ -505,7 +544,6 @@ fn canonical_or_original(path: &Path) -> PathBuf { #[cfg(test)] mod tests { - use miden_debug_types::DefaultSourceManager; use tempfile::TempDir; use super::*; @@ -530,7 +568,7 @@ mod tests { /// Computes a test fingerprint with a fresh source manager. fn test_fingerprint(options: &Options, project_dir: &Path, version: &str, rev: &str) -> String { - fingerprint(options, project_dir, &DefaultSourceManager::default(), None, version, rev) + fingerprint(options, project_dir, None, None, version, rev) } #[test] @@ -650,19 +688,31 @@ mod tests { write_project(temp.path(), "root", ""); let options = Options::default(); - let missing = fingerprint( + let missing = fingerprint(&options, temp.path(), None, None, "1.2.3", "abc123"); + let present = fingerprint( &options, temp.path(), - &DefaultSourceManager::default(), + Some(OsStr::new("-C target-feature=+bulk-memory")), None, "1.2.3", "abc123", ); + + assert_ne!(missing, present); + } + + #[test] + fn fingerprint_changes_with_inherited_rustup_toolchain() { + let temp = TempDir::new().unwrap(); + write_project(temp.path(), "root", ""); + let options = Options::default(); + + let missing = fingerprint(&options, temp.path(), None, None, "1.2.3", "abc123"); let present = fingerprint( &options, temp.path(), - &DefaultSourceManager::default(), - Some(OsStr::new("-C target-feature=+bulk-memory")), + None, + Some(OsStr::new("nightly-2026-08-05")), "1.2.3", "abc123", ); @@ -670,6 +720,37 @@ mod tests { assert_ne!(missing, present); } + #[test] + fn fingerprint_resolves_workspace_members_before_classifying_paths() { + let temp = TempDir::new().unwrap(); + let dependency = temp.path().join("dep.masp"); + let application = temp.path().join("app"); + write_project(&dependency, "dep", ""); + write_project(&application, "app", "\n[dependencies]\ndep.workspace = true\n"); + std::fs::write( + temp.path().join("miden-project.toml"), + "[workspace]\nmembers = [\"dep.masp\", \"app\"]\n\n[workspace.dependencies]\ndep = { \ + path = \"dep.masp\" }\n", + ) + .unwrap(); + std::fs::write( + temp.path().join("Cargo.toml"), + "[workspace]\nmembers = [\"dep.masp\", \"app\"]\n", + ) + .unwrap(); + let options = Options::default(); + let before = test_fingerprint(&options, &application, "1.2.3", "abc123"); + + std::fs::write( + dependency.join("Cargo.toml"), + "[package]\nname = \"dep\"\nversion = \"2.0.0\"\n", + ) + .unwrap(); + let after = test_fingerprint(&options, &application, "1.2.3", "abc123"); + + assert_ne!(before, after); + } + #[test] fn fingerprint_changes_with_workspace_manifests() { let temp = TempDir::new().unwrap(); diff --git a/tools/cargo-miden/src/commands/build.rs b/tools/cargo-miden/src/commands/build.rs index cede95933..4ab3277da 100644 --- a/tools/cargo-miden/src/commands/build.rs +++ b/tools/cargo-miden/src/commands/build.rs @@ -35,6 +35,9 @@ impl BuildCommand { None => cwd.join("Cargo.toml"), }; let input = InputFile::from_path(&manifest_path).unwrap(); + // This root session is expected to name one selected package. Package-cache closure + // fingerprinting relies on workspace builds reaching this point once per selected member; + // an unselected workspace-root manifest is rejected during project preparation. let session = Rc::new( compiler_opts .into_session(input, None, None) From f82c939b7972dcaa6277abfe7a81b1c143067227 Mon Sep 17 00:00:00 2001 From: Denys Zadorozhnyi Date: Wed, 5 Aug 2026 14:29:24 +0300 Subject: [PATCH 06/21] test: pin package-cache build attribution and fingerprint rotation The cargo-miden cache tests had become tautological: the lookup helper only returned directories that already contained the expected package, and the pre-build cleanup that attributed the artifact to the build under test was removed with the fingerprint layout. The tests now snapshot the time before the build and assert the located package was written at or after it, the helper accepts only fingerprint-shaped directory names so a regression to a flat layout fails, and the masm test derives the expected file name from the dependency name instead of repeating a literal. The end-to-end FPI test previously covered only the same-fingerprint half of the design: a dependency source change rewrites the package in place and include_bytes! re-expands the consumer. A third build phase now covers the rotation half that #1302 is actually about: bumping the dependency's version in its manifests moves the cache to a new fingerprint directory, removes the old one, and the consumer's assembly carries only the new procedure root. The digest-recognition helper documents its coupling to the current u64-immediate lowering shape so a codegen change there is not misread as a stale-root regression. Also guards the swapp-note fixture mutation like the existing one, and corrects the persist_cargo_miden_dependency docs: that directory is a legacy fallback consulted only when MIDENC_PACKAGE_CACHE is unset. --- .../src/end_to_end/examples/mod.rs | 4 + tests/integration/src/sdk/mod.rs | 87 +++++++++++++++++-- tools/cargo-miden/tests/masm_dependency.rs | 16 +++- .../tests/p2id_cargo_miden_build.rs | 10 ++- tools/cargo-miden/tests/utils.rs | 10 +++ 5 files changed, 116 insertions(+), 11 deletions(-) diff --git a/tests/integration/src/end_to_end/examples/mod.rs b/tests/integration/src/end_to_end/examples/mod.rs index 73103ab5a..cd0e3010f 100644 --- a/tests/integration/src/end_to_end/examples/mod.rs +++ b/tests/integration/src/end_to_end/examples/mod.rs @@ -11,6 +11,10 @@ mod fibonacci; mod is_prime; mod storage_metadata; +/// Writes a dependency package to the legacy lookup directory used only when +/// `MIDENC_PACKAGE_CACHE` is unset. +/// +/// Compiler-driven builds set that variable and use their fingerprinted cache exclusively. fn persist_cargo_miden_dependency( project_path: impl AsRef, package: &miden_mast_package::Package, diff --git a/tests/integration/src/sdk/mod.rs b/tests/integration/src/sdk/mod.rs index a11ae679f..543f7b6e8 100644 --- a/tests/integration/src/sdk/mod.rs +++ b/tests/integration/src/sdk/mod.rs @@ -32,7 +32,11 @@ pub(crate) fn note_script_program( .unwrap() } -/// Writes a compiled package where `miden::generate!` expects Cargo Miden dependency artifacts. +/// Writes a compiled package to the legacy fallback used by `miden::generate!` only when +/// `MIDENC_PACKAGE_CACHE` is unset. +/// +/// When that variable is set, the macro searches only the fingerprinted cache populated by the +/// enclosing compiler-driven build and does not consult `target/miden/release`. fn persist_cargo_miden_dependency( project_path: impl AsRef, package: &miden_mast_package::Package, @@ -210,12 +214,18 @@ basic-wallet = { path = "../basic-wallet" } [package.metadata.miden.dependencies] basic-wallet = { wit = "../basic-wallet/target/generated-wit/" } "#; - let swapp_note_source = include_str!(concat!( + let original_swapp_note_source = include_str!(concat!( env!("CARGO_MANIFEST_DIR"), "/../fixtures/components/swapp-note/src/lib.rs" - )) - .replacen( - " let offered_asset = ¬e_assets[0];", + )); + let swapp_note_mutation = " let offered_asset = ¬e_assets[0];"; + assert_eq!( + original_swapp_note_source.matches(swapp_note_mutation).count(), + 1, + "the swapp-note fixture mutation must match exactly once" + ); + let swapp_note_source = original_swapp_note_source.replacen( + swapp_note_mutation, " let offered_asset = ¬e_assets[0];\n let foreign_wallet = \ Wallet::new(self.creator);\n foreign_wallet.receive_asset(*offered_asset);", 1, @@ -279,6 +289,10 @@ fn procedure_digest_felts(digest: &miden_core::Word) -> [u64; 4] { } /// Returns true when MASM reconstructs every felt in `digest` from its decimal `u32` limbs. +/// +/// This intentionally follows the current lowering shape for `u64` immediates. If codegen changes +/// that sequence, this helper can report a stale-root failure even when the embedded digest is +/// current, so update the recognizer alongside such a lowering change. fn masm_contains_procedure_digest(masm: &str, digest: &miden_core::Word) -> bool { let pattern = procedure_digest_felts(digest) .into_iter() @@ -576,6 +590,8 @@ fn rust_sdk_fpi_reexpands_after_dependency_package_changes() { let project = fpi_package_cache_regression_project(); let consumer = project.root().join("swapp-note"); let dependency_source = project.root().join("basic-wallet/src/lib.rs"); + let dependency_cargo_manifest = project.root().join("basic-wallet/Cargo.toml"); + let dependency_miden_manifest = project.root().join("basic-wallet/miden-project.toml"); let config = WasmTranslationConfig::default(); let mut first_build = CompilerTest::rust_source_cargo_miden(&consumer, config.clone(), []); @@ -594,16 +610,20 @@ fn rust_sdk_fpi_reexpands_after_dependency_package_changes() { ); let original_source = fs::read_to_string(&dependency_source).unwrap(); + assert_eq!( + original_source.matches(" self.add_asset(asset);").count(), + 1, + "fixture mutation must match exactly once" + ); let changed_source = original_source.replacen( " self.add_asset(asset);", " self.add_asset(asset);\n self.remove_asset(asset);\n \ self.add_asset(asset);", 1, ); - assert_ne!(original_source, changed_source, "fixture mutation must match exactly once"); fs::write(&dependency_source, changed_source).unwrap(); - let mut second_build = CompilerTest::rust_source_cargo_miden(&consumer, config, []); + let mut second_build = CompilerTest::rust_source_cargo_miden(&consumer, config.clone(), []); let second_masm = second_build.masm_src(); let second_cache = second_build.session.filesystem_package_cache_dir().unwrap(); let second_dependency = read_cached_dependency_package(&second_build, "basic-wallet"); @@ -626,6 +646,59 @@ fn rust_sdk_fpi_reexpands_after_dependency_package_changes() { "consumer MASM does not contain the new receive-asset root {:?}", procedure_digest_felts(&second_root), ); + + for manifest_path in [&dependency_cargo_manifest, &dependency_miden_manifest] { + let original_manifest = fs::read_to_string(manifest_path).unwrap(); + assert_eq!( + original_manifest.matches("version = \"0.1.0\"").count(), + 1, + "expected exactly one package-version field in {}", + manifest_path.display() + ); + let mut changed_manifest = + original_manifest.replacen("version = \"0.1.0\"", "version = \"0.1.1\"", 1); + if manifest_path == &dependency_miden_manifest { + assert_eq!( + changed_manifest.matches("@0.1.0").count(), + 1, + "expected exactly one namespace version in {}", + manifest_path.display() + ); + changed_manifest = changed_manifest.replacen("@0.1.0", "@0.1.1", 1); + } + fs::write(manifest_path, changed_manifest).unwrap(); + } + + let mut third_build = CompilerTest::rust_source_cargo_miden(&consumer, config, []); + let third_masm = third_build.masm_src(); + let third_cache = third_build.session.filesystem_package_cache_dir().unwrap(); + let third_dependency = read_cached_dependency_package(&third_build, "basic-wallet"); + let third_export = + find_manifest_procedure(&third_dependency, "basic-wallet receive-asset export", |path| { + path.ends_with("::\"receive-asset\"") + }); + let third_root = third_export.digest; + + assert_ne!(second_cache, third_cache, "manifest changes must rotate the cache path"); + assert!( + !second_cache.exists(), + "the obsolete fingerprint directory must be pruned after rotation: {}", + second_cache.display() + ); + for stale_root in [first_root, second_root] { + if stale_root != third_root { + assert!( + !masm_contains_procedure_digest(&third_masm, &stale_root), + "consumer MASM still contains stale receive-asset root {:?}", + procedure_digest_felts(&stale_root), + ); + } + } + assert!( + masm_contains_procedure_digest(&third_masm, &third_root), + "consumer MASM does not contain the post-rotation receive-asset root {:?}", + procedure_digest_felts(&third_root), + ); } #[test] diff --git a/tools/cargo-miden/tests/masm_dependency.rs b/tools/cargo-miden/tests/masm_dependency.rs index a5bd430b2..f6753adcb 100644 --- a/tools/cargo-miden/tests/masm_dependency.rs +++ b/tools/cargo-miden/tests/masm_dependency.rs @@ -11,7 +11,7 @@ //! to derive a per-target role across package boundaries: the Rust root is //! `TargetRole::Root`, the MASM library is a `TargetRole::Dependency`. -use std::{env, fs, path::Path}; +use std::{env, fs, path::Path, time::SystemTime}; use cargo_miden::run; @@ -155,6 +155,7 @@ fn build_rust_project_with_masm_path_dependency() { write_library_manifest(&project_path, project_name, dependency_name, "../masm_dep"); env::set_current_dir(&project_path).unwrap(); + let build_started_at = SystemTime::now(); let result = run(["cargo", "miden", "build"].into_iter().map(|s| s.to_string())); env::set_current_dir(&restore_dir).unwrap(); @@ -174,14 +175,23 @@ fn build_rust_project_with_masm_path_dependency() { // registry, or skipped. A materialized `.masp` for the dependency is produced only by // assembling it from its Miden Assembly sources. The package cache is uniqued by the build // inputs, so the package lives inside the build's fingerprint directory. + let dependency_package_name = + format!("{dependency_name}.{}", miden_mast_package::Package::EXTENSION); let dependency_package = - crate::utils::package_cache_fingerprint_dir(&project_path, "masm-dep.masp") - .join(format!("{dependency_name}.masp")); + crate::utils::package_cache_fingerprint_dir(&project_path, &dependency_package_name) + .join(dependency_package_name); assert!( dependency_package.exists(), "expected the masm dependency to be assembled and materialized at {}", dependency_package.display() ); + let modified = dependency_package.metadata().unwrap().modified().unwrap(); + assert!( + modified >= build_started_at, + "expected this build to rewrite {}, but its modification time {modified:?} predates the \ + build start {build_started_at:?}", + dependency_package.display() + ); fs::remove_dir_all(&root).unwrap(); } diff --git a/tools/cargo-miden/tests/p2id_cargo_miden_build.rs b/tools/cargo-miden/tests/p2id_cargo_miden_build.rs index d63c96cdc..fafb82d3a 100644 --- a/tools/cargo-miden/tests/p2id_cargo_miden_build.rs +++ b/tools/cargo-miden/tests/p2id_cargo_miden_build.rs @@ -1,4 +1,4 @@ -use std::env; +use std::{env, time::SystemTime}; use cargo_miden::run; @@ -33,6 +33,7 @@ fn p2id_build_materializes_basic_wallet_dependency() { // Build the p2id-note project, which pulls in basic-wallet as a Miden dependency. let restore_dir = env::current_dir().unwrap(); env::set_current_dir(&p2id_note_dir).unwrap(); + let build_started_at = SystemTime::now(); let result = run(["cargo", "miden", "build", "--release"].into_iter().map(|s| s.to_string())); env::set_current_dir(&restore_dir).unwrap(); @@ -58,4 +59,11 @@ fn p2id_build_materializes_basic_wallet_dependency() { "expected basic-wallet dependency package to be materialized at {}", dep_package.display() ); + let modified = dep_package.metadata().unwrap().modified().unwrap(); + assert!( + modified >= build_started_at, + "expected this build to rewrite {}, but its modification time {modified:?} predates the \ + build start {build_started_at:?}", + dep_package.display() + ); } diff --git a/tools/cargo-miden/tests/utils.rs b/tools/cargo-miden/tests/utils.rs index 9a295c20b..8099fe5a6 100644 --- a/tools/cargo-miden/tests/utils.rs +++ b/tools/cargo-miden/tests/utils.rs @@ -68,6 +68,16 @@ pub(crate) fn package_cache_fingerprint_dir(project_dir: &Path, expected_package listing.push(path.display().to_string()); continue; } + // Only compiler-owned fingerprint directories are candidates; unrelated directories may + // coexist under the cache parent and must not attribute an old package to this build. + let is_fingerprint = path.file_name().and_then(|name| name.to_str()).is_some_and(|name| { + name.len() == 16 + && name.bytes().all(|byte| byte.is_ascii_digit() || (b'a'..=b'f').contains(&byte)) + }); + if !is_fingerprint { + listing.push(path.display().to_string()); + continue; + } let contents = fs::read_dir(&path) .map(|entries| { From 43f0c54d980ce09ee5e5423d367f3ab30aa087ad Mon Sep 17 00:00:00 2001 From: Denys Zadorozhnyi Date: Wed, 5 Aug 2026 16:34:12 +0300 Subject: [PATCH 07/21] fix(session): make cache liveness locking blocking and permanent, and publication atomic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The third review round found the lock protocol violating its own contract on two legs. A builder observing WouldBlock — meaning a pruner was deleting its directory at that moment — continued unlocked for the whole build, and the constructor doc claimed a lock it did not hold. And because flock binds to the inode, unlinking lock files after pruning let a builder hold a lock on an unlinked inode while the next pruner locked a fresh file at the same path and deleted the live directory. Builders now take the blocking lock_shared on their fingerprint lock: pruners only ever try exclusive locks and never wait while holding one, and a builder waits only for its own lock while holding none, so the wait is deadlock-free and bounded by one in-progress removal. Lock files become permanent rendezvous objects — empty, bounded by the number of distinct fingerprints ever seen — which removes the inode ABA together with the three orphan-lock helpers. Preparation failures keep the cache configured so the first publication reports the concrete filesystem error, and the docs now describe that degraded mode instead of contradicting it, including that ownership is checked lexically by design and symlinked cache layouts are outside the contract. Package publication writes to a process-unique temp file and renames over the target, so a concurrent identical-fingerprint build can no longer expose a truncated package to a reader; the remaining read-versus-include_bytes window is documented as part of the same-fingerprint boundary. The fingerprint gains CARGO_ENCODED_RUSTFLAGS, which takes precedence over RUSTFLAGS when Cargo invokes rustc. The cache-layout machinery moves out of registry.rs into package_cache.rs, split into guard, create, lock, and sweep helpers, and the module docs now record what the memoized derivation assumes (root-session-only use, clones keeping their fingerprint, the deliberate target-dir exclusion), why the walk cannot reuse miden-project's resolver (it needs the registry whose cache path is being derived, and graph building performs git checkouts), which ambient inputs stay unfingerprinted and why, and the intended content-addressed end-state that belongs with the #1290 package redesign. --- midenc-session/src/lib.rs | 13 +- midenc-session/src/package_cache.rs | 543 +++++++++++++++++++++++++- midenc-session/src/registry.rs | 586 ++++------------------------ 3 files changed, 622 insertions(+), 520 deletions(-) diff --git a/midenc-session/src/lib.rs b/midenc-session/src/lib.rs index cff8e24b7..73491c7f8 100644 --- a/midenc-session/src/lib.rs +++ b/midenc-session/src/lib.rs @@ -84,7 +84,7 @@ pub struct Session { /// The build-input fingerprint used to isolate this session's package cache. /// /// Memoization assumes fingerprint-relevant [`Options`] are not mutated after the first cache - /// path request. + /// path request. Cloning the session copies the memoized value when present. #[cfg(feature = "std")] package_cache_fingerprint: std::sync::OnceLock, } @@ -380,6 +380,15 @@ impl Session { /// of it. Without `std`, this returns the existing flat `target/miden/packages/` path without a /// fingerprint component. /// + /// Only the root compilation session derives this path. Nested dependency sessions receive + /// the root value threaded through their build environment, rather than deriving paths from + /// their own locators. The root is intentionally tied to the project directory and ignores + /// `--target-dir`, so every participant in one build agrees on the cache location. + /// + /// [`Session`] is [`Clone`]. Once this method has initialized the fingerprint, a clone keeps + /// that memoized value even if its public options are later changed; callers must mutate + /// fingerprint-relevant options before the first path request. + /// /// Derived from the input locator rather than from a loaded manifest, which is what /// [`Session::new`] no longer has. That is also a repair: the manifest path was previously /// taken from a package that `fixup_cargo_target` had rebuilt for every executable @@ -406,11 +415,13 @@ impl Session { { let fingerprint = self.package_cache_fingerprint.get_or_init(|| { let inherited_rustflags = std::env::var_os("RUSTFLAGS"); + let inherited_cargo_encoded_rustflags = std::env::var_os("CARGO_ENCODED_RUSTFLAGS"); let inherited_rustup_toolchain = std::env::var_os("RUSTUP_TOOLCHAIN"); package_cache::fingerprint( &self.options, &project_dir, inherited_rustflags.as_deref(), + inherited_cargo_encoded_rustflags.as_deref(), inherited_rustup_toolchain.as_deref(), MIDENC_BUILD_VERSION, MIDENC_BUILD_REV, diff --git a/midenc-session/src/package_cache.rs b/midenc-session/src/package_cache.rs index ce025da4f..040165627 100644 --- a/midenc-session/src/package_cache.rs +++ b/midenc-session/src/package_cache.rs @@ -1,11 +1,18 @@ //! Build-input fingerprints for the filesystem package cache. //! //! The fingerprint models inputs that change the *set and identity* of packages visible to a -//! build. Source files and lockfiles are deliberately excluded: every resolved package is +//! build. Source files, lockfiles, `rust-toolchain.toml`, Cargo configuration files, and compiler +//! wrappers are deliberately excluded: they are content-only inputs, every resolved package is //! rewritten into the current cache before its consumers expand, and the generated //! `include_bytes!` reference makes Cargo re-expand when that package's contents change. +//! Same-path invalidation therefore inherits Cargo's file-freshness semantics: mtime-based unless +//! checksum freshness is enabled. //! Expansions also record `MIDENC_PACKAGE_CACHE`, so rotating the fingerprinted path re-expands //! consumers even if best-effort stale-directory pruning does not complete. +//! Concurrent builds with the same fingerprint share one directory. Package publication uses a +//! temporary file followed by atomic rename, preventing consumers from reading a torn package. +//! A macro read and rustc's later `include_bytes!` evaluation can still straddle a concurrent +//! rewrite; that narrow same-fingerprint window is accepted and bounded by the every-run rewrite. //! Registry and git dependencies contribute declaration text only; in particular, a git branch //! moving without a manifest edit is outside this fingerprint by design, as are a git package's //! transitive dependencies. Pinning a revision or deleting the cache directory recovers from a @@ -19,6 +26,16 @@ //! Similarly, a Cargo-only project with no sibling `miden-project.toml` contributes both root //! manifest slots and a load-failure marker, but its dependencies cannot be discovered and are //! not recursed. That degraded case is reported at debug level while fingerprinting. +//! +//! This manifest walk cannot reuse `miden_project::ProjectDependencyGraphBuilder`: constructing +//! that resolver requires the `PackageRegistry` whose cache path is being derived, which is +//! circular, and its `build` operation may perform network git checkouts. Cache-path derivation +//! must remain local and available before the registry exists. +//! +//! The intended end state is content-addressed package storage, or immutable generation +//! directories derived from the fully resolved dependency graph. That belongs with the #1290 +//! package redesign and #1300 macro-side package pins; this fingerprint remains the conservative +//! build-input generation key until then. use alloc::{ format, @@ -28,6 +45,7 @@ use alloc::{ use std::{ collections::BTreeSet, ffi::OsStr, + fs::{self, File, OpenOptions, TryLockError}, path::{Path, PathBuf}, }; @@ -39,14 +57,267 @@ use miden_project::{Dependency, DependencyVersionScheme, Project}; use crate::{DebugInfo, LinkLibrary, OptLevel, Options}; /// The number of lowercase hexadecimal characters in a package-cache fingerprint. -pub(crate) const FINGERPRINT_LEN: usize = 16; +const FINGERPRINT_LEN: usize = 16; /// Returns true when `name` satisfies the package-cache fingerprint format. -pub(crate) fn is_fingerprint(name: &str) -> bool { +fn is_fingerprint(name: &str) -> bool { name.len() == FINGERPRINT_LEN && name.bytes().all(|byte| byte.is_ascii_digit() || (b'a'..=b'f').contains(&byte)) } +/// The extension of a permanent sibling lock file for a fingerprint directory. +const BUILD_LOCK_EXTENSION: &str = "lock"; + +/// Prepares a filesystem package cache and returns its lifetime shared lock when available. +/// +/// Deletion is defense in depth. The primary invalidation is in the FPI expansion itself: it +/// records `option_env!("MIDENC_PACKAGE_CACHE")`, whose value carries the fingerprinted cache +/// path, so Cargo re-expands consumers whenever the fingerprint rotates. Pruning still removes +/// the `include_bytes!` targets of pre-fingerprint expansions, bounds stale package directories, +/// and takes dead caches out of circulation promptly. +/// +/// A build holds a shared `packages/.lock` lock for its registry's lifetime. Pruning +/// takes the corresponding exclusive lock while deleting the sibling directory. The permanent +/// lock lives outside that directory and is acquired before the directory is created, closing the +/// create-before-lock, unlock-before-delete, and unlink/recreate inode-ABA windows. +/// +/// `None` means locking was skipped for an unowned path or preparation degraded after an error. +/// The caller deliberately keeps the cache configured so package publication can report the +/// concrete filesystem failure. +pub(crate) fn prepare_and_lock_filesystem_cache(filesystem_cache: &Path) -> Option { + if !is_owned_filesystem_cache_path(filesystem_cache) { + prepare_unowned_filesystem_cache(filesystem_cache); + return None; + } + + let parent = filesystem_cache + .parent() + .expect("an owned filesystem cache path always has a packages parent"); + if !create_filesystem_cache_parent(parent) { + return None; + } + let filesystem_cache_lock = acquire_filesystem_cache_lock(filesystem_cache)?; + if !create_current_filesystem_cache(filesystem_cache) { + return None; + } + sweep_stale_filesystem_cache_entries(filesystem_cache, parent); + Some(filesystem_cache_lock) +} + +/// Creates an unowned cache path without locking it or sweeping its parent. +fn prepare_unowned_filesystem_cache(filesystem_cache: &Path) { + if let Err(err) = fs::create_dir_all(filesystem_cache) { + log::warn!( + target: "package-registry", + "failed to create filesystem package cache '{}': {err}; keeping the cache configured so package publication reports the failure", + filesystem_cache.display() + ); + return; + } + log::debug!( + target: "package-registry", + "filesystem package cache '{}' is outside the owned miden/packages/ layout; skipping locking and parent pruning", + filesystem_cache.display() + ); +} + +/// Creates the owned cache parent before its permanent lock file is opened. +fn create_filesystem_cache_parent(parent: &Path) -> bool { + if let Err(err) = fs::create_dir_all(parent) { + log::warn!( + target: "package-registry", + "failed to create filesystem package cache parent '{}': {err}; keeping the cache configured so package publication reports the failure", + parent.display() + ); + return false; + } + true +} + +/// Creates or recreates the current cache directory after its shared lock is held. +fn create_current_filesystem_cache(filesystem_cache: &Path) -> bool { + if let Err(err) = fs::create_dir_all(filesystem_cache) { + log::warn!( + target: "package-registry", + "failed to create filesystem package cache '{}': {err}; keeping the cache configured so package publication reports the failure", + filesystem_cache.display() + ); + return false; + } + true +} + +/// Opens the current fingerprint's sibling lock file and holds a shared builder lock. +/// +/// Waiting is deadlock-free: pruners only try exclusive locks and never wait while holding one, +/// while a builder waits only for its own lock and holds no other lock. The wait is therefore +/// bounded by one in-progress stale-directory removal. +fn acquire_filesystem_cache_lock(filesystem_cache: &Path) -> Option { + let lock_path = filesystem_cache_lock_path(filesystem_cache); + let lock = match OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(&lock_path) + { + Ok(lock) => lock, + Err(err) => { + log::warn!( + target: "package-registry", + "failed to open filesystem package cache liveness lock '{}': {err}; continuing without a liveness lock", + lock_path.display() + ); + return None; + } + }; + + if let Err(err) = lock.lock_shared() { + log::warn!( + target: "package-registry", + "failed to lock filesystem package cache '{}': {err}; continuing without a liveness lock", + filesystem_cache.display() + ); + return None; + } + Some(lock) +} + +/// Removes dead fingerprint directories and legacy flat package files from `parent`. +fn sweep_stale_filesystem_cache_entries(filesystem_cache: &Path, parent: &Path) { + let entries = match fs::read_dir(parent) { + Ok(entries) => entries, + Err(err) => { + log::debug!( + target: "package-registry", + "failed to inspect filesystem package cache '{}': {err}", + parent.display() + ); + return; + } + }; + let current_lock_path = filesystem_cache_lock_path(filesystem_cache); + + for entry in entries { + let entry = match entry { + Ok(entry) => entry, + Err(err) => { + log::debug!( + target: "package-registry", + "failed to inspect an entry in filesystem package cache '{}': {err}", + parent.display() + ); + continue; + } + }; + let path = entry.path(); + if path == filesystem_cache || path == current_lock_path { + continue; + } + let file_type = match entry.file_type() { + Ok(file_type) => file_type, + Err(err) => { + log::debug!( + target: "package-registry", + "failed to inspect filesystem package cache entry '{}': {err}", + path.display() + ); + continue; + } + }; + + let is_stale_fingerprint = + file_type.is_dir() && is_package_cache_fingerprint(&entry.file_name()); + let is_legacy_package = file_type.is_file() + && path + .extension() + .is_some_and(|extension| extension.eq_ignore_ascii_case(Package::EXTENSION)); + if is_stale_fingerprint { + prune_stale_fingerprint(&path, parent); + } else if is_legacy_package && let Err(err) = fs::remove_file(&path) { + warn_prune_failure(&path, parent, &err); + } + } +} + +/// Deletes a stale fingerprint directory while holding its exclusive permanent sibling lock. +fn prune_stale_fingerprint(fingerprint_dir: &Path, parent: &Path) { + let lock_path = filesystem_cache_lock_path(fingerprint_dir); + let lock = match OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(&lock_path) + { + Ok(lock) => lock, + Err(err) => { + log::warn!( + target: "package-registry", + "cannot verify liveness of stale filesystem package cache '{}': {err}; skipping deletion", + fingerprint_dir.display() + ); + return; + } + }; + + match lock.try_lock() { + Ok(()) => { + if let Err(err) = fs::remove_dir_all(fingerprint_dir) { + warn_prune_failure(fingerprint_dir, parent, &err); + } + } + Err(TryLockError::WouldBlock) => { + log::debug!( + target: "package-registry", + "skipping live filesystem package cache '{}' during stale-cache pruning", + fingerprint_dir.display() + ) + } + Err(TryLockError::Error(err)) => { + log::warn!( + target: "package-registry", + "cannot verify liveness of stale filesystem package cache '{}': {err}; skipping deletion", + fingerprint_dir.display() + ) + } + } +} + +/// Logs a best-effort cleanup failure with the exact directory a user can remove. +fn warn_prune_failure(path: &Path, parent: &Path, err: &std::io::Error) { + log::warn!( + target: "package-registry", + "failed to prune stale filesystem package cache entry '{}': {err}; stale cache entries may survive; delete '{}' manually", + path.display(), + parent.display() + ); +} + +/// Returns the permanent sibling lock path associated with a fingerprint directory. +fn filesystem_cache_lock_path(filesystem_cache: &Path) -> PathBuf { + filesystem_cache.with_extension(BUILD_LOCK_EXTENSION) +} + +/// Returns true when a path is lexically owned by the `miden/packages/` layout. +fn is_owned_filesystem_cache_path(filesystem_cache: &Path) -> bool { + filesystem_cache.file_name().is_some_and(is_package_cache_fingerprint) + && filesystem_cache + .parent() + .and_then(Path::file_name) + .is_some_and(|name| name == OsStr::new("packages")) + && filesystem_cache + .parent() + .and_then(Path::parent) + .and_then(Path::file_name) + .is_some_and(|name| name == OsStr::new("miden")) +} + +/// Returns true when `name` has the cache fingerprint format owned by `midenc`. +fn is_package_cache_fingerprint(name: &OsStr) -> bool { + name.to_str().is_some_and(is_fingerprint) +} + /// Computes the filesystem package cache fingerprint for a project build. /// /// Failures while reading or loading manifests are recorded as markers instead of being @@ -57,6 +328,7 @@ pub(crate) fn fingerprint( options: &Options, project_dir: &Path, inherited_rustflags: Option<&OsStr>, + inherited_cargo_encoded_rustflags: Option<&OsStr>, inherited_rustup_toolchain: Option<&OsStr>, compiler_version: &str, compiler_revision: &str, @@ -64,7 +336,13 @@ pub(crate) fn fingerprint( let mut transcript = Transcript::new(); transcript.field("compiler.version", compiler_version.as_bytes()); transcript.field("compiler.revision", compiler_revision.as_bytes()); - record_options(&mut transcript, options, inherited_rustflags, inherited_rustup_toolchain); + record_options( + &mut transcript, + options, + inherited_rustflags, + inherited_cargo_encoded_rustflags, + inherited_rustup_toolchain, + ); let source_manager = DefaultSourceManager::default(); let mut manifests = ManifestClosure::new(&mut transcript, &source_manager); @@ -128,6 +406,7 @@ fn record_options( transcript: &mut Transcript, options: &Options, inherited_rustflags: Option<&OsStr>, + inherited_cargo_encoded_rustflags: Option<&OsStr>, inherited_rustup_toolchain: Option<&OsStr>, ) { let Options { @@ -151,6 +430,8 @@ fn record_options( color: _, diagnostics: _, current_dir: _, + // The cache root is intentionally tied to the project directory rather than + // `--target-dir`, so every nested build participant derives the same location. target_dir: _, output_dir: _, output_file: _, @@ -201,6 +482,10 @@ fn record_options( "options.inherited_rustflags", inherited_rustflags.map(OsStr::as_encoded_bytes), ); + transcript.optional_bytes_field( + "options.inherited_cargo_encoded_rustflags", + inherited_cargo_encoded_rustflags.map(OsStr::as_encoded_bytes), + ); transcript.optional_bytes_field( "options.inherited_rustup_toolchain", inherited_rustup_toolchain.map(OsStr::as_encoded_bytes), @@ -544,10 +829,232 @@ fn canonical_or_original(path: &Path) -> PathBuf { #[cfg(test)] mod tests { + use std::{fs::OpenOptions, sync::mpsc, time::Duration}; + use tempfile::TempDir; use super::*; + #[test] + fn creating_a_filesystem_cache_prunes_only_stale_owned_entries() { + let temp = TempDir::new().unwrap(); + let parent = temp.path().join("miden").join("packages"); + let current = parent.join("fedcba9876543210"); + let stale = parent.join("0123456789abcdef"); + let unrelated_directory = parent.join("not-a-midenc-cache"); + let uppercase_directory = parent.join("ABCDEF0123456789"); + let legacy_package = parent.join("legacy.masp"); + let uppercase_legacy_package = parent.join("uppercase.MASP"); + let permanent_orphan_lock = parent.join("1111111111111111.lock"); + let live_precreation_lock_path = parent.join("2222222222222222.lock"); + let unrelated_file = parent.join("keep.txt"); + + for directory in [¤t, &stale, &unrelated_directory, &uppercase_directory] { + fs::create_dir_all(directory).unwrap(); + } + let current_marker = current.join("keep"); + fs::write(¤t_marker, b"current").unwrap(); + fs::write(stale.join("old.masp"), b"stale").unwrap(); + fs::write(&legacy_package, b"legacy").unwrap(); + fs::write(&uppercase_legacy_package, b"legacy").unwrap(); + fs::write(&permanent_orphan_lock, b"").unwrap(); + let live_precreation_lock = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(&live_precreation_lock_path) + .unwrap(); + live_precreation_lock.try_lock_shared().unwrap(); + fs::write(&unrelated_file, b"unrelated").unwrap(); + + let current_lock = + prepare_and_lock_filesystem_cache(¤t).expect("current cache must be locked"); + + assert!(current_marker.exists(), "the current cache must remain intact"); + assert!(!stale.exists(), "a stale fingerprint directory must be removed"); + assert!( + filesystem_cache_lock_path(&stale).exists(), + "the stale fingerprint's rendezvous lock must remain permanent" + ); + assert!(!legacy_package.exists(), "a legacy flat package must be removed"); + assert!( + !uppercase_legacy_package.exists(), + "legacy package extensions must be matched case-insensitively" + ); + assert!( + permanent_orphan_lock.exists(), + "an orphan fingerprint lock is a permanent rendezvous object" + ); + assert!( + live_precreation_lock_path.exists(), + "a lock held before its directory is created must remain permanent" + ); + assert!(unrelated_directory.exists(), "unowned directories must be retained"); + assert!(uppercase_directory.exists(), "non-lowercase directories must be retained"); + assert!(unrelated_file.exists(), "unowned files must be retained"); + + drop(live_precreation_lock); + drop(current_lock); + } + + #[test] + fn builder_waits_for_an_in_progress_prune_and_recreates_its_cache() { + let temp = TempDir::new().unwrap(); + let current = temp.path().join("miden").join("packages").join("fedcba9876543210"); + fs::create_dir_all(¤t).unwrap(); + let lock_path = filesystem_cache_lock_path(¤t); + let exclusive_lock = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(&lock_path) + .unwrap(); + exclusive_lock.try_lock().unwrap(); + fs::remove_dir_all(¤t).unwrap(); + + let (started_tx, started_rx) = mpsc::channel(); + let (completed_tx, completed_rx) = mpsc::channel(); + let thread_cache = current.clone(); + let builder = std::thread::spawn(move || { + started_tx.send(()).unwrap(); + completed_tx.send(prepare_and_lock_filesystem_cache(&thread_cache)).unwrap(); + }); + + started_rx.recv_timeout(Duration::from_secs(1)).unwrap(); + assert!( + completed_rx.recv_timeout(Duration::from_millis(100)).is_err(), + "the builder must wait while the pruner holds the exclusive lock" + ); + drop(exclusive_lock); + + let builder_lock = completed_rx + .recv_timeout(Duration::from_secs(5)) + .unwrap() + .expect("the builder must acquire a shared lock after pruning completes"); + builder.join().unwrap(); + assert!(current.is_dir(), "the builder must recreate the pruned cache directory"); + let exclusive_contender = + OpenOptions::new().read(true).write(true).open(lock_path).unwrap(); + assert!(matches!(exclusive_contender.try_lock(), Err(TryLockError::WouldBlock))); + drop(builder_lock); + } + + #[test] + fn live_stale_fingerprint_survives_until_its_lock_is_released() { + let temp = TempDir::new().unwrap(); + let parent = temp.path().join("miden").join("packages"); + let current = parent.join("fedcba9876543210"); + let stale = parent.join("0123456789abcdef"); + fs::create_dir_all(&stale).unwrap(); + + let stale_lock_path = filesystem_cache_lock_path(&stale); + let stale_lock = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(&stale_lock_path) + .unwrap(); + stale_lock.try_lock_shared().unwrap(); + + let current_lock = + prepare_and_lock_filesystem_cache(¤t).expect("current cache must be locked"); + assert!(filesystem_cache_lock_path(¤t).exists()); + let current_contender = OpenOptions::new() + .read(true) + .write(true) + .open(filesystem_cache_lock_path(¤t)) + .unwrap(); + current_contender.try_lock_shared().unwrap(); + assert!(stale.exists(), "a live sibling cache must not be pruned"); + + drop(stale_lock); + let second_lock = prepare_and_lock_filesystem_cache(¤t) + .expect("same-input builders share the lock"); + assert!(!stale.exists(), "the stale cache must be pruned after its build exits"); + assert!(stale_lock_path.exists(), "the stale sibling lock must remain permanent"); + + drop(second_lock); + drop(current_lock); + } + + #[test] + fn same_fingerprint_contender_remains_live_after_first_builder_exits() { + let temp = TempDir::new().unwrap(); + let parent = temp.path().join("miden").join("packages"); + let shared = parent.join("fedcba9876543210"); + let different = parent.join("0123456789abcdef"); + + let first = + prepare_and_lock_filesystem_cache(&shared).expect("first builder must lock the cache"); + let contender = prepare_and_lock_filesystem_cache(&shared) + .expect("same-input contender must share the lock"); + drop(first); + + let different_lock = prepare_and_lock_filesystem_cache(&different) + .expect("different-input builder must lock its cache"); + + assert!(shared.exists(), "the live contender's cache must not be pruned"); + + drop(different_lock); + drop(contender); + } + + #[test] + fn cache_creation_failure_does_not_sweep_siblings() { + let temp = TempDir::new().unwrap(); + let parent = temp.path().join("miden").join("packages"); + let current = parent.join("fedcba9876543210"); + let stale = parent.join("0123456789abcdef"); + fs::create_dir_all(&stale).unwrap(); + fs::write(¤t, b"not a directory").unwrap(); + + let lock = prepare_and_lock_filesystem_cache(¤t); + + assert!(lock.is_none()); + assert!(stale.exists(), "siblings must survive when the current cache cannot be created"); + } + + #[test] + fn arbitrary_cache_path_cannot_sweep_its_parent() { + let temp = TempDir::new().unwrap(); + let parent = temp.path().join("arbitrary-parent"); + let current = parent.join("cache"); + let fingerprint_sibling = parent.join("0123456789abcdef"); + let package_sibling = parent.join("unrelated.masp"); + fs::create_dir_all(&fingerprint_sibling).unwrap(); + fs::write(&package_sibling, b"unrelated").unwrap(); + + let lock = prepare_and_lock_filesystem_cache(¤t); + + assert!(lock.is_none()); + assert!(current.is_dir(), "an arbitrary cache path is still created"); + assert!(!filesystem_cache_lock_path(¤t).exists()); + assert!(fingerprint_sibling.exists()); + assert!(package_sibling.exists()); + } + + #[test] + fn fingerprint_name_outside_owned_layout_cannot_sweep_its_parent() { + let temp = TempDir::new().unwrap(); + let parent = temp.path().join("shared"); + let current = parent.join("fedcba9876543210"); + let fingerprint_sibling = parent.join("0123456789abcdef"); + let package_sibling = parent.join("unrelated.masp"); + fs::create_dir_all(&fingerprint_sibling).unwrap(); + fs::write(&package_sibling, b"unrelated").unwrap(); + + let lock = prepare_and_lock_filesystem_cache(¤t); + + assert!(lock.is_none()); + assert!(current.is_dir(), "an out-of-layout cache path is still created"); + assert!(!filesystem_cache_lock_path(¤t).exists()); + assert!(fingerprint_sibling.exists()); + assert!(package_sibling.exists()); + } + /// Writes a minimal Miden project and Cargo manifest to `dir`. fn write_project(dir: &Path, name: &str, dependencies: &str) { std::fs::create_dir_all(dir).unwrap(); @@ -568,7 +1075,7 @@ mod tests { /// Computes a test fingerprint with a fresh source manager. fn test_fingerprint(options: &Options, project_dir: &Path, version: &str, rev: &str) -> String { - fingerprint(options, project_dir, None, None, version, rev) + fingerprint(options, project_dir, None, None, None, version, rev) } #[test] @@ -688,12 +1195,33 @@ mod tests { write_project(temp.path(), "root", ""); let options = Options::default(); - let missing = fingerprint(&options, temp.path(), None, None, "1.2.3", "abc123"); + let missing = fingerprint(&options, temp.path(), None, None, None, "1.2.3", "abc123"); let present = fingerprint( &options, temp.path(), Some(OsStr::new("-C target-feature=+bulk-memory")), None, + None, + "1.2.3", + "abc123", + ); + + assert_ne!(missing, present); + } + + #[test] + fn fingerprint_changes_with_inherited_cargo_encoded_rustflags() { + let temp = TempDir::new().unwrap(); + write_project(temp.path(), "root", ""); + let options = Options::default(); + + let missing = fingerprint(&options, temp.path(), None, None, None, "1.2.3", "abc123"); + let present = fingerprint( + &options, + temp.path(), + None, + Some(OsStr::new("-Ctarget-feature=+bulk-memory\u{1f}--cfg=fixture")), + None, "1.2.3", "abc123", ); @@ -707,11 +1235,12 @@ mod tests { write_project(temp.path(), "root", ""); let options = Options::default(); - let missing = fingerprint(&options, temp.path(), None, None, "1.2.3", "abc123"); + let missing = fingerprint(&options, temp.path(), None, None, None, "1.2.3", "abc123"); let present = fingerprint( &options, temp.path(), None, + None, Some(OsStr::new("nightly-2026-08-05")), "1.2.3", "abc123", diff --git a/midenc-session/src/registry.rs b/midenc-session/src/registry.rs index 64300a974..a47a46e8f 100644 --- a/midenc-session/src/registry.rs +++ b/midenc-session/src/registry.rs @@ -40,6 +40,10 @@ pub struct HybridPackageRegistry { artifacts: FxHashMap>>, #[cfg(any(test, feature = "std"))] filesystem_cache: Option, + /// Holds the current fingerprint's shared liveness lock for the registry's lifetime. + /// + /// The file is never read; dropping it releases this cache directory for pruning. `None` + /// means locking was intentionally skipped or cache preparation degraded after an error. #[cfg(any(test, feature = "std"))] filesystem_cache_lock: Option, } @@ -69,14 +73,16 @@ impl HybridPackageRegistry { /// Get a new instance of the registry, using the current compiler options and an optional /// filesystem cache directory. /// - /// A cache path in the owned `miden/packages/` layout is created and locked for - /// the registry's lifetime. During construction, dead sibling fingerprint directories and - /// legacy flat `.masp` entries are pruned as defense in depth; FPI expansions track the cache - /// path themselves for correctness. Any other path is created but deliberately neither locked - /// nor used to sweep its parent. + /// A cache path in the owned `miden/packages/` layout is prepared and, when that + /// succeeds, locked for the registry's lifetime. During construction, dead sibling fingerprint + /// directories and legacy flat `.masp` entries are pruned as defense in depth; FPI expansions + /// track the cache path themselves for correctness. Any other path is created if possible but + /// deliberately neither locked nor used to sweep its parent. Ownership is checked lexically by + /// design; symlinked cache layouts are outside this contract. /// - /// Cleanup is best-effort and its failures are reported only through the `package-registry` - /// log target. Package insertion failures are still returned to the caller. + /// Cache preparation and cleanup failures are reported only through the `package-registry` log + /// target. The configured cache remains enabled after a preparation failure, so the first + /// package publication reports the concrete filesystem error to the caller. #[cfg(any(test, feature = "std"))] pub fn new_with_filesystem_cache( options: &crate::Options, @@ -84,7 +90,9 @@ impl HybridPackageRegistry { ) -> Result { use alloc::string::ToString; - let filesystem_cache_lock = filesystem_cache.as_deref().and_then(prepare_filesystem_cache); + let filesystem_cache_lock = filesystem_cache + .as_deref() + .and_then(crate::package_cache::prepare_and_lock_filesystem_cache); // Load system libraries let mut registry = if options.sysroot.is_some() { @@ -186,7 +194,7 @@ impl HybridPackageRegistry { #[cfg(any(test, feature = "std"))] if let Some(filesystem_cache) = self.filesystem_cache.as_deref() { - package.write_masp_file(filesystem_cache).map_err(|err| { + write_package_atomically(&package, filesystem_cache).map_err(|err| { InstallPackageError::FilesystemCacheInsertion { package: package.name.clone(), err, @@ -222,336 +230,44 @@ impl HybridPackageRegistry { } } -/// The extension of a sibling lock file that keeps a fingerprint directory live. -#[cfg(any(test, feature = "std"))] -const BUILD_LOCK_EXTENSION: &str = "lock"; - -/// Creates and locks the current cache directory, then removes dead stale entries owned by -/// `midenc`. -/// -/// Deletion is defense in depth. The primary invalidation is in the FPI expansion itself: it -/// records `option_env!("MIDENC_PACKAGE_CACHE")`, whose value carries the fingerprinted cache -/// path, so Cargo re-expands consumers whenever the fingerprint rotates — even when a stale -/// directory survives here. Pruning still removes the `include_bytes!` targets of expansions -/// made by pre-fingerprint macro versions (the legacy flat files), keeps the parent directory -/// bounded, and takes dead caches out of circulation promptly. -/// -/// A build holds a shared `packages/.lock` lock for its registry's lifetime. Pruning -/// requires the corresponding exclusive lock and holds it while deleting the sibling fingerprint -/// directory, so every live same-input build protects that directory. The lock lives outside the -/// deletable directory and is acquired before that directory is created, closing both prior -/// create-before-lock and unlock-before-delete windows. -/// -/// After deletion the pruner closes and removes the now-orphaned lock file. A builder can acquire -/// that file between close and unlink; this residual unlink race is accepted because pruning is -/// hygiene-level defense in depth, while `option_env!("MIDENC_PACKAGE_CACHE")` in FPI expansions -/// provides the correctness boundary. Legacy flat `.masp` files have no lock and retain the -/// accepted one-time race with a pre-fingerprint compiler. Cleanup remains best-effort so it -/// cannot obscure the current build's own diagnostics; package writes still report their failures -/// normally. +/// Publishes `package` into `filesystem_cache` with an atomic replacement of the final path. #[cfg(any(test, feature = "std"))] -fn prepare_filesystem_cache(filesystem_cache: &std::path::Path) -> Option { - if !is_owned_filesystem_cache_path(filesystem_cache) { - if let Err(err) = std::fs::create_dir_all(filesystem_cache) { - log::warn!( - target: "package-registry", - "failed to create filesystem package cache '{}': {err}; skipping cache preparation", - filesystem_cache.display() - ); - return None; - } - log::debug!( - target: "package-registry", - "filesystem package cache '{}' is outside the owned miden/packages/ layout; skipping locking and parent pruning", - filesystem_cache.display() - ); - return None; - } - - let parent = filesystem_cache - .parent() - .expect("an owned filesystem cache path always has a packages parent"); - if let Err(err) = std::fs::create_dir_all(parent) { - log::warn!( - target: "package-registry", - "failed to create filesystem package cache parent '{}': {err}; skipping cache preparation", - parent.display() - ); - return None; - } - let filesystem_cache_lock = acquire_filesystem_cache_lock(filesystem_cache)?; - if let Err(err) = std::fs::create_dir_all(filesystem_cache) { - log::warn!( - target: "package-registry", - "failed to create filesystem package cache '{}': {err}; skipping cache preparation", - filesystem_cache.display() - ); - return None; - } - - let entries = match std::fs::read_dir(parent) { - Ok(entries) => entries, - Err(err) => { - log::debug!( - target: "package-registry", - "failed to inspect filesystem package cache '{}': {err}", - parent.display() - ); - return Some(filesystem_cache_lock); - } +fn write_package_atomically( + package: &Package, + filesystem_cache: &std::path::Path, +) -> std::io::Result<()> { + use std::{ + ffi::OsString, + io::{Error, ErrorKind}, + sync::atomic::{AtomicU64, Ordering}, }; - let current_lock_path = filesystem_cache_lock_path(filesystem_cache); - - for entry in entries { - let entry = match entry { - Ok(entry) => entry, - Err(err) => { - log::debug!( - target: "package-registry", - "failed to inspect an entry in filesystem package cache '{}': {err}", - parent.display() - ); - continue; - } - }; - let path = entry.path(); - if path == filesystem_cache || path == current_lock_path { - continue; - } - let file_type = match entry.file_type() { - Ok(file_type) => file_type, - Err(err) => { - log::debug!( - target: "package-registry", - "failed to inspect filesystem package cache entry '{}': {err}", - path.display() - ); - continue; - } - }; - let is_stale_fingerprint = - file_type.is_dir() && is_package_cache_fingerprint(&entry.file_name()); - let is_legacy_package = file_type.is_file() - && path - .extension() - .is_some_and(|extension| extension.eq_ignore_ascii_case(Package::EXTENSION)); - if is_stale_fingerprint { - prune_stale_fingerprint(&path, parent); - } else if is_legacy_package { - if let Err(err) = std::fs::remove_file(&path) { - warn_prune_failure(&path, parent, &err); - } - } else if file_type.is_file() - && let Some(fingerprint) = package_cache_fingerprint_from_lock(&entry.file_name()) - && !parent.join(fingerprint).is_dir() - { - prune_orphaned_fingerprint_lock(&path, parent); - } - } - - Some(filesystem_cache_lock) -} - -/// Opens the current fingerprint's sibling lock file and tries to hold a shared builder lock. -#[cfg(any(test, feature = "std"))] -fn acquire_filesystem_cache_lock(filesystem_cache: &std::path::Path) -> Option { - use std::fs::{OpenOptions, TryLockError}; - - let lock_path = filesystem_cache_lock_path(filesystem_cache); - let lock = match OpenOptions::new() - .read(true) - .write(true) - .create(true) - .truncate(false) - .open(&lock_path) - { - Ok(lock) => lock, - Err(err) => { - log::warn!( - target: "package-registry", - "failed to open filesystem package cache liveness lock '{}': {err}", - lock_path.display() - ); - return None; - } - }; - - match lock.try_lock_shared() { - Ok(()) => Some(lock), - Err(TryLockError::WouldBlock) => { - log::debug!( - target: "package-registry", - "filesystem package cache '{}' is being pruned; skipping cache preparation", - filesystem_cache.display() - ); - None - } - Err(TryLockError::Error(err)) => { - log::warn!( - target: "package-registry", - "failed to lock filesystem package cache '{}': {err}", - filesystem_cache.display() - ); - None - } - } -} - -/// Deletes a stale fingerprint directory while holding its exclusive sibling lock. -#[cfg(any(test, feature = "std"))] -fn prune_stale_fingerprint(fingerprint_dir: &std::path::Path, parent: &std::path::Path) { - use std::fs::{OpenOptions, TryLockError}; - - let lock_path = filesystem_cache_lock_path(fingerprint_dir); - let lock = match OpenOptions::new() - .read(true) - .write(true) - .create(true) - .truncate(false) - .open(&lock_path) - { - Ok(lock) => lock, - Err(err) => { - log::warn!( - target: "package-registry", - "cannot verify liveness of stale filesystem package cache '{}': {err}; skipping deletion", - fingerprint_dir.display() - ); - return; - } - }; - - match lock.try_lock() { - Ok(()) => { - if let Err(err) = std::fs::remove_dir_all(fingerprint_dir) { - warn_prune_failure(fingerprint_dir, parent, &err); - return; - } - drop(lock); - remove_orphaned_lock_file(&lock_path, fingerprint_dir, parent); - } - Err(TryLockError::WouldBlock) => { - log::debug!( - target: "package-registry", - "skipping live filesystem package cache '{}' during stale-cache pruning", - fingerprint_dir.display() - ) - } - Err(TryLockError::Error(err)) => { - log::warn!( - target: "package-registry", - "cannot verify liveness of stale filesystem package cache '{}': {err}; skipping deletion", - fingerprint_dir.display() - ) - } - } -} - -/// Deletes an orphaned sibling lock after verifying that no builder holds it. -#[cfg(any(test, feature = "std"))] -fn prune_orphaned_fingerprint_lock(lock_path: &std::path::Path, parent: &std::path::Path) { - use std::{fs::TryLockError, io::ErrorKind}; - - let lock = match std::fs::File::open(lock_path) { - Ok(lock) => lock, - Err(err) if err.kind() == ErrorKind::NotFound => return, - Err(err) => { - log::warn!( - target: "package-registry", - "cannot verify liveness of orphaned filesystem package cache lock '{}': {err}; skipping deletion", - lock_path.display() - ); - return; - } - }; - match lock.try_lock() { - Ok(()) => { - drop(lock); - let fingerprint_dir = lock_path.with_extension(""); - remove_orphaned_lock_file(lock_path, &fingerprint_dir, parent); - } - Err(TryLockError::WouldBlock) => { - log::debug!( - target: "package-registry", - "skipping live filesystem package cache lock '{}' during orphan cleanup", - lock_path.display() - ) - } - Err(TryLockError::Error(err)) => { - log::warn!( - target: "package-registry", - "cannot verify liveness of orphaned filesystem package cache lock '{}': {err}; skipping deletion", - lock_path.display() - ) - } - } -} + static NEXT_TEMP_ID: AtomicU64 = AtomicU64::new(0); -/// Removes an unlocked lock file if its fingerprint directory remains absent. -#[cfg(any(test, feature = "std"))] -fn remove_orphaned_lock_file( - lock_path: &std::path::Path, - fingerprint_dir: &std::path::Path, - parent: &std::path::Path, -) { - use std::io::ErrorKind; - - if fingerprint_dir.exists() { - return; - } - if let Err(err) = std::fs::remove_file(lock_path) - && err.kind() != ErrorKind::NotFound - { - warn_prune_failure(lock_path, parent, &err); + let package_name: &str = &package.name; + let destination = filesystem_cache.join(package_name).with_extension(Package::EXTENSION); + let destination_name = destination.file_name().ok_or_else(|| { + Error::new( + ErrorKind::InvalidInput, + format!("package cache destination '{}' has no file name", destination.display()), + ) + })?; + let mut temp_name = OsString::from("."); + temp_name.push(destination_name); + temp_name.push(format!( + ".tmp-{}-{}", + std::process::id(), + NEXT_TEMP_ID.fetch_add(1, Ordering::Relaxed) + )); + let temp_path = destination.with_file_name(temp_name); + + let result = package + .write_to_file(&temp_path) + .and_then(|()| std::fs::rename(&temp_path, &destination)); + if result.is_err() { + let _ = std::fs::remove_file(&temp_path); } -} - -/// Logs a best-effort cleanup failure with the exact directory a user can remove. -#[cfg(any(test, feature = "std"))] -fn warn_prune_failure(path: &std::path::Path, parent: &std::path::Path, err: &std::io::Error) { - log::warn!( - target: "package-registry", - "failed to prune stale filesystem package cache entry '{}': {err}; stale cache entries may survive; delete '{}' manually", - path.display(), - parent.display() - ); -} - -/// Returns the sibling lock path associated with a fingerprint directory. -#[cfg(any(test, feature = "std"))] -fn filesystem_cache_lock_path(filesystem_cache: &std::path::Path) -> std::path::PathBuf { - filesystem_cache.with_extension(BUILD_LOCK_EXTENSION) -} - -/// Returns true when a path is owned by the `miden/packages/` cache layout. -#[cfg(any(test, feature = "std"))] -fn is_owned_filesystem_cache_path(filesystem_cache: &std::path::Path) -> bool { - use std::ffi::OsStr; - - filesystem_cache.file_name().is_some_and(is_package_cache_fingerprint) - && filesystem_cache - .parent() - .and_then(std::path::Path::file_name) - .is_some_and(|name| name == OsStr::new("packages")) - && filesystem_cache - .parent() - .and_then(std::path::Path::parent) - .and_then(std::path::Path::file_name) - .is_some_and(|name| name == OsStr::new("miden")) -} - -/// Extracts a fingerprint from an owned sibling `.lock` filename. -#[cfg(any(test, feature = "std"))] -fn package_cache_fingerprint_from_lock(name: &std::ffi::OsStr) -> Option<&str> { - let fingerprint = name.to_str()?.strip_suffix(".lock")?; - crate::package_cache::is_fingerprint(fingerprint).then_some(fingerprint) -} - -/// Returns true when `name` has the cache fingerprint format owned by `midenc`. -#[cfg(any(test, feature = "std"))] -fn is_package_cache_fingerprint(name: &std::ffi::OsStr) -> bool { - name.to_str().is_some_and(crate::package_cache::is_fingerprint) + result } impl HybridPackageRegistry { @@ -655,6 +371,14 @@ mod tests { b"damaged", "an accepted same-digest install must repair the cached package" ); + assert!( + std::fs::read_dir(cached_package.parent().unwrap()).unwrap().all(|entry| !entry + .unwrap() + .file_name() + .to_string_lossy() + .contains(".tmp-")), + "successful publication must not leave its temporary file behind" + ); let mut conflict = (*crate::LinkLibrary::tx_kernel().load(&options).unwrap()).clone(); conflict.name = package.name.clone(); @@ -667,67 +391,25 @@ mod tests { Err(InstallPackageError::AlreadyInstalledWithDifferentDigest { .. }) )); assert_eq!( - std::fs::read(cached_package).unwrap(), + std::fs::read(&cached_package).unwrap(), b"keep-on-conflict", "a rejected install must not touch the cached package" ); - } - #[test] - fn creating_a_filesystem_cache_prunes_only_stale_owned_entries() { - let temp = TempDir::new().unwrap(); - let parent = temp.path().join("miden").join("packages"); - let current = parent.join("fedcba9876543210"); - let stale = parent.join("0123456789abcdef"); - let unrelated_directory = parent.join("not-a-midenc-cache"); - let uppercase_directory = parent.join("ABCDEF0123456789"); - let legacy_package = parent.join("legacy.masp"); - let uppercase_legacy_package = parent.join("uppercase.MASP"); - let orphan_lock = parent.join("1111111111111111.lock"); - let live_precreation_lock_path = parent.join("2222222222222222.lock"); - let unrelated_file = parent.join("keep.txt"); - - for directory in [¤t, &stale, &unrelated_directory, &uppercase_directory] { - std::fs::create_dir_all(directory).unwrap(); - } - let current_marker = current.join("keep"); - std::fs::write(¤t_marker, b"current").unwrap(); - std::fs::write(stale.join("old.masp"), b"stale").unwrap(); - std::fs::write(&legacy_package, b"legacy").unwrap(); - std::fs::write(&uppercase_legacy_package, b"legacy").unwrap(); - std::fs::write(&orphan_lock, b"").unwrap(); - let live_precreation_lock = OpenOptions::new() - .read(true) - .write(true) - .create(true) - .truncate(false) - .open(&live_precreation_lock_path) - .unwrap(); - live_precreation_lock.try_lock_shared().unwrap(); - std::fs::write(&unrelated_file, b"unrelated").unwrap(); - - let current_lock = - prepare_filesystem_cache(¤t).expect("current cache must be locked"); - - assert!(current_marker.exists(), "the current cache must remain intact"); - assert!(!stale.exists(), "a stale fingerprint directory must be removed"); - assert!(!filesystem_cache_lock_path(&stale).exists(), "the stale lock must be removed"); - assert!(!legacy_package.exists(), "a legacy flat package must be removed"); - assert!( - !uppercase_legacy_package.exists(), - "legacy package extensions must be matched case-insensitively" - ); - assert!(!orphan_lock.exists(), "an unlocked orphan fingerprint lock must be removed"); + std::fs::remove_file(&cached_package).unwrap(); + std::fs::create_dir(&cached_package).unwrap(); + assert!(matches!( + registry.install_if_missing(Arc::clone(&package)), + Err(InstallPackageError::FilesystemCacheInsertion { .. }) + )); assert!( - live_precreation_lock_path.exists(), - "a lock held before its directory is created must survive orphan cleanup" + std::fs::read_dir(cached_package.parent().unwrap()).unwrap().all(|entry| !entry + .unwrap() + .file_name() + .to_string_lossy() + .contains(".tmp-")), + "failed publication must clean up its temporary file" ); - assert!(unrelated_directory.exists(), "unowned directories must be retained"); - assert!(uppercase_directory.exists(), "non-lowercase directories must be retained"); - assert!(unrelated_file.exists(), "unowned files must be retained"); - - drop(live_precreation_lock); - drop(current_lock); } #[test] @@ -744,130 +426,10 @@ mod tests { assert_eq!(registry.filesystem_cache_dir(), Some(current.as_path())); assert!(current.is_dir()); assert!(registry.filesystem_cache_lock.is_some()); - let contender = OpenOptions::new() - .read(true) - .write(true) - .open(filesystem_cache_lock_path(¤t)) - .unwrap(); + let lock_path = current.with_extension("lock"); + let contender = OpenOptions::new().read(true).write(true).open(&lock_path).unwrap(); contender.try_lock_shared().unwrap(); - let stale_checker = OpenOptions::new() - .read(true) - .write(true) - .open(filesystem_cache_lock_path(¤t)) - .unwrap(); + let stale_checker = OpenOptions::new().read(true).write(true).open(lock_path).unwrap(); assert!(matches!(stale_checker.try_lock(), Err(std::fs::TryLockError::WouldBlock))); } - - #[test] - fn live_stale_fingerprint_survives_until_its_lock_is_released() { - let temp = TempDir::new().unwrap(); - let parent = temp.path().join("miden").join("packages"); - let current = parent.join("fedcba9876543210"); - let stale = parent.join("0123456789abcdef"); - std::fs::create_dir_all(&stale).unwrap(); - - let stale_lock_path = filesystem_cache_lock_path(&stale); - let stale_lock = OpenOptions::new() - .read(true) - .write(true) - .create(true) - .truncate(false) - .open(&stale_lock_path) - .unwrap(); - stale_lock.try_lock_shared().unwrap(); - - let current_lock = - prepare_filesystem_cache(¤t).expect("current cache must be locked"); - assert!(filesystem_cache_lock_path(¤t).exists()); - let current_contender = OpenOptions::new() - .read(true) - .write(true) - .open(filesystem_cache_lock_path(¤t)) - .unwrap(); - current_contender.try_lock_shared().unwrap(); - assert!(stale.exists(), "a live sibling cache must not be pruned"); - - drop(stale_lock); - let second_lock = - prepare_filesystem_cache(¤t).expect("same-input builders share the lock"); - assert!(!stale.exists(), "the stale cache must be pruned after its build exits"); - assert!(!stale_lock_path.exists(), "the stale sibling lock must be removed"); - - drop(second_lock); - drop(current_lock); - } - - #[test] - fn same_fingerprint_contender_remains_live_after_first_builder_exits() { - let temp = TempDir::new().unwrap(); - let parent = temp.path().join("miden").join("packages"); - let shared = parent.join("fedcba9876543210"); - let different = parent.join("0123456789abcdef"); - - let first = prepare_filesystem_cache(&shared).expect("first builder must lock the cache"); - let contender = - prepare_filesystem_cache(&shared).expect("same-input contender must share the lock"); - drop(first); - - let different_lock = prepare_filesystem_cache(&different) - .expect("different-input builder must lock its cache"); - - assert!(shared.exists(), "the live contender's cache must not be pruned"); - - drop(different_lock); - drop(contender); - } - - #[test] - fn cache_creation_failure_does_not_sweep_siblings() { - let temp = TempDir::new().unwrap(); - let parent = temp.path().join("miden").join("packages"); - let current = parent.join("fedcba9876543210"); - let stale = parent.join("0123456789abcdef"); - std::fs::create_dir_all(&stale).unwrap(); - std::fs::write(¤t, b"not a directory").unwrap(); - - let lock = prepare_filesystem_cache(¤t); - - assert!(lock.is_none()); - assert!(stale.exists(), "siblings must survive when the current cache cannot be created"); - } - - #[test] - fn arbitrary_cache_path_cannot_sweep_its_parent() { - let temp = TempDir::new().unwrap(); - let parent = temp.path().join("arbitrary-parent"); - let current = parent.join("cache"); - let fingerprint_sibling = parent.join("0123456789abcdef"); - let package_sibling = parent.join("unrelated.masp"); - std::fs::create_dir_all(&fingerprint_sibling).unwrap(); - std::fs::write(&package_sibling, b"unrelated").unwrap(); - - let lock = prepare_filesystem_cache(¤t); - - assert!(lock.is_none()); - assert!(current.is_dir(), "an arbitrary cache path is still created"); - assert!(!filesystem_cache_lock_path(¤t).exists()); - assert!(fingerprint_sibling.exists()); - assert!(package_sibling.exists()); - } - - #[test] - fn fingerprint_name_outside_owned_layout_cannot_sweep_its_parent() { - let temp = TempDir::new().unwrap(); - let parent = temp.path().join("shared"); - let current = parent.join("fedcba9876543210"); - let fingerprint_sibling = parent.join("0123456789abcdef"); - let package_sibling = parent.join("unrelated.masp"); - std::fs::create_dir_all(&fingerprint_sibling).unwrap(); - std::fs::write(&package_sibling, b"unrelated").unwrap(); - - let lock = prepare_filesystem_cache(¤t); - - assert!(lock.is_none()); - assert!(current.is_dir(), "an out-of-layout cache path is still created"); - assert!(!filesystem_cache_lock_path(¤t).exists()); - assert!(fingerprint_sibling.exists()); - assert!(package_sibling.exists()); - } } From edfd26f5046c777262d2cd2d71200430fc4f184b Mon Sep 17 00:00:00 2001 From: Denys Zadorozhnyi Date: Wed, 5 Aug 2026 16:34:57 +0300 Subject: [PATCH 08/21] fix(base-macros): match dependency package extensions case-insensitively MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The compiler-side cache writers and pruner match the .masp extension case-insensitively via Package::EXTENSION, but the macro-side reader filtered with a case-sensitive literal — so on default-case-insensitive filesystems the pruner would delete a Foo.MASP the reader could never have resolved. Align the reader on the same rule. --- CHANGELOG.md | 3 +++ sdk/base-macros/src/fpi.rs | 5 ++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d06e694aa..5c29984f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 an empty candidate list and a `target/miden/` hint that the cache lookup never consults #1302 +- The FPI dependency package lookup matches the `.masp` extension case-insensitively, aligning + the macro-side reader with the compiler's cache writers on case-insensitive filesystems #1302 + ## [0.10.0-rc.1] ### Compiler and `midenc` diff --git a/sdk/base-macros/src/fpi.rs b/sdk/base-macros/src/fpi.rs index 85240c085..2aeac30e8 100644 --- a/sdk/base-macros/src/fpi.rs +++ b/sdk/base-macros/src/fpi.rs @@ -1785,7 +1785,10 @@ fn find_dependency_package_in_dir( })? .into_iter() .map(|entry| entry.path()) - .filter(|path| path.extension().is_some_and(|ext| ext == "masp")) + .filter(|path| { + path.extension() + .is_some_and(|extension| extension.eq_ignore_ascii_case(Package::EXTENSION)) + }) .collect::>(); packages.sort(); From 73086706d57a5fff40c2b3c931d3df7f62a7e21d Mon Sep 17 00:00:00 2001 From: Denys Zadorozhnyi Date: Wed, 5 Aug 2026 16:34:57 +0300 Subject: [PATCH 09/21] test: isolate the package-cache env tracking and harden attribution asserts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A new fixture pins the load-bearing invalidation mechanism by itself: two prepopulated cache directories whose basic-wallet packages embed different receive-asset roots, and two plain cargo builds of an unchanged consumer sharing one target directory, differing only in the MIDENC_PACKAGE_CACHE value. The embedded roots must follow the environment value in both directions — proof that the option_env! recorded by FPI expansions re-expands consumers on cache rotation without any help from manifest changes or midenc's own driver. The build-attribution asserts in the cargo-miden tests tolerate whole-second mtime truncation, and the fingerprint-directory helper documents its newest-mtime tie-break. The integration-network compile_rust_package helper no longer persists packages to target/miden/: nothing under that suite reads the path, and compiler-driven builds resolve dependencies exclusively through the fingerprinted cache. --- .../src/mockchain/support/helpers.rs | 9 +- tests/integration/src/sdk/mod.rs | 155 +++++++++++++++++- tools/cargo-miden/tests/masm_dependency.rs | 12 +- .../tests/p2id_cargo_miden_build.rs | 11 +- tools/cargo-miden/tests/utils.rs | 2 + 5 files changed, 169 insertions(+), 20 deletions(-) diff --git a/tests/integration-network/src/mockchain/support/helpers.rs b/tests/integration-network/src/mockchain/support/helpers.rs index 543b81d47..53b23b74d 100644 --- a/tests/integration-network/src/mockchain/support/helpers.rs +++ b/tests/integration-network/src/mockchain/support/helpers.rs @@ -69,6 +69,7 @@ pub(crate) fn block_on(future: F) -> F::Output { // COMPILATION // ================================================================================================ +/// Compiles a Rust project and returns its Miden package. pub(crate) fn compile_rust_package(project_path: impl AsRef, release: bool) -> Arc { let project_path = project_path.as_ref(); let config = WasmTranslationConfig::default(); @@ -79,13 +80,7 @@ pub(crate) fn compile_rust_package(project_path: impl AsRef, release: bool } let mut test = builder.build(); - let package = test.compile_package(); - let profile = if release { "release" } else { "debug" }; - package - .write_masp_file(project_path.join("target").join("miden").join(profile)) - .expect("failed to persist compiled Miden package"); - - package + test.compile_package() } /// Returns the root of the note script exported by the compiled package. diff --git a/tests/integration/src/sdk/mod.rs b/tests/integration/src/sdk/mod.rs index 543f7b6e8..89b7dee13 100644 --- a/tests/integration/src/sdk/mod.rs +++ b/tests/integration/src/sdk/mod.rs @@ -2,7 +2,7 @@ use std::{fs, path::Path, sync::Arc}; use miden_assembly::ast::types::{FunctionType, Type}; use miden_core::serde::Serializable; -use miden_mast_package::{PackageExport, ProcedureExport, QualifiedProcedureName}; +use miden_mast_package::{Package, PackageExport, ProcedureExport, QualifiedProcedureName}; use miden_protocol::note::NoteScript; use midenc_frontend_wasm::WasmTranslationConfig; @@ -140,6 +140,7 @@ fn assert_component_export_signatures_match_wit(package: &miden_mast_package::Pa } /// Creates a generated workspace containing the existing basic-wallet/swapp-note FPI pair. +#[track_caller] fn fpi_package_cache_regression_project() -> crate::Project { let sdk_path = sdk_crate_path(); let workspace_manifest = r#" @@ -262,18 +263,28 @@ basic-wallet = { wit = "../basic-wallet/target/generated-wit/" } } /// Reads the named dependency package from a compiled consumer's filesystem cache. -fn read_cached_dependency_package( - test: &CompilerTest, - package_name: &str, -) -> miden_mast_package::Package { +fn read_cached_dependency_package(test: &CompilerTest, package_name: &str) -> Package { let cache_dir = test .session .filesystem_package_cache_dir() .expect("a Cargo Miden project must have a filesystem package cache"); - let path = cache_dir.join(format!("{package_name}.masp")); + let path = fs::read_dir(&cache_dir) + .unwrap_or_else(|err| { + panic!("failed to read package cache '{}': {err}", cache_dir.display()) + }) + .map(|entry| entry.expect("failed to read an entry from the package cache").path()) + .find(|path| { + path.file_stem().is_some_and(|stem| stem == package_name) + && path + .extension() + .is_some_and(|extension| extension.eq_ignore_ascii_case(Package::EXTENSION)) + }) + .unwrap_or_else(|| { + panic!("failed to find package '{package_name}' in cache '{}'", cache_dir.display()) + }); let bytes = fs::read(&path) .unwrap_or_else(|err| panic!("failed to read cached package '{}': {err}", path.display())); - miden_mast_package::Package::read_from_bytes_unchecked(&bytes) + Package::read_from_bytes_unchecked(&bytes) .unwrap_or_else(|err| panic!("failed to decode cached package '{}': {err}", path.display())) } @@ -308,6 +319,50 @@ fn masm_contains_procedure_digest(masm: &str, digest: &miden_core::Word) -> bool normalized_masm.contains(&pattern) } +/// Returns true when Wasm constructs every felt of `digest` for an FPI call. +/// +/// The four root elements remain consecutive among the module's `i64.const` instructions even +/// when checked felt construction inserts control flow between them. +fn wat_contains_procedure_digest(wat: &str, digest: &miden_core::Word) -> bool { + let tokens = wat.split_whitespace().collect::>(); + let constants = tokens + .windows(2) + .filter(|tokens| tokens[0].trim_matches(['(', ')']) == "i64.const") + .filter_map(|tokens| tokens[1].trim_matches(['(', ')']).parse::().ok()) + .collect::>(); + let expected = procedure_digest_felts(digest).map(|felt| felt as i64); + constants.windows(expected.len()).any(|window| window == expected) +} + +/// Builds `consumer` directly with one prepopulated package cache and returns its Wasm as WAT. +fn build_consumer_wat_with_package_cache( + consumer: &Path, + cargo_target_dir: &Path, + package_cache_dir: &Path, +) -> String { + let output = std::process::Command::new("cargo") + .args(["build", "--release", "--locked", "--manifest-path"]) + .arg(consumer.join("Cargo.toml")) + .env("CARGO_TARGET_DIR", cargo_target_dir) + .env("MIDENC_PACKAGE_CACHE", package_cache_dir) + .env("RUSTFLAGS", "--cfg miden -C target-feature=+bulk-memory,+wide-arithmetic") + .env_remove("CARGO_ENCODED_RUSTFLAGS") + .current_dir(consumer) + .output() + .expect("failed to spawn Cargo for the option_env isolation fixture"); + assert!( + output.status.success(), + "option_env isolation fixture failed to build:\n{}", + String::from_utf8_lossy(&output.stderr) + ); + + let wasm_path = cargo_target_dir.join("wasm32-wasip2/release/swapp_note.wasm"); + let wasm = fs::read(&wasm_path).unwrap_or_else(|err| { + panic!("failed to read fixture Wasm '{}': {err}", wasm_path.display()) + }); + midenc_frontend_wasm::wasm_to_wat(&wasm).expect("failed to print fixture Wasm") +} + fn component_namespace(name: &str) -> String { let package = name.replace('_', "-"); format!("miden:{package}/miden-{package}@0.0.1") @@ -701,6 +756,92 @@ fn rust_sdk_fpi_reexpands_after_dependency_package_changes() { ); } +/// Changing only `MIDENC_PACKAGE_CACHE` must re-expand FPI roots in an unchanged consumer. +#[test] +fn rust_sdk_fpi_reexpands_after_only_package_cache_env_changes() { + let project = fpi_package_cache_regression_project(); + let dependency = project.root().join("basic-wallet"); + let consumer = project.root().join("swapp-note"); + let dependency_source = dependency.join("src/lib.rs"); + let config = WasmTranslationConfig::default(); + + let mut first_dependency_build = + CompilerTest::rust_source_cargo_miden(&dependency, config.clone(), []); + let first_package = first_dependency_build.compile_package(); + let first_root = find_manifest_procedure( + &first_package, + "original basic-wallet receive-asset export", + |path| path.ends_with("::\"receive-asset\""), + ) + .digest; + + let original_source = fs::read_to_string(&dependency_source).unwrap(); + assert_eq!( + original_source.matches(" self.add_asset(asset);").count(), + 1, + "fixture mutation must match exactly once" + ); + let changed_source = original_source.replacen( + " self.add_asset(asset);", + " self.add_asset(asset);\n self.remove_asset(asset);\n \ + self.add_asset(asset);", + 1, + ); + fs::write(&dependency_source, changed_source).unwrap(); + + let mut second_dependency_build = + CompilerTest::rust_source_cargo_miden(&dependency, config, []); + let second_package = second_dependency_build.compile_package(); + let second_root = find_manifest_procedure( + &second_package, + "changed basic-wallet receive-asset export", + |path| path.ends_with("::\"receive-asset\""), + ) + .digest; + assert_ne!(first_root, second_root, "the prepopulated packages must embed different roots"); + + let first_cache = project.root().join("option-env-cache-a"); + let second_cache = project.root().join("option-env-cache-b"); + for cache in [&first_cache, &second_cache] { + fs::create_dir_all(cache).unwrap(); + } + first_package + .write_masp_file(&first_cache) + .expect("failed to prepopulate the first package cache"); + second_package + .write_masp_file(&second_cache) + .expect("failed to prepopulate the second package cache"); + + let cargo_target_dir = project.root().join("option-env-cargo-target"); + if cargo_target_dir.exists() { + fs::remove_dir_all(&cargo_target_dir).unwrap(); + } + + // Both Cargo invocations have identical arguments, sources, manifests, generated WIT, target + // directory, and flags. The cache environment value is the sole changed build input. + let first_wat = + build_consumer_wat_with_package_cache(&consumer, &cargo_target_dir, &first_cache); + let second_wat = + build_consumer_wat_with_package_cache(&consumer, &cargo_target_dir, &second_cache); + + assert!( + wat_contains_procedure_digest(&first_wat, &first_root), + "the first consumer build did not embed its cache's receive-asset root" + ); + assert!( + !wat_contains_procedure_digest(&first_wat, &second_root), + "the first consumer build unexpectedly embedded the second cache's root" + ); + assert!( + wat_contains_procedure_digest(&second_wat, &second_root), + "changing MIDENC_PACKAGE_CACHE did not re-expand the consumer with the second root" + ); + assert!( + !wat_contains_procedure_digest(&second_wat, &first_root), + "the second consumer build retained the stale root from the first cache" + ); +} + #[test] fn rust_sdk_cross_ctx_word_arg_account_and_note() { let config = WasmTranslationConfig::default(); diff --git a/tools/cargo-miden/tests/masm_dependency.rs b/tools/cargo-miden/tests/masm_dependency.rs index f6753adcb..1111ebb16 100644 --- a/tools/cargo-miden/tests/masm_dependency.rs +++ b/tools/cargo-miden/tests/masm_dependency.rs @@ -11,7 +11,11 @@ //! to derive a per-target role across package boundaries: the Rust root is //! `TargetRole::Root`, the MASM library is a `TargetRole::Dependency`. -use std::{env, fs, path::Path, time::SystemTime}; +use std::{ + env, fs, + path::Path, + time::{Duration, SystemTime, UNIX_EPOCH}, +}; use cargo_miden::run; @@ -186,10 +190,12 @@ fn build_rust_project_with_masm_path_dependency() { dependency_package.display() ); let modified = dependency_package.metadata().unwrap().modified().unwrap(); + let attribution_floor = + build_started_at.checked_sub(Duration::from_secs(1)).unwrap_or(UNIX_EPOCH); assert!( - modified >= build_started_at, + modified >= attribution_floor, "expected this build to rewrite {}, but its modification time {modified:?} predates the \ - build start {build_started_at:?}", + one-second-tolerant build attribution floor {attribution_floor:?}", dependency_package.display() ); diff --git a/tools/cargo-miden/tests/p2id_cargo_miden_build.rs b/tools/cargo-miden/tests/p2id_cargo_miden_build.rs index fafb82d3a..521452c7b 100644 --- a/tools/cargo-miden/tests/p2id_cargo_miden_build.rs +++ b/tools/cargo-miden/tests/p2id_cargo_miden_build.rs @@ -1,4 +1,7 @@ -use std::{env, time::SystemTime}; +use std::{ + env, + time::{Duration, SystemTime, UNIX_EPOCH}, +}; use cargo_miden::run; @@ -60,10 +63,12 @@ fn p2id_build_materializes_basic_wallet_dependency() { dep_package.display() ); let modified = dep_package.metadata().unwrap().modified().unwrap(); + let attribution_floor = + build_started_at.checked_sub(Duration::from_secs(1)).unwrap_or(UNIX_EPOCH); assert!( - modified >= build_started_at, + modified >= attribution_floor, "expected this build to rewrite {}, but its modification time {modified:?} predates the \ - build start {build_started_at:?}", + one-second-tolerant build attribution floor {attribution_floor:?}", dep_package.display() ); } diff --git a/tools/cargo-miden/tests/utils.rs b/tools/cargo-miden/tests/utils.rs index 8099fe5a6..62c324496 100644 --- a/tools/cargo-miden/tests/utils.rs +++ b/tools/cargo-miden/tests/utils.rs @@ -96,6 +96,8 @@ pub(crate) fn package_cache_fingerprint_dir(project_dir: &Path, expected_package } } + // `read_dir` order is unspecified, so equal mtimes at the filesystem's precision leave this + // diagnostic attribution tie nondeterministic. No production behavior depends on the choice. candidates .into_iter() .max_by_key(|(modified, _)| *modified) From 7aee3c57937f8ef18e0b00f8123addeffbff8c38 Mon Sep 17 00:00:00 2001 From: Denys Zadorozhnyi Date: Wed, 5 Aug 2026 18:59:53 +0300 Subject: [PATCH 10/21] fix(compile): set CARGO_ENCODED_RUSTFLAGS authoritatively for nested cargo builds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cargo prefers CARGO_ENCODED_RUSTFLAGS over RUSTFLAGS, and the nested cargo inherited the caller's environment — so a CI image or build-script context exporting the encoded variable silently replaced every mandatory Miden flag: no --cfg miden, no wasm target features, no immediate-abort panic strategy, with nothing attributing the resulting breakage to the inherited variable. cargo_env now emits the composed flags in both spellings; the explicit encoded value makes any inherited one inert. The encoding splits the composed string on whitespace, which is exactly how cargo interprets the plain variable, so the flags cannot change meaning between the two forms. --- midenc-compile/src/pipeline/frontends/rust.rs | 33 +++++++++++++++---- 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/midenc-compile/src/pipeline/frontends/rust.rs b/midenc-compile/src/pipeline/frontends/rust.rs index 79cddf351..22a2252a0 100644 --- a/midenc-compile/src/pipeline/frontends/rust.rs +++ b/midenc-compile/src/pipeline/frontends/rust.rs @@ -1613,20 +1613,27 @@ pub(crate) mod manifest { /// variable is *unset* rather than set to an empty path, which is what makes the nested build /// fall back to its own default. /// + /// The composed rust flags are emitted twice: as `RUSTFLAGS` and, authoritatively, as + /// `CARGO_ENCODED_RUSTFLAGS`. Cargo prefers the encoded variable, so an inherited value from + /// the caller's environment would otherwise silently replace every mandatory Miden flag — + /// `--cfg miden`, the target features, the panic strategy. Setting it explicitly makes the + /// inherited value inert; the encoding splits on whitespace, which is exactly how cargo + /// itself interprets the plain variable. + /// /// Named — rather than left inline where it was — so that this can be asserted without /// spawning `cargo -Z build-std` against the SDK. pub(super) fn cargo_env( filesystem_cache_dir: Option<&Path>, extra_rust_flags: String, ) -> Vec<(&'static str, String)> { + let encoded_rust_flags = + extra_rust_flags.split_whitespace().collect::>().join("\x1f"); + let mut env = + vec![("RUSTFLAGS", extra_rust_flags), ("CARGO_ENCODED_RUSTFLAGS", encoded_rust_flags)]; if let Some(filesystem_cache_dir) = filesystem_cache_dir { - vec![ - ("RUSTFLAGS", extra_rust_flags), - ("MIDENC_PACKAGE_CACHE", filesystem_cache_dir.to_string_lossy().into_owned()), - ] - } else { - vec![("RUSTFLAGS", extra_rust_flags)] + env.push(("MIDENC_PACKAGE_CACHE", filesystem_cache_dir.to_string_lossy().into_owned())); } + env } /// Returns the Cargo profile value for a compiler optimization level. @@ -3179,6 +3186,20 @@ path = "lib.rs" ); } + #[test] + fn the_encoded_rustflags_override_inherited_values_with_the_same_flags() { + let rustflags = String::from("-C target-feature=+bulk-memory --cfg miden"); + + let env = manifest::cargo_env(None, rustflags); + let (_, encoded) = env + .iter() + .find(|(key, _)| *key == "CARGO_ENCODED_RUSTFLAGS") + .expect("the encoded variable must be set so an inherited value cannot override it"); + // 0x1f-separated units, one per whitespace-separated flag — cargo's own split rule for + // the plain variable. + assert_eq!(*encoded, "-C\x1ftarget-feature=+bulk-memory\x1f--cfg\x1fmiden"); + } + /// A WebAssembly module with a body, for the lowering half of the entry point. const MANIFEST_WAT: &str = r#" (module From b19db1100e58ece76c14b0cf5cb53927aac94f23 Mon Sep 17 00:00:00 2001 From: Denys Zadorozhnyi Date: Wed, 5 Aug 2026 18:59:53 +0300 Subject: [PATCH 11/21] fix(session): keep the liveness lock through cache-create failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two preparation legs degraded harder than the documented contract. A lock-open failure aborted preparation entirely while its log claimed the build was continuing — the cache directory then materialized anyway through the first publication's create_dir_all, unlocked and unswept. And a cache-create failure returned None after the shared lock was already acquired, releasing the one guard that would protect the directory a later publication recreates. The first leg now creates the directory before returning, and the second keeps and returns the held lock, skipping only the sweep; the function doc states what Some and None actually mean. With the encoded rustflags now set authoritatively for nested builds, the inherited CARGO_ENCODED_RUSTFLAGS value has no effect on what gets built, so it leaves the fingerprint (a comment records why, so it is not re-added). The cache-path producer and the owned-layout validator were two unlinked lexical derivations; the parent-path construction moves next to the validator and the session derivation test asserts the produced path satisfies the ownership check, so a future layout change breaks a test instead of silently disabling locking and pruning. --- midenc-session/src/lib.rs | 11 ++- midenc-session/src/package_cache.rs | 137 ++++++++++++++++------------ 2 files changed, 87 insertions(+), 61 deletions(-) diff --git a/midenc-session/src/lib.rs b/midenc-session/src/lib.rs index 73491c7f8..1c6ad8bfb 100644 --- a/midenc-session/src/lib.rs +++ b/midenc-session/src/lib.rs @@ -410,18 +410,19 @@ impl Session { // one directory must not resolve to two caches. #[cfg(feature = "std")] let project_dir = project_dir.canonicalize().unwrap_or(project_dir); + #[cfg(feature = "std")] + let package_cache_dir = package_cache::package_cache_parent(&project_dir); + #[cfg(not(feature = "std"))] let package_cache_dir = project_dir.join("target").join("miden").join("packages"); #[cfg(feature = "std")] { let fingerprint = self.package_cache_fingerprint.get_or_init(|| { let inherited_rustflags = std::env::var_os("RUSTFLAGS"); - let inherited_cargo_encoded_rustflags = std::env::var_os("CARGO_ENCODED_RUSTFLAGS"); let inherited_rustup_toolchain = std::env::var_os("RUSTUP_TOOLCHAIN"); package_cache::fingerprint( &self.options, &project_dir, inherited_rustflags.as_deref(), - inherited_cargo_encoded_rustflags.as_deref(), inherited_rustup_toolchain.as_deref(), MIDENC_BUILD_VERSION, MIDENC_BUILD_REV, @@ -800,5 +801,11 @@ mod tests { let cache_dir = session.filesystem_package_cache_dir().unwrap(); let expected_parent = temp.path().canonicalize().unwrap().join("target/miden/packages"); assert_eq!(cache_dir.parent(), Some(expected_parent.as_path())); + assert!( + package_cache::is_owned_filesystem_cache_path(&cache_dir), + "the derived cache path must satisfy the owned-layout check, or locking and pruning \ + silently degrade: {}", + cache_dir.display() + ); } } diff --git a/midenc-session/src/package_cache.rs b/midenc-session/src/package_cache.rs index 040165627..19465cdba 100644 --- a/midenc-session/src/package_cache.rs +++ b/midenc-session/src/package_cache.rs @@ -81,9 +81,11 @@ const BUILD_LOCK_EXTENSION: &str = "lock"; /// lock lives outside that directory and is acquired before the directory is created, closing the /// create-before-lock, unlock-before-delete, and unlink/recreate inode-ABA windows. /// -/// `None` means locking was skipped for an unowned path or preparation degraded after an error. -/// The caller deliberately keeps the cache configured so package publication can report the -/// concrete filesystem failure. +/// `Some` means the shared liveness lock is held, even when a later preparation step degraded. +/// `None` means locking was skipped: the path is unowned, or opening/locking the lock file +/// failed. Every leg keeps the cache configured and the cache directory created when possible, +/// so package publication proceeds against the expected path and reports any concrete +/// filesystem failure itself. pub(crate) fn prepare_and_lock_filesystem_cache(filesystem_cache: &Path) -> Option { if !is_owned_filesystem_cache_path(filesystem_cache) { prepare_unowned_filesystem_cache(filesystem_cache); @@ -96,9 +98,19 @@ pub(crate) fn prepare_and_lock_filesystem_cache(filesystem_cache: &Path) -> Opti if !create_filesystem_cache_parent(parent) { return None; } - let filesystem_cache_lock = acquire_filesystem_cache_lock(filesystem_cache)?; - if !create_current_filesystem_cache(filesystem_cache) { + let Some(filesystem_cache_lock) = acquire_filesystem_cache_lock(filesystem_cache) else { + // No lock could be held, but the build still runs against this path: create the + // directory now rather than leaving it to the first publication, so the failure mode + // is only "unprotected and unswept", not "missing". + create_current_filesystem_cache(filesystem_cache); return None; + }; + if !create_current_filesystem_cache(filesystem_cache) { + // The shared lock is already held — keep it. A later publication may still recreate + // the directory (its writer creates parent directories), and the lock is what stops a + // concurrent pruner from classifying that recreated cache as dead. Only the sweep is + // skipped in this degraded mode. + return Some(filesystem_cache_lock); } sweep_stale_filesystem_cache_entries(filesystem_cache, parent); Some(filesystem_cache_lock) @@ -300,7 +312,17 @@ fn filesystem_cache_lock_path(filesystem_cache: &Path) -> PathBuf { } /// Returns true when a path is lexically owned by the `miden/packages/` layout. -fn is_owned_filesystem_cache_path(filesystem_cache: &Path) -> bool { +/// Returns the package-cache parent directory for a project directory. +/// +/// This is the producer half of the owned-layout contract: the path it builds must satisfy +/// [`is_owned_filesystem_cache_path`] once a fingerprint component is appended, or the locking +/// and pruning protocol silently degrades to a debug log. `Session::filesystem_package_cache_dir` +/// derives through here, and its unit test asserts the coupling. +pub(crate) fn package_cache_parent(project_dir: &Path) -> PathBuf { + project_dir.join("target").join("miden").join("packages") +} + +pub(crate) fn is_owned_filesystem_cache_path(filesystem_cache: &Path) -> bool { filesystem_cache.file_name().is_some_and(is_package_cache_fingerprint) && filesystem_cache .parent() @@ -328,7 +350,6 @@ pub(crate) fn fingerprint( options: &Options, project_dir: &Path, inherited_rustflags: Option<&OsStr>, - inherited_cargo_encoded_rustflags: Option<&OsStr>, inherited_rustup_toolchain: Option<&OsStr>, compiler_version: &str, compiler_revision: &str, @@ -336,13 +357,7 @@ pub(crate) fn fingerprint( let mut transcript = Transcript::new(); transcript.field("compiler.version", compiler_version.as_bytes()); transcript.field("compiler.revision", compiler_revision.as_bytes()); - record_options( - &mut transcript, - options, - inherited_rustflags, - inherited_cargo_encoded_rustflags, - inherited_rustup_toolchain, - ); + record_options(&mut transcript, options, inherited_rustflags, inherited_rustup_toolchain); let source_manager = DefaultSourceManager::default(); let mut manifests = ManifestClosure::new(&mut transcript, &source_manager); @@ -406,7 +421,6 @@ fn record_options( transcript: &mut Transcript, options: &Options, inherited_rustflags: Option<&OsStr>, - inherited_cargo_encoded_rustflags: Option<&OsStr>, inherited_rustup_toolchain: Option<&OsStr>, ) { let Options { @@ -482,10 +496,9 @@ fn record_options( "options.inherited_rustflags", inherited_rustflags.map(OsStr::as_encoded_bytes), ); - transcript.optional_bytes_field( - "options.inherited_cargo_encoded_rustflags", - inherited_cargo_encoded_rustflags.map(OsStr::as_encoded_bytes), - ); + // Inherited `CARGO_ENCODED_RUSTFLAGS` is deliberately NOT fingerprinted: `cargo_env` sets + // the variable authoritatively for every nested build, so the inherited value has no effect + // on what gets built. transcript.optional_bytes_field( "options.inherited_rustup_toolchain", inherited_rustup_toolchain.map(OsStr::as_encoded_bytes), @@ -941,6 +954,49 @@ mod tests { drop(builder_lock); } + #[test] + fn cache_create_failure_keeps_the_acquired_liveness_lock_and_skips_the_sweep() { + let temp = TempDir::new().unwrap(); + let parent = temp.path().join("miden").join("packages"); + let current = parent.join("fedcba9876543210"); + let stale = parent.join("0123456789abcdef"); + std::fs::create_dir_all(&stale).unwrap(); + // A regular file at the fingerprint path makes `create_dir_all` fail after the shared + // lock is already held. + std::fs::write(¤t, b"not a directory").unwrap(); + + let lock = prepare_and_lock_filesystem_cache(¤t); + + assert!(lock.is_some(), "the acquired liveness lock must survive a create failure"); + let contender = OpenOptions::new() + .read(true) + .write(true) + .open(filesystem_cache_lock_path(¤t)) + .unwrap(); + assert!( + matches!(contender.try_lock(), Err(TryLockError::WouldBlock)), + "the shared lock must still protect the cache path" + ); + assert!(stale.exists(), "the sweep must be skipped in the degraded mode"); + } + + #[test] + fn lock_open_failure_still_creates_the_cache_directory() { + let temp = TempDir::new().unwrap(); + let parent = temp.path().join("miden").join("packages"); + let current = parent.join("fedcba9876543210"); + // A directory at the lock path makes the lock file unopenable. + std::fs::create_dir_all(filesystem_cache_lock_path(¤t)).unwrap(); + + let lock = prepare_and_lock_filesystem_cache(¤t); + + assert!(lock.is_none(), "no lock can be held when its file cannot be opened"); + assert!( + current.is_dir(), + "the cache directory must be created so the build runs against the expected path" + ); + } + #[test] fn live_stale_fingerprint_survives_until_its_lock_is_released() { let temp = TempDir::new().unwrap(); @@ -1002,21 +1058,6 @@ mod tests { drop(contender); } - #[test] - fn cache_creation_failure_does_not_sweep_siblings() { - let temp = TempDir::new().unwrap(); - let parent = temp.path().join("miden").join("packages"); - let current = parent.join("fedcba9876543210"); - let stale = parent.join("0123456789abcdef"); - fs::create_dir_all(&stale).unwrap(); - fs::write(¤t, b"not a directory").unwrap(); - - let lock = prepare_and_lock_filesystem_cache(¤t); - - assert!(lock.is_none()); - assert!(stale.exists(), "siblings must survive when the current cache cannot be created"); - } - #[test] fn arbitrary_cache_path_cannot_sweep_its_parent() { let temp = TempDir::new().unwrap(); @@ -1075,7 +1116,7 @@ mod tests { /// Computes a test fingerprint with a fresh source manager. fn test_fingerprint(options: &Options, project_dir: &Path, version: &str, rev: &str) -> String { - fingerprint(options, project_dir, None, None, None, version, rev) + fingerprint(options, project_dir, None, None, version, rev) } #[test] @@ -1195,33 +1236,12 @@ mod tests { write_project(temp.path(), "root", ""); let options = Options::default(); - let missing = fingerprint(&options, temp.path(), None, None, None, "1.2.3", "abc123"); + let missing = fingerprint(&options, temp.path(), None, None, "1.2.3", "abc123"); let present = fingerprint( &options, temp.path(), Some(OsStr::new("-C target-feature=+bulk-memory")), None, - None, - "1.2.3", - "abc123", - ); - - assert_ne!(missing, present); - } - - #[test] - fn fingerprint_changes_with_inherited_cargo_encoded_rustflags() { - let temp = TempDir::new().unwrap(); - write_project(temp.path(), "root", ""); - let options = Options::default(); - - let missing = fingerprint(&options, temp.path(), None, None, None, "1.2.3", "abc123"); - let present = fingerprint( - &options, - temp.path(), - None, - Some(OsStr::new("-Ctarget-feature=+bulk-memory\u{1f}--cfg=fixture")), - None, "1.2.3", "abc123", ); @@ -1235,12 +1255,11 @@ mod tests { write_project(temp.path(), "root", ""); let options = Options::default(); - let missing = fingerprint(&options, temp.path(), None, None, None, "1.2.3", "abc123"); + let missing = fingerprint(&options, temp.path(), None, None, "1.2.3", "abc123"); let present = fingerprint( &options, temp.path(), None, - None, Some(OsStr::new("nightly-2026-08-05")), "1.2.3", "abc123", From 1c7e49c7cceb803a56be8b864ca81b1f9907db0e Mon Sep 17 00:00:00 2001 From: Denys Zadorozhnyi Date: Wed, 5 Aug 2026 19:00:25 +0300 Subject: [PATCH 12/21] test: drop the dead legacy package persistence from the integration suites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Compiler-driven builds always set MIDENC_PACKAGE_CACHE and the FPI macro has no fallback once it is set, so persisting compiled dependency packages to target/miden/release never influenced any test — the five calls only wrote artifacts into the checked-in example and fixture trees. An editor-driven consumer expansion without the variable is served by the dependency's own cargo miden build output, not by test-suite side effects. The counter-note test also loses its dependency pre-build, which existed only to feed the persistence; the consumer build compiles the contract itself. --- .../examples/basic_wallet_package_sizes.rs | 2 -- .../src/end_to_end/examples/counter_note.rs | 16 ++--------- .../src/end_to_end/examples/mod.rs | 15 ----------- tests/integration/src/sdk/mod.rs | 27 ------------------- 4 files changed, 2 insertions(+), 58 deletions(-) diff --git a/tests/integration/src/end_to_end/examples/basic_wallet_package_sizes.rs b/tests/integration/src/end_to_end/examples/basic_wallet_package_sizes.rs index 35366050c..b7b7a62b5 100644 --- a/tests/integration/src/end_to_end/examples/basic_wallet_package_sizes.rs +++ b/tests/integration/src/end_to_end/examples/basic_wallet_package_sizes.rs @@ -1,7 +1,6 @@ use midenc_expect_test::expect; use midenc_frontend_wasm::WasmTranslationConfig; -use super::persist_cargo_miden_dependency; use crate::{CompilerTest, testing::stripped_mast_size_str}; fn no_debug_flags() -> [String; 2] { @@ -19,7 +18,6 @@ fn basic_wallet_and_p2id() { let account_package = account_test.compile_package(); assert!(account_package.is_library(), "expected library"); expect!["7982"].assert_eq(stripped_mast_size_str(&account_package).as_str()); - persist_cargo_miden_dependency("../../examples/basic-wallet", account_package.as_ref()); let mut tx_script_test = CompilerTest::rust_source_cargo_miden( "../../examples/basic-wallet-tx-script", diff --git a/tests/integration/src/end_to_end/examples/counter_note.rs b/tests/integration/src/end_to_end/examples/counter_note.rs index a1aeb8ba0..85f5eb69e 100644 --- a/tests/integration/src/end_to_end/examples/counter_note.rs +++ b/tests/integration/src/end_to_end/examples/counter_note.rs @@ -1,25 +1,13 @@ use miden_protocol::note::NoteScript; use midenc_frontend_wasm::WasmTranslationConfig; -use super::persist_cargo_miden_dependency; use crate::{CompilerTestBuilder, assert_helpers::assert_unique_protocol_export}; #[test] fn counter_note() { let config = WasmTranslationConfig::default(); - let counter_contract_builder = CompilerTestBuilder::rust_source_cargo_miden( - "../../examples/counter-contract", - config.clone(), - [], - ); - let mut counter_contract = counter_contract_builder.build(); - let counter_contract_package = counter_contract.compile_package(); - persist_cargo_miden_dependency( - "../../examples/counter-contract", - counter_contract_package.as_ref(), - ); - - // build and check counter-note + // The counter-note build compiles its counter-contract dependency itself and resolves it + // through the fingerprinted package cache; no separate dependency pre-build is needed. let builder = CompilerTestBuilder::rust_source_cargo_miden("../../examples/counter-note", config, []); diff --git a/tests/integration/src/end_to_end/examples/mod.rs b/tests/integration/src/end_to_end/examples/mod.rs index cd0e3010f..2ff4a3adc 100644 --- a/tests/integration/src/end_to_end/examples/mod.rs +++ b/tests/integration/src/end_to_end/examples/mod.rs @@ -1,5 +1,3 @@ -use std::path::Path; - mod auth_component_no_auth; mod auth_component_rpo_falcon512; mod basic_wallet_package_sizes; @@ -10,16 +8,3 @@ mod counter_note; mod fibonacci; mod is_prime; mod storage_metadata; - -/// Writes a dependency package to the legacy lookup directory used only when -/// `MIDENC_PACKAGE_CACHE` is unset. -/// -/// Compiler-driven builds set that variable and use their fingerprinted cache exclusively. -fn persist_cargo_miden_dependency( - project_path: impl AsRef, - package: &miden_mast_package::Package, -) { - package - .write_masp_file(project_path.as_ref().join("target").join("miden").join("release")) - .expect("failed to persist compiled Miden dependency package"); -} diff --git a/tests/integration/src/sdk/mod.rs b/tests/integration/src/sdk/mod.rs index 89b7dee13..1c877e9b2 100644 --- a/tests/integration/src/sdk/mod.rs +++ b/tests/integration/src/sdk/mod.rs @@ -32,20 +32,6 @@ pub(crate) fn note_script_program( .unwrap() } -/// Writes a compiled package to the legacy fallback used by `miden::generate!` only when -/// `MIDENC_PACKAGE_CACHE` is unset. -/// -/// When that variable is set, the macro searches only the fingerprinted cache populated by the -/// enclosing compiler-driven build and does not consult `target/miden/release`. -fn persist_cargo_miden_dependency( - project_path: impl AsRef, - package: &miden_mast_package::Package, -) { - package - .write_masp_file(project_path.as_ref().join("target").join("miden").join("release")) - .expect("failed to persist compiled Miden dependency package"); -} - fn find_manifest_procedure<'a>( package: &'a miden_mast_package::Package, description: &str, @@ -483,10 +469,6 @@ fn rust_sdk_cross_ctx_account_and_note() { [], ); let account_package = test.compile_package(); - persist_cargo_miden_dependency( - "../fixtures/components/cross-ctx-account", - account_package.as_ref(), - ); assert!(account_package.is_library()); let exports = account_package .manifest @@ -540,10 +522,6 @@ fn rust_sdk_cross_ctx_account_and_note_word() { [], ); let account_package = test.compile_package(); - persist_cargo_miden_dependency( - "../fixtures/components/cross-ctx-account-word", - account_package.as_ref(), - ); assert!(account_package.is_library()); assert_component_export_signatures_match_wit(account_package.as_ref()); let expected_module_prefix = "::\"miden:cross-ctx-account-word/"; @@ -851,11 +829,6 @@ fn rust_sdk_cross_ctx_word_arg_account_and_note() { [], ); let account_package = test.compile_package(); - persist_cargo_miden_dependency( - "../fixtures/components/cross-ctx-account-word-arg", - account_package.as_ref(), - ); - assert!(account_package.is_library()); let expected_module_prefix = "::\"miden:cross-ctx-account-word-arg/"; let expected_function_suffix = "\"process-word\""; From 3e5ef9ccd7963d014558986584081ebf70eda1d1 Mon Sep 17 00:00:00 2001 From: Denys Zadorozhnyi Date: Wed, 5 Aug 2026 19:00:25 +0300 Subject: [PATCH 13/21] docs(sdk): note the FPI cache-path tracking in the changelog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The option_env!(MIDENC_PACKAGE_CACHE) recording is the most user-visible behavior change on this branch — consumer crates now recompile whenever the fingerprinted cache path rotates — and it was missing from the changelog entries for #1302. --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c29984f7..e980cc50e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,9 +13,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 the searched `MIDENC_PACKAGE_CACHE` directory and the expected package file names, instead of an empty candidate list and a `target/miden/` hint that the cache lookup never consults #1302 - - The FPI dependency package lookup matches the `.masp` extension case-insensitively, aligning the macro-side reader with the compiler's cache writers on case-insensitive filesystems #1302 +- FPI expansions record `option_env!("MIDENC_PACKAGE_CACHE")`, so a consumer crate recompiles — + and re-reads its dependency procedure roots — whenever the compiler's fingerprinted package + cache path rotates, even if a stale cache directory survives on disk #1302 ## [0.10.0-rc.1] From 091b63677e129a5988f71c0c9fea08cdfbcf8664 Mon Sep 17 00:00:00 2001 From: Denys Zadorozhnyi Date: Mon, 10 Aug 2026 12:00:11 +0300 Subject: [PATCH 14/21] fix(compile): merge inherited encoded rustflags instead of discarding them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous fix emitted CARGO_ENCODED_RUSTFLAGS derived only from the composed plain flags, which neutralized the inherited value — including flags a caller legitimately passed that way, the only spelling cargo itself uses for build scripts. That treated the two inherited spellings asymmetrically: plain RUSTFLAGS was appended, encoded flags were dropped. The composition now works on an argument list. Inherited flags follow cargo's own precedence — a non-empty encoded value (0x1f-separated, arguments may contain spaces) replaces the plain spelling — and are folded in after the mandatory Miden flags and before the explicit --rustflags, preserving the existing override order. The combined list is emitted authoritatively in both spellings; the plain form is a lossy space-join, which cargo ignores in favor of the encoded one. Because the inherited encoded value influences the nested build again, it returns to the package-cache fingerprint alongside the plain spelling. --- midenc-compile/src/pipeline/frontends/rust.rs | 176 +++++++++++++----- midenc-session/src/lib.rs | 2 + midenc-session/src/package_cache.rs | 48 ++++- 3 files changed, 175 insertions(+), 51 deletions(-) diff --git a/midenc-compile/src/pipeline/frontends/rust.rs b/midenc-compile/src/pipeline/frontends/rust.rs index 22a2252a0..3de1e9a2c 100644 --- a/midenc-compile/src/pipeline/frontends/rust.rs +++ b/midenc-compile/src/pipeline/frontends/rust.rs @@ -1563,29 +1563,32 @@ pub(crate) mod manifest { let rustup_toolchain = crate::rust::rustup_toolchain(); let cargo_build_args = build_cargo_args(cargo_opts, compiler_opts.optimize); - // Enable memcopy and 128-bit arithmetic ops - let mut extra_rust_flags = String::from("-C target-feature=+bulk-memory,+wide-arithmetic"); - // Propagate the Miden VM target signal to the entire crate graph so Cargo can use it for - // cfg-based dependency selection. - extra_rust_flags.push_str(" --cfg miden"); - // Enable errors on missing stub functions - extra_rust_flags.push_str(" -C link-args=--fatal-warnings"); - // Remove the source file paths in the data segment for panics - // https://doc.rust-lang.org/beta/unstable-book/compiler-flags/location-detail.html - extra_rust_flags.push_str(" -Zlocation-detail=none"); - // Build with panic=immediate-abort - extra_rust_flags.push_str(" -Zunstable-options"); - extra_rust_flags.push_str(" -Cpanic=immediate-abort"); - if let Ok(inherited) = std::env::var("RUSTFLAGS") - && !inherited.is_empty() - { - extra_rust_flags.push(' '); - extra_rust_flags.push_str(&inherited); - } - if let Some(explicit) = compiler_opts.rustflags.as_deref() { - extra_rust_flags.push(' '); - extra_rust_flags.push_str(explicit); - } + let mandatory_rust_flags = [ + // Enable memcopy and 128-bit arithmetic ops + "-C", + "target-feature=+bulk-memory,+wide-arithmetic", + // Propagate the Miden VM target signal to the entire crate graph so Cargo can use it + // for cfg-based dependency selection. + "--cfg", + "miden", + // Enable errors on missing stub functions + "-C", + "link-args=--fatal-warnings", + // Remove the source file paths in the data segment for panics + // https://doc.rust-lang.org/beta/unstable-book/compiler-flags/location-detail.html + "-Zlocation-detail=none", + // Build with panic=immediate-abort + "-Zunstable-options", + "-Cpanic=immediate-abort", + ]; + let inherited_encoded = std::env::var_os("CARGO_ENCODED_RUSTFLAGS"); + let inherited_plain = std::env::var_os("RUSTFLAGS"); + let extra_rust_flags = merge_rust_flags( + &mandatory_rust_flags, + inherited_encoded.as_deref(), + inherited_plain.as_deref(), + compiler_opts.rustflags.as_deref(), + ); let wasi = if compiler_opts.target_requires_protocol() { "wasip2" @@ -1593,7 +1596,7 @@ pub(crate) mod manifest { "wasip1" }; - let env = cargo_env(filesystem_cache_dir, extra_rust_flags); + let env = cargo_env(filesystem_cache_dir, &extra_rust_flags); let mut wasm_outputs = run_cargo(wasi, rustup_toolchain.as_deref(), &cargo_build_args, env)?; @@ -1613,29 +1616,58 @@ pub(crate) mod manifest { /// variable is *unset* rather than set to an empty path, which is what makes the nested build /// fall back to its own default. /// - /// The composed rust flags are emitted twice: as `RUSTFLAGS` and, authoritatively, as - /// `CARGO_ENCODED_RUSTFLAGS`. Cargo prefers the encoded variable, so an inherited value from - /// the caller's environment would otherwise silently replace every mandatory Miden flag — - /// `--cfg miden`, the target features, the panic strategy. Setting it explicitly makes the - /// inherited value inert; the encoding splits on whitespace, which is exactly how cargo - /// itself interprets the plain variable. + /// The composed rust flags are emitted twice: as `RUSTFLAGS` (space-joined, lossy for + /// arguments that contain spaces) and, authoritatively, as `CARGO_ENCODED_RUSTFLAGS` + /// (0x1f-joined). Cargo prefers the encoded variable, so emitting it explicitly is what keeps + /// the mandatory Miden flags — `--cfg miden`, the target features, the panic strategy — from + /// being replaced by an inherited value; the caller's own flags survive because + /// [`merge_rust_flags`] folds them into the composed list first. /// /// Named — rather than left inline where it was — so that this can be asserted without /// spawning `cargo -Z build-std` against the SDK. pub(super) fn cargo_env( filesystem_cache_dir: Option<&Path>, - extra_rust_flags: String, + extra_rust_flags: &[String], ) -> Vec<(&'static str, String)> { - let encoded_rust_flags = - extra_rust_flags.split_whitespace().collect::>().join("\x1f"); - let mut env = - vec![("RUSTFLAGS", extra_rust_flags), ("CARGO_ENCODED_RUSTFLAGS", encoded_rust_flags)]; + let mut env = vec![ + ("RUSTFLAGS", extra_rust_flags.join(" ")), + ("CARGO_ENCODED_RUSTFLAGS", extra_rust_flags.join("\x1f")), + ]; if let Some(filesystem_cache_dir) = filesystem_cache_dir { env.push(("MIDENC_PACKAGE_CACHE", filesystem_cache_dir.to_string_lossy().into_owned())); } env } + /// Composes the rustc flag list for the nested cargo build. + /// + /// Inherited flags follow cargo's own precedence: a non-empty `CARGO_ENCODED_RUSTFLAGS` + /// (0x1f-separated; arguments may contain spaces) replaces plain `RUSTFLAGS` + /// (whitespace-split). Inherited flags are folded in after the mandatory set and before the + /// explicit `--rustflags`, preserving the pre-existing override order — nothing a caller + /// passes is dropped. + pub(super) fn merge_rust_flags( + mandatory: &[&str], + inherited_encoded: Option<&std::ffi::OsStr>, + inherited_plain: Option<&std::ffi::OsStr>, + explicit: Option<&str>, + ) -> Vec { + let mut args: Vec = mandatory.iter().map(|flag| flag.to_string()).collect(); + let encoded = + inherited_encoded.and_then(|value| value.to_str()).filter(|value| !value.is_empty()); + let plain = + inherited_plain.and_then(|value| value.to_str()).filter(|value| !value.is_empty()); + if let Some(encoded) = encoded { + args.extend(encoded.split('\x1f').filter(|arg| !arg.is_empty()).map(str::to_string)); + } else if let Some(plain) = plain { + args.extend(plain.split_whitespace().map(str::to_string)); + } + if let Some(explicit) = explicit { + args.extend(explicit.split_whitespace().map(str::to_string)); + } + args + } + /// Returns the Cargo profile value for a compiler optimization level. fn cargo_profile_opt_level(opt_level: OptLevel) -> &'static str { match opt_level { @@ -3161,12 +3193,13 @@ path = "lib.rs" /// which is minutes of work and a network fetch. #[test] fn the_filesystem_cache_directory_is_handed_to_the_nested_cargo_build() { - let rustflags = String::from("-C target-feature=+bulk-memory"); + let rustflags = vec!["-C".to_string(), "target-feature=+bulk-memory".to_string()]; let cache = std::path::Path::new("/tmp/midenc-package-cache"); - let with = manifest::cargo_env(Some(cache), rustflags.clone()); + let with = manifest::cargo_env(Some(cache), &rustflags); assert!( - with.iter().any(|(key, value)| *key == "RUSTFLAGS" && *value == rustflags), + with.iter() + .any(|(key, value)| *key == "RUSTFLAGS" && *value == rustflags.join(" ")), "the rust flags must survive the cache directory being added: {with:?}" ); let (_, found) = with @@ -3175,31 +3208,86 @@ path = "lib.rs" .expect("a cache directory must be delivered to the nested build"); assert_eq!(found, &cache.display().to_string()); - let without = manifest::cargo_env(None, rustflags.clone()); + let without = manifest::cargo_env(None, &rustflags); assert!( !without.iter().any(|(key, _)| *key == "MIDENC_PACKAGE_CACHE"), "no cache directory means the variable is unset, not set to nothing: {without:?}" ); assert!( - without.iter().any(|(key, value)| *key == "RUSTFLAGS" && *value == rustflags), + without + .iter() + .any(|(key, value)| *key == "RUSTFLAGS" && *value == rustflags.join(" ")), "and the rust flags are handed over either way: {without:?}" ); } #[test] fn the_encoded_rustflags_override_inherited_values_with_the_same_flags() { - let rustflags = String::from("-C target-feature=+bulk-memory --cfg miden"); + let rustflags = vec![ + "-C".to_string(), + "target-feature=+bulk-memory".to_string(), + "--cfg".to_string(), + "miden".to_string(), + ]; - let env = manifest::cargo_env(None, rustflags); + let env = manifest::cargo_env(None, &rustflags); let (_, encoded) = env .iter() .find(|(key, _)| *key == "CARGO_ENCODED_RUSTFLAGS") .expect("the encoded variable must be set so an inherited value cannot override it"); - // 0x1f-separated units, one per whitespace-separated flag — cargo's own split rule for - // the plain variable. + // 0x1f-separated units, one per argument. assert_eq!(*encoded, "-C\x1ftarget-feature=+bulk-memory\x1f--cfg\x1fmiden"); } + #[test] + fn merged_rust_flags_preserve_inherited_encoded_arguments() { + use std::ffi::OsStr; + + let mandatory = ["--cfg", "miden"]; + + // A non-empty encoded value wins over the plain spelling, mirroring cargo's precedence, + // and its 0x1f-separated arguments survive verbatim — including ones with spaces. + let merged = manifest::merge_rust_flags( + &mandatory, + Some(OsStr::new("-C\x1flink-args=--flag one --flag two")), + Some(OsStr::new("--cfg plain_ignored")), + Some("--cfg explicit"), + ); + assert_eq!( + merged, + vec![ + "--cfg".to_string(), + "miden".to_string(), + "-C".to_string(), + "link-args=--flag one --flag two".to_string(), + "--cfg".to_string(), + "explicit".to_string(), + ] + ); + + // Without an encoded value the plain spelling is whitespace-split, as cargo does. + let merged = + manifest::merge_rust_flags(&mandatory, None, Some(OsStr::new("-C opt-level=3")), None); + assert_eq!( + merged, + vec![ + "--cfg".to_string(), + "miden".to_string(), + "-C".to_string(), + "opt-level=3".to_string(), + ] + ); + + // Empty inherited values are treated as unset. + let merged = manifest::merge_rust_flags( + &mandatory, + Some(OsStr::new("")), + Some(OsStr::new("")), + None, + ); + assert_eq!(merged, vec!["--cfg".to_string(), "miden".to_string()]); + } + /// A WebAssembly module with a body, for the lowering half of the entry point. const MANIFEST_WAT: &str = r#" (module diff --git a/midenc-session/src/lib.rs b/midenc-session/src/lib.rs index 1c6ad8bfb..dc407aca2 100644 --- a/midenc-session/src/lib.rs +++ b/midenc-session/src/lib.rs @@ -418,11 +418,13 @@ impl Session { { let fingerprint = self.package_cache_fingerprint.get_or_init(|| { let inherited_rustflags = std::env::var_os("RUSTFLAGS"); + let inherited_cargo_encoded_rustflags = std::env::var_os("CARGO_ENCODED_RUSTFLAGS"); let inherited_rustup_toolchain = std::env::var_os("RUSTUP_TOOLCHAIN"); package_cache::fingerprint( &self.options, &project_dir, inherited_rustflags.as_deref(), + inherited_cargo_encoded_rustflags.as_deref(), inherited_rustup_toolchain.as_deref(), MIDENC_BUILD_VERSION, MIDENC_BUILD_REV, diff --git a/midenc-session/src/package_cache.rs b/midenc-session/src/package_cache.rs index 19465cdba..0d490f00e 100644 --- a/midenc-session/src/package_cache.rs +++ b/midenc-session/src/package_cache.rs @@ -350,6 +350,7 @@ pub(crate) fn fingerprint( options: &Options, project_dir: &Path, inherited_rustflags: Option<&OsStr>, + inherited_cargo_encoded_rustflags: Option<&OsStr>, inherited_rustup_toolchain: Option<&OsStr>, compiler_version: &str, compiler_revision: &str, @@ -357,7 +358,13 @@ pub(crate) fn fingerprint( let mut transcript = Transcript::new(); transcript.field("compiler.version", compiler_version.as_bytes()); transcript.field("compiler.revision", compiler_revision.as_bytes()); - record_options(&mut transcript, options, inherited_rustflags, inherited_rustup_toolchain); + record_options( + &mut transcript, + options, + inherited_rustflags, + inherited_cargo_encoded_rustflags, + inherited_rustup_toolchain, + ); let source_manager = DefaultSourceManager::default(); let mut manifests = ManifestClosure::new(&mut transcript, &source_manager); @@ -421,6 +428,7 @@ fn record_options( transcript: &mut Transcript, options: &Options, inherited_rustflags: Option<&OsStr>, + inherited_cargo_encoded_rustflags: Option<&OsStr>, inherited_rustup_toolchain: Option<&OsStr>, ) { let Options { @@ -496,9 +504,13 @@ fn record_options( "options.inherited_rustflags", inherited_rustflags.map(OsStr::as_encoded_bytes), ); - // Inherited `CARGO_ENCODED_RUSTFLAGS` is deliberately NOT fingerprinted: `cargo_env` sets - // the variable authoritatively for every nested build, so the inherited value has no effect - // on what gets built. + // Both inherited spellings are fingerprinted: the nested build merges them into its composed + // flag list (the encoded variable taking cargo's precedence over the plain one), so either + // one changes what gets built. + transcript.optional_bytes_field( + "options.inherited_cargo_encoded_rustflags", + inherited_cargo_encoded_rustflags.map(OsStr::as_encoded_bytes), + ); transcript.optional_bytes_field( "options.inherited_rustup_toolchain", inherited_rustup_toolchain.map(OsStr::as_encoded_bytes), @@ -1116,7 +1128,7 @@ mod tests { /// Computes a test fingerprint with a fresh source manager. fn test_fingerprint(options: &Options, project_dir: &Path, version: &str, rev: &str) -> String { - fingerprint(options, project_dir, None, None, version, rev) + fingerprint(options, project_dir, None, None, None, version, rev) } #[test] @@ -1236,12 +1248,33 @@ mod tests { write_project(temp.path(), "root", ""); let options = Options::default(); - let missing = fingerprint(&options, temp.path(), None, None, "1.2.3", "abc123"); + let missing = fingerprint(&options, temp.path(), None, None, None, "1.2.3", "abc123"); let present = fingerprint( &options, temp.path(), Some(OsStr::new("-C target-feature=+bulk-memory")), None, + None, + "1.2.3", + "abc123", + ); + + assert_ne!(missing, present); + } + + #[test] + fn fingerprint_changes_with_inherited_cargo_encoded_rustflags() { + let temp = TempDir::new().unwrap(); + write_project(temp.path(), "root", ""); + let options = Options::default(); + + let missing = fingerprint(&options, temp.path(), None, None, None, "1.2.3", "abc123"); + let present = fingerprint( + &options, + temp.path(), + None, + Some(OsStr::new("-Ctarget-feature=+bulk-memory\u{1f}--cfg=fixture")), + None, "1.2.3", "abc123", ); @@ -1255,11 +1288,12 @@ mod tests { write_project(temp.path(), "root", ""); let options = Options::default(); - let missing = fingerprint(&options, temp.path(), None, None, "1.2.3", "abc123"); + let missing = fingerprint(&options, temp.path(), None, None, None, "1.2.3", "abc123"); let present = fingerprint( &options, temp.path(), None, + None, Some(OsStr::new("nightly-2026-08-05")), "1.2.3", "abc123", From 268d30b0688a6e9a2d1b1b0c2a126807738f944b Mon Sep 17 00:00:00 2001 From: Denys Zadorozhnyi Date: Mon, 10 Aug 2026 12:02:51 +0300 Subject: [PATCH 15/21] fix(session): serialize identical-fingerprint builds and fingerprint search paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The builder lock was shared, so two concurrent builds with one fingerprint both published to the same package filenames — and because search paths were excluded from the fingerprint, two builds differing only in -L directories could legitimately share a directory while resolving different libraries, letting one build's macros read the other build's procedure roots while the assembler links its own in-memory package. Builders now hold the exclusive lock for the registry's lifetime, so identical-fingerprint builds serialize: the second waits for the first instead of interleaving writes and reads in a shared directory. The wait remains deadlock-free — pruners never block and never wait while holding a lock, and a builder waits only for its own lock while holding none. And search paths join the fingerprint: -l resolution scans them for the first stem match, so they select packages exactly like the recorded sysroot and per-library paths, and same-fingerprint builds must agree on selection now that they trust each other's files in sequence. --- midenc-session/src/package_cache.rs | 98 +++++++++++++++++++++-------- midenc-session/src/registry.rs | 11 ++-- 2 files changed, 78 insertions(+), 31 deletions(-) diff --git a/midenc-session/src/package_cache.rs b/midenc-session/src/package_cache.rs index 0d490f00e..458d2b875 100644 --- a/midenc-session/src/package_cache.rs +++ b/midenc-session/src/package_cache.rs @@ -68,7 +68,7 @@ fn is_fingerprint(name: &str) -> bool { /// The extension of a permanent sibling lock file for a fingerprint directory. const BUILD_LOCK_EXTENSION: &str = "lock"; -/// Prepares a filesystem package cache and returns its lifetime shared lock when available. +/// Prepares a filesystem package cache and returns its lifetime builder lock when available. /// /// Deletion is defense in depth. The primary invalidation is in the FPI expansion itself: it /// records `option_env!("MIDENC_PACKAGE_CACHE")`, whose value carries the fingerprinted cache @@ -76,12 +76,15 @@ const BUILD_LOCK_EXTENSION: &str = "lock"; /// the `include_bytes!` targets of pre-fingerprint expansions, bounds stale package directories, /// and takes dead caches out of circulation promptly. /// -/// A build holds a shared `packages/.lock` lock for its registry's lifetime. Pruning -/// takes the corresponding exclusive lock while deleting the sibling directory. The permanent -/// lock lives outside that directory and is acquired before the directory is created, closing the +/// A build holds the exclusive `packages/.lock` lock for its registry's lifetime, +/// so identical-fingerprint builds serialize: the second waits for the first instead of sharing +/// the directory and rewriting the same package files with possibly divergent bytes. Pruning +/// takes the same lock non-blockingly while deleting the sibling directory. The permanent lock +/// lives outside that directory and is acquired before the directory is created, closing the /// create-before-lock, unlock-before-delete, and unlink/recreate inode-ABA windows. /// -/// `Some` means the shared liveness lock is held, even when a later preparation step degraded. +/// `Some` means the exclusive liveness lock is held, even when a later preparation step +/// degraded. /// `None` means locking was skipped: the path is unowned, or opening/locking the lock file /// failed. Every leg keeps the cache configured and the cache directory created when possible, /// so package publication proceeds against the expected path and reports any concrete @@ -106,7 +109,7 @@ pub(crate) fn prepare_and_lock_filesystem_cache(filesystem_cache: &Path) -> Opti return None; }; if !create_current_filesystem_cache(filesystem_cache) { - // The shared lock is already held — keep it. A later publication may still recreate + // The exclusive lock is already held — keep it. A later publication may still recreate // the directory (its writer creates parent directories), and the lock is what stops a // concurrent pruner from classifying that recreated cache as dead. Only the sweep is // skipped in this degraded mode. @@ -146,7 +149,7 @@ fn create_filesystem_cache_parent(parent: &Path) -> bool { true } -/// Creates or recreates the current cache directory after its shared lock is held. +/// Creates or recreates the current cache directory after its builder lock is held. fn create_current_filesystem_cache(filesystem_cache: &Path) -> bool { if let Err(err) = fs::create_dir_all(filesystem_cache) { log::warn!( @@ -159,7 +162,7 @@ fn create_current_filesystem_cache(filesystem_cache: &Path) -> bool { true } -/// Opens the current fingerprint's sibling lock file and holds a shared builder lock. +/// Opens the current fingerprint's sibling lock file and holds the exclusive builder lock. /// /// Waiting is deadlock-free: pruners only try exclusive locks and never wait while holding one, /// while a builder waits only for its own lock and holds no other lock. The wait is therefore @@ -184,7 +187,7 @@ fn acquire_filesystem_cache_lock(filesystem_cache: &Path) -> Option { } }; - if let Err(err) = lock.lock_shared() { + if let Err(err) = lock.lock() { log::warn!( target: "package-registry", "failed to lock filesystem package cache '{}': {err}; continuing without a liveness lock", @@ -443,7 +446,7 @@ fn record_options( optimize, debug, output_types: _, - search_paths: _, + search_paths, link_libraries, link_modules: _, sysroot, @@ -480,9 +483,12 @@ fn record_options( // Deliberate exclusions are classified here so adding an `Options` field forces a choice. // Output paths, naming, diagnostics, printing, and stop flags do not select dependency - // packages. Search paths, link modules, remapped paths, custom flags, and similar - // content-affecting controls self-heal through the in-run package rewrite; the package-name - // set itself is driven by the manifest closure recorded below. + // packages. Link modules, remapped paths, custom flags, and similar content-affecting + // controls self-heal through the in-run package rewrite; the package-name set itself is + // driven by the manifest closure recorded below. Search paths ARE recorded: `-l` resolution + // scans them for the first stem match, so they select packages the same way the recorded + // sysroot and per-library paths do — and same-fingerprint builds must agree on selection, + // because they serialize on one directory and trust each other's files. transcript.field("options.profile", profile.as_bytes()); transcript.field("options.optimize", opt_level_name(*optimize).as_bytes()); transcript.field("options.debug", debug_info_name(*debug).as_bytes()); @@ -526,6 +532,16 @@ fn record_options( transcript.field("options.link_library.linkage", linkage.as_bytes()); } + let mut search_paths = search_paths + .iter() + .map(|path| path.as_os_str().as_encoded_bytes().to_vec()) + .collect::>(); + search_paths.sort(); + transcript.field("options.search_paths.count", &(search_paths.len() as u64).to_le_bytes()); + for path in search_paths { + transcript.field("options.search_path", &path); + } + transcript.optional_bytes_field( "options.sysroot", sysroot.as_deref().map(|path| path.as_os_str().as_encoded_bytes()), @@ -890,7 +906,7 @@ mod tests { .truncate(false) .open(&live_precreation_lock_path) .unwrap(); - live_precreation_lock.try_lock_shared().unwrap(); + live_precreation_lock.try_lock().unwrap(); fs::write(&unrelated_file, b"unrelated").unwrap(); let current_lock = @@ -957,7 +973,7 @@ mod tests { let builder_lock = completed_rx .recv_timeout(Duration::from_secs(5)) .unwrap() - .expect("the builder must acquire a shared lock after pruning completes"); + .expect("the builder must acquire the lock after pruning completes"); builder.join().unwrap(); assert!(current.is_dir(), "the builder must recreate the pruned cache directory"); let exclusive_contender = @@ -1025,7 +1041,7 @@ mod tests { .truncate(false) .open(&stale_lock_path) .unwrap(); - stale_lock.try_lock_shared().unwrap(); + stale_lock.try_lock().unwrap(); let current_lock = prepare_and_lock_filesystem_cache(¤t).expect("current cache must be locked"); @@ -1035,21 +1051,24 @@ mod tests { .write(true) .open(filesystem_cache_lock_path(¤t)) .unwrap(); - current_contender.try_lock_shared().unwrap(); + assert!( + matches!(current_contender.try_lock_shared(), Err(TryLockError::WouldBlock)), + "the builder lock is exclusive; nothing else may attach to a live cache" + ); assert!(stale.exists(), "a live sibling cache must not be pruned"); drop(stale_lock); + drop(current_lock); let second_lock = prepare_and_lock_filesystem_cache(¤t) - .expect("same-input builders share the lock"); + .expect("the next build must acquire the released lock"); assert!(!stale.exists(), "the stale cache must be pruned after its build exits"); assert!(stale_lock_path.exists(), "the stale sibling lock must remain permanent"); drop(second_lock); - drop(current_lock); } #[test] - fn same_fingerprint_contender_remains_live_after_first_builder_exits() { + fn same_fingerprint_contender_waits_for_the_first_builder() { let temp = TempDir::new().unwrap(); let parent = temp.path().join("miden").join("packages"); let shared = parent.join("fedcba9876543210"); @@ -1057,17 +1076,30 @@ mod tests { let first = prepare_and_lock_filesystem_cache(&shared).expect("first builder must lock the cache"); - let contender = prepare_and_lock_filesystem_cache(&shared) - .expect("same-input contender must share the lock"); - drop(first); + + let (completed_tx, completed_rx) = mpsc::channel(); + let thread_cache = shared.clone(); + let contender = std::thread::spawn(move || { + completed_tx.send(prepare_and_lock_filesystem_cache(&thread_cache)).unwrap(); + }); + assert!( + completed_rx.recv_timeout(Duration::from_millis(100)).is_err(), + "an identical-fingerprint contender must wait for the first builder" + ); let different_lock = prepare_and_lock_filesystem_cache(&different) - .expect("different-input builder must lock its cache"); + .expect("a different-input builder must lock its own cache without waiting"); + assert!(shared.exists(), "the waiting contender's cache must not be pruned"); - assert!(shared.exists(), "the live contender's cache must not be pruned"); + drop(first); + let contender_lock = completed_rx + .recv_timeout(Duration::from_secs(5)) + .unwrap() + .expect("the contender must acquire the lock once the first builder exits"); + contender.join().unwrap(); drop(different_lock); - drop(contender); + drop(contender_lock); } #[test] @@ -1242,6 +1274,20 @@ mod tests { assert_ne!(baseline, test_fingerprint(&optimized, temp.path(), "1.2.3", "abc123")); } + #[test] + fn fingerprint_changes_with_search_paths() { + let temp = TempDir::new().unwrap(); + write_project(temp.path(), "root", ""); + let options = Options::default(); + let baseline = test_fingerprint(&options, temp.path(), "1.2.3", "abc123"); + + // Search paths select which package a bare `-l` name resolves to, so they are part of + // the fingerprint like the sysroot and per-library paths feeding the same decision. + let mut with_search_path = options.clone(); + with_search_path.search_paths.push(temp.path().join("libs")); + assert_ne!(baseline, test_fingerprint(&with_search_path, temp.path(), "1.2.3", "abc123")); + } + #[test] fn fingerprint_changes_with_inherited_rustflags() { let temp = TempDir::new().unwrap(); diff --git a/midenc-session/src/registry.rs b/midenc-session/src/registry.rs index a47a46e8f..ace944fa8 100644 --- a/midenc-session/src/registry.rs +++ b/midenc-session/src/registry.rs @@ -40,7 +40,7 @@ pub struct HybridPackageRegistry { artifacts: FxHashMap>>, #[cfg(any(test, feature = "std"))] filesystem_cache: Option, - /// Holds the current fingerprint's shared liveness lock for the registry's lifetime. + /// Holds the current fingerprint's exclusive liveness lock for the registry's lifetime. /// /// The file is never read; dropping it releases this cache directory for pruning. `None` /// means locking was intentionally skipped or cache preparation degraded after an error. @@ -427,9 +427,10 @@ mod tests { assert!(current.is_dir()); assert!(registry.filesystem_cache_lock.is_some()); let lock_path = current.with_extension("lock"); - let contender = OpenOptions::new().read(true).write(true).open(&lock_path).unwrap(); - contender.try_lock_shared().unwrap(); - let stale_checker = OpenOptions::new().read(true).write(true).open(lock_path).unwrap(); - assert!(matches!(stale_checker.try_lock(), Err(std::fs::TryLockError::WouldBlock))); + let contender = OpenOptions::new().read(true).write(true).open(lock_path).unwrap(); + assert!( + matches!(contender.try_lock(), Err(std::fs::TryLockError::WouldBlock)), + "the registry must hold the exclusive builder lock for its lifetime" + ); } } From 8f9663c78ad6d665a93b1efd3809a15388eb84d1 Mon Sep 17 00:00:00 2001 From: Denys Zadorozhnyi Date: Mon, 10 Aug 2026 12:04:08 +0300 Subject: [PATCH 16/21] fix(session): reach the cache sweep only through session-derived paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The public constructor accepted any caller-supplied path and, when its final components happened to spell miden/packages/<16-hex>, enabled recursive deletion of sibling fingerprint directories and flat .masp files — a path suffix is not proof that midenc owns a location. The public constructor now only creates and configures the directory; it never locks and never sweeps. The cache lifecycle protocol moves to a crate-private constructor that Session::package_registry uses for the paths it derives itself, whose owned layout the derivation guarantees (pruning re-checks it lexically as a belt). A test pins that an owned-looking caller-supplied path neither sweeps its parent nor leaves a lock file behind. Also updates the module doc left stale by the exclusive-lock change: identical-fingerprint builds serialize on the builder lock rather than sharing the directory. --- midenc-session/src/lib.rs | 2 +- midenc-session/src/package_cache.rs | 8 +-- midenc-session/src/registry.rs | 87 +++++++++++++++++++++++++---- 3 files changed, 80 insertions(+), 17 deletions(-) diff --git a/midenc-session/src/lib.rs b/midenc-session/src/lib.rs index dc407aca2..b4416da9c 100644 --- a/midenc-session/src/lib.rs +++ b/midenc-session/src/lib.rs @@ -362,7 +362,7 @@ impl Session { /// Get a new package registry instance for this session pub fn package_registry(&self) -> Result, Report> { - registry::HybridPackageRegistry::new_with_filesystem_cache( + registry::HybridPackageRegistry::new_with_derived_filesystem_cache( &self.options, self.filesystem_package_cache_dir(), ) diff --git a/midenc-session/src/package_cache.rs b/midenc-session/src/package_cache.rs index 458d2b875..6263e8507 100644 --- a/midenc-session/src/package_cache.rs +++ b/midenc-session/src/package_cache.rs @@ -9,10 +9,10 @@ //! checksum freshness is enabled. //! Expansions also record `MIDENC_PACKAGE_CACHE`, so rotating the fingerprinted path re-expands //! consumers even if best-effort stale-directory pruning does not complete. -//! Concurrent builds with the same fingerprint share one directory. Package publication uses a -//! temporary file followed by atomic rename, preventing consumers from reading a torn package. -//! A macro read and rustc's later `include_bytes!` evaluation can still straddle a concurrent -//! rewrite; that narrow same-fingerprint window is accepted and bounded by the every-run rewrite. +//! Concurrent builds with the same fingerprint serialize on the directory's exclusive builder +//! lock, so their publications and macro reads never interleave. Package publication still uses +//! a temporary file followed by atomic rename, so even a reader outside the protocol cannot see +//! a torn package. //! Registry and git dependencies contribute declaration text only; in particular, a git branch //! moving without a manifest edit is outside this fingerprint by design, as are a git package's //! transitive dependencies. Pinning a revision or deleting the cache directory recovers from a diff --git a/midenc-session/src/registry.rs b/midenc-session/src/registry.rs index ace944fa8..e9f2a7a21 100644 --- a/midenc-session/src/registry.rs +++ b/midenc-session/src/registry.rs @@ -73,26 +73,60 @@ impl HybridPackageRegistry { /// Get a new instance of the registry, using the current compiler options and an optional /// filesystem cache directory. /// - /// A cache path in the owned `miden/packages/` layout is prepared and, when that - /// succeeds, locked for the registry's lifetime. During construction, dead sibling fingerprint - /// directories and legacy flat `.masp` entries are pruned as defense in depth; FPI expansions - /// track the cache path themselves for correctness. Any other path is created if possible but - /// deliberately neither locked nor used to sweep its parent. Ownership is checked lexically by - /// design; symlinked cache layouts are outside this contract. - /// - /// Cache preparation and cleanup failures are reported only through the `package-registry` log - /// target. The configured cache remains enabled after a preparation failure, so the first - /// package publication reports the concrete filesystem error to the caller. + /// The directory is created when possible and every package installed into the registry is + /// published into it, but a caller-supplied path is NEVER locked and NEVER used to sweep its + /// parent — a path shape alone does not prove midenc owns the location. The locking and + /// stale-cache pruning protocol runs only for cache paths the `Session` itself derives; see + /// [`Self::new_with_derived_filesystem_cache`]. #[cfg(any(test, feature = "std"))] pub fn new_with_filesystem_cache( options: &crate::Options, filesystem_cache: Option, ) -> Result { - use alloc::string::ToString; + if let Some(filesystem_cache) = filesystem_cache.as_deref() + && let Err(err) = std::fs::create_dir_all(filesystem_cache) + { + log::warn!( + target: "package-registry", + "failed to create filesystem package cache '{}': {err}; keeping the cache configured so package publication reports the failure", + filesystem_cache.display() + ); + } + Self::construct(options, filesystem_cache, None) + } + /// Get a new instance of the registry for a `Session`-derived filesystem cache path. + /// + /// This is the only entry that runs the cache lifecycle protocol: the fingerprint directory + /// is prepared and locked for the registry's lifetime, and dead sibling fingerprint + /// directories plus legacy flat `.masp` entries are pruned as defense in depth — FPI + /// expansions track the cache path themselves for correctness. It is crate-private because + /// the destructive sweep must be reachable only for internally derived paths, whose owned + /// `miden/packages/` layout the session derivation guarantees (and pruning + /// re-checks lexically as a belt; symlinked layouts are outside the contract). + /// + /// Cache preparation and cleanup failures are reported only through the `package-registry` + /// log target. The configured cache remains enabled after a preparation failure, so the + /// first package publication reports the concrete filesystem error to the caller. + #[cfg(any(test, feature = "std"))] + pub(crate) fn new_with_derived_filesystem_cache( + options: &crate::Options, + filesystem_cache: Option, + ) -> Result { let filesystem_cache_lock = filesystem_cache .as_deref() .and_then(crate::package_cache::prepare_and_lock_filesystem_cache); + Self::construct(options, filesystem_cache, filesystem_cache_lock) + } + + /// Builds the registry with system libraries, link libraries, and the given cache state. + #[cfg(any(test, feature = "std"))] + fn construct( + options: &crate::Options, + filesystem_cache: Option, + filesystem_cache_lock: Option, + ) -> Result { + use alloc::string::ToString; // Load system libraries let mut registry = if options.sysroot.is_some() { @@ -417,7 +451,7 @@ mod tests { let temp = TempDir::new().unwrap(); let current = temp.path().join("miden").join("packages").join("fedcba9876543210"); - let registry = HybridPackageRegistry::new_with_filesystem_cache( + let registry = HybridPackageRegistry::new_with_derived_filesystem_cache( &crate::Options::default(), Some(current.clone()), ) @@ -433,4 +467,33 @@ mod tests { "the registry must hold the exclusive builder lock for its lifetime" ); } + + #[test] + fn public_constructor_never_sweeps_or_locks_a_caller_supplied_path() { + let temp = TempDir::new().unwrap(); + let parent = temp.path().join("miden").join("packages"); + let current = parent.join("fedcba9876543210"); + let sibling = parent.join("0123456789abcdef"); + std::fs::create_dir_all(&sibling).unwrap(); + + let registry = HybridPackageRegistry::new_with_filesystem_cache( + &crate::Options::default(), + Some(current.clone()), + ) + .unwrap(); + + assert!(current.is_dir(), "the configured cache directory is still created"); + assert!( + sibling.exists(), + "a caller-supplied path must never sweep its parent, owned-looking or not" + ); + assert!( + registry.filesystem_cache_lock.is_none(), + "a caller-supplied path is never locked" + ); + assert!( + !current.with_extension("lock").exists(), + "no lock file is created for a caller-supplied path" + ); + } } From 5e9b306e1c8dfdf1c25b8b9655b655f5c2d7d389 Mon Sep 17 00:00:00 2001 From: Denys Zadorozhnyi Date: Mon, 10 Aug 2026 12:08:55 +0300 Subject: [PATCH 17/21] fix(session): fingerprint workspace members declared by their manifest path miden-project accepts a workspace member referenced either by its directory or by its manifest file (dep/miden-project.toml), and such paths are workspace-root-relative. The fingerprint walk missed both halves of that: the Workspace-scheme lookup compared the manifest-file spelling against member directories, and a member-naming dependency that arrives Path-classified (which is what Project::load produces) was resolved against the declaring manifest's directory instead of the workspace root. Either way the member recorded an unresolved marker and its manifest closure escaped the fingerprint. Both arms now resolve through one helper that normalizes a manifest-file spelling to its parent and looks the member up in the loaded workspace, falling back to ordinary path resolution for non-members. A regression test declares a member by manifest path and asserts the member's manifest changes rotate the fingerprint. --- midenc-session/src/package_cache.rs | 66 ++++++++++++++++++++++++++--- 1 file changed, 60 insertions(+), 6 deletions(-) diff --git a/midenc-session/src/package_cache.rs b/midenc-session/src/package_cache.rs index 6263e8507..82f8b2cff 100644 --- a/midenc-session/src/package_cache.rs +++ b/midenc-session/src/package_cache.rs @@ -710,7 +710,16 @@ impl<'a> ManifestClosure<'a> { match dependency.scheme() { DependencyVersionScheme::Registry(_) => {} DependencyVersionScheme::Path { path, .. } => { - self.visit_path_dependency(dependency, manifest_dir, path.inner()); + // A path dependency naming a workspace member (by directory or by manifest + // file, both accepted spellings) is workspace-root-relative, not + // manifest-dir-relative — mirror the classifier and resolve it through the + // member list first. + if let Some(member_manifest) = workspace_member_manifest(workspace, path.inner()) + { + self.visit_project(&member_manifest, Some(dependency.name().as_ref())); + } else { + self.visit_path_dependency(dependency, manifest_dir, path.inner()); + } } DependencyVersionScheme::WorkspacePath { path, .. } => { if let Some(workspace_root) = @@ -727,11 +736,7 @@ impl<'a> ManifestClosure<'a> { } } DependencyVersionScheme::Workspace { member, .. } => { - if let Some(manifest_path) = workspace - .and_then(|workspace| { - workspace.get_member_by_relative_path(member.inner().path()) - }) - .and_then(|package| package.manifest_path().map(Path::to_path_buf)) + if let Some(manifest_path) = workspace_member_manifest(workspace, member.inner()) { self.visit_project(&manifest_path, Some(dependency.name().as_ref())); } else { @@ -852,6 +857,25 @@ fn optional_display(value: Option<&impl core::fmt::Display>) -> String { value.map(ToString::to_string).unwrap_or_else(|| "".into()) } +/// Resolves a dependency URI to a workspace member's manifest path, when it names one. +/// +/// Members may be declared by their directory (`dep`) or by their manifest file +/// (`dep/miden-project.toml`) — both spellings are accepted by `miden-project`'s classifier and +/// are workspace-root-relative. The workspace lookup compares member directories, so the +/// manifest-file spelling is normalized to its parent first. +fn workspace_member_manifest( + workspace: Option<&miden_project::Workspace>, + uri: &miden_project::Uri, +) -> Option { + if uri.scheme().is_some_and(|scheme| scheme != "file") { + return None; + } + let member_dir = locator_project_dir(Path::new(uri.path())); + workspace? + .get_member_by_relative_path(member_dir.as_path()) + .and_then(|package| package.manifest_path().map(Path::to_path_buf)) +} + /// Returns the directory whose sibling project manifests describe a locator. fn locator_project_dir(locator: &Path) -> PathBuf { if locator.file_name().is_some_and(|name| { @@ -1274,6 +1298,36 @@ mod tests { assert_ne!(baseline, test_fingerprint(&optimized, temp.path(), "1.2.3", "abc123")); } + #[test] + fn fingerprint_walks_workspace_members_declared_by_manifest_path() { + let temp = TempDir::new().unwrap(); + let root = temp.path(); + std::fs::write( + root.join("miden-project.toml"), + "[workspace]\nmembers = [\"dep\", \"app\"]\n\n[workspace.package]\nversion = \"1.0.0\"\n", + ) + .unwrap(); + write_project(&root.join("dep"), "dep", ""); + // The member is declared by its manifest FILE, a spelling miden-project accepts and + // classifies as a workspace member; the lookup must normalize it to the member dir. + write_project( + &root.join("app"), + "app", + "\n[dependencies]\ndep = { path = \"dep/miden-project.toml\" }\n", + ); + let options = Options::default(); + let before = test_fingerprint(&options, &root.join("app"), "1.2.3", "abc123"); + + std::fs::write( + root.join("dep").join("Cargo.toml"), + "[package]\nname = \"dep\"\nversion = \"2.0.0\"\n", + ) + .unwrap(); + let after = test_fingerprint(&options, &root.join("app"), "1.2.3", "abc123"); + + assert_ne!(before, after, "the member's manifest closure must be part of the fingerprint"); + } + #[test] fn fingerprint_changes_with_search_paths() { let temp = TempDir::new().unwrap(); From ea7c95393af34c9e30da1c8af5e49037e325d96b Mon Sep 17 00:00:00 2001 From: Denys Zadorozhnyi Date: Mon, 10 Aug 2026 12:11:38 +0300 Subject: [PATCH 18/21] fix(base-macros): qualify the emitted cache-tracking paths The rebuild-tracking constants land in the consumer crate's scope, where a user-defined Option type, option_env macro, or include_bytes macro would shadow the unqualified names and silently break the cache tracking. The emissions now spell ::core::option::Option, ::core::option_env!, and ::core::include_bytes!, extracted into two documented helpers with a unit test pinning the qualified spellings. --- sdk/base-macros/src/fpi.rs | 59 +++++++++++++++++++++++++++----------- 1 file changed, 43 insertions(+), 16 deletions(-) diff --git a/sdk/base-macros/src/fpi.rs b/sdk/base-macros/src/fpi.rs index 2aeac30e8..1deaf6ff4 100644 --- a/sdk/base-macros/src/fpi.rs +++ b/sdk/base-macros/src/fpi.rs @@ -1162,18 +1162,9 @@ pub(crate) fn augment_foreign_account_bindings( let bindings = file.into_token_stream(); let package_includes = include_paths .into_iter() - .map(|path| { - let utf8_path = path.to_str().ok_or_else(|| { - Error::new( - Span::call_site(), - format!("path '{}' contains invalid UTF-8", path.display()), - ) - })?; - Ok(quote! { - const _: &[u8] = include_bytes!(#utf8_path); - }) - }) + .map(|path| package_include_tokens(&path)) .collect::>>()?; + let cache_tracking = package_cache_tracking_tokens(); Ok(quote! { #[doc(hidden)] @@ -1188,14 +1179,36 @@ pub(crate) fn augment_foreign_account_bindings( #(#trait_items)* #active_account_item #(#package_includes)* - // Record the package cache location in the consumer's dep-info. The value carries the - // build-input fingerprint, so Cargo re-expands this macro whenever the fingerprint - // rotates — even when a stale cache directory survives on disk. The `include_bytes!` - // constants above cover content changes at an unchanged path. - const _: Option<&str> = option_env!("MIDENC_PACKAGE_CACHE"); + #cache_tracking }) } +/// Emits the rebuild-tracking constant for one dependency package file. +/// +/// Everything here lands in consumer scope, so the macro path is fully qualified: a +/// consumer-defined `include_bytes` must not shadow the rebuild tracking. +fn package_include_tokens(path: &std::path::Path) -> syn::Result { + let utf8_path = path.to_str().ok_or_else(|| { + Error::new(Span::call_site(), format!("path '{}' contains invalid UTF-8", path.display())) + })?; + Ok(quote! { + const _: &[u8] = ::core::include_bytes!(#utf8_path); + }) +} + +/// Emits the dep-info record of the package cache location. +/// +/// The value carries the build-input fingerprint, so Cargo re-expands the consumer whenever the +/// fingerprint rotates — even when a stale cache directory survives on disk. The +/// `include_bytes!` constants cover content changes at an unchanged path. Both the type and the +/// macro are fully qualified because this lands in consumer scope, where a user-defined `Option` +/// or `option_env` must not shadow the tracking. +fn package_cache_tracking_tokens() -> TokenStream2 { + quote! { + const _: ::core::option::Option<&str> = ::core::option_env!("MIDENC_PACKAGE_CACHE"); + } +} + /// Builds one generated component trait and its empty attachment impl for the account wrapper. /// /// The trait is emitted with the wrapper struct's visibility (`vis`): a `pub` wrapper gets a `pub` @@ -2153,6 +2166,20 @@ interface api { std::fs::remove_dir_all(temp_root).unwrap(); } + #[test] + fn emitted_cache_tracking_is_hygienic_in_consumer_scope() { + // The emitted constants land in the consumer crate's scope. Fully qualified paths keep + // the tracking working where a user defines their own `Option`, `option_env`, or + // `include_bytes`. + let tracking = package_cache_tracking_tokens().to_string(); + assert!(tracking.contains(":: core :: option :: Option"), "{tracking}"); + assert!(tracking.contains(":: core :: option_env !"), "{tracking}"); + assert!(tracking.contains("MIDENC_PACKAGE_CACHE"), "{tracking}"); + + let include = package_include_tokens(Path::new("/cache/dep.masp")).unwrap().to_string(); + assert!(include.contains(":: core :: include_bytes !"), "{include}"); + } + #[test] fn missing_cached_dependency_package_message_describes_the_cache_contract() { let dependency = SelectedDependency { From a6538452246a30414ab987b3ecba01f037e9fb3e Mon Sep 17 00:00:00 2001 From: Denys Zadorozhnyi Date: Mon, 10 Aug 2026 12:12:06 +0300 Subject: [PATCH 19/21] chore: apply rustfmt to the review fix batch --- midenc-compile/src/pipeline/frontends/rust.rs | 10 ++++++---- midenc-session/src/package_cache.rs | 9 ++++----- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/midenc-compile/src/pipeline/frontends/rust.rs b/midenc-compile/src/pipeline/frontends/rust.rs index 3de1e9a2c..6d48e091b 100644 --- a/midenc-compile/src/pipeline/frontends/rust.rs +++ b/midenc-compile/src/pipeline/frontends/rust.rs @@ -1653,10 +1653,12 @@ pub(crate) mod manifest { explicit: Option<&str>, ) -> Vec { let mut args: Vec = mandatory.iter().map(|flag| flag.to_string()).collect(); - let encoded = - inherited_encoded.and_then(|value| value.to_str()).filter(|value| !value.is_empty()); - let plain = - inherited_plain.and_then(|value| value.to_str()).filter(|value| !value.is_empty()); + let encoded = inherited_encoded + .and_then(|value| value.to_str()) + .filter(|value| !value.is_empty()); + let plain = inherited_plain + .and_then(|value| value.to_str()) + .filter(|value| !value.is_empty()); if let Some(encoded) = encoded { args.extend(encoded.split('\x1f').filter(|arg| !arg.is_empty()).map(str::to_string)); } else if let Some(plain) = plain { diff --git a/midenc-session/src/package_cache.rs b/midenc-session/src/package_cache.rs index 82f8b2cff..981992301 100644 --- a/midenc-session/src/package_cache.rs +++ b/midenc-session/src/package_cache.rs @@ -714,8 +714,7 @@ impl<'a> ManifestClosure<'a> { // file, both accepted spellings) is workspace-root-relative, not // manifest-dir-relative — mirror the classifier and resolve it through the // member list first. - if let Some(member_manifest) = workspace_member_manifest(workspace, path.inner()) - { + if let Some(member_manifest) = workspace_member_manifest(workspace, path.inner()) { self.visit_project(&member_manifest, Some(dependency.name().as_ref())); } else { self.visit_path_dependency(dependency, manifest_dir, path.inner()); @@ -736,8 +735,7 @@ impl<'a> ManifestClosure<'a> { } } DependencyVersionScheme::Workspace { member, .. } => { - if let Some(manifest_path) = workspace_member_manifest(workspace, member.inner()) - { + if let Some(manifest_path) = workspace_member_manifest(workspace, member.inner()) { self.visit_project(&manifest_path, Some(dependency.name().as_ref())); } else { log::debug!( @@ -1304,7 +1302,8 @@ mod tests { let root = temp.path(); std::fs::write( root.join("miden-project.toml"), - "[workspace]\nmembers = [\"dep\", \"app\"]\n\n[workspace.package]\nversion = \"1.0.0\"\n", + "[workspace]\nmembers = [\"dep\", \"app\"]\n\n[workspace.package]\nversion = \ + \"1.0.0\"\n", ) .unwrap(); write_project(&root.join("dep"), "dep", ""); From 9f9f77c282a659691676b840570774c5bd48fa50 Mon Sep 17 00:00:00 2001 From: Denys Zadorozhnyi Date: Mon, 10 Aug 2026 12:58:06 +0300 Subject: [PATCH 20/21] test(cargo-miden): spell the package extension inline The rebase target removed miden-mast-package from cargo-miden's dependencies, so the test can no longer name Package::EXTENSION; the literal carries a comment pointing at its source of truth. --- tools/cargo-miden/tests/masm_dependency.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/cargo-miden/tests/masm_dependency.rs b/tools/cargo-miden/tests/masm_dependency.rs index 1111ebb16..ca069b655 100644 --- a/tools/cargo-miden/tests/masm_dependency.rs +++ b/tools/cargo-miden/tests/masm_dependency.rs @@ -179,8 +179,9 @@ fn build_rust_project_with_masm_path_dependency() { // registry, or skipped. A materialized `.masp` for the dependency is produced only by // assembling it from its Miden Assembly sources. The package cache is uniqued by the build // inputs, so the package lives inside the build's fingerprint directory. - let dependency_package_name = - format!("{dependency_name}.{}", miden_mast_package::Package::EXTENSION); + // `.masp` is `miden_mast_package::Package::EXTENSION`, spelled inline because cargo-miden + // no longer links miden-mast-package. + let dependency_package_name = format!("{dependency_name}.masp"); let dependency_package = crate::utils::package_cache_fingerprint_dir(&project_path, &dependency_package_name) .join(dependency_package_name); From 8b200345a7f375c7b3cc4ca7dc2abf1a37bab812 Mon Sep 17 00:00:00 2001 From: Denys Zadorozhnyi Date: Mon, 10 Aug 2026 16:23:39 +0300 Subject: [PATCH 21/21] fix(benches): load execution dependencies from fingerprinted package caches The examples-benchmark job fails on this branch at the first example that executes: the runner loads dependency packages from the flat `target/miden/packages/` directory, but the compiler now writes them into a fingerprinted subdirectory and sweeps flat legacy packages away. The executor then starts with no dependency forests, and execution stops on the first external procedure with a missing MAST root. Scan the package cache directory and one level of subdirectories when collecting dependencies. One runner binary drives both compilers during a comparison, so it must read the flat layout of the baseline compiler and the fingerprinted layout of the candidate. Duplicate packages across cache entries are safe because execution resolves procedures by digest. --- benches/src/lib.rs | 59 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 53 insertions(+), 6 deletions(-) diff --git a/benches/src/lib.rs b/benches/src/lib.rs index 78e327874..7eced516b 100644 --- a/benches/src/lib.rs +++ b/benches/src/lib.rs @@ -191,12 +191,7 @@ impl BenchmarkRunner { .with_context(|| format!("failed to parse {}", inputs_path.display()))?; let mut executor = Executor::from_config(config); let packages_dir = project_dir.join("target/miden/packages"); - let mut dependencies = fs::read_dir(&packages_dir) - .with_context(|| format!("failed to read {}", packages_dir.display()))? - .map(|entry| entry.map(|entry| entry.path())) - .collect::>>()?; - dependencies.retain(|path| path.extension().is_some_and(|ext| ext == "masp")); - dependencies.sort(); + let dependencies = collect_dependency_packages(&packages_dir)?; for dependency in dependencies { executor .with_package(load_package(&dependency)?) @@ -214,6 +209,42 @@ impl BenchmarkRunner { } } +/// Collects the dependency packages that the executor must load for an example. +/// +/// The compiler stores dependency packages in the project package cache. Old compiler versions +/// write them directly into `target/miden/packages/`. New compiler versions write them into a +/// fingerprinted subdirectory of that cache. One runner binary drives both compiler versions +/// during a benchmark comparison, so the scan reads the cache directory and one level of +/// subdirectories. Package resolution at execution time is digest-addressed, so packages from a +/// stale cache entry are inert. +fn collect_dependency_packages(packages_dir: &Path) -> Result> { + let mut dependencies = Vec::new(); + for path in read_dir_paths(packages_dir)? { + if path.is_dir() { + dependencies + .extend(read_dir_paths(&path)?.into_iter().filter(|path| is_package_path(path))); + } else if is_package_path(&path) { + dependencies.push(path); + } + } + dependencies.sort(); + Ok(dependencies) +} + +/// Lists the entry paths of a directory. +fn read_dir_paths(dir: &Path) -> Result> { + fs::read_dir(dir) + .with_context(|| format!("failed to read {}", dir.display()))? + .map(|entry| entry.map(|entry| entry.path())) + .collect::>>() + .with_context(|| format!("failed to read {}", dir.display())) +} + +/// Returns true when a path names a serialized Miden package file. +fn is_package_path(path: &Path) -> bool { + path.extension().is_some_and(|ext| ext == "masp") +} + fn discover_cases(workspace_root: &Path) -> Result> { let examples_dir = workspace_root.join("examples"); let mut cases = Vec::new(); @@ -358,6 +389,22 @@ mod tests { ); } + #[test] + fn collects_packages_from_flat_and_fingerprinted_cache_layouts() { + let cache = tempfile::tempdir().unwrap(); + fs::write(cache.path().join("legacy.masp"), []).unwrap(); + fs::write(cache.path().join("1234567890abcdef.lock"), []).unwrap(); + let fingerprint = cache.path().join("1234567890abcdef"); + fs::create_dir_all(&fingerprint).unwrap(); + fs::write(fingerprint.join("miden-core.masp"), []).unwrap(); + fs::write(fingerprint.join("README.txt"), []).unwrap(); + + assert_eq!( + collect_dependency_packages(cache.path()).unwrap(), + vec![fingerprint.join("miden-core.masp"), cache.path().join("legacy.masp")] + ); + } + #[test] fn mast_size_excludes_package_metadata() { let mut package = Assembler::default()