From 9190f2c3b33393768bd4dceff74287d600d30729 Mon Sep 17 00:00:00 2001 From: Alex Huszagh Date: Sun, 26 Jun 2022 10:27:21 -0500 Subject: [PATCH] Fix parsing of config `build.env.passthrough` values. Ensure `build.env.passthrough` values are correctly parsed, and do not return `None`, and ensure that they merge as expected with `target.(...).env.passthrough` values. --- CHANGELOG.md | 4 ++ src/config.rs | 137 +++++++++++++++++++++++++++++++++++++++++----- src/cross_toml.rs | 51 ++++++++++++++++- 3 files changed, 177 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b21ff7ab5..04fba491c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ This project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] - ReleaseDate +### Fixed + +- #867 - fixed parsing of `build.env,passthrough` config values. + ## [v0.2.2] - 2022-06-24 ### Added diff --git a/src/config.rs b/src/config.rs index bcb3d4977..0c29fbb57 100644 --- a/src/config.rs +++ b/src/config.rs @@ -428,7 +428,12 @@ mod tests { mod test_config { use super::*; - use std::matches; + + macro_rules! s { + ($x:literal) => { + $x.to_string() + }; + } fn toml(content: &str) -> Result { Ok(CrossToml::parse_from_cross(content) @@ -440,10 +445,19 @@ mod tests { pub fn env_and_toml_build_xargo_then_use_env() -> Result<()> { let mut map = HashMap::new(); map.insert("CROSS_BUILD_XARGO", "true"); + map.insert( + "CROSS_BUILD_PRE_BUILD", + "apt-get update\napt-get install zlib-dev", + ); let env = Environment::new(Some(map)); let config = Config::new_with(Some(toml(TOML_BUILD_XARGO_FALSE)?), env); - assert!(matches!(config.xargo(&target()), Some(true))); + assert_eq!(config.xargo(&target()), Some(true)); + assert_eq!(config.build_std(&target()), None); + assert_eq!( + config.pre_build(&target())?, + Some(vec![s!("apt-get update"), s!("apt-get install zlib-dev")]) + ); Ok(()) } @@ -456,8 +470,9 @@ mod tests { let env = Environment::new(Some(map)); let config = Config::new_with(Some(toml(TOML_TARGET_XARGO_FALSE)?), env); - assert!(matches!(config.xargo(&target()), Some(true))); - assert!(matches!(config.build_std(&target()), Some(true))); + assert_eq!(config.xargo(&target()), Some(true)); + assert_eq!(config.build_std(&target()), Some(true)); + assert_eq!(config.pre_build(&target())?, None); Ok(()) } @@ -466,9 +481,82 @@ mod tests { pub fn env_target_and_toml_build_xargo_then_use_toml() -> Result<()> { let mut map = HashMap::new(); map.insert("CROSS_TARGET_AARCH64_UNKNOWN_LINUX_GNU_XARGO", "true"); + let env = Environment::new(Some(map)); let config = Config::new_with(Some(toml(TOML_BUILD_XARGO_FALSE)?), env); - assert!(matches!(config.xargo(&target()), Some(true))); + assert_eq!(config.xargo(&target()), Some(true)); + assert_eq!(config.build_std(&target()), None); + assert_eq!(config.pre_build(&target())?, None); + + Ok(()) + } + + #[test] + pub fn env_target_and_toml_build_pre_build_then_use_toml() -> Result<()> { + let mut map = HashMap::new(); + map.insert( + "CROSS_TARGET_AARCH64_UNKNOWN_LINUX_GNU_PRE_BUILD", + "dpkg --add-architecture arm64", + ); + + let env = Environment::new(Some(map)); + let config = Config::new_with(Some(toml(TOML_BUILD_PRE_BUILD)?), env); + assert_eq!( + config.pre_build(&target())?, + Some(vec![s!("dpkg --add-architecture arm64")]) + ); + + Ok(()) + } + + #[test] + pub fn toml_build_passthrough_then_use_target_passthrough_both() -> Result<()> { + let map = HashMap::new(); + let env = Environment::new(Some(map)); + let config = Config::new_with(Some(toml(TOML_ARRAYS_BOTH)?), env); + assert_eq!( + config.env_passthrough(&target())?, + Some(vec![s!("VAR1"), s!("VAR2"), s!("VAR3"), s!("VAR4")]) + ); + assert_eq!( + config.env_volumes(&target())?, + Some(vec![s!("VOLUME3"), s!("VOLUME4")]) + ); + // TODO(ahuszagh) Need volumes + + Ok(()) + } + + #[test] + pub fn toml_build_passthrough() -> Result<()> { + let map = HashMap::new(); + let env = Environment::new(Some(map)); + let config = Config::new_with(Some(toml(TOML_ARRAYS_BUILD)?), env); + assert_eq!( + config.env_passthrough(&target())?, + Some(vec![s!("VAR1"), s!("VAR2")]) + ); + assert_eq!( + config.env_volumes(&target())?, + Some(vec![s!("VOLUME1"), s!("VOLUME2")]) + ); + + Ok(()) + } + + #[test] + pub fn toml_target_passthrough() -> Result<()> { + let map = HashMap::new(); + let env = Environment::new(Some(map)); + let config = Config::new_with(Some(toml(TOML_ARRAYS_TARGET)?), env); + assert_eq!( + config.env_passthrough(&target())?, + Some(vec![s!("VAR3"), s!("VAR4")]) + ); + assert_eq!( + config.env_volumes(&target())?, + Some(vec![s!("VOLUME3"), s!("VOLUME4")]) + ); Ok(()) } @@ -510,7 +598,7 @@ mod tests { pub fn no_env_and_no_toml_default_target_then_none() -> Result<()> { let config = Config::new_with(None, Environment::new(None)); let config_target = config.target(&target_list()); - assert!(matches!(config_target, None)); + assert_eq!(config_target, None); Ok(()) } @@ -523,10 +611,7 @@ mod tests { let config = Config::new_with(Some(toml(TOML_DEFAULT_TARGET)?), env); let config_target = config.target(&target_list()).unwrap(); - assert!(matches!( - config_target.triple(), - "armv7-unknown-linux-musleabihf" - )); + assert_eq!(config_target.triple(), "armv7-unknown-linux-musleabihf"); Ok(()) } @@ -537,10 +622,7 @@ mod tests { let config = Config::new_with(Some(toml(TOML_DEFAULT_TARGET)?), env); let config_target = config.target(&target_list()).unwrap(); - assert!(matches!( - config_target.triple(), - "aarch64-unknown-linux-gnu" - )); + assert_eq!(config_target.triple(), "aarch64-unknown-linux-gnu"); Ok(()) } @@ -548,6 +630,11 @@ mod tests { static TOML_BUILD_XARGO_FALSE: &str = r#" [build] xargo = false + "#; + + static TOML_BUILD_PRE_BUILD: &str = r#" + [build] + pre-build = ["apt-get update && apt-get install zlib-dev"] "#; static TOML_TARGET_XARGO_FALSE: &str = r#" @@ -560,6 +647,28 @@ mod tests { volumes = ["VOLUME3", "VOLUME4"] [target.aarch64-unknown-linux-gnu] xargo = false + "#; + + static TOML_ARRAYS_BOTH: &str = r#" + [build.env] + passthrough = ["VAR1", "VAR2"] + volumes = ["VOLUME1", "VOLUME2"] + + [target.aarch64-unknown-linux-gnu.env] + passthrough = ["VAR3", "VAR4"] + volumes = ["VOLUME3", "VOLUME4"] + "#; + + static TOML_ARRAYS_BUILD: &str = r#" + [build.env] + passthrough = ["VAR1", "VAR2"] + volumes = ["VOLUME1", "VOLUME2"] + "#; + + static TOML_ARRAYS_TARGET: &str = r#" + [target.aarch64-unknown-linux-gnu.env] + passthrough = ["VAR3", "VAR4"] + volumes = ["VOLUME3", "VOLUME4"] "#; static TOML_DEFAULT_TARGET: &str = r#" diff --git a/src/cross_toml.rs b/src/cross_toml.rs index eec270ccb..453359cda 100644 --- a/src/cross_toml.rs +++ b/src/cross_toml.rs @@ -259,7 +259,11 @@ 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]>) { - self.get_vec(target, |_| None, |t| t.env.passthrough.as_deref()) + self.get_vec( + target, + |build| build.env.passthrough.as_deref(), + |t| t.env.passthrough.as_deref(), + ) } /// Returns the list of environment variables to pass through for `build` and `target` @@ -382,6 +386,12 @@ where mod tests { use super::*; + macro_rules! s { + ($x:literal) => { + $x.to_string() + }; + } + #[test] pub fn parse_empty_toml() -> Result<()> { let cfg = CrossToml { @@ -700,6 +710,45 @@ mod tests { let cfg_merged = cfg1.merge(cfg2)?; assert_eq!(cfg_expected, cfg_merged); + // need to test individual values. i've broken this down into + // tests on values for better error reporting + let build = &cfg_expected.build; + assert_eq!(build.build_std, Some(true)); + assert_eq!(build.xargo, Some(false)); + assert_eq!(build.default_target, Some(s!("aarch64-unknown-linux-gnu"))); + assert_eq!(build.pre_build, None); + assert_eq!(build.dockerfile, None); + assert_eq!(build.env.passthrough, Some(vec![s!("VAR3"), s!("VAR4")])); + assert_eq!(build.env.volumes, Some(vec![])); + + let targets = &cfg_expected.targets; + let aarch64 = &targets[&Target::new_built_in("aarch64-unknown-linux-gnu")]; + assert_eq!(aarch64.build_std, Some(true)); + assert_eq!(aarch64.xargo, Some(false)); + assert_eq!(aarch64.image, Some(s!("test-image1"))); + assert_eq!(aarch64.pre_build, None); + assert_eq!(aarch64.dockerfile, None); + assert_eq!(aarch64.env.passthrough, Some(vec![s!("VAR1")])); + assert_eq!(aarch64.env.volumes, Some(vec![s!("VOL1_ARG")])); + + let target2 = &targets[&Target::new_custom("target2")]; + assert_eq!(target2.build_std, Some(false)); + assert_eq!(target2.xargo, Some(false)); + assert_eq!(target2.image, Some(s!("test-image2-precedence"))); + assert_eq!(target2.pre_build, None); + assert_eq!(target2.dockerfile, None); + assert_eq!(target2.env.passthrough, Some(vec![s!("VAR2_PRECEDENCE")])); + assert_eq!(target2.env.volumes, Some(vec![s!("VOL2_ARG_PRECEDENCE")])); + + let target3 = &targets[&Target::new_custom("target3")]; + assert_eq!(target3.build_std, Some(true)); + assert_eq!(target3.xargo, Some(false)); + assert_eq!(target3.image, Some(s!("test-image3"))); + assert_eq!(target3.pre_build, None); + assert_eq!(target3.dockerfile, None); + assert_eq!(target3.env.passthrough, Some(vec![s!("VAR3")])); + assert_eq!(target3.env.volumes, Some(vec![s!("VOL3_ARG")])); + Ok(()) } }