[install] fix re-install of git dependency (#2519)

- add tests for re-install of npm alias & GitHub URL

2 files changed, 536 insertions, 10 deletions

diff --git a/src/install/install.zig b/src/install/install.zig
index a133b7b48..767b6fc4b 100644
--- a/src/install/install.zig
+++ b/src/install/install.zig
@@ -3055,7 +3055,9 @@ pub const PackageManager = struct {
 
                     var entry = this.task_queue.getOrPutContext(this.allocator, checkout_id, .{}) catch unreachable;
                     if (!entry.found_existing) entry.value_ptr.* = .{};
-                    try entry.value_ptr.append(this.allocator, ctx);
+                    if (this.lockfile.buffers.resolutions.items[id] == invalid_package_id) {
+                        try entry.value_ptr.append(this.allocator, ctx);
+                    }
 
                     if (dependency.behavior.isPeer()) return;
 
@@ -4102,7 +4104,10 @@ pub const PackageManager = struct {
                                     repo.package_name = pkg.name;
                                     try manager.processDependencyListItem(dep, &any_root);
                                 },
-                                else => unreachable,
+                                else => {
+                                    // if it's a node_module folder to install, handle that after we process all the dependencies within the onExtract callback.
+                                    dependency_list_entry.value_ptr.append(manager.allocator, dep) catch unreachable;
+                                },
                             }
                         }
                     }
