Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
87 changes: 86 additions & 1 deletion src/install/lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use bun_collections::{
HashMap as BunHashMap, IdentityContext, LinearFifo, linear_fifo::DynamicBuffer,
};
use bun_core::fmt::PathSep;
use bun_core::{Global, Output};
use bun_core::{Global, Output, UnwrapOrOom as _};
use bun_paths::{MAX_PATH_BYTES, PathBuffer, SEP, SEP_STR, platform, resolve_path};
// `bun_install` sits above `bun_resolver` in the crate graph (no cycle), so use
// the real resolver `FileSystem` directly — same as `PackageManager.rs`.
Expand Down Expand Up @@ -498,6 +498,21 @@ impl Lockfile {
}

pub fn load_from_dir<'a, const ATTEMPT_LOADING_FROM_OTHER_LOCKFILE: bool>(
&'a mut self,
dir: Fd,
manager: Option<&mut PackageManager>,
log: &mut bun_ast::Log,
) -> LoadResult<'a> {
let mut result =
self.read_from_dir::<ATTEMPT_LOADING_FROM_OTHER_LOCKFILE>(dir, manager, log);
if let LoadResult::Ok(ok) = &mut result {
ok.lockfile.rebase_folder_dependencies().unwrap_or_oom();
}
result
}

/// bun.lock, bun.lockb or, failing both, a foreign lockfile to migrate from.
fn read_from_dir<'a, const ATTEMPT_LOADING_FROM_OTHER_LOCKFILE: bool>(
&'a mut self,
dir: Fd,
mut manager: Option<&mut PackageManager>,
Expand Down Expand Up @@ -712,6 +727,76 @@ impl Lockfile {
})
}

/// A lockfile stores a folder dependency as its literal, relative to the
/// declaring package; give the rows of root, workspace and `file:` packages
/// the top-level relative `value.folder` that `Package::parse` gives them.
/// Idempotent, since the literal is left as is.
Comment thread
robobun marked this conversation as resolved.
pub(crate) fn rebase_folder_dependencies(&mut self) -> Result<(), AllocError> {
let Lockfile {
packages,
buffers,
string_pool,
..
} = self;
let Buffers {
string_bytes,
dependencies,
..
} = buffers;
let pkgs = packages.slice();
let mut path_buf = bun_paths::path_buffer_pool::get();

for (pkg_resolution, pkg_dependencies) in pkgs
.items_resolution()
.iter()
.zip(pkgs.items_dependencies())
{
let pkg_dir: SemverString = match pkg_resolution.tag {
ResolutionTag::Root => SemverString::default(),
ResolutionTag::Workspace => *pkg_resolution.workspace(),
ResolutionTag::Folder => *pkg_resolution.folder(),
_ => continue,
};

for dep in pkg_dependencies.mut_(dependencies.as_mut_slice()) {
if dep.version.tag != dependency::Tag::Folder {
continue;
}
let relative = {
let buf = string_bytes.as_slice();
let literal = dep.version.literal.sliced(buf);
let Some(declared) = dependency::parse_with_tag(
dep.name,
Some(dep.name_hash),
literal.slice,
dependency::Tag::Folder,
&literal,
None,
None,
) else {
continue;
};
let declared_folder = *declared.folder();
package::folder_relative_to_top_level_dir(
pkg_dir.slice(buf),
declared_folder.slice(buf),
&mut path_buf[..],
)
};
let Some(relative) = relative else {
continue;
};
dep.version.value.folder = SemverStringBuf {
bytes: &mut *string_bytes,
pool: &mut *string_pool,
}
.append(relative)?;
}
}

Ok(())
}

pub(crate) fn is_resolved_dependency_disabled(
&self,
dep_id: DependencyID,
Expand Down
45 changes: 28 additions & 17 deletions src/install/lockfile/Package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,30 @@ pub(crate) fn value_loc_of(source: &bun_ast::Source, key_loc: bun_ast::Loc) -> b
crate::bun_json::property_value_loc(&source.contents, key_loc).unwrap_or(key_loc)
}

