From facd45dcf5a191ffdfddc3bf17816f3e76224eed Mon Sep 17 00:00:00 2001 From: Tobias Hunger Date: Sun, 6 Dec 2020 22:14:44 +0100 Subject: [PATCH] Mage clippy happy! --- gng-build-agent/src/engine.rs | 23 ++- gng-build-agent/src/engine/rhai_modules.rs | 31 +++- gng-build-agent/src/lib.rs | 1 + gng-build-agent/src/main.rs | 22 +-- gng-build-agent/src/source_packet.rs | 9 +- gng-build/src/case_officer.rs | 76 ++++---- gng-build/src/lib.rs | 18 +- gng-build/src/main.rs | 1 + gng-build/src/message_handler.rs | 55 +++--- gng_build_shared/src/lib.rs | 21 +-- gng_build_shared/src/source_packet.rs | 2 +- gng_shared/src/lib.rs | 42 ++++- gng_shared/src/packet.rs | 206 +++++++++++---------- 13 files changed, 288 insertions(+), 219 deletions(-) diff --git a/gng-build-agent/src/engine.rs b/gng-build-agent/src/engine.rs index 4dbc78f..e55163a 100644 --- a/gng-build-agent/src/engine.rs +++ b/gng-build-agent/src/engine.rs @@ -32,27 +32,27 @@ fn register_custom_functionality(engine: &mut rhai::Engine) { // - Custom Functions: // ---------------------------------------------------------------------- -fn version_epoch(input: ImmutableString) -> std::result::Result> { +fn version_epoch(input: &ImmutableString) -> std::result::Result> { let version = Version::try_from(input.to_string()).map_err(|e| e.to_string())?; Ok(Dynamic::from(version.epoch())) } -fn version_upstream(input: ImmutableString) -> std::result::Result> { +fn version_upstream(input: &ImmutableString) -> std::result::Result> { let version = Version::try_from(input.to_string()).map_err(|e| e.to_string())?; Ok(version.upstream().into()) } -fn version_release(input: ImmutableString) -> std::result::Result> { +fn version_release(input: &ImmutableString) -> std::result::Result> { let version = Version::try_from(input.to_string()).map_err(|e| e.to_string())?; Ok(version.release().into()) } -fn hash_algorithm(input: ImmutableString) -> std::result::Result> { +fn hash_algorithm(input: &ImmutableString) -> std::result::Result> { let hash = Hash::try_from(input.to_string()).map_err(|e| e.to_string())?; Ok(hash.algorithm().into()) } -fn hash_value(input: ImmutableString) -> std::result::Result> { +fn hash_value(input: &ImmutableString) -> std::result::Result> { let hash = Hash::try_from(input.to_string()).map_err(|e| e.to_string())?; Ok(hash.value().into()) } @@ -90,6 +90,9 @@ impl<'a> EngineBuilder<'a> { } /// Evaluate a script file + /// + /// # Errors + /// * `Error::Script`: When the build script is invalid pub fn eval_pkgsrc_directory(&mut self) -> crate::Result> { let mut engine = std::mem::replace(&mut self.engine, rhai::Engine::new()); let mut scope = std::mem::replace(&mut self.scope, rhai::Scope::<'a>::new()); @@ -117,7 +120,7 @@ impl<'a> EngineBuilder<'a> { })?; let ast = engine - .compile_file_with_scope(&mut scope, build_file) + .compile_file_with_scope(&scope, build_file) .map_err(|e| { crate::Error::Script( format!("Compilation of build script {} failed", build_file_str), @@ -150,6 +153,9 @@ pub struct Engine<'a> { impl<'a> Engine<'a> { /// Evaluate an expression + /// + /// # Errors + /// * `Error::Script`: When the expression is invalid pub fn evaluate(&mut self, expression: &str) -> crate::Result where T: Clone + Send + Sync + serde::de::DeserializeOwned + 'static, @@ -167,12 +173,15 @@ impl<'a> Engine<'a> { } /// Call a function (without arguments!) + /// + /// # Errors + /// * `Error::Script`: When the function is not defined in rhai script pub fn call(&mut self, name: &str) -> crate::Result where T: Clone + Sync + Send + 'static, { self.engine - .call_fn(&mut self.scope, &mut self.ast, name, ()) + .call_fn(&mut self.scope, &self.ast, name, ()) .map_err(|e| { crate::Error::Script(format!("Failed to call function {}", name), e.to_string()) }) diff --git a/gng-build-agent/src/engine/rhai_modules.rs b/gng-build-agent/src/engine/rhai_modules.rs index 065783a..16eef7b 100644 --- a/gng-build-agent/src/engine/rhai_modules.rs +++ b/gng-build-agent/src/engine/rhai_modules.rs @@ -3,9 +3,14 @@ //! Filesystem related rhai module: -use rhai::plugin::*; -use rhai::EvalAltResult; +// #![allow(clippy::wildcard_imports)] // rhai depends on all symbols being available +use rhai::plugin::{ + export_module, mem, new_vec, CallableFunction, FnAccess, FnNamespace, Module, + NativeCallContext, PluginFunction, TypeId, +}; +use rhai::{Dynamic, EvalAltResult, ImmutableString}; +use std::convert::TryFrom; use std::os::unix::fs::PermissionsExt; // Define a module for filesystem-related tasks. @@ -16,20 +21,30 @@ mod fs_module { mode: rhai::INT, path: &str, ) -> std::result::Result> { - if mode > 0o7777 || mode < 0 { + if !(0..=0o7777).contains(&mode) { return Err(format!("Invalid mode 0o{:o} for {}.", mode, path).into()); } - let mode = mode as u32; - std::fs::set_permissions(path, std::fs::Permissions::from_mode(mode)) - .map_err(|_| format!("Failed to change permissions on {}.", path))?; + let mode = u32::try_from(mode).expect("Was in a safe range just now!"); + std::fs::set_permissions(path, std::fs::Permissions::from_mode(mode)).map_err(|e| { + format!( + "Failed to change permissions on {}: {}", + path, + e.to_string() + ) + })?; Ok(rhai::Dynamic::from(true)) } #[rhai_fn(return_raw)] pub fn mkdir(directory: &str) -> std::result::Result> { - std::fs::create_dir(directory) - .map_err(|_| format!("Failed to create directory {}.", directory))?; + std::fs::create_dir(directory).map_err(|e| { + format!( + "Failed to create directory {}: {}", + directory, + e.to_string() + ) + })?; Ok(rhai::Dynamic::from(true)) } diff --git a/gng-build-agent/src/lib.rs b/gng-build-agent/src/lib.rs index 3bf0712..e29d712 100644 --- a/gng-build-agent/src/lib.rs +++ b/gng-build-agent/src/lib.rs @@ -13,6 +13,7 @@ )] // Clippy: #![warn(clippy::all, clippy::nursery, clippy::pedantic)] +#![allow(clippy::non_ascii_literal, clippy::module_name_repetitions)] // ---------------------------------------------------------------------- // - Error Handling: diff --git a/gng-build-agent/src/main.rs b/gng-build-agent/src/main.rs index e852b8e..b75d7cf 100644 --- a/gng-build-agent/src/main.rs +++ b/gng-build-agent/src/main.rs @@ -13,15 +13,13 @@ )] // Clippy: #![warn(clippy::all, clippy::nursery, clippy::pedantic)] +#![allow(clippy::non_ascii_literal, clippy::module_name_repetitions)] use gng_build_shared::constants::container as cc; use gng_build_shared::constants::environment as ce; -use gng_build_shared::SourcePacket; use structopt::StructOpt; -use std::path::Path; - // - Helpers: // ---------------------------------------------------------------------- @@ -47,14 +45,14 @@ enum Args { } fn get_env(key: &str, default: &str) -> String { - let result = std::env::var(key).unwrap_or(default.to_owned()); + let result = std::env::var(key).unwrap_or_else(|_| default.to_owned()); std::env::remove_var(key); result } fn get_message_prefix() -> String { let message_prefix = - std::env::var(ce::GNG_AGENT_MESSAGE_PREFIX).unwrap_or(String::from("MSG:")); + std::env::var(ce::GNG_AGENT_MESSAGE_PREFIX).unwrap_or_else(|_| String::from("MSG:")); std::env::remove_var(ce::GNG_AGENT_MESSAGE_PREFIX); message_prefix @@ -75,12 +73,6 @@ fn send_message(message_prefix: &str, message_type: &gng_build_shared::MessageTy struct Context<'a> { engine: gng_build_agent::engine::Engine<'a>, - source_packet: SourcePacket, - message_prefix: String, -} - -fn query(ctx: &mut Context) -> eyre::Result<()> { - Ok(()) } fn prepare(ctx: &mut Context) -> eyre::Result<()> { @@ -157,14 +149,10 @@ fn main() -> eyre::Result<()> { &serde_json::to_string(&source_packet)?, ); - let mut ctx = Context { - engine, - source_packet, - message_prefix, - }; + let mut ctx = Context { engine }; match args { - Args::QUERY => query(&mut ctx), + Args::QUERY => Ok(()), Args::PREPARE => prepare(&mut ctx), Args::BUILD => build(&mut ctx), Args::CHECK => check(&mut ctx), diff --git a/gng-build-agent/src/source_packet.rs b/gng-build-agent/src/source_packet.rs index b6f7ed3..222566f 100644 --- a/gng-build-agent/src/source_packet.rs +++ b/gng-build-agent/src/source_packet.rs @@ -10,16 +10,17 @@ use gng_shared::{Name, Version}; // ---------------------------------------------------------------------- /// Create a `SourcePacket` from an `Engine` +/// +/// # Errors +/// Passes along `Error::Script` from the evaluation pub fn from_engine( engine: &mut crate::engine::Engine, ) -> crate::Result { let source_name = engine.evaluate::("source_name")?; let version = engine.evaluate::("version")?; let license = engine.evaluate::("license")?; - let url = engine.evaluate::("url").unwrap_or(String::new()); - let bug_url = engine - .evaluate::("bug_url") - .unwrap_or(String::new()); + let url = engine.evaluate::("url").unwrap_or_default(); + let bug_url = engine.evaluate::("bug_url").unwrap_or_default(); let build_dependencies = engine.evaluate::>("build_dependencies")?; let check_dependencies = engine.evaluate::>("check_dependencies")?; diff --git a/gng-build/src/case_officer.rs b/gng-build/src/case_officer.rs index 9e10319..b75c9b8 100644 --- a/gng-build/src/case_officer.rs +++ b/gng-build/src/case_officer.rs @@ -50,7 +50,7 @@ fn bind(read_only: bool, from: &Path, to: &Path) -> OsString { result } -fn overlay(paths: &Vec) -> OsString { +fn overlay(paths: &[PathBuf]) -> OsString { if paths.is_empty() { return OsString::new(); } @@ -105,12 +105,23 @@ fn find_type_and_contents<'a>(message_prefix: &'a str, line: &'a str) -> (&'a st ) } +fn build_script(pkgsrc_directory: &Path) -> eyre::Result { + let build_file = pkgsrc_directory.join("build.rhai"); + if !build_file.is_file() { + return Err(eyre::eyre!(format!( + "No build.rhai file found in {}.", + pkgsrc_directory.to_string_lossy() + ))); + } + Ok(build_file) +} + fn validate_executable(path: &mut Option) -> eyre::Result { if path.is_none() { return Err(eyre::eyre!("Path is not set.")); } - let path = path.take().expect("It was some just a if ago!").clone(); + let path = path.take().expect("It was some just a if ago!"); if !path.is_file() { return Err(eyre::eyre!( "Path \"{}\" is not a file.", @@ -220,18 +231,10 @@ impl CaseOfficerBuilder { self } - fn build_script(&self, pkgsrc_directory: &Path) -> eyre::Result { - let build_file = pkgsrc_directory.join("build.rhai"); - if !build_file.is_file() { - return Err(eyre::eyre!(format!( - "No build.rhai file found in {}.", - pkgsrc_directory.to_string_lossy() - ))); - } - Ok(build_file) - } - /// Evaluate a script file + /// + /// # Errors + /// Generic Error pub fn build(&mut self, pkgsrc_directory: &Path) -> eyre::Result { let mut temp_dirs = Vec::with_capacity(3); @@ -254,7 +257,7 @@ impl CaseOfficerBuilder { )?; Ok(CaseOfficer { - build_script: self.build_script(pkgsrc_directory)?, + build_script: build_script(pkgsrc_directory)?, nspawn_binary: validate_executable(&mut Some(std::mem::take(&mut self.nspawn_binary)))?, agent: validate_executable(&mut self.agent)?, @@ -321,7 +324,7 @@ impl CaseOfficer { Mode::INSTALL => self.mode_args( true, false, - &mut vec![overlay(&vec![ + &mut vec![overlay(&[ self.root_directory.join("usr"), self.inst_directory.clone(), PathBuf::from("/usr"), @@ -332,7 +335,7 @@ impl CaseOfficer { }; let mut result = vec![ - bind(true, &self.agent, &Path::new("/gng/build-agent")), + bind(true, &self.agent, Path::new("/gng/build-agent")), OsString::from("--quiet"), OsString::from("--settings=off"), OsString::from("--register=off"), @@ -349,25 +352,20 @@ impl CaseOfficer { OsString::from("--read-only"), setenv( ce::GNG_BUILD_AGENT, - &cc::GNG_BUILD_AGENT_EXECUTABLE.to_str().unwrap(), + cc::GNG_BUILD_AGENT_EXECUTABLE.to_str().unwrap(), ), - setenv(ce::GNG_WORK_DIR, &cc::GNG_WORK_DIR.to_str().unwrap()), - setenv(ce::GNG_INST_DIR, &cc::GNG_INST_DIR.to_str().unwrap()), - setenv(ce::GNG_AGENT_MESSAGE_PREFIX, &message_prefix), + setenv(ce::GNG_WORK_DIR, cc::GNG_WORK_DIR.to_str().unwrap()), + setenv(ce::GNG_INST_DIR, cc::GNG_INST_DIR.to_str().unwrap()), + setenv(ce::GNG_AGENT_MESSAGE_PREFIX, message_prefix), ]; - let rust_log = std::env::var("RUST_LOG"); - if rust_log.is_ok() { + if let Ok(rust_log) = std::env::var("RUST_LOG") { let mut env_var = OsString::from("--setenv=RUST_LOG="); - env_var.push(rust_log.unwrap()); + env_var.push(rust_log); result.push(env_var) } - result.push(bind( - true, - &self.build_script, - &Path::new("/gng/build.rhai"), - )); + result.push(bind(true, &self.build_script, Path::new("/gng/build.rhai"))); result.append(&mut extra); @@ -377,7 +375,7 @@ impl CaseOfficer { #[tracing::instrument(level = "debug")] fn run_agent( &mut self, - args: &Vec, + args: &[OsString], new_mode: &Mode, message_prefix: String, ) -> eyre::Result<()> { @@ -390,7 +388,7 @@ impl CaseOfficer { .spawn()?; tracing::trace!("Processing output of gng-build-agent"); - self.handle_agent_output(&mut child, new_mode, message_prefix)?; + self.handle_agent_output(&mut child, new_mode, &message_prefix)?; tracing::trace!("Waiting for gng-build-agent to finish"); let exit_status = child.wait()?; @@ -410,7 +408,7 @@ impl CaseOfficer { fn switch_mode(&mut self, new_mode: &Mode) -> eyre::Result<()> { tracing::debug!("Switching mode to {:?}", new_mode); - for h in self.message_handlers.iter_mut() { + for h in &mut self.message_handlers { h.prepare(new_mode)?; } @@ -422,7 +420,7 @@ impl CaseOfficer { self.run_agent(&nspawn_args, new_mode, message_prefix)?; - for h in self.message_handlers.iter_mut() { + for h in &mut self.message_handlers { h.verify(new_mode)?; } @@ -432,18 +430,18 @@ impl CaseOfficer { fn process_line(&mut self, mode: &Mode, message_prefix: &str, line: &str) -> eyre::Result<()> { lazy_static::lazy_static! { static ref PREFIX: String = - std::env::var(ce::GNG_AGENT_OUTPUT_PREFIX).unwrap_or(String::from("AGENT> ")); + std::env::var(ce::GNG_AGENT_OUTPUT_PREFIX).unwrap_or_else(|_| String::from("AGENT> ")); } - let (message_type, line) = find_type_and_contents(&message_prefix, &line); - if message_type == "" { + let (message_type, line) = find_type_and_contents(message_prefix, line); + if message_type.is_empty() { println!("{}{}", *PREFIX, line); } else { tracing::trace!("{}MSG_{}: {}", *PREFIX, message_type, line); let message_type = gng_build_shared::MessageType::try_from(String::from(message_type)) .map_err(|e| eyre::eyre!(e))?; - for h in self.message_handlers.iter_mut() { + for h in &mut self.message_handlers { if h.handle(mode, &message_type, line)? { break; } @@ -456,7 +454,7 @@ impl CaseOfficer { &mut self, child: &mut std::process::Child, mode: &Mode, - message_prefix: String, + message_prefix: &str, ) -> eyre::Result<()> { let reader = BufReader::new( child @@ -467,7 +465,7 @@ impl CaseOfficer { for line in reader.lines() { match line { - Ok(line) => self.process_line(mode, &message_prefix, &line)?, + Ok(line) => self.process_line(mode, message_prefix, &line)?, Err(e) => return Err(eyre::eyre!(e)), } } @@ -480,7 +478,7 @@ impl CaseOfficer { "Agent failed with exit-status: {}.", exit_code ))), - None => Err(eyre::eyre!(format!("Agent killed by signal."))), + None => Err(eyre::eyre!("Agent killed by signal.")), } } diff --git a/gng-build/src/lib.rs b/gng-build/src/lib.rs index ded7f2e..33d2a3d 100644 --- a/gng-build/src/lib.rs +++ b/gng-build/src/lib.rs @@ -13,6 +13,7 @@ )] // Clippy: #![warn(clippy::all, clippy::nursery, clippy::pedantic)] +#![allow(clippy::non_ascii_literal, clippy::module_name_repetitions)] // ---------------------------------------------------------------------- // - Modes: @@ -37,20 +38,21 @@ pub enum Mode { impl Default for Mode { fn default() -> Self { - Mode::QUERY + Self::QUERY } } impl Mode { /// The next mode to go to - pub fn next(self) -> Option { + #[must_use] + pub const fn next(self) -> Option { match self { - Mode::QUERY => Some(Mode::PREPARE), // default entry point - Mode::PREPARE => Some(Mode::BUILD), // default entry point - Mode::BUILD => Some(Mode::CHECK), - Mode::CHECK => Some(Mode::INSTALL), - Mode::INSTALL => Some(Mode::PACKAGE), - Mode::PACKAGE => None, + Self::QUERY => Some(Self::PREPARE), // default entry point + Self::PREPARE => Some(Self::BUILD), // default entry point + Self::BUILD => Some(Self::CHECK), + Self::CHECK => Some(Self::INSTALL), + Self::INSTALL => Some(Self::PACKAGE), + Self::PACKAGE => None, } } } diff --git a/gng-build/src/main.rs b/gng-build/src/main.rs index 492c96c..2d88e0a 100644 --- a/gng-build/src/main.rs +++ b/gng-build/src/main.rs @@ -13,6 +13,7 @@ )] // Clippy: #![warn(clippy::all, clippy::nursery, clippy::pedantic)] +#![allow(clippy::non_ascii_literal, clippy::module_name_repetitions)] use std::path::PathBuf; diff --git a/gng-build/src/message_handler.rs b/gng-build/src/message_handler.rs index 612f3e3..328ca16 100644 --- a/gng-build/src/message_handler.rs +++ b/gng-build/src/message_handler.rs @@ -9,16 +9,31 @@ use sha3::{Digest, Sha3_256}; // - Helper: // ---------------------------------------------------------------------- +fn hash_str(input: &str) -> Vec { + let mut hasher = Sha3_256::new(); + hasher.update(input.as_bytes()); + let mut v = Vec::with_capacity(Sha3_256::output_size()); + v.extend_from_slice(&hasher.finalize()); + + v +} + // ---------------------------------------------------------------------- // - Message Handler: // ---------------------------------------------------------------------- /// An object used to handle messages from the `gng-build-agent` pub trait MessageHandler { - /// Verify output of `gng-build-agent` after that has quit successfully: + /// Verify state before `gng-build-agent` is started + /// + /// # Errors + /// Generic Error fn prepare(&mut self, mode: &crate::Mode) -> eyre::Result<()>; - /// Handle one message + /// Handle one message from `gng-build-agent` + /// + /// # Errors + /// Generic Error fn handle( &mut self, mode: &crate::Mode, @@ -26,7 +41,10 @@ pub trait MessageHandler { message: &str, ) -> eyre::Result; - /// Verify output of `gng-build-agent` after that has quit successfully: + /// Verify state after `gng-build-agent` has quit successfully + /// + /// # Errors + /// Generic Error fn verify(&mut self, mode: &crate::Mode) -> eyre::Result<()>; } @@ -41,20 +59,9 @@ pub struct ImmutableSourceDataHandler { first_message: bool, } -impl ImmutableSourceDataHandler { - fn hash_message(&mut self, message: &str) -> Vec { - let mut hasher = Sha3_256::new(); - hasher.update(message.as_bytes()); - let mut v = Vec::with_capacity(Sha3_256::output_size()); - v.extend_from_slice(&hasher.finalize()); - - v - } -} - impl Default for ImmutableSourceDataHandler { fn default() -> Self { - ImmutableSourceDataHandler { + Self { hash: None, first_message: true, } @@ -63,7 +70,7 @@ impl Default for ImmutableSourceDataHandler { impl MessageHandler for ImmutableSourceDataHandler { #[tracing::instrument(level = "trace")] - fn prepare(&mut self, _mode: &crate::Mode) -> eyre::Result<()> { + fn prepare(&mut self, mode: &crate::Mode) -> eyre::Result<()> { self.first_message = true; Ok(()) } @@ -71,7 +78,7 @@ impl MessageHandler for ImmutableSourceDataHandler { #[tracing::instrument(level = "trace")] fn handle( &mut self, - _mode: &crate::Mode, + mode: &crate::Mode, message_type: &gng_build_shared::MessageType, message: &str, ) -> eyre::Result { @@ -87,16 +94,14 @@ impl MessageHandler for ImmutableSourceDataHandler { self.first_message = false; - let v = self.hash_message(&message); + let v = hash_str(message); match self.hash.as_ref() { None => { self.hash = Some(v); - return Ok(false); - } - Some(vg) if *vg == v => { - return Ok(false); + Ok(false) } + Some(vg) if *vg == v => Ok(false), Some(_) => { tracing::error!("Source data changed, aborting!"); panic!("gng-build-agent did not react as expected!"); @@ -105,7 +110,7 @@ impl MessageHandler for ImmutableSourceDataHandler { } #[tracing::instrument(level = "trace")] - fn verify(&mut self, _mode: &crate::Mode) -> eyre::Result<()> { + fn verify(&mut self, mode: &crate::Mode) -> eyre::Result<()> { if self.first_message { tracing::error!("The build agent did not send any message!"); panic!("gng-build-agent did not react as expected!"); @@ -136,7 +141,7 @@ impl PacketHandler { impl Default for PacketHandler { fn default() -> Self { - PacketHandler { + Self { source_packet: None, } } @@ -157,7 +162,7 @@ impl MessageHandler for PacketHandler { return Ok(false); } - self.source_packet = Some(serde_json::from_str(&message).map_err(|e| eyre::eyre!(e))?); + self.source_packet = Some(serde_json::from_str(message).map_err(|e| eyre::eyre!(e))?); Ok(false) } diff --git a/gng_build_shared/src/lib.rs b/gng_build_shared/src/lib.rs index 4f18cce..647e4a1 100644 --- a/gng_build_shared/src/lib.rs +++ b/gng_build_shared/src/lib.rs @@ -13,6 +13,7 @@ )] // Clippy: #![warn(clippy::all, clippy::nursery, clippy::pedantic)] +#![allow(clippy::non_ascii_literal, clippy::module_name_repetitions)] // ---------------------------------------------------------------------- // - Constants: @@ -38,16 +39,16 @@ pub mod constants { /// Environment variable names: pub mod environment { - /// GNG_BUILD_AGENT: + /// `GNG_BUILD_AGENT` environment variable name pub const GNG_BUILD_AGENT: &str = "GNG_BUILD_AGENT"; - /// GNG_SRC_DIR: + /// `GNG_WORK_DIR` environment variable name pub const GNG_WORK_DIR: &str = "GNG_WORK_DIR"; - /// GNG_INST_DIR: + /// `GNG_INST_DIR` environment variable name pub const GNG_INST_DIR: &str = "GNG_INST_DIR"; - /// GNG_AGENT_MESSAGE_PREFIX: + /// `GNG_AGENT_MESSAGE_PREFIX` environment variable name pub const GNG_AGENT_MESSAGE_PREFIX: &str = "GNG_AGENT_MESSAGE_PREFIX"; - /// A prefix to mark up normal messages as originating in `gng-build-agent` + /// `GNG_AGENT_OUTPUT_PREFIX` environment variable name pub const GNG_AGENT_OUTPUT_PREFIX: &str = "GNG_AGENT_OUTPUT_PREFIX"; } } @@ -64,7 +65,7 @@ impl std::convert::TryFrom for MessageType { fn try_from(value: String) -> Result { if &value == "DATA" { - Ok(MessageType::DATA) + Ok(Self::DATA) } else { Err(format!("Failed to convert {} to MessageType", value)) } @@ -73,11 +74,9 @@ impl std::convert::TryFrom for MessageType { impl std::convert::From<&MessageType> for String { fn from(mt: &MessageType) -> Self { - let result = match mt { - MessageType::DATA => String::from("DATA"), - }; - assert_eq!(result.len(), 4); - result + match mt { + MessageType::DATA => Self::from("DATA"), + } } } diff --git a/gng_build_shared/src/source_packet.rs b/gng_build_shared/src/source_packet.rs index 18dba05..8a8a75a 100644 --- a/gng_build_shared/src/source_packet.rs +++ b/gng_build_shared/src/source_packet.rs @@ -7,7 +7,7 @@ use gng_shared::{GpgKeyId, Hash, Name, Version}; // - Helper: // ---------------------------------------------------------------------- -fn always_true() -> bool { +const fn always_true() -> bool { true } diff --git a/gng_shared/src/lib.rs b/gng_shared/src/lib.rs index 9bd0a8c..b141548 100644 --- a/gng_shared/src/lib.rs +++ b/gng_shared/src/lib.rs @@ -13,6 +13,7 @@ )] // Clippy: #![warn(clippy::all, clippy::nursery, clippy::pedantic)] +#![allow(clippy::non_ascii_literal, clippy::module_name_repetitions)] use std::os::unix::fs::PermissionsExt; @@ -25,7 +26,7 @@ use std::os::unix::fs::PermissionsExt; pub enum Error { /// Conversion error. #[error("Conversion error: {0}")] - Conversion(&'static str), + Conversion(String), /// Not sure what actually went wrong... #[error("unknown error")] @@ -40,11 +41,13 @@ pub type Result = std::result::Result; // ---------------------------------------------------------------------- /// Return `true` if the program is run by the `root` user. +#[must_use] pub fn is_root() -> bool { nix::unistd::Uid::effective().is_root() } /// Return `true` if the `path` is executable +#[must_use] pub fn is_executable(path: &std::path::Path) -> bool { match std::fs::metadata(path) { Err(_) => false, @@ -52,6 +55,43 @@ pub fn is_executable(path: &std::path::Path) -> bool { } } +/// Return `true` if all characters are lowercase 'a' to 'z', '0' to '9' or '_' +#[must_use] +pub fn all_name_chars(input: &str) -> bool { + input + .chars() + .all(|c| ('a'..='z').contains(&c) || ('0'..='9').contains(&c) || (c == '_')) +} + +/// Return `true` if all characters are lowercase 'a' to 'z', '0' to '9', '.' or '_' +#[must_use] +pub fn all_version_chars(input: &str) -> bool { + input + .chars() + .all(|c| ('a'..='z').contains(&c) || ('0'..='9').contains(&c) || (c == '_') || (c == '.')) +} + +/// Return `true` if all characters are (lc) hex digits or separators like '-', ' ' or '_' +#[must_use] +pub fn all_hex_or_separator(input: &str) -> bool { + input.chars().all(|c| { + ('0'..='9').contains(&c) + || ('a'..='f').contains(&c) + || (c == ' ') + || (c == '-') + || (c == '_') + }) +} + +/// Return `true` if all characters are lowercase 'a' to 'z', '0' to '9', '.' or '_' +#[must_use] +pub fn start_alnum_char(input: &str) -> bool { + input + .chars() + .take(1) + .all(|c| ('a'..='z').contains(&c) || ('0'..='9').contains(&c)) +} + // ---------------------------------------------------------------------- // - Sub-Modules: // ---------------------------------------------------------------------- diff --git a/gng_shared/src/packet.rs b/gng_shared/src/packet.rs index dd08ec7..58cde3c 100644 --- a/gng_shared/src/packet.rs +++ b/gng_shared/src/packet.rs @@ -16,14 +16,14 @@ pub struct GpgKeyId(String); impl GpgKeyId { /// Create a new `GpgKeyId` from a string input - pub fn new(value: &str) -> crate::Result { + /// + /// # Errors + /// * `Error::Conversion`: When the input string is not a valid GPG Key ID + pub fn new(value: &str) -> crate::Result { let value = value.to_lowercase(); - if !value - .chars() - .all(|c| (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') || (c == ' ') || (c == '-')) - { + if !crate::all_hex_or_separator(&value) { return Err(crate::Error::Conversion( - "A GPG Key ID must be hex with optional ' ' or '-' characters.", + "A GPG Key ID must be hex with optional ' ' or '-' characters.".into(), )); } let value = value @@ -35,10 +35,10 @@ impl GpgKeyId { .join(" "); if value.chars().count() != (16 + 3) { return Err(crate::Error::Conversion( - "A GPG Key ID must contain 16 hex digits.", + "A GPG Key ID must contain 16 hex digits.".into(), )); } - Ok(GpgKeyId(value)) + Ok(Self(value)) } } @@ -52,7 +52,7 @@ impl std::convert::TryFrom for GpgKeyId { type Error = crate::Error; fn try_from(value: String) -> Result { - GpgKeyId::new(&value[..]) + Self::new(&value[..]) } } @@ -76,11 +76,14 @@ fn to_hex(input: &[u8]) -> String { fn from_hex(input: &str, output: &mut [u8]) -> Result<(), crate::Error> { if input.len() != output.len() * 2 { - return Err(crate::Error::Conversion("Hash value length is invalid.")); + return Err(crate::Error::Conversion( + "Hash value length is invalid.".into(), + )); } for i in 0..output.len() { - output[i] = u8::from_str_radix(&input[(i * 2)..(i * 2) + 2], 16) - .map_err(|_| crate::Error::Conversion("Hash value must be hex characters only."))?; + output[i] = u8::from_str_radix(&input[(i * 2)..(i * 2) + 2], 16).map_err(|e| { + crate::Error::Conversion(format!("Hex conversion failed: {}", e.to_string())) + })?; } Ok(()) @@ -99,42 +102,51 @@ pub enum Hash { } impl Hash { - /// Create a 'Hash::NONE` - pub fn none() -> crate::Result { - Ok(Hash::NONE()) + /// Create a `NONE` hash + #[must_use] + pub const fn none() -> Self { + Self::NONE() } - /// Create a `Hash::SHA256` - pub fn sha256(value: &str) -> crate::Result { + /// Create a `SHA256` hash + /// + /// # Errors + /// * `Error::Conversion`: When the input string is not a valid `Hash` + pub fn sha256(value: &str) -> crate::Result { let mut v = [0_u8; 32]; - from_hex(&value, &mut v)?; + from_hex(value, &mut v)?; - Ok(Hash::SHA256(v)) + Ok(Self::SHA256(v)) } - /// Create a `Hash::SHA512` - pub fn sha512(value: &str) -> crate::Result { + /// Create a `SHA512` hash + /// + /// # Errors + /// * `Error::Conversion`: When the input string is not a valid `Hash` + pub fn sha512(value: &str) -> crate::Result { let mut v = [0_u8; 64]; - from_hex(&value, &mut v)?; + from_hex(value, &mut v)?; - Ok(Hash::SHA512(v)) + Ok(Self::SHA512(v)) } /// The hash algorithm + #[must_use] pub fn algorithm(&self) -> String { match self { - Hash::NONE() => String::from("none"), - Hash::SHA256(_) => String::from("sha256"), - Hash::SHA512(_) => String::from("sha512"), + Self::NONE() => String::from("none"), + Self::SHA256(_) => String::from("sha256"), + Self::SHA512(_) => String::from("sha512"), } } /// The hash value + #[must_use] pub fn value(&self) -> String { match self { - Hash::NONE() => String::new(), - Hash::SHA256(v) => to_hex(&v[..]), - Hash::SHA512(v) => to_hex(&v[..]), + Self::NONE() => String::new(), + Self::SHA256(v) => to_hex(&v[..]), + Self::SHA512(v) => to_hex(&v[..]), } } } @@ -151,27 +163,28 @@ impl std::convert::TryFrom for Hash { fn try_from(value: String) -> Result { let value = value.to_lowercase(); if value == "none" { - Hash::none() - } else if value.starts_with("sha256:") { - Hash::sha256(&value[7..]) - } else if value.starts_with("sha512:") { - Hash::sha512(&value[7..]) - } else { - Err(crate::Error::Conversion("Unsupported hash type.")) + return Ok(Self::none()); + } + if let Some(v) = value.strip_prefix("sha256:") { + return Self::sha256(v); + } + if let Some(v) = value.strip_prefix("sha512:") { + return Self::sha512(v); } + Err(crate::Error::Conversion("Unsupported hash type.".into())) } } impl std::default::Default for Hash { fn default() -> Self { - Hash::NONE() + Self::NONE() } } impl std::fmt::Display for Hash { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match *self { - Hash::NONE() => write!(f, "{}", self.algorithm()), + Self::NONE() => write!(f, "{}", self.algorithm()), _ => write!(f, "{}:{}", self.algorithm(), self.value()), } } @@ -188,34 +201,33 @@ pub struct Name(String); impl Name { /// Create a package 'Name' from a '&str' - pub fn new(value: &str) -> crate::Result { + /// + /// # Errors + /// * `Error::Conversion`: When the input string is not a valid `Name` + pub fn new(value: &str) -> crate::Result { if value.is_empty() { - return Err(crate::Error::Conversion(&"Package name can not be empty.")); + return Err(crate::Error::Conversion( + "Package name can not be empty.".into(), + )); } - if !value - .chars() - .take(1) - .all(|c| (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9')) - { + if !crate::start_alnum_char(value) { return Err(crate::Error::Conversion( - &"Package name must start with a number or lowercase letter.", + "Package name must start with a number or lowercase letter.".into(), )); } - if !value - .chars() - .all(|c| (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || (c == '_')) - { + if !crate::all_name_chars(value) { return Err(crate::Error::Conversion( - &"Package name must consist of numbers, lowercase letter or '_' characters only.", + "Package name must consist of numbers, lowercase letter or '_' characters only." + .into(), )); } - Ok(Name(value.to_string())) + Ok(Self(value.to_string())) } } impl std::convert::From for String { fn from(name: Name) -> Self { - name.0.clone() + name.0 } } @@ -223,7 +235,7 @@ impl std::convert::TryFrom for Name { type Error = crate::Error; fn try_from(value: String) -> Result { - Name::new(&value[..]) + Self::new(&value[..]) } } @@ -251,48 +263,37 @@ pub struct Version { impl Version { /// Create a package `Version` from an `epoch`, a `version` and an `release` - pub fn new(epoch: u32, upstream: &str, release: &str) -> crate::Result { + /// + /// # Errors + /// * `Error::Conversion`: When the input string is not a valid `Version` + pub fn new(epoch: u32, upstream: &str, release: &str) -> crate::Result { if upstream.is_empty() { return Err(crate::Error::Conversion( - "Version part of a package version can not be empty.", + "Version part of a package version can not be empty.".into(), )); } - if !upstream - .chars() - .all(|c| (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || (c == '_') || (c == '.')) - { + if !crate::all_version_chars(upstream) { return Err(crate::Error::Conversion( - &"Package version must consist of numbers, lowercase letters, '.' or '_' characters only.", + "Package version must consist of numbers, lowercase letters, '.' or '_' characters only.".into(), )); } - if !upstream - .chars() - .take(1) - .all(|c| (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9')) - { + if !crate::start_alnum_char(upstream) { return Err(crate::Error::Conversion( - &"Package version must start with a numbers or lowercase letter.", + "Package version must start with a numbers or lowercase letter.".into(), )); } - if !release - .chars() - .all(|c| (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || (c == '_') || (c == '.')) - { + if !crate::all_version_chars(release) { return Err(crate::Error::Conversion( - &"Package version release must consist of numbers, lowercase letters, '.' or '_' characters only.", + "Package version release must consist of numbers, lowercase letters, '.' or '_' characters only.".into(), )); } - if !release - .chars() - .take(1) - .all(|c| (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9')) - { + if !crate::start_alnum_char(release) { return Err(crate::Error::Conversion( - &"Package version release must start with a numbers or lowercase letter.", + "Package version release must start with a numbers or lowercase letter.".into(), )); } - Ok(Version { + Ok(Self { epoch, upstream: upstream.to_string(), release: release.to_string(), @@ -300,15 +301,19 @@ impl Version { } /// Return the epoch of a `Version` - pub fn epoch(&self) -> u32 { + #[must_use] + pub const fn epoch(&self) -> u32 { self.epoch } /// Return the epoch of a `Version` + #[must_use] pub fn upstream(&self) -> String { self.upstream.clone() } + /// Return the epoch of a `Version` + #[must_use] pub fn release(&self) -> String { self.release.clone() } @@ -325,34 +330,39 @@ impl std::convert::TryFrom for Version { fn try_from(value: String) -> Result { let epoch; - let version; + let upstream; let release; - let input = &value[..]; - - let mut index = input.chars().position(|c| c == ':').unwrap_or(0); - if index > 0 { - epoch = input[..index] + let epoch_upstream_release = &value[..]; + let mut colon_index = epoch_upstream_release + .chars() + .position(|c| c == ':') + .unwrap_or(0); + if colon_index > 0 { + epoch = epoch_upstream_release[..colon_index] .parse::() - .or(Err(crate::Error::Conversion( - "Invalid epoch value in version string found.", - )))?; - index += 1; + .map_err(|e| { + crate::Error::Conversion(format!("Invalid epoch value: {}", e.to_string())) + })?; + colon_index += 1; } else { epoch = 0; } - let input = &value[index..]; - let index = input.chars().position(|c| c == '-').unwrap_or(0); - if index > 0 { - version = &input[..index]; - release = &input[(index + 1)..]; + let upstream_and_release = &value[colon_index..]; + let dash_index = upstream_and_release + .chars() + .position(|c| c == '-') + .unwrap_or(0); + if dash_index > 0 { + upstream = &upstream_and_release[..dash_index]; + release = &upstream_and_release[(dash_index + 1)..]; } else { - version = input; + upstream = upstream_and_release; release = ""; } - Version::new(epoch, version, release) + Self::new(epoch, upstream, release) } } @@ -446,7 +456,7 @@ mod tests { #[test] fn test_package_hash_ok() { - assert_eq!(Hash::none().unwrap(), Hash::NONE()); + assert_eq!(Hash::none(), Hash::NONE()); assert_eq!( Hash::sha256("000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f")