Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(install): handle transitive folder dependencies #10445

Merged
merged 46 commits into from
Jun 15, 2024
Merged
Changes from 1 commit
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
663a513
symlink transitive folder dependencies
dylan-conway Apr 17, 2024
1b06662
add test
dylan-conway Apr 17, 2024
20c60be
one more test
dylan-conway Apr 17, 2024
86c4e3b
Merge branch 'main' into dylan/link-transitive-folder-dep
dylan-conway Apr 17, 2024
aaf90dd
fix memory issue and windows
dylan-conway Apr 18, 2024
96fa411
fix reinstalls
dylan-conway Apr 18, 2024
2577659
reinstall tests
dylan-conway Apr 18, 2024
452aed7
installWithSymlink
dylan-conway Apr 19, 2024
02d355c
dont ignore node_modules
dylan-conway Apr 19, 2024
4e5be96
revert, update tests
dylan-conway Apr 19, 2024
e3f5267
one more test
dylan-conway Apr 19, 2024
7f18f61
unused
dylan-conway Apr 19, 2024
1980182
Merge branch 'main' into dylan/link-transitive-folder-dep
dylan-conway Apr 20, 2024
9f4ea30
use isRootDependency
dylan-conway Apr 20, 2024
a2cf449
handle windows abs paths
dylan-conway Apr 20, 2024
7324b3d
Merge branch 'main' into dylan/link-transitive-folder-dep
dylan-conway Apr 22, 2024
73685dc
match npm folder resolving
dylan-conway Apr 22, 2024
9fc25e6
windows abs paths and more test
dylan-conway Apr 22, 2024
13f1f7a
logic
dylan-conway Apr 22, 2024
ed51e0d
Merge branch 'main' into dylan/link-transitive-folder-dep
dylan-conway Apr 22, 2024
25e30a9
Merge branch 'main' into dylan/link-transitive-folder-dep
dylan-conway Apr 23, 2024
170b756
Merge branch 'main' into dylan/link-transitive-folder-dep
Jarred-Sumner Apr 23, 2024
0d96fac
Apply formatting changes
Jarred-Sumner Apr 23, 2024
3ea4eb5
check for destination directory
dylan-conway Apr 23, 2024
e5b0b2e
Merge branch 'main' into dylan/link-transitive-folder-dep
dylan-conway Apr 23, 2024
a0fd526
Merge branch 'main' into dylan/link-transitive-folder-dep
dylan-conway Jun 6, 2024
50d7b47
fix merge
dylan-conway Jun 6, 2024
ac16065
fix merge
dylan-conway Jun 6, 2024
06f7b1f
Merge branch 'main' into dylan/link-transitive-folder-dep
dylan-conway Jun 11, 2024
6a59760
maybe fix merge
dylan-conway Jun 11, 2024
df6704f
test for transitive self file dependency
dylan-conway Jun 11, 2024
b2b4140
remove only
dylan-conway Jun 11, 2024
8693e10
fix windows
dylan-conway Jun 11, 2024
c9f2ae2
Merge branch 'main' into dylan/link-transitive-folder-dep
dylan-conway Jun 11, 2024
8dfc685
Merge branch 'main' into dylan/link-transitive-folder-dep
dylan-conway Jun 13, 2024
6651c6e
test for hoisted and unhoisted workspace file deps
dylan-conway Jun 13, 2024
effe1b6
update
dylan-conway Jun 13, 2024
87ee712
Merge branch 'main' into dylan/link-transitive-folder-dep
dylan-conway Jun 14, 2024
8e5ff64
use index
dylan-conway Jun 14, 2024
9610eb6
Merge branch 'main' into dylan/link-transitive-folder-dep
dylan-conway Jun 14, 2024
ca33606
uupdate
dylan-conway Jun 14, 2024
62979d7
Merge branch 'main' into dylan/link-transitive-folder-dep
dylan-conway Jun 14, 2024
cabe0f1
windows fixup
dylan-conway Jun 15, 2024
4cf0f67
ebusy
dylan-conway Jun 15, 2024
27ff236
readlink
dylan-conway Jun 15, 2024
75ccb61
err
dylan-conway Jun 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
test for hoisted and unhoisted workspace file deps
dylan-conway committed Jun 13, 2024
commit 6651c6e86ba6d7541b1cb09ce17b58f900ed6527
52 changes: 12 additions & 40 deletions src/install/install.zig
Original file line number Diff line number Diff line change
@@ -936,6 +936,7 @@ pub fn NewPackageInstall(comptime kind: PkgInstallKind) type {
patch: Patch = .{},
file_count: u32 = 0,
node_modules: *const PackageManager.NodeModulesFolder,
lockfile: *const Lockfile,

const ThisPackageInstall = @This();

@@ -1098,8 +1099,7 @@ pub fn NewPackageInstall(comptime kind: PkgInstallKind) type {
switch (resolution.tag) {
.git => this.verifyGitResolution(&resolution.value.git, buf, root_node_modules_dir),
.github => this.verifyGitResolution(&resolution.value.github, buf, root_node_modules_dir),
// TODO: this should be any workspace tree id, not just root
.folder => if (this.node_modules.tree_id == 0)
.folder => if (this.lockfile.isWorkspaceTreeId(this.node_modules.tree_id))
this.verifyPackageJSONNameAndVersion(root_node_modules_dir, resolution.tag)
else
this.verifyTransitiveSymlinkedFolder(root_node_modules_dir),
@@ -2145,36 +2145,6 @@ pub fn NewPackageInstall(comptime kind: PkgInstallKind) type {
}
}

pub fn isDanglingSymlinkAt(dirfd: bun.FileDescriptor, path: [:0]const u8) bool {
if (comptime Environment.isLinux) {
const rc = bun.sys.system.openat(dirfd.cast(), path, @as(u32, std.os.O.PATH), @as(u32, 0));
switch (bun.sys.getErrno(rc)) {
.SUCCESS => {
_ = bun.sys.close(bun.toFD(rc));
return false;
},
else => return true,
}
} else if (comptime Environment.isWindows) {
switch (bun.sys.sys_uv.openat(dirfd, path, 0, 0)) {
.err => return true,
.result => |fd| {
_ = bun.sys.close(bun.toFD(fd));
return false;
},
}
} else {
const rc = bun.sys.system.openat(dirfd.cast(), path, @as(u32, 0), @as(u32, 0));
switch (bun.sys.getErrno(rc)) {
.SUCCESS => {
_ = bun.sys.close(bun.toFD(rc));
return false;
},
else => return true,
}
}
}

pub fn isDanglingWindowsBinLink(node_mod_fd: bun.FileDescriptor, path: []const u16, temp_buffer: []u8) bool {
const WinBinLinkingShim = @import("./windows-shim/BinLinkingShim.zig");
const bin_path = bin_path: {
@@ -2385,8 +2355,7 @@ pub fn NewPackageInstall(comptime kind: PkgInstallKind) type {

var supported_method_to_use = method_;

// TODO: this should apply to all workspace tree ids, not just the root
if (resolution_tag == .folder and this.node_modules.tree_id != 0) {
if (resolution_tag == .folder and !this.lockfile.isWorkspaceTreeId(this.node_modules.tree_id)) {
supported_method_to_use = .symlink;
}

@@ -2613,10 +2582,11 @@ pub const PackageManager = struct {

root_package_json_file: std.fs.File,

/// The package id corresponding to the workspace the install is happening in
/// The package id corresponding to the workspace the install is happening in. Could be root, or
/// could be any of the workspaces.
root_package_id: struct {
id: ?PackageID = null,
pub fn get(this: *@This(), lockfile: *Lockfile, workspace_name_hash: ?PackageNameHash) PackageID {
pub fn get(this: *@This(), lockfile: *const Lockfile, workspace_name_hash: ?PackageNameHash) PackageID {
return this.id orelse {
this.id = lockfile.getWorkspacePackageID(workspace_name_hash);
return this.id.?;
@@ -10168,6 +10138,7 @@ pub const PackageManager = struct {
.package_version = resolution_label,
// dummy value
.node_modules = &dummy_node_modules,
.lockfile = manager.lockfile,
};

switch (pkg_install.installWithMethod(true, tmpdir, .copyfile, pkg.resolution.tag)) {
@@ -11116,6 +11087,7 @@ pub const PackageManager = struct {
} else PackageInstall.Patch.NULL,
.package_version = package_version,
.node_modules = &this.node_modules,
.lockfile = this.lockfile,
};
debug("Installing {s}@{s}", .{ name, resolution.fmt(buf, .posix) });
const pkg_has_patch = !installer.patch.isNull();
@@ -11136,8 +11108,7 @@ pub const PackageManager = struct {
.folder => {
const folder = resolution.value.folder.slice(buf);

// TODO: this applies to all workspace tree ids, not just root
if (this.current_tree_id == 0) {
if (this.lockfile.isWorkspaceTreeId(this.current_tree_id)) {
// Handle when a package depends on itself via file:
// example:
// "mineflayer": "file:."
@@ -11376,8 +11347,9 @@ pub const PackageManager = struct {
const install_result = switch (resolution.tag) {
.symlink, .workspace => installer.installFromLink(this.skip_delete, destination_dir),
else => result: {
// TODO: this applies to all workspace tree ids, not just root
if (resolution.tag == .folder and this.current_tree_id != 0) {
if (resolution.tag == .folder and !this.lockfile.isWorkspaceTreeId(this.current_tree_id)) {
// This is a transitive folder dependency. It is installed with a single symlink to the target folder/file,
// and is not hoisted.
const dirname = std.fs.path.dirname(this.node_modules.path.items) orelse this.node_modules.path.items;

installer.cache_dir = this.root_node_modules_folder.openDir(dirname, .{ .iterate = true, .access_sub_paths = true }) catch
59 changes: 32 additions & 27 deletions src/install/lockfile.zig
Original file line number Diff line number Diff line change
@@ -320,12 +320,12 @@ pub const Tree = struct {

pub fn toTree(out: External) Tree {
return .{
.id = @as(Id, @bitCast(out[0..4].*)),
.dependency_id = @as(Id, @bitCast(out[4..8].*)),
.parent = @as(Id, @bitCast(out[8..12].*)),
.id = @bitCast(out[0..4].*),
.dependency_id = @bitCast(out[4..8].*),
.parent = @bitCast(out[8..12].*),
.dependencies = .{
.off = @as(u32, @bitCast(out[12..16].*)),
.len = @as(u32, @bitCast(out[16..20].*)),
.off = @bitCast(out[12..16].*),
.len = @bitCast(out[16..20].*),
},
};
}
@@ -575,7 +575,7 @@ pub const Tree = struct {

const dependency = builder.dependencies[dep_id];
// Do not hoist folder dependencies
const destination = if (resolutions[pid].tag == .folder and !builder.lockfile.isWorkspaceDependency(dep_id))
const destination = if (resolutions[pid].tag == .folder)
next.id
else
try next.hoistDependency(
@@ -824,18 +824,18 @@ pub fn clean(
}

/// Is this a direct dependency of the workspace root package.json?
pub fn isWorkspaceRootDependency(this: *Lockfile, id: DependencyID) bool {
pub fn isWorkspaceRootDependency(this: *const Lockfile, id: DependencyID) bool {
return this.packages.items(.dependencies)[0].contains(id);
}

/// Is this a direct dependency of the workspace the install is taking place in?
pub fn isRootDependency(this: *Lockfile, manager: *PackageManager, id: DependencyID) bool {
pub fn isRootDependency(this: *const Lockfile, manager: *PackageManager, id: DependencyID) bool {
return this.packages.items(.dependencies)[manager.root_package_id.get(this, manager.workspace_name_hash)].contains(id);
}

/// Is this a direct dependency of any workspace (including workspace root)
/// Is this a direct dependency of any workspace (including workspace root)?
/// TODO make this faster by caching the workspace package ids
pub fn isWorkspaceDependency(this: *Lockfile, id: DependencyID) bool {
pub fn isWorkspaceDependency(this: *const Lockfile, id: DependencyID) bool {
const packages = this.packages.slice();
const resolutions = packages.items(.resolution);
const dependencies_lists = packages.items(.dependencies);
@@ -847,6 +847,28 @@ pub fn isWorkspaceDependency(this: *Lockfile, id: DependencyID) bool {
return false;
}

/// Does this tree id belong to a workspace (including workspace root)?
pub fn isWorkspaceTreeId(this: *const Lockfile, id: Tree.Id) bool {
return id == 0 or this.buffers.dependencies.items[this.buffers.trees.items[id].dependency_id].behavior.isWorkspaceOnly();
}

pub fn getWorkspacePackageID(this: *const Lockfile, workspace_name_hash: ?PackageNameHash) PackageID {
return if (workspace_name_hash) |workspace_name_hash_| brk: {
const packages = this.packages.slice();
const name_hashes = packages.items(.name_hash);
const resolutions = packages.items(.resolution);
const metas = packages.items(.meta);
for (resolutions, name_hashes, metas) |res, name_hash, meta| {
dylan-conway marked this conversation as resolved.
Show resolved Hide resolved
if (res.tag == .workspace and name_hash == workspace_name_hash_) {
break :brk meta.id;
}
}

// should not hit this, default to root just in case
break :brk 0;
} else 0;
}

pub fn cleanWithLogger(
old: *Lockfile,
manager: *PackageManager,
@@ -1042,23 +1064,6 @@ pub fn cleanWithLogger(
return new;
}

pub fn getWorkspacePackageID(this: *const Lockfile, workspace_name_hash: ?PackageNameHash) PackageID {
return if (workspace_name_hash) |workspace_name_hash_| brk: {
const packages = this.packages.slice();
const name_hashes = packages.items(.name_hash);
const resolutions = packages.items(.resolution);
const metas = packages.items(.meta);
for (resolutions, name_hashes, metas) |res, name_hash, meta| {
if (res.tag == .workspace and name_hash == workspace_name_hash_) {
break :brk meta.id;
}
}

// should not hit this, default to root just in case
break :brk 0;
} else 0;
}

pub const MetaHashFormatter = struct {
meta_hash: *const MetaHash,

1 change: 1 addition & 0 deletions src/install/patch_install.zig
Original file line number Diff line number Diff line change
@@ -348,6 +348,7 @@ pub const PatchTask = struct {
.package_version = resolution_label,
// dummy value
.node_modules = &dummy_node_modules,
.lockfile = this.manager.lockfile,
};

switch (pkg_install.installImpl(true, system_tmpdir, .copyfile, this.callback.apply.resolution.tag)) {
Loading
Loading