serve service: Use digest when requesting serve to set up a tree

...instead of passing just the Git hash, which imposes the remote
to always be native.

The serve service proto file is updated accordingly.

8 files changed, 72 insertions, 51 deletions

diff --git a/src/buildtool/serve_api/remote/serve_api.hpp b/src/buildtool/serve_api/remote/serve_api.hpp
index 64e39092..3a9443fa 100644
--- a/src/buildtool/serve_api/remote/serve_api.hpp
+++ b/src/buildtool/serve_api/remote/serve_api.hpp
@@ -120,8 +120,8 @@ class ServeApi final {
     }
 
     [[nodiscard]] auto GetTreeFromRemote(
-        std::string const& tree_id) const noexcept -> bool {
-        return stc_.GetRemoteTree(tree_id);
+        ArtifactDigest const& digest) const noexcept -> bool {
+        return stc_.GetRemoteTree(digest);
     }
 
     [[nodiscard]] auto ServeTargetVariables(std::string const& target_root_id,
diff --git a/src/buildtool/serve_api/remote/source_tree_client.cpp b/src/buildtool/serve_api/remote/source_tree_client.cpp
index 5f0bce80..d0053d47 100644
--- a/src/buildtool/serve_api/remote/source_tree_client.cpp
+++ b/src/buildtool/serve_api/remote/source_tree_client.cpp
@@ -330,10 +330,10 @@ auto SourceTreeClient::CheckRootTree(std::string const& tree_id) const noexcept
     return true;  // tree found
 }
 
