Merge #867

867: Fix parsing of config `build.env.passthrough` values. r=Emilgardis a=Alexhuszagh 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. Co-authored-by: Alex Huszagh <[email protected]>
cross-rs · Jun 26, 2022 · 037070d · 037070d
2 parents 48d2b7b + 9190f2c
commit 037070d
Show file tree

Hide file tree

Showing 3 changed files with 177 additions and 15 deletions.
diff --git 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
@@ -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<crate::CrossToml> {
             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,17 +622,19 @@ 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(())
         }
 
         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
@@ -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(())
     }
 }