Skip to content

Commit

Permalink
fix(install): fix dependency install order (#10240)
Browse files Browse the repository at this point in the history
* packages wait for parent trees before install

* use `directoryExistsAt`

* missing increment

* fix faccessat

* swap destination and target

* update

* force

* only create destination dir before installing package

* fix windows symlink/junction

* increment, false on extracting

* done
  • Loading branch information
dylan-conway committed Apr 16, 2024
1 parent 24a411f commit 5a81dc8
Show file tree
Hide file tree
Showing 7 changed files with 510 additions and 324 deletions.
3 changes: 2 additions & 1 deletion src/bun.zig
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ else
pub const huge_allocator_threshold: comptime_int = @import("./memory_allocator.zig").huge_threshold;

pub const callmod_inline: std.builtin.CallModifier = if (builtin.mode == .Debug) .auto else .always_inline;
pub const callconv_inline: std.builtin.CallingConvention = if (builtin.mode == .Debug) .Unspecified else .Inline;

/// We cannot use a threadlocal memory allocator for FileSystem-related things
/// FileSystem is a singleton.
Expand Down Expand Up @@ -3089,7 +3090,7 @@ pub inline fn debugAssert(cheap_value_only_plz: bool) void {
}
}

pub inline fn assert(value: bool) void {
pub fn assert(value: bool) callconv(callconv_inline) void {
if (comptime !Environment.allow_assert) {
return;
}
Expand Down
1 change: 0 additions & 1 deletion src/fmt.zig
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ pub inline fn utf16(slice_: []const u16) FormatUTF16 {

pub const FormatUTF16 = struct {
buf: []const u16,
escape_backslashes: bool = false,
path_fmt_opts: ?PathFormatOptions = null,
pub fn format(self: @This(), comptime _: []const u8, _: anytype, writer: anytype) !void {
if (self.path_fmt_opts) |opts| {
Expand Down
779 changes: 472 additions & 307 deletions src/install/install.zig

Large diffs are not rendered by default.

13 changes: 13 additions & 0 deletions src/install/lockfile.zig
Original file line number Diff line number Diff line change
Expand Up @@ -738,6 +738,13 @@ pub fn cleanWithLogger(
// We will only shrink the number of packages here.
// never grow

// preinstall_state is used during installPackages. the indexes(package ids) need
// to be remapped
var preinstall_state = PackageManager.instance.preinstall_state;
var old_preinstall_state = preinstall_state.clone(old.allocator) catch bun.outOfMemory();
defer old_preinstall_state.deinit(old.allocator);
@memset(preinstall_state.items, .unknown);

if (updates.len > 0) {
try old.preprocessUpdateRequests(updates, exact_versions);
}
Expand Down Expand Up @@ -775,6 +782,7 @@ pub fn cleanWithLogger(
.mapping = package_id_mapping,
.clone_queue = clone_queue_,
.log = log,
.old_preinstall_state = old_preinstall_state,
};

// try clone_queue.ensureUnusedCapacity(root.dependencies.len);
Expand Down Expand Up @@ -925,6 +933,7 @@ const Cloner = struct {
trees: Tree.List = Tree.List{},
trees_count: u32 = 1,
log: *logger.Log,
old_preinstall_state: std.ArrayListUnmanaged(Install.PreinstallState),

pub fn flush(this: *Cloner) anyerror!void {
const max_package_id = this.old.packages.len;
Expand Down Expand Up @@ -2841,6 +2850,10 @@ pub const Package = extern struct {

package_id_mapping[this.meta.id] = new_package.meta.id;

if (PackageManager.instance.preinstall_state.items.len > 0) {
PackageManager.instance.preinstall_state.items[new_package.meta.id] = cloner.old_preinstall_state.items[this.meta.id];
}

for (old_dependencies, dependencies) |old_dep, *new_dep| {
new_dep.* = try old_dep.clone(
old_string_buf,
Expand Down
8 changes: 8 additions & 0 deletions src/install/resolution.zig
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ pub const Resolution = extern struct {
return this.tag.isGit();
}

pub fn canEnqueueInstallTask(this: *const Resolution) bool {
return this.tag.canEnqueueInstallTask();
}

pub fn order(
lhs: *const Resolution,
rhs: *const Resolution,
Expand Down Expand Up @@ -337,5 +341,9 @@ pub const Resolution = extern struct {
pub fn isGit(this: Tag) bool {
return this == .git or this == .github or this == .gitlab;
}

pub fn canEnqueueInstallTask(this: Tag) bool {
return this == .npm or this == .local_tarball or this == .remote_tarball or this == .git or this == .github;
}
};
};
2 changes: 1 addition & 1 deletion src/resolver/resolver.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1935,7 +1935,7 @@ pub const Resolver = struct {
const dir_path_for_resolution = manager.pathForResolution(resolved_package_id, resolution, bufs(.path_in_global_disk_cache)) catch |err| {
// if it's missing, we need to install it
if (err == error.FileNotFound) {
switch (manager.getPreinstallState(resolved_package_id, manager.lockfile)) {
switch (manager.getPreinstallState(resolved_package_id)) {
.done => {
var path = Fs.Path.init(import_path);
path.is_disabled = true;
Expand Down
28 changes: 14 additions & 14 deletions src/sys.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1733,11 +1733,11 @@ pub const WindowsSymlinkOptions = packed struct {
pub var has_failed_to_create_symlink = false;
};

pub fn symlinkOrJunctionOnWindows(sym: [:0]const u8, target: [:0]const u8) Maybe(void) {
pub fn symlinkOrJunctionOnWindows(dest: [:0]const u8, target: [:0]const u8) Maybe(void) {
if (!WindowsSymlinkOptions.has_failed_to_create_symlink) {
var sym16: bun.WPathBuffer = undefined;
var target16: bun.WPathBuffer = undefined;
const sym_path = bun.strings.toNTPath(&sym16, sym);
const sym_path = bun.strings.toNTPath(&sym16, dest);
const target_path = bun.strings.toNTPath(&target16, target);
switch (symlinkW(sym_path, target_path, .{ .directory = true })) {
.result => {
Expand All @@ -1751,17 +1751,17 @@ pub fn symlinkOrJunctionOnWindows(sym: [:0]const u8, target: [:0]const u8) Maybe
}
}

return sys_uv.symlinkUV(sym, target, bun.windows.libuv.UV_FS_SYMLINK_JUNCTION);
return sys_uv.symlinkUV(target, dest, bun.windows.libuv.UV_FS_SYMLINK_JUNCTION);
}

pub fn symlinkW(sym: [:0]const u16, target: [:0]const u16, options: WindowsSymlinkOptions) Maybe(void) {
pub fn symlinkW(dest: [:0]const u16, target: [:0]const u16, options: WindowsSymlinkOptions) Maybe(void) {
while (true) {
const flags = options.flags();

if (windows.kernel32.CreateSymbolicLinkW(sym, target, flags) == 0) {
if (windows.kernel32.CreateSymbolicLinkW(dest, target, flags) == 0) {
const errno = bun.windows.Win32Error.get();
log("CreateSymbolicLinkW({}, {}, {any}) = {s}", .{
bun.fmt.fmtPath(u16, sym, .{}),
bun.fmt.fmtPath(u16, dest, .{}),
bun.fmt.fmtPath(u16, target, .{}),
flags,
@tagName(errno),
Expand All @@ -1788,7 +1788,7 @@ pub fn symlinkW(sym: [:0]const u16, target: [:0]const u16, options: WindowsSymli
}

log("CreateSymbolicLinkW({}, {}, {any}) = 0", .{
bun.fmt.fmtPath(u16, sym, .{}),
bun.fmt.fmtPath(u16, dest, .{}),
bun.fmt.fmtPath(u16, target, .{}),
flags,
});
Expand Down Expand Up @@ -2160,24 +2160,24 @@ pub fn directoryExistsAt(dir_: anytype, subpath: anytype) JSC.Maybe(bool) {
}

if (comptime !has_sentinel) {
const path = std.os.toPosixPath(subpath) catch return JSC.Maybe(bool){ .err = Error.oom };
const path = std.os.toPosixPath(subpath) catch return JSC.Maybe(bool){ .err = Error.fromCode(.NAMETOOLONG, .access) };
return directoryExistsAt(dir_fd, path);
}

if (comptime Environment.isLinux) {
// avoid loading the libc symbol for this to reduce chances of GLIBC minimum version requirements
const rc = linux.faccessat(dir_fd.cast(), subpath, linux.O.DIRECTORY | linux.O.RDONLY, 0);
syslog("faccessat({}, {}, O_DIRECTORY | O_RDONLY, 0) = {d}", .{ dir_fd, bun.fmt.fmtOSPath(subpath, .{}), rc });
const rc = linux.faccessat(dir_fd.cast(), subpath, linux.F_OK, 0);
syslog("faccessat({}, {}, O_DIRECTORY | O_RDONLY, 0) = {d}", .{ dir_fd, bun.fmt.fmtOSPath(subpath, .{}), if (rc == 0) 0 else @intFromEnum(linux.getErrno(rc)) });
if (rc == 0) {
return JSC.Maybe(bool){ .result = true };
}

return JSC.Maybe(bool){ .result = false };
}

// on toher platforms use faccessat from libc
const rc = std.c.faccessat(dir_fd.cast(), subpath, std.os.O.DIRECTORY | std.os.O.RDONLY, 0);
syslog("faccessat({}, {}, O_DIRECTORY | O_RDONLY, 0) = {d}", .{ dir_fd, bun.fmt.fmtOSPath(subpath, .{}), rc });
// on other platforms use faccessat from libc
const rc = std.c.faccessat(dir_fd.cast(), subpath, std.os.F_OK, 0);
syslog("faccessat({}, {}, O_DIRECTORY | O_RDONLY, 0) = {d}", .{ dir_fd, bun.fmt.fmtOSPath(subpath, .{}), if (rc == 0) 0 else @intFromEnum(std.c.getErrno(rc)) });
if (rc == 0) {
return JSC.Maybe(bool){ .result = true };
}
Expand All @@ -2187,7 +2187,7 @@ pub fn directoryExistsAt(dir_: anytype, subpath: anytype) JSC.Maybe(bool) {

pub fn existsAt(fd: bun.FileDescriptor, subpath: []const u8) bool {
if (comptime Environment.isPosix) {
return system.faccessat(bun.toFD(fd), &(std.os.toPosixPath(subpath) catch return false), 0, 0) == 0;
return system.faccessat(fd.cast(), &(std.os.toPosixPath(subpath) catch return false), 0, 0) == 0;
}

if (comptime Environment.isWindows) {
Expand Down

0 comments on commit 5a81dc8

Please sign in to comment.