fix(install): serialize updated workspaces versions correctly for bun.lockb (#22932)

### What does this PR do?
This change was missing after changing semver core numbers to use u64.

Also fixes potentially serializing uninitialized bytes from resolution
unions.
### How did you verify your code works?
Added a test for migrating a bun.lockb with most features used.
This commit is contained in:
Dylan Conway
2025-09-24 02:42:57 -07:00
committed by GitHub
parent 92bc522e85
commit 084eeb945e
6 changed files with 180 additions and 64 deletions

View File

@@ -2047,7 +2047,15 @@ pub fn Package(comptime SemverIntType: type) type {
}
}
comptime assertNoUninitializedPadding(@TypeOf(value));
try writer.writeAll(std.mem.sliceAsBytes(value));
if (comptime strings.eqlComptime(field.name, "resolution")) {
// copy each resolution to make sure the union is zero initialized
for (value) |val| {
const copy = val.copy();
try writer.writeAll(std.mem.asBytes(&copy));
}
} else {
try writer.writeAll(std.mem.sliceAsBytes(value));
}
}
const really_end_at = try stream.getPos();

View File

@@ -337,11 +337,30 @@ pub fn load(
);
defer workspace_package_name_hashes.deinit(allocator);
var workspace_versions_list = try Lockfile.Buffers.readArray(
stream,
allocator,
std.ArrayListUnmanaged(Semver.Version),
);
var workspace_versions_list = workspace_versions_list: {
if (!migrate_from_v2) {
break :workspace_versions_list try Lockfile.Buffers.readArray(
stream,
allocator,
std.ArrayListUnmanaged(Semver.Version),
);
}
var old_versions_list = try Lockfile.Buffers.readArray(
stream,
allocator,
std.ArrayListUnmanaged(Semver.VersionType(u32)),
);
defer old_versions_list.deinit(allocator);
var versions_list: std.ArrayListUnmanaged(Semver.Version) = try .initCapacity(allocator, old_versions_list.items.len);
for (old_versions_list.items) |old_version| {
versions_list.appendAssumeCapacity(old_version.migrate());
}
break :workspace_versions_list versions_list;
};
comptime {
if (PackageNameHash != @TypeOf((VersionHashMap.KV{ .key = undefined, .value = undefined }).key)) {
@compileError("VersionHashMap must be in sync with serialization");

View File

@@ -108,7 +108,7 @@ fn jsonStringifyDependency(this: *const Lockfile, w: anytype, dep_id: Dependency
try w.write(dep.name.slice(sb));
try w.objectField("version");
try w.print("catalog:{s}", .{info.fmtJson(sb, .{ .quote = false })});
try w.print("\"catalog:{s}\"", .{info.fmtJson(sb, .{ .quote = false })});
},
}

View File

@@ -176,6 +176,25 @@ pub fn ResolutionType(comptime SemverIntType: type) type {
};
}
pub fn copy(this: *const Resolution) Resolution {
return switch (this.tag) {
.npm => .init(.{ .npm = this.value.npm }),
.local_tarball => .init(.{ .local_tarball = this.value.local_tarball }),
.folder => .init(.{ .folder = this.value.folder }),
.remote_tarball => .init(.{ .remote_tarball = this.value.remote_tarball }),
.workspace => .init(.{ .workspace = this.value.workspace }),
.symlink => .init(.{ .symlink = this.value.symlink }),
.single_file_module => .init(.{ .single_file_module = this.value.single_file_module }),
.git => .init(.{ .git = this.value.git }),
.github => .init(.{ .github = this.value.github }),
.root => .init(.{ .root = {} }),
.uninitialized => .init(.{ .uninitialized = {} }),
else => {
std.debug.panic("Internal error: unexpected resolution tag: {}", .{this.tag});
},
};
}
pub fn fmt(this: *const This, string_bytes: []const u8, path_sep: bun.fmt.PathFormatOptions.Sep) Formatter {
return Formatter{
.resolution = this,
@@ -377,11 +396,17 @@ pub fn ResolutionType(comptime SemverIntType: type) type {
single_file_module: String,
pub var zero: Value = @bitCast(std.mem.zeroes([@sizeOf(Value)]u8));
/// To avoid undefined memory between union values, we must zero initialize the union first.
pub fn init(field: bun.meta.Tagged(Value, Tag)) Value {
return switch (field) {
inline else => |v, t| @unionInit(Value, @tagName(t), v),
};
var value = zero;
switch (field) {
inline else => |v, t| {
@field(value, @tagName(t)) = v;
},
}
return value;
}
};

Binary file not shown.

View File

@@ -6,58 +6,122 @@ import { cp } from "node:fs/promises";
import { join } from "node:path";
const { parseLockfile } = install_test_helpers;
test("old binary lockfile migrates successfully", async () => {
const oldLockfileContents = await file(join(import.meta.dir, "fixtures/bun.lockb.v2")).text();
using testDir = tempDir("migrate-bun-lockb-v2", {
"bunfig.toml": "install.saveTextLockfile = false",
"package.json": JSON.stringify({
name: "migrate-bun-lockb-v2",
dependencies: {
jquery: "~3.7.1",
"is-even": "^1.0.0",
},
}),
const tests = [
{
name: "migrate-bun-lockb-v2",
lockfile: "bun.lockb.v2",
files: {
"bunfig.toml": "install.saveTextLockfile = false",
"package.json": JSON.stringify({
name: "migrate-bun-lockb-v2",
dependencies: {
jquery: "~3.7.1",
"is-even": "^1.0.0",
},
}),
},
},
{
name: "migrate-bun-lockb-v2-most-features",
lockfile: "bun.lockb.v2-most-features",
files: {
"bunfig.toml": "install.saveTextLockfile = false",
"packages/pkg1/package.json": JSON.stringify({
"name": "pkg-wat",
"dependencies": {
"jquery": "3.7.0",
"pkg-wat-2": "workspace:",
},
}),
"packages/pkg2/package.json": JSON.stringify({
"name": "pkg-wat-2",
"dependencies": {
"kind-of": "6.0.3",
},
}),
"package.json": JSON.stringify({
"name": "migrate-everything",
"dependencies": {
"false": "^0.0.4",
"jquery": "~3.7.1",
"scheduler": "^0.23.0",
},
"devDependencies": {
"zod": "^3.22.4",
"esbuild": "0.25.10",
"react": "catalog:",
},
"optionalDependencies": {
"is-number": "^7.0.0",
},
"peerDependencies": {
"lodash": "^4.17.21",
"is-even": "^1.0.0",
},
"peerDependenciesMeta": {
"lodash": {
"optional": true,
},
},
"resolutions": {
"scheduler": "0.20.0",
},
"trustedDependencies": ["esbuild"],
"workspaces": {
"packages": ["packages/*"],
"catalog": {
"react": ">19.0.0",
},
},
}),
},
},
];
for (const testInfo of tests) {
test(`migrate ${testInfo.name}`, async () => {
const oldLockfileContents = await file(join(import.meta.dir, "fixtures", testInfo.lockfile)).text();
using testDir = tempDir(testInfo.name, testInfo.files);
await cp(join(import.meta.dir, "fixtures", testInfo.lockfile), join(testDir, "bun.lockb"));
const oldLockfile = parseLockfile(testDir);
let { stderr, exited } = spawn({
cmd: [bunExe(), "install"],
cwd: testDir,
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
let err = await stderr.text();
expect(await exited).toBe(0);
expect(err).toContain("Saved lockfile");
const newLockfileContents = await file(join(testDir, "bun.lockb")).bytes();
const newLockfile = parseLockfile(testDir);
// contents should be different due to semver numbers changing size
expect(newLockfileContents).not.toEqual(oldLockfileContents);
// but parse result should be the same
expect(newLockfile).toEqual(oldLockfile);
// another install should not change the lockfile
({ stderr, exited } = spawn({
cmd: [bunExe(), "install"],
cwd: testDir,
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
}));
expect(await exited).toBe(0);
const newLockfileContents2 = await file(join(testDir, "bun.lockb")).bytes();
const newLockfile2 = parseLockfile(testDir);
expect(newLockfileContents2).toEqual(newLockfileContents);
expect(newLockfile2).toEqual(newLockfile);
});
await cp(join(import.meta.dir, "fixtures/bun.lockb.v2"), join(testDir, "bun.lockb"));
const oldLockfile = parseLockfile(testDir);
let { stderr, exited } = spawn({
cmd: [bunExe(), "install"],
cwd: testDir,
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
let err = await stderr.text();
expect(await exited).toBe(0);
expect(err).toContain("Saved lockfile");
const newLockfileContents = await file(join(testDir, "bun.lockb")).bytes();
const newLockfile = parseLockfile(testDir);
// contents should be different due to semver numbers changing size
expect(newLockfileContents).not.toEqual(oldLockfileContents);
// but parse result should be the same
expect(newLockfile).toEqual(oldLockfile);
// another install should not change the lockfile
({ stderr, exited } = spawn({
cmd: [bunExe(), "install"],
cwd: testDir,
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
}));
expect(await exited).toBe(0);
expect(await stderr.text()).not.toContain("Saved lockfile");
const newLockfileContents2 = await file(join(testDir, "bun.lockb")).bytes();
const newLockfile2 = parseLockfile(testDir);
expect(newLockfileContents2).toEqual(newLockfileContents);
expect(newLockfile2).toEqual(newLockfile);
});
}