diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_unowned_directory.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_unowned_directory.md index 3b2d533636..2e3dd9b5c7 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_unowned_directory.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_implode/snapshots/implode_unowned_directory.md @@ -58,6 +58,7 @@ Only the newly owned cache is removed; the unmarked parent is preserved ✓ Vite+ removed its verified files and shell entries from your system. note: Some paths were preserved to protect unverified data. Inspect them before removing anything: + /Documents/.install.lock /Documents/0.0.0-test /Documents/bin /Documents/config.json diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_rollback/mock-manifest.json b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_rollback/mock-manifest.json new file mode 100644 index 0000000000..985af23f80 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_rollback/mock-manifest.json @@ -0,0 +1,88 @@ +{ + "vite-plus/999.0.1": { + "name": "vite-plus", + "version": "999.0.1", + "dist": { + "tarball": "unused", + "integrity": "sha512-unused" + } + }, + "@voidzero-dev/vite-plus-cli-darwin-arm64/999.0.1": { + "name": "@voidzero-dev/vite-plus-cli-darwin-arm64", + "version": "999.0.1", + "dist": { + "tarball": "{REGISTRY}/@voidzero-dev/vite-plus-cli-darwin-arm64/-/vite-plus-cli-999.0.1.tgz", + "integrity": "sha512-up15UEnVySULq4e34BEg8s9e2A6VLyjwUZrD++6PWXpfwFwkkysfTM21sIsx5mKwwHlfpRRO43C9zySvPMMTCQ==", + "attestations": { + "provenance": { + "predicateType": "https://slsa.dev/provenance/v1" + } + } + } + }, + "@voidzero-dev/vite-plus-cli-darwin-x64/999.0.1": { + "name": "@voidzero-dev/vite-plus-cli-darwin-x64", + "version": "999.0.1", + "dist": { + "tarball": "{REGISTRY}/@voidzero-dev/vite-plus-cli-darwin-x64/-/vite-plus-cli-999.0.1.tgz", + "integrity": "sha512-up15UEnVySULq4e34BEg8s9e2A6VLyjwUZrD++6PWXpfwFwkkysfTM21sIsx5mKwwHlfpRRO43C9zySvPMMTCQ==", + "attestations": { + "provenance": { + "predicateType": "https://slsa.dev/provenance/v1" + } + } + } + }, + "@voidzero-dev/vite-plus-cli-linux-arm64-gnu/999.0.1": { + "name": "@voidzero-dev/vite-plus-cli-linux-arm64-gnu", + "version": "999.0.1", + "dist": { + "tarball": "{REGISTRY}/@voidzero-dev/vite-plus-cli-linux-arm64-gnu/-/vite-plus-cli-999.0.1.tgz", + "integrity": "sha512-up15UEnVySULq4e34BEg8s9e2A6VLyjwUZrD++6PWXpfwFwkkysfTM21sIsx5mKwwHlfpRRO43C9zySvPMMTCQ==", + "attestations": { + "provenance": { + "predicateType": "https://slsa.dev/provenance/v1" + } + } + } + }, + "@voidzero-dev/vite-plus-cli-linux-x64-gnu/999.0.1": { + "name": "@voidzero-dev/vite-plus-cli-linux-x64-gnu", + "version": "999.0.1", + "dist": { + "tarball": "{REGISTRY}/@voidzero-dev/vite-plus-cli-linux-x64-gnu/-/vite-plus-cli-999.0.1.tgz", + "integrity": "sha512-up15UEnVySULq4e34BEg8s9e2A6VLyjwUZrD++6PWXpfwFwkkysfTM21sIsx5mKwwHlfpRRO43C9zySvPMMTCQ==", + "attestations": { + "provenance": { + "predicateType": "https://slsa.dev/provenance/v1" + } + } + } + }, + "@voidzero-dev/vite-plus-cli-linux-arm64-musl/999.0.1": { + "name": "@voidzero-dev/vite-plus-cli-linux-arm64-musl", + "version": "999.0.1", + "dist": { + "tarball": "{REGISTRY}/@voidzero-dev/vite-plus-cli-linux-arm64-musl/-/vite-plus-cli-999.0.1.tgz", + "integrity": "sha512-up15UEnVySULq4e34BEg8s9e2A6VLyjwUZrD++6PWXpfwFwkkysfTM21sIsx5mKwwHlfpRRO43C9zySvPMMTCQ==", + "attestations": { + "provenance": { + "predicateType": "https://slsa.dev/provenance/v1" + } + } + } + }, + "@voidzero-dev/vite-plus-cli-linux-x64-musl/999.0.1": { + "name": "@voidzero-dev/vite-plus-cli-linux-x64-musl", + "version": "999.0.1", + "dist": { + "tarball": "{REGISTRY}/@voidzero-dev/vite-plus-cli-linux-x64-musl/-/vite-plus-cli-999.0.1.tgz", + "integrity": "sha512-up15UEnVySULq4e34BEg8s9e2A6VLyjwUZrD++6PWXpfwFwkkysfTM21sIsx5mKwwHlfpRRO43C9zySvPMMTCQ==", + "attestations": { + "provenance": { + "predicateType": "https://slsa.dev/provenance/v1" + } + } + } + } +} diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_rollback/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_rollback/snapshots.toml index a5efec52e6..053824f0ff 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_rollback/snapshots.toml +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_rollback/snapshots.toml @@ -5,3 +5,53 @@ skip-platforms = ["windows"] steps = [ { argv = ["vp", "upgrade", "--rollback"], comment = "should fail with no previous version", continue-on-failure = true }, ] + +[[case]] +name = "command_upgrade_rollback_deployed" +vp = "global" +skip-platforms = ["windows"] +comment = "A rollback holds the installation lock while an unmarked target completes in-place setup, so the child must not take that lock." +env = { VP_SKIP_DEPS_INSTALL = "1", VP_NODE_MANAGER = "no", VP_SELF_SETUP_NO_MODIFY_PATH = "1" } +steps = [ + { argv = ["vpt", "mkdir", "external"], snapshot = false }, + { argv = ["vpt", "cp", "$VP_HOME/bin/vp", "external/vp"], snapshot = false }, + { argv = ["vpt", "chmod", "+x", "external/vp"], snapshot = false }, + { argv = ["./external/vp"], tty = false, envs = [["VP_HOME", "${workspace}/home"], ["VP_VERSION", "rollback-a"]], comment = "Deploy the first version", snapshot = false }, + { argv = ["./external/vp"], tty = false, envs = [["VP_HOME", "${workspace}/home"], ["VP_VERSION", "rollback-b"]], comment = "Deploy the second version, which records the first as the rollback target", snapshot = false }, + ["vpt", "print-file", "home/.previous-version"], + { argv = ["vpt", "rm", "home/rollback-a/bin/.vp-setup-complete"], comment = "The rollback target must run in-place setup during the shim refresh", snapshot = false }, + # The progress line prints the running CLI version, which changes on every release. + { argv = ["vp", "upgrade", "--rollback"], envs = [["VP_HOME", "${workspace}/home"]], comment = "A deadlock fails this step by timeout", timeout = 60000, snapshot = false }, + ["vpt", "print-file", "home/.previous-version"], + ["vpt", "stat-file", "home/rollback-a/bin/.vp-setup-complete", "--assert", "file"], +] + +[[case]] +name = "command_upgrade_failed_reinstall_keeps_existing_version" +vp = "global" +skip-platforms = ["windows"] +comment = "A failed upgrade into an existing version directory keeps that directory, which can be the rollback target." +# mock-manifest.json and tarballs/ serve a local 999.0.1 payload with a valid integrity. +steps = [ + { argv = ["vpt", "write-file", "$VP_HOME/999.0.1/keep.txt", "rollback target\n"], snapshot = false }, + { argv = ["vpt", "write-file", "$VP_HOME/999.0.1/bin", "not a directory\n"], comment = "Make extraction fail after the download is verified", snapshot = false }, + # `--silent` keeps the platform suffix out of the snapshot. + { argv = ["vp", "upgrade", "999.0.1", "--silent"], continue-on-failure = true }, + ["vpt", "print-file", "$VP_HOME/999.0.1/keep.txt"], +] + +[[case]] +name = "command_upgrade_repairs_broken_active_version" +vp = "global" +skip-platforms = ["windows"] +comment = "An upgrade to the version that `current` names still installs it when the active binary is missing." +env = { VP_SKIP_DEPS_INSTALL = "1", VP_NODE_MANAGER = "no", VP_SELF_SETUP_NO_MODIFY_PATH = "1" } +steps = [ + { argv = ["vpt", "mkdir", "external"], snapshot = false }, + { argv = ["vpt", "cp", "$VP_HOME/bin/vp", "external/vp"], snapshot = false }, + { argv = ["vpt", "chmod", "+x", "external/vp"], snapshot = false }, + { argv = ["./external/vp"], tty = false, envs = [["VP_HOME", "${workspace}/home"], ["VP_VERSION", "999.0.1"]], comment = "Activate a 999.0.1 directory", snapshot = false }, + { argv = ["vpt", "rm", "-r", "home/999.0.1/bin"], comment = "Break the active installation", snapshot = false }, + # The fixture payload has no `vp` binary, so this error proves that the upgrade ran instead of reporting that it is up to date. + { argv = ["vp", "upgrade", "999.0.1", "--silent"], envs = [["VP_HOME", "${workspace}/home"]], continue-on-failure = true }, +] diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_rollback/snapshots/command_upgrade_failed_reinstall_keeps_existing_version.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_rollback/snapshots/command_upgrade_failed_reinstall_keeps_existing_version.md new file mode 100644 index 0000000000..4af4cdc700 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_rollback/snapshots/command_upgrade_failed_reinstall_keeps_existing_version.md @@ -0,0 +1,27 @@ +# command_upgrade_failed_reinstall_keeps_existing_version + +A failed upgrade into an existing version directory keeps that directory, which can be the rollback target. + +## `vpt write-file $VP_HOME/999.0.1/keep.txt 'rollback target +'` + + +## `vpt write-file $VP_HOME/999.0.1/bin 'not a directory +'` + +Make extraction fail after the download is verified + + +## `vp upgrade 999.0.1 --silent` + +**Exit code:** 1 + +``` +error: I/O error: File exists (os error 17) +``` + +## `vpt print-file $VP_HOME/999.0.1/keep.txt` + +``` +rollback target +``` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_rollback/snapshots/command_upgrade_repairs_broken_active_version.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_rollback/snapshots/command_upgrade_repairs_broken_active_version.md new file mode 100644 index 0000000000..8bf22e4b94 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_rollback/snapshots/command_upgrade_repairs_broken_active_version.md @@ -0,0 +1,30 @@ +# command_upgrade_repairs_broken_active_version + +An upgrade to the version that `current` names still installs it when the active binary is missing. + +## `vpt mkdir external` + + +## `vpt cp $VP_HOME/bin/vp external/vp` + + +## `vpt chmod +x external/vp` + + +## `VP_HOME=${workspace}/home VP_VERSION=999.0.1 ./external/vp` + +Activate a 999.0.1 directory + + +## `vpt rm -r home/999.0.1/bin` + +Break the active installation + + +## `VP_HOME=${workspace}/home vp upgrade 999.0.1 --silent` + +**Exit code:** 1 + +``` +error: Upgrade error: Binary not found after extraction. The download may be corrupted. +``` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_rollback/snapshots/command_upgrade_rollback_deployed.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_rollback/snapshots/command_upgrade_rollback_deployed.md new file mode 100644 index 0000000000..91f0458aea --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_rollback/snapshots/command_upgrade_rollback_deployed.md @@ -0,0 +1,50 @@ +# command_upgrade_rollback_deployed + +A rollback holds the installation lock while an unmarked target completes in-place setup, so the child must not take that lock. + +## `vpt mkdir external` + + +## `vpt cp $VP_HOME/bin/vp external/vp` + + +## `vpt chmod +x external/vp` + + +## `VP_HOME=${workspace}/home VP_VERSION=rollback-a ./external/vp` + +Deploy the first version + + +## `VP_HOME=${workspace}/home VP_VERSION=rollback-b ./external/vp` + +Deploy the second version, which records the first as the rollback target + + +## `vpt print-file home/.previous-version` + +``` +rollback-a +``` + +## `vpt rm home/rollback-a/bin/.vp-setup-complete` + +The rollback target must run in-place setup during the shim refresh + + +## `VP_HOME=${workspace}/home vp upgrade --rollback` + +A deadlock fails this step by timeout + + +## `vpt print-file home/.previous-version` + +``` +rollback-b +``` + +## `vpt stat-file home/rollback-a/bin/.vp-setup-complete --assert file` + +``` +home/rollback-a/bin/.vp-setup-complete: file +``` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_rollback/tarballs/vite-plus-cli-999.0.1.tgz b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_rollback/tarballs/vite-plus-cli-999.0.1.tgz new file mode 100644 index 0000000000..de50c189de Binary files /dev/null and b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_upgrade_rollback/tarballs/vite-plus-cli-999.0.1.tgz differ diff --git a/crates/vp_global_cli/src/commands/upgrade/mod.rs b/crates/vp_global_cli/src/commands/upgrade/mod.rs index 0d2678bfd4..7751b9f4fd 100644 --- a/crates/vp_global_cli/src/commands/upgrade/mod.rs +++ b/crates/vp_global_cli/src/commands/upgrade/mod.rs @@ -179,6 +179,31 @@ pub async fn execute(options: UpgradeOptions) -> Result { // Step 7: Verify integrity integrity::verify_integrity(&platform_data, &resolved.platform_integrity)?; + // Hold the lock until cleanup finishes. Another process may have activated + // the target version while this one waited. A `current` link whose binary + // is missing still needs this install to repair it. + let _install_lock = install::lock_install_dir(install_dir).await?; + let active_install_dir = install::read_current_version(install_dir).await; + if !options.force + && active_install_dir + .as_deref() + .is_some_and(|active| install::is_install_dir_for_version(active, &resolved.version)) + && tokio::fs::try_exists( + install_dir.join("current").join("bin").join(vp_setup::VP_BINARY_NAME), + ) + .await + .unwrap_or(false) + { + if !options.silent { + vp_shared::output::print_stdout_line(format_args!( + "\n{} Already up to date ({})", + style(output::CHECK).green(), + resolved.version + )); + } + return Ok(ExitStatus::default()); + } + if !options.silent { output::info("installing..."); } @@ -226,7 +251,7 @@ async fn install_version( let install_dir_name = install::target_install_dir_name(version, active_install_dir.as_deref(), force); let version_dir = install_dir.join(&install_dir_name); - tokio::fs::create_dir_all(&version_dir).await?; + let created = install::create_version_dir(&version_dir).await?; // Step 9: Extract platform binary and install via npm let result = install_platform_and_main( @@ -242,10 +267,8 @@ async fn install_version( ) .await; - // On failure, clean up the version directory if result.is_err() { - tracing::debug!("Cleaning up failed install at {}", version_dir.as_path().display()); - let _ = tokio::fs::remove_dir_all(&version_dir).await; + install::discard_failed_version_dir(&version_dir, created).await; } result @@ -395,6 +418,7 @@ async fn execute_rollback( install_dir: &AbsolutePathBuf, silent: bool, ) -> Result { + let _install_lock = install::lock_install_dir(install_dir).await?; let previous = install::read_previous_version(install_dir) .await? .ok_or_else(|| Error::Upgrade("No previous version found. Cannot rollback.".into()))?; diff --git a/crates/vp_global_cli/src/self_setup.rs b/crates/vp_global_cli/src/self_setup.rs index e75debcaa1..093aa74091 100644 --- a/crates/vp_global_cli/src/self_setup.rs +++ b/crates/vp_global_cli/src/self_setup.rs @@ -286,6 +286,11 @@ async fn run( None }; + // A deployment changes `current`, `.previous-version`, and old versions like + // `vp upgrade`. In-place setup is the child of an upgrade that holds the lock. + let _install_lock = + if deploy { Some(install::lock_install_dir(&dirs.data).await?) } else { None }; + // 1. Prepare the payload before activating it. Upgrade has already done this in the in-place case. let previous_version = install::read_current_version(&dirs.data).await; let version_dir = if let Some(package) = user_package { diff --git a/crates/vp_installer/src/legacy.rs b/crates/vp_installer/src/legacy.rs index 6e293a7b63..0eae0fe6d0 100644 --- a/crates/vp_installer/src/legacy.rs +++ b/crates/vp_installer/src/legacy.rs @@ -14,6 +14,8 @@ pub(super) async fn install( target_version: &str, platform_data: &[u8], ) -> Result<(), Box> { + // Serialize activation with `vp upgrade` and standalone deployment. + let install_lock = install::lock_install_dir(&dirs.data).await?; let current_version = install::read_current_version(&dirs.data).await; // Same version only if the binary is intact — a corrupted install needs a full reinstall. @@ -40,7 +42,7 @@ pub(super) async fn install( if !same_version { let install_dir = &dirs.data; let version_dir = install_dir.join(target_version); - tokio::fs::create_dir_all(&version_dir).await?; + let created = install::create_version_dir(&version_dir).await?; let result = install_new_version( opts, @@ -52,13 +54,13 @@ pub(super) async fn install( ) .await; - // On failure, clean up the partial version directory (matches vp upgrade behavior) if result.is_err() { - let _ = tokio::fs::remove_dir_all(&version_dir).await; + install::discard_failed_version_dir(&version_dir, created).await; } result?; } + drop(install_lock); // --- Post-activation setup (always runs, even for same-version repair) --- // All steps below are best-effort: the core install succeeded once `current` diff --git a/crates/vp_setup/src/install.rs b/crates/vp_setup/src/install.rs index 84a08be4cd..727a3c6432 100644 --- a/crates/vp_setup/src/install.rs +++ b/crates/vp_setup/src/install.rs @@ -828,6 +828,62 @@ fn forced_reinstall_dir_name(version: &str) -> String { format!("{version}+force.{}.{}", process::id(), nanos) } +const INSTALL_LOCK_FILE: &str = ".install.lock"; + +/// Serialize installation changes in `install_dir` across processes. +/// +/// Hold the returned file while changing version directories, `current`, +/// `.previous-version`, or old-version cleanup. A child that only refreshes +/// shims runs in place and must not take this lock, or its parent deadlocks. +pub async fn lock_install_dir(install_dir: &AbsolutePath) -> Result { + tokio::fs::create_dir_all(install_dir).await?; + let path = install_dir.join(INSTALL_LOCK_FILE); + tokio::task::spawn_blocking(move || { + // Keep the file in place so every process locks the same inode. + let file = std::fs::OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(path.as_path())?; + file.lock()?; + Ok::<_, std::io::Error>(file) + }) + .await + .map_err(|error| Error::Setup(format!("Task join error: {error}").into()))? + .map_err(Into::into) +} + +/// Create `version_dir` and return whether this call created it. +/// +/// The directory can already exist because a kept version or the rollback +/// target is being installed again. Pass the result to +/// [`discard_failed_version_dir`] so a failed install keeps that directory. +pub async fn create_version_dir(version_dir: &AbsolutePath) -> Result { + if let Some(parent) = version_dir.parent() { + tokio::fs::create_dir_all(parent).await?; + } + match tokio::fs::create_dir(version_dir).await { + Ok(()) => Ok(true), + Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => Ok(false), + Err(error) => Err(error.into()), + } +} + +/// Remove a failed install only when [`create_version_dir`] created it. +pub async fn discard_failed_version_dir(version_dir: &AbsolutePath, created: bool) { + if !created { + tracing::debug!("Keeping existing version directory {}", version_dir.as_path().display()); + return; + } + tracing::debug!("Cleaning up failed install at {}", version_dir.as_path().display()); + if let Err(error) = tokio::fs::remove_dir_all(version_dir).await + && error.kind() != std::io::ErrorKind::NotFound + { + tracing::warn!("Failed to remove {}: {error}", version_dir.as_path().display()); + } +} + /// Clean up old version directories, keeping at most `max_keep` versions. /// /// Sorts by creation time (newest first, matching install.sh behavior) and removes @@ -1358,6 +1414,46 @@ mod tests { assert!(result.is_err(), "cleanup_old_versions should error on non-existent dir"); } + #[tokio::test] + async fn install_lock_waits_for_its_owner() { + let temp = tempfile::tempdir().unwrap(); + let data = AbsolutePathBuf::new(temp.path().join("data")).unwrap(); + + let first = lock_install_dir(&data).await.unwrap(); + let second = lock_install_dir(&data); + tokio::pin!(second); + assert!( + tokio::time::timeout(std::time::Duration::from_millis(150), &mut second).await.is_err() + ); + drop(first); + let second = + tokio::time::timeout(std::time::Duration::from_secs(5), second).await.unwrap().unwrap(); + + let contender = std::fs::File::open(data.join(INSTALL_LOCK_FILE)).unwrap(); + assert!(matches!(contender.try_lock(), Err(std::fs::TryLockError::WouldBlock))); + drop(second); + assert!(contender.try_lock().is_ok()); + } + + #[tokio::test] + async fn failed_install_removes_only_a_directory_it_created() { + let temp = tempfile::tempdir().unwrap(); + let data = AbsolutePathBuf::new(temp.path().join("data")).unwrap(); + + let fresh = data.join("1.2.0"); + assert!(create_version_dir(&fresh).await.unwrap()); + discard_failed_version_dir(&fresh, true).await; + assert!(!fresh.as_path().exists()); + + // A kept version or rollback target survives a failed reinstall. + let existing = data.join("1.1.0"); + std::fs::create_dir_all(existing.join("bin")).unwrap(); + std::fs::write(existing.join("bin").join("vp"), "kept").unwrap(); + assert!(!create_version_dir(&existing).await.unwrap()); + discard_failed_version_dir(&existing, false).await; + assert_eq!(std::fs::read_to_string(existing.join("bin").join("vp")).unwrap(), "kept"); + } + #[tokio::test] async fn test_write_upgrade_log_creates_log_in_parent_dir() { let temp = tempfile::tempdir().unwrap();