Skip to content

Commit ff09166

Browse files
committed
Keep relocated Yarn store inside the project
Review fixes for #859: - walk a relocated store only when it lies inside the project, as for a relocated pnpm virtual store: a home rc's folder may be shared by other projects; - keep the project's spelling of store paths instead of the canonical one (macOS /private/var, Windows verbatim paths); - a workspace member's node_modules uses the root project's store; - an empty nearest pnpmStoreFolder admits nothing, as before; - walk each store once per scan, dedupe the fan-out's ancestors and share the store-entry mapping. Assisted-by: Claude Code:claude-opus-5-5
1 parent 32d935e commit ff09166

2 files changed

Lines changed: 112 additions & 70 deletions

File tree

‎crates/socket-patch-core/src/crawlers/npm_crawler.rs‎

Lines changed: 109 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -1519,37 +1519,53 @@ fn relocated_pnpm_virtual_store_sync(nm: &Path) -> Option<PathBuf> {
15191519
Some(dir)
15201520
}
15211521

1522-
/// The store of Yarn's pnpm linker for the project whose `node_modules`
1523-
/// is `nm`, when `pnpmStoreFolder` moved it out of `node_modules/.store`
1524-
/// (#859): `.cache/.store`, `store`, set in an rc file, the home rc file
1525-
/// or `YARN_PNPM_STORE_FOLDER`. Yarn then keeps every package there, at
1526-
/// `<store>/<slug>/package`, and `nm` holds only links to the direct
1527-
/// dependencies: the store is the only home of the transitive ones, which
1528-
/// a walk of `nm` alone never reaches (or a hidden name hides from the
1529-
/// workspace walk). Its entries have the `node_modules/.store` shape, so
1530-
/// the same enumeration applies ([`NpmCrawler::list_npm_store_entries_sync`]).
1522+
/// The store of Yarn's pnpm linker for the project `nm` belongs to, when
1523+
/// `pnpmStoreFolder` moved it out of `node_modules/.store` (#859):
1524+
/// `.cache/.store`, `store`, set in an rc file, the home rc file or
1525+
/// `YARN_PNPM_STORE_FOLDER`. Yarn then keeps every package there, at
1526+
/// `<store>/<slug>/package`, and the `node_modules` dirs hold only links
1527+
/// to the direct dependencies: the store is the only home of the
1528+
/// transitive ones, which a walk of `nm` alone never reaches (or a hidden
1529+
/// name hides from the workspace walk). Its entries have the
1530+
/// `node_modules/.store` shape, so the same enumeration applies
1531+
/// ([`NpmCrawler::list_npm_store_entries_sync`]).
15311532
///
1532-
/// Only for a Yarn project (`yarn.lock` beside `nm`) whose active linker
1533-
/// is pnpm, and only for a real directory that is not the default
1534-
/// location (walked by name) and does not contain the project. Returned
1535-
/// canonical.
1533+
/// The project is the nearest folder at or above `nm`'s parent holding a
1534+
/// `yarn.lock` (a workspace member's `node_modules` uses the root's
1535+
/// store), and its active linker must be pnpm. Like a relocated pnpm
1536+
/// virtual store ([`relocated_pnpm_virtual_store_sync`]), only a store
1537+
/// strictly inside the project, reached through real directories, counts:
1538+
/// one elsewhere (a home rc's folder) may be shared with other projects,
1539+
/// and patching it would patch them too. `None` for the default location,
1540+
/// which the walks find by name. The path keeps the project's spelling.
15361541
fn relocated_yarn_pnpm_store_sync(nm: &Path) -> Option<PathBuf> {
1537-
if nm.file_name() != Some(OsStr::new("node_modules")) {
1538-
return None;
1539-
}
1540-
let project = nm.parent()?;
1541-
if !project.join("yarn.lock").is_file() {
1542+
if nm.file_name() != Some(OsStr::new("node_modules")) || !is_dir_sync(nm) {
15421543
return None;
15431544
}
1545+
let project = nm
1546+
.parent()?
1547+
.ancestors()
1548+
.find(|dir| dir.join("yarn.lock").is_file())?;
15441549
let store = super::pkg_managers::active_yarn_pnpm_store(project)?;
1545-
let store = std::fs::canonicalize(store).ok()?;
1546-
if !is_dir_sync(&store)
1547-
|| std::fs::canonicalize(project).is_ok_and(|p| p.starts_with(&store))
1548-
|| std::fs::canonicalize(nm.join(NPM_LINKED_STORE_NAME)).is_ok_and(|d| d == store)
1550+
let (Ok(real_store), Ok(real_project)) =
1551+
(std::fs::canonicalize(store), std::fs::canonicalize(project))
1552+
else {
1553+
return None;
1554+
};
1555+
let below = path_below(&real_project, &real_store)?;
1556+
if below == Path::new("node_modules").join(NPM_LINKED_STORE_NAME)
1557+
|| std::fs::canonicalize(nm.join(NPM_LINKED_STORE_NAME)).is_ok_and(|d| d == real_store)
15491558
{
15501559
return None;
15511560
}
1552-
Some(store)
1561+
let mut dir = project.to_path_buf();
1562+
for component in below.components() {
1563+
dir.push(component);
1564+
if !std::fs::symlink_metadata(&dir).is_ok_and(|m| m.is_dir()) {
1565+
return None;
1566+
}
1567+
}
1568+
Some(dir)
15531569
}
15541570

15551571
/// The entries of pnpm's global virtual store (`enableGlobalVirtualStore`)
@@ -1731,6 +1747,16 @@ struct StoreEntry {
17311747
}
17321748

17331749
impl StoreEntry {
1750+
/// The entries of a store enumeration, keeping what each advertises.
1751+
fn from_dirs(dirs: Vec<StoreEntryDir>) -> Vec<StoreEntry> {
1752+
dirs.into_iter()
1753+
.map(|e| StoreEntry {
1754+
advertised: e.advertised,
1755+
node_modules: e.node_modules,
1756+
})
1757+
.collect()
1758+
}
1759+
17341760
fn pnpm(entries: Vec<(String, PathBuf)>) -> Vec<StoreEntry> {
17351761
entries
17361762
.into_iter()
@@ -2388,11 +2414,21 @@ impl NpmCrawler {
23882414

23892415
fn crawl_all_sync(options: &CrawlerOptions) -> (Vec<CrawledPackage>, Vec<PathBuf>) {
23902416
let nm_paths = Self::node_modules_paths_sync(options);
2391-
let gathered: Vec<Vec<ScanEvent>> = par_map(&nm_paths, |nm_path| {
2417+
// Yarn's relocated pnpm-linker store (#859), walked once, from the
2418+
// first root of its project, after that root's own entries like
2419+
// the stores found by name.
2420+
let mut seen_stores = HashSet::new();
2421+
let roots: Vec<(&PathBuf, Option<PathBuf>)> = nm_paths
2422+
.iter()
2423+
.map(|nm| {
2424+
let store = relocated_yarn_pnpm_store_sync(nm)
2425+
.filter(|store| seen_stores.insert(store.clone()));
2426+
(nm, store)
2427+
})
2428+
.collect();
2429+
let gathered: Vec<Vec<ScanEvent>> = par_map(roots, |(nm_path, store)| {
23922430
let mut events = Self::gather_node_modules(nm_path, None, false);
2393-
// Yarn's relocated pnpm-linker store, after the importer's own
2394-
// entries like the stores found by name (#859).
2395-
if let Some(store) = relocated_yarn_pnpm_store_sync(nm_path) {
2431+
if let Some(store) = store {
23962432
let entries = Self::list_npm_store_entries_sync(&store, true);
23972433
events.extend(Self::gather_store_entries(entries));
23982434
}
@@ -2551,15 +2587,9 @@ impl NpmCrawler {
25512587
// The root's relocated Yarn pnpm-linker store (#859), contributed
25522588
// by the root's visit like a store found there by name.
25532589
let mut yarn_store = relocated_yarn_pnpm_store_sync(node_modules_path).map(|store| {
2554-
NestedNodeModules::StoreEntries(
2555-
Self::list_npm_store_entries_sync(&store, false)
2556-
.into_iter()
2557-
.map(|e| StoreEntry {
2558-
advertised: e.advertised,
2559-
node_modules: e.node_modules,
2560-
})
2561-
.collect(),
2562-
)
2590+
NestedNodeModules::StoreEntries(StoreEntry::from_dirs(
2591+
Self::list_npm_store_entries_sync(&store, false),
2592+
))
25632593
});
25642594
while !level.is_empty() {
25652595
// A bundled node_modules may link back to an ancestor (or to
@@ -2847,13 +2877,9 @@ impl NpmCrawler {
28472877
return Vec::new();
28482878
}
28492879
let store = nm_path.join(&entry.name);
2850-
let entries = Self::list_pnpm_shaped_store_entries_sync(&store, layout, false, false)
2851-
.into_iter()
2852-
.map(|e| StoreEntry {
2853-
advertised: e.advertised,
2854-
node_modules: e.node_modules,
2855-
})
2856-
.collect();
2880+
let entries = StoreEntry::from_dirs(Self::list_pnpm_shaped_store_entries_sync(
2881+
&store, layout, false, false,
2882+
));
28572883
return vec![NestedNodeModules::StoreEntries(entries)];
28582884
}
28592885
// vlt's store has the same transitive-only-home property: every
@@ -2872,13 +2898,10 @@ impl NpmCrawler {
28722898
if !entry.file_type.is_some_and(|ft| ft.is_dir()) {
28732899
return Vec::new();
28742900
}
2875-
let entries = Self::list_npm_store_entries_sync(&nm_path.join(&entry.name), false)
2876-
.into_iter()
2877-
.map(|e| StoreEntry {
2878-
advertised: e.advertised,
2879-
node_modules: e.node_modules,
2880-
})
2881-
.collect();
2901+
let entries = StoreEntry::from_dirs(Self::list_npm_store_entries_sync(
2902+
&nm_path.join(&entry.name),
2903+
false,
2904+
));
28822905
return vec![NestedNodeModules::StoreEntries(entries)];
28832906
}
28842907
// A pnpm virtual store relocated by `virtualStoreDir`, found
@@ -2893,13 +2916,7 @@ impl NpmCrawler {
28932916
let entries = reachable_pnpm_global_virtual_store_entries_sync(nm_path);
28942917
return vec![NestedNodeModules::StoreEntries(entries)];
28952918
};
2896-
let entries = Self::list_pnpm_store_entries_sync(&store, false)
2897-
.into_iter()
2898-
.map(|e| StoreEntry {
2899-
advertised: e.advertised,
2900-
node_modules: e.node_modules,
2901-
})
2902-
.collect();
2919+
let entries = StoreEntry::from_dirs(Self::list_pnpm_store_entries_sync(&store, false));
29032920
return vec![NestedNodeModules::StoreEntries(entries)];
29042921
}
29052922
// pnpm <=3: the virtual store is a hidden `.<registry-host>` dir
@@ -3103,6 +3120,7 @@ impl NpmCrawler {
31033120
let stores: Vec<PathBuf> = results
31043121
.iter()
31053122
.filter_map(|nm| relocated_yarn_pnpm_store_sync(nm))
3123+
.filter_map(|store| std::fs::canonicalize(store).ok())
31063124
.collect();
31073125
if !stores.is_empty() {
31083126
results.retain(|nm| {
@@ -4140,21 +4158,22 @@ async fn find_store_peer_variant_copies_reusing(
41404158
// project's `node_modules` names no such store, its settings do. Only
41414159
// a store holding the primary itself counts, as above.
41424160
if let Some(canonical) = canonical_pkg.clone() {
4143-
let projects: Vec<PathBuf> = chains
4144-
.into_iter()
4145-
.flatten()
4146-
.flat_map(|start| {
4147-
start
4148-
.ancestors()
4149-
.skip(1)
4150-
.map(|dir| dir.join("node_modules"))
4151-
})
4152-
.collect();
4161+
let mut projects: Vec<PathBuf> = Vec::new();
4162+
for start in chains.into_iter().flatten() {
4163+
for dir in start.ancestors().skip(1) {
4164+
let nm = dir.join("node_modules");
4165+
if !projects.contains(&nm) {
4166+
projects.push(nm);
4167+
}
4168+
}
4169+
}
41534170
let yarn_stores = run_walk(move || {
41544171
projects
41554172
.iter()
41564173
.filter_map(|nm| relocated_yarn_pnpm_store_sync(nm))
4157-
.filter(|store| canonical.starts_with(store))
4174+
.filter(|store| {
4175+
std::fs::canonicalize(store).is_ok_and(|real| canonical.starts_with(real))
4176+
})
41584177
.collect::<Vec<_>>()
41594178
})
41604179
.await;
@@ -7513,7 +7532,8 @@ mod tests {
75137532
async fn test_yarn4_pnpm_linker_relocated_store_transitive_packages_are_found() {
75147533
for folder in [".cache/.store", "store"] {
75157534
let tmp = tempfile::tempdir().unwrap();
7516-
let root: PathBuf = tmp.path().components().collect();
7535+
let root: PathBuf = tmp.path().join("proj").components().collect();
7536+
std::fs::create_dir_all(&root).unwrap();
75177537
let nm = root.join("node_modules");
75187538
let store = root.join(folder);
75197539
std::fs::write(root.join("yarn.lock"), "").unwrap();
@@ -7553,6 +7573,26 @@ mod tests {
75537573
)
75547574
.await;
75557575

7576+
// A workspace member's `node_modules` uses the root's store.
7577+
let member_nm = root.join("packages").join("a").join("node_modules");
7578+
std::fs::create_dir_all(&member_nm).unwrap();
7579+
assert_eq!(
7580+
relocated_yarn_pnpm_store_sync(&member_nm),
7581+
Some(store.clone()),
7582+
"{folder}: member"
7583+
);
7584+
// A store outside the project may be shared: never walked.
7585+
std::fs::write(
7586+
root.join(".yarnrc.yml"),
7587+
"nodeLinker: pnpm\npnpmStoreFolder: ../shared\n",
7588+
)
7589+
.unwrap();
7590+
std::fs::create_dir_all(root.join("../shared")).unwrap();
7591+
assert!(
7592+
relocated_yarn_pnpm_store_sync(&nm).is_none(),
7593+
"{folder}: outside the project"
7594+
);
7595+
75567596
// An inactive linker leaves the folder alone.
75577597
std::fs::write(
75587598
root.join(".yarnrc.yml"),

‎crates/socket-patch-core/src/crawlers/pkg_managers.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,8 +392,8 @@ pub(crate) fn yarn_pnpm_store_folder_in(project_root: &Path, env: &YarnEnv) -> O
392392
}
393393
let from_rc = |dir: &Path| {
394394
let rc = crate::utils::fs::read_regular_to_string_sync(&dir.join(&env.rc_filename)).ok()?;
395+
// An empty value is still the nearest setting: it admits nothing.
395396
crate::formats::yarn::rc::raw_setting(&rc, "pnpmStoreFolder")
396-
.filter(|raw| !raw.is_empty())
397397
.map(|raw| expand(&raw).map(|value| dir.join(value)))
398398
};
399399
let found = start
@@ -860,6 +860,8 @@ mod tests {
860860
assert_eq!(store(&env), Some(member.join("x")), "fallback");
861861
std::fs::write(member.join(".yarnrc.yml"), "pnpmStoreFolder: \"${S}\"\n").unwrap();
862862
assert_eq!(store(&env), None, "unset variable");
863+
std::fs::write(member.join(".yarnrc.yml"), "pnpmStoreFolder: ''\n").unwrap();
864+
assert_eq!(store(&env), None, "empty nearest setting");
863865
env.pnpm_store_folder = Some("from-env".into());
864866
assert_eq!(store(&env), Some(member.join("from-env")), "environment");
865867
}

0 commit comments

Comments
 (0)