install: reuse the package of a file: folder that several packages declare

src/install/PackageManager/PackageManagerEnqueue.rs

@@ -2925,13 +2925,27 @@ fn get_or_put_resolved_package(
                     );
                 }
 
+                let path_is_top_level_relative =
+                    this.lockfile.is_trusted_folder_dependency(dependency_id);
+
                 // transitive folder dependencies do not have their dependencies resolved
                 if crate::bin::bin_target_escapes_package_dir(this.lockfile.str(&folder))
-                    && !this.lockfile.is_trusted_folder_dependency(dependency_id)
+                    && !path_is_top_level_relative
                 {
                     break 'res FolderResolutionValue::Err(crate::Error::MissingPackageJSON);
                 }
 
+                // One such path is one folder, so reuse the package another declarer loaded
+                if path_is_top_level_relative {
+                    if let Some(existing_id) = this.lockfile.get_package_id(
+                        name_hash,
+                        None,
+                        &Resolution::init(ResolutionTagged::Folder(folder)),
+                    ) {
+                        break 'res FolderResolutionValue::PackageId(existing_id);
+                    }
+                }
+
                 let mut package = Package::default();
 
                 {
@@ -2962,7 +2976,6 @@ fn get_or_put_resolved_package(
                     builder.clamp();
                 }
 
-                // these are always new
                 package = this.lockfile.append_package(&package).unwrap_or_oom();
 
                 break 'res FolderResolutionValue::NewPackageId(package.meta.id);

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

@@ -11216,9 +11216,9 @@ it("installs file: dependencies that depend on each other", async () => {
 
         "b": ["b@file:packages/b", { "dependencies": { "a": "file:../a" } }],
 
-        "a/b": ["b@file:packages/b", {}],
+        "a/b": ["b@file:packages/b", { "dependencies": { "a": "file:../a" } }],
 
-        "b/a": ["a@file:packages/a", {}],
+        "b/a": ["a@file:packages/a", { "dependencies": { "b": "file:../b" } }],
       }
     }"
   `);
@@ -11267,6 +11267,168 @@ it("installs file: dependencies that depend on each other from a lockfile that o
   `);
 });
 
