[install] handle `workspace:*` correctly (#3994)

- parse as path so it works on unversioned workspaces
- fix missed storage of workspace version

fixes #3985

2 files changed, 158 insertions, 54 deletions

diff --git a/src/install/lockfile.zig b/src/install/lockfile.zig
index 7d29084c6..869d46f4d 100644
--- a/src/install/lockfile.zig
+++ b/src/install/lockfile.zig
@@ -2564,14 +2564,15 @@ pub const Package = extern struct {
             .npm => String.Builder.stringHash(dependency_version.value.npm.name.slice(buf)),
             .workspace => if (strings.hasPrefixComptime(sliced.slice, "workspace:")) brk: {
                 const input = sliced.slice["workspace:".len..];
-                const at = strings.lastIndexOfChar(input, '@') orelse 0;
-                if (at > 0) {
-                    workspace_range = Semver.Query.parse(allocator, input[at + 1 ..], sliced) catch return error.InstallFailed;
-                    break :brk String.Builder.stringHash(input[0..at]);
-                } else {
+                if (!strings.eqlComptime(input, "*")) {
+                    const at = strings.lastIndexOfChar(input, '@') orelse 0;
+                    if (at > 0) {
+                        workspace_range = Semver.Query.parse(allocator, input[at + 1 ..], sliced) catch return error.InstallFailed;
+                        break :brk String.Builder.stringHash(input[0..at]);
+                    }
                     workspace_range = Semver.Query.parse(allocator, input, sliced) catch null;
-                    break :brk external_alias.hash;
                 }
+                break :brk external_alias.hash;
             } else external_alias.hash,
             else => external_alias.hash,
         };
@@ -2632,70 +2633,71 @@ pub const Package = extern struct {
                     dependency_version.value.workspace = path;
                 }
             } else {
-                {
-                    const workspace = dependency_version.value.workspace.slice(buf);
-                    const path = string_builder.append(
-                        String,
-                        if (strings.eqlComptime(workspace, "*")) "*" else Path.relative(
+                const workspace = dependency_version.value.workspace.slice(buf);
+                const path = string_builder.append(
+                    String,
+                    if (strings.eqlComptime(workspace, "*")) "*" else Path.relative(
+                        FileSystem.instance.top_level_dir,
+                        Path.joinAbsString(
                             FileSystem.instance.top_level_dir,
-                            Path.joinAbsString(
-                                FileSystem.instance.top_level_dir,
-                                &[_]string{
-                                    source.path.name.dir,
-                                    workspace,
-                                },
-                                .posix,
-                            ),
+                            &[_]string{
+                                source.path.name.dir,
+                                workspace,
+                            },
+                            .posix,
                         ),
-                    );
-                    if (comptime Environment.allow_assert) {
-                        std.debug.assert(path.len() > 0);
-                        std.debug.assert(!std.fs.path.isAbsolute(path.slice(buf)));
-                    }
-                    dependency_version.literal = path;
-                    dependency_version.value.workspace = path;
+                    ),
+                );
+                if (comptime Environment.allow_assert) {
+                    std.debug.assert(path.len() > 0);
+                    std.debug.assert(!std.fs.path.isAbsolute(path.slice(buf)));
+                }
+                dependency_version.literal = path;
+                dependency_version.value.workspace = path;
 
-                    var workspace_entry = try lockfile.workspace_paths.getOrPut(allocator, @truncate(name_hash));
-                    if (workspace_entry.found_existing) {
-                        const old_path = workspace_entry.value_ptr.*;
+                var workspace_entry = try lockfile.workspace_paths.getOrPut(allocator, @truncate(name_hash));
+                if (workspace_entry.found_existing) {
+                    if (strings.eqlComptime(workspace, "*")) return null;
 
-                        if (strings.eqlComptime(workspace, "*")) {
-                            return null;
-                        } else if (strings.eqlComptime(old_path.slice(buf), "*")) brk: {
-                            workspace_entry.value_ptr.* = path;
-                            for (package_dependencies[0..dependencies_count]) |*package_dep| {
-                                if (String.Builder.stringHash(package_dep.realname().slice(buf)) == name_hash) {
-                                    if (package_dep.version.tag != .workspace) break :brk;
-                                    package_dep.version = dependency_version;
-                                    return null;
-                                }
-                            }
-                            return error.InstallFailed;
-                        } else if (strings.eql(old_path.slice(buf), path.slice(buf))) {
-                            return null;
-                        } else {
-                            log.addErrorFmt(&source, logger.Loc.Empty, allocator, "Workspace name \"{s}\" already exists", .{
-                                external_alias.slice(buf),
-                            }) catch {};
-                            return error.InstallFailed;
-                        }
+                    const old_path = workspace_entry.value_ptr.*.slice(buf);
+                    if (!strings.eqlComptime(old_path, "*")) {
+                        if (strings.eql(old_path, path.slice(buf))) return null;
+
+                        log.addErrorFmt(&source, logger.Loc.Empty, allocator, "Workspace name \"{s}\" already exists", .{
+                            external_alias.slice(buf),
+                        }) catch {};
+                        return error.InstallFailed;
                     }
-                    workspace_entry.value_ptr.* = path;
                 }
+                workspace_entry.value_ptr.* = path;
 
                 if (workspace_version) |ver| {
                     try lockfile.workspace_versions.put(allocator, @truncate(name_hash), ver);
 
                     for (package_dependencies[0..dependencies_count]) |*package_dep| {
-                        // `dependencies` & `workspaces` defined within the same `package.json`
-                        if (package_dep.version.tag == .npm and
-                            String.Builder.stringHash(package_dep.version.value.npm.name.slice(buf)) == name_hash and
-                            package_dep.version.value.npm.version.satisfies(ver))
+                        if (switch (package_dep.version.tag) {
+                            // `dependencies` & `workspaces` defined within the same `package.json`
+                            .npm => String.Builder.stringHash(package_dep.realname().slice(buf)) == name_hash and
+                                package_dep.version.value.npm.version.satisfies(ver),
+                            // `workspace:*`
+                            .workspace => workspace_entry.found_existing and
+                                String.Builder.stringHash(package_dep.realname().slice(buf)) == name_hash,
+                            else => false,
+                        }) {
+                            package_dep.version = dependency_version;
+                            return null;
+                        }
+                    }
+                } else if (workspace_entry.found_existing) {
+                    for (package_dependencies[0..dependencies_count]) |*package_dep| {
+                        if (package_dep.version.tag == .workspace and
+                            String.Builder.stringHash(package_dep.realname().slice(buf)) == name_hash)
                         {
                             package_dep.version = dependency_version;
                             return null;
                         }
                     }
+                    return error.InstallFailed;
                 }
             },
             else => {},
diff --git a/test/cli/install/bun-install.test.ts b/test/cli/install/bun-install.test.ts
index cd467c97e..16525122e 100644
--- a/test/cli/install/bun-install.test.ts
+++ b/test/cli/install/bun-install.test.ts
@@ -5457,3 +5457,105 @@ it("should handle `workspace:` with alias & @scope", async () => {
   );
   await access(join(package_dir, "bun.lockb"));
 });
+
+it("should handle `workspace:*` on both root & child", async () => {
+  const urls: string[] = [];
+  setHandler(dummyRegistry(urls));
+  await writeFile(
+    join(package_dir, "package.json"),
+    JSON.stringify({
+      name: "foo",
+      workspaces: ["packages/*"],
+      dependencies: {
+        bar: "workspace:*",
+      },
+    }),
+  );
+  await mkdir(join(package_dir, "packages", "bar"), { recursive: true });
+  const bar_package = JSON.stringify({
+    name: "bar",
+    version: "0.1.2",
+  });
+  await writeFile(join(package_dir, "packages", "bar", "package.json"), bar_package);
+  await mkdir(join(package_dir, "packages", "baz"), { recursive: true });
+  const baz_package = JSON.stringify({
+    name: "baz",
+    version: "1.2.3",
+    devDependencies: {
+      bar: "workspace:*",
+    },
+  });
+  await writeFile(join(package_dir, "packages", "baz", "package.json"), baz_package);
+  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).not.toContain("error:");
+  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([
+    " + baz@workspace:packages/baz",
+    " + bar@workspace:packages/bar",
+    "",
+    " 2 packages installed",
+  ]);
+  expect(await exited1).toBe(0);
+  expect(urls.sort()).toBeEmpty();
+  expect(requested).toBe(0);
+  expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([".cache", "bar", "baz"]);
+  expect(await readlink(join(package_dir, "node_modules", "bar"))).toBe(join("..", "packages", "bar"));
+  expect(await readdirSorted(join(package_dir, "node_modules", "bar"))).toEqual(["package.json"]);
+  expect(await file(join(package_dir, "node_modules", "bar", "package.json")).text()).toEqual(bar_package);
+  expect(await readlink(join(package_dir, "node_modules", "baz"))).toBe(join("..", "packages", "baz"));
+  expect(await readdirSorted(join(package_dir, "node_modules", "baz"))).toEqual(["package.json"]);
+  expect(await file(join(package_dir, "node_modules", "baz", "package.json")).text()).toEqual(baz_package);
+  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 });
+  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("error:");
+  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([
+    " + baz@workspace:packages/baz",
+    " + bar@workspace:packages/bar",
+    "",
+    " 2 packages installed",
+  ]);
+  expect(await exited2).toBe(0);
+  expect(urls.sort()).toBeEmpty();
+  expect(requested).toBe(0);
+  expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual(["bar", "baz"]);
+  expect(await readlink(join(package_dir, "node_modules", "bar"))).toBe(join("..", "packages", "bar"));
+  expect(await readdirSorted(join(package_dir, "node_modules", "bar"))).toEqual(["package.json"]);
+  expect(await file(join(package_dir, "node_modules", "bar", "package.json")).text()).toEqual(bar_package);
+  expect(await readlink(join(package_dir, "node_modules", "baz"))).toBe(join("..", "packages", "baz"));
+  expect(await readdirSorted(join(package_dir, "node_modules", "baz"))).toEqual(["package.json"]);
+  expect(await file(join(package_dir, "node_modules", "baz", "package.json")).text()).toEqual(baz_package);
+  await access(join(package_dir, "bun.lockb"));
+});