diff --git a/src/commands/connect/upload.rs b/src/commands/connect/upload.rs index 4ef2876..ed8973a 100644 --- a/src/commands/connect/upload.rs +++ b/src/commands/connect/upload.rs @@ -369,7 +369,7 @@ impl ConnectUploadCommand { Ok((runtime, num_artifacts)) } - /// Read `avocado.yaml` (converted YAML→JSON) and `.avocado/lock.json` + /// Read `avocado.yaml` (converted YAML→JSON) and the lock file /// (raw JSON) to ship alongside the runtime create request. fn read_config_and_lockfile( &self, @@ -384,7 +384,16 @@ impl ConnectUploadCommand { let project_root = Path::new(&self.config_path) .parent() .unwrap_or_else(|| Path::new(".")); - let lockfile_path = project_root.join(".avocado").join("lock.json"); + // Prefer the top-level `avocado.lock`, falling back to the legacy + // `.avocado/lock.json` for not-yet-migrated projects. + let lockfile_path = { + let new_path = crate::utils::lockfile::LockFile::get_path(project_root); + if new_path.exists() { + new_path + } else { + crate::utils::lockfile::LockFile::legacy_path(project_root) + } + }; let lockfile_json = if lockfile_path.exists() { let content = std::fs::read_to_string(&lockfile_path) .with_context(|| format!("Failed to read {}", lockfile_path.display()))?; diff --git a/src/commands/init.rs b/src/commands/init.rs index e56cf03..3cdcf8c 100644 --- a/src/commands/init.rs +++ b/src/commands/init.rs @@ -848,26 +848,45 @@ impl InitCommand { format!("Failed to read existing '{}'", gitignore_path.display()) })?; - // Check if .avocado-state is already in the .gitignore - if !existing_content.contains(".avocado-state") { - // Append to existing .gitignore + // Ensure both the legacy state file and the `.avocado/` scratch dir + // are ignored. `avocado.lock` is intentionally NOT ignored — it's a + // tracked, committed pin file (like Cargo.lock). + // + // Match whole lines (trimmed), not substrings: a substring test would + // treat a pre-existing `.avocado/lock.json` line as already covering + // `.avocado/` (so the broad scratch ignore is never added), and a bare + // `.avocado` entry would both fail a `.avocado/` substring test and be + // double-appended. + let existing_lines: std::collections::HashSet<&str> = + existing_content.lines().map(str::trim).collect(); + let mut to_add = String::new(); + if !existing_lines.contains(".avocado-state") { + to_add.push_str(".avocado-state\n"); + } + if !existing_lines.contains(".avocado/") { + to_add.push_str(".avocado/\n"); + } + if !to_add.is_empty() { let mut updated_content = existing_content; if !updated_content.ends_with('\n') { updated_content.push('\n'); } - updated_content.push_str("\n# Avocado state files\n.avocado-state\n"); + updated_content.push_str("\n# Avocado scratch + state (avocado.lock is tracked)\n"); + updated_content.push_str(&to_add); fs::write(&gitignore_path, updated_content) .with_context(|| format!("Failed to update '{}'", gitignore_path.display()))?; - self.emit_info("✓ Updated .gitignore to ignore .avocado-state files."); + self.emit_info("✓ Updated .gitignore to ignore Avocado scratch files."); } return Ok(()); } - // Create new .gitignore with Avocado-specific entries - let gitignore_content = "# Avocado state files\n.avocado-state\n"; + // Create new .gitignore with Avocado-specific entries. `avocado.lock` + // is deliberately tracked (committed), so it is not listed here. + let gitignore_content = + "# Avocado scratch + state (avocado.lock is tracked)\n.avocado-state\n.avocado/\n"; fs::write(&gitignore_path, gitignore_content).with_context(|| { format!( diff --git a/src/commands/load.rs b/src/commands/load.rs index 7046ff7..8f4f9ed 100644 --- a/src/commands/load.rs +++ b/src/commands/load.rs @@ -8,6 +8,7 @@ use std::io::Read; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; +use crate::utils::lockfile::LockFile; use crate::utils::output::{print_info, print_success, print_warning, OutputLevel}; use crate::utils::volume::VolumeState; @@ -207,6 +208,43 @@ fn import_archive( .with_context(|| "Failed to restore .avocado/ directory")?; } + // Restore the lock file, normalized to the top-level avocado.lock. Prefer the + // archive's top-level avocado.lock; fall back to a legacy .avocado/lock.json + // from older bundles so their pins aren't silently ignored. Don't clobber an + // existing local lock without --force (mirrors the avocado.yaml handling + // above), then drop any restored legacy copy so the two can't diverge. + let dest_lockfile = LockFile::get_path(config_dir); + let archive_lockfile = { + let top = archive_config_dir.join("avocado.lock"); + let legacy = archive_config_dir.join(".avocado").join("lock.json"); + if top.is_file() { + Some(top) + } else if legacy.is_file() { + Some(legacy) + } else { + None + } + }; + if let Some(src) = archive_lockfile { + if dest_lockfile.exists() && !force { + print_warning( + &format!( + "{} already exists, skipping (use --force to overwrite)", + dest_lockfile.display() + ), + OutputLevel::Normal, + ); + } else { + fs::copy(&src, &dest_lockfile).with_context(|| "Failed to restore avocado.lock")?; + } + } + // The canonical lock now lives at the top level; drop any legacy copy the + // .avocado/ restore brought in so `load()` can't read a divergent file. + let restored_legacy = LockFile::legacy_path(config_dir); + if restored_legacy.exists() { + let _ = fs::remove_file(&restored_legacy); + } + // Restore src_dir if archive includes it let archive_src_dir = temp_path.join("avocado-state/src"); if archive_src_dir.is_dir() { diff --git a/src/commands/save.rs b/src/commands/save.rs index 09811c1..6ce1b1c 100644 --- a/src/commands/save.rs +++ b/src/commands/save.rs @@ -13,6 +13,7 @@ use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use crate::utils::config::Config; +use crate::utils::lockfile::LockFile; use crate::utils::output::{print_info, print_success, OutputLevel}; use crate::utils::target::resolve_target_required; use crate::utils::volume::VolumeState; @@ -204,13 +205,31 @@ fn build_save_archive( )?; } - // 3. Add .avocado/ directory if present + // 3. Add .avocado/ directory if present. + // + // First migrate any legacy `.avocado/lock.json` to the top-level + // `avocado.lock` so the bundle ships exactly one (canonical) lock file — + // otherwise the `.avocado/` archive below and step 3b would both carry a lock + // and they could disagree on restore. `load()` + `save_replacing()` writes + // `avocado.lock` and (via `write_to_disk`) drops the legacy file. let config_dir = config_path.parent().unwrap_or(Path::new(".")); + if LockFile::legacy_path(config_dir).exists() { + if let Ok(lock) = LockFile::load(config_dir) { + let _ = lock.save_replacing(config_dir); + } + } + let avocado_dir = config_dir.join(".avocado"); if avocado_dir.is_dir() { tar.append_dir_all(format!("{ARCHIVE_PREFIX}/config/.avocado"), &avocado_dir)?; } + // 3b. Add the top-level avocado.lock (relocated out of .avocado/) if present. + let lockfile = LockFile::get_path(config_dir); + if lockfile.is_file() { + tar.append_path_with_name(&lockfile, format!("{ARCHIVE_PREFIX}/config/avocado.lock"))?; + } + // 4. Add src_dir if requested if let Some(src_dir) = src_dir { let pb = ProgressBar::new_spinner(); diff --git a/src/utils/lockfile.rs b/src/utils/lockfile.rs index 992550b..b05ee5b 100644 --- a/src/utils/lockfile.rs +++ b/src/utils/lockfile.rs @@ -42,11 +42,15 @@ static LOCKFILE_SAVE_GATE: Mutex<()> = Mutex::new(()); /// exactly as before (track the live channel head). const LOCKFILE_VERSION: u32 = 7; -/// Lock file name -const LOCKFILE_NAME: &str = "lock.json"; +/// Lock file name. Lives at the top level of `src_dir` (like `Cargo.lock` / +/// `flake.lock`) so `.avocado/` can be a purely-scratch, gitignored directory. +const LOCKFILE_NAME: &str = "avocado.lock"; -/// Lock file directory within src_dir -const LOCKFILE_DIR: &str = ".avocado"; +/// Legacy lock file location, used before the lock was promoted to a top-level +/// `avocado.lock`: `/.avocado/lock.json`. Read once and migrated to the +/// new path by [`LockFile::load`]. +const LEGACY_LOCKFILE_DIR: &str = ".avocado"; +const LEGACY_LOCKFILE_NAME: &str = "lock.json"; /// Represents different sysroot types for package installation #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -480,9 +484,15 @@ impl LockFile { } } - /// Get the lock file path for a given src_dir + /// Get the lock file path for a given src_dir. pub fn get_path(src_dir: &Path) -> PathBuf { - src_dir.join(LOCKFILE_DIR).join(LOCKFILE_NAME) + src_dir.join(LOCKFILE_NAME) + } + + /// Pre-relocation lock file path (`/.avocado/lock.json`). Used by + /// [`LockFile::load`] to migrate an existing lock to the new top-level path. + pub fn legacy_path(src_dir: &Path) -> PathBuf { + src_dir.join(LEGACY_LOCKFILE_DIR).join(LEGACY_LOCKFILE_NAME) } /// Load lock file from disk, or return a new one if it doesn't exist @@ -500,20 +510,41 @@ impl LockFile { /// `{ packages: , extensions: {} }`. pub fn load(src_dir: &Path) -> Result { let path = Self::get_path(src_dir); - - if !path.exists() { + let legacy_path = Self::legacy_path(src_dir); + + // Prefer the top-level `avocado.lock`; fall back to reading a legacy + // `.avocado/lock.json`. `load()` is read-only: it never writes or + // relocates, so it can be called (including from within `save()`, which + // holds the save gate) without racing or leaving the legacy file + // half-migrated. Physical migration — writing `avocado.lock` and dropping + // the legacy file — happens on the next `save()` via `write_to_disk`, + // which runs under the gate. + let (content, read_path) = if path.exists() { + let content = fs::read_to_string(&path) + .with_context(|| format!("Failed to read lock file: {}", path.display()))?; + (content, path) + } else if legacy_path.exists() { + let content = fs::read_to_string(&legacy_path) + .with_context(|| format!("Failed to read lock file: {}", legacy_path.display()))?; + (content, legacy_path) + } else { return Ok(Self::new()); - } + }; + + let lock_file = Self::parse_and_migrate(&content, &read_path)?; - let content = fs::read_to_string(&path) - .with_context(|| format!("Failed to read lock file: {}", path.display()))?; + Ok(lock_file) + } - // First, try to parse as current (v6) format. The new fields use + /// Parse lock file `content`, migrating older on-disk versions to the + /// current `LOCKFILE_VERSION`. `path` is used only for error messages. + fn parse_and_migrate(content: &str, path: &Path) -> Result { + // First, try to parse as current format. The newer fields use // `#[serde(default)]`, so v3/v4 files parse here and differ only in // the `version` field (bumped below). v5 files have the old runtime // shape and fail to parse, so they fall through to the explicit // migration path. - if let Ok(mut lock_file) = serde_json::from_str::(&content) { + if let Ok(mut lock_file) = serde_json::from_str::(content) { if lock_file.version > LOCKFILE_VERSION { anyhow::bail!( "Lock file version {} is newer than supported version {}. Please upgrade avocado-cli.", @@ -537,7 +568,7 @@ impl LockFile { } // Fall through: parse as JSON and migrate from older shapes. - let old_lock: serde_json::Value = serde_json::from_str(&content) + let old_lock: serde_json::Value = serde_json::from_str(content) .with_context(|| format!("Failed to parse lock file: {}", path.display()))?; let version = old_lock @@ -799,7 +830,12 @@ impl LockFile { // top so concurrent writers' updates accumulate instead of // clobbering each other. let path = Self::get_path(src_dir); - let to_write = if path.exists() { + // Merge with whatever `load()` returns so a save that happens before an + // explicit load doesn't clobber existing pins. `load()` reads the + // top-level `avocado.lock` if present, else a legacy `.avocado/lock.json` + // (an existing `avocado.lock` takes precedence — a legacy file alongside + // it is ignored). `write_to_disk` below then drops the legacy file. + let to_write = if path.exists() || Self::legacy_path(src_dir).exists() { let on_disk = Self::load(src_dir).context( "Failed to reload lock file before merging — concurrent writer left disk in an unreadable state", )?; @@ -844,7 +880,7 @@ impl LockFile { .with_context(|| "Failed to serialize lock file")?; let content_with_newline = format!("{content}\n"); - let tmp_path = path.with_extension("json.tmp"); + let tmp_path = path.with_extension("lock.tmp"); fs::write(&tmp_path, content_with_newline) .with_context(|| format!("Failed to write lock file temp: {}", tmp_path.display()))?; fs::rename(&tmp_path, &path).with_context(|| { @@ -854,6 +890,16 @@ impl LockFile { path.display() ) })?; + + // Physical migration point: now that the canonical `avocado.lock` is on + // disk, drop any legacy `.avocado/lock.json` so the two can't diverge and + // no bundle ships both. Runs under the save gate (every writer holds it), + // and is best-effort: a lingering legacy is harmless since `load()` always + // prefers `avocado.lock`. + let legacy = Self::legacy_path(src_dir); + if legacy.exists() { + let _ = fs::remove_file(&legacy); + } Ok(()) } @@ -1796,10 +1842,55 @@ wget 1.21-r0.core2_64 let path = LockFile::get_path(std::path::Path::new("/home/user/project")); assert_eq!( path, + std::path::PathBuf::from("/home/user/project/avocado.lock") + ); + let legacy = LockFile::legacy_path(std::path::Path::new("/home/user/project")); + assert_eq!( + legacy, std::path::PathBuf::from("/home/user/project/.avocado/lock.json") ); } + #[test] + fn test_legacy_lock_is_migrated_to_top_level() { + let tmp = tempfile::tempdir().unwrap(); + let src_dir = tmp.path(); + + // Seed a legacy `.avocado/lock.json` and ensure the new path is absent. + let legacy = LockFile::legacy_path(src_dir); + fs::create_dir_all(legacy.parent().unwrap()).unwrap(); + let mut seed = LockFile::new(); + let sdk_x86 = SysrootType::Sdk("x86_64".to_string()); + seed.set_locked_version("qemux86-64", &sdk_x86, "curl", "7.88.1-r0.x86_64"); + fs::write(&legacy, serde_json::to_string_pretty(&seed).unwrap()).unwrap(); + + // `load()` reads the legacy pins but does NOT write/relocate on its own. + let loaded = LockFile::load(src_dir).unwrap(); + assert_eq!( + loaded + .get_locked_version("qemux86-64", &sdk_x86, "curl") + .map(String::as_str), + Some("7.88.1-r0.x86_64") + ); + assert!(!LockFile::get_path(src_dir).exists()); + assert!(legacy.exists()); + + // Migration happens on the next save (the gated write path): avocado.lock + // is written and the legacy file is dropped, pins preserved. + loaded.save(src_dir).unwrap(); + assert!(LockFile::get_path(src_dir).exists()); + assert!(!legacy.exists()); + + // Re-loading reads the new path and is stable. + let reloaded = LockFile::load(src_dir).unwrap(); + assert_eq!( + reloaded + .get_locked_version("qemux86-64", &sdk_x86, "curl") + .map(String::as_str), + Some("7.88.1-r0.x86_64") + ); + } + #[test] fn test_multiple_sysroots_same_target() { let mut lock = LockFile::new(); @@ -3011,9 +3102,7 @@ avocado-sdk-toolchain 0.1.0-r0.x86_64_avocadosdk } }"#; let temp_dir = tempfile::tempdir().unwrap(); - let lock_dir = temp_dir.path().join(LOCKFILE_DIR); - fs::create_dir_all(&lock_dir).unwrap(); - fs::write(lock_dir.join(LOCKFILE_NAME), v5_json).unwrap(); + fs::write(LockFile::get_path(temp_dir.path()), v5_json).unwrap(); let loaded = LockFile::load(temp_dir.path()).unwrap(); assert_eq!(loaded.version, LOCKFILE_VERSION); @@ -3049,9 +3138,7 @@ avocado-sdk-toolchain 0.1.0-r0.x86_64_avocadosdk } }"#; let temp_dir = tempfile::tempdir().unwrap(); - let lock_dir = temp_dir.path().join(LOCKFILE_DIR); - fs::create_dir_all(&lock_dir).unwrap(); - fs::write(lock_dir.join(LOCKFILE_NAME), v4_json).unwrap(); + fs::write(LockFile::get_path(temp_dir.path()), v4_json).unwrap(); let loaded = LockFile::load(temp_dir.path()).unwrap(); assert_eq!(loaded.version, LOCKFILE_VERSION); // bumped to v5 @@ -3079,9 +3166,7 @@ avocado-sdk-toolchain 0.1.0-r0.x86_64_avocadosdk let v6_json = r#"{"version":6,"distro_release":"2026","targets":{"qemux86-64":{"rootfs":{"avocado-pkg-rootfs":"1.0.0-r0"},"runtimes":{"dev":{"packages":{"base":"2.0.0-r0"}}}}}} "#; let temp_dir = TempDir::new().unwrap(); - let lock_dir = temp_dir.path().join(LOCKFILE_DIR); - fs::create_dir_all(&lock_dir).unwrap(); - fs::write(lock_dir.join(LOCKFILE_NAME), v6_json).unwrap(); + fs::write(LockFile::get_path(temp_dir.path()), v6_json).unwrap(); let loaded = LockFile::load(temp_dir.path()).unwrap(); // Version bumped to v7.