Repository: kaspar030/laze (language: python)

Issue:
feat: basic expression evaluation
e.g., allow a variable to be `$(max(1,5))`, or `$(dirname $(outfile))`.


Discussion / hints:

Fix patch:
diff --git a/Cargo.lock b/Cargo.lock
index 6cbc3c48..96a9124e 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -365,6 +365,12 @@ dependencies = [
  "libc",
 ]
 
+[[package]]
+name = "evalexpr"
+version = "11.0.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "ec3b4e80fd8edee77d6fc35d59862d5765dae7512a95f49fd7bf03847c85b26a"
+
 [[package]]
 name = "fnv"
 version = "1.0.7"
@@ -541,6 +547,7 @@ dependencies = [
  "clap_complete",
  "clap_mangen",
  "derive_builder",
+ "evalexpr",
  "im",
  "indexmap 1.9.3",
  "itertools",
diff --git a/Cargo.toml b/Cargo.toml
index 08138b6b..5c15477b 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -36,10 +36,11 @@ solvent = { version = "0.8.3", features = ["deterministic"] }
 url = "2.4.0"
 rust-embed = "6.8.1"
 task_partitioner = "0.1.1"
+
 clap_complete = "4.3.2"
 clap_mangen = "0.2.12"
 camino = { version = "1.1.6", features = ["serde1"] }
-
+evalexpr = "11.0.0"
 
 [profile.release]
 lto = "fat"
diff --git a/src/data.rs b/src/data.rs
index b01f8675..8cc91b09 100644
--- a/src/data.rs
+++ b/src/data.rs
@@ -384,8 +384,16 @@ pub fn load(filename: &Utf8Path, build_dir: &Utf8Path) -> Result<(ContextBag, Fi
             context_.tasks = Some(
                 tasks
                     .iter()
-                    .map(|(name, task)| (name.clone(), task.with_env(&flattened_early_env)))
-                    .collect(),
+                    .map(|(name, task)| {
+                        let task = task.with_env(&flattened_early_env);
+                        match task {
+                            Ok(task) => Ok((name.clone(), task)),
+                            Err(e) => Err(e),
+                        }
+                        .with_context(|| format!("task \"{}\"", name.clone()))
+                    })
+                    .collect::<Result<_, _>>()
+                    .with_context(|| format!("{:?} context \"{}\"", &filename, context.name))?,
             )
         }
 
diff --git a/src/generate.rs b/src/generate.rs
index 2b62f334..be5361d8 100644
--- a/src/generate.rs
+++ b/src/generate.rs
@@ -10,7 +10,7 @@ use std::hash::Hasher;
 use std::io::prelude::*;
 use std::time::Instant;
 
-use anyhow::Result;
+use anyhow::{Context as _, Result};
 use camino::{Utf8Path, Utf8PathBuf};
 use indexmap::{IndexMap, IndexSet};
 use itertools::Itertools;
@@ -212,18 +212,21 @@ impl Generator {
                     self.disable.as_ref(),
                     &self.cli_env.as_ref(),
                 )
-                .unwrap()
+                .with_context(|| format!("binary \"{}\"", bin.name))
+                .with_context(|| format!("builder \"{}\"", builder.name))
                 {
-                    Some((build_info, ninja_entries)) => Some((
+                    Ok(Some((build_info, ninja_entries))) => Some(Ok((
                         builder.name.clone(),
                         bin.name.clone(),
                         build_info,
                         ninja_entries,
-                    )),
-                    _ => None,
+                    ))),
+                    Ok(None) => None,
+                    Err(e) => Some(Err(e)),
                 }
             })
-            .collect::<Vec<(String, String, BuildInfo, IndexSet<String>)>>();
+            .collect::<Result<Vec<(String, String, BuildInfo, IndexSet<String>)>, anyhow::Error>>(
+            )?;
 
         let mut combined_ninja_entries = IndexSet::new();
         let builds = builds
