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
Show file tree
Hide file tree
Changes from 38 commits
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
311 changes: 169 additions & 142 deletions src/install/install.zig

Large diffs are not rendered by default.

96 changes: 59 additions & 37 deletions src/install/lockfile.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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].*),
},
};
}
Expand Down Expand Up @@ -474,7 +474,7 @@ pub const Tree = struct {
const Builder = struct {
allocator: Allocator,
name_hashes: []const PackageNameHash,
list: ArrayList = .{},
list: bun.MultiArrayList(Entry) = .{},
resolutions: []const PackageID,
dependencies: []const Dependency,
resolution_lists: []const Lockfile.DependencyIDSlice,
Expand Down Expand Up @@ -504,8 +504,6 @@ pub const Tree = struct {
dependencies: Lockfile.DependencyIDList,
};

pub const ArrayList = bun.MultiArrayList(Entry);

/// Flatten the multi-dimensional ArrayList of package IDs into a single easily serializable array
pub fn clean(this: *Builder) !DependencyIDList {
const end = @as(Id, @truncate(this.list.len));
Expand Down Expand Up @@ -565,6 +563,8 @@ pub const Tree = struct {
const next: *Tree = &trees[builder.list.len - 1];
const name_hashes: []const PackageNameHash = builder.name_hashes;
const max_package_id = @as(PackageID, @truncate(name_hashes.len));
const resolutions = builder.lockfile.packages.items(.resolution);

var dep_id = resolution_list.off;
const end = dep_id + resolution_list.len;

Expand All @@ -574,16 +574,19 @@ pub const Tree = struct {
if (pid >= max_package_id) continue;

const dependency = builder.dependencies[dep_id];

const destination = try next.hoistDependency(
true,
pid,
dep_id,
&dependency,
dependency_lists,
trees,
builder,
);
// Do not hoist folder dependencies
const destination = if (resolutions[pid].tag == .folder)
next.id
else
try next.hoistDependency(
true,
pid,
dep_id,
&dependency,
dependency_lists,
trees,
builder,
);

switch (destination) {
Tree.dependency_loop, Tree.hoisted => continue,
Expand Down Expand Up @@ -821,15 +824,51 @@ 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)?
/// TODO make this faster by caching the workspace package ids
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);
for (resolutions, dependencies_lists) |resolution, dependencies| {
if (resolution.tag != .workspace and resolution.tag != .root) continue;
if (dependencies.contains(id)) return true;
}

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,
Expand Down Expand Up @@ -1025,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,

Expand Down
3 changes: 2 additions & 1 deletion src/install/patch_install.zig
Original file line number Diff line number Diff line change
Expand Up @@ -348,9 +348,10 @@ 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)) {
switch (pkg_install.installImpl(true, system_tmpdir, .copyfile, this.callback.apply.resolution.tag)) {
.success => {},
.fail => |reason| {
return try log.addErrorFmtNoLoc(
Expand Down
23 changes: 18 additions & 5 deletions src/sys.zig
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ pub const Tag = enum(u8) {
rename,
stat,
symlink,
symlinkat,
unlink,
utimes,
write,
Expand Down Expand Up @@ -1745,9 +1746,19 @@ pub fn chown(path: [:0]const u8, uid: os.uid_t, gid: os.gid_t) Maybe(void) {
}
}

pub fn symlink(from: [:0]const u8, to: [:0]const u8) Maybe(void) {
pub fn symlink(target: [:0]const u8, dest: [:0]const u8) Maybe(void) {
while (true) {
if (Maybe(void).errnoSys(sys.symlink(from, to), .symlink)) |err| {
if (Maybe(void).errnoSys(sys.symlink(target, dest), .symlink)) |err| {
if (err.getErrno() == .INTR) continue;
return err;
}
return Maybe(void).success;
}
}

pub fn symlinkat(target: [:0]const u8, dirfd: bun.FileDescriptor, dest: [:0]const u8) Maybe(void) {
while (true) {
if (Maybe(void).errnoSys(sys.symlinkat(target, dirfd.cast(), dest), .symlinkat)) |err| {
if (err.getErrno() == .INTR) continue;
return err;
}
Expand All @@ -1774,12 +1785,14 @@ pub const WindowsSymlinkOptions = packed struct {
pub var has_failed_to_create_symlink = false;
};

pub fn symlinkOrJunctionOnWindows(dest: [:0]const u8, target: [:0]const u8) Maybe(void) {
pub fn symlinkOrJunction(dest: [:0]const u8, target: [:0]const u8) Maybe(void) {
if (comptime !Environment.isWindows) @compileError("symlinkOrJunction is windows only");

if (!WindowsSymlinkOptions.has_failed_to_create_symlink) {
var sym16: bun.WPathBuffer = undefined;
var target16: bun.WPathBuffer = undefined;
const sym_path = bun.strings.toNTPath(&sym16, dest);
const target_path = bun.strings.toNTPath(&target16, target);
const sym_path = bun.strings.toWPathNormalizeAutoExtend(&sym16, dest);
const target_path = bun.strings.toWPathNormalizeAutoExtend(&target16, target);
switch (symlinkW(sym_path, target_path, .{ .directory = true })) {
.result => {
return Maybe(void).success;
Expand Down
9 changes: 5 additions & 4 deletions test/cli/install/bun-add.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ for (const pathType of ["absolute", "relative"]) {
"",
"1 package installed",
]);

expect(await exited).toBe(0);
},
);
Expand Down Expand Up @@ -1671,17 +1672,17 @@ it("should add dependencies to workspaces directly", async () => {
"package.json",
]);
expect(await file(join(package_dir, "package.json")).text()).toEqual(bar_package);
expect(await readdirSorted(join(package_dir, "moo"))).toEqual(["bunfig.toml", "package.json"]);
expect(await readdirSorted(join(package_dir, "moo"))).toEqual(["bunfig.toml", "node_modules", "package.json"]);
expect(await readdirSorted(join(package_dir, "moo", "node_modules", "foo"))).toEqual(["package.json"]);
expect(await file(join(package_dir, "moo", "node_modules", "foo", "package.json")).text()).toEqual(foo_package);
expect(await file(join(package_dir, "moo", "package.json")).json()).toEqual({
name: "moo",
version: "0.3.0",
dependencies: {
foo: `file:${add_path.replace(/\\/g, "/")}`,
},
});
expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([".cache", "foo", "moo"]);
expect(await readdirSorted(join(package_dir, "node_modules", "foo"))).toEqual(["package.json"]);
expect(await file(join(package_dir, "node_modules", "foo", "package.json")).text()).toEqual(foo_package);
expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([".cache", "moo"]);
});

it("should redirect 'install --save X' to 'add'", async () => {
Expand Down
Loading
Loading