From a45d78b8c495ab54f0bd32702fb54005f874f436 Mon Sep 17 00:00:00 2001 From: stefan-mysten <135084671+stefan-mysten@users.noreply.github.com> Date: Tue, 12 May 2026 09:05:22 -0700 Subject: [PATCH 1/2] Fix issue with release archives being mismatched for the binary being installed --- src/handlers/download.rs | 198 +++++++++++++++++++++++++++++++++----- src/handlers/mod.rs | 67 +++++++++++++ src/handlers/show.rs | 4 +- tests/integration_test.rs | 35 ++++++- tests/test_utils.rs | 39 +++++++- 5 files changed, 316 insertions(+), 27 deletions(-) diff --git a/src/handlers/download.rs b/src/handlers/download.rs index ec5dab9..aa4bd4f 100644 --- a/src/handlers/download.rs +++ b/src/handlers/download.rs @@ -1,12 +1,14 @@ // Copyright (c) Mysten Labs, Inc. // SPDX-License-Identifier: Apache-2.0 -use crate::handlers::release::{ - ensure_version_prefix, find_last_release_by_network, find_networks_with_version, -}; +use crate::handlers::release::{ensure_version_prefix, find_networks_with_version}; use crate::handlers::version::extract_version_from_release; use crate::registry::BinaryConfig; -use crate::{handlers::release::release_list, paths::release_archive_dir, types::Release}; +use crate::{ + handlers::release::release_list, + paths::release_archive_dir, + types::{Asset, Release}, +}; use anyhow::{Context, Error, anyhow, bail}; use futures_util::StreamExt; use indicatif::{HumanBytes, ProgressBar, ProgressStyle}; @@ -22,8 +24,56 @@ use std::{cmp::min, io::Write, path::PathBuf, time::Instant}; use tracing::debug; +fn archive_prefix(config: &BinaryConfig) -> &str { + if config.shared_repo_binary { + config.repository.rsplit('/').next().unwrap_or(&config.name) + } else { + &config.name + } +} + +fn archive_filename_matches( + config: &BinaryConfig, + filename: &str, + network: &str, + version: Option<&str>, + os: &str, + arch: &str, +) -> bool { + let prefix = match version { + Some(version) => format!( + "{}-{}-{}-", + archive_prefix(config), + network, + ensure_version_prefix(version) + ), + None => format!("{}-{network}-", archive_prefix(config)), + }; + let tgz_suffix = format!("-{os}-{arch}.tgz"); + let zip_suffix = format!("-{os}-{arch}.zip"); + + filename.starts_with(&prefix) + && (filename.ends_with(&tgz_suffix) || filename.ends_with(&zip_suffix)) + && (version.is_some() || extract_version_from_release(filename).is_ok()) +} + +fn find_matching_asset<'a>( + release: &'a Release, + config: &BinaryConfig, + network: &str, + version: Option<&str>, + os: &str, + arch: &str, +) -> Option<&'a Asset> { + release.assets.iter().find(|asset| { + archive_filename_matches(config, &asset.name, network, version, os, arch) + }) +} + fn find_cached_release_archive( - tag: &str, + config: &BinaryConfig, + network: &str, + version: &str, os: &str, arch: &str, ) -> Result, anyhow::Error> { @@ -39,11 +89,7 @@ fn find_cached_release_archive( entry.with_context(|| format!("Cannot read entry in {}", cache_dir.display()))?; let filename = entry.file_name().to_string_lossy().to_string(); - if filename.contains(tag) - && filename.contains(os) - && filename.contains(arch) - && (filename.ends_with(".tgz") || filename.ends_with(".zip")) - { + if archive_filename_matches(config, &filename, network, Some(version), os, arch) { return Ok(Some(filename)); } } @@ -159,7 +205,7 @@ pub async fn download_release_at_version( let tag = format!("{}-{}", network, version); - if let Some(filename) = find_cached_release_archive(&tag, &os, &arch)? { + if let Some(filename) = find_cached_release_archive(config, network, &version, &os, &arch)? { println!("Found {filename} in cache"); return Ok(filename); } @@ -172,9 +218,18 @@ pub async fn download_release_at_version( if let Some(release) = releases .iter() - .find(|r| r.assets.iter().any(|a| a.name.contains(&tag))) + .find(|r| find_matching_asset(r, config, network, Some(&version), &os, &arch).is_some()) { - download_asset_from_github(release, &os, &arch, github_token).await + download_asset_from_github( + release, + config, + network, + Some(&version), + &os, + &arch, + github_token, + ) + .await } else { headers.insert(USER_AGENT, HeaderValue::from_static("suiup")); @@ -206,7 +261,16 @@ pub async fn download_release_at_version( } let release: Release = parse_json_response(response, &url, "GitHub release").await?; - download_asset_from_github(&release, &os, &arch, github_token).await + download_asset_from_github( + &release, + config, + network, + Some(&version), + &os, + &arch, + github_token, + ) + .await } } @@ -223,16 +287,29 @@ pub async fn download_latest_release( let (os, arch) = detect_os_arch()?; - let last_release = find_last_release_by_network(releases.0.clone(), network) - .await + let last_release = releases + .0 + .iter() + .find(|release| find_matching_asset(release, config, network, None, &os, &arch).is_some()) .ok_or_else(|| generate_network_suggestions_error(config, &releases.0, None, network))?; + let asset = find_matching_asset(last_release, config, network, None, &os, &arch).ok_or_else( + || { + anyhow!( + "Asset not found for {} on {} {}-{}", + config.name, + network, + os, + arch + ) + }, + )?; println!( "Last {network} release: {}", - extract_version_from_release(&last_release.assets[0].name)? + extract_version_from_release(&asset.name)? ); - download_asset_from_github(&last_release, &os, &arch, github_token).await + download_asset(asset, github_token).await } pub async fn download_file( @@ -426,16 +503,34 @@ where /// architecture and OS async fn download_asset_from_github( release: &Release, + config: &BinaryConfig, + network: &str, + version: Option<&str>, os: &str, arch: &str, github_token: Option, ) -> Result { - let asset = release - .assets - .iter() - .find(|&a| a.name.contains(arch) && a.name.contains(os.to_string().to_lowercase().as_str())) - .ok_or_else(|| anyhow!("Asset not found for {os}-{arch}"))?; + let asset = find_matching_asset(release, config, network, version, os, arch).ok_or_else( + || { + let version_display = version + .map(ensure_version_prefix) + .map(|version| format!(" {version}")) + .unwrap_or_default(); + anyhow!( + "Asset not found for {} {}{} {}-{}", + config.name, + network, + version_display, + os, + arch + ) + }, + )?; + + download_asset(asset, github_token).await +} +async fn download_asset(asset: &Asset, github_token: Option) -> Result { let url = asset.clone().browser_download_url; let name = asset.clone().name; let path = release_archive_dir(); @@ -463,6 +558,63 @@ mod tests { } } + #[test] + fn test_cached_archive_matches_binary_prefix() { + let config = BinaryRegistry::global().get("walrus").unwrap(); + + assert!(archive_filename_matches( + config, + "walrus-testnet-v1.48.1-macos-arm64.tgz", + "testnet", + Some("v1.48.1"), + "macos", + "arm64" + )); + assert!(!archive_filename_matches( + config, + "sui-testnet-v1.48.1-macos-arm64.tgz", + "testnet", + Some("v1.48.1"), + "macos", + "arm64" + )); + } + + #[test] + fn test_cached_archive_matches_shared_repo_prefix() { + let config = BinaryRegistry::global().get("sui-node").unwrap(); + + assert!(archive_filename_matches( + config, + "sui-testnet-v1.39.3-macos-arm64.tgz", + "testnet", + Some("v1.39.3"), + "macos", + "arm64" + )); + } + + #[test] + fn test_find_matching_asset_prefers_requested_binary() { + let config = BinaryRegistry::global().get("walrus").unwrap(); + let release = create_test_release(vec![ + "sui-testnet-v1.48.1-macos-arm64.tgz", + "walrus-testnet-v1.48.1-macos-arm64.tgz", + ]); + + let asset = find_matching_asset( + &release, + config, + "testnet", + Some("v1.48.1"), + "macos", + "arm64", + ) + .unwrap(); + + assert_eq!(asset.name, "walrus-testnet-v1.48.1-macos-arm64.tgz"); + } + #[test] fn test_generate_network_suggestions_error_with_version() { let config = BinaryRegistry::global().get("sui").unwrap(); diff --git a/src/handlers/mod.rs b/src/handlers/mod.rs index 4bbdaa7..6070afe 100644 --- a/src/handlers/mod.rs +++ b/src/handlers/mod.rs @@ -305,6 +305,7 @@ fn extract_component(orig_binary: &str, network: String, filename: &str) -> Resu let binary = orig_binary.to_string(); #[cfg(windows)] let binary = format!("{}.exe", orig_binary); + let mut extracted = false; // Check if the current entry matches the file name for file in archive @@ -368,10 +369,19 @@ fn extract_component(orig_binary: &str, network: String, filename: &str) -> Resu })?; } } + extracted = true; break; } } + if !extracted { + return Err(anyhow!( + "Binary '{}' not found in release archive {}", + binary, + filename + )); + } + Ok(()) } @@ -430,8 +440,12 @@ pub fn installed_binaries_grouped_by_network( mod tests { use super::check_if_binaries_exist; use crate::paths::binaries_dir; + use crate::paths::release_archive_dir; + use flate2::Compression; + use flate2::write::GzEncoder; use std::fs::{self, File}; use std::io::Write; + use tar::Builder; use std::path::PathBuf; // --- Tests ----------------------------------------------------------------- @@ -518,4 +532,57 @@ mod tests { crate::remove_env_var!(var); } } + + fn create_test_archive(archive_path: &std::path::Path, entries: &[(&str, &[u8])]) { + let file = File::create(archive_path).unwrap(); + let encoder = GzEncoder::new(file, Compression::default()); + let mut builder = Builder::new(encoder); + + for (name, contents) in entries { + let mut header = tar::Header::new_gnu(); + header.set_mode(0o755); + header.set_size(contents.len() as u64); + header.set_cksum(); + builder.append_data(&mut header, name, &contents[..]).unwrap(); + } + + let encoder = builder.into_inner().unwrap(); + encoder.finish().unwrap(); + } + + #[test] + fn test_extract_component_errors_when_binary_missing_from_archive() { + let temp = tempfile::TempDir::new().unwrap(); + #[cfg(windows)] + let (var, original) = ("LOCALAPPDATA", std::env::var("LOCALAPPDATA").ok()); + #[cfg(not(windows))] + let (var, original) = ("XDG_CACHE_HOME", std::env::var("XDG_CACHE_HOME").ok()); + crate::set_env_var!(var, temp.path()); + + let releases_dir = release_archive_dir(); + fs::create_dir_all(&releases_dir).unwrap(); + + let filename = "sui-testnet-v1.48.1-macos-arm64.tgz"; + let archive_path = releases_dir.join(filename); + #[cfg(windows)] + let archive_entry = "sui.exe"; + #[cfg(not(windows))] + let archive_entry = "sui"; + create_test_archive(&archive_path, &[(archive_entry, b"sui payload")]); + + let error = + super::extract_component("walrus", "testnet".to_string(), filename).unwrap_err(); + let error_message = error.to_string(); + + #[cfg(windows)] + assert!(error_message.contains("Binary 'walrus.exe' not found")); + #[cfg(not(windows))] + assert!(error_message.contains("Binary 'walrus' not found")); + + if let Some(val) = original { + crate::set_env_var!(var, val); + } else { + crate::remove_env_var!(var); + } + } } diff --git a/src/handlers/show.rs b/src/handlers/show.rs index b630632..b4a46f5 100644 --- a/src/handlers/show.rs +++ b/src/handlers/show.rs @@ -30,8 +30,8 @@ fn load_default_binaries() -> Result { fn load_installed_binaries() -> Result, Error> { let installed_binaries = installed_binaries_grouped_by_network(None)?; let binaries = installed_binaries - .into_iter() - .flat_map(|(_, binaries)| binaries.to_owned()) + .into_values() + .flat_map(|binaries| binaries.to_owned()) .collect(); Ok(binaries) } diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 7286f2a..6587a44 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -5,7 +5,7 @@ mod test_utils; #[cfg(test)] mod tests { - use crate::test_utils::TestEnv; + use crate::test_utils::{TestEnv, detect_os_arch_for_tests}; use anyhow::Result; use assert_cmd::Command; use assert_cmd::cargo::cargo_bin_cmd; @@ -471,6 +471,39 @@ mod tests { Ok(()) } + #[tokio::test] + async fn test_walrus_install_prefers_matching_cached_archive() -> Result<()> { + let test_env = TestEnv::new()?; + test_env.initialize_paths()?; + + let (os, arch) = detect_os_arch_for_tests(); + let version = "v1.48.1"; + let sui_archive = format!("sui-testnet-{version}-{os}-{arch}.tgz"); + let walrus_archive = format!("walrus-testnet-{version}-{os}-{arch}.tgz"); + + test_env.create_cached_archive(&sui_archive, "sui", b"sui payload")?; + test_env.create_cached_archive(&walrus_archive, "walrus", b"walrus payload")?; + + let mut cmd = suiup_command(vec!["install", "walrus@testnet-v1.48.1", "-y"], &test_env); + + #[cfg(windows)] + let extracted_binary = test_env + .data_dir + .join("suiup/binaries/testnet/walrus-v1.48.1.exe"); + #[cfg(not(windows))] + let extracted_binary = test_env + .data_dir + .join("suiup/binaries/testnet/walrus-v1.48.1"); + + cmd.assert() + .success() + .stdout(predicate::str::contains(format!("Found {walrus_archive} in cache"))); + + assert_eq!(fs::read(extracted_binary)?, b"walrus payload"); + + Ok(()) + } + #[tokio::test] async fn test_show_default_flag() -> Result<()> { let test_env = TestEnv::new()?; diff --git a/tests/test_utils.rs b/tests/test_utils.rs index af7d716..dcf9fb6 100644 --- a/tests/test_utils.rs +++ b/tests/test_utils.rs @@ -2,6 +2,8 @@ // SPDX-License-Identifier: Apache-2.0 use anyhow::{Result, anyhow}; +use flate2::Compression; +use flate2::write::GzEncoder; use std::env; use std::path::{Path, PathBuf}; use std::sync::{LazyLock, Mutex, MutexGuard}; @@ -9,6 +11,7 @@ use suiup::paths::{ get_cache_home, get_config_home, get_data_home, get_default_bin_dir, initialize, }; use suiup::{remove_env_var, set_env_var}; +use tar::Builder; use tempfile::TempDir; pub struct TestEnv { @@ -148,6 +151,40 @@ impl TestEnv { Ok(()) } + + pub fn create_cached_archive( + &self, + filename: &str, + binary_name: &str, + contents: &[u8], + ) -> Result { + let _guard = ZIP_FILES_MUTEX + .lock() + .expect("failed to lock zip-files mutex"); + let releases_dir = self.cache_dir.join("suiup").join("releases"); + std::fs::create_dir_all(&releases_dir)?; + + let archive_path = releases_dir.join(filename); + let file = std::fs::File::create(&archive_path)?; + let encoder = GzEncoder::new(file, Compression::default()); + let mut builder = Builder::new(encoder); + + #[cfg(windows)] + let entry_name = format!("{binary_name}.exe"); + #[cfg(not(windows))] + let entry_name = binary_name.to_string(); + + let mut header = tar::Header::new_gnu(); + header.set_mode(0o755); + header.set_size(contents.len() as u64); + header.set_cksum(); + builder.append_data(&mut header, entry_name, contents)?; + + let encoder = builder.into_inner()?; + encoder.finish()?; + + Ok(archive_path) + } } impl Drop for TestEnv { @@ -162,7 +199,7 @@ impl Drop for TestEnv { } } -fn detect_os_arch_for_tests() -> (&'static str, &'static str) { +pub(crate) fn detect_os_arch_for_tests() -> (&'static str, &'static str) { let os = if cfg!(target_os = "macos") { "macos" } else if cfg!(target_os = "linux") { From b6f62c390a6274751ea0ade30f26bce0150a9ccb Mon Sep 17 00:00:00 2001 From: stefan-mysten <135084671+stefan-mysten@users.noreply.github.com> Date: Tue, 12 May 2026 09:10:44 -0700 Subject: [PATCH 2/2] Formatting --- src/handlers/download.rs | 26 ++++++++++++++------------ src/handlers/mod.rs | 6 ++++-- tests/integration_test.rs | 4 +++- 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/handlers/download.rs b/src/handlers/download.rs index aa4bd4f..66940f2 100644 --- a/src/handlers/download.rs +++ b/src/handlers/download.rs @@ -65,9 +65,10 @@ fn find_matching_asset<'a>( os: &str, arch: &str, ) -> Option<&'a Asset> { - release.assets.iter().find(|asset| { - archive_filename_matches(config, &asset.name, network, version, os, arch) - }) + release + .assets + .iter() + .find(|asset| archive_filename_matches(config, &asset.name, network, version, os, arch)) } fn find_cached_release_archive( @@ -292,8 +293,8 @@ pub async fn download_latest_release( .iter() .find(|release| find_matching_asset(release, config, network, None, &os, &arch).is_some()) .ok_or_else(|| generate_network_suggestions_error(config, &releases.0, None, network))?; - let asset = find_matching_asset(last_release, config, network, None, &os, &arch).ok_or_else( - || { + let asset = + find_matching_asset(last_release, config, network, None, &os, &arch).ok_or_else(|| { anyhow!( "Asset not found for {} on {} {}-{}", config.name, @@ -301,8 +302,7 @@ pub async fn download_latest_release( os, arch ) - }, - )?; + })?; println!( "Last {network} release: {}", @@ -510,8 +510,8 @@ async fn download_asset_from_github( arch: &str, github_token: Option, ) -> Result { - let asset = find_matching_asset(release, config, network, version, os, arch).ok_or_else( - || { + let asset = + find_matching_asset(release, config, network, version, os, arch).ok_or_else(|| { let version_display = version .map(ensure_version_prefix) .map(|version| format!(" {version}")) @@ -524,13 +524,15 @@ async fn download_asset_from_github( os, arch ) - }, - )?; + })?; download_asset(asset, github_token).await } -async fn download_asset(asset: &Asset, github_token: Option) -> Result { +async fn download_asset( + asset: &Asset, + github_token: Option, +) -> Result { let url = asset.clone().browser_download_url; let name = asset.clone().name; let path = release_archive_dir(); diff --git a/src/handlers/mod.rs b/src/handlers/mod.rs index 6070afe..baec048 100644 --- a/src/handlers/mod.rs +++ b/src/handlers/mod.rs @@ -445,8 +445,8 @@ mod tests { use flate2::write::GzEncoder; use std::fs::{self, File}; use std::io::Write; - use tar::Builder; use std::path::PathBuf; + use tar::Builder; // --- Tests ----------------------------------------------------------------- // Internal helper (exposed for tests inside this module) to build the final path; this @@ -543,7 +543,9 @@ mod tests { header.set_mode(0o755); header.set_size(contents.len() as u64); header.set_cksum(); - builder.append_data(&mut header, name, &contents[..]).unwrap(); + builder + .append_data(&mut header, name, &contents[..]) + .unwrap(); } let encoder = builder.into_inner().unwrap(); diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 6587a44..55cd436 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -497,7 +497,9 @@ mod tests { cmd.assert() .success() - .stdout(predicate::str::contains(format!("Found {walrus_archive} in cache"))); + .stdout(predicate::str::contains(format!( + "Found {walrus_archive} in cache" + ))); assert_eq!(fs::read(extracted_binary)?, b"walrus payload");