From e9f60030d76010576ef2611ce4d740c8fc4d0474 Mon Sep 17 00:00:00 2001 From: thirteenowls Date: Fri, 10 May 2024 03:54:11 +0200 Subject: [PATCH 1/3] Remove unnecessary `Result`s from return types of infallible functions --- src/bin/commands/run.rs | 2 +- src/config.rs | 95 ++++++++++++++++++++--------------------- src/docker/shared.rs | 25 ++++------- src/lib.rs | 6 +-- 4 files changed, 59 insertions(+), 69 deletions(-) diff --git a/src/bin/commands/run.rs b/src/bin/commands/run.rs index e6ad62aff..acd6fd9a0 100644 --- a/src/bin/commands/run.rs +++ b/src/bin/commands/run.rs @@ -66,7 +66,7 @@ impl Run { let image = match docker::get_image(&config, &target, false) { Ok(i) => i, Err(docker::GetImageError::NoCompatibleImages(..)) - if config.dockerfile(&target)?.is_some() => + if config.dockerfile(&target).is_some() => { "scratch".into() } diff --git a/src/config.rs b/src/config.rs index ed8dbcf36..fac620efd 100644 --- a/src/config.rs +++ b/src/config.rs @@ -277,7 +277,7 @@ impl Config { env: impl for<'a> Fn(&'a Environment, &Target) -> (Option>, Option>), config: impl for<'a> Fn(&'a CrossToml, &Target) -> (Option<&'a [String]>, Option<&'a [String]>), sum: bool, - ) -> Result>> { + ) -> Option> { if sum { let (mut env_build, env_target) = env(&self.env, target); env_build @@ -294,14 +294,14 @@ impl Config { target: &Target, env: impl for<'a> Fn(&'a Environment, &Target) -> (Option, Option), config: impl for<'a> Fn(&'a CrossToml, &Target) -> (Option<&'a U>, Option<&'a U>), - ) -> Result> + ) -> Option where U: ToOwned + ?Sized, { let (env_build, env_target) = env(&self.env, target); if let Some(env_target) = env_target { - return Ok(Some(env_target)); + return Some(env_target); } let (build, target) = self @@ -312,13 +312,13 @@ impl Config { // FIXME: let expression if target.is_none() && env_build.is_some() { - return Ok(env_build); + return env_build; } if target.is_none() { - Ok(build.map(ToOwned::to_owned)) + build.map(ToOwned::to_owned) } else { - Ok(target.map(ToOwned::to_owned)) + target.map(ToOwned::to_owned) } } @@ -327,11 +327,11 @@ impl Config { target: &Target, env: impl Fn(&Environment, &Target) -> (Option, Option), config: impl Fn(&CrossToml, &Target) -> (Option, Option), - ) -> Result> { + ) -> Option { let (env_build, env_target) = env(&self.env, target); if let Some(env_target) = env_target { - return Ok(Some(env_target)); + return Some(env_target); } let (build, target) = self @@ -342,13 +342,13 @@ impl Config { // FIXME: let expression if target.is_none() && env_build.is_some() { - return Ok(env_build); + return env_build; } if target.is_none() { - Ok(build) + build } else { - Ok(target) + target } } @@ -361,7 +361,7 @@ impl Config { self.bool_from_config(target, Environment::xargo, CrossToml::xargo) } - pub fn build_std(&self, target: &Target) -> Result> { + pub fn build_std(&self, target: &Target) -> Option { self.get_from_ref(target, Environment::build_std, CrossToml::build_std) } @@ -369,25 +369,25 @@ impl Config { self.bool_from_config(target, Environment::zig, CrossToml::zig) } - pub fn zig_version(&self, target: &Target) -> Result> { + pub fn zig_version(&self, target: &Target) -> Option { self.get_from_value(target, Environment::zig_version, CrossToml::zig_version) } pub fn zig_image(&self, target: &Target) -> Result> { let (b, t) = self.env.zig_image(target)?; - self.get_from_value(target, |_, _| (b.clone(), t.clone()), CrossToml::zig_image) + Ok(self.get_from_value(target, |_, _| (b.clone(), t.clone()), CrossToml::zig_image)) } pub fn image(&self, target: &Target) -> Result> { let env = self.env.image(target)?; - self.get_from_ref( + Ok(self.get_from_ref( target, move |_, _| (None, env.clone()), |toml, target| (None, toml.image(target)), - ) + )) } - pub fn runner(&self, target: &Target) -> Result> { + pub fn runner(&self, target: &Target) -> Option { self.get_from_ref( target, |env, target| (None, env.runner(target)), @@ -411,7 +411,7 @@ impl Config { self.env.build_opts() } - pub fn env_passthrough(&self, target: &Target) -> Result>> { + pub fn env_passthrough(&self, target: &Target) -> Option> { self.vec_from_config( target, Environment::passthrough, @@ -420,7 +420,7 @@ impl Config { ) } - pub fn env_volumes(&self, target: &Target) -> Result>> { + pub fn env_volumes(&self, target: &Target) -> Option> { self.get_from_ref(target, Environment::volumes, CrossToml::env_volumes) } @@ -433,11 +433,11 @@ impl Config { .and_then(|t| t.default_target(target_list)) } - pub fn dockerfile(&self, target: &Target) -> Result> { + pub fn dockerfile(&self, target: &Target) -> Option { self.get_from_ref(target, Environment::dockerfile, CrossToml::dockerfile) } - pub fn dockerfile_context(&self, target: &Target) -> Result> { + pub fn dockerfile_context(&self, target: &Target) -> Option { self.get_from_ref( target, Environment::dockerfile_context, @@ -445,17 +445,14 @@ impl Config { ) } - pub fn dockerfile_build_args( - &self, - target: &Target, - ) -> Result>> { + pub fn dockerfile_build_args(&self, target: &Target) -> Option> { // This value does not support env variables self.toml .as_ref() - .map_or(Ok(None), |t| Ok(t.dockerfile_build_args(target))) + .and_then(|t| t.dockerfile_build_args(target)) } - pub fn pre_build(&self, target: &Target) -> Result> { + pub fn pre_build(&self, target: &Target) -> Option { self.get_from_ref(target, Environment::pre_build, CrossToml::pre_build) } @@ -464,7 +461,7 @@ impl Config { &'a self, env_values: Option>, toml_getter: impl FnOnce(&'a CrossToml) -> (Option<&'a [String]>, Option<&'a [String]>), - ) -> Result>> { + ) -> Option> { let mut defined = false; let mut collect = vec![]; if let Some(vars) = env_values { @@ -481,9 +478,9 @@ impl Config { } } if !defined { - Ok(None) + None } else { - Ok(Some(collect)) + Some(collect) } } } @@ -635,9 +632,9 @@ mod tests { let env = Environment::new(Some(map)); let config = Config::new_with(Some(toml(TOML_BUILD_XARGO_FALSE)?), env); assert_eq!(config.xargo(&target()), Some(true)); - assert_eq!(config.build_std(&target())?, None); + assert_eq!(config.build_std(&target()), None); assert_eq!( - config.pre_build(&target())?, + config.pre_build(&target()), Some(PreBuild::Lines(vec![ s!("apt-get update"), s!("apt-get install zlib-dev") @@ -657,10 +654,10 @@ mod tests { let config = Config::new_with(Some(toml(TOML_TARGET_XARGO_FALSE)?), env); assert_eq!(config.xargo(&target()), Some(true)); assert_eq!( - config.build_std(&target())?, + config.build_std(&target()), Some(BuildStd::Crates(vec!["core".to_owned()])) ); - assert_eq!(config.pre_build(&target())?, None); + assert_eq!(config.pre_build(&target()), None); Ok(()) } @@ -673,8 +670,8 @@ mod tests { let env = Environment::new(Some(map)); let config = Config::new_with(Some(toml(TOML_BUILD_XARGO_FALSE)?), env); assert_eq!(config.xargo(&target()), Some(true)); - assert_eq!(config.build_std(&target())?, None); - assert_eq!(config.pre_build(&target())?, None); + assert_eq!(config.build_std(&target()), None); + assert_eq!(config.pre_build(&target()), None); Ok(()) } @@ -690,7 +687,7 @@ mod tests { let env = Environment::new(Some(map)); let config = Config::new_with(Some(toml(TOML_BUILD_PRE_BUILD)?), env); assert_eq!( - config.pre_build(&target())?, + config.pre_build(&target()), Some(PreBuild::Single { line: s!("dpkg --add-architecture arm64"), env: true @@ -711,14 +708,14 @@ mod tests { let env = Environment::new(Some(map)); let config = Config::new_with(Some(toml(TOML_BUILD_DOCKERFILE)?), env); - assert_eq!(config.dockerfile(&target())?, Some(s!("Dockerfile4"))); - assert_eq!(config.dockerfile(&target2())?, Some(s!("Dockerfile3"))); + assert_eq!(config.dockerfile(&target()), Some(s!("Dockerfile4"))); + assert_eq!(config.dockerfile(&target2()), Some(s!("Dockerfile3"))); let map = HashMap::new(); let env = Environment::new(Some(map)); let config = Config::new_with(Some(toml(TOML_BUILD_DOCKERFILE)?), env); - assert_eq!(config.dockerfile(&target())?, Some(s!("Dockerfile2"))); - assert_eq!(config.dockerfile(&target2())?, Some(s!("Dockerfile1"))); + assert_eq!(config.dockerfile(&target()), Some(s!("Dockerfile2"))); + assert_eq!(config.dockerfile(&target2()), Some(s!("Dockerfile1"))); Ok(()) } @@ -729,11 +726,11 @@ mod tests { let env = Environment::new(Some(map)); let config = Config::new_with(Some(toml(TOML_ARRAYS_BOTH)?), env); assert_eq!( - config.env_passthrough(&target())?, + config.env_passthrough(&target()), Some(vec![s!("VAR1"), s!("VAR2"), s!("VAR3"), s!("VAR4")]) ); assert_eq!( - config.env_volumes(&target())?, + config.env_volumes(&target()), Some(vec![s!("VOLUME3"), s!("VOLUME4")]) ); @@ -746,11 +743,11 @@ mod tests { let env = Environment::new(Some(map)); let config = Config::new_with(Some(toml(TOML_ARRAYS_BUILD)?), env); assert_eq!( - config.env_passthrough(&target())?, + config.env_passthrough(&target()), Some(vec![s!("VAR1"), s!("VAR2")]) ); assert_eq!( - config.env_volumes(&target())?, + config.env_volumes(&target()), Some(vec![s!("VOLUME1"), s!("VOLUME2")]) ); @@ -763,11 +760,11 @@ mod tests { let env = Environment::new(Some(map)); let config = Config::new_with(Some(toml(TOML_ARRAYS_TARGET)?), env); assert_eq!( - config.env_passthrough(&target())?, + config.env_passthrough(&target()), Some(vec![s!("VAR3"), s!("VAR4")]) ); assert_eq!( - config.env_volumes(&target())?, + config.env_volumes(&target()), Some(vec![s!("VOLUME3"), s!("VOLUME4")]) ); @@ -782,7 +779,7 @@ mod tests { let config = Config::new_with(Some(toml(TOML_BUILD_VOLUMES)?), env); let expected = [s!("VOLUME1"), s!("VOLUME2")]; - let result = config.env_volumes(&target()).unwrap().unwrap_or_default(); + let result = config.env_volumes(&target()).unwrap_or_default(); dbg!(&result); assert!(result.len() == 2); assert!(result.contains(&expected[0])); @@ -798,7 +795,7 @@ mod tests { let config = Config::new_with(Some(toml(TOML_BUILD_VOLUMES)?), env); let expected = [s!("VOLUME3"), s!("VOLUME4")]; - let result = config.env_volumes(&target()).unwrap().unwrap_or_default(); + let result = config.env_volumes(&target()).unwrap_or_default(); dbg!(&result); assert!(result.len() == 2); assert!(result.contains(&expected[0])); diff --git a/src/docker/shared.rs b/src/docker/shared.rs index 85ee9d850..c519d59e6 100644 --- a/src/docker/shared.rs +++ b/src/docker/shared.rs @@ -72,15 +72,8 @@ impl DockerOptions { #[must_use] pub fn needs_custom_image(&self) -> bool { - self.config - .dockerfile(&self.target) - .unwrap_or_default() - .is_some() - || self - .config - .pre_build(&self.target) - .unwrap_or_default() - .is_some() + self.config.dockerfile(&self.target).is_some() + || self.config.pre_build(&self.target).is_some() } pub(crate) fn custom_image_build( @@ -93,8 +86,8 @@ impl DockerOptions { msg_info.note("cannot install armhf system packages via apt for `arm-unknown-linux-gnueabihf`, since they are for ARMv7a targets but this target is ARMv6. installation of all packages for the armhf architecture has been blocked.")?; } - if let Some(path) = self.config.dockerfile(&self.target)? { - let context = self.config.dockerfile_context(&self.target)?; + if let Some(path) = self.config.dockerfile(&self.target) { + let context = self.config.dockerfile_context(&self.target); let is_custom_image = self.config.image(&self.target)?.is_some(); @@ -114,13 +107,13 @@ impl DockerOptions { self, paths, self.config - .dockerfile_build_args(&self.target)? + .dockerfile_build_args(&self.target) .unwrap_or_default(), msg_info, ) .wrap_err("when building dockerfile")?; } - let pre_build = self.config.pre_build(&self.target)?; + let pre_build = self.config.pre_build(&self.target); if let Some(pre_build) = pre_build { match pre_build { @@ -1014,7 +1007,7 @@ impl DockerCommandExt for Command { let mut warned = false; for ref var in options .config - .env_passthrough(&options.target)? + .env_passthrough(&options.target) .unwrap_or_default() { validate_env_var( @@ -1030,7 +1023,7 @@ impl DockerCommandExt for Command { self.args(["-e", var]); } - let runner = options.config.runner(&options.target)?; + let runner = options.config.runner(&options.target); let cross_runner = format!("CROSS_RUNNER={}", runner.unwrap_or_default()); self.args(["-e", &format!("XARGO_HOME={}", dirs.xargo_mount_path())]) .args(["-e", &format!("CARGO_HOME={}", dirs.cargo_mount_path())]) @@ -1164,7 +1157,7 @@ impl DockerCommandExt for Command { let mut warned = false; for ref var in options .config - .env_volumes(&options.target)? + .env_volumes(&options.target) .unwrap_or_default() { let (var, value) = validate_env_var( diff --git a/src/lib.rs b/src/lib.rs index aaf015a38..33061ff32 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -771,14 +771,14 @@ pub fn setup( .clone() .or_else(|| config.target(&target_list)) .unwrap_or_else(|| Target::from(host.triple(), &target_list)); - let build_std = config.build_std(&target)?.unwrap_or_default(); + let build_std = config.build_std(&target).unwrap_or_default(); let uses_xargo = !build_std.enabled() && config.xargo(&target).unwrap_or(!target.is_builtin()); let uses_zig = config.zig(&target).unwrap_or(false); - let zig_version = config.zig_version(&target)?; + let zig_version = config.zig_version(&target); let image = match docker::get_image(&config, &target, uses_zig) { Ok(i) => i, Err(docker::GetImageError::NoCompatibleImages(..)) - if config.dockerfile(&target)?.is_some() => + if config.dockerfile(&target).is_some() => { "scratch".into() } From 1826adbfab2027fd2c7c39210b469dea701f3756 Mon Sep 17 00:00:00 2001 From: thirteenowls Date: Fri, 10 May 2024 06:38:58 +0200 Subject: [PATCH 2/3] Simplify config internals, remove duplicated code --- src/config.rs | 140 ++++++++++++++++++++------------------------------ 1 file changed, 55 insertions(+), 85 deletions(-) diff --git a/src/config.rs b/src/config.rs index fac620efd..73d98a7a1 100644 --- a/src/config.rs +++ b/src/config.rs @@ -4,6 +4,7 @@ use crate::docker::{ImagePlatform, PossibleImage}; use crate::shell::MessageInfo; use crate::{CrossToml, Result, Target, TargetList}; +use std::borrow::Cow; use std::collections::HashMap; use std::env; use std::str::FromStr; @@ -33,9 +34,8 @@ impl Environment { target: &Target, convert: impl Fn(&str) -> T, ) -> (Option, Option) { - let target_values = self.get_target_var(target, var).map(|ref s| convert(s)); - let build_values = self.get_build_var(var).map(|ref s| convert(s)); + let target_values = self.get_target_var(target, var).map(|ref s| convert(s)); (build_values, target_values) } @@ -209,6 +209,10 @@ pub fn try_bool_from_envvar(envvar: &str) -> Option { } } +fn map_opt_tuple U>(t: (Option, Option), f: F) -> (Option, Option) { + (t.0.map(&f), t.1.map(&f)) +} + #[derive(Debug)] pub struct Config { toml: Option, @@ -243,30 +247,33 @@ impl Config { Ok(()) } - fn bool_from_config( + fn get_from_value_inner( &self, target: &Target, - env: impl Fn(&Environment, &Target) -> (Option, Option), - config: impl Fn(&CrossToml, &Target) -> (Option, Option), - ) -> Option { + env: impl for<'a> FnOnce(&'a Environment, &Target) -> (Option, Option), + config: impl for<'a> FnOnce(&'a CrossToml, &Target) -> (Option>, Option>), + ) -> Option + where + U: ToOwned + ?Sized, + { let (env_build, env_target) = env(&self.env, target); - let (toml_build, toml_target) = if let Some(ref toml) = self.toml { - config(toml, target) - } else { - (None, None) - }; + let (toml_build, toml_target) = self + .toml + .as_ref() + .map(|toml| config(toml, target)) + .unwrap_or_default(); match (env_target, toml_target) { (Some(value), _) => return Some(value), - (None, Some(value)) => return Some(value), + (None, Some(value)) => return Some(value.into_owned()), (None, None) => {} - }; + } match (env_build, toml_build) { (Some(value), _) => return Some(value), - (None, Some(value)) => return Some(value), + (None, Some(value)) => return Some(value.into_owned()), (None, None) => {} - }; + } None } @@ -274,15 +281,18 @@ impl Config { fn vec_from_config( &self, target: &Target, - env: impl for<'a> Fn(&'a Environment, &Target) -> (Option>, Option>), - config: impl for<'a> Fn(&'a CrossToml, &Target) -> (Option<&'a [String]>, Option<&'a [String]>), + env: impl for<'a> FnOnce(&'a Environment, &Target) -> (Option>, Option>), + config: impl for<'a> FnOnce( + &'a CrossToml, + &Target, + ) -> (Option<&'a [String]>, Option<&'a [String]>), sum: bool, ) -> Option> { if sum { let (mut env_build, env_target) = env(&self.env, target); - env_build - .as_mut() - .map(|b| env_target.map(|mut t| b.append(&mut t))); + if let (Some(b), Some(mut t)) = (&mut env_build, env_target) { + b.append(&mut t); + } self.sum_of_env_toml_values(env_build, |t| config(t, target)) } else { self.get_from_ref(target, env, config) @@ -292,64 +302,29 @@ impl Config { fn get_from_ref( &self, target: &Target, - env: impl for<'a> Fn(&'a Environment, &Target) -> (Option, Option), - config: impl for<'a> Fn(&'a CrossToml, &Target) -> (Option<&'a U>, Option<&'a U>), + env: impl for<'a> FnOnce(&'a Environment, &Target) -> (Option, Option), + config: impl for<'a> FnOnce(&'a CrossToml, &Target) -> (Option<&'a U>, Option<&'a U>), ) -> Option where U: ToOwned + ?Sized, { - let (env_build, env_target) = env(&self.env, target); - - if let Some(env_target) = env_target { - return Some(env_target); - } - - let (build, target) = self - .toml - .as_ref() - .map(|t| config(t, target)) - .unwrap_or_default(); - - // FIXME: let expression - if target.is_none() && env_build.is_some() { - return env_build; - } - - if target.is_none() { - build.map(ToOwned::to_owned) - } else { - target.map(ToOwned::to_owned) - } + self.get_from_value_inner(target, env, |toml, target| { + map_opt_tuple(config(toml, target), |v| Cow::Borrowed(v)) + }) } fn get_from_value( &self, target: &Target, - env: impl Fn(&Environment, &Target) -> (Option, Option), - config: impl Fn(&CrossToml, &Target) -> (Option, Option), - ) -> Option { - let (env_build, env_target) = env(&self.env, target); - - if let Some(env_target) = env_target { - return Some(env_target); - } - - let (build, target) = self - .toml - .as_ref() - .map(|t| config(t, target)) - .unwrap_or_default(); - - // FIXME: let expression - if target.is_none() && env_build.is_some() { - return env_build; - } - - if target.is_none() { - build - } else { - target - } + env: impl FnOnce(&Environment, &Target) -> (Option, Option), + config: impl FnOnce(&CrossToml, &Target) -> (Option, Option), + ) -> Option + where + T: ToOwned, + { + self.get_from_value_inner::(target, env, |toml, target| { + map_opt_tuple(config(toml, target), |v| Cow::Owned(v)) + }) } #[cfg(test)] @@ -358,7 +333,7 @@ impl Config { } pub fn xargo(&self, target: &Target) -> Option { - self.bool_from_config(target, Environment::xargo, CrossToml::xargo) + self.get_from_value(target, Environment::xargo, CrossToml::xargo) } pub fn build_std(&self, target: &Target) -> Option { @@ -366,7 +341,7 @@ impl Config { } pub fn zig(&self, target: &Target) -> Option { - self.bool_from_config(target, Environment::zig, CrossToml::zig) + self.get_from_value(target, Environment::zig, CrossToml::zig) } pub fn zig_version(&self, target: &Target) -> Option { @@ -374,15 +349,15 @@ impl Config { } pub fn zig_image(&self, target: &Target) -> Result> { - let (b, t) = self.env.zig_image(target)?; - Ok(self.get_from_value(target, |_, _| (b.clone(), t.clone()), CrossToml::zig_image)) + let env = self.env.zig_image(target)?; + Ok(self.get_from_value(target, |_, _| env, CrossToml::zig_image)) } pub fn image(&self, target: &Target) -> Result> { let env = self.env.image(target)?; Ok(self.get_from_ref( target, - move |_, _| (None, env.clone()), + |_, _| (None, env), |toml, target| (None, toml.image(target)), )) } @@ -459,29 +434,26 @@ impl Config { // FIXME: remove when we disable sums in 0.3.0. fn sum_of_env_toml_values<'a>( &'a self, - env_values: Option>, + env_values: Option>, toml_getter: impl FnOnce(&'a CrossToml) -> (Option<&'a [String]>, Option<&'a [String]>), ) -> Option> { let mut defined = false; let mut collect = vec![]; - if let Some(vars) = env_values { - collect.extend(vars.as_ref().iter().cloned()); - defined = true; + if env_values.is_some() { + return env_values; } else if let Some((build, target)) = self.toml.as_ref().map(toml_getter) { if let Some(build) = build { collect.extend(build.iter().cloned()); defined = true; } + if let Some(target) = target { collect.extend(target.iter().cloned()); defined = true; } } - if !defined { - None - } else { - Some(collect) - } + + defined.then_some(collect) } } @@ -527,7 +499,6 @@ mod tests { } mod test_environment { - use super::*; #[test] @@ -603,7 +574,6 @@ mod tests { #[cfg(test)] mod test_config { - use super::*; macro_rules! s { From 415bbd3eb0a304ae7915d53444a088f31405739c Mon Sep 17 00:00:00 2001 From: thirteenowls Date: Fri, 10 May 2024 07:06:59 +0200 Subject: [PATCH 3/3] Replace unwieldy tuple return types in config with new dedicated type --- .changes/1489.json | 4 ++ src/config.rs | 118 +++++++++++++++++++++++++++------------------ src/cross_toml.rs | 34 +++++++------ 3 files changed, 94 insertions(+), 62 deletions(-) create mode 100644 .changes/1489.json diff --git a/.changes/1489.json b/.changes/1489.json new file mode 100644 index 000000000..a34bae916 --- /dev/null +++ b/.changes/1489.json @@ -0,0 +1,4 @@ +{ + "description": "Simplify config internals", + "type": "internal" +} diff --git a/src/config.rs b/src/config.rs index 73d98a7a1..853745cff 100644 --- a/src/config.rs +++ b/src/config.rs @@ -9,6 +9,37 @@ use std::collections::HashMap; use std::env; use std::str::FromStr; +#[derive(Debug)] +pub struct ConfVal { + pub build: Option, + pub target: Option, +} + +impl ConfVal { + pub fn new(build: Option, target: Option) -> Self { + Self { build, target } + } + + pub fn map U>(self, f: F) -> ConfVal { + ConfVal { + build: self.build.map(&f), + target: self.target.map(&f), + } + } +} + +impl Default for ConfVal { + fn default() -> Self { + Self::new(None, None) + } +} + +impl PartialEq<(Option, Option)> for ConfVal { + fn eq(&self, other: &(Option, Option)) -> bool { + self.build == other.0 && self.target == other.1 + } +} + #[derive(Debug)] struct Environment(&'static str, Option>); @@ -33,11 +64,11 @@ impl Environment { var: &str, target: &Target, convert: impl Fn(&str) -> T, - ) -> (Option, Option) { + ) -> ConfVal { let build_values = self.get_build_var(var).map(|ref s| convert(s)); let target_values = self.get_target_var(target, var).map(|ref s| convert(s)); - (build_values, target_values) + ConfVal::new(build_values, target_values) } fn target_path(target: &Target, key: &str) -> String { @@ -60,11 +91,11 @@ impl Environment { self.get_var(&self.build_var_name(&Self::target_path(target, key))) } - fn xargo(&self, target: &Target) -> (Option, Option) { + fn xargo(&self, target: &Target) -> ConfVal { self.get_values_for("XARGO", target, bool_from_envvar) } - fn build_std(&self, target: &Target) -> (Option, Option) { + fn build_std(&self, target: &Target) -> ConfVal { self.get_values_for("BUILD_STD", target, |v| { if let Some(value) = try_bool_from_envvar(v) { BuildStd::Bool(value) @@ -74,15 +105,15 @@ impl Environment { }) } - fn zig(&self, target: &Target) -> (Option, Option) { + fn zig(&self, target: &Target) -> ConfVal { self.get_values_for("ZIG", target, bool_from_envvar) } - fn zig_version(&self, target: &Target) -> (Option, Option) { + fn zig_version(&self, target: &Target) -> ConfVal { self.get_values_for("ZIG_VERSION", target, ToOwned::to_owned) } - fn zig_image(&self, target: &Target) -> Result<(Option, Option)> { + fn zig_image(&self, target: &Target) -> Result> { let get_build = |env: &Environment, var: &str| env.get_build_var(var); let get_target = |env: &Environment, var: &str| env.get_target_var(target, var); let env_build = get_possible_image( @@ -100,7 +131,7 @@ impl Environment { get_target, )?; - Ok((env_build, env_target)) + Ok(ConfVal::new(env_build, env_target)) } fn image(&self, target: &Target) -> Result> { @@ -108,15 +139,15 @@ impl Environment { get_possible_image(self, "IMAGE", "IMAGE_TOOLCHAIN", get_target, get_target) } - fn dockerfile(&self, target: &Target) -> (Option, Option) { + fn dockerfile(&self, target: &Target) -> ConfVal { self.get_values_for("DOCKERFILE", target, ToOwned::to_owned) } - fn dockerfile_context(&self, target: &Target) -> (Option, Option) { + fn dockerfile_context(&self, target: &Target) -> ConfVal { self.get_values_for("DOCKERFILE_CONTEXT", target, ToOwned::to_owned) } - fn pre_build(&self, target: &Target) -> (Option, Option) { + fn pre_build(&self, target: &Target) -> ConfVal { self.get_values_for("PRE_BUILD", target, |v| { let v: Vec<_> = v.split('\n').map(String::from).collect(); if v.len() == 1 { @@ -134,11 +165,11 @@ impl Environment { self.get_target_var(target, "RUNNER") } - fn passthrough(&self, target: &Target) -> (Option>, Option>) { + fn passthrough(&self, target: &Target) -> ConfVal> { self.get_values_for("ENV_PASSTHROUGH", target, split_to_cloned_by_ws) } - fn volumes(&self, target: &Target) -> (Option>, Option>) { + fn volumes(&self, target: &Target) -> ConfVal> { self.get_values_for("ENV_VOLUMES", target, split_to_cloned_by_ws) } @@ -209,10 +240,6 @@ pub fn try_bool_from_envvar(envvar: &str) -> Option { } } -fn map_opt_tuple U>(t: (Option, Option), f: F) -> (Option, Option) { - (t.0.map(&f), t.1.map(&f)) -} - #[derive(Debug)] pub struct Config { toml: Option, @@ -250,26 +277,26 @@ impl Config { fn get_from_value_inner( &self, target: &Target, - env: impl for<'a> FnOnce(&'a Environment, &Target) -> (Option, Option), - config: impl for<'a> FnOnce(&'a CrossToml, &Target) -> (Option>, Option>), + env: impl for<'a> FnOnce(&'a Environment, &Target) -> ConfVal, + config: impl for<'a> FnOnce(&'a CrossToml, &Target) -> ConfVal>, ) -> Option where U: ToOwned + ?Sized, { - let (env_build, env_target) = env(&self.env, target); - let (toml_build, toml_target) = self + let env = env(&self.env, target); + let toml = self .toml .as_ref() .map(|toml| config(toml, target)) .unwrap_or_default(); - match (env_target, toml_target) { + match (env.target, toml.target) { (Some(value), _) => return Some(value), (None, Some(value)) => return Some(value.into_owned()), (None, None) => {} } - match (env_build, toml_build) { + match (env.build, toml.build) { (Some(value), _) => return Some(value), (None, Some(value)) => return Some(value.into_owned()), (None, None) => {} @@ -281,19 +308,16 @@ impl Config { fn vec_from_config( &self, target: &Target, - env: impl for<'a> FnOnce(&'a Environment, &Target) -> (Option>, Option>), - config: impl for<'a> FnOnce( - &'a CrossToml, - &Target, - ) -> (Option<&'a [String]>, Option<&'a [String]>), + env: impl for<'a> FnOnce(&'a Environment, &Target) -> ConfVal>, + config: impl for<'a> FnOnce(&'a CrossToml, &Target) -> ConfVal<&'a [String]>, sum: bool, ) -> Option> { if sum { - let (mut env_build, env_target) = env(&self.env, target); - if let (Some(b), Some(mut t)) = (&mut env_build, env_target) { - b.append(&mut t); + let mut env = env(&self.env, target); + if let (Some(b), Some(t)) = (&mut env.build, &mut env.target) { + b.append(t); } - self.sum_of_env_toml_values(env_build, |t| config(t, target)) + self.sum_of_env_toml_values(env.build, |t| config(t, target)) } else { self.get_from_ref(target, env, config) } @@ -302,28 +326,28 @@ impl Config { fn get_from_ref( &self, target: &Target, - env: impl for<'a> FnOnce(&'a Environment, &Target) -> (Option, Option), - config: impl for<'a> FnOnce(&'a CrossToml, &Target) -> (Option<&'a U>, Option<&'a U>), + env: impl for<'a> FnOnce(&'a Environment, &Target) -> ConfVal, + config: impl for<'a> FnOnce(&'a CrossToml, &Target) -> ConfVal<&'a U>, ) -> Option where U: ToOwned + ?Sized, { self.get_from_value_inner(target, env, |toml, target| { - map_opt_tuple(config(toml, target), |v| Cow::Borrowed(v)) + config(toml, target).map(|v| Cow::Borrowed(v)) }) } fn get_from_value( &self, target: &Target, - env: impl FnOnce(&Environment, &Target) -> (Option, Option), - config: impl FnOnce(&CrossToml, &Target) -> (Option, Option), + env: impl FnOnce(&Environment, &Target) -> ConfVal, + config: impl FnOnce(&CrossToml, &Target) -> ConfVal, ) -> Option where T: ToOwned, { self.get_from_value_inner::(target, env, |toml, target| { - map_opt_tuple(config(toml, target), |v| Cow::Owned(v)) + config(toml, target).map(|v| Cow::Owned(v)) }) } @@ -357,16 +381,16 @@ impl Config { let env = self.env.image(target)?; Ok(self.get_from_ref( target, - |_, _| (None, env), - |toml, target| (None, toml.image(target)), + |_, _| ConfVal::new(None, env), + |toml, target| ConfVal::new(None, toml.image(target)), )) } pub fn runner(&self, target: &Target) -> Option { self.get_from_ref( target, - |env, target| (None, env.runner(target)), - |toml, target| (None, toml.runner(target)), + |env, target| ConfVal::new(None, env.runner(target)), + |toml, target| ConfVal::new(None, toml.runner(target)), ) } @@ -435,19 +459,19 @@ impl Config { fn sum_of_env_toml_values<'a>( &'a self, env_values: Option>, - toml_getter: impl FnOnce(&'a CrossToml) -> (Option<&'a [String]>, Option<&'a [String]>), + toml_getter: impl FnOnce(&'a CrossToml) -> ConfVal<&'a [String]>, ) -> Option> { let mut defined = false; let mut collect = vec![]; if env_values.is_some() { return env_values; - } else if let Some((build, target)) = self.toml.as_ref().map(toml_getter) { - if let Some(build) = build { + } else if let Some(toml) = self.toml.as_ref().map(toml_getter) { + if let Some(build) = toml.build { collect.extend(build.iter().cloned()); defined = true; } - if let Some(target) = target { + if let Some(target) = toml.target { collect.extend(target.iter().cloned()); defined = true; } @@ -564,7 +588,7 @@ mod tests { let env = Environment::new(Some(map)); - let (build, target) = env.passthrough(&target()); + let ConfVal { build, target } = env.passthrough(&target()); assert!(build.as_ref().unwrap().contains(&"TEST1".to_owned())); assert!(build.as_ref().unwrap().contains(&"TEST2".to_owned())); assert!(target.as_ref().unwrap().contains(&"PASS1".to_owned())); diff --git a/src/cross_toml.rs b/src/cross_toml.rs index 8e1403771..87328d328 100644 --- a/src/cross_toml.rs +++ b/src/cross_toml.rs @@ -5,6 +5,7 @@ //! //! [1]: https://github.com/cross-rs/cross/blob/main/docs/config_file.md +use crate::config::ConfVal; use crate::docker::custom::PreBuild; use crate::docker::PossibleImage; use crate::shell::MessageInfo; @@ -279,7 +280,7 @@ impl CrossToml { } /// Returns the `{}.dockerfile` or `{}.dockerfile.file` part of `Cross.toml` - pub fn dockerfile(&self, target: &Target) -> (Option<&String>, Option<&String>) { + pub fn dockerfile(&self, target: &Target) -> ConfVal<&String> { self.get_ref( target, |b| b.dockerfile.as_ref().map(|c| &c.file), @@ -288,7 +289,7 @@ impl CrossToml { } /// Returns the `target.{}.dockerfile.context` part of `Cross.toml` - pub fn dockerfile_context(&self, target: &Target) -> (Option<&String>, Option<&String>) { + pub fn dockerfile_context(&self, target: &Target) -> ConfVal<&String> { self.get_ref( target, |b| b.dockerfile.as_ref().and_then(|c| c.context.as_ref()), @@ -313,7 +314,7 @@ impl CrossToml { } /// Returns the `build.dockerfile.pre-build` and `target.{}.dockerfile.pre-build` part of `Cross.toml` - pub fn pre_build(&self, target: &Target) -> (Option<&PreBuild>, Option<&PreBuild>) { + pub fn pre_build(&self, target: &Target) -> ConfVal<&PreBuild> { self.get_ref(target, |b| b.pre_build.as_ref(), |t| t.pre_build.as_ref()) } @@ -323,17 +324,17 @@ impl CrossToml { } /// Returns the `build.xargo` or the `target.{}.xargo` part of `Cross.toml` - pub fn xargo(&self, target: &Target) -> (Option, Option) { + pub fn xargo(&self, target: &Target) -> ConfVal { self.get_value(target, |b| b.xargo, |t| t.xargo) } /// Returns the `build.build-std` or the `target.{}.build-std` part of `Cross.toml` - pub fn build_std(&self, target: &Target) -> (Option<&BuildStd>, Option<&BuildStd>) { + pub fn build_std(&self, target: &Target) -> ConfVal<&BuildStd> { self.get_ref(target, |b| b.build_std.as_ref(), |t| t.build_std.as_ref()) } /// Returns the `{}.zig` or `{}.zig.version` part of `Cross.toml` - pub fn zig(&self, target: &Target) -> (Option, Option) { + pub fn zig(&self, target: &Target) -> ConfVal { self.get_value( target, |b| b.zig.as_ref().and_then(|z| z.enable), @@ -342,7 +343,7 @@ impl CrossToml { } /// Returns the `{}.zig` or `{}.zig.version` part of `Cross.toml` - pub fn zig_version(&self, target: &Target) -> (Option, Option) { + pub fn zig_version(&self, target: &Target) -> ConfVal { self.get_value( target, |b| b.zig.as_ref().and_then(|c| c.version.clone()), @@ -351,7 +352,7 @@ impl CrossToml { } /// Returns the `{}.zig.image` part of `Cross.toml` - pub fn zig_image(&self, target: &Target) -> (Option, Option) { + pub fn zig_image(&self, target: &Target) -> ConfVal { self.get_value( target, |b| b.zig.as_ref().and_then(|c| c.image.clone()), @@ -360,7 +361,7 @@ impl CrossToml { } /// Returns the list of environment variables to pass through for `build` and `target` - pub fn env_passthrough(&self, target: &Target) -> (Option<&[String]>, Option<&[String]>) { + pub fn env_passthrough(&self, target: &Target) -> ConfVal<&[String]> { self.get_ref( target, |build| build.env.passthrough.as_deref(), @@ -369,7 +370,7 @@ impl CrossToml { } /// Returns the list of environment variables to pass through for `build` and `target` - pub fn env_volumes(&self, target: &Target) -> (Option<&[String]>, Option<&[String]>) { + pub fn env_volumes(&self, target: &Target) -> ConfVal<&[String]> { self.get_ref( target, |build| build.env.volumes.as_deref(), @@ -395,10 +396,10 @@ impl CrossToml { target_triple: &Target, get_build: impl Fn(&CrossBuildConfig) -> Option, get_target: impl Fn(&CrossTargetConfig) -> Option, - ) -> (Option, Option) { + ) -> ConfVal { let build = get_build(&self.build); let target = self.get_target(target_triple).and_then(get_target); - (build, target) + ConfVal::new(build, target) } fn get_ref( @@ -406,10 +407,10 @@ impl CrossToml { target_triple: &Target, get_build: impl Fn(&CrossBuildConfig) -> Option<&T>, get_target: impl Fn(&CrossTargetConfig) -> Option<&T>, - ) -> (Option<&T>, Option<&T>) { + ) -> ConfVal<&T> { let build = get_build(&self.build); let target = self.get_target(target_triple).and_then(get_target); - (build, target) + ConfVal::new(build, target) } } @@ -1063,7 +1064,10 @@ mod tests { assert!(unused.is_empty()); assert!(matches!( toml.pre_build(&Target::new_built_in("aarch64-unknown-linux-gnu")), - (Some(&PreBuild::Lines(_)), Some(&PreBuild::Single { .. })) + ConfVal { + build: Some(&PreBuild::Lines(_)), + target: Some(&PreBuild::Single { .. }), + }, )); Ok(()) }