diff options
5 files changed, 104 insertions, 27 deletions
diff --git a/src/buildtool/execution_engine/executor/executor.hpp b/src/buildtool/execution_engine/executor/executor.hpp index 70d32ed6..73a0a89a 100644 --- a/src/buildtool/execution_engine/executor/executor.hpp +++ b/src/buildtool/execution_engine/executor/executor.hpp @@ -62,6 +62,7 @@ class ExecutorImpl { IExecutionApi const& api, ExecutionProperties const& properties, std::vector<DispatchEndpoint> const& dispatch_list, + HashFunction hash_function, gsl::not_null<Auth const*> const& auth, gsl::not_null<RetryConfig const*> const& retry_config, std::chrono::milliseconds const& timeout, @@ -112,7 +113,7 @@ class ExecutorImpl { } auto alternative_api = GetAlternativeEndpoint( - properties, dispatch_list, auth, retry_config); + properties, dispatch_list, auth, retry_config, hash_function); if (alternative_api) { if (not api.ParallelRetrieveToCas( std::vector<Artifact::ObjectInfo>{Artifact::ObjectInfo{ @@ -175,7 +176,8 @@ class ExecutorImpl { gsl::not_null<DependencyGraph::ArtifactNode const*> const& artifact, gsl::not_null<const RepositoryConfig*> const& repo_config, IExecutionApi const& remote_api, - IExecutionApi const& local_api) noexcept -> bool { + IExecutionApi const& local_api, + HashFunction hash_function) 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 @@ -233,8 +235,8 @@ class ExecutorImpl { return oss.str(); }); auto repo = artifact->Content().Repository(); - auto new_info = - UploadFile(remote_api, repo, repo_config, *file_path_opt); + auto new_info = UploadFile( + remote_api, hash_function, repo, repo_config, *file_path_opt); if (not new_info) { Logger::Log(LogLevel::Error, "artifact in {} could not be uploaded to CAS.", @@ -441,6 +443,7 @@ class ExecutorImpl { /// \returns The computed object info on success [[nodiscard]] static auto UploadFile( IExecutionApi const& api, + HashFunction hash_function, std::string const& repo, gsl::not_null<const RepositoryConfig*> const& repo_config, std::filesystem::path const& file_path) noexcept @@ -457,8 +460,8 @@ class ExecutorImpl { if (not content.has_value()) { return std::nullopt; } - auto digest = ArtifactDigest::Create<ObjectType::File>( - HashFunction::Instance(), *content); + auto digest = + ArtifactDigest::Create<ObjectType::File>(hash_function, *content); if (not api.Upload(ArtifactBlobContainer{ {ArtifactBlob{digest, std::move(*content), @@ -671,8 +674,8 @@ class ExecutorImpl { const ExecutionProperties& properties, const std::vector<DispatchEndpoint>& dispatch_list, const gsl::not_null<Auth const*>& auth, - const gsl::not_null<RetryConfig const*>& retry_config) - -> std::unique_ptr<BazelApi> { + const gsl::not_null<RetryConfig const*>& retry_config, + HashFunction hash_function) -> std::unique_ptr<BazelApi> { for (auto const& [pred, endpoint] : dispatch_list) { bool match = true; for (auto const& [k, v] : pred) { @@ -694,7 +697,7 @@ class ExecutorImpl { auth, retry_config, config, - HashFunction::Instance()); + hash_function); } } return nullptr; @@ -713,6 +716,7 @@ class Executor { gsl::not_null<IExecutionApi const*> const& remote_api, ExecutionProperties properties, std::vector<DispatchEndpoint> dispatch_list, + HashFunction hash_function, gsl::not_null<Auth const*> const& auth, gsl::not_null<RetryConfig const*> const& retry_config, gsl::not_null<Statistics*> const& stats, @@ -724,6 +728,7 @@ class Executor { remote_api_{*remote_api}, properties_{std::move(properties)}, dispatch_list_{std::move(dispatch_list)}, + hash_function_{hash_function}, auth_{*auth}, retry_config_{*retry_config}, stats_{stats}, @@ -748,6 +753,7 @@ class Executor { Impl::MergeProperties(properties_, action->ExecutionProperties()), dispatch_list_, + hash_function_, &auth_, &retry_config_, Impl::ScaleTime(timeout_, action->TimeoutScale()), @@ -768,6 +774,7 @@ class Executor { remote_api_, Impl::MergeProperties(properties_, action->ExecutionProperties()), dispatch_list_, + hash_function_, &auth_, &retry_config_, Impl::ScaleTime(timeout_, action->TimeoutScale()), @@ -791,13 +798,21 @@ class Executor { // to avoid always creating a logger we might not need, which is a // non-copyable and non-movable object, we need some code duplication if (logger_ != nullptr) { - return Impl::VerifyOrUploadArtifact( - *logger_, artifact, repo_config_, remote_api_, local_api_); + return Impl::VerifyOrUploadArtifact(*logger_, + artifact, + repo_config_, + remote_api_, + local_api_, + hash_function_); } Logger logger("artifact:" + ToHexString(artifact->Content().Id())); - return Impl::VerifyOrUploadArtifact( - logger, artifact, repo_config_, remote_api_, local_api_); + return Impl::VerifyOrUploadArtifact(logger, + artifact, + repo_config_, + remote_api_, + local_api_, + hash_function_); } private: @@ -806,6 +821,7 @@ class Executor { IExecutionApi const& remote_api_; ExecutionProperties properties_; std::vector<DispatchEndpoint> dispatch_list_; + HashFunction const hash_function_; Auth const& auth_; RetryConfig const& retry_config_; gsl::not_null<Statistics*> stats_; @@ -832,6 +848,7 @@ class Rebuilder { gsl::not_null<IExecutionApi const*> const& api_cached, ExecutionProperties properties, std::vector<DispatchEndpoint> dispatch_list, + HashFunction hash_function, gsl::not_null<Auth const*> const& auth, gsl::not_null<RetryConfig const*> const& retry_config, gsl::not_null<Statistics*> const& stats, @@ -843,6 +860,7 @@ class Rebuilder { api_cached_{*api_cached}, properties_{std::move(properties)}, dispatch_list_{std::move(dispatch_list)}, + hash_function_{hash_function}, auth_{*auth}, retry_config_{*retry_config}, stats_{stats}, @@ -860,6 +878,7 @@ class Rebuilder { remote_api_, Impl::MergeProperties(properties_, action->ExecutionProperties()), dispatch_list_, + hash_function_, &auth_, &retry_config_, Impl::ScaleTime(timeout_, action->TimeoutScale()), @@ -878,6 +897,7 @@ class Rebuilder { api_cached_, Impl::MergeProperties(properties_, action->ExecutionProperties()), dispatch_list_, + hash_function_, &auth_, &retry_config_, Impl::ScaleTime(timeout_, action->TimeoutScale()), @@ -904,8 +924,12 @@ 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, repo_config_, remote_api_, local_api_); + return Impl::VerifyOrUploadArtifact(logger, + artifact, + repo_config_, + remote_api_, + local_api_, + hash_function_); } [[nodiscard]] auto DumpFlakyActions() const noexcept -> nlohmann::json { @@ -927,6 +951,7 @@ class Rebuilder { IExecutionApi const& api_cached_; ExecutionProperties properties_; std::vector<DispatchEndpoint> dispatch_list_; + HashFunction const hash_function_; Auth const& auth_; RetryConfig const& retry_config_; gsl::not_null<Statistics*> stats_; diff --git a/src/buildtool/graph_traverser/graph_traverser.hpp b/src/buildtool/graph_traverser/graph_traverser.hpp index ae154c6a..0bd9c9c2 100644 --- a/src/buildtool/graph_traverser/graph_traverser.hpp +++ b/src/buildtool/graph_traverser/graph_traverser.hpp @@ -364,6 +364,7 @@ class GraphTraverser { &*apis_.remote, platform_properties_, dispatch_list_, + HashFunction::Instance(), &apis_.auth, &apis_.retry_config, stats_, @@ -398,6 +399,7 @@ class GraphTraverser { &*api_cached, platform_properties_, dispatch_list_, + HashFunction::Instance(), &apis_.auth, &apis_.retry_config, stats_, diff --git a/test/buildtool/execution_engine/executor/TARGETS b/test/buildtool/execution_engine/executor/TARGETS index cb33854a..c5a9b810 100644 --- a/test/buildtool/execution_engine/executor/TARGETS +++ b/test/buildtool/execution_engine/executor/TARGETS @@ -13,6 +13,7 @@ , ["@", "src", "src/buildtool/file_system", "file_system_manager"] , ["@", "src", "src/buildtool/progress_reporting", "progress"] , ["@", "src", "src/buildtool/crypto", "hash_function"] + , ["@", "src", "src/buildtool/compatibility", "compatibility"] , ["@", "catch2", "", "catch2"] , ["@", "gsl", "", "gsl"] , ["utils", "test_remote_config"] @@ -33,6 +34,8 @@ , ["@", "src", "src/buildtool/execution_engine/dag", "dag"] , ["@", "src", "src/buildtool/execution_engine/executor", "executor"] , ["@", "src", "src/buildtool/progress_reporting", "progress"] + , ["@", "src", "src/buildtool/crypto", "hash_function"] + , ["@", "src", "src/buildtool/compatibility", "compatibility"] , ["", "catch-main"] , ["@", "catch2", "", "catch2"] , ["@", "gsl", "", "gsl"] diff --git a/test/buildtool/execution_engine/executor/executor.test.cpp b/test/buildtool/execution_engine/executor/executor.test.cpp index c1e8f06a..6a71f497 100644 --- a/test/buildtool/execution_engine/executor/executor.test.cpp +++ b/test/buildtool/execution_engine/executor/executor.test.cpp @@ -27,6 +27,8 @@ #include "src/buildtool/common/artifact_description.hpp" #include "src/buildtool/common/repository_config.hpp" #include "src/buildtool/common/statistics.hpp" +#include "src/buildtool/compatibility/compatibility.hpp" +#include "src/buildtool/crypto/hash_function.hpp" #include "src/buildtool/execution_api/common/execution_api.hpp" #include "src/buildtool/execution_engine/executor/executor.hpp" #include "src/buildtool/file_system/file_system_manager.hpp" @@ -270,6 +272,10 @@ TEST_CASE("Executor: Process artifact", "[executor]") { DependencyGraph g; auto [config, repo_config] = CreateTest(&g, workspace_path); + HashFunction const hash_function{Compatibility::IsCompatible() + ? HashFunction::JustHash::Compatible + : HashFunction::JustHash::Native}; + auto const local_cpp_id = ArtifactDescription::CreateLocal("local.cpp", "").Id(); @@ -289,6 +295,7 @@ TEST_CASE("Executor: Process artifact", "[executor]") { api.get(), /*properties=*/{}, /*dispatch_list=*/{}, + hash_function, &auth, &retry_config, &stats, @@ -311,6 +318,7 @@ TEST_CASE("Executor: Process artifact", "[executor]") { api.get(), /*properties=*/{}, /*dispatch_list=*/{}, + hash_function, &auth, &retry_config, &stats, @@ -333,6 +341,7 @@ TEST_CASE("Executor: Process artifact", "[executor]") { api.get(), /*properties=*/{}, /*dispatch_list=*/{}, + hash_function, &auth, &retry_config, &stats, @@ -350,6 +359,10 @@ TEST_CASE("Executor: Process action", "[executor]") { DependencyGraph g; auto [config, repo_config] = CreateTest(&g, workspace_path); + HashFunction const hash_function{Compatibility::IsCompatible() + ? HashFunction::JustHash::Compatible + : HashFunction::JustHash::Native}; + auto const local_cpp_id = ArtifactDescription::CreateLocal("local.cpp", "").Id(); @@ -376,6 +389,7 @@ TEST_CASE("Executor: Process action", "[executor]") { api.get(), /*properties=*/{}, /*dispatch_list=*/{}, + hash_function, &auth, &retry_config, &stats, @@ -401,6 +415,7 @@ TEST_CASE("Executor: Process action", "[executor]") { api.get(), /*properties=*/{}, /*dispatch_list=*/{}, + hash_function, &auth, &retry_config, &stats, @@ -426,6 +441,7 @@ TEST_CASE("Executor: Process action", "[executor]") { api.get(), /*properties=*/{}, /*dispatch_list=*/{}, + hash_function, &auth, &retry_config, &stats, @@ -454,6 +470,7 @@ TEST_CASE("Executor: Process action", "[executor]") { api.get(), /*properties=*/{}, /*dispatch_list=*/{}, + hash_function, &auth, &retry_config, &stats, @@ -479,6 +496,7 @@ TEST_CASE("Executor: Process action", "[executor]") { api.get(), /*properties=*/{}, /*dispatch_list=*/{}, + hash_function, &auth, &retry_config, &stats, @@ -507,6 +525,7 @@ TEST_CASE("Executor: Process action", "[executor]") { api.get(), /*properties=*/{}, /*dispatch_list=*/{}, + hash_function, &auth, &retry_config, &stats, diff --git a/test/buildtool/execution_engine/executor/executor_api.test.hpp b/test/buildtool/execution_engine/executor/executor_api.test.hpp index 7009b075..4990e805 100644 --- a/test/buildtool/execution_engine/executor/executor_api.test.hpp +++ b/test/buildtool/execution_engine/executor/executor_api.test.hpp @@ -29,6 +29,7 @@ #include "src/buildtool/common/remote/retry_config.hpp" #include "src/buildtool/common/repository_config.hpp" #include "src/buildtool/common/statistics.hpp" +#include "src/buildtool/compatibility/compatibility.hpp" #include "src/buildtool/crypto/hash_function.hpp" #include "src/buildtool/execution_api/common/execution_api.hpp" #include "src/buildtool/execution_api/remote/config.hpp" @@ -51,12 +52,15 @@ static inline void RunBlobUpload(RepositoryConfig* repo_config, ApiFactory const& factory) { SetupConfig(repo_config); auto api = factory(); + HashFunction const hash_function{Compatibility::IsCompatible() + ? HashFunction::JustHash::Compatible + : HashFunction::JustHash::Native}; + std::string const blob = "test"; CHECK(api->Upload(ArtifactBlobContainer{{ArtifactBlob{ - ArtifactDigest{ - HashFunction::Instance().ComputeBlobHash(blob).HexString(), - blob.size(), - /*is_tree=*/false}, + ArtifactDigest{hash_function.ComputeBlobHash(blob).HexString(), + blob.size(), + /*is_tree=*/false}, blob, /*is_exec=*/false}}})); } @@ -134,12 +138,17 @@ static inline void RunHelloWorldCompilation( RetryConfig retry_config{}; // default retry config + HashFunction const hash_function{Compatibility::IsCompatible() + ? HashFunction::JustHash::Compatible + : HashFunction::JustHash::Native}; + auto api = factory(); Executor runner{repo_config, api.get(), api.get(), remote_config->platform_properties, remote_config->dispatch, + hash_function, auth, &retry_config, stats, @@ -263,12 +272,17 @@ static inline void RunGreeterCompilation( RetryConfig retry_config{}; // default retry config + HashFunction const hash_function{Compatibility::IsCompatible() + ? HashFunction::JustHash::Compatible + : HashFunction::JustHash::Native}; + auto api = factory(); Executor runner{repo_config, api.get(), api.get(), remote_config->platform_properties, remote_config->dispatch, + hash_function, auth, &retry_config, stats, @@ -401,16 +415,20 @@ static inline void TestUploadAndDownloadTrees( env.emplace("PATH", "/bin:/usr/bin"); } + HashFunction const hash_function{Compatibility::IsCompatible() + ? HashFunction::JustHash::Compatible + : HashFunction::JustHash::Native}; + auto foo = std::string{"foo"}; auto bar = std::string{"bar"}; - auto foo_digest = ArtifactDigest{ - HashFunction::Instance().ComputeBlobHash(foo).HexString(), - foo.size(), - /*is_tree=*/false}; - auto bar_digest = ArtifactDigest{ - HashFunction::Instance().ComputeBlobHash(bar).HexString(), - bar.size(), - /*is_tree=*/false}; + auto foo_digest = + ArtifactDigest{hash_function.ComputeBlobHash(foo).HexString(), + foo.size(), + /*is_tree=*/false}; + auto bar_digest = + ArtifactDigest{hash_function.ComputeBlobHash(bar).HexString(), + bar.size(), + /*is_tree=*/false}; // upload blobs auto api = factory(); @@ -438,6 +456,7 @@ static inline void TestUploadAndDownloadTrees( api.get(), remote_config->platform_properties, remote_config->dispatch, + hash_function, auth, &retry_config, stats, @@ -555,6 +574,10 @@ static inline void TestRetrieveOutputDirectories( SetupConfig(repo_config); auto tmpdir = GetTestDir(); + HashFunction const hash_function{Compatibility::IsCompatible() + ? HashFunction::JustHash::Compatible + : HashFunction::JustHash::Native}; + auto const make_tree_id = std::string{"make_tree"}; auto const* make_tree_cmd = "mkdir -p baz/baz/\n" @@ -608,6 +631,7 @@ static inline void TestRetrieveOutputDirectories( api.get(), remote_config->platform_properties, remote_config->dispatch, + hash_function, auth, &retry_config, stats, @@ -663,6 +687,7 @@ static inline void TestRetrieveOutputDirectories( api.get(), remote_config->platform_properties, remote_config->dispatch, + hash_function, auth, &retry_config, stats, @@ -735,6 +760,7 @@ static inline void TestRetrieveOutputDirectories( api.get(), remote_config->platform_properties, remote_config->dispatch, + hash_function, auth, &retry_config, stats, @@ -809,6 +835,7 @@ static inline void TestRetrieveOutputDirectories( api.get(), remote_config->platform_properties, remote_config->dispatch, + hash_function, auth, &retry_config, stats, @@ -836,6 +863,7 @@ static inline void TestRetrieveOutputDirectories( api.get(), remote_config->platform_properties, remote_config->dispatch, + hash_function, auth, &retry_config, stats, |