diff --git a/bun.lockb b/bun.lockb index 2062b91a58..90d939917c 100755 Binary files a/bun.lockb and b/bun.lockb differ diff --git a/src/install/install.zig b/src/install/install.zig index 9c7d1cd739..8ca1f839a4 100644 --- a/src/install/install.zig +++ b/src/install/install.zig @@ -5796,9 +5796,15 @@ pub const PackageManager = struct { this.log_level = if (default_disable_progress_bar) LogLevel.default_no_progress else LogLevel.default; PackageManager.verbose_install = false; } + + // If the lockfile is frozen, don't save it to disk. + if (this.enable.frozen_lockfile) { + this.do.save_lockfile = false; + this.enable.force_save_lockfile = false; + } } - pub const Do = struct { + pub const Do = packed struct { save_lockfile: bool = true, load_lockfile: bool = true, install_packages: bool = true, @@ -5812,7 +5818,7 @@ pub const PackageManager = struct { trust_dependencies_from_args: bool = false, }; - pub const Enable = struct { + pub const Enable = packed struct { manifest_cache: bool = true, manifest_cache_control: bool = true, cache: bool = true, @@ -7572,6 +7578,16 @@ pub const PackageManager = struct { cli.positionals = args.positionals(); + if (cli.production and cli.trusted) { + Output.errGeneric("The '--production' and '--trust' flags together are not supported because the --trust flag potentially modifies the lockfile after installing packages\n", .{}); + Global.crash(); + } + + if (cli.frozen_lockfile and cli.trusted) { + Output.errGeneric("The '--frozen-lockfile' and '--trust' flags together are not supported because the --trust flag potentially modifies the lockfile after installing packages\n", .{}); + Global.crash(); + } + return cli; } }; @@ -9958,6 +9974,16 @@ pub const PackageManager = struct { const packages_len_before_install = manager.lockfile.packages.len; + if (manager.options.enable.frozen_lockfile) { + if (manager.lockfile.hasMetaHashChanged(PackageManager.verbose_install or manager.options.do.print_meta_hash_string, packages_len_before_install) catch false) { + if (comptime log_level != .silent) { + Output.prettyErrorln("error: lockfile had changes, but lockfile is frozen", .{}); + Output.note("try re-running without --frozen-lockfile and commit the updated lockfile", .{}); + } + Global.crash(); + } + } + var install_summary = PackageInstall.Summary{}; if (manager.options.do.install_packages) { install_summary = try manager.installPackages( @@ -9966,7 +9992,10 @@ pub const PackageManager = struct { ); } - const did_meta_hash_change = try manager.lockfile.hasMetaHashChanged( + const did_meta_hash_change = + // If the lockfile was frozen, we already checked it + !manager.options.enable.frozen_lockfile and + try manager.lockfile.hasMetaHashChanged( PackageManager.verbose_install or manager.options.do.print_meta_hash_string, @min(packages_len_before_install, manager.lockfile.packages.len), ); @@ -9979,14 +10008,6 @@ pub const PackageManager = struct { manager.package_json_updates.len > 0 or (load_lockfile_result == .ok and load_lockfile_result.ok.serializer_result.packages_need_update); - if (did_meta_hash_change and manager.options.enable.frozen_lockfile) { - if (comptime log_level != .silent) { - Output.prettyErrorln("error: lockfile had changes, but lockfile is frozen", .{}); - Output.note("try re-running without --frozen-lockfile and commit the updated lockfile", .{}); - } - Global.crash(); - } - // It's unnecessary work to re-save the lockfile if there are no changes if (manager.options.do.save_lockfile and (should_save_lockfile or manager.lockfile.isEmpty() or manager.options.enable.force_save_lockfile)) diff --git a/src/install/lockfile.zig b/src/install/lockfile.zig index b555cdc626..fb2f0650bd 100644 --- a/src/install/lockfile.zig +++ b/src/install/lockfile.zig @@ -415,6 +415,7 @@ pub const Tree = struct { queue: Lockfile.TreeFiller, log: *logger.Log, old_lockfile: *Lockfile, + prefer_dev_dependencies: bool = false, pub fn maybeReportError(this: *Builder, comptime fmt: string, args: anytype) void { this.log.addErrorFmt(null, logger.Loc.Empty, this.allocator, fmt, args) catch {}; @@ -559,7 +560,17 @@ pub const Tree = struct { for (this_dependencies) |dep_id| { const dep = builder.dependencies[dep_id]; if (dep.name_hash != dependency.name_hash) continue; - if (builder.resolutions[dep_id] != package_id and !dependency.behavior.isPeer()) { + const mismatch = builder.resolutions[dep_id] != package_id; + + if (mismatch and dep.behavior.isDev() != dependency.behavior.isDev()) { + if (builder.prefer_dev_dependencies and dep.behavior.isDev()) { + return hoisted; // 2 + } + + return dependency_loop; // 3 + } + + if (mismatch and !dependency.behavior.isPeer()) { if (as_defined and !dep.behavior.isPeer()) { builder.maybeReportError("Package \"{}@{}\" has a dependency loop\n Resolution: \"{}@{}\"\n Dependency: \"{}@{}\"", .{ builder.packageName(package_id), @@ -949,6 +960,7 @@ const Cloner = struct { .dependencies = this.lockfile.buffers.dependencies.items, .log = this.log, .old_lockfile = this.old, + .prefer_dev_dependencies = PackageManager.instance.options.local_package_features.dev_dependencies, }; try (Tree{}).processSubtree(Tree.root_dep_id, &builder); diff --git a/test/cli/install/registry/bun-install-registry.test.ts b/test/cli/install/registry/bun-install-registry.test.ts index df78a45ccc..00de84bd17 100644 --- a/test/cli/install/registry/bun-install-registry.test.ts +++ b/test/cli/install/registry/bun-install-registry.test.ts @@ -50,6 +50,198 @@ afterEach(async () => { await rm(packageDir, { force: true, recursive: true }); }); +describe.each(["--production", "without --production"])("%s", flag => { + const prod = flag === "--production"; + const order = ["devDependencies", "dependencies"]; + // const stdio = process.versions.bun.includes("debug") ? "inherit" : "ignore"; + const stdio = "ignore"; + + if (prod) { + test("modifying package.json with --production should not save lockfile", async () => { + await writeFile( + join(packageDir, "package.json"), + JSON.stringify({ + name: "foo", + version: "1.0.0", + dependencies: { + "bin-change-dir": "1.0.0", + }, + devDependencies: { + "bin-change-dir": "1.0.1", + "basic-1": "1.0.0", + }, + }), + ); + + var { exited } = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: stdio, + stdin: stdio, + stderr: stdio, + env, + }); + + expect(await exited).toBe(0); + + const initialHash = Bun.hash(await file(join(packageDir, "bun.lockb")).arrayBuffer()); + + expect(await file(join(packageDir, "node_modules", "bin-change-dir", "package.json")).json()).toMatchObject({ + name: "bin-change-dir", + version: "1.0.1", + }); + + var { exited } = spawn({ + cmd: [bunExe(), "install", "--production"], + cwd: packageDir, + stdout: stdio, + stdin: stdio, + stderr: stdio, + env, + }); + + expect(await exited).toBe(0); + + expect(await file(join(packageDir, "node_modules", "bin-change-dir", "package.json")).json()).toMatchObject({ + name: "bin-change-dir", + version: "1.0.0", + }); + + var { exited } = spawn({ + cmd: [bunExe(), "install", "--production", "bin-change-dir@1.0.1"], + cwd: packageDir, + stdout: stdio, + stdin: stdio, + stderr: stdio, + env, + }); + + expect(await exited).toBe(1); + + // We should not have saved bun.lockb + expect(Bun.hash(await file(join(packageDir, "bun.lockb")).arrayBuffer())).toBe(initialHash); + + // We should not have installed bin-change-dir@1.0.1 + expect(await file(join(packageDir, "node_modules", "bin-change-dir", "package.json")).json()).toMatchObject({ + name: "bin-change-dir", + version: "1.0.0", + }); + + // This is a no-op. It should work. + var { exited } = spawn({ + cmd: [bunExe(), "install", "--production", "bin-change-dir@1.0.0"], + cwd: packageDir, + stdout: stdio, + stdin: stdio, + stderr: stdio, + env, + }); + + expect(await exited).toBe(0); + + // We should not have saved bun.lockb + expect(Bun.hash(await file(join(packageDir, "bun.lockb")).arrayBuffer())).toBe(initialHash); + + // We should have installed bin-change-dir@1.0.0 + expect(await file(join(packageDir, "node_modules", "bin-change-dir", "package.json")).json()).toMatchObject({ + name: "bin-change-dir", + version: "1.0.0", + }); + }); + } + + test(`should prefer ${order[+prod % 2]} over ${order[1 - (+prod % 2)]}`, async () => { + await writeFile( + join(packageDir, "package.json"), + JSON.stringify({ + name: "foo", + version: "1.0.0", + dependencies: { + "bin-change-dir": "1.0.0", + }, + devDependencies: { + "bin-change-dir": "1.0.1", + "basic-1": "1.0.0", + }, + }), + ); + + let initialLockfileHash; + async function saveWithoutProd() { + var hash; + // First install without --production + // so that the lockfile is up to date + var { exited } = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: stdio, + stdin: stdio, + stderr: stdio, + env, + }); + expect(await exited).toBe(0); + + await Promise.all([ + (async () => + expect(await file(join(packageDir, "node_modules", "bin-change-dir", "package.json")).json()).toMatchObject({ + name: "bin-change-dir", + version: "1.0.1", + }))(), + (async () => + expect(await file(join(packageDir, "node_modules", "basic-1", "package.json")).json()).toMatchObject({ + name: "basic-1", + version: "1.0.0", + }))().then( + async () => await rm(join(packageDir, "node_modules", "basic-1"), { recursive: true, force: true }), + ), + + (async () => (hash = Bun.hash(await file(join(packageDir, "bun.lockb")).arrayBuffer())))(), + ]); + + return hash; + } + if (prod) { + initialLockfileHash = await saveWithoutProd(); + } + + var { exited } = spawn({ + cmd: [bunExe(), "install", prod ? "--production" : ""].filter(Boolean), + cwd: packageDir, + stdout: stdio, + stdin: stdio, + stderr: stdio, + env, + }); + + expect(await exited).toBe(0); + expect(await file(join(packageDir, "node_modules", "bin-change-dir", "package.json")).json()).toMatchObject({ + name: "bin-change-dir", + version: prod ? "1.0.0" : "1.0.1", + }); + + if (!prod) { + expect(await file(join(packageDir, "node_modules", "basic-1", "package.json")).json()).toMatchObject({ + name: "basic-1", + version: "1.0.0", + }); + } else { + // it should not install devDependencies + expect(await exists(join(packageDir, "node_modules", "basic-1"))).toBeFalse(); + + // it should not mutate the lockfile when there were no changes to begin with. + const newHash = Bun.hash(await file(join(packageDir, "bun.lockb")).arrayBuffer()); + + expect(newHash).toBe(initialLockfileHash!); + } + + if (prod) { + // lets now try to install again without --production + const newHash = await saveWithoutProd(); + expect(newHash).toBe(initialLockfileHash); + } + }); +}); + test("basic 1", async () => { await writeFile( join(packageDir, "package.json"),