From 26d8ed8ca54cced1e686490ace0c696080ffb4d4 Mon Sep 17 00:00:00 2001 From: jlanson Date: Thu, 12 Dec 2024 15:22:31 -0500 Subject: [PATCH 1/2] feat: add uncompressed artifacts to path for started plugin process Signed-off-by: jlanson --- Cargo.lock | 2 +- hipcheck/src/plugin/manager.rs | 28 +++++++++++++++++----------- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9f28bfbf..fa25d978 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1333,7 +1333,7 @@ checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" [[package]] name = "hipcheck" -version = "3.7.0" +version = "3.8.0" dependencies = [ "anyhow", "async-stream", diff --git a/hipcheck/src/plugin/manager.rs b/hipcheck/src/plugin/manager.rs index 142ea0ca..77c774a1 100644 --- a/hipcheck/src/plugin/manager.rs +++ b/hipcheck/src/plugin/manager.rs @@ -86,30 +86,35 @@ impl PluginExecutor { )); }; + let Ok(canon_working_dir) = plugin.working_dir.canonicalize() else { + return Err(hc_error!( + "Failed to canonicalize plugin working dir: {:?}", + &plugin.working_dir + )); + }; + // Entrypoints are often "" which can overlap with existing binaries on the // system like "git", "npm", so we must search for from within the plugin // cache subfolder. First, grab the existing path. - let Some(mut os_paths) = + let Some(mut which_os_paths) = std::env::var_os("PATH").map(|s| std::env::split_paths(&s).collect::>()) else { return Err(hc_error!("Unable to get system PATH var")); }; - let Ok(canon_working_dir) = plugin.working_dir.canonicalize() else { - return Err(hc_error!( - "Failed to canonicalize plugin working dir: {:?}", - &plugin.working_dir - )); - }; + // Add canonicalized plugin cache dir to end of PATH for plugin exec + let mut cmd_os_paths = which_os_paths.clone(); + cmd_os_paths.push(canon_working_dir.clone()); + let cmd_path = std::env::join_paths(cmd_os_paths).unwrap(); - // Add canonicalized plugin cache dir to temp PATH - os_paths.insert(0, canon_working_dir.clone()); - let search_path = std::env::join_paths(os_paths).unwrap(); + // Add canonicalized plugin cache dir to front of PATH for bin-searching + which_os_paths.insert(0, canon_working_dir.clone()); + let which_path = std::env::join_paths(which_os_paths).unwrap(); // Find the binary_str using temp PATH let Ok(canon_bin_path) = which::which_in::<&str, &OsString, &Path>( bin_path_str, - Some(&search_path), + Some(&which_path), canon_working_dir.as_ref(), ) else { return Err(hc_error!( @@ -138,6 +143,7 @@ impl PluginExecutor { // Spawn plugin process log::debug!("Spawning '{}' on port {}", &plugin.entrypoint, port_str); let Ok(mut proc) = Command::new(&canon_bin_path) + .env("PATH", &cmd_path) .args(spawn_args) // @Temporary - directly forward stdout/stderr from plugin to shell .stdout(std::io::stdout()) From 1356559805dd4200be6ec8181fee022b41c70d2e Mon Sep 17 00:00:00 2001 From: jlanson Date: Thu, 12 Dec 2024 16:18:13 -0500 Subject: [PATCH 2/2] fix: use existing cached plugins and clean download artifacts Signed-off-by: jlanson --- hipcheck/src/plugin/retrieval.rs | 45 ++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 14 deletions(-) diff --git a/hipcheck/src/plugin/retrieval.rs b/hipcheck/src/plugin/retrieval.rs index 99eb3ffa..5842ec6a 100644 --- a/hipcheck/src/plugin/retrieval.rs +++ b/hipcheck/src/plugin/retrieval.rs @@ -2,7 +2,7 @@ use crate::{ cache::plugin::HcPluginCache, - error::Error, + error::{Context, Error}, hc_error, plugin::{ download_manifest::DownloadManifestEntry, get_current_arch, try_get_bin_for_entrypoint, @@ -56,18 +56,15 @@ fn retrieve_plugin( return Ok(()); } - // TODO: if the plugin.kdl file for the plugin already exists, then should we skip the retrieval process? - // if plugin_cache.plugin_kdl(&plugin_id).exists() - log::debug!( - "Retrieving Plugin ID {:?} from {:?}", + "Retrieving Plugin ID {} from {:?}", plugin_id, manifest_location ); let plugin_manifest = match manifest_location { Some(ManifestLocation::Url(plugin_url)) => { - retrieve_plugin_from_network(plugin_id.clone(), plugin_url, plugin_cache)? + retrieve_plugin_from_network(plugin_id.clone(), plugin_url, plugin_cache, false)? } Some(ManifestLocation::Local(plugin_manifest_path)) => { retrieve_local_plugin(plugin_id.clone(), plugin_manifest_path, plugin_cache)? @@ -96,7 +93,15 @@ fn retrieve_plugin_from_network( plugin_id: PluginId, plugin_url: &Url, plugin_cache: &HcPluginCache, + force: bool, ) -> Result { + // Use existing cache entry if not force + let target_manifest = plugin_cache.plugin_kdl(&plugin_id); + if target_manifest.is_file() && !force { + log::debug!("Using existing entry in cache for {}", &plugin_id); + return PluginManifest::from_file(target_manifest); + } + let current_arch = get_current_arch(); let version = plugin_id.version(); let download_manifest = retrieve_download_manifest(plugin_url)?; @@ -211,6 +216,7 @@ fn download_and_unpack_plugin( output_path.as_path(), download_dir.as_path(), download_manifest_entry.compress.format, + true, ) .map_err(|e| { // delete any leftover remnants @@ -310,6 +316,7 @@ fn extract_plugin( bundle_path: &Path, extract_dir: &Path, archive_format: ArchiveFormat, + delete_bundle: bool, ) -> Result<(), Error> { let file = File::open(bundle_path).map_err(|e| { hc_error!( @@ -356,19 +363,29 @@ fn extract_plugin( } }; - for child in read_dir(extract_dir)? { - let child = child?; + let bundle_path_name = bundle_path.file_name().unwrap().to_string_lossy(); + let extension = format!(".{}", archive_format); - if child.file_type()?.is_file() { - continue; - } + // If extracting archive caused a dir with the same name as the archive to be created, copy + // the contents of that dir up a level + if let Some(subdir_name) = bundle_path_name.strip_suffix(&extension) { + let extract_subdir = pathbuf![extract_dir, subdir_name]; - for extracted_content in read_dir(child.path())? { - let extracted_content = extracted_content?; - move_to_extract_dir(extract_dir, &extracted_content)?; + // Also does implicit exists() check + if extract_subdir.is_dir() { + for extracted_content in read_dir(&extract_subdir)? { + let extracted_content = extracted_content?; + move_to_extract_dir(extract_dir, &extracted_content)?; + } + std::fs::remove_dir_all(extract_subdir) + .context("Failed to clean up plugin's extracted subdir")?; } } + if delete_bundle { + std::fs::remove_file(bundle_path)?; + } + Ok(()) }