From f4c28aa9286afec0529e9e685554f4eedd060237 Mon Sep 17 00:00:00 2001 From: Filip Tibell Date: Sat, 22 Oct 2022 11:17:58 +0200 Subject: [PATCH 1/4] Split trust prompting from trust check --- src/tool_storage.rs | 24 ++++++++++++++++-------- src/trust.rs | 6 ++++++ 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/tool_storage.rs b/src/tool_storage.rs index cf5ec27..1cede28 100644 --- a/src/tool_storage.rs +++ b/src/tool_storage.rs @@ -19,7 +19,7 @@ use crate::tool_id::ToolId; use crate::tool_name::ToolName; use crate::tool_source::{Asset, GitHubSource, Release}; use crate::tool_spec::ToolSpec; -use crate::trust::{TrustCache, TrustMode}; +use crate::trust::{TrustCache, TrustMode, TrustStatus}; pub struct ToolStorage { pub storage_dir: PathBuf, @@ -154,7 +154,7 @@ impl ToolStorage { let installed_path = self.storage_dir.join("installed.txt"); let installed = InstalledToolsCache::read(&installed_path)?; - self.trust_check(spec.name(), trust)?; + self.trust_ensure(spec.name(), trust)?; log::info!("Installing tool: {}", spec); @@ -234,7 +234,7 @@ impl ToolStorage { return Ok(()); } - self.trust_check(id.name(), trust)?; + self.trust_ensure(id.name(), trust)?; log::info!("Installing tool: {id}"); @@ -319,11 +319,9 @@ impl ToolStorage { .collect() } - fn trust_check(&self, name: &ToolName, mode: TrustMode) -> anyhow::Result<()> { - let trusted = TrustCache::read(&self.home)?; - let is_trusted = trusted.tools.contains(name); - - if !is_trusted { + fn trust_ensure(&self, name: &ToolName, mode: TrustMode) -> anyhow::Result<()> { + let status = self.trust_check(name)?; + if status == TrustStatus::NotTrusted { if mode == TrustMode::Check { // If the terminal isn't interactive, tell the user that they // need to open an interactive terminal to trust this tool. @@ -358,6 +356,16 @@ impl ToolStorage { Ok(()) } + fn trust_check(&self, name: &ToolName) -> anyhow::Result { + let trusted = TrustCache::read(&self.home)?; + let is_trusted = trusted.tools.contains(name); + if is_trusted { + Ok(TrustStatus::Trusted) + } else { + Ok(TrustStatus::NotTrusted) + } + } + fn install_executable(&self, id: &ToolId, mut contents: impl Read) -> anyhow::Result<()> { let output_path = self.exe_path(id); diff --git a/src/trust.rs b/src/trust.rs index 3ec0a86..a670933 100644 --- a/src/trust.rs +++ b/src/trust.rs @@ -13,6 +13,12 @@ pub enum TrustMode { NoCheck, } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum TrustStatus { + Trusted, + NotTrusted, +} + #[derive(Debug)] pub struct TrustCache { pub tools: BTreeSet, From 881f0dd3d315634860be68d529e9c95dc43637ff Mon Sep 17 00:00:00 2001 From: Filip Tibell Date: Sat, 22 Oct 2022 12:11:30 +0200 Subject: [PATCH 2/4] Split trust check from install when installing multiple tools --- src/tool_storage.rs | 49 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 39 insertions(+), 10 deletions(-) diff --git a/src/tool_storage.rs b/src/tool_storage.rs index 1cede28..4aeebc6 100644 --- a/src/tool_storage.rs +++ b/src/tool_storage.rs @@ -139,11 +139,39 @@ impl ToolStorage { let current_dir = current_dir().context("Failed to get current working directory")?; let manifests = Manifest::discover(&self.home, ¤t_dir)?; - for manifest in manifests { - for (alias, tool_id) in manifest.tools { - self.install_exact(&tool_id, trust)?; - self.link(&alias)?; - } + // Installing all tools is split into multiple steps: + // 1. Trust check, which requires user input and may yield + // 2. Installation of trusted tools, which will be in parallel in the future + + let all_tools: Vec<_> = manifests + .iter() + .flat_map(|manifest| &manifest.tools) + .collect(); + + for (_, tool_id) in &all_tools { + self.trust_check(tool_id.name(), trust)?; + } + + let trusted_tools: Vec<_> = all_tools + .iter() + .filter(|(_, tool_id)| { + matches!(self.trust_status(tool_id.name()), Ok(TrustStatus::Trusted)) + }) + .collect(); + + for (alias, tool_id) in &trusted_tools { + self.install_exact(tool_id, trust)?; + self.link(alias)?; + } + + if trusted_tools.len() < all_tools.len() { + log::warn!( + "Installed {} tool{} out of {} ({} failed)", + trusted_tools.len(), + if trusted_tools.len() != 1 { "s" } else { "" }, + all_tools.len(), + (all_tools.len() - trusted_tools.len()), + ); } Ok(()) @@ -154,7 +182,7 @@ impl ToolStorage { let installed_path = self.storage_dir.join("installed.txt"); let installed = InstalledToolsCache::read(&installed_path)?; - self.trust_ensure(spec.name(), trust)?; + self.trust_check(spec.name(), trust)?; log::info!("Installing tool: {}", spec); @@ -234,7 +262,7 @@ impl ToolStorage { return Ok(()); } - self.trust_ensure(id.name(), trust)?; + self.trust_check(id.name(), trust)?; log::info!("Installing tool: {id}"); @@ -319,8 +347,9 @@ impl ToolStorage { .collect() } - fn trust_ensure(&self, name: &ToolName, mode: TrustMode) -> anyhow::Result<()> { - let status = self.trust_check(name)?; + fn trust_check(&self, name: &ToolName, mode: TrustMode) -> anyhow::Result<()> { + let status = self.trust_status(name)?; + if status == TrustStatus::NotTrusted { if mode == TrustMode::Check { // If the terminal isn't interactive, tell the user that they @@ -356,7 +385,7 @@ impl ToolStorage { Ok(()) } - fn trust_check(&self, name: &ToolName) -> anyhow::Result { + fn trust_status(&self, name: &ToolName) -> anyhow::Result { let trusted = TrustCache::read(&self.home)?; let is_trusted = trusted.tools.contains(name); if is_trusted { From b812e5bf491da8fae57905ae0d6ef165a11baf34 Mon Sep 17 00:00:00 2001 From: Filip Tibell Date: Sat, 22 Oct 2022 15:48:01 +0200 Subject: [PATCH 3/4] Implement --skip-untrusted flag for `aftman install` * Also let installation install all trusted tools before bailing --- Cargo.lock | 5 +++-- Cargo.toml | 1 + src/cli/mod.rs | 5 ++++- src/tool_storage.rs | 46 +++++++++++++++++++-------------------------- 4 files changed, 27 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 480a7ca..14db2bd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -21,6 +21,7 @@ dependencies = [ "env_logger", "fs-err", "insta", + "itertools", "log", "once_cell", "reqwest", @@ -579,9 +580,9 @@ checksum = "879d54834c8c76457ef4293a689b2a8c59b076067ad77b15efafbb05f92a592b" [[package]] name = "itertools" -version = "0.10.3" +version = "0.10.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a9a9d19fa1e79b6215ff29b9d6880b706147f16e9b1dbb1e4e5947b5b02bc5e3" +checksum = "b0fd2260e829bddf4cb6ea802289de2f86d6a7a690192fbe91b3f46e0f2c8473" dependencies = [ "either", ] diff --git a/Cargo.toml b/Cargo.toml index 2013701..4457439 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,6 +20,7 @@ dialoguer = "0.9.0" dirs = "3.0.2" env_logger = "0.9.0" fs-err = "2.6.0" +itertools = "0.10.5" log = "0.4.14" once_cell = "1.9.0" reqwest = { version = "0.11.4", features = ["blocking"] } diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 3430b56..ff2bafa 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -155,6 +155,9 @@ pub struct InstallSubcommand { /// recommended to only run this on CI machines. #[clap(long)] pub no_trust_check: bool, + /// Skip / don't error if a tool was not trusted during install. + #[clap(long)] + pub skip_untrusted: bool, } impl InstallSubcommand { @@ -165,7 +168,7 @@ impl InstallSubcommand { TrustMode::Check }; - tools.install_all(trust) + tools.install_all(trust, self.skip_untrusted) } } diff --git a/src/tool_storage.rs b/src/tool_storage.rs index 4aeebc6..32ed2ce 100644 --- a/src/tool_storage.rs +++ b/src/tool_storage.rs @@ -9,6 +9,7 @@ use std::path::{Path, PathBuf}; use anyhow::{bail, Context}; use fs_err::File; +use itertools::{Either, Itertools}; use once_cell::unsync::OnceCell; use crate::auth::AuthManifest; @@ -135,43 +136,35 @@ impl ToolStorage { } /// Install all tools from all reachable manifest files. - pub fn install_all(&self, trust: TrustMode) -> anyhow::Result<()> { + pub fn install_all(&self, trust: TrustMode, skip_untrusted: bool) -> anyhow::Result<()> { let current_dir = current_dir().context("Failed to get current working directory")?; let manifests = Manifest::discover(&self.home, ¤t_dir)?; // Installing all tools is split into multiple steps: - // 1. Trust check, which requires user input and may yield + // 1. Trust check, which may prompt the user and yield if untrusted // 2. Installation of trusted tools, which will be in parallel in the future + // 3. Reporting of installation trust errors, unless trust errors are skipped - let all_tools: Vec<_> = manifests + let (trusted_tools, trust_errors): (Vec<_>, Vec<_>) = manifests .iter() .flat_map(|manifest| &manifest.tools) - .collect(); - - for (_, tool_id) in &all_tools { - self.trust_check(tool_id.name(), trust)?; - } - - let trusted_tools: Vec<_> = all_tools - .iter() - .filter(|(_, tool_id)| { - matches!(self.trust_status(tool_id.name()), Ok(TrustStatus::Trusted)) - }) - .collect(); + .partition_map( + |(alias, tool_id)| match self.trust_check(tool_id.name(), trust) { + Ok(_) => Either::Left((alias, tool_id)), + Err(e) => Either::Right(e), + }, + ); for (alias, tool_id) in &trusted_tools { self.install_exact(tool_id, trust)?; self.link(alias)?; } - if trusted_tools.len() < all_tools.len() { - log::warn!( - "Installed {} tool{} out of {} ({} failed)", - trusted_tools.len(), - if trusted_tools.len() != 1 { "s" } else { "" }, - all_tools.len(), - (all_tools.len() - trusted_tools.len()), - ); + if !trust_errors.is_empty() && !skip_untrusted { + bail!( + "Installation trust check failed for the following tools:\n{}", + trust_errors.iter().map(|e| format!(" {e}")).join("\n") + ) } Ok(()) @@ -371,11 +364,10 @@ impl ToolStorage { .interact_opt()?; if let Some(false) | None = proceed { - eprintln!( - "Skipping installation of {} and exiting with an error.", - name + bail!( + "Tool {name} is not trusted. \ + Run `aftman trust {name}` in your terminal to trust it.", ); - std::process::exit(1); } } From 65dbdd4909d6b671e4d203f16b46fd17ea72b62a Mon Sep 17 00:00:00 2001 From: Filip Tibell Date: Sat, 22 Oct 2022 16:11:44 +0200 Subject: [PATCH 4/4] Update README --- README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 1740f0a..0797034 100644 --- a/README.md +++ b/README.md @@ -123,13 +123,15 @@ aftman add rojo-rbx/rojo@6.2.0 rojo6 Usage: ```bash -aftman install [--no-trust-check] +aftman install [--no-trust-check] [--skip-untrusted] ``` Install all tools listed in `aftman.toml` files based on your current directory. If `--no-trust-check` is given, all tools will be installed, regardless of whether they are known. This should generally only be used in CI environments. To trust a specific tool before running `aftman install`, use `aftman trust ` instead. +If `--skip-untrusted` is given, only already trusted tools will be installed, others will be skipped and not emit any errors. + ### `aftman self-install` Usage: