diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index bc94e121e..c70f3c20a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -173,6 +173,7 @@ jobs: path: target/distrib/ merge-multiple: true - name: Install dependencies + if: ${{ matrix.packages_install != null && matrix.packages_install != '' }} shell: bash run: | set -euo pipefail @@ -187,7 +188,24 @@ jobs: fi # cargo-dist emits apt-get install without -y; keep release jobs non-interactive. - sed -i -E 's/^([[:space:]]*sudo[[:space:]]+)?apt-get[[:space:]]+install[[:space:]]+/\1apt-get install -y /' "$install_script" + awk ' + /^[[:space:]]*sudo[[:space:]]+apt-get[[:space:]]+install([[:space:]]|$)/ { + if ($0 !~ /(^|[[:space:]])(-y|--yes)([[:space:]]|$)/) { + sub(/sudo[[:space:]]+apt-get[[:space:]]+install[[:space:]]+/, "sudo apt-get install -y ") + } + print + next + } + /^[[:space:]]*apt-get[[:space:]]+install([[:space:]]|$)/ { + if ($0 !~ /(^|[[:space:]])(-y|--yes)([[:space:]]|$)/) { + sub(/apt-get[[:space:]]+install[[:space:]]+/, "apt-get install -y ") + } + print + next + } + { print } + ' "$install_script" > "${install_script}.normalized" + mv "${install_script}.normalized" "$install_script" DEBIAN_FRONTEND=noninteractive bash -euxo pipefail "$install_script" - name: Build artifacts run: | diff --git a/Cargo.lock b/Cargo.lock index d6c3506ce..735a764d9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4586,7 +4586,7 @@ checksum = "051eb1abcf10076295e815102942cc58f9d5e3b4560e46e53c21e8ff6f3af7b1" [[package]] name = "vx" -version = "0.9.5" +version = "0.9.6" dependencies = [ "anyhow", "assert_cmd", @@ -4603,7 +4603,7 @@ dependencies = [ [[package]] name = "vx-args" -version = "0.9.5" +version = "0.9.6" dependencies = [ "indexmap", "regex", @@ -4616,7 +4616,7 @@ dependencies = [ [[package]] name = "vx-bridge" -version = "0.9.5" +version = "0.9.6" dependencies = [ "tempfile", "tracing", @@ -4626,7 +4626,7 @@ dependencies = [ [[package]] name = "vx-cache" -version = "0.9.5" +version = "0.9.6" dependencies = [ "anyhow", "bincode", @@ -4641,7 +4641,7 @@ dependencies = [ [[package]] name = "vx-cli" -version = "0.9.5" +version = "0.9.6" dependencies = [ "anyhow", "async-trait", @@ -4705,7 +4705,7 @@ dependencies = [ [[package]] name = "vx-config" -version = "0.9.5" +version = "0.9.6" dependencies = [ "anyhow", "chrono", @@ -4727,7 +4727,7 @@ dependencies = [ [[package]] name = "vx-console" -version = "0.9.5" +version = "0.9.6" dependencies = [ "anstream", "anstyle", @@ -4748,7 +4748,7 @@ dependencies = [ [[package]] name = "vx-ecosystem-pm" -version = "0.9.5" +version = "0.9.6" dependencies = [ "anyhow", "async-trait", @@ -4766,7 +4766,7 @@ dependencies = [ [[package]] name = "vx-env" -version = "0.9.5" +version = "0.9.6" dependencies = [ "anyhow", "colored", @@ -4788,7 +4788,7 @@ dependencies = [ [[package]] name = "vx-extension" -version = "0.9.5" +version = "0.9.6" dependencies = [ "anyhow", "async-trait", @@ -4810,7 +4810,7 @@ dependencies = [ [[package]] name = "vx-installer" -version = "0.9.5" +version = "0.9.6" dependencies = [ "anyhow", "async-trait", @@ -4843,7 +4843,7 @@ dependencies = [ [[package]] name = "vx-manifest" -version = "0.9.5" +version = "0.9.6" dependencies = [ "pretty_assertions", "rstest", @@ -4861,7 +4861,7 @@ dependencies = [ [[package]] name = "vx-metrics" -version = "0.9.5" +version = "0.9.6" dependencies = [ "anyhow", "chrono", @@ -4884,7 +4884,7 @@ dependencies = [ [[package]] name = "vx-migration" -version = "0.9.5" +version = "0.9.6" dependencies = [ "async-trait", "chrono", @@ -4906,14 +4906,14 @@ dependencies = [ [[package]] name = "vx-msbuild-bridge" -version = "0.9.5" +version = "0.9.6" dependencies = [ "workspace-hack", ] [[package]] name = "vx-output-filter" -version = "0.9.5" +version = "0.9.6" dependencies = [ "anyhow", "regex", @@ -4927,7 +4927,7 @@ dependencies = [ [[package]] name = "vx-paths" -version = "0.9.5" +version = "0.9.6" dependencies = [ "anyhow", "chrono", @@ -4946,7 +4946,7 @@ dependencies = [ [[package]] name = "vx-project-analyzer" -version = "0.9.5" +version = "0.9.6" dependencies = [ "anyhow", "async-trait", @@ -4971,7 +4971,7 @@ dependencies = [ [[package]] name = "vx-resolver" -version = "0.9.5" +version = "0.9.6" dependencies = [ "anyhow", "async-trait", @@ -5005,7 +5005,7 @@ dependencies = [ [[package]] name = "vx-runtime" -version = "0.9.5" +version = "0.9.6" dependencies = [ "anyhow", "async-trait", @@ -5035,7 +5035,7 @@ dependencies = [ [[package]] name = "vx-runtime-archive" -version = "0.9.5" +version = "0.9.6" dependencies = [ "anyhow", "flate2", @@ -5054,7 +5054,7 @@ dependencies = [ [[package]] name = "vx-runtime-core" -version = "0.9.5" +version = "0.9.6" dependencies = [ "anyhow", "async-trait", @@ -5071,7 +5071,7 @@ dependencies = [ [[package]] name = "vx-runtime-http" -version = "0.9.5" +version = "0.9.6" dependencies = [ "anyhow", "async-trait", @@ -5104,7 +5104,7 @@ dependencies = [ [[package]] name = "vx-setup" -version = "0.9.5" +version = "0.9.6" dependencies = [ "anyhow", "chrono", @@ -5121,7 +5121,7 @@ dependencies = [ [[package]] name = "vx-shim" -version = "0.9.5" +version = "0.9.6" dependencies = [ "anyhow", "serde", @@ -5139,11 +5139,11 @@ dependencies = [ [[package]] name = "vx-star-metadata" -version = "0.9.5" +version = "0.9.6" [[package]] name = "vx-starlark" -version = "0.9.5" +version = "0.9.6" dependencies = [ "anyhow", "async-trait", @@ -5173,7 +5173,7 @@ dependencies = [ [[package]] name = "vx-system-pm" -version = "0.9.5" +version = "0.9.6" dependencies = [ "anyhow", "async-trait", @@ -5196,7 +5196,7 @@ dependencies = [ [[package]] name = "vx-version-fetcher" -version = "0.9.5" +version = "0.9.6" dependencies = [ "anyhow", "async-trait", @@ -5214,7 +5214,7 @@ dependencies = [ [[package]] name = "vx-versions" -version = "0.9.5" +version = "0.9.6" dependencies = [ "anyhow", "async-trait", diff --git a/crates/vx-runtime/src/runtime/install_impl.rs b/crates/vx-runtime/src/runtime/install_impl.rs index ed4372898..f8c7e5ac8 100644 --- a/crates/vx-runtime/src/runtime/install_impl.rs +++ b/crates/vx-runtime/src/runtime/install_impl.rs @@ -98,15 +98,11 @@ pub async fn default_install_inner( ); debug!("Executable relative path: {}", params.exe_relative); - // Check if already installed - if let Some(result) = already_installed_result(¶ms, version, &install_path, ctx) { - return Ok(result); - } - let _install_lock = InstallLock::acquire(&install_path).await?; - // Another vx process may have completed the install while this process was - // waiting for the lock. Re-check before cleaning or downloading. + // Hold the lock before trusting files in the install directory. Another vx + // process may have created the executable path while extraction is still in + // progress, especially on Windows archive installs. if let Some(result) = already_installed_result(¶ms, version, &install_path, ctx) { return Ok(result); } diff --git a/crates/vx-runtime/tests/install_lock_tests.rs b/crates/vx-runtime/tests/install_lock_tests.rs index b83117980..7407ba0dd 100644 --- a/crates/vx-runtime/tests/install_lock_tests.rs +++ b/crates/vx-runtime/tests/install_lock_tests.rs @@ -3,7 +3,7 @@ use std::path::Path; use std::sync::{ Arc, - atomic::{AtomicUsize, Ordering}, + atomic::{AtomicBool, AtomicUsize, Ordering}, }; use std::time::Duration; @@ -66,15 +66,41 @@ impl Installer for SlowInstaller { tokio::time::sleep(Duration::from_millis(200)).await; let exe_path = bin_dir.join(exe_file_name()); - std::fs::write(&exe_path, b"#!/bin/sh\nexit 0\n")?; - - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - let mut permissions = std::fs::metadata(&exe_path)?.permissions(); - permissions.set_mode(0o755); - std::fs::set_permissions(&exe_path, permissions)?; + write_test_executable(&exe_path)?; + + Ok(()) + } +} + +#[derive(Debug)] +struct EarlyExecutableInstaller { + executable_created: AtomicBool, +} + +impl EarlyExecutableInstaller { + fn new() -> Self { + Self { + executable_created: AtomicBool::new(false), } + } +} + +#[async_trait] +impl Installer for EarlyExecutableInstaller { + async fn extract(&self, _archive: &Path, _dest: &Path) -> Result<()> { + bail!("not used") + } + + async fn download_and_extract(&self, _url: &str, dest: &Path) -> Result<()> { + let bin_dir = dest.join("bin"); + std::fs::create_dir_all(&bin_dir)?; + + let exe_path = bin_dir.join(exe_file_name()); + write_test_executable(&exe_path)?; + self.executable_created.store(true, Ordering::SeqCst); + + tokio::time::sleep(Duration::from_millis(300)).await; + std::fs::write(dest.join("install-complete"), b"ok")?; Ok(()) } @@ -123,6 +149,51 @@ async fn concurrent_installs_wait_for_the_first_completed_install() { assert!(!lock_path.exists(), "install lock should be removed"); } +#[tokio::test] +async fn install_waits_for_lock_before_trusting_existing_executable() { + let temp_dir = tempfile::tempdir().expect("temp dir should be created"); + let installer = Arc::new(EarlyExecutableInstaller::new()); + let ctx = RuntimeContext::new( + Arc::new(RealPathProvider::with_base_dir(temp_dir.path())), + Arc::new(NoopHttpClient), + Arc::new(RealFileSystem::new()), + installer.clone(), + ); + + let ctx_a = ctx.clone(); + let first = tokio::spawn(async move { + default_install_inner(make_params(), "1.0.0", &ctx_a, |_, _| Ok(())).await + }); + + tokio::time::timeout(Duration::from_secs(2), async { + while !installer.executable_created.load(Ordering::SeqCst) { + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("first install should create executable"); + + let second = default_install_inner(make_params(), "1.0.0", &ctx, |_, _| Ok(())) + .await + .expect("second install should wait for the first install"); + + let complete_marker = ctx + .paths + .version_store_dir("race-tool", "1.0.0") + .join("install-complete"); + assert!( + complete_marker.exists(), + "second install returned before the first install finished" + ); + + let first = first + .await + .expect("first task should not panic") + .expect("first install should succeed"); + + assert_eq!(first.executable_path, second.executable_path); +} + fn make_params() -> InstallParams<'static> { InstallParams { name: "race-tool", @@ -147,3 +218,17 @@ fn exe_file_name() -> &'static str { fn exe_extensions() -> &'static [&'static str] { if cfg!(windows) { &[".exe"] } else { &[] } } + +fn write_test_executable(exe_path: &Path) -> Result<()> { + std::fs::write(exe_path, b"#!/bin/sh\nexit 0\n")?; + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mut permissions = std::fs::metadata(exe_path)?.permissions(); + permissions.set_mode(0o755); + std::fs::set_permissions(exe_path, permissions)?; + } + + Ok(()) +}