From 92a8d09e498712aec2ba0e54a1ad85194ebd83af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Fri, 24 May 2024 00:43:38 +0100 Subject: [PATCH] fix(npm): set up node_modules/.bin/ entries for package that provide bin entrypoints (#23496) Closes https://github.com/denoland/deno/issues/23036 --------- Co-authored-by: Nathan Whitaker --- Cargo.lock | 1 + cli/Cargo.toml | 1 + cli/npm/managed/resolvers/local.rs | 60 ++++++++++ .../managed/resolvers/local/bin_entries.rs | 109 ++++++++++++++++++ cli/util/path.rs | 5 + .../registry/npm/@denotest/bin/1.0.0/cli.mjs | 2 + tests/specs/task/bin_package/__test__.jsonc | 44 +++++++ .../specs/task/bin_package/already-set-up.out | 9 ++ tests/specs/task/bin_package/deno.json | 3 + tests/specs/task/bin_package/npm-run.out | 6 + tests/specs/task/bin_package/package.json | 9 ++ tests/specs/task/bin_package/task.out | 6 + 12 files changed, 255 insertions(+) create mode 100644 cli/npm/managed/resolvers/local/bin_entries.rs create mode 100644 tests/specs/task/bin_package/__test__.jsonc create mode 100644 tests/specs/task/bin_package/already-set-up.out create mode 100644 tests/specs/task/bin_package/deno.json create mode 100644 tests/specs/task/bin_package/npm-run.out create mode 100644 tests/specs/task/bin_package/package.json create mode 100644 tests/specs/task/bin_package/task.out diff --git a/Cargo.lock b/Cargo.lock index 8a7899b8cf..4ce435af30 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1136,6 +1136,7 @@ dependencies = [ "once_cell", "open", "p256", + "pathdiff", "percent-encoding", "phf 0.11.2", "pretty_assertions", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index c2edd2b606..8c033f55a8 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -122,6 +122,7 @@ notify.workspace = true once_cell.workspace = true open = "5.0.1" p256.workspace = true +pathdiff = "0.2.1" percent-encoding.workspace = true phf.workspace = true quick-junit = "^0.3.5" diff --git a/cli/npm/managed/resolvers/local.rs b/cli/npm/managed/resolvers/local.rs index 9d0ca3f8c5..055fdfb23a 100644 --- a/cli/npm/managed/resolvers/local.rs +++ b/cli/npm/managed/resolvers/local.rs @@ -2,6 +2,8 @@ //! Code for local node_modules resolution. +mod bin_entries; + use std::borrow::Cow; use std::cmp::Ordering; use std::collections::HashMap; @@ -24,6 +26,7 @@ use deno_ast::ModuleSpecifier; use deno_core::anyhow::bail; use deno_core::anyhow::Context; use deno_core::error::AnyError; +use deno_core::parking_lot::Mutex; use deno_core::unsync::spawn; use deno_core::unsync::JoinHandle; use deno_core::url::Url; @@ -262,6 +265,10 @@ async fn sync_resolution_with_fs( fs::create_dir_all(&deno_node_modules_dir).with_context(|| { format!("Creating '{}'", deno_local_registry_dir.display()) })?; + let bin_node_modules_dir_path = root_node_modules_dir_path.join(".bin"); + fs::create_dir_all(&bin_node_modules_dir_path).with_context(|| { + format!("Creating '{}'", bin_node_modules_dir_path.display()) + })?; let single_process_lock = LaxSingleProcessFsFlag::lock( deno_local_registry_dir.join(".deno.lock"), @@ -286,6 +293,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))); for package in &package_partitions.packages { if let Some(current_pkg) = newest_packages_by_name.get_mut(&package.id.nv.name) @@ -313,6 +321,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 handle = spawn(async move { cache.ensure_package(&package.id.nv, &package.dist).await?; let pb_guard = pb.update_with_prompt( @@ -334,6 +343,13 @@ async fn sync_resolution_with_fs( } // write out a file that indicates this folder has been initialized fs::write(initialized_file, "")?; + + if package.bin.is_some() { + bin_entries_to_setup + .lock() + .push((package.clone(), package_path)); + } + // finally stop showing the progress bar drop(pb_guard); // explicit for clarity Ok(()) @@ -464,6 +480,50 @@ 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, + )?; + } + } + } + } + } + } + setup_cache.save(); drop(single_process_lock); drop(pb_clear_guard); diff --git a/cli/npm/managed/resolvers/local/bin_entries.rs b/cli/npm/managed/resolvers/local/bin_entries.rs new file mode 100644 index 0000000000..8e43cf98b6 --- /dev/null +++ b/cli/npm/managed/resolvers/local/bin_entries.rs @@ -0,0 +1,109 @@ +// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. + +use crate::npm::managed::NpmResolutionPackage; +use deno_core::anyhow::Context; +use deno_core::error::AnyError; +use std::path::Path; + +pub(super) fn set_up_bin_entry( + package: &NpmResolutionPackage, + bin_name: &str, + #[allow(unused_variables)] bin_script: &str, + #[allow(unused_variables)] package_path: &Path, + bin_node_modules_dir_path: &Path, +) -> Result<(), AnyError> { + #[cfg(windows)] + { + set_up_bin_shim(package, bin_name, bin_node_modules_dir_path)?; + } + #[cfg(unix)] + { + symlink_bin_entry( + package, + bin_name, + bin_script, + package_path, + bin_node_modules_dir_path, + )?; + } + Ok(()) +} + +#[cfg(windows)] +fn set_up_bin_shim( + package: &NpmResolutionPackage, + bin_name: &str, + bin_node_modules_dir_path: &Path, +) -> Result<(), AnyError> { + use std::fs; + let mut cmd_shim = bin_node_modules_dir_path.join(bin_name); + + cmd_shim.set_extension("cmd"); + let shim = format!("@deno run -A npm:{}/{bin_name} %*", package.id.nv); + if cmd_shim.exists() { + if let Ok(contents) = fs::read_to_string(cmd_shim) { + if contents == shim { + // up to date + return Ok(()); + } + } + return Ok(()); + } + fs::write(&cmd_shim, shim).with_context(|| { + format!("Can't set up '{}' bin at {}", bin_name, cmd_shim.display()) + })?; + + Ok(()) +} + +#[cfg(unix)] +fn symlink_bin_entry( + _package: &NpmResolutionPackage, + bin_name: &str, + bin_script: &str, + package_path: &Path, + bin_node_modules_dir_path: &Path, +) -> Result<(), AnyError> { + use std::os::unix::fs::symlink; + let link = bin_node_modules_dir_path.join(bin_name); + let original = package_path.join(bin_script); + + // Don't bother setting up another link if it already exists + if link.exists() { + let resolved = std::fs::read_link(&link).ok(); + if let Some(resolved) = resolved { + if resolved != original { + log::warn!( + "{} Trying to set up '{}' bin for \"{}\", but an entry pointing to \"{}\" already exists. Skipping...", + deno_terminal::colors::yellow("Warning"), + bin_name, + resolved.display(), + original.display() + ); + } + return Ok(()); + } + } + + use std::os::unix::fs::PermissionsExt; + let mut perms = std::fs::metadata(&original).unwrap().permissions(); + if perms.mode() & 0o111 == 0 { + // if the original file is not executable, make it executable + perms.set_mode(perms.mode() | 0o111); + std::fs::set_permissions(&original, perms).with_context(|| { + format!("Setting permissions on '{}'", original.display()) + })?; + } + let original_relative = + crate::util::path::relative_path(bin_node_modules_dir_path, &original) + .unwrap_or(original); + symlink(&original_relative, &link).with_context(|| { + format!( + "Can't set up '{}' bin at {}", + bin_name, + original_relative.display() + ) + })?; + + Ok(()) +} diff --git a/cli/util/path.rs b/cli/util/path.rs index 3c848edea7..16378e30b0 100644 --- a/cli/util/path.rs +++ b/cli/util/path.rs @@ -173,6 +173,11 @@ pub fn path_with_stem_suffix(path: &Path, suffix: &str) -> PathBuf { } } +#[cfg_attr(windows, allow(dead_code))] +pub fn relative_path(from: &Path, to: &Path) -> Option { + pathdiff::diff_paths(to, from) +} + /// Gets if the provided character is not supported on all /// kinds of file systems. pub fn is_banned_path_char(c: char) -> bool { diff --git a/tests/registry/npm/@denotest/bin/1.0.0/cli.mjs b/tests/registry/npm/@denotest/bin/1.0.0/cli.mjs index 0ae8e91903..7f6d1793a0 100644 --- a/tests/registry/npm/@denotest/bin/1.0.0/cli.mjs +++ b/tests/registry/npm/@denotest/bin/1.0.0/cli.mjs @@ -1,3 +1,5 @@ +#!/usr/bin/env -S node + import process from "node:process"; for (const arg of process.argv.slice(2)) { diff --git a/tests/specs/task/bin_package/__test__.jsonc b/tests/specs/task/bin_package/__test__.jsonc new file mode 100644 index 0000000000..19bad633e0 --- /dev/null +++ b/tests/specs/task/bin_package/__test__.jsonc @@ -0,0 +1,44 @@ +{ + "tests": { + "sets_up_bin_dir": { + "tempDir": true, + "steps": [ + // {"commandName": "npm", "args": "install", "output": "\nadded 1 package in [WILDCARD]\n"}, + { + "args": "task sayhi", + "output": "task.out" + }, + { + "if": "unix", + "commandName": "./node_modules/.bin/cli-esm", + "args": "hi hello", + "output": "hi\nhello\n" + }, + { + "if": "windows", + "commandName": "./node_modules/.bin/cli-esm.cmd", + "args": "hi hello", + "output": "hi\nhello\n" + }, + { + "commandName": "npm", + "args": "run sayhi", + "output": "npm-run.out" + } + ] + }, + "warns_if_already_setup": { + "tempDir": true, + "steps": [{ + "if": "unix", + "commandName": "npm", + "args": "install", + "output": "\nadded 1 package in [WILDCARD]\n" + }, { + "if": "unix", + "args": "task sayhi", + "output": "already-set-up.out" + }] + } + } +} diff --git a/tests/specs/task/bin_package/already-set-up.out b/tests/specs/task/bin_package/already-set-up.out new file mode 100644 index 0000000000..336459daa2 --- /dev/null +++ b/tests/specs/task/bin_package/already-set-up.out @@ -0,0 +1,9 @@ +Download http://localhost:4260/@denotest/bin +Download http://localhost:4260/@denotest/bin/1.0.0.tgz +Initialize @denotest/bin@1.0.0 +Warning Trying to set up [WILDCARD] bin for [WILDCARD], but an entry pointing to [WILDCARD] already exists. Skipping... +Warning Trying to set up [WILDCARD] bin for [WILDCARD] but an entry pointing to [WILDCARD] already exists. Skipping... +Warning Trying to set up [WILDCARD] bin for [WILDCARD], but an entry pointing to [WILDCARD] already exists. Skipping... +Task sayhi cli-esm hi hello +hi +hello diff --git a/tests/specs/task/bin_package/deno.json b/tests/specs/task/bin_package/deno.json new file mode 100644 index 0000000000..176354f98f --- /dev/null +++ b/tests/specs/task/bin_package/deno.json @@ -0,0 +1,3 @@ +{ + "nodeModulesDir": true +} diff --git a/tests/specs/task/bin_package/npm-run.out b/tests/specs/task/bin_package/npm-run.out new file mode 100644 index 0000000000..0c636787a5 --- /dev/null +++ b/tests/specs/task/bin_package/npm-run.out @@ -0,0 +1,6 @@ + +> sayhi +> cli-esm hi hello + +hi +hello diff --git a/tests/specs/task/bin_package/package.json b/tests/specs/task/bin_package/package.json new file mode 100644 index 0000000000..c0a34548f5 --- /dev/null +++ b/tests/specs/task/bin_package/package.json @@ -0,0 +1,9 @@ +{ + "name": "bin_package", + "devDependencies": { + "@denotest/bin": "1.0.0" + }, + "scripts": { + "sayhi": "cli-esm hi hello" + } +} diff --git a/tests/specs/task/bin_package/task.out b/tests/specs/task/bin_package/task.out new file mode 100644 index 0000000000..69b4f75082 --- /dev/null +++ b/tests/specs/task/bin_package/task.out @@ -0,0 +1,6 @@ +Download http://localhost:4260/@denotest/bin +Download http://localhost:4260/@denotest/bin/1.0.0.tgz +Initialize @denotest/bin@1.0.0 +Task sayhi cli-esm hi hello +hi +hello