Skip to content

Commit

Permalink
fix(npm): set up node_modules/.bin/ entries for package that provide …
Browse files Browse the repository at this point in the history
…bin entrypoints (denoland#23496)

Closes denoland#23036

---------

Co-authored-by: Nathan Whitaker <[email protected]>
  • Loading branch information
bartlomieju and nathanwhit committed May 23, 2024
1 parent 959739f commit 92a8d09
Show file tree
Hide file tree
Showing 12 changed files with 255 additions and 0 deletions.
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
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
60 changes: 60 additions & 0 deletions cli/npm/managed/resolvers/local.rs
Original file line number Diff line number Diff line change
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 @@ -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"),
Expand All @@ -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)
Expand Down Expand Up @@ -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(
Expand All @@ -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(())
Expand Down Expand Up @@ -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);
Expand Down
109 changes: 109 additions & 0 deletions cli/npm/managed/resolvers/local/bin_entries.rs
Original file line number Diff line number Diff line change
@@ -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(())
}
5 changes: 5 additions & 0 deletions cli/util/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Download http:https://localhost:4260/@denotest/bin
Download http:https://localhost:4260/@denotest/bin/1.0.0.tgz
Initialize @denotest/[email protected]
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"nodeModulesDir": true
}
6 changes: 6 additions & 0 deletions tests/specs/task/bin_package/npm-run.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@

> sayhi
> cli-esm hi hello

hi
hello
9 changes: 9 additions & 0 deletions tests/specs/task/bin_package/package.json
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Download http:https://localhost:4260/@denotest/bin
Download http:https://localhost:4260/@denotest/bin/1.0.0.tgz
Initialize @denotest/[email protected]
Task sayhi cli-esm hi hello
hi
hello

0 comments on commit 92a8d09

Please sign in to comment.