diff --git a/CHANGELOG.md b/CHANGELOG.md index 66e9643..8f4ecb4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +## 0.2.3 + +- Expand doctor from nickname migration to managed library health checks and repair. +- Recreate missing global/project library roots, footprint/model directories, valid empty symbol libraries, tables and managed registrations. +- Repair stale owned registration paths, wrong KiCad types and disabled owned entries, while preserving unrelated registrations. +- Validate managed files, check registered paths and project/component references, and repair model paths using conservative import matching. +- Report unresolved missing components, models and search paths with a nonzero exit status after safe repairs; never invent deleted component content or overwrite corrupt libraries. +- Preserve preview, backups, nickname migration history and idempotent repairs. + +Validation: 67 automated tests, Clippy, formatting, and release build passed. Live KiCad integration requires manual verification. + ## 0.2.2 - Use `UniLoader_Global` and `UniLoader_Local` nicknames for global and project libraries, preventing project entries from shadowing global managed libraries. diff --git a/Cargo.lock b/Cargo.lock index 9467a96..8cb9dbc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2869,7 +2869,7 @@ checksum = "ebc1c04c71510c7f702b52b7c350734c9ff1295c464a03335b00bb84fc54f853" [[package]] name = "uniloader" -version = "0.2.2" +version = "0.2.3" dependencies = [ "anyhow", "chrono", diff --git a/Cargo.toml b/Cargo.toml index 902d797..bbe4ef1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "uniloader" -version = "0.2.2" +version = "0.2.3" edition = "2024" rust-version = "1.95" diff --git a/README.md b/README.md index 83520f9..9d72ba2 100644 --- a/README.md +++ b/README.md @@ -59,9 +59,9 @@ No-argument startup also runs the watcher. `--noninteractive` leaves unresolved choices pending; `--force` on manual imports deliberately revisits a completed source. Check an imported component in KiCad before using it in a board. -## Upgrade existing libraries to v0.2.2 +## Check and repair libraries (v0.2.3) -Close KiCad and stop the watcher, then preview and apply the nickname migration: +Close KiCad and stop the watcher, then preview and apply repairs: ```sh uniloader doctor --global @@ -71,9 +71,14 @@ uniloader doctor --project /path/to/Board/Board.kicad_pro --fix ``` Repeat the project commands for each existing project. Project repair also checks -and repairs the configured global libraries. Existing library filenames stay the -same. Changed files are backed up; see the [setup guide](docs/SETUP.md#nickname-migration-v022) -for migration scope and recovery. Reopen KiCad afterwards. +and repairs the configured global libraries. Doctor recreates missing managed +folders, empty symbol libraries and table entries, repairs owned registration +paths and nicknames, and checks component and model references. It repairs model +paths when a unique matching file is available. Deleted components require +restoring or reimporting with `--force`; doctor reports unresolved issues and +exits nonzero, including after applying safe repairs. Changed files are backed +up; see the [setup guide](docs/SETUP.md#library-health-and-repair-v023). +Reopen KiCad afterwards. ## Scope diff --git a/docs/SETUP.md b/docs/SETUP.md index 06c18c3..f14e176 100644 --- a/docs/SETUP.md +++ b/docs/SETUP.md @@ -172,9 +172,12 @@ assets. Source archives are preserved by default. Incomplete or invalid ZIPs are retried for the configured period, then remain recoverable rather than stopping subsequent downloads. -## Nickname migration (v0.2.2) +## Library health and repair (v0.2.3) -Before upgrading existing libraries, close KiCad and stop the watcher: +Doctor includes the v0.2.2 nickname migration and now checks and rebuilds managed +library infrastructure, including when the global library root has been deleted. + +Before applying repairs, close KiCad and stop the watcher: ```sh systemctl --user stop uniloader.service @@ -185,21 +188,56 @@ uniloader doctor --project /path/to/Board/Board.kicad_pro --fix systemctl --user start uniloader.service ``` -Without `--fix`, doctor previews the files it would change. `--global` checks the -configured global tables and managed symbol library. `--project` checks those -plus the selected project's tables, managed symbol library, and `.kicad_sch` and -`.kicad_pcb` files recursively inside its directory (including subsheets). Run it -for every existing project; projects elsewhere are not automatically discovered -or modified. External subsheets must be migrated separately. Set -`libraries.kicad_config` to the correct version directory when needed. +Without `--fix`, doctor previews directories and files it would create or repair. +`--global` checks the configured global tables, managed symbol library and managed +footprints/models. `--project` checks those plus the selected project's libraries, +tables, and `.kicad_sch` and `.kicad_pcb` files recursively inside its directory +(including subsheets). Run it for every existing project; projects elsewhere are not automatically discovered +or modified. External subsheets must be checked separately. Set +`libraries.kicad_config` to the correct version directory when needed: doctor +requires an explicit or uniquely detected KiCad configuration for both scopes. -Doctor identifies managed registrations by their resolved library paths, preserves -unrelated registrations and metadata, and refuses an occupied target nickname. +Doctor recreates missing managed roots, `UniLoader.pretty/` and `models/` +directories, valid empty `UniLoader.kicad_sym` files, and missing symbol/footprint +tables or managed entries. Existing malformed managed libraries, tables, duplicate +aliases, and wrong file/directory types block the repair plan before it is applied; +corrupt content is not replaced with an empty library. + +Doctor identifies managed registrations by their resolved library paths. Stale +paths can also be repaired when the registration carries the UniLoader nickname +and `Imported by UniLoader` description. It points owned entries at the configured +global root or portable project paths, restores the KiCad type, and enables owned +entries that were disabled. It preserves unrelated registrations and metadata, +and refuses an occupied target nickname. An unrelated entry using our nickname +without ownership evidence is reported for manual resolution. It updates symbol footprint properties, schematic library IDs and cached symbol IDs, and placed PCB footprint IDs according to the original project-over-global resolution. An old ambiguous ID keeps that original resolution; doctor cannot -recover a different intended library. Asset filenames and model paths stay the -same. Changed S-expression files are reserialized, so whitespace may change. +recover a different intended library. Asset filenames stay the same. Managed +footprints are parsed and their model paths repaired using the same conservative +candidate matching as import: a unique match or identical-content candidates +can be copied/reused in managed model storage. Missing or ambiguous models remain +unchanged and are reported; transforms and embedded model references are kept. +Changed S-expression files are reserialized, so whitespace may change. + +Doctor checks active KiCad registrations for unresolved path variables, missing +paths, wrong types and readability; validates managed symbol/footprint files; and +checks managed symbol bindings and project symbol/footprint references against +the effective global/project registrations and actual components. Missing +configured footprint/model search roots are reported. Third-party registrations +are checked but not repointed, recreated or removed. Disabled third-party entries +and non-KiCad plugins are excluded from reference resolution. It does not audit +the watcher service or recover component content from download history. + +Recreating an empty library restores its structure, not deleted symbols, +footprints or models. Restore those from your backups or deliberately reimport +the original component with `uniloader import SOURCE --global --force` (or +`--project FILE --force`). A normal import may skip a previously completed source. +Doctor reports remaining missing components and models rather than fabricating +placeholders. It exits nonzero for unresolved findings, including when `--fix` +has successfully applied the safe portion of the plan. Preview also exits nonzero +for unresolved findings. No unresolved findings means only that the checked +scope passed, not that missing historical components were recovered. Repairs use the import writer lock, atomic file replacement, and the normal backup/history mechanism. The command prints backup paths under diff --git a/src/doctor.rs b/src/doctor.rs index 8bbcf04..e77a604 100644 --- a/src/doctor.rs +++ b/src/doctor.rs @@ -1,4 +1,5 @@ -//! Explicit, backed-up migration of managed library registrations and references. +//! Backed-up repair and health checks for managed KiCad libraries. +mod checks; use crate::{ config::Config, destination::{self, Destination}, @@ -18,10 +19,11 @@ fn table(dest: &Destination, kind: &str, vars: &Renames, plan: &mut Plan) -> Res ("fp-lib-table", &dest.pretty_dir) }; let path = dest.table_dir.join(file); - if !path.exists() { - return Ok(Renames::new()); - } - let original = fs::read_to_string(&path)?; + let original = if path.exists() { + fs::read_to_string(&path)? + } else { + format!("({kind} (version 7))") + }; let mut node = parse(&original)?; if node.head() != Some(kind) { bail!("Invalid table {}", path.display()); @@ -51,6 +53,7 @@ fn table(dest: &Destination, kind: &str, vars: &Renames, plan: &mut Plan) -> Res bail!("Duplicate nickname {name} in {}", path.display()); } } + let mut managed = false; for entry in node .children_mut() .iter_mut() @@ -61,26 +64,50 @@ fn table(dest: &Destination, kind: &str, vars: &Renames, plan: &mut Plan) -> Res .and_then(Node::value) .context("Library entry missing name")? .to_owned(); - // Only change entries that resolve to our managed file/directory. let uri = entry .field("uri") .and_then(Node::value) .context("Library entry missing URI")?; - let Ok(resolved) = destination::resolve_variables(uri, dest.project.as_deref(), vars) - else { - continue; - }; - let resolved = std::path::PathBuf::from(resolved); - let resolved = if resolved.is_absolute() { - resolved - } else { - dest.table_dir.join(resolved) - }; - let matches = resolved == *target - || (target.exists() && resolved.canonicalize().ok() == target.canonicalize().ok()); - if !matches || entry.field("type").and_then(Node::value) != Some("KiCad") { + let resolved = destination::resolve_variables(uri, dest.project.as_deref(), vars) + .ok() + .map(std::path::PathBuf::from) + .map(|p| { + if p.is_absolute() { + p + } else { + dest.table_dir.join(p) + } + }); + let matches = resolved.as_ref().is_some_and(|p| { + p == target || (target.exists() && p.canonicalize().ok() == target.canonicalize().ok()) + }); + // A stale path can be rebuilt only with explicit UniLoader ownership. + let owned = entry.field("descr").and_then(Node::value) == Some("Imported by UniLoader") + && (name == "UniLoader" || name.starts_with("UniLoader_")); + if !matches && !owned { continue; } + if managed { + bail!( + "Multiple managed registrations in {}; resolve duplicate aliases before repair", + path.display() + ); + } + managed = true; + let new_uri = if kind == "sym_lib_table" { + dest.symbol_uri() + } else { + dest.footprint_uri() + }; + entry.field_mut("uri").unwrap().children_mut()[1] = Node::string(new_uri); + if let Some(field) = entry.field_mut("type") { + *field = parse("(type \"KiCad\")")?; + } else { + entry.children_mut().push(parse("(type \"KiCad\")")?); + } + entry + .children_mut() + .retain(|n| n.head() != Some("disabled")); let wanted = dest.nickname(); if name == wanted || name @@ -106,10 +133,35 @@ fn table(dest: &Destination, kind: &str, vars: &Renames, plan: &mut Plan) -> Res history.insert(name.clone(), wanted.into()); changes.insert(name, wanted.into()); } + if !managed { + if occupied.contains_key(dest.nickname()) { + bail!( + "Nickname {} is occupied in {}; refusing to repoint an unrelated library", + dest.nickname(), + path.display() + ); + } + let uri = if kind == "sym_lib_table" { + dest.symbol_uri() + } else { + dest.footprint_uri() + }; + node = parse( + &destination::register_table(Some(&node.to_string()), kind, dest.nickname(), &uri)?.0, + )?; + changes.extend( + history + .iter() + .filter(|(old, new)| { + new.as_str() == dest.nickname() && !occupied.contains_key(*old) + }) + .map(|(old, new)| (old.clone(), new.clone())), + ); + } if history != original_history { plan.put(history_path, serde_json::to_vec_pretty(&history)?)?; } - if node != parse(&original)? { + if !path.exists() || node != parse(&original)? { plan.put(path, format!("{node}\n").into_bytes())?; } Ok(changes) @@ -155,15 +207,15 @@ fn references(node: &mut Node, symbols: &Renames, footprints: &Renames, parent: } } fn patch(path: &Path, symbols: &Renames, footprints: &Renames, plan: &mut Plan) -> Result<()> { - if !path.is_file() { + let Some(original) = checks::text(path, plan)? else { return Ok(()); - } - let original = fs::read_to_string(path).with_context(|| format!("Read {}", path.display()))?; + }; let mut node = parse(&original).with_context(|| format!("Parse {}", path.display()))?; let before = node.clone(); references(&mut node, symbols, footprints, None); if node != before { - plan.put(path.to_owned(), format!("{node}\n").into_bytes())?; + plan.writes + .insert(path.to_owned(), format!("{node}\n").into_bytes()); } Ok(()) } @@ -186,24 +238,26 @@ fn overlay(dest: &Destination, file: &str, global: &Renames, local: Renames) -> pub fn run(cfg: &Config, project: Option<&Path>, fix: bool) -> Result<()> { let _lock = Lock::acquire()?; - let kicad = importer::kicad_config(cfg, project.is_none())?; + let kicad = importer::kicad_config(cfg, true)?; + let vars = importer::variables(cfg, &kicad); let global = Destination::new(&cfg.libraries.path, None, &kicad)?; let mut plan = Plan::default(); - let sym = table( - &global, - "sym_lib_table", - &cfg.libraries.variables, - &mut plan, - )?; - let fp = table(&global, "fp_lib_table", &cfg.libraries.variables, &mut plan)?; + let mut issues = Vec::new(); + checks::structure(&global, &mut plan)?; + let sym = table(&global, "sym_lib_table", &vars, &mut plan)?; + let fp = table(&global, "fp_lib_table", &vars, &mut plan)?; patch(&global.symbol_path, &sym, &fp, &mut plan)?; + checks::footprints(cfg, &global, &vars, &mut plan, &mut issues)?; + let mut local_destination = None; if let Some(project) = project { let local = Destination::new(&cfg.libraries.path, Some(project), &kicad)?; - let local_sym = table(&local, "sym_lib_table", &cfg.libraries.variables, &mut plan)?; - let local_fp = table(&local, "fp_lib_table", &cfg.libraries.variables, &mut plan)?; + checks::structure(&local, &mut plan)?; + let local_sym = table(&local, "sym_lib_table", &vars, &mut plan)?; + let local_fp = table(&local, "fp_lib_table", &vars, &mut plan)?; let sym = overlay(&local, "sym-lib-table", &sym, local_sym)?; let fp = overlay(&local, "fp-lib-table", &fp, local_fp)?; patch(&local.symbol_path, &sym, &fp, &mut plan)?; + checks::footprints(cfg, &local, &vars, &mut plan, &mut issues)?; for entry in walkdir::WalkDir::new(local.project.as_ref().unwrap()).follow_links(false) { let entry = entry?; if entry.file_type().is_file() @@ -215,6 +269,37 @@ pub fn run(cfg: &Config, project: Option<&Path>, fix: bool) -> Result<()> { patch(entry.path(), &sym, &fp, &mut plan)?; } } + local_destination = Some(local); + } + checks::audit( + &global, + local_destination.as_ref(), + &vars, + &plan, + &mut issues, + )?; + for (label, paths) in [ + ("footprint search", &cfg.libraries.search_paths), + ("model search", &cfg.libraries.model_search_paths), + ] { + for path in paths { + if !path.exists() { + issues.push(format!( + "Missing configured {label} path: {}", + path.display() + )); + } + } + } + for directory in &plan.directories { + println!( + "{} directory: {}", + if fix { "Create" } else { "Would create" }, + directory.display() + ); + } + for message in &plan.messages { + println!("{message}"); } for path in plan.writes.keys() { println!( @@ -223,13 +308,12 @@ pub fn run(cfg: &Config, project: Option<&Path>, fix: bool) -> Result<()> { path.display() ); } - if plan.writes.is_empty() { - println!("No nickname repairs needed."); + issues.sort(); + issues.dedup(); + if plan.writes.is_empty() && plan.directories.is_empty() { + println!("No automatic repairs needed."); } else if fix { - let mut record = Record::new( - global.root.clone(), - crate::storage::hash(b"doctor-nicknames"), - ); + let mut record = Record::new(global.root.clone(), crate::storage::hash(b"doctor-health")); record.id = format!( "doctor-{}", chrono::Utc::now().timestamp_nanos_opt().unwrap_or_default() @@ -237,7 +321,10 @@ pub fn run(cfg: &Config, project: Option<&Path>, fix: bool) -> Result<()> { record.status = "importing".into(); plan.commit(&mut record)?; record.status = "completed".into(); - record.message = "Library nickname migration completed".into(); + record.message = format!( + "Doctor repairs completed; {} unresolved issue(s)", + issues.len() + ); record.save()?; for backup in record.backups { println!("Backup: {}", backup.display()); @@ -248,5 +335,16 @@ pub fn run(cfg: &Config, project: Option<&Path>, fix: bool) -> Result<()> { println!( "Run doctor --project /path/to/Board.kicad_pro for each existing project. Close KiCad and stop the watcher before --fix; reopen KiCad afterwards." ); + for issue in &issues { + eprintln!("Unresolved: {issue}"); + } + if !issues.is_empty() { + bail!( + "Doctor found {} unresolved issue(s); safe repairs {}. Restore deleted components from backups or reimport with --force where needed", + issues.len(), + if fix { "applied" } else { "previewed only" } + ); + } + println!("No unresolved issues found in the checked scope."); Ok(()) } diff --git a/src/doctor/checks.rs b/src/doctor/checks.rs new file mode 100644 index 0000000..04e5e13 --- /dev/null +++ b/src/doctor/checks.rs @@ -0,0 +1,421 @@ +//! Inspect the staged repair state, so preview and fix run the same checks. +use crate::{ + config::Config, + destination::{self, Destination}, + models, + sexpr::{Node, parse}, + storage::Plan, +}; +use anyhow::{Context, Result, bail}; +use std::{ + collections::BTreeMap, + fs, + path::{Path, PathBuf}, +}; + +type Registry = BTreeMap; +type SymbolCache = BTreeMap>; + +pub(super) fn text(path: &Path, plan: &Plan) -> Result> { + if let Some(bytes) = plan.writes.get(path) { + return Ok(Some(String::from_utf8(bytes.clone())?)); + } + match fs::read_to_string(path) { + Ok(text) => Ok(Some(text)), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None), + Err(e) => Err(e).with_context(|| format!("Read {}", path.display())), + } +} + +pub(super) fn structure(dest: &Destination, plan: &mut Plan) -> Result<()> { + for path in [ + &dest.root, + &dest.table_dir, + &dest.pretty_dir, + &dest.models_dir, + ] { + for ancestor in path.ancestors() { + match ancestor.symlink_metadata() { + Ok(_) if !ancestor.is_dir() => bail!( + "Expected a managed directory or directory ancestor (check broken symlinks): {}", + ancestor.display() + ), + Err(e) if e.kind() != std::io::ErrorKind::NotFound => { + return Err(e) + .with_context(|| format!("Inspect directory {}", ancestor.display())); + } + _ => {} + } + } + if !path.is_dir() { + plan.directories.push(path.clone()); + } else { + fs::read_dir(path).with_context(|| format!("Read directory {}", path.display()))?; + } + } + if dest.symbol_path.symlink_metadata().is_ok() && !dest.symbol_path.is_file() { + bail!( + "Expected a managed symbol library file: {}", + dest.symbol_path.display() + ); + } + if !dest.symbol_path.exists() { + plan.put( + dest.symbol_path.clone(), + b"(kicad_symbol_lib (version 20211014) (generator \"uniloader\"))\n".to_vec(), + )?; + plan.messages.push(format!("Recreate empty symbol library: {}. Deleted symbols require restore or reimport --force.", dest.symbol_path.display())); + } else { + let node = parse(&fs::read_to_string(&dest.symbol_path)?)?; + if node.head() != Some("kicad_symbol_lib") + || node + .field("version") + .and_then(Node::value) + .and_then(|v| v.parse::().ok()) + .is_none() + { + bail!( + "Invalid managed symbol library {}; restore it from backup (not overwritten)", + dest.symbol_path.display() + ); + } + } + Ok(()) +} + +pub(super) fn footprints( + cfg: &Config, + dest: &Destination, + vars: &RegistryStrings, + plan: &mut Plan, + issues: &mut Vec, +) -> Result<()> { + if !dest.pretty_dir.is_dir() { + return Ok(()); + } + for entry in fs::read_dir(&dest.pretty_dir)? { + let path = entry?.path(); + if !path.extension().is_some_and(|e| e == "kicad_mod") { + continue; + } + let original = text(&path, plan)?.context("Missing footprint")?; + let mut node = parse(&original).with_context(|| format!("Parse {}", path.display()))?; + if !matches!(node.head(), Some("footprint" | "module")) { + bail!("Invalid managed footprint {}", path.display()); + } + let before = node.clone(); + let repaired = models::repair_models_with_choices( + &mut node, + &path, + &dest.root, + dest, + &cfg.libraries.model_search_paths, + vars, + true, + &BTreeMap::new(), + ); + match repaired { + Ok(models) => { + for message in models + .messages + .into_iter() + .filter(|m| m.contains("unresolved")) + { + issues.push(format!("{}: {message}", path.display())); + } + for (target, bytes) in models.copies { + plan.put(target, bytes)?; + } + if node != before { + plan.writes.insert(path, format!("{node}\n").into_bytes()); + } + } + Err(error) => issues.push(format!("{}: {error:#}", path.display())), + } + } + Ok(()) +} + +type RegistryStrings = BTreeMap; +fn registrations( + dest: &Destination, + file: &str, + kind: &str, + vars: &RegistryStrings, + plan: &Plan, + effective: &mut Registry, + issues: &mut Vec, +) -> Result<()> { + let path = dest.table_dir.join(file); + let Some(raw) = text(&path, plan)? else { + return Ok(()); + }; + let node = parse(&raw)?; + if node.head() != Some(kind) { + bail!("Invalid library table {}", path.display()); + } + for entry in node.children().iter().filter(|n| n.head() == Some("lib")) { + let name = entry + .field("name") + .and_then(Node::value) + .context("Library entry missing name")?; + effective.remove(name); + if entry.field("disabled").is_some() { + continue; + } + if entry.field("type").and_then(Node::value) != Some("KiCad") { + continue; + } + let uri = entry + .field("uri") + .and_then(Node::value) + .context("Library entry missing URI")?; + match destination::resolve_variables(uri, dest.project.as_deref(), vars) { + Ok(resolved) => { + let resolved = PathBuf::from(resolved); + let resolved = if resolved.is_absolute() { + resolved + } else { + dest.table_dir.join(resolved) + }; + let valid = if kind == "sym_lib_table" { + resolved.is_file() || plan.writes.contains_key(&resolved) + } else { + resolved.is_dir() || plan.directories.contains(&resolved) + }; + if !valid { + issues.push(format!("Registered library {name} in {} has missing/wrong-type path: {} (unmanaged paths are not created)", path.display(), resolved.display())); + } else if kind == "sym_lib_table" && resolved.is_file() { + fs::File::open(&resolved) + .with_context(|| format!("Read {}", resolved.display()))?; + } else if kind == "fp_lib_table" && resolved.is_dir() { + fs::read_dir(&resolved) + .with_context(|| format!("Read {}", resolved.display()))?; + } + effective.insert(name.into(), resolved); + } + Err(e) => issues.push(format!("Library {name} in {}: {e:#}", path.display())), + } + } + Ok(()) +} +fn reference( + value: &str, + symbols: bool, + file: &Path, + registry: &Registry, + plan: &Plan, + issues: &mut Vec, + cache: &mut SymbolCache, +) -> Result<()> { + if value.is_empty() { + return Ok(()); + } + let Some((nickname, item)) = value.split_once(':') else { + issues.push(format!( + "{}: unqualified library reference {value}", + file.display() + )); + return Ok(()); + }; + let Some(library) = registry.get(nickname) else { + issues.push(format!( + "{}: reference {value} has no enabled KiCad library registration", + file.display() + )); + return Ok(()); + }; + if item.is_empty() || item.contains(['/', '\\', ':']) || item == "." || item == ".." { + issues.push(format!( + "{}: invalid library reference {value}", + file.display() + )); + return Ok(()); + } + let found = if symbols { + if !cache.contains_key(library) { + let mut names = std::collections::BTreeSet::new(); + if let Some(raw) = text(library, plan)? { + match parse(&raw) { + Ok(lib) if lib.head() == Some("kicad_symbol_lib") => { + names.extend( + lib.children() + .iter() + .filter(|n| n.head() == Some("symbol")) + .filter_map(Node::value) + .map(str::to_owned), + ); + } + _ => issues.push(format!( + "Invalid registered symbol library {}", + library.display() + )), + } + } + cache.insert(library.clone(), names); + } + cache[library].contains(item) + } else { + let path = library.join(format!("{item}.kicad_mod")); + path.is_file() || plan.writes.contains_key(&path) + }; + if !found { + issues.push(format!( + "{}: missing {} {value}; restore or reimport the component", + file.display(), + if symbols { "symbol" } else { "footprint" } + )); + } + Ok(()) +} +fn links( + node: &Node, + file: &Path, + sym: &Registry, + fp: &Registry, + plan: &Plan, + issues: &mut Vec, + cache: &mut SymbolCache, +) -> Result<()> { + match node.head() { + Some("lib_id") => { + if let Some(value) = node.value() { + reference(value, true, file, sym, plan, issues, cache)?; + } + } + Some("footprint") if node.value().is_some_and(|s| s.contains(':')) => { + reference(node.value().unwrap(), false, file, fp, plan, issues, cache)?; + } + Some("property") if node.value() == Some("Footprint") => { + if let Some(value) = node.children().get(2).and_then(Node::atom) { + reference(value, false, file, fp, plan, issues, cache)?; + } + } + _ => {} + } + for child in node.children() { + links(child, file, sym, fp, plan, issues, cache)?; + } + Ok(()) +} +fn inspect( + path: &Path, + sym: &Registry, + fp: &Registry, + plan: &Plan, + issues: &mut Vec, + cache: &mut SymbolCache, +) -> Result<()> { + if let Some(raw) = text(path, plan)? { + let node = parse(&raw).with_context(|| format!("Parse {}", path.display()))?; + let expected = match path.extension().and_then(|e| e.to_str()) { + Some("kicad_sym") => Some("kicad_symbol_lib"), + Some("kicad_sch") => Some("kicad_sch"), + Some("kicad_pcb") => Some("kicad_pcb"), + _ => None, + }; + if let Some(expected) = expected + && node.head() != Some(expected) + { + bail!( + "Invalid KiCad file {}: expected {}", + path.display(), + expected + ); + } + links(&node, path, sym, fp, plan, issues, cache)?; + if node.head() == Some("kicad_symbol_lib") { + let mut names = std::collections::BTreeSet::new(); + for symbol in node + .children() + .iter() + .filter(|n| n.head() == Some("symbol")) + { + let name = symbol.value().context("Symbol missing name")?; + if !names.insert(name) { + issues.push(format!("{}: duplicate symbol {name}", path.display())); + } + } + for symbol in node + .children() + .iter() + .filter(|n| n.head() == Some("symbol")) + { + if let Some(parent) = symbol.field("extends").and_then(Node::value) + && !names.contains(parent) + { + issues.push(format!( + "{}: symbol {} extends missing parent {parent}", + path.display(), + symbol.value().unwrap_or("?") + )); + } + } + } + } + Ok(()) +} + +pub(super) fn audit( + global: &Destination, + local: Option<&Destination>, + vars: &RegistryStrings, + plan: &Plan, + issues: &mut Vec, +) -> Result<()> { + let mut cache = SymbolCache::new(); + let mut sym = Registry::new(); + let mut fp = Registry::new(); + registrations( + global, + "sym-lib-table", + "sym_lib_table", + vars, + plan, + &mut sym, + issues, + )?; + registrations( + global, + "fp-lib-table", + "fp_lib_table", + vars, + plan, + &mut fp, + issues, + )?; + inspect(&global.symbol_path, &sym, &fp, plan, issues, &mut cache)?; + if let Some(local) = local { + registrations( + local, + "sym-lib-table", + "sym_lib_table", + vars, + plan, + &mut sym, + issues, + )?; + registrations( + local, + "fp-lib-table", + "fp_lib_table", + vars, + plan, + &mut fp, + issues, + )?; + inspect(&local.symbol_path, &sym, &fp, plan, issues, &mut cache)?; + for entry in walkdir::WalkDir::new(local.project.as_ref().unwrap()).follow_links(false) { + let entry = entry?; + if entry.file_type().is_file() + && entry + .path() + .extension() + .is_some_and(|e| e == "kicad_sch" || e == "kicad_pcb") + { + inspect(entry.path(), &sym, &fp, plan, issues, &mut cache)?; + } + } + } + Ok(()) +} diff --git a/src/importer.rs b/src/importer.rs index 8890f25..1076bab 100644 --- a/src/importer.rs +++ b/src/importer.rs @@ -156,7 +156,7 @@ fn target( } } } -fn variables(cfg: &Config, kicad: &Path) -> BTreeMap { +pub(crate) fn variables(cfg: &Config, kicad: &Path) -> BTreeMap { let mut vars = cfg.libraries.variables.clone(); if let Some(version) = kicad .file_name() diff --git a/src/main.rs b/src/main.rs index ffc55ea..68c01fb 100644 --- a/src/main.rs +++ b/src/main.rs @@ -75,7 +75,7 @@ enum Commands { #[arg(long)] dry_run: bool, }, - /// Check managed nicknames; --fix backs up and migrates libraries/references. + /// Check libraries and paths; --fix recreates missing managed libraries and repairs references. Doctor { #[arg(long, required_unless_present = "project", conflicts_with = "project")] global: bool, diff --git a/tests/import_cli.rs b/tests/import_cli.rs index 970c3f9..2cd6206 100644 --- a/tests/import_cli.rs +++ b/tests/import_cli.rs @@ -900,7 +900,7 @@ fn doctor_migrates_both_scopes_and_project_references_with_backups() { assert_eq!(link(symbols(&global)[0]), "UniLoader_Global:Package"); let again = h.cmd(&["doctor", "--project", h.project.to_str().unwrap(), "--fix"]); ok(&again); - assert!(output(&again).contains("No nickname repairs needed")); + assert!(output(&again).contains("No automatic repairs needed")); ok(&h.import(&source, &["--force"])); } @@ -962,10 +962,196 @@ fn doctor_updates_global_references_in_projects_after_global_migration() { r#"(sym_lib_table (lib (name "UniLoader_2") (type "KiCad") (uri "/unrelated.kicad_sym")))"#, ) .unwrap(); - ok(&h.cmd(&["doctor", "--project", h.project.to_str().unwrap(), "--fix"])); + let checked = h.cmd(&["doctor", "--project", h.project.to_str().unwrap(), "--fix"]); + assert!(!checked.status.success()); + assert!(output(&checked).contains("unmanaged paths are not created")); assert!( fs::read_to_string(&sch) .unwrap() .contains("\"UniLoader_2:Part\"") ); } + +#[test] +fn doctor_bootstraps_empty_libraries_and_preview_creates_nothing() { + let h = Harness::new("separate"); + let preview = h.cmd(&["doctor", "--global"]); + ok(&preview); + assert!(output(&preview).contains("Would create directory")); + assert!(!h.root.join("global").exists()); + assert!(!h.root.join("kicad-config").exists()); + ok(&h.cmd(&["doctor", "--global", "--fix"])); + for path in [ + "global", + "global/UniLoader.pretty", + "global/models", + "kicad-config", + ] { + assert!(h.root.join(path).is_dir()); + } + let lib = + parse(&fs::read_to_string(h.root.join("global/UniLoader.kicad_sym")).unwrap()).unwrap(); + assert_eq!(lib.head(), Some("kicad_symbol_lib")); + assert!(symbols(&lib).is_empty()); + for file in ["fp-lib-table", "sym-lib-table"] { + assert!( + fs::read_to_string(h.root.join("kicad-config").join(file)) + .unwrap() + .contains("UniLoader_Global") + ); + } + let again = h.cmd(&["doctor", "--global", "--fix"]); + ok(&again); + assert!(output(&again).contains("No automatic repairs needed")); +} + +#[test] +fn doctor_rebuilds_deleted_global_and_project_libraries_and_reports_lost_parts() { + let h = Harness::new("separate"); + let source = h.bundle( + "source", + &[ + ("a.kicad_sym", symbol("Part", "Package")), + ("Package.kicad_mod", footprint("Package", 1.0, "")), + ], + ); + ok(&h.cmd(&["import", source.to_str().unwrap(), "--global"])); + ok(&h.import(&source, &[])); + let sch = h.root.join("project/board.kicad_sch"); + fs::write(&sch, r#"(kicad_sch (symbol (lib_id "UniLoader_Global:Part") (property "Footprint" "UniLoader_Local:Package")))"#).unwrap(); + fs::remove_dir_all(h.root.join("global")).unwrap(); + fs::remove_dir_all(h.root.join("kicad-config")).unwrap(); + fs::remove_dir_all(h.root.join("project/libraries")).unwrap(); + fs::remove_file(h.root.join("project/fp-lib-table")).unwrap(); + let preview = h.cmd(&["doctor", "--project", h.project.to_str().unwrap()]); + assert!(!preview.status.success()); + assert!(output(&preview).contains("missing symbol UniLoader_Global:Part")); + assert!(!h.root.join("global").exists()); + let fixed = h.cmd(&["doctor", "--project", h.project.to_str().unwrap(), "--fix"]); + assert!(!fixed.status.success()); + assert!(output(&fixed).contains("safe repairs applied")); + for root in ["global", "project/libraries"] { + assert!(h.root.join(root).join("UniLoader.pretty").is_dir()); + assert!(h.root.join(root).join("models").is_dir()); + let lib = + parse(&fs::read_to_string(h.root.join(root).join("UniLoader.kicad_sym")).unwrap()) + .unwrap(); + assert_eq!(lib.head(), Some("kicad_symbol_lib")); + assert!(symbols(&lib).is_empty()); + } + ok(&h.cmd(&["import", source.to_str().unwrap(), "--global", "--force"])); + ok(&h.import(&source, &["--force"])); + ok(&h.cmd(&["doctor", "--project", h.project.to_str().unwrap(), "--fix"])); +} + +#[test] +fn doctor_repairs_owned_stale_paths_disabled_entries_and_types() { + let h = Harness::new("separate"); + ok(&h.cmd(&["doctor", "--project", h.project.to_str().unwrap(), "--fix"])); + for dir in ["kicad-config", "project"] { + for file in ["sym-lib-table", "fp-lib-table"] { + let path = h.root.join(dir).join(file); + let mut table = parse(&fs::read_to_string(&path).unwrap()).unwrap(); + let entry = table.field_mut("lib").unwrap(); + entry.field_mut("uri").unwrap().children_mut()[1] = + Node::string("${DELETED_ROOT}/obsolete"); + entry.field_mut("type").unwrap().children_mut()[1] = Node::string("Legacy"); + entry.children_mut().push(parse("(disabled)").unwrap()); + entry + .children_mut() + .push(parse("(future_metadata \"keep\")").unwrap()); + fs::write(path, table.to_string()).unwrap(); + } + } + let preview = h.cmd(&["doctor", "--project", h.project.to_str().unwrap()]); + ok(&preview); + assert!( + fs::read_to_string(h.root.join("project/fp-lib-table")) + .unwrap() + .contains("DELETED_ROOT") + ); + ok(&h.cmd(&["doctor", "--project", h.project.to_str().unwrap(), "--fix"])); + for dir in ["kicad-config", "project"] { + for file in ["sym-lib-table", "fp-lib-table"] { + let text = fs::read_to_string(h.root.join(dir).join(file)).unwrap(); + assert!(!text.contains("DELETED_ROOT")); + assert!(!text.contains("disabled")); + assert!(text.contains("KiCad")); + assert!(text.contains("future_metadata")); + if dir == "project" { + assert!(text.contains("${KIPRJMOD}/libraries/")); + } + } + } +} + +#[test] +fn doctor_detects_deleted_footprints_and_repairs_models_without_guessing() { + let h = Harness::new("separate"); + ok(&h.cmd(&["doctor", "--global", "--fix"])); + let fp = h.root.join("global/UniLoader.pretty/Package.kicad_mod"); + let original = footprint( + "Package", + 1.0, + r#"(model "C:\obsolete\part.step" (offset (xyz 1 2 3)))"#, + ); + fs::write(&fp, &original).unwrap(); + fs::write(h.root.join("global/models/part.step"), "RECOVERED MODEL").unwrap(); + ok(&h.cmd(&["doctor", "--global"])); + assert_eq!(fs::read_to_string(&fp).unwrap(), original); + ok(&h.cmd(&["doctor", "--global", "--fix"])); + let repaired = parse(&fs::read_to_string(&fp).unwrap()).unwrap(); + assert_eq!( + repaired + .field("model") + .unwrap() + .field("offset") + .unwrap() + .field("xyz") + .unwrap() + .children()[1] + .atom(), + Some("1") + ); + assert!(Path::new(repaired.field("model").unwrap().value().unwrap()).is_file()); + // Competing model files are reported; no arbitrary candidate is selected. + fs::write(&fp, &original).unwrap(); + fs::create_dir_all(h.root.join("global/other")).unwrap(); + fs::write(h.root.join("global/other/part.step"), "DIFFERENT MODEL").unwrap(); + let ambiguous = h.cmd(&["doctor", "--global", "--fix"]); + assert!(!ambiguous.status.success()); + assert!(output(&ambiguous).contains("ambiguous model")); + assert_eq!(fs::read_to_string(&fp).unwrap(), original); + fs::remove_dir_all(h.root.join("global/other")).unwrap(); + fs::remove_file(h.root.join("global/models/part.step")).unwrap(); + let missing = h.cmd(&["doctor", "--global", "--fix"]); + assert!(!missing.status.success()); + assert!(output(&missing).contains("Missing model")); + assert_eq!(fs::read_to_string(&fp).unwrap(), original); + fs::write( + h.root.join("global/UniLoader.kicad_sym"), + symbol("Part", "UniLoader_Global:Package"), + ) + .unwrap(); + fs::remove_dir_all(h.root.join("global/UniLoader.pretty")).unwrap(); + let deleted = h.cmd(&["doctor", "--global", "--fix"]); + assert!(!deleted.status.success()); + assert!(output(&deleted).contains("missing footprint UniLoader_Global:Package")); + assert!(h.root.join("global/UniLoader.pretty").is_dir()); +} + +#[test] +fn doctor_does_not_overwrite_corrupt_libraries_or_partially_apply_a_failed_plan() { + let h = Harness::new("separate"); + fs::create_dir_all(h.root.join("project/libraries")).unwrap(); + let bad = h.root.join("project/libraries/UniLoader.kicad_sym"); + fs::write(&bad, "(kicad_symbol_lib (broken").unwrap(); + let result = h.cmd(&["doctor", "--project", h.project.to_str().unwrap(), "--fix"]); + assert!(!result.status.success()); + assert_eq!( + fs::read_to_string(bad).unwrap(), + "(kicad_symbol_lib (broken" + ); + assert!(!h.root.join("global").exists()); + assert!(!h.root.join("kicad-config").exists()); +}