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
35 changes: 34 additions & 1 deletion src/install/PackageInstaller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ use bun_core::fmt::PathSep;
use bun_core::{Global, Output};
use bun_core::{ZStr, strings};
use bun_paths::resolve_path::{dirname, join_abs_string_z, join_z_buf};
use bun_paths::{AbsPath, AutoAbsPath, MAX_PATH_BYTES, PathBuffer, SEP, platform};
use bun_paths::{
AbsPath, AutoAbsPath, AutoAbsPathChecked, MAX_PATH_BYTES, PathBuffer, SEP, platform,
};
use bun_semver::String;
use bun_sys::{self as Syscall, Dir, Fd};

Expand Down Expand Up @@ -587,11 +589,15 @@ impl<'a> PackageInstaller<'a> {
let pkg_resolutions_lists = pkgs.items_resolutions();
let pkg_resolutions_buffer = lockfile.buffers.resolutions.as_slice();
let pkg_names = pkgs.items_name();
let pkg_resolutions = pkgs.items_resolution();

let completed_trees = &self.completed_trees;
let tree = &mut self.trees[tree_id as usize];
let mut deferred: Vec<DependencyID> = Vec::new();

let mut real_folder_buf = bun_paths::path_buffer_pool::get();
let mut real_cache_dir: Option<AbsPath> = None;

while let Some(dep_id) = tree.binaries.remove_or_null() {
debug_assert!((dep_id as usize) < lockfile.buffers.dependencies.as_slice().len());
let package_id = lockfile.buffers.resolutions.as_slice()[dep_id as usize];
Expand All @@ -604,6 +610,7 @@ impl<'a> PackageInstaller<'a> {
.slice(string_buf);
let package_name_ = strings::StringOrTinyString::init(alias);
let mut target_package_name = package_name_;
let mut target_package_id = package_id;
let mut can_retry_without_native_binlink_optimization = false;
let mut target_node_modules_path_opt: Option<AbsPath> = None;
let mut defer_this_bin = false;
Expand Down Expand Up @@ -669,6 +676,7 @@ impl<'a> PackageInstaller<'a> {
pkg_names[replacement_pkg_id as usize].slice(string_buf);
target_package_name =
strings::StringOrTinyString::init(replacement_name);
target_package_id = replacement_pkg_id;
can_retry_without_native_binlink_optimization = true;
}
}
Expand Down Expand Up @@ -696,6 +704,29 @@ impl<'a> PackageInstaller<'a> {
};

loop {
let installed_from: Option<&[u8]> = {
let target_resolution = &pkg_resolutions[target_package_id as usize];
if target_resolution.tag == resolution::Tag::Folder {
// Folders with bins are root- or workspace-declared, hence root-relative.
let mut folder = AutoAbsPathChecked::init_top_level_dir();
match folder.join(&[target_resolution.folder().slice(string_buf)]) {
Ok(()) => {
Syscall::realpath(folder.slice_z(), &mut real_folder_buf).ok()
}
Err(_) => None,
}
Comment thread
claude[bot] marked this conversation as resolved.
} else if target_resolution.tag.can_enqueue_install_task() {
if real_cache_dir.is_none() {
real_cache_dir =
AbsPath::init_fd_path(manager.get_cache_directory()).ok();
}
real_cache_dir.as_ref().map(AbsPath::slice)
} else {
// Directory-symlinked packages: never chmod through their own links.
None
}
};

// `node_modules_path` (mut) and `target_node_modules_path`
// (read-only) refer to the same buffer when no replacement is
// set. Derive both from a single `*mut` so the read pointer
Expand Down Expand Up @@ -724,6 +755,7 @@ impl<'a> PackageInstaller<'a> {
abs_target_buf: link_target_buf,
abs_dest_buf: link_dest_buf,
rel_buf: link_rel_buf,
installed_from,
err: None,
skipped_due_to_missing_bin: false,
};
Expand All @@ -742,6 +774,7 @@ impl<'a> PackageInstaller<'a> {
);
}
target_package_name = package_name_;
target_package_id = package_id;
target_node_modules_path_opt = None;
continue;
}
Expand Down
74 changes: 41 additions & 33 deletions src/install/bin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,9 @@ pub struct Linker<'a> {
pub abs_dest_buf: &'a mut [u8],
pub rel_buf: &'a mut [u8],

/// Real path the target package was installed from; see `make_executable`.
pub installed_from: Option<&'a [u8]>,

pub err: Option<Error>,
pub skipped_due_to_missing_bin: bool,
}
Expand Down Expand Up @@ -953,10 +956,41 @@ impl<'a> Linker<'a> {

#[cfg(not(windows))]
{
Self::make_executable(self.installed_from, abs_target);
Self::try_normalize_shebang(abs_target);
}
}

