install: resolve file: and link: dependencies on the same directory as separate packages

src/install/PackageManager.rs

@@ -161,7 +161,10 @@ Learn more about these at <magenta>https://bun.com/docs/cli/pm<r>.
 
 use crate::lockfile_real::package as Package;
 use crate::package_manager_task as Task;
-use crate::resolvers::folder_resolver::{Entry as FolderResolutionEntry, FolderResolution};
+use crate::resolvers::folder_resolver::{
+    Entry as FolderResolutionEntry, FolderResolution, Key as FolderResolutionKey,
+    Kind as FolderResolutionKind,
+};
 use bun_install::lockfile::{self, Lockfile};
 use bun_install::{
     Dependency, DependencyID, NetworkTask, PackageID, PackageManifestMap,
@@ -252,8 +255,7 @@ type RepositoryMap = HashMap<Task::Id, Fd /* , IdentityContext<Task::Id>, 80 */>
 /// process.
 type AppendedTaskPackageMap =
     HashMap<Task::Id, PackageID /* , IdentityContext<Task::Id>, 80 */>;
-pub(crate) type FolderResolutionMap =
-    HashMap<u64, FolderResolutionEntry /* , IdentityContext<u64>, 80 */>;
+pub(crate) type FolderResolutionMap = HashMap<FolderResolutionKey, FolderResolutionEntry>;
 pub(crate) type NpmAliasMap =
     HashMap<PackageNameHash, crate::dependency::Version /* , IdentityContext<u64>, 80 */>;
 
@@ -2107,8 +2109,8 @@ pub fn init(
     {
         // make sure folder packages can find the root package without creating a new one
         // Posix-normalize the
-        // separators before hashing; `FolderResolution.hash` is always fed `/`-separated
-        // bytes by every resolver-side caller. On Windows `getFdPath` yields `\`, so
+        // separators before hashing; the folder resolver always builds its `Key` from
+        // `/`-separated bytes. On Windows `getFdPath` yields `\`, so
         // hashing the raw bytes would seed a key the resolver never looks up — copy into
         // a stack buffer and convert separators in place.
         // SAFETY: ROOT_PACKAGE_JSON_PATH set above on the main thread.
@@ -2119,7 +2121,7 @@ pub fn init(
         resolve_path::dangerously_convert_path_to_posix_in_place::<u8>(normalized);
         // SAFETY: singleton fully initialized; main thread, no workers yet.
         unsafe { &mut *manager_ptr }.folders.put(
-            crate::resolvers::folder_resolver::hash(normalized),
+            FolderResolutionKey::new(FolderResolutionKind::Folder, normalized),
             FolderResolutionEntry {
                 abs_path: Box::<[u8]>::from(&*normalized),
                 resolution: FolderResolution::PackageId(0),

src/install/resolvers/folder_resolver.rs

@@ -80,8 +80,46 @@ impl<'a> fmt::Display for PackageWorkspaceSearchPathFormatter<'a> {
     }
 }
 
+/// What a directory's `package.json` is read as. Each kind parses it with
+/// different `Features` and produces a different `Resolution`, so one
+/// directory is cached separately per kind: a `link:` and a `file:` pointing
+/// at the same directory are two packages, and whichever resolves first must
+/// not be handed out for the other.
+#[derive(Copy, Clone, PartialEq, Eq, Hash)]
+pub(crate) enum Kind {
+    /// A `file:` dependency or a workspace member (`Features::FOLDER` /
+    /// `Features::WORKSPACE`, dependencies installed). These share an entry
+    /// so a `file:` pointing at a workspace member, or at the root package
+    /// seeded by `PackageManager::init`, resolves to that existing package.
+    Folder,
+    /// A `link:` target (`Features::LINK`: its dependencies are not
+    /// installed, `Resolution::Symlink`).
+    Link,
+    /// An npm package already extracted into the cache (`Features::NPM`,
+    /// `Resolution::Npm`).
+    CacheFolder,
+}
+
+/// Key of the folder-resolution map.
+#[derive(Copy, Clone, PartialEq, Eq, Hash)]
+pub(crate) struct Key {
+    kind: Kind,
+    /// Hash of the normalized absolute `package.json` path; `Entry::abs_path`
+    /// holds the bytes it was computed from.
+    abs_hash: u64,
+}
+
+impl Key {
+    pub(crate) fn new(kind: Kind, normalized_abs_path: &[u8]) -> Key {
+        Key {
+            kind,
+            abs_hash: bun_wyhash::hash(normalized_abs_path),
+        }
+    }
+}
+
 /// Value stored in the folder-resolution map: the resolution plus the
-/// normalized absolute `package.json` path the key hash was computed from.
+/// normalized absolute `package.json` path `Key::abs_hash` was computed from.
 /// Lookups compare the path, since a different path whose hash collides must
 /// not reuse this resolution.
 pub struct Entry {
@@ -96,10 +134,6 @@ fn normalize(path: &[u8]) -> &[u8] {
     FileSystem::instance().normalize(path)
 }
 
-pub(crate) fn hash(normalized_path: &[u8]) -> u64 {
-    bun_wyhash::hash(normalized_path)
-}
-
 // ── NewResolver ───────────────────────────────────────────────────────────
 // The const-generic tag requires `#[derive(ConstParamTy)]` (already on `Tag`).
 struct NewResolver<'a, const TAG: ResolutionTag> {
@@ -377,6 +411,20 @@ pub enum GlobalOrRelative<'a> {
     CacheFolder(&'a [u8]),
 }
 
+impl GlobalOrRelative<'_> {
+    /// Must agree with the resolver `get_or_put` picks for each variant.
+    fn kind(self) -> Kind {
+        match self {
+            GlobalOrRelative::Global(_) => Kind::Link,
+            GlobalOrRelative::Relative(
+                dependency::version::Tag::Folder | dependency::version::Tag::Workspace,
+            ) => Kind::Folder,
+            GlobalOrRelative::Relative(_) => unreachable!(),
+            GlobalOrRelative::CacheFolder(_) => Kind::CacheFolder,
+        }
+    }
+}
+
 pub(crate) fn get_or_put(
     global_or_relative: GlobalOrRelative<'_>,
     version: &dependency::Version,
@@ -400,7 +448,7 @@ pub(crate) fn get_or_put(
         // `(&[u8]).as_ptr().cast_mut()` would be UB under Stacked/Tree
         // Borrows: those pointers carry read-only provenance, and the
         // optimizer may assume `abs`'s bytes are unchanged when computing
-        // `hash(abs.as_bytes())` below.
+        // the `Key` below.
         //
         // Instead: capture lengths, let the shared borrows of `joined` die,
         // then take a fresh `&mut joined[..abs_len]` (write provenance) and
@@ -421,13 +469,13 @@ pub(crate) fn get_or_put(
             &rel_buf[..rel_len],
         )
     };
-    let abs_hash = hash(abs.as_bytes());
+    let key = Key::new(global_or_relative.kind(), abs.as_bytes());
 
     // Check first, compute, then insert, because read_package_json_from_disk
     // needs &mut manager. Compare the stored path, not just its hash: a
     // different path whose hash collides must not reuse this resolution. On a
     // collision, resolve fresh without caching so the first path's entry stays.
-    let hash_collision = match manager.folders.get(&abs_hash) {
+    let hash_collision = match manager.folders.get(&key) {
         Some(existing) if *existing.abs_path == *abs.as_bytes() => return existing.resolution,
         Some(_) => true,
         None => false,
@@ -498,7 +546,7 @@ pub(crate) fn get_or_put(
             };
             if !hash_collision {
                 manager.folders.insert(
-                    abs_hash,
+                    key,
                     Entry {
                         abs_path: abs.as_bytes().into(),
                         resolution: stored,
@@ -511,7 +559,7 @@ pub(crate) fn get_or_put(
 
     if !hash_collision {
         manager.folders.insert(
-            abs_hash,
+            key,
             Entry {
                 abs_path: abs.as_bytes().into(),
                 resolution: FolderResolution::PackageId(package.meta.id),

test/cli/install/bun-install.test.ts

@@ -9917,6 +9917,52 @@ it("fails when a transitive file: dependency's folder does not exist", async ()
   expect(exitCode).toBe(1);
 });
 
+// A `file:` dependency is a folder package whose dependencies get installed; a
+// `link:` dependency on the same directory is a bare symlink with none. The
+// folder resolver caches resolutions per directory, so whichever of the two
+// resolved first used to be handed out for the other one as well. Dependencies
+// resolve in alias order, so "a" is resolved before "z".
+for (const [first, second] of [
+  ["link", "file"],
+  ["file", "link"],
+] as const) {
+  it(`resolves file: and link: to the same directory as separate packages (${first}: resolves first)`, async () => {
+    using dir = tempDir("file-and-link-to-same-dir", {
+      "shared/package.json": JSON.stringify({
+        name: "shared",
+        version: "1.0.0",
+        dependencies: { extra: "file:../app/extra" },
+      }),
+      "app/extra/package.json": JSON.stringify({ name: "extra", version: "1.0.0" }),
+      "app/package.json": JSON.stringify({
+        name: "app",
+        dependencies: { a: `${first}:../shared`, z: `${second}:../shared` },
+      }),
+    });
+    const projectDir = join(String(dir), "app");
+    const fileAlias = first === "file" ? "a" : "z";
+    const linkAlias = first === "link" ? "a" : "z";
+
+    await using proc = spawn({
+      cmd: [bunExe(), "install", "--lockfile-only"],
+      cwd: projectDir,
+      stdout: "pipe",
+      stderr: "pipe",
+      env,
+    });
+    const [err, , exitCode] = await Promise.all([proc.stderr.text(), proc.stdout.text(), proc.exited]);
+
+    expect(err).not.toContain("error:");
+    expect(exitCode).toBe(0);
+    const lockfile = Bun.JSONC.parse(await file(join(projectDir, "bun.lock")).text()) as any;
+    expect(lockfile.packages).toEqual({
+      [fileAlias]: ["shared@file:../shared", { dependencies: { extra: "file:../app/extra" } }],
+      [`${fileAlias}/extra`]: ["extra@file:extra", {}],
+      [linkAlias]: ["shared@link:../shared", {}],
+    });
+  });
+}
+
 it("does not extract a local file: tarball outside the temp dir for a dependency alias containing '..' path segments", async () => {
   // For `file:` tarball dependencies, the dependency alias (the key in
   // `dependencies`) is used to derive the temporary extraction folder name.