* Fix assertion failure when package-lock.json is out of sync with package.json

* Fixes #9433

* Update bun.lockb

---------

Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com>
This commit is contained in:
Jarred Sumner
2024-03-17 03:42:10 -07:00
committed by GitHub
parent c8ec56ec1b
commit 9e8bdaba93
4 changed files with 237 additions and 12 deletions

BIN
bun.lockb

Binary file not shown.

View File

@@ -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("<r><red>error<r><d>:<r> lockfile had changes, but lockfile is frozen", .{});
Output.note("try re-running without <d>--frozen-lockfile<r> 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("<r><red>error<r><d>:<r> lockfile had changes, but lockfile is frozen", .{});
Output.note("try re-running without <d>--frozen-lockfile<r> 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))

View File

@@ -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);

View File

@@ -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"),