Skip to content

Commit 38186cc

Browse files
Copilotbartlomiejuclaude
authored
fix(npm): create shims for all bin entries during global npm install (#32607)
Fixes #26133 When running `deno install -g npm:pyright`, only the first bin entry (`pyright`) got a shim. The second entry (`pyright-langserver`) was missing. This affects any npm package with multiple `bin` entries in `package.json`. ### Changes - **Name resolution for multi-bin packages** — `resolve_name_from_npm_package_info()` now returns the bin name matching the package name (or unscoped name for `@scope/pkg`) when multiple entries exist, instead of returning `None` - **Extra bin entry resolution** — `resolve_all_bin_entries_from_npm()` on `BinNameResolver` returns all `(name, script_path)` pairs; `BinaryNameAndUrl::resolve()` returns extra entries with proper `npm:pkg@version/script_path` URLs - **Shared config directory** — Extra bin shims reference the primary entry's config dir via a `config_name` field on `BinaryNameAndUrl`, so all shims from the same package share one `.pkg/` config directory with `deno.json`, `package.json`, and `node_modules` - **Install** — `install_global()` creates shims for all entries and persists extra names to `extra_bin_entries.json` in the config dir - **Rollback on failure** — If an extra bin shim fails to install (e.g., name conflict without `-f`), the primary shim and any already-created extra shims are cleaned up to avoid partial installs - **Uninstall** — `uninstall()` reads `extra_bin_entries.json` and removes all associated shims - **Skipped when `--name` is provided** — explicit naming implies single-entry intent ### Test coverage - New `@denotest/multi-bin` test package with two bin entries - Spec test: install creates both shims, both execute correctly, uninstall removes both - Unit tests for multi-bin name inference and entry resolution --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: bartlomieju <13602871+bartlomieju@users.noreply.github.com> Co-authored-by: Bartek Iwańczuk <biwanczuk@gmail.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 49a9e13 commit 38186cc

11 files changed

Lines changed: 388 additions & 25 deletions

File tree

cli/tools/installer/bin_name_resolver.rs

Lines changed: 152 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ use std::path::PathBuf;
55
use deno_core::error::AnyError;
66
use deno_core::url::Url;
77
use deno_npm::registry::NpmRegistryApi;
8-
use deno_npm::resolution::NpmPackageVersionResolver;
98
use deno_npm::resolution::NpmVersionResolver;
109
use deno_semver::npm::NpmPackageReqReference;
1110

@@ -109,41 +108,87 @@ impl<'a> BinNameResolver<'a> {
109108
Some(stem.to_string())
110109
}
111110

112-
async fn resolve_name_from_npm(
111+
/// Fetches and resolves the bin entries for an npm package.
112+
/// Returns all (bin_name, script_path) pairs.
113+
async fn resolve_npm_bin_entries(
113114
&self,
114115
npm_ref: &NpmPackageReqReference,
115-
) -> Result<Option<String>, AnyError> {
116+
) -> Result<Option<Vec<(String, String)>>, AnyError> {
116117
let package_info = self
117118
.npm_registry_api
118119
.package_info(&npm_ref.req().name)
119120
.await?;
120121
let version_resolver =
121122
self.npm_version_resolver.get_for_package(&package_info);
122-
Ok(self.resolve_name_from_npm_package_info(&version_resolver, npm_ref))
123-
}
124-
125-
fn resolve_name_from_npm_package_info(
126-
&self,
127-
version_resolver: &NpmPackageVersionResolver,
128-
npm_ref: &NpmPackageReqReference,
129-
) -> Option<String> {
130123
let version_info = version_resolver
131124
.resolve_best_package_version_info(
132125
&npm_ref.req().version_req,
133126
Vec::new().into_iter(),
134127
)
135-
.ok()?;
136-
let bin_entries = version_info.bin.as_ref()?;
128+
.ok();
129+
let Some(version_info) = version_info else {
130+
return Ok(None);
131+
};
132+
let Some(bin_entries) = version_info.bin.as_ref() else {
133+
return Ok(None);
134+
};
137135
match bin_entries {
138-
deno_npm::registry::NpmPackageVersionBinEntry::String(_) => {}
139-
deno_npm::registry::NpmPackageVersionBinEntry::Map(data) => {
140-
if data.len() == 1 {
141-
return Some(data.keys().next().unwrap().clone());
142-
}
136+
deno_npm::registry::NpmPackageVersionBinEntry::String(script) => {
137+
let name = &npm_ref.req().name;
138+
let bin_name = name.rsplit('/').next().unwrap_or(name.as_str());
139+
Ok(Some(vec![(bin_name.to_string(), script.clone())]))
140+
}
141+
deno_npm::registry::NpmPackageVersionBinEntry::Map(data) => Ok(Some(
142+
data.iter().map(|(k, v)| (k.clone(), v.clone())).collect(),
143+
)),
144+
}
145+
}
146+
147+
async fn resolve_name_from_npm(
148+
&self,
149+
npm_ref: &NpmPackageReqReference,
150+
) -> Result<Option<String>, AnyError> {
151+
let entries = self.resolve_npm_bin_entries(npm_ref).await?;
152+
Ok(Self::infer_name_from_bin_entries(
153+
entries.as_deref(),
154+
npm_ref,
155+
))
156+
}
157+
158+
fn infer_name_from_bin_entries(
159+
entries: Option<&[(String, String)]>,
160+
npm_ref: &NpmPackageReqReference,
161+
) -> Option<String> {
162+
let entries = entries?;
163+
if entries.len() == 1 {
164+
return Some(entries[0].0.clone());
165+
}
166+
if entries.len() > 1 {
167+
// When there are multiple bin entries, check if one matches the
168+
// package name (the npm default bin convention). For scoped packages
169+
// like @scope/name, check the unscoped portion.
170+
let pkg_name = &npm_ref.req().name;
171+
let unscoped_name = pkg_name
172+
.strip_prefix('@')
173+
.and_then(|s| s.split_once('/'))
174+
.map(|(_, name)| name)
175+
.unwrap_or(pkg_name.as_str());
176+
if entries.iter().any(|(name, _)| name == unscoped_name) {
177+
return Some(unscoped_name.to_string());
143178
}
144179
}
145180
None
146181
}
182+
183+
/// Resolves all bin entries for an npm package URL.
184+
/// Returns a list of (bin_name, script_path) pairs.
185+
pub async fn resolve_all_bin_entries_from_npm(
186+
&self,
187+
url: &Url,
188+
) -> Option<Vec<(String, String)>> {
189+
let npm_ref = NpmPackageReqReference::from_specifier(url).ok()?;
190+
self.resolve_npm_bin_entries(&npm_ref).await.ok().flatten()
191+
}
147192
}
148193

149194
#[cfg(test)]
@@ -313,4 +358,93 @@ mod test {
313358
Some("gemini".to_string())
314359
);
315360
}
361+
362+
#[tokio::test]
363+
async fn install_infer_name_multi_bin_npm() {
364+
// Package with multiple bins where one matches the package name
365+
let _ = rustls::crypto::aws_lc_rs::default_provider().install_default();
366+
let http_client = HttpClientProvider::new(None, None);
367+
let registry_api = TestNpmRegistryApi::default();
368+
registry_api.with_version_info(("pyright", "1.0.0"), |info| {
369+
info.bin = Some(deno_npm::registry::NpmPackageVersionBinEntry::Map(
370+
HashMap::from([
371+
("pyright".to_string(), "index.js".to_string()),
372+
(
373+
"pyright-langserver".to_string(),
374+
"langserver.index.js".to_string(),
375+
),
376+
]),
377+
))
378+
});
379+
let npm_version_resolver = NpmVersionResolver::default();
380+
let resolver =
381+
BinNameResolver::new(&http_client, &registry_api, &npm_version_resolver);
382+
383+
// Should infer "pyright" as the primary name since it matches the package name
384+
let name = resolver
385+
.infer_name_from_url(&Url::parse("npm:pyright").unwrap())
386+
.await;
387+
assert_eq!(name, Some("pyright".to_string()));
388+
}
389+
390+
#[tokio::test]
391+
async fn install_infer_name_multi_bin_scoped_npm() {
392+
// Scoped package with multiple bins where one matches the unscoped package name
393+
let _ = rustls::crypto::aws_lc_rs::default_provider().install_default();
394+
let http_client = HttpClientProvider::new(None, None);
395+
let registry_api = TestNpmRegistryApi::default();
396+
registry_api.with_version_info(("@denotest/multi-bin", "1.0.0"), |info| {
397+
info.bin = Some(deno_npm::registry::NpmPackageVersionBinEntry::Map(
398+
HashMap::from([
399+
("multi-bin".to_string(), "./cli.mjs".to_string()),
400+
("multi-bin-server".to_string(), "./server.mjs".to_string()),
401+
]),
402+
))
403+
});
404+
let npm_version_resolver = NpmVersionResolver::default();
405+
let resolver =
406+
BinNameResolver::new(&http_client, &registry_api, &npm_version_resolver);
407+
408+
// Should infer "multi-bin" as the primary name
409+
let name = resolver
410+
.infer_name_from_url(&Url::parse("npm:@denotest/multi-bin").unwrap())
411+
.await;
412+
assert_eq!(name, Some("multi-bin".to_string()));
413+
}
414+
415+
#[tokio::test]
416+
async fn resolve_all_bin_entries_multi_bin() {
417+
let _ = rustls::crypto::aws_lc_rs::default_provider().install_default();
418+
let http_client = HttpClientProvider::new(None, None);
419+
let registry_api = TestNpmRegistryApi::default();
420+
registry_api.with_version_info(("pyright", "1.0.0"), |info| {
421+
info.bin = Some(deno_npm::registry::NpmPackageVersionBinEntry::Map(
422+
HashMap::from([
423+
("pyright".to_string(), "index.js".to_string()),
424+
(
425+
"pyright-langserver".to_string(),
426+
"langserver.index.js".to_string(),
427+
),
428+
]),
429+
))
430+
});
431+
let npm_version_resolver = NpmVersionResolver::default();
432+
let resolver =
433+
BinNameResolver::new(&http_client, &registry_api, &npm_version_resolver);
434+
435+
let entries = resolver
436+
.resolve_all_bin_entries_from_npm(&Url::parse("npm:pyright").unwrap())
437+
.await;
438+
let mut entries = entries.unwrap();
439+
entries.sort_by(|a, b| a.0.cmp(&b.0));
440+
assert_eq!(entries.len(), 2);
441+
assert_eq!(entries[0], ("pyright".to_string(), "index.js".to_string()));
442+
assert_eq!(
443+
entries[1],
444+
(
445+
"pyright-langserver".to_string(),
446+
"langserver.index.js".to_string()
447+
)
448+
);
449+
}
316450
}

0 commit comments

Comments
 (0)