From e6127d3cbd4fb8b3dd82bc740692877b72f977b7 Mon Sep 17 00:00:00 2001 From: Paul Cristian Sarbu Date: Mon, 21 Oct 2024 15:37:44 +0200 Subject: Enable compatible mode for just-mr and SourceTree serve service... ...by using the new local api that can handle any remote endpoint, irrespective of protocol. Also ensure all tests for the serve service are now being run both in native and compatible modes. --- src/buildtool/serve_api/serve_service/TARGETS | 2 ++ .../serve_service/serve_server_implementation.cpp | 34 ++++++++++++++++------ src/other_tools/just_mr/TARGETS | 2 ++ src/other_tools/just_mr/fetch.cpp | 17 ++++++++--- src/other_tools/just_mr/main.cpp | 31 -------------------- src/other_tools/just_mr/setup.cpp | 17 ++++++++--- 6 files changed, 55 insertions(+), 48 deletions(-) (limited to 'src') diff --git a/src/buildtool/serve_api/serve_service/TARGETS b/src/buildtool/serve_api/serve_service/TARGETS index 27e0d58c..80c79f2f 100644 --- a/src/buildtool/serve_api/serve_service/TARGETS +++ b/src/buildtool/serve_api/serve_service/TARGETS @@ -74,6 +74,8 @@ , ["src/buildtool/execution_api/execution_service", "cas_server"] , ["src/buildtool/execution_api/execution_service", "execution_server"] , ["src/buildtool/execution_api/execution_service", "operations_server"] + , ["src/buildtool/execution_api/local", "local"] + , ["src/buildtool/execution_api/serve", "mr_local_api"] , ["src/buildtool/logging", "log_level"] , ["src/buildtool/storage", "config"] , ["src/buildtool/storage", "storage"] diff --git a/src/buildtool/serve_api/serve_service/serve_server_implementation.cpp b/src/buildtool/serve_api/serve_service/serve_server_implementation.cpp index f03a21e8..88fad61a 100644 --- a/src/buildtool/serve_api/serve_service/serve_server_implementation.cpp +++ b/src/buildtool/serve_api/serve_service/serve_server_implementation.cpp @@ -37,6 +37,8 @@ #include "src/buildtool/execution_api/execution_service/cas_server.hpp" #include "src/buildtool/execution_api/execution_service/execution_server.hpp" #include "src/buildtool/execution_api/execution_service/operations_server.hpp" +#include "src/buildtool/execution_api/local/local_api.hpp" +#include "src/buildtool/execution_api/serve/mr_local_api.hpp" #include "src/buildtool/file_system/file_system_manager.hpp" #include "src/buildtool/file_system/git_repo.hpp" #include "src/buildtool/logging/log_level.hpp" @@ -132,7 +134,9 @@ auto ServeServerImpl::Run( std::unique_ptr secondary_storage_config = nullptr; std::unique_ptr secondary_storage = nullptr; std::unique_ptr secondary_local_context = nullptr; - if (not ProtocolTraits::IsNative(hash_type)) { + IExecutionApi::Ptr secondary_local_api = nullptr; + auto const is_compat = not ProtocolTraits::IsNative(hash_type); + if (is_compat) { auto config = StorageConfig::Builder{} .SetBuildRoot(local_context->storage_config->build_root) @@ -150,16 +154,28 @@ auto ServeServerImpl::Run( LocalContext{.exec_config = local_context->exec_config, .storage_config = &*secondary_storage_config, .storage = &*secondary_storage}); + secondary_local_api = + std::make_shared(&*secondary_local_context); } - SourceTreeService sts{&serve_config, - &apis, - /*native_context=*/secondary_local_context != nullptr - ? &*secondary_local_context - : local_context, - /*compat_context=*/secondary_local_context != nullptr - ? &*local_context - : nullptr}; + // setup the overall local api, aware of compatibility + IExecutionApi::Ptr mr_local_api = std::make_shared( + is_compat ? &*secondary_local_context : local_context, + is_compat ? &*secondary_local_api : &*apis.local, + is_compat ? &*local_context : nullptr, + is_compat ? &*apis.local : nullptr); + // setup the apis to pass to SourceTreeService + auto const mr_apis = ApiBundle{.hash_function = apis.hash_function, + .local = mr_local_api, + .remote = apis.remote}; + + SourceTreeService sts{ + &serve_config, + &mr_apis, + is_compat ? &*secondary_local_context + : local_context, // native_context + is_compat ? &*local_context : nullptr // compat_context + }; // set up the server grpc::ServerBuilder builder; diff --git a/src/other_tools/just_mr/TARGETS b/src/other_tools/just_mr/TARGETS index a1180e56..002e8fda 100644 --- a/src/other_tools/just_mr/TARGETS +++ b/src/other_tools/just_mr/TARGETS @@ -131,6 +131,7 @@ , ["src/buildtool/execution_api/remote", "bazel"] , ["src/buildtool/execution_api/remote", "config"] , ["src/buildtool/execution_api/remote", "context"] + , ["src/buildtool/execution_api/serve", "mr_local_api"] , ["src/buildtool/logging", "logging"] , ["src/buildtool/main", "retry"] , ["src/buildtool/multithreading", "async_map_utils"] @@ -206,6 +207,7 @@ , ["src/buildtool/execution_api/remote", "bazel"] , ["src/buildtool/execution_api/remote", "config"] , ["src/buildtool/execution_api/remote", "context"] + , ["src/buildtool/execution_api/serve", "mr_local_api"] , ["src/buildtool/file_system/symlinks_map", "resolve_symlinks_map"] , ["src/buildtool/logging", "logging"] , ["src/buildtool/main", "retry"] diff --git a/src/other_tools/just_mr/fetch.cpp b/src/other_tools/just_mr/fetch.cpp index fdea8812..99f21eb1 100644 --- a/src/other_tools/just_mr/fetch.cpp +++ b/src/other_tools/just_mr/fetch.cpp @@ -32,6 +32,7 @@ #include "src/buildtool/execution_api/remote/bazel/bazel_api.hpp" #include "src/buildtool/execution_api/remote/config.hpp" #include "src/buildtool/execution_api/remote/context.hpp" +#include "src/buildtool/execution_api/serve/mr_local_api.hpp" #include "src/buildtool/logging/log_level.hpp" #include "src/buildtool/logging/logger.hpp" #include "src/buildtool/main/retry.hpp" @@ -341,6 +342,7 @@ auto MultiRepoFetch(std::shared_ptr const& config, std::unique_ptr compat_storage = nullptr; std::unique_ptr compat_local_context = nullptr; std::optional compat_lock = std::nullopt; + IExecutionApi::Ptr compat_local_api = nullptr; if (common_args.compatible) { auto config = StorageConfig::Builder{} .SetBuildRoot(native_storage_config.build_root) @@ -366,8 +368,16 @@ auto MultiRepoFetch(std::shared_ptr const& config, "Failed to acquire compatible storage gc lock"); return kExitConfigError; } + compat_local_api = std::make_shared(&*compat_local_context); } + // setup the overall local api, aware of compatibility + IExecutionApi::Ptr mr_local_api = std::make_shared( + &native_local_context, + &*native_local_api, + common_args.compatible ? &*compat_local_context : nullptr, + common_args.compatible ? &*compat_local_api : nullptr); + // setup authentication config auto const auth_config = JustMR::Utils::CreateAuthConfig(auth_args); if (not auth_config) { @@ -404,8 +414,7 @@ auto MultiRepoFetch(std::shared_ptr const& config, config, &hash_fct); } - bool const has_remote_api = - remote_api != nullptr and not common_args.compatible; + bool const has_remote_api = remote_api != nullptr; // pack the remote context RemoteContext const remote_context{.auth = &*auth_config, @@ -420,8 +429,8 @@ auto MultiRepoFetch(std::shared_ptr const& config, } auto const apis = ApiBundle{.hash_function = hash_fct, - .local = native_local_api, - .remote = has_remote_api ? remote_api : native_local_api}; + .local = mr_local_api, + .remote = has_remote_api ? remote_api : mr_local_api}; auto serve = ServeApi::Create( *serve_config, compat_local_context != nullptr diff --git a/src/other_tools/just_mr/main.cpp b/src/other_tools/just_mr/main.cpp index bcc42bac..e4eaa18c 100644 --- a/src/other_tools/just_mr/main.cpp +++ b/src/other_tools/just_mr/main.cpp @@ -365,13 +365,6 @@ auto main(int argc, char* argv[]) -> int { // Run subcommands known to just and `do` if (arguments.cmd == SubCommand::kJustDo or arguments.cmd == SubCommand::kJustSubCmd) { - // check setup configuration arguments for validity - if (arguments.common.compatible and arguments.common.fetch_absent) { - Logger::Log(LogLevel::Error, - "Fetching absent repositories only available in " - "native mode!"); - return kExitConfigError; - } return CallJust(config_file, arguments.common, arguments.setup, @@ -402,13 +395,6 @@ auto main(int argc, char* argv[]) -> int { // Run subcommand `setup` or `setup-env` if (arguments.cmd == SubCommand::kSetup or arguments.cmd == SubCommand::kSetupEnv) { - // check setup configuration arguments for validity - if (arguments.common.compatible and arguments.common.fetch_absent) { - Logger::Log(LogLevel::Error, - "Fetching absent repositories only available in " - "native mode!"); - return kExitConfigError; - } auto mr_config_path = MultiRepoSetup( config, arguments.common, @@ -442,23 +428,6 @@ auto main(int argc, char* argv[]) -> int { // Run subcommand `fetch` if (arguments.cmd == SubCommand::kFetch) { - // check fetch configuration arguments for validity - if (arguments.common.compatible) { - if (arguments.common.remote_execution_address and - arguments.fetch.backup_to_remote) { - Logger::Log( - LogLevel::Error, - "Remote backup for fetched archives only available " - "in native mode!"); - return kExitConfigError; - } - if (arguments.common.fetch_absent) { - Logger::Log(LogLevel::Error, - "Fetching absent repositories only available " - "in native mode!"); - return kExitConfigError; - } - } return MultiRepoFetch(config, arguments.common, arguments.setup, diff --git a/src/other_tools/just_mr/setup.cpp b/src/other_tools/just_mr/setup.cpp index f6ada636..4ab032de 100644 --- a/src/other_tools/just_mr/setup.cpp +++ b/src/other_tools/just_mr/setup.cpp @@ -34,6 +34,7 @@ #include "src/buildtool/execution_api/remote/bazel/bazel_api.hpp" #include "src/buildtool/execution_api/remote/config.hpp" #include "src/buildtool/execution_api/remote/context.hpp" +#include "src/buildtool/execution_api/serve/mr_local_api.hpp" #include "src/buildtool/file_system/symlinks_map/resolve_symlinks_map.hpp" #include "src/buildtool/logging/log_level.hpp" #include "src/buildtool/logging/logger.hpp" @@ -153,6 +154,7 @@ auto MultiRepoSetup(std::shared_ptr const& config, std::unique_ptr compat_storage = nullptr; std::unique_ptr compat_local_context = nullptr; std::optional compat_lock = std::nullopt; + IExecutionApi::Ptr compat_local_api = nullptr; if (common_args.compatible) { auto config = StorageConfig::Builder{} .SetBuildRoot(native_storage_config.build_root) @@ -178,8 +180,16 @@ auto MultiRepoSetup(std::shared_ptr const& config, "Failed to acquire compatible storage gc lock"); return std::nullopt; } + compat_local_api = std::make_shared(&*compat_local_context); } + // setup the overall local api, aware of compatibility + IExecutionApi::Ptr mr_local_api = std::make_shared( + &native_local_context, + &*native_local_api, + common_args.compatible ? &*compat_local_context : nullptr, + common_args.compatible ? &*compat_local_api : nullptr); + // setup authentication config auto const auth_config = JustMR::Utils::CreateAuthConfig(auth_args); if (not auth_config) { @@ -216,8 +226,7 @@ auto MultiRepoSetup(std::shared_ptr const& config, config, &hash_fct); } - bool const has_remote_api = - remote_api != nullptr and not common_args.compatible; + bool const has_remote_api = remote_api != nullptr; // pack the remote context RemoteContext const remote_context{.auth = &*auth_config, @@ -232,8 +241,8 @@ auto MultiRepoSetup(std::shared_ptr const& config, } auto const apis = ApiBundle{.hash_function = hash_fct, - .local = native_local_api, - .remote = has_remote_api ? remote_api : native_local_api}; + .local = mr_local_api, + .remote = has_remote_api ? remote_api : mr_local_api}; auto serve = ServeApi::Create( *serve_config, compat_local_context != nullptr -- cgit v1.2.3