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
1 change: 1 addition & 0 deletions src/install/PackageInstaller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1414,6 +1414,7 @@ impl<'a> PackageInstaller<'a> {
pkg_name.slice(string_buf!()),
resolution.npm().version,
patch_contents_hash,
resolution.npm().url.slice(string_buf!()),
);
installer.cache_dir = package_manager::get_cache_directory(self.manager_mut());
}
Expand Down
70 changes: 56 additions & 14 deletions src/install/PackageManager/PackageManagerDirectories.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ use std::io::Write as _;
use bun_alloc::AllocError;

use crate::Error;
use crate::Npm;
use crate::bun_fs::FileSystem;
use crate::lockfile_real::bun_lock::url_is_under_registry;
use crate::lockfile_real::package::PackageColumns;
use crate::repository::Repository;
use bun_core::ZStr;
Expand All @@ -17,6 +19,7 @@ use bun_semver::{self as Semver, String as SemverString};
#[cfg(windows)]
use bun_sys::FdDirExt;
use bun_sys::{self as sys, Dir, Fd, File};
use bun_url::URL;

use crate::bun_progress::Node as ProgressNode;

Expand Down Expand Up @@ -610,17 +613,48 @@ pub fn cached_github_folder_name_print_auto(
ZStr::EMPTY
}

/// A folder name shows the host only when it is a plain name. The hash after it is the identity.
fn folder_name_host(hostname: &[u8]) -> &[u8] {
let shown = &hostname[..hostname.len().min(32)];
if shown
.iter()
.all(|b| b.is_ascii_alphanumeric() || matches!(b, b'.' | b'-'))
{
shown
} else {
b""
}
}

/// `<registry>/<name>/-/…`, the layout `ExtractTarball::build_url` writes. A
/// registry nested under `registry` has more path before `<name>` and does not match.
Comment thread
robobun marked this conversation as resolved.
fn is_package_tarball_on_registry(url: &[u8], registry: &[u8], name: &[u8]) -> bool {
url.strip_prefix(bun_core::strings::without_trailing_slash(registry))
.and_then(|rest| rest.strip_prefix(b"/"))
.and_then(|rest| rest.strip_prefix(name))
.is_some_and(|rest| rest.starts_with(b"/-/"))
}

/// `<name>@<version>@@@<ver>` on the default registry, `@@<host>__<hash of the
/// registry URL>@@@<ver>` on any other configured registry, and `@@<host>__<hash
/// of tarball_url>@@@<ver>` for a tarball that is on neither (a lockfile URL).
Comment thread
robobun marked this conversation as resolved.
// TODO: normalize to alphanumeric
pub fn cached_npm_package_folder_name_print<'a>(
this: &PackageManager,
buf: &'a mut [u8],
name: &[u8],
version: Semver::Version,
patch_hash: Option<u64>,
tarball_url: &[u8],
) -> &'a ZStr {
let scope = this.scope_for_package_name(name);