/// `folder` as declared by the package.json in `pkg_dir`, made relative to the
/// top-level dir: the form `Version.value.folder` has in root, workspace and
/// `file:` packages. `None` if it does not fit in `buf`.
Comment thread
robobun marked this conversation as resolved.
Outdated
pub(crate) fn folder_relative_to_top_level_dir<'a>(
pkg_dir: &[u8],
folder: &[u8],
buf: &'a mut [u8],
) -> Option<&'a [u8]> {
let top_level_dir = FileSystem::instance().top_level_dir();
let joined = resolve_path::join_abs_string_buf_checked::<path::platform::Auto>(
top_level_dir,
buf,
&[pkg_dir, folder],
)?;
let relative = resolve_path::relative(top_level_dir, joined);
// empty when the package depends on itself
let relative: &[u8] = if relative.is_empty() { b"." } else { relative };
let len = relative.len();
buf[..len].copy_from_slice(relative);
#[cfg(windows)]
path::dangerously_convert_path_to_posix_in_place::<u8>(&mut buf[..len]);
Some(&buf[..len])
}

#[cold]
fn invalid_trusted_dependencies(
log: &mut bun_ast::Log,
Expand Down Expand Up @@ -1846,10 +1870,10 @@ impl Package<u64> {
dependency::version::Tag::Folder => {
let folder = *dependency_version.folder();
let mut folder_buf = PathBuffer::uninit();
let Some(joined) = resolve_path::join_abs_string_buf_checked::<path::platform::Auto>(
FileSystem::instance().top_level_dir(),
let Some(relative) = folder_relative_to_top_level_dir(
source.path.name().dir,
folder.slice(buf),
&mut folder_buf.0,
&[source.path.name().dir, folder.slice(buf)],
) else {
log.add_error_fmt(
source,
Expand All @@ -1861,20 +1885,7 @@ impl Package<u64> {
);
return Err(crate::Error::InstallFailed);
};
let relative: &[u8] =
resolve_path::relative(FileSystem::instance().top_level_dir(), joined);
#[cfg(windows)]
let relative: &[u8] = {
let len = relative.len();
folder_buf.0[..len].copy_from_slice(relative);
path::dangerously_convert_path_to_posix_in_place::<u8>(
&mut folder_buf.0[..len],
);
&folder_buf.0[..len]
};
// if relative is empty, we are linking the package to itself
dependency_version.value.folder = string_builder
.append::<String>(if relative.is_empty() { b"." } else { relative });
dependency_version.value.folder = string_builder.append::<String>(relative);
}
dependency::version::Tag::Npm => {
if let Some(workspace_version) = workspace_version {
Expand Down
43 changes: 43 additions & 0 deletions src/install/pnpm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1620,6 +1620,7 @@ pub(crate) fn migrate_pnpm_lockfile<'a>(
// pnpm records `os`/`cpu` for every `packages:` entry whose manifest
// declares them, including `file:` folders, tarballs, and git packages.
crate::migration::clear_non_registry_platform_constraints(lockfile);
declare_folder_dependencies_relative_to_their_package(lockfile)?;

lockfile.resolve(log)?;

Expand Down Expand Up @@ -1780,6 +1781,48 @@ fn declared_package_peers(
Ok(peers)
}

/// pnpm records a folder package's `file:` dependencies relative to the lockfile
/// (`file:sub-dep/child` for a declared `file:./child`), which the resolution
/// pass above keys on; bun.lock stores them relative to the declaring package,
/// as declared. Importers already carry the declared `specifier`.
Comment thread
robobun marked this conversation as resolved.
fn declare_folder_dependencies_relative_to_their_package(
lockfile: &mut Lockfile,
) -> Result<(), AllocError> {
let mut literal: Vec<u8> = Vec::new();
for pkg_id in 0..lockfile.packages.len() {
let pkg_resolution = lockfile.packages.items_resolution()[pkg_id];
if pkg_resolution.tag != resolution::Tag::Folder {
continue;
}
let pkg_dir = *pkg_resolution.folder();
let rows = lockfile.packages.items_dependencies()[pkg_id];

for dep_id in rows.begin() as usize..rows.end() as usize {
let dep = &lockfile.buffers.dependencies[dep_id];
if dep.version.tag != dependency::VersionTag::Folder {
continue;
}
let folder = *dep.version.folder();

literal.clear();
literal.extend_from_slice(b"file:");
let relative = bun_paths::resolve_path::relative(
pkg_dir.slice(string_bytes!(lockfile)),
folder.slice(string_bytes!(lockfile)),
);
literal.extend_from_slice(if relative.is_empty() { b"." } else { relative });
#[cfg(windows)]
bun_paths::dangerously_convert_path_to_posix_in_place::<u8>(
&mut literal[b"file:".len()..],
);

let appended = sbuf!(lockfile).append(&literal)?;
lockfile.buffers.dependencies[dep_id].version.literal = appended;
}
}
Ok(())
}

fn parse_append_package_dependencies(
lockfile: &mut Lockfile,
package_obj: &Expr,
Expand Down
110 changes: 110 additions & 0 deletions test/cli/install/bun-lock.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1826,3 +1826,113 @@ describe.each(["hoisted", "isolated"] as const)("peer no published version satis
expect(await file(lockfilePath).text()).toBe(lockfile);
});
});

// A lockfile stores the `file:` path of a dependency exactly as the declaring
// package.json wrote it, i.e. relative to that package.json, while a fresh parse
// of the same package.json stores it relative to the project. The rows below
// only get resolved again from the loaded lockfile (an override for their name
// went away, or `bun update <name>` targets them), which is when the two forms
// have to agree.
describe.concurrent("re-resolving file: dependencies declared by a file: package or a workspace", () => {
const pkg = (name: string, version: string, dependencies?: Record<string, string>) =>
JSON.stringify({ name, version, dependencies });
const app = (overrides?: Record<string, string>) =>
JSON.stringify({ name: "app", dependencies: { a: "file:./vendor/a" }, overrides });
const installedVersion = async (packageDir: string, ...segments: string[]) =>
(await file(join(packageDir, ...segments, "package.json")).json()).version;

// `a` declares `b` relative to itself; the override redirects `b` elsewhere.
const vendored = {
"vendor/a/package.json": pkg("a", "1.0.0", { b: "file:../b" }),
"vendor/b/package.json": pkg("b", "1.0.0"),
"vendor/b2/package.json": pkg("b", "2.0.0"),
};

it.each([
["bun.lock", true],
["bun.lockb", false],
])("removing an override resolves the file: package's dependency next to it again (%s)", async (_, text) => {
const { packageDir, packageJson } = await registry.createTestDir({
bunfigOpts: { linker: "hoisted", saveTextLockfile: text },
files: { ...vendored, "package.json": app({ b: "file:./vendor/b2" }) },
});
const run = makeInstallRunner(packageDir);

await run(["install"]);
expect(await installedVersion(packageDir, "node_modules", "a", "node_modules", "b")).toBe("2.0.0");
expect(await exists(join(packageDir, text ? "bun.lock" : "bun.lockb"))).toBe(true);

await write(packageJson, app());
await run(["install"]);
expect(await installedVersion(packageDir, "node_modules", "a", "node_modules", "b")).toBe("1.0.0");
await run(["install", "--frozen-lockfile"]);

await run(["install", "--save-text-lockfile", "--lockfile-only"]);
const lockfile = await file(join(packageDir, "bun.lock")).text();
expect(lockfile).toContain('"a": ["a@file:vendor/a", { "dependencies": { "b": "file:../b" } }]');
expect(lockfile).toContain('"a/b": ["b@file:vendor/b", {}]');
});

it("a path without .. is resolved from the declaring package, not from the project root", async () => {
const { packageDir, packageJson } = await registry.createTestDir({
bunfigOpts: { linker: "hoisted" },
files: {
"vendor/a/package.json": pkg("a", "1.0.0", { b: "file:./b" }),
"vendor/a/b/package.json": pkg("b", "1.0.0"),
// same relative path from the project root
"b/package.json": pkg("b", "9.9.9"),
"vendor/b2/package.json": pkg("b", "2.0.0"),
"package.json": app({ b: "file:./vendor/b2" }),
},
});
const run = makeInstallRunner(packageDir);

await run(["install"]);
await write(packageJson, app());
await run(["install"]);
expect(await file(join(packageDir, "bun.lock")).text()).toContain('"a/b": ["b@file:vendor/a/b", {}]');
expect(await installedVersion(packageDir, "node_modules", "a", "node_modules", "b")).toBe("1.0.0");
});

it("bun update <name> re-resolves the file: package's dependency", async () => {
const { packageDir } = await registry.createTestDir({
bunfigOpts: { linker: "hoisted" },
files: { ...vendored, "package.json": app() },
});
const run = makeInstallRunner(packageDir);

await run(["install"]);
const lockfile = await file(join(packageDir, "bun.lock")).text();
expect(lockfile).toContain('"a/b": ["b@file:vendor/b", {}]');

await run(["update", "b"]);
expect(await file(join(packageDir, "bun.lock")).text()).toBe(lockfile);
expect(await installedVersion(packageDir, "node_modules", "a", "node_modules", "b")).toBe("1.0.0");
});

it("removing an override resolves a workspace's file: dependency relative to the workspace again", async () => {
const workspaceRoot = (overrides?: Record<string, string>) =>
JSON.stringify({ name: "app", workspaces: ["packages/*"], overrides });
const { packageDir, packageJson } = await registry.createTestDir({
bunfigOpts: { linker: "hoisted" },
files: {
"packages/ws1/package.json": pkg("ws1", "1.0.0", { b: "file:../../vendor/b" }),
"vendor/b/package.json": pkg("b", "1.0.0"),
"vendor/b2/package.json": pkg("b", "2.0.0"),
"package.json": workspaceRoot({ b: "file:./vendor/b2" }),
},
});
const run = makeInstallRunner(packageDir);

await run(["install"]);
expect(await file(join(packageDir, "bun.lock")).text()).toContain('"ws1/b": ["b@file:vendor/b2", {}]');

await write(packageJson, workspaceRoot());
await run(["install"]);
const lockfile = await file(join(packageDir, "bun.lock")).text();
expect(lockfile).toContain('"b": "file:../../vendor/b"');
expect(lockfile).toContain('"ws1/b": ["b@file:vendor/b", {}]');
expect(await installedVersion(packageDir, "packages", "ws1", "node_modules", "b")).toBe("1.0.0");
await run(["install", "--frozen-lockfile"]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ exports[`pnpm-lock.yaml v9 snapshot alias whose dep-path version is a file: dire
"packages": {
"fork": ["bar@github:o/bar#aeb6b15f9c9957c8fa56f9731e914c4d8a6d2f2b", {}, ""],

"outer": ["outer@file:outer", { "dependencies": { "config": "file:shared/config", "fork": "https://codeload.github.com/o/bar/tar.gz/aeb6b15f9c9957c8fa56f9731e914c4d8a6d2f2b" } }],
"outer": ["outer@file:outer", { "dependencies": { "config": "file:../shared/config", "fork": "https://codeload.github.com/o/bar/tar.gz/aeb6b15f9c9957c8fa56f9731e914c4d8a6d2f2b" } }],

"outer/config": ["hi2@file:shared/config", {}],
}
Expand Down
Loading
Loading