diff --git a/crates/pcb-zen-core/src/lang/eval.rs b/crates/pcb-zen-core/src/lang/eval.rs index eea5e022..00d763d6 100644 --- a/crates/pcb-zen-core/src/lang/eval.rs +++ b/crates/pcb-zen-core/src/lang/eval.rs @@ -37,6 +37,9 @@ use crate::lang::{ file::file_globals, module::{FrozenModuleValue, ModulePath}, }; +use crate::load_spec::LoadSpec; +use crate::resolution::ResolutionResult; +use crate::{config, FileProvider, ResolveContext}; use crate::{convert::ModuleConverter, lang::context::FrozenPendingChild}; use crate::{Diagnostic, Diagnostics, WithDiagnostics}; @@ -82,8 +85,8 @@ pub struct EvalOutput { pub signature: Vec, /// Print output collected during evaluation pub print_output: Vec, - /// Load resolver used for this evaluation, when available - pub load_resolver: Arc, + /// Eval config (file provider, path specs, etc.) + pub config: EvalContextConfig, /// Session keeps the frozen heap alive for the lifetime of this output. /// Also provides access to the module tree. session: EvalSession, @@ -165,14 +168,6 @@ impl EvalOutput { } result } - - /// If the underlying resolver is a CoreLoadResolver, return a reference to it - pub fn core_resolver(&self) -> Option> { - let load_resolver = self.load_resolver.clone(); - (load_resolver as Arc) - .downcast::() - .ok() - } } /// Handle to shared evaluation session state. Cheaply cloneable. @@ -212,8 +207,15 @@ pub struct EvalContextConfig { /// Wrapped in Arc since it's the same for all contexts. pub(crate) builtin_docs: Arc>, - /// Load resolver for resolving load() paths - pub(crate) load_resolver: Arc, + /// File provider for reading files and checking existence. + pub(crate) file_provider: Arc, + + /// Resolution result from dependency resolution. + pub(crate) resolution: Arc, + + /// Maps resolved paths to their original LoadSpecs. + /// Shared across all contexts so that nested loads see parent tracking. + pub(crate) path_to_spec: Arc>>, /// The fully qualified path of the module we are evaluating (e.g., "root", "root.child") pub(crate) module_path: ModulePath, @@ -242,11 +244,29 @@ pub struct EvalContextConfig { } impl EvalContextConfig { - /// Create a new root EvalContextConfig with the given load resolver. - pub fn new(load_resolver: Arc) -> Self { + /// Create a new root EvalContextConfig. + /// + /// The resolution's `package_resolutions` keys should already be + /// canonicalized (see [`EvalContext::new`] which handles this). + pub fn new(file_provider: Arc, resolution: Arc) -> Self { + use std::sync::OnceLock; + static BUILTIN_DOCS: OnceLock>> = OnceLock::new(); + let builtin_docs = BUILTIN_DOCS + .get_or_init(|| { + let globals = EvalContext::build_globals(); + let mut docs = HashMap::new(); + for (name, item) in globals.documentation().members { + docs.insert(name.clone(), item.render_as_code(&name)); + } + Arc::new(docs) + }) + .clone(); + Self { - builtin_docs: Arc::new(Self::build_builtin_docs()), - load_resolver, + builtin_docs, + file_provider, + resolution, + path_to_spec: Arc::new(RwLock::new(HashMap::new())), module_path: ModulePath::root(), load_chain: HashSet::new(), source_path: None, @@ -257,16 +277,6 @@ impl EvalContextConfig { } } - /// Build the builtin docs map from globals. - fn build_builtin_docs() -> HashMap { - let globals = EvalContext::build_globals(); - let mut builtin_docs = HashMap::new(); - for (name, item) in globals.documentation().members { - builtin_docs.insert(name.clone(), item.render_as_code(&name)); - } - builtin_docs - } - /// Set the source path of the module we are evaluating. pub fn set_source_path(mut self, path: PathBuf) -> Self { self.source_path = Some(path); @@ -307,7 +317,9 @@ impl EvalContextConfig { Self { builtin_docs: self.builtin_docs.clone(), - load_resolver: self.load_resolver.clone(), + file_provider: self.file_provider.clone(), + resolution: self.resolution.clone(), + path_to_spec: self.path_to_spec.clone(), module_path: child_module_path, load_chain: child_load_chain, source_path: Some(target_path), @@ -331,7 +343,9 @@ impl EvalContextConfig { Self { builtin_docs: self.builtin_docs.clone(), - load_resolver: self.load_resolver.clone(), + file_provider: self.file_provider.clone(), + resolution: self.resolution.clone(), + path_to_spec: self.path_to_spec.clone(), module_path: child_module_path, load_chain: HashSet::new(), source_path: None, @@ -341,6 +355,271 @@ impl EvalContextConfig { eager: self.eager, } } + + pub(crate) fn file_provider(&self) -> &dyn FileProvider { + &*self.file_provider + } + + fn insert_load_spec(&self, resolved_path: PathBuf, spec: LoadSpec) { + self.path_to_spec + .write() + .unwrap() + .insert(resolved_path, spec); + } + + /// Manually track a file. Useful for entrypoints. + pub fn track_file(&self, path: &Path) { + let canonical_path = self.file_provider.canonicalize(path).unwrap(); + if self.get_load_spec(&canonical_path).is_some() { + return; + } + let load_spec = LoadSpec::local_path(&canonical_path); + self.insert_load_spec(canonical_path, load_spec); + } + + /// Get the LoadSpec for a specific resolved file path. + pub fn get_load_spec(&self, path: &Path) -> Option { + self.path_to_spec.read().unwrap().get(path).cloned() + } + + /// Convenience method to resolve a load path string directly. + pub fn resolve_path(&self, path: &str, current_file: &Path) -> Result { + let mut context = self.resolve_context(path, current_file)?; + self.resolve(&mut context) + } + + /// Convenience method to resolve a LoadSpec directly. + pub fn resolve_spec( + &self, + load_spec: &LoadSpec, + current_file: &Path, + ) -> Result { + let mut context = self.resolve_context_from_spec(load_spec, current_file)?; + self.resolve(&mut context) + } + + pub(crate) fn resolve_context<'a>( + &'a self, + path: &str, + current_file: &Path, + ) -> Result, anyhow::Error> { + let load_spec = LoadSpec::parse(path) + .ok_or_else(|| anyhow::anyhow!("Invalid load path spec: {}", path))?; + self.resolve_context_from_spec(&load_spec, current_file) + } + + pub(crate) fn resolve_context_from_spec<'a>( + &'a self, + load_spec: &LoadSpec, + current_file: &Path, + ) -> Result, anyhow::Error> { + let current_file = self.file_provider.canonicalize(current_file)?; + self.track_file(¤t_file); + let context = ResolveContext::new(self.file_provider(), current_file, load_spec.clone()); + Ok(context) + } + + /// Find the package root for a given file by walking up directories. + fn find_package_root_for_file(&self, file: &Path) -> anyhow::Result { + let mut current = file.parent(); + while let Some(dir) = current { + if self.resolution.package_resolutions.contains_key(dir) { + return Ok(dir.to_path_buf()); + } + let pcb_toml = dir.join("pcb.toml"); + if self.file_provider.exists(&pcb_toml) { + return Ok(dir.to_path_buf()); + } + current = dir.parent(); + } + anyhow::bail!( + "Internal error: current file not in any package: {}", + file.display() + ) + } + + fn resolved_map_for_package_root( + &self, + package_root: &Path, + ) -> anyhow::Result<&BTreeMap> { + self.resolution + .package_resolutions + .get(package_root) + .ok_or_else(|| { + anyhow::anyhow!( + "Dependency map not loaded for package '{}'", + package_root.display() + ) + }) + } + + /// Expand alias using the resolution map. + fn expand_alias(&self, context: &ResolveContext, alias: &str) -> Result { + let package_root = self.find_package_root_for_file(&context.current_file)?; + let resolved_map = self.resolved_map_for_package_root(&package_root)?; + + for url in resolved_map.keys() { + if let Some(last_segment) = url.rsplit('/').next() { + if last_segment == alias { + return Ok(url.clone()); + } + } + } + + for (kicad_alias, base_url, _) in config::KICAD_ASSETS { + if *kicad_alias == alias { + return Ok(base_url.to_string()); + } + } + + anyhow::bail!("Unknown alias '@{}'", alias) + } + + /// Remote resolution: longest prefix match against package's declared deps. + fn try_resolve_workspace( + &self, + context: &ResolveContext, + package_root: &Path, + ) -> Result { + let spec = context.latest_spec(); + + let (base, path) = match spec { + LoadSpec::Github { + user, repo, path, .. + } => (format!("github.com/{}/{}", user, repo), path), + LoadSpec::Gitlab { + project_path, path, .. + } => (format!("gitlab.com/{}", project_path), path), + LoadSpec::Package { package, path, .. } => (package.clone(), path), + _ => unreachable!(), + }; + + let mut full_url = base; + for component in path.components() { + if let std::path::Component::Normal(part) = component { + full_url.push('/'); + full_url.push_str(&part.to_string_lossy()); + } + } + + let resolved_map = self.resolved_map_for_package_root(package_root)?; + + let best_match = resolved_map.iter().rev().find(|(dep_url, _)| { + full_url.starts_with(dep_url.as_str()) + && (full_url.len() == dep_url.len() + || full_url.as_bytes().get(dep_url.len()) == Some(&b'/')) + }); + + let Some((matched_dep, root_path)) = best_match else { + let package_hint = full_url + .rsplit_once('/') + .map(|(prefix, _)| prefix) + .unwrap_or(&full_url); + anyhow::bail!( + "No declared dependency matches '{}'\n \ + Add a dependency that covers this path to [dependencies] in pcb.toml", + package_hint + ); + }; + + let relative_path = full_url + .strip_prefix(matched_dep.as_str()) + .and_then(|s| s.strip_prefix('/')) + .unwrap_or(""); + + let full_path = if relative_path.is_empty() { + root_path.clone() + } else { + root_path.join(relative_path) + }; + + if !self.file_provider.exists(&full_path) { + anyhow::bail!( + "File not found: {} (resolved to: {}, dep root: {})", + relative_path, + full_path.display(), + root_path.display() + ); + } + + self.insert_load_spec(full_path.clone(), spec.clone()); + Ok(full_path) + } + + /// URL resolution: translate canonical URL to cache path using resolution map. + fn resolve_url(&self, context: &mut ResolveContext) -> Result { + let package_root = self.find_package_root_for_file(&context.current_file)?; + self.try_resolve_workspace(context, &package_root) + } + + /// Relative path resolution: resolve relative to current file with boundary enforcement. + fn resolve_relative(&self, context: &mut ResolveContext) -> Result { + let LoadSpec::Path { path, .. } = context.latest_spec() else { + unreachable!("resolve_relative called on non-Path spec"); + }; + + let package_root = self.find_package_root_for_file(&context.current_file)?; + + let current_dir = context + .current_file + .parent() + .ok_or_else(|| anyhow::anyhow!("Current file has no parent directory"))?; + + let resolved_path = current_dir.join(path); + + let canonical_resolved = context.file_provider.canonicalize(&resolved_path)?; + let canonical_root = context.file_provider.canonicalize(&package_root)?; + + if !canonical_resolved.starts_with(&canonical_root) { + anyhow::bail!( + "Cannot load outside package boundary: '{}' would escape package root '{}'", + path.display(), + package_root.display() + ); + } + + crate::validate_path_case_with_canonical(path, &canonical_resolved)?; + + Ok(canonical_resolved) + } + + /// Resolve a load path. Supports aliases, URLs, and relative paths. + pub(crate) fn resolve(&self, context: &mut ResolveContext) -> Result { + // Expand aliases + if let LoadSpec::Package { package, path, .. } = context.latest_spec() { + let expanded_url = self.expand_alias(context, package)?; + let full_url = if path.as_os_str().is_empty() { + expanded_url + } else { + format!("{}/{}", expanded_url, path.display()) + }; + + let expanded_spec = LoadSpec::parse(&full_url) + .ok_or_else(|| anyhow::anyhow!("Failed to parse expanded alias: {}", full_url))?; + context.push_spec(expanded_spec)?; + } + + let resolved_path = match context.latest_spec() { + LoadSpec::Github { .. } | LoadSpec::Gitlab { .. } => self.resolve_url(context)?, + LoadSpec::Path { .. } => self.resolve_relative(context)?, + LoadSpec::Package { .. } => unreachable!("Package checked above"), + }; + + if !context.file_provider.exists(&resolved_path) + && !context.original_spec().allow_not_exist() + { + return Err(anyhow::anyhow!( + "File not found: {}", + resolved_path.display() + )); + } + + if context.file_provider.exists(&resolved_path) { + crate::validate_path_case(context.file_provider, &resolved_path)?; + } + + Ok(resolved_path) + } } impl Default for EvalSession { @@ -557,20 +836,17 @@ fn json_value_to_heap_value<'v>(json: &serde_json::Value, heap: &'v Heap) -> Val impl EvalContext { /// Create a new EvalContext with a fresh session. - pub fn new(load_resolver: Arc) -> Self { - let config = EvalContextConfig::new(load_resolver); + /// + /// Canonicalizes `package_resolutions` keys so that path lookups during + /// evaluation match the canonicalized file paths used elsewhere. + pub fn new(file_provider: Arc, resolution: ResolutionResult) -> Self { + let mut resolution = resolution; + resolution.canonicalize_keys(&*file_provider); + let config = EvalContextConfig::new(file_provider, Arc::new(resolution)); EvalSession::default().create_context(config) } - /// Create an EvalContext that shares an existing session. - /// Useful for creating a context that can access module_tree from a previous evaluation. - pub fn with_session(session: EvalSession, load_resolver: Arc) -> Self { - let config = EvalContextConfig::new(load_resolver); - session.create_context(config) - } - /// Create an EvalContext from an existing session and config. - /// This is the preferred way to create contexts for parallel evaluation. pub fn from_session_and_config(session: EvalSession, config: EvalContextConfig) -> Self { session.create_context(config) } @@ -580,6 +856,11 @@ impl EvalContext { &self.config } + /// Get the session. + pub fn session(&self) -> &EvalSession { + &self.session + } + /// Get the source path of the module we are evaluating. pub fn source_path(&self) -> Option<&PathBuf> { self.config.source_path.as_ref() @@ -590,11 +871,6 @@ impl EvalContext { &self.config.module_path } - /// Get the load resolver. - pub fn load_resolver(&self) -> &Arc { - &self.config.load_resolver - } - /// Check if strict IO/config checking is enabled. pub fn strict_io_config(&self) -> bool { self.config.strict_io_config @@ -610,14 +886,8 @@ impl EvalContext { self.config.child_for_load(child_module_path, target_path) } - pub fn file_provider(&self) -> &dyn crate::FileProvider { - self.config.load_resolver.file_provider() - } - - /// Set the file provider for this context - pub fn set_load_resolver(mut self, load_resolver: Arc) -> Self { - self.config.load_resolver = load_resolver; - self + pub fn file_provider(&self) -> &dyn FileProvider { + self.config.file_provider() } /// Enable or disable strict IO/config placeholder checking for subsequent evaluations. @@ -647,7 +917,9 @@ impl EvalContext { } let child_config = EvalContextConfig { builtin_docs: self.config.builtin_docs.clone(), - load_resolver: self.config.load_resolver.clone(), + file_provider: self.config.file_provider.clone(), + resolution: self.config.resolution.clone(), + path_to_spec: self.config.path_to_spec.clone(), module_path, load_chain: self.config.load_chain.clone(), source_path: None, @@ -880,7 +1152,7 @@ impl EvalContext { } }; - self.config.load_resolver.track_file(source_path); + self.config.track_file(source_path); // Fetch contents: prefer explicit override, otherwise read from disk. let contents_owned = match &self.config.contents { @@ -951,7 +1223,7 @@ impl EvalContext { Ok(_) => { // Extract needed references before freezing (which moves self.module) let session_ref = self.session.clone(); - let load_resolver_ref = self.config.load_resolver.clone(); + let config_ref = self.config.clone(); let frozen_module = { let _span = info_span!("freeze_module").entered(); @@ -1042,7 +1314,7 @@ impl EvalContext { sch_module: extra.module.clone(), signature, print_output, - load_resolver: load_resolver_ref.clone(), + config: config_ref.clone(), session: session_ref.clone(), }; @@ -1382,9 +1654,9 @@ impl EvalContext { self.config.source_path.as_deref() } - /// Get the load resolver if available - pub fn get_load_resolver(&self) -> &Arc { - &self.config.load_resolver + /// Get the eval config + pub fn get_config(&self) -> &EvalContextConfig { + &self.config } /// Append a diagnostic to this context's local collection. @@ -1407,8 +1679,7 @@ impl EvalContext { "Trying to load path {path} with current path {:?}", self.config.source_path ); - let load_resolver = self.config.load_resolver.clone(); - let file_provider = load_resolver.file_provider(); + let load_config = &self.config; let module_path = self.config.source_path.clone(); let Some(current_file) = module_path.as_ref() else { @@ -1419,8 +1690,8 @@ impl EvalContext { }; // Resolve the load path to an absolute path - let mut resolve_context = load_resolver.resolve_context(path, current_file)?; - let canonical_path = load_resolver.resolve(&mut resolve_context)?; + let mut resolve_context = load_config.resolve_context(path, current_file)?; + let canonical_path = load_config.resolve(&mut resolve_context)?; // Check for cyclic imports using per-context load chain (thread-safe) if self.config.load_chain.contains(&canonical_path) { @@ -1445,7 +1716,7 @@ impl EvalContext { let span = span.or_else(|| self.resolve_load_span(path)); - if file_provider.is_directory(&canonical_path) { + if load_config.file_provider.is_directory(&canonical_path) { return Err(starlark::Error::new_other(anyhow::anyhow!( "Directory load syntax is no longer supported" ))); diff --git a/crates/pcb-zen-core/src/lang/file.rs b/crates/pcb-zen-core/src/lang/file.rs index 550081ce..d9608fcd 100644 --- a/crates/pcb-zen-core/src/lang/file.rs +++ b/crates/pcb-zen-core/src/lang/file.rs @@ -35,7 +35,7 @@ pub(crate) fn file_globals(builder: &mut GlobalsBuilder) { // Resolve the path using the load resolver let resolved_path = eval_context - .get_load_resolver() + .get_config() .resolve_path(&path, current_file) .map_err(|e| anyhow::anyhow!("Failed to resolve file path '{}': {}", path, e))?; @@ -89,7 +89,7 @@ pub(crate) fn file_globals(builder: &mut GlobalsBuilder) { // Use the load resolver to resolve the LoadSpec to an absolute path let resolved_path = eval_context - .get_load_resolver() + .get_config() .resolve_spec(&load_spec, current_file) .map_err(|e| anyhow::anyhow!("Failed to resolve path '{}': {}", path, e))?; diff --git a/crates/pcb-zen-core/src/lang/spice_model.rs b/crates/pcb-zen-core/src/lang/spice_model.rs index 379d2e73..281ff375 100644 --- a/crates/pcb-zen-core/src/lang/spice_model.rs +++ b/crates/pcb-zen-core/src/lang/spice_model.rs @@ -176,7 +176,7 @@ where .ok_or_else(|| starlark::Error::new_other(anyhow!("No source path available")))?; let resolved_path = eval_ctx - .get_load_resolver() + .get_config() .resolve_path(&path, std::path::Path::new(¤t_file)) .map_err(|e| { starlark::Error::new_other(anyhow!("Failed to resolve spice model path: {}", e)) diff --git a/crates/pcb-zen-core/src/lang/symbol.rs b/crates/pcb-zen-core/src/lang/symbol.rs index 0c8d05ff..829d273d 100644 --- a/crates/pcb-zen-core/src/lang/symbol.rs +++ b/crates/pcb-zen-core/src/lang/symbol.rs @@ -232,7 +232,7 @@ impl<'v> SymbolValue { .ok_or_else(|| starlark::Error::new_other(anyhow!("No source path available")))?; let resolved_path = eval_ctx - .get_load_resolver() + .get_config() .resolve_path(&library_path, std::path::Path::new(¤t_file)) .map_err(|e| { starlark::Error::new_other(anyhow!("Failed to resolve library path: {}", e)) diff --git a/crates/pcb-zen-core/src/lib.rs b/crates/pcb-zen-core/src/lib.rs index e8a70a5d..29c85de0 100644 --- a/crates/pcb-zen-core/src/lib.rs +++ b/crates/pcb-zen-core/src/lib.rs @@ -1,6 +1,5 @@ use std::{ - any::Any, - collections::{BTreeMap, HashMap}, + collections::HashMap, path::{Path, PathBuf}, sync::{Arc, RwLock}, }; @@ -61,7 +60,7 @@ pub use diagnostics::{ DiagnosticsReport, LoadError, WithDiagnostics, }; pub use lang::error::SuppressedDiagnostics; -pub use lang::eval::{EvalContext, EvalOutput}; +pub use lang::eval::{EvalContext, EvalContextConfig, EvalOutput}; pub use load_spec::LoadSpec; pub use passes::{ AggregatePass, CommentSuppressPass, FilterHiddenPass, JsonExportPass, LspFilterPass, @@ -101,6 +100,12 @@ pub trait FileProvider: Send + Sync { /// Canonicalize a path (make it absolute) fn canonicalize(&self, path: &std::path::Path) -> Result; + + /// Global package cache directory (e.g. `~/.pcb/cache`). + /// Returns empty path by default (WASM / in-memory providers). + fn cache_dir(&self) -> std::path::PathBuf { + std::path::PathBuf::new() + } } /// Blanket implementation of FileProvider for Arc where T: FileProvider @@ -134,6 +139,10 @@ impl FileProvider for Arc { ) -> Result { (**self).canonicalize(path) } + + fn cache_dir(&self) -> std::path::PathBuf { + (**self).cache_dir() + } } #[derive(Debug, Clone, thiserror::Error)] @@ -281,6 +290,12 @@ impl FileProvider for DefaultFileProvider { result } + + fn cache_dir(&self) -> std::path::PathBuf { + dirs::home_dir() + .expect("Cannot determine home directory") + .join(".pcb/cache") + } } /// Information about a package alias including its target and source @@ -295,11 +310,10 @@ pub struct AliasInfo { /// Context struct for load resolution operations /// Contains input parameters and computed state for path resolution -pub struct ResolveContext<'a> { +pub(crate) struct ResolveContext<'a> { // Input parameters pub file_provider: &'a dyn FileProvider, pub current_file: PathBuf, - pub current_file_spec: LoadSpec, // Resolution history - specs get pushed as they're resolved further // Index 0 = original spec, later indices = progressively resolved specs @@ -311,13 +325,11 @@ impl<'a> ResolveContext<'a> { pub fn new( file_provider: &'a dyn FileProvider, current_file: PathBuf, - current_spec: LoadSpec, load_spec: LoadSpec, ) -> Self { Self { file_provider, current_file, - current_file_spec: current_spec, spec_history: vec![load_spec], } } @@ -350,71 +362,6 @@ impl<'a> ResolveContext<'a> { } } -pub trait LoadResolver: Send + Sync + Any { - /// Convenience method to resolve a load path string directly - /// This encapsulates the common pattern of parsing a path and creating a ResolveContext - fn resolve_path(&self, path: &str, current_file: &Path) -> Result { - let mut context = self.resolve_context(path, current_file)?; - self.resolve(&mut context) - } - - /// Convenience method to resolve a LoadSpec directly - fn resolve_spec( - &self, - load_spec: &LoadSpec, - current_file: &Path, - ) -> Result { - let mut context = self.resolve_context_from_spec(load_spec, current_file)?; - self.resolve(&mut context) - } - - fn resolve_context<'a>( - &'a self, - path: &str, - current_file: &Path, - ) -> Result, anyhow::Error> { - let let_spec = LoadSpec::parse(path) - .ok_or_else(|| anyhow::anyhow!("Invalid load path spec: {}", path))?; - self.resolve_context_from_spec(&let_spec, current_file) - } - - fn resolve_context_from_spec<'a>( - &'a self, - load_spec: &LoadSpec, - current_file: &Path, - ) -> Result, anyhow::Error> { - let current_file = self.file_provider().canonicalize(current_file)?; - self.track_file(¤t_file); - let current_spec = self.get_load_spec(¤t_file).ok_or_else(|| { - anyhow::anyhow!( - "Current file should have a LoadSpec: {}", - current_file.display() - ) - })?; - let context = ResolveContext::new( - self.file_provider(), - current_file, - current_spec, - load_spec.clone(), - ); - Ok(context) - } - - fn file_provider(&self) -> &dyn FileProvider; - - /// Resolve a LoadSpec to an absolute file path using the provided context - /// - /// The context contains the load specification, current file, and other state needed for resolution. - /// Returns the resolved absolute path that should be loaded. - fn resolve(&self, context: &mut ResolveContext) -> Result; - - /// Manually track a file. Useful for entrypoints. - fn track_file(&self, path: &Path); - - /// Get the LoadSpec for a specific resolved file path - fn get_load_spec(&self, path: &Path) -> Option; -} - /// File extension constants and utilities pub mod file_extensions { use std::ffi::OsStr; @@ -459,330 +406,24 @@ pub fn normalize_path(path: &Path) -> PathBuf { } std::path::Component::ParentDir => { if !normalized.pop() { - // If we can't pop (e.g., at root), keep the parent dir normalized.push(".."); } } std::path::Component::Normal(name) => { normalized.push(name); } - std::path::Component::CurDir => { - // Skip current directory - } + std::path::Component::CurDir => {} } } normalized } -/// Core load resolver that handles all path resolution logic. -/// This resolver handles workspace paths, relative paths -pub struct CoreLoadResolver { - file_provider: Arc, - /// Resolution map: Package Root -> Import URL -> Resolved Path - /// Contains workspace packages AND transitive remote deps - /// BTreeMap enables longest prefix matching for nested package paths - package_resolutions: HashMap>, - /// Maps resolved paths to their original LoadSpecs - /// This allows us to resolve relative paths from remote files correctly - path_to_spec: Arc>>, -} - -impl CoreLoadResolver { - /// Create a new CoreLoadResolver with the given file provider and remote fetcher. - pub fn new( - file_provider: Arc, - package_resolutions: HashMap>, - ) -> Self { - // Canonicalize package_resolutions keys to match canonicalized file paths during lookup. - // On Windows, canonicalize() adds \\?\ UNC prefix which must match for HashMap lookups. - let package_resolutions = package_resolutions - .into_iter() - .map(|(root, deps)| { - let canon_root = file_provider.canonicalize(&root).unwrap_or(root); - (canon_root, deps) - }) - .collect(); - - Self { - file_provider, - path_to_spec: Arc::new(RwLock::new(HashMap::new())), - package_resolutions, - } - } - - fn insert_load_spec(&self, resolved_path: PathBuf, spec: LoadSpec) { - self.path_to_spec - .write() - .unwrap() - .insert(resolved_path, spec); - } - - /// Find the package root for a given file by walking up directories - /// - /// First tries package_resolutions map (workspace packages), then walks up looking for pcb.toml (cached packages) - fn find_package_root_for_file( - &self, - file: &Path, - package_resolutions: &HashMap>, - ) -> anyhow::Result { - let mut current = file.parent(); - while let Some(dir) = current { - // Check workspace package resolutions first - if package_resolutions.contains_key(dir) { - return Ok(dir.to_path_buf()); - } - - // Check for pcb.toml (handles cached packages) - let pcb_toml = dir.join("pcb.toml"); - if self.file_provider.exists(&pcb_toml) { - return Ok(dir.to_path_buf()); - } - - current = dir.parent(); - } - anyhow::bail!( - "Internal error: current file not in any package: {}", - file.display() - ) - } - - fn resolved_map_for_package_root( - &self, - package_root: &Path, - ) -> anyhow::Result<&BTreeMap> { - self.package_resolutions.get(package_root).ok_or_else(|| { - anyhow::anyhow!( - "Dependency map not loaded for package '{}'", - package_root.display() - ) - }) - } - - /// Expand alias using the resolution map - /// - /// Aliases are auto-generated from the last path segment of dependency URLs. - /// For example, "github.com/diodeinc/stdlib" generates the alias "stdlib". - fn expand_alias(&self, context: &ResolveContext, alias: &str) -> Result { - // Find the package root for the current file - let package_root = - self.find_package_root_for_file(&context.current_file, &self.package_resolutions)?; - - // Get the resolution map for this package - let resolved_map = self.resolved_map_for_package_root(&package_root)?; - - // Derive alias from resolution map keys by matching last path segment - // Also include KiCad asset aliases - for url in resolved_map.keys() { - if let Some(last_segment) = url.rsplit('/').next() { - if last_segment == alias { - return Ok(url.clone()); - } - } - } - - // Check KiCad asset aliases - for (kicad_alias, base_url, _) in config::KICAD_ASSETS { - if *kicad_alias == alias { - return Ok(base_url.to_string()); - } - } - - anyhow::bail!("Unknown alias '@{}'", alias) - } - - /// remote resolution: longest prefix match against package's declared deps - fn try_resolve_workspace( - &self, - context: &ResolveContext, - package_root: &Path, - ) -> Result { - let spec = context.latest_spec(); - - // Build full URL from spec - let (base, path) = match spec { - LoadSpec::Github { - user, repo, path, .. - } => (format!("github.com/{}/{}", user, repo), path), - LoadSpec::Gitlab { - project_path, path, .. - } => (format!("gitlab.com/{}", project_path), path), - LoadSpec::Package { package, path, .. } => (package.clone(), path), - _ => unreachable!(), - }; - - let mut full_url = base; - for component in path.components() { - if let std::path::Component::Normal(part) = component { - full_url.push('/'); - full_url.push_str(&part.to_string_lossy()); - } - } - - let resolved_map = self.resolved_map_for_package_root(package_root)?; - - // Longest prefix match - let best_match = resolved_map.iter().rev().find(|(dep_url, _)| { - full_url.starts_with(dep_url.as_str()) - && (full_url.len() == dep_url.len() - || full_url.as_bytes().get(dep_url.len()) == Some(&b'/')) - }); - - let Some((matched_dep, root_path)) = best_match else { - // Strip the filename from full_url to show the package path - let package_hint = full_url - .rsplit_once('/') - .map(|(prefix, _)| prefix) - .unwrap_or(&full_url); - anyhow::bail!( - "No declared dependency matches '{}'\n \ - Add a dependency that covers this path to [dependencies] in pcb.toml", - package_hint - ); - }; - - let relative_path = full_url - .strip_prefix(matched_dep.as_str()) - .and_then(|s| s.strip_prefix('/')) - .unwrap_or(""); - - let full_path = if relative_path.is_empty() { - root_path.clone() - } else { - root_path.join(relative_path) - }; - - if !self.file_provider.exists(&full_path) { - anyhow::bail!( - "File not found: {} (resolved to: {}, dep root: {})", - relative_path, - full_path.display(), - root_path.display() - ); - } - - self.insert_load_spec(full_path.clone(), spec.clone()); - Ok(full_path) - } - - /// URL resolution: translate canonical URL to cache path using resolution map - fn resolve_url(&self, context: &mut ResolveContext) -> Result { - // Find which package the current file belongs to - let package_root = - self.find_package_root_for_file(&context.current_file, &self.package_resolutions)?; - - // Use existing try_resolve_workspace which does longest-prefix matching - self.try_resolve_workspace(context, &package_root) - } - - /// relative path resolution: resolve relative to current file with boundary enforcement - fn resolve_relative(&self, context: &mut ResolveContext) -> Result { - let LoadSpec::Path { path, .. } = context.latest_spec() else { - unreachable!("resolve_relative called on non-Path spec"); - }; - - // Find package root for boundary enforcement - let package_root = - self.find_package_root_for_file(&context.current_file, &self.package_resolutions)?; - - // Resolve relative to current file's directory - let current_dir = context - .current_file - .parent() - .ok_or_else(|| anyhow::anyhow!("Current file has no parent directory"))?; - - let resolved_path = current_dir.join(path); - - // Canonicalize both paths for boundary check - let canonical_resolved = context.file_provider.canonicalize(&resolved_path)?; - let canonical_root = context.file_provider.canonicalize(&package_root)?; - - // Enforce package boundary: resolved path must stay within package root - if !canonical_resolved.starts_with(&canonical_root) { - anyhow::bail!( - "Cannot load outside package boundary: '{}' would escape package root '{}'", - path.display(), - package_root.display() - ); - } - - // Case sensitivity check: compare original filename to canonical filename - validate_path_case_with_canonical(path, &canonical_resolved)?; - - Ok(canonical_resolved) - } -} - -impl LoadResolver for CoreLoadResolver { - fn file_provider(&self) -> &dyn FileProvider { - &*self.file_provider - } - - /// resolution: Toolchain + package-level aliases, URLs, and relative paths - /// - /// supports three load patterns: - /// 1. Aliases: load("@stdlib/units.zen") - expanded via toolchain or package aliases - /// 2. Canonical URLs: load("github.com/user/repo/path.zen") - looked up in resolution map - /// 3. Relative paths: load("./utils.zen") - resolved relative to current file with boundary checks - fn resolve(&self, context: &mut ResolveContext) -> Result { - // Expand aliases: package-level first, then toolchain-level - if let LoadSpec::Package { package, path, .. } = context.latest_spec() { - let expanded_url = self.expand_alias(context, package)?; - let full_url = if path.as_os_str().is_empty() { - expanded_url - } else { - format!("{}/{}", expanded_url, path.display()) - }; - - // Reparse the expanded URL as a proper LoadSpec - let expanded_spec = LoadSpec::parse(&full_url) - .ok_or_else(|| anyhow::anyhow!("Failed to parse expanded alias: {}", full_url))?; - context.push_spec(expanded_spec)?; - } - - let resolved_path = match context.latest_spec() { - // URL loads: github.com/... or gitlab.com/... - LoadSpec::Github { .. } | LoadSpec::Gitlab { .. } => self.resolve_url(context)?, - // Relative path loads: ./utils.zen or ../sibling.zen - LoadSpec::Path { .. } => self.resolve_relative(context)?, - LoadSpec::Package { .. } => unreachable!("Package checked above"), - }; - - // Validate existence - if !context.file_provider.exists(&resolved_path) - && !context.original_spec().allow_not_exist() - { - return Err(anyhow::anyhow!( - "File not found: {}", - resolved_path.display() - )); - } - - // Case sensitivity validation - if context.file_provider.exists(&resolved_path) { - validate_path_case(context.file_provider, &resolved_path)?; - } - - Ok(resolved_path) - } - - fn track_file(&self, path: &Path) { - let canonical_path = self.file_provider.canonicalize(path).unwrap(); - if self.get_load_spec(&canonical_path).is_some() { - // If already tracked, do nothing - return; - } - let load_spec = LoadSpec::local_path(&canonical_path); - self.insert_load_spec(canonical_path, load_spec); - } - - fn get_load_spec(&self, path: &Path) -> Option { - self.path_to_spec.read().unwrap().get(path).cloned() - } -} - /// Validate filename case matches exactly on disk. /// Prevents macOS/Windows working but Linux CI failing. -fn validate_path_case(file_provider: &dyn FileProvider, path: &Path) -> anyhow::Result<()> { +pub(crate) fn validate_path_case( + file_provider: &dyn FileProvider, + path: &Path, +) -> anyhow::Result<()> { // Use canonicalize to get the actual case on disk. // On macOS/Windows, canonicalize returns the true filesystem case. let canonical = file_provider.canonicalize(path)?; @@ -790,7 +431,10 @@ fn validate_path_case(file_provider: &dyn FileProvider, path: &Path) -> anyhow:: } /// Validate filename case when we already have the canonical path. -fn validate_path_case_with_canonical(original: &Path, canonical: &Path) -> anyhow::Result<()> { +pub(crate) fn validate_path_case_with_canonical( + original: &Path, + canonical: &Path, +) -> anyhow::Result<()> { let Some(expected_filename) = original.file_name() else { return Ok(()); }; diff --git a/crates/pcb-zen-core/src/moved.rs b/crates/pcb-zen-core/src/moved.rs index 6b8b60d1..accba3bc 100644 --- a/crates/pcb-zen-core/src/moved.rs +++ b/crates/pcb-zen-core/src/moved.rs @@ -105,11 +105,9 @@ mod tests { use super::*; fn eval_context(test_path: PathBuf) -> EvalContext { - let load_resolver = Arc::new(crate::CoreLoadResolver::new( - Arc::new(crate::DefaultFileProvider::new()), - HashMap::default(), - )); - EvalContext::new(load_resolver).set_source_path(test_path) + let file_provider = Arc::new(crate::DefaultFileProvider::new()); + EvalContext::new(file_provider, crate::resolution::ResolutionResult::empty()) + .set_source_path(test_path) } #[test] diff --git a/crates/pcb-zen-core/src/resolution.rs b/crates/pcb-zen-core/src/resolution.rs index 4e68e363..c45559fd 100644 --- a/crates/pcb-zen-core/src/resolution.rs +++ b/crates/pcb-zen-core/src/resolution.rs @@ -8,7 +8,7 @@ //! - Native: checks patches, vendor/, then ~/.pcb/cache //! - WASM: only checks vendor/ (everything must be pre-vendored) -use std::collections::{BTreeMap, HashMap}; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::path::{Path, PathBuf}; use semver::Version; @@ -360,6 +360,124 @@ impl PackagePathResolver for NativePathResolver { } } +/// Result of dependency resolution. +/// +/// This is a data-only type defined in core so it can be referenced by +/// `EvalContext` / `EvalOutput`. Construction happens in `pcb-zen` which +/// performs the actual resolution. +#[derive(Debug, Clone)] +pub struct ResolutionResult { + /// Snapshot of workspace info at the time of resolution + pub workspace_info: WorkspaceInfo, + /// Map from Package Root (Absolute Path) -> Import URL -> Resolved Absolute Path + pub package_resolutions: HashMap>, + /// Package dependencies in the build closure: ModuleLine -> Version + pub closure: HashMap, + /// Asset dependencies: (module_path, ref) -> resolved_path + pub assets: HashMap<(String, String), PathBuf>, + /// Whether the lockfile (pcb.sum) was updated during resolution + pub lockfile_changed: bool, +} + +impl ResolutionResult { + /// Create an empty resolution result with no dependencies. + pub fn empty() -> Self { + Self { + workspace_info: WorkspaceInfo { + root: PathBuf::new(), + cache_dir: PathBuf::new(), + config: None, + packages: BTreeMap::new(), + lockfile: None, + errors: vec![], + }, + package_resolutions: HashMap::new(), + closure: HashMap::new(), + assets: HashMap::new(), + lockfile_changed: false, + } + } + + /// Canonicalize `package_resolutions` keys using the given file provider. + pub fn canonicalize_keys(&mut self, file_provider: &dyn crate::FileProvider) { + self.package_resolutions = self + .package_resolutions + .iter() + .map(|(root, deps)| { + let canon = file_provider + .canonicalize(root) + .unwrap_or_else(|_| root.clone()); + (canon, deps.clone()) + }) + .collect(); + } + + /// Compute the transitive dependency closure for a package. + pub fn package_closure(&self, package_url: &str) -> PackageClosure { + let workspace_info = &self.workspace_info; + let mut closure = PackageClosure::default(); + let mut visited: HashSet = HashSet::new(); + let mut stack: Vec = vec![package_url.to_string()]; + + let cache = &workspace_info.cache_dir; + let vendor_base = workspace_info.root.join("vendor"); + + let get_pkg_root = |module_path: &str, version: &str| -> PathBuf { + let vendor_path = vendor_base.join(module_path).join(version); + if vendor_path.exists() { + vendor_path + } else { + cache.join(module_path).join(version) + } + }; + + while let Some(url) = stack.pop() { + if !visited.insert(url.clone()) { + continue; + } + + if let Some(pkg) = workspace_info.packages.get(&url) { + closure.local_packages.insert(url.clone()); + for dep_url in pkg.config.dependencies.keys() { + stack.push(dep_url.clone()); + } + for (asset_url, asset_spec) in &pkg.config.assets { + if let Ok(ref_str) = crate::extract_asset_ref_strict(asset_spec) { + closure.assets.insert((asset_url.clone(), ref_str)); + } + } + } else if let Some((line, version)) = self.closure.iter().find(|(l, _)| l.path == url) { + let version_str = version.to_string(); + closure + .remote_packages + .insert((url.clone(), version_str.clone())); + let pkg_root = get_pkg_root(&line.path, &version_str); + if let Some(deps) = self.package_resolutions.get(&pkg_root) { + for dep_url in deps.keys() { + stack.push(dep_url.clone()); + } + } + } + } + + for (asset_path, asset_ref) in self.assets.keys() { + closure + .assets + .insert((asset_path.clone(), asset_ref.clone())); + } + + closure + } +} + +/// Transitive dependency closure for a package +#[derive(Debug, Clone, Default)] +pub struct PackageClosure { + pub local_packages: HashSet, + pub remote_packages: HashSet<(String, String)>, + pub assets: HashSet<(String, String)>, +} + #[cfg(test)] mod tests { use super::*; @@ -432,6 +550,7 @@ mod tests { let workspace = WorkspaceInfo { root: workspace_root.clone(), + cache_dir: PathBuf::new(), config: None, packages, lockfile: None, diff --git a/crates/pcb-zen-core/src/workspace.rs b/crates/pcb-zen-core/src/workspace.rs index 492e7ec4..b0cf6cf1 100644 --- a/crates/pcb-zen-core/src/workspace.rs +++ b/crates/pcb-zen-core/src/workspace.rs @@ -78,6 +78,10 @@ pub struct DiscoveryError { pub struct WorkspaceInfo { /// Workspace root directory pub root: PathBuf, + /// Global package cache directory (e.g. `~/.pcb/cache`). + /// Set by native workspace discovery; empty on WASM. + #[serde(skip)] + pub cache_dir: PathBuf, /// Root pcb.toml config (if present) #[serde(skip_serializing_if = "Option::is_none")] pub config: Option, @@ -488,6 +492,7 @@ pub fn get_workspace_info( Ok(WorkspaceInfo { root: workspace_root, + cache_dir: file_provider.cache_dir(), config, packages, lockfile, diff --git a/crates/pcb-zen-core/tests/common/mod.rs b/crates/pcb-zen-core/tests/common/mod.rs index ae57f3ff..6604565e 100644 --- a/crates/pcb-zen-core/tests/common/mod.rs +++ b/crates/pcb-zen-core/tests/common/mod.rs @@ -205,7 +205,7 @@ macro_rules! snapshot_eval { fn $name() { use std::sync::Arc; use std::collections::{HashMap, BTreeMap}; - use pcb_zen_core::{EvalContext, CoreLoadResolver, SortPass, DiagnosticsPass}; + use pcb_zen_core::{EvalContext, SortPass, DiagnosticsPass}; use $crate::common::InMemoryFileProvider; let mut files = HashMap::new(); @@ -215,19 +215,13 @@ macro_rules! snapshot_eval { files.insert(file.clone(), content.clone()); } - // The last file in the list is the main file let main_file = file_list.last().unwrap().0.clone(); - let mut package_resolution = HashMap::default(); - package_resolution.insert(".".into(), BTreeMap::default()); + let file_provider: Arc = Arc::new(InMemoryFileProvider::new(files)); + let mut resolution = pcb_zen_core::resolution::ResolutionResult::empty(); + resolution.package_resolutions.insert(".".into(), BTreeMap::default()); - let load_resolver = Arc::new(CoreLoadResolver::new( - Arc::new(InMemoryFileProvider::new(files)), - package_resolution, - )); - - - let ctx = EvalContext::new(load_resolver) + let ctx = EvalContext::new(file_provider, resolution) .set_source_path(std::path::PathBuf::from(&main_file)); let result = ctx.eval(); diff --git a/crates/pcb-zen-core/tests/not_connected.rs b/crates/pcb-zen-core/tests/not_connected.rs index 79246b09..768dbd9d 100644 --- a/crates/pcb-zen-core/tests/not_connected.rs +++ b/crates/pcb-zen-core/tests/not_connected.rs @@ -1,6 +1,6 @@ use std::sync::Arc; -use pcb_zen_core::{CoreLoadResolver, DiagnosticsPass, EvalContext, SortPass}; +use pcb_zen_core::{DiagnosticsPass, EvalContext, SortPass}; mod common; use common::InMemoryFileProvider; @@ -9,12 +9,12 @@ fn eval_to_schematic( files: std::collections::HashMap, main: &str, ) -> pcb_zen_core::WithDiagnostics { - let load_resolver = Arc::new(CoreLoadResolver::new( - Arc::new(InMemoryFileProvider::new(files)), - Default::default(), - )); + let file_provider: Arc = + Arc::new(InMemoryFileProvider::new(files)); + let resolution = pcb_zen_core::resolution::ResolutionResult::empty(); - let ctx = EvalContext::new(load_resolver).set_source_path(std::path::PathBuf::from(main)); + let ctx = + EvalContext::new(file_provider, resolution).set_source_path(std::path::PathBuf::from(main)); let eval = ctx.eval(); assert!(eval.is_success(), "eval failed: {:?}", eval.diagnostics); let eval_output = eval.output.expect("expected EvalOutput on success"); diff --git a/crates/pcb-zen-core/tests/refdes.rs b/crates/pcb-zen-core/tests/refdes.rs index fff70aec..fdcd3b71 100644 --- a/crates/pcb-zen-core/tests/refdes.rs +++ b/crates/pcb-zen-core/tests/refdes.rs @@ -3,17 +3,13 @@ mod common; use common::InMemoryFileProvider; use pcb_sch::InstanceKind; -use pcb_zen_core::{CoreLoadResolver, EvalContext}; +use pcb_zen_core::EvalContext; use std::collections::HashMap; use std::path::PathBuf; use std::sync::Arc; #[test] fn refdes_assignment_uses_natural_hier_name_sort() { - // Regression test for natural sorting: `Resistor_2` should allocate before `Resistor_10`. - // - // This exercises the full Starlark eval -> schematic conversion pipeline, not just the - // `pcb-sch` allocation helper. let mut decls = String::new(); decls.push_str("vcc = Net(\"VCC\")\n\n"); for i in 1..=20 { @@ -25,47 +21,55 @@ fn refdes_assignment_uses_natural_hier_name_sort() { let mut files = HashMap::new(); files.insert("main.zen".to_string(), decls); - let load_resolver = Arc::new(CoreLoadResolver::new( - Arc::new(InMemoryFileProvider::new(files)), - Default::default(), - )); + let file_provider: Arc = + Arc::new(InMemoryFileProvider::new(files)); + let resolution = pcb_zen_core::resolution::ResolutionResult::empty(); - let ctx = EvalContext::new(load_resolver).set_source_path(PathBuf::from("/main.zen")); + let ctx = + EvalContext::new(file_provider, resolution).set_source_path(PathBuf::from("/main.zen")); let result = ctx.eval(); + assert!(result.is_success(), "eval failed: {:?}", result.diagnostics); + + let eval_output = result.output.unwrap(); + let sch_result = eval_output.to_schematic_with_diagnostics(); assert!( - result.is_success(), - "eval failed:\n{}", - result - .diagnostics - .iter() - .map(|d| d.to_string()) - .collect::>() - .join("\n") + sch_result.output.is_some(), + "schematic conversion failed: {:?}", + sch_result.diagnostics ); - let sch = result.output.unwrap().to_schematic().unwrap(); - - let mut name_to_refdes: HashMap = HashMap::new(); - for (inst_ref, inst) in &sch.instances { - if inst.kind != InstanceKind::Component { - continue; - } - let Some(refdes) = inst.reference_designator.as_ref() else { - continue; - }; - let Some(name) = inst_ref.instance_path.last() else { - continue; - }; - name_to_refdes.insert(name.clone(), refdes.clone()); - } + let sch = sch_result.output.unwrap(); + let refdes_map: HashMap = sch + .instances + .iter() + .filter(|(_, inst)| matches!(inst.kind, InstanceKind::Component { .. })) + .filter_map(|(iref, inst)| { + let hier_name = iref + .instance_path + .iter() + .map(|s| s.as_str()) + .collect::>() + .join("."); + inst.reference_designator + .as_ref() + .map(|rd| (hier_name, rd.clone())) + }) + .collect(); + // Natural sort order: 1,2,...,9,10,...,20 => R1–R20 for i in 1..=20 { - let name = format!("Resistor_{i}"); - let expected = format!("R{i}"); + let hier = format!("root.Resistor_{i}"); + let expected_refdes = format!("R{i}"); + let actual = refdes_map.get(&hier).unwrap_or_else(|| { + // Try without "root." prefix + let short = format!("Resistor_{i}"); + refdes_map.get(&short).unwrap_or_else(|| { + panic!("missing refdes for {hier} or {short}, map: {refdes_map:?}") + }) + }); assert_eq!( - name_to_refdes.get(&name), - Some(&expected), - "unexpected refdes for {name}" + actual, &expected_refdes, + "expected {hier} -> {expected_refdes}, got {actual}" ); } } diff --git a/crates/pcb-zen-wasm/src/lib.rs b/crates/pcb-zen-wasm/src/lib.rs index 158c2787..6f3736d0 100644 --- a/crates/pcb-zen-wasm/src/lib.rs +++ b/crates/pcb-zen-wasm/src/lib.rs @@ -3,7 +3,7 @@ use pcb_zen_core::resolution::{build_resolution_map, VendoredPathResolver}; use pcb_zen_core::workspace::get_workspace_info; use pcb_zen_core::{EvalContext, FileProvider, FileProviderError, Lockfile}; use serde::{Deserialize, Serialize}; -use std::collections::{BTreeMap, HashMap, HashSet}; +use std::collections::{HashMap, HashSet}; use std::io::{Cursor, Read}; use std::path::{Component, Path, PathBuf}; use std::sync::{Arc, Mutex}; @@ -191,10 +191,9 @@ impl FileProvider for ZipFileProvider { fn resolve_packages( file_provider: F, workspace_root: &Path, -) -> Result>, String> { +) -> Result { let lockfile_path = workspace_root.join("pcb.sum"); - // Parse lockfile let lockfile_content = file_provider .read_file(&lockfile_path) .map_err(|e| format!("Failed to read {}: {}", lockfile_path.display(), e))?; @@ -202,19 +201,22 @@ fn resolve_packages( .map_err(|e| format!("Failed to parse {}: {}", lockfile_path.display(), e))?; let vendor_dir = workspace_root.join("vendor"); - - // Create the vendored path resolver let resolver = VendoredPathResolver::from_lockfile(file_provider.clone(), vendor_dir, &lockfile); - // Discover workspace using shared logic let workspace = get_workspace_info(&file_provider, workspace_root) .map_err(|e| format!("Failed to discover workspace metadata: {e}"))?; - // Build resolution map for workspace members and all vendored packages - let results = build_resolution_map(&file_provider, &resolver, &workspace, resolver.closure()); + let package_resolutions = + build_resolution_map(&file_provider, &resolver, &workspace, resolver.closure()); - Ok(results) + Ok(pcb_zen_core::resolution::ResolutionResult { + workspace_info: workspace, + package_resolutions, + closure: HashMap::new(), + assets: HashMap::new(), + lockfile_changed: false, + }) } fn diagnostic_to_json(diag: &pcb_zen_core::Diagnostic) -> DiagnosticInfo { @@ -263,18 +265,13 @@ pub fn evaluate_impl( let main_path = PathBuf::from(&main_file); let workspace_root = find_workspace_root(file_provider.as_ref(), &main_path) .map_err(|e| format!("Failed to find workspace root: {e}"))?; - let resolutions = resolve_packages(file_provider.clone(), &workspace_root) + let resolution = resolve_packages(file_provider.clone(), &workspace_root) .map_err(|e| format!("Failed to resolve dependencies: {e}"))?; - let load_resolver = Arc::new(pcb_zen_core::CoreLoadResolver::new( - file_provider.clone(), - resolutions, - )); - let inputs: HashMap = serde_json::from_str(inputs_json).map_err(|e| format!("Failed to parse inputs: {e}"))?; - let mut ctx = EvalContext::new(load_resolver).set_source_path(main_path); + let mut ctx = EvalContext::new(file_provider.clone(), resolution).set_source_path(main_path); if !inputs.is_empty() { ctx.set_json_inputs(starlark::collections::SmallMap::from_iter(inputs)); } diff --git a/crates/pcb-zen/src/cache_index.rs b/crates/pcb-zen/src/cache_index.rs index 7bff33c5..3ad14d9f 100644 --- a/crates/pcb-zen/src/cache_index.rs +++ b/crates/pcb-zen/src/cache_index.rs @@ -2,6 +2,7 @@ use anyhow::{Context, Result}; use pcb_zen_core::config::Lockfile; +use pcb_zen_core::FileProvider; use r2d2::{Pool, PooledConnection}; use r2d2_sqlite::SqliteConnectionManager; use rusqlite::{params, OptionalExtension}; @@ -318,9 +319,7 @@ fn index_path() -> PathBuf { } pub fn cache_base() -> PathBuf { - dirs::home_dir() - .expect("Cannot determine home directory") - .join(".pcb/cache") + pcb_zen_core::DefaultFileProvider::new().cache_dir() } /// Ensure the workspace cache symlink exists. diff --git a/crates/pcb-zen/src/lib.rs b/crates/pcb-zen/src/lib.rs index 6cbe8b9c..22ddc64a 100644 --- a/crates/pcb-zen/src/lib.rs +++ b/crates/pcb-zen/src/lib.rs @@ -1,5 +1,3 @@ -//! Diode Star – evaluate .zen designs and return schematic data structures. - pub mod archive; pub mod ast_utils; mod auto_deps; @@ -20,17 +18,17 @@ use std::sync::Arc; use pcb_sch::Schematic; use pcb_zen_core::config::find_workspace_root; -use pcb_zen_core::FileProvider; -use pcb_zen_core::{CoreLoadResolver, DefaultFileProvider, EvalContext, EvalOutput, LoadResolver}; +use pcb_zen_core::resolution::ResolutionResult; +use pcb_zen_core::{DefaultFileProvider, EvalContext, EvalOutput, FileProvider}; pub use pcb_zen_core::file_extensions; pub use pcb_zen_core::{Diagnostic, Diagnostics, WithDiagnostics}; pub use resolve::{ - copy_dir_all, ensure_sparse_checkout, resolve_dependencies, vendor_deps, ResolutionResult, + copy_dir_all, ensure_sparse_checkout, print_dep_tree, resolve_dependencies, vendor_deps, VendorResult, }; pub use starlark::errors::EvalSeverity; -pub use workspace::{get_workspace_info, MemberPackage, PackageClosure, WorkspaceInfo}; +pub use workspace::{get_workspace_info, MemberPackage, WorkspaceInfo}; pub use tags::get_all_versions_for_repo; @@ -44,20 +42,15 @@ pub fn eval(file: &Path, resolution_result: ResolutionResult) -> WithDiagnostics let workspace_root = find_workspace_root(&*file_provider, &abs_path).expect("failed to find workspace root"); - let load_resolver = Arc::new(CoreLoadResolver::new( - file_provider.clone(), - resolution_result.package_resolutions, - )); + let ctx = EvalContext::new(file_provider.clone(), resolution_result); // Track workspace-level pcb.toml if present for dependency awareness let pcb_toml_path = workspace_root.join("pcb.toml"); if file_provider.exists(&pcb_toml_path) { - load_resolver.track_file(&pcb_toml_path); + ctx.config().track_file(&pcb_toml_path); } - EvalContext::new(load_resolver) - .set_source_path(abs_path) - .eval() + ctx.set_source_path(abs_path).eval() } /// Evaluate `file` and return a [`Schematic`]. diff --git a/crates/pcb-zen/src/lsp/mod.rs b/crates/pcb-zen/src/lsp/mod.rs index 62d65c7c..6d5e5234 100644 --- a/crates/pcb-zen/src/lsp/mod.rs +++ b/crates/pcb-zen/src/lsp/mod.rs @@ -17,8 +17,7 @@ use pcb_zen_core::file_extensions::is_kicad_symbol_file; use pcb_zen_core::lang::symbol::invalidate_symbol_library; use pcb_zen_core::lang::type_info::ParameterInfo; use pcb_zen_core::{ - CoreLoadResolver, DefaultFileProvider, EvalContext, FileProvider, FileProviderError, - LoadResolver, + DefaultFileProvider, EvalContext, EvalContextConfig, FileProvider, FileProviderError, }; use serde::{Deserialize, Serialize}; use serde_json::Value as JsonValue; @@ -36,7 +35,7 @@ pub struct LspEvalContext { inner: EvalContext, builtin_docs: HashMap, file_provider: Arc, - load_resolver_cache: RwLock>>, + resolution_cache: RwLock>>, workspace_root_cache: RwLock>, open_files: Arc>>, netlist_subscriptions: Arc>>>, @@ -104,24 +103,14 @@ impl FileProvider for OverlayFileProvider { fn canonicalize(&self, path: &Path) -> Result { self.base.canonicalize(path) } + + fn cache_dir(&self) -> std::path::PathBuf { + self.base.cache_dir() + } } /// Create a load resolver rooted at `workspace_root` with optional dependency resolution. -fn create_load_resolver( - file_provider: Arc, - workspace_root: PathBuf, -) -> Arc { - // Resolve dependencies if this is a workspace - // LSP uses locked=false since interactive development should allow auto-deps - let package_resolutions = crate::get_workspace_info(&file_provider, &workspace_root) - .and_then(|mut ws| crate::resolve_dependencies(&mut ws, false, false)) - .map(|res| res.package_resolutions); - - Arc::new(CoreLoadResolver::new( - file_provider, - package_resolutions.unwrap_or_default(), - )) -} +use pcb_zen_core::resolution::ResolutionResult; impl Default for LspEvalContext { fn default() -> Self { @@ -155,14 +144,16 @@ impl Default for LspEvalContext { base: base_provider, open_files: open_files.clone(), }); - let load_resolver = create_load_resolver(file_provider.clone(), std::env::temp_dir()); - let inner = EvalContext::new(load_resolver); + let resolution = crate::get_workspace_info(&file_provider, &std::env::temp_dir()) + .and_then(|mut ws| crate::resolve_dependencies(&mut ws, false, false)) + .unwrap_or_else(|_| ResolutionResult::empty()); + let inner = EvalContext::new(file_provider.clone(), resolution); Self { inner, builtin_docs, file_provider, - load_resolver_cache: RwLock::new(HashMap::new()), + resolution_cache: RwLock::new(HashMap::new()), workspace_root_cache: RwLock::new(HashMap::new()), open_files, netlist_subscriptions: Arc::new(RwLock::new(HashMap::new())), @@ -221,22 +212,22 @@ impl LspEvalContext { ) } - fn maybe_invalidate_load_resolver_cache(&self, path: &Path) -> bool { + fn maybe_invalidate_resolution_cache(&self, path: &Path) -> bool { if Self::is_dependency_manifest(path) { - self.load_resolver_cache.write().unwrap().clear(); + self.resolution_cache.write().unwrap().clear(); self.workspace_root_cache.write().unwrap().clear(); return true; } false } - fn maybe_invalidate_load_resolver_on_saved_source(&self, path: &Path) { + fn maybe_invalidate_on_saved_source(&self, path: &Path) { let is_source = matches!( path.extension().and_then(|ext| ext.to_str()), Some("zen" | "star") ); if is_source { - self.load_resolver_cache.write().unwrap().clear(); + self.resolution_cache.write().unwrap().clear(); } } @@ -282,8 +273,9 @@ impl LspEvalContext { let uri = LspUrl::File(path_buf.to_path_buf()); let maybe_contents = self.get_load_contents(&uri).ok().flatten(); - let load_resolver = self.load_resolver_for(path_buf); - let mut ctx = EvalContext::new(load_resolver).set_source_path(path_buf.to_path_buf()); + let config = self.config_for(path_buf); + let mut ctx = EvalContext::from_session_and_config(Default::default(), config) + .set_source_path(path_buf.to_path_buf()); if let Some(contents) = maybe_contents { ctx = ctx.set_source_contents(contents); @@ -351,23 +343,29 @@ impl LspEvalContext { workspace_root } - fn load_resolver_for(&self, file_path: &Path) -> Arc { + /// Return the cached, canonicalized resolution for the workspace that owns `file_path`. + fn resolution_for(&self, file_path: &Path) -> Arc { let workspace_root = self.workspace_root_for(file_path); - if let Some(resolver) = self - .load_resolver_cache - .read() - .unwrap() - .get(&workspace_root) - { - return resolver.clone(); + if let Some(cached) = self.resolution_cache.read().unwrap().get(&workspace_root) { + return cached.clone(); } - let resolver = create_load_resolver(self.file_provider.clone(), workspace_root.clone()); - self.load_resolver_cache + let mut resolution = crate::get_workspace_info(&self.file_provider, &workspace_root) + .and_then(|mut ws| crate::resolve_dependencies(&mut ws, false, false)) + .unwrap_or_else(|_| ResolutionResult::empty()); + resolution.canonicalize_keys(&*self.file_provider); + let resolution = Arc::new(resolution); + self.resolution_cache .write() .unwrap() - .insert(workspace_root, resolver.clone()); - resolver + .insert(workspace_root, resolution.clone()); + resolution + } + + /// Create a fresh EvalContextConfig for the given file. + fn config_for(&self, file_path: &Path) -> EvalContextConfig { + EvalContextConfig::new(self.file_provider.clone(), self.resolution_for(file_path)) + .set_eager(self.inner.is_eager()) } /// Create LSP-specific diagnostic passes @@ -514,7 +512,7 @@ impl LspContext for LspEvalContext { self.inner .set_file_contents(path.to_path_buf(), contents.to_string()); self.maybe_invalidate_symbol_library(path); - self.maybe_invalidate_load_resolver_cache(path); + self.maybe_invalidate_resolution_cache(path); } } @@ -526,13 +524,13 @@ impl LspContext for LspEvalContext { self.inner.clear_file_contents(&canon); } self.maybe_invalidate_symbol_library(path); - self.maybe_invalidate_load_resolver_cache(path); + self.maybe_invalidate_resolution_cache(path); } } fn did_save_file(&self, uri: &LspUrl) { if let LspUrl::File(path) = uri { - self.maybe_invalidate_load_resolver_on_saved_source(path); + self.maybe_invalidate_on_saved_source(path); } } @@ -546,7 +544,7 @@ impl LspContext for LspEvalContext { should_revalidate = true; } - if self.maybe_invalidate_load_resolver_cache(path) { + if self.maybe_invalidate_resolution_cache(path) { should_revalidate = true; } @@ -587,14 +585,12 @@ impl LspContext for LspEvalContext { match uri { LspUrl::File(path) => { let workspace_root = self.workspace_root_for(path); - let load_resolver = self.load_resolver_for(path); + let config = self.config_for(path); - // Parse and analyze the file with the load resolver set - let mut result = self - .inner - .child_context(None) - .set_load_resolver(load_resolver) - .parse_and_analyze_file(path.clone(), content); + // Parse and analyze the file with the right resolution + let ctx = + EvalContext::from_session_and_config(self.inner.session().clone(), config); + let mut result = ctx.parse_and_analyze_file(path.clone(), content); // Apply LSP-specific diagnostic passes let passes = self.create_lsp_diagnostic_passes(&workspace_root); @@ -631,8 +627,8 @@ impl LspContext for LspEvalContext { // Use the load resolver from the inner context match current_file { LspUrl::File(current_path) => { - let load_resolver = self.load_resolver_for(current_path); - let resolved = load_resolver.resolve_path(path, current_path)?; + let config = self.config_for(current_path); + let resolved = config.resolve_path(path, current_path)?; Ok(LspUrl::File(resolved)) } _ => Err(anyhow::anyhow!("Cannot resolve load from non-file URL")), @@ -673,11 +669,8 @@ impl LspContext for LspEvalContext { match current_file { LspUrl::File(current_path) => { // Try to resolve as a file path - let load_resolver = self.load_resolver_for(current_path); - if let Ok(resolved) = load_resolver - .resolve_context(literal, current_path) - .and_then(|mut c| load_resolver.resolve(&mut c)) - { + let config = self.config_for(current_path); + if let Ok(resolved) = config.resolve_path(literal, current_path) { if resolved.exists() { return Ok(Some(StringLiteralResult { url: LspUrl::File(resolved), @@ -798,11 +791,8 @@ impl LspContext for LspEvalContext { // Check if the load path is a directory match current_file { LspUrl::File(current_path) => { - let load_resolver = self.load_resolver_for(current_path); - if let Ok(resolved) = load_resolver - .resolve_context(load_path, current_path) - .and_then(|mut c| load_resolver.resolve(&mut c)) - { + let config = self.config_for(current_path); + if let Ok(resolved) = config.resolve_path(load_path, current_path) { if resolved.is_dir() { return Ok(Some(Hover { contents: HoverContents::Markup(MarkupContent { @@ -916,8 +906,9 @@ impl LspContext for LspEvalContext { let maybe_contents = self.get_load_contents(¶ms.uri).ok().flatten(); // Evaluate the module - let load_resolver = self.load_resolver_for(path_buf); - let ctx = EvalContext::new(load_resolver); + let config = self.config_for(path_buf); + let ctx = + EvalContext::from_session_and_config(Default::default(), config); let eval_result = if let Some(contents) = maybe_contents { ctx.set_source_path(path_buf.clone()) diff --git a/crates/pcb-zen/src/resolve.rs b/crates/pcb-zen/src/resolve.rs index 6ec8cdd7..2b68fa08 100644 --- a/crates/pcb-zen/src/resolve.rs +++ b/crates/pcb-zen/src/resolve.rs @@ -6,6 +6,7 @@ use pcb_zen_core::config::{ }; use pcb_zen_core::resolution::{ build_resolution_map, semver_family, ModuleLine, NativePathResolver, PackagePathResolver, + ResolutionResult, }; use pcb_zen_core::DefaultFileProvider; use rayon::prelude::*; @@ -132,93 +133,80 @@ impl From for PackageManifest { } } -#[derive(Default, Debug, Clone)] -pub struct ResolutionResult { - /// Map from Package Root (Absolute Path) -> Import URL -> Resolved Absolute Path - pub package_resolutions: HashMap>, - /// Package dependencies in the build closure: ModuleLine -> Version - pub closure: HashMap, - /// Asset dependencies: (module_path, ref) -> resolved_path - pub assets: HashMap<(String, String), PathBuf>, - /// Whether the lockfile (pcb.sum) was updated during resolution - pub lockfile_changed: bool, -} +/// Print the dependency tree to stdout. +pub fn print_dep_tree(resolution: &ResolutionResult) { + let workspace_info = &resolution.workspace_info; + let workspace_name = workspace_info + .root + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or("workspace"); -impl ResolutionResult { - /// Print the dependency tree to stdout - pub fn print_tree(&self, workspace_info: &WorkspaceInfo) { - let workspace_name = workspace_info - .root - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or("workspace"); - - // Index closure by path for fast lookup - let by_path: HashMap<&str, &Version> = self - .closure - .iter() - .map(|(line, version)| (line.path.as_str(), version)) - .collect(); + // Index closure by path for fast lookup + let by_path: HashMap<&str, &Version> = resolution + .closure + .iter() + .map(|(line, version)| (line.path.as_str(), version)) + .collect(); - // Collect root deps (direct deps from workspace packages) - let mut root_deps: Vec = Vec::new(); - for pkg in workspace_info.packages.values() { - for url in pkg.config.dependencies.keys() { - if by_path.contains_key(url.as_str()) && !root_deps.contains(url) { - root_deps.push(url.clone()); - } + // Collect root deps (direct deps from workspace packages) + let mut root_deps: Vec = Vec::new(); + for pkg in workspace_info.packages.values() { + for url in pkg.config.dependencies.keys() { + if by_path.contains_key(url.as_str()) && !root_deps.contains(url) { + root_deps.push(url.clone()); } } - root_deps.sort(); + } + root_deps.sort(); - if root_deps.is_empty() { - return; - } + if root_deps.is_empty() { + return; + } - // Build dep graph: url -> Vec by reading pcb.toml from resolved paths - let mut dep_graph: HashMap> = HashMap::new(); - for line in self.closure.keys() { - if dep_graph.contains_key(&line.path) { - continue; - } - for deps in self.package_resolutions.values() { - if let Some(resolved) = deps.get(&line.path) { - let pcb_toml = resolved.join("pcb.toml"); - if let Ok(content) = std::fs::read_to_string(&pcb_toml) { - if let Ok(config) = PcbToml::parse(&content) { - let mut transitive: Vec = config - .dependencies - .keys() - .filter(|dep_url| by_path.contains_key(dep_url.as_str())) - .cloned() - .collect(); - transitive.sort(); - dep_graph.insert(line.path.clone(), transitive); - } + // Build dep graph: url -> Vec by reading pcb.toml from resolved paths + let mut dep_graph: HashMap> = HashMap::new(); + for line in resolution.closure.keys() { + if dep_graph.contains_key(&line.path) { + continue; + } + for deps in resolution.package_resolutions.values() { + if let Some(resolved) = deps.get(&line.path) { + let pcb_toml = resolved.join("pcb.toml"); + if let Ok(content) = std::fs::read_to_string(&pcb_toml) { + if let Ok(config) = PcbToml::parse(&content) { + let mut transitive: Vec = config + .dependencies + .keys() + .filter(|dep_url| by_path.contains_key(dep_url.as_str())) + .cloned() + .collect(); + transitive.sort(); + dep_graph.insert(line.path.clone(), transitive); } - break; } + break; } } - - let mut printed = HashSet::new(); - let _ = crate::tree::print_tree(workspace_name.to_string(), root_deps, |url| { - let version = by_path - .get(url.as_str()) - .map(|v| v.to_string()) - .unwrap_or_else(|| "?".into()); - let name = url.split('/').skip(1).collect::>().join("/"); - let already = !printed.insert(url.clone()); - - let label = format!("{} v{}{}", name, version, if already { " (*)" } else { "" }); - let children = if already { - vec![] - } else { - dep_graph.get(url).cloned().unwrap_or_default() - }; - (label, children) - }); } + + let mut printed = HashSet::new(); + let _ = crate::tree::print_tree(workspace_name.to_string(), root_deps, |url| { + let version = by_path + .get(url.as_str()) + .map(|v| v.to_string()) + .unwrap_or_else(|| "?".into()); + let name = url.split('/').skip(1).collect::>().join("/"); + let already = !printed.insert(url.clone()); + + let label = format!("{} v{}{}", name, version, if already { " (*)" } else { "" }); + let children = if already { + vec![] + } else { + dep_graph.get(url).cloned().unwrap_or_default() + }; + (label, children) + }); } /// Result of vendoring operation @@ -719,6 +707,7 @@ pub fn resolve_dependencies( build_native_resolution_map(workspace_info, &closure, &patches, &asset_paths, offline)?; Ok(ResolutionResult { + workspace_info: workspace_info.clone(), package_resolutions, closure, assets: asset_paths, @@ -742,12 +731,12 @@ pub fn resolve_dependencies( /// Pruning should be disabled when offline (can't re-fetch deleted deps). #[instrument(name = "vendor_deps", skip_all)] pub fn vendor_deps( - workspace_info: &WorkspaceInfo, resolution: &ResolutionResult, additional_patterns: &[String], target_vendor_dir: Option<&Path>, prune: bool, ) -> Result { + let workspace_info = &resolution.workspace_info; let vendor_dir = target_vendor_dir .map(PathBuf::from) .unwrap_or_else(|| workspace_info.root.join("vendor")); @@ -773,7 +762,7 @@ pub fn vendor_deps( } log::debug!("Vendor patterns: {:?}", patterns); - let cache = cache_base(); + let cache = &workspace_info.cache_dir; let workspace_vendor = workspace_info.root.join("vendor"); // Build glob matcher @@ -2208,5 +2197,3 @@ fn update_lockfile( Ok(new_lockfile) } - -// PackageClosure and package_closure() method are now in workspace.rs diff --git a/crates/pcb-zen/src/workspace.rs b/crates/pcb-zen/src/workspace.rs index 600ef336..dc9e2410 100644 --- a/crates/pcb-zen/src/workspace.rs +++ b/crates/pcb-zen/src/workspace.rs @@ -6,7 +6,7 @@ use anyhow::Result; use rayon::prelude::*; -use std::collections::{BTreeMap, HashMap, HashSet}; +use std::collections::{BTreeMap, HashMap}; use std::path::{Path, PathBuf}; use pcb_zen_core::config::PcbToml; @@ -16,10 +16,8 @@ use semver::Version; // Re-export core types pub use pcb_zen_core::workspace::{BoardInfo, DiscoveryError, MemberPackage, WorkspaceInfo}; -use crate::cache_index::cache_base; use crate::canonical::{compute_content_hash_from_dir, compute_manifest_hash}; use crate::git; -use crate::resolve::ResolutionResult; use crate::tags; // Re-export compute_tag_prefix from tags module for backwards compatibility @@ -37,20 +35,11 @@ pub enum DirtyReason { }, } -/// Transitive dependency closure for a package -#[derive(Debug, Clone, Default)] -pub struct PackageClosure { - pub local_packages: HashSet, - pub remote_packages: HashSet<(String, String)>, - pub assets: HashSet<(String, String)>, -} - /// Extension methods for WorkspaceInfo that require native features (git, filesystem) pub trait WorkspaceInfoExt { fn reload(&mut self) -> Result<()>; fn dirty_packages(&self) -> BTreeMap; fn populate_dirty(&mut self); - fn package_closure(&self, package_url: &str, resolution: &ResolutionResult) -> PackageClosure; fn board_name_for_zen(&self, zen_path: &Path) -> Option; fn board_info_for_zen(&self, zen_path: &Path) -> Option; fn package_url_for_zen(&self, zen_path: &Path) -> Option; @@ -91,63 +80,6 @@ impl WorkspaceInfoExt for WorkspaceInfo { } } - fn package_closure(&self, package_url: &str, resolution: &ResolutionResult) -> PackageClosure { - let mut closure = PackageClosure::default(); - let mut visited: HashSet = HashSet::new(); - let mut stack: Vec = vec![package_url.to_string()]; - - let cache = cache_base(); - let vendor_base = self.root.join("vendor"); - - let get_pkg_root = |module_path: &str, version: &str| -> PathBuf { - let vendor_path = vendor_base.join(module_path).join(version); - if vendor_path.exists() { - vendor_path - } else { - cache.join(module_path).join(version) - } - }; - - while let Some(url) = stack.pop() { - if !visited.insert(url.clone()) { - continue; - } - - if let Some(pkg) = self.packages.get(&url) { - closure.local_packages.insert(url.clone()); - for dep_url in pkg.config.dependencies.keys() { - stack.push(dep_url.clone()); - } - for (asset_url, asset_spec) in &pkg.config.assets { - if let Ok(ref_str) = pcb_zen_core::extract_asset_ref_strict(asset_spec) { - closure.assets.insert((asset_url.clone(), ref_str)); - } - } - } else if let Some((line, version)) = - resolution.closure.iter().find(|(l, _)| l.path == url) - { - let version_str = version.to_string(); - closure - .remote_packages - .insert((url.clone(), version_str.clone())); - let pkg_root = get_pkg_root(&line.path, &version_str); - if let Some(deps) = resolution.package_resolutions.get(&pkg_root) { - for dep_url in deps.keys() { - stack.push(dep_url.clone()); - } - } - } - } - - for (asset_path, asset_ref) in resolution.assets.keys() { - closure - .assets - .insert((asset_path.clone(), asset_ref.clone())); - } - - closure - } - fn board_name_for_zen(&self, zen_path: &Path) -> Option { let canon = zen_path.canonicalize().ok()?; self.boards() diff --git a/crates/pcb/src/bom.rs b/crates/pcb/src/bom.rs index cfec4101..0504761f 100644 --- a/crates/pcb/src/bom.rs +++ b/crates/pcb/src/bom.rs @@ -2,7 +2,7 @@ use std::io::{self, Write}; use std::path::{Path, PathBuf}; use crate::build::create_diagnostics_passes; -use crate::release::extract_layout_path; +use crate::release::discover_layout_from_output; use anyhow::{Context, Result}; use clap::{Args, ValueEnum}; use pcb_layout::utils; @@ -89,8 +89,7 @@ pub fn execute(args: BomArgs) -> Result<()> { crate::file_walker::require_zen_file(&args.file)?; // Resolve dependencies before evaluation - let (_workspace_info, resolution_result) = - crate::resolve::resolve(args.file.parent(), args.offline, args.locked)?; + let resolution_result = crate::resolve::resolve(args.file.parent(), args.offline, args.locked)?; let file_name = args.file.file_name().unwrap().to_string_lossy(); @@ -99,7 +98,12 @@ pub fn execute(args: BomArgs) -> Result<()> { // Evaluate the design let eval_result = pcb_zen::eval(&args.file, resolution_result); - let layout_path = extract_layout_path(&args.file, &eval_result)?; + let layout_path = eval_result + .output + .as_ref() + .and_then(|output| discover_layout_from_output(&args.file, output).transpose()) + .transpose()? + .map(|d| d.layout_dir); let eval_output = eval_result.output_result().map_err(|mut diagnostics| { // Apply passes and render diagnostics if there are errors diagnostics.apply_passes(&create_diagnostics_passes(&[], &[])); diff --git a/crates/pcb/src/build.rs b/crates/pcb/src/build.rs index c246eadb..cf05986d 100644 --- a/crates/pcb/src/build.rs +++ b/crates/pcb/src/build.rs @@ -3,6 +3,7 @@ use clap::Args; use log::debug; use pcb_sch::Schematic; use pcb_ui::prelude::*; +use pcb_zen_core::resolution::ResolutionResult; use std::path::{Path, PathBuf}; use tracing::{info_span, instrument}; @@ -111,7 +112,7 @@ pub fn build( deny_warnings: bool, has_errors: &mut bool, has_warnings: &mut bool, - resolution_result: pcb_zen::ResolutionResult, + resolution_result: ResolutionResult, ) -> Option { let file_name = zen_path.file_name().unwrap().to_string_lossy(); @@ -186,12 +187,14 @@ pub fn execute(args: BuildArgs) -> Result<()> { let mut has_errors = false; // Resolve dependencies before finding .zen files - let (workspace_info, resolution_result) = + let resolution_result = crate::resolve::resolve(args.path.as_deref(), args.offline, args.locked)?; // Process .zen files using shared walker - always recursive for directories - let zen_files = - file_walker::collect_workspace_zen_files(args.path.as_deref(), &workspace_info)?; + let zen_files = file_walker::collect_workspace_zen_files( + args.path.as_deref(), + &resolution_result.workspace_info, + )?; // Process each .zen file let deny_warnings = args.deny.contains(&"warnings".to_string()); diff --git a/crates/pcb/src/info.rs b/crates/pcb/src/info.rs index e679be8c..2cf93d37 100644 --- a/crates/pcb/src/info.rs +++ b/crates/pcb/src/info.rs @@ -55,7 +55,7 @@ pub fn execute(args: InfoArgs) -> Result<()> { println!(); println!("{}", "Dependencies".with_style(Style::Blue).bold()); let result = pcb_zen::resolve_dependencies(&mut workspace_info, false, false)?; - result.print_tree(&workspace_info); + pcb_zen::print_dep_tree(&result); } Ok(()) diff --git a/crates/pcb/src/layout.rs b/crates/pcb/src/layout.rs index 353081ed..3445aa9e 100644 --- a/crates/pcb/src/layout.rs +++ b/crates/pcb/src/layout.rs @@ -65,8 +65,7 @@ pub fn execute(mut args: LayoutArgs) -> Result<()> { let locked = args.locked || std::env::var("CI").is_ok(); // Resolve dependencies before building - let (_workspace_info, resolution_result) = - crate::resolve::resolve(args.file.parent(), args.offline, locked)?; + let resolution_result = crate::resolve::resolve(args.file.parent(), args.offline, locked)?; let zen_path = &args.file; let file_name = zen_path.file_name().unwrap().to_string_lossy().to_string(); diff --git a/crates/pcb/src/mcp.rs b/crates/pcb/src/mcp.rs index 2b9b8974..79c19f48 100644 --- a/crates/pcb/src/mcp.rs +++ b/crates/pcb/src/mcp.rs @@ -188,7 +188,7 @@ fn run_layout(args: Option, ctx: &McpContext) -> Result { let sync_board_config = get_bool("sync_board_config", true); let no_open = get_bool("no_open", false); - let (_, resolution_result) = crate::resolve::resolve(zen_path.parent(), false, false)?; + let resolution_result = crate::resolve::resolve(zen_path.parent(), false, false)?; let mut has_errors = false; let mut has_warnings = false; diff --git a/crates/pcb/src/open.rs b/crates/pcb/src/open.rs index 336c3934..d34f6d11 100644 --- a/crates/pcb/src/open.rs +++ b/crates/pcb/src/open.rs @@ -23,8 +23,7 @@ pub fn execute(args: OpenArgs) -> Result<()> { crate::file_walker::require_zen_file(&args.file)?; // Resolve dependencies before building - let (_workspace_info, resolution_result) = - crate::resolve::resolve(args.file.parent(), args.offline, args.locked)?; + let resolution_result = crate::resolve::resolve(args.file.parent(), args.offline, args.locked)?; let zen_path = &args.file; let file_name = zen_path.file_name().unwrap().to_string_lossy(); diff --git a/crates/pcb/src/publish.rs b/crates/pcb/src/publish.rs index 5bb00da8..98f7f63f 100644 --- a/crates/pcb/src/publish.rs +++ b/crates/pcb/src/publish.rs @@ -602,7 +602,7 @@ fn build_workspace(workspace: &WorkspaceInfo, suppress: &[String]) -> Result<()> let mut ws = workspace.clone(); let resolution = pcb_zen::resolve_dependencies(&mut ws, false, false)?; - pcb_zen::vendor_deps(&ws, &resolution, &[], None, true)?; + pcb_zen::vendor_deps(&resolution, &[], None, true)?; let mut has_errors = false; let mut has_warnings = false; diff --git a/crates/pcb/src/release.rs b/crates/pcb/src/release.rs index a6f56019..9f194113 100644 --- a/crates/pcb/src/release.rs +++ b/crates/pcb/src/release.rs @@ -7,11 +7,10 @@ use pcb_ui::{Colorize, Spinner, Style, StyledText}; use crate::bom::generate_bom_with_fallback; use pcb_zen::workspace::{get_workspace_info, WorkspaceInfoExt}; -use pcb_zen::{PackageClosure, ResolutionResult}; -use pcb_zen_core::DefaultFileProvider; -use pcb_zen_core::{EvalOutput, WithDiagnostics}; - use pcb_zen::WorkspaceInfo; +use pcb_zen_core::resolution::{PackageClosure, ResolutionResult}; +use pcb_zen_core::DefaultFileProvider; +use pcb_zen_core::EvalOutput; use inquire::Confirm; use std::collections::HashSet; @@ -111,7 +110,6 @@ impl ReleaseLayout { } struct ReleaseInfo { - config: WorkspaceInfo, zen_path: PathBuf, board_name: String, version: String, @@ -128,12 +126,16 @@ struct ReleaseInfo { } impl ReleaseInfo { + fn workspace_info(&self) -> &pcb_zen::WorkspaceInfo { + &self.resolution.workspace_info + } + fn workspace_root(&self) -> &Path { - &self.config.root + &self.resolution.workspace_info.root } fn board_display_name(&self) -> String { - self.config + self.workspace_info() .board_name_for_zen(&self.zen_path) .unwrap_or_else(|| { self.zen_path @@ -310,9 +312,10 @@ pub fn build_board_release( let resolution = pcb_zen::resolve_dependencies(&mut workspace, false, true)?; // Find the package URL for this board - let closure = workspace + let closure = resolution + .workspace_info .package_url_for_zen(&zen_path) - .map(|url| workspace.package_closure(&url, &resolution)); + .map(|url| resolution.package_closure(&url)); info_spinner.set_message("Evaluating zen file"); @@ -350,23 +353,23 @@ pub fn build_board_release( let eval_output = eval_result.output.unwrap(); + let workspace_root = &resolution.workspace_info.root; + // Get git hash for metadata - let git_hash = - git::rev_parse_head(&workspace.root).unwrap_or_else(|| "unknown".to_string()); + let git_hash = git::rev_parse_head(workspace_root).unwrap_or_else(|| "unknown".to_string()); // Use provided version, or fall back to short git hash let version = version.unwrap_or_else(|| { - git::rev_parse_short_head(&workspace.root).unwrap_or_else(|| "unknown".to_string()) + git::rev_parse_short_head(workspace_root).unwrap_or_else(|| "unknown".to_string()) }); // Create release staging directory in workspace root with flat structure - let staging_dir = workspace - .root + let staging_dir = workspace_root .join(".pcb/releases") .join(format!("{}-{}", board_name, version)); // Output directory and name use defaults - let output_dir = workspace.root.join(".pcb/releases"); + let output_dir = workspace_root.join(".pcb/releases"); let output_name = format!("{}-{}.zip", board_name, version); // Delete existing staging dir and recreate @@ -383,7 +386,7 @@ pub fn build_board_release( Some(discovered) => match discovered .kicad_files .kicad_pro - .strip_prefix(&workspace.root) + .strip_prefix(workspace_root) { Ok(kicad_pro_rel) => Some(ReleaseLayout { kicad_pro_rel: kicad_pro_rel.to_path_buf(), @@ -402,7 +405,6 @@ pub fn build_board_release( let schematic = eval_output.to_schematic()?; let info = ReleaseInfo { - config: workspace, zen_path, board_name, version, @@ -539,10 +541,10 @@ fn create_metadata_json(info: &ReleaseInfo) -> serde_json::Value { // Get board description if available let board_description = info - .config + .workspace_info() .board_info_for_zen(&info.zen_path) .map(|b| b.description) - .filter(|d| !d.is_empty()); + .filter(|d: &String| !d.is_empty()); let mut release_obj = serde_json::json!({ "schema_version": RELEASE_SCHEMA_VERSION, @@ -593,26 +595,14 @@ fn create_metadata_json(info: &ReleaseInfo) -> serde_json::Value { }) } -/// Extract layout path from zen evaluation result (public for bom.rs) -/// Returns None if no layout_path property exists or the layout directory doesn't exist -pub fn extract_layout_path( - zen_path: &Path, - eval: &WithDiagnostics, -) -> Result> { - let Some(output) = eval.output.as_ref() else { - return Ok(None); - }; - Ok(discover_layout_from_output(zen_path, output)?.map(|d| d.layout_dir)) -} - -struct DiscoveredLayout { - layout_dir: PathBuf, +pub(crate) struct DiscoveredLayout { + pub(crate) layout_dir: PathBuf, kicad_files: layout_utils::KiCadLayoutFiles, } /// Discover layout info from zen evaluation output. /// Returns None if no layout_path property exists or the layout directory doesn't contain KiCad files. -fn discover_layout_from_output( +pub(crate) fn discover_layout_from_output( zen_path: &Path, output: &EvalOutput, ) -> Result> { @@ -677,14 +667,14 @@ fn copy_sources(info: &ReleaseInfo, _spinner: &Spinner) -> Result<()> { if let Some(closure) = &info.closure { // Precompute all package roots for nested package exclusion let all_pkg_roots: HashSet = info - .config + .workspace_info() .packages .values() .map(|p| workspace_root.join(&p.rel_path)) .collect(); for pkg_url in &closure.local_packages { - if let Some(pkg) = info.config.packages.get(pkg_url) { + if let Some(pkg) = info.workspace_info().packages.get(pkg_url) { let dest = src_dir.join(&pkg.rel_path); copy_dir_all(&pkg.dir(workspace_root), &dest, &all_pkg_roots)?; debug!("Copied package {} to {}", pkg_url, dest.display()); @@ -694,7 +684,6 @@ fn copy_sources(info: &ReleaseInfo, _spinner: &Spinner) -> Result<()> { // 3. Vendor remote dependencies using vendor_deps with "**" pattern let result = pcb_zen::vendor_deps( - &info.config, &info.resolution, &["**".to_string()], Some(&vendor_dir), diff --git a/crates/pcb/src/resolve.rs b/crates/pcb/src/resolve.rs index 1215627f..7c6988d7 100644 --- a/crates/pcb/src/resolve.rs +++ b/crates/pcb/src/resolve.rs @@ -1,18 +1,15 @@ use std::path::Path; use anyhow::{bail, Result}; +use pcb_zen_core::resolution::ResolutionResult; use pcb_zen_core::DefaultFileProvider; use tracing::instrument; use pcb_zen::{get_workspace_info, resolve_dependencies}; #[instrument(name = "vendor", skip_all)] -fn vendor( - workspace_info: &pcb_zen::WorkspaceInfo, - res: &pcb_zen::ResolutionResult, - prune: bool, -) -> Result { - pcb_zen::vendor_deps(workspace_info, res, &[], None, prune) +fn vendor(res: &ResolutionResult, prune: bool) -> Result { + pcb_zen::vendor_deps(res, &[], None, prune) } /// Resolve dependencies for a workspace/board. @@ -25,11 +22,7 @@ fn vendor( /// - The lockfile (pcb.sum) will not be written /// - Resolution will fail if pcb.toml or pcb.sum would need to be modified #[instrument(name = "resolve_dependencies", skip_all)] -pub fn resolve( - input_path: Option<&Path>, - offline: bool, - locked: bool, -) -> Result<(pcb_zen::WorkspaceInfo, pcb_zen::ResolutionResult)> { +pub fn resolve(input_path: Option<&Path>, offline: bool, locked: bool) -> Result { let cwd; let path = match input_path { // Handle both None and empty paths (e.g., "file.zen".parent() returns Some("")) @@ -56,7 +49,7 @@ pub fn resolve( // Sync vendor dir: add missing, prune stale (only prune when not offline and not locked) let prune = !offline && !locked; - let vendor_result = vendor(&workspace_info, &res, prune)?; + let vendor_result = vendor(&res, prune)?; // If we pruned stale entries, re-run resolution so the dep map points to valid paths if vendor_result.pruned_count > 0 { @@ -67,5 +60,5 @@ pub fn resolve( res = resolve_dependencies(&mut workspace_info, offline, locked)?; } - Ok((workspace_info, res)) + Ok(res) } diff --git a/crates/pcb/src/route.rs b/crates/pcb/src/route.rs index f11ab59f..538b2f41 100644 --- a/crates/pcb/src/route.rs +++ b/crates/pcb/src/route.rs @@ -46,8 +46,7 @@ pub fn execute(args: RouteArgs) -> Result<()> { } // Resolve dependencies - let (_workspace_info, resolution_result) = - crate::resolve::resolve(args.file.parent(), false, false)?; + let resolution_result = crate::resolve::resolve(args.file.parent(), false, false)?; let zen_path = &args.file; let board_name = zen_path.file_stem().unwrap().to_string_lossy(); diff --git a/crates/pcb/src/sim.rs b/crates/pcb/src/sim.rs index e0761394..5a9c5ecd 100644 --- a/crates/pcb/src/sim.rs +++ b/crates/pcb/src/sim.rs @@ -47,8 +47,7 @@ pub fn execute(args: SimArgs) -> Result<()> { let mut out = get_output_writer(&args.output.to_string_lossy())?; // Resolve dependencies before building - let (_workspace_info, resolution_result) = - crate::resolve::resolve(zen_path.parent(), args.offline, args.locked)?; + let resolution_result = crate::resolve::resolve(zen_path.parent(), args.offline, args.locked)?; let Some(schematic) = build_zen( zen_path, diff --git a/crates/pcb/src/test.rs b/crates/pcb/src/test.rs index 2e8ed946..6893df98 100644 --- a/crates/pcb/src/test.rs +++ b/crates/pcb/src/test.rs @@ -77,7 +77,7 @@ pub struct TestSummary { pub fn test( zen_path: &Path, passes: Vec>, - resolution_result: pcb_zen::ResolutionResult, + resolution_result: pcb_zen_core::resolution::ResolutionResult, ) -> (Vec, bool) { let file_name = zen_path.file_name().unwrap().to_string_lossy(); @@ -144,9 +144,9 @@ fn execute_testbench_checks( let mut passed_checks = 0; // Create an EvalContext that shares the session (including module tree) with the output - let eval_ctx = EvalContext::with_session( + let eval_ctx = EvalContext::from_session_and_config( eval_output.session().clone(), - eval_output.load_resolver.clone(), + eval_output.config.clone(), ) .set_source_path(std::path::PathBuf::from(testbench.source_path())); @@ -224,12 +224,14 @@ fn execute_testbench_checks( pub fn execute(args: TestArgs) -> Result<()> { // Resolve dependencies before finding .zen files - let (workspace_info, resolution_result) = + let resolution_result = crate::resolve::resolve(args.path.as_deref(), args.offline, args.locked)?; // Process .zen files using shared walker - always recursive for directories - let zen_paths = - file_walker::collect_workspace_zen_files(args.path.as_deref(), &workspace_info)?; + let zen_paths = file_walker::collect_workspace_zen_files( + args.path.as_deref(), + &resolution_result.workspace_info, + )?; let mut all_test_results: Vec = Vec::new(); let mut has_errors = false; diff --git a/crates/pcb/src/update.rs b/crates/pcb/src/update.rs index 3b0934b5..123b58f0 100644 --- a/crates/pcb/src/update.rs +++ b/crates/pcb/src/update.rs @@ -411,6 +411,7 @@ mod tests { let ws = WorkspaceInfo { root: root.clone(), + cache_dir: PathBuf::new(), config: None, packages, lockfile: None, @@ -438,6 +439,7 @@ mod tests { let ws = WorkspaceInfo { root: root.clone(), + cache_dir: PathBuf::new(), config: None, packages: BTreeMap::new(), lockfile: None, diff --git a/crates/pcb/src/vendor.rs b/crates/pcb/src/vendor.rs index 47f00c02..de91ffe8 100644 --- a/crates/pcb/src/vendor.rs +++ b/crates/pcb/src/vendor.rs @@ -48,13 +48,7 @@ pub fn execute(args: VendorArgs) -> Result<()> { }; // Always prune for explicit vendor command - let result = vendor_deps( - &workspace_info, - &resolution, - &additional_patterns, - None, - true, - )?; + let result = vendor_deps(&resolution, &additional_patterns, None, true)?; if result.package_count == 0 && result.asset_count == 0 { println!("{} Vendor directory is up to date", "✓".green().bold());