diff --git a/CLAUDE.md b/CLAUDE.md index d33709d12c..836816ae11 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -186,7 +186,10 @@ Configured in `package.json`: ```json { "perry": { "compilePackages": ["@noble/curves", "@noble/hashes"] } } ``` -First-resolved directory cached in `compile_package_dirs`; subsequent imports redirect to the same copy (dedup). +Bare imports resolve from the importer outward, matching Node/Bun. Every canonical +package root reached through `compilePackages` is tracked as a separate instance, +so nested versions compile with distinct module/symbol identities; multiple links +to the same physical root still deduplicate by canonical path. ## Known Limitations diff --git a/changelog.d/8533-package-instance-identity.md b/changelog.d/8533-package-instance-identity.md new file mode 100644 index 0000000000..aad4c1942c --- /dev/null +++ b/changelog.d/8533-package-instance-identity.md @@ -0,0 +1,7 @@ +**Compiler: preserve installed package-instance identity during native dependency compilation (#8516).** + +Perry previously keyed `compilePackages` discovery by package name. Once one copy of a package had been found, every importer was redirected to that directory—even when normal Node/Bun resolution selected a different nested installation. A dependency tree containing `dup-pkg@1` at the root and `dup-pkg@2` beneath another package therefore compiled one copy and silently bound both importers to it. + +Bare package resolution now walks outward from each importer, and the compiler tracks every canonical package root in a deterministic set. Distinct installed roots receive their existing path-derived module, linker-symbol, and object-cache identities; multiple links to the same physical root still deduplicate naturally. Native-package routing remains an explicit package policy and native-addon checks now run for every resolved package instance. + +The regression suite links and runs a four-module two-version fixture and verifies `top-v1 nested-v2`, plus resolver/cache tests that cover discovery-order independence. diff --git a/crates/perry/src/commands/compile/collect_modules.rs b/crates/perry/src/commands/compile/collect_modules.rs index 4e07034d55..abd27ca669 100644 --- a/crates/perry/src/commands/compile/collect_modules.rs +++ b/crates/perry/src/commands/compile/collect_modules.rs @@ -182,7 +182,7 @@ fn collect_module_one( let is_perry_native = is_in_node_modules && is_in_perry_native_package(&canonical); let is_in_compiled_pkg = ctx.aot_discovered_modules.contains(&canonical) || (is_in_node_modules && is_in_compile_package(&canonical, &ctx.compile_packages)) - || ctx.compile_package_dirs.values().any(|dir| { + || ctx.compile_package_dirs.iter().any(|dir| { if canonical.starts_with(dir) { // Exclude nested node_modules/ inside the compiled package // (e.g., @solana/web3.js/node_modules/bs58/ is NOT part of @solana/web3.js) @@ -1257,26 +1257,25 @@ fn collect_module_one( match kind { ModuleKind::NativeCompiled => { - // Record compile package directory for dedup (first-found wins). - // When the same package exists in multiple nested node_modules/, - // we always resolve to the first-found copy to avoid duplicate symbols. + // Record every resolved compile-package root. Package + // identity is the canonical root, not the package name: + // nested versions remain distinct while two symlinks to + // the same physical copy canonicalize together. let module_name = &import.source; if !module_name.starts_with('.') && !module_name.starts_with('/') { let (pkg_name, _) = parse_package_specifier(module_name); - if ctx.compile_packages.contains(&pkg_name) - && !ctx.compile_package_dirs.contains_key(&pkg_name) - { + if ctx.compile_packages.contains(&pkg_name) { if let Some(pkg_dir) = extract_compile_package_dir(&resolved_path, &pkg_name) { - ctx.compile_package_dirs.insert(pkg_name, pkg_dir); + ctx.compile_package_dirs.insert(pkg_dir); } else { // Symlinked local package: canonical path is outside node_modules. // Walk up from resolved_path to find the package root (dir with package.json). let mut dir = resolved_path.parent(); while let Some(d) = dir { if d.join("package.json").exists() { - ctx.compile_package_dirs.insert(pkg_name, d.to_path_buf()); + ctx.compile_package_dirs.insert(d.to_path_buf()); break; } dir = d.parent(); diff --git a/crates/perry/src/commands/compile/collect_modules/native_addon.rs b/crates/perry/src/commands/compile/collect_modules/native_addon.rs index b79d7291e5..de1e880894 100644 --- a/crates/perry/src/commands/compile/collect_modules/native_addon.rs +++ b/crates/perry/src/commands/compile/collect_modules/native_addon.rs @@ -33,7 +33,7 @@ fn package_root_for_compile_package( path: &std::path::Path, ) -> Option { ctx.compile_package_dirs - .values() + .iter() .filter(|dir| path.starts_with(dir)) .max_by_key(|dir| dir.components().count()) .cloned() diff --git a/crates/perry/src/commands/compile/collect_modules/tests.rs b/crates/perry/src/commands/compile/collect_modules/tests.rs index 7c5c71d160..315789d238 100644 --- a/crates/perry/src/commands/compile/collect_modules/tests.rs +++ b/crates/perry/src/commands/compile/collect_modules/tests.rs @@ -508,7 +508,6 @@ fn guard_compile_package( let mut ctx = CompilationContext::new(root.to_path_buf()); ctx.compile_packages.insert(package_name.to_string()); ctx.compile_package_dirs.insert( - package_name.to_string(), root.join("node_modules") .join(package_name) .canonicalize() diff --git a/crates/perry/src/commands/compile/resolve.rs b/crates/perry/src/commands/compile/resolve.rs index fcdc50590a..75a14ab132 100644 --- a/crates/perry/src/commands/compile/resolve.rs +++ b/crates/perry/src/commands/compile/resolve.rs @@ -29,7 +29,7 @@ use anyhow::{anyhow, Result}; use perry_hir::ModuleKind; -use std::collections::{HashMap, HashSet}; +use std::collections::{BTreeSet, HashMap, HashSet}; use std::fs; use std::path::{Component, Path, PathBuf}; use std::sync::{Mutex, OnceLock}; @@ -131,10 +131,10 @@ mod bun_store_tests; #[cfg(test)] mod declaration_map_source_tests; #[cfg(test)] -mod dedup_version_tests; -#[cfg(test)] mod extension_resolution_tests; #[cfg(test)] +mod package_instance_tests; +#[cfg(test)] mod tests; /// Packages that Perry provides built-in native extensions for. @@ -192,83 +192,6 @@ pub(super) fn extract_compile_package_dir( .map(Path::to_path_buf) } -/// Read a package directory's declared `version`, if it has a readable -/// `package.json` with a string `version` field. -fn package_json_version(package_dir: &Path) -> Option { - let raw = fs::read_to_string(package_dir.join("package.json")).ok()?; - let value: serde_json::Value = serde_json::from_str(&raw).ok()?; - value - .get("version") - .and_then(|v| v.as_str()) - .map(str::to_string) -} - -/// Whether `chosen` is a *different* copy from `found` with a *different* -/// declared version. Identical versions are a genuine duplicate install and -/// collapsing them is intended; differing versions mean the build silently -/// dropped one of them. -pub(super) fn dedup_collapses_distinct_versions(chosen: &Path, found: &Path) -> bool { - if chosen == found { - return false; - } - match (package_json_version(chosen), package_json_version(found)) { - (Some(a), Some(b)) => a != b, - // A copy with no readable version can't be proven distinct; stay - // quiet rather than warning on every unversioned local link. - _ => false, - } -} - -/// The copy plain Node resolution would have picked for `package_name` as -/// imported from `importer_path`: the nearest ancestor `node_modules` that -/// holds the package. Perry's compile-package path deliberately searches the -/// project root first instead (see `search_paths` in `resolve_import`), so -/// the two can disagree. -pub(super) fn node_nearest_package_dir( - package_name: &str, - importer_path: &Path, -) -> Option { - let start = importer_path.parent().unwrap_or(importer_path); - ancestor_node_modules_dirs(start) - .into_iter() - .map(|node_modules| node_modules.join(package_name)) - .find(|candidate| candidate.is_dir()) -} - -/// Warn, at most once per package, when the compile-package resolution path -/// hands an importer a different *version* than Node would have. Covers both -/// the root-first search order and the `compile_package_dirs` first-found -/// dedup, since `chosen` is the directory actually used. -fn warn_on_version_shadowed_resolution(package_name: &str, chosen: &Path, importer_path: &Path) { - static WARNED: OnceLock>> = OnceLock::new(); - let Some(nearest) = node_nearest_package_dir(package_name, importer_path) else { - return; - }; - if !dedup_collapses_distinct_versions(chosen, &nearest) { - return; - } - let warned = WARNED.get_or_init(|| Mutex::new(HashSet::new())); - let Ok(mut warned) = warned.lock() else { - return; - }; - if !warned.insert(package_name.to_string()) { - return; - } - let chosen_version = package_json_version(chosen).unwrap_or_else(|| "?".to_string()); - let nearest_version = package_json_version(&nearest).unwrap_or_else(|| "?".to_string()); - eprintln!( - " warning: `{package_name}` is installed at two different versions and \ - Perry compiles ONE copy per package name. `{importer}` gets \ - {chosen_version} (from {chosen}); Node would have given it \ - {nearest_version} (from {nearest}). Deduplicate the dependency (npm \ - dedupe / a package override), or list the package explicitly in \ - `perry.compilePackages` only where you want it compiled.", - importer = importer_path.display(), - chosen = chosen.display(), - nearest = nearest.display(), - ); -} - /// Check if a file path is inside a package listed in compile_packages pub(super) fn is_in_compile_package(path: &Path, compile_packages: &HashSet) -> bool { compile_packages.iter().any(|pkg_name| { @@ -1432,7 +1355,7 @@ pub(super) fn resolve_import( importer_path: &Path, project_root: &Path, compile_packages: &HashSet, - compile_package_dirs: &HashMap, + compile_package_dirs: &BTreeSet, ) -> Option<(PathBuf, ModuleKind)> { // Check if it's a native Rust stdlib module. Refs #665: when the user has // explicitly opted the package into `perry.compilePackages`, they want @@ -1509,7 +1432,7 @@ pub(super) fn resolve_import( // dependent file never enters `ctx.native_modules`, and importing // modules see `imported_classes=[]` for symbols re-exported from it. let in_compile_pkg = is_in_compile_package(&canonical, compile_packages) - || compile_package_dirs.values().any(|dir| { + || compile_package_dirs.iter().any(|dir| { if canonical.starts_with(dir) { let relative = canonical.strip_prefix(dir).unwrap_or(canonical.as_path()); !relative.to_string_lossy().contains("node_modules/") @@ -1554,14 +1477,12 @@ pub(super) fn resolve_import( // Handle node_modules (bare specifiers) let (package_name, subpath) = parse_package_specifier(import_source); - // For compile_packages, search project root first to prefer ESM versions - // over nested CJS copies (e.g., @solana/web3.js/node_modules/bs58 is CJS, - // but the top-level node_modules/bs58 has ESM support) - let search_paths = if compile_packages.contains(&package_name) { - [Some(project_root), importer_path.parent()] - } else { - [importer_path.parent(), Some(project_root)] - }; + // Bare package imports are importer-relative. Start at the importing + // module and walk its ancestors exactly as Node/Bun do; the project root + // remains an additive fallback for lexical/canonical path edge cases. + // A compilePackages opt-in controls how the resolved copy is compiled, not + // which installed copy wins resolution. + let search_paths = [importer_path.parent(), Some(project_root)]; for start in search_paths.iter().flatten() { for node_modules in ancestor_node_modules_dirs(start) { @@ -1583,40 +1504,23 @@ pub(super) fn resolve_import( } // Packages listed in perry.compilePackages are compiled natively if compile_packages.contains(&package_name) { - // Deduplicate: if we've already resolved this package from a - // different node_modules location, use the first-found directory - // to avoid duplicate symbols from identical package copies - let effective_dir = compile_package_dirs - .get(&package_name) - .unwrap_or(&package_dir); - // #7137 follow-up. Two mechanisms route this import away from - // the copy Node would have used: the root-first `search_paths` - // order just above (chosen for compile packages so a top-level - // ESM copy beats a nested CJS one), and this first-found - // `compile_package_dirs` dedup. Both were narrow while - // `compile_packages` held only hand-listed names — opting a - // package in was a deliberate act. The auto-compile default - // puts the WHOLE reachable graph in that set, so both now apply - // to every bare specifier in the project, and a tree carrying - // two majors of one package silently gets one of them. Report - // it when the versions actually differ. - warn_on_version_shadowed_resolution(&package_name, effective_dir, importer_path); // Prefer TypeScript source over compiled JS if let Some(src_entry) = - resolve_package_source_entry(effective_dir, subpath.as_deref()) + resolve_package_source_entry(&package_dir, subpath.as_deref()) { return Some((src_entry.canonicalize().ok()?, ModuleKind::NativeCompiled)); } // Fall back to normal resolution but still mark as NativeCompiled if let Some(fallback_entry) = - resolve_package_entry(effective_dir, subpath.as_deref()) + resolve_package_entry(&package_dir, subpath.as_deref()) { return Some(( fallback_entry.canonicalize().ok()?, ModuleKind::NativeCompiled, )); } - // If effective_dir failed (shouldn't happen), try the local dir + // The entry resolved above is the same package instance and is + // kept as the final fallback for unusual package metadata. return Some((entry.canonicalize().ok()?, ModuleKind::NativeCompiled)); } // For other node_modules packages, classify by file @@ -1704,7 +1608,7 @@ pub(super) fn resolve_import( if let Some(canonical) = tsconfig_paths::resolve_tsconfig_paths(import_source, importer_path) { let in_compile_pkg = is_in_compile_package(&canonical, compile_packages) || compile_package_dirs - .values() + .iter() .any(|dir| canonical.starts_with(dir)); let in_node_modules = canonical.to_string_lossy().contains("node_modules"); let kind = if is_js_file(&canonical) && !in_compile_pkg && in_node_modules { diff --git a/crates/perry/src/commands/compile/resolve/dedup_version_tests.rs b/crates/perry/src/commands/compile/resolve/dedup_version_tests.rs deleted file mode 100644 index 7b9740b175..0000000000 --- a/crates/perry/src/commands/compile/resolve/dedup_version_tests.rs +++ /dev/null @@ -1,118 +0,0 @@ -use super::*; - -/// #7137 follow-up. Compile-package dedup keeps one directory per package -/// name and routes every importer to it. That is right for a genuine -/// duplicate install and wrong-but-silent when the two copies are different -/// versions — which auto-compile made reachable for a project's entire -/// dependency graph rather than only for hand-listed packages. -/// -/// `dedup_collapses_distinct_versions` is the predicate behind the warning, -/// so it has to be true exactly in the lossy case. -fn write_pkg(dir: &std::path::Path, version: &str) { - std::fs::create_dir_all(dir).expect("mkdir"); - std::fs::write( - dir.join("package.json"), - format!(r#"{{"name":"dup-pkg","version":"{version}"}}"#), - ) - .expect("write package.json"); -} - -#[test] -fn differing_versions_are_reported_as_collapsing() { - let root = tempfile::tempdir().expect("tempdir"); - let chosen = root.path().join("node_modules/dup-pkg"); - let found = root.path().join("sub/node_modules/dup-pkg"); - write_pkg(&chosen, "1.0.0"); - write_pkg(&found, "2.0.0"); - - assert!( - dedup_collapses_distinct_versions(&chosen, &found), - "1.0.0 substituted for 2.0.0 is a silent loss and must be reported" - ); -} - -#[test] -fn identical_versions_are_a_plain_duplicate_install() { - let root = tempfile::tempdir().expect("tempdir"); - let chosen = root.path().join("node_modules/dup-pkg"); - let found = root.path().join("sub/node_modules/dup-pkg"); - write_pkg(&chosen, "1.0.0"); - write_pkg(&found, "1.0.0"); - - assert!( - !dedup_collapses_distinct_versions(&chosen, &found), - "collapsing two copies of the same version is the intended dedup" - ); -} - -#[test] -fn same_directory_is_never_a_collapse() { - let root = tempfile::tempdir().expect("tempdir"); - let only = root.path().join("node_modules/dup-pkg"); - write_pkg(&only, "1.0.0"); - - assert!( - !dedup_collapses_distinct_versions(&only, &only), - "the first copy resolving to itself is not a substitution" - ); -} - -/// A copy with no readable `package.json` cannot be proven distinct — a -/// local symlinked workspace package often has no version at the resolved -/// path. Warning there would be noise on every such build. -#[test] -fn unreadable_version_stays_quiet() { - let root = tempfile::tempdir().expect("tempdir"); - let chosen = root.path().join("node_modules/dup-pkg"); - let found = root.path().join("linked/dup-pkg"); - write_pkg(&chosen, "1.0.0"); - std::fs::create_dir_all(&found).expect("mkdir"); - - assert!( - !dedup_collapses_distinct_versions(&chosen, &found), - "an unversioned copy must not produce a warning" - ); -} - -/// The shadowing is not (only) the `compile_package_dirs` dedup: for a -/// package in `compile_packages`, `resolve_import` searches the PROJECT ROOT -/// before the importer's own ancestors. So a nested copy is passed over even -/// on its first resolution. `node_nearest_package_dir` is what Node would -/// have picked, and is what the warning compares against. -#[test] -fn nearest_dir_is_the_importers_own_node_modules() { - let root = tempfile::tempdir().expect("tempdir"); - let top = root.path().join("node_modules/dup-pkg"); - let nested = root.path().join("sub/node_modules/dup-pkg"); - write_pkg(&top, "1.0.0"); - write_pkg(&nested, "2.0.0"); - let importer = root.path().join("sub/child.ts"); - std::fs::write(&importer, "export {};\n").expect("write importer"); - - let nearest = node_nearest_package_dir("dup-pkg", &importer).expect("nearest copy found"); - assert_eq!( - nearest, nested, - "Node resolves a bare specifier from the importer's nearest node_modules" - ); - assert!( - dedup_collapses_distinct_versions(&top, &nearest), - "compiling the root 1.0.0 for an importer Node would give 2.0.0 is a \ - silent version substitution" - ); -} - -/// An importer with no nearer copy resolves to the same directory Perry -/// chose — nothing was shadowed, so nothing is reported. -#[test] -fn importer_without_a_nearer_copy_is_not_shadowed() { - let root = tempfile::tempdir().expect("tempdir"); - let top = root.path().join("node_modules/dup-pkg"); - write_pkg(&top, "1.0.0"); - let importer = root.path().join("sub/child.ts"); - std::fs::create_dir_all(importer.parent().unwrap()).expect("mkdir"); - std::fs::write(&importer, "export {};\n").expect("write importer"); - - let nearest = node_nearest_package_dir("dup-pkg", &importer).expect("root copy found"); - assert_eq!(nearest, top); - assert!(!dedup_collapses_distinct_versions(&top, &nearest)); -} diff --git a/crates/perry/src/commands/compile/resolve/package_instance_tests.rs b/crates/perry/src/commands/compile/resolve/package_instance_tests.rs new file mode 100644 index 0000000000..6681e74af8 --- /dev/null +++ b/crates/perry/src/commands/compile/resolve/package_instance_tests.rs @@ -0,0 +1,108 @@ +use super::*; + +fn write_pkg(dir: &Path, version: &str, marker: &str) { + std::fs::create_dir_all(dir).expect("mkdir package"); + std::fs::write( + dir.join("package.json"), + format!(r#"{{"name":"dup-pkg","version":"{version}","main":"index.js"}}"#), + ) + .expect("write package.json"); + std::fs::write( + dir.join("index.js"), + format!("export default '{marker}';\n"), + ) + .expect("write package entry"); +} + +fn two_version_fixture(root: &Path) -> (PathBuf, PathBuf, PathBuf, PathBuf) { + let top = root.join("node_modules/dup-pkg"); + let holder = root.join("node_modules/holder"); + let nested = holder.join("node_modules/dup-pkg"); + write_pkg(&top, "1.0.0", "top-v1"); + write_pkg(&nested, "2.0.0", "nested-v2"); + std::fs::create_dir_all(&holder).expect("mkdir holder"); + + let root_importer = root.join("main.ts"); + let nested_importer = holder.join("index.js"); + std::fs::write(&root_importer, "export {};\n").expect("write root importer"); + std::fs::write(&nested_importer, "export {};\n").expect("write nested importer"); + (top, nested, root_importer, nested_importer) +} + +#[test] +fn compile_package_resolution_preserves_importer_relative_instances() { + let fixture = tempfile::tempdir().expect("tempdir"); + let root = fixture.path(); + let (top, nested, root_importer, nested_importer) = two_version_fixture(root); + let compile_packages = HashSet::from(["dup-pkg".to_string()]); + + // Model the real collection order: the top-level copy has already been + // discovered and recorded before the nested importer is resolved. + let mut package_roots = BTreeSet::new(); + package_roots.insert(top.canonicalize().expect("canonical top package")); + + let (top_entry, top_kind) = resolve_import( + "dup-pkg", + &root_importer, + root, + &compile_packages, + &package_roots, + ) + .expect("resolve top-level package"); + let (nested_entry, nested_kind) = resolve_import( + "dup-pkg", + &nested_importer, + root, + &compile_packages, + &package_roots, + ) + .expect("resolve nested package"); + + assert_eq!(top_kind, ModuleKind::NativeCompiled); + assert_eq!(nested_kind, ModuleKind::NativeCompiled); + assert_eq!(top_entry, top.join("index.js").canonicalize().unwrap()); + assert_eq!( + nested_entry, + nested.join("index.js").canonicalize().unwrap(), + "a previously discovered copy with the same package name must not redirect this importer" + ); + assert_ne!(top_entry, nested_entry); +} + +#[test] +fn resolve_cache_keys_the_same_specifier_by_importer_directory() { + let fixture = tempfile::tempdir().expect("tempdir"); + let root = fixture.path(); + let (top, nested, root_importer, nested_importer) = two_version_fixture(root); + let mut ctx = CompilationContext::new(root.to_path_buf()); + ctx.compile_packages.insert("dup-pkg".to_string()); + ctx.compile_package_dirs + .insert(top.canonicalize().expect("canonical top package")); + + let (top_entry, _) = + cached_resolve_import("dup-pkg", &root_importer, &mut ctx).expect("top resolution"); + let (nested_entry, _) = + cached_resolve_import("dup-pkg", &nested_importer, &mut ctx).expect("nested resolution"); + + assert_eq!(top_entry, top.join("index.js").canonicalize().unwrap()); + assert_eq!( + nested_entry, + nested.join("index.js").canonicalize().unwrap() + ); + assert_eq!(ctx.resolve_cache.len(), 2); +} + +#[test] +fn package_root_identity_deduplicates_only_the_same_canonical_instance() { + let fixture = tempfile::tempdir().expect("tempdir"); + let root = fixture.path(); + let (top, nested, _, _) = two_version_fixture(root); + let top = top.canonicalize().unwrap(); + let nested = nested.canonicalize().unwrap(); + let mut roots = BTreeSet::new(); + + assert!(roots.insert(top.clone())); + assert!(!roots.insert(top)); + assert!(roots.insert(nested)); + assert_eq!(roots.len(), 2); +} diff --git a/crates/perry/src/commands/compile/resolve/tests.rs b/crates/perry/src/commands/compile/resolve/tests.rs index 95ed9c0792..1edbe85023 100644 --- a/crates/perry/src/commands/compile/resolve/tests.rs +++ b/crates/perry/src/commands/compile/resolve/tests.rs @@ -1450,7 +1450,7 @@ mod module_spec_tests { #[cfg(test)] mod declaration_sidecar_tests { use super::*; - use std::collections::{HashMap, HashSet}; + use std::collections::HashSet; mod compile_package; @@ -1511,7 +1511,7 @@ mod declaration_sidecar_tests { &importer, root, &HashSet::new(), - &HashMap::new(), + &BTreeSet::new(), ) .expect("resolve typed-js"); @@ -1544,7 +1544,7 @@ mod declaration_sidecar_tests { &importer, root, &compile_packages, - &HashMap::new(), + &BTreeSet::new(), ) .expect("resolve typed-js"); @@ -1644,7 +1644,7 @@ mod declaration_sidecar_tests { /// (ink's `` rendered `[object Object]`). mod subpath_imports_tests { use super::super::{resolve_import, ModuleKind}; - use std::collections::{HashMap, HashSet}; + use std::collections::{BTreeSet, HashSet}; use std::path::PathBuf; fn write_chalk_like_package(root: &std::path::Path) -> PathBuf { @@ -1694,7 +1694,7 @@ mod subpath_imports_tests { let importer = pkg.join("source/index.js"); let compile_packages: HashSet = ["chalky".to_string()].into_iter().collect(); - let compile_package_dirs: HashMap = HashMap::new(); + let compile_package_dirs: BTreeSet = BTreeSet::new(); let (resolved, kind) = resolve_import( "#ansi-styles", @@ -1722,7 +1722,7 @@ mod subpath_imports_tests { let importer = pkg.join("source/index.js"); let compile_packages: HashSet = ["chalky".to_string()].into_iter().collect(); - let compile_package_dirs: HashMap = HashMap::new(); + let compile_package_dirs: BTreeSet = BTreeSet::new(); let (resolved, _) = resolve_import( "#supports-color", @@ -1749,7 +1749,7 @@ mod subpath_imports_tests { let importer = pkg.join("source/index.js"); assert!( - resolve_import("#nope", &importer, root, &HashSet::new(), &HashMap::new(),).is_none(), + resolve_import("#nope", &importer, root, &HashSet::new(), &BTreeSet::new(),).is_none(), "a `#` specifier missing from the imports map must not resolve" ); } diff --git a/crates/perry/src/commands/compile/resolve/tests/declaration_sidecar_tests/compile_package.rs b/crates/perry/src/commands/compile/resolve/tests/declaration_sidecar_tests/compile_package.rs index e813826dd1..70201ed6b3 100644 --- a/crates/perry/src/commands/compile/resolve/tests/declaration_sidecar_tests/compile_package.rs +++ b/crates/perry/src/commands/compile/resolve/tests/declaration_sidecar_tests/compile_package.rs @@ -40,7 +40,7 @@ fn subpath_exports_do_not_fall_back_to_src_index() { &importer, root, &compile_packages, - &HashMap::new(), + &BTreeSet::new(), ) .expect("resolve pkg/feature"); diff --git a/crates/perry/src/commands/compile/run_pipeline.rs b/crates/perry/src/commands/compile/run_pipeline.rs index 3d48f0c155..8061f5ea55 100644 --- a/crates/perry/src/commands/compile/run_pipeline.rs +++ b/crates/perry/src/commands/compile/run_pipeline.rs @@ -2578,7 +2578,7 @@ pub fn run_with_parse_cache( // with MODULE_NOT_FOUND even though it was compiled. || ctx .compile_package_dirs - .values() + .iter() .any(|dir| p.starts_with(dir)) }) .map(|(p, m)| { diff --git a/crates/perry/src/commands/compile/types.rs b/crates/perry/src/commands/compile/types.rs index 2468c4d45d..f18753ccaf 100644 --- a/crates/perry/src/commands/compile/types.rs +++ b/crates/perry/src/commands/compile/types.rs @@ -666,8 +666,10 @@ pub struct CompilationContext { /// from `perry.toml` + CLI overrides and reused by every codegen /// backend so native, JS and arkts agree byte-for-byte. pub app_metadata: perry_codegen::AppMetadata, - /// First-resolved directory for each compile package (deduplication across nested node_modules) - pub compile_package_dirs: HashMap, + /// Canonical roots of every resolved compile-package instance. A sorted + /// set keeps cache/native-addon classification deterministic while allowing + /// nested installations of the same package name to remain distinct. + pub compile_package_dirs: BTreeSet, /// Compile package roots already checked for unsupported Node native addon markers. pub checked_compile_package_native_addon_roots: HashSet, /// #1680 (Phase 2 of #1677): build-time codegen steps declared in the @@ -1131,7 +1133,7 @@ impl CompilationContext { fast_math: false, fp_contract_mode: perry_codegen::FpContractMode::Off, app_metadata: perry_codegen::AppMetadata::default(), - compile_package_dirs: HashMap::new(), + compile_package_dirs: BTreeSet::new(), checked_compile_package_native_addon_roots: HashSet::new(), codegen_steps: Vec::new(), codegen_dir: None, diff --git a/crates/perry/tests/issue_8516_package_instance_identity.rs b/crates/perry/tests/issue_8516_package_instance_identity.rs new file mode 100644 index 0000000000..082d2371cf --- /dev/null +++ b/crates/perry/tests/issue_8516_package_instance_identity.rs @@ -0,0 +1,150 @@ +//! Regression for #8516: two installed versions of one package must remain +//! separate native modules and resolve relative to their respective importers. + +use std::path::{Path, PathBuf}; +use std::process::Command; +use std::sync::Once; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn workspace_root() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("../..") + .canonicalize() + .expect("canonicalize workspace root") +} + +fn target_debug_dir() -> PathBuf { + let target = std::env::var_os("CARGO_TARGET_DIR") + .map(PathBuf::from) + .unwrap_or_else(|| workspace_root().join("target")); + if cfg!(windows) { + target.join("x86_64-pc-windows-msvc").join("debug") + } else { + target.join("debug") + } +} + +fn ensure_runtime_archive() { + static BUILD_RUNTIME: Once = Once::new(); + BUILD_RUNTIME.call_once(|| { + let cargo = std::env::var_os("CARGO").unwrap_or_else(|| "cargo".into()); + let mut command = Command::new(cargo); + command + .current_dir(workspace_root()) + .arg("build") + .arg("-p") + .arg("perry-runtime-static") + .arg("-p") + .arg("perry-stdlib-static"); + if cfg!(windows) { + command.arg("--target").arg("x86_64-pc-windows-msvc"); + } + let build = command.output().expect("build static runtime archives"); + assert!( + build.status.success(), + "runtime archive build failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&build.stdout), + String::from_utf8_lossy(&build.stderr) + ); + }); +} + +fn write(path: &Path, source: &str) { + std::fs::create_dir_all(path.parent().expect("fixture parent")).expect("mkdir fixture"); + std::fs::write(path, source).expect("write fixture"); +} + +fn write_package(root: &Path, name: &str, version: &str, source: &str) { + write( + &root.join("package.json"), + &format!( + r#"{{"name":"{name}","version":"{version}","type":"module","exports":"./index.js"}}"# + ), + ); + write(&root.join("index.js"), source); +} + +#[test] +fn nested_versions_match_importer_relative_node_resolution() { + let fixture = tempfile::tempdir().expect("tempdir"); + let root = fixture.path(); + write( + &root.join("package.json"), + r#"{ + "name": "package-instance-fixture", + "type": "module", + "perry": { + "compilePackages": "auto", + "allow": { "compilePackages": "auto" } + } + }"#, + ); + write_package( + &root.join("node_modules/dup-pkg"), + "dup-pkg", + "1.0.0", + "export function identify() { return 'top-v1'; }\n", + ); + write_package( + &root.join("node_modules/holder/node_modules/dup-pkg"), + "dup-pkg", + "2.0.0", + "export function identify() { return 'nested-v2'; }\n", + ); + write_package( + &root.join("node_modules/holder"), + "holder", + "1.0.0", + "import { identify } from 'dup-pkg';\nexport function child() { return identify(); }\n", + ); + write( + &root.join("main.ts"), + "import { identify } from 'dup-pkg';\n\ + import { child } from 'holder';\n\ + console.log(identify(), child());\n", + ); + + ensure_runtime_archive(); + let binary = root.join(if cfg!(windows) { "main.exe" } else { "main" }); + let compile = Command::new(perry_bin()) + .current_dir(root) + .arg("compile") + .arg("main.ts") + .arg("--no-cache") + .arg("-o") + .arg(&binary) + .env("PERRY_NO_AUTO_OPTIMIZE", "1") + .env("PERRY_RUNTIME_DIR", target_debug_dir()) + .output() + .expect("compile fixture"); + assert!( + compile.status.success(), + "compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + let diagnostics = format!( + "{}{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + assert!( + !diagnostics.contains("ONE copy per package name"), + "the obsolete one-copy warning must be gone:\n{diagnostics}" + ); + + let run = Command::new(&binary) + .current_dir(root) + .output() + .expect("run compiled fixture"); + assert!( + run.status.success(), + "compiled fixture failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + assert_eq!(String::from_utf8_lossy(&run.stdout), "top-v1 nested-v2\n"); +} diff --git a/docs/src/packages/porting.md b/docs/src/packages/porting.md index 28a9fca703..6743b25905 100644 --- a/docs/src/packages/porting.md +++ b/docs/src/packages/porting.md @@ -70,7 +70,7 @@ In your project's `package.json`: } ``` -This is what tells Perry to pull the package into the native compile instead of routing it through a JavaScript runtime. See [Project Configuration](../getting-started/project-config.md#compilepackages) for the full semantics — including how first-resolved directories get cached so transitive copies dedup. +This is what tells Perry to pull the package into the native compile instead of routing it through a JavaScript runtime. See [Project Configuration](../getting-started/project-config.md#compilepackages) for the full semantics. Package resolution remains importer-relative: nested installations compile as separate module instances, while links that canonicalize to the same physical package root share one instance. ### 2. Try compiling