diff --git a/src/libarchive/lib.rs b/src/libarchive/lib.rs index 3965e9b415fb..4c20e66b36ff 100644 --- a/src/libarchive/lib.rs +++ b/src/libarchive/lib.rs @@ -105,6 +105,8 @@ pub mod lib { fn archive_write_new() -> *mut Archive; fn archive_write_free(a: *mut Archive) -> Result; fn archive_write_close(a: *mut Archive) -> Result; + fn archive_write_set_bytes_in_last_block(a: *mut Archive, bytes: c_int) -> Result; + fn archive_set_error(a: *mut Archive, err: c_int, fmt: *const c_char, ...); fn archive_write_set_format_pax_restricted(a: *mut Archive) -> Result; fn archive_write_add_filter_gzip(a: *mut Archive) -> Result; fn archive_write_set_filter_option( @@ -114,7 +116,6 @@ pub mod lib { value: *const c_char, ) -> Result; fn archive_write_set_options(a: *mut Archive, opts: *const c_char) -> Result; - fn archive_write_open_filename(a: *mut Archive, filename: *const c_char) -> Result; fn archive_write_header(a: *mut Archive, entry: *mut Entry) -> Result; fn archive_write_data(a: *mut Archive, data: *const c_void, size: usize) -> la_ssize_t; fn archive_write_finish_entry(a: *mut Archive) -> Result; @@ -370,14 +371,15 @@ pub mod lib { // SAFETY: FFI call with no preconditions. unsafe { archive_write_new() } } - pub fn write_free(&self) -> Result { - // SAFETY: self came from archive_write_new(); not used after this. - unsafe { archive_write_free(self.as_mut_ptr()) } - } pub fn write_close(&self) -> Result { // SAFETY: self valid. unsafe { archive_write_close(self.as_mut_ptr()) } } + /// Padding unit for the last output block; the default pads it to 10 KiB, 1 disables padding. + pub fn write_set_bytes_in_last_block(&self, bytes: c_int) -> Result { + // SAFETY: self valid. + unsafe { archive_write_set_bytes_in_last_block(self.as_mut_ptr(), bytes) } + } pub fn write_set_format_pax_restricted(&self) -> Result { // SAFETY: self valid. unsafe { archive_write_set_format_pax_restricted(self.as_mut_ptr()) } @@ -406,10 +408,6 @@ pub mod lib { // SAFETY: self valid; ZStr guarantees NUL-termination. unsafe { archive_write_set_options(self.as_mut_ptr(), opts.as_ptr().cast()) } } - pub fn write_open_filename(&self, filename: &ZStr) -> Result { - // SAFETY: self valid; ZStr guarantees NUL-termination. - unsafe { archive_write_open_filename(self.as_mut_ptr(), filename.as_ptr().cast()) } - } pub fn write_header(&self, entry: &Entry) -> Result { // SAFETY: self valid; entry came from Entry::new()/read_next_header(). // `Entry` has interior mutability so `&Entry -> *mut Entry` is sound. @@ -831,7 +829,7 @@ pub mod lib { } pub unsafe extern "C" fn write_callback( - _a: *mut Archive, + a: *mut Archive, client_data: *mut c_void, buff: *const c_void, length: usize, @@ -845,6 +843,10 @@ pub mod lib { let data = unsafe { core::slice::from_raw_parts(buff.cast::(), length) }; if this.list.try_reserve(length).is_err() { this.had_error = true; + // libarchive sets no error of its own for a failed client write. + // SAFETY: `a` is the archive this callback was invoked for; the + // format string has no conversions, so no varargs are read. + unsafe { archive_set_error(a, libc::ENOMEM, c"No memory".as_ptr()) }; return -1; } this.list.extend_from_slice(data); diff --git a/src/runtime/cli/pack_command.rs b/src/runtime/cli/pack_command.rs index 4b767be83280..d838e0669c8c 100644 --- a/src/runtime/cli/pack_command.rs +++ b/src/runtime/cli/pack_command.rs @@ -19,7 +19,10 @@ use bun_parsers::json as JSON; // lift via `bun_ast::Expr::from(t2_expr)` at the call site. use bun_ast::{E, Expr, ExprData}; use bun_js_printer as js_printer; -use bun_libarchive::lib::{Archive, Entry as ArchiveEntry, Result as ArchiveStatus}; +use bun_libarchive::lib::{ + Archive, Entry as ArchiveEntry, GrowingBuffer, Result as ArchiveStatus, WriteArchive, + archive_write_open2, +}; use bun_paths::{self as path, PathBuffer, SEP_STR}; // `bun.ptr.CowString = CowSlice(u8)` — the lifetime-free struct port (init_owned/ // borrow_subslice/length live on `cow_slice::CowSliceZ`). @@ -1691,18 +1694,6 @@ const fn zstr_lit(s: &'static [u8]) -> &'static ZStr { /// Extension trait wrapping `*mut Archive` so existing `archive.method()` call /// sites compile without per-call `unsafe { &* }`. trait ArchivePtrExt { - fn write_set_format_pax_restricted(self) -> ArchiveResult; - fn write_add_filter_gzip(self) -> ArchiveResult; - fn write_set_filter_option( - self, - module: Option<&ZStr>, - key: &ZStr, - value: &ZStr, - ) -> ArchiveResult; - fn write_set_options(self, opts: &ZStr) -> ArchiveResult; - fn write_open_filename(self, path: &ZStr) -> ArchiveResult; - fn write_close(self) -> ArchiveResult; - fn write_free(self) -> ArchiveResult; fn error_string(self) -> &'static [u8]; fn read_support_format_tar(self) -> ArchiveResult; fn read_support_format_gnutar(self) -> ArchiveResult; @@ -1715,44 +1706,11 @@ trait ArchivePtrExt { fn read_free(self) -> ArchiveResult; } impl ArchivePtrExt for *mut Archive { - #[inline] - fn write_set_format_pax_restricted(self) -> ArchiveResult { - Archive::opaque_ref(self).write_set_format_pax_restricted() - } - #[inline] - fn write_add_filter_gzip(self) -> ArchiveResult { - Archive::opaque_ref(self).write_add_filter_gzip() - } - #[inline] - fn write_set_filter_option( - self, - module: Option<&ZStr>, - key: &ZStr, - value: &ZStr, - ) -> ArchiveResult { - Archive::opaque_ref(self).write_set_filter_option(module, key, value) - } - #[inline] - fn write_set_options(self, opts: &ZStr) -> ArchiveResult { - Archive::opaque_ref(self).write_set_options(opts) - } - #[inline] - fn write_open_filename(self, path: &ZStr) -> ArchiveResult { - Archive::opaque_ref(self).write_open_filename(path) - } - #[inline] - fn write_close(self) -> ArchiveResult { - Archive::opaque_ref(self).write_close() - } - #[inline] - fn write_free(self) -> ArchiveResult { - Archive::opaque_ref(self).write_free() - } #[inline] fn error_string(self) -> &'static [u8] { // Every `ArchivePtrExt` call site holds a live `*mut Archive` from - // `archive_{read,write}_new()` (the trait exists precisely so those - // sites avoid per-call `unsafe { &* }`). + // `archive_read_new()` (the trait exists precisely so those sites + // avoid per-call `unsafe { &* }`). Archive::opaque_ref(self).error_string() } #[inline] @@ -2428,15 +2386,6 @@ pub(crate) fn pack( } if FOR_PUBLISH { - let mut dest_buf = PathBuffer::uninit(); - let (abs_tarball_dest, _) = tarball_destination( - opt_pack_destination(ctx.manager), - opt_pack_filename(ctx.manager), - abs_workspace_path, - package_name, - package_version, - &mut dest_buf[..], - ); // Note: `manager`/`command_ctx` reborrowed via raw pointer — // both are process-lifetime // singletons (see `cli::command::GLOBAL_CLI_CTX`). @@ -2450,7 +2399,6 @@ pub(crate) fn pack( command_ctx: unsafe { &mut *std::ptr::from_mut(ctx.command_ctx) }, package_name: package_name.into(), package_version: package_version.into(), - abs_tarball_path: ZStr::boxed(abs_tarball_dest.as_bytes()), tarball_bytes: Box::new([]), uses_workspaces: false, publish_script, @@ -2465,7 +2413,9 @@ pub(crate) fn pack( let mut print_buf: Vec = Vec::new(); - let archive = Archive::write_new(); + // Must outlive `archive`, whose close (also run by its Drop) writes into it. + let mut tarball_buffer = GrowingBuffer::init(); + let archive = WriteArchive::new(); match archive.write_set_format_pax_restricted() { ArchiveResult::Failed | ArchiveResult::Fatal | ArchiveResult::Warn => { @@ -2528,43 +2478,48 @@ pub(crate) fn pack( _ => {} } - let mut dest_buf = PathBuffer::uninit(); - let (abs_tarball_dest, abs_tarball_dest_dir_end) = tarball_destination( - opt_pack_destination(ctx.manager), - opt_pack_filename(ctx.manager), - abs_workspace_path, - package_name, - package_version, - &mut dest_buf[..], - ); - // Note: reshaped for borrowck — abs_tarball_dest borrows dest_buf - let abs_tarball_dest_len = abs_tarball_dest.as_bytes().len(); - - { - // create the directory if it doesn't exist - let most_likely_a_slash = dest_buf[abs_tarball_dest_dir_end]; - dest_buf[abs_tarball_dest_dir_end] = 0; - // SAFETY: NUL written above - let abs_tarball_dest_dir = ZStr::from_buf(&dest_buf[..], abs_tarball_dest_dir_end); - let _ = bun_sys::Dir::cwd().make_path(abs_tarball_dest_dir.as_bytes()); - dest_buf[abs_tarball_dest_dir_end] = most_likely_a_slash; - } - - // SAFETY: dest_buf[abs_tarball_dest_len] == 0 (written by tarball_destination) - let abs_tarball_dest = ZStr::from_buf(&dest_buf[..], abs_tarball_dest_len); - - // TODO: experiment with `archive.writeOpenMemory()` - match archive.write_open_filename(abs_tarball_dest) { + // Same as `archive_write_open_filename` set for a file, so the output stays byte-identical. + match archive.write_set_bytes_in_last_block(1) { ArchiveResult::Failed | ArchiveResult::Fatal | ArchiveResult::Warn => { Output::err_generic( - "failed to open tarball file destination: \"{}\"", - format_args!("{}", bstr::BStr::new(abs_tarball_dest.as_bytes())), + "failed to set archive block padding: {}", + format_args!("{}", bstr::BStr::new(archive.error_string())), ); Global::crash(); } _ => {} } + // `bun publish` uploads the bytes from memory; only `bun pm pack` writes a file. + let mut dest_buf = PathBuffer::uninit(); + let abs_tarball_dest: Option<&ZStr> = if FOR_PUBLISH { + None + } else { + Some(pack_destination( + ctx.manager, + abs_workspace_path, + package_name, + package_version, + &mut dest_buf, + )) + }; + + if archive_write_open2( + &archive, + (&raw mut tarball_buffer).cast(), + Some(GrowingBuffer::open_callback), + Some(GrowingBuffer::write_callback), + Some(GrowingBuffer::close_callback), + None, + ) != 0 + { + Output::err_generic( + "failed to open archive for writing: {}", + format_args!("{}", bstr::BStr::new(archive.error_string())), + ); + Global::crash(); + } + // append removed items from `pack_queue` with their file size let mut pack_list: PackList = Vec::new(); @@ -2572,8 +2527,7 @@ pub(crate) fn pack( let mut file_reader: Box = new_boxed_buffered_file_reader(File::from_fd(Fd::invalid())); - // SAFETY: `archive` is the live `archive_write_new()` handle opened above. - let mut entry = ArchiveEntry::new2(unsafe { &*archive }); + let mut entry = ArchiveEntry::new2(&archive); { let mut progress = Progress::Progress::default(); @@ -2589,15 +2543,7 @@ pub(crate) fn pack( // uses below, so call `complete_one()` explicitly at every loop-body // exit and `end()` once after the loops. - entry = archive_package_json( - ctx, - // SAFETY: `archive` is the non-null `*mut Archive` returned by - // `Archive::write_new()` above; only this thread accesses it. - unsafe { &mut *archive }, - entry, - &root_dir, - &edited_package_json, - )?; + entry = archive_package_json(ctx, &archive, entry, &root_dir, &edited_package_json); if log_level.show_progress() { node.as_mut() .expect("infallible: progress active") @@ -2671,13 +2617,11 @@ pub(crate) fn pack( &item.path, &mut read_buf, &mut file_reader, - // SAFETY: `archive` is the non-null `*mut Archive` returned by - // `Archive::write_new()` above; only this thread accesses it. - unsafe { &mut *archive }, + &archive, entry, &mut print_buf, &bins, - )?; + ); if log_level.show_progress() { node.as_mut() @@ -2726,13 +2670,11 @@ pub(crate) fn pack( &item.path, &mut read_buf, &mut file_reader, - // SAFETY: `archive` is the non-null `*mut Archive` returned by - // `Archive::write_new()` above; only this thread accesses it. - unsafe { &mut *archive }, + &archive, entry, &mut print_buf, &bins, - )?; + ); if log_level.show_progress() { node.as_mut() @@ -2761,101 +2703,25 @@ pub(crate) fn pack( _ => {} } - match archive.write_free() { - ArchiveResult::Failed | ArchiveResult::Fatal | ArchiveResult::Warn => { - Output::err_generic( - "failed to free archive: {}", - format_args!("{}", bstr::BStr::new(archive.error_string())), - ); - Global::crash(); - } - _ => {} - } + drop(archive); + // Only fails after a write into the buffer failed, which crashed above. + let tarball_bytes = tarball_buffer.to_owned_slice()?; let mut shasum: [u8; sha::SHA1::DIGEST] = [0; sha::SHA1::DIGEST]; - let mut integrity: [u8; sha::SHA512::DIGEST] = [0; sha::SHA512::DIGEST]; - - let tarball_bytes: Option> = 'tarball_bytes: { - let tarball_file = match File::open(abs_tarball_dest, bun_sys::O::RDONLY, 0) { - Ok(f) => f, - Err(err) => { - Output::err( - err, - "failed to open tarball at: \"{}\"", - format_args!("{}", bstr::BStr::new(abs_tarball_dest.as_bytes())), - ); - Global::crash(); - } - }; - - let mut sha1 = sha::SHA1::init(); - let mut sha512 = sha::SHA512::init(); - - if FOR_PUBLISH { - let bytes = match tarball_file.read_to_end() { - Ok(b) => b, - Err(err) => { - Output::err( - err, - "failed to read tarball: \"{}\"", - format_args!("{}", bstr::BStr::new(abs_tarball_dest.as_bytes())), - ); - Global::crash(); - } - }; - - sha1.update(&bytes); - sha512.update(&bytes); - - sha1.r#final(&mut shasum); - sha512.r#final(&mut integrity); + let mut sha1 = sha::SHA1::init(); + sha1.update(&tarball_bytes); + sha1.r#final(&mut shasum); - ctx.stats.packed_size = bytes.len(); - - break 'tarball_bytes Some(bytes); - } - - reset_buffered_file_reader(&mut file_reader, File::from_fd(tarball_file.into_raw())); - - let mut size: usize = 0; - let mut read = match buffered_file_reader_read(&mut file_reader, &mut read_buf) { - Ok(n) => n, - Err(err) => { - Output::err( - err, - "failed to read tarball: \"{}\"", - format_args!("{}", bstr::BStr::new(abs_tarball_dest.as_bytes())), - ); - Global::crash(); - } - }; - while read > 0 { - sha1.update(&read_buf[..read]); - sha512.update(&read_buf[..read]); - size += read; - read = match buffered_file_reader_read(&mut file_reader, &mut read_buf) { - Ok(n) => n, - Err(err) => { - Output::err( - err, - "failed to read tarball: \"{}\"", - format_args!("{}", bstr::BStr::new(abs_tarball_dest.as_bytes())), - ); - Global::crash(); - } - }; - } + let mut integrity: [u8; sha::SHA512::DIGEST] = [0; sha::SHA512::DIGEST]; + let mut sha512 = sha::SHA512::init(); + sha512.update(&tarball_bytes); + sha512.r#final(&mut integrity); - sha1.r#final(&mut shasum); - sha512.r#final(&mut integrity); + ctx.stats.packed_size = tarball_bytes.len(); - ctx.stats.packed_size = size; - None - }; - let _ = core::mem::replace( - &mut file_reader.unbuffered_reader, - File::from_fd(Fd::invalid()), - ); + if let Some(abs_tarball_dest) = abs_tarball_dest { + write_tarball(abs_tarball_dest, &tarball_bytes); + } let normalized_pkg_info: Option> = if FOR_PUBLISH { // The mutated tree is consumed inside `normalized_package` (it prints @@ -2882,7 +2748,7 @@ pub(crate) fn pack( edited_package_json.len(), ); - if !FOR_PUBLISH { + if let Some(abs_tarball_dest) = abs_tarball_dest { if opt_pack_destination(ctx.manager).is_empty() && opt_pack_filename(ctx.manager).is_empty() { Context::print_tarball_path( @@ -2923,8 +2789,7 @@ pub(crate) fn pack( command_ctx: unsafe { &mut *std::ptr::from_mut(ctx.command_ctx) }, package_name: package_name.into(), package_version: package_version.into(), - abs_tarball_path: ZStr::boxed(abs_tarball_dest.as_bytes()), - tarball_bytes: tarball_bytes.unwrap_or_default().into_boxed_slice(), + tarball_bytes: tarball_bytes.into_boxed_slice(), uses_workspaces: false, publish_script, postpublish_script, @@ -3131,13 +2996,83 @@ impl<'a> fmt::Display for TarballNameFormatter<'a> { } } +/// Resolves where `bun pm pack` writes the tarball and creates that directory. +fn pack_destination<'a>( + manager: &PackageManager, + abs_workspace_path: &[u8], + package_name: &[u8], + package_version: &[u8], + buf: &'a mut PathBuffer, +) -> &'a ZStr { + let (abs_tarball_dest, dir_end) = tarball_destination( + opt_pack_destination(manager), + opt_pack_filename(manager), + abs_workspace_path, + package_name, + package_version, + &mut buf[..], + ); + let len = abs_tarball_dest.as_bytes().len(); + let _ = bun_sys::Dir::cwd().make_path(&buf[..dir_end]); + // SAFETY: buf[len] == 0 (written by tarball_destination) + ZStr::from_buf(&buf[..], len) +} + +/// The only write to disk, and so the only failure that has a partial file to remove. +fn write_tarball(abs_tarball_dest: &ZStr, tarball_bytes: &[u8]) { + let file = match File::create(Fd::cwd(), abs_tarball_dest, true) { + Ok(file) => file, + Err(err) => { + Output::err( + err, + "failed to open tarball file destination: \"{}\"", + format_args!("{}", bstr::BStr::new(abs_tarball_dest.as_bytes())), + ); + Global::crash(); + } + }; + if let Err(err) = file.write_all(tarball_bytes) { + Output::err( + err, + "failed to write tarball: \"{}\"", + format_args!("{}", bstr::BStr::new(abs_tarball_dest.as_bytes())), + ); + // Windows cannot delete the file while it is open. + drop(file); + // A `--filename` that is a symlink or a device is not ours to delete. + if let Ok(stat) = bun_sys::lstat(abs_tarball_dest) { + if bun_sys::kind_from_mode(stat.st_mode as bun_sys::Mode) == bun_sys::FileKind::File { + let _ = bun_sys::unlink(abs_tarball_dest); + } + } + Global::crash(); + } +} + +/// Returns the number of bytes libarchive accepted for the current entry. +fn write_entry_data(archive: &Archive, pathname: &[u8], data: &[u8]) -> usize { + match usize::try_from(archive.write_data(data)) { + Ok(written) => written, + Err(_) => { + Output::err_generic( + "failed to write \"{}\" to tarball: {}", + ( + bstr::BStr::new(pathname), + bstr::BStr::new(archive.error_string()), + ), + ); + Global::crash(); + } + } +} + fn archive_package_json( ctx: &mut Context<'_>, - archive: &mut Archive, + archive: &Archive, entry: *mut ArchiveEntry, root_dir: &Dir, edited_package_json: &[u8], -) -> Result<*mut ArchiveEntry, AllocError> { +) -> *mut ArchiveEntry { // `entry` is the same pointer after `.clear()`. let entry = ArchiveEntry::opaque_ref(entry); let stat = match bun_sys::fstatat(Fd::from_std_dir(root_dir), bun_core::zstr!("package.json")) { @@ -3165,20 +3100,16 @@ fn archive_package_json( ArchiveStatus::Failed | ArchiveStatus::Fatal | ArchiveStatus::Warn => { Output::err_generic( "failed to write tarball header: {}", - format_args!( - "{}", - bstr::BStr::new(std::ptr::from_mut::(archive).error_string()) - ), + format_args!("{}", bstr::BStr::new(archive.error_string())), ); Global::crash(); } _ => {} } - ctx.stats.unpacked_size += - usize::try_from(archive.write_data(edited_package_json)).expect("int cast"); + ctx.stats.unpacked_size += write_entry_data(archive, b"package.json", edited_package_json); - Ok(entry.clear()) + entry.clear() } fn add_archive_entry( @@ -3188,11 +3119,11 @@ fn add_archive_entry( filename: &ZStr, read_buf: &mut [u8], file_reader: &mut BufferedFileReader, - archive: &mut Archive, + archive: &Archive, entry: *mut ArchiveEntry, print_buf: &mut Vec, bins: &[BinInfo], -) -> Result<*mut ArchiveEntry, AllocError> { +) -> *mut ArchiveEntry { // `entry` is the same pointer after `.clear()`. let entry = ArchiveEntry::opaque_ref(entry); write!( @@ -3231,10 +3162,7 @@ fn add_archive_entry( ArchiveStatus::Failed | ArchiveStatus::Fatal => { Output::err_generic( "failed to write tarball header: {}", - format_args!( - "{}", - bstr::BStr::new(std::ptr::from_mut::(archive).error_string()) - ), + format_args!("{}", bstr::BStr::new(archive.error_string())), ); Global::crash(); } @@ -3256,7 +3184,7 @@ fn add_archive_entry( }; while read > 0 { ctx.stats.unpacked_size += - usize::try_from(archive.write_data(&read_buf[..read])).expect("int cast"); + write_entry_data(archive, filename.as_bytes(), &read_buf[..read]); read = match buffered_file_reader_read(file_reader, read_buf) { Ok(n) => n, Err(err) => { @@ -3276,7 +3204,7 @@ fn add_archive_entry( // close a fd we don't own. reset_buffered_file_reader(file_reader, File::from_fd(Fd::invalid())); - Ok(entry.clear()) + entry.clear() } /// Strips workspace and catalog protocols from dependency versions then diff --git a/src/runtime/cli/publish_command.rs b/src/runtime/cli/publish_command.rs index ac6e1736eb84..1cd0fdcc20a2 100644 --- a/src/runtime/cli/publish_command.rs +++ b/src/runtime/cli/publish_command.rs @@ -97,7 +97,6 @@ pub(crate) struct Context<'a, const DIRECTORY_PUBLISH: bool> { pub(crate) package_name: Box<[u8]>, pub(crate) package_version: Box<[u8]>, - pub(crate) abs_tarball_path: Box, pub(crate) tarball_bytes: Box<[u8]>, pub(crate) uses_workspaces: bool, @@ -438,7 +437,6 @@ impl<'a, const DIRECTORY_PUBLISH: bool> Context<'a, DIRECTORY_PUBLISH> { command_ctx: ctx, package_name, package_version, - abs_tarball_path: ZStr::boxed(abs_tarball_path.as_bytes()), tarball_bytes: tarball_bytes.into(), uses_workspaces: false, normalized_pkg_info, @@ -657,9 +655,6 @@ impl PublishCommand { } }; - // TODO: read this into memory - let _ = bun_sys::unlink(&context.abs_tarball_path); - if let Err(err) = Self::publish::(&context) { match err { PublishError::OutOfMemory => bun_core::out_of_memory(), @@ -2027,10 +2022,7 @@ impl PublishCommand { install::dependency::without_build_tag(&ctx.package_version); let mut buf: Vec = Vec::with_capacity( - ctx.package_name.len() * 5 - + version_without_build_tag.len() * 4 - + ctx.abs_tarball_path.len() - + encoded_tarball_len, + ctx.package_name.len() * 5 + version_without_build_tag.len() * 4 + encoded_tarball_len, ); let _ = write!( diff --git a/test/cli/install/bun-pack.test.ts b/test/cli/install/bun-pack.test.ts index b89765712514..dfc8565eb858 100644 --- a/test/cli/install/bun-pack.test.ts +++ b/test/cli/install/bun-pack.test.ts @@ -1,7 +1,7 @@ -import { file, spawn, write } from "bun"; +import { file, gunzipSync, spawn, write } from "bun"; import { readTarball } from "bun:internal-for-testing"; import { beforeEach, describe, expect, test } from "bun:test"; -import { exists, mkdir, rm } from "fs/promises"; +import { chmod, exists, lstat, mkdir, rm, symlink } from "fs/promises"; import { bunEnv, bunExe, isLinux, isWindows, pack, runBunInstall, tempDir, tmpdirSync } from "harness"; import fs from "node:fs/promises"; import { join } from "path"; @@ -51,6 +51,22 @@ test("basic", async () => { expect(tarball.entries).toMatchObject([{ "pathname": "package/package.json" }, { "pathname": "package/index.js" }]); }); +// The archive ends right after the two end-of-archive blocks. libarchive's default for a custom +// output sink would pad it to a full 10 KiB record instead, which would change the size and shasum +// of every tarball bun produces. +test("the archive is not padded to a full tar record", async () => { + await Promise.all([ + write(join(packageDir, "package.json"), JSON.stringify({ name: "pack-unpadded", version: "1.0.0" })), + write(join(packageDir, "index.js"), "module.exports = 1;"), + ]); + + await pack(packageDir, bunEnv); + + const tar = gunzipSync(await file(join(packageDir, "pack-unpadded-1.0.0.tgz")).bytes()); + // header + data for each of the two entries, then the two end-of-archive blocks + expect(tar.byteLength).toBe(6 * 512); +}); + test("in subdirectory", async () => { await Promise.all([ write( @@ -562,6 +578,106 @@ describe("flags", () => { }); }); +// The tarball used to be streamed to its destination while it was being built, so any failure on +// the way exited 1 and left a truncated `-.tgz` behind for the next `bun publish +// ./*.tgz` to pick up. `ulimit -f 0` (RLIMIT_FSIZE) makes every write to the tarball fail with +// EFBIG, which unlike a permission based setup also works when the tests run as root; setting it +// needs a POSIX shell. +describe.skipIf(isWindows)("a failed pack leaves no tarball behind", () => { + const packageJson = JSON.stringify({ name: "pack-failed", version: "1.0.0" }); + + async function packExpectingFailure(cwd: string, { fileSizeLimit }: { fileSizeLimit: boolean }, ...args: string[]) { + await using proc = spawn({ + cmd: fileSizeLimit + ? ["/bin/sh", "-c", 'ulimit -f 0 && exec "$0" pm pack "$@"', bunExe(), ...args] + : [bunExe(), "pm", "pack", ...args], + cwd, + stdout: "pipe", + stderr: "pipe", + stdin: "ignore", + env: bunEnv, + }); + const [, err, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { err, exitCode }; + } + + test.concurrent.each([ + { args: [], tarball: "pack-failed-1.0.0.tgz" }, + { args: ["--destination=out"], tarball: join("out", "pack-failed-1.0.0.tgz") }, + { args: ["--filename=custom.tgz"], tarball: "custom.tgz" }, + ])("when the tarball cannot be written (args: $args)", async ({ args, tarball }) => { + await using dir = tempDir("pack-failed", { + "package.json": packageJson, + "index.js": "module.exports = 1;", + }); + + const { err, exitCode } = await packExpectingFailure(dir, { fileSizeLimit: true }, ...args); + + expect(err).toContain("EFBIG"); + expect(err).toContain('failed to write tarball: "'); + expect({ exitCode, tarballExists: await exists(join(dir, tarball)) }).toEqual({ + exitCode: 1, + tarballExists: false, + }); + }); + + // Root can read a mode 000 file, so this one only runs as a regular user. + test.concurrent.skipIf(process.getuid?.() === 0)("when one of the files cannot be opened", async () => { + await using dir = tempDir("pack-failed-unreadable", { + "package.json": packageJson, + "index.js": "module.exports = 1;", + "unreadable.js": "module.exports = 2;", + }); + await chmod(join(dir, "unreadable.js"), 0o000); + + const { err, exitCode } = await packExpectingFailure(dir, { fileSizeLimit: false }); + + expect(err).toContain('EACCES: Permission denied: failed to open file: "unreadable.js"'); + expect({ exitCode, tarballExists: await exists(join(dir, "pack-failed-1.0.0.tgz")) }).toEqual({ + exitCode: 1, + tarballExists: false, + }); + }); + + // Only the regular file pack itself created (or truncated) is removed; a destination that is + // something else, like a symlink, is not pack's to delete. + test.concurrent("does not delete a --filename that is a symlink", async () => { + await using dir = tempDir("pack-failed-symlink", { + "package.json": packageJson, + "index.js": "module.exports = 1;", + "target.tgz": "", + }); + await symlink("target.tgz", join(dir, "link.tgz")); + + const { err, exitCode } = await packExpectingFailure(dir, { fileSizeLimit: true }, "--filename=link.tgz"); + + expect(err).toContain('failed to write tarball: "link.tgz"'); + expect({ exitCode, linkIsSymlink: (await lstat(join(dir, "link.tgz"))).isSymbolicLink() }).toEqual({ + exitCode: 1, + linkIsSymlink: true, + }); + }); + + // A destination that cannot even be opened was not written to, so it is left as it is (root can + // open a read-only file, hence the skip). + test.concurrent.skipIf(process.getuid?.() === 0)("keeps a destination it cannot open for writing", async () => { + await using dir = tempDir("pack-failed-readonly-dest", { + "package.json": packageJson, + "index.js": "module.exports = 1;", + "pack-failed-1.0.0.tgz": "an earlier tarball", + }); + await chmod(join(dir, "pack-failed-1.0.0.tgz"), 0o444); + + const { err, exitCode } = await packExpectingFailure(dir, { fileSizeLimit: false }); + + expect(err).toContain('EACCES: Permission denied: failed to open tarball file destination: "'); + expect({ exitCode, destination: await file(join(dir, "pack-failed-1.0.0.tgz")).text() }).toEqual({ + exitCode: 1, + destination: "an earlier tarball", + }); + }); +}); + test("shasum and integrity are consistent", async () => { await Promise.all([ write( diff --git a/test/cli/install/bun-publish.test.ts b/test/cli/install/bun-publish.test.ts index c7f9df4fc406..960ca7322352 100644 --- a/test/cli/install/bun-publish.test.ts +++ b/test/cli/install/bun-publish.test.ts @@ -765,6 +765,27 @@ describe("--dry-run", async () => { expect(await exists(join(registry.packagesPath, "dry-run-1"))).toBeFalse(); }); + // Publishing from a directory used to unlink `-.tgz` in the package directory + // (where it packed to), also with --dry-run, where nothing had been packed and the file was + // whatever the user had put there. + test("leaves a tarball that is already in the package directory alone", async () => { + const { packageDir, packageJson } = await registry.createTestDir(); + const bunfig = await registry.authBunfig("dryrunexisting"); + await Promise.all([ + rm(join(registry.packagesPath, "dry-run-3"), { recursive: true, force: true }), + write(join(packageDir, "bunfig.toml"), bunfig), + write(packageJson, JSON.stringify({ name: "dry-run-3", version: "3.3.3" })), + ]); + await pack(packageDir, env); + const tarball = join(packageDir, "dry-run-3-3.3.3.tgz"); + const packed = await file(tarball).bytes(); + + const { exitCode } = await publish(env, packageDir, "--dry-run"); + + expect({ exitCode, tarballExists: await exists(tarball) }).toEqual({ exitCode: 0, tarballExists: true }); + expect(await file(tarball).bytes()).toEqual(packed); + expect(await exists(join(registry.packagesPath, "dry-run-3"))).toBeFalse(); + }); test("does not publish from tarball path", async () => { const { packageDir, packageJson } = await registry.createTestDir(); const bunfig = await registry.authBunfig("dryruntarball"); @@ -963,6 +984,32 @@ test("attempting to publish a private package should fail", async () => { expect(await exists(join(packageDir, "publish-pkg-6-6.6.6.tgz"))).toBeTrue(); }); +// Publishing from a directory used to pack into that directory and delete the file again after +// publishing, so any failure in between (here: postpack) left the tarball behind. +test("a failed publish does not leave a tarball in the package directory", async () => { + const { packageDir, packageJson } = await registry.createTestDir(); + await Promise.all([ + write( + packageJson, + JSON.stringify({ + name: "publish-postpack-failed", + version: "1.0.0", + scripts: { postpack: "exit 3" }, + }), + ), + write(join(packageDir, "index.js"), "module.exports = 1;"), + write(join(packageDir, "bunfig.toml"), await registry.authBunfig("postpackfailed")), + ]); + + const { err, exitCode } = await publish(env, packageDir); + + expect(err).toContain('script "postpack" exited with code 3'); + expect({ exitCode, tarballExists: await exists(join(packageDir, "publish-postpack-failed-1.0.0.tgz")) }).toEqual({ + exitCode: 3, + tarballExists: false, + }); +}); + describe("access", async () => { test("--access", async () => { const { packageDir, packageJson } = await registry.createTestDir();