-auto SourceTreeClient::GetRemoteTree(std::string const& tree_id) const noexcept
-    -> bool {
+auto SourceTreeClient::GetRemoteTree(
+    ArtifactDigest const& digest) const noexcept -> bool {
     justbuild::just_serve::GetRemoteTreeRequest request{};
-    request.set_tree(tree_id);
+    (*request.mutable_digest()) = ArtifactDigestFactory::ToBazel(digest);
 
     grpc::ClientContext context;
     justbuild::just_serve::GetRemoteTreeResponse response;
diff --git a/src/buildtool/serve_api/remote/source_tree_client.hpp b/src/buildtool/serve_api/remote/source_tree_client.hpp
index 15638afc..32fb910c 100644
--- a/src/buildtool/serve_api/remote/source_tree_client.hpp
+++ b/src/buildtool/serve_api/remote/source_tree_client.hpp
@@ -127,11 +127,11 @@ class SourceTreeClient {
 
     /// \brief Retrieve tree from the CAS of the associated remote-execution
     /// endpoint and makes it available for a serve-orchestrated build.
-    /// \param[in] tree_id Identifier of the Git tree to retrieve.
+    /// \param[in] digest Tree to retrieve.
     /// \returns Flag to state whether tree was successfully imported into the
     /// local Git storage or not.
-    [[nodiscard]] auto GetRemoteTree(std::string const& tree_id) const noexcept
-        -> bool;
+    [[nodiscard]] auto GetRemoteTree(
+        ArtifactDigest const& digest) const noexcept -> bool;
 
   private:
     HashFunction const& hash_function_;  // hash function of the remote
diff --git a/src/buildtool/serve_api/serve_service/just_serve.proto b/src/buildtool/serve_api/serve_service/just_serve.proto
index fa0362fe..5e52ce03 100644
--- a/src/buildtool/serve_api/serve_service/just_serve.proto
+++ b/src/buildtool/serve_api/serve_service/just_serve.proto
@@ -286,8 +286,11 @@ message CheckRootTreeResponse {
 // A request message for
 // [SourceTree.GetRemoteTree][justbuild.just_serve.SourceTree.GetRemoteTree].
 message GetRemoteTreeRequest {
-  // The Git tree identifier.
-  string tree = 1;
+  reserved 1; // The Git tree identifier in an earlier version of the API.
+
+  // The tree digest, which can be used to retrieve it from the associated
+  // remote-execution endpoint.
+  build.bazel.remote.execution.v2.Digest digest = 2;
 }
 
 // A response message for
@@ -347,9 +350,9 @@ service SourceTree {
   // There are no method-specific errors.
   rpc CheckRootTree(CheckRootTreeRequest) returns (CheckRootTreeResponse) {}
 
-  // Retrieve a given Git-tree from the CAS of the associated
-  // remote-execution endpoint and make it available in a location where this
-  // serve instance can build against.
+  // Retrieve a given tree from the CAS of the associated remote-execution
+  // endpoint and make it available in a location where this serve instance
+  // can build against.
   //
   // There are no method-specific errors.
   rpc GetRemoteTree(GetRemoteTreeRequest) returns (GetRemoteTreeResponse) {}
diff --git a/src/buildtool/serve_api/serve_service/source_tree.cpp b/src/buildtool/serve_api/serve_service/source_tree.cpp
index 0e629d8d..3478a377 100644
--- a/src/buildtool/serve_api/serve_service/source_tree.cpp
+++ b/src/buildtool/serve_api/serve_service/source_tree.cpp
@@ -1665,7 +1665,6 @@ auto SourceTreeService::GetRemoteTree(
     ::grpc::ServerContext* /* context */,
     const ::justbuild::just_serve::GetRemoteTreeRequest* request,
     GetRemoteTreeResponse* response) -> ::grpc::Status {
-    auto const& tree_id{request->tree()};
     // acquire locks
     auto lock = GarbageCollector::SharedLock(*native_context_->storage_config);
     if (not lock) {
@@ -1675,15 +1674,12 @@ auto SourceTreeService::GetRemoteTree(
     }
 
     // get tree from remote CAS into tmp dir
-    auto const digest = ArtifactDigestFactory::Create(
-        native_context_->storage_config->hash_function.GetType(),
-        tree_id,
-        0,
-        /*is_tree=*/true);
-    if (not digest or not apis_.remote->IsAvailable(*digest)) {
+    auto const remote_digest = ArtifactDigestFactory::FromBazel(
+        apis_.hash_function.GetType(), request->digest());
+    if (not remote_digest or not apis_.remote->IsAvailable(*remote_digest)) {
         logger_->Emit(LogLevel::Error,
                       "Remote CAS does not contain expected tree {}",
-                      tree_id);
+                      remote_digest->hash());
         response->set_status(GetRemoteTreeResponse::FAILED_PRECONDITION);
         return ::grpc::Status::OK;
     }
@@ -1693,42 +1689,36 @@ auto SourceTreeService::GetRemoteTree(
         logger_->Emit(LogLevel::Error,
                       "Failed to create tmp directory for copying git-tree {} "
                       "from remote CAS",
-                      digest->hash());
+                      remote_digest->hash());
         response->set_status(GetRemoteTreeResponse::INTERNAL_ERROR);
         return ::grpc::Status::OK;
     }
     if (not apis_.remote->RetrieveToPaths(
-            {Artifact::ObjectInfo{.digest = *digest, .type = ObjectType::Tree}},
+            {Artifact::ObjectInfo{.digest = *remote_digest,
+                                  .type = ObjectType::Tree}},
             {tmp_dir->GetPath()},
             &(*apis_.local))) {
         logger_->Emit(LogLevel::Error,
                       "Failed to retrieve tree {} from remote CAS",
-                      tree_id);
+                      remote_digest->hash());
         response->set_status(GetRemoteTreeResponse::FAILED_PRECONDITION);
         return ::grpc::Status::OK;
     }
     // Import from tmp dir to Git cache
-    auto res =
-        CommonImportToGit(tmp_dir->GetPath(),
-                          fmt::format("Content of tree {}", tree_id)  // message
-        );
+    auto res = CommonImportToGit(
+        tmp_dir->GetPath(),
+        fmt::format("Content of tree {}", remote_digest->hash())  // message
+    );
     if (not res) {
         // report the error
         logger_->Emit(LogLevel::Error, "{}", res.error());
         response->set_status(GetRemoteTreeResponse::INTERNAL_ERROR);
         return ::grpc::Status::OK;
     }
-    auto const& imported_tree_id = *res;
-    // sanity check
-    if (imported_tree_id != tree_id) {
-        logger_->Emit(
-            LogLevel::Error,
-            "Unexpected mismatch in imported tree:\nexpected {}, but got {}",
-            tree_id,
-            imported_tree_id);
-        response->set_status(GetRemoteTreeResponse::INTERNAL_ERROR);
-        return ::grpc::Status::OK;
-    }
+    logger_->Emit(LogLevel::Debug,
+                  "GetRemoteTree: imported tree {} to Git as {}",
+                  remote_digest->hash(),
+                  *res);
     // success!
     response->set_status(GetRemoteTreeResponse::OK);
     return ::grpc::Status::OK;
diff --git a/src/buildtool/serve_api/serve_service/source_tree.hpp b/src/buildtool/serve_api/serve_service/source_tree.hpp
index 7a63bcf9..e46f21bf 100644
--- a/src/buildtool/serve_api/serve_service/source_tree.hpp
+++ b/src/buildtool/serve_api/serve_service/source_tree.hpp
@@ -122,9 +122,9 @@ class SourceTreeService final
         const ::justbuild::just_serve::CheckRootTreeRequest* request,
         CheckRootTreeResponse* response) -> ::grpc::Status override;
 
-    // Retrieve a given Git-tree from the CAS of the associated
-    // remote-execution endpoint and make it available in a location where this
-    // serve instance can build against.
+    // Retrieve a given tree from the CAS of the associated remote-execution
+    // endpoint and make it available in a location where this serve instance
+    // can build against.
     //
     // There are no method-specific errors.
     auto GetRemoteTree(
diff --git a/src/other_tools/root_maps/TARGETS b/src/other_tools/root_maps/TARGETS
index 61e16cb7..921dbf80 100644
--- a/src/other_tools/root_maps/TARGETS
+++ b/src/other_tools/root_maps/TARGETS
@@ -209,6 +209,7 @@
     , ["src/buildtool/common", "config"]
     , ["src/buildtool/crypto", "hash_function"]
     , ["src/buildtool/execution_api/serve", "mr_git_api"]
+    , ["src/buildtool/execution_api/serve", "utils"]
     , ["src/buildtool/file_system", "object_type"]
     ]
   }
diff --git a/src/other_tools/root_maps/root_utils.cpp b/src/other_tools/root_maps/root_utils.cpp
index 0e61c548..89e71653 100644
--- a/src/other_tools/root_maps/root_utils.cpp
+++ b/src/other_tools/root_maps/root_utils.cpp
@@ -21,6 +21,7 @@
 #include "src/buildtool/common/repository_config.hpp"
 #include "src/buildtool/crypto/hash_function.hpp"
 #include "src/buildtool/execution_api/serve/mr_git_api.hpp"
+#include "src/buildtool/execution_api/serve/utils.hpp"
 #include "src/buildtool/file_system/object_type.hpp"
 
 auto CheckServeHasAbsentRoot(ServeApi const& serve,
@@ -48,6 +49,14 @@ auto EnsureAbsentRootOnServe(
     IExecutionApi const* remote_api,
     AsyncMapConsumerLoggerPtr const& logger,
     bool no_sync_is_fatal) -> bool {
+    auto const native_digest = ArtifactDigestFactory::Create(
+        HashFunction::Type::GitSHA1, tree_id, 0, /*is_tree=*/true);
+    if (not native_digest) {
+        (*logger)(fmt::format("Failed to create digest for {}", tree_id),
+                  /*fatal=*/true);
+        return false;
+    }
+    // check if upload is required
     if (remote_api != nullptr) {
         // upload tree to remote CAS
         auto repo = RepositoryConfig{};
@@ -57,18 +66,15 @@ auto EnsureAbsentRootOnServe(
                 /*fatal=*/true);
             return false;
         }
-        auto const digest = ArtifactDigestFactory::Create(
-            HashFunction::Type::GitSHA1, tree_id, 0, /*is_tree=*/true);
-
         auto git_api = MRGitApi{&repo,
                                 native_storage_config,
                                 compat_storage_config,
                                 compat_storage,
                                 local_api};
-        if (not digest or not git_api.RetrieveToCas(
-                              {Artifact::ObjectInfo{.digest = *digest,
-                                                    .type = ObjectType::Tree}},
-                              *remote_api)) {
+        if (not git_api.RetrieveToCas(
+                {Artifact::ObjectInfo{.digest = *native_digest,
+                                      .type = ObjectType::Tree}},
+                *remote_api)) {
             (*logger)(fmt::format("Failed to sync tree {} from repository {}",
                                   tree_id,
                                   repo_path.string()),
@@ -76,8 +82,29 @@ auto EnsureAbsentRootOnServe(
             return false;
         }
     }
-    // ask serve endpoint to retrieve the uploaded tree
-    if (not serve.GetTreeFromRemote(tree_id)) {
+    // ask serve endpoint to retrieve the uploaded tree; this can only happen if
+    // we have access to a digest that the remote knows
+    ArtifactDigest remote_digest = *native_digest;
+    if (compat_storage_config != nullptr) {
+        // in compatible mode, get compatible digest from mapping, if exists
+        auto cached_obj = MRApiUtils::ReadRehashedDigest(*native_digest,
+                                                         *native_storage_config,
+                                                         *compat_storage_config,
+                                                         /*from_git=*/true);
+        if (not cached_obj) {
+            (*logger)(cached_obj.error(), /*fatal=*/true);
+            return false;
+        }
+        if (not *cached_obj) {
+            // digest is not known; respond based on no_sync_is_fatal flag
+            (*logger)(fmt::format("No digest provided to sync root tree {}.",
+                                  tree_id),
+                      /*fatal=*/no_sync_is_fatal);
+            return not no_sync_is_fatal;
+        }
+        remote_digest = cached_obj->value().digest;
+    }
+    if (not serve.GetTreeFromRemote(remote_digest)) {
         // respond based on no_sync_is_fatal flag
         (*logger)(
             fmt::format("Serve endpoint failed to sync root tree {}.", tree_id),