Skip to content

pack: pack a file listed under several bin names once, strip the trailing slash from directories.bin #38720

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 7 commits into
base: main
Choose a base branch
from
+266 −80
Open

pack: pack a file listed under several bin names once, strip the trailing slash from directories.bin #38720

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3bb187a
pack: pack a file listed under several bin names once; strip the trai…
robobun Aug 14, 2026
477e158
pack: name the bin dedupe and package-root conditions instead of comm…
robobun Aug 14, 2026
4d8ba0c
pack: resolve every bin value through one bin_subpath helper; share i…
robobun Aug 15, 2026
7c41d5b
pack: shorten the bin_subpath doc comment
robobun Aug 15, 2026
766710c
pack: drop the bin_subpath doc comment
robobun Aug 15, 2026
d19c2a8
publish: resolve string and object bin values through one bin_target;…
robobun Aug 15, 2026
10b42b0
pack/publish: share the package-root-or-outside check between bin_sub…
robobun Aug 15, 2026
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
54 changes: 32 additions & 22 deletions src/runtime/cli/pack_command.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1446,7 +1446,7 @@ fn get_bundled_deps(
// ───────────────────────────────────────────────────────────────────────────

#[derive(Clone, Copy, PartialEq, Eq)]
enum BinType {
pub(crate) enum BinType {
File,
Dir,
}
Expand All @@ -1463,13 +1463,9 @@ fn get_package_bins(json: &Expr) -> Result<Vec<BinInfo>, AllocError> {

if let Some(bin) = json.as_property(b"bin") {
if let Some(bin_str) = bin.expr.as_string(pack_bump()) {
let normalized = resolve_path::normalize_buf::<resolve_path::platform::Posix>(
bin_str,
&mut path_buf,
);
if !bin_path_escapes_root(normalized) {
if let Some(subpath) = bin_subpath(bin_str, BinType::File, &mut path_buf) {
bins.push(BinInfo {
path: ZBox::from_bytes(normalized),
path: ZBox::from_bytes(subpath),
ty: BinType::File,
});
}
Expand All @@ -1484,13 +1480,16 @@ fn get_package_bins(json: &Expr) -> Result<Vec<BinInfo>, AllocError> {
for bin_prop in bin_obj.properties.slice() {
if let Some(bin_prop_value) = &bin_prop.value {
if let Some(bin_str) = bin_prop_value.as_string(pack_bump()) {
let normalized = resolve_path::normalize_buf::<resolve_path::platform::Posix>(
bin_str,
&mut path_buf,
);
if !bin_path_escapes_root(normalized) {
let Some(subpath) = bin_subpath(bin_str, BinType::File, &mut path_buf)
else {
continue;
};
let already_listed = bins.iter().any(|existing| {
strings::eql_long(existing.path.as_bytes(), subpath, true)
});
if !already_listed {
bins.push(BinInfo {
path: ZBox::from_bytes(normalized),
path: ZBox::from_bytes(subpath),
ty: BinType::File,
});
}
Expand All @@ -1506,13 +1505,9 @@ fn get_package_bins(json: &Expr) -> Result<Vec<BinInfo>, AllocError> {
if let ExprData::EObject(directories_obj) = &directories.expr.data {
if let Some(bin) = directories_obj.as_property(b"bin") {
if let Some(bin_str) = bin.expr.as_string(pack_bump()) {
let normalized = resolve_path::normalize_buf::<resolve_path::platform::Posix>(
bin_str,
&mut path_buf,
);
if !bin_path_escapes_root(normalized) {
if let Some(subpath) = bin_subpath(bin_str, BinType::Dir, &mut path_buf) {
bins.push(BinInfo {
path: ZBox::from_bytes(normalized),
path: ZBox::from_bytes(subpath),
ty: BinType::Dir,
});
}
Expand All @@ -1524,6 +1519,22 @@ fn get_package_bins(json: &Expr) -> Result<Vec<BinInfo>, AllocError> {
Ok(bins)
}

pub(crate) fn bin_subpath<'a>(value: &[u8], ty: BinType, buf: &'a mut [u8]) -> Option<&'a [u8]> {
let normalized: &'a [u8] =
resolve_path::normalize_buf::<resolve_path::platform::Posix>(value, buf);
let subpath = match ty {
BinType::Dir => strings::without_trailing_slash(normalized),
BinType::File if normalized.ends_with(b"/") || normalized == b"package.json" => {
return None;
}
BinType::File => normalized,
};
if subpath.is_empty() || subpath == b"." || bin_path_escapes_root(subpath) {
return None;
}
Some(subpath)
}

fn bin_path_escapes_root(p: &[u8]) -> bool {
path::is_absolute_loose(p) || p == b".." || p.starts_with(b"../")
}
Expand All @@ -1537,9 +1548,8 @@ fn is_package_bin(bins: &[BinInfo], maybe_bin_path: &[u8]) -> bool {
}
}
BinType::Dir => {
let bin_without_trailing = strings::without_trailing_slash(bin.path.as_bytes());
if maybe_bin_path.starts_with(bin_without_trailing) {
let remain = &maybe_bin_path[bin_without_trailing.len()..];
if maybe_bin_path.starts_with(bin.path.as_bytes()) {
let remain = &maybe_bin_path[bin.path.as_bytes().len()..];
if remain.len() > 1
&& remain[0] == b'/'
&& strings::index_of_char(&remain[1..], b'/').is_none()
Expand Down
95 changes: 39 additions & 56 deletions src/runtime/cli/publish_command.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1610,6 +1610,13 @@ impl PublishCommand {
None
}

/// `None` when a `bin` value resolves to the package root (`""`, `"."`), which npm drops too.
fn bin_target<'a>(value: &[u8], path_buf: &'a mut [u8]) -> Option<&'a ZStr> {
let target: &'a ZStr = normalize_buf_z::<path::platform::Posix>(value, path_buf);
let is_package_root = target.is_empty() || target.as_bytes() == b".";
(!is_package_root).then_some(target)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

fn normalize_bin(
json: &mut Expr,
bump: &bun_alloc::Arena,
Expand All @@ -1629,31 +1636,26 @@ impl PublishCommand {
match &bin_query.expr.data {
ExprData::EString(bin_str) => {
let mut bin_props: Vec<G::Property> = Vec::new();
let normalized = strings::without_prefix_comptime_z(
normalize_buf_z::<path::platform::Posix>(
bin_str.string(bump)?,
&mut *path_buf,
),
b"./",
);
if !bun_sys::exists_at(workspace_root, normalized) {
bun_core::warn!(
"bin '{}' does not exist",
bstr::BStr::new(normalized.as_bytes()),
);
}
if let Some(value) = Self::bin_target(bin_str.string(bump)?, &mut *path_buf) {
if !bun_sys::exists_at(workspace_root, value) {
bun_core::warn!(
"bin '{}' does not exist",
bstr::BStr::new(value.as_bytes()),
);
}

bin_props.push(G::Property {
key: Some(Expr::init(
E::String::init(leak!(package_name)),
bun_ast::Loc::EMPTY,
)),
value: Some(Expr::init(
E::String::init(leak!(normalized.as_bytes())),
bun_ast::Loc::EMPTY,
)),
..Default::default()
});
bin_props.push(G::Property {
key: Some(Expr::init(
E::String::init(leak!(package_name)),
bun_ast::Loc::EMPTY,
)),
value: Some(Expr::init(
E::String::init(leak!(value.as_bytes())),
bun_ast::Loc::EMPTY,
)),
..Default::default()
});
}

json.data
.e_object_mut()
Expand Down Expand Up @@ -1695,32 +1697,17 @@ impl PublishCommand {
continue;
}

let value: Option<bun_core::ZBox> = 'value: {
if let Some(value) = &bin_prop.value {
if let Some(vs) = value.data.as_e_string() {
if vs.len() != 0 {
break 'value Some(bun_core::ZBox::from_bytes(
strings::without_prefix_comptime_z(
// replace separators
normalize_buf_z::<path::platform::Posix>(
vs.string(bump)?,
&mut *path_buf,
),
b"./",
)
.as_bytes(),
));
}
}
}
None
let Some(value) =
bin_prop.value.as_ref().and_then(|v| v.data.as_e_string())
else {
continue;
};
let Some(value) = value else { continue };
if value.is_empty() {
let Some(value) = Self::bin_target(value.string(bump)?, &mut *path_buf)
else {
continue;
}
};

if !bun_sys::exists_at(workspace_root, &value) {
if !bun_sys::exists_at(workspace_root, value) {
bun_core::warn!(
"bin '{}' does not exist",
bstr::BStr::new(value.as_bytes()),
Expand Down Expand Up @@ -1761,16 +1748,12 @@ impl PublishCommand {
return Ok(());
};
let mut bin_props: Vec<G::Property> = Vec::new();
let normalized_bin_dir = bun_core::ZBox::from_bytes(
strings::without_trailing_slash(strings::without_prefix(
normalize_buf::<path::platform::Posix>(bin_dir_str, &mut *path_buf),
b"./",
)),
);

if normalized_bin_dir.is_empty() {
let Some(bin_dir_subpath) =
pack::bin_subpath(bin_dir_str, pack::BinType::Dir, &mut *path_buf)
else {
return Ok(());
}
};
let normalized_bin_dir = bun_core::ZBox::from_bytes(bin_dir_subpath);
Comment thread
claude[bot] marked this conversation as resolved.

let bin_dir = match bun_sys::openat(
workspace_root,
Expand Down
146 changes: 146 additions & 0 deletions test/cli/install/bun-pack.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1647,6 +1647,152 @@ describe("bins", () => {
},
]);
});

test("the same file under several bin names is packed once", async () => {
await Promise.all([
write(
join(packageDir, "package.json"),
JSON.stringify({
name: "pack-bins-same-file",
version: "1.0.0",
bin: {
"one": "cli.js",
"two": "cli.js",
"three": "./cli.js",
},
}),
),
write(join(packageDir, "cli.js"), "console.log('hello')"),
]);

const { out } = await pack(packageDir, bunEnv);

const tarball = readTarball(join(packageDir, "pack-bins-same-file-1.0.0.tgz"));
expect(tarball.entries).toMatchObject([{ pathname: "package/package.json" }, { pathname: "package/cli.js" }]);
expect(tarball.entries[1].perm & 0o111).toBe(0o111);
expect(out).toContain("Total files: 2");
});

// None of these name a file that can be packed as a bin: the package root, a
// file spelled as a directory, a directory, and the root package.json (which
// is always in the tarball). The package packs as if "bin" were absent.
test.each(["", ".", "cli.js/", "lib/", "package.json"])('"bin" of %p is ignored', async bin => {
await Promise.all([
write(
join(packageDir, "package.json"),
JSON.stringify({
name: "pack-bins-not-a-file",
version: "1.0.0",
bin,
}),
),
write(join(packageDir, "cli.js"), "console.log('cli')"),
write(join(packageDir, "lib", "a.js"), "console.log('a')"),
]);

await pack(packageDir, bunEnv);

const tarball = readTarball(join(packageDir, "pack-bins-not-a-file-1.0.0.tgz"));
expect(
tarball.entries.map(entry => ({ pathname: entry.pathname, executable: (entry.perm & 0o111) !== 0 })),
).toEqual([
{ pathname: "package/package.json", executable: false },
{ pathname: "package/cli.js", executable: false },
{ pathname: "package/lib/a.js", executable: false },
]);
});

test("ignored entries of a bin object do not affect the others", async () => {
await Promise.all([
write(
join(packageDir, "package.json"),
JSON.stringify({
name: "pack-bins-partly-ignored",
version: "1.0.0",
bin: {
"dir": "lib/",
"cli": "cli.js",
"pkg": "package.json",
},
}),
),
write(join(packageDir, "cli.js"), "console.log('cli')"),
write(join(packageDir, "lib", "a.js"), "console.log('a')"),
]);

await pack(packageDir, bunEnv);

const tarball = readTarball(join(packageDir, "pack-bins-partly-ignored-1.0.0.tgz"));
expect(
tarball.entries.map(entry => ({ pathname: entry.pathname, executable: (entry.perm & 0o111) !== 0 })),
).toEqual([
{ pathname: "package/package.json", executable: false },
{ pathname: "package/cli.js", executable: true },
{ pathname: "package/lib/a.js", executable: false },
]);
});

// The bin directory is packed by its own walk, and the walk over the rest of
// the package has to skip it. Each `files` value below routes that skip
// through a different walk.
describe('"directories.bin" with a trailing slash', () => {
test.each([
[undefined, ["package/index.js", "package/lib/bins/bin.js", "package/lib/index.js"]],
[["index.js"], ["package/index.js", "package/lib/bins/bin.js"]],
[["lib"], ["package/lib/bins/bin.js", "package/lib/index.js"]],
[["lib/bins"], ["package/lib/bins/bin.js"]],
])("files: %p", async (files, expected) => {
await Promise.all([
write(
join(packageDir, "package.json"),
JSON.stringify({
name: "pack-bins-dir-trailing-slash",
version: "1.0.0",
files,
directories: {
bin: "./lib/bins/",
},
}),
),
write(join(packageDir, "index.js"), "console.log('index')"),
write(join(packageDir, "lib", "index.js"), "console.log('lib')"),
write(join(packageDir, "lib", "bins", "bin.js"), "console.log('bin')"),
]);

await pack(packageDir, bunEnv);

const tarball = readTarball(join(packageDir, "pack-bins-dir-trailing-slash-1.0.0.tgz"));
expect(tarball.entries.map(entry => entry.pathname)).toEqual(["package/package.json", ...expected]);
const bin = tarball.entries.find(entry => entry.pathname === "package/lib/bins/bin.js");
expect(bin.perm & 0o111).toBe(0o111);
});
});

test.each(["", ".", "./"])('"directories.bin" of %p (the package root) is ignored', async bin => {
await Promise.all([
write(
join(packageDir, "package.json"),
JSON.stringify({
name: "pack-bins-dir-root",
version: "1.0.0",
directories: {
bin,
},
}),
),
write(join(packageDir, "index.js"), "console.log('index')"),
write(join(packageDir, "lib", "a.js"), "console.log('a')"),
]);

await pack(packageDir, bunEnv);

const tarball = readTarball(join(packageDir, "pack-bins-dir-root-1.0.0.tgz"));
expect(tarball.entries).toMatchObject([
{ pathname: "package/package.json" },
{ pathname: "package/index.js" },
{ pathname: "package/lib/a.js" },
]);
});
});

test("unicode", async () => {
Expand Down
Loading
Loading