Skip to content
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
3 changes: 3 additions & 0 deletions .yamllint
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
extends: default
ignore: |
tests/data/jinja_for*.yml
34 changes: 20 additions & 14 deletions docs/netsuke-design.md
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,9 @@ level keys.
future evolution of the schema while maintaining backward compatibility. This
version string should be parsed and validated using the `semver` crate.[^4]

- `vars`: A mapping of global key-value string pairs. These variables are
available for substitution in rule commands and target definitions and are
exposed to the Jinja templating context.
- `vars`: A mapping of global key-value pairs. Keys must be strings. Values may
be strings, numbers, booleans, or sequences. These variables seed the Jinja
templating context and drive control flow within the manifest.

- `macros`: An optional list of Jinja macro definitions. Each item provides a
`signature` string using standard Jinja syntax and a `body` declared with the
Expand Down Expand Up @@ -414,7 +414,7 @@ pub struct NetsukeManifest {
pub netsuke_version: Version,

#[serde(default)]
pub vars: HashMap<String, String>,
pub vars: HashMap<String, serde_yml::Value>,

#[serde(default)]
pub rules: Vec<Rule>,
Expand Down Expand Up @@ -466,7 +466,7 @@ pub struct Target {
pub order_only_deps: StringOrList,

#[serde(default)]
pub vars: HashMap<String, String>,
pub vars: HashMap<String, serde_yml::Value>,

/// Run this target when requested even if a file with the same name exists.
#[serde(default)]
Expand Down Expand Up @@ -585,15 +585,14 @@ Unknown fields are rejected to surface user errors early. `StringOrList`
provides a default `Empty` variant, so optional lists are trivial to represent.
The manifest version is parsed using the `semver` crate to validate that it
follows semantic versioning rules. Global and target variable maps now share
the `HashMap<String, String>` type for consistency. This keeps YAML manifests
concise while ensuring forward compatibility. Targets also accept optional
`phony` and `always` booleans. They default to `false`, making it explicit when
an action should run regardless of file timestamps. Targets listed in the
`actions` section are deserialised using a custom helper so they are always
treated as `phony` tasks. This ensures preparation actions never generate build
artefacts. Convenience functions in `src/manifest.rs` load a manifest from a
string or a file path, returning `anyhow::Result` for straightforward error
handling.
the `HashMap<String, serde_yml::Value>` type so booleans and sequences are
preserved for Jinja control flow. Targets also accept optional `phony` and
`always` booleans. They default to `false`, making it explicit when an action
should run regardless of file timestamps. Targets listed in the `actions`
section are deserialised using a custom helper so they are always treated as
`phony` tasks. This ensures preparation actions never generate build artefacts.
Convenience functions in `src/manifest.rs` load a manifest from a string or a
file path, returning `anyhow::Result` for straightforward error handling.

### 3.5 Testing

Expand Down Expand Up @@ -666,6 +665,13 @@ lenient undefined behaviour. The resulting YAML is parsed to obtain the global
variables, which are then injected into the environment before a second, strict
render pass produces the final manifest for deserialisation.

The parser copies `vars` values into the environment using
`Value::from_serializable`. This preserves native YAML types so Jinja's
`{% if %}` and `{% for %}` constructs can branch on booleans or iterate over
sequences. Keys must be strings; any non-string key causes manifest parsing to
fail. Attempting to iterate over a non-sequence results in a render error
surfaced during manifest loading.

### 4.3 User-Defined Macros

Netsuke allows users to declare reusable Jinja macros directly in the manifest.
Expand Down
2 changes: 1 addition & 1 deletion docs/roadmap.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ configurations with variables, control flow, and custom functions.

- [ ] **Dynamic Features and Custom Functions:**

- [ ] Implement support for basic Jinja control structures (`{% if %}` and
- [x] Implement support for basic Jinja control structures (`{% if %}` and
`{% for %}`)

- [ ] Implement the foreach key for target generation.
Expand Down
9 changes: 6 additions & 3 deletions src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@

use semver::Version;
use serde::{Deserialize, Serialize, de::Deserializer};
use std::collections::HashMap;

/// Map type for `vars` blocks, preserving YAML values.
pub type Vars = HashMap<String, serde_yml::Value>;

fn deserialize_actions<'de, D>(deserializer: D) -> Result<Vec<Target>, D::Error>
where
Expand All @@ -30,7 +34,6 @@ where
}
Ok(actions)
}
use std::collections::HashMap;

/// Top-level manifest structure parsed from a `Netsukefile`.
///
Expand Down Expand Up @@ -61,7 +64,7 @@ pub struct NetsukeManifest {

/// Global key/value pairs available to recipes.
#[serde(default)]
pub vars: HashMap<String, String>,
pub vars: Vars,

/// Named rule templates that can be referenced by targets.
#[serde(default)]
Expand Down Expand Up @@ -187,7 +190,7 @@ pub struct Target {

/// Target-scoped variables available during command execution.
#[serde(default)]
pub vars: HashMap<String, String>,
pub vars: Vars,

/// Declares that the target does not correspond to a real file.
#[serde(default)]
Expand Down
30 changes: 12 additions & 18 deletions src/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
//! They wrap `serde_yml` and add basic file handling.

use crate::ast::NetsukeManifest;
use anyhow::{Context, Result};
use anyhow::{Context, Result, bail};
use minijinja::{Environment, UndefinedBehavior, context, value::Value};
use std::{collections::HashMap, fs, path::Path};
use std::{fs, path::Path};

/// Parse a manifest string using Jinja for templating.
///
Expand Down Expand Up @@ -48,22 +48,16 @@ pub fn from_str(yaml: &str) -> Result<NetsukeManifest> {

let doc: serde_yml::Value =
serde_yml::from_str(&rendered).context("first-pass YAML parse error")?;
let vars = doc
.get("vars")
.and_then(|v| v.as_mapping())
.map(|m| {
m.iter()
.filter_map(|(k, v)| k.as_str().and_then(|key| v.as_str().map(|val| (key, val))))
.map(|(k, v)| (k.to_string(), v.to_string()))
.collect::<HashMap<_, _>>()
})
.unwrap_or_default();

// Populate the environment with the extracted variables for subsequent
// rendering. Undefined variables now trigger errors to surface template
// mistakes early.
for (key, value) in vars {
env.add_global(key, Value::from(value));
if let Some(vars) = doc.get("vars").and_then(|v| v.as_mapping()) {
// Copy each key-value pair into the environment, preserving native YAML
// types so control structures like `{% if %}` and `{% for %}` can operate
// on booleans and sequences.
for (k, v) in vars {
let Some(key) = k.as_str() else {
bail!("non-string key in 'vars' mapping: {k:?}");
};
env.add_global(key, Value::from_serialize(v.clone()));
}
}

env.set_undefined_behavior(UndefinedBehavior::Strict);
Expand Down
11 changes: 11 additions & 0 deletions tests/data/jinja_for.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
netsuke_version: 1.0.0
vars:
items:
- foo
- bar
targets:
{% for item in items %}
- name: "{{ item }}"
command: "echo {{ item }}"
{% endfor %}

9 changes: 9 additions & 0 deletions tests/data/jinja_for_invalid.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
netsuke_version: 1.0.0
vars:
items: 1
targets:
{% for item in items %}
- name: "{{ item }}"
command: "echo {{ item }}"
{% endfor %}

6 changes: 6 additions & 0 deletions tests/data/jinja_if.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
netsuke_version: 1.0.0
vars:
enable: true
targets:
- name: hello
command: "{% if enable %}echo on{% else %}echo off{% endif %}"
6 changes: 6 additions & 0 deletions tests/data/jinja_if_disabled.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
netsuke_version: 1.0.0
vars:
enable: false
targets:
- name: hello
command: "{% if enable %}echo on{% else %}echo off{% endif %}"
26 changes: 26 additions & 0 deletions tests/features/manifest.feature
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,29 @@ Feature: Manifest Parsing
Given the manifest file "tests/data/jinja_undefined.yml" is parsed
When the parsing result is checked
Then parsing the manifest fails

Scenario: Rendering Jinja conditionals in a manifest
Given the manifest file "tests/data/jinja_if.yml" is parsed
When the manifest is checked
Then the first target name is "hello"
And the first target command is "echo on"

Scenario: Rendering Jinja conditionals in a manifest (disabled)
Given the manifest file "tests/data/jinja_if_disabled.yml" is parsed
When the manifest is checked
Then the first target name is "hello"
And the first target command is "echo off"

Scenario: Rendering Jinja loops in a manifest
Given the manifest file "tests/data/jinja_for.yml" is parsed
When the manifest is checked
Then the manifest has 2 targets
And the target 1 name is "foo"
And the target 1 command is "echo foo"
And the target 2 name is "bar"
And the target 2 command is "echo bar"

Scenario: Parsing fails when a Jinja loop iterates over a non-list
Given the manifest file "tests/data/jinja_for_invalid.yml" is parsed
When the parsing result is checked
Then parsing the manifest fails
121 changes: 96 additions & 25 deletions tests/manifest_jinja_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,17 @@
use netsuke::{ast::Recipe, manifest};
use rstest::rstest;

fn manifest_yaml(body: &str) -> String {
format!("netsuke_version: 1.0.0\n{body}")
}

#[rstest]
fn renders_global_vars() {
let yaml = r"
netsuke_version: 1.0.0
vars:
who: world
targets:
- name: hello
command: echo {{ who }}
";

let manifest = manifest::from_str(yaml).expect("parse");
let yaml = manifest_yaml(
"vars:\n who: world\ntargets:\n - name: hello\n command: echo {{ who }}\n",
);

let manifest = manifest::from_str(&yaml).expect("parse");
let first = manifest.targets.first().expect("target");
if let Recipe::Command { command } = &first.recipe {
assert_eq!(command, "echo world");
Expand All @@ -25,24 +24,96 @@ targets:

#[rstest]
fn undefined_variable_errors() {
let yaml = r"
netsuke_version: 1.0.0
targets:
- name: hello
command: echo {{ missing }}
";
let yaml = manifest_yaml("targets:\n - name: hello\n command: echo {{ missing }}\n");

assert!(manifest::from_str(yaml).is_err());
assert!(manifest::from_str(&yaml).is_err());
}

#[rstest]
fn syntax_error_errors() {
let yaml = r"
netsuke_version: 1.0.0
targets:
- name: hello
command: echo {{ who
";

assert!(manifest::from_str(yaml).is_err());
let yaml = manifest_yaml("targets:\n - name: hello\n command: echo {{ who\n");

assert!(manifest::from_str(&yaml).is_err());
}

#[rstest]
#[case(true, "echo on")]
#[case(false, "echo off")]
fn renders_if_blocks(#[case] flag: bool, #[case] expected: &str) {
let cmd = "{% if flag %}echo on{% else %}echo off{% endif %}";
let yaml = manifest_yaml(&format!(
concat!(
"vars:\n",
" flag: {flag}\n",
"targets:\n",
" - name: test\n",
" command: {cmd}\n",
),
flag = flag,
cmd = cmd,
));

let manifest = manifest::from_str(&yaml).expect("parse");
let first = manifest.targets.first().expect("target");
if let Recipe::Command { command } = &first.recipe {
assert_eq!(command, expected);
} else {
panic!("Expected command recipe, got: {:?}", first.recipe);
}
}

#[rstest]
fn renders_for_loops() {
let yaml = manifest_yaml(
"vars:\n items:\n - a\n - b\ntargets:\n{% for item in items %}\n - name: \"{{ item }}\"\n command: \"echo {{ item }}\"\n{% endfor %}\n",
);

let manifest = manifest::from_str(&yaml).expect("parse");
assert_eq!(manifest.targets.len(), 2);
let names: Vec<_> = manifest
.targets
.iter()
.map(|t| match &t.name {
netsuke::ast::StringOrList::String(s) => s.clone(),
other => panic!("Expected String, got: {other:?}"),
})
.collect();
assert_eq!(names, vec!["a", "b"]);

let commands: Vec<_> = manifest
.targets
.iter()
.map(|t| match &t.recipe {
Recipe::Command { command } => command.clone(),
other => panic!("Expected command recipe, got: {other:?}"),
})
.collect();
assert_eq!(commands, vec!["echo a", "echo b"]);
}

#[rstest]
fn for_loop_non_iterable_errors() {
let yaml = manifest_yaml(
"vars:\n items: 1\ntargets:\n{% for item in items %}\n - name: \"{{ item }}\"\n command: \"echo {{ item }}\"\n{% endfor %}\n",
);

assert!(manifest::from_str(&yaml).is_err());
}

#[rstest]
fn undefined_in_if_errors() {
let yaml = manifest_yaml(
"targets:\n - name: test\n command: {% if missing %}echo hi{% endif %}\n",
);

assert!(manifest::from_str(&yaml).is_err());
}

#[rstest]
fn undefined_in_for_errors() {
let yaml = manifest_yaml(
"targets:\n{% for item in missing %}\n - name: \"{{ item }}\"\n command: \"echo {{ item }}\"\n{% endfor %}\n",
);

assert!(manifest::from_str(&yaml).is_err());
}
Loading
Loading