Fix release archive name mismatch

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,57 @@ 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<Option<String>, anyhow::Error> {
@@ -39,11 +90,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 +206,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 +219,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 +262,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 +288,28 @@ 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,36 @@ 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<String>,
 ) -> Result<String, anyhow::Error> {
-    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<String>,
+) -> Result<String, anyhow::Error> {
     let url = asset.clone().browser_download_url;
     let name = asset.clone().name;
     let path = release_archive_dir();
@@ -463,6 +560,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();

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,9 +440,13 @@ 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 std::path::PathBuf;
+    use tar::Builder;
 
     // --- Tests -----------------------------------------------------------------
     // Internal helper (exposed for tests inside this module) to build the final path; this
@@ -518,4 +532,59 @@ 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);
+        }
+    }
 }

src/handlers/show.rs

@@ -30,8 +30,8 @@ fn load_default_binaries() -> Result<Binaries, Error> {
 fn load_installed_binaries() -> Result<Vec<BinaryVersion>, 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)
 }