diff options
19 files changed, 219 insertions, 160 deletions
diff --git a/src/buildtool/build_engine/expression/target_result.cpp b/src/buildtool/build_engine/expression/target_result.cpp index 3d2e2dda..c4f43f89 100644 --- a/src/buildtool/build_engine/expression/target_result.cpp +++ b/src/buildtool/build_engine/expression/target_result.cpp @@ -21,6 +21,7 @@ #include "gsl/gsl" #include "src/buildtool/build_engine/expression/expression.hpp" +#include "src/buildtool/common/artifact_description.hpp" #include "src/buildtool/logging/log_level.hpp" #include "src/buildtool/logging/logger.hpp" @@ -40,7 +41,8 @@ namespace { if (auto it = replacements.find(artifact); it != replacements.end()) { auto const& info = it->second; - return ArtifactDescription{info.digest, info.type}.ToJson(); + return ArtifactDescription::CreateKnown(info.digest, info.type) + .ToJson(); } throw std::runtime_error{ "No replacement for non-known artifact found."}; 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 81f679e5..abcc9d40 100644 --- a/src/buildtool/build_engine/target_map/built_in_rules.cpp +++ b/src/buildtool/build_engine/target_map/built_in_rules.cpp @@ -32,6 +32,7 @@ #include "src/buildtool/build_engine/expression/expression_ptr.hpp" #include "src/buildtool/build_engine/target_map/export.hpp" #include "src/buildtool/build_engine/target_map/utils.hpp" +#include "src/buildtool/common/artifact_description.hpp" #include "src/buildtool/common/repository_config.hpp" #include "src/utils/cpp/path.hpp" #include "src/utils/cpp/vector.hpp" @@ -268,9 +269,9 @@ void BlobGenRuleWithDeps( auto stage = ExpressionPtr{Expression::map_t{ name_val->String(), - ExpressionPtr{ArtifactDescription{ + ExpressionPtr{ArtifactDescription::CreateKnown( ArtifactDigest::Create<ObjectType::File>(data_val->String()), - blob_type}}}}; + blob_type)}}}; auto analysis_result = std::make_shared<AnalysedTarget const>( TargetResult{.artifact_stage = stage, @@ -511,7 +512,7 @@ void TreeRuleWithDeps( std::vector<Tree::Ptr> trees{}; trees.emplace_back(std::move(tree)); auto result_stage = Expression::map_t::underlying_map_t{}; - result_stage.emplace(name, ArtifactDescription{tree_id}); + result_stage.emplace(name, ArtifactDescription::CreateTree(tree_id)); auto result = ExpressionPtr{Expression::map_t{result_stage}}; auto analysis_result = std::make_shared<AnalysedTarget const>( @@ -1351,8 +1352,8 @@ void GenericRuleWithDeps( for (const auto& path : container) { artifacts.emplace( path, - ExpressionPtr{ArtifactDescription{ - action_identifier, std::filesystem::path{path}}}); + ExpressionPtr{ArtifactDescription::CreateAction( + action_identifier, std::filesystem::path{path})}); } } diff --git a/src/buildtool/build_engine/target_map/target_map.cpp b/src/buildtool/build_engine/target_map/target_map.cpp index 7bc74800..1c8d3743 100644 --- a/src/buildtool/build_engine/target_map/target_map.cpp +++ b/src/buildtool/build_engine/target_map/target_map.cpp @@ -37,6 +37,7 @@ #include "src/buildtool/build_engine/expression/function_map.hpp" #include "src/buildtool/build_engine/target_map/built_in_rules.hpp" #include "src/buildtool/build_engine/target_map/utils.hpp" +#include "src/buildtool/common/artifact_description.hpp" #include "src/buildtool/common/repository_config.hpp" #include "src/buildtool/common/statistics.hpp" #include "src/buildtool/logging/log_level.hpp" @@ -706,14 +707,16 @@ void withDependencies( auto action_id = action->Id(); actions.emplace_back(std::move(action)); for (auto const& out : outputs) { - result.emplace(out, - ExpressionPtr{ArtifactDescription{ - action_id, std::filesystem::path{out}}}); + result.emplace( + out, + ExpressionPtr{ArtifactDescription::CreateAction( + action_id, std::filesystem::path{out})}); } for (auto const& out : output_dirs) { - result.emplace(out, - ExpressionPtr{ArtifactDescription{ - action_id, std::filesystem::path{out}}}); + result.emplace( + out, + ExpressionPtr{ArtifactDescription::CreateAction( + action_id, std::filesystem::path{out})}); } return ExpressionPtr{Expression::map_t{result}}; @@ -727,9 +730,9 @@ void withDependencies( data->ToString())}; } blobs.emplace_back(data->String()); - return ExpressionPtr{ArtifactDescription{ + return ExpressionPtr{ArtifactDescription::CreateKnown( ArtifactDigest::Create<ObjectType::File>(data->String()), - ObjectType::File}}; + ObjectType::File)}; }}, {"SYMLINK", @@ -747,9 +750,9 @@ void withDependencies( } blobs.emplace_back(data->String()); - return ExpressionPtr{ArtifactDescription{ + return ExpressionPtr{ArtifactDescription::CreateKnown( ArtifactDigest::Create<ObjectType::Symlink>(data->String()), - ObjectType::Symlink}}; + ObjectType::Symlink)}; }}, {"TREE", @@ -783,7 +786,7 @@ void withDependencies( auto tree = std::make_shared<Tree>(std::move(artifacts)); auto tree_id = tree->Id(); trees.emplace_back(std::move(tree)); - return ExpressionPtr{ArtifactDescription{tree_id}}; + return ExpressionPtr{ArtifactDescription::CreateTree(tree_id)}; }}, {"VALUE_NODE", [](auto&& eval, auto const& expr, auto const& env) { @@ -1693,7 +1696,9 @@ void TreeTarget( auto tree = std::make_shared<Tree>(std::move(artifacts)); auto tree_id = tree->Id(); auto tree_map = ExpressionPtr{Expression::map_t{ - name, ExpressionPtr{ArtifactDescription{tree_id}}}}; + name, + ExpressionPtr{ + ArtifactDescription::CreateTree(tree_id)}}}; auto analysis_result = std::make_shared<AnalysedTarget const>( TargetResult{.artifact_stage = tree_map, diff --git a/src/buildtool/common/artifact_description.cpp b/src/buildtool/common/artifact_description.cpp index 24c34944..73d0af05 100644 --- a/src/buildtool/common/artifact_description.cpp +++ b/src/buildtool/common/artifact_description.cpp @@ -52,22 +52,32 @@ namespace { -> std::optional<ArtifactDescription>; } // namespace -ArtifactDescription::ArtifactDescription(std::filesystem::path path, - std::string repository) noexcept - : data_{std::make_pair(std::move(path), std::move(repository))} {} +auto ArtifactDescription::CreateLocal(std::filesystem::path path, + std::string repository) noexcept + -> ArtifactDescription { + Local data{std::move(path), std::move(repository)}; + return ArtifactDescription{std::move(data)}; +} -ArtifactDescription::ArtifactDescription( - ArtifactDigest digest, - ObjectType file_type, - std::optional<std::string> repo) noexcept - : data_{std::make_tuple(std::move(digest), file_type, std::move(repo))} {} +auto ArtifactDescription::CreateAction(std::string action_id, + std::filesystem::path path) noexcept + -> ArtifactDescription { + Action data{std::move(action_id), std::move(path)}; + return ArtifactDescription{std::move(data)}; +} -ArtifactDescription::ArtifactDescription(std::string action_id, - std::filesystem::path path) noexcept - : data_{std::make_pair(std::move(action_id), std::move(path))} {} +auto ArtifactDescription::CreateKnown(ArtifactDigest digest, + ObjectType file_type, + std::optional<std::string> repo) noexcept + -> ArtifactDescription { + Known data{std::move(digest), file_type, std::move(repo)}; + return ArtifactDescription{std::move(data)}; +} -ArtifactDescription::ArtifactDescription(std::string tree_id) noexcept - : data_{std::move(tree_id)} {} +auto ArtifactDescription::CreateTree(std::string tree_id) noexcept + -> ArtifactDescription { + return ArtifactDescription{std::move(tree_id)}; +} auto ArtifactDescription::FromJson(nlohmann::json const& json) noexcept -> std::optional<ArtifactDescription> { @@ -225,14 +235,14 @@ auto DescribeTreeArtifact(std::string const& tree_id) noexcept auto CreateLocalArtifactDescription(nlohmann::json const& data) -> std::optional<ArtifactDescription> { - auto const path = + auto path = ExtractValueAs<std::string>(data, "path", [](std::string const& error) { Logger::Log(LogLevel::Error, "{}\ncan not retrieve value for \"path\" from " "LOCAL artifact's data.", error); }); - auto const repository = ExtractValueAs<std::string>( + auto repository = ExtractValueAs<std::string>( data, "repository", [](std::string const& error) { Logger::Log(LogLevel::Error, "{}\ncan not retrieve value for \"path\" from " @@ -240,7 +250,8 @@ auto CreateLocalArtifactDescription(nlohmann::json const& data) error); }); if (path.has_value() and repository.has_value()) { - return ArtifactDescription{std::filesystem::path{*path}, *repository}; + return ArtifactDescription::CreateLocal(std::move(*path), + std::move(*repository)); } return std::nullopt; } @@ -271,16 +282,15 @@ auto CreateKnownArtifactDescription(nlohmann::json const& data) if (blob_id.has_value() and size.has_value() and file_type.has_value() and file_type->size() == 1) { auto const& object_type = FromChar((*file_type)[0]); - return ArtifactDescription{ - ArtifactDigest{*blob_id, *size, IsTreeObject(object_type)}, - object_type}; + ArtifactDigest digest{*blob_id, *size, IsTreeObject(object_type)}; + return ArtifactDescription::CreateKnown(std::move(digest), object_type); } return std::nullopt; } auto CreateActionArtifactDescription(nlohmann::json const& data) -> std::optional<ArtifactDescription> { - auto const action_id = + auto action_id = ExtractValueAs<std::string>(data, "id", [](std::string const& error) { Logger::Log(LogLevel::Error, "{}\ncan not retrieve value for \"id\" from " @@ -288,7 +298,7 @@ auto CreateActionArtifactDescription(nlohmann::json const& data) error); }); - auto const path = + auto path = ExtractValueAs<std::string>(data, "path", [](std::string const& error) { Logger::Log(LogLevel::Error, "{}\ncan not retrieve value for \"path\" from " @@ -296,14 +306,15 @@ auto CreateActionArtifactDescription(nlohmann::json const& data) error); }); if (action_id.has_value() and path.has_value()) { - return ArtifactDescription{*action_id, std::filesystem::path{*path}}; + return ArtifactDescription::CreateAction(std::move(*action_id), + std::move(*path)); } return std::nullopt; } auto CreateTreeArtifactDescription(nlohmann::json const& data) -> std::optional<ArtifactDescription> { - auto const tree_id = + auto tree_id = ExtractValueAs<std::string>(data, "id", [](std::string const& error) { Logger::Log(LogLevel::Error, "{}\ncan not retrieve value for \"id\" from " @@ -312,7 +323,7 @@ auto CreateTreeArtifactDescription(nlohmann::json const& data) }); if (tree_id.has_value()) { - return ArtifactDescription{*tree_id}; + return ArtifactDescription::CreateTree(std::move(*tree_id)); } return std::nullopt; } diff --git a/src/buildtool/common/artifact_description.hpp b/src/buildtool/common/artifact_description.hpp index dd312083..84845ed2 100644 --- a/src/buildtool/common/artifact_description.hpp +++ b/src/buildtool/common/artifact_description.hpp @@ -25,7 +25,7 @@ #include "src/buildtool/common/artifact_digest.hpp" #include "src/buildtool/file_system/object_type.hpp" -class ArtifactDescription { +class ArtifactDescription final { using Local = std::pair<std::filesystem::path, std::string>; using Known = std::tuple<ArtifactDigest, ObjectType, std::optional<std::string>>; @@ -33,18 +33,22 @@ class ArtifactDescription { using Tree = std::string; public: - explicit ArtifactDescription(std::filesystem::path path, - std::string repository) noexcept; + [[nodiscard]] static auto CreateLocal(std::filesystem::path path, + std::string repository) noexcept + -> ArtifactDescription; - ArtifactDescription( + [[nodiscard]] static auto CreateAction(std::string action_id, + std::filesystem::path path) noexcept + -> ArtifactDescription; + + [[nodiscard]] static auto CreateKnown( ArtifactDigest digest, ObjectType file_type, - std::optional<std::string> repo = std::nullopt) noexcept; - - ArtifactDescription(std::string action_id, - std::filesystem::path path) noexcept; + std::optional<std::string> repo = std::nullopt) noexcept + -> ArtifactDescription; - explicit ArtifactDescription(std::string tree_id) noexcept; + [[nodiscard]] static auto CreateTree(std::string tree_id) noexcept + -> ArtifactDescription; [[nodiscard]] auto Id() const& noexcept -> ArtifactIdentifier const& { return id_; @@ -82,7 +86,11 @@ class ArtifactDescription { private: std::variant<Local, Known, Action, Tree> data_; - ArtifactIdentifier id_{ComputeId(ToJson())}; + ArtifactIdentifier id_; + + template <typename T> + explicit ArtifactDescription(T data) noexcept + : data_{std::move(data)}, id_{ComputeId(ToJson())} {} [[nodiscard]] static auto ComputeId(nlohmann::json const& desc) noexcept -> ArtifactIdentifier; diff --git a/src/buildtool/common/artifact_factory.hpp b/src/buildtool/common/artifact_factory.hpp index e933a16e..0ab0efc3 100644 --- a/src/buildtool/common/artifact_factory.hpp +++ b/src/buildtool/common/artifact_factory.hpp @@ -26,6 +26,7 @@ #include "src/buildtool/common/action.hpp" #include "src/buildtool/common/action_description.hpp" #include "src/buildtool/common/artifact.hpp" +#include "src/buildtool/common/artifact_description.hpp" #include "src/buildtool/common/identifier.hpp" #include "src/buildtool/file_system/object_type.hpp" #include "src/buildtool/logging/logger.hpp" @@ -54,28 +55,30 @@ class ArtifactFactory { [[nodiscard]] static auto DescribeLocalArtifact( std::filesystem::path const& src_path, std::string repository) noexcept -> nlohmann::json { - return ArtifactDescription{src_path, std::move(repository)}.ToJson(); + return ArtifactDescription::CreateLocal(src_path, std::move(repository)) + .ToJson(); } [[nodiscard]] static auto DescribeKnownArtifact( std::string const& blob_id, std::size_t size, ObjectType type = ObjectType::File) noexcept -> nlohmann::json { - return ArtifactDescription{ - ArtifactDigest{blob_id, size, IsTreeObject(type)}, type} + return ArtifactDescription::CreateKnown( + ArtifactDigest{blob_id, size, IsTreeObject(type)}, type) .ToJson(); } [[nodiscard]] static auto DescribeActionArtifact( std::string const& action_id, std::string const& out_path) noexcept -> nlohmann::json { - return ArtifactDescription{action_id, std::filesystem::path{out_path}} + return ArtifactDescription::CreateAction( + action_id, std::filesystem::path{out_path}) .ToJson(); } [[nodiscard]] static auto DescribeTreeArtifact( std::string const& tree_id) noexcept -> nlohmann::json { - return ArtifactDescription{tree_id}.ToJson(); + return ArtifactDescription::CreateTree(tree_id).ToJson(); } [[nodiscard]] static auto DescribeAction( diff --git a/src/buildtool/common/tree.hpp b/src/buildtool/common/tree.hpp index 6e066c75..e9d5ef84 100644 --- a/src/buildtool/common/tree.hpp +++ b/src/buildtool/common/tree.hpp @@ -51,7 +51,7 @@ class Tree { } [[nodiscard]] auto Output() const -> ArtifactDescription { - return ArtifactDescription{id_}; + return ArtifactDescription::CreateTree(id_); } [[nodiscard]] static auto FromJson(std::string const& id, diff --git a/src/buildtool/execution_engine/dag/dag.cpp b/src/buildtool/execution_engine/dag/dag.cpp index a3c25402..c36ec664 100644 --- a/src/buildtool/execution_engine/dag/dag.cpp +++ b/src/buildtool/execution_engine/dag/dag.cpp @@ -22,7 +22,7 @@ auto DependencyGraph::CreateOutputArtifactNodes( -> std::pair<std::vector<DependencyGraph::NamedArtifactNodePtr>, std::vector<DependencyGraph::NamedArtifactNodePtr>> { if (is_tree_action) { // create tree artifact - auto artifact = ArtifactDescription{action_id}.ToArtifact(); + auto artifact = ArtifactDescription::CreateTree(action_id).ToArtifact(); auto const node_id = AddArtifact(std::move(artifact)); return std::make_pair(std::vector<NamedArtifactNodePtr>{}, std::vector<NamedArtifactNodePtr>{ @@ -32,10 +32,9 @@ auto DependencyGraph::CreateOutputArtifactNodes( // create action artifacts auto node_creator = [this, &action_id](auto* nodes, auto const& paths) { for (auto const& artifact_path : paths) { - auto artifact = - ArtifactDescription{action_id, - std::filesystem::path{artifact_path}} - .ToArtifact(); + auto artifact = ArtifactDescription::CreateAction( + action_id, std::filesystem::path{artifact_path}) + .ToArtifact(); auto const node_id = AddArtifact(std::move(artifact)); nodes->emplace_back(NamedArtifactNodePtr{ artifact_path, &(*artifact_nodes_[node_id])}); diff --git a/src/buildtool/file_system/file_root.hpp b/src/buildtool/file_system/file_root.hpp index db7ff07d..8b85f5a9 100644 --- a/src/buildtool/file_system/file_root.hpp +++ b/src/buildtool/file_system/file_root.hpp @@ -230,10 +230,10 @@ class FileRoot { if (auto id = data->Hash()) { auto const& size = data->Size(); if (size) { - return ArtifactDescription{ + return ArtifactDescription::CreateKnown( ArtifactDigest{*id, *size, /*is_tree=*/true}, ObjectType::Tree, - repository}; + repository); } } } catch (...) { @@ -575,23 +575,23 @@ class FileRoot { if (Compatibility::IsCompatible()) { auto compatible_hash = Compatibility::RegisterGitEntry( entry->Hash(), *entry->Blob(), repository); - return ArtifactDescription{ + return ArtifactDescription::CreateKnown( ArtifactDigest{compatible_hash, *entry->Size(), /*is_tree=*/false}, - entry->Type()}; + entry->Type()); } - return ArtifactDescription{ + return ArtifactDescription::CreateKnown( ArtifactDigest{ entry->Hash(), *entry->Size(), /*is_tree=*/false}, entry->Type(), - repository}; + repository); } } return std::nullopt; } if (std::holds_alternative<fs_root_t>(root_)) { - return ArtifactDescription{file_path, repository}; + return ArtifactDescription::CreateLocal(file_path, repository); } return std::nullopt; // absent roots are neither LOCAL nor KNOWN } diff --git a/test/buildtool/build_engine/expression/TARGETS b/test/buildtool/build_engine/expression/TARGETS index a4d56e57..e7d15616 100644 --- a/test/buildtool/build_engine/expression/TARGETS +++ b/test/buildtool/build_engine/expression/TARGETS @@ -19,6 +19,7 @@ , ["", "catch-main"] , ["utils", "container_matchers"] , ["@", "src", "src/buildtool/build_engine/expression", "expression"] + , ["@", "src", "src/buildtool/common", "common"] ] , "stage": ["test", "buildtool", "build_engine", "expression"] } diff --git a/test/buildtool/build_engine/expression/expression.test.cpp b/test/buildtool/build_engine/expression/expression.test.cpp index 0bc5439b..8dd0aa92 100644 --- a/test/buildtool/build_engine/expression/expression.test.cpp +++ b/test/buildtool/build_engine/expression/expression.test.cpp @@ -22,6 +22,7 @@ #include "src/buildtool/build_engine/expression/configuration.hpp" #include "src/buildtool/build_engine/expression/expression.hpp" #include "src/buildtool/build_engine/expression/function_map.hpp" +#include "src/buildtool/common/artifact_description.hpp" #include "test/utils/container_matchers.hpp" TEST_CASE("Expression access", "[expression]") { // NOLINT @@ -38,7 +39,8 @@ TEST_CASE("Expression access", "[expression]") { // NOLINT auto boolean = ExpressionPtr{true}; auto number = ExpressionPtr{number_t{1}}; auto string = ExpressionPtr{"2"s}; - auto artifact = ExpressionPtr{artifact_t{path{"local_path"}}}; + auto artifact = + ExpressionPtr{ArtifactDescription::CreateTree(path{"local_path"})}; auto result = ExpressionPtr{result_t{boolean, number, string}}; auto list = ExpressionPtr{list_t{number}}; auto map = ExpressionPtr{map_t{{"3"s, number}}}; @@ -78,7 +80,8 @@ TEST_CASE("Expression access", "[expression]") { // NOLINT CHECK(string->String() == "2"s); CHECK_THROWS_AS(string->Artifact(), Expression::ExpressionTypeError); - CHECK(artifact->Artifact() == artifact_t{path{"local_path"}}); + CHECK(artifact->Artifact() == + ArtifactDescription::CreateTree(path{"local_path"})); CHECK_THROWS_AS(artifact->String(), Expression::ExpressionTypeError); CHECK(result->Result() == result_t{boolean, number, string}); @@ -154,13 +157,14 @@ TEST_CASE("Expression access", "[expression]") { // NOLINT CHECK(string == Expression::FromJson(R"("2")"_json)); CHECK(string != ""s); CHECK(string != Expression{""s}); - CHECK(string != artifact_t{path{"local_path"}}); + CHECK(string != ArtifactDescription::CreateTree(path{"local_path"})); CHECK(string != artifact); CHECK(string != Expression::FromJson(R"("")"_json)); CHECK(artifact == artifact); - CHECK(artifact == artifact_t{path{"local_path"}}); - CHECK(artifact == Expression{artifact_t{path{"local_path"}}}); + CHECK(artifact == ArtifactDescription::CreateTree(path{"local_path"})); + CHECK(artifact == + Expression{ArtifactDescription::CreateTree(path{"local_path"})}); CHECK(artifact != ""s); CHECK(artifact != string); @@ -191,16 +195,16 @@ TEST_CASE("Expression access", "[expression]") { // NOLINT CHECK(map != Expression::FromJson(R"(["3",1])"_json)); // compare nullptr != null != false != 0 != "" != [] != {} - auto exprs = - std::vector<ExpressionPtr>{ExpressionPtr{nullptr}, - ExpressionPtr{artifact_t{path{""}}}, - ExpressionPtr{result_t{}}, - Expression::FromJson("null"_json), - Expression::FromJson("false"_json), - Expression::FromJson("0"_json), - Expression::FromJson(R"("")"_json), - Expression::FromJson("[]"_json), - Expression::FromJson("{}"_json)}; + auto exprs = std::vector<ExpressionPtr>{ + ExpressionPtr{nullptr}, + ExpressionPtr{ArtifactDescription::CreateTree(path{""})}, + ExpressionPtr{result_t{}}, + Expression::FromJson("null"_json), + Expression::FromJson("false"_json), + Expression::FromJson("0"_json), + Expression::FromJson(R"("")"_json), + Expression::FromJson("[]"_json), + Expression::FromJson("{}"_json)}; for (auto const& l : exprs) { for (auto const& r : exprs) { if (&l != &r) { @@ -1697,7 +1701,7 @@ TEST_CASE("Expression hash computation", "[expression]") { auto boolean = ExpressionPtr{false}; auto number = ExpressionPtr{number_t{}}; auto string = ExpressionPtr{""s}; - auto artifact = ExpressionPtr{artifact_t{path{""}}}; + auto artifact = ExpressionPtr{ArtifactDescription::CreateTree(path{""})}; auto result = ExpressionPtr{result_t{}}; auto list = ExpressionPtr{list_t{}}; auto map = ExpressionPtr{map_t{}}; @@ -1718,9 +1722,11 @@ TEST_CASE("Expression hash computation", "[expression]") { CHECK_FALSE(string->ToHash() == Expression{" "s}.ToHash()); CHECK_FALSE(artifact->ToHash().empty()); - CHECK(artifact->ToHash() == Expression{artifact_t{path{""}}}.ToHash()); - CHECK_FALSE(artifact->ToHash() == - Expression{artifact_t{path{" "}}}.ToHash()); + CHECK(artifact->ToHash() == + Expression{ArtifactDescription::CreateTree(path{""})}.ToHash()); + CHECK_FALSE( + artifact->ToHash() == + Expression{ArtifactDescription::CreateTree(path{" "})}.ToHash()); CHECK_FALSE(result->ToHash().empty()); CHECK(result->ToHash() == Expression{result_t{}}.ToHash()); diff --git a/test/buildtool/common/action_description.test.cpp b/test/buildtool/common/action_description.test.cpp index 61361c58..9d82ec9d 100644 --- a/test/buildtool/common/action_description.test.cpp +++ b/test/buildtool/common/action_description.test.cpp @@ -18,16 +18,17 @@ #include "nlohmann/json.hpp" #include "src/buildtool/common/action.hpp" #include "src/buildtool/common/action_description.hpp" +#include "src/buildtool/common/artifact_description.hpp" #include "src/buildtool/common/artifact_factory.hpp" TEST_CASE("From JSON", "[action_description]") { using path = std::filesystem::path; - auto desc = - ActionDescription{{"output0", "output1"}, - {"dir0", "dir1"}, - Action{"id", {"command", "line"}, {{"env", "vars"}}}, - {{"path0", ArtifactDescription{path{"input0"}}}, - {"path1", ArtifactDescription{path{"input1"}}}}}; + auto desc = ActionDescription{ + {"output0", "output1"}, + {"dir0", "dir1"}, + Action{"id", {"command", "line"}, {{"env", "vars"}}}, + {{"path0", ArtifactDescription::CreateTree(path{"input0"})}, + {"path1", ArtifactDescription::CreateTree(path{"input1"})}}}; auto const& action = desc.GraphAction(); auto json = ArtifactFactory::DescribeAction(desc.OutputFiles(), desc.OutputDirs(), diff --git a/test/buildtool/common/artifact_description.test.cpp b/test/buildtool/common/artifact_description.test.cpp index 60d62961..d07efa93 100644 --- a/test/buildtool/common/artifact_description.test.cpp +++ b/test/buildtool/common/artifact_description.test.cpp @@ -28,8 +28,8 @@ } TEST_CASE("Local artifact", "[artifact_description]") { - auto local_desc = - ArtifactDescription{std::filesystem::path{"local_path"}, "repo"}; + auto local_desc = ArtifactDescription::CreateLocal( + std::filesystem::path{"local_path"}, "repo"); auto local = local_desc.ToArtifact(); auto local_from_factory = ArtifactFactory::FromDescription(local_desc.ToJson()); @@ -38,27 +38,27 @@ TEST_CASE("Local artifact", "[artifact_description]") { TEST_CASE("Known artifact", "[artifact_description]") { SECTION("File object") { - auto known_desc = ArtifactDescription{ + auto known_desc = ArtifactDescription::CreateKnown( ArtifactDigest{std::string{"f_fake_hash"}, 0, /*is_tree=*/false}, - ObjectType::File}; + ObjectType::File); auto known = known_desc.ToArtifact(); auto known_from_factory = ArtifactFactory::FromDescription(known_desc.ToJson()); CHECK(known == *known_from_factory); } SECTION("Executable object") { - auto known_desc = ArtifactDescription{ + auto known_desc = ArtifactDescription::CreateKnown( ArtifactDigest{std::string{"x_fake_hash"}, 1, /*is_tree=*/false}, - ObjectType::Executable}; + ObjectType::Executable); auto known = known_desc.ToArtifact(); auto known_from_factory = ArtifactFactory::FromDescription(known_desc.ToJson()); CHECK(known == *known_from_factory); } SECTION("Symlink object") { - auto known_desc = ArtifactDescription{ + auto known_desc = ArtifactDescription::CreateKnown( ArtifactDigest{std::string{"l_fake_hash"}, 2, /*is_tree=*/false}, - ObjectType::Symlink}; + ObjectType::Symlink); auto known = known_desc.ToArtifact(); auto known_from_factory = ArtifactFactory::FromDescription(known_desc.ToJson()); @@ -67,8 +67,8 @@ TEST_CASE("Known artifact", "[artifact_description]") { } TEST_CASE("Action artifact", "[artifact_description]") { - auto action_desc = - ArtifactDescription{"action_id", std::filesystem::path{"out_path"}}; + auto action_desc = ArtifactDescription::CreateAction( + "action_id", std::filesystem::path{"out_path"}); auto action = action_desc.ToArtifact(); auto action_from_factory = ArtifactFactory::FromDescription(action_desc.ToJson()); diff --git a/test/buildtool/execution_engine/dag/dag.test.cpp b/test/buildtool/execution_engine/dag/dag.test.cpp index 508c954e..cac4b791 100644 --- a/test/buildtool/execution_engine/dag/dag.test.cpp +++ b/test/buildtool/execution_engine/dag/dag.test.cpp @@ -130,7 +130,8 @@ TEST_CASE("AddAction({single action, more outputs, no inputs})", "[dag]") { TEST_CASE("AddAction({single action, single output, source file})", "[dag]") { using path = std::filesystem::path; std::string const action_id = "action_id"; - auto const src_description = ArtifactDescription{path{"main.cpp"}, "repo"}; + auto const src_description = + ArtifactDescription::CreateLocal(path{"main.cpp"}, "repo"); auto const& src_id = src_description.Id(); DependencyGraph g; SECTION("Input file in the same path than it is locally") { @@ -205,13 +206,17 @@ TEST_CASE("Add executable and library", "[dag]") { std::string const make_lib_id = "make_lib"; std::vector<std::string> const make_exec_cmd = {"build", "exec"}; std::vector<std::string> const make_lib_cmd = {"build", "lib.a"}; - auto const main_desc = ArtifactDescription{path{"main.cpp"}, ""}; + auto const main_desc = + ArtifactDescription::CreateLocal(path{"main.cpp"}, ""); auto const& main_id = main_desc.Id(); - auto const lib_hpp_desc = ArtifactDescription{path{"lib/lib.hpp"}, ""}; + auto const lib_hpp_desc = + ArtifactDescription::CreateLocal(path{"lib/lib.hpp"}, ""); auto const& lib_hpp_id = lib_hpp_desc.Id(); - auto const lib_cpp_desc = ArtifactDescription{path{"lib/lib.cpp"}, ""}; + auto const lib_cpp_desc = + ArtifactDescription::CreateLocal(path{"lib/lib.cpp"}, ""); auto const& lib_cpp_id = lib_cpp_desc.Id(); - auto const lib_a_desc = ArtifactDescription{make_lib_id, "lib.a"}; + auto const lib_a_desc = + ArtifactDescription::CreateAction(make_lib_id, "lib.a"); auto const& lib_a_id = lib_a_desc.Id(); auto const make_exec_desc = @@ -219,7 +224,8 @@ TEST_CASE("Add executable and library", "[dag]") { {}, Action{make_exec_id, make_exec_cmd, {}}, {{"main.cpp", main_desc}, {"lib.a", lib_a_desc}}}; - auto const& exec_out_id = ArtifactDescription{make_exec_id, "exec"}.Id(); + auto const& exec_out_id = + ArtifactDescription::CreateAction(make_exec_id, "exec").Id(); auto const make_lib_desc = ActionDescription{ {"lib.a"}, @@ -293,8 +299,10 @@ TEST_CASE("AddAction(Empty mandatory non-empty field in action description)", TEST_CASE("Adding cyclic dependencies produces invalid graph", "[dag]") { std::string const action1_id = "action1"; std::string const action2_id = "action2"; - auto const out1_desc = ArtifactDescription(action1_id, "out1"); - auto const out2_desc = ArtifactDescription(action2_id, "out2"); + auto const out1_desc = + ArtifactDescription::CreateAction(action1_id, "out1"); + auto const out2_desc = + ArtifactDescription::CreateAction(action2_id, "out2"); auto const action1_desc = ActionDescription{{"out1"}, diff --git a/test/buildtool/execution_engine/executor/executor.test.cpp b/test/buildtool/execution_engine/executor/executor.test.cpp index dd9f59db..9dc2f667 100644 --- a/test/buildtool/execution_engine/executor/executor.test.cpp +++ b/test/buildtool/execution_engine/executor/executor.test.cpp @@ -234,9 +234,10 @@ class TestApi : public IExecutionApi { -> std::pair<TestApiConfig, RepositoryConfig> { using path = std::filesystem::path; - auto const local_cpp_desc = ArtifactDescription{path{"local.cpp"}, ""}; - auto const known_cpp_desc = ArtifactDescription{ - ArtifactDigest{"known.cpp", 0, /*is_tree=*/false}, ObjectType::File}; + auto const local_cpp_desc = + ArtifactDescription::CreateLocal(path{"local.cpp"}, ""); + auto const known_cpp_desc = ArtifactDescription::CreateKnown( + ArtifactDigest{"known.cpp", 0, /*is_tree=*/false}, ObjectType::File); auto const test_action_desc = ActionDescription{ {"output1.exe", "output2.exe"}, diff --git a/test/buildtool/execution_engine/executor/executor_api.test.hpp b/test/buildtool/execution_engine/executor/executor_api.test.hpp index e499efae..18e30c70 100644 --- a/test/buildtool/execution_engine/executor/executor_api.test.hpp +++ b/test/buildtool/execution_engine/executor/executor_api.test.hpp @@ -99,7 +99,7 @@ static inline void RunHelloWorldCompilation( using path = std::filesystem::path; SetupConfig(repo_config); auto const main_cpp_desc = - ArtifactDescription{path{"data/hello_world/main.cpp"}, ""}; + ArtifactDescription::CreateLocal(path{"data/hello_world/main.cpp"}, ""); auto const& main_cpp_id = main_cpp_desc.Id(); std::string const make_hello_id = "make_hello"; auto* env_path = std::getenv("PATH"); @@ -118,7 +118,7 @@ static inline void RunHelloWorldCompilation( env}, {{"src/main.cpp", main_cpp_desc}}}; auto const exec_desc = - ArtifactDescription{make_hello_id, "out/hello_world"}; + ArtifactDescription::CreateAction(make_hello_id, "out/hello_world"); auto const& exec_id = exec_desc.Id(); DependencyGraph g; @@ -174,10 +174,10 @@ static inline void RunGreeterCompilation( using path = std::filesystem::path; SetupConfig(repo_config); auto const greet_hpp_desc = - ArtifactDescription{path{"data/greeter/greet.hpp"}, ""}; + ArtifactDescription::CreateLocal(path{"data/greeter/greet.hpp"}, ""); auto const& greet_hpp_id = greet_hpp_desc.Id(); auto const greet_cpp_desc = - ArtifactDescription{path{"data/greeter"} / greetcpp, ""}; + ArtifactDescription::CreateLocal(path{"data/greeter"} / greetcpp, ""); auto const& greet_cpp_id = greet_cpp_desc.Id(); std::string const compile_greet_id = "compile_greet"; @@ -205,7 +205,7 @@ static inline void RunGreeterCompilation( {"src/greet.cpp", greet_cpp_desc}}}; auto const greet_o_desc = - ArtifactDescription{compile_greet_id, "out/greet.o"}; + ArtifactDescription::CreateAction(compile_greet_id, "out/greet.o"); auto const& greet_o_id = greet_o_desc.Id(); std::string const make_lib_id = "make_lib"; @@ -216,11 +216,11 @@ static inline void RunGreeterCompilation( {{"greet.o", greet_o_desc}}}; auto const main_cpp_desc = - ArtifactDescription{path{"data/greeter/main.cpp"}, ""}; + ArtifactDescription::CreateLocal(path{"data/greeter/main.cpp"}, ""); auto const& main_cpp_id = main_cpp_desc.Id(); auto const libgreet_desc = - ArtifactDescription{make_lib_id, "out/libgreet.a"}; + ArtifactDescription::CreateAction(make_lib_id, "out/libgreet.a"); auto const& libgreet_id = libgreet_desc.Id(); std::string const make_exe_id = "make_exe"; @@ -242,7 +242,8 @@ static inline void RunGreeterCompilation( {"include/greet.hpp", greet_hpp_desc}, {"lib/libgreet.a", libgreet_desc}}}; - auto const exec_id = ArtifactDescription(make_exe_id, "out/greeter").Id(); + auto const exec_id = + ArtifactDescription::CreateAction(make_exe_id, "out/greeter").Id(); DependencyGraph g; CHECK(g.Add({compile_greet_desc, make_lib_desc, make_exe_desc})); @@ -402,8 +403,10 @@ static inline void TestUploadAndDownloadTrees( ArtifactBlob{bar_digest, bar, /*is_exec=*/false}}})); // define known artifacts - auto foo_desc = ArtifactDescription{foo_digest, ObjectType::File}; - auto bar_desc = ArtifactDescription{bar_digest, ObjectType::Symlink}; + auto foo_desc = + ArtifactDescription::CreateKnown(foo_digest, ObjectType::File); + auto bar_desc = + ArtifactDescription::CreateKnown(bar_digest, ObjectType::Symlink); DependencyGraph g{}; auto foo_id = g.AddArtifact(foo_desc); @@ -560,7 +563,8 @@ static inline void TestRetrieveOutputDirectories( SECTION("entire action output as directory") { auto const make_tree_desc = create_action({}, {""}); - auto const root_desc = ArtifactDescription{make_tree_id, ""}; + auto const root_desc = + ArtifactDescription::CreateAction(make_tree_id, ""); DependencyGraph g{}; REQUIRE(g.AddAction(make_tree_desc)); @@ -605,9 +609,12 @@ static inline void TestRetrieveOutputDirectories( SECTION("disjoint files and directories") { auto const make_tree_desc = create_action({"foo", "bar"}, {"baz"}); - auto const foo_desc = ArtifactDescription{make_tree_id, "foo"}; - auto const bar_desc = ArtifactDescription{make_tree_id, "bar"}; - auto const baz_desc = ArtifactDescription{make_tree_id, "baz"}; + auto const foo_desc = + ArtifactDescription::CreateAction(make_tree_id, "foo"); + auto const bar_desc = + ArtifactDescription::CreateAction(make_tree_id, "bar"); + auto const baz_desc = + ArtifactDescription::CreateAction(make_tree_id, "baz"); DependencyGraph g{}; REQUIRE(g.AddAction(make_tree_desc)); @@ -669,10 +676,14 @@ static inline void TestRetrieveOutputDirectories( SECTION("nested files and directories") { auto const make_tree_desc = create_action({"foo", "baz/bar"}, {"", "baz/baz"}); - auto const root_desc = ArtifactDescription{make_tree_id, ""}; - auto const foo_desc = ArtifactDescription{make_tree_id, "foo"}; - auto const bar_desc = ArtifactDescription{make_tree_id, "baz/bar"}; - auto const baz_desc = ArtifactDescription{make_tree_id, "baz/baz"}; + auto const root_desc = + ArtifactDescription::CreateAction(make_tree_id, ""); + auto const foo_desc = + ArtifactDescription::CreateAction(make_tree_id, "foo"); + auto const bar_desc = + ArtifactDescription::CreateAction(make_tree_id, "baz/bar"); + auto const baz_desc = + ArtifactDescription::CreateAction(make_tree_id, "baz/baz"); DependencyGraph g{}; REQUIRE(g.AddAction(make_tree_desc)); @@ -750,7 +761,8 @@ static inline void TestRetrieveOutputDirectories( SECTION("non-existing outputs") { SECTION("non-existing file") { auto const make_tree_desc = create_action({"fool"}, {}); - auto const fool_desc = ArtifactDescription{make_tree_id, "fool"}; + auto const fool_desc = + ArtifactDescription::CreateAction(make_tree_id, "fool"); DependencyGraph g{}; REQUIRE(g.AddAction(make_tree_desc)); @@ -775,7 +787,8 @@ static inline void TestRetrieveOutputDirectories( SECTION("non-existing directory") { auto const make_tree_desc = create_action({"bazel"}, {}); - auto const bazel_desc = ArtifactDescription{make_tree_id, "bazel"}; + auto const bazel_desc = + ArtifactDescription::CreateAction(make_tree_id, "bazel"); DependencyGraph g{}; REQUIRE(g.AddAction(make_tree_desc)); diff --git a/test/buildtool/execution_engine/traverser/TARGETS b/test/buildtool/execution_engine/traverser/TARGETS index 9f1e1387..77603634 100644 --- a/test/buildtool/execution_engine/traverser/TARGETS +++ b/test/buildtool/execution_engine/traverser/TARGETS @@ -6,6 +6,7 @@ [ ["@", "catch2", "", "catch2"] , ["", "catch-main"] , ["utils", "container_matchers"] + , ["@", "src", "src/buildtool/common", "common"] , ["@", "src", "src/buildtool/common", "artifact_factory"] , ["@", "src", "src/buildtool/execution_engine/dag", "dag"] , ["@", "src", "src/buildtool/execution_engine/traverser", "traverser"] diff --git a/test/buildtool/execution_engine/traverser/traverser.test.cpp b/test/buildtool/execution_engine/traverser/traverser.test.cpp index 2354aaf1..150f3d8c 100644 --- a/test/buildtool/execution_engine/traverser/traverser.test.cpp +++ b/test/buildtool/execution_engine/traverser/traverser.test.cpp @@ -167,10 +167,9 @@ class TestProject { command.emplace_back("BUILD"); for (auto const& output : outputs) { command.push_back(output); - auto const out_id = ArtifactDescription{ - action_id, - std::filesystem::path{ - output}}.Id(); + auto const out_id = ArtifactDescription::CreateAction( + action_id, std::filesystem::path{output}) + .Id(); auto [_, is_inserted] = artifacts_to_be_built_.insert(out_id); if (!is_inserted) { return false; diff --git a/test/buildtool/file_system/file_root.test.cpp b/test/buildtool/file_system/file_root.test.cpp index e5f84cfb..1f7bf927 100644 --- a/test/buildtool/file_system/file_root.test.cpp +++ b/test/buildtool/file_system/file_root.test.cpp @@ -367,8 +367,8 @@ TEST_CASE("Creating artifact descriptions", "[file_root]") { auto desc = root.ToArtifactDescription("baz/foo", "repo"); REQUIRE(desc); - CHECK(*desc == - ArtifactDescription(std::filesystem::path{"baz/foo"}, "repo")); + CHECK(*desc == ArtifactDescription::CreateLocal( + std::filesystem::path{"baz/foo"}, "repo")); CHECK(root.ToArtifactDescription("does_not_exist", "repo")); } @@ -381,17 +381,17 @@ TEST_CASE("Creating artifact descriptions", "[file_root]") { auto foo = root->ToArtifactDescription("baz/foo", "repo"); REQUIRE(foo); - CHECK(*foo == - ArtifactDescription{ArtifactDigest{kFooId, 3, /*is_tree=*/false}, - ObjectType::File, - "repo"}); + CHECK(*foo == ArtifactDescription::CreateKnown( + ArtifactDigest{kFooId, 3, /*is_tree=*/false}, + ObjectType::File, + "repo")); auto bar = root->ToArtifactDescription("baz/bar", "repo"); REQUIRE(bar); - CHECK(*bar == - ArtifactDescription{ArtifactDigest{kBarId, 3, /*is_tree=*/false}, - ObjectType::Executable, - "repo"}); + CHECK(*bar == ArtifactDescription::CreateKnown( + ArtifactDigest{kBarId, 3, /*is_tree=*/false}, + ObjectType::Executable, + "repo")); CHECK_FALSE(root->ToArtifactDescription("baz", "repo")); CHECK_FALSE(root->ToArtifactDescription("does_not_exist", "repo")); @@ -404,8 +404,8 @@ TEST_CASE("Creating artifact descriptions", "[file_root]") { auto desc = root.ToArtifactDescription("baz/foo", "repo"); REQUIRE(desc); - CHECK(*desc == - ArtifactDescription(std::filesystem::path{"baz/foo"}, "repo")); + CHECK(*desc == ArtifactDescription::CreateLocal( + std::filesystem::path{"baz/foo"}, "repo")); CHECK(root.ToArtifactDescription("does_not_exist", "repo")); } @@ -419,17 +419,17 @@ TEST_CASE("Creating artifact descriptions", "[file_root]") { auto foo = root->ToArtifactDescription("baz/foo", "repo"); REQUIRE(foo); - CHECK(*foo == - ArtifactDescription{ArtifactDigest{kFooId, 3, /*is_tree=*/false}, - ObjectType::File, - "repo"}); + CHECK(*foo == ArtifactDescription::CreateKnown( + ArtifactDigest{kFooId, 3, /*is_tree=*/false}, + ObjectType::File, + "repo")); auto bar = root->ToArtifactDescription("baz/bar", "repo"); REQUIRE(bar); - CHECK(*bar == - ArtifactDescription{ArtifactDigest{kBarId, 3, /*is_tree=*/false}, - ObjectType::Executable, - "repo"}); + CHECK(*bar == ArtifactDescription::CreateKnown( + ArtifactDigest{kBarId, 3, /*is_tree=*/false}, + ObjectType::Executable, + "repo")); CHECK_FALSE(root->ToArtifactDescription("baz", "repo")); CHECK_FALSE(root->ToArtifactDescription("does_not_exist", "repo")); |