@@ -832,7 +835,7 @@ fn configure_build(
     global_env_flattened.insert(&out_string, outfile.to_string());
     let tasks = build
         .build_context
-        .collect_tasks(contexts, &global_env_flattened);
+        .collect_tasks(contexts, &global_env_flattened)?;
 
     Ok(Some((
         BuildInfo {
diff --git a/src/model/context.rs b/src/model/context.rs
index be20a12f..da4e1d24 100644
--- a/src/model/context.rs
+++ b/src/model/context.rs
@@ -1,3 +1,4 @@
+use anyhow::{Context as _, Error};
 use indexmap::{IndexMap, IndexSet};
 use std::collections::HashMap;
 use std::hash::{Hash, Hasher};
@@ -135,18 +136,22 @@ impl Context {
         &self,
         contexts: &ContextBag,
         env: &im::HashMap<&String, String>,
-    ) -> IndexMap<String, Task> {
+    ) -> Result<IndexMap<String, Task>, Error> {
         let mut result = IndexMap::new();
         let mut parents = Vec::new();
         self.get_parents(contexts, &mut parents);
         for parent in parents {
             if let Some(tasks) = &parent.tasks {
                 for (name, task) in tasks {
-                    result.insert(name.clone(), task.with_env(env));
+                    result.insert(
+                        name.clone(),
+                        task.with_env(env)
+                            .with_context(|| format!("task \"{}\"", name))?,
+                    );
                 }
             }
         }
-        result
+        Ok(result)
     }
 
     pub fn collect_disabled_modules(&self, contexts: &ContextBag) -> IndexSet<String> {
diff --git a/src/model/task.rs b/src/model/task.rs
index 67369e29..b7984bff 100644
--- a/src/model/task.rs
+++ b/src/model/task.rs
@@ -30,6 +30,7 @@ impl Task {
             use shell_words::join;
             use std::process::Command;
             let mut command = Command::new("sh");
+            let cmd = cmd.replace("$$", "$");
             if verbose > 0 {
                 command.arg("-x");
             }
@@ -55,19 +56,15 @@ impl Task {
         Ok(())
     }
 
-    pub fn with_env(&self, env: &im::HashMap<&String, String>) -> Task {
-        Task {
+    pub fn with_env(&self, env: &im::HashMap<&String, String>) -> Result<Task, Error> {
+        Ok(Task {
             build: self.build,
             cmd: self
                 .cmd
                 .iter()
-                .map(|cmd| {
-                    nested_env::expand(cmd, env, nested_env::IfMissing::Ignore)
-                        .unwrap()
-                        .replace("$$", "$")
-                })
-                .collect(),
+                .map(|cmd| nested_env::expand(cmd, env, nested_env::IfMissing::Ignore))
+                .collect::<Result<Vec<String>, _>>()?,
             ignore_ctrl_c: self.ignore_ctrl_c,
-        }
+        })
     }
 }
diff --git a/src/nested_env/expand.rs b/src/nested_env/expand.rs
index 0af298a1..b50e3dbe 100644
--- a/src/nested_env/expand.rs
+++ b/src/nested_env/expand.rs
@@ -1,14 +1,17 @@
 /* this is based on "far" (https://forge.typ3.tech/charles/far) */
 
+use evalexpr::EvalexprError;
 use im::HashMap;
+use std::borrow::Cow;
 use std::error;
 use std::fmt;
 
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq)]
 pub enum ExpandError {
     Missing(String),
     Unclosed(usize),
     Cycle(String),
+    Expr(EvalexprError),
 }
 
 #[derive(Debug, Copy, Clone)]
@@ -25,6 +28,7 @@ impl fmt::Display for ExpandError {
             ExpandError::Missing(s) => write!(f, "missing variable \"{}\"", s),
             ExpandError::Cycle(s) => write!(f, "cycle involving variable \"{}\"", s),
             ExpandError::Unclosed(start) => write!(f, "unclosed brace at pos {}", start),
+            ExpandError::Expr(e) => write!(f, "expression error: {}", e),
         }
     }
 }
@@ -143,7 +147,12 @@ where
         result = result.replace("\\${", "${");
     }
 
-    Ok(result)
+    let evaluated = super::expr::eval(&result);
+    match evaluated {
+        Ok(Cow::Owned(s)) => Ok(s),
+        Err(e) => Err(ExpandError::Expr(e)),
+        _ => Ok(result),
+    }
 }
 
 #[cfg(test)]
diff --git a/src/nested_env/expr.rs b/src/nested_env/expr.rs
new file mode 100644
index 00000000..7e182d2c
--- /dev/null
+++ b/src/nested_env/expr.rs
@@ -0,0 +1,85 @@
+use std::borrow::Cow;
+
+use evalexpr::EvalexprError;
+
+pub fn eval<'a>(input: &'a str) -> Result<Cow<'a, str>, EvalexprError> {
+    eval_recursive(input, false)
+}
+
+fn eval_recursive<'a>(input: &'a str, is_eval: bool) -> Result<Cow<'a, str>, EvalexprError> {
+    let mut result = String::new();
+    let mut start = 0;
+    let mut level = 0;
+    let mut input_changed = false;
+
+    for (i, character) in input.char_indices() {
+        if character == '$'
+            && i + 1 < input.len()
+            && input[i + 1..i + 2] == *"("
+            && (i == 0 || (input[i - 1..i] != *"$"))
+        {
+            if level == 0 {
+                start = i + 1;
+            }
+        } else if character == '(' && start > 0 {
+            level += 1;
+        } else if character == ')' && level > 0 && start > 0 {
+            level -= 1;
+            if level == 0 {
+                input_changed = true;
+                result.push_str(&eval_recursive(&input[start + 1..i], true)?);
+            }
+        } else if level == 0 {
+            result.push(character);
+        }
+    }
+
+    if is_eval {
+        let expr = evalexpr::eval(&result)?.to_string();
+        input_changed = true;
+        result = expr;
+    }
+
+    if input_changed {
+        Ok(result.into())
+    } else {
+        Ok(input.into())
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn basic() {
+        let result = eval("foo $(1+$(1+1)) after_foo");
+        assert_eq!(result.unwrap(), "foo 3 after_foo");
+    }
+    #[test]
+    fn nested_braces() {
+        let result = eval("$((0))");
+        assert_eq!(result.unwrap(), "0");
+    }
+    #[test]
+    fn basic_eval_max() {
+        let result = eval("$(max(1,2,3,4))");
+        assert_eq!(result.unwrap(), "4");
+    }
+    #[test]
+    fn basic_eval_add() {
+        let result = eval("$(str::to_uppercase \"foobar\")");
+        assert_eq!(result.unwrap(), "\"FOOBAR\"");
+    }
+    #[test]
+    fn unchanged() {
+        let result = eval("just some text");
+        assert_eq!(result.unwrap(), Cow::Borrowed("just some text"));
+    }
+    #[test]
+    fn escaped_dollar() {
+        let literal = "just some $$(foo) text";
+        let result = eval(literal);
+        assert_eq!(result.unwrap(), Cow::Borrowed(literal));
+    }
+}
diff --git a/src/nested_env/mod.rs b/src/nested_env/mod.rs
index 683cdf69..5cb7cc43 100644
--- a/src/nested_env/mod.rs
+++ b/src/nested_env/mod.rs
@@ -2,6 +2,7 @@ use im::{vector, HashMap, Vector};
 use itertools::join;
 
 mod expand;
