Skip to content

pack/publish: pack() consumes Context, returns Publish::Context<'a> #37670

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
robobun wants to merge 1 commit into
base: main
Choose a base branch
from
+48 −105
Open

pack/publish: pack() consumes Context, returns Publish::Context<'a> #37670

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 34 additions & 67 deletions src/runtime/cli/pack_command.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,21 +69,12 @@ fn file_to_source_at(dir: &Dir, path: &ZStr) -> bun_sys::Maybe<bun_ast::Source>
))
}

/// `manager.log` deref — set once at `init()`.
/// Raw-pointer receiver so the borrow doesn't conflict with the simultaneous
/// `&mut workspace_package_json_cache` borrow at the call site.
#[inline]
fn pm_log<'a>(m: *mut PackageManager) -> &'a mut bun_ast::Log {
// SAFETY: `m` came from `&mut PackageManager`; `log` is non-null after
// `PackageManager::init()`.
unsafe { &mut *(*m).log }
}
/// `manager.workspace_package_json_cache` field projection via raw pointer.
/// Raw projection: `pack()` holds the `&mut MapEntry` this yields across other `ctx.manager` uses.
#[inline]
fn pm_workspace_cache<'a>(
m: *mut PackageManager,
) -> &'a mut WorkspacePackageJSONCache::WorkspacePackageJSONCache {
// SAFETY: `m` came from `&mut PackageManager`; field disjoint from `log`.
// SAFETY: `m` is `pack()`'s live `ctx.manager`; `pack()` touches the cache only through here.
unsafe { &mut (*m).workspace_package_json_cache }
}
#[inline]
Expand All @@ -106,7 +97,8 @@ pub(crate) struct PackCommand;
// Context
// ───────────────────────────────────────────────────────────────────────────

