diff --git a/crates/vite_global_cli/src/commands/global/install.rs b/crates/vite_global_cli/src/commands/global/install.rs index c37f71d2b2..c998dd5ab2 100644 --- a/crates/vite_global_cli/src/commands/global/install.rs +++ b/crates/vite_global_cli/src/commands/global/install.rs @@ -3,8 +3,8 @@ use std::{ collections::{HashMap, HashSet}, io::{IsTerminal, Read, Write}, - process::Stdio, - time::Duration, + process::{self, Stdio}, + time::{Duration, SystemTime, UNIX_EPOCH}, }; use futures::{StreamExt, stream::FuturesUnordered}; @@ -338,12 +338,7 @@ pub async fn install( // 4.5 Commit the install by discarding the backup and reporting the installed bins. if let Some(backup) = backup { - if let Err(error) = backup.discard().await { - if first_error.is_none() { - first_error = Some(package_error(&package_name, error)); - } - continue; - } + backup.discard().await; } // 4.6 Print success message @@ -461,8 +456,7 @@ impl PackageBackup { return Ok(None); } - let backup_dir = get_tmp_dir()?.join("packages").join(package_name); - remove_dir_all_if_exists(&backup_dir).await?; + let backup_dir = unique_backup_dir(package_name)?; if let Some(parent) = backup_dir.parent() { tokio::fs::create_dir_all(parent).await?; } @@ -487,11 +481,31 @@ impl PackageBackup { Ok(()) } - async fn discard(self) -> Result<(), Error> { - remove_dir_all_if_exists(&self.backup_dir).await + async fn discard(self) { + if let Err(error) = remove_dir_all_if_exists(&self.backup_dir).await { + tracing::warn!( + "Failed to remove old global package backup at {}: {}", + self.backup_dir.as_path().display(), + error + ); + } } } +fn unique_backup_dir(package_name: &str) -> Result { + let base = get_tmp_dir()?.join("packages").join(package_name); + let package_dir_name = + base.as_path().file_name().and_then(|name| name.to_str()).unwrap_or("package"); + let timestamp = SystemTime::now().duration_since(UNIX_EPOCH).unwrap_or_default().as_nanos(); + let backup_name = format!("{package_dir_name}.{}.{}.old", process::id(), timestamp); + + let mut backup_path = base.as_path().to_path_buf(); + backup_path.set_file_name(backup_name); + + AbsolutePathBuf::new(backup_path) + .ok_or_else(|| Error::ConfigError("Invalid global package backup path".into())) +} + async fn cleanup_failed_install( package_name: &str, backup: Option, @@ -1022,6 +1036,48 @@ mod tests { } } + #[tokio::test] + async fn test_package_backup_uses_unique_tmp_dir_for_scoped_package() { + use tempfile::TempDir; + use vite_path::AbsolutePathBuf; + + let temp_dir = TempDir::new().unwrap(); + let temp_path = temp_dir.path().to_path_buf(); + let _env_guard = vite_shared::EnvConfig::test_guard( + vite_shared::EnvConfig::for_test_with_home(&temp_path), + ); + + let package_dir = + AbsolutePathBuf::new(temp_path.join("packages").join("@scope").join("pkg")).unwrap(); + tokio::fs::create_dir_all(&package_dir).await.unwrap(); + tokio::fs::write(package_dir.join("marker").as_path(), "current").await.unwrap(); + + let stale_backup = + AbsolutePathBuf::new(temp_path.join("tmp").join("packages").join("@scope").join("pkg")) + .unwrap(); + tokio::fs::create_dir_all(&stale_backup).await.unwrap(); + tokio::fs::write(stale_backup.join("stale").as_path(), "locked").await.unwrap(); + + let backup = PackageBackup::create("@scope/pkg", &package_dir) + .await + .unwrap() + .expect("existing package should be backed up"); + + assert_ne!(backup.backup_dir.as_path(), stale_backup.as_path()); + assert!( + stale_backup.join("stale").as_path().exists(), + "stale fixed backup should be left untouched" + ); + assert!( + backup.backup_dir.join("marker").as_path().exists(), + "current package should be moved into the unique backup" + ); + assert!( + !package_dir.as_path().exists(), + "original package directory should be moved out before reinstall" + ); + } + #[test] fn test_is_local_package_spec_relative_paths() { assert!(is_local_package_spec(".")); diff --git a/packages/cli/snap-tests-global/env-install-stale-backup/.node-version b/packages/cli/snap-tests-global/env-install-stale-backup/.node-version new file mode 100644 index 0000000000..85e502778f --- /dev/null +++ b/packages/cli/snap-tests-global/env-install-stale-backup/.node-version @@ -0,0 +1 @@ +22.22.0 diff --git a/packages/cli/snap-tests-global/env-install-stale-backup/env-install-stale-backup-pkg/cli.js b/packages/cli/snap-tests-global/env-install-stale-backup/env-install-stale-backup-pkg/cli.js new file mode 100644 index 0000000000..e801213849 --- /dev/null +++ b/packages/cli/snap-tests-global/env-install-stale-backup/env-install-stale-backup-pkg/cli.js @@ -0,0 +1,2 @@ +#!/usr/bin/env node +console.log('env-install-stale-backup-cli'); diff --git a/packages/cli/snap-tests-global/env-install-stale-backup/env-install-stale-backup-pkg/package.json b/packages/cli/snap-tests-global/env-install-stale-backup/env-install-stale-backup-pkg/package.json new file mode 100644 index 0000000000..766eb36c7c --- /dev/null +++ b/packages/cli/snap-tests-global/env-install-stale-backup/env-install-stale-backup-pkg/package.json @@ -0,0 +1,7 @@ +{ + "name": "@scope/env-install-stale-backup-pkg", + "version": "1.0.0", + "bin": { + "env-install-stale-backup-cli": "./cli.js" + } +} diff --git a/packages/cli/snap-tests-global/env-install-stale-backup/snap.txt b/packages/cli/snap-tests-global/env-install-stale-backup/snap.txt new file mode 100644 index 0000000000..d017b22867 --- /dev/null +++ b/packages/cli/snap-tests-global/env-install-stale-backup/snap.txt @@ -0,0 +1,26 @@ +> vp install -g ./env-install-stale-backup-pkg # Install scoped package globally +info: Installing 1 global package with Node.js +✓ Installed @scope/env-install-stale-backup-pkg + Bins: env-install-stale-backup-cli + +> node -e "const fs = require('fs'); const path = require('path'); const cp = require('child_process'); const dir = path.join(process.env.VP_HOME, 'tmp/packages/@scope/env-install-stale-backup-pkg'); fs.mkdirSync(dir, { recursive: true }); const exe = path.join(dir, 'locked-node.exe'); fs.copyFileSync(process.execPath, exe); fs.writeFileSync(path.join(dir, 'stale.txt'), 'stale'); const child = cp.spawn(exe, ['-e', 'setTimeout(() => {}, 60000)'], { detached: true, stdio: 'ignore' }); child.unref(); fs.writeFileSync(path.join(dir, 'pid.txt'), String(child.pid));" # Seed stale locked fixed backup path +> vp install -g ./env-install-stale-backup-pkg # Reinstall should ignore stale backup +info: Installing 1 global package with Node.js +✓ Installed @scope/env-install-stale-backup-pkg + Bins: env-install-stale-backup-cli + +> node -e "const fs = require('fs'); const path = require('path'); console.log(fs.readFileSync(path.join(process.env.VP_HOME, 'tmp/packages/@scope/env-install-stale-backup-pkg/stale.txt'), 'utf8'));" # Stale backup should be untouched +stale + +> node -e "const fs = require('fs'); const path = require('path'); console.log(fs.readFileSync(path.join(process.env.VP_HOME, 'bins/env-install-stale-backup-cli.json'), 'utf8'));" # Bin config should still point to package +{ + "name": "env-install-stale-backup-cli", + "package": "@scope/env-install-stale-backup-pkg", + "version": "1.0.0", + "nodeVersion": "22.22.0", + "source": "vp" +} + +> node -e "const fs = require('fs'); const path = require('path'); const pidPath = path.join(process.env.VP_HOME, 'tmp/packages/@scope/env-install-stale-backup-pkg/pid.txt'); try { process.kill(Number(fs.readFileSync(pidPath, 'utf8')), 'SIGTERM'); } catch {}" # Stop stale backup lock process +> vp remove -g @scope/env-install-stale-backup-pkg # Cleanup +Uninstalled @scope/env-install-stale-backup-pkg diff --git a/packages/cli/snap-tests-global/env-install-stale-backup/steps.json b/packages/cli/snap-tests-global/env-install-stale-backup/steps.json new file mode 100644 index 0000000000..d947e688d8 --- /dev/null +++ b/packages/cli/snap-tests-global/env-install-stale-backup/steps.json @@ -0,0 +1,13 @@ +{ + "env": {}, + "ignoredPlatforms": ["linux", "darwin"], + "commands": [ + "vp install -g ./env-install-stale-backup-pkg # Install scoped package globally", + "node -e \"const fs = require('fs'); const path = require('path'); const cp = require('child_process'); const dir = path.join(process.env.VP_HOME, 'tmp/packages/@scope/env-install-stale-backup-pkg'); fs.mkdirSync(dir, { recursive: true }); const exe = path.join(dir, 'locked-node.exe'); fs.copyFileSync(process.execPath, exe); fs.writeFileSync(path.join(dir, 'stale.txt'), 'stale'); const child = cp.spawn(exe, ['-e', 'setTimeout(() => {}, 60000)'], { detached: true, stdio: 'ignore' }); child.unref(); fs.writeFileSync(path.join(dir, 'pid.txt'), String(child.pid));\" # Seed stale locked fixed backup path", + "vp install -g ./env-install-stale-backup-pkg # Reinstall should ignore stale backup", + "node -e \"const fs = require('fs'); const path = require('path'); console.log(fs.readFileSync(path.join(process.env.VP_HOME, 'tmp/packages/@scope/env-install-stale-backup-pkg/stale.txt'), 'utf8'));\" # Stale backup should be untouched", + "node -e \"const fs = require('fs'); const path = require('path'); console.log(fs.readFileSync(path.join(process.env.VP_HOME, 'bins/env-install-stale-backup-cli.json'), 'utf8'));\" # Bin config should still point to package", + "node -e \"const fs = require('fs'); const path = require('path'); const pidPath = path.join(process.env.VP_HOME, 'tmp/packages/@scope/env-install-stale-backup-pkg/pid.txt'); try { process.kill(Number(fs.readFileSync(pidPath, 'utf8')), 'SIGTERM'); } catch {}\" # Stop stale backup lock process", + "vp remove -g @scope/env-install-stale-backup-pkg # Cleanup" + ] +}