+mod expr;
 
 pub use expand::{expand, IfMissing};
 


Test patch:
diff --git a/src/tests/31_expr/build_expected/build-global.ninja b/src/tests/31_expr/build_expected/build-global.ninja
new file mode 100644
index 00000000..081d7601
--- /dev/null
+++ b/src/tests/31_expr/build_expected/build-global.ninja
@@ -0,0 +1,18 @@
+builddir = build
+build ALWAYS: phony
+rule CC_14556393563797253018
+  command = echo result = 2  ${in} > ${out}
+  description = CC
+
+build build/objects/single_app.7419491670495048065.o: $
+    CC_14556393563797253018 $
+    single_app.c
+
+rule LINK_5506617845631750009
+  command = cat ${in} > ${out}
+  description = LINK
+
+build build/single_builder/single_app/single_app.elf: $
+    LINK_5506617845631750009 $
+    build/objects/single_app.7419491670495048065.o
+
diff --git a/src/tests/31_expr/build_expected/objects/single_app.7419491670495048065.o b/src/tests/31_expr/build_expected/objects/single_app.7419491670495048065.o
new file mode 100644
index 00000000..6b9bf13a
--- /dev/null
+++ b/src/tests/31_expr/build_expected/objects/single_app.7419491670495048065.o
@@ -0,0 +1,1 @@
+result = 2 single_app.c
diff --git a/src/tests/31_expr/build_expected/single_builder/single_app/single_app.elf b/src/tests/31_expr/build_expected/single_builder/single_app/single_app.elf
new file mode 100644
index 00000000..6b9bf13a
--- /dev/null
+++ b/src/tests/31_expr/build_expected/single_builder/single_app/single_app.elf
@@ -0,0 +1,1 @@
+result = 2 single_app.c
diff --git a/src/tests/31_expr/laze-project.yml b/src/tests/31_expr/laze-project.yml
new file mode 100644
index 00000000..82031561
--- /dev/null
+++ b/src/tests/31_expr/laze-project.yml
@@ -0,0 +1,22 @@
+builders:
+  - name: single_builder
+    rules:
+        - name: CC
+          in: 'c'
+          out: 'o'
+          cmd: 'echo ${LOCAL_VAR} ${GLOBAL_VAR} ${in} > ${out}'
+        - name: LINK
+          in: 'o'
+          cmd: 'cat ${in} > ${out}'
+
+    env:
+      bindir: build/${builder}/${app}
+
+apps:
+  - name: single_app
+    sources:
+      - single_app.c
+
+    env:
+      local:
+        LOCAL_VAR: "result = $(1+1)"
diff --git a/src/tests/31_expr/single_app.c b/src/tests/31_expr/single_app.c
new file mode 100644
index 00000000..b182e6d6
--- /dev/null
+++ b/src/tests/31_expr/single_app.c
@@ -0,0 +1,1 @@
+content of single_app.c
diff --git a/src/tests/31_expr/test.sh b/src/tests/31_expr/test.sh
new file mode 100755
index 00000000..75f9d1fa
--- /dev/null
+++ b/src/tests/31_expr/test.sh
@@ -0,0 +1,13 @@
+#!/bin/sh
+
+. ../test-common.sh
+
+cleanup
+build
+clean_temp_files
+
+diff -r build build_expected
+
+echo TEST_OK
+
+cleanup