diff options
6 files changed, 198 insertions, 39 deletions
diff --git a/src/buildtool/execution_api/common/execution_response.hpp b/src/buildtool/execution_api/common/execution_response.hpp index a08134dd..3ad4e2ba 100644 --- a/src/buildtool/execution_api/common/execution_response.hpp +++ b/src/buildtool/execution_api/common/execution_response.hpp @@ -30,6 +30,8 @@ class IExecutionResponse { public: using Ptr = std::unique_ptr<IExecutionResponse>; using ArtifactInfos = std::unordered_map<std::string, Artifact::ObjectInfo>; + // set of paths found in output_directory_symlinks list of the action result + using DirSymlinks = std::unordered_set<std::string>; enum class StatusCode { Failed, Success }; @@ -56,7 +58,11 @@ class IExecutionResponse { [[nodiscard]] virtual auto ActionDigest() const noexcept -> std::string = 0; - [[nodiscard]] virtual auto Artifacts() const noexcept -> ArtifactInfos = 0; + [[nodiscard]] virtual auto Artifacts() noexcept -> ArtifactInfos = 0; + + // Artifacts plus extra useful info + [[nodiscard]] virtual auto ArtifactsWithDirSymlinks() noexcept + -> std::pair<ArtifactInfos, DirSymlinks> = 0; }; #endif // INCLUDED_SRC_BUILDTOOL_EXECUTION_API_COMMON_REMOTE_EXECUTION_RESPONSE_HPP diff --git a/src/buildtool/execution_api/execution_service/execution_server.cpp b/src/buildtool/execution_api/execution_service/execution_server.cpp index c34201a1..f2436684 100644 --- a/src/buildtool/execution_api/execution_service/execution_server.cpp +++ b/src/buildtool/execution_api/execution_service/execution_server.cpp @@ -268,11 +268,15 @@ static auto CreateTreeDigestFromDirectoryDigest( static auto AddOutputPaths(::bazel_re::ExecuteResponse* response, IExecutionResponse::Ptr const& execution, Storage const& storage) noexcept -> bool { - auto const& size = static_cast<int>(execution->Artifacts().size()); + auto const& artifacts_plus = execution->ArtifactsWithDirSymlinks(); + auto const& size = static_cast<int>(artifacts_plus.first.size()); response->mutable_result()->mutable_output_files()->Reserve(size); + response->mutable_result()->mutable_output_file_symlinks()->Reserve(size); + response->mutable_result()->mutable_output_directory_symlinks()->Reserve( + size); response->mutable_result()->mutable_output_directories()->Reserve(size); - for (auto const& [path, info] : execution->Artifacts()) { + for (auto const& [path, info] : artifacts_plus.first) { auto dgst = static_cast<::bazel_re::Digest>(info.digest); if (info.type == ObjectType::Tree) { @@ -295,6 +299,32 @@ static auto AddOutputPaths(::bazel_re::ExecuteResponse* response, response->mutable_result()->mutable_output_directories()->Add( std::move(out_dir)); } + else if (info.type == ObjectType::Symlink) { + ::bazel_re::OutputSymlink out_link; + *(out_link.mutable_path()) = path; + // recover the target of the symlink + auto cas_path = + storage.CAS().BlobPath(dgst, /*is_executable=*/false); + if (not cas_path) { + return false; + } + auto const& content = FileSystemManager::ReadFile(*cas_path); + if (not content) { + return false; + } + *(out_link.mutable_target()) = *content; + if (artifacts_plus.second.contains(path)) { + // directory symlink + response->mutable_result() + ->mutable_output_directory_symlinks() + ->Add(std::move(out_link)); + } + else { + // file symlinks + response->mutable_result()->mutable_output_file_symlinks()->Add( + std::move(out_link)); + } + } else { ::bazel_re::OutputFile out_file; *(out_file.mutable_path()) = path; diff --git a/src/buildtool/execution_api/local/local_response.hpp b/src/buildtool/execution_api/local/local_response.hpp index 7faa5619..135897d8 100644 --- a/src/buildtool/execution_api/local/local_response.hpp +++ b/src/buildtool/execution_api/local/local_response.hpp @@ -63,14 +63,55 @@ class LocalResponse final : public IExecutionResponse { return action_id_; } - auto Artifacts() const noexcept -> ArtifactInfos final { + auto Artifacts() noexcept -> ArtifactInfos final { + return ArtifactsWithDirSymlinks().first; + } + + auto ArtifactsWithDirSymlinks() noexcept + -> std::pair<ArtifactInfos, DirSymlinks> final { + // make sure to populate only once + populated_ = Populate(); + if (not populated_) { + return {}; + } + return std::pair(artifacts_, dir_symlinks_); + }; + + private: + std::string action_id_{}; + LocalAction::Output output_{}; + gsl::not_null<Storage const*> storage_; + ArtifactInfos artifacts_{}; + DirSymlinks dir_symlinks_{}; + bool populated_{false}; + + explicit LocalResponse( + std::string action_id, + LocalAction::Output output, + gsl::not_null<Storage const*> const& storage) noexcept + : action_id_{std::move(action_id)}, + output_{std::move(output)}, + storage_{storage} {} + + [[nodiscard]] auto Populate() noexcept -> bool { + if (populated_) { + return true; + } ArtifactInfos artifacts{}; auto const& action_result = output_.action; artifacts.reserve( - static_cast<std::size_t>(action_result.output_files().size()) + + static_cast<std::size_t>(action_result.output_files_size()) + + static_cast<std::size_t>( + action_result.output_file_symlinks_size()) + + static_cast<std::size_t>( + action_result.output_directory_symlinks_size()) + static_cast<std::size_t>( action_result.output_directories().size())); + DirSymlinks dir_symlinks{}; + dir_symlinks_.reserve(static_cast<std::size_t>( + action_result.output_directory_symlinks_size())); + // collect files and store them for (auto const& file : action_result.output_files()) { try { @@ -81,7 +122,34 @@ class LocalResponse final : public IExecutionResponse { .type = file.is_executable() ? ObjectType::Executable : ObjectType::File}); } catch (...) { - return {}; + return false; + } + } + + // collect all symlinks and store them + for (auto const& link : action_result.output_file_symlinks()) { + try { + artifacts.emplace( + link.path(), + Artifact::ObjectInfo{ + .digest = ArtifactDigest::Create<ObjectType::File>( + link.target()), + .type = ObjectType::Symlink}); + } catch (...) { + return false; + } + } + for (auto const& link : action_result.output_directory_symlinks()) { + try { + artifacts.emplace( + link.path(), + Artifact::ObjectInfo{ + .digest = ArtifactDigest::Create<ObjectType::File>( + link.target()), + .type = ObjectType::Symlink}); + dir_symlinks.emplace(link.path()); // add it to set + } catch (...) { + return false; } } @@ -94,25 +162,13 @@ class LocalResponse final : public IExecutionResponse { .digest = ArtifactDigest{dir.tree_digest()}, .type = ObjectType::Tree}); } catch (...) { - return {}; + return false; } } - - return artifacts; - }; - - private: - std::string action_id_{}; - LocalAction::Output output_{}; - gsl::not_null<Storage const*> storage_; - - explicit LocalResponse( - std::string action_id, - LocalAction::Output output, - gsl::not_null<Storage const*> const& storage) noexcept - : action_id_{std::move(action_id)}, - output_{std::move(output)}, - storage_{storage} {} + artifacts_ = std::move(artifacts); + dir_symlinks_ = std::move(dir_symlinks); + return true; + } }; #endif // INCLUDED_SRC_BUILDTOOL_EXECUTION_API_LOCAL_LOCAL_RESPONSE_HPP diff --git a/src/buildtool/execution_api/remote/bazel/bazel_response.cpp b/src/buildtool/execution_api/remote/bazel/bazel_response.cpp index 9889d8ed..05ff0c9a 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_response.cpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_response.cpp @@ -41,12 +41,36 @@ auto BazelResponse::ReadStringBlob(bazel_re::Digest const& id) noexcept return blobs[0].data; } -auto BazelResponse::Artifacts() const noexcept -> ArtifactInfos { +auto BazelResponse::Artifacts() noexcept -> ArtifactInfos { + return ArtifactsWithDirSymlinks().first; +} + +auto BazelResponse::ArtifactsWithDirSymlinks() noexcept + -> std::pair<ArtifactInfos, DirSymlinks> { + // make sure to populate only once + populated_ = Populate(); + if (not populated_) { + return {}; + } + return std::pair(artifacts_, dir_symlinks_); +} + +auto BazelResponse::Populate() noexcept -> bool { + if (populated_) { + return true; + } ArtifactInfos artifacts{}; auto const& action_result = output_.action_result; artifacts.reserve( - static_cast<std::size_t>(action_result.output_files().size()) + - static_cast<std::size_t>(action_result.output_directories().size())); + static_cast<std::size_t>(action_result.output_files_size()) + + static_cast<std::size_t>(action_result.output_file_symlinks_size()) + + static_cast<std::size_t>( + action_result.output_directory_symlinks_size()) + + static_cast<std::size_t>(action_result.output_directories_size())); + + DirSymlinks dir_symlinks{}; + dir_symlinks.reserve(static_cast<std::size_t>( + action_result.output_directory_symlinks_size())); // collect files and store them for (auto const& file : action_result.output_files()) { @@ -58,7 +82,34 @@ auto BazelResponse::Artifacts() const noexcept -> ArtifactInfos { ? ObjectType::Executable : ObjectType::File}); } catch (...) { - return {}; + return false; + } + } + + // collect all symlinks and store them + for (auto const& link : action_result.output_file_symlinks()) { + try { + artifacts.emplace( + link.path(), + Artifact::ObjectInfo{ + .digest = + ArtifactDigest::Create<ObjectType::File>(link.target()), + .type = ObjectType::Symlink}); + } catch (...) { + return false; + } + } + for (auto const& link : action_result.output_directory_symlinks()) { + try { + artifacts.emplace( + link.path(), + Artifact::ObjectInfo{ + .digest = + ArtifactDigest::Create<ObjectType::File>(link.target()), + .type = ObjectType::Symlink}); + dir_symlinks.emplace(link.path()); // add it to set + } catch (...) { + return false; } } @@ -73,10 +124,12 @@ auto BazelResponse::Artifacts() const noexcept -> ArtifactInfos { .digest = ArtifactDigest{tree.tree_digest()}, .type = ObjectType::Tree}); } catch (...) { - return {}; + return false; } } - return artifacts; + artifacts_ = std::move(artifacts); + dir_symlinks_ = std::move(dir_symlinks); + return true; } // obtain tree digests for output directories @@ -98,29 +151,31 @@ auto BazelResponse::Artifacts() const noexcept -> ArtifactInfos { auto tree = BazelMsgFactory::MessageFromString<bazel_re::Tree>( tree_blob.data); if (not tree) { - return {}; + return false; } - // The server does not store the Directory messages it just has - // sent us as part of the Tree message. If we want to be able to - // use the Directories as inputs for actions, we have to upload - // them manually. + // The server does not store the Directory messages it just + // has sent us as part of the Tree message. If we want to be + // able to use the Directories as inputs for actions, we + // have to upload them manually. auto root_digest = UploadTreeMessageDirectories(*tree); if (not root_digest) { - return {}; + return false; } artifacts.emplace( action_result.output_directories(pos).path(), Artifact::ObjectInfo{.digest = *root_digest, .type = ObjectType::Tree}); } catch (...) { - return {}; + return false; } ++pos; } tree_blobs = blob_reader.Next(); } - return artifacts; + artifacts_ = std::move(artifacts); + dir_symlinks_ = std::move(dir_symlinks); + return true; } auto BazelResponse::UploadTreeMessageDirectories( diff --git a/src/buildtool/execution_api/remote/bazel/bazel_response.hpp b/src/buildtool/execution_api/remote/bazel/bazel_response.hpp index 048c6291..358c98c8 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_response.hpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_response.hpp @@ -56,12 +56,18 @@ class BazelResponse final : public IExecutionResponse { return action_id_; } - auto Artifacts() const noexcept -> ArtifactInfos final; + auto Artifacts() noexcept -> ArtifactInfos final; + + auto ArtifactsWithDirSymlinks() noexcept + -> std::pair<ArtifactInfos, DirSymlinks> final; private: std::string action_id_{}; std::shared_ptr<BazelNetwork> const network_{}; BazelExecutionClient::ExecutionOutput output_{}; + ArtifactInfos artifacts_{}; + DirSymlinks dir_symlinks_{}; + bool populated_{false}; BazelResponse(std::string action_id, std::shared_ptr<BazelNetwork> network, @@ -78,6 +84,8 @@ class BazelResponse final : public IExecutionResponse { return id.size_bytes() != 0; } + [[nodiscard]] auto Populate() noexcept -> bool; + [[nodiscard]] auto UploadTreeMessageDirectories( bazel_re::Tree const& tree) const -> std::optional<ArtifactDigest>; }; diff --git a/test/buildtool/execution_engine/executor/executor.test.cpp b/test/buildtool/execution_engine/executor/executor.test.cpp index 1f0312f7..685e4d47 100644 --- a/test/buildtool/execution_engine/executor/executor.test.cpp +++ b/test/buildtool/execution_engine/executor/executor.test.cpp @@ -70,7 +70,7 @@ class TestResponse : public IExecutionResponse { [[nodiscard]] auto ActionDigest() const noexcept -> std::string final { return {}; } - [[nodiscard]] auto Artifacts() const noexcept -> ArtifactInfos final { + [[nodiscard]] auto Artifacts() noexcept -> ArtifactInfos final { ArtifactInfos artifacts{}; artifacts.reserve(config_.execution.outputs.size()); @@ -89,6 +89,10 @@ class TestResponse : public IExecutionResponse { return artifacts; } + [[nodiscard]] auto ArtifactsWithDirSymlinks() noexcept + -> std::pair<ArtifactInfos, DirSymlinks> final { + return {}; + } private: TestApiConfig config_{}; |