Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: special case true / false boolean values in variant config #801

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/env_vars.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,9 @@ pub fn python_vars(output: &Output) -> HashMap<String, String> {
}

if let Some(npy_version) = output.variant().get("numpy") {
let np_ver = npy_version.split('.').collect::<Vec<_>>();
let np_ver = format!("{}.{}", np_ver[0], np_ver[1]);
let np_ver = npy_version.to_string();
let parts = np_ver.split('.').collect::<Vec<_>>();
let np_ver = format!("{}.{}", parts[0], parts[1]);

result.insert("NPY_VER".to_string(), np_ver);
}
Expand All @@ -101,7 +102,7 @@ pub fn r_vars(output: &Output) -> HashMap<String, String> {
let mut result = HashMap::<String, String>::new();

if let Some(r_ver) = output.variant().get("r-base") {
result.insert("R_VER".to_string(), r_ver.clone());
result.insert("R_VER".to_string(), r_ver.to_string());

let r_bin = if output.host_platform().is_windows() {
output.prefix().join("Scripts/R.exe")
Expand Down
42 changes: 24 additions & 18 deletions src/hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ use serde::{Deserialize, Serialize};
use serde_json::ser::Formatter;
use sha1::{Digest, Sha1};

use crate::utils::VariantValue;

/// A hash will be added if all of these are true for any dependency:
///
/// 1. package is an explicit dependency in build, host, or run deps
Expand Down Expand Up @@ -94,12 +96,17 @@ pub struct HashInput(String);

impl HashInput {
/// Create a new hash input from a variant
pub fn from_variant(variant: &BTreeMap<String, String>) -> Self {
pub fn from_variant(variant: &BTreeMap<String, VariantValue>) -> Self {
// TODO maybe re-asses what `conda-build` does here - keep numbers or format as string?
let input: BTreeMap<String, String> = variant
.iter()
.map(|(k, v)| (k.clone(), v.to_string()))
.collect();
let mut buf = Vec::new();
let mut ser = serde_json::Serializer::with_formatter(&mut buf, PythonFormatter {});

// BTree has sorted keys, which is important for hashing
variant
input
.serialize(&mut ser)
.expect("Failed to serialize input");

Expand All @@ -124,7 +131,7 @@ impl std::fmt::Display for HashInfo {
}

impl HashInfo {
fn hash_prefix(variant: &BTreeMap<String, String>, noarch: &NoArchType) -> String {
fn hash_prefix(variant: &BTreeMap<String, VariantValue>, noarch: &NoArchType) -> String {
if noarch.is_python() {
return "py".to_string();
}
Expand All @@ -148,7 +155,7 @@ impl HashInfo {

map.insert(
prefix.to_string(),
short_version_from_spec(version_spec, version_length),
short_version_from_spec(&version_spec.to_string(), version_length),
);
}

Expand All @@ -174,7 +181,7 @@ impl HashInfo {
}

/// Compute the build string for a given variant
pub fn from_variant(variant: &BTreeMap<String, String>, noarch: &NoArchType) -> Self {
pub fn from_variant(variant: &BTreeMap<String, VariantValue>, noarch: &NoArchType) -> Self {
Self {
hash: Self::hash_from_input(&HashInput::from_variant(variant)),
prefix: Self::hash_prefix(variant, noarch),
Expand All @@ -188,25 +195,24 @@ mod tests {
use std::collections::BTreeMap;

#[test]
fn test_hash() {
fn test_hash() -> Result<(), Box<dyn std::error::Error>> {
let mut input = BTreeMap::new();
input.insert("rust_compiler".to_string(), "rust".to_string());
input.insert("build_platform".to_string(), "osx-64".to_string());
input.insert("c_compiler".to_string(), "clang".to_string());
input.insert("target_platform".to_string(), "osx-arm64".to_string());
input.insert("openssl".to_string(), "3".to_string());
input.insert("rust_compiler".to_string(), "rust".parse()?);
input.insert("build_platform".to_string(), "osx-64".parse()?);
input.insert("c_compiler".to_string(), "clang".parse()?);
input.insert("target_platform".to_string(), "osx-arm64".parse()?);
input.insert("openssl".to_string(), "3".parse()?);
input.insert(
"CONDA_BUILD_SYSROOT".to_string(),
"/Applications/Xcode_13.2.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.0.sdk".to_string(),
);
input.insert(
"channel_targets".to_string(),
"conda-forge main".to_string(),
"/Applications/Xcode_13.2.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.0.sdk".parse()?,
);
input.insert("python".to_string(), "3.11.* *_cpython".to_string());
input.insert("c_compiler_version".to_string(), "14".to_string());
input.insert("channel_targets".to_string(), "conda-forge main".parse()?);
input.insert("python".to_string(), "3.11.* *_cpython".parse()?);
input.insert("c_compiler_version".to_string(), "14".parse()?);

let build_string_from_output = HashInfo::from_variant(&input, &NoArchType::none());
assert_eq!(build_string_from_output.to_string(), "py311h507f6e9");

Ok(())
}
}
6 changes: 3 additions & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,9 @@ pub async fn get_build_output(
.load_preset(comfy_table::presets::UTF8_FULL_CONDENSED)
.apply_modifier(comfy_table::modifiers::UTF8_ROUND_CORNERS)
.set_header(vec!["Variant", "Version"]);
for (key, value) in discovered_output.used_vars.iter() {
table.add_row(vec![key, value]);
}
// for (key, value) in discovered_output.used_vars.iter() {
// table.add_row(vec![key, value]);
// }
tracing::info!("\n{}\n", table);
}
drop(enter);
Expand Down
7 changes: 4 additions & 3 deletions src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use crate::{
recipe::parser::{Recipe, Source},
render::resolved_dependencies::FinalizedDependencies,
system_tools::SystemTools,
utils::VariantValue,
};
/// A Git revision
#[derive(Serialize, Deserialize, Debug, Clone)]
Expand Down Expand Up @@ -213,7 +214,7 @@ pub struct BuildConfiguration {
/// The build platform (the platform that the build is running on)
pub build_platform: Platform,
/// The selected variant for this build
pub variant: BTreeMap<String, String>,
pub variant: BTreeMap<String, VariantValue>,
/// THe computed hash of the variant
pub hash: HashInfo,
/// The directories for the build (work, source, build, host, ...)
Expand Down Expand Up @@ -361,7 +362,7 @@ impl Output {
}

/// Shorthand to retrieve the variant configuration for this output
pub fn variant(&self) -> &BTreeMap<String, String> {
pub fn variant(&self) -> &BTreeMap<String, VariantValue> {
&self.build_configuration.variant
}

Expand Down Expand Up @@ -550,7 +551,7 @@ impl Output {
table.set_header(vec!["Key", "Value"]);
}
self.build_configuration.variant.iter().for_each(|(k, v)| {
table.add_row(vec![k, v]);
table.add_row(vec![k, v.to_string().as_str()]);
});
writeln!(f, "{}\n", table)?;

Expand Down
2 changes: 1 addition & 1 deletion src/post_process/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ fn find_system_libs(output: &Output) -> Result<GlobSet, globset::Error> {
.variant
.get("CONDA_BUILD_SYSROOT")
{
system_libs.add(Glob::new(&format!("{}/**/*", sysroot))?);
system_libs.add(Glob::new(&format!("{}/**/*", sysroot.to_string()))?);
} else {
for v in default_sysroot {
system_libs.add(Glob::new(v)?);
Expand Down
3 changes: 2 additions & 1 deletion src/recipe/custom_yaml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,7 @@ impl From<bool> for ScalarNode {
}
}
}

macro_rules! scalar_from_to_number {
($t:ident, $as:ident) => {
impl From<$t> for ScalarNode {
Expand Down Expand Up @@ -628,7 +629,7 @@ impl fmt::Debug for SequenceNode {
/// Mapping nodes in YAML are defined as a key/value mapping where the keys are
/// unique and always scalars, whereas values may be YAML nodes of any kind.
///
/// Because ther is an example that on the `context` key-value definition, a later
/// Because there is an example that on the `context` key-value definition, a later
/// key was defined as a jinja string using previous values, we need to care about
/// insertion order we use [`IndexMap`] for this.
///
Expand Down
77 changes: 37 additions & 40 deletions src/recipe/custom_yaml/rendered.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,11 @@ impl RenderedScalarNode {
_ => None,
}
}

/// Try to parse the value as integer
pub fn as_integer(&self) -> Option<i64> {
self.value.parse().ok()
}
}

impl HasSpan for RenderedScalarNode {
Expand Down Expand Up @@ -343,46 +348,6 @@ impl From<bool> for RenderedScalarNode {
}
}
}
macro_rules! scalar_from_to_number {
($t:ident, $as:ident) => {
impl From<$t> for RenderedScalarNode {
#[doc = "Convert from "]
#[doc = stringify!($t)]
#[doc = r#" into a node"#]
fn from(value: $t) -> Self {
format!("{}", value).into()
}
}

impl RenderedScalarNode {
#[doc = "Treat the scalar node as "]
#[doc = stringify!($t)]
#[doc = r#".

If this scalar node's value can be represented properly as
a number of the right kind then return it. This is essentially
a shortcut for using the `FromStr` trait on the return value of
`.as_str()`."#]
pub fn $as(&self) -> Option<$t> {
use std::str::FromStr;
$t::from_str(&self.value).ok()
}
}
};
}

scalar_from_to_number!(i8, as_i8);
scalar_from_to_number!(i16, as_i16);
scalar_from_to_number!(i32, as_i32);
scalar_from_to_number!(i64, as_i64);
scalar_from_to_number!(i128, as_i128);
scalar_from_to_number!(isize, as_isize);
scalar_from_to_number!(u8, as_u8);
scalar_from_to_number!(u16, as_u16);
scalar_from_to_number!(u32, as_u32);
scalar_from_to_number!(u64, as_u64);
scalar_from_to_number!(u128, as_u128);
scalar_from_to_number!(usize, as_usize);

/// A marked YAML sequence node
///
Expand Down Expand Up @@ -713,3 +678,35 @@ impl Render<RenderedSequenceNode> for SequenceNodeInternal {
Ok(RenderedSequenceNode::from(rendered))
}
}

#[cfg(test)]
mod test {
use super::RenderedScalarNode;

#[test]
fn test_scalar_value() {
let scalar = RenderedScalarNode::from("test");
assert_eq!(scalar.as_str(), "test");

let scalar = RenderedScalarNode::from("true");
assert_eq!(scalar.as_bool(), Some(true));

let scalar = RenderedScalarNode::from("false");
assert_eq!(scalar.as_bool(), Some(false));

let scalar = RenderedScalarNode::from("123");
assert_eq!(scalar.as_integer(), Some(123));

let scalar = RenderedScalarNode::from("123.45");
assert_eq!(scalar.as_integer(), None);

let scalar = RenderedScalarNode::from("'true'");
assert_eq!(scalar.as_bool(), None);

let scalar = RenderedScalarNode::from("'123'");
assert_eq!(scalar.as_integer(), None);

let scalar = RenderedScalarNode::from("'123'");
assert_eq!(scalar.as_str(), "'123'");
}
}
27 changes: 15 additions & 12 deletions src/recipe/jinja.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use rattler_conda_types::{PackageName, ParseStrictness, Platform, Version, Versi
use crate::render::pin::PinArgs;
pub use crate::render::pin::{Pin, PinExpression};
pub use crate::selectors::SelectorConfig;
use crate::utils::VariantValue;

use super::parser::{Dependency, PinCompatible, PinSubpackage};

Expand Down Expand Up @@ -64,7 +65,7 @@ impl<'a> Jinja<'a> {
if expr.is_empty() {
return Ok(Value::UNDEFINED);
}
let expr = self.env.compile_expression(&expr)?;
let expr = self.env.compile_expression(str)?;
expr.eval(self.context())
}
}
Expand Down Expand Up @@ -192,7 +193,7 @@ fn default_compiler(platform: Platform, language: &str) -> Option<String> {
fn compiler_stdlib_eval(
lang: &str,
platform: Platform,
variant: &Arc<BTreeMap<String, String>>,
variant: &Arc<BTreeMap<String, VariantValue>>,
prefix: &str,
) -> Result<String, minijinja::Error> {
let variant_key = format!("{lang}_{prefix}");
Expand All @@ -206,12 +207,12 @@ fn compiler_stdlib_eval(

let res = if let Some(name) = variant
.get(&variant_key)
.cloned()
.map(|v| v.to_string())
.or_else(|| default_fn(platform, lang))
{
// check if we also have a compiler version
if let Some(version) = variant.get(&variant_key_version) {
Some(format!("{name}_{platform} {version}"))
Some(format!("{name}_{platform} {}", version.to_string()))
} else {
Some(format!("{name}_{platform}"))
}
Expand Down Expand Up @@ -381,10 +382,10 @@ fn set_jinja(config: &SelectorConfig) -> minijinja::Environment<'static> {
let arch_str = arch.map(|arch| format!("{arch}"));

let cdt_arch = if let Some(s) = variant_clone.get("cdt_arch") {
s.as_str()
s.to_string()
} else {
match arch {
Some(Arch::X86) => "i686",
Some(Arch::X86) => "i686".to_string(),
_ => arch_str
.as_ref()
.ok_or_else(|| {
Expand All @@ -393,16 +394,18 @@ fn set_jinja(config: &SelectorConfig) -> minijinja::Environment<'static> {
"No target or build architecture provided.",
)
})?
.as_str(),
.to_string(),
}
};

let cdt_name = variant_clone.get("cdt_name").map_or_else(
|| match arch {
Some(Arch::S390X | Arch::Aarch64 | Arch::Ppc64le | Arch::Ppc64) => "cos7",
_ => "cos6",
Some(Arch::S390X | Arch::Aarch64 | Arch::Ppc64le | Arch::Ppc64) => {
"cos7".to_string()
}
_ => "cos6".to_string(),
},
String::as_str,
ToString::to_string,
);

let res = package_name.split_once(' ').map_or_else(
Expand Down Expand Up @@ -1085,7 +1088,7 @@ mod tests {
#[test]
#[rustfmt::skip]
fn eval_match() {
let variant = BTreeMap::from_iter(vec![("python".to_string(), "3.7".to_string())]);
let variant = BTreeMap::from_iter(vec![("python".to_string(), "3.7".parse().unwrap())]);

let options = SelectorConfig {
target_platform: Platform::Linux64,
Expand All @@ -1107,7 +1110,7 @@ mod tests {
#[test]
#[rustfmt::skip]
fn eval_complicated_match() {
let variant = BTreeMap::from_iter(vec![("python".to_string(), "3.7.* *_cpython".to_string())]);
let variant = BTreeMap::from_iter(vec![("python".to_string(), "3.7.* *_cpython".parse().unwrap())]);

let options = SelectorConfig {
target_platform: Platform::Linux64,
Expand Down
Loading
Loading