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: delay jinja evaluation for script #894

Merged
merged 2 commits into from
Sep 24, 2024
Merged
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 src/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ impl Output {
&self.build_configuration.directories.recipe_dir,
&self.build_configuration.directories.host_prefix,
Some(&self.build_configuration.directories.build_prefix),
None, // TODO fix this to be proper Jinja context
)
.await
.into_diagnostic()?;
Expand Down
18 changes: 17 additions & 1 deletion src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ use url::Url;
use crate::{
console_utils::github_integration_enabled,
hash::HashInfo,
recipe::parser::{Recipe, Source},
recipe::{
jinja::SelectorConfig,
parser::{Recipe, Source},
},
render::resolved_dependencies::FinalizedDependencies,
system_tools::SystemTools,
tool_configuration,
Expand Down Expand Up @@ -279,6 +282,19 @@ impl BuildConfiguration {
pub fn cross_compilation(&self) -> bool {
self.target_platform != self.build_platform
}

/// Construct a `SelectorConfig` from the given `BuildConfiguration`
pub fn selector_config(&self) -> SelectorConfig {
SelectorConfig {
target_platform: self.target_platform,
host_platform: self.host_platform,
build_platform: self.build_platform,
variant: self.variant.clone(),
hash: Some(self.hash.clone()),
experimental: false,
allow_undefined: false,
}
}
}

/// A package identifier
Expand Down
17 changes: 12 additions & 5 deletions src/package_test/run_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ impl Tests {
})?;

script
.run_script(env_vars, tmp_dir.path(), cwd, environment, None)
.run_script(env_vars, tmp_dir.path(), cwd, environment, None, None)
.await
.map_err(|e| TestError::TestFailed(e.to_string()))?;
}
Expand All @@ -136,7 +136,7 @@ impl Tests {
};

