diff options
author | Nathan Whitaker <17734409+nathanwhit@users.noreply.github.com> | 2024-05-29 17:45:22 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-05-29 17:45:22 -0700 |
commit | a379009bfdddc56d6400740ad7be86f8930952ab (patch) | |
tree | a1198bce9d62d9c2a59073921707c31b894c8eee /cli/npm/managed/resolvers/local.rs | |
parent | e084fe10a98556d4630b54bdda2ce23b3b5b8a60 (diff) |
fix(cli): Prefer npm bin entries provided by packages closer to the root (#24024)
Fixes #24012.
In the case of multiple packages providing a binary with a same name, we
were basically leaving the results undefined (since we set up things in
parallel, and whichever got set up first won). In addition, we were
warning about these cases, even though it's a situation that's expected
to occur.
Instead, in the case of a collision in the binary names, we prefer the
binary provided by the package with the least depth in the dependency
tree.
While I was at it, I also took moved more code to `bin_entries.rs` since
it was starting to get a bit cluttered.
Diffstat (limited to 'cli/npm/managed/resolvers/local.rs')
-rw-r--r-- | cli/npm/managed/resolvers/local.rs | 48 |
1 files changed, 5 insertions, 43 deletions
diff --git a/cli/npm/managed/resolvers/local.rs b/cli/npm/managed/resolvers/local.rs index d9cf79c08..f0c2a3f65 100644 --- a/cli/npm/managed/resolvers/local.rs +++ b/cli/npm/managed/resolvers/local.rs @@ -292,7 +292,7 @@ async fn sync_resolution_with_fs( Vec::with_capacity(package_partitions.packages.len()); let mut newest_packages_by_name: HashMap<&String, &NpmResolutionPackage> = HashMap::with_capacity(package_partitions.packages.len()); - let bin_entries_to_setup = Arc::new(Mutex::new(Vec::with_capacity(16))); + let bin_entries = Arc::new(Mutex::new(bin_entries::BinEntries::new())); for package in &package_partitions.packages { if let Some(current_pkg) = newest_packages_by_name.get_mut(&package.id.nv.name) @@ -320,7 +320,7 @@ async fn sync_resolution_with_fs( let pb = progress_bar.clone(); let cache = cache.clone(); let package = package.clone(); - let bin_entries_to_setup = bin_entries_to_setup.clone(); + let bin_entries_to_setup = bin_entries.clone(); let handle = spawn(async move { cache.ensure_package(&package.id.nv, &package.dist).await?; let pb_guard = pb.update_with_prompt( @@ -348,7 +348,7 @@ async fn sync_resolution_with_fs( if package.bin.is_some() { bin_entries_to_setup .lock() - .push((package.clone(), package_path)); + .add(package.clone(), package_path); } // finally stop showing the progress bar @@ -482,46 +482,8 @@ async fn sync_resolution_with_fs( // 6. Set up `node_modules/.bin` entries for packages that need it. { - let bin_entries = bin_entries_to_setup.lock(); - if !bin_entries.is_empty() && !bin_node_modules_dir_path.exists() { - fs::create_dir_all(&bin_node_modules_dir_path).with_context(|| { - format!("Creating '{}'", bin_node_modules_dir_path.display()) - })?; - } - for (package, package_path) in &*bin_entries { - let package = snapshot.package_from_id(&package.id).unwrap(); - if let Some(bin_entries) = &package.bin { - match bin_entries { - deno_npm::registry::NpmPackageVersionBinEntry::String(script) => { - // the default bin name doesn't include the organization - let name = package - .id - .nv - .name - .rsplit_once('/') - .map_or(package.id.nv.name.as_str(), |(_, name)| name); - bin_entries::set_up_bin_entry( - package, - name, - script, - package_path, - &bin_node_modules_dir_path, - )?; - } - deno_npm::registry::NpmPackageVersionBinEntry::Map(entries) => { - for (name, script) in entries { - bin_entries::set_up_bin_entry( - package, - name, - script, - package_path, - &bin_node_modules_dir_path, - )?; - } - } - } - } - } + let bin_entries = std::mem::take(&mut *bin_entries.lock()); + bin_entries.finish(snapshot, &bin_node_modules_dir_path)?; } setup_cache.save(); |