diff --git a/src/utils/config.rs b/src/utils/config.rs index 655890d..93a4d85 100644 --- a/src/utils/config.rs +++ b/src/utils/config.rs @@ -1911,13 +1911,23 @@ impl Config { ); } - // Load extension path state for path-based extensions - let ext_path_state = crate::utils::ext_fetch::ExtensionPathState::load_from_dir(&src_dir) - .ok() - .flatten(); - // For each remote extension, try to read its config for (ext_name, source) in remote_extensions { + // For a `type: path` source, resolve its host dir directly from the + // config declaration (relative to src_dir) — the config is the + // source of truth, so no separate state file is consulted. + let path_source_dir: Option = match &source { + ExtensionSource::Path { path, .. } => { + let p = if Path::new(path).is_absolute() { + PathBuf::from(path) + } else { + src_dir.join(path) + }; + Some(p.canonicalize().unwrap_or(p)) + } + _ => None, + }; + // Try multiple methods to read the extension config: // 0. Path-based extension: read directly from source path (for source: { type: path }) // 1. Direct container path (when running inside a container) @@ -1925,72 +1935,67 @@ impl Config { // 3. Local fallback path (for development) let ext_content = { - // Method 0: Check if this is a path-based extension (source: { type: path }) - // For path-based extensions, read from the registered source path on the host - if let Some(ref state) = ext_path_state { - if let Some(source_path) = state.path_mounts.get(&ext_name) { - let config_path_yaml = source_path.join("avocado.yaml"); - let config_path_yml = source_path.join("avocado.yml"); + // Method 0: path-based extension (source: { type: path }) — read + // its config directly from the declared source path on the host. + if let Some(ref source_path) = path_source_dir { + let config_path_yaml = source_path.join("avocado.yaml"); + let config_path_yml = source_path.join("avocado.yml"); - if verbose { - eprintln!( - "[DEBUG] Extension '{}' is path-based, checking: {}", - ext_name, - config_path_yaml.display() - ); - } + if verbose { + eprintln!( + "[DEBUG] Extension '{}' is path-based, checking: {}", + ext_name, + config_path_yaml.display() + ); + } - if config_path_yaml.exists() { - match fs::read_to_string(&config_path_yaml) { - Ok(content) => { - if verbose { - eprintln!( - "[DEBUG] Read {} bytes from path-based source", - content.len() - ); - } - content - } - Err(e) => { - if verbose { - eprintln!("[DEBUG] Failed to read: {e}"); - } - continue; + if config_path_yaml.exists() { + match fs::read_to_string(&config_path_yaml) { + Ok(content) => { + if verbose { + eprintln!( + "[DEBUG] Read {} bytes from path-based source", + content.len() + ); } + content } - } else if config_path_yml.exists() { - match fs::read_to_string(&config_path_yml) { - Ok(content) => { - if verbose { - eprintln!( - "[DEBUG] Read {} bytes from path-based source (.yml)", - content.len() - ); - } - content + Err(e) => { + if verbose { + eprintln!("[DEBUG] Failed to read: {e}"); } - Err(e) => { - if verbose { - eprintln!("[DEBUG] Failed to read: {e}"); - } - continue; + continue; + } + } + } else if config_path_yml.exists() { + match fs::read_to_string(&config_path_yml) { + Ok(content) => { + if verbose { + eprintln!( + "[DEBUG] Read {} bytes from path-based source (.yml)", + content.len() + ); } + content } - } else { - if verbose { - eprintln!( - "[DEBUG] Path-based source path has no avocado.yaml/yml: {}", - source_path.display() - ); + Err(e) => { + if verbose { + eprintln!("[DEBUG] Failed to read: {e}"); + } + continue; } - continue; } } else { - // Not a path-based extension, fall through to other methods - "".to_string() + if verbose { + eprintln!( + "[DEBUG] Path-based source path has no avocado.yaml/yml: {}", + source_path.display() + ); + } + continue; } } else { - // No path state, fall through to other methods + // package/git — fall through to other methods "".to_string() } }; @@ -2160,17 +2165,19 @@ impl Config { }; // Record this extension's source path - // For path-based extensions, use the actual host path - // For other remote extensions, use the container path - let ext_config_path_str = if let Some(ref state) = ext_path_state { - if let Some(source_path) = state.path_mounts.get(&ext_name) { - source_path - .join("avocado.yaml") - .to_string_lossy() - .to_string() + // For path-based extensions, use the actual host path — recording + // the config file that actually exists (avocado.yml is a valid + // alternative to avocado.yaml) so downstream relative-path + // resolution reads the real file. + // For other remote extensions, use the container path. + let ext_config_path_str = if let Some(ref source_path) = path_source_dir { + let yml = source_path.join("avocado.yml"); + let config_file = if yml.exists() { + yml } else { - format!("/opt/_avocado/{resolved_target}/includes/{ext_name}/avocado.yaml") - } + source_path.join("avocado.yaml") + }; + config_file.to_string_lossy().to_string() } else { format!("/opt/_avocado/{resolved_target}/includes/{ext_name}/avocado.yaml") }; diff --git a/src/utils/container.rs b/src/utils/container.rs index a701be3..5b9d824 100644 --- a/src/utils/container.rs +++ b/src/utils/container.rs @@ -482,6 +482,10 @@ pub struct SdkContainer { pub container_tool: String, pub cwd: PathBuf, pub src_dir: Option, + /// Path to the project's `avocado.yaml`, used to derive `type: path` + /// extension mounts directly from config at container launch (source of + /// truth), so no separate persisted state can drift from it. + pub config_path: Option, pub verbose: bool, } @@ -498,6 +502,7 @@ impl SdkContainer { container_tool: "docker".to_string(), cwd: env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), src_dir: None, + config_path: None, verbose: false, } } @@ -509,10 +514,17 @@ impl SdkContainer { container_tool, cwd: env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), src_dir: None, + config_path: None, verbose: false, } } + /// Set the config file path (used to derive `type: path` extension mounts). + pub fn with_config_path(mut self, config_path: Option) -> Self { + self.config_path = config_path; + self + } + /// Set verbose mode pub fn verbose(mut self, verbose: bool) -> Self { self.verbose = verbose; @@ -528,45 +540,97 @@ impl SdkContainer { /// Create a new SdkContainer with configuration from config file pub fn from_config(config_path: &str, config: &crate::utils::config::Config) -> Result { let src_dir = config.get_resolved_src_dir(config_path); - Ok(Self::new().with_src_dir(src_dir)) + Ok(Self::new() + .with_src_dir(src_dir) + .with_config_path(Some(PathBuf::from(config_path)))) } - /// Load extension path mounts from the state file + /// Derive extension path mounts directly from `avocado.yaml`. /// - /// Returns a HashMap of extension name -> host path for extensions that use - /// `source: { type: path }` and were registered via `avocado ext fetch`. + /// Returns a map of extension name -> resolved host path for every extension + /// whose `source: { type: path, path: … }` is declared in the config, to be + /// bindfs-mounted at `$AVOCADO_PREFIX/includes/` at container runtime. /// - /// These paths should be added to `RunConfig.ext_path_mounts` so they get - /// mounted via bindfs at container runtime. - pub fn load_ext_path_mounts(&self) -> Option> { - use crate::utils::ext_fetch::ExtensionPathState; + /// The config is the single source of truth: flipping an extension's source + /// between `package` and `path` takes effect immediately with no separate + /// state file to reconcile or go stale. Only lightweight parsing + + /// interpolation is done here (no remote-extension composition) since a + /// `type: path` source is declared inline and needs no network. + pub fn derive_ext_path_mounts(&self, target: &str) -> Option> { + let config_path = self.config_path.as_ref()?; + // Read/parse/interpolate failures disable path mounts, but warn (when + // verbose) rather than failing silently, so a broken avocado.yaml or + // interpolation error is diagnosable. + let warn = |stage: &str, e: &dyn std::fmt::Display| { + if self.verbose { + crate::utils::output::print_warning( + &format!( + "Could not {stage} {} to derive extension path mounts: {e}", + config_path.display() + ), + OutputLevel::Normal, + ); + } + }; + let content = match std::fs::read_to_string(config_path) { + Ok(c) => c, + Err(e) => { + warn("read", &e); + return None; + } + }; + let mut value: serde_yaml::Value = match serde_yaml::from_str(&content) { + Ok(v) => v, + Err(e) => { + warn("parse", &e); + return None; + } + }; + // Interpolate so templated extension keys (e.g. + // `avocado-bsp-{{ avocado.target.board }}`) and `path` values resolve. + if let Err(e) = crate::utils::interpolation::interpolate_config(&mut value, Some(target)) { + warn("interpolate", &e); + return None; + } + let extensions = value.get("extensions").and_then(|e| e.as_mapping())?; let src_dir = self.src_dir.as_ref().unwrap_or(&self.cwd); - match ExtensionPathState::load_from_dir(src_dir) { - Ok(Some(state)) if !state.path_mounts.is_empty() => { - if self.verbose { - print_info( - &format!( - "Loaded {} extension path mount(s) from state file", - state.path_mounts.len() - ), - OutputLevel::Normal, - ); - } - Some(state.path_mounts) - } - Ok(_) => None, - Err(e) => { - if self.verbose { - print_info( - &format!("Warning: Failed to load extension path state: {e}"), - OutputLevel::Normal, - ); - } - None + let mut mounts = HashMap::new(); + for (key, ext) in extensions { + let Some(name) = key.as_str() else { continue }; + let Some(source) = ext.get("source") else { + continue; + }; + if source.get("type").and_then(|t| t.as_str()) != Some("path") { + continue; } + let Some(path) = source.get("path").and_then(|p| p.as_str()) else { + continue; + }; + let resolved = if Path::new(path).is_absolute() { + PathBuf::from(path) + } else { + src_dir.join(path) + }; + let resolved = resolved.canonicalize().unwrap_or(resolved); + mounts.insert(name.to_string(), resolved); + } + + if mounts.is_empty() { + return None; + } + if self.verbose { + print_info( + &format!( + "Derived {} extension path mount(s) from {}", + mounts.len(), + config_path.display() + ), + OutputLevel::Normal, + ); } + Some(mounts) } /// Create a shared RunsOnContext for running multiple commands on a remote host @@ -716,11 +780,12 @@ impl SdkContainer { config.container_name.clone() }; - // Auto-populate ext_path_mounts from state file if not already set + // Auto-populate ext_path_mounts by deriving them from avocado.yaml if + // not already set explicitly by the caller. let effective_ext_path_mounts = if config.ext_path_mounts.is_some() { config.ext_path_mounts.clone() } else { - self.load_ext_path_mounts() + self.derive_ext_path_mounts(&config.target) }; // Create a modified config with the container name and ext_path_mounts @@ -1280,11 +1345,12 @@ impl SdkContainer { let bash_cmd = vec!["bash".to_string(), "-c".to_string(), full_command]; - // Auto-populate ext_path_mounts from state file if not already set + // Auto-populate ext_path_mounts by deriving them from avocado.yaml if + // not already set explicitly by the caller. let effective_ext_path_mounts = if config.ext_path_mounts.is_some() { config.ext_path_mounts.clone() } else { - self.load_ext_path_mounts() + self.derive_ext_path_mounts(&config.target) }; // Create effective config with ext_path_mounts @@ -2816,6 +2882,60 @@ mod tests { assert!(!container.verbose); } + #[test] + fn derive_ext_path_mounts_from_config() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + // A real extension dir the path source points at (must exist to canonicalize). + std::fs::create_dir_all(root.join("ext/local-thing")).unwrap(); + let cfg = root.join("avocado.yaml"); + std::fs::write( + &cfg, + r#" +extensions: + local-thing: + source: + type: path + path: ext/local-thing + a-package: + source: + type: package + version: "*" +"#, + ) + .unwrap(); + + let container = SdkContainer::new() + .with_src_dir(Some(root.to_path_buf())) + .with_config_path(Some(cfg)); + + let mounts = container + .derive_ext_path_mounts("qemux86-64") + .expect("path extension should produce a mount"); + // Only the `type: path` extension is mounted; package is ignored. + assert_eq!(mounts.len(), 1); + assert_eq!( + mounts.get("local-thing").unwrap(), + &root.join("ext/local-thing").canonicalize().unwrap() + ); + assert!(!mounts.contains_key("a-package")); + } + + #[test] + fn derive_ext_path_mounts_none_when_no_path_sources() { + let tmp = tempfile::tempdir().unwrap(); + let cfg = tmp.path().join("avocado.yaml"); + std::fs::write( + &cfg, + "extensions:\n a-package:\n source:\n type: package\n version: \"*\"\n", + ) + .unwrap(); + let container = SdkContainer::new() + .with_src_dir(Some(tmp.path().to_path_buf())) + .with_config_path(Some(cfg)); + assert!(container.derive_ext_path_mounts("qemux86-64").is_none()); + } + #[test] fn test_sdk_container_with_tool() { let container = SdkContainer::with_tool("podman".to_string()); diff --git a/src/utils/ext_fetch.rs b/src/utils/ext_fetch.rs index df3bdda..cf64969 100644 --- a/src/utils/ext_fetch.rs +++ b/src/utils/ext_fetch.rs @@ -5,91 +5,13 @@ //! - Git repositories (with optional sparse checkout) //! - Local filesystem paths (mounted via bindfs at runtime) -use anyhow::{Context, Result}; -use serde::{Deserialize, Serialize}; -use std::collections::HashMap; -use std::fs; +use anyhow::Result; use std::path::{Path, PathBuf}; use crate::utils::config::ExtensionSource; use crate::utils::container::{RunConfig, SdkContainer}; use crate::utils::output::{print_info, OutputLevel}; -/// State for extension path mounts stored in .avocado/ext-paths.json -#[derive(Debug, Clone, Default, Deserialize, Serialize)] -pub struct ExtensionPathState { - /// Map of extension name to host path for bindfs mounting - pub path_mounts: HashMap, -} - -impl ExtensionPathState { - /// Load extension path state from .avocado/ext-paths.json in the given directory - pub fn load_from_dir(dir_path: &Path) -> Result> { - let state_file = dir_path.join(".avocado").join("ext-paths.json"); - - if !state_file.exists() { - return Ok(None); - } - - let content = fs::read_to_string(&state_file).with_context(|| { - format!( - "Failed to read extension path state file: {}", - state_file.display() - ) - })?; - - let state: Self = serde_json::from_str(&content).with_context(|| { - format!( - "Failed to parse extension path state file: {}", - state_file.display() - ) - })?; - - Ok(Some(state)) - } - - /// Save extension path state to .avocado/ext-paths.json in the given directory - pub fn save_to_dir(&self, dir_path: &Path) -> Result<()> { - let state_dir = dir_path.join(".avocado"); - fs::create_dir_all(&state_dir).with_context(|| { - format!( - "Failed to create .avocado directory: {}", - state_dir.display() - ) - })?; - - let state_file = state_dir.join("ext-paths.json"); - let content = serde_json::to_string_pretty(self) - .with_context(|| "Failed to serialize extension path state".to_string())?; - - fs::write(&state_file, content).with_context(|| { - format!( - "Failed to write extension path state file: {}", - state_file.display() - ) - })?; - - Ok(()) - } - - /// Add a path mount for an extension - pub fn add_path_mount(&mut self, ext_name: String, host_path: PathBuf) { - self.path_mounts.insert(ext_name, host_path); - } - - /// Remove a path mount for an extension - #[allow(dead_code)] - pub fn remove_path_mount(&mut self, ext_name: &str) { - self.path_mounts.remove(ext_name); - } - - /// Get the path mount for an extension - #[allow(dead_code)] - pub fn get_path_mount(&self, ext_name: &str) -> Option<&PathBuf> { - self.path_mounts.get(ext_name) - } -} - /// Extension fetcher for downloading and installing remote extensions pub struct ExtensionFetcher { /// Path to the main configuration file @@ -474,33 +396,11 @@ echo "Successfully fetched extension '{ext_name}' from git" )); } - // Get the state directory (src_dir or config dir) - let state_dir = self.src_dir.clone().unwrap_or_else(|| { - Path::new(&self.config_path) - .parent() - .unwrap_or(Path::new(".")) - .to_path_buf() - }); - - // Load or create extension path state - let mut state = ExtensionPathState::load_from_dir(&state_dir)?.unwrap_or_default(); - - // Add the path mount for this extension - state.add_path_mount(ext_name.to_string(), resolved_source.clone()); - - // Save the state - state.save_to_dir(&state_dir)?; - - if self.verbose { - print_info( - &format!( - "Registered extension '{ext_name}' for bindfs mounting from: {}", - resolved_source.display() - ), - OutputLevel::Normal, - ); - } - + // No state file is written: `type: path` mounts are derived directly + // from avocado.yaml at container launch (see + // `SdkContainer::derive_ext_path_mounts`), so the config stays the + // single source of truth and can't drift. `ext fetch` for a path + // extension is now purely validation (the checks above). print_info( &format!( "Extension '{ext_name}' will be mounted via bindfs at runtime from: {}", @@ -565,31 +465,4 @@ mod tests { let _ = std::fs::remove_dir_all(&tmp_dir); } - - #[test] - fn test_extension_path_state_roundtrip() { - let tmp_dir = std::env::temp_dir().join("avocado_test_path_state"); - let _ = std::fs::remove_dir_all(&tmp_dir); - std::fs::create_dir_all(&tmp_dir).unwrap(); - - let mut state = ExtensionPathState::default(); - state.add_path_mount("ext-a".to_string(), PathBuf::from("/path/to/ext-a")); - state.add_path_mount("ext-b".to_string(), PathBuf::from("/path/to/ext-b")); - state.save_to_dir(&tmp_dir).unwrap(); - - let loaded = ExtensionPathState::load_from_dir(&tmp_dir) - .unwrap() - .unwrap(); - assert_eq!(loaded.path_mounts.len(), 2); - assert_eq!( - loaded.path_mounts.get("ext-a").unwrap(), - &PathBuf::from("/path/to/ext-a") - ); - assert_eq!( - loaded.path_mounts.get("ext-b").unwrap(), - &PathBuf::from("/path/to/ext-b") - ); - - let _ = std::fs::remove_dir_all(&tmp_dir); - } }