Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
38 changes: 30 additions & 8 deletions src/runtime/cli/pack_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@ pub(crate) struct Context<'a> {
/// pointer in this file. `manager.lockfile` is incorrect
pub(crate) lockfile: Option<&'a Lockfile>,

/// Where the command was run from; relative `--destination`/`--filename` resolve against it.
pub(crate) original_cwd: &'a [u8],

pub(crate) bundled_deps: Vec<BundledDep>,

pub(crate) stats: Stats,
Expand Down Expand Up @@ -201,6 +204,7 @@ impl PackCommand {
pub(crate) fn exec_with_manager(
ctx: Command::Context<'_>,
manager: &mut PackageManager,
original_cwd: &[u8],
) -> crate::Result<()> {
use bun_install::lockfile::{LoadResult, LoadStep};

Expand Down Expand Up @@ -273,6 +277,7 @@ impl PackCommand {
manager,
command_ctx: ctx,
lockfile: lockfile_ref,
original_cwd,
bundled_deps: Vec::new(),
stats: Stats::default(),
};
Expand Down Expand Up @@ -2402,6 +2407,7 @@ pub(crate) fn pack<const FOR_PUBLISH: bool>(
let (abs_tarball_dest, _) = tarball_destination(
opt_pack_destination(ctx.manager),
opt_pack_filename(ctx.manager),
ctx.original_cwd,
abs_workspace_path,
package_name,
package_version,
Expand Down Expand Up @@ -2432,6 +2438,7 @@ pub(crate) fn pack<const FOR_PUBLISH: bool>(
let (abs_tarball_dest, _) = tarball_destination(
opt_pack_destination(ctx.manager),
opt_pack_filename(ctx.manager),
ctx.original_cwd,
abs_workspace_path,
package_name,
package_version,
Expand Down Expand Up @@ -2532,6 +2539,7 @@ pub(crate) fn pack<const FOR_PUBLISH: bool>(
let (abs_tarball_dest, abs_tarball_dest_dir_end) = tarball_destination(
opt_pack_destination(ctx.manager),
opt_pack_filename(ctx.manager),
ctx.original_cwd,
abs_workspace_path,
package_name,
package_version,
Expand Down Expand Up @@ -2989,9 +2997,11 @@ fn has_unsafe_tarball_filename_part(value: &[u8]) -> bool {
|| strings::contains_any(value, b"\\:\0")
}

/// Returns the tarball path and the length of the directory the caller creates (0 for `--filename`).
fn tarball_destination<'a>(
pack_destination: &[u8],
pack_filename: &[u8],
original_cwd: &[u8],
abs_workspace_path: &[u8],
package_name: &[u8],
package_version: &[u8],
Expand All @@ -3008,25 +3018,37 @@ fn tarball_destination<'a>(
Global::crash();
}
if !pack_filename.is_empty() {
if pack_filename.len() + 1 > dest_buf.len() {
// Leave room for the NUL terminator written after the join.
let join_buf_len = dest_buf.len() - 1;
let Some(tarball_name_len) =
resolve_path::join_abs_string_buf_checked::<resolve_path::platform::Auto>(
original_cwd,
&mut dest_buf[..join_buf_len],
&[pack_filename],
)
.map(<[u8]>::len)
else {
Output::err_generic(
"archive filename too long: \"{}\"",
format_args!("{}", bstr::BStr::new(pack_filename)),
);
Global::crash();
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
dest_buf[..pack_filename.len()].copy_from_slice(pack_filename);
dest_buf[pack_filename.len()] = 0;
let tarball_name_len = pack_filename.len() + 1;
};
dest_buf[tarball_name_len] = 0;

// SAFETY: NUL written at pack_filename.len()
return (ZStr::from_buf(&dest_buf[..], tarball_name_len - 1), 0);
// SAFETY: NUL written at tarball_name_len
return (ZStr::from_buf(&dest_buf[..], tarball_name_len), 0);
} else {
let destination_base = if pack_destination.is_empty() {
abs_workspace_path
} else {
original_cwd
};
let (dir_len_trimmed, dir_len_full) = {
let tarball_destination_dir = resolve_path::join_abs_string_buf::<
resolve_path::platform::Auto,
>(
abs_workspace_path, dest_buf, &[pack_destination]
destination_base, dest_buf, &[pack_destination]
);
(
strings::without_trailing_slash(tarball_destination_dir).len(),
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/cli/package_manager_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ Learn more about these at <magenta>https://bun.com/docs/cli/pm<r>.\n";
ScanCommand::exec_with_manager(&mut *ctx, pm, &cwd)?;
Global::exit(0);
} else if strings::eql_comptime(subcommand, b"pack") {
PackCommand::exec_with_manager(ctx, pm)?;
PackCommand::exec_with_manager(ctx, pm, &cwd)?;
Global::exit(0);
} else if strings::eql_comptime(subcommand, b"whoami") {
let username = match Npm::whoami(pm) {
Expand Down
5 changes: 3 additions & 2 deletions src/runtime/cli/publish_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,7 @@ impl<'a, const DIRECTORY_PUBLISH: bool> Context<'a, DIRECTORY_PUBLISH> {
pub(crate) fn from_workspace(
ctx: Command::Context<'a>,
manager: &'a mut PackageManager,
original_cwd: &[u8],
) -> Result<Context<'static, true>, FromWorkspaceError> {
let mut lockfile = Lockfile::default();
let manager_ptr: *mut PackageManager = manager;
Expand Down Expand Up @@ -516,6 +517,7 @@ impl<'a, const DIRECTORY_PUBLISH: bool> Context<'a, DIRECTORY_PUBLISH> {
manager: unsafe { &mut *manager_ptr },
command_ctx: ctx,
lockfile: lockfile_ref,
original_cwd,
bundled_deps: Vec::new(),
stats: pack::Stats::default(),
};
Expand Down Expand Up @@ -549,7 +551,6 @@ impl PublishCommand {
Global::crash();
}
};
drop(original_cwd);
let manager_ptr: *mut PackageManager = manager;

if cli.positionals.len() > 1 {
Expand Down Expand Up @@ -628,7 +629,7 @@ impl PublishCommand {
return Ok(());
}

let context = match Context::<true>::from_workspace(ctx, manager) {
let context = match Context::<true>::from_workspace(ctx, manager, &original_cwd) {
Ok(c) => c,
Err(err) => {
use pack::PackError;
Expand Down
165 changes: 165 additions & 0 deletions test/cli/install/bun-pack.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,171 @@ describe("flags", () => {
// --dry-run never writes the tarball.
expect(await exists(join(packageDir, "pack-quiet-dry-test-1.1.1.tgz"))).toBeFalse();
});

describe("relative --destination and --filename resolve against the cwd", () => {
// `bun pm pack` chdirs to the package root, or to the workspace root above it, before packing.
// The flags were typed relative to the directory the command was run from, so that is what
// they must resolve against: not the directory bun chdir'd to, and not the packed package's directory.
const packageJsonEntry = { pathname: "package/package.json" };

test.concurrent("--filename from a workspace package", async () => {
using dir = tempDir("pack-cwd-filename-workspace", {
"package.json": JSON.stringify({ name: "pack-cwd-root", version: "0.0.0", workspaces: ["packages/*"] }),
"packages/pkg/package.json": JSON.stringify({ name: "pack-cwd-pkg", version: "1.2.3" }),
});
const pkgDir = join(String(dir), "packages", "pkg");

const { out } = await pack(pkgDir, bunEnv, "--quiet", "--filename=./pkg.tgz");

expect(out).toBe(`${join(pkgDir, "pkg.tgz")}\n`);
expect(await exists(join(String(dir), "pkg.tgz"))).toBeFalse();
expect(readTarball(join(pkgDir, "pkg.tgz")).entries).toMatchObject([packageJsonEntry]);
});

test.concurrent("--destination from a workspace package", async () => {
using dir = tempDir("pack-cwd-destination-workspace", {
"package.json": JSON.stringify({ name: "pack-cwd-root", version: "0.0.0", workspaces: ["packages/*"] }),
"packages/pkg/package.json": JSON.stringify({ name: "pack-cwd-pkg", version: "1.2.3" }),
});
const pkgDir = join(String(dir), "packages", "pkg");

const { out } = await pack(pkgDir, bunEnv, "--quiet", "--destination=./out");

const tarballPath = join(pkgDir, "out", "pack-cwd-pkg-1.2.3.tgz");
expect(out).toBe(`${tarballPath}\n`);
expect(await exists(join(String(dir), "out"))).toBeFalse();
expect(readTarball(tarballPath).entries).toMatchObject([packageJsonEntry]);
});

test.concurrent("--destination from a subdirectory of a workspace package", async () => {
using dir = tempDir("pack-cwd-destination-workspace-subdir", {
"package.json": JSON.stringify({ name: "pack-cwd-root", version: "0.0.0", workspaces: ["packages/*"] }),
"packages/pkg/package.json": JSON.stringify({ name: "pack-cwd-pkg", version: "1.2.3" }),
"packages/pkg/src": {},
});
const pkgDir = join(String(dir), "packages", "pkg");

// cwd is packages/pkg/src, the packed package is packages/pkg, and bun chdir'd to the workspace root
const { out } = await pack(join(pkgDir, "src"), bunEnv, "--quiet", "--destination=../dist");

const tarballPath = join(pkgDir, "dist", "pack-cwd-pkg-1.2.3.tgz");
expect(out).toBe(`${tarballPath}\n`);
expect(await exists(join(String(dir), "packages", "dist"))).toBeFalse();
expect(await exists(join(String(dir), "dist"))).toBeFalse();
expect(readTarball(tarballPath).entries).toMatchObject([packageJsonEntry]);
});

test.concurrent("--destination from a subdirectory of the package", async () => {
using dir = tempDir("pack-cwd-destination-subdir", {
"package.json": JSON.stringify({ name: "pack-cwd-subdir", version: "1.0.0" }),
"sub": {},
});
const subDir = join(String(dir), "sub");

const { out } = await pack(subDir, bunEnv, "--quiet", "--destination=./out");

const tarballPath = join(subDir, "out", "pack-cwd-subdir-1.0.0.tgz");
expect(out).toBe(`${tarballPath}\n`);
expect(await exists(join(String(dir), "out"))).toBeFalse();
expect(readTarball(tarballPath).entries).toMatchObject([packageJsonEntry]);
});

test.concurrent("--filename from a subdirectory of the package", async () => {
using dir = tempDir("pack-cwd-filename-subdir", {
"package.json": JSON.stringify({ name: "pack-cwd-subdir", version: "1.0.0" }),
// --filename does not create directories, so both candidate locations exist up front
"sub/out": {},
"out": {},
});
const subDir = join(String(dir), "sub");

const { out } = await pack(subDir, bunEnv, "--quiet", "--filename=out/pkg.tgz");

expect(out).toBe(`${join(subDir, "out", "pkg.tgz")}\n`);
expect(await exists(join(String(dir), "out", "pkg.tgz"))).toBeFalse();
expect(readTarball(join(subDir, "out", "pkg.tgz")).entries).toMatchObject([packageJsonEntry]);
});

test.concurrent("--dry-run prints the path the tarball would be written to", async () => {
using dir = tempDir("pack-cwd-dry-run", {
"package.json": JSON.stringify({ name: "pack-cwd-dry-run", version: "1.0.0" }),
"sub": {},
});
const subDir = join(String(dir), "sub");

const [{ out: destinationOut }, { out: filenameOut }] = await Promise.all([
pack(subDir, bunEnv, "--quiet", "--dry-run", "--destination=./out"),
pack(subDir, bunEnv, "--quiet", "--dry-run", "--filename=./pkg.tgz"),
]);

expect(destinationOut).toBe(`${join(subDir, "out", "pack-cwd-dry-run-1.0.0.tgz")}\n`);
expect(filenameOut).toBe(`${join(subDir, "pkg.tgz")}\n`);
expect(await exists(join(subDir, "out"))).toBeFalse();
expect(await exists(join(String(dir), "out"))).toBeFalse();
expect(await exists(join(subDir, "pkg.tgz"))).toBeFalse();
expect(await exists(join(String(dir), "pkg.tgz"))).toBeFalse();
});

test.concurrent("absolute paths are used as given", async () => {
using dir = tempDir("pack-cwd-absolute", {
"package.json": JSON.stringify({ name: "pack-cwd-root", version: "0.0.0", workspaces: ["packages/*"] }),
"packages/pkg/package.json": JSON.stringify({ name: "pack-cwd-pkg", version: "1.2.3" }),
"elsewhere": {},
});
const pkgDir = join(String(dir), "packages", "pkg");
const elsewhere = join(String(dir), "elsewhere");

const [{ out: filenameOut }, { out: destinationOut }] = await Promise.all([
pack(pkgDir, bunEnv, "--quiet", `--filename=${join(elsewhere, "named.tgz")}`),
pack(pkgDir, bunEnv, "--quiet", `--destination=${join(elsewhere, "out")}`),
]);

expect(filenameOut).toBe(`${join(elsewhere, "named.tgz")}\n`);
expect(destinationOut).toBe(`${join(elsewhere, "out", "pack-cwd-pkg-1.2.3.tgz")}\n`);
expect(readTarball(join(elsewhere, "named.tgz")).entries).toMatchObject([packageJsonEntry]);
expect(readTarball(join(elsewhere, "out", "pack-cwd-pkg-1.2.3.tgz")).entries).toMatchObject([packageJsonEntry]);
});

test.concurrent("the default location is still the packed package's directory", async () => {
using dir = tempDir("pack-cwd-default", {
"package.json": JSON.stringify({ name: "pack-cwd-root", version: "0.0.0", workspaces: ["packages/*"] }),
"packages/pkg/package.json": JSON.stringify({ name: "pack-cwd-pkg", version: "1.2.3" }),
"packages/pkg/src": {},
});
const pkgDir = join(String(dir), "packages", "pkg");

const { out } = await pack(join(pkgDir, "src"), bunEnv, "--quiet");

expect(out).toBe("pack-cwd-pkg-1.2.3.tgz\n");
expect(await exists(join(pkgDir, "src", "pack-cwd-pkg-1.2.3.tgz"))).toBeFalse();
expect(await exists(join(String(dir), "pack-cwd-pkg-1.2.3.tgz"))).toBeFalse();
expect(readTarball(join(pkgDir, "pack-cwd-pkg-1.2.3.tgz")).entries).toMatchObject([packageJsonEntry]);
});

// PATH_MAX is 4096 on Linux and 1024 on macOS. Windows' path buffer is larger than any command line.
describe.skipIf(isWindows)("--filename longer than PATH_MAX", () => {
const longName = Buffer.alloc(5000, "f").toString();
const packageJson = JSON.stringify({ name: "pack-cwd-long-filename", version: "1.0.0" });

test.concurrent("is reported as an error", async () => {
using dir = tempDir("pack-cwd-long-filename", { "package.json": packageJson });

const { err } = await packExpectError(String(dir), bunEnv, `--filename=${longName}`);

expect(err).toContain(`error: archive filename too long: "${longName}"\n`);
});

test.concurrent("packs when the resolved path fits", async () => {
using dir = tempDir("pack-cwd-long-filename-normalized", { "package.json": packageJson });

// `<5000 bytes>/../pkg.tgz` resolves to `<cwd>/pkg.tgz`.
const { out } = await pack(String(dir), bunEnv, "--quiet", `--filename=${longName}/../pkg.tgz`);

expect(out).toBe(`${join(String(dir), "pkg.tgz")}\n`);
expect(readTarball(join(String(dir), "pkg.tgz")).entries).toMatchObject([packageJsonEntry]);
});
});
});
});

test("shasum and integrity are consistent", async () => {
Expand Down
Loading