Execution response: Add output symlink paths

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_{};