diff options
4 files changed, 75 insertions, 42 deletions
diff --git a/src/buildtool/execution_engine/executor/executor.hpp b/src/buildtool/execution_engine/executor/executor.hpp index fb3593c8..70dc7d2b 100644 --- a/src/buildtool/execution_engine/executor/executor.hpp +++ b/src/buildtool/execution_engine/executor/executor.hpp @@ -107,7 +107,8 @@ class ExecutorImpl { [[nodiscard]] static auto VerifyOrUploadArtifact( Logger const& logger, gsl::not_null<DependencyGraph::ArtifactNode const*> const& artifact, - gsl::not_null<IExecutionApi*> const& api) noexcept -> bool { + gsl::not_null<IExecutionApi*> const& remote_api, + gsl::not_null<IExecutionApi*> const& local_api) noexcept -> bool { auto const object_info_opt = artifact->Content().Info(); auto const file_path_opt = artifact->Content().FilePath(); // If there is no object info and no file path, the artifact can not be @@ -129,16 +130,24 @@ class ExecutorImpl { << std::endl; return oss.str(); }); - if (not api->IsAvailable(object_info_opt->digest) and - not VerifyOrUploadKnownArtifact( - api, - artifact->Content().Repository(), - object_info_opt->digest)) { - Logger::Log( - LogLevel::Error, - "artifact {} should be present in CAS but is missing.", - ToHexString(artifact->Content().Id())); - return false; + if (not remote_api->IsAvailable(object_info_opt->digest)) { + // Check if requested artifact is available in local CAS and + // upload to remote CAS in case it is. + if (local_api->IsAvailable(object_info_opt->digest) and + local_api->RetrieveToCas({*object_info_opt}, remote_api)) { + return true; + } + + if (not VerifyOrUploadKnownArtifact( + remote_api, + artifact->Content().Repository(), + object_info_opt->digest)) { + Logger::Log( + LogLevel::Error, + "artifact {} should be present in CAS but is missing.", + ToHexString(artifact->Content().Id())); + return false; + } } return true; } @@ -156,7 +165,7 @@ class ExecutorImpl { return oss.str(); }); auto repo = artifact->Content().Repository(); - auto new_info = UploadFile(api, repo, *file_path_opt); + auto new_info = UploadFile(remote_api, repo, *file_path_opt); if (not new_info) { Logger::Log(LogLevel::Error, "artifact in {} could not be uploaded to CAS.", @@ -532,10 +541,14 @@ class Executor { public: explicit Executor( - IExecutionApi* api, + IExecutionApi* local_api, + IExecutionApi* remote_api, std::map<std::string, std::string> properties, std::chrono::milliseconds timeout = IExecutionAction::kDefaultTimeout) - : api_{api}, properties_{std::move(properties)}, timeout_{timeout} {} + : local_api_{local_api}, + remote_api_{remote_api}, + properties_{std::move(properties)}, + timeout_{timeout} {} /// \brief Run an action in a blocking manner /// This method must be thread-safe as it could be called in parallel @@ -549,7 +562,7 @@ class Executor { auto const response = Impl::ExecuteAction( logger, action, - api_, + remote_api_, properties_, timeout_, action->NoCache() ? CF::DoNotCacheOutput : CF::CacheOutput); @@ -565,11 +578,13 @@ class Executor { gsl::not_null<DependencyGraph::ArtifactNode const*> const& artifact) const noexcept -> bool { Logger logger("artifact:" + ToHexString(artifact->Content().Id())); - return Impl::VerifyOrUploadArtifact(logger, artifact, api_); + return Impl::VerifyOrUploadArtifact( + logger, artifact, remote_api_, local_api_); } private: - gsl::not_null<IExecutionApi*> api_; + gsl::not_null<IExecutionApi*> local_api_; + gsl::not_null<IExecutionApi*> remote_api_; std::map<std::string, std::string> properties_; std::chrono::milliseconds timeout_; }; @@ -586,11 +601,13 @@ class Rebuilder { /// \param properties Platform properties for execution. /// \param timeout Timeout for action execution. Rebuilder( - IExecutionApi* api, + IExecutionApi* local_api, + IExecutionApi* remote_api, IExecutionApi* api_cached, std::map<std::string, std::string> properties, std::chrono::milliseconds timeout = IExecutionAction::kDefaultTimeout) - : api_{api}, + : local_api_{local_api}, + remote_api_{remote_api}, api_cached_{api_cached}, properties_{std::move(properties)}, timeout_{timeout} {} @@ -600,8 +617,12 @@ class Rebuilder { const noexcept -> bool { auto const& action_id = action->Content().Id(); Logger logger("rebuild:" + action_id); - auto response = Impl::ExecuteAction( - logger, action, api_, properties_, timeout_, CF::PretendCached); + auto response = Impl::ExecuteAction(logger, + action, + remote_api_, + properties_, + timeout_, + CF::PretendCached); if (not response) { return true; // action without response (e.g., tree action) @@ -630,7 +651,8 @@ class Rebuilder { gsl::not_null<DependencyGraph::ArtifactNode const*> const& artifact) const noexcept -> bool { Logger logger("artifact:" + ToHexString(artifact->Content().Id())); - return Impl::VerifyOrUploadArtifact(logger, artifact, api_); + return Impl::VerifyOrUploadArtifact( + logger, artifact, remote_api_, local_api_); } [[nodiscard]] auto DumpFlakyActions() const noexcept -> nlohmann::json { @@ -646,7 +668,8 @@ class Rebuilder { } private: - gsl::not_null<IExecutionApi*> api_; + gsl::not_null<IExecutionApi*> local_api_; + gsl::not_null<IExecutionApi*> remote_api_; gsl::not_null<IExecutionApi*> api_cached_; std::map<std::string, std::string> properties_; std::chrono::milliseconds timeout_; diff --git a/src/buildtool/graph_traverser/graph_traverser.hpp b/src/buildtool/graph_traverser/graph_traverser.hpp index 718df364..6e9fc20e 100644 --- a/src/buildtool/graph_traverser/graph_traverser.hpp +++ b/src/buildtool/graph_traverser/graph_traverser.hpp @@ -329,7 +329,8 @@ class GraphTraverser { [[nodiscard]] auto Traverse( DependencyGraph const& g, std::vector<ArtifactIdentifier> const& artifact_ids) const -> bool { - Executor executor{&(*remote_api_), + Executor executor{&(*local_api_), + &(*remote_api_), RemoteExecutionConfig::PlatformProperties(), clargs_.build.timeout}; bool traversing{}; @@ -355,7 +356,8 @@ class GraphTraverser { // setup rebuilder with api for cache endpoint auto api_cached = CreateExecutionApi(RemoteExecutionConfig::CacheAddress()); - Rebuilder executor{&(*remote_api_), + Rebuilder executor{&(*local_api_), + &(*remote_api_), &(*api_cached), RemoteExecutionConfig::PlatformProperties(), clargs_.build.timeout}; diff --git a/test/buildtool/execution_engine/executor/executor.test.cpp b/test/buildtool/execution_engine/executor/executor.test.cpp index 0ea030d0..73710f9a 100755 --- a/test/buildtool/execution_engine/executor/executor.test.cpp +++ b/test/buildtool/execution_engine/executor/executor.test.cpp @@ -130,8 +130,8 @@ class TestApi : public IExecutionApi { bool /*unused*/) noexcept -> bool final { return false; // not needed by Executor } - auto RetrieveToCas(std::vector<Artifact::ObjectInfo> const& artifacts_info, - gsl::not_null<IExecutionApi*> api) noexcept + auto RetrieveToCas(std::vector<Artifact::ObjectInfo> const& /*unused*/, + gsl::not_null<IExecutionApi*> const& /*unused*/) noexcept -> bool final { return false; // not needed by Executor } @@ -235,7 +235,7 @@ TEST_CASE("Executor: Process artifact", "[executor]") { SECTION("Processing succeeds for valid config") { auto api = TestApi::Ptr{new TestApi{config}}; - Executor runner{api.get(), {}}; + Executor runner{api.get(), api.get(), {}}; CHECK(runner.Process(g.ArtifactNodeWithId(local_cpp_id))); CHECK(runner.Process(g.ArtifactNodeWithId(known_cpp_id))); @@ -245,7 +245,7 @@ TEST_CASE("Executor: Process artifact", "[executor]") { config.artifacts["local.cpp"].uploads = false; auto api = TestApi::Ptr{new TestApi{config}}; - Executor runner{api.get(), {}}; + Executor runner{api.get(), api.get(), {}}; CHECK(not runner.Process(g.ArtifactNodeWithId(local_cpp_id))); CHECK(runner.Process(g.ArtifactNodeWithId(known_cpp_id))); @@ -255,7 +255,7 @@ TEST_CASE("Executor: Process artifact", "[executor]") { config.artifacts["known.cpp"].available = false; auto api = TestApi::Ptr{new TestApi{config}}; - Executor runner{api.get(), {}}; + Executor runner{api.get(), api.get(), {}}; CHECK(runner.Process(g.ArtifactNodeWithId(local_cpp_id))); CHECK(not runner.Process(g.ArtifactNodeWithId(known_cpp_id))); @@ -288,7 +288,7 @@ TEST_CASE("Executor: Process action", "[executor]") { SECTION("Processing succeeds for valid config") { auto api = TestApi::Ptr{new TestApi{config}}; - Executor runner{api.get(), {}}; + Executor runner{api.get(), api.get(), {}}; CHECK(runner.Process(g.ArtifactNodeWithId(local_cpp_id))); CHECK(runner.Process(g.ArtifactNodeWithId(known_cpp_id))); @@ -301,7 +301,7 @@ TEST_CASE("Executor: Process action", "[executor]") { config.response.cached = false; auto api = TestApi::Ptr{new TestApi{config}}; - Executor runner{api.get(), {}}; + Executor runner{api.get(), api.get(), {}}; CHECK(runner.Process(g.ArtifactNodeWithId(local_cpp_id))); CHECK(runner.Process(g.ArtifactNodeWithId(known_cpp_id))); @@ -314,7 +314,7 @@ TEST_CASE("Executor: Process action", "[executor]") { config.artifacts["output2.exe"].available = false; auto api = TestApi::Ptr{new TestApi{config}}; - Executor runner{api.get(), {}}; + Executor runner{api.get(), api.get(), {}}; CHECK(runner.Process(g.ArtifactNodeWithId(local_cpp_id))); CHECK(runner.Process(g.ArtifactNodeWithId(known_cpp_id))); @@ -330,7 +330,7 @@ TEST_CASE("Executor: Process action", "[executor]") { config.execution.failed = true; auto api = TestApi::Ptr{new TestApi{config}}; - Executor runner{api.get(), {}}; + Executor runner{api.get(), api.get(), {}}; CHECK(runner.Process(g.ArtifactNodeWithId(local_cpp_id))); CHECK(runner.Process(g.ArtifactNodeWithId(known_cpp_id))); @@ -343,7 +343,7 @@ TEST_CASE("Executor: Process action", "[executor]") { config.response.exit_code = 1; auto api = TestApi::Ptr{new TestApi{config}}; - Executor runner{api.get(), {}}; + Executor runner{api.get(), api.get(), {}}; CHECK(runner.Process(g.ArtifactNodeWithId(local_cpp_id))); CHECK(runner.Process(g.ArtifactNodeWithId(known_cpp_id))); @@ -359,7 +359,7 @@ TEST_CASE("Executor: Process action", "[executor]") { config.execution.outputs = {"output1.exe" /*, "output2.exe"*/}; auto api = TestApi::Ptr{new TestApi{config}}; - Executor runner{api.get(), {}}; + Executor runner{api.get(), api.get(), {}}; CHECK(runner.Process(g.ArtifactNodeWithId(local_cpp_id))); CHECK(runner.Process(g.ArtifactNodeWithId(known_cpp_id))); diff --git a/test/buildtool/execution_engine/executor/executor_api.test.hpp b/test/buildtool/execution_engine/executor/executor_api.test.hpp index 4fe06aa1..58b19897 100755 --- a/test/buildtool/execution_engine/executor/executor_api.test.hpp +++ b/test/buildtool/execution_engine/executor/executor_api.test.hpp @@ -88,7 +88,8 @@ static inline void RunHelloWorldCompilation(ApiFactory const& factory, CHECK(g.ArtifactNodeWithId(exec_id)->HasBuilderAction()); auto api = factory(); - Executor runner{api.get(), RemoteExecutionConfig::PlatformProperties()}; + Executor runner{ + api.get(), api.get(), RemoteExecutionConfig::PlatformProperties()}; // upload local artifacts auto const* main_cpp_node = g.ArtifactNodeWithId(main_cpp_id); @@ -190,7 +191,8 @@ static inline void RunGreeterCompilation(ApiFactory const& factory, CHECK(g.Add({compile_greet_desc, make_lib_desc, make_exe_desc})); auto api = factory(); - Executor runner{api.get(), RemoteExecutionConfig::PlatformProperties()}; + Executor runner{ + api.get(), api.get(), RemoteExecutionConfig::PlatformProperties()}; // upload local artifacts for (auto const& id : {greet_hpp_id, greet_cpp_id, main_cpp_id}) { @@ -296,7 +298,8 @@ static inline void TestUploadAndDownloadTrees(ApiFactory const& factory, auto foo_id = g.AddArtifact(foo_desc); auto bar_id = g.AddArtifact(bar_desc); - Executor runner{api.get(), RemoteExecutionConfig::PlatformProperties()}; + Executor runner{ + api.get(), api.get(), RemoteExecutionConfig::PlatformProperties()}; REQUIRE(runner.Process(g.ArtifactNodeWithId(foo_id))); REQUIRE(runner.Process(g.ArtifactNodeWithId(bar_id))); @@ -435,7 +438,8 @@ static inline void TestRetrieveOutputDirectories(ApiFactory const& factory, // run action auto api = factory(); - Executor runner{api.get(), RemoteExecutionConfig::PlatformProperties()}; + Executor runner{ + api.get(), api.get(), RemoteExecutionConfig::PlatformProperties()}; REQUIRE(runner.Process(action)); // read output @@ -478,7 +482,8 @@ static inline void TestRetrieveOutputDirectories(ApiFactory const& factory, // run action auto api = factory(); - Executor runner{api.get(), RemoteExecutionConfig::PlatformProperties()}; + Executor runner{ + api.get(), api.get(), RemoteExecutionConfig::PlatformProperties()}; REQUIRE(runner.Process(action)); // read output @@ -537,7 +542,8 @@ static inline void TestRetrieveOutputDirectories(ApiFactory const& factory, // run action auto api = factory(); - Executor runner{api.get(), RemoteExecutionConfig::PlatformProperties()}; + Executor runner{ + api.get(), api.get(), RemoteExecutionConfig::PlatformProperties()}; REQUIRE(runner.Process(action)); // read output @@ -602,6 +608,7 @@ static inline void TestRetrieveOutputDirectories(ApiFactory const& factory, // run action auto api = factory(); Executor runner{api.get(), + api.get(), RemoteExecutionConfig::PlatformProperties()}; CHECK_FALSE(runner.Process(action)); } @@ -621,6 +628,7 @@ static inline void TestRetrieveOutputDirectories(ApiFactory const& factory, // run action auto api = factory(); Executor runner{api.get(), + api.get(), RemoteExecutionConfig::PlatformProperties()}; CHECK_FALSE(runner.Process(action)); } |