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: make undefined more strict #1247

Open
wants to merge 2 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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ rattler_solve = { version = "1.2.3", default-features = false, features = ["reso
rattler_virtual_packages = { version = "1.1.10", default-features = false }
rattler_package_streaming = { version = "0.22.13", default-features = false }
lazy_static = "1.5.0"
strum = "0.26.3"

[dev-dependencies]
insta = { version = "1.41.1", features = ["yaml"] }
Expand Down
2 changes: 1 addition & 1 deletion src/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,10 +217,10 @@
.unwrap();

install_environments(&self, &finalized_dependencies, tool_configuration)
.await

Check warning on line 220 in src/cache.rs

View workflow job for this annotation

GitHub Actions / Format, Lint and Test the Python bindings

Diff in /home/runner/work/rattler-build/rattler-build/src/cache.rs
.into_diagnostic()?;

let selector_config = self.build_configuration.selector_config();
let selector_config = self.build_configuration.selector_config(self.recipe.build().number());
let mut jinja = Jinja::new(selector_config.clone());
for (k, v) in self.recipe.context.iter() {
jinja
Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ pub async fn get_build_output(
target_platform,
host_platform,
hash: None,
build_number: None,
build_platform: args.build_platform,
variant: BTreeMap::new(),
experimental: args.common.experimental,
Expand Down
3 changes: 2 additions & 1 deletion src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -355,13 +355,14 @@ impl BuildConfiguration {
}

/// Construct a `SelectorConfig` from the given `BuildConfiguration`
pub fn selector_config(&self) -> SelectorConfig {
pub fn selector_config(&self, build_number: u64) -> SelectorConfig {
SelectorConfig {
target_platform: self.target_platform,
host_platform: self.host_platform.platform,
build_platform: self.build_platform.platform,
variant: self.variant.clone(),
hash: Some(self.hash.clone()),
build_number: Some(build_number),
experimental: false,
allow_undefined: false,
}
Expand Down
2 changes: 1 addition & 1 deletion src/package_test/serialize_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@

Ok(test_files)
}
}

Check warning on line 55 in src/package_test/serialize_test.rs

View workflow job for this annotation

GitHub Actions / Format, Lint and Test the Python bindings

Diff in /home/runner/work/rattler-build/rattler-build/src/package_test/serialize_test.rs

fn default_jinja_context(output: &Output) -> Jinja {
let selector_config = output.build_configuration.selector_config();
let selector_config = output.build_configuration.selector_config(output.recipe.build().number());
let jinja = Jinja::new(selector_config).with_context(&output.recipe.context);
jinja
}
Expand Down
3 changes: 3 additions & 0 deletions src/recipe/jinja.rs
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,9 @@ fn set_jinja(config: &SelectorConfig) -> minijinja::Environment<'static> {
} = config.clone();

let mut env = Environment::empty();
if !allow_undefined {
env.set_undefined_behavior(minijinja::UndefinedBehavior::Strict);
}
default_tests(&mut env);
default_filters(&mut env);

Expand Down
2 changes: 1 addition & 1 deletion src/script/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,10 +371,10 @@
let host_prefix = self.build_configuration.directories.host_prefix.clone();
let target_platform = self.build_configuration.target_platform;
let mut env_vars = env_vars::vars(self, "BUILD");
env_vars.extend(env_vars::os_vars(&host_prefix, &target_platform));

Check warning on line 374 in src/script/mod.rs

View workflow job for this annotation

GitHub Actions / Format, Lint and Test the Python bindings

Diff in /home/runner/work/rattler-build/rattler-build/src/script/mod.rs
env_vars.extend(self.env_vars_from_variant());

let selector_config = self.build_configuration.selector_config();
let selector_config = self.build_configuration.selector_config(self.recipe.build().number());
let jinja = Jinja::new(selector_config.clone()).with_context(&self.recipe.context);

self.recipe
Expand Down
42 changes: 31 additions & 11 deletions src/selectors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

use minijinja::value::Value;
use rattler_conda_types::Platform;
use strum::IntoEnumIterator;

/// The selector config is used to render the recipe.
#[derive(Clone, Debug)]
Expand All @@ -22,6 +23,8 @@
pub build_platform: Platform,
/// The hash, if available
pub hash: Option<HashInfo>,
/// The build number if available
pub build_number: Option<u64>,
/// The variant config
pub variant: BTreeMap<NormalizedKey, String>,
/// Enable experimental features
Expand All @@ -45,15 +48,20 @@
Value::from_safe_string(self.host_platform.to_string()),
);

if let Some(platform) = self.host_platform.only_platform() {
context.insert(
platform.to_string(),
Value::from_safe_string(platform.to_string()),
);
}

if let Some(arch) = self.target_platform.arch() {
context.insert(arch.to_string(), Value::from(true));
for platform in Platform::iter() {
if let Some(only_platform) = platform.only_platform() {
context.insert(
only_platform.to_string(),
Value::from(self.host_platform == platform),
);
}

if let Some(arch) = platform.arch() {
context.insert(
arch.to_string(),
Value::from(self.host_platform.arch() == Some(arch)),
);
}
}

context.insert(
Expand Down Expand Up @@ -88,14 +96,25 @@

/// Create a new selector config from an existing one, replacing the variant
pub fn with_variant(
&self,
self,
variant: BTreeMap<NormalizedKey, String>,
target_platform: Platform,
) -> Self {
Self {
variant,
target_platform,
..self.clone()
..self
}

Check warning on line 107 in src/selectors.rs

View workflow job for this annotation

GitHub Actions / Format, Lint and Test the Python bindings

Diff in /home/runner/work/rattler-build/rattler-build/src/selectors.rs
}

/// Finish the selector config by adding the hash and build number.
/// After this, all variables are defined and `allow_undefined` is set to false.
pub fn finish(self, hash: HashInfo, build_number: u64) -> Self {
Self {
hash: Some(hash),
build_number: Some(build_number),
allow_undefined: false,
..self
}
}
}
Expand All @@ -107,6 +126,7 @@
host_platform: Platform::current(),
build_platform: Platform::current(),
hash: None,
build_number: None,
variant: Default::default(),
experimental: false,
allow_undefined: false,
Expand Down
10 changes: 7 additions & 3 deletions src/variant_render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,9 @@
let mut rendered_outputs = Vec::new();
// TODO: figure out if we can pre-compute the `noarch` value.
for output in outputs {
let config_with_variant =
selector_config.with_variant(combination.clone(), selector_config.target_platform);
let config_with_variant = selector_config
.clone()
.with_variant(combination.clone(), selector_config.target_platform);

let parsed_recipe = Recipe::from_node(output, config_with_variant).map_err(|err| {
let errs: ParseErrors = err
Expand Down Expand Up @@ -381,7 +382,10 @@
for (idx, output) in r.raw_outputs.vec.iter().enumerate() {
// use the correct target_platform here?
let config_with_variant = selector_config
.with_variant(combination.clone(), selector_config.target_platform);
.clone()

Check warning on line 385 in src/variant_render.rs

View workflow job for this annotation

GitHub Actions / Format, Lint and Test the Python bindings

Diff in /home/runner/work/rattler-build/rattler-build/src/variant_render.rs
.with_variant(combination.clone(), selector_config.target_platform)
// TODO finish with the real hash
.finish(HashInfo::from_variant(&Default::default(), &rattler_conda_types::NoArchType::none()), 123);

let parsed_recipe = Recipe::from_node(output, config_with_variant.clone()).unwrap();

Expand Down
Loading