@@ -6573,6 +6578,14 @@ pub const PackageManager = struct {
                     .fail => |cause| {
                         if (cause.isPackageMissingFromCache()) {
                             switch (resolution.tag) {
+                                .git => {
+                                    this.manager.enqueueGitForCheckout(
+                                        dependency_id,
+                                        alias,
+                                        resolution,
+                                        .{ .node_modules_folder = this.node_modules_folder.dir.fd },
+                                    );
+                                },
                                 .github => {
                                     this.manager.enqueueTarballForDownload(
                                         dependency_id,
@@ -6584,9 +6597,8 @@ pub const PackageManager = struct {
                                 .local_tarball => {
                                     this.manager.enqueueTarballForReading(
                                         dependency_id,
-                                        package_id,
                                         alias,
-                                        resolution.value.local_tarball.slice(buf),
+                                        resolution,
                                         .{ .node_modules_folder = this.node_modules_folder.dir.fd },
                                     );
                                 },
@@ -6658,10 +6670,60 @@ pub const PackageManager = struct {
         }
     };
 
+    pub fn enqueueGitForCheckout(
+        this: *PackageManager,
+        dependency_id: DependencyID,
+        alias: string,
+        resolution: *const Resolution,
+        task_context: TaskCallbackContext,
+    ) void {
+        const repository = &resolution.value.git;
+        const url = this.lockfile.str(&repository.repo);
+        const clone_id = Task.Id.forGitClone(url);
+        const resolved = this.lockfile.str(&repository.resolved);
+        const checkout_id = Task.Id.forGitCheckout(url, resolved);
+        var checkout_queue = this.task_queue.getOrPut(this.allocator, checkout_id) catch unreachable;
+        if (!checkout_queue.found_existing) {
+            checkout_queue.value_ptr.* = .{};
+        }
+
+        checkout_queue.value_ptr.append(
+            this.allocator,
+            task_context,
+        ) catch unreachable;
+
+        if (checkout_queue.found_existing) return;
+
+        if (this.git_repositories.get(clone_id)) |repo_fd| {
+            this.task_batch.push(ThreadPool.Batch.from(this.enqueueGitCheckout(
+                checkout_id,
+                repo_fd,
+                dependency_id,
+                alias,
+                resolution.*,
+                resolved,
+            )));
+        } else {
+            var clone_queue = this.task_queue.getOrPut(this.allocator, clone_id) catch unreachable;
+            if (!clone_queue.found_existing) {
+                clone_queue.value_ptr.* = .{};
+            }
+
+            clone_queue.value_ptr.append(
+                this.allocator,
+                .{ .dependency = dependency_id },
+            ) catch unreachable;
+
+            if (clone_queue.found_existing) return;
+
+            this.task_batch.push(ThreadPool.Batch.from(this.enqueueGitClone(clone_id, alias, repository)));
+        }
+    }
+
     pub fn enqueuePackageForDownload(
         this: *PackageManager,
         name: []const u8,
-        dependency_id: PackageID,
+        dependency_id: DependencyID,
         package_id: PackageID,
         version: Semver.Version,
         url: []const u8,
@@ -6695,7 +6757,7 @@ pub const PackageManager = struct {
 
     pub fn enqueueTarballForDownload(
         this: *PackageManager,
-        dependency_id: PackageID,
+        dependency_id: DependencyID,
         package_id: PackageID,
         url: string,
         task_context: TaskCallbackContext,
@@ -6728,12 +6790,12 @@ pub const PackageManager = struct {
 
     pub fn enqueueTarballForReading(
         this: *PackageManager,
-        dependency_id: PackageID,
-        package_id: PackageID,
+        dependency_id: DependencyID,
         alias: string,
-        path: string,
+        resolution: *const Resolution,
         task_context: TaskCallbackContext,
     ) void {
+        const path = this.lockfile.str(&resolution.value.local_tarball);
         const task_id = Task.Id.forTarball(path);
         var task_queue = this.task_queue.getOrPut(this.allocator, task_id) catch unreachable;
         if (!task_queue.found_existing) {
@@ -6752,7 +6814,7 @@ pub const PackageManager = struct {
             dependency_id,
             alias,
             path,
-            this.lockfile.packages.items(.resolution)[package_id],
+            resolution.*,
         )));
     }
 
diff --git a/test/cli/install/bun-install.test.ts b/test/cli/install/bun-install.test.ts
index aa40b9220..42b988404 100644
--- a/test/cli/install/bun-install.test.ts
+++ b/test/cli/install/bun-install.test.ts
@@ -1581,6 +1581,153 @@ it("should handle scoped alias on unscoped dependency", async () => {
   await access(join(package_dir, "bun.lockb"));
 });
 
+it("should handle aliased dependency with existing lockfile", async () => {
+  const urls: string[] = [];
+  setHandler(
+    dummyRegistry(urls, {
+      "0.0.2": {},
+      "0.0.3": {
+        bin: {
+          "baz-run": "index.js",
+        },
+      },
+      "0.1.0": {
+        dependencies: {
+          bar: "0.0.2",
+          baz: "latest",
+        },
+      },
+      latest: "0.0.3",
+    }),
+  );
+  await writeFile(
+    join(package_dir, "package.json"),
+    JSON.stringify({
+      name: "foo",
+      version: "0.0.1",
+      dependencies: {
+        "moz": "npm:@barn/moo@0.1.0",
+      },
+    }),
+  );
+  const {
+    stdout: stdout1,
+    stderr: stderr1,
+    exited: exited1,
+  } = spawn({
+    cmd: [bunExe(), "install"],
+    cwd: package_dir,
+    stdout: null,
+    stdin: "pipe",
+    stderr: "pipe",
+    env,
+  });
+  expect(stderr1).toBeDefined();
+  const err1 = await new Response(stderr1).text();
+  expect(err1).toContain("Saved lockfile");
+  expect(stdout1).toBeDefined();
+  const out1 = await new Response(stdout1).text();
+  expect(out1.replace(/\s*\[[0-9\.]+m?s\]\s*$/, "").split(/\r?\n/)).toEqual([
+    " + moz@0.1.0",
+    "",
+    " 3 packages installed",
+  ]);
+  expect(await exited1).toBe(0);
+  expect(urls.sort()).toEqual([
+    `${root_url}/@barn/moo`,
+    `${root_url}/@barn/moo-0.1.0.tgz`,
+    `${root_url}/bar`,
+    `${root_url}/bar-0.0.2.tgz`,
+    `${root_url}/baz`,
+    `${root_url}/baz-0.0.3.tgz`,
+  ]);
+  expect(requested).toBe(6);
+  expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([".bin", ".cache", "bar", "baz", "moz"]);
+  expect(await readdirSorted(join(package_dir, "node_modules", ".bin"))).toEqual(["baz-run"]);
+  expect(await readlink(join(package_dir, "node_modules", ".bin", "baz-run"))).toBe(join("..", "baz", "index.js"));
+  expect(await readdirSorted(join(package_dir, "node_modules", "bar"))).toEqual(["package.json"]);
+  expect(await file(join(package_dir, "node_modules", "bar", "package.json")).json()).toEqual({
+    name: "bar",
+    version: "0.0.2",
+  });
+  expect(await readdirSorted(join(package_dir, "node_modules", "baz"))).toEqual(["index.js", "package.json"]);
+  expect(await file(join(package_dir, "node_modules", "baz", "package.json")).json()).toEqual({
+    name: "baz",
+    version: "0.0.3",
+    bin: {
+      "baz-run": "index.js",
+    },
+  });
+  expect(await readdirSorted(join(package_dir, "node_modules", "moz"))).toEqual(["package.json"]);
+  expect(await file(join(package_dir, "node_modules", "moz", "package.json")).json()).toEqual({
+    name: "@barn/moo",
+    version: "0.1.0",
+    dependencies: {
+      bar: "0.0.2",
+      baz: "latest",
+    },
+  });
+  await access(join(package_dir, "bun.lockb"));
+  // Perform `bun install` again but with lockfile from before
+  await rm(join(package_dir, "node_modules"), { force: true, recursive: true });
+  urls.length = 0;
+  const {
+    stdout: stdout2,
+    stderr: stderr2,
+    exited: exited2,
+  } = spawn({
+    cmd: [bunExe(), "install"],
+    cwd: package_dir,
+    stdout: null,
+    stdin: "pipe",
+    stderr: "pipe",
+    env,
+  });
+  expect(stderr2).toBeDefined();
+  const err2 = await new Response(stderr2).text();
+  expect(err2).not.toContain("Saved lockfile");
+  expect(stdout2).toBeDefined();
+  const out2 = await new Response(stdout2).text();
+  expect(out2.replace(/\s*\[[0-9\.]+m?s\]\s*$/, "").split(/\r?\n/)).toEqual([
+    " + moz@0.1.0",
+    "",
+    " 3 packages installed",
+  ]);
+  expect(await exited2).toBe(0);
+  expect(urls.sort()).toEqual([
+    `${root_url}/@barn/moo-0.1.0.tgz`,
+    `${root_url}/bar-0.0.2.tgz`,
+    `${root_url}/baz-0.0.3.tgz`,
+  ]);
+  expect(requested).toBe(9);
+  expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([".bin", ".cache", "bar", "baz", "moz"]);
+  expect(await readdirSorted(join(package_dir, "node_modules", ".bin"))).toEqual(["baz-run"]);
+  expect(await readlink(join(package_dir, "node_modules", ".bin", "baz-run"))).toBe(join("..", "baz", "index.js"));
+  expect(await readdirSorted(join(package_dir, "node_modules", "bar"))).toEqual(["package.json"]);
+  expect(await file(join(package_dir, "node_modules", "bar", "package.json")).json()).toEqual({
+    name: "bar",
+    version: "0.0.2",
+  });
+  expect(await readdirSorted(join(package_dir, "node_modules", "baz"))).toEqual(["index.js", "package.json"]);
+  expect(await file(join(package_dir, "node_modules", "baz", "package.json")).json()).toEqual({
+    name: "baz",
+    version: "0.0.3",
+    bin: {
+      "baz-run": "index.js",
+    },
+  });
+  expect(await readdirSorted(join(package_dir, "node_modules", "moz"))).toEqual(["package.json"]);
+  expect(await file(join(package_dir, "node_modules", "moz", "package.json")).json()).toEqual({
+    name: "@barn/moo",
+    version: "0.1.0",
+    dependencies: {
+      bar: "0.0.2",
+      baz: "latest",
+    },
+  });
+  await access(join(package_dir, "bun.lockb"));
+});
+
 it("should handle GitHub URL in dependencies (user/repo)", async () => {
   const urls: string[] = [];
   setHandler(dummyRegistry(urls));
@@ -2010,6 +2157,131 @@ it("should handle GitHub URL in dependencies (git+https://github.com/user/repo.g
   await access(join(package_dir, "bun.lockb"));
 });
 
+it("should handle GitHub URL with existing lockfile", async () => {
+  const urls: string[] = [];
+  setHandler(dummyRegistry(urls));
+  await writeFile(
+    join(package_dir, "bunfig.toml"),
+    `
+[install]
+cache = false
+`,
+  );
+  await writeFile(
+    join(package_dir, "package.json"),
+    JSON.stringify({
+      name: "foo",
+      version: "0.0.1",
+      dependencies: {
+        "html-minifier": "kangax/html-minifier#v4.0.0",
+      },
+    }),
+  );
+  const {
+    stdout: stdout1,
+    stderr: stderr1,
+    exited: exited1,
+  } = spawn({
+    cmd: [bunExe(), "install"],
+    cwd: package_dir,
+    stdout: null,
+    stdin: "pipe",
+    stderr: "pipe",
+    env,
+  });
+  expect(stderr1).toBeDefined();
+  const err1 = await new Response(stderr1).text();
+  expect(err1).toContain("Saved lockfile");
+  expect(stdout1).toBeDefined();
+  const out1 = await new Response(stdout1).text();
+  expect(out1.replace(/\s*\[[0-9\.]+m?s\]\s*$/, "").split(/\r?\n/)).toEqual([
+    " + html-minifier@github:kangax/html-minifier#4beb325",
+    "",
+    " 12 packages installed",
+  ]);
+  expect(await exited1).toBe(0);
+  expect(urls.sort()).toEqual([]);
+  expect(requested).toBe(0);
+  expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([
+    ".bin",
+    ".cache",
+    "camel-case",
+    "clean-css",
+    "commander",
+    "he",
+    "html-minifier",
+    "lower-case",
+    "no-case",
+    "param-case",
+    "relateurl",
+    "source-map",
+    "uglify-js",
+    "upper-case",
+  ]);
+  expect(await readdirSorted(join(package_dir, "node_modules", ".bin"))).toEqual(["he", "html-minifier", "uglifyjs"]);
+  expect(await readlink(join(package_dir, "node_modules", ".bin", "he"))).toBe(join("..", "he", "bin", "he"));
+  expect(await readlink(join(package_dir, "node_modules", ".bin", "html-minifier"))).toBe(
+    join("..", "html-minifier", "cli.js"),
+  );
+  expect(await readlink(join(package_dir, "node_modules", ".bin", "uglifyjs"))).toBe(
+    join("..", "uglify-js", "bin", "uglifyjs"),
+  );
+  await access(join(package_dir, "bun.lockb"));
+  // Perform `bun install` again but with lockfile from before
+  await rm(join(package_dir, "node_modules"), { force: true, recursive: true });
+  urls.length = 0;
+  const {
+    stdout: stdout2,
+    stderr: stderr2,
+    exited: exited2,
+  } = spawn({
+    cmd: [bunExe(), "install"],
+    cwd: package_dir,
+    stdout: null,
+    stdin: "pipe",
+    stderr: "pipe",
+    env,
+  });
+  expect(stderr2).toBeDefined();
+  const err2 = await new Response(stderr2).text();
+  expect(err2).not.toContain("Saved lockfile");
+  expect(stdout2).toBeDefined();
+  const out2 = await new Response(stdout2).text();
+  expect(out2.replace(/\s*\[[0-9\.]+m?s\]\s*$/, "").split(/\r?\n/)).toEqual([
+    " + html-minifier@github:kangax/html-minifier#4beb325",
+    "",
+    " 12 packages installed",
+  ]);
+  expect(await exited2).toBe(0);
+  expect(urls.sort()).toEqual([]);
+  expect(requested).toBe(0);
+  expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([
+    ".bin",
+    ".cache",
+    "camel-case",
+    "clean-css",
+    "commander",
+    "he",
+    "html-minifier",
+    "lower-case",
+    "no-case",
+    "param-case",
+    "relateurl",
+    "source-map",
+    "uglify-js",
+    "upper-case",
+  ]);
+  expect(await readdirSorted(join(package_dir, "node_modules", ".bin"))).toEqual(["he", "html-minifier", "uglifyjs"]);
+  expect(await readlink(join(package_dir, "node_modules", ".bin", "he"))).toBe(join("..", "he", "bin", "he"));
+  expect(await readlink(join(package_dir, "node_modules", ".bin", "html-minifier"))).toBe(
+    join("..", "html-minifier", "cli.js"),
+  );
+  expect(await readlink(join(package_dir, "node_modules", ".bin", "uglifyjs"))).toBe(
+    join("..", "uglify-js", "bin", "uglifyjs"),
+  );
+  await access(join(package_dir, "bun.lockb"));
+});
+
 it("should consider peerDependencies during hoisting", async () => {
   const urls: string[] = [];
   setHandler(
@@ -2704,6 +2976,198 @@ it("should de-duplicate committish in Git URLs", async () => {
   await access(join(package_dir, "bun.lockb"));
 });
 
+it("should handle Git URL with existing lockfile", async () => {
+  const urls: string[] = [];
+  setHandler(dummyRegistry(urls));
+  await writeFile(
+    join(package_dir, "bunfig.toml"),
+    `
+[install]
+cache = false
+`,
+  );
+  await writeFile(
+    join(package_dir, "package.json"),
+    JSON.stringify({
+      name: "foo",
+      version: "0.0.1",
+      dependencies: {
+        "html-minifier": "git+https://git@github.com/kangax/html-minifier#v4.0.0",
+      },
+    }),
+  );
+  const {
+    stdout: stdout1,
+    stderr: stderr1,
+    exited: exited1,
+  } = spawn({
+    cmd: [bunExe(), "install"],
+    cwd: package_dir,
+    stdout: null,
+    stdin: "pipe",
+    stderr: "pipe",
+    env,
+  });
+  expect(stderr1).toBeDefined();
+  const err1 = await new Response(stderr1).text();
+  expect(err1).toContain("Saved lockfile");
+  expect(stdout1).toBeDefined();
+  const out1 = await new Response(stdout1).text();
+  expect(out1.replace(/\s*\[[0-9\.]+m?s\]\s*$/, "").split(/\r?\n/)).toEqual([
+    " + html-minifier@git+https://git@github.com/kangax/html-minifier#4beb325eb01154a40c0cbebff2e5737bbd7071ab",
+    "",
+    " 12 packages installed",
+  ]);
+  expect(await exited1).toBe(0);
+  expect(urls.sort()).toEqual([]);
+  expect(requested).toBe(0);
+  expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([
+    ".bin",
+    ".cache",
+    "camel-case",
+    "clean-css",
+    "commander",
+    "he",
+    "html-minifier",
+    "lower-case",
+    "no-case",
+    "param-case",
+    "relateurl",
+    "source-map",
+    "uglify-js",
+    "upper-case",
+  ]);
+  expect(await readdirSorted(join(package_dir, "node_modules", ".bin"))).toEqual(["he", "html-minifier", "uglifyjs"]);
+  expect(await readlink(join(package_dir, "node_modules", ".bin", "he"))).toBe(join("..", "he", "bin", "he"));
+  expect(await readlink(join(package_dir, "node_modules", ".bin", "html-minifier"))).toBe(
+    join("..", "html-minifier", "cli.js"),
+  );
+  expect(await readlink(join(package_dir, "node_modules", ".bin", "uglifyjs"))).toBe(
+    join("..", "uglify-js", "bin", "uglifyjs"),
+  );
+  await access(join(package_dir, "bun.lockb"));
+  // Perform `bun install` again but with lockfile from before
+  await rm(join(package_dir, "node_modules"), { force: true, recursive: true });
+  urls.length = 0;
+  const {
+    stdout: stdout2,
+    stderr: stderr2,
+    exited: exited2,
+  } = spawn({
+    cmd: [bunExe(), "install"],
+    cwd: package_dir,
+    stdout: null,
+    stdin: "pipe",
+    stderr: "pipe",
+    env,
+  });
+  expect(stderr2).toBeDefined();
+  const err2 = await new Response(stderr2).text();
+  expect(err2).not.toContain("Saved lockfile");
+  expect(stdout2).toBeDefined();
+  const out2 = await new Response(stdout2).text();
+  expect(out2.replace(/\s*\[[0-9\.]+m?s\]\s*$/, "").split(/\r?\n/)).toEqual([
+    " + html-minifier@git+https://git@github.com/kangax/html-minifier#4beb325eb01154a40c0cbebff2e5737bbd7071ab",
+    "",
+    " 12 packages installed",
+  ]);
+  expect(await exited2).toBe(0);
+  expect(urls.sort()).toEqual([]);
+  expect(requested).toBe(0);
+  expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([
+    ".bin",
+    ".cache",
+    "camel-case",
+    "clean-css",
+    "commander",
+    "he",
+    "html-minifier",
+    "lower-case",
+    "no-case",
+    "param-case",
+    "relateurl",
+    "source-map",
+    "uglify-js",
+    "upper-case",
+  ]);
+  expect(await readdirSorted(join(package_dir, "node_modules", ".bin"))).toEqual(["he", "html-minifier", "uglifyjs"]);
+  expect(await readlink(join(package_dir, "node_modules", ".bin", "he"))).toBe(join("..", "he", "bin", "he"));
+  expect(await readlink(join(package_dir, "node_modules", ".bin", "html-minifier"))).toBe(
+    join("..", "html-minifier", "cli.js"),
+  );
+  expect(await readlink(join(package_dir, "node_modules", ".bin", "uglifyjs"))).toBe(
+    join("..", "uglify-js", "bin", "uglifyjs"),
+  );
+  await access(join(package_dir, "bun.lockb"));
+  // Perform `bun install` again but with cache & lockfile from before
+  [
+    ".bin",
+    "camel-case",
+    "clean-css",
+    "commander",
+    "he",
+    "html-minifier",
+    "lower-case",
+    "no-case",
+    "param-case",
+    "relateurl",
+    "source-map",
+    "uglify-js",
+    "upper-case",
+  ].forEach(async dir => await rm(join(package_dir, "node_modules", dir), { force: true, recursive: true }));
+  urls.length = 0;
+  const {
+    stdout: stdout3,
+    stderr: stderr3,
+    exited: exited3,
+  } = spawn({
+    cmd: [bunExe(), "install"],
+    cwd: package_dir,
+    stdout: null,
+    stdin: "pipe",
+    stderr: "pipe",
+    env,
+  });
+  expect(stderr3).toBeDefined();
+  const err3 = await new Response(stderr3).text();
+  expect(err3).not.toContain("Saved lockfile");
+  expect(stdout3).toBeDefined();
+  const out3 = await new Response(stdout3).text();
+  expect(out3.replace(/\s*\[[0-9\.]+m?s\]\s*$/, "").split(/\r?\n/)).toEqual([
+    " + html-minifier@git+https://git@github.com/kangax/html-minifier#4beb325eb01154a40c0cbebff2e5737bbd7071ab",
+    "",
+    " 12 packages installed",
+  ]);
+  expect(await exited3).toBe(0);
+  expect(urls.sort()).toEqual([]);
+  expect(requested).toBe(0);
+  expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([
+    ".bin",
+    ".cache",
+    "camel-case",
+    "clean-css",
+    "commander",
+    "he",
+    "html-minifier",
+    "lower-case",
+    "no-case",
+    "param-case",
+    "relateurl",
+    "source-map",
+    "uglify-js",
+    "upper-case",
+  ]);
+  expect(await readdirSorted(join(package_dir, "node_modules", ".bin"))).toEqual(["he", "html-minifier", "uglifyjs"]);
+  expect(await readlink(join(package_dir, "node_modules", ".bin", "he"))).toBe(join("..", "he", "bin", "he"));
+  expect(await readlink(join(package_dir, "node_modules", ".bin", "html-minifier"))).toBe(
+    join("..", "html-minifier", "cli.js"),
+  );
+  expect(await readlink(join(package_dir, "node_modules", ".bin", "uglifyjs"))).toBe(
+    join("..", "uglify-js", "bin", "uglifyjs"),
+  );
+  await access(join(package_dir, "bun.lockb"));
+});
+
 it("should prefer optionalDependencies over dependencies of the same name", async () => {
   const urls: string[] = [];
   setHandler(