diff options
author | Paul Cristian Sarbu <paul.cristian.sarbu@huawei.com> | 2024-07-25 14:35:28 +0200 |
---|---|---|
committer | Paul Cristian Sarbu <paul.cristian.sarbu@huawei.com> | 2024-07-30 12:10:06 +0200 |
commit | 69f56ad981da59f026c83b321522ad68283934c5 (patch) | |
tree | d85218053bb97bf623b541d9bc98fc9a3b0cbb44 | |
parent | ab0e0b86fff94bed224f894be6b555272857e336 (diff) | |
download | justbuild-69f56ad981da59f026c83b321522ad68283934c5.tar.gz |
Pass LocalContext to LocalApi
The context is passed by not_null const pointer to avoid binding to
temporaries. The LocalApi also stores the context as const ref for
further access and passing it to LocalAction.
11 files changed, 197 insertions, 136 deletions
diff --git a/src/buildtool/execution_api/common/api_bundle.cpp b/src/buildtool/execution_api/common/api_bundle.cpp index 96b24376..edb29970 100644 --- a/src/buildtool/execution_api/common/api_bundle.cpp +++ b/src/buildtool/execution_api/common/api_bundle.cpp @@ -29,10 +29,7 @@ ApiBundle::ApiBundle( retry_config{*retry_config}, remote_config{*remote_exec_config}, hash_function{local_context->storage_config->hash_function}, - local{std::make_shared<LocalApi>(local_context->storage_config, - local_context->storage, - local_context->exec_config, - repo_config)}, + local{std::make_shared<LocalApi>(local_context, repo_config)}, remote{CreateRemote(remote_exec_config->remote_address)} {} auto ApiBundle::CreateRemote(std::optional<ServerAddress> const& address) const diff --git a/src/buildtool/execution_api/local/TARGETS b/src/buildtool/execution_api/local/TARGETS index c753ba92..f3949310 100644 --- a/src/buildtool/execution_api/local/TARGETS +++ b/src/buildtool/execution_api/local/TARGETS @@ -25,11 +25,10 @@ [ ["@", "fmt", "", "fmt"] , ["@", "gsl", "", "gsl"] , ["@", "grpc", "", "grpc++"] - , "config" + , "context" , ["src/buildtool/common", "common"] , ["src/buildtool/common", "config"] , ["src/buildtool/storage", "storage"] - , ["src/buildtool/storage", "config"] , ["src/buildtool/execution_api/common", "common"] , ["src/buildtool/execution_api/common", "common_api"] , ["src/buildtool/execution_api/bazel_msg", "bazel_msg_factory"] diff --git a/src/buildtool/execution_api/local/local_action.cpp b/src/buildtool/execution_api/local/local_action.cpp index b44fbd5c..aac6bb39 100644 --- a/src/buildtool/execution_api/local/local_action.cpp +++ b/src/buildtool/execution_api/local/local_action.cpp @@ -103,14 +103,15 @@ auto LocalAction::Execute(Logger const* logger) noexcept } if (do_cache) { - if (auto result = storage_.ActionCache().CachedResult(*action)) { + if (auto result = + local_context_.storage->ActionCache().CachedResult(*action)) { if (result->exit_code() == 0 and ActionResultContainsExpectedOutputs( *result, output_files_, output_dirs_)) { return IExecutionResponse::Ptr{ new LocalResponse{action->hash(), {std::move(*result), /*is_cached=*/true}, - &storage_}}; + local_context_.storage}}; } } } @@ -131,11 +132,13 @@ auto LocalAction::Execute(Logger const* logger) noexcept } output->is_cached = true; - return IExecutionResponse::Ptr{new LocalResponse{ - action_cached->hash(), std::move(*output), &storage_}}; + return IExecutionResponse::Ptr{ + new LocalResponse{action_cached->hash(), + std::move(*output), + local_context_.storage}}; } return IExecutionResponse::Ptr{new LocalResponse{ - action->hash(), std::move(*output), &storage_}}; + action->hash(), std::move(*output), local_context_.storage}}; } } @@ -145,7 +148,7 @@ auto LocalAction::Execute(Logger const* logger) noexcept auto LocalAction::Run(bazel_re::Digest const& action_id) const noexcept -> std::optional<Output> { auto exec_path = - CreateUniquePath(storage_config_.ExecutionRoot() / + CreateUniquePath(local_context_.storage_config->ExecutionRoot() / NativeSupport::Unprefix(action_id.hash())); if (not exec_path) { @@ -166,7 +169,7 @@ auto LocalAction::Run(bazel_re::Digest const& action_id) const noexcept } // prepare actual command by including the launcher - auto cmdline = exec_config_.launcher; + auto cmdline = local_context_.exec_config->launcher; std::copy(cmdline_.begin(), cmdline_.end(), std::back_inserter(cmdline)); SystemCommand system{"LocalExecution"}; @@ -186,8 +189,8 @@ auto LocalAction::Run(bazel_re::Digest const& action_id) const noexcept if (CollectAndStoreOutputs(&result.action, build_root)) { if (cache_flag_ == CacheFlag::CacheOutput) { - if (not storage_.ActionCache().StoreResult(action_id, - result.action)) { + if (not local_context_.storage->ActionCache().StoreResult( + action_id, result.action)) { logger_.Emit(LogLevel::Warning, "failed to store action results"); } @@ -217,8 +220,8 @@ auto LocalAction::StageInput( blob_path = lookup->second->GetPath() / kCopyFileName; } else { - blob_path = - storage_.CAS().BlobPath(info.digest, IsExecutableObject(info.type)); + blob_path = local_context_.storage->CAS().BlobPath( + info.digest, IsExecutableObject(info.type)); } if (not blob_path) { @@ -257,7 +260,8 @@ auto LocalAction::StageInput( res.error().message()); return false; } - TmpDirPtr new_copy_dir = storage_config_.CreateTypedTmpDir("blob-copy"); + TmpDirPtr new_copy_dir = + local_context_.storage_config->CreateTypedTmpDir("blob-copy"); if (new_copy_dir == nullptr) { logger_.Emit(LogLevel::Warning, "Failed to create a temporary directory for a blob copy"); @@ -291,7 +295,7 @@ auto LocalAction::StageInputs( if (FileSystemManager::IsRelativePath(exec_path)) { return false; } - auto reader = TreeReader<LocalCasReader>{&storage_.CAS()}; + auto reader = TreeReader<LocalCasReader>{&local_context_.storage->CAS()}; auto result = reader.RecursivelyReadTreeLeafs( root_digest_, exec_path, /*include_trees=*/true); if (not result) { @@ -363,7 +367,7 @@ auto LocalAction::CollectOutputFileOrSymlink( } if (IsSymlinkObject(*type)) { auto content = FileSystemManager::ReadSymlink(file_path); - if (content and storage_.CAS().StoreBlob(*content)) { + if (content and local_context_.storage->CAS().StoreBlob(*content)) { auto out_symlink = bazel_re::OutputSymlink{}; out_symlink.set_path(local_path); out_symlink.set_target(*content); @@ -372,8 +376,8 @@ auto LocalAction::CollectOutputFileOrSymlink( } else if (IsFileObject(*type)) { bool is_executable = IsExecutableObject(*type); - auto digest = - storage_.CAS().StoreBlob</*kOwner=*/true>(file_path, is_executable); + auto digest = local_context_.storage->CAS().StoreBlob</*kOwner=*/true>( + file_path, is_executable); if (digest) { auto out_file = bazel_re::OutputFile{}; out_file.set_path(local_path); @@ -402,7 +406,7 @@ auto LocalAction::CollectOutputDirOrSymlink( } if (IsSymlinkObject(*type)) { auto content = FileSystemManager::ReadSymlink(dir_path); - if (content and storage_.CAS().StoreBlob(*content)) { + if (content and local_context_.storage->CAS().StoreBlob(*content)) { auto out_symlink = bazel_re::OutputSymlink{}; out_symlink.set_path(local_path); out_symlink.set_target(*content); @@ -410,7 +414,8 @@ auto LocalAction::CollectOutputDirOrSymlink( } } else if (IsTreeObject(*type)) { - if (auto digest = CreateDigestFromLocalOwnedTree(storage_, dir_path)) { + if (auto digest = CreateDigestFromLocalOwnedTree( + *local_context_.storage, dir_path)) { auto out_dir = bazel_re::OutputDirectory{}; out_dir.set_path(local_path); out_dir.set_allocated_tree_digest( @@ -489,7 +494,7 @@ auto LocalAction::CollectAndStoreOutputs( auto LocalAction::DigestFromOwnedFile(std::filesystem::path const& file_path) const noexcept -> gsl::owner<bazel_re::Digest*> { - if (auto digest = storage_.CAS().StoreBlob</*kOwner=*/true>( + if (auto digest = local_context_.storage->CAS().StoreBlob</*kOwner=*/true>( file_path, /*is_executable=*/false)) { return new bazel_re::Digest{std::move(*digest)}; } diff --git a/src/buildtool/execution_api/local/local_action.hpp b/src/buildtool/execution_api/local/local_action.hpp index 47e4e5a4..b2016ed7 100644 --- a/src/buildtool/execution_api/local/local_action.hpp +++ b/src/buildtool/execution_api/local/local_action.hpp @@ -28,10 +28,8 @@ #include "src/buildtool/execution_api/bazel_msg/bazel_msg_factory.hpp" #include "src/buildtool/execution_api/common/execution_action.hpp" #include "src/buildtool/execution_api/common/execution_response.hpp" -#include "src/buildtool/execution_api/local/config.hpp" +#include "src/buildtool/execution_api/local/context.hpp" #include "src/buildtool/logging/logger.hpp" -#include "src/buildtool/storage/config.hpp" -#include "src/buildtool/storage/storage.hpp" #include "src/utils/cpp/tmp_dir.hpp" class LocalApi; @@ -64,9 +62,7 @@ class LocalAction final : public IExecutionAction { private: Logger logger_{"LocalExecution"}; - StorageConfig const& storage_config_; - Storage const& storage_; - LocalExecutionConfig const& exec_config_; + LocalContext const& local_context_; ArtifactDigest const root_digest_{}; std::vector<std::string> const cmdline_{}; std::vector<std::string> output_files_{}; @@ -77,18 +73,14 @@ class LocalAction final : public IExecutionAction { CacheFlag cache_flag_{CacheFlag::CacheOutput}; explicit LocalAction( - gsl::not_null<StorageConfig const*> storage_config, - gsl::not_null<Storage const*> const& storage, - gsl::not_null<LocalExecutionConfig const*> const& exec_config, + gsl::not_null<LocalContext const*> local_context, ArtifactDigest root_digest, std::vector<std::string> command, std::vector<std::string> output_files, std::vector<std::string> output_dirs, std::map<std::string, std::string> env_vars, std::map<std::string, std::string> const& properties) noexcept - : storage_config_{*storage_config}, - storage_{*storage}, - exec_config_{*exec_config}, + : local_context_{*local_context}, root_digest_{std::move(root_digest)}, cmdline_{std::move(command)}, output_files_{std::move(output_files)}, @@ -113,7 +105,7 @@ class LocalAction final : public IExecutionAction { .env_vars = &env_vars, .properties = &properties_, .exec_dir = &exec_dir, - .hash_function = storage_config_.hash_function, + .hash_function = local_context_.storage_config->hash_function, .timeout = timeout_, .skip_action_cache = do_not_cache}; return BazelMsgFactory::CreateActionDigestFromCommandLine(request); diff --git a/src/buildtool/execution_api/local/local_api.hpp b/src/buildtool/execution_api/local/local_api.hpp index b407297d..3a40272e 100644 --- a/src/buildtool/execution_api/local/local_api.hpp +++ b/src/buildtool/execution_api/local/local_api.hpp @@ -43,26 +43,19 @@ #include "src/buildtool/execution_api/common/tree_reader.hpp" #include "src/buildtool/execution_api/execution_service/cas_utils.hpp" #include "src/buildtool/execution_api/git/git_api.hpp" -#include "src/buildtool/execution_api/local/config.hpp" +#include "src/buildtool/execution_api/local/context.hpp" #include "src/buildtool/execution_api/local/local_action.hpp" #include "src/buildtool/execution_api/local/local_cas_reader.hpp" #include "src/buildtool/file_system/file_system_manager.hpp" #include "src/buildtool/logging/log_level.hpp" #include "src/buildtool/logging/logger.hpp" -#include "src/buildtool/storage/config.hpp" -#include "src/buildtool/storage/storage.hpp" /// \brief API for local execution. class LocalApi final : public IExecutionApi { public: - explicit LocalApi( - gsl::not_null<StorageConfig const*> const& storage_config, - gsl::not_null<Storage const*> const& storage, - gsl::not_null<LocalExecutionConfig const*> const& exec_config, - RepositoryConfig const* repo_config = nullptr) noexcept - : storage_config_{*storage_config}, - storage_{*storage}, - exec_config_{*exec_config}, + explicit LocalApi(gsl::not_null<LocalContext const*> const& local_context, + RepositoryConfig const* repo_config = nullptr) noexcept + : local_context_{*local_context}, git_api_{CreateFallbackApi(repo_config)} {} [[nodiscard]] auto CreateAction( @@ -73,9 +66,7 @@ class LocalApi final : public IExecutionApi { std::map<std::string, std::string> const& env_vars, std::map<std::string, std::string> const& properties) const noexcept -> IExecutionAction::Ptr final { - return IExecutionAction::Ptr{new LocalAction{&storage_config_, - &storage_, - &exec_config_, + return IExecutionAction::Ptr{new LocalAction{&local_context_, root_digest, command, output_files, @@ -100,7 +91,8 @@ class LocalApi final : public IExecutionApi { auto const& info = artifacts_info[i]; if (IsTreeObject(info.type)) { // read object infos from sub tree and call retrieve recursively - auto reader = TreeReader<LocalCasReader>{&storage_.CAS()}; + auto reader = + TreeReader<LocalCasReader>{&local_context_.storage->CAS()}; auto const result = reader.RecursivelyReadTreeLeafs( info.digest, output_paths[i]); if (not result) { @@ -114,7 +106,7 @@ class LocalApi final : public IExecutionApi { } } else { - auto const blob_path = storage_.CAS().BlobPath( + auto const blob_path = local_context_.storage->CAS().BlobPath( info.digest, IsExecutableObject(info.type)); if (not blob_path) { if (git_api_ and not git_api_->RetrieveToPaths( @@ -142,7 +134,8 @@ class LocalApi final : public IExecutionApi { std::vector<Artifact::ObjectInfo> const& artifacts_info, std::vector<int> const& fds, bool raw_tree) const noexcept -> bool final { - auto dumper = StreamDumper<LocalCasReader>{&storage_.CAS()}; + auto dumper = + StreamDumper<LocalCasReader>{&local_context_.storage->CAS()}; return CommonRetrieveToFds( artifacts_info, fds, @@ -186,7 +179,8 @@ class LocalApi final : public IExecutionApi { auto const& info = missing_artifacts_info->back_map[dgst]; // Recursively process trees. if (IsTreeObject(info.type)) { - auto reader = TreeReader<LocalCasReader>{&storage_.CAS()}; + auto reader = + TreeReader<LocalCasReader>{&local_context_.storage->CAS()}; auto const& result = reader.ReadDirectTreeEntries( info.digest, std::filesystem::path{}); if (not result or not RetrieveToCas(result->infos, api)) { @@ -197,9 +191,9 @@ class LocalApi final : public IExecutionApi { // Determine artifact path. auto const& path = IsTreeObject(info.type) - ? storage_.CAS().TreePath(info.digest) - : storage_.CAS().BlobPath(info.digest, - IsExecutableObject(info.type)); + ? local_context_.storage->CAS().TreePath(info.digest) + : local_context_.storage->CAS().BlobPath( + info.digest, IsExecutableObject(info.type)); if (not path) { return false; } @@ -215,9 +209,11 @@ class LocalApi final : public IExecutionApi { ArtifactDigest digest = IsTreeObject(info.type) ? ArtifactDigest::Create<ObjectType::Tree>( - storage_config_.hash_function, *content) + local_context_.storage_config->hash_function, + *content) : ArtifactDigest::Create<ObjectType::File>( - storage_config_.hash_function, *content); + local_context_.storage_config->hash_function, + *content); // Collect blob and upload to remote CAS if transfer size reached. if (not UpdateContainerAndUpload<ArtifactDigest>( @@ -243,10 +239,11 @@ class LocalApi final : public IExecutionApi { -> std::optional<std::string> override { std::optional<std::filesystem::path> location{}; if (IsTreeObject(artifact_info.type)) { - location = storage_.CAS().TreePath(artifact_info.digest); + location = + local_context_.storage->CAS().TreePath(artifact_info.digest); } else { - location = storage_.CAS().BlobPath( + location = local_context_.storage->CAS().BlobPath( artifact_info.digest, IsExecutableObject(artifact_info.type)); } std::optional<std::string> content = std::nullopt; @@ -266,8 +263,9 @@ class LocalApi final : public IExecutionApi { auto const is_tree = NativeSupport::IsTree( static_cast<bazel_re::Digest>(blob.digest).hash()); auto cas_digest = - is_tree ? storage_.CAS().StoreTree(*blob.data) - : storage_.CAS().StoreBlob(*blob.data, blob.is_exec); + is_tree ? local_context_.storage->CAS().StoreTree(*blob.data) + : local_context_.storage->CAS().StoreBlob(*blob.data, + blob.is_exec); if (not cas_digest or not std::equal_to<bazel_re::Digest>{}( *cas_digest, blob.digest)) { return false; @@ -290,7 +288,7 @@ class LocalApi final : public IExecutionApi { return CommonUploadTreeCompatible( *this, *build_root, - [&cas = storage_.CAS()]( + [&cas = local_context_.storage->CAS()]( std::vector<bazel_re::Digest> const& digests, std::vector<std::string>* targets) { targets->reserve(digests.size()); @@ -309,18 +307,19 @@ class LocalApi final : public IExecutionApi { -> bool final { return static_cast<bool>( NativeSupport::IsTree(static_cast<bazel_re::Digest>(digest).hash()) - ? storage_.CAS().TreePath(digest) - : storage_.CAS().BlobPath(digest, false)); + ? local_context_.storage->CAS().TreePath(digest) + : local_context_.storage->CAS().BlobPath(digest, false)); } [[nodiscard]] auto IsAvailable(std::vector<ArtifactDigest> const& digests) const noexcept -> std::vector<ArtifactDigest> final { std::vector<ArtifactDigest> result; for (auto const& digest : digests) { - auto const& path = NativeSupport::IsTree( - static_cast<bazel_re::Digest>(digest).hash()) - ? storage_.CAS().TreePath(digest) - : storage_.CAS().BlobPath(digest, false); + auto const& path = + NativeSupport::IsTree( + static_cast<bazel_re::Digest>(digest).hash()) + ? local_context_.storage->CAS().TreePath(digest) + : local_context_.storage->CAS().BlobPath(digest, false); if (not path) { result.push_back(digest); } @@ -332,7 +331,8 @@ class LocalApi final : public IExecutionApi { const noexcept -> std::optional<std::vector<ArtifactDigest>> final { Logger::Log(LogLevel::Debug, "SplitBlob({})", blob_digest.hash()); auto split_result = CASUtils::SplitBlobFastCDC( - static_cast<bazel_re::Digest>(blob_digest), storage_); + static_cast<bazel_re::Digest>(blob_digest), + *local_context_.storage); if (not split_result) { Logger::Log(LogLevel::Error, split_result.error().error_message()); return std::nullopt; @@ -381,8 +381,10 @@ class LocalApi final : public IExecutionApi { [](auto const& artifact_digest) { return static_cast<bazel_re::Digest>(artifact_digest); }); - auto splice_result = CASUtils::SpliceBlob( - static_cast<bazel_re::Digest>(blob_digest), digests, storage_); + auto splice_result = + CASUtils::SpliceBlob(static_cast<bazel_re::Digest>(blob_digest), + digests, + *local_context_.storage); if (not splice_result) { Logger::Log(LogLevel::Error, splice_result.error().error_message()); return std::nullopt; @@ -395,9 +397,7 @@ class LocalApi final : public IExecutionApi { } private: - StorageConfig const& storage_config_; - Storage const& storage_; - LocalExecutionConfig const& exec_config_; + LocalContext const& local_context_; std::optional<GitApi> const git_api_; [[nodiscard]] static auto CreateFallbackApi( diff --git a/src/buildtool/serve_api/serve_service/TARGETS b/src/buildtool/serve_api/serve_service/TARGETS index 8e59ccd8..7dbcc01d 100644 --- a/src/buildtool/serve_api/serve_service/TARGETS +++ b/src/buildtool/serve_api/serve_service/TARGETS @@ -110,6 +110,7 @@ , ["src/buildtool/common/remote", "retry_config"] , ["src/buildtool/execution_api/local", "context"] , ["src/buildtool/execution_api/remote", "config"] + , ["src/buildtool/execution_api/local", "context"] , ["src/buildtool/file_system", "file_system_manager"] , ["src/buildtool/graph_traverser", "graph_traverser"] , ["src/buildtool/logging", "log_level"] diff --git a/test/buildtool/execution_api/local/TARGETS b/test/buildtool/execution_api/local/TARGETS index 41870428..46b5abb1 100644 --- a/test/buildtool/execution_api/local/TARGETS +++ b/test/buildtool/execution_api/local/TARGETS @@ -8,6 +8,7 @@ , ["@", "src", "src/buildtool/common", "artifact_description"] , ["@", "src", "src/buildtool/common", "config"] , ["@", "src", "src/buildtool/execution_api/local", "config"] + , ["@", "src", "src/buildtool/execution_api/local", "context"] , ["@", "src", "src/buildtool/execution_api/local", "local"] , ["@", "src", "src/buildtool/file_system", "file_system_manager"] , ["@", "src", "src/buildtool/logging", "log_level"] @@ -26,6 +27,7 @@ [ ["@", "catch2", "", "catch2"] , ["", "catch-main"] , ["@", "src", "src/buildtool/execution_api/local", "config"] + , ["@", "src", "src/buildtool/execution_api/local", "context"] , ["@", "src", "src/buildtool/execution_api/local", "local"] , ["buildtool/execution_api/common", "api_test"] , ["utils", "test_storage_config"] diff --git a/test/buildtool/execution_api/local/local_api.test.cpp b/test/buildtool/execution_api/local/local_api.test.cpp index 33bd8d38..22d6bdbd 100644 --- a/test/buildtool/execution_api/local/local_api.test.cpp +++ b/test/buildtool/execution_api/local/local_api.test.cpp @@ -17,6 +17,7 @@ #include "catch2/catch_test_macros.hpp" #include "src/buildtool/execution_api/local/config.hpp" +#include "src/buildtool/execution_api/local/context.hpp" #include "src/buildtool/execution_api/local/local_api.hpp" #include "src/buildtool/storage/config.hpp" #include "src/buildtool/storage/storage.hpp" @@ -27,23 +28,15 @@ namespace { class FactoryApi final { public: explicit FactoryApi( - gsl::not_null<StorageConfig const*> const& storage_config, - gsl::not_null<Storage const*> const& storage, - gsl::not_null<LocalExecutionConfig const*> const& - local_exec_config) noexcept - : storage_config_{*storage_config}, - storage_{*storage}, - local_exec_config_{*local_exec_config} {} + gsl::not_null<LocalContext const*> const& local_context) noexcept + : local_context_{*local_context} {} [[nodiscard]] auto operator()() const -> IExecutionApi::Ptr { - return IExecutionApi::Ptr{ - new LocalApi{&storage_config_, &storage_, &local_exec_config_}}; + return IExecutionApi::Ptr{new LocalApi{&local_context_}}; } private: - StorageConfig const& storage_config_; - Storage const& storage_; - LocalExecutionConfig const& local_exec_config_; + LocalContext const& local_context_; }; } // namespace @@ -52,7 +45,11 @@ TEST_CASE("LocalAPI: No input, no output", "[execution_api]") { auto const storage_config = TestStorageConfig::Create(); auto const storage = Storage::Create(&storage_config.Get()); auto const local_exec_config = CreateLocalExecConfig(); - FactoryApi api_factory(&storage_config.Get(), &storage, &local_exec_config); + // pack the local context instances to be passed to LocalApi + LocalContext const local_context{.exec_config = &local_exec_config, + .storage_config = &storage_config.Get(), + .storage = &storage}; + FactoryApi api_factory(&local_context); TestNoInputNoOutput(api_factory, {}, /*is_hermetic=*/true); } @@ -61,7 +58,11 @@ TEST_CASE("LocalAPI: No input, create output", "[execution_api]") { auto const storage_config = TestStorageConfig::Create(); auto const storage = Storage::Create(&storage_config.Get()); auto const local_exec_config = CreateLocalExecConfig(); - FactoryApi api_factory(&storage_config.Get(), &storage, &local_exec_config); + // pack the local context instances to be passed to LocalApi + LocalContext const local_context{.exec_config = &local_exec_config, + .storage_config = &storage_config.Get(), + .storage = &storage}; + FactoryApi api_factory(&local_context); TestNoInputCreateOutput(api_factory, {}, /*is_hermetic=*/true); } @@ -70,7 +71,11 @@ TEST_CASE("LocalAPI: One input copied to output", "[execution_api]") { auto const storage_config = TestStorageConfig::Create(); auto const storage = Storage::Create(&storage_config.Get()); auto const local_exec_config = CreateLocalExecConfig(); - FactoryApi api_factory(&storage_config.Get(), &storage, &local_exec_config); + // pack the local context instances to be passed to LocalApi + LocalContext const local_context{.exec_config = &local_exec_config, + .storage_config = &storage_config.Get(), + .storage = &storage}; + FactoryApi api_factory(&local_context); TestOneInputCopiedToOutput(api_factory, {}, /*is_hermetic=*/true); } @@ -79,7 +84,11 @@ TEST_CASE("LocalAPI: Non-zero exit code, create output", "[execution_api]") { auto const storage_config = TestStorageConfig::Create(); auto const storage = Storage::Create(&storage_config.Get()); auto const local_exec_config = CreateLocalExecConfig(); - FactoryApi api_factory(&storage_config.Get(), &storage, &local_exec_config); + // pack the local context instances to be passed to LocalApi + LocalContext const local_context{.exec_config = &local_exec_config, + .storage_config = &storage_config.Get(), + .storage = &storage}; + FactoryApi api_factory(&local_context); TestNonZeroExitCodeCreateOutput(api_factory, {}); } @@ -88,7 +97,11 @@ TEST_CASE("LocalAPI: Retrieve two identical trees to path", "[execution_api]") { auto const storage_config = TestStorageConfig::Create(); auto const storage = Storage::Create(&storage_config.Get()); auto const local_exec_config = CreateLocalExecConfig(); - FactoryApi api_factory(&storage_config.Get(), &storage, &local_exec_config); + // pack the local context instances to be passed to LocalApi + LocalContext const local_context{.exec_config = &local_exec_config, + .storage_config = &storage_config.Get(), + .storage = &storage}; + FactoryApi api_factory(&local_context); TestRetrieveTwoIdenticalTreesToPath( api_factory, {}, "two_trees", /*is_hermetic=*/true); @@ -99,7 +112,11 @@ TEST_CASE("LocalAPI: Retrieve file and symlink with same content to path", auto const storage_config = TestStorageConfig::Create(); auto const storage = Storage::Create(&storage_config.Get()); auto const local_exec_config = CreateLocalExecConfig(); - FactoryApi api_factory(&storage_config.Get(), &storage, &local_exec_config); + // pack the local context instances to be passed to LocalApi + LocalContext const local_context{.exec_config = &local_exec_config, + .storage_config = &storage_config.Get(), + .storage = &storage}; + FactoryApi api_factory(&local_context); TestRetrieveFileAndSymlinkWithSameContentToPath( api_factory, {}, "file_and_symlink", /*is_hermetic=*/true); @@ -109,7 +126,11 @@ TEST_CASE("LocalAPI: Retrieve mixed blobs and trees", "[execution_api]") { auto const storage_config = TestStorageConfig::Create(); auto const storage = Storage::Create(&storage_config.Get()); auto const local_exec_config = CreateLocalExecConfig(); - FactoryApi api_factory(&storage_config.Get(), &storage, &local_exec_config); + // pack the local context instances to be passed to LocalApi + LocalContext const local_context{.exec_config = &local_exec_config, + .storage_config = &storage_config.Get(), + .storage = &storage}; + FactoryApi api_factory(&local_context); TestRetrieveMixedBlobsAndTrees( api_factory, {}, "blobs_and_trees", /*is_hermetic=*/true); @@ -119,7 +140,11 @@ TEST_CASE("LocalAPI: Create directory prior to execution", "[execution_api]") { auto const storage_config = TestStorageConfig::Create(); auto const storage = Storage::Create(&storage_config.Get()); auto const local_exec_config = CreateLocalExecConfig(); - FactoryApi api_factory(&storage_config.Get(), &storage, &local_exec_config); + // pack the local context instances to be passed to LocalApi + LocalContext const local_context{.exec_config = &local_exec_config, + .storage_config = &storage_config.Get(), + .storage = &storage}; + FactoryApi api_factory(&local_context); TestCreateDirPriorToExecution(api_factory, {}, /*is_hermetic=*/true); } diff --git a/test/buildtool/execution_api/local/local_execution.test.cpp b/test/buildtool/execution_api/local/local_execution.test.cpp index 8aea5950..a08f3b87 100644 --- a/test/buildtool/execution_api/local/local_execution.test.cpp +++ b/test/buildtool/execution_api/local/local_execution.test.cpp @@ -22,6 +22,7 @@ #include "src/buildtool/common/artifact_description.hpp" #include "src/buildtool/common/repository_config.hpp" #include "src/buildtool/execution_api/local/config.hpp" +#include "src/buildtool/execution_api/local/context.hpp" #include "src/buildtool/execution_api/local/local_api.hpp" #include "src/buildtool/file_system/file_system_manager.hpp" #include "src/buildtool/logging/log_level.hpp" @@ -64,11 +65,16 @@ namespace { TEST_CASE("LocalExecution: No input, no output", "[execution_api]") { auto const storage_config = TestStorageConfig::Create(); auto const storage = Storage::Create(&storage_config.Get()); + auto const local_exec_config = CreateLocalExecConfig(); + + // pack the local context instances to be passed to LocalApi + LocalContext const local_context{.exec_config = &local_exec_config, + .storage_config = &storage_config.Get(), + .storage = &storage}; RepositoryConfig repo_config{}; - auto const local_exec_config = CreateLocalExecConfig(); - auto api = LocalApi( - &storage_config.Get(), &storage, &local_exec_config, &repo_config); + + auto api = LocalApi(&local_context, &repo_config); std::string test_content("test"); std::vector<std::string> const cmdline = {"echo", "-n", test_content}; @@ -112,11 +118,16 @@ TEST_CASE("LocalExecution: No input, no output, env variables used", "[execution_api]") { auto const storage_config = TestStorageConfig::Create(); auto const storage = Storage::Create(&storage_config.Get()); + auto const local_exec_config = CreateLocalExecConfig(); + + // pack the local context instances to be passed to LocalApi + LocalContext const local_context{.exec_config = &local_exec_config, + .storage_config = &storage_config.Get(), + .storage = &storage}; RepositoryConfig repo_config{}; - auto const local_exec_config = CreateLocalExecConfig(); - auto api = LocalApi( - &storage_config.Get(), &storage, &local_exec_config, &repo_config); + + auto api = LocalApi(&local_context, &repo_config); std::string test_content("test from env var"); std::vector<std::string> const cmdline = { @@ -165,11 +176,16 @@ TEST_CASE("LocalExecution: No input, no output, env variables used", TEST_CASE("LocalExecution: No input, create output", "[execution_api]") { auto const storage_config = TestStorageConfig::Create(); auto const storage = Storage::Create(&storage_config.Get()); + auto const local_exec_config = CreateLocalExecConfig(); + + // pack the local context instances to be passed to LocalApi + LocalContext const local_context{.exec_config = &local_exec_config, + .storage_config = &storage_config.Get(), + .storage = &storage}; RepositoryConfig repo_config{}; - auto const local_exec_config = CreateLocalExecConfig(); - auto api = LocalApi( - &storage_config.Get(), &storage, &local_exec_config, &repo_config); + + auto api = LocalApi(&local_context, &repo_config); std::string test_content("test"); auto test_digest = ArtifactDigest::Create<ObjectType::File>( @@ -225,11 +241,16 @@ TEST_CASE("LocalExecution: No input, create output", "[execution_api]") { TEST_CASE("LocalExecution: One input copied to output", "[execution_api]") { auto const storage_config = TestStorageConfig::Create(); auto const storage = Storage::Create(&storage_config.Get()); + auto const local_exec_config = CreateLocalExecConfig(); + + // pack the local context instances to be passed to LocalApi + LocalContext const local_context{.exec_config = &local_exec_config, + .storage_config = &storage_config.Get(), + .storage = &storage}; RepositoryConfig repo_config{}; - auto const local_exec_config = CreateLocalExecConfig(); - auto api = LocalApi( - &storage_config.Get(), &storage, &local_exec_config, &repo_config); + + auto api = LocalApi(&local_context, &repo_config); std::string test_content("test"); auto test_digest = ArtifactDigest::Create<ObjectType::File>( @@ -298,11 +319,16 @@ TEST_CASE("LocalExecution: One input copied to output", "[execution_api]") { TEST_CASE("LocalExecution: Cache failed action's result", "[execution_api]") { auto const storage_config = TestStorageConfig::Create(); auto const storage = Storage::Create(&storage_config.Get()); + auto const local_exec_config = CreateLocalExecConfig(); + + // pack the local context instances to be passed to LocalApi + LocalContext const local_context{.exec_config = &local_exec_config, + .storage_config = &storage_config.Get(), + .storage = &storage}; RepositoryConfig repo_config{}; - auto const local_exec_config = CreateLocalExecConfig(); - auto api = LocalApi( - &storage_config.Get(), &storage, &local_exec_config, &repo_config); + + auto api = LocalApi(&local_context, &repo_config); auto flag = GetTestDir() / "flag"; std::vector<std::string> const cmdline = { diff --git a/test/buildtool/execution_engine/executor/TARGETS b/test/buildtool/execution_engine/executor/TARGETS index c5a9b810..58a6dbdb 100644 --- a/test/buildtool/execution_engine/executor/TARGETS +++ b/test/buildtool/execution_engine/executor/TARGETS @@ -52,6 +52,7 @@ , ["@", "src", "src/buildtool/common", "common"] , ["@", "src", "src/buildtool/common", "config"] , ["@", "src", "src/buildtool/execution_api/local", "config"] + , ["@", "src", "src/buildtool/execution_api/local", "context"] , ["@", "src", "src/buildtool/execution_api/local", "local"] , ["@", "src", "src/buildtool/execution_api/remote", "config"] , ["@", "src", "src/buildtool/execution_engine/executor", "executor"] diff --git a/test/buildtool/execution_engine/executor/executor_api_local.test.cpp b/test/buildtool/execution_engine/executor/executor_api_local.test.cpp index 4fee58da..2e1dfaa2 100644 --- a/test/buildtool/execution_engine/executor/executor_api_local.test.cpp +++ b/test/buildtool/execution_engine/executor/executor_api_local.test.cpp @@ -18,6 +18,7 @@ #include "src/buildtool/common/repository_config.hpp" #include "src/buildtool/common/statistics.hpp" #include "src/buildtool/execution_api/local/config.hpp" +#include "src/buildtool/execution_api/local/context.hpp" #include "src/buildtool/execution_api/local/local_api.hpp" #include "src/buildtool/execution_api/remote/config.hpp" #include "src/buildtool/execution_engine/executor/executor.hpp" @@ -31,20 +32,29 @@ TEST_CASE("Executor<LocalApi>: Upload blob", "[executor]") { auto const storage_config = TestStorageConfig::Create(); auto const storage = Storage::Create(&storage_config.Get()); - LocalExecutionConfig local_exec_config{}; + + // pack the local context instances to be passed to LocalApi + LocalContext const local_context{.exec_config = &local_exec_config, + .storage_config = &storage_config.Get(), + .storage = &storage}; + RepositoryConfig repo_config{}; TestBlobUpload(&repo_config, [&] { - return std::make_unique<LocalApi>( - &storage_config.Get(), &storage, &local_exec_config, &repo_config); + return std::make_unique<LocalApi>(&local_context, &repo_config); }); } TEST_CASE("Executor<LocalApi>: Compile hello world", "[executor]") { auto const storage_config = TestStorageConfig::Create(); auto const storage = Storage::Create(&storage_config.Get()); - LocalExecutionConfig local_exec_config{}; + + // pack the local context instances to be passed to LocalApi + LocalContext const local_context{.exec_config = &local_exec_config, + .storage_config = &storage_config.Get(), + .storage = &storage}; + RepositoryConfig repo_config{}; Statistics stats{}; Progress progress{}; @@ -55,10 +65,7 @@ TEST_CASE("Executor<LocalApi>: Compile hello world", "[executor]") { &stats, &progress, [&] { - return std::make_unique<LocalApi>(&storage_config.Get(), - &storage, - &local_exec_config, - &repo_config); + return std::make_unique<LocalApi>(&local_context, &repo_config); }, &*auth_config); } @@ -66,8 +73,13 @@ TEST_CASE("Executor<LocalApi>: Compile hello world", "[executor]") { TEST_CASE("Executor<LocalApi>: Compile greeter", "[executor]") { auto const storage_config = TestStorageConfig::Create(); auto const storage = Storage::Create(&storage_config.Get()); - LocalExecutionConfig local_exec_config{}; + + // pack the local context instances to be passed to LocalApi + LocalContext const local_context{.exec_config = &local_exec_config, + .storage_config = &storage_config.Get(), + .storage = &storage}; + RepositoryConfig repo_config{}; Statistics stats{}; Progress progress{}; @@ -78,10 +90,7 @@ TEST_CASE("Executor<LocalApi>: Compile greeter", "[executor]") { &stats, &progress, [&] { - return std::make_unique<LocalApi>(&storage_config.Get(), - &storage, - &local_exec_config, - &repo_config); + return std::make_unique<LocalApi>(&local_context, &repo_config); }, &*auth_config); } @@ -89,8 +98,13 @@ TEST_CASE("Executor<LocalApi>: Compile greeter", "[executor]") { TEST_CASE("Executor<LocalApi>: Upload and download trees", "[executor]") { auto const storage_config = TestStorageConfig::Create(); auto const storage = Storage::Create(&storage_config.Get()); - LocalExecutionConfig local_exec_config{}; + + // pack the local context instances to be passed to LocalApi + LocalContext const local_context{.exec_config = &local_exec_config, + .storage_config = &storage_config.Get(), + .storage = &storage}; + RepositoryConfig repo_config{}; Statistics stats{}; Progress progress{}; @@ -101,10 +115,7 @@ TEST_CASE("Executor<LocalApi>: Upload and download trees", "[executor]") { &stats, &progress, [&] { - return std::make_unique<LocalApi>(&storage_config.Get(), - &storage, - &local_exec_config, - &repo_config); + return std::make_unique<LocalApi>(&local_context, &repo_config); }, &*auth_config); } @@ -112,8 +123,13 @@ TEST_CASE("Executor<LocalApi>: Upload and download trees", "[executor]") { TEST_CASE("Executor<LocalApi>: Retrieve output directories", "[executor]") { auto const storage_config = TestStorageConfig::Create(); auto const storage = Storage::Create(&storage_config.Get()); - LocalExecutionConfig local_exec_config{}; + + // pack the local context instances to be passed to LocalApi + LocalContext const local_context{.exec_config = &local_exec_config, + .storage_config = &storage_config.Get(), + .storage = &storage}; + RepositoryConfig repo_config{}; Statistics stats{}; Progress progress{}; @@ -124,10 +140,7 @@ TEST_CASE("Executor<LocalApi>: Retrieve output directories", "[executor]") { &stats, &progress, [&] { - return std::make_unique<LocalApi>(&storage_config.Get(), - &storage, - &local_exec_config, - &repo_config); + return std::make_unique<LocalApi>(&local_context, &repo_config); }, &*auth_config); } |