fix(pnpm migration): correctly resolve link: dependencies on the root package (#23111)

### What does this PR do?
Two things:
- we weren't adding the root package to the `pkg_map`.
- `link:` dependency paths in `"snapshots"` weren't being joined with
the top level dir.
### How did you verify your code works?
Manually and added a test.
This commit is contained in:
Dylan Conway
2025-09-30 00:10:15 -07:00
committed by GitHub
parent badcfe8a14
commit 25e156c95b
7 changed files with 107 additions and 17 deletions

View File

@@ -286,7 +286,9 @@ pub const OSPathSlice = paths.OSPathSlice;
pub const OSPathBuffer = paths.OSPathBuffer;
pub const Path = paths.Path;
pub const AbsPath = paths.AbsPath;
pub const AutoAbsPath = paths.AutoAbsPath;
pub const RelPath = paths.RelPath;
pub const AutoRelPath = paths.AutoRelPath;
pub const EnvPath = paths.EnvPath;
pub const path_buffer_pool = paths.path_buffer_pool;
pub const w_path_buffer_pool = paths.w_path_buffer_pool;

View File

@@ -242,7 +242,7 @@ pub fn migratePnpmLockfile(
continue;
}
var pkg_json_path: bun.AbsPath(.{ .sep = .auto }) = .initTopLevelDir();
var pkg_json_path: bun.AutoAbsPath = .initTopLevelDir();
defer pkg_json_path.deinit();
pkg_json_path.append(importer_path);
@@ -285,7 +285,7 @@ pub fn migratePnpmLockfile(
var importer_dep_res_versions: bun.StringArrayHashMap(bun.StringArrayHashMap([]const u8)) = .init(allocator);
{
var pkg_json_path: bun.AbsPath(.{ .sep = .auto }) = .initTopLevelDir();
var pkg_json_path: bun.AutoAbsPath = .initTopLevelDir();
defer pkg_json_path.deinit();
pkg_json_path.append("package.json");
@@ -329,6 +329,8 @@ pub fn migratePnpmLockfile(
var pkg_map: bun.StringArrayHashMap(PackageID) = .init(allocator);
try pkg_map.putNoClobber(bun.fs.FileSystem.instance.top_level_dir, 0);
const workspace_pkgs_off = lockfile.packages.len;
workspaces: for (lockfile.workspace_paths.values()) |workspace_path| {
@@ -348,7 +350,7 @@ pub fn migratePnpmLockfile(
.value = .{ .workspace = try string_buf.append(path) },
};
var path_buf: bun.AbsPath(.{ .sep = .auto }) = .initTopLevelDir();
var path_buf: bun.AutoAbsPath = .initTopLevelDir();
defer path_buf.deinit();
path_buf.append(path);
@@ -445,13 +447,13 @@ pub fn migratePnpmLockfile(
if (strings.withoutPrefixIfPossibleComptime(version_without_suffix, "link:")) |link_path| {
// create a link package for the workspace dependency only if it doesn't already exist
if (dep.version.tag == .workspace) {
var link_path_buf: bun.AbsPath(.{ .sep = .auto }) = .initTopLevelDir();
var link_path_buf: bun.AutoAbsPath = .initTopLevelDir();
defer link_path_buf.deinit();
link_path_buf.append(workspace_path);
link_path_buf.join(&.{link_path});
for (lockfile.workspace_paths.values()) |existing_workspace_path| {
var workspace_path_buf: bun.AbsPath(.{ .sep = .auto }) = .initTopLevelDir();
var workspace_path_buf: bun.AutoAbsPath = .initTopLevelDir();
defer workspace_path_buf.deinit();
workspace_path_buf.append(existing_workspace_path.slice(string_buf.bytes.items));
@@ -469,7 +471,7 @@ pub fn migratePnpmLockfile(
.resolution = .init(.{ .symlink = try string_buf.append(link_path) }),
};
var abs_link_path: bun.AbsPath(.{ .sep = .auto }) = .initTopLevelDir();
var abs_link_path: bun.AutoAbsPath = .initTopLevelDir();
defer abs_link_path.deinit();
abs_link_path.join(&.{ workspace_path, link_path });
@@ -685,7 +687,7 @@ pub fn migratePnpmLockfile(
// implicit workspace dependencies
if (dep.behavior.isWorkspace()) {
const workspace_path = dep.version.value.workspace.slice(string_buf);
var path_buf: bun.AbsPath(.{ .sep = .auto }) = .initTopLevelDir();
var path_buf: bun.AutoAbsPath = .initTopLevelDir();
defer path_buf.deinit();
path_buf.join(&.{workspace_path});
if (pkg_map.get(path_buf.slice())) |workspace_pkg_id| {
@@ -706,7 +708,7 @@ pub fn migratePnpmLockfile(
const version_without_suffix = removeSuffix(version);
if (strings.withoutPrefixIfPossibleComptime(version_without_suffix, "link:")) |maybe_symlink_or_folder_or_workspace_path| {
var path_buf: bun.AbsPath(.{ .sep = .auto }) = .initTopLevelDir();
var path_buf: bun.AutoAbsPath = .initTopLevelDir();
defer path_buf.deinit();
path_buf.join(&.{maybe_symlink_or_folder_or_workspace_path});
if (pkg_map.get(path_buf.slice())) |pkg_id| {
@@ -755,7 +757,7 @@ pub fn migratePnpmLockfile(
const version_without_suffix = removeSuffix(version);
if (strings.withoutPrefixIfPossibleComptime(version_without_suffix, "link:")) |maybe_symlink_or_folder_or_workspace_path| {
var path_buf: bun.AbsPath(.{ .sep = .auto }) = .initTopLevelDir();
var path_buf: bun.AutoAbsPath = .initTopLevelDir();
defer path_buf.deinit();
path_buf.join(&.{ workspace_path, maybe_symlink_or_folder_or_workspace_path });
if (pkg_map.get(path_buf.slice())) |link_pkg_id| {
@@ -795,8 +797,11 @@ pub fn migratePnpmLockfile(
switch (dep.version.tag) {
.folder, .symlink, .workspace => {
const maybe_symlink_or_folder_or_workspace_path = strings.withoutPrefixComptime(version_without_suffix, "link:");
if (pkg_map.get(maybe_symlink_or_folder_or_workspace_path)) |workspace_pkg_id| {
lockfile.buffers.resolutions.items[dep_id] = workspace_pkg_id;
var path_buf: bun.AutoAbsPath = .initTopLevelDir();
defer path_buf.deinit();
path_buf.join(&.{maybe_symlink_or_folder_or_workspace_path});
if (pkg_map.get(path_buf.slice())) |link_pkg_id| {
lockfile.buffers.resolutions.items[dep_id] = link_pkg_id;
continue;
}
},
@@ -1172,7 +1177,7 @@ fn parseAppendImporterDependencies(
continue;
}
var path_buf: bun.AbsPath(.{ .sep = .auto }) = .initTopLevelDir();
var path_buf: bun.AutoAbsPath = .initTopLevelDir();
defer path_buf.deinit();
path_buf.append(path);

View File

@@ -1,6 +1,8 @@
pub const Path = paths.Path;
pub const AbsPath = paths.AbsPath;
pub const AutoAbsPath = paths.AutoAbsPath;
pub const RelPath = paths.RelPath;
pub const AutoRelPath = paths.AutoRelPath;
pub const EnvPath = @import("./paths/EnvPath.zig").EnvPath;

View File

@@ -227,12 +227,16 @@ pub fn AbsPath(comptime opts: Options) type {
return Path(copy);
}
pub const AutoAbsPath = Path(.{ .kind = .abs, .sep = .auto });
pub fn RelPath(comptime opts: Options) type {
var copy = opts;
copy.kind = .rel;
return Path(copy);
}
pub const AutoRelPath = Path(.{ .kind = .rel, .sep = .auto });
pub fn Path(comptime opts: Options) type {
const Result = opts.ResultFn();

View File

@@ -83,17 +83,53 @@ describe.todo("patched packages", () => {
});
});
describe.todo("folder dependencies", () => {
test("basic", async () => {
describe("folder dependencies", () => {
test.todo("basic", async () => {
const { packageDir, packageJson } = await verdaccio.createTestDir({
files: join(import.meta.dir, "pnpm/folder-dependencies-basic"),
});
});
test("links are resolved correctly", async () => {
// link:
test("links to the root package are resolved correctly", async () => {
const { packageDir, packageJson } = await verdaccio.createTestDir({
files: join(import.meta.dir, "pnpm/folder-dependencies-links"),
files: join(import.meta.dir, "pnpm/root-package-link-resolution"),
});
let proc = spawn({
cmd: [bunExe(), "install"],
cwd: packageDir,
env,
stdout: "pipe",
stderr: "pipe",
});
const [out, err, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(exitCode).toBe(0);
expect(err).toContain("Saved lockfile");
expect(
await Promise.all([
file(join(packageDir, "node_modules", "two-range-deps", "package.json")).json(),
file(join(packageDir, "node_modules", "no-deps", "package.json")).json(),
]),
).toMatchInlineSnapshot(`
[
{
"dependencies": {
"@types/is-number": ">=1.0.0",
"no-deps": "^1.0.0",
},
"name": "two-range-deps",
"version": "1.0.0",
},
{
"dependencies": {
"two-range-deps": "1.0.0",
},
"name": "transitive-root-link-pkg",
},
]
`);
});
});

View File

@@ -0,0 +1,6 @@
{
"name": "transitive-root-link-pkg",
"dependencies": {
"two-range-deps": "1.0.0"
}
}

View File

@@ -0,0 +1,35 @@
lockfileVersion: '9.0'
settings:
autoInstallPeers: true
excludeLinksFromLockfile: false
importers:
.:
dependencies:
two-range-deps:
specifier: 1.0.0
version: 1.0.0
packages:
'@types/is-number@2.0.0':
resolution: {integrity: sha512-GEeIxCB+NpM1NrDBqmkYPeU8bI//i+xPzdOY4E1YHet51IcFmz4js6k57m69fLl/cbn7sOR7wj9RNNw53X8AiA==}
no-deps@1.1.0:
resolution: {integrity: sha512-ebG2pipYAKINcNI3YxdsiAgFvNGp2gdRwxAKN2LYBm9+YxuH/lHH2sl+GKQTuGiNfCfNZRMHUyyLPEJD6HWm7w==}
two-range-deps@1.0.0:
resolution: {integrity: sha512-N+6kPy/GxuMncNz/EKuIrwdoYbh1qmvHDnw1UbM3sQE184kBn+6qAQgtf1wgT9dJnt6X+tWcTzSmfDvtJikVBA==}
snapshots:
'@types/is-number@2.0.0': {}
no-deps@1.1.0: {}
two-range-deps@1.0.0:
dependencies:
'@types/is-number': 2.0.0
no-deps: 'link:'