Skip to content

Commit

Permalink
fix(install): migrate package-lock.json with dependency on root packa…
Browse files Browse the repository at this point in the history
…ge (#14811)

Co-authored-by: Jarred Sumner <[email protected]>
  • Loading branch information
dylan-conway and Jarred-Sumner authored Oct 25, 2024
1 parent 61534c7 commit b895738
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 31 deletions.
42 changes: 12 additions & 30 deletions src/install/install.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1162,6 +1162,7 @@ pub fn NewPackageInstall(comptime kind: PkgInstallKind) type {
switch (resolution.tag) {
.git => this.verifyGitResolution(&resolution.value.git, buf, root_node_modules_dir),
.github => this.verifyGitResolution(&resolution.value.github, buf, root_node_modules_dir),
.root => this.verifyTransitiveSymlinkedFolder(root_node_modules_dir),
.folder => if (this.lockfile.isWorkspaceTreeId(this.node_modules.tree_id))
this.verifyPackageJSONNameAndVersion(root_node_modules_dir, resolution.tag)
else
Expand Down Expand Up @@ -2395,36 +2396,10 @@ pub fn NewPackageInstall(comptime kind: PkgInstallKind) type {
if (bun.sys.File.writeAll(.{ .handle = tag_fd }, this.package_version).asErr()) |e| return .{ .fail = .{ .err = bun.errnoToZigErr(e.getErrno()), .step = Step.patching } };
}

pub fn installWithMethod(this: *@This(), skip_delete: bool, destination_dir: std.fs.Dir, method: Method, resolution_tag: Resolution.Tag) Result {
// If this fails, we don't care.
// we'll catch it the next error
if (!skip_delete and !strings.eqlComptime(this.destination_dir_subpath, ".")) this.uninstallBeforeInstall(destination_dir);

if (comptime kind == .regular) return this.installImpl(skip_delete, destination_dir, method, resolution_tag);

const result = this.installImpl(skip_delete, destination_dir, method, resolution_tag);
if (result == .fail) return result;
const fd = bun.toFD(destination_dir.fd);
const subpath = bun.path.joinZ(&[_][]const u8{ this.destination_dir_subpath, ".bun-patch-tag" }, .auto);
const tag_fd = switch (bun.sys.openat(fd, subpath, bun.O.CREAT | bun.O.WRONLY | bun.O.TRUNC, 0o666)) {
.err => |e| return .{ .fail = .{ .err = bun.errnoToZigErr(e.getErrno()), .step = Step.patching } },
.result => |f| f,
};
defer _ = bun.sys.close(tag_fd);
if (bun.sys.File.writeAll(.{ .handle = tag_fd }, this.package_version).asErr()) |e| return .{ .fail = .{ .err = bun.errnoToZigErr(e.getErrno()), .step = Step.patching } };
return result;
}

pub fn installImpl(this: *@This(), skip_delete: bool, destination_dir: std.fs.Dir, method_: Method, resolution_tag: Resolution.Tag) Result {
// If this fails, we don't care.
// we'll catch it the next error
if (!skip_delete and !strings.eqlComptime(this.destination_dir_subpath, ".")) this.uninstallBeforeInstall(destination_dir);
defer {
if (kind == .patch) {
const fd = bun.toFD(destination_dir.fd);
_ = fd; // autofix
}
}

var supported_method_to_use = method_;

Expand Down Expand Up @@ -12756,6 +12731,10 @@ pub const PackageManager = struct {
}
installer.cache_dir = std.fs.cwd();
},
.root => {
installer.cache_dir_subpath = ".";
installer.cache_dir = std.fs.cwd();
},
.symlink => {
const directory = this.manager.globalLinkDir() catch |err| {
if (comptime log_level != .silent) {
Expand Down Expand Up @@ -12954,15 +12933,18 @@ pub const PackageManager = struct {
const install_result = switch (resolution.tag) {
.symlink, .workspace => installer.installFromLink(this.skip_delete, destination_dir),
else => result: {
if (resolution.tag == .folder and !this.lockfile.isWorkspaceTreeId(this.current_tree_id)) {
if (resolution.tag == .root or (resolution.tag == .folder and !this.lockfile.isWorkspaceTreeId(this.current_tree_id))) {
// This is a transitive folder dependency. It is installed with a single symlink to the target folder/file,
// and is not hoisted.
const dirname = std.fs.path.dirname(this.node_modules.path.items) orelse this.node_modules.path.items;

installer.cache_dir = this.root_node_modules_folder.openDir(dirname, .{ .iterate = true, .access_sub_paths = true }) catch |err|
break :result PackageInstall.Result.fail(err, .opening_cache_dir);

const result = installer.install(this.skip_delete, destination_dir, resolution.tag);
const result = if (resolution.tag == .root)
installer.installFromLink(this.skip_delete, destination_dir)
else
installer.install(this.skip_delete, destination_dir, resolution.tag);

if (result.isFail() and (result.fail.err == error.ENOENT or result.fail.err == error.FileNotFound))
break :result PackageInstall.Result.success();
Expand Down Expand Up @@ -12996,7 +12978,7 @@ pub const PackageManager = struct {
break :brk .{ false, false };
};

if (resolution.tag == .workspace or is_trusted) {
if (resolution.tag != .root and (resolution.tag == .workspace or is_trusted)) {
if (this.enqueueLifecycleScripts(
alias,
log_level,
Expand Down Expand Up @@ -13139,7 +13121,7 @@ pub const PackageManager = struct {
break :brk .{ false, false, false };
};

if (is_trusted) {
if (resolution.tag != .root and is_trusted) {
if (this.enqueueLifecycleScripts(
alias,
log_level,
Expand Down
2 changes: 1 addition & 1 deletion src/install/lockfile.zig
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ pub const Scripts = struct {
};

pub fn isEmpty(this: *const Lockfile) bool {
return this.packages.len == 0 or this.packages.len == 1 or this.packages.get(0).resolutions.len == 0;
return this.packages.len == 0 or (this.packages.len == 1 and this.packages.get(0).resolutions.len == 0);
}

pub const LoadFromDiskResult = union(enum) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "test-pkg",
"version": "2.2.2",
"dependencies": {
"test-pkg": "."
}
}
13 changes: 13 additions & 0 deletions test/cli/install/migration/migrate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,19 @@ test.todo("migrate workspace from npm during `bun add`", async () => {
expect(svelte_version).toBe("3.0.0");
});

test("migrate package with dependency on root package", async () => {
const testDir = tmpdirSync();

fs.cpSync(join(import.meta.dir, "migrate-package-with-dependency-on-root"), testDir, { recursive: true });

Bun.spawnSync([bunExe(), "install"], {
env: bunEnv,
cwd: join(testDir),
});

expect(fs.existsSync(join(testDir, "node_modules", "test-pkg", "package.json"))).toBeTrue();
});

test("migrate from npm lockfile that is missing `resolved` properties", async () => {
const testDir = tmpdirSync();

Expand Down

0 comments on commit b895738

Please sign in to comment.