if scope.name.is_empty() && !this.options.did_override_default_scope {
// bun.lock stores a registry.npmjs.org URL as "" and rebuilds it under the
// configured registry, so both spellings have to name the registry's slot.
Comment thread
robobun marked this conversation as resolved.
let from_registry = tarball_url.is_empty()
|| url_is_under_registry(tarball_url, Npm::Registry::DEFAULT_URL.as_bytes())
|| is_package_tarball_on_registry(tarball_url, scope.url.href(), name);
if from_registry && scope.name.is_empty() && !this.options.did_override_default_scope {
let include_version_number = true;
return cached_npm_package_folder_print_basename(
buf,
Expand All @@ -639,21 +673,22 @@ pub fn cached_npm_package_folder_name_print<'a>(
// reshaped for borrowck — resume the cursor at the basename's
// tail instead of holding the returned `&ZStr` across the re-borrow.
let scope_url = scope.url.url();
let (hostname, hash) = if from_registry {
(scope_url.hostname, scope.url_hash)
} else {
(
URL::parse(tarball_url).hostname,
Semver::semver_string::Builder::string_hash(tarball_url),
)
};
let mut w = ByteCursor {
buf,
at: spanned_len,
};
let available = w.buf.len() - spanned_len;
if scope_url.hostname.len() > 32 || available < 64 {
let visible_hostname = &scope_url.hostname[..scope_url.hostname.len().min(12)];
w.put(b"@@");
w.put(visible_hostname);
w.put(b"__");
w.put_u64_hex16::<true>(Semver::semver_string::Builder::string_hash(scope_url.href));
} else {
w.put(b"@@");
w.put(scope_url.hostname);
}
w.put(b"@@");
w.put(folder_name_host(hostname));
w.put(b"__");
w.put_u64_hex16::<true>(hash);
Comment thread
robobun marked this conversation as resolved.
w.put_cache_version(Some(CacheVersion::CURRENT));
w.put_patch_hash(patch_hash);
w.finish_z()
Expand Down Expand Up @@ -682,13 +717,15 @@ pub fn cached_npm_package_folder_name(
name: &[u8],
version: Semver::Version,
patch_hash: Option<u64>,
tarball_url: &[u8],
) -> &'static ZStr {
cached_npm_package_folder_name_print(
this,
cached_package_folder_name_buf(),
name,
version,
patch_hash,
tarball_url,
)
}

Expand Down Expand Up @@ -860,6 +897,7 @@ pub fn path_for_cached_npm_path<'a>(
buf: &'a mut PathBuffer,
package_name: &[u8],
version: Semver::Version,
tarball_url: &[u8],
) -> Result<&'a mut [u8], Error> {
let mut cache_path_buf = bun_paths::path_buffer_pool::get();

Expand All @@ -869,6 +907,7 @@ pub fn path_for_cached_npm_path<'a>(
package_name,
version,
None,
tarball_url,
);
let cache_path_len = cache_path.as_bytes().len();
// reshaped for borrowck — drop borrow before mutating buffer
Expand Down Expand Up @@ -928,8 +967,9 @@ pub fn path_for_resolution<'a>(
// mutably (for `get_cache_directory`), so the `&this.lockfile`
// borrow can't be held across it. Copy the name out first.
let package_name = this.lockfile.str(&package_name_).to_vec();
let tarball_url = this.lockfile.str(&npm.url).to_vec();