script
.run_script(env_vars, tmp_dir.path(), cwd, environment, None)
.run_script(env_vars, tmp_dir.path(), cwd, environment, None, None)
.await
.map_err(|e| TestError::TestFailed(e.to_string()))?;
}
Expand Down Expand Up @@ -472,7 +472,7 @@ impl PythonTest {

let tmp_dir = tempfile::tempdir()?;
script
.run_script(Default::default(), tmp_dir.path(), path, prefix, None)
.run_script(Default::default(), tmp_dir.path(), path, prefix, None, None)
.await
.map_err(|e| TestError::TestFailed(e.to_string()))?;

Expand All @@ -487,7 +487,7 @@ impl PythonTest {
..Script::default()
};
script
.run_script(Default::default(), path, path, prefix, None)
.run_script(Default::default(), path, path, prefix, None, None)
.await
.map_err(|e| TestError::TestFailed(e.to_string()))?;

Expand Down Expand Up @@ -588,7 +588,14 @@ impl CommandsTest {

tracing::info!("Testing commands:");
self.script
.run_script(env_vars, tmp_dir.path(), path, &run_env, build_env.as_ref())
.run_script(
env_vars,
tmp_dir.path(),
path,
&run_env,
build_env.as_ref(),
None,
)
.await
.map_err(|e| TestError::TestFailed(e.to_string()))?;

Expand Down
42 changes: 33 additions & 9 deletions src/recipe/custom_yaml/rendered.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ impl TryFrom<&marked_yaml::Node> for RenderedNode {
#[derive(Clone)]
pub struct RenderedScalarNode {
span: marked_yaml::Span,
source: String,
value: String,
}

Expand All @@ -241,12 +242,16 @@ impl Serialize for RenderedScalarNode {
}

impl RenderedScalarNode {
pub fn new(span: marked_yaml::Span, value: String) -> Self {
Self { span, value }
pub fn new(span: marked_yaml::Span, source: String, value: String) -> Self {
Self {
span,
source,
value,
}
}

pub fn new_blank() -> Self {
Self::new(marked_yaml::Span::new_blank(), String::new())
Self::new(marked_yaml::Span::new_blank(), String::new(), String::new())
}

/// Treat the scalar node as a string
Expand All @@ -256,6 +261,11 @@ impl RenderedScalarNode {
&self.value
}

/// Return the source with the original Jinja template
pub fn source(&self) -> &str {
&self.source
}

/// Treat the scalar node as a boolean
///
/// If the scalar contains any of the following then it is true:
Expand Down Expand Up @@ -320,14 +330,18 @@ impl fmt::Debug for RenderedScalarNode {
impl<'a> From<&'a str> for RenderedScalarNode {
/// Convert from any borrowed string into a node
fn from(value: &'a str) -> Self {
Self::new(marked_yaml::Span::new_blank(), value.to_owned())
Self::new(
marked_yaml::Span::new_blank(),
value.to_owned(),
value.to_owned(),
)
}
}

impl From<String> for RenderedScalarNode {
/// Convert from any owned string into a node
fn from(value: String) -> Self {
Self::new(marked_yaml::Span::new_blank(), value)
Self::new(marked_yaml::Span::new_blank(), value.clone(), value)
}
}

Expand All @@ -353,7 +367,11 @@ impl From<MarkedScalarNode> for RenderedScalarNode {

impl From<&MarkedScalarNode> for RenderedScalarNode {
fn from(value: &MarkedScalarNode) -> Self {
Self::new(*value.span(), value.as_str().to_owned())
Self::new(
*value.span(),
value.as_str().to_owned(),
value.as_str().to_owned(),
)
}
}

Expand Down Expand Up @@ -614,6 +632,7 @@ impl Render<RenderedNode> for Node {
Node::Null(n) => Ok(RenderedNode::Null(RenderedScalarNode::new(
*n.span(),
n.as_str().to_owned(),
n.as_str().to_owned(),
))),
}
}
Expand All @@ -628,7 +647,7 @@ impl Render<RenderedNode> for ScalarNode {
label = jinja_error_to_label(&err),
)]
})?;
let rendered = RenderedScalarNode::new(*self.span(), rendered);
let rendered = RenderedScalarNode::new(*self.span(), self.as_str().to_string(), rendered);

if rendered.is_empty() {
Ok(RenderedNode::Null(rendered))
Expand All @@ -652,7 +671,7 @@ impl Render<Option<RenderedNode>> for ScalarNode {
)]
})?;

let rendered = RenderedScalarNode::new(*self.span(), rendered);
let rendered = RenderedScalarNode::new(*self.span(), self.as_str().to_string(), rendered);

if rendered.is_empty() {
Ok(None)
Expand All @@ -679,7 +698,11 @@ impl Render<RenderedMappingNode> for MappingNode {
let mut rendered = IndexMap::new();

for (key, value) in self.iter() {
let key = RenderedScalarNode::new(*key.span(), key.as_str().to_owned());
let key = RenderedScalarNode::new(
*key.span(),
key.as_str().to_owned(),
key.as_str().to_owned(),
);
let value: RenderedNode = value.render(jinja, &format!("{name}.{}", key.as_str()))?;
if value.is_null() {
continue;
Expand All @@ -700,6 +723,7 @@ impl Render<RenderedNode> for SequenceNode {
return Ok(RenderedNode::Null(RenderedScalarNode::new(
*self.span(),
String::new(),
String::new(),
)));
}

Expand Down
54 changes: 29 additions & 25 deletions src/recipe/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ use crate::recipe::custom_yaml::Node;
pub struct Recipe {
/// The schema version of this recipe YAML file
pub schema_version: u64,
/// The context values of this recipe
pub context: IndexMap<String, String>,
/// The package information
pub package: Package,
/// The cache build that should be used for this package
Expand Down Expand Up @@ -155,37 +157,38 @@ impl Recipe {
})?;

// add context values
if let Some(context) = root_node.get("context") {
let context = context.as_mapping().ok_or_else(|| {
let mut context = IndexMap::new();

if let Some(context_map) = root_node.get("context") {
let context_map = context_map.as_mapping().ok_or_else(|| {
vec![_partialerror!(
*context.span(),
*context_map.span(),
ErrorKind::ExpectedMapping,
help = "`context` must always be a mapping"
)]
})?;

context
.iter()
.map(|(k, v)| {
let val = v.as_scalar().ok_or_else(|| {
vec![_partialerror!(
*v.span(),
ErrorKind::ExpectedScalar,
help = "`context` values must always be scalars (strings)"
)]
})?;
let rendered: Option<ScalarNode> =
val.render(&jinja, &format!("context.{}", k.as_str()))?;

if let Some(rendered) = rendered {
jinja.context_mut().insert(
k.as_str().to_owned(),
Value::from_safe_string(rendered.as_str().to_string()),
);
}
Ok(())
})
.flatten_errors()?;
for (k, v) in context_map.iter() {
let val = v.as_scalar().ok_or_else(|| {
vec![_partialerror!(
*v.span(),
ErrorKind::ExpectedScalar,
help = "`context` values must always be scalars (strings)"
)]
})?;
let rendered: Option<ScalarNode> =
val.render(&jinja, &format!("context.{}", k.as_str()))?;

if let Some(rendered) = rendered {
context.insert(k.as_str().to_string(), rendered.as_str().to_string());
// also immediately insert into jinja context so that the value can be used
// in later jinja expressions
jinja.context_mut().insert(
k.as_str().to_string(),
Value::from_safe_string(rendered.as_str().to_string()),
);
}
}
}

let rendered_node: RenderedMappingNode = root_node.render(&jinja, "ROOT")?;
Expand Down Expand Up @@ -261,6 +264,7 @@ impl Recipe {

let recipe = Recipe {
schema_version,
context,
package: package.ok_or_else(|| {
vec![_partialerror!(
*root_node.span(),
Expand Down
15 changes: 9 additions & 6 deletions src/recipe/parser/script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,16 +217,19 @@ impl TryConvertNode<Script> for RenderedNode {

impl TryConvertNode<Script> for RenderedScalarNode {
fn try_convert(&self, _name: &str) -> Result<Script, Vec<PartialParsingError>> {
Ok(ScriptContent::CommandOrPath(self.as_str().to_owned()).into())
Ok(ScriptContent::CommandOrPath(self.source().to_owned()).into())
}
}

impl TryConvertNode<Script> for RenderedSequenceNode {
fn try_convert(&self, name: &str) -> Result<Script, Vec<PartialParsingError>> {
let strings = self
.iter()
.map(|node| node.try_convert(name))
.collect::<Result<Vec<String>, _>>()?;
fn try_convert(&self, _name: &str) -> Result<Script, Vec<PartialParsingError>> {
let mut strings: Vec<String> = Vec::new();

for string in self.iter() {
if let RenderedNode::Scalar(s) = string {
strings.push(s.source().to_owned());
}
}

if strings.len() == 1 {
Ok(ScriptContent::CommandOrPath(strings[0].clone()).into())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ source: src/recipe/parser.rs
expression: recipe
---
schema_version: 1
context: {}
package:
name: zlib
version: 0.1.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ source: src/recipe/parser.rs
expression: recipe
---
schema_version: 1
context: {}
package:
name: zlib
version: 0.1.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ source: src/recipe/parser.rs
expression: "serde_yaml::to_string(&recipe).unwrap()"
---
schema_version: 1
context: {}
package:
name: single_output
version: 0.1.0
Expand Down Expand Up @@ -120,7 +121,8 @@ requirements:
from_package:
- python
tests:
- script: echo FOO
- script: |
echo FOO
requirements:
run:
- bash
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ source: src/recipe/parser.rs
expression: recipe
---
schema_version: 1
context: {}
package:
name: blib
version: 0.1.0
build:
number: 0
script: test succeeded
script: "${{ \"test succeeded\" if true }}"
requirements: {}
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@ source: src/recipe/parser.rs
expression: recipe
---
schema_version: 1
context: {}
package:
name: zlib
version: 0.1.0
build:
number: 0
script: "echo 'world'"
script: "${{ \"echo 'world'\" if true }}"
prefix_detection:
force_file_type:
binary:
Expand Down
Loading
Loading