Skip to content

compile: zero-config auto-compile makes root-first resolution shadow nested package versions vs Node — resolution-order decision needed (#7137/#7195 follow-up) #7197

Closed as not planned

Description

@proggeramlug

Deferred decision from PR #7195's post-merge verification of #7137 (zero-config auto-compile):

For any package in compile_packages, resolve_import searches the project root before the importer's own ancestors — a deliberate inversion (top-level ESM beating nested CJS) that was narrow while the set held only hand-listed names. #7137's auto-compile puts the entire reachable dependency graph in that set, so the inversion now applies to every bare specifier in every project.

Measured divergence vs Node 26.5.1 ([email protected] top-level + [email protected] nested under a dependency):

top: child:
node A-top-level-v1 B-nested-v2
perry A-top-level-v1 A-top-level-v1 — silent version shadowing

#7195 (merged) added a report when this fires (zero false positives across eight other fixtures) — but the underlying decision is open:

  1. Match Node (ancestor-first for auto-compiled packages, keep root-first only for explicitly-listed ones)?
  2. Keep root-first and rely on the report (simpler mental model, but silently un-Node for nested version conflicts — common in large trees)?
  3. Something version-aware?

Real npm trees hit nested duplicates constantly, and a wrong-version import is the silent-wrong-answer class. The report makes it visible; this issue is for deciding the semantics. Context: the root-first rationale is documented at the resolve_import site; #7137's compile_package_dirs dedup interacts (first-resolved wins and is cached).

Activity

  1. proggeramlug commented on Aug 10, 2026

    @proggeramlug
    ContributorAuthor

    Reproduced on main @ db44b31b7, then tested option 1 directly. Option 1 as written does not fix the divergence, which reframes the decision this issue is asking for.

    The divergence still reproduces

    Built the fixture ([email protected] top-level, [email protected] nested under holder), perry-dev, PERRY_NO_AUTO_OPTIMIZE=1, compilePackages: "auto":

    top: child:
    node 26.5.1 A-top-level-v1 B-nested-v2
    perry A-top-level-v1 A-top-level-v1

    and #7195's report fires exactly once, naming both copies correctly.

    Option 1, measured

    I removed the search-order inversion outright — the strongest form of option 1, Node order for every package, not just auto-compiled ones:

    -    let search_paths = if compile_packages.contains(&package_name) {
    -        [Some(project_root), importer_path.parent()]
    -    } else {
    -        [importer_path.parent(), Some(project_root)]
    -    };
    +    let search_paths = [importer_path.parent(), Some(project_root)];

    Result: unchanged. child: is still A-top-level-v1. Swapping the import order in the entry module (so holder resolves before the top-level dup-pkg) is also unchanged.

    Why — and what the real decision is

    The build reports Found 3 module(s). Node's semantics need four: two distinct dup-pkg module instances. #7195's own warning text already states the binding constraint —

    dup-pkg is installed at two different versions and Perry compiles ONE copy per package name

    So the search order is not what routes this import; the name-keyed compile_package_dirs dedup is, and it is downstream of whatever order the search produced. Matching Node is therefore not a resolution-order choice at all — it requires compiling two copies of one package name into one binary, with distinct module prefixes and distinct symbols, and the dedup exists precisely to prevent that (duplicate symbols from identical copies).

    That makes the three options in this issue not comparable as written:

    • Option 1 (ancestor-first) — measured above, changes nothing on its own. It would only matter after per-copy compilation exists, to decide which copy each importer gets.
    • Option 2 (keep root-first + report) — is what ships today, and the report is accurate and correctly scoped (fires once, names both copies, zero false positives on this repo's own flat tree, where it correctly stays silent).
    • Option 3 (version-aware) — is the same feature as "compile two copies", since version-awareness without per-copy compilation can only pick a different single winner.

    So the decision is really: is "two versions of one package in one binary" a capability Perry wants? If yes, that is the work, and the resolution order becomes a detail inside it. If no, option 2 is already the answer and this issue can close as decided — with the report as the mitigation, which is what it is doing.

    I have not implemented per-copy compilation: it touches module naming, symbol prefixes, the object cache key and the dedup that exists to keep the link clean, and it is a capability decision rather than a bug fix.

    Correcting one thing in the issue body for the next reader: it lists the two mechanisms as jointly responsible, which is right, but the fix shape it proposes only addresses the first, and the first turns out not to bind.

  2. proggeramlug commented on Aug 13, 2026

    @proggeramlug
    ContributorAuthor

    Validated again on current main @ 1b53332f8.

    Closing this as the semantics decision, not as a claim that Perry now matches Node for nested duplicate versions.

    The earlier audit's conclusion still holds:

    • Changing search_paths to ancestor-first cannot fix the reproducer. compile_package_dirs is still a HashMap<String, PathBuf>, and resolve_import redirects every copy of a package name through that one chosen directory. Current main's resolver/identity code is unchanged from the db44b31b7 full-fixture reproduction; the only later change in the inspected config/resolver slice is unrelated update configuration.
    • The shipped contract is therefore option 2: Perry compiles one copy per package name, deduplicating nested installs to avoid duplicate linker symbols, and fix(compile): keep the zero-config default additive and surface version-shadowed resolution #7195 warns when the selected copy's declared version differs from the copy Node would choose. The warning names both versions, both paths, and the redirected importer, and points to npm dedupe / overrides / explicit compilePackages scoping.
    • This is also the documented architecture: CLAUDE.md states that the first-resolved directory is cached in compile_package_dirs, and the user docs describe nested node_modules dedup.
    • No later PR changed package identity, and no separate multiversion-package tracker exists.
    • Re-ran commands::compile::resolve::dedup_version_tests on current main in a resolver-only non-inprocess test build: 6 passed, 0 failed. (The ordinary Windows link first hit the known official-LLVM packaging failure windows: perry.exe cannot link against the official LLVM 22 release — /MT vs /MD, bundled rpmalloc, and missing inkwell target backends #7985 before tests could execute; disabling the unrelated in-process LLVM feature for this resolver-only run avoids that host issue.)

    Supporting two versions of one package in one binary is not a resolution-order bug fix. It needs path/version-keyed package identities plus distinct module/symbol prefixes, object-cache keys, and importer routing. If Perry wants that capability later, it should be opened as a scoped feature/RFC with those requirements; reopening this search-order decision would understate the work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions