diff options
author | Sascha Roloff <sascha.roloff@huawei.com> | 2022-10-13 11:39:13 +0200 |
---|---|---|
committer | Sascha Roloff <sascha.roloff@huawei.com> | 2022-10-14 12:46:42 +0200 |
commit | f7e4ebf0e2913ff1b987e40baab5c0c809e6c0de (patch) | |
tree | 09ae121b46743a7eb682d9ba1361a59566c558bc | |
parent | 4d53961509015534fa10a59f4b86a20ae8a66017 (diff) | |
download | justbuild-f7e4ebf0e2913ff1b987e40baab5c0c809e6c0de.tar.gz |
Remove default value of ArtifactDigest::Create template parameter
This enforces the explicit specification, which object type, either file or
tree, should be used to create an artifact digest. This also avoids subtile
errors at locations as in the previous commit, where files as well as trees are
supposed to be handled, but digest creation mistakenly defaults to file object
type.
20 files changed, 61 insertions, 50 deletions
diff --git a/src/buildtool/build_engine/target_map/built_in_rules.cpp b/src/buildtool/build_engine/target_map/built_in_rules.cpp index bac1f5cd..a34dfb03 100644 --- a/src/buildtool/build_engine/target_map/built_in_rules.cpp +++ b/src/buildtool/build_engine/target_map/built_in_rules.cpp @@ -183,7 +183,8 @@ void FileGenRuleWithDeps( auto stage = ExpressionPtr{Expression::map_t{ file_name_val->String(), ExpressionPtr{ArtifactDescription{ - ArtifactDigest::Create(data_val->String()), ObjectType::File}}}}; + ArtifactDigest::Create<ObjectType::File>(data_val->String()), + ObjectType::File}}}}; auto vars_set = std::unordered_set<std::string>{}; vars_set.insert(param_vars->begin(), param_vars->end()); diff --git a/src/buildtool/build_engine/target_map/target_map.cpp b/src/buildtool/build_engine/target_map/target_map.cpp index ac06250b..760e8306 100644 --- a/src/buildtool/build_engine/target_map/target_map.cpp +++ b/src/buildtool/build_engine/target_map/target_map.cpp @@ -582,7 +582,8 @@ void withDependencies( } blobs.emplace_back(data->String()); return ExpressionPtr{ArtifactDescription{ - ArtifactDigest::Create(data->String()), ObjectType::File}}; + ArtifactDigest::Create<ObjectType::File>(data->String()), + ObjectType::File}}; }}, {"TREE", [&trees](auto&& eval, auto const& expr, auto const& env) { diff --git a/src/buildtool/common/artifact_digest.hpp b/src/buildtool/common/artifact_digest.hpp index c9250cf9..11fab86d 100644 --- a/src/buildtool/common/artifact_digest.hpp +++ b/src/buildtool/common/artifact_digest.hpp @@ -70,7 +70,7 @@ class ArtifactDigest { return std::equal_to<bazel_re::Digest>{}(*this, other); } - template <ObjectType kType = ObjectType::File> + template <ObjectType kType> [[nodiscard]] static auto Create(std::string const& content) noexcept -> ArtifactDigest { if constexpr (kType == ObjectType::Tree) { diff --git a/src/buildtool/execution_api/bazel_msg/bazel_blob.hpp b/src/buildtool/execution_api/bazel_msg/bazel_blob.hpp index 524e2fc8..5e92a393 100644 --- a/src/buildtool/execution_api/bazel_msg/bazel_blob.hpp +++ b/src/buildtool/execution_api/bazel_msg/bazel_blob.hpp @@ -39,7 +39,8 @@ struct BazelBlob { if (not content.has_value()) { return std::nullopt; } - return BazelBlob{ArtifactDigest::Create(*content), *content}; + return BazelBlob{ArtifactDigest::Create<ObjectType::File>(*content), + *content}; } #endif // INCLUDED_SRC_BUILDTOOL_EXECUTION_API_BAZEL_MSG_BAZEL_BLOB_HPP diff --git a/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.cpp b/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.cpp index da1d6554..a9022e55 100644 --- a/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.cpp +++ b/src/buildtool/execution_api/bazel_msg/bazel_msg_factory.cpp @@ -274,7 +274,7 @@ template <class T> auto msg = CreateDirectoryNode(dir_name); auto content_creator = [&dir] { return SerializeMessage(dir); }; auto digest_creator = [](std::string const& content) -> bazel_re::Digest { - return ArtifactDigest::Create(content); + return ArtifactDigest::Create<ObjectType::File>(content); }; return DirectoryNodeBundle::Create(msg, content_creator, digest_creator); } @@ -305,7 +305,7 @@ template <class T> auto content_creator = [&msg] { return SerializeMessage(msg); }; auto digest_creator = [](std::string const& content) -> bazel_re::Digest { - return ArtifactDigest::Create(content); + return ArtifactDigest::Create<ObjectType::File>(content); }; return CommandBundle::Create(msg, content_creator, digest_creator); @@ -341,7 +341,7 @@ template <class T> auto content_creator = [&msg] { return SerializeMessage(msg); }; auto digest_creator = [](std::string const& content) -> bazel_re::Digest { - return ArtifactDigest::Create(content); + return ArtifactDigest::Create<ObjectType::File>(content); }; return ActionBundle::Create(msg, content_creator, digest_creator); diff --git a/src/buildtool/execution_api/remote/bazel/bazel_network.cpp b/src/buildtool/execution_api/remote/bazel/bazel_network.cpp index 0252328b..5bf254e1 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_network.cpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_network.cpp @@ -269,9 +269,9 @@ auto BazelNetwork::RecursivelyReadTreeLeafs( dir_map->reserve(dirs.size()); for (auto& dir : dirs) { try { - dir_map->emplace( - ArtifactDigest::Create(dir.SerializeAsString()), - std::move(dir)); + dir_map->emplace(ArtifactDigest::Create<ObjectType::File>( + dir.SerializeAsString()), + std::move(dir)); } catch (...) { return std::nullopt; } diff --git a/src/buildtool/execution_api/remote/bazel/bazel_response.cpp b/src/buildtool/execution_api/remote/bazel/bazel_response.cpp index 97fae4f9..615c0177 100644 --- a/src/buildtool/execution_api/remote/bazel/bazel_response.cpp +++ b/src/buildtool/execution_api/remote/bazel/bazel_response.cpp @@ -24,7 +24,7 @@ namespace { auto ProcessDirectoryMessage(bazel_re::Directory const& dir) noexcept -> std::optional<BazelBlob> { auto data = dir.SerializeAsString(); - auto digest = ArtifactDigest::Create(data); + auto digest = ArtifactDigest::Create<ObjectType::File>(data); return BazelBlob{std::move(digest), std::move(data)}; } diff --git a/src/buildtool/execution_engine/executor/executor.hpp b/src/buildtool/execution_engine/executor/executor.hpp index fc169200..ec280b01 100644 --- a/src/buildtool/execution_engine/executor/executor.hpp +++ b/src/buildtool/execution_engine/executor/executor.hpp @@ -382,7 +382,7 @@ class ExecutorImpl { if (not content.has_value()) { return std::nullopt; } - auto digest = ArtifactDigest::Create(*content); + auto digest = ArtifactDigest::Create<ObjectType::File>(*content); if (not api->Upload( BlobContainer{{BazelBlob{digest, std::move(*content)}}})) { return std::nullopt; diff --git a/src/buildtool/graph_traverser/graph_traverser.hpp b/src/buildtool/graph_traverser/graph_traverser.hpp index 916932fd..063c1b85 100644 --- a/src/buildtool/graph_traverser/graph_traverser.hpp +++ b/src/buildtool/graph_traverser/graph_traverser.hpp @@ -284,7 +284,7 @@ class GraphTraverser { std::vector<std::string> const& blobs) const noexcept -> bool { BlobContainer container; for (auto const& blob : blobs) { - auto digest = ArtifactDigest::Create(blob); + auto digest = ArtifactDigest::Create<ObjectType::File>(blob); Logger::Log(LogLevel::Trace, [&]() { return fmt::format( "Uploaded blob {}, its digest has id {} and size {}.", diff --git a/test/buildtool/execution_api/bazel/bazel_ac_client.test.cpp b/test/buildtool/execution_api/bazel/bazel_ac_client.test.cpp index 0d44849b..ca0c0568 100644 --- a/test/buildtool/execution_api/bazel/bazel_ac_client.test.cpp +++ b/test/buildtool/execution_api/bazel/bazel_ac_client.test.cpp @@ -43,7 +43,7 @@ TEST_CASE("Bazel internals: AC Client", "[!hide][execution_api]") { std::string instance_name{"remote-execution"}; std::string content("test"); - auto test_digest = ArtifactDigest::Create(content); + auto test_digest = ArtifactDigest::Create<ObjectType::File>(content); auto action_id = CreateAction(instance_name, {"echo", "-n", content}, diff --git a/test/buildtool/execution_api/bazel/bazel_cas_client.test.cpp b/test/buildtool/execution_api/bazel/bazel_cas_client.test.cpp index c44cf33c..1081c952 100644 --- a/test/buildtool/execution_api/bazel/bazel_cas_client.test.cpp +++ b/test/buildtool/execution_api/bazel/bazel_cas_client.test.cpp @@ -30,7 +30,7 @@ TEST_CASE("Bazel internals: CAS Client", "[execution_api]") { SECTION("Valid digest and blob") { // digest of "test" - auto digest = ArtifactDigest::Create(content); + auto digest = ArtifactDigest::Create<ObjectType::File>(content); // Valid blob BazelBlob blob{digest, content}; diff --git a/test/buildtool/execution_api/bazel/bazel_execution_client.test.cpp b/test/buildtool/execution_api/bazel/bazel_execution_client.test.cpp index d5e00ad8..d136e818 100755 --- a/test/buildtool/execution_api/bazel/bazel_execution_client.test.cpp +++ b/test/buildtool/execution_api/bazel/bazel_execution_client.test.cpp @@ -25,8 +25,8 @@ TEST_CASE("Bazel internals: Execution Client", "[execution_api]") { std::string instance_name{"remote-execution"}; std::string content("test"); - auto test_digest = - static_cast<bazel_re::Digest>(ArtifactDigest::Create(content)); + auto test_digest = static_cast<bazel_re::Digest>( + ArtifactDigest::Create<ObjectType::File>(content)); BazelExecutionClient execution_client(info->host, info->port); @@ -93,8 +93,8 @@ TEST_CASE("Bazel internals: Execution Client using env variables", std::string instance_name{"remote-execution"}; std::string content("contents of env variable"); - auto test_digest = - static_cast<bazel_re::Digest>(ArtifactDigest::Create(content)); + auto test_digest = static_cast<bazel_re::Digest>( + ArtifactDigest::Create<ObjectType::File>(content)); BazelExecutionClient execution_client(info->host, info->port); diff --git a/test/buildtool/execution_api/bazel/bazel_network.test.cpp b/test/buildtool/execution_api/bazel/bazel_network.test.cpp index 6d58d463..ccfcd695 100644 --- a/test/buildtool/execution_api/bazel/bazel_network.test.cpp +++ b/test/buildtool/execution_api/bazel/bazel_network.test.cpp @@ -30,9 +30,12 @@ TEST_CASE("Bazel network: write/read blobs", "[execution_api]") { std::string content_bar("bar"); std::string content_baz(kLargeSize, 'x'); // single larger blob - BazelBlob foo{ArtifactDigest::Create(content_foo), content_foo}; - BazelBlob bar{ArtifactDigest::Create(content_bar), content_bar}; - BazelBlob baz{ArtifactDigest::Create(content_baz), content_baz}; + BazelBlob foo{ArtifactDigest::Create<ObjectType::File>(content_foo), + content_foo}; + BazelBlob bar{ArtifactDigest::Create<ObjectType::File>(content_bar), + content_bar}; + BazelBlob baz{ArtifactDigest::Create<ObjectType::File>(content_baz), + content_baz}; // Search blobs via digest REQUIRE(network.UploadBlobs(BlobContainer{{foo, bar, baz}})); @@ -71,8 +74,10 @@ TEST_CASE("Bazel network: read blobs with unknown size", "[execution_api]") { std::string content_foo("foo"); std::string content_bar(kLargeSize, 'x'); // single larger blob - BazelBlob foo{ArtifactDigest::Create(content_foo), content_foo}; - BazelBlob bar{ArtifactDigest::Create(content_bar), content_bar}; + BazelBlob foo{ArtifactDigest::Create<ObjectType::File>(content_foo), + content_foo}; + BazelBlob bar{ArtifactDigest::Create<ObjectType::File>(content_bar), + content_bar}; // Upload blobs REQUIRE(network.UploadBlobs(BlobContainer{{foo, bar}})); diff --git a/test/buildtool/execution_api/bazel/bytestream_client.test.cpp b/test/buildtool/execution_api/bazel/bytestream_client.test.cpp index 3c2c9ca8..22d41437 100644 --- a/test/buildtool/execution_api/bazel/bytestream_client.test.cpp +++ b/test/buildtool/execution_api/bazel/bytestream_client.test.cpp @@ -32,8 +32,8 @@ TEST_CASE("ByteStream Client: Transfer single blob", "[execution_api]") { std::string content("foobar"); // digest of "foobar" - auto digest = - static_cast<bazel_re::Digest>(ArtifactDigest::Create(content)); + auto digest = static_cast<bazel_re::Digest>( + ArtifactDigest::Create<ObjectType::File>(content)); CHECK(stream.Write(fmt::format("{}/uploads/{}/blobs/{}/{}", instance_name, @@ -61,8 +61,8 @@ TEST_CASE("ByteStream Client: Transfer single blob", "[execution_api]") { } // digest of "instance_nameinstance_nameinstance_..." - auto digest = - static_cast<bazel_re::Digest>(ArtifactDigest::Create(content)); + auto digest = static_cast<bazel_re::Digest>( + ArtifactDigest::Create<ObjectType::File>(content)); CHECK(stream.Write(fmt::format("{}/uploads/{}/blobs/{}/{}", instance_name, @@ -108,9 +108,9 @@ TEST_CASE("ByteStream Client: Transfer multiple blobs", "[execution_api]") { SECTION("Upload small blobs") { std::string instance_name{"remote-execution"}; - BazelBlob foo{ArtifactDigest::Create("foo"), "foo"}; - BazelBlob bar{ArtifactDigest::Create("bar"), "bar"}; - BazelBlob baz{ArtifactDigest::Create("baz"), "baz"}; + BazelBlob foo{ArtifactDigest::Create<ObjectType::File>("foo"), "foo"}; + BazelBlob bar{ArtifactDigest::Create<ObjectType::File>("bar"), "bar"}; + BazelBlob baz{ArtifactDigest::Create<ObjectType::File>("baz"), "baz"}; CHECK(stream.WriteMany<BazelBlob>( {foo, bar, baz}, @@ -155,9 +155,12 @@ TEST_CASE("ByteStream Client: Transfer multiple blobs", "[execution_api]") { content_baz[i] = instance_name[(i + 2) % instance_name.size()]; } - BazelBlob foo{ArtifactDigest::Create(content_foo), content_foo}; - BazelBlob bar{ArtifactDigest::Create(content_bar), content_bar}; - BazelBlob baz{ArtifactDigest::Create(content_baz), content_baz}; + BazelBlob foo{ArtifactDigest::Create<ObjectType::File>(content_foo), + content_foo}; + BazelBlob bar{ArtifactDigest::Create<ObjectType::File>(content_bar), + content_bar}; + BazelBlob baz{ArtifactDigest::Create<ObjectType::File>(content_baz), + content_baz}; CHECK(stream.WriteMany<BazelBlob>( {foo, bar, baz}, diff --git a/test/buildtool/execution_api/common/api_test.hpp b/test/buildtool/execution_api/common/api_test.hpp index 08aa3bd2..fdc13643 100644 --- a/test/buildtool/execution_api/common/api_test.hpp +++ b/test/buildtool/execution_api/common/api_test.hpp @@ -103,7 +103,7 @@ using ExecProps = std::map<std::string, std::string>; ExecProps const& props, bool is_hermetic = false) { std::string test_content("test"); - auto test_digest = ArtifactDigest::Create(test_content); + auto test_digest = ArtifactDigest::Create<ObjectType::File>(test_content); std::string output_path{"output_file"}; @@ -180,7 +180,7 @@ using ExecProps = std::map<std::string, std::string>; ExecProps const& props, bool is_hermetic = false) { std::string test_content("test"); - auto test_digest = ArtifactDigest::Create(test_content); + auto test_digest = ArtifactDigest::Create<ObjectType::File>(test_content); auto input_artifact_opt = ArtifactFactory::FromDescription(ArtifactFactory::DescribeKnownArtifact( @@ -264,7 +264,7 @@ using ExecProps = std::map<std::string, std::string>; ApiFactory const& api_factory, ExecProps const& props) { std::string test_content("test"); - auto test_digest = ArtifactDigest::Create(test_content); + auto test_digest = ArtifactDigest::Create<ObjectType::File>(test_content); std::string output_path{"output_file"}; diff --git a/test/buildtool/execution_api/local/local_ac.test.cpp b/test/buildtool/execution_api/local/local_ac.test.cpp index 2e5e1819..eaee5a7c 100644 --- a/test/buildtool/execution_api/local/local_ac.test.cpp +++ b/test/buildtool/execution_api/local/local_ac.test.cpp @@ -30,7 +30,7 @@ TEST_CASE_METHOD(HermeticLocalTestFixture, LocalCAS cas{}; LocalAC ac{&cas}; - auto action_id = ArtifactDigest::Create("action"); + auto action_id = ArtifactDigest::Create<ObjectType::File>("action"); CHECK(not ac.CachedResult(action_id)); CHECK(RunDummyExecution(&ac, action_id, "result")); @@ -44,8 +44,8 @@ TEST_CASE_METHOD(HermeticLocalTestFixture, LocalCAS cas{}; LocalAC ac{&cas}; - auto action_id1 = ArtifactDigest::Create("action1"); - auto action_id2 = ArtifactDigest::Create("action2"); + auto action_id1 = ArtifactDigest::Create<ObjectType::File>("action1"); + auto action_id2 = ArtifactDigest::Create<ObjectType::File>("action2"); CHECK(not ac.CachedResult(action_id1)); CHECK(not ac.CachedResult(action_id2)); @@ -73,8 +73,8 @@ TEST_CASE_METHOD(HermeticLocalTestFixture, LocalCAS cas{}; LocalAC ac{&cas}; - auto action_id1 = ArtifactDigest::Create("action1"); - auto action_id2 = ArtifactDigest::Create("action2"); + auto action_id1 = ArtifactDigest::Create<ObjectType::File>("action1"); + auto action_id2 = ArtifactDigest::Create<ObjectType::File>("action2"); CHECK(not ac.CachedResult(action_id1)); CHECK(not ac.CachedResult(action_id2)); @@ -102,7 +102,7 @@ TEST_CASE_METHOD(HermeticLocalTestFixture, LocalCAS cas{}; LocalAC ac{&cas}; - auto action_id = ArtifactDigest::Create("same action"); + auto action_id = ArtifactDigest::Create<ObjectType::File>("same action"); CHECK(not ac.CachedResult(action_id)); std::string result_content1{}; diff --git a/test/buildtool/execution_api/local/local_cas.test.cpp b/test/buildtool/execution_api/local/local_cas.test.cpp index 182456d4..14a86628 100644 --- a/test/buildtool/execution_api/local/local_cas.test.cpp +++ b/test/buildtool/execution_api/local/local_cas.test.cpp @@ -22,7 +22,7 @@ TEST_CASE_METHOD(HermeticLocalTestFixture, "LocalCAS", "[execution_api]") { std::string test_content{"test"}; - auto test_digest = ArtifactDigest::Create(test_content); + auto test_digest = ArtifactDigest::Create<ObjectType::File>(test_content); SECTION("CAS for files") { LocalCAS<ObjectType::File> cas{}; diff --git a/test/buildtool/execution_api/local/local_execution.test.cpp b/test/buildtool/execution_api/local/local_execution.test.cpp index 5e69dde0..14dad898 100755 --- a/test/buildtool/execution_api/local/local_execution.test.cpp +++ b/test/buildtool/execution_api/local/local_execution.test.cpp @@ -132,7 +132,7 @@ TEST_CASE_METHOD(HermeticLocalTestFixture, auto api = LocalApi{}; std::string test_content("test"); - auto test_digest = ArtifactDigest::Create(test_content); + auto test_digest = ArtifactDigest::Create<ObjectType::File>(test_content); std::string output_path{"output_file"}; std::vector<std::string> const cmdline = { @@ -187,7 +187,7 @@ TEST_CASE_METHOD(HermeticLocalTestFixture, auto api = LocalApi{}; std::string test_content("test"); - auto test_digest = ArtifactDigest::Create(test_content); + auto test_digest = ArtifactDigest::Create<ObjectType::File>(test_content); REQUIRE(api.Upload(BlobContainer{{BazelBlob{test_digest, test_content}}}, false)); diff --git a/test/buildtool/execution_api/local/local_storage.test.cpp b/test/buildtool/execution_api/local/local_storage.test.cpp index 63b2cc4b..b85fc24d 100644 --- a/test/buildtool/execution_api/local/local_storage.test.cpp +++ b/test/buildtool/execution_api/local/local_storage.test.cpp @@ -24,7 +24,7 @@ TEST_CASE_METHOD(HermeticLocalTestFixture, std::string test_bytes("test"); LocalStorage storage{}; - auto test_digest = ArtifactDigest::Create(test_bytes); + auto test_digest = ArtifactDigest::Create<ObjectType::File>(test_bytes); // check blob not in storage CHECK(not storage.BlobPath(test_digest, true)); diff --git a/test/utils/remote_execution/bazel_action_creator.hpp b/test/utils/remote_execution/bazel_action_creator.hpp index 6abc9e32..f6ed773a 100644 --- a/test/utils/remote_execution/bazel_action_creator.hpp +++ b/test/utils/remote_execution/bazel_action_creator.hpp @@ -58,12 +58,12 @@ }); auto cmd_data = cmd.SerializeAsString(); - auto cmd_id = ArtifactDigest::Create(cmd_data); + auto cmd_id = ArtifactDigest::Create<ObjectType::File>(cmd_data); blobs.emplace_back(cmd_id, cmd_data); bazel_re::Directory empty_dir; auto dir_data = empty_dir.SerializeAsString(); - auto dir_id = ArtifactDigest::Create(dir_data); + auto dir_id = ArtifactDigest::Create<ObjectType::File>(dir_data); blobs.emplace_back(dir_id, dir_data); bazel_re::Action action; @@ -74,7 +74,7 @@ gsl::owner<bazel_re::Digest*>{new bazel_re::Digest{dir_id}}); auto action_data = action.SerializeAsString(); - auto action_id = ArtifactDigest::Create(action_data); + auto action_id = ArtifactDigest::Create<ObjectType::File>(action_data); blobs.emplace_back(action_id, action_data); BazelCasClient cas_client(info->host, info->port); |