Skip to content
Merged
15 changes: 2 additions & 13 deletions src/install/PackageInstaller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1456,19 +1456,8 @@ impl<'a> PackageInstaller<'a> {
} else {
// transitive folder dependencies are not hoisted
if folder.len() >= self.folder_path_buf.len()
|| (bin::bin_target_escapes_package_dir(folder) && {
// overrides/resolutions are only ever parsed from the root
// package.json, so a folder path that reached here via an
// override was written by the user and is trusted the same
// as a direct dependency of the root.
let dep = &self.lockfile().buffers.dependencies.as_slice()
[dependency_id as usize];
!self.lockfile().overrides.contains_name(
dep.name_hash,
dep.name.slice(string_buf!()),
string_buf!(),
)
})
|| (bin::bin_target_escapes_package_dir(folder)
&& !self.lockfile().is_trusted_folder_dependency(dependency_id))
{
if log_level != Options::LogLevel::Silent {
bun_core::pretty_errorln!(
Expand Down
17 changes: 4 additions & 13 deletions src/install/PackageManager/PackageManagerEnqueue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2849,19 +2849,10 @@ fn get_or_put_resolved_package(
}

// transitive folder dependencies do not have their dependencies resolved
if crate::bin::bin_target_escapes_package_dir(this.lockfile.str(&folder)) {
// overrides/resolutions are only ever parsed from the root
// package.json, so a folder path that reached here via an
// override was written by the user and is trusted the same
// as a direct dependency of the root.
let buf = this.lockfile.buffers.string_bytes.as_slice();
if !this.lockfile.overrides.contains_name(
dependency.name_hash,
dependency.name.slice(buf),
buf,
) {
break 'res FolderResolutionValue::Err(crate::Error::MissingPackageJSON);
}
if crate::bin::bin_target_escapes_package_dir(this.lockfile.str(&folder))
&& !this.lockfile.is_trusted_folder_dependency(dependency_id)
{
break 'res FolderResolutionValue::Err(crate::Error::MissingPackageJSON);
}

let mut package = Package::default();
Expand Down
9 changes: 4 additions & 5 deletions src/install/PackageManager/PackageManagerResolution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,11 +326,10 @@ impl PackageManager {
continue;
}

let features = match pkg_resolutions[parent_id].tag {
ResolutionTag::Root | ResolutionTag::Workspace | ResolutionTag::Folder => {
self.options.local_package_features
}
_ => self.options.remote_package_features,
let features = if pkg_resolutions[parent_id].tag.is_local_package() {
self.options.local_package_features
} else {
self.options.remote_package_features
};
// even if optional dependencies are enabled, it's still allowed to fail
if failed_dep.behavior.is_optional() || !failed_dep.behavior.is_enabled(features) {
Expand Down
44 changes: 44 additions & 0 deletions src/install/lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -860,6 +860,50 @@ impl Lockfile {
0
}

/// Does the root or a workspace depend on package `id` directly?
pub(crate) fn is_workspace_declared_package(&self, id: PackageID) -> bool {
let packages = self.packages.slice();
let resolutions = self.buffers.resolutions.as_slice();
for (pkg_id, res_list) in packages.items_resolutions().iter().enumerate() {
let tag = packages.items_resolution()[pkg_id].tag;
if tag != ResolutionTag::Workspace && tag != ResolutionTag::Root {
continue;
}
if res_list.get(resolutions).contains(&id) {
return true;
}
}
false
}

/// Is dependency `id` declared by the root, a workspace, or a `file:` package
/// one of them depends on directly? Checked, not assumed: a migrated lockfile
/// can carry dependencies for a folder that a registry package shipped.
Comment thread
robobun marked this conversation as resolved.
Resolved
pub(crate) fn is_dependency_of_local_package(&self, id: DependencyID) -> bool {
let Some(parent_id) = self.get_parent_pkg_of_dependency(id) else {
return false;
};
match self.packages.items_resolution()[parent_id as usize].tag {
ResolutionTag::Root | ResolutionTag::Workspace => true,
ResolutionTag::Folder => self.is_workspace_declared_package(parent_id),
_ => false,
}
}

/// May the folder path of dependency `id` leave its package directory? Yes
/// when a local package declares the dependency, or when a plain override or
/// resolution supplies the path (those are only ever parsed from the root
/// package.json). Both are user authored, like a root `file:` dependency.
Comment thread
robobun marked this conversation as resolved.
Resolved
pub(crate) fn is_trusted_folder_dependency(&self, id: DependencyID) -> bool {
if self.is_dependency_of_local_package(id) {
return true;
}
let buf = self.buffers.string_bytes.as_slice();
let dep = &self.buffers.dependencies[id as usize];
self.overrides
.contains_name(dep.name_hash, dep.name.slice(buf), buf)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

/// Does this tree id belong to a workspace (including workspace root)?
/// TODO(dylan-conway) fix!
pub(crate) fn is_workspace_tree_id(&self, id: tree::Id) -> bool {
Expand Down
9 changes: 4 additions & 5 deletions src/install/lockfile/Tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -602,11 +602,10 @@ pub(crate) fn is_filtered_dependency_or_workspace(
return true;
}

let dep_features = match parent_res.tag {
crate::resolution::Tag::Root
| crate::resolution::Tag::Workspace
| crate::resolution::Tag::Folder => manager.options.local_package_features,
_ => manager.options.remote_package_features,
let dep_features = if parent_res.tag.is_local_package() {
manager.options.local_package_features
} else {
manager.options.remote_package_features
};

if !dep.behavior.is_enabled(dep_features) {
Expand Down
14 changes: 14 additions & 0 deletions src/install/migration/npm_lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ struct Migrator<'a> {
link_entries: DynamicBitSet,
shadowed: DynamicBitSet,
skipped_external: DynamicBitSet,
/// Targets the root or a workspace depends on directly.
local_declared: DynamicBitSet,
entry_package_ids: Vec<PackageID>,
queue: Vec<(u32, PackageID)>,
probe: Vec<u8>,
Expand Down Expand Up @@ -142,6 +144,7 @@ pub(super) fn migrate_packages(
link_entries: DynamicBitSet::init_empty(entry_count)?,
shadowed: DynamicBitSet::init_empty(entry_count)?,
skipped_external: DynamicBitSet::init_empty(entry_count)?,
local_declared: DynamicBitSet::init_empty(entry_count)?,
entry_package_ids: vec![INVALID_PACKAGE_ID; entry_count],
queue: Vec::new(),
probe: Vec::new(),
Expand Down Expand Up @@ -765,9 +768,20 @@ impl<'a> Migrator<'a> {
}

let version_tag = version.tag;
// Trust a `file:` spec only in a `file:` package the root or a workspace
// declares, not in a folder a registry package ships (`is_trusted_folder_dependency`).
Comment thread
robobun marked this conversation as resolved.
Resolved
let declares_folder = res_tag == resolution::Tag::Folder
&& version_tag == DepTag::Folder
&& self.local_declared.is_set(j as usize);
let mut found = self.find_target(key, name);
if let Some((t, _)) = found
&& is_local
{
self.local_declared.set(t as usize);
}
if let Some((t, through_link)) = found
&& !is_local
&& !declares_folder
&& self.is_external_folder(t, through_link)
{
self.skip_external(t, name);
Expand Down
7 changes: 7 additions & 0 deletions src/install/resolution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1014,6 +1014,13 @@ impl Tag {
self == Tag::Git || self == Tag::Github
}

/// The root, a workspace, or a `file:` folder package: a package.json on
/// disk rather than content fetched from a registry, git host, or tarball.
/// These get `local_package_features`.
Comment thread
robobun marked this conversation as resolved.
Resolved
pub(crate) fn is_local_package(self) -> bool {
self == Tag::Root || self == Tag::Workspace || self == Tag::Folder
}

pub(crate) fn can_enqueue_install_task(self) -> bool {
self == Tag::Npm
|| self == Tag::LocalTarball
Expand Down
Loading
Loading