Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
10 changes: 10 additions & 0 deletions src/install/isolated_install/Installer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,16 @@ impl<'a> Installer<'a> {
break 'state (node_id, CompleteState::Skipped);
}

// A member displaced by a root dependency is reachable only
// through non-workspace edges, and workspace store tasks re-run
// every install, so a completed one is not an install.
Comment thread
robobun marked this conversation as resolved.
let pkg_id = nodes.items_pkg_id()[node_id.get() as usize];
if self.lockfile().packages.slice().items_resolution()[pkg_id as usize].tag
== ResolutionTag::Workspace
{
break 'state (node_id, CompleteState::Skipped);
}

break 'state (node_id, state);
};

Expand Down
56 changes: 33 additions & 23 deletions src/install/lockfile/Package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1750,31 +1750,41 @@
.append::<String>(if relative.is_empty() { b"." } else { relative });
}
dependency::version::Tag::Npm => {
if let Some(workspace_version) = workspace_version {
let satisfies =
dependency_version
.npm()
.version
.satisfies(workspace_version, buf, buf);
if workspace_path.is_some() {
let satisfies = match workspace_version {
Some(workspace_version) => {
dependency_version
.npm()
.version
.satisfies(workspace_version, buf, buf)
}
// A versionless member is only linkable by a wildcard
// range, matching `get_or_put_resolved_package`.
Comment thread
robobun marked this conversation as resolved.
None => dependency_version.npm().version.is_star(),
};
if pm.options.link_workspace_packages && satisfies {
// `String::sliced` takes `&'a self`; bind the unwrapped
// value so the borrow outlives the parse call.
let wp = workspace_path.unwrap();
let path = wp.sliced(buf);
if let Some(mut dep) = dependency::parse_with_tag(
external_alias.value,
Some(external_alias.hash),
path.slice,
dependency::version::Tag::Workspace,
&path,
Some(&mut *log),
Some(&mut *pm),
) {
// Whole-struct move so `Drop` frees the old npm
// chain; keep the existing `literal`.
dep.literal = dependency_version.literal;
dependency_version = dep;
if workspace_version.is_some() {
// `String::sliced` takes `&'a self`; bind the unwrapped
// value so the borrow outlives the parse call.
Comment thread
robobun marked this conversation as resolved.
Outdated
let wp = workspace_path.unwrap();

Check failure on line 1769 in src/install/lockfile/Package.rs

View workflow job for this annotation

GitHub Actions / cargo clippy

called `unwrap` on `workspace_path` after checking its variant with `is_some`
let path = wp.sliced(buf);
if let Some(mut dep) = dependency::parse_with_tag(
external_alias.value,
Some(external_alias.hash),
path.slice,
dependency::version::Tag::Workspace,
&path,
Some(&mut *log),
Some(&mut *pm),
) {
// Whole-struct move so `Drop` frees the old npm
// chain; keep the existing `literal`.
Comment thread
robobun marked this conversation as resolved.
dep.literal = dependency_version.literal;
dependency_version = dep;
}
}
// For a versionless member the dependency stays as-is;
// resolution links it to the workspace package.
Comment thread
robobun marked this conversation as resolved.
} else {
// It doesn't satisfy, but a workspace shares the same name. Override the workspace with the other dependency
for dep in &mut package_dependencies[0..dependencies_count as usize] {
Expand Down
89 changes: 79 additions & 10 deletions src/install/lockfile/bun.lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1604,6 +1604,10 @@ pub(crate) fn parse_into_binary_lockfile(
) -> Result<(), ParseError> {
lockfile.init_empty();

let link_workspace_packages = manager
.as_deref()
.is_none_or(|m| m.options.link_workspace_packages);

let Some(lockfile_version_expr) = root.get(b"lockfileVersion") else {
log.add_error(Some(source), root.loc, b"Missing lockfile version");
return Err(ParseError::InvalidLockfileVersion);
Expand Down Expand Up @@ -2100,6 +2104,7 @@ pub(crate) fn parse_into_binary_lockfile(
None,
None,
Some(&workspaces_obj),
link_workspace_packages,
)?;

let mut root_pkg = Package::default();
Expand All @@ -2126,6 +2131,7 @@ pub(crate) fn parse_into_binary_lockfile(

if lockfile_version != Version::V0 {
// these are the `workspaceOnly` packages
let pkgs_expr_for_claims = root.get(b"packages");
// snapshot the workspace-path handles up front so the loop
// body can take `&mut *lockfile` (`parse_append_dependencies`,
// `append_package_dedupe`) without conflicting with the
Expand Down Expand Up @@ -2168,6 +2174,7 @@ pub(crate) fn parse_into_binary_lockfile(
None,
None,
None,
link_workspace_packages,
)?;

pkg.dependencies = DependencySlice::new(off, len);
Expand All @@ -2187,17 +2194,22 @@ pub(crate) fn parse_into_binary_lockfile(
// there should be no duplicates
let pkg_id = lockfile.append_package_dedupe(&mut pkg)?;

let entry = pkg_map.get_or_put(name)?;
if entry.found_existing {
log.add_error_fmt(
source,
row.key_loc,
format_args!("Duplicate workspace name: '{}'", bstr::BStr::new(name)),
);
return Err(ParseError::InvalidWorkspaceObject);
}
// A member displaced by a root dependency only appears
// nested; pre-claiming its name would falsely collide with
// the root `packages` key the other package owns.
Comment thread
robobun marked this conversation as resolved.
if member_owns_packages_key(&pkgs_expr_for_claims, name, path) {
let entry = pkg_map.get_or_put(name)?;
if entry.found_existing {
log.add_error_fmt(
source,
row.key_loc,
format_args!("Duplicate workspace name: '{}'", bstr::BStr::new(name)),
);
return Err(ParseError::InvalidWorkspaceObject);
}

*entry.value_ptr = pkg_id;
*entry.value_ptr = pkg_id;
}

workspace_pkgs_len += 1;
continue 'workspaces;
Expand Down Expand Up @@ -2521,6 +2533,7 @@ pub(crate) fn parse_into_binary_lockfile(
Some(pkg_path),
Some(&bundled_pkgs),
None,
link_workspace_packages,
)?;

pkg.dependencies = DependencySlice::new(off, len);
Expand Down Expand Up @@ -3153,6 +3166,33 @@ fn map_dep_to_pkg(
}
}

/// A workspace member normally owns the root `packages` key matching its
/// name. When a root dependency replaced the member's workspace dependency
/// (`Package::parse_dependency`), that key holds the other package and the
/// member only appears nested, so its name must not pre-claim the key.
Comment thread
robobun marked this conversation as resolved.
fn member_owns_packages_key(pkgs_expr: &Option<Expr>, name: &[u8], path: &[u8]) -> bool {
let Some(pkgs) = pkgs_expr else {
return true;
};
let Some(value) = pkgs.get(name) else {
return true;
};
if !value.is_array() {
return true;
}
let Some(first) = array_items(&value).first().and_then(|item| item.as_str()) else {
return true;
};
// a member's own entry is written as "<name>@workspace:<path>"
let Some(rest) = first.get(name.len() + 1..) else {
return false;
};
strings::has_prefix(first, name)
&& first[name.len()] == b'@'
&& strings::has_prefix(rest, b"workspace:")
&& strings::eql(&rest[b"workspace:".len()..], path)
}

fn dependency_resolution_failure(
dep: &Dependency,
pkg_path: Option<&[u8]>,
Expand Down Expand Up @@ -3213,6 +3253,8 @@ fn parse_append_dependencies<const CHECK_FOR_BUNDLED: bool, const IS_ROOT: bool>
pkg_path: Option<&[u8]>,
bundled_pkgs: Option<&PkgPathSet>,
workspaces_obj: Option<&Expr>,
// Only meaningful when `IS_ROOT`.
link_workspace_packages: bool,
) -> Result<(u32, u32), ParseError> {
// Clearing on entry is equivalent to clearing on every exit path for all
// callers (none read the buf between calls) and also covers early-error exits.
Expand Down Expand Up @@ -3362,6 +3404,33 @@ fn parse_append_dependencies<const CHECK_FOR_BUNDLED: bool, const IS_ROOT: bool>
.expect("infallible: is_string checked");
let name_hash = StringBuilder::string_hash(name);

// Mirror `Package::parse_dependency`: an npm range that
// cannot link this member replaced its workspace dependency,
// so the loaded root dependency list must match.
Comment thread
robobun marked this conversation as resolved.
Outdated
let overridden = {
let bytes = lockfile.buffers.string_bytes.as_slice();
let member_version = lockfile.workspace_versions.get(&name_hash).copied();
lockfile.buffers.dependencies.as_slice()[off..]
.iter()
.any(|dep| {
if dep.version.tag != DependencyVersionTag::Npm {
return false;
}
let npm = dep.version.npm();
if StringBuilder::string_hash(npm.name.slice(bytes)) != name_hash {
return false;
}
let satisfies = match member_version {
Some(version) => npm.version.satisfies(version, bytes, bytes),
None => npm.version.is_star(),
};
!(link_workspace_packages && satisfies)
})
};
if overridden {
continue 'workspaces;
}

let dep = Dependency {
name: sbuf!(lockfile).append_with_hash(name, name_hash)?,
name_hash,
Expand Down
Loading
Loading