Skip to content

Commit 72c4ca1

Browse files
authored
Fix pcb update behavior for kicad-style asset dependencies (#602)
* Fix KiCad asset dependency resolution to follow MVS * Skip prerelease update tags * Normalize legacy assets into dependencies at parse time
1 parent 42f83f2 commit 72c4ca1

File tree

5 files changed

+217
-36
lines changed

5 files changed

+217
-36
lines changed

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@ and this project adheres to Semantic Versioning (https://semver.org/spec/v2.0.0.
1616

1717
- Added typed part metadata with `builtin.Part(...)` and `Component(part=...)`, including JSON netlist serialization and `list[Part]` support for `properties["alternatives"]`.
1818

19+
### Fixed
20+
21+
- Apply MVS-selected KiCad asset versions in resolution/materialization and sibling promotion, preventing `@kicad-*` alias failures after patch updates.
22+
- `pcb update` now ignores prerelease dependency versions when selecting updates.
23+
1924
## [0.3.50] - 2026-03-02
2025

2126
### Changed

crates/pcb-zen-core/src/config.rs

Lines changed: 67 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,67 @@ pub struct PcbToml {
5252
pub access: Option<AccessConfig>,
5353
}
5454

55+
fn asset_targets_repo(asset_key: &str, repo: &str) -> bool {
56+
asset_key == repo
57+
|| (asset_key.starts_with(repo) && asset_key.as_bytes().get(repo.len()) == Some(&b'/'))
58+
}
59+
60+
fn parse_asset_semver(spec: &AssetDependencySpec) -> Option<Version> {
61+
let raw = match spec {
62+
AssetDependencySpec::Ref(v) => Some(v.as_str()),
63+
AssetDependencySpec::Detailed(d) => d.version.as_deref(),
64+
}?;
65+
let raw = raw.strip_prefix('v').unwrap_or(raw);
66+
Version::parse(raw).ok()
67+
}
68+
5569
impl PcbToml {
70+
fn add_implicit_legacy_asset_dependencies(&mut self) {
71+
if self.assets.is_empty() {
72+
return;
73+
}
74+
75+
let entries = self
76+
.workspace
77+
.as_ref()
78+
.map(|w| w.kicad_library.clone())
79+
.unwrap_or_else(default_kicad_library);
80+
let repos: Vec<&String> = entries
81+
.iter()
82+
.flat_map(|entry| {
83+
std::iter::once(&entry.symbols)
84+
.chain(std::iter::once(&entry.footprints))
85+
.chain(entry.models.values())
86+
})
87+
.collect();
88+
89+
let mut selected = BTreeMap::<String, Version>::new();
90+
for (asset_key, spec) in &self.assets {
91+
let Some(version) = parse_asset_semver(spec) else {
92+
continue;
93+
};
94+
95+
for repo in &repos {
96+
if !asset_targets_repo(asset_key, repo) {
97+
continue;
98+
}
99+
let should_update = match selected.get(repo.as_str()) {
100+
Some(cur) => version > *cur,
101+
None => true,
102+
};
103+
if should_update {
104+
selected.insert((*repo).clone(), version.clone());
105+
}
106+
}
107+
}
108+
109+
for (repo, version) in selected {
110+
self.dependencies
111+
.entry(repo)
112+
.or_insert_with(|| DependencySpec::Version(version.to_string()));
113+
}
114+
}
115+
56116
/// Check if this uses legacy V1-only constructs.
57117
fn requires_v1(&self) -> bool {
58118
!self.packages.is_empty() || self.module.is_some()
@@ -99,7 +159,9 @@ impl PcbToml {
99159

100160
/// Parse from TOML string
101161
pub fn parse(content: &str) -> Result<Self> {
102-
toml::from_str(content).map_err(|e| anyhow::anyhow!("{e}"))
162+
let mut parsed: Self = toml::from_str(content).map_err(|e| anyhow::anyhow!("{e}"))?;
163+
parsed.add_implicit_legacy_asset_dependencies();
164+
Ok(parsed)
103165
}
104166

105167
/// Parse from file, rendering errors with ariadne-style diagnostics
@@ -120,7 +182,7 @@ impl PcbToml {
120182

121183
/// Parse TOML content with path context for error reporting
122184
pub fn parse_with_path(content: &str, path: &Path) -> Result<Self> {
123-
toml::from_str(content).map_err(|e| {
185+
let mut parsed: Self = toml::from_str(content).map_err(|e| {
124186
if let Some(span) = e.span() {
125187
let path_str = path.display().to_string();
126188
let mut buf = Vec::new();
@@ -140,7 +202,9 @@ impl PcbToml {
140202
} else {
141203
anyhow::anyhow!("failed to parse {}: {e}", path.display())
142204
}
143-
})
205+
})?;
206+
parsed.add_implicit_legacy_asset_dependencies();
207+
Ok(parsed)
144208
}
145209

146210
/// Extract and parse inline pcb.toml from .zen file content

crates/pcb-zen-core/src/resolution.rs

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -102,26 +102,30 @@ fn resolve_package_deps<R: PackagePathResolver>(
102102
}
103103
}
104104

105-
// If any repo from a [[workspace.kicad_library]] entry is referenced (via
106-
// [dependencies] or [assets]), resolve all sibling repos from that entry.
105+
// If any repo from a [[workspace.kicad_library]] entry is referenced via
106+
// dependencies, resolve all sibling repos from that entry.
107107
// e.g. adding kicad-symbols as a dep also brings in kicad-footprints + models.
108108
for entry in workspace.kicad_library_entries() {
109109
let repos: Vec<&String> = [&entry.symbols, &entry.footprints]
110110
.into_iter()
111111
.chain(entry.models.values())
112112
.collect();
113-
let has_reference = repos.iter().any(|repo| {
114-
let prefix = format!("{repo}/");
115-
map.contains_key(repo.as_str())
116-
|| config
117-
.assets
118-
.keys()
119-
.any(|a| a.starts_with(&prefix) || a == repo.as_str())
120-
});
113+
let has_reference = repos.iter().any(|repo| map.contains_key(repo.as_str()));
121114
if !has_reference {
122115
continue;
123116
}
124-
let version_str = entry.version.to_string();
117+
// Use the version from an already-resolved sibling repo path so this
118+
// stays aligned with selected/MVS resolution.
119+
let Some(version_str) = repos.iter().find_map(|repo| {
120+
map.get(*repo).and_then(|path| {
121+
path.file_name()
122+
.and_then(|name| name.to_str())
123+
.map(ToOwned::to_owned)
124+
})
125+
}) else {
126+
// No resolved sibling to anchor version selection; skip promotion.
127+
continue;
128+
};
125129
for repo in repos {
126130
if !map.contains_key(repo.as_str())
127131
&& let Some(path) = resolver.resolve_package(repo, &version_str)

crates/pcb-zen/src/resolve.rs

Lines changed: 71 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -795,6 +795,17 @@ pub fn resolve_dependencies(
795795

796796
log::debug!("Phase 2: Build closure");
797797

798+
let selected_kicad_assets: BTreeMap<String, Version> = selected
799+
.iter()
800+
.filter(|(line, version)| {
801+
matches!(
802+
match_kicad_managed_repo(&kicad_entries, &line.path, version),
803+
KicadRepoMatch::SelectorMatched
804+
)
805+
})
806+
.map(|(line, version)| (line.path.clone(), version.clone()))
807+
.collect();
808+
798809
// Phase 2: Build the final dependency set using only selected versions
799810
// Path-patched forks are now workspace members, so their deps are included automatically
800811
let closure = build_closure(&workspace_info.packages, &selected, &manifest_cache);
@@ -803,7 +814,7 @@ pub fn resolve_dependencies(
803814

804815
// Phase 2.5: Materialize asset dependencies (KiCad symbol/footprint/model repos).
805816
log::debug!("Phase 2.5: Materialize asset dependencies");
806-
materialize_asset_deps(workspace_info, offline)?;
817+
materialize_asset_deps(workspace_info, &selected_kicad_assets, offline)?;
807818
log::debug!("Materialized asset dependencies");
808819

809820
// Phase 3: (Removed - sparse checkout and hashing now done in Phase 1)
@@ -865,7 +876,8 @@ pub fn resolve_dependencies(
865876

866877
log::debug!("dependency resolution complete");
867878

868-
let package_resolutions = build_native_resolution_map(workspace_info, &closure, &patches);
879+
let package_resolutions =
880+
build_native_resolution_map(workspace_info, &closure, &selected_kicad_assets, &patches);
869881

870882
Ok(ResolutionResult {
871883
workspace_info: workspace_info.clone(),
@@ -1088,6 +1100,7 @@ fn prune_dir(
10881100
fn build_native_resolution_map(
10891101
workspace_info: &WorkspaceInfo,
10901102
closure: &HashMap<ModuleLine, Version>,
1103+
selected_kicad_assets: &BTreeMap<String, Version>,
10911104
patches: &BTreeMap<String, PatchSpec>,
10921105
) -> HashMap<PathBuf, BTreeMap<String, PathBuf>> {
10931106
// Use workspace cache path (symlink) for stable workspace-relative paths in generated files
@@ -1131,6 +1144,15 @@ fn build_native_resolution_map(
11311144
.insert(line.family.clone(), abs_path);
11321145
}
11331146
}
1147+
for (repo, version) in selected_kicad_assets {
1148+
let version_str = version.to_string();
1149+
if let Some(abs_path) = base_resolver.resolve_package(repo, &version_str) {
1150+
families
1151+
.entry(repo.clone())
1152+
.or_default()
1153+
.insert(semver_family(version), abs_path);
1154+
}
1155+
}
11341156

11351157
let resolver = MvsFamilyResolver {
11361158
families,
@@ -1296,14 +1318,17 @@ fn pseudo_matches_rev(version: &Version, rev: &str) -> bool {
12961318
}
12971319

12981320
/// Materialize asset dependencies selected by dependency resolution.
1299-
fn materialize_asset_deps(workspace_info: &WorkspaceInfo, offline: bool) -> Result<()> {
1300-
let targets = workspace_info.asset_dep_versions();
1301-
if targets.is_empty() {
1321+
fn materialize_asset_deps(
1322+
workspace_info: &WorkspaceInfo,
1323+
selected_kicad_assets: &BTreeMap<String, Version>,
1324+
offline: bool,
1325+
) -> Result<()> {
1326+
if selected_kicad_assets.is_empty() {
13021327
return Ok(());
13031328
}
13041329

13051330
let workspace_cache = workspace_info.root.join(".pcb/cache");
1306-
let missing: Vec<(String, Version)> = targets
1331+
let missing: Vec<(String, Version)> = selected_kicad_assets
13071332
.iter()
13081333
.filter(|(repo, version)| {
13091334
!workspace_cache
@@ -2207,9 +2232,48 @@ mod tests {
22072232

22082233
let mut workspace = workspace_with_root_config(config);
22092234
workspace.root = temp.path().to_path_buf();
2210-
let err = materialize_asset_deps(&workspace, true)
2235+
let selected_kicad_assets = BTreeMap::from([(
2236+
"gitlab.com/kicad/libraries/kicad-symbols".to_string(),
2237+
Version::new(9, 0, 0),
2238+
)]);
2239+
let err = materialize_asset_deps(&workspace, &selected_kicad_assets, true)
22112240
.expect_err("expected offline mode to require cached asset deps");
22122241

22132242
assert!(err.to_string().contains("not cached"));
22142243
}
2244+
2245+
#[test]
2246+
fn test_build_native_resolution_map_uses_selected_kicad_asset_version() {
2247+
let temp = TempDir::new().unwrap();
2248+
let root = temp.path().to_path_buf();
2249+
let cache_root = root.join(".pcb/cache/gitlab.com/kicad/libraries/kicad-symbols/9.0.8");
2250+
std::fs::create_dir_all(&cache_root).unwrap();
2251+
2252+
let mut config = PcbToml::default();
2253+
config.dependencies.insert(
2254+
"gitlab.com/kicad/libraries/kicad-symbols".to_string(),
2255+
DependencySpec::Version("9.0.3".to_string()),
2256+
);
2257+
2258+
let mut workspace = workspace_with_root_config(config.clone());
2259+
workspace.root = root.clone();
2260+
2261+
let selected_kicad_assets = BTreeMap::from([(
2262+
"gitlab.com/kicad/libraries/kicad-symbols".to_string(),
2263+
Version::new(9, 0, 8),
2264+
)]);
2265+
let resolutions = build_native_resolution_map(
2266+
&workspace,
2267+
&HashMap::new(),
2268+
&selected_kicad_assets,
2269+
&BTreeMap::new(),
2270+
);
2271+
let deps = resolutions
2272+
.get(&root)
2273+
.expect("expected workspace root dependency map");
2274+
let resolved = deps
2275+
.get("gitlab.com/kicad/libraries/kicad-symbols")
2276+
.expect("expected symbols dependency to resolve");
2277+
assert_eq!(resolved, &cache_root);
2278+
}
22152279
}

crates/pcb/src/update.rs

Lines changed: 59 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,36 @@ fn is_patch_bump(current: &Version, candidate: &Version) -> bool {
5656
candidate.major == current.major && candidate.minor == current.minor && candidate > current
5757
}
5858

59+
fn select_update_candidates<'a>(
60+
available: &'a [Version],
61+
current: &Version,
62+
policy: AutoUpdatePolicy,
63+
) -> (Option<&'a Version>, Option<&'a Version>) {
64+
let current_family = semver_family(current);
65+
let stable_newer: Vec<&Version> = available
66+
.iter()
67+
.filter(|v| v.pre.is_empty() && *v > current)
68+
.collect();
69+
70+
let non_breaking = match policy {
71+
AutoUpdatePolicy::SemverFamily => stable_newer
72+
.iter()
73+
.copied()
74+
.find(|v| semver_family(v) == current_family),
75+
AutoUpdatePolicy::PatchOnly => stable_newer
76+
.iter()
77+
.copied()
78+
.find(|v| is_patch_bump(current, v)),
79+
};
80+
81+
let breaking = stable_newer
82+
.iter()
83+
.copied()
84+
.find(|v| semver_family(v) != current_family);
85+
86+
(non_breaking, breaking)
87+
}
88+
5989
fn detect_update_scope(workspace: &WorkspaceInfo, start_path: &Path) -> UpdateScope {
6090
// Start from a directory; if a file was provided, use its parent dir.
6191
let candidate_dir = if start_path.is_file() {
@@ -326,17 +356,7 @@ fn find_version_updates(
326356
continue;
327357
};
328358

329-
let current_family = semver_family(&current);
330-
331-
// Auto-update policy (non-breaking)
332-
let non_breaking = match policy {
333-
AutoUpdatePolicy::SemverFamily => available
334-
.iter()
335-
.find(|v| semver_family(v) == current_family && *v > &current),
336-
AutoUpdatePolicy::PatchOnly => {
337-
available.iter().find(|v| is_patch_bump(&current, v))
338-
}
339-
};
359+
let (non_breaking, breaking) = select_update_candidates(available, &current, policy);
340360

341361
if let Some(v) = non_breaking {
342362
pending.push(PendingUpdate {
@@ -349,10 +369,7 @@ fn find_version_updates(
349369
}
350370

351371
// Breaking update (different family)
352-
if let Some(v) = available
353-
.iter()
354-
.find(|v| semver_family(v) != current_family && *v > &current)
355-
{
372+
if let Some(v) = breaking {
356373
pending.push(PendingUpdate {
357374
url: url.clone(),
358375
current: current.clone(),
@@ -387,6 +404,33 @@ mod tests {
387404
assert!(!is_patch_bump(&cur0, &Version::parse("0.4.0").unwrap()));
388405
}
389406

407+
#[test]
408+
fn test_select_update_candidates_ignores_prereleases() {
409+
let current = Version::parse("9.0.7").unwrap();
410+
let rc_only = vec![
411+
Version::parse("10.0.0-rc1").unwrap(),
412+
Version::parse("9.0.8-rc1").unwrap(),
413+
];
414+
let with_stable = vec![
415+
Version::parse("10.0.0-rc1").unwrap(),
416+
Version::parse("9.0.8").unwrap(),
417+
Version::parse("9.0.8-rc1").unwrap(),
418+
];
419+
420+
let (non_breaking, breaking) =
421+
select_update_candidates(&rc_only, &current, AutoUpdatePolicy::SemverFamily);
422+
assert!(non_breaking.is_none());
423+
assert!(breaking.is_none());
424+
425+
let (non_breaking, breaking) =
426+
select_update_candidates(&with_stable, &current, AutoUpdatePolicy::SemverFamily);
427+
assert_eq!(
428+
non_breaking.map(|v| v.to_string()),
429+
Some("9.0.8".to_string())
430+
);
431+
assert!(breaking.is_none());
432+
}
433+
390434
#[test]
391435
fn test_detect_update_scope_member_package_dir() {
392436
let td = tempfile::tempdir().unwrap();

0 commit comments

Comments
 (0)