Skip to content

Commit 06da550

Browse files
committed
fix(install): persist Windows PATH independently of shell setup
1 parent f7e75e2 commit 06da550

10 files changed

Lines changed: 408 additions & 32 deletions

File tree

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
[[case]]
2+
name = "command_env_doctor_missing_fallback"
3+
vp = "global"
4+
steps = [
5+
{ argv = ["node", "verify.mjs"], comment = "Doctor names the missing fallback directory even when the main shim directory is on PATH." },
6+
]
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
# command_env_doctor_missing_fallback
2+
3+
## `node verify.mjs`
4+
5+
Doctor names the missing fallback directory even when the main shim directory is on PATH.
6+
7+
```
8+
✗ Fallback dir not in PATH
9+
note: Expected: <home>/.vite-plus/fallback-bin
10+
```
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import assert from "node:assert/strict";
2+
import { spawnSync } from "node:child_process";
3+
import path from "node:path";
4+
5+
const bin = path.join(process.env.VP_HOME, "bin");
6+
const result = spawnSync(
7+
path.join(bin, process.platform === "win32" ? "vp.exe" : "vp"),
8+
["env", "doctor", "node"],
9+
{
10+
encoding: "utf8",
11+
env: { ...process.env, PATH: bin },
12+
},
13+
);
14+
assert.equal(result.status, 1, `${result.stdout}\n${result.stderr}`);
15+
const lines = result.stdout.replace(/\u001B\[[0-9;]*m/g, "").split(/\r?\n/);
16+
const index = lines.findIndex((line) => line.includes("Fallback dir"));
17+
assert.ok(index >= 0, result.stdout);
18+
assert.match(lines[index], /not in PATH/);
19+
assert.match(lines[index + 1], /note: Expected: .*[/\\]fallback-bin$/);
20+
console.log(lines.slice(index, index + 2).join("\n"));

‎crates/vp_global_cli/Cargo.toml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ vt_str = { workspace = true }
5050
vt_workspace = { workspace = true }
5151
uuid = { workspace = true, features = ["v4"] }
5252

53+
[target.'cfg(windows)'.dependencies]
54+
winreg = { workspace = true }
55+
5356
[dev-dependencies]
5457
serial_test = { workspace = true }
5558
temp-env = { workspace = true }

‎crates/vp_global_cli/src/commands/env/doctor.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -560,6 +560,10 @@ async fn check_path(scope: EnvScope) -> bool {
560560
let fallback = vp_shared::EnvConfig::get().dirs.fallback_bin();
561561
if !paths.iter().any(|path| path == fallback.as_path()) {
562562
print_check(&style(output::CROSS).red().to_string(), "Fallback dir", "not in PATH");
563+
print_hint(&format!(
564+
"Expected: {}",
565+
abbreviate_home(&fallback.as_path().display().to_string())
566+
));
563567
print_path_fix(&vp_shared::EnvConfig::get().dirs.config);
564568
return false;
565569
}

‎crates/vp_global_cli/src/self_setup.rs‎

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22
33
mod external;
44
mod shell;
5+
#[cfg(windows)]
6+
mod windows_path;
7+
#[cfg(all(test, windows))]
8+
mod windows_tests;
59

610
use std::{path::Path, process::ExitCode};
711

@@ -274,14 +278,7 @@ async fn run(source: &Path, bundled: bool) -> Result<AbsolutePathBuf, Error> {
274278
if deploy {
275279
remove_previous_install(previous_install.as_deref()).await?;
276280
}
277-
if std::env::var(env_vars::VP_SELF_SETUP_NO_MODIFY_PATH).as_deref() != Ok("1") {
278-
if let Err(error) = shell::configure().await {
279-
output::warn(&format!(
280-
"Could not configure shell profiles: {error}. Add {} to PATH manually.",
281-
dirs.bin.as_path().display()
282-
));
283-
}
284-
}
281+
configure_shells().await?;
285282
let mut settings = config::load_config().await?;
286283
if let Some(mode) = node_override.or(default_mode) {
287284
settings.node_shim_mode = mode;
@@ -302,8 +299,8 @@ async fn run(source: &Path, bundled: bool) -> Result<AbsolutePathBuf, Error> {
302299

303300
// Existing Windows installs also need the fallback directory in their persistent PATH.
304301
#[cfg(windows)]
305-
if in_place && std::env::var(env_vars::VP_SELF_SETUP_NO_MODIFY_PATH).as_deref() != Ok("1") {
306-
shell::configure().await?;
302+
if in_place {
303+
configure_shells().await?;
307304
}
308305

309306
// 2. Activate a standalone download; an upgrade hook must not overwrite rollback history.
@@ -345,6 +342,28 @@ async fn run(source: &Path, bundled: bool) -> Result<AbsolutePathBuf, Error> {
345342
Ok(binary)
346343
}
347344

345+
async fn configure_shells() -> Result<(), Error> {
346+
if std::env::var(env_vars::VP_SELF_SETUP_NO_MODIFY_PATH).as_deref() == Ok("1") {
347+
return Ok(());
348+
}
349+
let dirs = &EnvConfig::get().dirs;
350+
// Windows PATH is required even when an optional shell (such as an older
351+
// Nushell) cannot be configured. Do not mark setup complete if this fails.
352+
#[cfg(windows)]
353+
windows_path::configure(&dirs.bin, &dirs.fallback_bin())
354+
.map_err(|error| Error::Other(format!("Could not configure user PATH: {error}").into()))?;
355+
if let Err(error) = shell::configure().await {
356+
#[cfg(windows)]
357+
output::warn(&format!("Could not configure shell profiles: {error}."));
358+
#[cfg(not(windows))]
359+
output::warn(&format!(
360+
"Could not configure shell profiles: {error}. Add {} to PATH manually.",
361+
dirs.bin.as_path().display()
362+
));
363+
}
364+
Ok(())
365+
}
366+
348367
fn interactive() -> bool {
349368
std::env::var_os("CI").is_none() && vp_shared::is_stderr_terminal()
350369
}

‎crates/vp_global_cli/src/self_setup/shell.rs‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -90,27 +90,5 @@ pub(super) async fn configure() -> Result<(), Error> {
9090
}
9191
}
9292
}
93-
#[cfg(windows)]
94-
{
95-
let bin = setup::escape_powershell_single_quoted_string(&config.dirs.bin.to_string());
96-
let fallback =
97-
setup::escape_powershell_single_quoted_string(&config.dirs.fallback_bin().to_string());
98-
let script = format!(
99-
"$bin = '{bin}'; $fallback = '{fallback}'; $path = @([Environment]::GetEnvironmentVariable('Path', 'User') -split ';' | Where-Object {{ $_ -and $_ -ne $bin -and $_ -ne $fallback }}); [Environment]::SetEnvironmentVariable('Path', ((@($bin) + $path + @($fallback)) -join ';'), 'User')"
100-
);
101-
let result = tokio::process::Command::new("powershell")
102-
.args(["-NoProfile", "-NonInteractive", "-Command", &script])
103-
.output()
104-
.await?;
105-
if !result.status.success() {
106-
return Err(Error::Other(
107-
format!(
108-
"Could not configure user PATH: {}",
109-
String::from_utf8_lossy(&result.stderr)
110-
)
111-
.into(),
112-
));
113-
}
114-
}
11593
Ok(())
11694
}
Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
//! Persist Windows PATH without depending on an installed shell.
2+
3+
use std::io;
4+
5+
use vt_path::AbsolutePath;
6+
use winreg::{
7+
RegKey, RegValue,
8+
enums::{HKEY_CURRENT_USER, KEY_READ, KEY_WRITE, REG_EXPAND_SZ, REG_SZ},
9+
};
10+
11+
pub(super) fn configure(bin: &AbsolutePath, fallback: &AbsolutePath) -> io::Result<()> {
12+
let hkcu = RegKey::predef(HKEY_CURRENT_USER);
13+
let (environment, _) = hkcu.create_subkey_with_flags("Environment", KEY_READ | KEY_WRITE)?;
14+
if update_path(&environment, &bin.to_string(), &fallback.to_string())? {
15+
broadcast_settings_change();
16+
}
17+
Ok(())
18+
}
19+
20+
// Accept an open key so tests never need to change the real user environment.
21+
fn update_path(environment: &RegKey, bin: &str, fallback: &str) -> io::Result<bool> {
22+
let original = match environment.get_raw_value("Path") {
23+
Ok(value) => Some(value),
24+
Err(error) if error.kind() == io::ErrorKind::NotFound => None,
25+
Err(error) => return Err(error),
26+
};
27+
let (current, kind) = match &original {
28+
Some(value) => (decode_path(value)?, value.vtype.clone()),
29+
None => (String::new(), REG_EXPAND_SZ),
30+
};
31+
let matches = |entry: &str, directory: &str| {
32+
entry
33+
.trim_end_matches(['\\', '/'])
34+
.eq_ignore_ascii_case(directory.trim_end_matches(['\\', '/']))
35+
};
36+
let mut entries = vec![bin];
37+
if !current.is_empty() {
38+
entries.extend(
39+
current.split(';').filter(|entry| !matches(entry, bin) && !matches(entry, fallback)),
40+
);
41+
}
42+
entries.push(fallback);
43+
let updated = RegValue {
44+
vtype: kind,
45+
bytes: entries.join(";").encode_utf16().chain([0]).flat_map(u16::to_le_bytes).collect(),
46+
};
47+
if original.as_ref() == Some(&updated) {
48+
return Ok(false);
49+
}
50+
environment.set_raw_value("Path", &updated)?;
51+
Ok(true)
52+
}
53+
54+
fn decode_path(value: &RegValue) -> io::Result<String> {
55+
if !matches!(value.vtype, REG_SZ | REG_EXPAND_SZ) || !value.bytes.len().is_multiple_of(2) {
56+
return Err(io::Error::new(io::ErrorKind::InvalidData, "Invalid user PATH registry value"));
57+
}
58+
let mut units: Vec<_> =
59+
value.bytes.as_chunks::<2>().0.iter().copied().map(u16::from_le_bytes).collect();
60+
while units.last() == Some(&0) {
61+
units.pop();
62+
}
63+
if units.contains(&0) {
64+
return Err(io::Error::new(io::ErrorKind::InvalidData, "User PATH contains a NUL"));
65+
}
66+
String::from_utf16(&units).map_err(|error| io::Error::new(io::ErrorKind::InvalidData, error))
67+
}
68+
69+
/// Notify Explorer and other applications that future processes need the new PATH.
70+
fn broadcast_settings_change() {
71+
#[link(name = "user32")]
72+
unsafe extern "system" {
73+
fn SendMessageTimeoutW(
74+
window: isize,
75+
message: u32,
76+
wparam: usize,
77+
lparam: isize,
78+
flags: u32,
79+
timeout: u32,
80+
result: *mut usize,
81+
) -> isize;
82+
}
83+
let environment: Vec<u16> = "Environment".encode_utf16().chain([0]).collect();
84+
// HWND_BROADCAST, WM_SETTINGCHANGE, SMTO_ABORTIFHUNG.
85+
// The string remains live until this synchronous call returns.
86+
unsafe {
87+
SendMessageTimeoutW(
88+
0xffff,
89+
0x001a,
90+
0,
91+
environment.as_ptr() as isize,
92+
0x0002,
93+
5000,
94+
std::ptr::null_mut(),
95+
);
96+
}
97+
}
98+
99+
#[cfg(test)]
100+
mod tests {
101+
use winreg::enums::{REG_BINARY, REG_MULTI_SZ};
102+
103+
use super::*;
104+
105+
struct TestKey {
106+
key: RegKey,
107+
path: String,
108+
}
109+
110+
impl TestKey {
111+
fn new() -> Self {
112+
let path = format!("Software\\VitePlusTests\\{}", uuid::Uuid::new_v4());
113+
let (key, _) = RegKey::predef(HKEY_CURRENT_USER).create_subkey(&path).unwrap();
114+
Self { key, path }
115+
}
116+
}
117+
118+
impl Drop for TestKey {
119+
fn drop(&mut self) {
120+
RegKey::predef(HKEY_CURRENT_USER).delete_subkey_all(&self.path).unwrap();
121+
}
122+
}
123+
124+
fn value(path: &str, kind: winreg::enums::RegType) -> RegValue<'static> {
125+
RegValue {
126+
vtype: kind,
127+
bytes: path.encode_utf16().chain([0]).flat_map(u16::to_le_bytes).collect(),
128+
}
129+
}
130+
131+
#[test]
132+
fn preserves_variables_empty_entries_and_registry_type() {
133+
let key = TestKey::new();
134+
for kind in [REG_EXPAND_SZ, REG_SZ] {
135+
let before = "%PNPM_HOME%;;C:\\工具\\bin";
136+
key.key.set_raw_value("Path", &value(before, kind.clone())).unwrap();
137+
assert!(update_path(&key.key, r"C:\Vite+\bin", r"C:\Vite+\fallback-bin").unwrap());
138+
assert_eq!(
139+
key.key.get_raw_value("Path").unwrap(),
140+
value(&format!("C:\\Vite+\\bin;{before};C:\\Vite+\\fallback-bin"), kind)
141+
);
142+
assert!(!update_path(&key.key, r"C:\Vite+\bin", r"C:\Vite+\fallback-bin").unwrap());
143+
}
144+
}
145+
146+
#[test]
147+
fn creates_missing_path_and_handles_empty_path() {
148+
let key = TestKey::new();
149+
for initial in [None, Some(value("", REG_EXPAND_SZ))] {
150+
if let Some(initial) = initial {
151+
key.key.set_raw_value("Path", &initial).unwrap();
152+
}
153+
update_path(&key.key, r"C:\bin", r"C:\fallback").unwrap();
154+
assert_eq!(
155+
key.key.get_raw_value("Path").unwrap(),
156+
value(r"C:\bin;C:\fallback", REG_EXPAND_SZ)
157+
);
158+
}
159+
}
160+
161+
#[test]
162+
fn moves_existing_entries_to_opposite_ends_without_duplicates() {
163+
let key = TestKey::new();
164+
key.key.set_value("Path", &r"C:\tools;c:\BIN\;C:\fallback;C:\bin;C:\other").unwrap();
165+
update_path(&key.key, r"C:\bin", r"C:\fallback").unwrap();
166+
assert_eq!(
167+
key.key.get_value::<String, _>("Path").unwrap(),
168+
r"C:\bin;C:\tools;C:\other;C:\fallback"
169+
);
170+
}
171+
172+
#[test]
173+
fn refuses_invalid_values_without_overwriting_them() {
174+
let key = TestKey::new();
175+
for original in [
176+
RegValue { vtype: REG_BINARY, bytes: vec![1, 2].into() },
177+
value("first\0second", REG_MULTI_SZ),
178+
RegValue { vtype: REG_SZ, bytes: vec![1].into() },
179+
RegValue { vtype: REG_SZ, bytes: vec![0, 0xd8, 0, 0].into() },
180+
value("first\0second", REG_SZ),
181+
] {
182+
key.key.set_raw_value("Path", &original).unwrap();
183+
assert_eq!(
184+
update_path(&key.key, r"C:\bin", r"C:\fallback").unwrap_err().kind(),
185+
io::ErrorKind::InvalidData
186+
);
187+
assert_eq!(key.key.get_raw_value("Path").unwrap(), original);
188+
}
189+
}
190+
191+
#[test]
192+
fn propagates_write_errors() {
193+
let key = TestKey::new();
194+
key.key.set_value("Path", &r"C:\tools").unwrap();
195+
let readonly =
196+
RegKey::predef(HKEY_CURRENT_USER).open_subkey_with_flags(&key.path, KEY_READ).unwrap();
197+
assert_eq!(
198+
update_path(&readonly, r"C:\bin", r"C:\fallback").unwrap_err().kind(),
199+
io::ErrorKind::PermissionDenied
200+
);
201+
assert_eq!(key.key.get_value::<String, _>("Path").unwrap(), r"C:\tools");
202+
}
203+
}

0 commit comments

Comments
 (0)