fix(install): npm lockfile migration bugfix (#11311)

src/install/install.zig

@@ -2432,9 +2432,6 @@ pub const PackageManager = struct {
     cpu_count: u32 = 0,
     package_json_updates: []UpdateRequest = &[_]UpdateRequest{},
 
-    // used for looking up workspaces that aren't loaded into Lockfile.workspace_paths
-    workspaces: std.StringArrayHashMap(Semver.Version),
-
     // progress bar stuff when not stack allocated
     root_progress_node: *std.Progress.Node = undefined,
 
@@ -7443,18 +7440,6 @@ pub const PackageManager = struct {
             Output.flush();
         }
 
-        var workspaces = std.StringArrayHashMap(Semver.Version).init(ctx.allocator);
-        for (workspace_names.values()) |entry| {
-            if (entry.version) |version_string| {
-                const sliced_version = SlicedString.init(version_string, version_string);
-                const result = Semver.Version.parse(sliced_version);
-                if (result.valid and result.wildcard == .none) {
-                    try workspaces.put(entry.name, result.version.min());
-                    continue;
-                }
-            }
-        }
-
         workspace_names.map.deinit();
 
         var manager = &instance;
@@ -7474,7 +7459,6 @@ pub const PackageManager = struct {
             .resolve_tasks = .{},
             .lockfile = undefined,
             .root_package_json_file = root_package_json_file,
-            .workspaces = workspaces,
             // .progress
             .event_loop = .{
                 .mini = JSC.MiniEventLoop.init(bun.default_allocator),
@@ -7578,7 +7562,6 @@ pub const PackageManager = struct {
             .event_loop = .{
                 .js = JSC.VirtualMachine.get().eventLoop(),
             },
-            .workspaces = std.StringArrayHashMap(Semver.Version).init(allocator),
             .original_package_json_path = original_package_json_path[0..original_package_json_path.len :0],
         };
         manager.lockfile = try allocator.create(Lockfile);

src/install/lockfile.zig

@@ -3433,7 +3433,6 @@ pub const Package = extern struct {
             const from_deps = from.dependencies.get(from_lockfile.buffers.dependencies.items);
             const from_resolutions = from.resolutions.get(from_lockfile.buffers.resolutions.items);
             var to_i: usize = 0;
-            var skipped_workspaces: usize = 0;
 
             if (from_lockfile.overrides.map.count() != to_lockfile.overrides.map.count()) {
                 summary.overrides_changed = true;
@@ -3576,11 +3575,6 @@ pub const Package = extern struct {
                         if (from_dep.name_hash == to_deps[to_i].name_hash) break :found;
                     }
 
-                    if (PackageManager.instance.workspaces.contains(from_lockfile.str(&from_dep.name))) {
-                        skipped_workspaces += 1;
-                        continue;
-                    }
-
                     // We found a removed dependency!
                     // We don't need to remove it
                     // It will be cleaned up later
@@ -3675,7 +3669,7 @@ pub const Package = extern struct {
             // Use saturating arithmetic here because a migrated
             // package-lock.json could be out of sync with the package.json, so the
             // number of from_deps could be greater than to_deps.
-            summary.add = @truncate((to_deps.len + skipped_workspaces) -| (from_deps.len -| summary.remove));
+            summary.add = @truncate((to_deps.len) -| (from_deps.len -| summary.remove));
 
             inline for (Lockfile.Scripts.names) |hook| {
                 if (!@field(to.scripts, hook).eql(
@@ -3807,13 +3801,6 @@ pub const Package = extern struct {
         if (comptime tag == null) {
             workspace_path = lockfile.workspace_paths.get(name_hash);
             workspace_version = lockfile.workspace_versions.get(name_hash);
-
-            if (workspace_path == null or workspace_version == null) {
-                if (PackageManager.instance.workspaces.get(lockfile.str(&external_alias.value))) |_workspace_version| {
-                    workspace_path = external_alias.value;
-                    workspace_version = _workspace_version;
-                }
-            }
         }
 
         if (comptime tag != null) {
@@ -3911,7 +3898,7 @@ pub const Package = extern struct {
                     dependency_version.value.workspace = path;
 
                     const workspace_entry = try lockfile.workspace_paths.getOrPut(allocator, name_hash);
-                    const found_matching_workspace = workspace_entry.found_existing or PackageManager.instance.workspaces.contains(lockfile.str(&external_alias.value));
+                    const found_matching_workspace = workspace_entry.found_existing;
 
                     if (workspace_version) |ver| {
                         try lockfile.workspace_versions.put(allocator, name_hash, ver);
@@ -4130,17 +4117,13 @@ pub const Package = extern struct {
                 continue;
             }
 
-            var abs_package_json_path: stringZ = Path.joinAbsStringBufZ(
+            const abs_package_json_path: stringZ = Path.joinAbsStringBufZ(
                 source.path.name.dir,
                 filepath_buf,
                 &.{ input_path, "package.json" },
                 .auto,
             );
 
-            if (comptime Environment.isWindows) {
-                abs_package_json_path = Path.normalizeStringZ(abs_package_json_path, true, .posix);
-            }
-
             const workspace_entry = processWorkspaceName(
                 allocator,
                 json_cache,
@@ -4181,16 +4164,26 @@ pub const Package = extern struct {
 
             if (workspace_entry.name.len == 0) continue;
 
+            const rel_input_path = Path.relativePlatform(
+                source.path.name.dir,
+                strings.withoutSuffixComptime(abs_package_json_path, std.fs.path.sep_str ++ "package.json"),
+                .auto,
+                true,
+            );
+            if (comptime Environment.isWindows) {
+                Path.dangerouslyConvertPathToPosixInPlace(u8, @constCast(rel_input_path));
+            }
+
             if (string_builder) |builder| {
                 builder.count(workspace_entry.name);
-                builder.count(input_path);
+                builder.count(rel_input_path);
                 builder.cap += bun.MAX_PATH_BYTES;
                 if (workspace_entry.version) |version_string| {
                     builder.count(version_string);
                 }
             }
 
-            try workspace_names.insert(input_path, .{
+            try workspace_names.insert(rel_input_path, .{
                 .name = workspace_entry.name,
                 .name_loc = workspace_entry.name_loc,
                 .version = workspace_entry.version,
@@ -4299,16 +4292,20 @@ pub const Package = extern struct {
                     const workspace_path: string = Path.relativePlatform(
                         source.path.name.dir,
                         abs_workspace_dir_path,
-                        .posix,
+                        .auto,
                         true,
                     );
-
-                    Path.dangerouslyConvertPathToPosixInPlace(u8, @constCast(workspace_path));
+                    if (comptime Environment.isWindows) {
+                        Path.dangerouslyConvertPathToPosixInPlace(u8, @constCast(workspace_path));
+                    }
 
                     if (string_builder) |builder| {
                         builder.count(workspace_entry.name);
                         builder.count(workspace_path);
                         builder.cap += bun.MAX_PATH_BYTES;
+                        if (workspace_entry.version) |version| {
+                            builder.count(version);
+                        }
                     }
 
                     try workspace_names.insert(workspace_path, .{

test/cli/install/migration/lockfile-with-workspaces/.gitignore

@@ -0,0 +1 @@
+!package-lock.json

test/cli/install/migration/lockfile-with-workspaces/package.json

@@ -0,0 +1,9 @@
+{
+  "name": "npm-lockfile-test",
+  "workspaces": [
+    "./packages/pkg0",
+    "packages\\pkg1",
+    ".//packages/pkg2",
+    "packages/../packages/pkg3"
+  ]
+}

test/cli/install/migration/lockfile-with-workspaces/packages/pkg0/package.json

@@ -0,0 +1,3 @@
+{
+    "name": "pkg0"
+}

test/cli/install/migration/lockfile-with-workspaces/packages/pkg1/package.json

@@ -0,0 +1,3 @@
+{
+    "name": "pkg1"
+}

test/cli/install/migration/lockfile-with-workspaces/packages/pkg2/package.json

@@ -0,0 +1,3 @@
+{
+    "name": "pkg2"
+}

test/cli/install/migration/lockfile-with-workspaces/packages/pkg3/package.json

@@ -0,0 +1,3 @@
+{
+    "name": "pkg3"
+}

test/cli/install/migration/migrate.test.ts

@@ -77,3 +77,23 @@ test("migrate from npm lockfile that is missing `resolved` properties", async ()
   expect(await Bun.file(join(testDir, "node_modules/lodash/package.json")).json()).toHaveProperty("version", "4.17.21");
   expect(exitCode).toBe(0);
 });
+
+test("npm lockfile with relative workspaces", async () => {
+  const testDir = tmpdirSync();
+  fs.cpSync(join(import.meta.dir, "lockfile-with-workspaces"), testDir, { recursive: true });
+  const { exitCode, stderr } = Bun.spawnSync([bunExe(), "install"], {
+    env: bunEnv,
+    cwd: testDir,
+  });
+  const err = stderr.toString();
+  expect(err).toContain("migrated lockfile from package-lock.json");
+
+  expect(err).not.toContain("InvalidNPMLockfile");
+  for (let i = 0; i < 4; i++) {
+    expect(await Bun.file(join(testDir, "node_modules", "pkg" + i, "package.json")).json()).toEqual({
+      "name": "pkg" + i,
+    });
+  }
+
+  expect(exitCode).toBe(0);
+});