diff --git a/xbuild/src/cargo/config.rs b/xbuild/src/cargo/config.rs index 8ad539f9..bfeeb916 100644 --- a/xbuild/src/cargo/config.rs +++ b/xbuild/src/cargo/config.rs @@ -1,10 +1,19 @@ -use anyhow::Result; +use anyhow::{Context, Result}; use serde::Deserialize; -use std::path::Path; +use std::{ + borrow::Cow, + collections::BTreeMap, + env::VarError, + ops::Deref, + path::{Path, PathBuf}, +}; -#[derive(Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "kebab-case")] pub struct Config { pub build: Option, + /// + pub env: Option>, } impl Config { @@ -12,6 +21,30 @@ impl Config { let contents = std::fs::read_to_string(path)?; Ok(toml::from_str(&contents)?) } +} + +#[derive(Debug)] +pub struct LocalizedConfig { + pub config: Config, + /// The directory containing `./.cargo/config.toml` + pub workspace: PathBuf, +} + +impl Deref for LocalizedConfig { + type Target = Config; + + fn deref(&self) -> &Self::Target { + &self.config + } +} + +impl LocalizedConfig { + pub fn new(workspace: PathBuf) -> Result { + Ok(Self { + config: Config::parse_from_toml(workspace.join(".cargo/config.toml"))?, + workspace, + }) + } /// Search for and open `.cargo/config.toml` in any parent of the workspace root path. pub fn find_cargo_config_for_workspace(workspace: impl AsRef) -> Result> { @@ -19,15 +52,217 @@ impl Config { let workspace = dunce::canonicalize(workspace)?; workspace .ancestors() - .map(|dir| dir.join(".cargo/config.toml")) - .find(|p| p.is_file()) - .map(Config::parse_from_toml) + .find(|dir| dir.join(".cargo/config.toml").is_file()) + .map(|p| p.to_path_buf()) + .map(Self::new) .transpose() } + + /// Propagate environment variables from this `.cargo/config.toml` to the process environment + /// using [`std::env::set_var()`]. + /// + /// Note that this is automatically performed when calling [`Subcommand::new()`][super::Subcommand::new()]. + pub fn set_env_vars(&self) -> Result<()> { + if let Some(env) = &self.config.env { + for (key, env_option) in env { + // Existing environment variables always have precedence unless + // the extended format is used to set `force = true`: + if !matches!(env_option, EnvOption::Value { force: true, .. }) + && std::env::var_os(key).is_some() + { + continue; + } + + std::env::set_var(key, env_option.resolve_value(&self.workspace)?.as_ref()) + } + } + + Ok(()) + } } -#[derive(Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize, PartialEq, Eq)] pub struct Build { - #[serde(rename = "target-dir")] pub target_dir: Option, } + +/// Serializable environment variable in cargo config, configurable as per +/// , +#[derive(Clone, Debug, Deserialize, PartialEq, Eq)] +#[serde(untagged)] +pub enum EnvOption { + String(String), + Value { + value: String, + #[serde(default)] + force: bool, + #[serde(default)] + relative: bool, + }, +} + +impl EnvOption { + /// Retrieve the value and canonicalize it relative to `config_parent` when [`EnvOption::Value::relative`] is set. + /// + /// `config_parent` is the directory containing `.cargo/config.toml` where this was parsed from. + pub fn resolve_value(&self, config_parent: impl AsRef) -> Result> { + Ok(match self { + Self::Value { + value, + relative: true, + force: _, + } => { + let value = config_parent.as_ref().join(value); + let value = dunce::canonicalize(&value) + .with_context(|| format!("Failed to canonicalize `{}`", value.display()))?; + value + .into_os_string() + .into_string() + .map_err(VarError::NotUnicode)? + .into() + } + Self::String(value) | Self::Value { value, .. } => value.into(), + }) + } +} + +#[test] +fn test_env_parsing() { + let toml = r#" +[env] +# Set ENV_VAR_NAME=value for any process run by Cargo +ENV_VAR_NAME = "value" +# Set even if already present in environment +ENV_VAR_NAME_2 = { value = "value", force = true } +# Value is relative to .cargo directory containing `config.toml`, make absolute +ENV_VAR_NAME_3 = { value = "relative/path", relative = true }"#; + + let mut env = BTreeMap::new(); + env.insert( + "ENV_VAR_NAME".to_string(), + EnvOption::String("value".into()), + ); + env.insert( + "ENV_VAR_NAME_2".to_string(), + EnvOption::Value { + value: "value".into(), + force: true, + relative: false, + }, + ); + env.insert( + "ENV_VAR_NAME_3".to_string(), + EnvOption::Value { + value: "relative/path".into(), + force: false, + relative: true, + }, + ); + + assert_eq!( + toml::from_str::(toml), + Ok(Config { + build: None, + env: Some(env) + }) + ); +} + +#[test] +fn test_env_precedence_rules() { + let toml = r#" +[env] +CARGO_SUBCOMMAND_TEST_ENV_NOT_FORCED = "not forced" +CARGO_SUBCOMMAND_TEST_ENV_FORCED = { value = "forced", force = true }"#; + + let config = LocalizedConfig { + config: toml::from_str::(toml).unwrap(), + workspace: PathBuf::new(), + }; + + // Check if all values are propagated to the environment + config.set_env_vars().unwrap(); + + assert!(matches!( + std::env::var("CARGO_SUBCOMMAND_TEST_ENV_NOT_SET"), + Err(VarError::NotPresent) + )); + assert_eq!( + std::env::var("CARGO_SUBCOMMAND_TEST_ENV_NOT_FORCED").unwrap(), + "not forced" + ); + assert_eq!( + std::env::var("CARGO_SUBCOMMAND_TEST_ENV_FORCED").unwrap(), + "forced" + ); + + // Set some environment values + std::env::set_var( + "CARGO_SUBCOMMAND_TEST_ENV_NOT_FORCED", + "not forced process environment value", + ); + std::env::set_var( + "CARGO_SUBCOMMAND_TEST_ENV_FORCED", + "forced process environment value", + ); + + config.set_env_vars().unwrap(); + + assert_eq!( + std::env::var("CARGO_SUBCOMMAND_TEST_ENV_NOT_FORCED").unwrap(), + // Value remains what is set in the process environment, + // and is not overwritten by set_env_vars() + "not forced process environment value" + ); + assert_eq!( + std::env::var("CARGO_SUBCOMMAND_TEST_ENV_FORCED").unwrap(), + // Value is overwritten thanks to force=true, despite + // also being set in the process environment + "forced" + ); +} + +#[test] +fn test_env_canonicalization() { + use std::ffi::OsStr; + + let toml = r#" +[env] +CARGO_SUBCOMMAND_TEST_ENV_SRC_DIR = { value = "src", force = true, relative = true } +"#; + + let config = LocalizedConfig { + config: toml::from_str::(toml).unwrap(), + workspace: PathBuf::new(), + }; + + config.set_env_vars().unwrap(); + + let path = std::env::var("CARGO_SUBCOMMAND_TEST_ENV_SRC_DIR") + .expect("Canonicalization for a known-to-exist ./src folder should not fail"); + let path = PathBuf::from(path); + assert!(path.is_absolute()); + assert!(path.is_dir()); + assert_eq!(path.file_name(), Some(OsStr::new("src"))); + + let toml = r#" +[env] +CARGO_SUBCOMMAND_TEST_ENV_INEXISTENT_DIR = { value = "blahblahthisfolderdoesntexist", force = true, relative = true } +"#; + + let config = LocalizedConfig { + config: toml::from_str::(toml).unwrap(), + workspace: PathBuf::new(), + }; + + let e = config.set_env_vars().unwrap_err(); + + assert_eq!( + e.to_string(), + "Failed to canonicalize `blahblahthisfolderdoesntexist`" + ); + assert_eq!( + e.root_cause().to_string(), + "No such file or directory (os error 2)" + ); +} diff --git a/xbuild/src/cargo/mod.rs b/xbuild/src/cargo/mod.rs index d90c0158..3c9c4546 100644 --- a/xbuild/src/cargo/mod.rs +++ b/xbuild/src/cargo/mod.rs @@ -10,7 +10,7 @@ mod utils; pub use artifact::{Artifact, CrateType}; -use self::config::Config; +use self::config::LocalizedConfig; use self::manifest::Manifest; use crate::{CompileTarget, Opt}; @@ -74,11 +74,10 @@ impl Cargo { // TODO: Find, parse, and merge _all_ config files following the hierarchical structure: // https://doc.rust-lang.org/cargo/reference/config.html#hierarchical-structure - let config = Config::find_cargo_config_for_workspace(package_root)?; - // TODO: Import LocalizedConfig code from cargo-subcommand and propagate `[env]` - // if let Some(config) = &config { - // config.set_env_vars().unwrap(); - // } + let config = LocalizedConfig::find_cargo_config_for_workspace(package_root)?; + if let Some(config) = &config { + config.set_env_vars().unwrap(); + } let target_dir = target_dir .or_else(|| { @@ -101,7 +100,7 @@ impl Cargo { .unwrap_or_else(|| &manifest_path) .parent() .unwrap() - .join(utils::get_target_dir_name(config.as_ref()).unwrap()) + .join(utils::get_target_dir_name(config.as_deref()).unwrap()) }); Ok(Self { diff --git a/xbuild/src/lib.rs b/xbuild/src/lib.rs index fd522bf2..3ea1ca15 100644 --- a/xbuild/src/lib.rs +++ b/xbuild/src/lib.rs @@ -387,7 +387,7 @@ pub struct BuildTargetArgs { /// Build artifacts for target device. To find the device /// identifier of a connected device run `x devices`. #[clap(long, conflicts_with = "format", conflicts_with = "store")] - device: Option, + device: Option, /// Build artifacts with format. Can be one of `aab`, /// `apk`, `appbundle`, `appdir`, `appimage`, `dmg`, /// `exe`, `ipa`, `msix`. @@ -424,6 +424,9 @@ impl BuildTargetArgs { Some(Device::host()) } else { self.device + .as_ref() + .map(|device| device.parse()) + .transpose()? }; let platform = if let Some(platform) = self.platform { platform