Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(npm): set up node_modules/.bin/ entries for package that provide bin entrypoints #23496

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions cli/Cargo.toml
Expand Up @@ -123,6 +123,7 @@ once_cell.workspace = true
open = "5.0.1"
os_pipe.workspace = true
p256.workspace = true
pathdiff = "0.2.1"
percent-encoding.workspace = true
phf.workspace = true
pin-project.workspace = true
Expand Down
60 changes: 60 additions & 0 deletions cli/npm/managed/resolvers/local.rs
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -267,6 +270,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"),
Expand All @@ -291,6 +298,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)
Expand Down Expand Up @@ -319,6 +327,7 @@ async fn sync_resolution_with_fs(
let cache = cache.clone();
let registry_url = registry_url.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, &registry_url)
Expand All @@ -342,6 +351,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(())
Expand Down Expand Up @@ -472,6 +488,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);
Expand Down
109 changes: 109 additions & 0 deletions 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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add to the spec test a step that would cause this codepath to be hit on an already setup node_modules directory?

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(())
}
5 changes: 5 additions & 0 deletions cli/util/path.rs
Expand Up @@ -147,6 +147,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<PathBuf> {
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 {
Expand Down
2 changes: 2 additions & 0 deletions 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)) {
Expand Down
44 changes: 44 additions & 0 deletions 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"
}]
}
}
}
9 changes: 9 additions & 0 deletions 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
3 changes: 3 additions & 0 deletions tests/specs/task/bin_package/deno.json
@@ -0,0 +1,3 @@
{
"nodeModulesDir": true
}
6 changes: 6 additions & 0 deletions tests/specs/task/bin_package/npm-run.out
@@ -0,0 +1,6 @@

> sayhi
> cli-esm hi hello

hi
hello
9 changes: 9 additions & 0 deletions 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"
}
}
6 changes: 6 additions & 0 deletions 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