diff options
author | Klaus Aehlig <klaus.aehlig@huawei.com> | 2024-08-01 10:26:13 +0200 |
---|---|---|
committer | Klaus Aehlig <klaus.aehlig@huawei.com> | 2024-08-01 19:00:47 +0200 |
commit | 03f948f4b10e916052a2234448b6658b80ee9143 (patch) | |
tree | 8eb0e2c26a31fd83fa0691b0d8fde55a15942a9d | |
parent | ed71beee3e3a2bbfcba24281ad9e28a0f6df4054 (diff) | |
download | justbuild-03f948f4b10e916052a2234448b6658b80ee9143.tar.gz |
Execution API: support cwd
... following the remote-execution standard that all output paths (but
none of the input paths) are relative to the working directory.
Therefore, the executor has to do the path translation. For our
implementation of the API interface
- the local API now handles cwd correctly,
- the remote API forwards cwd correctly, and
- the git API continues to report actions as not implemented.
17 files changed, 80 insertions, 23 deletions
diff --git a/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.cpp b/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.cpp index 09462bec..d4a096fb 100644 --- a/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.cpp +++ b/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.cpp @@ -210,6 +210,7 @@ template <class T> // [Action][build.bazel.remote.execution.v2.Action] for migration. // (https://github.com/bazelbuild/remote-apis/blob/e1fe21be4c9ae76269a5a63215bb3c72ed9ab3f0/build/bazel/remote/execution/v2/remote_execution.proto#L646) msg.set_allocated_platform(CreatePlatform(*request.properties).release()); + msg.set_working_directory(*request.cwd); std::copy(request.command_line->begin(), request.command_line->end(), pb::back_inserter(msg.mutable_arguments())); diff --git a/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.hpp b/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.hpp index c48e605d..0b006273 100644 --- a/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.hpp +++ b/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.hpp @@ -144,6 +144,9 @@ struct BazelMsgFactory::ActionDigestRequest final { /// \brief The command line. VectorPtr<std::string> const command_line; + /// \brief The workingg direcotry + gsl::not_null<std::string const*> const cwd; + /// \brief The paths of output files. VectorPtr<std::string> const output_files; diff --git a/src/buildtool/execution_api/common/execution_api.hpp b/src/buildtool/execution_api/common/execution_api.hpp index 2108c68d..c9ee276b 100644 --- a/src/buildtool/execution_api/common/execution_api.hpp +++ b/src/buildtool/execution_api/common/execution_api.hpp @@ -46,7 +46,8 @@ class IExecutionApi { /// \brief Create a new action. /// \param[in] root_digest Digest of the build root. /// \param[in] command Command as argv vector - /// \param[in] output_files List of paths to output files. + /// \param[in] cwd Working direcotry, relative to execution root + /// \param[in] output_files List of paths to output files, relative to cwd /// \param[in] output_dirs List of paths to output directories. /// \param[in] env_vars The environment variables to set. /// \param[in] properties Platform properties to set. @@ -54,6 +55,7 @@ class IExecutionApi { [[nodiscard]] virtual auto CreateAction( ArtifactDigest const& root_digest, std::vector<std::string> const& command, + std::string const& cwd, std::vector<std::string> const& output_files, std::vector<std::string> const& output_dirs, std::map<std::string, std::string> const& env_vars, diff --git a/src/buildtool/execution_api/execution_service/execution_server.cpp b/src/buildtool/execution_api/execution_service/execution_server.cpp index b77ad9d3..2e56bd04 100644 --- a/src/buildtool/execution_api/execution_service/execution_server.cpp +++ b/src/buildtool/execution_api/execution_service/execution_server.cpp @@ -136,6 +136,7 @@ auto ExecutionServiceImpl::GetIExecutionAction( auto i_execution_action = api_.CreateAction( ArtifactDigest{action.input_root_digest()}, {c->arguments().begin(), c->arguments().end()}, + c->working_directory(), {c->output_files().begin(), c->output_files().end()}, {c->output_directories().begin(), c->output_directories().end()}, env_vars, diff --git a/src/buildtool/execution_api/git/git_api.hpp b/src/buildtool/execution_api/git/git_api.hpp index 139291ef..43afc14a 100644 --- a/src/buildtool/execution_api/git/git_api.hpp +++ b/src/buildtool/execution_api/git/git_api.hpp @@ -42,6 +42,7 @@ class GitApi final : public IExecutionApi { [[nodiscard]] auto CreateAction( ArtifactDigest const& /*root_digest*/, std::vector<std::string> const& /*command*/, + std::string const& /*cwd*/, std::vector<std::string> const& /*output_files*/, std::vector<std::string> const& /*output_dirs*/, std::map<std::string, std::string> const& /*env_vars*/, diff --git a/src/buildtool/execution_api/local/local_action.cpp b/src/buildtool/execution_api/local/local_action.cpp index aac6bb39..8cc1d242 100644 --- a/src/buildtool/execution_api/local/local_action.cpp +++ b/src/buildtool/execution_api/local/local_action.cpp @@ -82,6 +82,7 @@ class BuildCleanupAnchor { auto LocalAction::Execute(Logger const* logger) noexcept -> IExecutionResponse::Ptr { + auto do_cache = CacheEnabled(cache_flag_); auto action = CreateActionDigest(root_digest_, not do_cache); if (not action) { @@ -174,7 +175,7 @@ auto LocalAction::Run(bazel_re::Digest const& action_id) const noexcept SystemCommand system{"LocalExecution"}; auto const exit_code = - system.Execute(cmdline, env_vars_, build_root, *exec_path); + system.Execute(cmdline, env_vars_, build_root / cwd_, *exec_path); if (exit_code.has_value()) { Output result{}; result.action.set_exit_code(*exit_code); @@ -187,7 +188,7 @@ auto LocalAction::Run(bazel_re::Digest const& action_id) const noexcept result.action.set_allocated_stderr_digest(digest_ptr); } - if (CollectAndStoreOutputs(&result.action, build_root)) { + if (CollectAndStoreOutputs(&result.action, build_root / cwd_)) { if (cache_flag_ == CacheFlag::CacheOutput) { if (not local_context_.storage->ActionCache().StoreResult( action_id, result.action)) { @@ -343,14 +344,15 @@ auto LocalAction::CreateDirectoryStructure( }; return std::all_of(output_files_.begin(), output_files_.end(), - [&exec_path, &create_dir](auto const& local_path) { - auto dir = (exec_path / local_path).parent_path(); + [this, &exec_path, &create_dir](auto const& local_path) { + auto dir = + (exec_path / cwd_ / local_path).parent_path(); return create_dir(dir); }) and std::all_of(output_dirs_.begin(), output_dirs_.end(), - [&exec_path, &create_dir](auto const& local_path) { - return create_dir(exec_path / local_path); + [this, &exec_path, &create_dir](auto const& local_path) { + return create_dir(exec_path / cwd_ / local_path); }); } diff --git a/src/buildtool/execution_api/local/local_action.hpp b/src/buildtool/execution_api/local/local_action.hpp index b2016ed7..4d256302 100644 --- a/src/buildtool/execution_api/local/local_action.hpp +++ b/src/buildtool/execution_api/local/local_action.hpp @@ -65,6 +65,7 @@ class LocalAction final : public IExecutionAction { LocalContext const& local_context_; ArtifactDigest const root_digest_{}; std::vector<std::string> const cmdline_{}; + std::string const cwd_{}; std::vector<std::string> output_files_{}; std::vector<std::string> output_dirs_{}; std::map<std::string, std::string> const env_vars_{}; @@ -76,6 +77,7 @@ class LocalAction final : public IExecutionAction { gsl::not_null<LocalContext const*> local_context, ArtifactDigest root_digest, std::vector<std::string> command, + std::string cwd, std::vector<std::string> output_files, std::vector<std::string> output_dirs, std::map<std::string, std::string> env_vars, @@ -83,6 +85,7 @@ class LocalAction final : public IExecutionAction { : local_context_{*local_context}, root_digest_{std::move(root_digest)}, cmdline_{std::move(command)}, + cwd_{std::move(cwd)}, output_files_{std::move(output_files)}, output_dirs_{std::move(output_dirs)}, env_vars_{std::move(env_vars)}, @@ -100,6 +103,7 @@ class LocalAction final : public IExecutionAction { BazelMsgFactory::ActionDigestRequest request{ .command_line = &cmdline_, + .cwd = &cwd_, .output_files = &output_files_, .output_dirs = &output_dirs_, .env_vars = &env_vars, diff --git a/src/buildtool/execution_api/local/local_api.hpp b/src/buildtool/execution_api/local/local_api.hpp index 3a40272e..eadf45b7 100644 --- a/src/buildtool/execution_api/local/local_api.hpp +++ b/src/buildtool/execution_api/local/local_api.hpp @@ -61,6 +61,7 @@ class LocalApi final : public IExecutionApi { [[nodiscard]] auto CreateAction( ArtifactDigest const& root_digest, std::vector<std::string> const& command, + std::string const& cwd, std::vector<std::string> const& output_files, std::vector<std::string> const& output_dirs, std::map<std::string, std::string> const& env_vars, @@ -69,6 +70,7 @@ class LocalApi final : public IExecutionApi { return IExecutionAction::Ptr{new LocalAction{&local_context_, root_digest, command, + cwd, output_files, output_dirs, env_vars, diff --git a/src/buildtool/execution_api/remote/bazel/bazel_action.cpp b/src/buildtool/execution_api/remote/bazel/bazel_action.cpp index 4190ea19..dcbddb8d 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_action.cpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_action.cpp @@ -26,6 +26,7 @@ BazelAction::BazelAction( std::shared_ptr<BazelNetwork> network, bazel_re::Digest root_digest, std::vector<std::string> command, + std::string cwd, std::vector<std::string> output_files, std::vector<std::string> output_dirs, std::map<std::string, std::string> const& env_vars, @@ -33,6 +34,7 @@ BazelAction::BazelAction( : network_{std::move(network)}, root_digest_{std::move(root_digest)}, cmdline_{std::move(command)}, + cwd_{std::move(cwd)}, output_files_{std::move(output_files)}, output_dirs_{std::move(output_dirs)}, env_vars_{BazelMsgFactory::CreateMessageVectorFromMap< @@ -122,6 +124,7 @@ auto BazelAction::CreateBundlesForAction(BazelBlobContainer* blobs, } BazelMsgFactory::ActionDigestRequest request{ .command_line = &cmdline_, + .cwd = &cwd_, .output_files = &output_files_, .output_dirs = &output_dirs_, .env_vars = &env_vars_, diff --git a/src/buildtool/execution_api/remote/bazel/bazel_action.hpp b/src/buildtool/execution_api/remote/bazel/bazel_action.hpp index 8316cec1..7bf62a45 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_action.hpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_action.hpp @@ -44,6 +44,7 @@ class BazelAction final : public IExecutionAction { std::shared_ptr<BazelNetwork> const network_; bazel_re::Digest const root_digest_; std::vector<std::string> const cmdline_; + std::string const cwd_; std::vector<std::string> output_files_; std::vector<std::string> output_dirs_; std::vector<bazel_re::Command_EnvironmentVariable> const env_vars_; @@ -55,6 +56,7 @@ class BazelAction final : public IExecutionAction { std::shared_ptr<BazelNetwork> network, bazel_re::Digest root_digest, std::vector<std::string> command, + std::string cwd, std::vector<std::string> output_files, std::vector<std::string> output_dirs, std::map<std::string, std::string> const& env_vars, diff --git a/src/buildtool/execution_api/remote/bazel/bazel_api.cpp b/src/buildtool/execution_api/remote/bazel/bazel_api.cpp index d3dc1b2f..2f54a377 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_api.cpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_api.cpp @@ -213,6 +213,7 @@ BazelApi::~BazelApi() = default; auto BazelApi::CreateAction( ArtifactDigest const& root_digest, std::vector<std::string> const& command, + std::string const& cwd, std::vector<std::string> const& output_files, std::vector<std::string> const& output_dirs, std::map<std::string, std::string> const& env_vars, @@ -221,6 +222,7 @@ auto BazelApi::CreateAction( return std::unique_ptr<BazelAction>{new BazelAction{network_, root_digest, command, + cwd, output_files, output_dirs, env_vars, diff --git a/src/buildtool/execution_api/remote/bazel/bazel_api.hpp b/src/buildtool/execution_api/remote/bazel/bazel_api.hpp index 5946f0f7..6b2a6f17 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_api.hpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_api.hpp @@ -58,6 +58,7 @@ class BazelApi final : public IExecutionApi { [[nodiscard]] auto CreateAction( ArtifactDigest const& root_digest, std::vector<std::string> const& command, + std::string const& cwd, std::vector<std::string> const& output_files, std::vector<std::string> const& output_dirs, std::map<std::string, std::string> const& env_vars, diff --git a/src/buildtool/execution_engine/executor/TARGETS b/src/buildtool/execution_engine/executor/TARGETS index bac83670..15ca2441 100644 --- a/src/buildtool/execution_engine/executor/TARGETS +++ b/src/buildtool/execution_engine/executor/TARGETS @@ -19,6 +19,7 @@ , ["src/buildtool/execution_api/remote", "bazel"] , ["src/buildtool/progress_reporting", "progress"] , ["src/utils/cpp", "hex_string"] + , ["src/utils/cpp", "path_rebase"] , ["src/utils/cpp", "prefix"] , ["@", "gsl", "", "gsl"] , ["src/buildtool/common", "common"] diff --git a/src/buildtool/execution_engine/executor/executor.hpp b/src/buildtool/execution_engine/executor/executor.hpp index 6541fe24..bf107e31 100644 --- a/src/buildtool/execution_engine/executor/executor.hpp +++ b/src/buildtool/execution_engine/executor/executor.hpp @@ -46,6 +46,7 @@ #include "src/buildtool/logging/logger.hpp" #include "src/buildtool/progress_reporting/progress.hpp" #include "src/utils/cpp/hex_string.hpp" +#include "src/utils/cpp/path_rebase.hpp" #include "src/utils/cpp/prefix.hpp" /// \brief Implementations for executing actions and uploading artifacts. @@ -125,11 +126,17 @@ class ExecutorImpl { } } + auto base = action->Content().Cwd(); + auto cwd_relative_output_files = + RebasePathStringsRelativeTo(base, action->OutputFilePaths()); + auto cwd_relative_output_dirs = + RebasePathStringsRelativeTo(base, action->OutputDirPaths()); auto remote_action = (alternative_api ? *alternative_api : api) .CreateAction(*root_digest, action->Command(), - action->OutputFilePaths(), - action->OutputDirPaths(), + base, + cwd_relative_output_files, + cwd_relative_output_dirs, action->Env(), merged_properties); @@ -477,11 +484,16 @@ class ExecutorImpl { IExecutionResponse::ArtifactInfos const& artifacts, gsl::not_null<DependencyGraph::ActionNode const*> const& action, bool fail_artifacts) noexcept { + auto base = action->Content().Cwd(); for (auto const& [name, node] : action->OutputFiles()) { - node->Content().SetObjectInfo(artifacts.at(name), fail_artifacts); + node->Content().SetObjectInfo( + artifacts.at(RebasePathStringRelativeTo(base, name)), + fail_artifacts); } for (auto const& [name, node] : action->OutputDirs()) { - node->Content().SetObjectInfo(artifacts.at(name), fail_artifacts); + node->Content().SetObjectInfo( + artifacts.at(RebasePathStringRelativeTo(base, name)), + fail_artifacts); } } @@ -507,11 +519,14 @@ class ExecutorImpl { /// are present in the artifacts map [[nodiscard]] static auto CheckOutputsExist( IExecutionResponse::ArtifactInfos const& artifacts, - std::vector<Action::LocalPath> const& outputs) noexcept -> bool { - return std::all_of( - outputs.begin(), outputs.end(), [&artifacts](auto const& output) { - return artifacts.contains(output); - }); + std::vector<Action::LocalPath> const& outputs, + std::string base) noexcept -> bool { + return std::all_of(outputs.begin(), + outputs.end(), + [&artifacts, &base](auto const& output) { + return artifacts.contains( + RebasePathStringRelativeTo(base, output)); + }); } /// \brief Parse response and write object info to DAG's artifact nodes. @@ -566,8 +581,10 @@ class ExecutorImpl { auto output_dirs = action->OutputDirPaths(); if (artifacts.empty() or - not CheckOutputsExist(artifacts, output_files) or - not CheckOutputsExist(artifacts, output_dirs)) { + not CheckOutputsExist( + artifacts, output_files, action->Content().Cwd()) or + not CheckOutputsExist( + artifacts, output_dirs, action->Content().Cwd())) { logger.Emit(LogLevel::Error, [&] { std::string message{ "action executed with missing outputs.\n" diff --git a/test/buildtool/execution_api/common/api_test.hpp b/test/buildtool/execution_api/common/api_test.hpp index 6d8e7928..54394b62 100644 --- a/test/buildtool/execution_api/common/api_test.hpp +++ b/test/buildtool/execution_api/common/api_test.hpp @@ -70,8 +70,13 @@ using ExecProps = std::map<std::string, std::string>; auto api = api_factory(); - auto action = api->CreateAction( - *api->UploadTree({}), {"echo", "-n", test_content}, {}, {}, {}, props); + auto action = api->CreateAction(*api->UploadTree({}), + {"echo", "-n", test_content}, + "", + {}, + {}, + {}, + props); SECTION("Cache execution result in action cache") { action->SetCacheFlag(IExecutionAction::CacheFlag::CacheOutput); @@ -147,6 +152,7 @@ using ExecProps = std::map<std::string, std::string>; {"/bin/sh", "-c", "set -e\necho -n " + test_content + " > " + output_path}, + "", {output_path}, {}, {}, @@ -238,6 +244,7 @@ using ExecProps = std::map<std::string, std::string>; auto action = api->CreateAction(*api->UploadTree({{input_path, &input_artifact}}), {"cp", input_path, output_path}, + "", {output_path}, {}, {}, @@ -320,6 +327,7 @@ using ExecProps = std::map<std::string, std::string>; "-c", "set -e\necho -n " + test_content + " > " + output_path + "\nexit 1\n"}, + "", {output_path}, {}, {}, @@ -412,6 +420,7 @@ using ExecProps = std::map<std::string, std::string>; auto action = api->CreateAction(*api->UploadTree({}), {"/bin/sh", "-c", make_cmd("root")}, + "", {}, {"root"}, env, @@ -484,6 +493,7 @@ TestRetrieveFileAndSymlinkWithSameContentToPath(ApiFactory const& api_factory, auto action = api->CreateAction(*api->UploadTree({}), {"/bin/sh", "-c", make_cmd("root")}, + "", {}, {"root"}, env, @@ -552,6 +562,7 @@ TestRetrieveFileAndSymlinkWithSameContentToPath(ApiFactory const& api_factory, auto action = api->CreateAction(*api->UploadTree({}), {"/bin/sh", "-c", cmd}, + "", {foo_path.string(), link_path.string()}, {bar_path.parent_path().string()}, env, @@ -619,6 +630,7 @@ TestRetrieveFileAndSymlinkWithSameContentToPath(ApiFactory const& api_factory, {"/bin/sh", "-c", fmt::format("set -e\n [ -d {} ]", output_path.string())}, + "", {}, {output_path}, {}, diff --git a/test/buildtool/execution_api/local/local_execution.test.cpp b/test/buildtool/execution_api/local/local_execution.test.cpp index a08f3b87..49141076 100644 --- a/test/buildtool/execution_api/local/local_execution.test.cpp +++ b/test/buildtool/execution_api/local/local_execution.test.cpp @@ -79,7 +79,7 @@ TEST_CASE("LocalExecution: No input, no output", "[execution_api]") { std::string test_content("test"); std::vector<std::string> const cmdline = {"echo", "-n", test_content}; auto action = - api.CreateAction(*api.UploadTree({}), cmdline, {}, {}, {}, {}); + api.CreateAction(*api.UploadTree({}), cmdline, "", {}, {}, {}, {}); REQUIRE(action); SECTION("Cache execution result in action cache") { @@ -134,6 +134,7 @@ TEST_CASE("LocalExecution: No input, no output, env variables used", "/bin/sh", "-c", "set -e\necho -n ${MYCONTENT}"}; auto action = api.CreateAction(*api.UploadTree({}), cmdline, + "", {}, {}, {{"MYCONTENT", test_content}}, @@ -198,7 +199,7 @@ TEST_CASE("LocalExecution: No input, create output", "[execution_api]") { "set -e\necho -n " + test_content + " > " + output_path}; auto action = api.CreateAction( - *api.UploadTree({}), cmdline, {output_path}, {}, {}, {}); + *api.UploadTree({}), cmdline, "", {output_path}, {}, {}, {}); REQUIRE(action); SECTION("Cache execution result in action cache") { @@ -273,6 +274,7 @@ TEST_CASE("LocalExecution: One input copied to output", "[execution_api]") { auto action = api.CreateAction(*api.UploadTree({{input_path, &local_artifact}}), cmdline, + "", {output_path}, {}, {}, @@ -335,7 +337,7 @@ TEST_CASE("LocalExecution: Cache failed action's result", "[execution_api]") { "sh", "-c", fmt::format("[ -f '{}' ]", flag.string())}; auto action = - api.CreateAction(*api.UploadTree({}), cmdline, {}, {}, {}, {}); + api.CreateAction(*api.UploadTree({}), cmdline, "", {}, {}, {}, {}); REQUIRE(action); action->SetCacheFlag(IExecutionAction::CacheFlag::CacheOutput); diff --git a/test/buildtool/execution_engine/executor/executor.test.cpp b/test/buildtool/execution_engine/executor/executor.test.cpp index 1148d0ff..6a32a497 100644 --- a/test/buildtool/execution_engine/executor/executor.test.cpp +++ b/test/buildtool/execution_engine/executor/executor.test.cpp @@ -146,6 +146,7 @@ class TestApi : public IExecutionApi { [[nodiscard]] auto CreateAction( ArtifactDigest const& /*unused*/, std::vector<std::string> const& /*unused*/, + std::string const& /*unused*/, std::vector<std::string> const& /*unused*/, std::vector<std::string> const& /*unused*/, std::map<std::string, std::string> const& /*unused*/, |