Skip to content

Commit

Permalink
Fix the order of values for string (as well as the pre-build vec) val…
Browse files Browse the repository at this point in the history 
…ues when parsing from config files and environment variables.

The old behavior was:
1. Envvar build.
2. Envvar target
3. Config target
4. Config build

The new behavior is:
1. Envvar target
2. Config target
3. Envvar build.
4. Config build

Some functions were also made more general.
  • Loading branch information
@Alexhuszagh
Alexhuszagh committed Jul 1, 2022
1 parent a60d486 commit 13f906c
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 49 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).

### Fixed

- #897 - ensure `target.$(...)` config options override `build` ones when parsing strings and vecs.
- #885 - handle symlinks when using remote docker.
- #868 - ignore the `CARGO` environment variable.
- #867 - fixed parsing of `build.env.passthrough` config values.
Expand Down
91 changes: 68 additions & 23 deletions src/config.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,21 +69,18 @@ impl Environment {
self.get_target_var(target, "IMAGE")
}

fn dockerfile(&self, target: &Target) -> Option<String> {
let res = self.get_values_for("DOCKERFILE", target, |s| s.to_string());
res.0.or(res.1)
fn dockerfile(&self, target: &Target) -> (Option<String>, Option<String>) {
self.get_values_for("DOCKERFILE", target, |s| s.to_string())
}

fn dockerfile_context(&self, target: &Target) -> Option<String> {
let res = self.get_values_for("DOCKERFILE_CONTEXT", target, |s| s.to_string());
res.0.or(res.1)
fn dockerfile_context(&self, target: &Target) -> (Option<String>, Option<String>) {
self.get_values_for("DOCKERFILE_CONTEXT", target, |s| s.to_string())
}

fn pre_build(&self, target: &Target) -> Option<Vec<String>> {
let res = self.get_values_for("PRE_BUILD", target, |v| {
fn pre_build(&self, target: &Target) -> (Option<Vec<String>>, Option<Vec<String>>) {
self.get_values_for("PRE_BUILD", target, |v| {
v.split('\n').map(String::from).collect()
});
res.0.or(res.1)
})
}

fn runner(&self, target: &Target) -> Option<String> {
Expand Down Expand Up @@ -205,11 +202,29 @@ impl Config {
config: impl for<'a> Fn(&'a CrossToml, &Target) -> (Option<&'a [String]>, Option<&'a [String]>),
sum: bool,
) -> Result<Option<Vec<String>>> {
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)));
self.sum_of_env_toml_values(env_build, |t| config(t, target))
} else {
self.get_from_ref(target, env, config)
}
}