pub(crate) struct Context<'a> {
/// `'l` is the caller-local lockfile; `'a` flows on into the `Publish::Context` `pack()` returns.
pub(crate) struct Context<'a, 'l> {
pub(crate) manager: &'a mut PackageManager,
// allocator param dropped — global mimalloc (see PORTING.md §Allocators)
pub(crate) command_ctx: Command::Context<'a>,
Expand All @@ -115,7 +107,7 @@ pub(crate) struct Context<'a> {
/// it's possible we will need it for finding
/// workspace versions. This is the only valid lockfile
/// pointer in this file. `manager.lockfile` is incorrect
pub(crate) lockfile: Option<&'a Lockfile>,
pub(crate) lockfile: Option<&'l Lockfile>,

pub(crate) bundled_deps: Vec<BundledDep>,

Expand All @@ -130,7 +122,7 @@ pub struct Stats {
pub(crate) bundled_deps: usize,
}

impl<'a> Context<'a> {
impl Context<'_, '_> {
pub(crate) fn print_summary(
stats: Stats,
maybe_shasum: Option<&[u8; sha::SHA1::DIGEST]>,
Expand Down Expand Up @@ -215,14 +207,8 @@ impl PackCommand {
}

let mut lockfile = Lockfile::default();
// `log` is non-null after `PackageManager::init()`.
let log_ptr: *mut bun_ast::Log = manager.log;
let manager_ptr: *mut PackageManager = manager;
// SAFETY: `manager_ptr`/`log_ptr` came from live `&mut`; reborrowed
// disjointly (`log` is a separate allocation from the manager fields
// `load_from_cwd` touches).
let load_from_disk_result = lockfile
.load_from_cwd::<false>(Some(unsafe { &mut *manager_ptr }), unsafe { &mut *log_ptr });
let log = manager.log_mut();
let load_from_disk_result = lockfile.load_from_cwd::<false>(Some(&mut *manager), log);

let lockfile_ref: Option<&Lockfile> = match load_from_disk_result {
LoadResult::Ok(ok) => Some(&*ok.lockfile),
Expand Down Expand Up @@ -256,8 +242,8 @@ impl PackCommand {
);
}
}
if pm_log(manager_ptr).has_errors() {
let _ = pm_log(manager_ptr).print(std::ptr::from_mut(Output::error_writer()));
if log.has_errors() {
let _ = log.print(std::ptr::from_mut(Output::error_writer()));
}
Global::crash();
}
Expand All @@ -269,7 +255,7 @@ impl PackCommand {
// package.json path before constructing `Context`.
let abs_pkg_json = ZBox::from_bytes(manager.original_package_json_path.as_bytes());

let mut pack_ctx = Context {
let pack_ctx = Context {
manager,
command_ctx: ctx,
lockfile: lockfile_ref,
Expand All @@ -278,7 +264,7 @@ impl PackCommand {
};

// just pack the current workspace
if let Err(err) = pack::<false>(&mut pack_ctx, &abs_pkg_json) {
if let Err(err) = pack::<false>(pack_ctx, &abs_pkg_json) {
match err {
PackError::OutOfMemory => bun_core::out_of_memory(),
PackError::MissingPackageName | PackError::MissingPackageVersion => {
Expand Down Expand Up @@ -1896,23 +1882,16 @@ fn opt_pack_gzip_level(m: &PackageManager) -> Option<&[u8]> {
// Const generics cannot vary the
// return type directly, so both instantiations return an Option that is
// `Some` only when FOR_PUBLISH == true.
pub(crate) type PackReturn<'a, const FOR_PUBLISH: bool> = Option<Publish::Context<'a, true>>;

pub(crate) fn pack<const FOR_PUBLISH: bool>(
ctx: &mut Context<'_>,
pub(crate) fn pack<'a, const FOR_PUBLISH: bool>(
mut ctx: Context<'a, '_>,
abs_package_json_path: &ZStr,
) -> Result<PackReturn<'static, FOR_PUBLISH>, PackError<FOR_PUBLISH>> {
// Raw pointer for the `pm_workspace_cache`/`pm_log` disjoint-field
// projections and the `'static` lifetime extension when returning
// `Publish::Context`.
) -> Result<Option<Publish::Context<'a, true>>, PackError<FOR_PUBLISH>> {
let manager_ptr: *mut PackageManager = &raw mut *ctx.manager;
let log_level = ctx.manager.options.log_level;
let bump = pack_bump();
// Note: `workspace_package_json_cache` and `log` are disjoint fields on
// `PackageManager`; route through raw-pointer field projections so the
// two `&mut` borrows don't conflict.
let log = ctx.manager.log_mut();
let mut json = match pm_workspace_cache(manager_ptr).get_with_path(
pm_log(manager_ptr),
log,
abs_package_json_path.as_bytes(),
WorkspacePackageJSONCache::GetJSONOptions {
guess_indentation: true,
Expand All @@ -1933,7 +1912,7 @@ pub(crate) fn pack<const FOR_PUBLISH: bool>(
"failed to parse package.json: {}",
format_args!("{}", bstr::BStr::new(abs_package_json_path.as_bytes())),
);
let _ = pm_log(manager_ptr).print(std::ptr::from_mut(Output::error_writer()));
let _ = log.print(std::ptr::from_mut(Output::error_writer()));
Global::crash();
}
WorkspacePackageJSONCache::GetResult::Entry(entry) => entry,
Expand Down Expand Up @@ -2163,8 +2142,9 @@ pub(crate) fn pack<const FOR_PUBLISH: bool>(
let _ = pm_workspace_cache(manager_ptr).map.remove(cache_key);

// Re-read package.json from disk
let log = ctx.manager.log_mut();
json = match pm_workspace_cache(manager_ptr).get_with_path(
pm_log(manager_ptr),
log,
abs_package_json_path.as_bytes(),
WorkspacePackageJSONCache::GetJSONOptions {
guess_indentation: true,
Expand All @@ -2185,7 +2165,7 @@ pub(crate) fn pack<const FOR_PUBLISH: bool>(
"failed to parse package.json: {}",
format_args!("{}", bstr::BStr::new(abs_package_json_path.as_bytes())),
);
let _ = pm_log(manager_ptr).print(std::ptr::from_mut(Output::error_writer()));
let _ = log.print(std::ptr::from_mut(Output::error_writer()));
Global::crash();
}
WorkspacePackageJSONCache::GetResult::Entry(entry) => entry,
Expand Down Expand Up @@ -2376,7 +2356,7 @@ pub(crate) fn pack<const FOR_PUBLISH: bool>(
let mut bundled_pack_queue = iterate_bundled_deps(
&mut ctx.bundled_deps,
&mut ctx.stats,
pm_log(manager_ptr),
ctx.manager.log_mut(),
&root_dir,
log_level,
)?;
Expand All @@ -2388,7 +2368,7 @@ pub(crate) fn pack<const FOR_PUBLISH: bool>(
// don't create the tarball, but run scripts if they exist

print_archived_files_and_packages::<true>(
ctx,
&mut ctx,
&root_dir,
PackListOrQueue::Queue(&mut pack_queue),
0,
Expand Down Expand Up @@ -2446,17 +2426,9 @@ pub(crate) fn pack<const FOR_PUBLISH: bool>(
package_version,
&mut dest_buf[..],
);
// Note: `manager`/`command_ctx` reborrowed via raw pointer —
// both are process-lifetime
// singletons (see `cli::command::GLOBAL_CLI_CTX`).
return Ok(Some(Publish::Context {
// SAFETY: `manager_ptr` was derived from `&mut *ctx.manager`; the
// process-lifetime singleton outlives the returned `Publish::Context`.
manager: unsafe { &mut *manager_ptr },
// SAFETY: `ctx.command_ctx` aliases the process-lifetime
// `GLOBAL_CLI_CTX` singleton (see note above); reborrowed
// disjointly from `manager`.
command_ctx: unsafe { &mut *std::ptr::from_mut(ctx.command_ctx) },
manager: ctx.manager,
command_ctx: ctx.command_ctx,
package_name: package_name.into(),
package_version: package_version.into(),
abs_tarball_path: ZStr::boxed(abs_tarball_dest.as_bytes()),
Expand Down Expand Up @@ -2599,7 +2571,7 @@ pub(crate) fn pack<const FOR_PUBLISH: bool>(
// exit and `end()` once after the loops.

entry = archive_package_json(
ctx,
&mut ctx,
// SAFETY: `archive` is the non-null `*mut Archive` returned by
// `Archive::write_new()` above; only this thread accesses it.
unsafe { &mut *archive },
Expand Down Expand Up @@ -2674,7 +2646,7 @@ pub(crate) fn pack<const FOR_PUBLISH: bool>(
});

entry = add_archive_entry(
ctx,
&mut ctx,
fd,
&stat,
&item.path,
Expand Down Expand Up @@ -2729,7 +2701,7 @@ pub(crate) fn pack<const FOR_PUBLISH: bool>(
};

entry = add_archive_entry(
ctx,
&mut ctx,
file.handle,
&stat,
&item.path,
Expand Down Expand Up @@ -2885,7 +2857,7 @@ pub(crate) fn pack<const FOR_PUBLISH: bool>(
};

print_archived_files_and_packages::<false>(
ctx,
&mut ctx,
&root_dir,
PackListOrQueue::List(&pack_list),
edited_package_json.len(),
Expand Down Expand Up @@ -2923,13 +2895,8 @@ pub(crate) fn pack<const FOR_PUBLISH: bool>(

if FOR_PUBLISH {
return Ok(Some(Publish::Context {
// SAFETY: `manager_ptr` was derived from `&mut *ctx.manager`; the
// process-lifetime singleton outlives the returned `Publish::Context`.
manager: unsafe { &mut *manager_ptr },
// SAFETY: `ctx.command_ctx` aliases the process-lifetime
// `GLOBAL_CLI_CTX` singleton (see dry-run note above);
// reborrowed disjointly from `manager`.
command_ctx: unsafe { &mut *std::ptr::from_mut(ctx.command_ctx) },
manager: ctx.manager,
command_ctx: ctx.command_ctx,
package_name: package_name.into(),
package_version: package_version.into(),
abs_tarball_path: ZStr::boxed(abs_tarball_dest.as_bytes()),
Expand Down Expand Up @@ -3143,7 +3110,7 @@ impl<'a> fmt::Display for TarballNameFormatter<'a> {
}

fn archive_package_json(
ctx: &mut Context<'_>,
ctx: &mut Context<'_, '_>,
archive: &mut Archive,
entry: *mut ArchiveEntry,
root_dir: &Dir,
Expand Down Expand Up @@ -3193,7 +3160,7 @@ fn archive_package_json(
}

fn add_archive_entry(
ctx: &mut Context<'_>,
ctx: &mut Context<'_, '_>,
file: Fd,
stat: &bun_sys::Stat,
filename: &ZStr,
Expand Down Expand Up @@ -3816,7 +3783,7 @@ enum PackListOrQueue<'a> {
}

fn print_archived_files_and_packages<const IS_DRY_RUN: bool>(
ctx: &mut Context<'_>,
ctx: &mut Context<'_, '_>,
root_dir_std: &Dir,
pack_list: PackListOrQueue<'_>,
package_json_len: usize,
Expand Down
52 changes: 14 additions & 38 deletions src/runtime/cli/publish_command.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,22 +452,13 @@ impl<'a, const DIRECTORY_PUBLISH: bool> Context<'a, DIRECTORY_PUBLISH> {

/// `bun publish` without a tarball path. Automatically pack the current workspace and get
/// information required for publishing
// Note: the return type is pinned to `Context<'static, true>`, the only
// valid shape. `'static` matches `pack::pack`'s return —
// the embedded `&mut PackageManager` / `Command::Context` are process-
// lifetime singletons reborrowed through raw pointers there.
pub(crate) fn from_workspace(
ctx: Command::Context<'a>,
manager: &'a mut PackageManager,
) -> Result<Context<'static, true>, FromWorkspaceError> {
) -> Result<Context<'a, true>, FromWorkspaceError> {
let mut lockfile = Lockfile::default();
let manager_ptr: *mut PackageManager = manager;
let log: &mut bun_ast::Log = manager.log_mut();
// SAFETY: `manager_ptr` was just derived from `manager: &'a mut PackageManager`;
// `log` borrows the disjoint `.log` field, so the re-derived `&mut`
// never touches memory the live `log` borrow covers.
let load_from_disk_result =
lockfile.load_from_cwd::<false>(Some(unsafe { &mut *manager_ptr }), log);
let load_from_disk_result = lockfile.load_from_cwd::<false>(Some(&mut *manager), log);

let lockfile_ref: Option<&Lockfile> = match load_from_disk_result {
LoadResult::Ok(ok) => Some(&*ok.lockfile),
Expand Down Expand Up @@ -504,26 +495,19 @@ impl<'a, const DIRECTORY_PUBLISH: bool> Context<'a, DIRECTORY_PUBLISH> {

// Note: capture the package.json path before constructing
// `pack::Context` so the `&mut PackageManager` borrow doesn't conflict.
// SAFETY: `manager_ptr` came from `&'a mut PackageManager`.
let abs_pkg_json = bun_core::ZBox::from_bytes(
unsafe { &*manager_ptr }
.original_package_json_path
.as_bytes(),
);
let abs_pkg_json =
bun_core::ZBox::from_bytes(manager.original_package_json_path.as_bytes());

let mut pack_ctx = pack::Context {
// SAFETY: `manager_ptr` came from `&'a mut PackageManager`;
// `lockfile_ref` borrows the local `lockfile`, not the manager,
// so the re-derived `&mut` is the only live manager borrow.
manager: unsafe { &mut *manager_ptr },
let pack_ctx = pack::Context {
manager,
command_ctx: ctx,
lockfile: lockfile_ref,
bundled_deps: Vec::new(),
stats: pack::Stats::default(),
};

// `pack::<true>` returns `Some(Context<true>)` on success.
Ok(pack::pack::<true>(&mut pack_ctx, &abs_pkg_json)?
Ok(pack::pack::<true>(pack_ctx, &abs_pkg_json)?
.expect("pack::<true> always yields a publish context"))
}
}
Expand Down Expand Up @@ -552,7 +536,6 @@ impl PublishCommand {
}
};
drop(original_cwd);
let manager_ptr: *mut PackageManager = manager;

if cli.positionals.len() > 1 {
let context = match Context::<false>::from_tarball_path(
Expand Down Expand Up @@ -584,8 +567,8 @@ impl PublishCommand {
);
}
FromTarballError::InvalidPackageJSON => {
// SAFETY: `manager.log` is set once at init.
let _ = unsafe { &mut *(*manager_ptr).log }
let _ = manager
.log_mut()
.print(std::ptr::from_mut(Output::error_writer()));
Output::err_generic("failed to parse tarball package.json", ());
}
Expand Down Expand Up @@ -711,23 +694,18 @@ impl PublishCommand {
.put(b"npm_command", b"publish")
.map_err(|_| crate::Error::Alloc(bun_alloc::AllocError))?;

// Note: reshaped for borrowck — `command_ctx: &mut ContextData`
// is held by `context`; `run_package_script_foreground` needs
// `&mut ContextData` too. Re-derive from the raw pointer.
let cmd_ctx_ptr: *mut crate::cli::command::ContextData = context.command_ctx;
let use_system_shell = context.command_ctx.debug.use_system_shell;

if let Some(publish_script) = &context.publish_script {
if let Err(e) = Run::run_package_script_foreground(
// SAFETY: see above.
unsafe { &mut *cmd_ctx_ptr },
context.command_ctx,
publish_script,
b"publish",
&abs_workspace_path,
script_env,
&[],
context.manager.options.log_level == LogLevel::Silent,
// SAFETY: see above.
unsafe { &*cmd_ctx_ptr }.debug.use_system_shell,
use_system_shell,
) {
if matches!(e, crate::Error::MissingShell) {
Output::err_generic(
Expand All @@ -742,16 +720,14 @@ impl PublishCommand {

if let Some(postpublish_script) = &context.postpublish_script {
if let Err(e) = Run::run_package_script_foreground(
// SAFETY: see above.
unsafe { &mut *cmd_ctx_ptr },
context.command_ctx,
postpublish_script,
b"postpublish",
&abs_workspace_path,
script_env,
&[],
context.manager.options.log_level == LogLevel::Silent,
// SAFETY: see above.
unsafe { &*cmd_ctx_ptr }.debug.use_system_shell,
use_system_shell,
) {
if matches!(e, crate::Error::MissingShell) {
Output::err_generic(
Expand Down