diff options
| -rw-r--r-- | doc/concepts/expressions.org | 5 | ||||
| -rw-r--r-- | src/buildtool/build_engine/expression/TARGETS | 1 | ||||
| -rw-r--r-- | src/buildtool/build_engine/expression/evaluator.cpp | 39 | ||||
| -rw-r--r-- | test/buildtool/build_engine/expression/expression.test.cpp | 12 |
4 files changed, 50 insertions, 7 deletions
diff --git a/doc/concepts/expressions.org b/doc/concepts/expressions.org index c8d2a9a5..50737413 100644 --- a/doc/concepts/expressions.org +++ b/doc/concepts/expressions.org @@ -255,7 +255,10 @@ those) argument(s) to obtain the final result. ~"subdir"~ argument and the base name of the old key. It is an error if conflicts occur in this way; in case of such a user error, the argument ~"msg"~ is also evaluated and the result - of that evaluatino reported in the error message. + of that evaluation reported in the error message. Note that + conflicts can also occur in non-flat staging if two keys are + different as strings, but name the same path (like ~"foo.txt"~ + and ~"./foo.txt"~), and are assigned different values. ***** Binary functions diff --git a/src/buildtool/build_engine/expression/TARGETS b/src/buildtool/build_engine/expression/TARGETS index 2a72ed8c..6acd30c2 100644 --- a/src/buildtool/build_engine/expression/TARGETS +++ b/src/buildtool/build_engine/expression/TARGETS @@ -39,6 +39,7 @@ , ["src/utils/cpp", "hex_string"] , ["src/utils/cpp", "concepts"] , ["src/utils/cpp", "atomic"] + , ["src/utils/cpp", "path"] , ["@", "gsl-lite", "", "gsl-lite"] ] , "stage": ["src", "buildtool", "build_engine", "expression"] diff --git a/src/buildtool/build_engine/expression/evaluator.cpp b/src/buildtool/build_engine/expression/evaluator.cpp index 479de3b8..cfd0be31 100644 --- a/src/buildtool/build_engine/expression/evaluator.cpp +++ b/src/buildtool/build_engine/expression/evaluator.cpp @@ -10,6 +10,7 @@ #include "fmt/core.h" #include "src/buildtool/build_engine/expression/configuration.hpp" #include "src/buildtool/build_engine/expression/function_map.hpp" +#include "src/utils/cpp/path.hpp" namespace { @@ -558,8 +559,8 @@ auto ToSubdirExpr(SubExprEvaluator&& eval, if (flat) { for (auto const& el : d->Map()) { std::filesystem::path k{el.first}; - auto new_path = subdir / k.filename(); - if (result.contains(new_path) && !(result[new_path] == el.second)) { + auto new_key = ToNormalPath(subdir / k.filename()).string(); + if (result.contains(new_key) && !(result[new_key] == el.second)) { // Check if the user specifed an error message for that case, // otherwise just generate a generic error message. auto msg_expr = expr->Map().Find("msg"); @@ -568,7 +569,7 @@ auto ToSubdirExpr(SubExprEvaluator&& eval, "Flat staging of {} to subdir {} conflicts on path {}", d->ToString(), subdir.string(), - new_path.string())}; + new_key)}; } std::string msg; try { @@ -581,16 +582,42 @@ auto ToSubdirExpr(SubExprEvaluator&& eval, std::stringstream ss{}; ss << msg << std::endl; ss << "Reason: flat staging to subdir " << subdir.string() - << " conflicts on path " << new_path.string() << std::endl; + << " conflicts on path " << new_key << std::endl; ss << "Map to flatly stage was " << d->ToString() << std::endl; throw Evaluator::EvaluationError(ss.str(), false, true); } - result[new_path] = el.second; + result[new_key] = el.second; } } else { for (auto const& el : d->Map()) { - result[(subdir / el.first).string()] = el.second; + auto new_key = ToNormalPath(subdir / el.first).string(); + if (auto it = result.find(new_key); + it != result.end() && !(it->second == el.second)) { + auto msg_expr = expr->Map().Find("msg"); + if (not msg_expr) { + throw Evaluator::EvaluationError{fmt::format( + "Staging of {} to subdir {} conflicts on path {}", + d->ToString(), + subdir.string(), + new_key)}; + } + std::string msg; + try { + auto msg_val = eval(msg_expr->get(), env); + msg = msg_val->ToString(); + } catch (std::exception const&) { + msg = + "[non evaluating term] " + msg_expr->get()->ToString(); + } + std::stringstream ss{}; + ss << msg << std::endl; + ss << "Reason: staging to subdir " << subdir.string() + << " conflicts on new path " << new_key << std::endl; + ss << "Map to stage was " << d->ToString() << std::endl; + throw Evaluator::EvaluationError(ss.str(), false, true); + } + result.emplace(std::move(new_key), el.second); } } return ExpressionPtr{Expression::map_t{result}}; diff --git a/test/buildtool/build_engine/expression/expression.test.cpp b/test/buildtool/build_engine/expression/expression.test.cpp index d80d8861..cbc8c7bf 100644 --- a/test/buildtool/build_engine/expression/expression.test.cpp +++ b/test/buildtool/build_engine/expression/expression.test.cpp @@ -1103,6 +1103,18 @@ TEST_CASE("Expression Evaluation", "[expression]") { // NOLINT R"({"prefix/foo": "hello", "prefix/bar": "world"})"_json)); } + SECTION("to_subdir expression with conflict") { + auto expr = Expression::FromJson(R"( + { "type": "to_subdir" + , "subdir": "prefix" + , "$1": { "type": "literal" + , "$1": { "foo": "hello" + , "./foo": "world" }}})"_json); + REQUIRE(expr); + + CHECK_FALSE(expr.Evaluate(env, fcts)); + } + SECTION("flat to_subdir without proper conflict") { auto expr = Expression::FromJson(R"( { "type": "to_subdir" |