fn get_from_ref<T, U>(
&self,
target: &Target,
env: impl for<'a> Fn(&'a Environment, &Target) -> (Option<T>, Option<T>),
config: impl for<'a> Fn(&'a CrossToml, &Target) -> (Option<&'a U>, Option<&'a U>),
) -> Result<Option<T>>
where
U: ToOwned<Owned = T> + ?Sized,
{
let (env_build, env_target) = env(&self.env, target);

if sum {
return self.sum_of_env_toml_values(env_target, |t| config(t, target));
} else if let Some(env_target) = env_target {
if let Some(env_target) = env_target {
return Ok(Some(env_target));
}

Expand Down Expand Up @@ -262,7 +277,7 @@ impl Config {
}

pub fn env_volumes(&self, target: &Target) -> Result<Option<Vec<String>>> {
self.vec_from_config(target, Environment::volumes, CrossToml::env_volumes, false)
self.get_from_ref(target, Environment::volumes, CrossToml::env_volumes)
}

pub fn target(&self, target_list: &TargetList) -> Option<Target> {
Expand All @@ -275,11 +290,11 @@ impl Config {
}

pub fn dockerfile(&self, target: &Target) -> Result<Option<String>> {
self.string_from_config(target, Environment::dockerfile, CrossToml::dockerfile)
self.get_from_ref(target, Environment::dockerfile, CrossToml::dockerfile)
}

pub fn dockerfile_context(&self, target: &Target) -> Result<Option<String>> {
self.string_from_config(
self.get_from_ref(
target,
Environment::dockerfile_context,
CrossToml::dockerfile_context,
Expand All @@ -297,14 +312,10 @@ impl Config {
}

pub fn pre_build(&self, target: &Target) -> Result<Option<Vec<String>>> {
self.vec_from_config(
target,
|e, t| (None, e.pre_build(t)),
CrossToml::pre_build,
false,
)
self.get_from_ref(target, Environment::pre_build, CrossToml::pre_build)
}

// FIXME: remove when we disable sums in 0.3.0.
fn sum_of_env_toml_values<'a>(
&'a self,
env_values: Option<impl AsRef<[String]>>,
Expand Down Expand Up @@ -369,6 +380,11 @@ mod tests {
Target::from("aarch64-unknown-linux-gnu", &target_list)
}

fn target2() -> Target {
let target_list = target_list();
Target::from("armv7-unknown-linux-musleabihf", &target_list)
}

mod test_environment {

use super::*;
Expand Down Expand Up @@ -514,6 +530,29 @@ mod tests {
Ok(())
}

#[test]
pub fn env_target_then_toml_target_then_env_build_then_toml_build() -> Result<()> {
let mut map = HashMap::new();
map.insert("CROSS_BUILD_DOCKERFILE", "Dockerfile3");
map.insert(
"CROSS_TARGET_AARCH64_UNKNOWN_LINUX_GNU_DOCKERFILE",
"Dockerfile4",
);

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")));

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")));

Ok(())
}

#[test]
pub fn toml_build_passthrough_then_use_target_passthrough_both() -> Result<()> {
let map = HashMap::new();
Expand All @@ -527,7 +566,6 @@ mod tests {
config.env_volumes(&target())?,
Some(vec![s!("VOLUME3"), s!("VOLUME4")])
);
// TODO(ahuszagh) Need volumes

Ok(())
}
Expand Down Expand Up @@ -640,6 +678,13 @@ mod tests {
static TOML_BUILD_PRE_BUILD: &str = r#"
[build]
pre-build = ["apt-get update && apt-get install zlib-dev"]
"#;

static TOML_BUILD_DOCKERFILE: &str = r#"
[build]
dockerfile = "Dockerfile1"
[target.aarch64-unknown-linux-gnu]
dockerfile = "Dockerfile2"
"#;

static TOML_TARGET_XARGO_FALSE: &str = r#"
Expand Down
48 changes: 22 additions & 26 deletions src/cross_toml.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,17 +220,17 @@ impl CrossToml {
}

/// Returns the `{}.dockerfile` or `{}.dockerfile.file` part of `Cross.toml`
pub fn dockerfile(&self, target: &Target) -> Option<String> {
self.get_string(
pub fn dockerfile(&self, target: &Target) -> (Option<&String>, Option<&String>) {
self.get_ref(
target,
|b| b.dockerfile.as_ref().map(|c| &c.file),
|t| t.dockerfile.as_ref().map(|c| &c.file),
)
}

/// Returns the `target.{}.dockerfile.context` part of `Cross.toml`
pub fn dockerfile_context(&self, target: &Target) -> Option<String> {
self.get_string(
pub fn dockerfile_context(&self, target: &Target) -> (Option<&String>, Option<&String>) {
self.get_ref(
target,
|b| b.dockerfile.as_ref().and_then(|c| c.context.as_ref()),
|t| t.dockerfile.as_ref().and_then(|c| c.context.as_ref()),
Expand All @@ -255,7 +255,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<&[String]>, Option<&[String]>) {
self.get_vec(
self.get_ref(
target,
|b| b.pre_build.as_deref(),
|t| t.pre_build.as_deref(),
Expand All @@ -269,17 +269,17 @@ impl CrossToml {

/// Returns the `build.xargo` or the `target.{}.xargo` part of `Cross.toml`
pub fn xargo(&self, target: &Target) -> (Option<bool>, Option<bool>) {
self.get_bool(target, |b| b.xargo, |t| t.xargo)
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<bool>, Option<bool>) {
self.get_bool(target, |b| b.build_std, |t| t.build_std)
self.get_value(target, |b| b.build_std, |t| t.build_std)
}

/// 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(
self.get_ref(
target,
|build| build.env.passthrough.as_deref(),
|t| t.env.passthrough.as_deref(),
Expand All @@ -288,7 +288,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]>) {
self.get_vec(
self.get_ref(
target,
|build| build.env.volumes.as_deref(),
|t| t.env.volumes.as_deref(),
Expand Down Expand Up @@ -320,30 +320,26 @@ impl CrossToml {
.map(ToOwned::to_owned)
}

fn get_bool(
fn get_value<T>(
&self,
target: &Target,
get_build: impl Fn(&CrossBuildConfig) -> Option<bool>,
get_target: impl Fn(&CrossTargetConfig) -> Option<bool>,
) -> (Option<bool>, Option<bool>) {
target_triple: &Target,
get_build: impl Fn(&CrossBuildConfig) -> Option<T>,
get_target: impl Fn(&CrossTargetConfig) -> Option<T>,
) -> (Option<T>, Option<T>) {
let build = get_build(&self.build);
let target = self.get_target(target).and_then(get_target);

let target = self.get_target(target_triple).and_then(get_target);
(build, target)
}

fn get_vec(
fn get_ref<T: ?Sized>(
&self,
target_triple: &Target,
build: impl Fn(&CrossBuildConfig) -> Option<&[String]>,
target: impl Fn(&CrossTargetConfig) -> Option<&[String]>,
) -> (Option<&[String]>, Option<&[String]>) {
let target = if let Some(t) = self.get_target(target_triple) {
target(t)
} else {
None
};
(build(&self.build), target)
get_build: impl Fn(&CrossBuildConfig) -> Option<&T>,
get_target: impl Fn(&CrossTargetConfig) -> Option<&T>,
) -> (Option<&T>, Option<&T>) {
let build = get_build(&self.build);
let target = self.get_target(target_triple).and_then(get_target);
(build, target)
}
}

Expand Down

0 comments on commit 13f906c

Please sign in to comment.