Skip to content

Commit de79b17

Browse files
authored
fix(cli): expose configured package manager to direct commands (#1696)
## Summary Direct built-in commands like `vp test --coverage` now expose the package manager declared in the workspace root `package.json#packageManager` to child processes. This allows tools such as Vitest coverage to spawn the configured package manager without failing with `spawn pnpm ENOENT`. ## Changes - Prepends the explicitly configured package manager bin directory to the direct command environment - Applies to direct built-ins such as `vp test`, `vp build`, `vp lint`, etc. - Uses only explicit `package.json#packageManager` Fixes #1690.
1 parent f0f63e3 commit de79b17

6 files changed

Lines changed: 249 additions & 1 deletion

File tree

packages/cli/binding/src/cli/mod.rs

Lines changed: 218 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ async fn execute_direct_subcommand(
6060
.map(|(k, v)| (Arc::from(k.as_os_str()), Arc::from(v.as_os_str())))
6161
.collect(),
6262
);
63+
let envs = envs_with_explicit_package_manager_path(cwd, envs).await?;
6364

6465
let status = match subcommand {
6566
SynthesizableSubcommand::Check {
@@ -113,6 +114,96 @@ async fn execute_direct_subcommand(
113114
Ok(status)
114115
}
115116

117+
fn is_path_env_key(key: &OsStr) -> bool {
118+
if cfg!(windows) { key.eq_ignore_ascii_case("PATH") } else { key == "PATH" }
119+
}
120+
121+
fn try_prepend_to_env_path(
122+
envs: &Arc<FxHashMap<Arc<OsStr>, Arc<OsStr>>>,
123+
bin_prefix: &AbsolutePath,
124+
) -> Result<Arc<FxHashMap<Arc<OsStr>, Arc<OsStr>>>, Error> {
125+
let path_key = envs
126+
.keys()
127+
.find(|key| is_path_env_key(key.as_ref()))
128+
.cloned()
129+
.unwrap_or_else(|| Arc::from(OsStr::new("PATH")));
130+
let current_path =
131+
envs.get(&path_key).map_or_else(Default::default, |path| path.to_os_string());
132+
let paths = if current_path.is_empty() {
133+
Vec::new()
134+
} else {
135+
env::split_paths(&current_path).collect::<Vec<_>>()
136+
};
137+
138+
if paths.first().is_some_and(|path| path == bin_prefix.as_path()) {
139+
return Ok(Arc::clone(envs));
140+
}
141+
142+
let new_path = env::join_paths(
143+
std::iter::once(bin_prefix.as_path().to_path_buf()).chain(paths.into_iter()),
144+
)
145+
.map_err(|error| Error::Anyhow(anyhow::Error::new(error)))?;
146+
147+
let mut envs = FxHashMap::clone(envs);
148+
envs.insert(path_key, Arc::from(new_path.as_os_str()));
149+
Ok(Arc::new(envs))
150+
}
151+
152+
fn prepend_to_env_path(
153+
envs: &Arc<FxHashMap<Arc<OsStr>, Arc<OsStr>>>,
154+
bin_prefix: &AbsolutePath,
155+
) -> Arc<FxHashMap<Arc<OsStr>, Arc<OsStr>>> {
156+
match try_prepend_to_env_path(envs, bin_prefix) {
157+
Ok(updated_envs) => updated_envs,
158+
Err(error) => {
159+
tracing::debug!(
160+
?error,
161+
"failed to prepend managed package manager bin to direct command PATH"
162+
);
163+
Arc::clone(envs)
164+
}
165+
}
166+
}
167+
168+
async fn envs_with_explicit_package_manager_path(
169+
cwd: &AbsolutePath,
170+
envs: Arc<FxHashMap<Arc<OsStr>, Arc<OsStr>>>,
171+
) -> Result<Arc<FxHashMap<Arc<OsStr>, Arc<OsStr>>>, Error> {
172+
let Some(resolution) =
173+
(match vite_install::package_manager::resolve_package_manager_from_package_json(cwd) {
174+
Ok(resolution) => resolution,
175+
Err(error) => {
176+
tracing::debug!(
177+
?error,
178+
"failed to resolve explicit packageManager for direct command PATH setup"
179+
);
180+
return Ok(envs);
181+
}
182+
})
183+
else {
184+
return Ok(envs);
185+
};
186+
187+
let (install_dir, _, _) = match vite_install::download_package_manager(
188+
resolution.package_manager_type,
189+
&resolution.version,
190+
resolution.hash.as_deref(),
191+
)
192+
.await
193+
{
194+
Ok(result) => result,
195+
Err(error) => {
196+
tracing::debug!(
197+
?error,
198+
"failed to ensure managed package manager for direct command PATH setup"
199+
);
200+
return Ok(envs);
201+
}
202+
};
203+
204+
Ok(prepend_to_env_path(&envs, &install_dir.join("bin")))
205+
}
206+
116207
/// Execute a vite-task command (run, cache) through Session.
117208
async fn execute_vite_task_command(
118209
command: vite_task::Command,
@@ -224,10 +315,136 @@ async fn execute_pm_command(
224315

225316
#[cfg(test)]
226317
mod tests {
227-
use std::path::PathBuf;
318+
use std::{
319+
ffi::OsStr,
320+
fs,
321+
path::PathBuf,
322+
sync::Arc,
323+
time::{SystemTime, UNIX_EPOCH},
324+
};
228325

326+
use rustc_hash::FxHashMap;
327+
use vite_path::AbsolutePathBuf;
229328
use vite_task::config::UserRunConfig;
230329

330+
use super::{envs_with_explicit_package_manager_path, prepend_to_env_path};
331+
332+
fn envs_with_path(path: &std::ffi::OsStr) -> Arc<FxHashMap<Arc<OsStr>, Arc<OsStr>>> {
333+
Arc::new(FxHashMap::from_iter([(Arc::from(OsStr::new("PATH")), Arc::from(path))]))
334+
}
335+
336+
#[test]
337+
fn prepends_package_manager_bin_to_env_path() {
338+
let cwd = std::env::current_dir().expect("current_dir should exist");
339+
let old_bin = cwd.join("old-bin");
340+
let pm_bin = AbsolutePathBuf::new(cwd.join("pm-bin")).expect("pm bin should be absolute");
341+
let original_path = std::env::join_paths([old_bin.as_path()]).expect("valid PATH");
342+
let envs = envs_with_path(original_path.as_os_str());
343+
344+
let updated = prepend_to_env_path(&envs, &pm_bin);
345+
let path_value = updated.get(OsStr::new("PATH")).expect("PATH should exist");
346+
let paths = std::env::split_paths(path_value).collect::<Vec<_>>();
347+
348+
assert_eq!(paths.first().map(std::path::PathBuf::as_path), Some(pm_bin.as_path()));
349+
assert_eq!(paths.get(1).map(std::path::PathBuf::as_path), Some(old_bin.as_path()));
350+
}
351+
352+
#[test]
353+
fn does_not_duplicate_package_manager_bin_when_already_first() {
354+
let cwd = std::env::current_dir().expect("current_dir should exist");
355+
let pm_bin = AbsolutePathBuf::new(cwd.join("pm-bin")).expect("pm bin should be absolute");
356+
let original_path = std::env::join_paths([pm_bin.as_path()]).expect("valid PATH");
357+
let envs = envs_with_path(original_path.as_os_str());
358+
359+
let updated = prepend_to_env_path(&envs, &pm_bin);
360+
let path_value = updated.get(OsStr::new("PATH")).expect("PATH should exist");
361+
let paths = std::env::split_paths(path_value).collect::<Vec<_>>();
362+
363+
assert_eq!(paths, vec![pm_bin.as_path().to_path_buf()]);
364+
}
365+
366+
#[test]
367+
fn creates_path_when_env_map_has_no_path() {
368+
let cwd = std::env::current_dir().expect("current_dir should exist");
369+
let pm_bin = AbsolutePathBuf::new(cwd.join("pm-bin")).expect("pm bin should be absolute");
370+
let envs = Arc::new(FxHashMap::default());
371+
372+
let updated = prepend_to_env_path(&envs, &pm_bin);
373+
let path_value = updated.get(OsStr::new("PATH")).expect("PATH should be created");
374+
let paths = std::env::split_paths(path_value).collect::<Vec<_>>();
375+
376+
assert_eq!(paths, vec![pm_bin.as_path().to_path_buf()]);
377+
}
378+
379+
#[test]
380+
fn preserves_path_key_casing_on_windows() {
381+
let cwd = std::env::current_dir().expect("current_dir should exist");
382+
let old_bin = cwd.join("old-bin");
383+
let pm_bin = AbsolutePathBuf::new(cwd.join("pm-bin")).expect("pm bin should be absolute");
384+
let original_path = std::env::join_paths([old_bin.as_path()]).expect("valid PATH");
385+
let key = if cfg!(windows) { "Path" } else { "PATH" };
386+
let envs = Arc::new(FxHashMap::from_iter([(
387+
Arc::from(OsStr::new(key)),
388+
Arc::from(original_path.as_os_str()),
389+
)]));
390+
391+
let updated = prepend_to_env_path(&envs, &pm_bin);
392+
let path_value = updated.get(OsStr::new(key)).expect("existing PATH key should be updated");
393+
let paths = std::env::split_paths(path_value).collect::<Vec<_>>();
394+
395+
assert_eq!(paths.first().map(std::path::PathBuf::as_path), Some(pm_bin.as_path()));
396+
assert_eq!(paths.get(1).map(std::path::PathBuf::as_path), Some(old_bin.as_path()));
397+
}
398+
399+
#[tokio::test]
400+
async fn ignores_invalid_explicit_package_manager() {
401+
let suffix =
402+
SystemTime::now().duration_since(UNIX_EPOCH).expect("time should be valid").as_nanos();
403+
let temp_dir = std::env::temp_dir().join(format!("vite-plus-invalid-pm-{suffix}"));
404+
fs::create_dir_all(&temp_dir).expect("temp dir should be created");
405+
fs::write(
406+
temp_dir.join("package.json"),
407+
r#"{"name":"fixture","packageManager":"unknown@1.0.0"}"#,
408+
)
409+
.expect("package.json should be written");
410+
let cwd = AbsolutePathBuf::new(temp_dir.clone()).expect("temp dir should be absolute");
411+
let original_path = std::env::join_paths([temp_dir.join("old-bin")]).expect("valid PATH");
412+
let envs = envs_with_path(original_path.as_os_str());
413+
414+
let updated = envs_with_explicit_package_manager_path(&cwd, Arc::clone(&envs))
415+
.await
416+
.expect("package manager preflight errors should not fail direct commands");
417+
418+
assert_eq!(updated.get(OsStr::new("PATH")), envs.get(OsStr::new("PATH")));
419+
fs::remove_dir_all(temp_dir).expect("temp dir should be removed");
420+
}
421+
422+
#[tokio::test]
423+
async fn ignores_lockfile_without_explicit_package_manager() {
424+
let suffix =
425+
SystemTime::now().duration_since(UNIX_EPOCH).expect("time should be valid").as_nanos();
426+
let temp_dir = std::env::temp_dir().join(format!("vite-plus-no-pm-{suffix}"));
427+
fs::create_dir_all(&temp_dir).expect("temp dir should be created");
428+
fs::write(temp_dir.join("package.json"), r#"{"name":"fixture"}"#)
429+
.expect("package.json should be written");
430+
fs::write(temp_dir.join("pnpm-lock.yaml"), "lockfileVersion: '9.0'\n")
431+
.expect("lockfile should be written");
432+
let cwd = AbsolutePathBuf::new(temp_dir.clone()).expect("temp dir should be absolute");
433+
let original_path = std::env::join_paths([temp_dir.join("old-bin")]).expect("valid PATH");
434+
let envs = envs_with_path(original_path.as_os_str());
435+
436+
let updated = envs_with_explicit_package_manager_path(&cwd, Arc::clone(&envs))
437+
.await
438+
.expect("missing packageManager should not error");
439+
440+
assert_eq!(updated.get(OsStr::new("PATH")), envs.get(OsStr::new("PATH")));
441+
assert_eq!(
442+
fs::read_to_string(temp_dir.join("package.json")).expect("package.json should exist"),
443+
r#"{"name":"fixture"}"#
444+
);
445+
fs::remove_dir_all(temp_dir).expect("temp dir should be removed");
446+
}
447+
231448
#[test]
232449
fn run_config_types_in_sync() {
233450
// Remove \r for cross-platform consistency
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"name": "test-managed-package-manager-path",
3+
"private": true,
4+
"type": "module",
5+
"packageManager": "pnpm@11.2.2"
6+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
> sh -c 'vp_bin=$(command -v vp) && node_path=$(command -v node) && node_bin=$(mktemp -d) && ln -s "$node_path" "$node_bin/node" && sanitized_path="$node_bin:/bin:/usr/bin" && PATH="$sanitized_path" node --version >/dev/null && if PATH="$sanitized_path" command -v pnpm >/dev/null; then echo "pnpm unexpectedly available on sanitized PATH"; exit 1; fi && PATH="$sanitized_path" "$vp_bin" test --slowTestThreshold 10000'
2+
RUN <cwd>
3+
4+
✓ src/managed-pm-path.test.ts (1 test) <variable>ms
5+
6+
Test Files 1 passed (1)
7+
Tests 1 passed (1)
8+
Start at <date>
9+
Duration <variable>ms (transform <variable>ms, setup <variable>ms, import <variable>ms, tests <variable>ms, environment <variable>ms)
10+
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import { execFileSync } from 'node:child_process';
2+
3+
import { expect, test } from '@voidzero-dev/vite-plus-test';
4+
5+
test('direct test command exposes the configured package manager on PATH', () => {
6+
const version = execFileSync('pnpm', ['--version'], { encoding: 'utf8' }).trim();
7+
expect(version).toBe('11.2.2');
8+
});
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"ignoredPlatforms": ["win32"],
3+
"commands": [
4+
"sh -c 'vp_bin=$(command -v vp) && node_path=$(command -v node) && node_bin=$(mktemp -d) && ln -s \"$node_path\" \"$node_bin/node\" && sanitized_path=\"$node_bin:/bin:/usr/bin\" && PATH=\"$sanitized_path\" node --version >/dev/null && if PATH=\"$sanitized_path\" command -v pnpm >/dev/null; then echo \"pnpm unexpectedly available on sanitized PATH\"; exit 1; fi && PATH=\"$sanitized_path\" \"$vp_bin\" test --slowTestThreshold 10000'"
5+
]
6+
}

vite.config.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ export default defineConfig({
5555
'./rolldown/**',
5656
'**/node_modules/**',
5757
'**/snap-tests/**',
58+
'**/snap-tests-global/**',
5859
// FIXME: Error: failed to prepare the command for injection: Invalid argument (os error 22)
5960
'packages/*/binding/__tests__/',
6061
],

0 commit comments

Comments
 (0)