Skip to content

Commit d6af7bc

Browse files
committed
fix(install): use managed npm for global commands
1 parent 90e7beb commit d6af7bc

12 files changed

Lines changed: 392 additions & 27 deletions

File tree

crates/vite_command/src/lib.rs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use std::os::fd::{BorrowedFd, RawFd};
33
use std::{
44
collections::HashMap,
55
ffi::OsStr,
6+
path::Path,
67
process::{ExitStatus, Stdio},
78
};
89

@@ -30,6 +31,15 @@ pub fn resolve_bin(
3031
path_env: Option<&OsStr>,
3132
cwd: impl AsRef<AbsolutePath>,
3233
) -> Result<AbsolutePathBuf, Error> {
34+
let candidate = Path::new(bin_name);
35+
if candidate.is_absolute() {
36+
if candidate.exists() {
37+
return AbsolutePathBuf::new(candidate.to_path_buf())
38+
.ok_or_else(|| Error::CannotFindBinaryPath(bin_name.into()));
39+
}
40+
return Err(Error::CannotFindBinaryPath(bin_name.into()));
41+
}
42+
3343
let current_path;
3444
let path_env = match path_env {
3545
Some(p) => p,
@@ -331,6 +341,36 @@ mod tests {
331341
assert!(result.is_ok(), "Should run command successfully, but got error: {:?}", result);
332342
}
333343

344+
#[tokio::test]
345+
async fn test_run_command_with_absolute_binary_path() {
346+
let temp_dir = create_temp_dir();
347+
let temp_dir_path =
348+
AbsolutePathBuf::new(temp_dir.path().canonicalize().unwrap().to_path_buf())
349+
.unwrap();
350+
351+
#[cfg(unix)]
352+
let (bin_path, args) = {
353+
use std::os::unix::fs::PermissionsExt;
354+
355+
let script_path = temp_dir_path.join("test-bin");
356+
std::fs::write(&script_path, "#!/bin/sh\nexit 0\n").unwrap();
357+
let mut permissions = std::fs::metadata(&script_path).unwrap().permissions();
358+
permissions.set_mode(0o755);
359+
std::fs::set_permissions(&script_path, permissions).unwrap();
360+
(script_path.as_path().display().to_string(), Vec::<&str>::new())
361+
};
362+
363+
#[cfg(not(unix))]
364+
let (bin_path, args) = {
365+
let current_exe = std::env::current_exe().unwrap();
366+
(current_exe.to_string_lossy().to_string(), vec!["--help"])
367+
};
368+
369+
let envs = HashMap::new();
370+
let result = run_command(&bin_path, &args, &envs, &temp_dir_path).await;
371+
assert!(result.is_ok(), "Should run absolute binary path, but got error: {:?}", result);
372+
}
373+
334374
#[tokio::test]
335375
async fn test_run_command_and_not_find_binary_path() {
336376
let temp_dir = create_temp_dir();

crates/vite_global_cli/src/commands/add.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use vite_install::{
66
};
77
use vite_path::AbsolutePathBuf;
88

9-
use super::prepend_js_runtime_to_path_env;
9+
use super::{managed_npm_bin_for_global_command, prepend_js_runtime_to_path_env};
1010
use crate::error::Error;
1111

1212
/// Add command for adding packages to dependencies.
@@ -35,7 +35,7 @@ impl AddCommand {
3535
allow_build: Option<&str>,
3636
pass_through_args: Option<&[String]>,
3737
) -> Result<ExitStatus, Error> {
38-
prepend_js_runtime_to_path_env(&self.cwd).await?;
38+
let node_bin_prefix = prepend_js_runtime_to_path_env(&self.cwd).await?;
3939
super::ensure_package_json(&self.cwd).await?;
4040

4141
let add_command_options = AddCommandOptions {
@@ -53,8 +53,15 @@ impl AddCommand {
5353

5454
// Detect package manager
5555
let package_manager = PackageManager::builder(&self.cwd).build_with_default().await?;
56+
let global_npm_bin_path = managed_npm_bin_for_global_command(global, &node_bin_prefix);
5657

57-
Ok(package_manager.run_add_command(&add_command_options, &self.cwd).await?)
58+
Ok(package_manager
59+
.run_add_command_with_global_npm_bin(
60+
&add_command_options,
61+
&self.cwd,
62+
global_npm_bin_path.as_deref(),
63+
)
64+
.await?)
5865
}
5966
}
6067

crates/vite_global_cli/src/commands/mod.rs

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,10 @@
2525
2626
use std::{collections::HashMap, io::BufReader};
2727

28-
use vite_install::package_manager::{PackageManager, PackageManagerType};
29-
use vite_path::AbsolutePath;
28+
use vite_install::package_manager::{
29+
PackageManager, PackageManagerType, npm_bin_path_from_node_bin_prefix,
30+
};
31+
use vite_path::{AbsolutePath, AbsolutePathBuf};
3032
use vite_shared::{PrependOptions, prepend_to_path_env};
3133

3234
use crate::{error::Error, js_executor::JsExecutor};
@@ -86,7 +88,9 @@ pub async fn ensure_package_json(project_path: &AbsolutePath) -> Result<(), Erro
8688
///
8789
/// If `project_path` contains a package.json, uses the project's runtime
8890
/// (based on devEngines.runtime). Otherwise, falls back to the CLI's runtime.
89-
pub async fn prepend_js_runtime_to_path_env(project_path: &AbsolutePath) -> Result<(), Error> {
91+
pub async fn prepend_js_runtime_to_path_env(
92+
project_path: &AbsolutePath,
93+
) -> Result<AbsolutePathBuf, Error> {
9094
let mut executor = JsExecutor::new(None);
9195

9296
// Use project runtime if package.json exists, otherwise use CLI runtime
@@ -104,7 +108,19 @@ pub async fn prepend_js_runtime_to_path_env(project_path: &AbsolutePath) -> Resu
104108
tracing::debug!("Set PATH to include {:?}", node_bin_prefix);
105109
}
106110

107-
Ok(())
111+
Ok(node_bin_prefix)
112+
}
113+
114+
pub(crate) fn managed_npm_bin_for_global_command(
115+
global: bool,
116+
node_bin_prefix: &AbsolutePath,
117+
) -> Option<AbsolutePathBuf> {
118+
if !global {
119+
return None;
120+
}
121+
122+
let npm_bin = npm_bin_path_from_node_bin_prefix(node_bin_prefix);
123+
npm_bin.as_path().exists().then_some(npm_bin)
108124
}
109125

110126
/// Build a PackageManager, converting PackageJsonNotFound into a friendly error message.
@@ -201,6 +217,25 @@ mod tests {
201217

202218
use super::*;
203219

220+
#[test]
221+
fn test_managed_npm_bin_for_global_command_uses_existing_adjacent_npm() {
222+
let temp_dir = tempfile::tempdir().unwrap();
223+
let node_bin_prefix = AbsolutePathBuf::new(temp_dir.path().to_path_buf()).unwrap();
224+
let expected = npm_bin_path_from_node_bin_prefix(&node_bin_prefix);
225+
std::fs::write(&expected, "").unwrap();
226+
227+
let npm_bin = managed_npm_bin_for_global_command(true, &node_bin_prefix).unwrap();
228+
assert_eq!(npm_bin, expected);
229+
assert!(managed_npm_bin_for_global_command(false, &node_bin_prefix).is_none());
230+
}
231+
232+
#[test]
233+
fn test_managed_npm_bin_for_global_command_falls_back_when_adjacent_npm_is_missing() {
234+
let temp_dir = tempfile::tempdir().unwrap();
235+
let node_bin_prefix = AbsolutePathBuf::new(temp_dir.path().to_path_buf()).unwrap();
236+
assert!(managed_npm_bin_for_global_command(true, &node_bin_prefix).is_none());
237+
}
238+
204239
#[test]
205240
fn test_has_vite_plus_in_dev_dependencies() {
206241
let temp_dir = tempfile::tempdir().unwrap();

crates/vite_global_cli/src/commands/outdated.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ use std::process::ExitStatus;
33
use vite_install::commands::outdated::{Format, OutdatedCommandOptions};
44
use vite_path::AbsolutePathBuf;
55

6-
use super::{build_package_manager, prepend_js_runtime_to_path_env};
6+
use super::{
7+
build_package_manager, managed_npm_bin_for_global_command, prepend_js_runtime_to_path_env,
8+
};
79
use crate::error::Error;
810

911
/// Outdated command for checking outdated packages.
@@ -36,7 +38,7 @@ impl OutdatedCommand {
3638
global: bool,
3739
pass_through_args: Option<&[String]>,
3840
) -> Result<ExitStatus, Error> {
39-
prepend_js_runtime_to_path_env(&self.cwd).await?;
41+
let node_bin_prefix = prepend_js_runtime_to_path_env(&self.cwd).await?;
4042

4143
let package_manager = build_package_manager(&self.cwd).await?;
4244

@@ -55,7 +57,14 @@ impl OutdatedCommand {
5557
global,
5658
pass_through_args,
5759
};
58-
Ok(package_manager.run_outdated_command(&outdated_command_options, &self.cwd).await?)
60+
let global_npm_bin_path = managed_npm_bin_for_global_command(global, &node_bin_prefix);
61+
Ok(package_manager
62+
.run_outdated_command_with_global_npm_bin(
63+
&outdated_command_options,
64+
&self.cwd,
65+
global_npm_bin_path.as_deref(),
66+
)
67+
.await?)
5968
}
6069
}
6170

crates/vite_global_cli/src/commands/remove.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ use std::process::ExitStatus;
33
use vite_install::commands::remove::RemoveCommandOptions;
44
use vite_path::AbsolutePathBuf;
55

6-
use super::{build_package_manager, prepend_js_runtime_to_path_env};
6+
use super::{
7+
build_package_manager, managed_npm_bin_for_global_command, prepend_js_runtime_to_path_env,
8+
};
79
use crate::error::Error;
810

911
/// Remove command for removing packages from dependencies.
@@ -31,7 +33,7 @@ impl RemoveCommand {
3133
global: bool,
3234
pass_through_args: Option<&[String]>,
3335
) -> Result<ExitStatus, Error> {
34-
prepend_js_runtime_to_path_env(&self.cwd).await?;
36+
let node_bin_prefix = prepend_js_runtime_to_path_env(&self.cwd).await?;
3537

3638
let package_manager = build_package_manager(&self.cwd).await?;
3739

@@ -46,7 +48,14 @@ impl RemoveCommand {
4648
save_prod,
4749
pass_through_args,
4850
};
49-
Ok(package_manager.run_remove_command(&remove_command_options, &self.cwd).await?)
51+
let global_npm_bin_path = managed_npm_bin_for_global_command(global, &node_bin_prefix);
52+
Ok(package_manager
53+
.run_remove_command_with_global_npm_bin(
54+
&remove_command_options,
55+
&self.cwd,
56+
global_npm_bin_path.as_deref(),
57+
)
58+
.await?)
5059
}
5160
}
5261

crates/vite_global_cli/src/commands/update.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ use std::process::ExitStatus;
33
use vite_install::commands::update::UpdateCommandOptions;
44
use vite_path::AbsolutePathBuf;
55

6-
use super::{build_package_manager, prepend_js_runtime_to_path_env};
6+
use super::{
7+
build_package_manager, managed_npm_bin_for_global_command, prepend_js_runtime_to_path_env,
8+
};
79
use crate::error::Error;
810

911
/// Update command for updating packages to their latest versions.
@@ -36,7 +38,7 @@ impl UpdateCommand {
3638
workspace_only: bool,
3739
pass_through_args: Option<&[String]>,
3840
) -> Result<ExitStatus, Error> {
39-
prepend_js_runtime_to_path_env(&self.cwd).await?;
41+
let node_bin_prefix = prepend_js_runtime_to_path_env(&self.cwd).await?;
4042

4143
let package_manager = build_package_manager(&self.cwd).await?;
4244

@@ -55,7 +57,14 @@ impl UpdateCommand {
5557
workspace_only,
5658
pass_through_args,
5759
};
58-
Ok(package_manager.run_update_command(&update_command_options, &self.cwd).await?)
60+
let global_npm_bin_path = managed_npm_bin_for_global_command(global, &node_bin_prefix);
61+
Ok(package_manager
62+
.run_update_command_with_global_npm_bin(
63+
&update_command_options,
64+
&self.cwd,
65+
global_npm_bin_path.as_deref(),
66+
)
67+
.await?)
5968
}
6069
}
6170

crates/vite_install/src/commands/add.rs

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use vite_shared::output;
77

88
use crate::package_manager::{
99
PackageManager, PackageManagerType, ResolveCommandResult, format_path_env,
10+
resolve_global_npm_bin_path,
1011
};
1112

1213
/// The type of dependency to save.
@@ -46,21 +47,46 @@ impl PackageManager {
4647
options: &AddCommandOptions<'_>,
4748
cwd: impl AsRef<AbsolutePath>,
4849
) -> Result<ExitStatus, Error> {
49-
let resolve_command = self.resolve_add_command(options);
50+
let resolve_command = self.resolve_add_command_with_global_npm_bin(options, None);
51+
run_command(&resolve_command.bin_path, &resolve_command.args, &resolve_command.envs, cwd)
52+
.await
53+
}
54+
55+
/// Run the add command with an explicit npm binary for global installs.
56+
/// Return the exit status of the command.
57+
#[must_use]
58+
pub async fn run_add_command_with_global_npm_bin(
59+
&self,
60+
options: &AddCommandOptions<'_>,
61+
cwd: impl AsRef<AbsolutePath>,
62+
global_npm_bin_path: Option<&AbsolutePath>,
63+
) -> Result<ExitStatus, Error> {
64+
let resolve_command =
65+
self.resolve_add_command_with_global_npm_bin(options, global_npm_bin_path);
5066
run_command(&resolve_command.bin_path, &resolve_command.args, &resolve_command.envs, cwd)
5167
.await
5268
}
5369

5470
/// Resolve the add command.
5571
#[must_use]
5672
pub fn resolve_add_command(&self, options: &AddCommandOptions) -> ResolveCommandResult {
73+
self.resolve_add_command_with_global_npm_bin(options, None)
74+
}
75+
76+
/// Resolve the add command with an explicit npm binary for global installs.
77+
#[must_use]
78+
pub fn resolve_add_command_with_global_npm_bin(
79+
&self,
80+
options: &AddCommandOptions,
81+
global_npm_bin_path: Option<&AbsolutePath>,
82+
) -> ResolveCommandResult {
5783
let bin_name: String;
5884
let envs = HashMap::from([("PATH".to_string(), format_path_env(self.get_bin_prefix()))]);
5985
let mut args: Vec<String> = Vec::new();
6086

6187
// global packages should use npm cli only
6288
if options.global {
63-
bin_name = "npm".into();
89+
bin_name = resolve_global_npm_bin_path(global_npm_bin_path);
6490
args.push("install".into());
6591
args.push("--global".into());
6692
if let Some(pass_through_args) = options.pass_through_args {
@@ -600,4 +626,24 @@ mod tests {
600626
assert_eq!(result.args, vec!["add", "--allow-build=react,napi", "react"]);
601627
assert_eq!(result.bin_path, "pnpm");
602628
}
629+
630+
#[test]
631+
fn test_global_add_uses_explicit_npm_bin() {
632+
let pm = create_mock_package_manager(PackageManagerType::Pnpm);
633+
let npm_bin = if cfg!(windows) {
634+
AbsolutePathBuf::new("C:\\node\\npm.cmd".into()).unwrap()
635+
} else {
636+
AbsolutePathBuf::new("/node/bin/npm".into()).unwrap()
637+
};
638+
let result = pm.resolve_add_command_with_global_npm_bin(
639+
&AddCommandOptions {
640+
packages: &["typescript".to_string()],
641+
global: true,
642+
..Default::default()
643+
},
644+
Some(&npm_bin),
645+
);
646+
assert_eq!(result.args, vec!["install", "--global", "typescript"]);
647+
assert_eq!(result.bin_path, npm_bin.as_path().display().to_string());
648+
}
603649
}

0 commit comments

Comments
 (0)