/// Follows only the symlink backend's own link into `installed_from`, never a package's.
#[cfg(not(windows))]
fn make_executable(installed_from: Option<&[u8]>, abs_target: &ZStr) {
let mode = 0o777 & !(UMASK.load(Ordering::Acquire) as Mode);
let _ = sys::lchmod(abs_target, mode);

let Some(installed_from) = installed_from else {
return;
};
let mut link_buf = path::path_buffer_pool::get();
let Ok(link_len) = sys::readlink(abs_target, link_buf.as_mut_slice()) else {
return;
};
// `sys::readlink` writes the NUL at `link_len`.
let link_target = ZStr::from_buf(&link_buf[..], link_len);
if !path::is_absolute(link_target.as_bytes()) {
return;
}
let mut real_buf = path::path_buffer_pool::get();
let Ok(real_target) = sys::realpath(link_target, &mut *real_buf) else {
return;
};
if resolve_path::is_parent_or_equal(installed_from, real_target)
!= resolve_path::ParentEqual::Parent
{
return;
}
let _ = sys::lchmod(link_target, mode);
}

#[cfg(not(windows))]
fn try_normalize_shebang(abs_target: &ZStr) {
let mut shebang_buf = [0u8; 2048];
Expand Down Expand Up @@ -1258,10 +1292,6 @@ impl<'a> Linker<'a> {

#[cfg(not(windows))]
fn create_symlink(&mut self, abs_target: &ZStr, abs_dest: &ZStr, global: bool) {
// hoisted from `defer { if (this.err == null) chmod }` — scopeguard
// cannot capture `&mut self.err` without conflicting with the body's writes,
// so each return path calls `Self::chmod_on_ok` explicitly instead.

let abs_dest_dir = resolve_path::dirname::<PlatformAuto>(abs_dest.as_bytes());
let rel_target =
resolve_path::relative_buf_z(self.rel_buf, abs_dest_dir, abs_target.as_bytes());
Expand All @@ -1272,15 +1302,13 @@ impl<'a> Linker<'a> {
sys::Result::Err(err) => {
if err.get_errno() != sys::Errno::EEXIST && err.get_errno() != sys::Errno::ENOENT {
self.err = Some(err.into());
Self::chmod_on_ok(self.err, abs_target);
return;
}

// ENOENT means `.bin` hasn't been created yet. Should only happen if this isn't global
if err.get_errno() == sys::Errno::ENOENT {
if global {
self.err = Some(err.into());
Self::chmod_on_ok(self.err, abs_target);
return;
}

Expand All @@ -1291,44 +1319,24 @@ impl<'a> Linker<'a> {
let _ = sys::Dir::cwd().make_path(self.node_modules_path.slice());
self.node_modules_path.set_length(node_modules_path_save);

match sys::symlink_running_executable(rel_target, abs_dest) {
sys::Result::Err(real_error) => {
// It was just created, no need to delete destination and symlink again
self.err = Some(real_error.into());
Self::chmod_on_ok(self.err, abs_target);
return;
}
sys::Result::Ok(()) => {
Self::chmod_on_ok(self.err, abs_target);
return;
}
// It was just created, no need to delete destination and symlink again
if let Err(real_error) = sys::symlink_running_executable(rel_target, abs_dest) {
self.err = Some(real_error.into());
}
return;
}

// beyond this error can only be `.EXIST`
debug_assert!(err.get_errno() == sys::Errno::EEXIST);
}
sys::Result::Ok(()) => {
Self::chmod_on_ok(self.err, abs_target);
return;
}
sys::Result::Ok(()) => return,
}

// delete and try again
let _ = sys::delete_tree_absolute(abs_dest.as_bytes());
if let Err(err) = sys::symlink_running_executable(rel_target, abs_dest) {
self.err = Some(err.into());
}
Self::chmod_on_ok(self.err, abs_target);
}

#[cfg(not(windows))]
fn chmod_on_ok(err: Option<Error>, abs_target: &ZStr) {
// hoisted from `defer` block in create_symlink
if err.is_none() {
let mode = 0o777 & !(UMASK.load(Ordering::Acquire) as Mode);
let _ = sys::lchmod(abs_target, mode);
}
}

#[cfg(not(windows))]
Expand Down Expand Up @@ -1592,7 +1600,7 @@ impl<'a> Linker<'a> {
// is called while `abs_target` / `abs_dest` borrow `self.abs_target_buf`
// / `self.abs_dest_buf`. `link_bin_or_create_shim` never reads or writes
// those two buffers (it only touches `rel_buf`, `node_modules_path`, `seen`, `err`,
// `skipped_due_to_missing_bin`). Detach the `abs_dest` borrow via a raw
// `skipped_due_to_missing_bin`, `installed_from`). Detach the `abs_dest` borrow via a raw
// pointer so borrowck allows the disjoint access; the SAFETY invariant
// is that `abs_dest_buf` is not aliased mutably for the lifetime of the
// detached slice. `package_dir` (`abs_target_buf[0..package_dir_len]`)
Expand Down Expand Up @@ -1810,7 +1818,7 @@ impl<'a> Linker<'a> {
// SAFETY: result lives in `self.abs_target_buf`, which
// `link_bin_or_create_shim` does not write to (only
// `rel_buf`/`node_modules_path`/`seen`/`err`/
// `skipped_due_to_missing_bin` are touched).
// `skipped_due_to_missing_bin`/`installed_from` are touched).
ZStr::from_raw(r.as_bytes().as_ptr(), r.len())
};

Expand Down
2 changes: 2 additions & 0 deletions src/install/isolated_install/Installer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1841,6 +1841,7 @@ impl Task {
abs_target_buf: &mut *abs_target_buf,
abs_dest_buf: &mut *abs_dest_buf,
rel_buf: &mut *rel_buf,
installed_from: None,
err: None,
skipped_due_to_missing_bin: false,
};
Expand Down Expand Up @@ -2403,6 +2404,7 @@ impl<'a> Installer<'a> {
abs_target_buf: &mut *link_target_buf,
abs_dest_buf: &mut *link_dest_buf,
rel_buf: &mut *link_rel_buf,
installed_from: None,
err: None,
skipped_due_to_missing_bin: false,
};
Expand Down
1 change: 1 addition & 0 deletions src/runtime/cli/link_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ fn link(ctx: command::Context) -> crate::Result<()> {
abs_target_buf: link_target_buf.as_mut_slice(),
abs_dest_buf: link_dest_buf.as_mut_slice(),
rel_buf: link_rel_buf.as_mut_slice(),
installed_from: None,
err: None,
skipped_due_to_missing_bin: false,
};
Expand Down
1 change: 1 addition & 0 deletions src/runtime/cli/unlink_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ fn unlink(ctx: &mut ContextData) -> crate::Result<()> {
abs_target_buf: link_target_buf.as_mut_slice(),
abs_dest_buf: link_dest_buf.as_mut_slice(),
rel_buf: link_rel_buf.as_mut_slice(),
installed_from: None,
err: None,
skipped_due_to_missing_bin: false,
};
Expand Down
119 changes: 119 additions & 0 deletions test/cli/install/bun-install-native-binlink.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,125 @@ describe.concurrent("native binlink altpath", () => {
}
});

// A `file:` folder outside the project is installed as one symlink per file, so the
// executable bit has to be set on the file behind the bin target. With a
// nativeDependencies package that file belongs to whichever package the bin ends up
// linked from: the platform package (installed from the cache) when the redirect
// succeeds, the folder itself when the platform package has no bin file and the linker
// retries without the redirect.
//
// POSIX-only: the Windows bin linker writes shims and never chmods anything.
describe.concurrent.skipIf(isWindows)("symlink-installed nativeDependencies package", () => {
async function setup(local: {
name: string;
bin: Record<string, string>;
target: string;
files: Record<string, string>;
}) {
const { packageDir } = await verdaccio.createTestDir({
files: {
"app/package.json": JSON.stringify({
name: "test-app",
version: "1.0.0",
dependencies: { [local.name]: `file:../${local.name}` },
nativeDependencies: [local.name],
}),
[`${local.name}/package.json`]: JSON.stringify({
name: local.name,
version: "1.0.0",
bin: local.bin,
optionalDependencies: { [local.target]: "1.0.0" },
}),
...Object.fromEntries(
Object.entries(local.files).map(([file, contents]) => [`${local.name}/${file}`, contents]),
),
},
});
const appDir = join(packageDir, "app");
await verdaccio.writeBunfig(appDir, { linker: "hoisted" });
const env = { ...bunEnv, BUN_INSTALL_CACHE_DIR: join(appDir, ".bun-cache") };
env.BUN_TMPDIR = env.TMPDIR = env.TEMP = join(appDir, ".bun-tmp");

async function install(...args: string[]) {
await using proc = spawn({
cmd: [bunExe(), "install", ...args],
cwd: appDir,
stdout: "pipe",
stdin: "ignore",
stderr: "pipe",
env,
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect({ stdout, stderr, exitCode }).toMatchObject({ exitCode: 0 });
}

async function runBin(name: string) {
await using proc = spawn({
cmd: [join(appDir, "node_modules", ".bin", name)],
cwd: appDir,
stdout: "pipe",
stdin: "ignore",
stderr: "pipe",
env,
});
const [out, err, code] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
return { out: out.trim(), err, code };
}

return {
localDir: join(packageDir, local.name),
appDir,
binDir: join(appDir, "node_modules", ".bin"),
install,
runBin,
};
}

test("chmods the platform package's file in the cache when it is installed with --backend symlink", async () => {
const { localDir, appDir, binDir, install, runBin } = await setup({
name: "local-native-binlink",
bin: { "local-native-cmd": "bin/main.js" },
target: "test-native-binlink-target",
files: { "bin/main.js": `#!/usr/bin/env node\nconsole.log("FAIL: main package bin");\n` },
});

await install("--backend", "symlink");

// The bin is redirected into the platform package, whose files are symlinks into
// the cache; the cached `bin/main.js` is 0644 in the fixture tarball.
const cachedBin = readBinTarget(binDir, "local-native-cmd");
expect(cachedBin.startsWith(realpathSync(join(appDir, ".bun-cache")) + sep)).toBeTrue();
expect(cachedBin).toContain("test-native-binlink-target@1.0.0");
expect(cachedBin).toEndWith(join("bin", "main.js"));
expect(statSync(cachedBin).mode & 0o111).not.toBe(0);
// The folder's own bin was not linked, so it is left alone.
expect(statSync(join(localDir, "bin", "main.js")).mode & 0o111).toBe(0);

expect(await runBin("local-native-cmd")).toEqual({
out: "SUCCESS: Using platform-specific bin (test-native-binlink-target)",
err: "",
code: 0,
});
});

test("chmods the folder's own bin when the platform package has no bin file", async () => {
const { localDir, binDir, install, runBin } = await setup({
name: "local-native-fallback",
bin: { "local-fallback-cmd": "cli.js" },
target: "test-native-binlink-fallback-target",
files: { "cli.js": `#!/usr/bin/env node\nconsole.log("SUCCESS: Using main package bin");\n` },
});
const folderBin = join(localDir, "cli.js");
expect(statSync(folderBin).mode & 0o111).toBe(0);

await install();

expect(readBinTarget(binDir, "local-fallback-cmd")).toBe(realpathSync(folderBin));
expect(statSync(folderBin).mode & 0o111).not.toBe(0);
expect(await runBin("local-fallback-cmd")).toEqual({ out: "SUCCESS: Using main package bin", err: "", code: 0 });
});
});

// The bin linker must not create a `node_modules/.bin` entry (nor chmod or rewrite the
// target) when a package's bin path resolves through an in-package symlink to a location
// outside the package directory. Bins that resolve inside the package must still link,
Expand Down
Loading
Loading