From 45ca9e08c3e8cc71c788461328c2091b59611883 Mon Sep 17 00:00:00 2001 From: Dylan Conway <35280289+dylan-conway@users.noreply.github.com> Date: Fri, 20 Dec 2024 00:34:21 -0800 Subject: [PATCH] fix(install): `peer/dev/optional = false` lockfile fix (#15874) Co-authored-by: Jarred Sumner --- docs/cli/bun-install.md | 3 + docs/cli/install.md | 14 + docs/install/index.md | 7 + src/ini.zig | 62 ++++ src/install/bun.lock.zig | 105 +++--- src/install/extract_tarball.zig | 4 +- src/install/install.zig | 309 +++++++++--------- src/install/lockfile.zig | 286 ++++++++++------ src/install/migration.zig | 2 +- src/install/patch_install.zig | 9 +- src/resolver/resolver.zig | 2 +- test/cli/install/bun-install.test.ts | 184 ++++++++--- test/cli/install/bun-run.test.ts | 4 +- .../bun-install-registry.test.ts.snap | 27 ++ .../registry/bun-install-registry.test.ts | 123 +++++++ test/harness.ts | 7 + 16 files changed, 802 insertions(+), 346 deletions(-) diff --git a/docs/cli/bun-install.md b/docs/cli/bun-install.md index f06e1843e50c04..a2eb2ee19610ff 100644 --- a/docs/cli/bun-install.md +++ b/docs/cli/bun-install.md @@ -57,12 +57,15 @@ frozenLockfile = false dryRun = true # Install optionalDependencies (default: true) +# Setting this to false is equivalent to the `--omit=optional` CLI argument optional = true # Install local devDependencies (default: true) +# Setting this to false is equivalent to the `--omit=dev` CLI argument dev = true # Install peerDependencies (default: true) +# Setting this to false is equivalent to the `--omit=peer` CLI argument peer = true # Max number of concurrent lifecycle scripts (default: (cpu count or GOMAXPROCS) x2) diff --git a/docs/cli/install.md b/docs/cli/install.md index 2fa9db74610841..25d46c98a1bf92 100644 --- a/docs/cli/install.md +++ b/docs/cli/install.md @@ -130,6 +130,20 @@ $ bun install --frozen-lockfile For more information on Bun's binary lockfile `bun.lockb`, refer to [Package manager > Lockfile](https://bun.sh/docs/install/lockfile). +## Omitting dependencies + +To omit dev, peer, or optional dependencies use the `--omit` flag. + +```bash +# Exclude "devDependencies" from the installation. This will apply to the +# root package and workspaces if they exist. Transitive dependencies will +# not have "devDependencies". +$ bun install --omit dev + +# Install only dependencies from "dependencies" +$ bun install --omit=dev --omit=peer --omit=optional +``` + ## Dry run To perform a dry run (i.e. don't actually install anything): diff --git a/docs/install/index.md b/docs/install/index.md index 5631b55adb1192..8f412a05e9adc7 100644 --- a/docs/install/index.md +++ b/docs/install/index.md @@ -55,6 +55,13 @@ To install dependencies without allowing changes to lockfile (useful on CI): $ bun install --frozen-lockfile ``` +To exclude dependency types from installing, use `--omit` with `dev`, `optional`, or `peer`: + +```bash +# Disable devDependencies and optionalDependencies +$ bun install --omit=dev --omit=optional +``` + To perform a dry run (i.e. don't actually install anything): ```bash diff --git a/src/ini.zig b/src/ini.zig index d13c34d696d53a..017670fbd39c62 100644 --- a/src/ini.zig +++ b/src/ini.zig @@ -971,6 +971,68 @@ pub fn loadNpmrc( } } + if (out.asProperty("omit")) |omit| { + switch (omit.expr.data) { + .e_string => |str| { + if (str.eqlComptime("dev")) { + install.save_dev = false; + } else if (str.eqlComptime("peer")) { + install.save_peer = false; + } else if (str.eqlComptime("optional")) { + install.save_optional = false; + } + }, + .e_array => |arr| { + for (arr.items.slice()) |item| { + switch (item.data) { + .e_string => |str| { + if (str.eqlComptime("dev")) { + install.save_dev = false; + } else if (str.eqlComptime("peer")) { + install.save_peer = false; + } else if (str.eqlComptime("optional")) { + install.save_optional = false; + } + }, + else => {}, + } + } + }, + else => {}, + } + } + + if (out.asProperty("include")) |omit| { + switch (omit.expr.data) { + .e_string => |str| { + if (str.eqlComptime("dev")) { + install.save_dev = true; + } else if (str.eqlComptime("peer")) { + install.save_peer = true; + } else if (str.eqlComptime("optional")) { + install.save_optional = true; + } + }, + .e_array => |arr| { + for (arr.items.slice()) |item| { + switch (item.data) { + .e_string => |str| { + if (str.eqlComptime("dev")) { + install.save_dev = true; + } else if (str.eqlComptime("peer")) { + install.save_peer = true; + } else if (str.eqlComptime("optional")) { + install.save_optional = true; + } + }, + else => {}, + } + } + }, + else => {}, + } + } + var registry_map = install.scoped orelse bun.Schema.Api.NpmRegistryMap{}; // Process scopes diff --git a/src/install/bun.lock.zig b/src/install/bun.lock.zig index 4a18b12e647e97..fef28d2b914963 100644 --- a/src/install/bun.lock.zig +++ b/src/install/bun.lock.zig @@ -246,11 +246,7 @@ pub const Stringifier = struct { // _ = this; // } - pub fn saveFromBinary(allocator: std.mem.Allocator, lockfile: *const BinaryLockfile) OOM!string { - var writer_buf = MutableString.initEmpty(allocator); - var buffered_writer = writer_buf.bufferedWriter(); - var writer = buffered_writer.writer(); - + pub fn saveFromBinary(allocator: std.mem.Allocator, lockfile: *const BinaryLockfile, writer: anytype) @TypeOf(writer).Error!void { const buf = lockfile.buffers.string_bytes.items; const deps_buf = lockfile.buffers.dependencies.items; const resolution_buf = lockfile.buffers.resolutions.items; @@ -594,15 +590,17 @@ pub const Stringifier = struct { TreeDepsSortCtx.isLessThan, ); - // first index is resolution for all dependency types - // npm -> [ "name@version", registry or "" (default), deps..., integrity, ... ] - // symlink -> [ "name@link:path", deps..., ... ] - // folder -> [ "name@path", deps..., ... ] - // workspace -> [ "name@workspace:path", version or "", deps..., ... ] - // tarball -> [ "name@tarball", deps..., ... ] - // root -> [ "name@root:", bins ] - // git -> [ "name@git+repo", deps..., ... ] - // github -> [ "name@github:user/repo", deps..., ... ] + // INFO = { prod/dev/optional/peer dependencies, os, cpu, libc (TODO), bin, binDir } + + // first index is resolution for each type of package + // npm -> [ "name@version", registry (TODO: remove if default), INFO, integrity] + // symlink -> [ "name@link:path", INFO ] + // folder -> [ "name@file:path", INFO ] + // workspace -> [ "name@workspace:path", INFO ] + // tarball -> [ "name@tarball", INFO ] + // root -> [ "name@root:", { bin, binDir } ] + // git -> [ "name@git+repo", INFO, .bun-tag string (TODO: remove this) ] + // github -> [ "name@github:user/repo", INFO, .bun-tag string (TODO: remove this) ] switch (res.tag) { .root => { @@ -716,9 +714,6 @@ pub const Stringifier = struct { } try decIndent(writer, indent); try writer.writeAll("}\n"); - - try buffered_writer.flush(); - return writer_buf.list.items; } /// Writes a single line object. Contains dependencies, os, cpu, libc (soon), and bin @@ -1218,21 +1213,26 @@ pub fn parseIntoBinaryLockfile( var optional_peers_buf: std.AutoHashMapUnmanaged(u64, void) = .{}; defer optional_peers_buf.deinit(allocator); - if (maybe_root_pkg) |root_pkg| { - const maybe_name = if (root_pkg.get("name")) |name| name.asString(allocator) orelse { + const root_pkg_exr = maybe_root_pkg orelse { + try log.addError(source, workspaces.loc, "Expected root package"); + return error.InvalidWorkspaceObject; + }; + + { + const maybe_name = if (root_pkg_exr.get("name")) |name| name.asString(allocator) orelse { try log.addError(source, name.loc, "Expected a string"); return error.InvalidWorkspaceObject; } else null; - const off, var len = try parseAppendDependencies(lockfile, allocator, &root_pkg, &string_buf, log, source, &optional_peers_buf); + const off, var len = try parseAppendDependencies(lockfile, allocator, &root_pkg_exr, &string_buf, log, source, &optional_peers_buf); - var pkg: BinaryLockfile.Package = .{}; - pkg.meta.id = 0; + var root_pkg: BinaryLockfile.Package = .{}; + root_pkg.meta.id = 0; if (maybe_name) |name| { const name_hash = String.Builder.stringHash(name); - pkg.name = try string_buf.appendWithHash(name, name_hash); - pkg.name_hash = name_hash; + root_pkg.name = try string_buf.appendWithHash(name, name_hash); + root_pkg.name_hash = name_hash; } workspaces: for (lockfile.workspace_paths.values()) |workspace_path| { @@ -1262,15 +1262,12 @@ pub fn parseIntoBinaryLockfile( } } - pkg.dependencies = .{ .off = off, .len = len }; - pkg.resolutions = .{ .off = off, .len = len }; + root_pkg.dependencies = .{ .off = off, .len = len }; + root_pkg.resolutions = .{ .off = off, .len = len }; - pkg.meta.id = 0; - try lockfile.packages.append(allocator, pkg); - try lockfile.getOrPutID(0, pkg.name_hash); - } else { - try log.addError(source, workspaces.loc, "Expected root package"); - return error.InvalidWorkspaceObject; + root_pkg.meta.id = 0; + try lockfile.packages.append(allocator, root_pkg); + try lockfile.getOrPutID(0, root_pkg.name_hash); } var pkg_map = bun.StringArrayHashMap(PackageID).init(allocator); @@ -1491,9 +1488,15 @@ pub fn parseIntoBinaryLockfile( const dep_id: DependencyID = @intCast(_dep_id); const dep = lockfile.buffers.dependencies.items[dep_id]; - if (pkg_map.get(dep.name.slice(lockfile.buffers.string_bytes.items))) |res_pkg_id| { - lockfile.buffers.resolutions.items[dep_id] = res_pkg_id; - } + const res_pkg_id = pkg_map.get(dep.name.slice(lockfile.buffers.string_bytes.items)) orelse { + if (dep.behavior.optional) { + continue; + } + try dependencyResolutionFailure(&dep, null, allocator, lockfile.buffers.string_bytes.items, source, log, root_pkg_exr.loc); + return error.InvalidPackageInfo; + }; + + lockfile.buffers.resolutions.items[dep_id] = res_pkg_id; } // TODO(dylan-conway) should we handle workspaces separately here for custom hoisting @@ -1533,10 +1536,10 @@ pub fn parseIntoBinaryLockfile( } if (offset == 0) { - if (dep.behavior.isOptionalPeer()) { + if (dep.behavior.optional) { continue :deps; } - try log.addError(source, key.loc, "Unable to resolve dependencies for package"); + try dependencyResolutionFailure(&dep, pkg_path, allocator, lockfile.buffers.string_bytes.items, source, log, key.loc); return error.InvalidPackageInfo; } @@ -1584,7 +1587,7 @@ pub fn parseIntoBinaryLockfile( } } - lockfile.hoist(log, if (manager) |pm| pm.options.local_package_features.dev_dependencies else true) catch |err| { + lockfile.hoist(log, .resolvable, {}) catch |err| { switch (err) { error.OutOfMemory => |oom| return oom, else => { @@ -1599,6 +1602,32 @@ pub fn parseIntoBinaryLockfile( lockfile.initEmpty(allocator); } +fn dependencyResolutionFailure(dep: *const Dependency, pkg_path: ?string, allocator: std.mem.Allocator, buf: string, source: *const logger.Source, log: *logger.Log, loc: logger.Loc) OOM!void { + const behavior_str = if (dep.behavior.isDev()) + "dev" + else if (dep.behavior.isOptional()) + "optional" + else if (dep.behavior.isPeer()) + "peer" + else if (dep.behavior.isWorkspaceOnly()) + "workspace" + else + "prod"; + + if (pkg_path) |path| { + try log.addErrorFmt(source, loc, allocator, "Failed to resolve {s} dependency '{s}' for package '{s}'", .{ + behavior_str, + dep.name.slice(buf), + path, + }); + } else { + try log.addErrorFmt(source, loc, allocator, "Failed to resolve root {s} dependency '{s}'", .{ + behavior_str, + dep.name.slice(buf), + }); + } +} + fn parseAppendDependencies( lockfile: *BinaryLockfile, allocator: std.mem.Allocator, diff --git a/src/install/extract_tarball.zig b/src/install/extract_tarball.zig index 8959383769515d..e8b8fb0c481d88 100644 --- a/src/install/extract_tarball.zig +++ b/src/install/extract_tarball.zig @@ -53,8 +53,8 @@ pub fn buildURL( full_name_: strings.StringOrTinyString, version: Semver.Version, string_buf: []const u8, -) !string { - return try buildURLWithPrinter( +) OOM!string { + return buildURLWithPrinter( registry_, full_name_, version, diff --git a/src/install/install.zig b/src/install/install.zig index efbe502056168c..521594194fe2f7 100644 --- a/src/install/install.zig +++ b/src/install/install.zig @@ -477,13 +477,17 @@ const NetworkTask = struct { this.http.schedule(this.allocator, batch); } + pub const ForTarballError = OOM || error{ + InvalidURL, + }; + pub fn forTarball( this: *NetworkTask, allocator: std.mem.Allocator, tarball_: *const ExtractTarball, scope: *const Npm.Registry.Scope, authorization: NetworkTask.Authorization, - ) !void { + ) ForTarballError!void { this.callback = .{ .extract = tarball_.* }; const tarball = &this.callback.extract; const tarball_url = tarball.url.slice(); @@ -504,11 +508,11 @@ const NetworkTask = struct { .args = .{ bun.fmt.QuotedFormatter{ .text = this.url_buf }, bun.fmt.QuotedFormatter{ .text = tarball.name.slice() } }, }; - this.package_manager.log.addErrorFmt(null, .{}, allocator, msg.fmt, msg.args) catch unreachable; + try this.package_manager.log.addErrorFmt(null, .{}, allocator, msg.fmt, msg.args); return error.InvalidURL; } - this.response_buffer = try MutableString.init(allocator, 0); + this.response_buffer = MutableString.initEmpty(allocator); this.allocator = allocator; var header_builder = HeaderBuilder{}; @@ -1024,10 +1028,6 @@ pub fn NewPackageInstall(comptime kind: PkgInstallKind) type { skipped: u32 = 0, successfully_installed: ?Bitset = null, - /// The lockfile used by `installPackages`. Might be different from the lockfile - /// on disk if `--production` is used and dev dependencies are removed. - lockfile_used_for_install: *Lockfile, - /// Package name hash -> number of scripts skipped. /// Multiple versions of the same package might add to the count, and each version /// might have a different number of scripts @@ -4245,8 +4245,6 @@ pub const PackageManager = struct { }; } else if (behavior.isPeer() and !install_peer) { return null; - } else if (behavior.isOptional() and !this.options.remote_package_features.optional_dependencies) { - return null; } // appendPackage sets the PackageID on the package @@ -4279,24 +4277,26 @@ pub const PackageManager = struct { // We don't need to download the tarball, but we should enqueue dependencies .done => .{ .package = package, .is_first_time = true }, // Do we need to download the tarball? - .extract => .{ - .package = package, - .is_first_time = true, - .task = .{ - .network_task = try this.generateNetworkTaskForTarball( - Task.Id.forNPMPackage( - this.lockfile.str(&name), - package.resolution.value.npm.version, - ), - manifest.str(&find_result.package.tarball_url), - dependency.behavior.isRequired(), - dependency_id, - package, - name_and_version_hash, - // its npm. - .allow_authorization, - ) orelse unreachable, - }, + .extract => extract: { + const task_id = Task.Id.forNPMPackage(this.lockfile.str(&name), package.resolution.value.npm.version); + bun.debugAssert(!this.network_dedupe_map.contains(task_id)); + + break :extract .{ + .package = package, + .is_first_time = true, + .task = .{ + .network_task = try this.generateNetworkTaskForTarball( + task_id, + manifest.str(&find_result.package.tarball_url), + dependency.behavior.isRequired(), + dependency_id, + package, + name_and_version_hash, + // its npm. + .allow_authorization, + ) orelse unreachable, + }, + }; }, .calc_patch_hash => .{ .package = package, @@ -4354,7 +4354,7 @@ pub const PackageManager = struct { package: Lockfile.Package, patch_name_and_version_hash: ?u64, authorization: NetworkTask.Authorization, - ) !?*NetworkTask { + ) NetworkTask.ForTarballError!?*NetworkTask { if (this.hasCreatedNetworkTask(task_id, is_required)) { return null; } @@ -4380,21 +4380,21 @@ pub const PackageManager = struct { this.allocator, &.{ .package_manager = this, - .name = try strings.StringOrTinyString.initAppendIfNeeded( + .name = strings.StringOrTinyString.initAppendIfNeeded( this.lockfile.str(&package.name), *FileSystem.FilenameStore, FileSystem.FilenameStore.instance, - ), + ) catch bun.outOfMemory(), .resolution = package.resolution, .cache_dir = this.getCacheDirectory(), .temp_dir = this.getTemporaryDirectory(), .dependency_id = dependency_id, .integrity = package.meta.integrity, - .url = try strings.StringOrTinyString.initAppendIfNeeded( + .url = strings.StringOrTinyString.initAppendIfNeeded( url, *FileSystem.FilenameStore, FileSystem.FilenameStore.instance, - ), + ) catch bun.outOfMemory(), }, scope, authorization, @@ -5344,15 +5344,13 @@ pub const PackageManager = struct { this.allocator, this.scopeForPackageName(name_str), if (loaded_manifest) |*manifest| manifest else null, - dependency.behavior.isOptional() or !this.options.do.install_peer_dependencies, + dependency.behavior.isOptional(), ); this.enqueueNetworkTask(network_task); } } else { - if (this.options.do.install_peer_dependencies) { - try this.peer_dependencies.writeItem(id); - return; - } + try this.peer_dependencies.writeItem(id); + return; } var manifest_entry_parse = try this.task_queue.getOrPutContext(this.allocator, task_id, .{}); @@ -5424,9 +5422,7 @@ pub const PackageManager = struct { if (dependency.behavior.isPeer()) { if (!install_peer) { - if (this.options.do.install_peer_dependencies) { - try this.peer_dependencies.writeItem(id); - } + try this.peer_dependencies.writeItem(id); return; } } @@ -5449,9 +5445,7 @@ pub const PackageManager = struct { if (dependency.behavior.isPeer()) { if (!install_peer) { - if (this.options.do.install_peer_dependencies) { - try this.peer_dependencies.writeItem(id); - } + try this.peer_dependencies.writeItem(id); return; } } @@ -5501,9 +5495,7 @@ pub const PackageManager = struct { if (dependency.behavior.isPeer()) { if (!install_peer) { - if (this.options.do.install_peer_dependencies) { - try this.peer_dependencies.writeItem(id); - } + try this.peer_dependencies.writeItem(id); return; } } @@ -5690,9 +5682,7 @@ pub const PackageManager = struct { if (dependency.behavior.isPeer()) { if (!install_peer) { - if (this.options.do.install_peer_dependencies) { - try this.peer_dependencies.writeItem(id); - } + try this.peer_dependencies.writeItem(id); return; } } @@ -7039,6 +7029,7 @@ pub const PackageManager = struct { .optional_dependencies = true, }, local_package_features: Features = .{ + .optional_dependencies = true, .dev_dependencies = true, .workspaces = true, }, @@ -7269,11 +7260,18 @@ pub const PackageManager = struct { if (config.save_dev) |save| { this.local_package_features.dev_dependencies = save; + // remote packages should never install dev dependencies + // (TODO: unless git dependency with postinstalls) + } + + if (config.save_optional) |save| { + this.remote_package_features.optional_dependencies = save; + this.local_package_features.optional_dependencies = save; } if (config.save_peer) |save| { - this.do.install_peer_dependencies = save; this.remote_package_features.peer_dependencies = save; + this.local_package_features.peer_dependencies = save; } if (config.exact) |exact| { @@ -7303,11 +7301,6 @@ pub const PackageManager = struct { this.max_concurrent_lifecycle_scripts = jobs; } - if (config.save_optional) |save| { - this.remote_package_features.optional_dependencies = save; - this.local_package_features.optional_dependencies = save; - } - this.explicit_global_directory = config.global_dir orelse this.explicit_global_directory; } @@ -7445,8 +7438,22 @@ pub const PackageManager = struct { this.enable.manifest_cache_control = false; } - if (cli.omit.dev) { - this.local_package_features.dev_dependencies = false; + if (cli.omit) |omit| { + if (omit.dev) { + this.local_package_features.dev_dependencies = false; + // remote packages should never install dev dependencies + // (TODO: unless git dependency with postinstalls) + } + + if (omit.optional) { + this.local_package_features.optional_dependencies = false; + this.remote_package_features.optional_dependencies = false; + } + + if (omit.peer) { + this.local_package_features.peer_dependencies = false; + this.remote_package_features.peer_dependencies = false; + } } if (cli.global or cli.ignore_scripts) { @@ -7461,8 +7468,6 @@ pub const PackageManager = struct { this.save_text_lockfile = save_text_lockfile; } - this.local_package_features.optional_dependencies = !cli.omit.optional; - const disable_progress_bar = default_disable_progress_bar or cli.no_progress; if (cli.verbose) { @@ -7568,7 +7573,6 @@ pub const PackageManager = struct { print_meta_hash_string: bool = false, verify_integrity: bool = true, summary: bool = true, - install_peer_dependencies: bool = true, trust_dependencies_from_args: bool = false, update_to_latest: bool = false, }; @@ -9434,6 +9438,7 @@ pub const PackageManager = struct { clap.parseParam("--concurrent-scripts Maximum number of concurrent jobs for lifecycle scripts (default 5)") catch unreachable, clap.parseParam("--network-concurrency Maximum number of concurrent network requests (default 48)") catch unreachable, clap.parseParam("--save-text-lockfile Save a text-based lockfile") catch unreachable, + clap.parseParam("--omit ... Exclude 'dev', 'optional', or 'peer' dependencies from install") catch unreachable, clap.parseParam("-h, --help Print this help menu") catch unreachable, }; @@ -9546,8 +9551,7 @@ pub const PackageManager = struct { development: bool = false, optional: bool = false, - no_optional: bool = false, - omit: Omit = Omit{}, + omit: ?Omit = null, exact: bool = false, @@ -9574,16 +9578,8 @@ pub const PackageManager = struct { const Omit = struct { dev: bool = false, - optional: bool = true, + optional: bool = false, peer: bool = false, - - pub inline fn toFeatures(this: Omit) Features { - return .{ - .dev_dependencies = this.dev, - .optional_dependencies = this.optional, - .peer_dependencies = this.peer, - }; - } }; pub fn printHelp(subcommand: Subcommand) void { @@ -9920,6 +9916,25 @@ pub const PackageManager = struct { cli.save_text_lockfile = true; } + const omit_values = args.options("--omit"); + + if (omit_values.len > 0) { + var omit: Omit = .{}; + for (omit_values) |omit_value| { + if (strings.eqlComptime(omit_value, "dev")) { + omit.dev = true; + } else if (strings.eqlComptime(omit_value, "optional")) { + omit.optional = true; + } else if (strings.eqlComptime(omit_value, "peer")) { + omit.peer = true; + } else { + Output.errGeneric("invalid `omit` value: '{s}'", .{omit_value}); + Global.crash(); + } + } + cli.omit = omit; + } + // commands that support --filter if (comptime subcommand.supportsWorkspaceFiltering()) { cli.filters = args.options("--filter"); @@ -12271,6 +12286,7 @@ pub const PackageManager = struct { options: *const PackageManager.Options, metas: []const Lockfile.Package.Meta, names: []const String, + pkg_dependencies: []const Lockfile.DependencySlice, pkg_name_hashes: []const PackageNameHash, bins: []const Bin, resolutions: []Resolution, @@ -12649,6 +12665,7 @@ pub const PackageManager = struct { this.pkg_name_hashes = packages.items(.name_hash); this.bins = packages.items(.bin); this.resolutions = packages.items(.resolution); + this.pkg_dependencies = packages.items(.dependencies); // fixes an assertion failure where a transitive dependency is a git dependency newly added to the lockfile after the list of dependencies has been resized // this assertion failure would also only happen after the lockfile has been written to disk and the summary is being printed. @@ -13084,7 +13101,14 @@ pub const PackageManager = struct { url, context, patch_name_and_version_hash, - ); + ) catch |err| switch (err) { + error.OutOfMemory => bun.outOfMemory(), + error.InvalidURL => this.failWithInvalidUrl( + pkg_has_patch, + is_pending_package_install, + log_level, + ), + }; }, .local_tarball => { this.manager.enqueueTarballForReading( @@ -13101,7 +13125,14 @@ pub const PackageManager = struct { resolution.value.remote_tarball.slice(this.lockfile.buffers.string_bytes.items), context, patch_name_and_version_hash, - ); + ) catch |err| switch (err) { + error.OutOfMemory => bun.outOfMemory(), + error.InvalidURL => this.failWithInvalidUrl( + pkg_has_patch, + is_pending_package_install, + log_level, + ), + }; }, .npm => { if (comptime Environment.isDebug) { @@ -13123,7 +13154,14 @@ pub const PackageManager = struct { resolution.value.npm.url.slice(this.lockfile.buffers.string_bytes.items), context, patch_name_and_version_hash, - ); + ) catch |err| switch (err) { + error.OutOfMemory => bun.outOfMemory(), + error.InvalidURL => this.failWithInvalidUrl( + pkg_has_patch, + is_pending_package_install, + log_level, + ), + }; }, else => { if (comptime Environment.allow_assert) { @@ -13414,6 +13452,16 @@ pub const PackageManager = struct { } } + fn failWithInvalidUrl( + this: *PackageInstaller, + pkg_has_patch: bool, + comptime is_pending_package_install: bool, + comptime log_level: Options.LogLevel, + ) void { + this.summary.fail += 1; + if (!pkg_has_patch) this.incrementTreeInstallCount(this.current_tree_id, null, !is_pending_package_install, log_level); + } + // returns true if scripts are enqueued fn enqueueLifecycleScripts( this: *PackageInstaller, @@ -13484,47 +13532,18 @@ pub const PackageManager = struct { pub fn installPackage( this: *PackageInstaller, - dependency_id: DependencyID, - comptime log_level: Options.LogLevel, - ) void { - this.installPackageImpl(dependency_id, log_level, true); - } - - pub fn installPackageImpl( - this: *PackageInstaller, - dependency_id: DependencyID, + dep_id: DependencyID, comptime log_level: Options.LogLevel, - comptime increment_tree_count: bool, ) void { - const package_id = this.lockfile.buffers.resolutions.items[dependency_id]; - const meta = &this.metas[package_id]; - const is_pending_package_install = false; - - if (meta.isDisabled()) { - if (comptime log_level.showProgress()) { - this.node.completeOne(); - } - if (comptime log_level.isVerbose()) { - const name = this.lockfile.str(&this.names[package_id]); - if (!meta.os.isMatch() and !meta.arch.isMatch()) { - Output.prettyErrorln("Skip installing '{s}' cpu & os mismatch", .{name}); - } else if (!meta.os.isMatch()) { - Output.prettyErrorln("Skip installing '{s}' os mismatch", .{name}); - } else if (!meta.arch.isMatch()) { - Output.prettyErrorln("Skip installing '{s}' cpu mismatch", .{name}); - } - } - - if (comptime increment_tree_count) this.incrementTreeInstallCount(this.current_tree_id, null, !is_pending_package_install, log_level); - return; - } + const package_id = this.lockfile.buffers.resolutions.items[dep_id]; const name = this.names[package_id]; const resolution = &this.resolutions[package_id]; const needs_verify = true; + const is_pending_package_install = false; this.installPackageWithNameAndResolution( - dependency_id, + dep_id, package_id, log_level, name, @@ -13587,6 +13606,8 @@ pub const PackageManager = struct { } } + const EnqueuePackageForDownloadError = NetworkTask.ForTarballError; + pub fn enqueuePackageForDownload( this: *PackageManager, name: []const u8, @@ -13596,23 +13617,23 @@ pub const PackageManager = struct { url: []const u8, task_context: TaskCallbackContext, patch_name_and_version_hash: ?u64, - ) void { + ) EnqueuePackageForDownloadError!void { const task_id = Task.Id.forNPMPackage(name, version); - var task_queue = this.task_queue.getOrPut(this.allocator, task_id) catch unreachable; + var task_queue = try this.task_queue.getOrPut(this.allocator, task_id); if (!task_queue.found_existing) { task_queue.value_ptr.* = .{}; } - task_queue.value_ptr.append( + try task_queue.value_ptr.append( this.allocator, task_context, - ) catch unreachable; + ); if (task_queue.found_existing) return; const is_required = this.lockfile.buffers.dependencies.items[dependency_id].behavior.isRequired(); - if (this.generateNetworkTaskForTarball( + if (try this.generateNetworkTaskForTarball( task_id, url, is_required, @@ -13620,7 +13641,7 @@ pub const PackageManager = struct { this.lockfile.packages.get(package_id), patch_name_and_version_hash, .allow_authorization, - ) catch unreachable) |task| { + )) |task| { task.schedule(&this.network_tarball_batch); if (this.network_tarball_batch.len > 0) { _ = this.scheduleTasks(); @@ -13628,6 +13649,8 @@ pub const PackageManager = struct { } } + const EnqueueTarballForDownloadError = NetworkTask.ForTarballError; + pub fn enqueueTarballForDownload( this: *PackageManager, dependency_id: DependencyID, @@ -13635,21 +13658,21 @@ pub const PackageManager = struct { url: string, task_context: TaskCallbackContext, patch_name_and_version_hash: ?u64, - ) void { + ) EnqueueTarballForDownloadError!void { const task_id = Task.Id.forTarball(url); - var task_queue = this.task_queue.getOrPut(this.allocator, task_id) catch unreachable; + var task_queue = try this.task_queue.getOrPut(this.allocator, task_id); if (!task_queue.found_existing) { task_queue.value_ptr.* = .{}; } - task_queue.value_ptr.append( + try task_queue.value_ptr.append( this.allocator, task_context, - ) catch unreachable; + ); if (task_queue.found_existing) return; - if (this.generateNetworkTaskForTarball( + if (try this.generateNetworkTaskForTarball( task_id, url, this.lockfile.buffers.dependencies.items[dependency_id].behavior.isRequired(), @@ -13657,7 +13680,7 @@ pub const PackageManager = struct { this.lockfile.packages.get(package_id), patch_name_and_version_hash, .no_authorization, - ) catch unreachable) |task| { + )) |task| { task.schedule(&this.network_tarball_batch); if (this.network_tarball_batch.len > 0) { _ = this.scheduleTasks(); @@ -13721,15 +13744,14 @@ pub const PackageManager = struct { ctx: Command.Context, comptime log_level: PackageManager.Options.LogLevel, ) !PackageInstall.Summary { - const original_lockfile = this.lockfile; - defer this.lockfile = original_lockfile; - if (!this.options.local_package_features.dev_dependencies) { - this.lockfile = try this.lockfile.maybeCloneFilteringRootPackages( - this, - this.options.local_package_features, - this.options.enable.exact_versions, - log_level, - ); + const original_trees = this.lockfile.buffers.trees; + const original_tree_dep_ids = this.lockfile.buffers.hoisted_dependencies; + + try this.lockfile.hoist(this.log, .filter, this); + + defer { + this.lockfile.buffers.trees = original_trees; + this.lockfile.buffers.hoisted_dependencies = original_tree_dep_ids; } var root_node: *Progress.Node = undefined; @@ -13744,7 +13766,7 @@ pub const PackageManager = struct { root_node = progress.start("", 0); download_node = root_node.start(ProgressStrings.download(), 0); - install_node = root_node.start(ProgressStrings.install(), this.lockfile.packages.len); + install_node = root_node.start(ProgressStrings.install(), this.lockfile.buffers.hoisted_dependencies.items.len); scripts_node = root_node.start(ProgressStrings.script(), 0); this.downloads_node = &download_node; this.scripts_node = &scripts_node; @@ -13795,9 +13817,7 @@ pub const PackageManager = struct { skip_delete = false; } - var summary = PackageInstall.Summary{ - .lockfile_used_for_install = this.lockfile, - }; + var summary = PackageInstall.Summary{}; { var iterator = Lockfile.Tree.Iterator(.node_modules).init(this.lockfile); @@ -13895,6 +13915,7 @@ pub const PackageManager = struct { .names = parts.items(.name), .pkg_name_hashes = parts.items(.name_hash), .resolutions = parts.items(.resolution), + .pkg_dependencies = parts.items(.dependencies), .lockfile = this.lockfile, .node = &install_node, .node_modules = .{ @@ -14671,9 +14692,7 @@ pub const PackageManager = struct { try waitForEverythingExceptPeers(manager); } - if (manager.options.do.install_peer_dependencies) { - try waitForPeers(manager); - } + try waitForPeers(manager); if (comptime log_level.showProgress()) { manager.endProgressBar(); @@ -14802,9 +14821,7 @@ pub const PackageManager = struct { const lockfile_before_install = manager.lockfile; - var install_summary = PackageInstall.Summary{ - .lockfile_used_for_install = manager.lockfile, - }; + var install_summary = PackageInstall.Summary{}; if (manager.options.do.install_packages) { install_summary = try manager.installPackages( ctx, @@ -14968,7 +14985,7 @@ pub const PackageManager = struct { if (comptime log_level != .silent) { if (manager.options.do.summary) { var printer = Lockfile.Printer{ - .lockfile = install_summary.lockfile_used_for_install, + .lockfile = manager.lockfile, .options = manager.options, .updates = manager.update_requests, .successfully_installed = install_summary.successfully_installed, @@ -15088,6 +15105,7 @@ pub const PackageManager = struct { const lockfile = this.lockfile; const resolutions_lists: []const Lockfile.DependencyIDSlice = lockfile.packages.items(.resolutions); const dependency_lists: []const Lockfile.DependencySlice = lockfile.packages.items(.dependencies); + const pkg_resolutions = lockfile.packages.items(.resolution); const dependencies_buffer = lockfile.buffers.dependencies.items; const resolutions_buffer = lockfile.buffers.resolutions.items; const end: PackageID = @truncate(lockfile.packages.len); @@ -15095,7 +15113,6 @@ pub const PackageManager = struct { var any_failed = false; const string_buf = lockfile.buffers.string_bytes.items; - const root_list = resolutions_lists[0]; for (resolutions_lists, dependency_lists, 0..) |resolution_list, dependency_list, parent_id| { for (resolution_list.get(resolutions_buffer), dependency_list.get(dependencies_buffer)) |package_id, failed_dep| { if (package_id < end) continue; @@ -15104,13 +15121,13 @@ pub const PackageManager = struct { // Need to keep this for now because old lockfiles might have a peer dependency without the optional flag set. if (failed_dep.behavior.isPeer()) continue; + const features = switch (pkg_resolutions[parent_id].tag) { + .root, .workspace, .folder => this.options.local_package_features, + else => this.options.remote_package_features, + }; // even if optional dependencies are enabled, it's still allowed to fail - if (failed_dep.behavior.optional or !failed_dep.behavior.isEnabled( - if (root_list.contains(@truncate(parent_id))) - this.options.local_package_features - else - this.options.remote_package_features, - )) continue; + if (failed_dep.behavior.optional or !failed_dep.behavior.isEnabled(features)) continue; + if (log_level != .silent) { if (failed_dep.name.isEmpty() or strings.eqlLong(failed_dep.name.slice(string_buf), failed_dep.version.literal.slice(string_buf), true)) { Output.errGeneric("{} failed to resolve", .{ diff --git a/src/install/lockfile.zig b/src/install/lockfile.zig index d6f7f7e3c9fcb8..515cc803ad8069 100644 --- a/src/install/lockfile.zig +++ b/src/install/lockfile.zig @@ -373,15 +373,23 @@ pub fn loadFromDir( switch (result) { .ok => { - if (bun.getenvZ("BUN_DEBUG_TEST_TEXT_LOCKFILE") != null) { + if (bun.getenvZ("BUN_DEBUG_TEST_TEXT_LOCKFILE") != null and manager != null) { // Convert the loaded binary lockfile into a text lockfile in memory, then // parse it back into a binary lockfile. - const text_lockfile_bytes = TextLockfile.Stringifier.saveFromBinary(allocator, result.ok.lockfile) catch |err| { + var writer_buf = MutableString.initEmpty(allocator); + var buffered_writer = writer_buf.bufferedWriter(); + const writer = buffered_writer.writer(); + + TextLockfile.Stringifier.saveFromBinary(allocator, result.ok.lockfile, writer) catch |err| { Output.panic("failed to convert binary lockfile to text lockfile: {s}", .{@errorName(err)}); }; + buffered_writer.flush() catch bun.outOfMemory(); + + const text_lockfile_bytes = writer_buf.list.items; + const source = logger.Source.initPathString("bun.lock", text_lockfile_bytes); initializeStore(); const json = JSON.parsePackageJSONUTF8(&source, log, allocator) catch |err| { @@ -485,7 +493,7 @@ pub const Tree = struct { pub const root_dep_id: DependencyID = invalid_package_id - 1; pub const invalid_id: Id = std.math.maxInt(Id); - pub const HoistResult = union(enum) { + pub const HoistDependencyResult = union(enum) { dependency_loop, hoisted, dest_id: Id, @@ -652,82 +660,96 @@ pub const Tree = struct { return .{ rel, depth }; } - const Builder = struct { - allocator: Allocator, - name_hashes: []const PackageNameHash, - list: bun.MultiArrayList(Entry) = .{}, - resolutions: []const PackageID, - dependencies: []const Dependency, - resolution_lists: []const Lockfile.DependencyIDSlice, - queue: Lockfile.TreeFiller, - log: *logger.Log, - lockfile: *Lockfile, - prefer_dev_dependencies: bool = false, - sort_buf: std.ArrayListUnmanaged(DependencyID) = .{}, + const BuilderMethod = enum { + /// Hoist, but include every dependency so it's resolvable if configuration + /// changes. For saving to disk. + resolvable, - pub fn maybeReportError(this: *Builder, comptime fmt: string, args: anytype) void { - this.log.addErrorFmt(null, logger.Loc.Empty, this.allocator, fmt, args) catch {}; - } + /// This will filter out disabled dependencies, resulting in more aggresive + /// hoisting compared to `hoist()`. We skip dependencies based on 'os', 'cpu', + /// 'libc' (TODO), and omitted dependency types (`--omit=dev/peer/optional`). + /// Dependencies of a disabled package are not included in the output. + filter, + }; - pub fn buf(this: *const Builder) []const u8 { - return this.lockfile.buffers.string_bytes.items; - } + pub fn Builder(comptime method: BuilderMethod) type { + return struct { + allocator: Allocator, + name_hashes: []const PackageNameHash, + list: bun.MultiArrayList(Entry) = .{}, + resolutions: []const PackageID, + dependencies: []const Dependency, + resolution_lists: []const Lockfile.DependencyIDSlice, + queue: Lockfile.TreeFiller, + log: *logger.Log, + lockfile: *const Lockfile, + manager: if (method == .filter) *const PackageManager else void, + sort_buf: std.ArrayListUnmanaged(DependencyID) = .{}, - pub fn packageName(this: *Builder, id: PackageID) String.Formatter { - return this.lockfile.packages.items(.name)[id].fmt(this.lockfile.buffers.string_bytes.items); - } + pub fn maybeReportError(this: *@This(), comptime fmt: string, args: anytype) void { + this.log.addErrorFmt(null, logger.Loc.Empty, this.allocator, fmt, args) catch {}; + } - pub fn packageVersion(this: *Builder, id: PackageID) Resolution.Formatter { - return this.lockfile.packages.items(.resolution)[id].fmt(this.lockfile.buffers.string_bytes.items, .auto); - } + pub fn buf(this: *const @This()) []const u8 { + return this.lockfile.buffers.string_bytes.items; + } - pub const Entry = struct { - tree: Tree, - dependencies: Lockfile.DependencyIDList, - }; + pub fn packageName(this: *@This(), id: PackageID) String.Formatter { + return this.lockfile.packages.items(.name)[id].fmt(this.lockfile.buffers.string_bytes.items); + } - /// 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)); - var i: Id = 0; - var total: u32 = 0; - const trees = this.list.items(.tree); - const dependencies = this.list.items(.dependencies); - - while (i < end) : (i += 1) { - total += trees[i].dependencies.len; - } - - var dependency_ids = try DependencyIDList.initCapacity(z_allocator, total); - var next = PackageIDSlice{}; - - for (trees, dependencies) |*tree, *child| { - if (tree.dependencies.len > 0) { - const len = @as(PackageID, @truncate(child.items.len)); - next.off += next.len; - next.len = len; - tree.dependencies = next; - dependency_ids.appendSliceAssumeCapacity(child.items); - child.deinit(this.allocator); - } + pub fn packageVersion(this: *@This(), id: PackageID) Resolution.Formatter { + return this.lockfile.packages.items(.resolution)[id].fmt(this.lockfile.buffers.string_bytes.items, .auto); } - this.queue.deinit(); - this.sort_buf.deinit(this.allocator); - return dependency_ids; - } - }; + pub const Entry = struct { + tree: Tree, + dependencies: Lockfile.DependencyIDList, + }; + + /// Flatten the multi-dimensional ArrayList of package IDs into a single easily serializable array + pub fn clean(this: *@This()) OOM!DependencyIDList { + var total: u32 = 0; + const trees = this.list.items(.tree); + const dependencies = this.list.items(.dependencies); + + for (trees) |*tree| { + total += tree.dependencies.len; + } + + var dependency_ids = try DependencyIDList.initCapacity(z_allocator, total); + var next = PackageIDSlice{}; + + for (trees, dependencies) |*tree, *child| { + if (tree.dependencies.len > 0) { + const len = @as(PackageID, @truncate(child.items.len)); + next.off += next.len; + next.len = len; + tree.dependencies = next; + dependency_ids.appendSliceAssumeCapacity(child.items); + child.deinit(this.allocator); + } + } + this.queue.deinit(); + this.sort_buf.deinit(this.allocator); + + return dependency_ids; + } + }; + } pub fn processSubtree( this: *const Tree, dependency_id: DependencyID, - builder: *Builder, + comptime method: BuilderMethod, + builder: *Builder(method), + log_level: if (method == .filter) PackageManager.Options.LogLevel else void, ) SubtreeError!void { - const package_id = switch (dependency_id) { + const parent_pkg_id = switch (dependency_id) { root_dep_id => 0, else => |id| builder.resolutions[id], }; - const resolution_list = builder.resolution_lists[package_id]; + const resolution_list = builder.resolution_lists[parent_pkg_id]; if (resolution_list.len == 0) return; @@ -746,7 +768,11 @@ 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); + + const pkgs = builder.lockfile.packages.slice(); + const pkg_resolutions = pkgs.items(.resolution); + const pkg_metas = pkgs.items(.meta); + const pkg_names = pkgs.items(.name); builder.sort_buf.clearRetainingCapacity(); try builder.sort_buf.ensureUnusedCapacity(builder.allocator, resolution_list.len); @@ -783,21 +809,48 @@ pub const Tree = struct { ); for (builder.sort_buf.items) |dep_id| { - const pid = builder.resolutions[dep_id]; + const pkg_id = builder.resolutions[dep_id]; // Skip unresolved packages, e.g. "peerDependencies" - if (pid >= max_package_id) continue; + if (pkg_id >= max_package_id) continue; + + // filter out disabled dependencies + if (comptime method == .filter) { + if (builder.lockfile.isPackageDisabled( + dep_id, + switch (pkg_resolutions[parent_pkg_id].tag) { + .root, .workspace, .folder => builder.manager.options.local_package_features, + else => builder.manager.options.remote_package_features, + }, + &pkg_metas[pkg_id], + )) { + if (log_level.isVerbose()) { + const meta = &pkg_metas[pkg_id]; + const name = builder.lockfile.str(&pkg_names[pkg_id]); + if (!meta.os.isMatch() and !meta.arch.isMatch()) { + Output.prettyErrorln("Skip installing '{s}' cpu & os mismatch", .{name}); + } else if (!meta.os.isMatch()) { + Output.prettyErrorln("Skip installing '{s}' os mismatch", .{name}); + } else if (!meta.arch.isMatch()) { + Output.prettyErrorln("Skip installing '{s}' cpu mismatch", .{name}); + } + } + + continue; + } + } const dependency = builder.dependencies[dep_id]; // Do not hoist folder dependencies - const res: HoistResult = if (resolutions[pid].tag == .folder) + const res: HoistDependencyResult = if (pkg_resolutions[pkg_id].tag == .folder) .{ .dest_id = next.id } else try next.hoistDependency( true, - pid, + pkg_id, &dependency, dependency_lists, trees, + method, builder, ); @@ -815,7 +868,7 @@ pub const Tree = struct { .dest_id => |dest_id| { dependency_lists[dest_id].append(builder.allocator, dep_id) catch bun.outOfMemory(); trees[dest_id].dependencies.len += 1; - if (builder.resolution_lists[pid].len > 0) { + if (builder.resolution_lists[pkg_id].len > 0) { try builder.queue.writeItem(.{ .tree_id = dest_id, .dependency_id = dep_id, @@ -843,8 +896,9 @@ pub const Tree = struct { dependency: *const Dependency, dependency_lists: []Lockfile.DependencyIDList, trees: []Tree, - builder: *Builder, - ) !HoistResult { + comptime method: BuilderMethod, + builder: *Builder(method), + ) !HoistDependencyResult { const this_dependencies = this.dependencies.get(dependency_lists[this.id].items); for (0..this_dependencies.len) |i| { const dep_id = this_dependencies[i]; @@ -857,14 +911,11 @@ pub const Tree = struct { } if (comptime as_defined) { - // same dev dependency as another package in the same package.json, but different version. - // choose dev dep over other if enabled if (dep.behavior.isDev() != dependency.behavior.isDev()) { - if (builder.prefer_dev_dependencies and dep.behavior.isDev()) { - return .hoisted; // 1 - } - - return .dependency_loop; // 3 + // will only happen in workspaces and root package because + // dev dependencies won't be included in other types of + // dependencies + return .hoisted; // 1 } } @@ -911,6 +962,7 @@ pub const Tree = struct { dependency, dependency_lists, trees, + method, builder, ) catch unreachable; if (!as_defined or id != .dependency_loop) return id; // 1 or 2 @@ -921,6 +973,19 @@ pub const Tree = struct { } }; +pub fn isPackageDisabled( + lockfile: *const Lockfile, + dep_id: DependencyID, + features: Features, + meta: *const Package.Meta, +) bool { + if (meta.isDisabled()) return true; + + const dep = lockfile.buffers.dependencies.items[dep_id]; + + return !dep.behavior.isEnabled(features); +} + /// This conditionally clones the lockfile with root packages marked as non-resolved /// that do not satisfy `Features`. The package may still end up installed even /// if it was e.g. in "devDependencies" and its a production install. In that case, @@ -1383,7 +1448,7 @@ const Cloner = struct { this.manager.clearCachedItemsDependingOnLockfileBuffer(); if (this.lockfile.packages.len != 0) { - try this.lockfile.hoist(this.log, this.manager.options.local_package_features.dev_dependencies); + try this.lockfile.hoist(this.log, .resolvable, {}); } // capacity is used for calculating byte size @@ -1393,10 +1458,15 @@ const Cloner = struct { } }; -pub fn hoist(lockfile: *Lockfile, log: *logger.Log, prefer_dev_dependencies: bool) Tree.SubtreeError!void { +pub fn hoist( + lockfile: *Lockfile, + log: *logger.Log, + comptime method: Tree.BuilderMethod, + manager: if (method == .filter) *PackageManager else void, +) Tree.SubtreeError!void { const allocator = lockfile.allocator; var slice = lockfile.packages.slice(); - var builder = Tree.Builder{ + var builder = Tree.Builder(method){ .name_hashes = slice.items(.name_hash), .queue = TreeFiller.init(allocator), .resolution_lists = slice.items(.resolutions), @@ -1405,23 +1475,17 @@ pub fn hoist(lockfile: *Lockfile, log: *logger.Log, prefer_dev_dependencies: boo .dependencies = lockfile.buffers.dependencies.items, .log = log, .lockfile = lockfile, - .prefer_dev_dependencies = prefer_dev_dependencies, + .manager = manager, }; - try (Tree{}).processSubtree(Tree.root_dep_id, &builder); + try (Tree{}).processSubtree(Tree.root_dep_id, method, &builder, if (method == .filter) manager.options.log_level else {}); // This goes breadth-first while (builder.queue.readItem()) |item| { - try builder.list.items(.tree)[item.tree_id].processSubtree(item.dependency_id, &builder); + try builder.list.items(.tree)[item.tree_id].processSubtree(item.dependency_id, method, &builder, if (method == .filter) manager.options.log_level else {}); } + lockfile.buffers.trees = std.ArrayListUnmanaged(Tree).fromOwnedSlice(builder.list.items(.tree)); lockfile.buffers.hoisted_dependencies = try builder.clean(); - { - const final = builder.list.items(.tree); - lockfile.buffers.trees = .{ - .items = final, - .capacity = final.len, - }; - } } const PendingResolution = struct { @@ -1581,6 +1645,7 @@ pub const Printer = struct { const dependencies = lockfile.buffers.dependencies.items; const workspace_res = packages_slice.items(.resolution)[workspace_package_id]; const names = packages_slice.items(.name); + const pkg_metas = packages_slice.items(.meta); bun.assert(workspace_res.tag == .workspace or workspace_res.tag == .root); const resolutions_list = packages_slice.items(.resolutions); var printed_section_header = false; @@ -1588,7 +1653,7 @@ pub const Printer = struct { // find the updated packages for (resolutions_list[workspace_package_id].begin()..resolutions_list[workspace_package_id].end()) |dep_id| { - switch (shouldPrintPackageInstall(this, manager, @intCast(dep_id), installed, id_map)) { + switch (shouldPrintPackageInstall(this, manager, @intCast(dep_id), installed, id_map, pkg_metas)) { .yes, .no, .@"return" => {}, .update => |update_info| { printed_new_install.* = true; @@ -1610,7 +1675,7 @@ pub const Printer = struct { } for (resolutions_list[workspace_package_id].begin()..resolutions_list[workspace_package_id].end()) |dep_id| { - switch (shouldPrintPackageInstall(this, manager, @intCast(dep_id), installed, id_map)) { + switch (shouldPrintPackageInstall(this, manager, @intCast(dep_id), installed, id_map, pkg_metas)) { .@"return" => return, .yes => {}, .no, .update => continue, @@ -1659,6 +1724,7 @@ pub const Printer = struct { dep_id: DependencyID, installed: *const Bitset, id_map: ?[]DependencyID, + pkg_metas: []const Package.Meta, ) ShouldPrintPackageInstallResult { const dependencies = this.lockfile.buffers.dependencies.items; const resolutions = this.lockfile.buffers.resolutions.items; @@ -1682,6 +1748,17 @@ pub const Printer = struct { if (!installed.isSet(package_id)) return .no; + // It's possible this package was installed but the dependency is disabled. + // Have "zod@1.0.0" in dependencies and `zod2@npm:zod@1.0.0` in devDependencies + // and install with --omit=dev. + if (this.lockfile.isPackageDisabled( + dep_id, + this.options.local_package_features, + &pkg_metas[package_id], + )) { + return .no; + } + const resolution = this.lockfile.packages.items(.resolution)[package_id]; if (resolution.tag == .npm) { const name = dependency.name.slice(this.lockfile.buffers.string_bytes.items); @@ -1801,6 +1878,7 @@ pub const Printer = struct { if (resolved.len == 0) return; const string_buf = this.lockfile.buffers.string_bytes.items; const resolutions_list = slice.items(.resolutions); + const pkg_metas = slice.items(.meta); const resolutions_buffer: []const PackageID = this.lockfile.buffers.resolutions.items; const dependencies_buffer: []const Dependency = this.lockfile.buffers.dependencies.items; if (dependencies_buffer.len == 0) return; @@ -1827,7 +1905,7 @@ pub const Printer = struct { for (workspaces_to_print.items) |workspace_dep_id| { const workspace_package_id = resolutions_buffer[workspace_dep_id]; for (resolutions_list[workspace_package_id].begin()..resolutions_list[workspace_package_id].end()) |dep_id| { - switch (shouldPrintPackageInstall(this, manager, @intCast(dep_id), installed, id_map)) { + switch (shouldPrintPackageInstall(this, manager, @intCast(dep_id), installed, id_map, pkg_metas)) { .yes => found_workspace_to_print = true, else => {}, } @@ -2242,13 +2320,23 @@ pub fn saveToDisk(this: *Lockfile, save_format: LoadResult.LockfileFormat, verbo assert(FileSystem.instance_loaded); } - const bytes = if (save_format == .text) - TextLockfile.Stringifier.saveFromBinary(bun.default_allocator, this) catch |err| { - switch (err) { + const bytes = bytes: { + if (save_format == .text) { + var writer_buf = MutableString.initEmpty(bun.default_allocator); + var buffered_writer = writer_buf.bufferedWriter(); + const writer = buffered_writer.writer(); + + TextLockfile.Stringifier.saveFromBinary(bun.default_allocator, this, writer) catch |err| switch (err) { error.OutOfMemory => bun.outOfMemory(), - } + }; + + buffered_writer.flush() catch |err| switch (err) { + error.OutOfMemory => bun.outOfMemory(), + }; + + break :bytes writer_buf.list.items; } - else bytes: { + var bytes = std.ArrayList(u8).init(bun.default_allocator); var total_size: usize = 0; diff --git a/src/install/migration.zig b/src/install/migration.zig index 5e3688865cf421..f57c59fb790e5a 100644 --- a/src/install/migration.zig +++ b/src/install/migration.zig @@ -1019,7 +1019,7 @@ pub fn migrateNPMLockfile( return error.NotAllPackagesGotResolved; } - try this.hoist(log, manager.options.local_package_features.dev_dependencies); + try this.hoist(log, .resolvable, {}); // if (Environment.isDebug) { // const dump_file = try std.fs.cwd().createFileZ("after-clean.json", .{}); diff --git a/src/install/patch_install.zig b/src/install/patch_install.zig index cb8b932aa12960..842ca4a01f40b9 100644 --- a/src/install/patch_install.zig +++ b/src/install/patch_install.zig @@ -211,12 +211,13 @@ pub const PatchTask = struct { }, .extract => { debug("pkg: {s} extract", .{pkg.name.slice(manager.lockfile.buffers.string_bytes.items)}); + + const task_id = Task.Id.forNPMPackage(manager.lockfile.str(&pkg.name), pkg.resolution.value.npm.version); + bun.debugAssert(!manager.network_dedupe_map.contains(task_id)); + const network_task = try manager.generateNetworkTaskForTarball( // TODO: not just npm package - Task.Id.forNPMPackage( - manager.lockfile.str(&pkg.name), - pkg.resolution.value.npm.version, - ), + task_id, url, manager.lockfile.buffers.dependencies.items[dep_id].behavior.isRequired(), dep_id, diff --git a/src/resolver/resolver.zig b/src/resolver/resolver.zig index c765c9f79bfee5..bf87b29dad6910 100644 --- a/src/resolver/resolver.zig +++ b/src/resolver/resolver.zig @@ -1939,7 +1939,7 @@ pub const Resolver = struct { .root_request_id = 0, }, null, - ); + ) catch |enqueue_download_err| return .{ .failure = enqueue_download_err }; return .{ .pending = .{ diff --git a/test/cli/install/bun-install.test.ts b/test/cli/install/bun-install.test.ts index 9097867d4fb48d..70addfbf375a7d 100644 --- a/test/cli/install/bun-install.test.ts +++ b/test/cli/install/bun-install.test.ts @@ -22,6 +22,7 @@ import { toHaveBins, runBunInstall, isWindows, + textLockfile, } from "harness"; import { join, sep, resolve } from "path"; import { @@ -1122,59 +1123,6 @@ it("should handle inter-dependency between workspaces (optionalDependencies)", a await access(join(package_dir, "bun.lockb")); }); -it("should ignore peerDependencies within workspaces", async () => { - await writeFile( - join(package_dir, "package.json"), - JSON.stringify({ - name: "Foo", - version: "0.0.1", - workspaces: ["packages/baz"], - peerDependencies: { - Bar: ">=0.0.2", - }, - }), - ); - await mkdir(join(package_dir, "packages", "baz"), { recursive: true }); - await writeFile( - join(package_dir, "packages", "baz", "package.json"), - JSON.stringify({ - name: "Baz", - version: "0.0.3", - peerDependencies: { - Moo: ">=0.0.4", - }, - }), - ); - await writeFile( - join(package_dir, "bunfig.toml"), - ` - [install] - peer = false - `, - ); - const { stdout, stderr, exited } = spawn({ - cmd: [bunExe(), "install"], - cwd: package_dir, - stdout: "pipe", - stdin: "pipe", - stderr: "pipe", - env, - }); - const err = await new Response(stderr).text(); - expect(err).toContain("Saved lockfile"); - const out = await new Response(stdout).text(); - expect(out.replace(/\s*\[[0-9\.]+m?s\]\s*$/, "").split(/\r?\n/)).toEqual([ - expect.stringContaining("bun install v1."), - "", - "1 package installed", - ]); - expect(await exited).toBe(0); - expect(requested).toBe(0); - expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual(["Baz"]); - expect(package_dir).toHaveWorkspaceLink(["Baz", "packages/baz"]); - await access(join(package_dir, "bun.lockb")); -}); - it("should handle installing the same peerDependency with different versions", async () => { const urls: string[] = []; setHandler(dummyRegistry(urls)); @@ -8458,3 +8406,133 @@ saveTextLockfile = true expect(await exists(join(package_dir, "bun.lockb"))).toBeFalse(); expect(await file(join(package_dir, "bun.lock")).text()).toMatchSnapshot(); }); + +test("providing invalid url in lockfile does not crash", async () => { + await Promise.all([ + write( + join(package_dir, "package.json"), + JSON.stringify({ + dependencies: { + "jquery": "3.7.1", + }, + }), + ), + write( + join(package_dir, "bun.lock"), + textLockfile(0, { + "workspaces": { + "": { + "dependencies": { + "jquery": "3.7.1", + }, + }, + }, + "packages": { + "jquery": [ + "jquery@3.7.1", + "invalid-url", + {}, + "sha512-+LGRog6RAsCJrrrg/IO6LGmpphNe5DiK30dGjCoxxeGv49B10/3XYGxPsAwrDlMFcFEvdAUavDT8r9k/hSyQqQ==", + ], + }, + }), + ), + ]); + + const { stderr, exited } = spawn({ + cmd: [bunExe(), "install"], + cwd: package_dir, + stderr: "pipe", + env, + }); + + const err = await Bun.readableStreamToText(stderr); + expect(err).toContain( + 'error: Expected tarball URL to start with https:// or http://, got "invalid-url" while fetching package "jquery"', + ); + expect(await exited).toBe(1); +}); + +test("optional dependencies do not need to be resolvable in text lockfile", async () => { + await Promise.all([ + write( + join(package_dir, "package.json"), + JSON.stringify({ + optionalDependencies: { + jquery: "3.7.1", + }, + }), + ), + write( + join(package_dir, "bun.lock"), + textLockfile(0, { + "workspaces": { + "": { + "optionalDependencies": { + "jquery": "3.7.1", + }, + }, + }, + "packages": {}, + }), + ), + ]); + + const { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "install"], + cwd: package_dir, + stdout: "pipe", + stderr: "pipe", + env, + }); + + const err = await Bun.readableStreamToText(stderr); + expect(err).not.toContain("Saved lockfile"); + const out = await Bun.readableStreamToText(stdout); + expect(out).not.toContain("1 package installed"); + + expect(await exited).toBe(0); +}); + +test("non-optional dependencies need to be resolvable in text lockfile", async () => { + await Promise.all([ + write( + join(package_dir, "package.json"), + JSON.stringify({ + dependencies: { + jquery: "3.7.1", + }, + }), + ), + write( + join(package_dir, "bun.lock"), + textLockfile(0, { + workspaces: { + "": { + dependencies: { + "jquery": "3.7.1", + }, + }, + }, + packages: {}, + }), + ), + ]); + + const { stdout, stderr, exited } = spawn({ + // --production to fail early + cmd: [bunExe(), "install", "--production"], + cwd: package_dir, + stdout: "pipe", + stderr: "pipe", + env, + }); + + const err = await Bun.readableStreamToText(stderr); + expect(err).not.toContain("Saved lockfile"); + expect(err).toContain("error: Failed to resolve root prod dependency 'jquery'"); + const out = await Bun.readableStreamToText(stdout); + expect(out).not.toContain("1 package installed"); + + expect(await exited).toBe(1); +}); diff --git a/test/cli/install/bun-run.test.ts b/test/cli/install/bun-run.test.ts index 52954e16c51a5f..99293d6013cbcb 100644 --- a/test/cli/install/bun-run.test.ts +++ b/test/cli/install/bun-run.test.ts @@ -275,7 +275,7 @@ console.log(minify("print(6 * 7)").code); BUN_INSTALL_CACHE_DIR: join(run_dir, ".cache"), }, }); - const err1 = await new Response(stderr1).text(); + const err1 = stderrForInstall(await new Response(stderr1).text()); expect(err1).toBe(""); expect(await readdirSorted(run_dir)).toEqual([".cache", "test.js"]); expect(await readdirSorted(join(run_dir, ".cache"))).toContain("uglify-js"); @@ -339,7 +339,7 @@ for (const entry of await decompress(Buffer.from(buffer))) { BUN_INSTALL_CACHE_DIR: join(run_dir, ".cache"), }, }); - const err1 = await new Response(stderr1).text(); + const err1 = stderrForInstall(await new Response(stderr1).text()); expect(err1).toBe(""); expect(await readdirSorted(run_dir)).toEqual([".cache", "test.js"]); expect(await readdirSorted(join(run_dir, ".cache"))).toContain("decompress"); diff --git a/test/cli/install/registry/__snapshots__/bun-install-registry.test.ts.snap b/test/cli/install/registry/__snapshots__/bun-install-registry.test.ts.snap index a58e5d1e733a60..9d2bebfe0ceb2a 100644 --- a/test/cli/install/registry/__snapshots__/bun-install-registry.test.ts.snap +++ b/test/cli/install/registry/__snapshots__/bun-install-registry.test.ts.snap @@ -311,3 +311,30 @@ exports[`hoisting text lockfile is hoisted 1`] = ` } " `; + +exports[`it should ignore peerDependencies within workspaces 1`] = ` +"{ + "lockfileVersion": 0, + "workspaces": { + "": { + "peerDependencies": { + "no-deps": ">=1.0.0", + }, + }, + "packages/baz": { + "name": "Baz", + "peerDependencies": { + "a-dep": ">=1.0.1", + }, + }, + }, + "packages": { + "Baz": ["Baz@workspace:packages/baz", { "peerDependencies": { "a-dep": ">=1.0.1" } }], + + "a-dep": ["a-dep@1.0.10", "http://localhost:1234/a-dep/-/a-dep-1.0.10.tgz", {}, "sha512-NeQ6Ql9jRW8V+VOiVb+PSQAYOvVoSimW+tXaR0CoJk4kM9RIk/XlAUGCsNtn5XqjlDO4hcH8NcyaL507InevEg=="], + + "no-deps": ["no-deps@2.0.0", "http://localhost:1234/no-deps/-/no-deps-2.0.0.tgz", {}, "sha512-W3duJKZPcMIG5rA1io5cSK/bhW9rWFz+jFxZsKS/3suK4qHDkQNxUTEXee9/hTaAoDCeHWQqogukWYKzfr6X4g=="], + } +} +" +`; diff --git a/test/cli/install/registry/bun-install-registry.test.ts b/test/cli/install/registry/bun-install-registry.test.ts index 18474bcb0c8f72..00248aa9c6d2aa 100644 --- a/test/cli/install/registry/bun-install-registry.test.ts +++ b/test/cli/install/registry/bun-install-registry.test.ts @@ -1838,6 +1838,64 @@ describe("text lockfile", () => { expect(await Bun.file(join(packageDir, "bun.lock")).text()).toBe(firstLockfile); }); + + for (const omit of ["dev", "peer", "optional"]) { + test(`resolvable lockfile with ${omit} dependencies disabled`, async () => { + await Promise.all([ + write( + join(packageDir, "package.json"), + JSON.stringify({ + name: "foo", + peerDependencies: { "no-deps": "1.0.0" }, + devDependencies: { "a-dep": "1.0.1" }, + optionalDependencies: { "basic-1": "1.0.0" }, + }), + ), + ]); + + let { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "install", "--save-text-lockfile", `--omit=${omit}`], + cwd: packageDir, + stdout: "pipe", + stderr: "pipe", + env, + }); + + let err = await Bun.readableStreamToText(stderr); + expect(err).toContain("Saved lockfile"); + expect(err).not.toContain("error:"); + + expect(await exited).toBe(0); + + const depName = omit === "dev" ? "a-dep" : omit === "peer" ? "no-deps" : "basic-1"; + + expect(await exists(join(packageDir, "node_modules", depName))).toBeFalse(); + + const lockfile = (await Bun.file(join(packageDir, "bun.lock")).text()).replaceAll( + /localhost:\d+/g, + "localhost:1234", + ); + + ({ stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "pipe", + stderr: "pipe", + env, + })); + + err = await Bun.readableStreamToText(stderr); + expect(err).not.toContain("Saved lockfile"); + expect(err).not.toContain("error:"); + expect(await exited).toBe(0); + + expect(await exists(join(packageDir, "node_modules", depName))).toBeTrue(); + + expect((await Bun.file(join(packageDir, "bun.lock")).text()).replaceAll(/localhost:\d+/g, "localhost:1234")).toBe( + lockfile, + ); + }); + } }); describe("optionalDependencies", () => { @@ -1974,6 +2032,71 @@ describe("optionalDependencies", () => { }); }); +test("it should ignore peerDependencies within workspaces", async () => { + await Promise.all([ + write( + packageJson, + JSON.stringify({ + name: "foo", + workspaces: ["packages/baz"], + peerDependencies: { + "no-deps": ">=1.0.0", + }, + }), + ), + write( + join(packageDir, "packages", "baz", "package.json"), + JSON.stringify({ + name: "Baz", + peerDependencies: { + "a-dep": ">=1.0.1", + }, + }), + ), + write(join(packageDir, ".npmrc"), `omit=peer`), + ]); + + const { exited } = spawn({ + cmd: [bunExe(), "install", "--save-text-lockfile"], + cwd: packageDir, + env, + }); + + expect(await exited).toBe(0); + + expect(await readdirSorted(join(packageDir, "node_modules"))).toEqual(["Baz"]); + expect( + (await file(join(packageDir, "bun.lock")).text()).replaceAll(/localhost:\d+/g, "localhost:1234"), + ).toMatchSnapshot(); + + // installing with them enabled works + await rm(join(packageDir, ".npmrc")); + await runBunInstall(env, packageDir, { savesLockfile: false }); + + expect(await readdirSorted(join(packageDir, "node_modules"))).toEqual(["Baz", "a-dep", "no-deps"]); +}); + +test("disabled dev/peer/optional dependencies are still included in the lockfile", async () => { + await Promise.all([ + write( + packageJson, + JSON.stringify({ + devDependencies: { + "no-deps": "1.0.0", + }, + peerDependencies: { + "a-dep": "1.0.1", + }, + optionalDependencies: { + "basic-1": "1.0.0", + }, + }), + ), + ]); + + await runBunInstall; +}); + test("tarball override does not crash", async () => { await write( packageJson, diff --git a/test/harness.ts b/test/harness.ts index a6e195002565a9..bbdc48006f666d 100644 --- a/test/harness.ts +++ b/test/harness.ts @@ -1427,3 +1427,10 @@ export function rmScope(path: string) { }, }; } + +export function textLockfile(version: number, pkgs: any): string { + return JSON.stringify({ + lockfileVersion: version, + ...pkgs, + }); +}