Skip to content

Commit 3c2cfd2

Browse files
authored
Prune vendor/ more aggressively, add pcb vendor --all (#367)
* Require confirmation for `pcb vendor` * Improve lockfile is out of date error message * Always prune vendor/ dir on build Don't wait for pcb.sum file change. If you accidentally vendor everything, it needs to be cleaned up without requiring a pcb.sum file change. * Add pcb vendor --all Make `pcb vendor` only vendor workspace.vendor by default. * Only prune vendor dir of not "--locked" and not "--offline"
1 parent d928282 commit 3c2cfd2

File tree

5 files changed

+191
-39
lines changed

5 files changed

+191
-39
lines changed

crates/pcb-zen/src/resolve_v2.rs

Lines changed: 136 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,8 @@ pub struct VendorResult {
287287
pub package_count: usize,
288288
/// Number of assets vendored
289289
pub asset_count: usize,
290+
/// Number of stale entries pruned from vendor/
291+
pub pruned_count: usize,
290292
/// Path to vendor directory
291293
pub vendor_dir: PathBuf,
292294
}
@@ -707,20 +709,42 @@ pub fn resolve_dependencies(
707709
if !is_standalone {
708710
log::debug!("Phase 4: Lockfile");
709711
let lockfile_path = workspace_root.join("pcb.sum");
710-
let old_lockfile = workspace_info.lockfile.clone().unwrap_or_default();
711-
let new_lockfile = update_lockfile(workspace_info, &closure, &asset_paths)?;
712+
let old_lockfile = workspace_info
713+
.lockfile
714+
.as_ref()
715+
.cloned()
716+
.unwrap_or_default();
717+
let new_lockfile = update_lockfile(&workspace_root, &old_lockfile, &closure, &asset_paths)?;
712718

713719
if locked {
714-
// In locked mode: fail only if entries would be added (deletions are safe)
715-
for key in new_lockfile.entries.keys() {
716-
if !old_lockfile.entries.contains_key(key) {
717-
anyhow::bail!(
718-
"Lockfile is out of date (--locked mode)\n \
719-
Run `pcb build` without --locked to update pcb.sum"
720-
);
721-
}
720+
// In locked mode: fail if new entries would be added (deletions are safe)
721+
let mut missing: Vec<_> = new_lockfile
722+
.entries
723+
.keys()
724+
.filter(|k| !old_lockfile.entries.contains_key(*k))
725+
.map(|(path, ver)| format!("{}@{}", path, ver))
726+
.collect();
727+
728+
if !missing.is_empty() {
729+
missing.sort();
730+
let list = missing
731+
.iter()
732+
.take(10)
733+
.map(|k| format!(" - {}", k))
734+
.collect::<Vec<_>>()
735+
.join("\n");
736+
let more = missing
737+
.len()
738+
.checked_sub(10)
739+
.filter(|&n| n > 0)
740+
.map(|n| format!("\n ... and {} more", n))
741+
.unwrap_or_default();
742+
anyhow::bail!(
743+
"Lockfile is out of date (--locked mode)\n\
744+
Missing entries in pcb.sum:\n{list}{more}\n\n\
745+
Run `pcb build` without --locked to update pcb.sum"
746+
);
722747
}
723-
// Don't write lockfile in locked mode
724748
} else {
725749
let old_content = std::fs::read_to_string(&lockfile_path).unwrap_or_default();
726750
let new_content = new_lockfile.to_string();
@@ -729,6 +753,8 @@ pub fn resolve_dependencies(
729753
log::debug!(" Updated {}", lockfile_path.display());
730754
lockfile_changed = true;
731755
}
756+
// Keep workspace_info.lockfile in sync
757+
workspace_info.lockfile = Some(new_lockfile);
732758
}
733759
}
734760

@@ -753,11 +779,18 @@ pub fn resolve_dependencies(
753779
/// If `target_vendor_dir` is provided, vendors to that directory instead of
754780
/// `workspace_info.root/vendor`. This is used by `pcb release` to vendor into
755781
/// the staging directory.
782+
///
783+
/// This function performs an incremental sync:
784+
/// - Adds any packages/assets from the resolution that are missing in vendor/
785+
/// - When `prune=true`, removes any {url}/{version-or-ref} directories not in the resolution
786+
///
787+
/// Pruning should be disabled when offline (can't re-fetch deleted deps).
756788
pub fn vendor_deps(
757789
workspace_info: &WorkspaceInfo,
758790
resolution: &ResolutionResult,
759791
additional_patterns: &[String],
760792
target_vendor_dir: Option<&Path>,
793+
prune: bool,
761794
) -> Result<VendorResult> {
762795
let vendor_dir = target_vendor_dir
763796
.map(PathBuf::from)
@@ -778,6 +811,7 @@ pub fn vendor_deps(
778811
return Ok(VendorResult {
779812
package_count: 0,
780813
asset_count: 0,
814+
pruned_count: 0,
781815
vendor_dir,
782816
});
783817
}
@@ -793,19 +827,23 @@ pub fn vendor_deps(
793827
}
794828
let glob_set = builder.build()?;
795829

796-
// Clean rebuild: delete and recreate vendor directory
797-
if vendor_dir.exists() {
798-
fs::remove_dir_all(&vendor_dir)?;
799-
}
800830
fs::create_dir_all(&vendor_dir)?;
801831

832+
// Track all desired {url}/{version-or-ref} roots for pruning stale entries
833+
let mut desired_roots: HashSet<PathBuf> = HashSet::new();
834+
802835
// Copy matching packages from workspace vendor or cache (vendor takes precedence)
803836
let mut package_count = 0;
804837
for (line, version) in &resolution.closure {
805838
if !glob_set.is_match(&line.path) {
806839
continue;
807840
}
808841
let version_str = version.to_string();
842+
843+
// Track this package root for pruning
844+
let rel_root = PathBuf::from(&line.path).join(&version_str);
845+
desired_roots.insert(rel_root);
846+
809847
let vendor_src = workspace_vendor.join(&line.path).join(&version_str);
810848
let cache_src = cache.join(&line.path).join(&version_str);
811849
let src = if vendor_src.exists() {
@@ -830,6 +868,10 @@ pub fn vendor_deps(
830868
// Split asset_key into (repo_url, subpath) for proper cache/vendor paths
831869
let (repo_url, subpath) = git::split_asset_repo_and_subpath(asset_key);
832870

871+
// Track the repo/ref root for pruning (assets share repo/ref roots)
872+
let rel_root = PathBuf::from(repo_url).join(ref_str);
873+
desired_roots.insert(rel_root);
874+
833875
// Source: check workspace vendor first, then cache
834876
let vendor_src = if subpath.is_empty() {
835877
workspace_vendor.join(repo_url).join(ref_str)
@@ -870,9 +912,17 @@ pub fn vendor_deps(
870912
}
871913
}
872914

915+
// Prune stale {url}/{version-or-ref} directories not in the resolution
916+
let pruned_count = if prune {
917+
prune_stale_vendor_roots(&vendor_dir, &desired_roots)?
918+
} else {
919+
0
920+
};
921+
873922
Ok(VendorResult {
874923
package_count,
875924
asset_count,
925+
pruned_count,
876926
vendor_dir,
877927
})
878928
}
@@ -897,6 +947,75 @@ fn copy_dir_all(src: &Path, dst: &Path) -> Result<()> {
897947
Ok(())
898948
}
899949

950+
/// Prune stale {path}/{version} directories from vendor/
951+
///
952+
/// Walks vendor/ recursively and removes directories not in desired_roots
953+
/// or on the path to a desired root. Returns the number of roots pruned.
954+
fn prune_stale_vendor_roots(vendor_dir: &Path, desired_roots: &HashSet<PathBuf>) -> Result<usize> {
955+
if !vendor_dir.exists() {
956+
return Ok(0);
957+
}
958+
959+
// Build set of ancestor paths (paths we must traverse to reach desired roots)
960+
let mut ancestors: HashSet<PathBuf> = HashSet::new();
961+
for root in desired_roots {
962+
let mut ancestor = PathBuf::new();
963+
for component in root.components() {
964+
ancestors.insert(ancestor.clone());
965+
ancestor.push(component);
966+
}
967+
}
968+
969+
let mut pruned = 0;
970+
prune_dir(
971+
vendor_dir,
972+
&PathBuf::new(),
973+
desired_roots,
974+
&ancestors,
975+
&mut pruned,
976+
)?;
977+
Ok(pruned)
978+
}
979+
980+
fn prune_dir(
981+
base: &Path,
982+
rel: &Path,
983+
desired_roots: &HashSet<PathBuf>,
984+
ancestors: &HashSet<PathBuf>,
985+
pruned: &mut usize,
986+
) -> Result<()> {
987+
for entry in fs::read_dir(base.join(rel))? {
988+
let entry = entry?;
989+
let name = entry.file_name();
990+
let child_rel = if rel.as_os_str().is_empty() {
991+
PathBuf::from(&name)
992+
} else {
993+
rel.join(&name)
994+
};
995+
996+
if entry.file_type()?.is_dir() {
997+
if desired_roots.contains(&child_rel) {
998+
// This is a desired root - keep everything inside it
999+
continue;
1000+
} else if ancestors.contains(&child_rel) {
1001+
// On path to a desired root - recurse to find what to prune
1002+
prune_dir(base, &child_rel, desired_roots, ancestors, pruned)?;
1003+
// Clean up if now empty
1004+
if entry.path().read_dir()?.next().is_none() {
1005+
fs::remove_dir(entry.path())?;
1006+
}
1007+
} else {
1008+
// Not needed - prune entire subtree
1009+
log::debug!("Pruning stale vendor path: {}", child_rel.display());
1010+
fs::remove_dir_all(entry.path())?;
1011+
*pruned += 1;
1012+
}
1013+
}
1014+
// Files at the root level of vendor/ shouldn't exist, ignore them
1015+
}
1016+
Ok(())
1017+
}
1018+
9001019
/// Build the per-package resolution map
9011020
///
9021021
/// When offline=true, only includes paths from workspace members, patches, and vendor/
@@ -2002,12 +2121,11 @@ fn ensure_sparse_checkout(
20022121
/// Creates a fresh lockfile containing only the entries needed for current resolution.
20032122
/// Unused entries from the old lockfile are automatically excluded.
20042123
fn update_lockfile(
2005-
workspace_info: &mut WorkspaceInfo,
2124+
workspace_root: &Path,
2125+
old_lockfile: &Lockfile,
20062126
closure: &HashMap<ModuleLine, Version>,
20072127
asset_paths: &HashMap<(String, String), PathBuf>,
20082128
) -> Result<Lockfile> {
2009-
let workspace_root = &workspace_info.root;
2010-
let old_lockfile = workspace_info.lockfile.take().unwrap_or_default();
20112129
let mut new_lockfile = Lockfile::default();
20122130

20132131
let total_count = closure.len() + asset_paths.len();

crates/pcb/src/publish.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,7 @@ fn build_workspace(workspace: &WorkspaceInfo, suppress: &[String]) -> Result<()>
463463
let resolution_result = if workspace.is_v2() {
464464
let mut ws = workspace.clone();
465465
let resolution = pcb_zen::resolve_dependencies(&mut ws, false, false)?;
466-
pcb_zen::vendor_deps(&ws, &resolution, &[], None)?;
466+
pcb_zen::vendor_deps(&ws, &resolution, &[], None, true)?;
467467
Some(resolution)
468468
} else {
469469
None

crates/pcb/src/release.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -783,6 +783,7 @@ fn copy_sources_v2(info: &ReleaseInfo, closure: &PackageClosure) -> Result<()> {
783783
resolution,
784784
&["**".to_string()],
785785
Some(&vendor_dir),
786+
true, // Always prune for release
786787
)?;
787788
debug!(
788789
"Vendored {} packages and {} assets",

crates/pcb/src/resolve.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,19 @@ pub fn resolve_v2_if_needed(
2929
let mut workspace_info = pcb_zen::get_workspace_info(&DefaultFileProvider::new(), path)?;
3030

3131
let resolution = if workspace_info.is_v2() {
32-
let res = pcb_zen::resolve_dependencies(&mut workspace_info, offline, locked)?;
33-
// Only re-vendor if lockfile changed (avoids expensive delete+copy on every build)
34-
if res.lockfile_changed {
35-
pcb_zen::vendor_deps(&workspace_info, &res, &[], None)?;
32+
let mut res = pcb_zen::resolve_dependencies(&mut workspace_info, offline, locked)?;
33+
34+
// Sync vendor dir: add missing, prune stale (only prune when not offline and not locked)
35+
let prune = !offline && !locked;
36+
let vendor_result = pcb_zen::vendor_deps(&workspace_info, &res, &[], None, prune)?;
37+
38+
// If we pruned stale entries, re-run resolution so the dep map points to valid paths
39+
if vendor_result.pruned_count > 0 {
40+
log::debug!(
41+
"Pruned {} stale vendor entries, re-running resolution",
42+
vendor_result.pruned_count
43+
);
44+
res = pcb_zen::resolve_dependencies(&mut workspace_info, offline, locked)?;
3645
}
3746
Some(res)
3847
} else {

crates/pcb/src/vendor.rs

Lines changed: 40 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@ pub struct VendorArgs {
2121
/// Continue vendoring even if some designs have build errors
2222
#[arg(long = "ignore-errors")]
2323
pub ignore_errors: bool,
24+
25+
/// Vendor all dependencies instead of just those in [workspace.vendor]
26+
#[arg(long = "all")]
27+
pub all: bool,
2428
}
2529

2630
pub fn execute(args: VendorArgs) -> Result<()> {
@@ -32,33 +36,53 @@ pub fn execute(args: VendorArgs) -> Result<()> {
3236

3337
// Check if this is a V2 workspace - use simplified closure-based vendoring
3438
if workspace_info.is_v2() {
35-
return execute_v2(&mut workspace_info);
39+
return execute_v2(&mut workspace_info, args.all);
3640
}
3741

3842
// V1 path: discover zen files and gather dependencies via evaluation
3943
execute_v1(&zen_path, args.ignore_errors, &workspace_info.root)
4044
}
4145

4246
/// V2 vendoring: uses dependency closure from resolution
43-
fn execute_v2(workspace_info: &mut pcb_zen::WorkspaceInfo) -> Result<()> {
44-
println!("V2 workspace detected - using closure-based vendoring\n");
47+
fn execute_v2(workspace_info: &mut pcb_zen::WorkspaceInfo, all: bool) -> Result<()> {
48+
log::debug!("V2 workspace detected - using closure-based vendoring");
49+
50+
if !all {
51+
println!(
52+
"{} `pcb build` automatically vendors [workspace.vendor] dependencies.",
53+
"Note:".yellow()
54+
);
55+
}
4556

4657
// Vendoring always needs network access (offline=false) and allows modifications (locked=false)
4758
let resolution = resolve_dependencies(workspace_info, false, false)?;
4859

49-
// Vendor everything - pass ["**"] pattern to match all packages and assets
50-
let result = vendor_deps(workspace_info, &resolution, &["**".to_string()], None)?;
51-
52-
println!();
53-
println!(
54-
"{} {}",
55-
"✓".green().bold(),
56-
format!(
57-
"Vendored {} packages and {} assets",
58-
result.package_count, result.asset_count
59-
)
60-
.bold()
61-
);
60+
// If --all, vendor everything with ["**"] pattern
61+
// Otherwise, pass empty patterns to use only [workspace.vendor] config
62+
let additional_patterns: Vec<String> = if all { vec!["**".to_string()] } else { vec![] };
63+
64+
// Always prune for explicit vendor command
65+
let result = vendor_deps(
66+
workspace_info,
67+
&resolution,
68+
&additional_patterns,
69+
None,
70+
true,
71+
)?;
72+
73+
if result.package_count == 0 && result.asset_count == 0 {
74+
println!("{} Vendor directory is up to date", "✓".green().bold());
75+
} else {
76+
println!(
77+
"{} {}",
78+
"✓".green().bold(),
79+
format!(
80+
"Vendored {} packages and {} assets",
81+
result.package_count, result.asset_count
82+
)
83+
.bold()
84+
);
85+
}
6286
println!(
6387
"Vendor directory: {}",
6488
result

0 commit comments

Comments
 (0)