diff options
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*/, |