From 4c50b6773f2ed923b9388dcc1e8d09a1673ec0d5 Mon Sep 17 00:00:00 2001 From: Leynos Date: Sat, 23 Aug 2025 11:04:48 +0100 Subject: [PATCH 01/11] Add glob helper for path expansion --- Cargo.lock | 1 + Cargo.toml | 1 + docs/netsuke-design.md | 7 +++-- docs/roadmap.md | 4 +-- src/manifest.rs | 43 +++++++++++++++++++++++++- tests/data/glob.yml | 5 +++ tests/data/glob_files/a.txt | 1 + tests/data/glob_files/b.txt | 1 + tests/data/glob_invalid.yml | 5 +++ tests/features/manifest.feature | 12 ++++++++ tests/manifest_glob_tests.rs | 54 +++++++++++++++++++++++++++++++++ 11 files changed, 129 insertions(+), 5 deletions(-) create mode 100644 tests/data/glob.yml create mode 100644 tests/data/glob_files/a.txt create mode 100644 tests/data/glob_files/b.txt create mode 100644 tests/data/glob_invalid.yml create mode 100644 tests/manifest_glob_tests.rs diff --git a/Cargo.lock b/Cargo.lock index 0ad2229a..e0e1278f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -924,6 +924,7 @@ dependencies = [ "assert_cmd", "clap", "cucumber", + "glob", "insta", "itertools 0.12.1", "itoa", diff --git a/Cargo.toml b/Cargo.toml index 7a929fbe..6ff48659 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,6 +15,7 @@ miette = { version = "7.6.0", features = ["fancy"] } sha2 = "0.10" itoa = "1" itertools = "0.12" +glob = "0.3" tracing = "0.1" tracing-subscriber = { version = "0.3", features = ["fmt"] } serde_json = "1" diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index 1328df11..8068c53f 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -736,8 +736,11 @@ providing a secure bridge to the underlying system. file path globbing. This is a critical feature for any modern build tool, allowing users to easily specify sets of source files (e.g., `src/**/*.c`). The results are returned sorted lexicographically and symlinks are followed - to keep builds deterministic. This function bridges a key feature gap, as - Ninja itself does not support globbing.[^3] + to keep builds deterministic. The implementation relies on the `glob` crate, + which follows symlinks by default. Invalid patterns or filesystem errors + surface as `InvalidOperation` to match MiniJinja's error semantics. This + function bridges a key feature gap, as Ninja itself does not support + globbing.[^3] - `python_version(requirement: &str) -> Result`: An example of a domain-specific helper function that demonstrates the extensibility of this diff --git a/docs/roadmap.md b/docs/roadmap.md index 48bb5020..421f99dc 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -104,8 +104,8 @@ configurations with variables, control flow, and custom functions. - [x] Implement the essential custom Jinja function env(var_name) to read system environment variables. - - [ ] Implement the critical glob(pattern) custom function to perform file - path globbing, with results sorted lexicographically. + - [x] Implement the critical glob(pattern) custom function to perform file + path globbing, with results sorted lexicographically. - [ ] Support user-defined Jinja macros declared in a top-level macros list, registering them with the environment before rendering. diff --git a/src/manifest.rs b/src/manifest.rs index 3a415812..852e6f54 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -141,6 +141,46 @@ fn env_var(name: &str) -> std::result::Result { } } +/// Expand a glob pattern into a sorted list of matching paths. +/// +/// Results are returned lexicographically to keep builds deterministic. +/// Invalid patterns or filesystem errors surface as Jinja evaluation errors so +/// manifests fail fast when input is incorrect. +/// +/// # Errors +/// +/// Returns an error if the glob pattern is invalid or a directory cannot be +/// read. +fn glob_paths(pattern: &str) -> std::result::Result, Error> { + use glob::{MatchOptions, glob_with}; + + let opts = MatchOptions { + case_sensitive: true, + require_literal_separator: false, + require_literal_leading_dot: false, + }; + let entries = glob_with(pattern, opts).map_err(|e| { + Error::new( + ErrorKind::InvalidOperation, + format!("invalid glob pattern '{pattern}': {e}"), + ) + })?; + let mut paths = Vec::new(); + for entry in entries { + match entry { + Ok(path) => paths.push(path.to_string_lossy().into_owned()), + Err(e) => { + return Err(Error::new( + ErrorKind::InvalidOperation, + format!("glob failed for '{pattern}': {e}"), + )); + } + } + } + paths.sort(); + Ok(paths) +} + /// Parse a manifest string using Jinja for value templating. /// /// The input YAML must be valid on its own. Jinja expressions are evaluated @@ -155,8 +195,9 @@ fn from_str_named(yaml: &str, name: &str) -> Result { let mut jinja = Environment::new(); jinja.set_undefined_behavior(UndefinedBehavior::Strict); - // Expose a strict environment variable accessor to templates. + // Expose custom helpers to templates. jinja.add_function("env", |name: String| env_var(&name)); + jinja.add_function("glob", |pattern: String| glob_paths(&pattern)); if let Some(vars) = doc.get("vars").and_then(|v| v.as_mapping()).cloned() { for (k, v) in vars { diff --git a/tests/data/glob.yml b/tests/data/glob.yml new file mode 100644 index 00000000..f1b4b1ed --- /dev/null +++ b/tests/data/glob.yml @@ -0,0 +1,5 @@ +netsuke_version: 1.0.0 +targets: + - foreach: glob('tests/data/glob_files/*.txt') + name: "{{ item | replace('tests/data/glob_files/', '') | replace('.txt', '.out') }}" + command: "cat {{ item }}" diff --git a/tests/data/glob_files/a.txt b/tests/data/glob_files/a.txt new file mode 100644 index 00000000..2e65efe2 --- /dev/null +++ b/tests/data/glob_files/a.txt @@ -0,0 +1 @@ +a \ No newline at end of file diff --git a/tests/data/glob_files/b.txt b/tests/data/glob_files/b.txt new file mode 100644 index 00000000..63d8dbd4 --- /dev/null +++ b/tests/data/glob_files/b.txt @@ -0,0 +1 @@ +b \ No newline at end of file diff --git a/tests/data/glob_invalid.yml b/tests/data/glob_invalid.yml new file mode 100644 index 00000000..539d2f7b --- /dev/null +++ b/tests/data/glob_invalid.yml @@ -0,0 +1,5 @@ +netsuke_version: 1.0.0 +targets: + - foreach: glob('[') + name: broken + command: "echo hi" diff --git a/tests/features/manifest.feature b/tests/features/manifest.feature index b3f8a595..1efc3739 100644 --- a/tests/features/manifest.feature +++ b/tests/features/manifest.feature @@ -91,6 +91,18 @@ Feature: Manifest Parsing And the target 2 command is "echo 'bar'" And the target 2 index is 1 + Scenario: Generating targets with glob + Given the manifest file "tests/data/glob.yml" is parsed + When the manifest is checked + Then the manifest has 2 targets + And the target 1 name is "a.out" + And the target 2 name is "b.out" + + Scenario: Parsing fails for an invalid glob pattern + Given the manifest file "tests/data/glob_invalid.yml" is parsed + When the parsing result is checked + Then parsing the manifest fails + Scenario: Parsing fails when a foreach expression is not iterable Given the manifest file "tests/data/foreach_invalid.yml" is parsed When the parsing result is checked diff --git a/tests/manifest_glob_tests.rs b/tests/manifest_glob_tests.rs new file mode 100644 index 00000000..878fd51b --- /dev/null +++ b/tests/manifest_glob_tests.rs @@ -0,0 +1,54 @@ +//! Tests for file globbing via the `glob()` Jinja helper. + +use netsuke::{ast::StringOrList, manifest}; +use rstest::rstest; +use std::fs; + +fn manifest_yaml(body: &str) -> String { + format!("netsuke_version: 1.0.0\n{body}") +} + +#[rstest] +fn glob_expands_sorted_matches() { + let dir = tempfile::tempdir().expect("temp dir"); + let b = dir.path().join("b.txt"); + let a = dir.path().join("a.txt"); + fs::write(&b, "b").expect("write b"); + fs::write(&a, "a").expect("write a"); + let dir_str = dir.path().display().to_string(); + let pattern = format!("{dir_str}/*.txt"); + let yaml = manifest_yaml(&format!( + concat!( + "targets:\n", + " - foreach: glob('{pattern}')\n", + " name: \"{{{{ item | replace('{dir}/', '') | replace('.txt', '.out') }}}}\"\n", + " command: echo hi\n", + ), + pattern = pattern, + dir = dir_str + )); + let manifest = manifest::from_str(&yaml).expect("parse"); + let names: Vec<_> = manifest + .targets + .iter() + .map(|t| match &t.name { + StringOrList::String(s) => s.clone(), + other => panic!("expected String, got {other:?}"), + }) + .collect(); + assert_eq!(names, vec!["a.out", "b.out"]); +} + +#[rstest] +fn glob_invalid_pattern_errors() { + let yaml = + manifest_yaml("targets:\n - foreach: glob('[')\n name: bad\n command: echo hi\n"); + let err = manifest::from_str(&yaml).expect_err("invalid pattern should error"); + assert!( + err.chain().any(|e| e + .to_string() + .to_lowercase() + .contains("invalid glob pattern")), + "unexpected error: {err}" + ); +} From 424a4c226b35602f929d1aa90bbe7c6da0abaf72 Mon Sep 17 00:00:00 2001 From: Leynos Date: Sun, 24 Aug 2025 20:05:38 +0100 Subject: [PATCH 02/11] Refine glob helper and tests --- Cargo.lock | 4 +- Cargo.toml | 2 +- docs/netsuke-design.md | 14 +++--- src/manifest.rs | 10 ++++- tests/data/glob_files/a.txt | 2 +- tests/data/glob_files/b.txt | 2 +- tests/features/manifest.feature | 1 + tests/manifest_glob_tests.rs | 80 ++++++++++++++++++++++++++++++--- tests/steps/manifest_steps.rs | 12 ++++- 9 files changed, 108 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e0e1278f..63a37022 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -583,9 +583,9 @@ checksum = "07e28edb80900c19c28f1072f2e8aeca7fa06b23cd4169cefe1af5aa3260783f" [[package]] name = "glob" -version = "0.3.2" +version = "0.3.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a8d1add55171497b4705a648c6b583acafb01d58050a51727785f0b2c8e0a2b2" +checksum = "0cc23270f6e1808e30a928bdc84dea0b9b4136a8bc82338574f23baf47bbd280" [[package]] name = "globset" diff --git a/Cargo.toml b/Cargo.toml index 6ff48659..018a04ae 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,7 +15,7 @@ miette = { version = "7.6.0", features = ["fancy"] } sha2 = "0.10" itoa = "1" itertools = "0.12" -glob = "0.3" +glob = "0.3.3" tracing = "0.1" tracing-subscriber = { version = "0.3", features = ["fmt"] } serde_json = "1" diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index 8068c53f..437aa3b8 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -735,12 +735,14 @@ providing a secure bridge to the underlying system. - `glob(pattern: &str) -> Result, Error>`: A function that performs file path globbing. This is a critical feature for any modern build tool, allowing users to easily specify sets of source files (e.g., `src/**/*.c`). - The results are returned sorted lexicographically and symlinks are followed - to keep builds deterministic. The implementation relies on the `glob` crate, - which follows symlinks by default. Invalid patterns or filesystem errors - surface as `InvalidOperation` to match MiniJinja's error semantics. This - function bridges a key feature gap, as Ninja itself does not support - globbing.[^3] + Patterns are case-sensitive, match dotfiles, and do not cross path separators + unless the pattern explicitly includes them. Matches are returned sorted + lexicographically and symlinks are followed to keep builds deterministic. + Empty results are represented as an empty list. The implementation relies on + the `glob` crate, which follows symlinks by default. Invalid patterns or + filesystem errors surface as `InvalidOperation` to match MiniJinja's error + semantics. This function bridges a key feature gap, as Ninja itself does not + support globbing.[^3] - `python_version(requirement: &str) -> Result`: An example of a domain-specific helper function that demonstrates the extensibility of this diff --git a/src/manifest.rs b/src/manifest.rs index 852e6f54..0a11ed5a 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -150,13 +150,19 @@ fn env_var(name: &str) -> std::result::Result { /// # Errors /// /// Returns an error if the glob pattern is invalid or a directory cannot be -/// read. +/// read. Patterns are evaluated in a case-sensitive manner, do not cross path +/// separators, and match dotfiles. Results are returned sorted for +/// determinism. fn glob_paths(pattern: &str) -> std::result::Result, Error> { use glob::{MatchOptions, glob_with}; + // Enforce shell-like semantics: + // - patterns are case-sensitive, + // - wildcards do not cross path separators, + // - dotfiles are matched by default. let opts = MatchOptions { case_sensitive: true, - require_literal_separator: false, + require_literal_separator: true, require_literal_leading_dot: false, }; let entries = glob_with(pattern, opts).map_err(|e| { diff --git a/tests/data/glob_files/a.txt b/tests/data/glob_files/a.txt index 2e65efe2..78981922 100644 --- a/tests/data/glob_files/a.txt +++ b/tests/data/glob_files/a.txt @@ -1 +1 @@ -a \ No newline at end of file +a diff --git a/tests/data/glob_files/b.txt b/tests/data/glob_files/b.txt index 63d8dbd4..61780798 100644 --- a/tests/data/glob_files/b.txt +++ b/tests/data/glob_files/b.txt @@ -1 +1 @@ -b \ No newline at end of file +b diff --git a/tests/features/manifest.feature b/tests/features/manifest.feature index 1efc3739..3ef18667 100644 --- a/tests/features/manifest.feature +++ b/tests/features/manifest.feature @@ -102,6 +102,7 @@ Feature: Manifest Parsing Given the manifest file "tests/data/glob_invalid.yml" is parsed When the parsing result is checked Then parsing the manifest fails + And the error message contains "glob pattern" Scenario: Parsing fails when a foreach expression is not iterable Given the manifest file "tests/data/foreach_invalid.yml" is parsed diff --git a/tests/manifest_glob_tests.rs b/tests/manifest_glob_tests.rs index 878fd51b..a7a1cf59 100644 --- a/tests/manifest_glob_tests.rs +++ b/tests/manifest_glob_tests.rs @@ -44,11 +44,81 @@ fn glob_invalid_pattern_errors() { let yaml = manifest_yaml("targets:\n - foreach: glob('[')\n name: bad\n command: echo hi\n"); let err = manifest::from_str(&yaml).expect_err("invalid pattern should error"); + let msg = format!("{err:?}").to_lowercase(); + assert!(msg.contains("invalid glob pattern"), "{msg}"); +} + +#[rstest] +fn glob_returns_empty_when_no_matches() { + let dir = tempfile::tempdir().expect("temp dir"); + let pattern = format!("{}/*.nomatch", dir.path().display()); + let yaml = manifest_yaml(&format!( + concat!( + "targets:\n", + " - foreach: glob('{pattern}')\n", + " name: none\n", + " command: echo hi\n", + ), + pattern = pattern, + )); + let manifest = manifest::from_str(&yaml).expect("parse"); assert!( - err.chain().any(|e| e - .to_string() - .to_lowercase() - .contains("invalid glob pattern")), - "unexpected error: {err}" + manifest.targets.is_empty(), + "glob with no matches should yield no targets" ); } + +#[rstest] +fn glob_does_not_cross_separator() { + let dir = tempfile::tempdir().expect("temp dir"); + std::fs::create_dir(dir.path().join("sub")).expect("create subdir"); + std::fs::write(dir.path().join("sub").join("x.txt"), "x").expect("write file"); + let pattern = format!("{}/*.txt", dir.path().display()); + let yaml = manifest_yaml(&format!( + concat!( + "targets:\n", + " - foreach: glob('{pattern}')\n", + " name: bad\n", + " command: echo hi\n", + ), + pattern = pattern, + )); + let manifest = manifest::from_str(&yaml).expect("parse"); + assert!(manifest.targets.is_empty(), "wildcards must not cross '/'"); +} + +#[rstest] +fn glob_matches_dotfiles_with_wildcards() { + let dir = tempfile::tempdir().expect("temp dir"); + std::fs::write(dir.path().join(".hidden.txt"), "h").expect("write file"); + let pattern = format!("{}/*.txt", dir.path().display()); + let yaml = manifest_yaml(&format!( + concat!( + "targets:\n", + " - foreach: glob('{pattern}')\n", + " name: ok\n", + " command: echo hi\n", + ), + pattern = pattern, + )); + let manifest = manifest::from_str(&yaml).expect("parse"); + assert_eq!(manifest.targets.len(), 1, "dotfiles should match"); +} + +#[rstest] +fn glob_is_case_sensitive() { + let dir = tempfile::tempdir().expect("temp dir"); + std::fs::write(dir.path().join("UPPER.TXT"), "x").expect("write file"); + let pattern = format!("{}/*.txt", dir.path().display()); + let yaml = manifest_yaml(&format!( + concat!( + "targets:\n", + " - foreach: glob('{pattern}')\n", + " name: fail\n", + " command: echo hi\n", + ), + pattern = pattern, + )); + let manifest = manifest::from_str(&yaml).expect("parse"); + assert!(manifest.targets.is_empty(), "glob should be case-sensitive"); +} diff --git a/tests/steps/manifest_steps.rs b/tests/steps/manifest_steps.rs index 31284b8d..db92b3f0 100644 --- a/tests/steps/manifest_steps.rs +++ b/tests/steps/manifest_steps.rs @@ -27,7 +27,7 @@ fn parse_manifest_inner(world: &mut CliWorld, path: &str) { } Err(e) => { world.manifest = None; - world.manifest_error = Some(e.to_string()); + world.manifest_error = Some(format!("{e:?}")); } } } @@ -160,6 +160,16 @@ fn manifest_parse_error(world: &mut CliWorld) { assert!(world.manifest_error.is_some(), "expected parse error"); } +#[then(expr = "the error message contains {string}")] +#[expect( + clippy::needless_pass_by_value, + reason = "Cucumber step requires owned String" +)] +fn manifest_error_contains(world: &mut CliWorld, text: String) { + let msg = world.manifest_error.as_ref().expect("expected parse error"); + assert!(msg.contains(&text), "{msg}"); +} + #[then(expr = "the first rule name is {string}")] #[expect( clippy::needless_pass_by_value, From 8eba940ae819f4c5db2c170b96f9b64438145939 Mon Sep 17 00:00:00 2001 From: Leynos Date: Mon, 25 Aug 2025 07:28:03 +0100 Subject: [PATCH 03/11] Test Windows-style globbing --- docs/netsuke-design.md | 13 +++++--- src/manifest.rs | 21 ++++++++++-- src/manifest/tests.rs | 6 ++++ tests/data/glob_windows.yml | 5 +++ tests/features/manifest.feature | 7 ++++ tests/manifest_glob_tests.rs | 57 +++++++++++++++++++++++++++++++++ 6 files changed, 101 insertions(+), 8 deletions(-) create mode 100644 tests/data/glob_windows.yml diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index 437aa3b8..d35dadf3 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -736,11 +736,14 @@ providing a secure bridge to the underlying system. file path globbing. This is a critical feature for any modern build tool, allowing users to easily specify sets of source files (e.g., `src/**/*.c`). Patterns are case-sensitive, match dotfiles, and do not cross path separators - unless the pattern explicitly includes them. Matches are returned sorted - lexicographically and symlinks are followed to keep builds deterministic. - Empty results are represented as an empty list. The implementation relies on - the `glob` crate, which follows symlinks by default. Invalid patterns or - filesystem errors surface as `InvalidOperation` to match MiniJinja's error + unless the pattern explicitly includes them. Callers may use either `/` or + `\` in patterns; these are normalised to the host platform before matching so + manifests behave consistently across operating systems. Results contain only + files (directories are ignored), are sorted lexicographically, and have path + separators normalised to `/` to keep builds deterministic. Empty results are + represented as an empty list. The implementation relies on the `glob` crate, + which follows symlinks by default. Invalid patterns surface as `SyntaxError`, + while filesystem errors use `InvalidOperation`, matching MiniJinja's error semantics. This function bridges a key feature gap, as Ninja itself does not support globbing.[^3] diff --git a/src/manifest.rs b/src/manifest.rs index 0a11ed5a..3dfcb4e5 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -165,16 +165,31 @@ fn glob_paths(pattern: &str) -> std::result::Result, Error> { require_literal_separator: true, require_literal_leading_dot: false, }; - let entries = glob_with(pattern, opts).map_err(|e| { + + // Normalise path separators so that callers may use either `/` or `\` + // regardless of platform. The `glob` crate expects the native separator. + let native_sep = std::path::MAIN_SEPARATOR.to_string(); + let normalized = pattern.replace('\\', "/").replace('/', &native_sep); + + let entries = glob_with(&normalized, opts).map_err(|e| { Error::new( - ErrorKind::InvalidOperation, + ErrorKind::SyntaxError, format!("invalid glob pattern '{pattern}': {e}"), ) })?; let mut paths = Vec::new(); for entry in entries { match entry { - Ok(path) => paths.push(path.to_string_lossy().into_owned()), + Ok(path) => match path.metadata() { + Ok(meta) if meta.is_file() => paths.push(path.to_string_lossy().replace('\\', "/")), + Ok(_) => {} + Err(e) => { + return Err(Error::new( + ErrorKind::InvalidOperation, + format!("glob failed for '{pattern}': {e}"), + )); + } + }, Err(e) => { return Err(Error::new( ErrorKind::InvalidOperation, diff --git a/src/manifest/tests.rs b/src/manifest/tests.rs index 54c15861..6dad47f2 100644 --- a/src/manifest/tests.rs +++ b/src/manifest/tests.rs @@ -18,3 +18,9 @@ fn yaml_hint_needles_are_lowercase() { ); } } + +#[test] +fn glob_paths_invalid_pattern_sets_syntax_error() { + let err = super::glob_paths("[").expect_err("expected pattern error"); + assert_eq!(err.kind(), minijinja::ErrorKind::SyntaxError); +} diff --git a/tests/data/glob_windows.yml b/tests/data/glob_windows.yml new file mode 100644 index 00000000..3dd3d339 --- /dev/null +++ b/tests/data/glob_windows.yml @@ -0,0 +1,5 @@ +netsuke_version: 1.0.0 +targets: + - foreach: glob('tests\\data\\glob_files\\*.txt') + name: "{{ item | replace('tests/data/glob_files/', '') | replace('.txt', '.out') }}" + command: "cat {{ item }}" diff --git a/tests/features/manifest.feature b/tests/features/manifest.feature index 3ef18667..930ed45a 100644 --- a/tests/features/manifest.feature +++ b/tests/features/manifest.feature @@ -98,6 +98,13 @@ Feature: Manifest Parsing And the target 1 name is "a.out" And the target 2 name is "b.out" + Scenario: Generating targets with glob using Windows separators + Given the manifest file "tests/data/glob_windows.yml" is parsed + When the manifest is checked + Then the manifest has 2 targets + And the target 1 name is "a.out" + And the target 2 name is "b.out" + Scenario: Parsing fails for an invalid glob pattern Given the manifest file "tests/data/glob_invalid.yml" is parsed When the parsing result is checked diff --git a/tests/manifest_glob_tests.rs b/tests/manifest_glob_tests.rs index a7a1cf59..43fc4df9 100644 --- a/tests/manifest_glob_tests.rs +++ b/tests/manifest_glob_tests.rs @@ -122,3 +122,60 @@ fn glob_is_case_sensitive() { let manifest = manifest::from_str(&yaml).expect("parse"); assert!(manifest.targets.is_empty(), "glob should be case-sensitive"); } + +#[rstest] +fn glob_accepts_windows_path_separators() { + let dir = tempfile::tempdir().expect("temp dir"); + fs::write(dir.path().join("a.txt"), "a").expect("write a"); + fs::write(dir.path().join("b.txt"), "b").expect("write b"); + let dir_win = dir.path().display().to_string().replace('/', "\\\\"); + let pattern = format!("{dir_win}\\\\*.txt"); + let yaml = manifest_yaml(&format!( + concat!( + "targets:\n", + " - foreach: glob('{pattern}')\n", + " name: \"{{{{ item | replace('{dir}/', '') | replace('.txt', '.out') }}}}\"\n", + " command: echo hi\n", + ), + pattern = pattern, + dir = dir.path().display() + )); + let manifest = manifest::from_str(&yaml).expect("parse"); + let names: Vec<_> = manifest + .targets + .iter() + .map(|t| match &t.name { + StringOrList::String(s) => s.clone(), + other => panic!("expected String, got {other:?}"), + }) + .collect(); + assert_eq!(names, vec!["a.out", "b.out"]); +} + +#[rstest] +fn glob_filters_directories() { + let dir = tempfile::tempdir().expect("temp dir"); + fs::write(dir.path().join("a.txt"), "a").expect("write a"); + fs::create_dir(dir.path().join("sub")).expect("create subdir"); + let pattern = format!("{}/*", dir.path().display()); + let yaml = manifest_yaml(&format!( + concat!( + "targets:\n", + " - foreach: glob('{pattern}')\n", + " name: \"{{{{ item | replace('{dir}/', '') }}}}\"\n", + " command: echo hi\n", + ), + pattern = pattern, + dir = dir.path().display() + )); + let manifest = manifest::from_str(&yaml).expect("parse"); + let names: Vec<_> = manifest + .targets + .iter() + .map(|t| match &t.name { + StringOrList::String(s) => s.clone(), + other => panic!("expected String, got {other:?}"), + }) + .collect(); + assert_eq!(names, vec!["a.txt"]); +} From 61166c3122cee9afa54c57e0e7012da9e02033d5 Mon Sep 17 00:00:00 2001 From: Leynos Date: Mon, 25 Aug 2025 18:56:40 +0100 Subject: [PATCH 04/11] Refactor glob tests for portability --- tests/manifest_glob_tests.rs | 73 ++++++++++++++++++++---------------- 1 file changed, 40 insertions(+), 33 deletions(-) diff --git a/tests/manifest_glob_tests.rs b/tests/manifest_glob_tests.rs index 43fc4df9..bfd3fa7b 100644 --- a/tests/manifest_glob_tests.rs +++ b/tests/manifest_glob_tests.rs @@ -1,6 +1,9 @@ //! Tests for file globbing via the `glob()` Jinja helper. -use netsuke::{ast::StringOrList, manifest}; +use netsuke::{ + ast::{NetsukeManifest, StringOrList}, + manifest, +}; use rstest::rstest; use std::fs; @@ -8,6 +11,17 @@ fn manifest_yaml(body: &str) -> String { format!("netsuke_version: 1.0.0\n{body}") } +fn target_names(manifest: &NetsukeManifest) -> Vec { + manifest + .targets + .iter() + .map(|t| match &t.name { + StringOrList::String(s) => s.clone(), + other => panic!("expected String, got {other:?}"), + }) + .collect() +} + #[rstest] fn glob_expands_sorted_matches() { let dir = tempfile::tempdir().expect("temp dir"); @@ -28,15 +42,7 @@ fn glob_expands_sorted_matches() { dir = dir_str )); let manifest = manifest::from_str(&yaml).expect("parse"); - let names: Vec<_> = manifest - .targets - .iter() - .map(|t| match &t.name { - StringOrList::String(s) => s.clone(), - other => panic!("expected String, got {other:?}"), - }) - .collect(); - assert_eq!(names, vec!["a.out", "b.out"]); + assert_eq!(target_names(&manifest), vec!["a.out", "b.out"]); } #[rstest] @@ -71,8 +77,8 @@ fn glob_returns_empty_when_no_matches() { #[rstest] fn glob_does_not_cross_separator() { let dir = tempfile::tempdir().expect("temp dir"); - std::fs::create_dir(dir.path().join("sub")).expect("create subdir"); - std::fs::write(dir.path().join("sub").join("x.txt"), "x").expect("write file"); + fs::create_dir(dir.path().join("sub")).expect("create subdir"); + fs::write(dir.path().join("sub").join("x.txt"), "x").expect("write file"); let pattern = format!("{}/*.txt", dir.path().display()); let yaml = manifest_yaml(&format!( concat!( @@ -90,7 +96,7 @@ fn glob_does_not_cross_separator() { #[rstest] fn glob_matches_dotfiles_with_wildcards() { let dir = tempfile::tempdir().expect("temp dir"); - std::fs::write(dir.path().join(".hidden.txt"), "h").expect("write file"); + fs::write(dir.path().join(".hidden.txt"), "h").expect("write file"); let pattern = format!("{}/*.txt", dir.path().display()); let yaml = manifest_yaml(&format!( concat!( @@ -108,7 +114,7 @@ fn glob_matches_dotfiles_with_wildcards() { #[rstest] fn glob_is_case_sensitive() { let dir = tempfile::tempdir().expect("temp dir"); - std::fs::write(dir.path().join("UPPER.TXT"), "x").expect("write file"); + fs::write(dir.path().join("UPPER.TXT"), "x").expect("write file"); let pattern = format!("{}/*.txt", dir.path().display()); let yaml = manifest_yaml(&format!( concat!( @@ -128,25 +134,27 @@ fn glob_accepts_windows_path_separators() { let dir = tempfile::tempdir().expect("temp dir"); fs::write(dir.path().join("a.txt"), "a").expect("write a"); fs::write(dir.path().join("b.txt"), "b").expect("write b"); - let dir_win = dir.path().display().to_string().replace('/', "\\\\"); + let dir_fwd = dir.path().display().to_string(); + let dir_win = dir_fwd.replace('/', "\\\\"); let pattern = format!("{dir_win}\\\\*.txt"); let yaml = manifest_yaml(&format!( concat!( "targets:\n", " - foreach: glob('{pattern}')\n", - " name: \"{{{{ item | replace('{dir}/', '') | replace('.txt', '.out') }}}}\"\n", + " name: \"{{{{ item }}}}\"\n", " command: echo hi\n", ), pattern = pattern, - dir = dir.path().display() )); let manifest = manifest::from_str(&yaml).expect("parse"); - let names: Vec<_> = manifest - .targets - .iter() - .map(|t| match &t.name { - StringOrList::String(s) => s.clone(), - other => panic!("expected String, got {other:?}"), + let prefix_fwd = format!("{dir_fwd}/"); + let prefix_back = format!("{dir_win}\\"); + let names: Vec<_> = target_names(&manifest) + .into_iter() + .map(|n| { + n.replace(&prefix_fwd, "") + .replace(&prefix_back, "") + .replace(".txt", ".out") }) .collect(); assert_eq!(names, vec!["a.out", "b.out"]); @@ -157,25 +165,24 @@ fn glob_filters_directories() { let dir = tempfile::tempdir().expect("temp dir"); fs::write(dir.path().join("a.txt"), "a").expect("write a"); fs::create_dir(dir.path().join("sub")).expect("create subdir"); - let pattern = format!("{}/*", dir.path().display()); + let dir_fwd = dir.path().display().to_string(); + let dir_win = dir_fwd.replace('/', "\\\\"); + let pattern = format!("{dir_fwd}/*"); let yaml = manifest_yaml(&format!( concat!( "targets:\n", " - foreach: glob('{pattern}')\n", - " name: \"{{{{ item | replace('{dir}/', '') }}}}\"\n", + " name: \"{{{{ item }}}}\"\n", " command: echo hi\n", ), pattern = pattern, - dir = dir.path().display() )); let manifest = manifest::from_str(&yaml).expect("parse"); - let names: Vec<_> = manifest - .targets - .iter() - .map(|t| match &t.name { - StringOrList::String(s) => s.clone(), - other => panic!("expected String, got {other:?}"), - }) + let prefix_fwd = format!("{dir_fwd}/"); + let prefix_back = format!("{dir_win}\\"); + let names: Vec<_> = target_names(&manifest) + .into_iter() + .map(|n| n.replace(&prefix_fwd, "").replace(&prefix_back, "")) .collect(); assert_eq!(names, vec!["a.txt"]); } From f84615298a1536e6dfa12cdd315ae08e89f7f01a Mon Sep 17 00:00:00 2001 From: Leynos Date: Mon, 25 Aug 2025 22:48:03 +0100 Subject: [PATCH 05/11] Refactor glob tests and document behaviour --- docs/netsuke-design.md | 31 ++--- src/manifest.rs | 17 ++- tests/manifest_glob_tests.rs | 234 +++++++++++++++++------------------ 3 files changed, 143 insertions(+), 139 deletions(-) diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index d35dadf3..1f71e31f 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -732,21 +732,22 @@ providing a secure bridge to the underlying system. error if the variable is undefined or contains invalid UTF-8 to ensure manifests fail fast on missing inputs. -- `glob(pattern: &str) -> Result, Error>`: A function that performs - file path globbing. This is a critical feature for any modern build tool, - allowing users to easily specify sets of source files (e.g., `src/**/*.c`). - Patterns are case-sensitive, match dotfiles, and do not cross path separators - unless the pattern explicitly includes them. Callers may use either `/` or - `\` in patterns; these are normalised to the host platform before matching so - manifests behave consistently across operating systems. Results contain only - files (directories are ignored), are sorted lexicographically, and have path - separators normalised to `/` to keep builds deterministic. Empty results are - represented as an empty list. The implementation relies on the `glob` crate, - which follows symlinks by default. Invalid patterns surface as `SyntaxError`, - while filesystem errors use `InvalidOperation`, matching MiniJinja's error - semantics. This function bridges a key feature gap, as Ninja itself does not - support globbing.[^3] - +- `glob(pattern: &str) -> Result, Error>`: A function that + performs file path globbing. This is a critical feature for any modern build + tool, allowing users to easily specify sets of source files (e.g., + `src/**/*.c`). Results are yielded in lexicographic order by the iterator and + Netsuke returns them as such. Symlinks are followed by the `glob` crate by + default. Matching is case-sensitive on all platforms for determinism. Note + that `glob_with` enforces `require_literal_separator = true` internally (even + if disabled in options), and wildcards do not cross path separators unless + `**` is used. Callers may use either `/` or `\` in patterns; these are + normalised to the host platform before matching so manifests behave + consistently across operating systems. Results contain only files + (directories are ignored) and have path separators normalised to `/`. Empty + results are represented as an empty list. Invalid patterns or filesystem + errors surface as `InvalidOperation` to match minijinja's error semantics. + This function bridges a key feature gap, as Ninja itself does not support + globbing.[^3] - `python_version(requirement: &str) -> Result`: An example of a domain-specific helper function that demonstrates the extensibility of this architecture. This function would execute `python --version` or diff --git a/src/manifest.rs b/src/manifest.rs index 3dfcb4e5..cbde2b7a 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -150,9 +150,9 @@ fn env_var(name: &str) -> std::result::Result { /// # Errors /// /// Returns an error if the glob pattern is invalid or a directory cannot be -/// read. Patterns are evaluated in a case-sensitive manner, do not cross path -/// separators, and match dotfiles. Results are returned sorted for -/// determinism. +/// read. Patterns are case-sensitive, do not cross path separators, and match +/// dotfiles. Results are yielded in lexicographic order by the iterator, so +/// Netsuke returns them without further sorting. fn glob_paths(pattern: &str) -> std::result::Result, Error> { use glob::{MatchOptions, glob_with}; @@ -181,7 +181,15 @@ fn glob_paths(pattern: &str) -> std::result::Result, Error> { for entry in entries { match entry { Ok(path) => match path.metadata() { - Ok(meta) if meta.is_file() => paths.push(path.to_string_lossy().replace('\\', "/")), + Ok(meta) if meta.is_file() => { + let s = path.to_str().ok_or_else(|| { + Error::new( + ErrorKind::InvalidOperation, + format!("glob matched a non-UTF-8 path: {}", path.display()), + ) + })?; + paths.push(s.replace('\\', "/")); + } Ok(_) => {} Err(e) => { return Err(Error::new( @@ -198,7 +206,6 @@ fn glob_paths(pattern: &str) -> std::result::Result, Error> { } } } - paths.sort(); Ok(paths) } diff --git a/tests/manifest_glob_tests.rs b/tests/manifest_glob_tests.rs index bfd3fa7b..845f6237 100644 --- a/tests/manifest_glob_tests.rs +++ b/tests/manifest_glob_tests.rs @@ -4,8 +4,8 @@ use netsuke::{ ast::{NetsukeManifest, StringOrList}, manifest, }; -use rstest::rstest; -use std::fs; +use rstest::{fixture, rstest}; +use std::{fs, path::Path}; fn manifest_yaml(body: &str) -> String { format!("netsuke_version: 1.0.0\n{body}") @@ -22,119 +22,142 @@ fn target_names(manifest: &NetsukeManifest) -> Vec { .collect() } -#[rstest] -fn glob_expands_sorted_matches() { - let dir = tempfile::tempdir().expect("temp dir"); - let b = dir.path().join("b.txt"); - let a = dir.path().join("a.txt"); - fs::write(&b, "b").expect("write b"); - fs::write(&a, "a").expect("write a"); - let dir_str = dir.path().display().to_string(); - let pattern = format!("{dir_str}/*.txt"); - let yaml = manifest_yaml(&format!( - concat!( - "targets:\n", - " - foreach: glob('{pattern}')\n", - " name: \"{{{{ item | replace('{dir}/', '') | replace('.txt', '.out') }}}}\"\n", - " command: echo hi\n", - ), - pattern = pattern, - dir = dir_str - )); - let manifest = manifest::from_str(&yaml).expect("parse"); - assert_eq!(target_names(&manifest), vec!["a.out", "b.out"]); +fn create_test_files(base: &Path, files: &[(&str, &str)]) { + for (rel, content) in files { + let path = base.join(rel); + if let Some(parent) = path.parent() { + fs::create_dir_all(parent).expect("create parent dirs"); + } + fs::write(path, content).expect("write file"); + } } -#[rstest] -fn glob_invalid_pattern_errors() { - let yaml = - manifest_yaml("targets:\n - foreach: glob('[')\n name: bad\n command: echo hi\n"); - let err = manifest::from_str(&yaml).expect_err("invalid pattern should error"); - let msg = format!("{err:?}").to_lowercase(); - assert!(msg.contains("invalid glob pattern"), "{msg}"); +fn create_test_dirs(base: &Path, dirs: &[&str]) { + for d in dirs { + fs::create_dir_all(base.join(d)).expect("create dir"); + } } -#[rstest] -fn glob_returns_empty_when_no_matches() { - let dir = tempfile::tempdir().expect("temp dir"); - let pattern = format!("{}/*.nomatch", dir.path().display()); - let yaml = manifest_yaml(&format!( - concat!( - "targets:\n", - " - foreach: glob('{pattern}')\n", - " name: none\n", - " command: echo hi\n", - ), - pattern = pattern, - )); - let manifest = manifest::from_str(&yaml).expect("parse"); - assert!( - manifest.targets.is_empty(), - "glob with no matches should yield no targets" - ); +#[fixture] +fn temp_dir() -> tempfile::TempDir { + tempfile::tempdir().expect("temp dir") } #[rstest] -fn glob_does_not_cross_separator() { - let dir = tempfile::tempdir().expect("temp dir"); - fs::create_dir(dir.path().join("sub")).expect("create subdir"); - fs::write(dir.path().join("sub").join("x.txt"), "x").expect("write file"); - let pattern = format!("{}/*.txt", dir.path().display()); - let yaml = manifest_yaml(&format!( - concat!( - "targets:\n", - " - foreach: glob('{pattern}')\n", - " name: bad\n", - " command: echo hi\n", - ), - pattern = pattern, - )); - let manifest = manifest::from_str(&yaml).expect("parse"); - assert!(manifest.targets.is_empty(), "wildcards must not cross '/'"); -} +#[case( + &[("b.txt", "b"), ("a.txt", "a")], + &[], + "*.txt", + "{{ item | replace('{dir}/', '') | replace('.txt', '.out') }}", + &["a.txt", "b.txt"], + "expands and sorts matches", +)] +#[case( + &[], + &[], + "*.nomatch", + "none", + &[], + "no targets when pattern has no matches", +)] +#[case( + &[("sub/x.txt", "x")], + &[], + "*.txt", + "bad", + &[], + "wildcards must not cross '/'", +)] +#[case( + &[(".hidden.txt", "h")], + &[], + "*.txt", + "ok", + &[".hidden.txt"], + "dotfiles should match", +)] +#[case( + &[("UPPER.TXT", "x")], + &[], + "*.txt", + "fail", + &[], + "glob should be case-sensitive", +)] +#[case( + &[("a.txt", "a")], + &["sub"], + "*", + "{{ item }}", + &["a.txt"], + "should filter directories", +)] +fn test_glob_behavior( + temp_dir: tempfile::TempDir, + #[case] files: &[(&str, &str)], + #[case] dirs: &[&str], + #[case] pattern_suffix: &str, + #[case] name_template: &str, + #[case] expected_partial: &[&str], + #[case] description: &str, +) { + create_test_files(temp_dir.path(), files); + create_test_dirs(temp_dir.path(), dirs); -#[rstest] -fn glob_matches_dotfiles_with_wildcards() { - let dir = tempfile::tempdir().expect("temp dir"); - fs::write(dir.path().join(".hidden.txt"), "h").expect("write file"); - let pattern = format!("{}/*.txt", dir.path().display()); + let dir_str = temp_dir.path().display().to_string(); + let pattern = format!("{dir_str}/{pattern_suffix}"); let yaml = manifest_yaml(&format!( concat!( "targets:\n", " - foreach: glob('{pattern}')\n", - " name: ok\n", + " name: \"{name_template}\"\n", " command: echo hi\n", ), pattern = pattern, + name_template = name_template.replace("{dir}", &dir_str) )); + let manifest = manifest::from_str(&yaml).expect("parse"); - assert_eq!(manifest.targets.len(), 1, "dotfiles should match"); + + if expected_partial.is_empty() { + assert!(manifest.targets.is_empty(), "{description}"); + } else { + let names = target_names(&manifest); + if expected_partial == [".hidden.txt"] { + assert_eq!(manifest.targets.len(), 1, "{description}"); + } else if name_template.contains("replace") { + let expected: Vec<_> = expected_partial + .iter() + .map(|s| s.replace(".txt", ".out")) + .collect(); + assert_eq!(names, expected, "{description}"); + } else { + let prefix_fwd = format!("{dir_str}/"); + let prefix_back = format!("{dir_str}\\"); + let expected: Vec<_> = expected_partial.iter().map(|&s| s.to_string()).collect(); + let normalised: Vec<_> = names + .into_iter() + .map(|n| n.replace(&prefix_fwd, "").replace(&prefix_back, "")) + .collect(); + assert_eq!(normalised, expected, "{description}"); + } + } } #[rstest] -fn glob_is_case_sensitive() { - let dir = tempfile::tempdir().expect("temp dir"); - fs::write(dir.path().join("UPPER.TXT"), "x").expect("write file"); - let pattern = format!("{}/*.txt", dir.path().display()); - let yaml = manifest_yaml(&format!( - concat!( - "targets:\n", - " - foreach: glob('{pattern}')\n", - " name: fail\n", - " command: echo hi\n", - ), - pattern = pattern, - )); - let manifest = manifest::from_str(&yaml).expect("parse"); - assert!(manifest.targets.is_empty(), "glob should be case-sensitive"); +fn glob_invalid_pattern_errors() { + let yaml = + manifest_yaml("targets:\n - foreach: glob('[')\n name: bad\n command: echo hi\n"); + let err = manifest::from_str(&yaml).expect_err("invalid pattern should error"); + let msg = format!("{err:?}").to_lowercase(); + assert!(msg.contains("invalid glob pattern"), "{msg}"); } #[rstest] -fn glob_accepts_windows_path_separators() { - let dir = tempfile::tempdir().expect("temp dir"); - fs::write(dir.path().join("a.txt"), "a").expect("write a"); - fs::write(dir.path().join("b.txt"), "b").expect("write b"); - let dir_fwd = dir.path().display().to_string(); +fn glob_accepts_windows_path_separators(temp_dir: tempfile::TempDir) { + fs::write(temp_dir.path().join("a.txt"), "a").expect("write a"); + fs::write(temp_dir.path().join("b.txt"), "b").expect("write b"); + let dir_fwd = temp_dir.path().display().to_string(); let dir_win = dir_fwd.replace('/', "\\\\"); let pattern = format!("{dir_win}\\\\*.txt"); let yaml = manifest_yaml(&format!( @@ -159,30 +182,3 @@ fn glob_accepts_windows_path_separators() { .collect(); assert_eq!(names, vec!["a.out", "b.out"]); } - -#[rstest] -fn glob_filters_directories() { - let dir = tempfile::tempdir().expect("temp dir"); - fs::write(dir.path().join("a.txt"), "a").expect("write a"); - fs::create_dir(dir.path().join("sub")).expect("create subdir"); - let dir_fwd = dir.path().display().to_string(); - let dir_win = dir_fwd.replace('/', "\\\\"); - let pattern = format!("{dir_fwd}/*"); - let yaml = manifest_yaml(&format!( - concat!( - "targets:\n", - " - foreach: glob('{pattern}')\n", - " name: \"{{{{ item }}}}\"\n", - " command: echo hi\n", - ), - pattern = pattern, - )); - let manifest = manifest::from_str(&yaml).expect("parse"); - let prefix_fwd = format!("{dir_fwd}/"); - let prefix_back = format!("{dir_win}\\"); - let names: Vec<_> = target_names(&manifest) - .into_iter() - .map(|n| n.replace(&prefix_fwd, "").replace(&prefix_back, "")) - .collect(); - assert_eq!(names, vec!["a.txt"]); -} From c6ca66b4d09957b5281b63edfb355028ed924ca3 Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 26 Aug 2025 00:04:29 +0100 Subject: [PATCH 06/11] Add test for glob empty matches --- tests/manifest_glob_tests.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/manifest_glob_tests.rs b/tests/manifest_glob_tests.rs index 845f6237..a68dd2c5 100644 --- a/tests/manifest_glob_tests.rs +++ b/tests/manifest_glob_tests.rs @@ -153,6 +153,26 @@ fn glob_invalid_pattern_errors() { assert!(msg.contains("invalid glob pattern"), "{msg}"); } +#[rstest] +fn glob_returns_empty_when_no_matches() { + let dir = tempfile::tempdir().expect("temp dir"); + let pattern = format!("{}/*.nomatch", dir.path().display()); + let yaml = manifest_yaml(&format!( + concat!( + "targets:\n", + " - foreach: glob('{pattern}')\n", + " name: test\n", + " command: echo hi\n", + ), + pattern = pattern, + )); + let manifest = manifest::from_str(&yaml).expect("parse manifest"); + assert!( + manifest.targets.is_empty(), + "expected no files to match the glob pattern", + ); +} + #[rstest] fn glob_accepts_windows_path_separators(temp_dir: tempfile::TempDir) { fs::write(temp_dir.path().join("a.txt"), "a").expect("write a"); From 48f047aa9f1ec5c56442f6dc78a16892ae6b63f3 Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 26 Aug 2025 08:43:27 +0100 Subject: [PATCH 07/11] Clarify glob path semantics --- src/manifest.rs | 30 +++++++++++++++++------------- tests/data/glob_invalid.yml | 2 +- tests/features/manifest.feature | 2 ++ tests/manifest_glob_tests.rs | 13 +++++++------ 4 files changed, 27 insertions(+), 20 deletions(-) diff --git a/src/manifest.rs b/src/manifest.rs index cbde2b7a..2da44d19 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -3,8 +3,9 @@ //! This module parses a `Netsukefile` without relying on a global Jinja //! preprocessing pass. The YAML is parsed first and Jinja expressions are //! evaluated only within string values or the `foreach` and `when` keys. It -//! exposes an `env()` function to surface environment variables, failing fast -//! when values are missing or invalid. +//! exposes `env()` to read environment variables and `glob()` to expand +//! filesystem patterns during template evaluation. Both helpers fail fast when +//! inputs are missing or patterns are invalid. use crate::{ ast::{NetsukeManifest, Recipe, StringOrList, Target, Vars}, @@ -119,13 +120,15 @@ fn map_yaml_error(err: YamlError, src: &str, name: &str) -> YamlDiagnostic { /// /// # Examples /// +/// The [`EnvLock`](test_support::env_lock::EnvLock) guard serialises access to +/// the process environment so tests do not interfere with each other. +/// /// ```rust,ignore -/// // SAFETY: Process environment mutation is unsafe in Rust 2024. Examples and -/// // tests serialise access with an environment lock and restore prior state to -/// // avoid races and leaks. -/// unsafe { std::env::set_var("FOO", "bar"); } +/// use test_support::env_lock::EnvLock; +/// let _guard = EnvLock::acquire(); +/// std::env::set_var("FOO", "bar"); /// assert_eq!(env("FOO").unwrap(), "bar"); -/// unsafe { std::env::remove_var("FOO"); } +/// std::env::remove_var("FOO"); /// ``` fn env_var(name: &str) -> std::result::Result { match std::env::var(name) { @@ -141,18 +144,19 @@ fn env_var(name: &str) -> std::result::Result { } } -/// Expand a glob pattern into a sorted list of matching paths. +/// Expand a glob pattern into a list of matching paths with deterministic ordering. /// -/// Results are returned lexicographically to keep builds deterministic. +/// Results are returned in lexicographic order to keep builds deterministic. /// Invalid patterns or filesystem errors surface as Jinja evaluation errors so /// manifests fail fast when input is incorrect. /// /// # Errors /// -/// Returns an error if the glob pattern is invalid or a directory cannot be -/// read. Patterns are case-sensitive, do not cross path separators, and match -/// dotfiles. Results are yielded in lexicographic order by the iterator, so -/// Netsuke returns them without further sorting. +/// Returns an error if the glob pattern is invalid, a directory cannot be +/// read, or a matched path is not valid UTF-8. +/// Matching is case-sensitive on all platforms, wildcards do not cross path +/// separators (use `**` to span directories), and leading-dot entries are +/// matched by wildcards. fn glob_paths(pattern: &str) -> std::result::Result, Error> { use glob::{MatchOptions, glob_with}; diff --git a/tests/data/glob_invalid.yml b/tests/data/glob_invalid.yml index 539d2f7b..cbe7d8a4 100644 --- a/tests/data/glob_invalid.yml +++ b/tests/data/glob_invalid.yml @@ -1,5 +1,5 @@ netsuke_version: 1.0.0 targets: - - foreach: glob('[') + - foreach: glob('[') # intentional: unmatched '[' to trigger a glob parse error name: broken command: "echo hi" diff --git a/tests/features/manifest.feature b/tests/features/manifest.feature index 930ed45a..f654aaf3 100644 --- a/tests/features/manifest.feature +++ b/tests/features/manifest.feature @@ -96,7 +96,9 @@ Feature: Manifest Parsing When the manifest is checked Then the manifest has 2 targets And the target 1 name is "a.out" + And the target 1 index is 0 And the target 2 name is "b.out" + And the target 2 index is 1 Scenario: Generating targets with glob using Windows separators Given the manifest file "tests/data/glob_windows.yml" is parsed diff --git a/tests/manifest_glob_tests.rs b/tests/manifest_glob_tests.rs index a68dd2c5..2926184b 100644 --- a/tests/manifest_glob_tests.rs +++ b/tests/manifest_glob_tests.rs @@ -72,7 +72,7 @@ fn temp_dir() -> tempfile::TempDir { &[(".hidden.txt", "h")], &[], "*.txt", - "ok", + "{{ item }}", &[".hidden.txt"], "dotfiles should match", )] @@ -123,9 +123,7 @@ fn test_glob_behavior( assert!(manifest.targets.is_empty(), "{description}"); } else { let names = target_names(&manifest); - if expected_partial == [".hidden.txt"] { - assert_eq!(manifest.targets.len(), 1, "{description}"); - } else if name_template.contains("replace") { + if name_template.contains("replace") { let expected: Vec<_> = expected_partial .iter() .map(|s| s.replace(".txt", ".out")) @@ -149,8 +147,11 @@ fn glob_invalid_pattern_errors() { let yaml = manifest_yaml("targets:\n - foreach: glob('[')\n name: bad\n command: echo hi\n"); let err = manifest::from_str(&yaml).expect_err("invalid pattern should error"); - let msg = format!("{err:?}").to_lowercase(); - assert!(msg.contains("invalid glob pattern"), "{msg}"); + assert!( + err.chain() + .any(|e| e.to_string().contains("invalid glob pattern")), + "unexpected error: {err}", + ); } #[rstest] From bc60cf54fda6a728a4856f06df0bf10d133330f9 Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 26 Aug 2025 09:17:40 +0100 Subject: [PATCH 08/11] Simplify glob tests and add Windows case check Unify name normalisation and remove redundant unmatched test. Add Windows-specific case-sensitivity coverage and assert invalid-pattern messages via alternate display. --- tests/manifest_glob_tests.rs | 72 +++++++++++++++--------------------- 1 file changed, 29 insertions(+), 43 deletions(-) diff --git a/tests/manifest_glob_tests.rs b/tests/manifest_glob_tests.rs index 2926184b..4c601543 100644 --- a/tests/manifest_glob_tests.rs +++ b/tests/manifest_glob_tests.rs @@ -49,7 +49,7 @@ fn temp_dir() -> tempfile::TempDir { &[], "*.txt", "{{ item | replace('{dir}/', '') | replace('.txt', '.out') }}", - &["a.txt", "b.txt"], + &["a.out", "b.out"], "expands and sorts matches", )] #[case( @@ -122,23 +122,13 @@ fn test_glob_behavior( if expected_partial.is_empty() { assert!(manifest.targets.is_empty(), "{description}"); } else { - let names = target_names(&manifest); - if name_template.contains("replace") { - let expected: Vec<_> = expected_partial - .iter() - .map(|s| s.replace(".txt", ".out")) - .collect(); - assert_eq!(names, expected, "{description}"); - } else { - let prefix_fwd = format!("{dir_str}/"); - let prefix_back = format!("{dir_str}\\"); - let expected: Vec<_> = expected_partial.iter().map(|&s| s.to_string()).collect(); - let normalised: Vec<_> = names - .into_iter() - .map(|n| n.replace(&prefix_fwd, "").replace(&prefix_back, "")) - .collect(); - assert_eq!(normalised, expected, "{description}"); - } + let prefix_fwd = format!("{dir_str}/"); + let prefix_back = format!("{dir_str}\\"); + let names: Vec<_> = target_names(&manifest) + .into_iter() + .map(|n| n.replace(&prefix_fwd, "").replace(&prefix_back, "")) + .collect(); + assert_eq!(names, expected_partial, "{description}"); } } @@ -147,31 +137,8 @@ fn glob_invalid_pattern_errors() { let yaml = manifest_yaml("targets:\n - foreach: glob('[')\n name: bad\n command: echo hi\n"); let err = manifest::from_str(&yaml).expect_err("invalid pattern should error"); - assert!( - err.chain() - .any(|e| e.to_string().contains("invalid glob pattern")), - "unexpected error: {err}", - ); -} - -#[rstest] -fn glob_returns_empty_when_no_matches() { - let dir = tempfile::tempdir().expect("temp dir"); - let pattern = format!("{}/*.nomatch", dir.path().display()); - let yaml = manifest_yaml(&format!( - concat!( - "targets:\n", - " - foreach: glob('{pattern}')\n", - " name: test\n", - " command: echo hi\n", - ), - pattern = pattern, - )); - let manifest = manifest::from_str(&yaml).expect("parse manifest"); - assert!( - manifest.targets.is_empty(), - "expected no files to match the glob pattern", - ); + let msg = format!("{err:#}"); + assert!(msg.contains("invalid glob pattern"), "{msg}"); } #[rstest] @@ -203,3 +170,22 @@ fn glob_accepts_windows_path_separators(temp_dir: tempfile::TempDir) { .collect(); assert_eq!(names, vec!["a.out", "b.out"]); } + +#[cfg(windows)] +#[rstest] +fn glob_is_case_sensitive_on_windows() { + let dir = tempfile::tempdir().expect("temp dir"); + fs::write(dir.path().join("UPPER.TXT"), "x").expect("write file"); + let pattern = format!("{}/*.txt", dir.path().display()); + let yaml = manifest_yaml(&format!( + concat!( + "targets:\n", + " - foreach: glob('{pattern}')\n", + " name: fail\n", + " command: echo hi\n", + ), + pattern = pattern, + )); + let manifest = manifest::from_str(&yaml).expect("parse"); + assert!(manifest.targets.is_empty()); +} From b47472c36a25e678c3e4cd2c25197295eff26b3f Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 26 Aug 2025 11:32:53 +0100 Subject: [PATCH 09/11] Note early metadata query in glob entry helper Explain the early metadata lookup in process_glob_entry so filesystem errors surface before UTF-8 handling. --- src/manifest.rs | 73 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 48 insertions(+), 25 deletions(-) diff --git a/src/manifest.rs b/src/manifest.rs index 2da44d19..78b4c4b2 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -144,6 +144,52 @@ fn env_var(name: &str) -> std::result::Result { } } +/// Process a single glob entry and normalise its output. +/// +/// Returns the entry path when it points to a file, skipping directories. +/// Paths are converted to UTF-8 and normalised to use forward slashes. +/// +/// # Examples +/// +/// ```rust,ignore +/// use glob::glob; +/// let entry = glob("Cargo.toml").unwrap().next().unwrap(); +/// let path = process_glob_entry(entry, "Cargo.toml") +/// .unwrap() +/// .unwrap(); +/// assert!(path.ends_with("Cargo.toml")); +/// ``` +fn process_glob_entry( + entry: std::result::Result, + pattern: &str, +) -> std::result::Result, Error> { + match entry { + Ok(path) => { + // Query metadata early to surface filesystem errors promptly. + let meta = path.metadata().map_err(|e| { + Error::new( + ErrorKind::InvalidOperation, + format!("glob failed for '{pattern}': {e}"), + ) + })?; + if !meta.is_file() { + return Ok(None); + } + let s = path.to_str().ok_or_else(|| { + Error::new( + ErrorKind::InvalidOperation, + format!("glob matched a non-UTF-8 path: {}", path.display()), + ) + })?; + Ok(Some(s.replace('\\', "/"))) + } + Err(e) => Err(Error::new( + ErrorKind::InvalidOperation, + format!("glob failed for '{pattern}': {e}"), + )), + } +} + /// Expand a glob pattern into a list of matching paths with deterministic ordering. /// /// Results are returned in lexicographic order to keep builds deterministic. @@ -183,31 +229,8 @@ fn glob_paths(pattern: &str) -> std::result::Result, Error> { })?; let mut paths = Vec::new(); for entry in entries { - match entry { - Ok(path) => match path.metadata() { - Ok(meta) if meta.is_file() => { - let s = path.to_str().ok_or_else(|| { - Error::new( - ErrorKind::InvalidOperation, - format!("glob matched a non-UTF-8 path: {}", path.display()), - ) - })?; - paths.push(s.replace('\\', "/")); - } - Ok(_) => {} - Err(e) => { - return Err(Error::new( - ErrorKind::InvalidOperation, - format!("glob failed for '{pattern}': {e}"), - )); - } - }, - Err(e) => { - return Err(Error::new( - ErrorKind::InvalidOperation, - format!("glob failed for '{pattern}': {e}"), - )); - } + if let Some(p) = process_glob_entry(entry, pattern)? { + paths.push(p); } } Ok(paths) From 2d642613b5e4cfec63d7d7af5ccf76e6714b5910 Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 26 Aug 2025 14:15:58 +0100 Subject: [PATCH 10/11] Normalise glob separators and refine glob tests --- docs/netsuke-design.md | 29 +++++++++++++---------------- src/manifest.rs | 33 +++++++++++++++++++++++++++++---- src/manifest/tests.rs | 13 +++++++++++++ tests/data/glob.yml | 2 +- tests/data/glob_windows.yml | 2 +- tests/features/manifest.feature | 3 +++ tests/manifest_glob_tests.rs | 28 ++++++++++++++++------------ tests/steps/manifest_steps.rs | 2 +- 8 files changed, 77 insertions(+), 35 deletions(-) diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index 1f71e31f..6567614a 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -732,22 +732,19 @@ providing a secure bridge to the underlying system. error if the variable is undefined or contains invalid UTF-8 to ensure manifests fail fast on missing inputs. -- `glob(pattern: &str) -> Result, Error>`: A function that - performs file path globbing. This is a critical feature for any modern build - tool, allowing users to easily specify sets of source files (e.g., - `src/**/*.c`). Results are yielded in lexicographic order by the iterator and - Netsuke returns them as such. Symlinks are followed by the `glob` crate by - default. Matching is case-sensitive on all platforms for determinism. Note - that `glob_with` enforces `require_literal_separator = true` internally (even - if disabled in options), and wildcards do not cross path separators unless - `**` is used. Callers may use either `/` or `\` in patterns; these are - normalised to the host platform before matching so manifests behave - consistently across operating systems. Results contain only files - (directories are ignored) and have path separators normalised to `/`. Empty - results are represented as an empty list. Invalid patterns or filesystem - errors surface as `InvalidOperation` to match minijinja's error semantics. - This function bridges a key feature gap, as Ninja itself does not support - globbing.[^3] +- `glob(pattern: &str) -> Result, Error>`: Expand filesystem + patterns (e.g., `src/**/*.c`) into a list of matched paths. Results are + yielded in lexicographic order by the iterator and returned unchanged. + Symlinks are followed by the `glob` crate by default. Matching is case- + sensitive on all platforms. `glob_with` enforces + `require_literal_separator = true` internally, so wildcards do not cross path + separators unless `**` is used. Callers may use `/` or `\` in patterns; these + are normalised to the host platform before matching. Results contain only + files (directories are ignored) and path separators are normalised to `/`. + Leading-dot entries are matched by wildcards. Empty results are represented + as `[]`. Invalid patterns surface as `SyntaxError`; filesystem iteration + errors surface as `InvalidOperation`, matching minijinja error semantics. + This fills a Ninja gap since Ninja itself does not support globbing.[^3] - `python_version(requirement: &str) -> Result`: An example of a domain-specific helper function that demonstrates the extensibility of this architecture. This function would execute `python --version` or diff --git a/src/manifest.rs b/src/manifest.rs index 78b4c4b2..b5326cc5 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -203,6 +203,33 @@ fn process_glob_entry( /// Matching is case-sensitive on all platforms, wildcards do not cross path /// separators (use `**` to span directories), and leading-dot entries are /// matched by wildcards. +/// Convert `/` and `\` to the host separator while preserving escapes for +/// glob metacharacters on Unix. +fn normalise_separators(pattern: &str) -> String { + let native = std::path::MAIN_SEPARATOR; + #[cfg(unix)] + { + let mut out = String::with_capacity(pattern.len()); + let mut it = pattern.chars().peekable(); + while let Some(c) = it.next() { + if c == '\\' && matches!(it.peek(), Some('[' | ']' | '{' | '}')) { + out.push('\\'); + continue; + } + if c == '/' || c == '\\' { + out.push(native); + } else { + out.push(c); + } + } + out + } + #[cfg(not(unix))] + { + pattern.replace('/', &native.to_string()) + } +} + fn glob_paths(pattern: &str) -> std::result::Result, Error> { use glob::{MatchOptions, glob_with}; @@ -216,10 +243,8 @@ fn glob_paths(pattern: &str) -> std::result::Result, Error> { require_literal_leading_dot: false, }; - // Normalise path separators so that callers may use either `/` or `\` - // regardless of platform. The `glob` crate expects the native separator. - let native_sep = std::path::MAIN_SEPARATOR.to_string(); - let normalized = pattern.replace('\\', "/").replace('/', &native_sep); + // Normalise separators so `/` and `\\` behave the same on all platforms. + let normalized = normalise_separators(pattern); let entries = glob_with(&normalized, opts).map_err(|e| { Error::new( diff --git a/src/manifest/tests.rs b/src/manifest/tests.rs index 6dad47f2..3436a1b5 100644 --- a/src/manifest/tests.rs +++ b/src/manifest/tests.rs @@ -24,3 +24,16 @@ fn glob_paths_invalid_pattern_sets_syntax_error() { let err = super::glob_paths("[").expect_err("expected pattern error"); assert_eq!(err.kind(), minijinja::ErrorKind::SyntaxError); } + +#[test] +fn glob_paths_ignores_directories() { + let dir = tempfile::tempdir().expect("temp dir"); + std::fs::create_dir(dir.path().join("sub")).expect("create dir"); + std::fs::write(dir.path().join("a.txt"), "a").expect("write file"); + let pattern = format!("{}/{}", dir.path().display(), "*"); + let out = super::glob_paths(&pattern).expect("glob ok"); + assert_eq!( + out, + vec![format!("{}/a.txt", dir.path().display()).replace('\\', "/")] + ); +} diff --git a/tests/data/glob.yml b/tests/data/glob.yml index f1b4b1ed..e0cfa4b1 100644 --- a/tests/data/glob.yml +++ b/tests/data/glob.yml @@ -2,4 +2,4 @@ netsuke_version: 1.0.0 targets: - foreach: glob('tests/data/glob_files/*.txt') name: "{{ item | replace('tests/data/glob_files/', '') | replace('.txt', '.out') }}" - command: "cat {{ item }}" + command: "echo {{ item }}" diff --git a/tests/data/glob_windows.yml b/tests/data/glob_windows.yml index 3dd3d339..984183df 100644 --- a/tests/data/glob_windows.yml +++ b/tests/data/glob_windows.yml @@ -2,4 +2,4 @@ netsuke_version: 1.0.0 targets: - foreach: glob('tests\\data\\glob_files\\*.txt') name: "{{ item | replace('tests/data/glob_files/', '') | replace('.txt', '.out') }}" - command: "cat {{ item }}" + command: "echo {{ item }}" diff --git a/tests/features/manifest.feature b/tests/features/manifest.feature index f654aaf3..84a98954 100644 --- a/tests/features/manifest.feature +++ b/tests/features/manifest.feature @@ -106,6 +106,8 @@ Feature: Manifest Parsing Then the manifest has 2 targets And the target 1 name is "a.out" And the target 2 name is "b.out" + And the target 1 index is 0 + And the target 2 index is 1 Scenario: Parsing fails for an invalid glob pattern Given the manifest file "tests/data/glob_invalid.yml" is parsed @@ -113,6 +115,7 @@ Feature: Manifest Parsing Then parsing the manifest fails And the error message contains "glob pattern" + Scenario: Parsing fails when a foreach expression is not iterable Given the manifest file "tests/data/foreach_invalid.yml" is parsed When the parsing result is checked diff --git a/tests/manifest_glob_tests.rs b/tests/manifest_glob_tests.rs index 4c601543..15e0806e 100644 --- a/tests/manifest_glob_tests.rs +++ b/tests/manifest_glob_tests.rs @@ -68,6 +68,14 @@ fn temp_dir() -> tempfile::TempDir { &[], "wildcards must not cross '/'", )] +#[case( + &[("sub/x.txt", "x")], + &[], + "**/*.txt", + "{{ item | replace('{dir}/', '') }}", + &["sub/x.txt"], + "** spans directories", +)] #[case( &[(".hidden.txt", "h")], &[], @@ -105,6 +113,7 @@ fn test_glob_behavior( create_test_dirs(temp_dir.path(), dirs); let dir_str = temp_dir.path().display().to_string(); + let dir_fwd = dir_str.replace('\\', "/"); let pattern = format!("{dir_str}/{pattern_suffix}"); let yaml = manifest_yaml(&format!( concat!( @@ -114,7 +123,7 @@ fn test_glob_behavior( " command: echo hi\n", ), pattern = pattern, - name_template = name_template.replace("{dir}", &dir_str) + name_template = name_template.replace("{dir}", &dir_fwd), )); let manifest = manifest::from_str(&yaml).expect("parse"); @@ -122,7 +131,7 @@ fn test_glob_behavior( if expected_partial.is_empty() { assert!(manifest.targets.is_empty(), "{description}"); } else { - let prefix_fwd = format!("{dir_str}/"); + let prefix_fwd = format!("{dir_fwd}/"); let prefix_back = format!("{dir_str}\\"); let names: Vec<_> = target_names(&manifest) .into_iter() @@ -132,6 +141,7 @@ fn test_glob_behavior( } } +#[rstest] #[rstest] fn glob_invalid_pattern_errors() { let yaml = @@ -159,24 +169,18 @@ fn glob_accepts_windows_path_separators(temp_dir: tempfile::TempDir) { )); let manifest = manifest::from_str(&yaml).expect("parse"); let prefix_fwd = format!("{dir_fwd}/"); - let prefix_back = format!("{dir_win}\\"); let names: Vec<_> = target_names(&manifest) .into_iter() - .map(|n| { - n.replace(&prefix_fwd, "") - .replace(&prefix_back, "") - .replace(".txt", ".out") - }) + .map(|n| n.replace(&prefix_fwd, "").replace(".txt", ".out")) .collect(); assert_eq!(names, vec!["a.out", "b.out"]); } #[cfg(windows)] #[rstest] -fn glob_is_case_sensitive_on_windows() { - let dir = tempfile::tempdir().expect("temp dir"); - fs::write(dir.path().join("UPPER.TXT"), "x").expect("write file"); - let pattern = format!("{}/*.txt", dir.path().display()); +fn glob_is_case_sensitive_on_windows(temp_dir: tempfile::TempDir) { + fs::write(temp_dir.path().join("UPPER.TXT"), "x").expect("write file"); + let pattern = format!("{}/*.txt", temp_dir.path().display()); let yaml = manifest_yaml(&format!( concat!( "targets:\n", diff --git a/tests/steps/manifest_steps.rs b/tests/steps/manifest_steps.rs index db92b3f0..129e26d5 100644 --- a/tests/steps/manifest_steps.rs +++ b/tests/steps/manifest_steps.rs @@ -27,7 +27,7 @@ fn parse_manifest_inner(world: &mut CliWorld, path: &str) { } Err(e) => { world.manifest = None; - world.manifest_error = Some(format!("{e:?}")); + world.manifest_error = Some(format!("{e:#}")); } } } From 15dd40c598d7c4a0bf00ec0ace06c44c6de04c2b Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 26 Aug 2025 16:09:38 +0100 Subject: [PATCH 11/11] Handle backslash prefixes in glob tests --- docs/netsuke-design.md | 2 ++ src/manifest.rs | 5 ++++- src/manifest/tests.rs | 6 ++++++ tests/features/manifest.feature | 1 + tests/manifest_glob_tests.rs | 7 ++++--- 5 files changed, 17 insertions(+), 4 deletions(-) diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index 6567614a..bd4d8fd5 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -744,6 +744,8 @@ providing a secure bridge to the underlying system. Leading-dot entries are matched by wildcards. Empty results are represented as `[]`. Invalid patterns surface as `SyntaxError`; filesystem iteration errors surface as `InvalidOperation`, matching minijinja error semantics. + On Unix, backslash escapes for glob metacharacters (`[`, `]`, `{`, `}`) are + preserved during separator normalisation. This fills a Ninja gap since Ninja itself does not support globbing.[^3] - `python_version(requirement: &str) -> Result`: An example of a domain-specific helper function that demonstrates the extensibility of this diff --git a/src/manifest.rs b/src/manifest.rs index b5326cc5..76343539 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -147,7 +147,8 @@ fn env_var(name: &str) -> std::result::Result { /// Process a single glob entry and normalise its output. /// /// Returns the entry path when it points to a file, skipping directories. -/// Paths are converted to UTF-8 and normalised to use forward slashes. +/// Requires matched paths to be valid UTF-8; output is normalised to use +/// forward slashes. /// /// # Examples /// @@ -175,6 +176,8 @@ fn process_glob_entry( if !meta.is_file() { return Ok(None); } + // Reject non-UTF-8 paths to avoid lossy round-trips and ensure + // manifests remain deterministic. let s = path.to_str().ok_or_else(|| { Error::new( ErrorKind::InvalidOperation, diff --git a/src/manifest/tests.rs b/src/manifest/tests.rs index 3436a1b5..f61179db 100644 --- a/src/manifest/tests.rs +++ b/src/manifest/tests.rs @@ -25,6 +25,12 @@ fn glob_paths_invalid_pattern_sets_syntax_error() { assert_eq!(err.kind(), minijinja::ErrorKind::SyntaxError); } +#[cfg(unix)] +#[test] +fn normalise_separators_preserves_bracket_escape() { + assert_eq!(super::normalise_separators("\\["), "\\["); +} + #[test] fn glob_paths_ignores_directories() { let dir = tempfile::tempdir().expect("temp dir"); diff --git a/tests/features/manifest.feature b/tests/features/manifest.feature index 84a98954..64622475 100644 --- a/tests/features/manifest.feature +++ b/tests/features/manifest.feature @@ -116,6 +116,7 @@ Feature: Manifest Parsing And the error message contains "glob pattern" + Scenario: Parsing fails when a foreach expression is not iterable Given the manifest file "tests/data/foreach_invalid.yml" is parsed When the parsing result is checked diff --git a/tests/manifest_glob_tests.rs b/tests/manifest_glob_tests.rs index 15e0806e..78c0a732 100644 --- a/tests/manifest_glob_tests.rs +++ b/tests/manifest_glob_tests.rs @@ -48,7 +48,7 @@ fn temp_dir() -> tempfile::TempDir { &[("b.txt", "b"), ("a.txt", "a")], &[], "*.txt", - "{{ item | replace('{dir}/', '') | replace('.txt', '.out') }}", + "{{ item | replace('{dir}/', '') | replace('{dir}\\\\', '') | replace('.txt', '.out') }}", &["a.out", "b.out"], "expands and sorts matches", )] @@ -123,7 +123,9 @@ fn test_glob_behavior( " command: echo hi\n", ), pattern = pattern, - name_template = name_template.replace("{dir}", &dir_fwd), + name_template = name_template + .replace("{dir}/", &format!("{dir_fwd}/")) + .replace("{dir}\\\\", &format!("{dir_str}\\\\\\\\")), )); let manifest = manifest::from_str(&yaml).expect("parse"); @@ -141,7 +143,6 @@ fn test_glob_behavior( } } -#[rstest] #[rstest] fn glob_invalid_pattern_errors() { let yaml =