diff options
| -rw-r--r-- | doc/concepts/built-in-rules.org | 28 | ||||
| -rw-r--r-- | src/buildtool/build_engine/target_map/built_in_rules.cpp | 127 | ||||
| -rw-r--r-- | test/end-to-end/TARGETS | 1 | ||||
| -rw-r--r-- | test/end-to-end/built-in-rules/TARGETS | 9 | ||||
| -rwxr-xr-x | test/end-to-end/built-in-rules/generic_out_dirs.sh | 71 |
5 files changed, 197 insertions, 39 deletions
diff --git a/doc/concepts/built-in-rules.org b/doc/concepts/built-in-rules.org index 0be32019..5d71e673 100644 --- a/doc/concepts/built-in-rules.org +++ b/doc/concepts/built-in-rules.org @@ -87,19 +87,25 @@ by giving precedence to the artifacts over the runfiles; conflicts within artifacts or runfiles are resolved in a latest-wins fashion using the order of the targets in the evaluated ~"deps"~ argument. -The fields ~"cmds"~, ~"outs"~, and ~"env"~ are evaluated fields -where ~"cmds"~ and ~"outs"~ have to evalaute to a list of strings, -and ~"env"~ has to evaluate to a map of strings. During their -evaluation, the functions ~"outs"~ and ~"runfiles"~ can be used to -access the logical paths of the artifcats and runfiles, respectively, -of a target specified in ~"deps"~. Here, ~"env"~ specifies the -environment in which the action is carried out and ~"outs"~ the -outputs, the action has to produce. Finally, the strings in ~"cmds"~ -are extended by a newline character and joined, and command of the -action is interpreting this string by ~sh~. +The fields ~"cmds"~, ~"out_dirs"~, ~"outs"~, and ~"env"~ are evaluated +fields where ~"cmds"~, ~"out_dirs"~, and ~"outs"~ have to evaluate to +a list of strings, and ~"env"~ has to evaluate to a map of +strings. During their evaluation, the functions ~"out_dirs"~, ~"outs"~ +and ~"runfiles"~ can be used to access the logical paths of the +directories, artifacts and runfiles, respectively, of a target +specified in ~"deps"~. Here, ~"env"~ specifies the environment in +which the action is carried out. ~"out_dirs"~ and ~"outs"~ define the +output directories and files, respectively, the action has to +produce. Since some artifacts are to be produced, at least one of +~"out_dirs"~ or ~"outs"~ must be a non-empty list of strings. It is an +error if one or more paths are present in both the ~"out_dirs"~ and +~"outs"~. Finally, the strings in ~"cmds"~ are extended by a newline +character and joined, and command of the action is interpreting this +string by ~sh~. The artifacts of this target are the outputs (as declared by -~"outs"~) of this action. Runfiles and provider map are empty. +~"out_dirs"~ and ~"outs"~) of this action. Runfiles and provider map +are empty. ** ~"file_gen"~ diff --git a/src/buildtool/build_engine/target_map/built_in_rules.cpp b/src/buildtool/build_engine/target_map/built_in_rules.cpp index 847bff8d..18786b60 100644 --- a/src/buildtool/build_engine/target_map/built_in_rules.cpp +++ b/src/buildtool/build_engine/target_map/built_in_rules.cpp @@ -3,17 +3,22 @@ #include <algorithm> #include <filesystem> #include <functional> +#include <iterator> #include <memory> #include <sstream> #include <string> #include <unordered_map> #include <unordered_set> +#include <fmt/format.h> +#include <nlohmann/json.hpp> + #include "src/buildtool/build_engine/base_maps/field_reader.hpp" #include "src/buildtool/build_engine/expression/expression_ptr.hpp" #include "src/buildtool/build_engine/target_map/export.hpp" #include "src/buildtool/build_engine/target_map/utils.hpp" #include "src/utils/cpp/path.hpp" +#include "src/utils/cpp/vector.hpp" namespace { @@ -24,6 +29,7 @@ auto const kGenericRuleFields = "env", "tainted", "type", + "out_dirs", "outs"}; auto const kFileGenRuleFields = @@ -609,36 +615,98 @@ void GenericRuleWithDeps( }}}); auto const& empty_list = Expression::kEmptyList; auto param_config = key.config.Prune(*param_vars); + auto outs_exp = desc->ReadOptionalExpression("outs", empty_list); - if (not outs_exp) { - return; + auto out_dirs_exp = desc->ReadOptionalExpression("out_dirs", empty_list); + + std::vector<std::string> outs{}; + std::vector<std::string> out_dirs{}; + if (outs_exp) { + auto outs_value = outs_exp.Evaluate( + param_config, string_fields_fcts, [&logger](auto const& msg) { + (*logger)(fmt::format("While evaluating outs:\n{}", msg), true); + }); + if (not outs_value) { + return; + } + if (not outs_value->IsList()) { + (*logger)(fmt::format("outs has to evaluate to a list of " + "strings, but found {}", + outs_value->ToString()), + true); + return; + } + if (not outs_value->List().empty()) { + outs.reserve(outs_value->List().size()); + for (auto const& x : outs_value->List()) { + if (not x->IsString()) { + (*logger)(fmt::format("outs has to evaluate to a list of " + "strings, but found entry {}", + x->ToString()), + true); + return; + } + outs.emplace_back(x->String()); + } + } } - auto outs_value = outs_exp.Evaluate( - param_config, string_fields_fcts, [&logger](auto const& msg) { - (*logger)(fmt::format("While evaluating outs:\n{}", msg), true); - }); - if (not outs_value) { + if (out_dirs_exp) { + auto out_dirs_value = out_dirs_exp.Evaluate( + param_config, string_fields_fcts, [&logger](auto const& msg) { + (*logger)(fmt::format("While evaluating out_dirs:\n{}", msg), + true); + }); + if (not out_dirs_value) { + return; + } + if (not out_dirs_value->IsList()) { + (*logger)(fmt::format("out_dirs has to evaluate to a list of " + "strings, but found {}", + out_dirs_value->ToString()), + true); + return; + } + if (not out_dirs_value->List().empty()) { + out_dirs.reserve(out_dirs_value->List().size()); + for (auto const& x : out_dirs_value->List()) { + if (not x->IsString()) { + (*logger)( + fmt::format("out_dirs has to evaluate to a list of " + "strings, but found entry {}", + x->ToString()), + true); + return; + } + out_dirs.emplace_back(x->String()); + } + } + } + + if (outs.empty() and out_dirs.empty()) { + (*logger)( + R"(At least one of "outs" and "out_dirs" must be specified for "generic")", + true); return; } - if ((not outs_value->IsList()) or outs_value->List().empty()) { - (*logger)(fmt::format("outs has to evaluate to a non-empty list of " - "strings, but found {}", - outs_value->ToString()), + + sort_and_deduplicate(&outs); + sort_and_deduplicate(&out_dirs); + + // looking for same paths in both outs and out_dirs + std::vector<std::string> intersection; + std::set_intersection(outs.begin(), + outs.end(), + out_dirs.begin(), + out_dirs.end(), + std::back_inserter(intersection)); + if (not intersection.empty()) { + (*logger)(fmt::format("outs and out_dirs for generic must be disjoint. " + "Found repeated entries:\n{}", + nlohmann::json(intersection).dump()), true); return; } - std::vector<std::string> outs{}; - outs.reserve(outs_value->List().size()); - for (auto const& x : outs_value->List()) { - if (not x->IsString()) { - (*logger)(fmt::format("outs has to evaluate to a non-empty list of " - "strings, but found entry {}", - x->ToString()), - true); - return; - } - outs.emplace_back(x->String()); - } + auto cmd_exp = desc->ReadOptionalExpression("cmds", empty_list); if (not cmd_exp) { return; @@ -709,7 +777,7 @@ void GenericRuleWithDeps( // Construct our single action, and its artifacts auto action = BuildMaps::Target::Utils::createAction(outs, - {}, + out_dirs, {"sh", "-c", cmd_ss.str()}, env_val, std::nullopt, @@ -717,10 +785,13 @@ void GenericRuleWithDeps( inputs); auto action_identifier = action->Id(); Expression::map_t::underlying_map_t artifacts; - for (auto const& path : outs) { - artifacts.emplace(path, - ExpressionPtr{ArtifactDescription{ - action_identifier, std::filesystem::path{path}}}); + for (const auto& container : {outs, out_dirs}) { + for (const auto& path : container) { + artifacts.emplace( + path, + ExpressionPtr{ArtifactDescription{ + action_identifier, std::filesystem::path{path}}}); + } } auto const& empty_map = Expression::kEmptyMap; diff --git a/test/end-to-end/TARGETS b/test/end-to-end/TARGETS index 720834fd..4737c7aa 100644 --- a/test/end-to-end/TARGETS +++ b/test/end-to-end/TARGETS @@ -8,6 +8,7 @@ , [["./", "generated-binary", "TESTS"], "generated-binary"] , [["./", "targets", "TESTS"], "targets"] , [["./", "user-errors", "TESTS"], "user-errors"] + , [["./", "built-in-rules", "TESTS"], "built-in-rules"] ] } } diff --git a/test/end-to-end/built-in-rules/TARGETS b/test/end-to-end/built-in-rules/TARGETS new file mode 100644 index 00000000..4991c5c3 --- /dev/null +++ b/test/end-to-end/built-in-rules/TARGETS @@ -0,0 +1,9 @@ +{ "generic_out_dirs": + { "type": ["@", "rules", "shell/test", "script"] + , "name": ["generic_out_dirs"] + , "test": ["generic_out_dirs.sh"] + , "deps": [["test/end-to-end", "tool-under-test"]] + } +, "TESTS": + {"type": "install", "tainted": ["test"], "deps": ["generic_out_dirs"]} +} diff --git a/test/end-to-end/built-in-rules/generic_out_dirs.sh b/test/end-to-end/built-in-rules/generic_out_dirs.sh new file mode 100755 index 00000000..8074bdab --- /dev/null +++ b/test/end-to-end/built-in-rules/generic_out_dirs.sh @@ -0,0 +1,71 @@ +#!/bin/bash + +set -e + +touch ROOT +mkdir -p lcl-build + +cat <<EOF > TARGETS +{ "gen_out_dirs": + {"type": "generic", "cmds": ["echo foo > out/foo.txt"], "out_dirs": ["out"]} +, "read_out_dirs": + { "type": "generic" + , "cmds": ["cat out/foo.txt > bar.txt"] + , "outs": ["bar.txt"] + , "deps": ["gen_out_dirs"] + } +, "missing_outs_and_out_dirs": + {"type": "generic", "cmds": ["echo foo > out/foo.txt"]} +, "out_dirs_contains_a_file": + {"type": "generic", "cmds": ["echo foo > foo.txt"], "out_dirs": ["foo.txt"]} +, "outs_contains_a_dir": + { "type": "generic" + , "cmds": ["mkdir out", "echo foo > out/foo.txt"] + , "outs": ["out"] + } +, "collision": + { "type": "generic" + , "cmds": ["touch foo"] + , "outs": ["foo"] + , "out_dirs": ["foo"] + } +} +EOF + +echo "read_out_dirs" >&2 +bin/tool-under-test build --local-build-root lcl-build read_out_dirs +echo "done" >&2 + +echo "missing_outs_and_out_dirs" >&2 +# at least one of "outs" or "out_dirs" must be declared. we expect to fail during the analysis phase +bin/tool-under-test analyse --log-limit 0 -f test.log missing_outs_and_out_dirs && exit 1 || : +grep 'outs' test.log && grep 'out_dirs' test.log +echo "done" >&2 + +echo "out_dirs_contains_a_file" >&2 +# the directories listed in "out_dirs" are created before the cmd is exectuted +# so, we expect the shell to complain that it cannot generate file "foo.txt" +# because it is a directory. We don't grep the shell error message because it can +# varay. +# the analysis phase should run fine +bin/tool-under-test analyse out_dirs_contains_a_file +echo "analysis ok" >&2 +bin/tool-under-test build --local-build-root lcl-build out_dirs_contains_a_file && exit 1 || : +echo "done" >&2 + +echo "outs_contains_a_dir" >&2 +# we declared an output file "out", which is actually a tree +# if we don't creat directory out, the shell will also complain about nonexisting directory "out" +# anlysis should run fine +bin/tool-under-test analyse outs_contains_a_dir +echo "analysis ok" >&2 +bin/tool-under-test build --local-build-root lcl-build -f test.log outs_contains_a_dir && exit 1 || : +# grep 'ERROR' test.log | grep 'output file' +echo "done" >&2 + +echo "collision" >&2 +# we expect an error during the analysis phase because outs and out_dirs must be disjoint +bin/tool-under-test analyse --log-limit 0 -f test.log collision && exit 1 || : +grep 'disjoint' test.log +echo "done" >&2 +exit |