path_for_cached_npm_path(this, buf, &package_name, npm.version)
path_for_cached_npm_path(this, buf, &package_name, npm.version, &tarball_url)
}
_ => Ok(&mut buf.0[..0]),
}
Expand Down Expand Up @@ -958,7 +998,9 @@ pub fn compute_cache_dir_and_subpath<'a>(
match resolution.tag {
ResolutionTag::Npm => {
let version = resolution.npm().version;
cache_dir_subpath = cached_npm_package_folder_name(manager, name, version, patch_hash);
let tarball_url = manager.lockfile.str(&resolution.npm().url);
cache_dir_subpath =
cached_npm_package_folder_name(manager, name, version, patch_hash, tarball_url);
cache_dir = get_cache_directory(manager);
}
ResolutionTag::Git => {
Expand Down
1 change: 1 addition & 0 deletions src/install/PackageManager/PackageManagerLifecycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ impl PackageManager {
name,
pkg.resolution.npm().version,
patch_hash,
self.lockfile.str(&pkg.resolution.npm().url),
)
}
ResolutionTag::LocalTarball => directories::cached_tarball_folder_name(
Expand Down
1 change: 1 addition & 0 deletions src/install/PackageManager/PackageManagerResolution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ impl PackageManager {
&mut buf,
package_name,
installed_version,
b"",
Comment thread
coderabbitai[bot] marked this conversation as resolved.
) {
Ok(p) => p,
Err(err) => {
Expand Down
1 change: 1 addition & 0 deletions src/install/extract_tarball.rs
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,7 @@ impl ExtractTarball {
name,
self.resolution.npm().version,
None,
self.url.slice(),
)
.as_bytes()
}
Expand Down
1 change: 1 addition & 0 deletions src/install/isolated_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2339,6 +2339,7 @@ pub(crate) fn install_isolated_packages(
pkg_name.slice(string_buf),
pkg_res.npm().version,
None,
pkg_res.npm().url.slice(string_buf),
),
ResolutionTag::Git => package_manager::cached_git_folder_name(
installer.manager(),
Expand Down
1 change: 1 addition & 0 deletions src/install/isolated_install/Installer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1101,6 +1101,7 @@ impl Task {
pkg_name.slice(string_buf),
pkg_res.npm().version,
patch_info.contents_hash(),
pkg_res.npm().url.slice(string_buf),
),
ResolutionTag::Git => directories::cached_git_folder_name(
manager,
Expand Down
28 changes: 21 additions & 7 deletions test/cli/install/bun-install-registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,10 @@ describe("auto-install", () => {
expect(err).not.toContain("error:");
expect(await exited).toBe(0);

expect(resolve(await readlink(join(packageDir, ".bun-cache", "is-number", "2.0.0@@localhost@@@1")))).toBe(
join(packageDir, ".bun-cache", "is-number@2.0.0@@localhost@@@1"),
);
const cacheFolder = registry.cacheFolderName(join(packageDir, ".bun-cache"), "is-number", "2.0.0");
expect(
resolve(await readlink(join(packageDir, ".bun-cache", "is-number", cacheFolder.slice("is-number@".length)))),
).toBe(join(packageDir, ".bun-cache", cacheFolder));
});
});

Expand Down Expand Up @@ -4071,7 +4072,9 @@ test("it should invalid cached package if package.json is missing", async () =>
expect(
await Promise.all([
readdirSorted(join(packageDir, "node_modules", "no-deps")),
readdirSorted(join(packageDir, ".bun-cache", "no-deps@2.0.0@@localhost@@@1")),
readdirSorted(
join(packageDir, ".bun-cache", registry.cacheFolderName(join(packageDir, ".bun-cache"), "no-deps", "2.0.0")),
),
]),
).toEqual([
["index.js", "package.json"],
Expand All @@ -4088,13 +4091,22 @@ test("it should invalid cached package if package.json is missing", async () =>
expect(out).not.toContain("+ no-deps@2.0.0");

// with cache package.json deleted, install is a no-op and cache is untouched
await rm(join(packageDir, ".bun-cache", "no-deps@2.0.0@@localhost@@@1", "package.json"));
await rm(
join(
packageDir,
".bun-cache",
registry.cacheFolderName(join(packageDir, ".bun-cache"), "no-deps", "2.0.0"),
"package.json",
),
);
({ out } = await runBunInstall(env, packageDir, { savesLockfile: false }));
expect(out).not.toContain("+ no-deps@2.0.0");
expect(
await Promise.all([
readdirSorted(join(packageDir, "node_modules", "no-deps")),
readdirSorted(join(packageDir, ".bun-cache", "no-deps@2.0.0@@localhost@@@1")),
readdirSorted(
join(packageDir, ".bun-cache", registry.cacheFolderName(join(packageDir, ".bun-cache"), "no-deps", "2.0.0")),
),
]),
).toEqual([["index.js", "package.json"], ["index.js"]]);

Expand All @@ -4106,7 +4118,9 @@ test("it should invalid cached package if package.json is missing", async () =>
expect(
await Promise.all([
readdirSorted(join(packageDir, "node_modules", "no-deps")),
readdirSorted(join(packageDir, ".bun-cache", "no-deps@2.0.0@@localhost@@@1")),
readdirSorted(
join(packageDir, ".bun-cache", registry.cacheFolderName(join(packageDir, ".bun-cache"), "no-deps", "2.0.0")),
),
]),
).toEqual([
["index.js", "package.json"],
Expand Down
Loading
Loading