summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKlaus Aehlig <klaus.aehlig@huawei.com>2024-08-02 12:00:44 +0200
committerKlaus Aehlig <klaus.aehlig@huawei.com>2024-08-02 14:09:02 +0200
commitb2f795e0e05743a234587f3a1c32c9c1d4ec524f (patch)
treec5eecfc0d37fd9e581cc5dccb3c93a8482cb0e72
parent217da1d00c8fd8351d79587f7394e1ca8f498cff (diff)
downloadjustbuild-b2f795e0e05743a234587f3a1c32c9c1d4ec524f.tar.gz
Actions with non-trivial cwd: add empty input tree, if required
While our local action execution implicitly creates the specified cwd with the first output file or directory, this behaviour is not mandated by the remote-execution protocol. There, an action definition has to ensure that cwd is a directory implied by the input files. Achieve this, by adding an empty input directory at cwd if this can be done without creating tree conflicts.
-rw-r--r--src/buildtool/build_engine/target_map/built_in_rules.cpp5
-rw-r--r--src/buildtool/build_engine/target_map/target_map.cpp7
-rw-r--r--src/buildtool/build_engine/target_map/utils.cpp41
-rw-r--r--src/buildtool/build_engine/target_map/utils.hpp4
4 files changed, 54 insertions, 3 deletions
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 e3cfc028..9a7bb125 100644
--- a/src/buildtool/build_engine/target_map/built_in_rules.cpp
+++ b/src/buildtool/build_engine/target_map/built_in_rules.cpp
@@ -1354,6 +1354,9 @@ void GenericRuleWithDeps(
for (auto const& dep : dependency_values) {
inputs = ExpressionPtr{Expression::map_t{inputs, (*dep)->Artifacts()}};
}
+ std::vector<Tree::Ptr> trees{};
+ inputs = BuildMaps::Target::Utils::add_dir_for(
+ cwd_value->String(), inputs, &trees);
std::vector<std::string> argv{};
argv.reserve(sh_val->List().size() + 1);
@@ -1393,7 +1396,7 @@ void GenericRuleWithDeps(
.runfiles = empty_map},
std::vector<ActionDescription::Ptr>{action},
std::vector<std::string>{},
- std::vector<Tree::Ptr>{},
+ std::move(trees),
std::move(effective_vars),
std::move(tainted),
std::move(implied_export),
diff --git a/src/buildtool/build_engine/target_map/target_map.cpp b/src/buildtool/build_engine/target_map/target_map.cpp
index 14d897b9..c7002270 100644
--- a/src/buildtool/build_engine/target_map/target_map.cpp
+++ b/src/buildtool/build_engine/target_map/target_map.cpp
@@ -513,7 +513,8 @@ void withDependencies(
return eval(expr->Get("default", empty_list), env);
}},
{"ACTION",
- [&actions, &rule](auto&& eval, auto const& expr, auto const& env) {
+ [&actions, &rule, &trees](
+ auto&& eval, auto const& expr, auto const& env) {
auto const& empty_map_exp = Expression::kEmptyMapExpr;
auto inputs_exp = eval(expr->Get("inputs", empty_map_exp), env);
if (not inputs_exp->IsMap()) {
@@ -605,6 +606,8 @@ void withDependencies(
"cwd has to be a non-upwards relative path, but found {}",
cwd_exp->ToString())};
}
+ auto final_inputs = BuildMaps::Target::Utils::add_dir_for(
+ cwd_exp->String(), inputs_exp, &trees);
auto env_exp = eval(expr->Get("env", empty_map_exp), env);
if (not env_exp->IsMap()) {
throw Evaluator::EvaluationError{
@@ -716,7 +719,7 @@ void withDependencies(
timeout_scale_exp->IsNumber() ? timeout_scale_exp->Number()
: 1.0,
execution_properties,
- inputs_exp);
+ final_inputs);
auto action_id = action->Id();
actions.emplace_back(std::move(action));
for (auto const& out : outputs) {
diff --git a/src/buildtool/build_engine/target_map/utils.cpp b/src/buildtool/build_engine/target_map/utils.cpp
index 71defc4c..c4f19248 100644
--- a/src/buildtool/build_engine/target_map/utils.cpp
+++ b/src/buildtool/build_engine/target_map/utils.cpp
@@ -149,6 +149,47 @@ auto BuildMaps::Target::Utils::tree_conflict(const ExpressionPtr& map)
return std::nullopt;
}
+auto BuildMaps::Target::Utils::add_dir_for(
+ const std::string& cwd,
+ ExpressionPtr stage,
+ gsl::not_null<std::vector<Tree::Ptr>*> trees) -> ExpressionPtr {
+ // if working top-level, there is nothing to add; this is also
+ // the common case
+ if ((cwd.empty()) or (cwd == ".")) {
+ return stage;
+ }
+ auto cwd_path = std::filesystem::path{cwd};
+ for (auto const& [path, artifact] : stage->Map()) {
+ if ((path.empty()) or (path == ".")) {
+ // top-level artifact (tree); cannot add tree for cwd
+ return stage;
+ }
+ auto p = std::filesystem::path{path};
+ for (auto c = cwd_path; not c.empty(); c = c.parent_path()) {
+ if (c == p) {
+ // adding cwd would add a tree conflict; so nothing to add
+ return stage;
+ }
+ }
+ for (; not p.empty(); p = p.parent_path()) {
+ if (p == cwd_path) {
+ // adding cwd would add a tree conflict; so nothing to add
+ return stage;
+ }
+ }
+ }
+ // As we can add cwd without tree conflicts, we have to in order to
+ // ensure that installing this stage implies a directory at cwd.
+ std::unordered_map<std::string, ArtifactDescription> artifacts{};
+ auto empty_tree = std::make_shared<Tree>(std::move(artifacts));
+ auto empty_tree_id = empty_tree->Id();
+ trees->emplace_back(std::move(empty_tree));
+ auto empty_tree_exp =
+ ExpressionPtr{ArtifactDescription::CreateTree(empty_tree_id)};
+ auto cwd_tree = ExpressionPtr{Expression::map_t{cwd, empty_tree_exp}};
+ return ExpressionPtr{Expression::map_t{stage, cwd_tree}};
+}
+
auto BuildMaps::Target::Utils::getTainted(
std::set<std::string>* tainted,
const Configuration& config,
diff --git a/src/buildtool/build_engine/target_map/utils.hpp b/src/buildtool/build_engine/target_map/utils.hpp
index d7722efb..8c469bbe 100644
--- a/src/buildtool/build_engine/target_map/utils.hpp
+++ b/src/buildtool/build_engine/target_map/utils.hpp
@@ -56,6 +56,10 @@ auto artifacts_tree(const ExpressionPtr& map)
auto tree_conflict(const ExpressionPtr& /* map */)
-> std::optional<std::string>;
+auto add_dir_for(const std::string& cwd,
+ ExpressionPtr stage,
+ gsl::not_null<std::vector<Tree::Ptr>*> trees) -> ExpressionPtr;
+
auto getTainted(std::set<std::string>* tainted,
const Configuration& config,
const ExpressionPtr& tainted_exp,