+it("reloads the lockfile it wrote when a file: dependency and the root declare the same folder", async () => {
+  using dir = tempDir("shared-file-dep", {
+    "package.json": JSON.stringify({
+      name: "my-app",
+      version: "1.0.0",
+      dependencies: {
+        lib: "file:./vendor/lib",
+        shared: "file:./vendor/shared",
+      },
+    }),
+    "vendor/lib/package.json": JSON.stringify({
+      name: "lib",
+      version: "1.0.0",
+      dependencies: { shared: "file:../shared" },
+    }),
+    "vendor/shared/package.json": JSON.stringify({
+      name: "shared",
+      version: "1.0.0",
+      dependencies: { nested: "file:../nested" },
+    }),
+    "vendor/nested/package.json": JSON.stringify({ name: "nested", version: "1.0.0" }),
+  });
+
+  const install = async (...args: string[]) => {
+    await using proc = spawn({
+      cmd: [bunExe(), "install", ...args],
+      cwd: String(dir),
+      stdout: "pipe",
+      stdin: "ignore",
+      stderr: "pipe",
+      env,
+    });
+    return await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+  };
+
+  const [, err, exitCode] = await install();
+  expect(err).toContain("Saved lockfile");
+  expect(err).not.toContain("error:");
+  expect(exitCode).toBe(0);
+  const lock = await file(join(String(dir), "bun.lock")).text();
+
+  // `lib/shared` was a second `shared` package with no dependencies. The parser
+  // makes one package of the two rows, so it looked for a `lib/shared/nested` row:
+  // "Failed to resolve prod dependency 'nested' for package 'lib/shared'".
+  await rm(join(String(dir), "node_modules"), { recursive: true, force: true });
+  const [, err2, exitCode2] = await install("--frozen-lockfile");
+  expect(err2).not.toContain("error:");
+  expect(err2).not.toContain("Ignoring lockfile");
+  expect(exitCode2).toBe(0);
+  expect(await file(join(String(dir), "bun.lock")).text()).toBe(lock);
+
+  expect(normalizeBunSnapshot(lock, dir)).toMatchInlineSnapshot(`
+    "{
+      "lockfileVersion": 2,
+      "configVersion": 1,
+      "workspaces": {
+        "": {
+          "name": "my-app",
+          "dependencies": {
+            "lib": "file:./vendor/lib",
+            "shared": "file:./vendor/shared",
+          },
+        },
+      },
+      "packages": {
+        "lib": ["lib@file:vendor/lib", { "dependencies": { "shared": "file:../shared" } }],
+
+        "shared": ["shared@file:vendor/shared", { "dependencies": { "nested": "file:../nested" } }],
+
+        "lib/shared": ["shared@file:vendor/shared", { "dependencies": { "nested": "file:../nested" } }],
+
+        "shared/nested": ["nested@file:vendor/nested", {}],
+
+        "lib/shared/nested": ["nested@file:vendor/nested", {}],
+      }
+    }"
+  `);
+});
+
+it("keeps a registry package's own file: folder apart from the root's folder of the same path", async () => {
+  // A registry package's `file:` path is relative to that package, so `file:x`
+  // under it names a different folder than the root's `file:x`. The root's copy
+  // has the trusted `postinstall`; it must not run in the package's copy.
+  const ctx = await createTestContext();
+  try {
+    const tarball = await new Bun.Archive(
+      {
+        "package/package.json": JSON.stringify({ name: "foo", version: "1.0.0", dependencies: { x: "file:x" } }),
+        "package/x/package.json": JSON.stringify({ name: "x", version: "6.6.6" }),
+        "package/x/build.js": `throw new Error("the postinstall of the root's x ran in foo's x");`,
+      },
+      { compress: "gzip" },
+    ).bytes();
+
+    setContextHandler(ctx, async request => {
+      if (request.url.endsWith(".tgz")) return new Response(tarball);
+      return Response.json({
+        name: "foo",
+        "dist-tags": { latest: "1.0.0" },
+        versions: {
+          "1.0.0": {
+            name: "foo",
+            version: "1.0.0",
+            dependencies: { x: "file:x" },
+            dist: {
+              tarball: `${ctx.registry_url}foo-1.0.0.tgz`,
+              integrity: `sha512-${new Bun.CryptoHasher("sha512").update(tarball).digest("base64")}`,
+            },
+          },
+        },
+      });
+    });
+
+    await Promise.all([
+      write(
+        join(ctx.package_dir, "package.json"),
+        JSON.stringify({
+          name: "my-app",
+          version: "1.0.0",
+          dependencies: { foo: "1.0.0", x: "file:x" },
+          trustedDependencies: ["x"],
+        }),
+      ),
+      write(
+        join(ctx.package_dir, "x", "package.json"),
+        JSON.stringify({ name: "x", version: "1.0.0", scripts: { postinstall: `${bunExe()} build.js` } }),
+      ),
+      write(join(ctx.package_dir, "x", "build.js"), `require("fs").writeFileSync("ran.txt", process.cwd());`),
+    ]);
+
+    await using proc = spawn({
+      cmd: [bunExe(), "install", "--save-text-lockfile"],
+      cwd: ctx.package_dir,
+      stdout: "pipe",
+      stdin: "ignore",
+      stderr: "pipe",
+      env,
+    });
+    const [err, out, exitCode] = await Promise.all([proc.stderr.text(), proc.stdout.text(), proc.exited]);
+    expect(err).not.toContain("error:");
+    expect(out).toContain("3 packages installed");
+    expect(exitCode).toBe(0);
+
+    // The script ran once, in the root's folder. `foo`'s copy has no marker and
+    // keeps its own version.
+    expect([
+      await file(join(ctx.package_dir, "node_modules", "x", "ran.txt")).text(),
+      await exists(join(ctx.package_dir, "node_modules", "foo", "node_modules", "x", "ran.txt")),
+      await file(join(ctx.package_dir, "node_modules", "foo", "node_modules", "x", "package.json")).json(),
+    ]).toEqual([join(ctx.package_dir, "node_modules", "x"), false, { name: "x", version: "6.6.6" }]);
+
+    // two rows, so two packages
+    const lock = await file(join(ctx.package_dir, "bun.lock")).text();
+    expect(lock.split("\n").filter(line => line.includes(`["x@file:x"`))).toEqual([
+      `    "x": ["x@file:x", {}],`,
+      `    "foo/x": ["x@file:x", {}],`,
+    ]);
+  } finally {
+    destroyTestContext(ctx);
+  }
+});
+
 it("fails when a transitive file: dependency's folder does not exist", async () => {
   using dir = tempDir("transitive-file-dep-missing", {
     "package.json": JSON.stringify({

test/cli/install/isolated-install.test.ts

@@ -554,6 +554,105 @@ test("can install folder dependencies on root package", async () => {
   ]);
 });
 
+test("a folder dependency shared by several folder dependencies is one package", async () => {
+  const { packageJson, packageDir } = await registry.createTestDir({ bunfigOpts: { linker: "isolated" } });
+
+  const siblings = Array.from({ length: 6 }, (_, i) => `sib${i}`);
+
+  await Promise.all([
+    write(
+      packageJson,
+      JSON.stringify({
+        name: "test-pkg-shared-folder-dep",
+        dependencies: {
+          shared: "file:./vend/shared",
+          ...Object.fromEntries(siblings.map(name => [name, `file:./vend/${name}`])),
+        },
+      }),
+    ),
+    write(join(packageDir, "vend", "shared", "package.json"), JSON.stringify({ name: "shared", version: "1.0.0" })),
+    write(join(packageDir, "vend", "shared", "index.js"), "module.exports = 'shared';"),
+    ...siblings.flatMap(name => [
+      write(
+        join(packageDir, "vend", name, "package.json"),
+        JSON.stringify({ name, version: "1.0.0", dependencies: { shared: "file:../shared" } }),
+      ),
+      write(join(packageDir, "vend", name, "index.js"), "module.exports = require('shared');"),
+    ]),
+  ]);
+
+  // A fresh resolve made one `shared` package per declarer. Each one linked
+  // into the same store entry at the same time: EEXIST from link().
+  const { out } = await runBunInstall(bunEnv, packageDir);
+  expect(out).toContain(`${siblings.length + 1} packages installed`);
+
+  const store = join(packageDir, "node_modules", ".bun");
+  expect(readlinkSync(join(packageDir, "node_modules", "shared"))).toBe(
+    join(".bun", "shared@file+vend+shared", "node_modules", "shared"),
+  );
+  expect(
+    await Promise.all(
+      siblings.map(name => readlink(join(store, `${name}@file+vend+${name}`, "node_modules", "shared"))),
+    ),
+  ).toEqual(siblings.map(() => join("..", "..", "shared@file+vend+shared", "node_modules", "shared")));
+  expect(await readdirSorted(join(store, "shared@file+vend+shared", "node_modules", "shared"))).toEqual([
+    "index.js",
+    "package.json",
+  ]);
+
+  const requireFromRoot = createRequire(packageJson);
+  expect(siblings.map(name => requireFromRoot(name))).toEqual(siblings.map(() => "shared"));
+});
+
+test("a file: override that several registry packages depend on is one package", async () => {
+  const { packageJson, packageDir } = await registry.createTestDir({ bunfigOpts: { linker: "isolated" } });
+
+  // Each of these declares `no-deps`.
+  const dependents = {
+    "one-dep": "1.0.0",
+    "one-fixed-dep": "1.0.0",
+    "one-range-dep": "1.0.0",
+    "one-range-dep-too": "1.0.0",
+    "normal-dep-and-dev-dep": "1.0.2",
+  };
+
+  await Promise.all([
+    write(
+      packageJson,
+      JSON.stringify({
+        name: "test-pkg-file-override",
+        dependencies: dependents,
+        overrides: { "no-deps": "file:./vendor/no-deps" },
+      }),
+    ),
+    write(join(packageDir, "vendor", "no-deps", "package.json"), JSON.stringify({ name: "no-deps", version: "9.9.9" })),
+  ]);
+
+  // CI exports one BUN_INSTALL_CACHE_DIR for the whole file. Manifests cached here
+  // would change the order in which later tests resolve the same packages.
+  const env = { ...bunEnv, BUN_INSTALL_CACHE_DIR: join(packageDir, ".bun-cache") };
+  const { out } = await runBunInstall(env, packageDir);
+  expect(out).toContain(`${Object.keys(dependents).length + 1} packages installed`);
+
+  // one entry for the override target, whatever path spelling its name carries
+  const store = join(packageDir, "node_modules", ".bun");
+  const entries = await readdirSorted(store);
+  expect(entries.filter(dir => dir.startsWith("no-deps@"))).toEqual([expect.stringMatching(/^no-deps@file\+/)]);
+  const overrideEntry = entries.find(dir => dir.startsWith("no-deps@"))!;
+
+  expect(
+    await Promise.all(
+      Object.entries(dependents).map(([name, version]) =>
+        readlink(join(store, `${name}@${version}`, "node_modules", "no-deps")),
+      ),
+    ),
+  ).toEqual(Object.keys(dependents).map(() => join("..", "..", overrideEntry, "node_modules", "no-deps")));
+  expect(await file(join(store, overrideEntry, "node_modules", "no-deps", "package.json")).json()).toEqual({
+    name: "no-deps",
+    version: "9.9.9",
+  });
+});
+
 describe("isolated workspaces", () => {
   test("basic", async () => {
     const { packageJson, packageDir } = await registry.createTestDir({ bunfigOpts: { linker: "isolated" } });