diff options
| -rw-r--r-- | src/buildtool/execution_api/common/api_bundle.cpp | 35 | ||||
| -rw-r--r-- | src/buildtool/execution_api/common/api_bundle.hpp | 10 | ||||
| -rw-r--r-- | src/buildtool/graph_traverser/graph_traverser.hpp | 6 | ||||
| -rw-r--r-- | src/buildtool/main/main.cpp | 18 | ||||
| -rw-r--r-- | src/buildtool/serve_api/serve_service/target.cpp | 4 | ||||
| -rw-r--r-- | src/other_tools/just_mr/fetch.cpp | 6 | ||||
| -rw-r--r-- | src/other_tools/just_mr/setup.cpp | 6 | ||||
| -rw-r--r-- | test/buildtool/build_engine/target_map/target_map.test.cpp | 48 | ||||
| -rw-r--r-- | test/buildtool/graph_traverser/graph_traverser.test.hpp | 28 |
9 files changed, 95 insertions, 66 deletions
diff --git a/src/buildtool/execution_api/common/api_bundle.cpp b/src/buildtool/execution_api/common/api_bundle.cpp index 15e84440..8808e5d8 100644 --- a/src/buildtool/execution_api/common/api_bundle.cpp +++ b/src/buildtool/execution_api/common/api_bundle.cpp @@ -18,16 +18,33 @@ #include "src/buildtool/execution_api/local/local_api.hpp" #include "src/buildtool/execution_api/remote/bazel/bazel_api.hpp" -ApiBundle::ApiBundle(gsl::not_null<LocalContext const*> const& local_context, - gsl::not_null<RemoteContext const*> const& remote_context, - RepositoryConfig const* repo_config) - : hash_function{local_context->storage_config->hash_function}, - local{std::make_shared<LocalApi>(local_context, repo_config)}, - remote{CreateRemote(remote_context->exec_config->remote_address, - remote_context->auth, - remote_context->retry_config)} {} +/// \note Some logic from MakeRemote is duplicated here as that method cannot +/// be used without the hash_function field being set prior to the call. +auto ApiBundle::Create( + gsl::not_null<LocalContext const*> const& local_context, + gsl::not_null<RemoteContext const*> const& remote_context, + RepositoryConfig const* repo_config) -> ApiBundle { + auto const hash_fct = local_context->storage_config->hash_function; + IExecutionApi::Ptr local_api = + std::make_shared<LocalApi>(local_context, repo_config); + IExecutionApi::Ptr remote_api = local_api; + if (auto const address = remote_context->exec_config->remote_address) { + ExecutionConfiguration config; + config.skip_cache_lookup = false; + remote_api = std::make_shared<BazelApi>("remote-execution", + address->host, + address->port, + remote_context->auth, + remote_context->retry_config, + config, + hash_fct); + } + return ApiBundle{.hash_function = hash_fct, + .local = std::move(local_api), + .remote = std::move(remote_api)}; +} -auto ApiBundle::CreateRemote( +auto ApiBundle::MakeRemote( std::optional<ServerAddress> const& address, gsl::not_null<Auth const*> const& authentication, gsl::not_null<RetryConfig const*> const& retry_config) const diff --git a/src/buildtool/execution_api/common/api_bundle.hpp b/src/buildtool/execution_api/common/api_bundle.hpp index 17c64d1e..28595010 100644 --- a/src/buildtool/execution_api/common/api_bundle.hpp +++ b/src/buildtool/execution_api/common/api_bundle.hpp @@ -29,10 +29,13 @@ /// same time. If the remote api cannot be instantiated, it falls back to /// exactly the same instance that local api is (&*remote == & *local). struct ApiBundle final { - explicit ApiBundle( + /// \brief Create an ApiBundle instance. + /// \note A creator method is used instead of a constructor to allow for + /// tests to instantiate ApiBundles with own implementations of the APIs. + [[nodiscard]] static auto Create( gsl::not_null<LocalContext const*> const& local_context, gsl::not_null<RemoteContext const*> const& remote_context, - RepositoryConfig const* repo_config); + RepositoryConfig const* repo_config) -> ApiBundle; /// \brief Create a Remote object based on the given arguments. /// \param address The endpoint address. @@ -40,13 +43,12 @@ struct ApiBundle final { /// \param retry_config The retry strategy configuration. /// \returns A configured api: BazelApi if a remote address is given, /// otherwise fall back to the already configured LocalApi instance. - [[nodiscard]] auto CreateRemote( + [[nodiscard]] auto MakeRemote( std::optional<ServerAddress> const& address, gsl::not_null<Auth const*> const& authentication, gsl::not_null<RetryConfig const*> const& retry_config) const -> gsl::not_null<IExecutionApi::Ptr>; - // Needed to be set before creating the remote (via CreateRemote) HashFunction const hash_function; // 7 bytes of alignment. diff --git a/src/buildtool/graph_traverser/graph_traverser.hpp b/src/buildtool/graph_traverser/graph_traverser.hpp index a63f7d8c..301ef722 100644 --- a/src/buildtool/graph_traverser/graph_traverser.hpp +++ b/src/buildtool/graph_traverser/graph_traverser.hpp @@ -385,9 +385,9 @@ class GraphTraverser { std::vector<ArtifactIdentifier> const& artifact_ids) const -> bool { // setup rebuilder with api for cache endpoint auto api_cached = - apis_.CreateRemote(remote_context_.exec_config->cache_address, - remote_context_.auth, - remote_context_.retry_config); + apis_.MakeRemote(remote_context_.exec_config->cache_address, + remote_context_.auth, + remote_context_.retry_config); Rebuilder executor{repo_config_, &*apis_.local, &*apis_.remote, diff --git a/src/buildtool/main/main.cpp b/src/buildtool/main/main.cpp index 18caa871..e97b5a27 100644 --- a/src/buildtool/main/main.cpp +++ b/src/buildtool/main/main.cpp @@ -822,9 +822,10 @@ auto main(int argc, char* argv[]) -> int { .retry_config = &retry_config, .exec_config = &remote_exec_config}; - ApiBundle const exec_apis{&local_context, - &remote_context, - /*repo_config=*/nullptr}; + auto const exec_apis = + ApiBundle::Create(&local_context, + &remote_context, + /*repo_config=*/nullptr); return execution_server->Run(&local_context, &remote_context, @@ -888,9 +889,10 @@ auto main(int argc, char* argv[]) -> int { .retry_config = &*retry_config, .exec_config = &*remote_exec_config}; - ApiBundle const serve_apis{&local_context, - &remote_context, - /*repo_config=*/nullptr}; + auto const serve_apis = + ApiBundle::Create(&local_context, + &remote_context, + /*repo_config=*/nullptr); auto serve = ServeApi::Create(*serve_config, &local_context, &remote_context, @@ -988,8 +990,8 @@ auto main(int argc, char* argv[]) -> int { .retry_config = &*retry_config, .exec_config = &*remote_exec_config}; - ApiBundle const main_apis{ - &local_context, &remote_context, &repo_config}; + auto const main_apis = + ApiBundle::Create(&local_context, &remote_context, &repo_config); GraphTraverser const traverser{ {jobs, std::move(arguments.build), diff --git a/src/buildtool/serve_api/serve_service/target.cpp b/src/buildtool/serve_api/serve_service/target.cpp index ec8ff458..9ad1520b 100644 --- a/src/buildtool/serve_api/serve_service/target.cpp +++ b/src/buildtool/serve_api/serve_service/target.cpp @@ -501,8 +501,8 @@ auto TargetService::ServeTarget( // Use a new ApiBundle that knows about local repository config and // dispatch endpoint for traversing - ApiBundle const local_apis{ - &local_context_, &dispatch_context, &repository_config}; + auto const local_apis = ApiBundle::Create( + &local_context_, &dispatch_context, &repository_config); GraphTraverser const traverser{ std::move(traverser_args), diff --git a/src/other_tools/just_mr/fetch.cpp b/src/other_tools/just_mr/fetch.cpp index 088c9ace..d236b7e8 100644 --- a/src/other_tools/just_mr/fetch.cpp +++ b/src/other_tools/just_mr/fetch.cpp @@ -440,9 +440,9 @@ auto MultiRepoFetch(std::shared_ptr<Configuration> const& config, .exec_config = &*remote_exec_config}; // setup the APIs for archive fetches; only happens if in native mode - ApiBundle const apis{&local_context, - &remote_context, - /*repo_config=*/nullptr}; + auto const apis = ApiBundle::Create(&local_context, + &remote_context, + /*repo_config=*/nullptr); bool const has_remote_api = apis.local != apis.remote and not common_args.compatible; diff --git a/src/other_tools/just_mr/setup.cpp b/src/other_tools/just_mr/setup.cpp index 9f9e09bd..ba5e39b4 100644 --- a/src/other_tools/just_mr/setup.cpp +++ b/src/other_tools/just_mr/setup.cpp @@ -161,9 +161,9 @@ auto MultiRepoSetup(std::shared_ptr<Configuration> const& config, .retry_config = &*retry_config, .exec_config = &*remote_exec_config}; - ApiBundle const apis{&local_context, - &remote_context, - /*repo_config=*/nullptr}; + auto const apis = ApiBundle::Create(&local_context, + &remote_context, + /*repo_config=*/nullptr); bool const has_remote_api = apis.local != apis.remote and not common_args.compatible; diff --git a/test/buildtool/build_engine/target_map/target_map.test.cpp b/test/buildtool/build_engine/target_map/target_map.test.cpp index ed859e31..ddcecdd0 100644 --- a/test/buildtool/build_engine/target_map/target_map.test.cpp +++ b/test/buildtool/build_engine/target_map/target_map.test.cpp @@ -126,9 +126,9 @@ TEST_CASE("simple targets", "[target_map]") { .retry_config = &retry_config, .exec_config = &remote_exec_config}; - ApiBundle const apis{&local_context, - &remote_context, - /*repo_config=*/nullptr}; + auto const apis = ApiBundle::Create(&local_context, + &remote_context, + /*repo_config=*/nullptr); auto serve = ServeApi::Create(*serve_config, &local_context, &remote_context, &apis); AnalyseContext ctx{.repo_config = &repo_config, @@ -589,9 +589,9 @@ TEST_CASE("configuration deduplication", "[target_map]") { .retry_config = &retry_config, .exec_config = &remote_exec_config}; - ApiBundle const apis{&local_context, - &remote_context, - /*repo_config=*/nullptr}; + auto const apis = ApiBundle::Create(&local_context, + &remote_context, + /*repo_config=*/nullptr); auto serve = ServeApi::Create(*serve_config, &local_context, &remote_context, &apis); @@ -699,9 +699,9 @@ TEST_CASE("generator functions in string arguments", "[target_map]") { .retry_config = &retry_config, .exec_config = &remote_exec_config}; - ApiBundle const apis{&local_context, - &remote_context, - /*repo_config=*/nullptr}; + auto const apis = ApiBundle::Create(&local_context, + &remote_context, + /*repo_config=*/nullptr); auto serve = ServeApi::Create(*serve_config, &local_context, &remote_context, &apis); @@ -821,9 +821,9 @@ TEST_CASE("built-in rules", "[target_map]") { .retry_config = &retry_config, .exec_config = &remote_exec_config}; - ApiBundle const apis{&local_context, - &remote_context, - /*repo_config=*/nullptr}; + auto const apis = ApiBundle::Create(&local_context, + &remote_context, + /*repo_config=*/nullptr); auto serve = ServeApi::Create(*serve_config, &local_context, &remote_context, &apis); @@ -1055,9 +1055,9 @@ TEST_CASE("target reference", "[target_map]") { .retry_config = &retry_config, .exec_config = &remote_exec_config}; - ApiBundle const apis{&local_context, - &remote_context, - /*repo_config=*/nullptr}; + auto const apis = ApiBundle::Create(&local_context, + &remote_context, + /*repo_config=*/nullptr); auto serve = ServeApi::Create(*serve_config, &local_context, &remote_context, &apis); @@ -1220,9 +1220,9 @@ TEST_CASE("trees", "[target_map]") { .retry_config = &retry_config, .exec_config = &remote_exec_config}; - ApiBundle const apis{&local_context, - &remote_context, - /*repo_config=*/nullptr}; + auto const apis = ApiBundle::Create(&local_context, + &remote_context, + /*repo_config=*/nullptr); auto serve = ServeApi::Create(*serve_config, &local_context, &remote_context, &apis); @@ -1349,9 +1349,9 @@ TEST_CASE("RESULT error reporting", "[target_map]") { .retry_config = &retry_config, .exec_config = &remote_exec_config}; - ApiBundle const apis{&local_context, - &remote_context, - /*repo_config=*/nullptr}; + auto const apis = ApiBundle::Create(&local_context, + &remote_context, + /*repo_config=*/nullptr); auto serve = ServeApi::Create(*serve_config, &local_context, &remote_context, &apis); @@ -1537,9 +1537,9 @@ TEST_CASE("wrong arguments", "[target_map]") { .retry_config = &retry_config, .exec_config = &remote_exec_config}; - ApiBundle const apis{&local_context, - &remote_context, - /*repo_config=*/nullptr}; + auto const apis = ApiBundle::Create(&local_context, + &remote_context, + /*repo_config=*/nullptr); auto serve = ServeApi::Create(*serve_config, &local_context, &remote_context, &apis); diff --git a/test/buildtool/graph_traverser/graph_traverser.test.hpp b/test/buildtool/graph_traverser/graph_traverser.test.hpp index 314d7bd6..6618e5bf 100644 --- a/test/buildtool/graph_traverser/graph_traverser.test.hpp +++ b/test/buildtool/graph_traverser/graph_traverser.test.hpp @@ -186,7 +186,8 @@ class TestProject { .retry_config = &retry_config, .exec_config = remote_config}; - ApiBundle const apis{&local_context, &remote_context, p.GetRepoConfig()}; + auto const apis = + ApiBundle::Create(&local_context, &remote_context, p.GetRepoConfig()); GraphTraverser const gt{clargs.gtargs, p.GetRepoConfig(), @@ -264,7 +265,8 @@ class TestProject { .retry_config = &retry_config, .exec_config = remote_config}; - ApiBundle const apis{&local_context, &remote_context, p.GetRepoConfig()}; + auto const apis = + ApiBundle::Create(&local_context, &remote_context, p.GetRepoConfig()); GraphTraverser const gt{clargs.gtargs, p.GetRepoConfig(), @@ -312,7 +314,8 @@ class TestProject { .retry_config = &retry_config, .exec_config = remote_config}; - ApiBundle const apis{&local_context, &remote_context, p.GetRepoConfig()}; + auto const apis = + ApiBundle::Create(&local_context, &remote_context, p.GetRepoConfig()); GraphTraverser const gt{clargs.gtargs, p.GetRepoConfig(), @@ -379,8 +382,8 @@ class TestProject { .retry_config = &retry_config, .exec_config = remote_config}; - ApiBundle const apis{ - &local_context, &remote_context, full_hello_world.GetRepoConfig()}; + auto const apis = ApiBundle::Create( + &local_context, &remote_context, full_hello_world.GetRepoConfig()); GraphTraverser const gt_upload{clargs_update_cpp.gtargs, full_hello_world.GetRepoConfig(), @@ -453,7 +456,8 @@ static void TestBlobsUploadedAndUsed( .retry_config = &retry_config, .exec_config = remote_config}; - ApiBundle const apis{&local_context, &remote_context, p.GetRepoConfig()}; + auto const apis = + ApiBundle::Create(&local_context, &remote_context, p.GetRepoConfig()); GraphTraverser gt{clargs.gtargs, p.GetRepoConfig(), @@ -509,7 +513,8 @@ static void TestEnvironmentVariablesSetAndUsed( .retry_config = &retry_config, .exec_config = remote_config}; - ApiBundle const apis{&local_context, &remote_context, p.GetRepoConfig()}; + auto const apis = + ApiBundle::Create(&local_context, &remote_context, p.GetRepoConfig()); GraphTraverser gt{clargs.gtargs, p.GetRepoConfig(), @@ -565,7 +570,8 @@ static void TestTreesUsed( .retry_config = &retry_config, .exec_config = remote_config}; - ApiBundle const apis{&local_context, &remote_context, p.GetRepoConfig()}; + auto const apis = + ApiBundle::Create(&local_context, &remote_context, p.GetRepoConfig()); GraphTraverser gt{clargs.gtargs, p.GetRepoConfig(), @@ -621,7 +627,8 @@ static void TestNestedTreesUsed( .retry_config = &retry_config, .exec_config = remote_config}; - ApiBundle const apis{&local_context, &remote_context, p.GetRepoConfig()}; + auto const apis = + ApiBundle::Create(&local_context, &remote_context, p.GetRepoConfig()); GraphTraverser gt{clargs.gtargs, p.GetRepoConfig(), @@ -676,7 +683,8 @@ static void TestFlakyHelloWorldDetected( .retry_config = &retry_config, .exec_config = remote_config}; - ApiBundle const apis{&local_context, &remote_context, p.GetRepoConfig()}; + auto const apis = + ApiBundle::Create(&local_context, &remote_context, p.GetRepoConfig()); { auto clargs = p.CmdLineArgs("_entry_points_ctimes"); |