diff --git a/src/install/install.zig b/src/install/install.zig index 77ec4b6d26a86e..2c357cef943cab 100644 --- a/src/install/install.zig +++ b/src/install/install.zig @@ -2341,13 +2341,29 @@ pub fn NewPackageInstall(comptime kind: PkgInstallKind) type { supported_method; } - pub fn packageMissingFromCache(this: *@This(), manager: *PackageManager, package_id: PackageID) bool { + pub fn packageMissingFromCache(this: *@This(), manager: *PackageManager, package_id: PackageID, resolution_tag: Resolution.Tag) bool { const state = manager.getPreinstallState(package_id); return switch (state) { .done => false, else => brk: { if (this.patch.isNull()) { - const exists = Syscall.directoryExistsAt(this.cache_dir.fd, this.cache_dir_subpath).unwrap() catch false; + const exists = switch (resolution_tag) { + .npm => package_json_exists: { + var buf = &PackageManager.cached_package_folder_name_buf; + + if (comptime Environment.isDebug) { + bun.assertWithLocation(bun.isSliceInBuffer(this.cache_dir_subpath, buf), @src()); + } + + const subpath_len = strings.withoutTrailingSlash(this.cache_dir_subpath).len; + buf[subpath_len] = std.fs.path.sep; + defer buf[subpath_len] = 0; + @memcpy(buf[subpath_len + 1 ..][0.."package.json\x00".len], "package.json\x00"); + const subpath = buf[0 .. subpath_len + 1 + "package.json".len :0]; + break :package_json_exists Syscall.existsAt(bun.toFD(this.cache_dir.fd), subpath); + }, + else => Syscall.directoryExistsAt(this.cache_dir.fd, this.cache_dir_subpath).unwrap() catch false, + }; if (exists) manager.setPreinstallState(package_id, manager.lockfile, .done); break :brk !exists; } @@ -2377,13 +2393,8 @@ pub fn NewPackageInstall(comptime kind: PkgInstallKind) type { } pub fn install(this: *@This(), skip_delete: bool, destination_dir: std.fs.Dir, 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, this.getInstallMethod(), resolution_tag); - const result = this.installImpl(skip_delete, destination_dir, this.getInstallMethod(), resolution_tag); + if (comptime kind == .regular) return result; 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" }); @@ -12656,7 +12667,6 @@ pub const PackageManager = struct { var installer = PackageInstall{ .progress = this.progress, .cache_dir = undefined, - .cache_dir_subpath = undefined, .destination_dir_subpath = destination_dir_subpath, .destination_dir_subpath_buf = &this.destination_dir_subpath_buf, .allocator = this.lockfile.allocator, @@ -12803,7 +12813,7 @@ pub const PackageManager = struct { this.summary.skipped += @intFromBool(!needs_install); if (needs_install) { - if (!remove_patch and resolution.tag.canEnqueueInstallTask() and installer.packageMissingFromCache(this.manager, package_id)) { + if (!remove_patch and resolution.tag.canEnqueueInstallTask() and installer.packageMissingFromCache(this.manager, package_id, resolution.tag)) { if (comptime Environment.allow_assert) { bun.assertWithLocation(resolution.canEnqueueInstallTask(), @src()); } diff --git a/src/install/npm.zig b/src/install/npm.zig index ef9bf8c3d1a1bd..bc4fc4a6377419 100644 --- a/src/install/npm.zig +++ b/src/install/npm.zig @@ -35,6 +35,7 @@ const http = bun.http; const OOM = bun.OOM; const Global = bun.Global; const PublishCommand = bun.CLI.PublishCommand; +const File = bun.sys.File; const Npm = @This(); @@ -1201,28 +1202,28 @@ pub const PackageManifest = struct { pub fn loadByFileID(allocator: std.mem.Allocator, scope: *const Registry.Scope, cache_dir: std.fs.Dir, file_id: u64) !?PackageManifest { var file_path_buf: [512 + 64]u8 = undefined; const file_name = try manifestFileName(&file_path_buf, file_id, scope); - var cache_file = cache_dir.openFileZ( - file_name, - .{ .mode = .read_only }, - ) catch return null; + const cache_file = File.openat(cache_dir, file_name, bun.O.RDONLY, 0).unwrap() catch return null; defer cache_file.close(); return loadByFile(allocator, scope, cache_file); } - pub fn loadByFile(allocator: std.mem.Allocator, scope: *const Registry.Scope, manifest_file: std.fs.File) !?PackageManifest { - const bytes = try manifest_file.readToEndAllocOptions( - allocator, - std.math.maxInt(u32), - manifest_file.getEndPos() catch null, - @alignOf(u8), - null, - ); - + pub fn loadByFile(allocator: std.mem.Allocator, scope: *const Registry.Scope, manifest_file: File) !?PackageManifest { + const bytes = try manifest_file.readToEnd(allocator).unwrap(); errdefer allocator.free(bytes); + if (bytes.len < header_bytes.len) { return null; } - return try readAll(bytes, scope); + + const manifest = try readAll(bytes, scope) orelse return null; + + if (manifest.versions.len == 0) { + // it's impossible to publish a package with zero versions, bust + // invalid entry + return null; + } + + return manifest; } fn readAll(bytes: []const u8, scope: *const Registry.Scope) !?PackageManifest { @@ -1311,7 +1312,7 @@ pub const PackageManifest = struct { }, }; - const maybe_package_manifest = Serializer.loadByFile(bun.default_allocator, &scope, manifest_file) catch |err| { + const maybe_package_manifest = Serializer.loadByFile(bun.default_allocator, &scope, File.from(manifest_file)) catch |err| { global.throw("failed to load manifest file: {s}", .{@errorName(err)}); return .zero; }; diff --git a/src/sys.zig b/src/sys.zig index 667bd4de3c9b41..e229a0491694c1 100644 --- a/src/sys.zig +++ b/src/sys.zig @@ -3013,14 +3013,14 @@ pub const File = struct { // "handle" matches std.fs.File handle: bun.FileDescriptor, - pub fn openat(other: anytype, path: anytype, flags: bun.Mode, mode: bun.Mode) Maybe(File) { + pub fn openat(other: anytype, path: [:0]const u8, flags: bun.Mode, mode: bun.Mode) Maybe(File) { return switch (This.openat(bun.toFD(other), path, flags, mode)) { .result => |fd| .{ .result = .{ .handle = fd } }, .err => |err| .{ .err = err }, }; } - pub fn open(path: anytype, flags: bun.Mode, mode: bun.Mode) Maybe(File) { + pub fn open(path: [:0]const u8, flags: bun.Mode, mode: bun.Mode) Maybe(File) { return File.openat(bun.FD.cwd(), path, flags, mode); } diff --git a/test/cli/install/registry/bun-install-registry.test.ts b/test/cli/install/registry/bun-install-registry.test.ts index 9efdf29e5d716c..038cd2e0a2c453 100644 --- a/test/cli/install/registry/bun-install-registry.test.ts +++ b/test/cli/install/registry/bun-install-registry.test.ts @@ -3056,6 +3056,69 @@ describe("binaries", () => { }); }); +test("it should invalid cached package if package.json is missing", async () => { + await Promise.all([ + write( + packageJson, + JSON.stringify({ + name: "foo", + dependencies: { + "no-deps": "2.0.0", + }, + }), + ), + ]); + + let { out } = await runBunInstall(env, packageDir); + expect(out).toContain("+ no-deps@2.0.0"); + + // node_modules and cache should be populated + expect( + await Promise.all([ + readdirSorted(join(packageDir, "node_modules", "no-deps")), + readdirSorted(join(packageDir, ".bun-cache", "no-deps@2.0.0@@localhost@@@1")), + ]), + ).toEqual([ + ["index.js", "package.json"], + ["index.js", "package.json"], + ]); + + // with node_modules package.json deleted, the package should be reinstalled + await rm(join(packageDir, "node_modules", "no-deps", "package.json")); + ({ out } = await runBunInstall(env, packageDir, { savesLockfile: false })); + expect(out).toContain("+ no-deps@2.0.0"); + + // another install is a no-op + ({ out } = await runBunInstall(env, packageDir, { savesLockfile: false })); + expect(out).not.toContain("+ no-deps@2.0.0"); + + // with cache package.json deleted, install is a no-op and cache is untouched + await rm(join(packageDir, ".bun-cache", "no-deps@2.0.0@@localhost@@@1", "package.json")); + ({ out } = await runBunInstall(env, packageDir, { savesLockfile: false })); + expect(out).not.toContain("+ no-deps@2.0.0"); + expect( + await Promise.all([ + readdirSorted(join(packageDir, "node_modules", "no-deps")), + readdirSorted(join(packageDir, ".bun-cache", "no-deps@2.0.0@@localhost@@@1")), + ]), + ).toEqual([["index.js", "package.json"], ["index.js"]]); + + // now with node_modules package.json deleted, the package AND the cache should + // be repopulated + await rm(join(packageDir, "node_modules", "no-deps", "package.json")); + ({ out } = await runBunInstall(env, packageDir, { savesLockfile: false })); + expect(out).toContain("+ no-deps@2.0.0"); + expect( + await Promise.all([ + readdirSorted(join(packageDir, "node_modules", "no-deps")), + readdirSorted(join(packageDir, ".bun-cache", "no-deps@2.0.0@@localhost@@@1")), + ]), + ).toEqual([ + ["index.js", "package.json"], + ["index.js", "package.json"], + ]); +}); + test("it should install with missing bun.lockb, node_modules, and/or cache", async () => { // first clean install await writeFile(