From 346925cf5efb7045ba25a208b83eff850c1417e9 Mon Sep 17 00:00:00 2001 From: Niket Naidu Date: Mon, 7 Nov 2022 08:37:11 +0000 Subject: [PATCH 01/19] Added Comparator for custom_generator --- .../target/include/target/custom_generator.h | 140 +++++++++++++++--- .../src/custom_generator/custom_generator.cpp | 67 +++------ 2 files changed, 144 insertions(+), 63 deletions(-) diff --git a/buildcc/lib/target/include/target/custom_generator.h b/buildcc/lib/target/include/target/custom_generator.h index e0263419..d8819bd5 100644 --- a/buildcc/lib/target/include/target/custom_generator.h +++ b/buildcc/lib/target/include/target/custom_generator.h @@ -86,13 +86,14 @@ class CustomBlobHandler { virtual std::vector Serialize() const = 0; }; -struct UserIdInfo : internal::CustomGeneratorSchema::IdInfo { - fs_unordered_set inputs; - GenerateCb generate_cb; - std::shared_ptr blob_handler{nullptr}; -}; - struct UserCustomGeneratorSchema : public internal::CustomGeneratorSchema { + struct UserIdInfo : internal::CustomGeneratorSchema::IdInfo { + fs_unordered_set inputs; // TODO, Remove + GenerateCb generate_cb; + std::shared_ptr blob_handler{nullptr}; + }; + + using UserIdPair = std::pair; std::unordered_map ids; void ConvertToInternal() { @@ -110,7 +111,8 @@ class CustomGenerator : public internal::BuilderInterface { CustomGenerator(const std::string &name, const TargetEnv &env) : name_(name), env_(env.GetTargetRootDir(), env.GetTargetBuildDir() / name), - serialization_(env_.GetTargetBuildDir() / fmt::format("{}.bin", name)) { + serialization_(env_.GetTargetBuildDir() / fmt::format("{}.json", name)), + comparator_(serialization_.GetLoad(), user_) { Initialize(); } virtual ~CustomGenerator() = default; @@ -184,8 +186,7 @@ class CustomGenerator : public internal::BuilderInterface { const std::string &id); void GenerateTask(); - void BuildGenerate(std::unordered_set &gen_selected_ids, - std::unordered_set &dummy_gen_selected_ids); + void BuildGenerate(); void InvokeDependencyCb(std::unordered_map &®istered_tasks) const noexcept; @@ -207,19 +208,114 @@ class CustomGenerator : public internal::BuilderInterface { void InvokeDependencyCb(const std::string &group_id, std::unordered_map &®istered_tasks) const noexcept { - if (!dependency_cb) { - return; + if (dependency_cb) { + try { + dependency_cb(std::move(registered_tasks)); + } catch (...) { + env::log_critical( + __FUNCTION__, + fmt::format("Dependency callback failed for group id {}", + group_id)); + env::set_task_state(env::TaskState::FAILURE); + } } - try { - dependency_cb(std::move(registered_tasks)); - } catch (...) { - env::log_critical( - __FUNCTION__, - fmt::format("Dependency callback failed for group id {}", - group_id)); - env::set_task_state(env::TaskState::FAILURE); + } + }; + + struct Comparator { + Comparator(const internal::CustomGeneratorSchema &s, + const UserCustomGeneratorSchema &us) + : schema(s), user_schema(us) {} + + enum class State { + kRemoved, + kAdded, + kCheckLater, + }; + + void AddAllIds() { + const auto &curr_ids = user_schema.ids; + for (const auto &[id, _] : curr_ids) { + id_state_info.at(State::kAdded).insert(id); } } + + void CompareIds() { + const auto &prev_ids = schema.internal_ids; + const auto &curr_ids = user_schema.ids; + + for (const auto &[prev_id, _] : prev_ids) { + if (curr_ids.find(prev_id) == curr_ids.end()) { + // Id Removed condition, previous id is not present in the current run + id_state_info.at(State::kRemoved).insert(prev_id); + } + } + + for (const auto &[curr_id, _] : curr_ids) { + if (prev_ids.find(curr_id) == prev_ids.end()) { + // Id Added condition + id_state_info.at(State::kAdded).insert(curr_id); + } else { + // Id Check Later condition + id_state_info.at(State::kCheckLater).insert(curr_id); + } + } + } + + void CompareGroups() { + const auto &prev_groups = schema.internal_groups; + const auto &curr_groups = user_schema.internal_groups; + + for (const auto &[prev_group_id, _] : prev_groups) { + if (curr_groups.find(prev_group_id) == curr_groups.end()) { + // Group Removed condition + group_state_info.at(State::kRemoved).insert(prev_group_id); + } + } + + for (const auto &[curr_group_id, curr_group_info] : curr_groups) { + if (prev_groups.find(curr_group_id) == prev_groups.end()) { + // Group Added condition + group_state_info.at(State::kAdded).insert(curr_group_id); + } else { + // Group Check Later condition + // NOTE, We cannot perform the check now since the input/metadata + // might be added during runtime (when task executes) + group_state_info.at(State::kCheckLater).insert(curr_group_id); + } + } + } + + const std::unordered_set &RemovedIds() const { + return id_state_info.at(State::kRemoved); + } + + const std::unordered_set &AddedIds() const { + return id_state_info.at(State::kAdded); + } + + bool AddedId(const std::string &id) const { + return id_state_info.at(State::kAdded).count(id) == 1; + } + + bool AddedGroupId(const std::string &group_id) const { + return group_state_info.at(State::kAdded).count(group_id) == 1; + } + + private: + const internal::CustomGeneratorSchema &schema; + const UserCustomGeneratorSchema &user_schema; + std::unordered_map> id_state_info{ + {State::kRemoved, std::unordered_set()}, + {State::kAdded, std::unordered_set()}, + {State::kCheckLater, std::unordered_set()}, + }; + + std::unordered_map> group_state_info{ + {State::kRemoved, std::unordered_set()}, + {State::kAdded, std::unordered_set()}, + {State::kCheckLater, std::unordered_set()}, + }; }; private: @@ -232,8 +328,12 @@ class CustomGenerator : public internal::BuilderInterface { std::unordered_map grouped_ids_; std::unordered_set ungrouped_ids_; + // Comparator + Comparator comparator_; + std::mutex success_schema_mutex_; - std::unordered_map success_schema_; + std::unordered_map + success_schema_; // Internal env::Command command_; diff --git a/buildcc/lib/target/src/custom_generator/custom_generator.cpp b/buildcc/lib/target/src/custom_generator/custom_generator.cpp index c7cb8725..f0c7ef1c 100644 --- a/buildcc/lib/target/src/custom_generator/custom_generator.cpp +++ b/buildcc/lib/target/src/custom_generator/custom_generator.cpp @@ -60,7 +60,7 @@ void CustomGenerator::AddIdInfo( fmt::format("Duplicate id {} detected", id)); ASSERT_FATAL(generate_cb, "Invalid callback provided"); - UserIdInfo schema; + UserCustomGeneratorSchema::UserIdInfo schema; for (const auto &i : inputs) { fs::path input = string_as_path(command_.Construct(i)); schema.inputs.emplace(std::move(input)); @@ -71,6 +71,7 @@ void CustomGenerator::AddIdInfo( } schema.generate_cb = generate_cb; schema.blob_handler = std::move(blob_handler); + user_.ids.try_emplace(id, std::move(schema)); ungrouped_ids_.emplace(id); } @@ -127,48 +128,30 @@ void CustomGenerator::Initialize() { tf_.name(name_); } -void CustomGenerator::BuildGenerate( - std::unordered_set &gen_selected_ids, - std::unordered_set &dummy_gen_selected_ids) { +/** + * @brief Check which ids need to be rebuilt + * + * @param gen_selected_ids + * @param dummy_gen_selected_ids + */ +void CustomGenerator::BuildGenerate() { if (!serialization_.IsLoaded()) { - std::for_each(user_.ids.begin(), user_.ids.end(), [&](const auto &iter) { - gen_selected_ids.insert(iter.first); - }); + comparator_.AddAllIds(); dirty_ = true; } else { - // DONE, Conditionally select internal_ids depending on what has - // changed - const auto &prev_ids = serialization_.GetLoad().internal_ids; - const auto &curr_ids = user_.ids; - - // DONE, MAP REMOVED condition Check if prev_ids exists in - // curr_ids If prev_ids does not exist in - // curr_ids, has been removed from existing build We need this - // condition to only set the dirty_ flag - for (const auto &[id, _] : prev_ids) { - if (curr_ids.find(id) == curr_ids.end()) { - // MAP REMOVED condition - IdRemoved(); - dirty_ = true; - break; - } + comparator_.CompareIds(); + + const bool is_removed = !comparator_.RemovedIds().empty(); + const bool is_added = !comparator_.AddedIds().empty(); + dirty_ = is_removed || is_added; + + if (is_removed) { + IdRemoved(); } - // DONE, MAP ADDED condition Check if curr_ids exists in - // prev_ids If curr_ids does not exist in - // prev_ids, has been added to existing build - for (const auto &[id, _] : curr_ids) { - if (prev_ids.find(id) == prev_ids.end()) { - // MAP ADDED condition - IdAdded(); - gen_selected_ids.insert(id); - dirty_ = true; - } else { - // MAP UPDATED condition (*checked later) - // This is because tasks can have dependencies amongst each other we can - // compute task level rebuilds later - dummy_gen_selected_ids.insert(id); - } + for (const auto &id : comparator_.AddedIds()) { + (void)id; + IdAdded(); } } } @@ -183,9 +166,7 @@ void CustomGenerator::GenerateTask() { std::unordered_map registered_tasks; // Selected ids for build - std::unordered_set selected_ids; - std::unordered_set dummy_selected_ids; - BuildGenerate(selected_ids, dummy_selected_ids); + BuildGenerate(); // Grouped tasks for (const auto &[first, second] : grouped_ids_) { @@ -199,7 +180,7 @@ void CustomGenerator::GenerateTask() { } for (const auto &id : group_metadata.ids) { - bool build = selected_ids.count(id) == 1; + bool build = comparator_.AddedId(id); auto task = CreateTaskRunner(s, build, id); task.name(id); reg_tasks.try_emplace(id, task); @@ -217,7 +198,7 @@ void CustomGenerator::GenerateTask() { // Ungrouped tasks for (const auto &id : ungrouped_ids_) { - bool build = selected_ids.count(id) == 1; + bool build = comparator_.AddedId(id); auto task = CreateTaskRunner(subflow, build, id); task.name(id); registered_tasks.try_emplace(id, task); From 3b1d63e4d8edb7f85bf3c840ae374fb6d3818ae5 Mon Sep 17 00:00:00 2001 From: Niket Naidu Date: Mon, 7 Nov 2022 08:50:06 +0000 Subject: [PATCH 02/19] Updated CreateTaskRunner and TaskRunner APIs --- .../target/include/target/custom_generator.h | 5 ++-- .../src/custom_generator/custom_generator.cpp | 28 ++++++++----------- 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/buildcc/lib/target/include/target/custom_generator.h b/buildcc/lib/target/include/target/custom_generator.h index d8819bd5..5f381a6d 100644 --- a/buildcc/lib/target/include/target/custom_generator.h +++ b/buildcc/lib/target/include/target/custom_generator.h @@ -181,9 +181,8 @@ class CustomGenerator : public internal::BuilderInterface { private: void Initialize(); - void TaskRunner(bool run, const std::string &id); - tf::Task CreateTaskRunner(tf::Subflow &subflow, bool build, - const std::string &id); + tf::Task CreateTaskRunner(tf::Subflow &subflow, const std::string &id); + void TaskRunner(const std::string &id); void GenerateTask(); void BuildGenerate(); diff --git a/buildcc/lib/target/src/custom_generator/custom_generator.cpp b/buildcc/lib/target/src/custom_generator/custom_generator.cpp index f0c7ef1c..a2267158 100644 --- a/buildcc/lib/target/src/custom_generator/custom_generator.cpp +++ b/buildcc/lib/target/src/custom_generator/custom_generator.cpp @@ -180,8 +180,7 @@ void CustomGenerator::GenerateTask() { } for (const auto &id : group_metadata.ids) { - bool build = comparator_.AddedId(id); - auto task = CreateTaskRunner(s, build, id); + auto task = CreateTaskRunner(s, id); task.name(id); reg_tasks.try_emplace(id, task); } @@ -198,8 +197,7 @@ void CustomGenerator::GenerateTask() { // Ungrouped tasks for (const auto &id : ungrouped_ids_) { - bool build = comparator_.AddedId(id); - auto task = CreateTaskRunner(subflow, build, id); + auto task = CreateTaskRunner(subflow, id); task.name(id); registered_tasks.try_emplace(id, task); } @@ -243,21 +241,21 @@ void CustomGenerator::InvokeDependencyCb( } } -tf::Task CustomGenerator::CreateTaskRunner(tf::Subflow &subflow, bool build, +tf::Task CustomGenerator::CreateTaskRunner(tf::Subflow &subflow, const std::string &id) { - return subflow.emplace([&, build, id]() { + return subflow.emplace([&, id]() { if (env::get_task_state() != env::TaskState::SUCCESS) { return; } try { - TaskRunner(build, id); + TaskRunner(id); } catch (...) { env::set_task_state(env::TaskState::FAILURE); } }); } -void CustomGenerator::TaskRunner(bool run, const std::string &id) { +void CustomGenerator::TaskRunner(const std::string &id) { // Convert { auto &curr_id_info = user_.ids.at(id); @@ -270,23 +268,21 @@ void CustomGenerator::TaskRunner(bool run, const std::string &id) { // Run const auto ¤t_id_info = user_.ids.at(id); - bool rerun = false; - if (run) { - rerun = true; - } else { + bool run = comparator_.AddedId(id); + if (!run) { const auto &previous_info = serialization_.GetLoad().internal_ids.at(id); - rerun = + run = internal::CheckPaths(previous_info.internal_inputs, current_id_info.internal_inputs) != internal::PathState::kNoChange || internal::CheckChanged(previous_info.outputs, current_id_info.outputs); - if (!rerun && current_id_info.blob_handler != nullptr) { - rerun = current_id_info.blob_handler->CheckChanged( + if (!run && current_id_info.blob_handler != nullptr) { + run = current_id_info.blob_handler->CheckChanged( previous_info.userblob, current_id_info.userblob); } } - if (rerun) { + if (run) { dirty_ = true; buildcc::CustomGeneratorContext ctx(command_, current_id_info.inputs, current_id_info.outputs, From 2395186dac0c2a838142ebeaccfe8173b01b5d5e Mon Sep 17 00:00:00 2001 From: Niket Naidu Date: Thu, 10 Nov 2022 04:36:53 +0000 Subject: [PATCH 03/19] Added GroupRunner --- .../target/include/target/custom_generator.h | 53 +++++++------- .../src/custom_generator/custom_generator.cpp | 71 +++++++++++-------- 2 files changed, 66 insertions(+), 58 deletions(-) diff --git a/buildcc/lib/target/include/target/custom_generator.h b/buildcc/lib/target/include/target/custom_generator.h index 5f381a6d..52d68d10 100644 --- a/buildcc/lib/target/include/target/custom_generator.h +++ b/buildcc/lib/target/include/target/custom_generator.h @@ -178,43 +178,18 @@ class CustomGenerator : public internal::BuilderInterface { const fs::path &GetBuildDir() const { return env_.GetTargetBuildDir(); } const std::string &Get(const std::string &file_identifier) const; -private: - void Initialize(); - - tf::Task CreateTaskRunner(tf::Subflow &subflow, const std::string &id); - void TaskRunner(const std::string &id); - - void GenerateTask(); - void BuildGenerate(); - - void InvokeDependencyCb(std::unordered_map - &®istered_tasks) const noexcept; - - // Recheck states - void IdRemoved(); - void IdAdded(); - void IdUpdated(); - -protected: - const env::Command &ConstCommand() const { return command_; } - env::Command &RefCommand() { return command_; } - private: struct GroupMetadata { std::vector ids; DependencyCb dependency_cb; - void InvokeDependencyCb(const std::string &group_id, - std::unordered_map + void InvokeDependencyCb(std::unordered_map &®istered_tasks) const noexcept { if (dependency_cb) { try { dependency_cb(std::move(registered_tasks)); } catch (...) { - env::log_critical( - __FUNCTION__, - fmt::format("Dependency callback failed for group id {}", - group_id)); + // TODO, Put a logging message here env::set_task_state(env::TaskState::FAILURE); } } @@ -317,6 +292,30 @@ class CustomGenerator : public internal::BuilderInterface { }; }; +private: + void Initialize(); + + tf::Task CreateGroupTask(tf::FlowBuilder &builder, + const GroupMetadata &group_metadata); + + tf::Task CreateTaskRunner(tf::Subflow &subflow, const std::string &id); + void TaskRunner(const std::string &id); + + void GenerateTask(); + void BuildGenerate(); + + void InvokeDependencyCb(std::unordered_map + &®istered_tasks) const noexcept; + + // Recheck states + void IdRemoved(); + void IdAdded(); + void IdUpdated(); + +protected: + const env::Command &ConstCommand() const { return command_; } + env::Command &RefCommand() { return command_; } + private: std::string name_; TargetEnv env_; diff --git a/buildcc/lib/target/src/custom_generator/custom_generator.cpp b/buildcc/lib/target/src/custom_generator/custom_generator.cpp index a2267158..c868ffa0 100644 --- a/buildcc/lib/target/src/custom_generator/custom_generator.cpp +++ b/buildcc/lib/target/src/custom_generator/custom_generator.cpp @@ -139,6 +139,7 @@ void CustomGenerator::BuildGenerate() { comparator_.AddAllIds(); dirty_ = true; } else { + // For IDS comparator_.CompareIds(); const bool is_removed = !comparator_.RemovedIds().empty(); @@ -153,6 +154,11 @@ void CustomGenerator::BuildGenerate() { (void)id; IdAdded(); } + + // For GROUPS + // Group Removed + // Group Added + // Group Updated } } @@ -163,34 +169,14 @@ void CustomGenerator::GenerateTask() { } try { - std::unordered_map registered_tasks; - // Selected ids for build BuildGenerate(); + std::unordered_map registered_tasks; + // Grouped tasks - for (const auto &[first, second] : grouped_ids_) { - const auto &group_id = first; - const auto &group_metadata = second; - auto group_task = subflow.emplace([&](tf::Subflow &s) { - std::unordered_map reg_tasks; - - if (env::get_task_state() != env::TaskState::SUCCESS) { - return; - } - - for (const auto &id : group_metadata.ids) { - auto task = CreateTaskRunner(s, id); - task.name(id); - reg_tasks.try_emplace(id, task); - } - - // Dependency callback - group_metadata.InvokeDependencyCb(group_id, std::move(reg_tasks)); - - // NOTE, Do not call detach otherwise this will fail - s.join(); - }); + for (const auto &[group_id, group_metadata] : grouped_ids_) { + auto group_task = CreateGroupTask(subflow, group_metadata); group_task.name(group_id); registered_tasks.try_emplace(group_id, group_task); } @@ -208,7 +194,7 @@ void CustomGenerator::GenerateTask() { // NOTE, Do not call detach otherwise this will fail subflow.join(); - // Store dummy_selected and successfully run schema + // Store if (dirty_) { UserCustomGeneratorSchema user_final_schema; user_final_schema.ids.insert(success_schema_.begin(), @@ -241,6 +227,28 @@ void CustomGenerator::InvokeDependencyCb( } } +tf::Task CustomGenerator::CreateGroupTask(tf::FlowBuilder &builder, + const GroupMetadata &group_metadata) { + return builder.emplace([&](tf::Subflow &s) { + if (env::get_task_state() != env::TaskState::SUCCESS) { + return; + } + + std::unordered_map registered_tasks; + for (const auto &id : group_metadata.ids) { + auto task = CreateTaskRunner(s, id); + task.name(id); + registered_tasks.try_emplace(id, task); + } + + // Dependency callback + group_metadata.InvokeDependencyCb(std::move(registered_tasks)); + + // NOTE, Do not call detach otherwise this will fail + s.join(); + }); +} + tf::Task CustomGenerator::CreateTaskRunner(tf::Subflow &subflow, const std::string &id) { return subflow.emplace([&, id]() { @@ -258,12 +266,13 @@ tf::Task CustomGenerator::CreateTaskRunner(tf::Subflow &subflow, void CustomGenerator::TaskRunner(const std::string &id) { // Convert { - auto &curr_id_info = user_.ids.at(id); - curr_id_info.internal_inputs = internal::path_schema_convert( - curr_id_info.inputs, internal::Path::CreateExistingPath); - curr_id_info.userblob = curr_id_info.blob_handler != nullptr - ? curr_id_info.blob_handler->GetSerializedData() - : std::vector(); + auto ¤t_id_info = user_.ids.at(id); + current_id_info.internal_inputs = internal::path_schema_convert( + current_id_info.inputs, internal::Path::CreateExistingPath); + current_id_info.userblob = + current_id_info.blob_handler != nullptr + ? current_id_info.blob_handler->GetSerializedData() + : std::vector(); } // Run From 83fab1932a6a5be32350fabd3eb9cf97823e5929 Mon Sep 17 00:00:00 2001 From: Niket Naidu Date: Sat, 19 Nov 2022 05:59:53 -0800 Subject: [PATCH 04/19] Removed dependency_cb --- .../target/include/target/custom_generator.h | 38 +- .../target/include/target/file_generator.h | 1 - .../include/target/template_generator.h | 1 - .../src/custom_generator/custom_generator.cpp | 27 +- .../test/target/test_custom_generator.cpp | 525 ------------------ 5 files changed, 19 insertions(+), 573 deletions(-) diff --git a/buildcc/lib/target/include/target/custom_generator.h b/buildcc/lib/target/include/target/custom_generator.h index 52d68d10..53917afb 100644 --- a/buildcc/lib/target/include/target/custom_generator.h +++ b/buildcc/lib/target/include/target/custom_generator.h @@ -147,8 +147,7 @@ class CustomGenerator : public internal::BuilderInterface { // TODO, Doc void AddGroupInfo(const std::string &group_id, - std::initializer_list ids, - const DependencyCb &dependency_cb = DependencyCb()); + std::initializer_list ids); // Callbacks /** @@ -165,7 +164,7 @@ class CustomGenerator : public internal::BuilderInterface { * @param dependency_cb Unordered map of `id` and `task` * The map can be safely mutated. */ - void AddDependencyCb(const DependencyCb &dependency_cb); + // void AddDependencyCb(const DependencyCb &dependency_cb); void Build() override; @@ -181,19 +180,18 @@ class CustomGenerator : public internal::BuilderInterface { private: struct GroupMetadata { std::vector ids; - DependencyCb dependency_cb; - - void InvokeDependencyCb(std::unordered_map - &®istered_tasks) const noexcept { - if (dependency_cb) { - try { - dependency_cb(std::move(registered_tasks)); - } catch (...) { - // TODO, Put a logging message here - env::set_task_state(env::TaskState::FAILURE); - } - } - } + // DependencyCb dependency_cb; + // void InvokeDependencyCb(std::unordered_map + // &®istered_tasks) const noexcept { + // if (dependency_cb) { + // try { + // dependency_cb(std::move(registered_tasks)); + // } catch (...) { + // // TODO, Put a logging message here + // env::set_task_state(env::TaskState::FAILURE); + // } + // } + // } }; struct Comparator { @@ -304,8 +302,8 @@ class CustomGenerator : public internal::BuilderInterface { void GenerateTask(); void BuildGenerate(); - void InvokeDependencyCb(std::unordered_map - &®istered_tasks) const noexcept; + // void InvokeDependencyCb(std::unordered_map + // &®istered_tasks) const noexcept; // Recheck states void IdRemoved(); @@ -336,8 +334,8 @@ class CustomGenerator : public internal::BuilderInterface { // Internal env::Command command_; - // Callbacks - DependencyCb dependency_cb_; + // // Callbacks + // DependencyCb dependency_cb_; }; } // namespace buildcc diff --git a/buildcc/lib/target/include/target/file_generator.h b/buildcc/lib/target/include/target/file_generator.h index e11b2b21..d49fddbe 100644 --- a/buildcc/lib/target/include/target/file_generator.h +++ b/buildcc/lib/target/include/target/file_generator.h @@ -68,7 +68,6 @@ class FileGenerator : public CustomGenerator { // Restrict access to certain custom generator APIs private: - using CustomGenerator::AddDependencyCb; using CustomGenerator::AddGroupInfo; using CustomGenerator::AddIdInfo; using CustomGenerator::Build; diff --git a/buildcc/lib/target/include/target/template_generator.h b/buildcc/lib/target/include/target/template_generator.h index 79706a4c..12c89aec 100644 --- a/buildcc/lib/target/include/target/template_generator.h +++ b/buildcc/lib/target/include/target/template_generator.h @@ -42,7 +42,6 @@ class TemplateGenerator : public CustomGenerator { // Restrict access to certain custom generator APIs private: - using CustomGenerator::AddDependencyCb; using CustomGenerator::AddGroupInfo; using CustomGenerator::AddIdInfo; using CustomGenerator::Build; diff --git a/buildcc/lib/target/src/custom_generator/custom_generator.cpp b/buildcc/lib/target/src/custom_generator/custom_generator.cpp index c868ffa0..26c8fd32 100644 --- a/buildcc/lib/target/src/custom_generator/custom_generator.cpp +++ b/buildcc/lib/target/src/custom_generator/custom_generator.cpp @@ -77,8 +77,7 @@ void CustomGenerator::AddIdInfo( } void CustomGenerator::AddGroupInfo(const std::string &group_id, - std::initializer_list ids, - const DependencyCb &dependency_cb) { + std::initializer_list ids) { // Verify that the ids exist // Remove those ids from ungrouped_ids for (const auto &id : ids) { @@ -93,14 +92,9 @@ void CustomGenerator::AddGroupInfo(const std::string &group_id, // Group map is used to group similar ids in a single subflow GroupMetadata group_metadata; group_metadata.ids = ids; - group_metadata.dependency_cb = dependency_cb; grouped_ids_.try_emplace(group_id, std::move(group_metadata)); } -void CustomGenerator::AddDependencyCb(const DependencyCb &dependency_cb) { - dependency_cb_ = dependency_cb; -} - void CustomGenerator::Build() { (void)serialization_.LoadFromFile(); @@ -188,9 +182,6 @@ void CustomGenerator::GenerateTask() { registered_tasks.try_emplace(id, task); } - // Dependencies between tasks - InvokeDependencyCb(std::move(registered_tasks)); - // NOTE, Do not call detach otherwise this will fail subflow.join(); @@ -214,19 +205,6 @@ void CustomGenerator::GenerateTask() { generate_task.name(kGenerateTaskName); } -void CustomGenerator::InvokeDependencyCb( - std::unordered_map &®istered_tasks) - const noexcept { - if (dependency_cb_) { - try { - dependency_cb_(std::move(registered_tasks)); - } catch (...) { - env::log_critical(__FUNCTION__, "Dependency callback failed"); - env::set_task_state(env::TaskState::FAILURE); - } - } -} - tf::Task CustomGenerator::CreateGroupTask(tf::FlowBuilder &builder, const GroupMetadata &group_metadata) { return builder.emplace([&](tf::Subflow &s) { @@ -241,9 +219,6 @@ tf::Task CustomGenerator::CreateGroupTask(tf::FlowBuilder &builder, registered_tasks.try_emplace(id, task); } - // Dependency callback - group_metadata.InvokeDependencyCb(std::move(registered_tasks)); - // NOTE, Do not call detach otherwise this will fail s.join(); }); diff --git a/buildcc/lib/target/test/target/test_custom_generator.cpp b/buildcc/lib/target/test/target/test_custom_generator.cpp index a6c6597b..0faa6940 100644 --- a/buildcc/lib/target/test/target/test_custom_generator.cpp +++ b/buildcc/lib/target/test/target/test_custom_generator.cpp @@ -115,66 +115,6 @@ TEST(CustomGeneratorTestGroup, Basic_Group) { } } -TEST(CustomGeneratorTestGroup, Basic_Group_Dependency) { - buildcc::CustomGenerator cgen("basic_group_dependency", ""); - cgen.AddIdInfo("id1", {"{current_root_dir}/dummy_main.c"}, - {"{current_build_dir}/dummy_main.o"}, BasicGenerateCb); - cgen.AddIdInfo("id2", {"{current_root_dir}/dummy_main.cpp"}, {}, - BasicGenerateCb); - cgen.AddGroupInfo("grouped_id1_and_id2", {"id1", "id2"}, [](auto &&task_map) { - task_map.at("id1").precede(task_map.at("id2")); - }); - cgen.Build(); - - mock().expectOneCall("BasicGenerateCb").andReturnValue(true); - mock().expectOneCall("BasicGenerateCb").andReturnValue(true); - buildcc::m::CustomGeneratorRunner(cgen); - - // Serialization check - { - buildcc::internal::CustomGeneratorSerialization serialization( - cgen.GetBinaryPath()); - CHECK_TRUE(serialization.LoadFromFile()); - - const auto &internal_map = serialization.GetLoad().internal_ids; - CHECK_EQUAL(internal_map.size(), 2); - const auto &id1_info = internal_map.at("id1"); - CHECK_EQUAL(id1_info.internal_inputs.size(), 1); - CHECK_EQUAL(id1_info.outputs.size(), 1); - - const auto &id2_info = internal_map.at("id2"); - CHECK_EQUAL(id2_info.internal_inputs.size(), 1); - CHECK_EQUAL(id2_info.outputs.size(), 0); - } -} - -TEST(CustomGeneratorTestGroup, Basic_Group_DependencyFailure) { - buildcc::CustomGenerator cgen("basic_group_dependency_failure", ""); - cgen.AddIdInfo("id1", {"{current_root_dir}/dummy_main.c"}, - {"{current_build_dir}/dummy_main.o"}, BasicGenerateCb); - cgen.AddIdInfo("id2", {"{current_root_dir}/dummy_main.cpp"}, {}, - BasicGenerateCb); - cgen.AddGroupInfo("grouped_id1_and_id2", {"id1", "id2"}, [](auto &&task_map) { - task_map.at("id1").precede(task_map.at("id2")); - buildcc::env::assert_fatal("Failure"); - }); - cgen.Build(); - - buildcc::m::CustomGeneratorRunner(cgen); - - // Serialization check - { - buildcc::internal::CustomGeneratorSerialization serialization( - cgen.GetBinaryPath()); - CHECK_TRUE(serialization.LoadFromFile()); - - const auto &internal_map = serialization.GetLoad().internal_ids; - CHECK_EQUAL(internal_map.size(), 0); - } - - CHECK(buildcc::env::get_task_state() == buildcc::env::TaskState::FAILURE); -} - bool FailureCb(const buildcc::CustomGeneratorContext &ctx) { (void)ctx; return false; @@ -184,34 +124,6 @@ bool SuccessCb(const buildcc::CustomGeneratorContext &ctx) { return true; } -// An ungrouped task a dependency on a grouped task and fail the -// ungrouped task -TEST(CustomGeneratorTestGroup, Basic_Group_DependencyFailure2) { - buildcc::CustomGenerator cgen("basic_group_dependency_failure2", ""); - cgen.AddIdInfo("id1", {"{current_root_dir}/dummy_main.c"}, - {"{current_build_dir}/dummy_main.o"}, FailureCb); - cgen.AddIdInfo("id2", {"{current_root_dir}/dummy_main.cpp"}, {}, SuccessCb); - cgen.AddGroupInfo("grouped_id2", {"id2"}); - cgen.AddDependencyCb([&](auto &&task_map) { - task_map.at("id1").precede(task_map.at("grouped_id2")); - }); - cgen.Build(); - - buildcc::m::CustomGeneratorRunner(cgen); - - // Serialization check - { - buildcc::internal::CustomGeneratorSerialization serialization( - cgen.GetBinaryPath()); - CHECK_TRUE(serialization.LoadFromFile()); - - const auto &internal_map = serialization.GetLoad().internal_ids; - CHECK_EQUAL(internal_map.size(), 0); - } - - CHECK(buildcc::env::get_task_state() == buildcc::env::TaskState::FAILURE); -} - // Behaviour // Initial: A | B -> Passes // Changes: (GID:NEW)[A -> B] -> No rebuild triggered @@ -254,65 +166,6 @@ static bool ProperDependency2(const buildcc::CustomGeneratorContext &ctx) { return rebuild_value; } -// ProperDependency2 depends on ProperDependency1 completion -TEST(CustomGeneratorTestGroup, Basic_ProperDependency_GoodCase) { - rebuild_value = false; - - buildcc::CustomGenerator cgen("basic_proper_dependency_good_case", ""); - cgen.AddIdInfo("id1", {"{current_root_dir}/dummy_main.c"}, - {"{current_build_dir}/dummy_main.o"}, ProperDependency1); - cgen.AddIdInfo("id2", {"{current_root_dir}/dummy_main.cpp"}, {}, - ProperDependency2); - cgen.AddDependencyCb( - [](auto &&task_map) { task_map.at("id1").precede(task_map.at("id2")); }); - cgen.Build(); - - mock().expectOneCall("ProperDependency1"); - mock().expectOneCall("ProperDependency2"); - buildcc::m::CustomGeneratorRunner(cgen); - - // Serialization check - { - buildcc::internal::CustomGeneratorSerialization serialization( - cgen.GetBinaryPath()); - CHECK_TRUE(serialization.LoadFromFile()); - - const auto &internal_map = serialization.GetLoad().internal_ids; - CHECK_EQUAL(internal_map.size(), 2); - } - - CHECK(buildcc::env::get_task_state() == buildcc::env::TaskState::SUCCESS); -} - -// ProperDependency2 depends on ProperDependency1 completion -TEST(CustomGeneratorTestGroup, Basic_ProperDependency_BadCase) { - rebuild_value = false; - - buildcc::CustomGenerator cgen("basic_proper_dependency_bad_case", ""); - cgen.AddIdInfo("id1", {"{current_root_dir}/dummy_main.c"}, - {"{current_build_dir}/dummy_main.o"}, ProperDependency1); - cgen.AddIdInfo("id2", {"{current_root_dir}/dummy_main.cpp"}, {}, - ProperDependency2); - cgen.AddDependencyCb( - [](auto &&task_map) { task_map.at("id2").precede(task_map.at("id1")); }); - cgen.Build(); - - mock().expectOneCall("ProperDependency2"); - buildcc::m::CustomGeneratorRunner(cgen); - - // Serialization check - { - buildcc::internal::CustomGeneratorSerialization serialization( - cgen.GetBinaryPath()); - CHECK_TRUE(serialization.LoadFromFile()); - - const auto &internal_map = serialization.GetLoad().internal_ids; - CHECK_EQUAL(internal_map.size(), 0); - } - - CHECK(buildcc::env::get_task_state() == buildcc::env::TaskState::FAILURE); -} - TEST(CustomGeneratorTestGroup, DefaultArgumentUsage) { buildcc::CustomGenerator cgen("default_argument_usage", ""); cgen.AddPatterns({ @@ -415,30 +268,6 @@ static void DependencyCb(std::unordered_map &&task_map) { task_map.at("id1").precede(task_map.at("id2")); } -TEST(CustomGeneratorTestGroup, AddDependency_BasicCheck) { - constexpr const char *const kGenName = "add_dependency_basic_check"; - { - buildcc::CustomGenerator cgen(kGenName, ""); - cgen.AddIdInfo("id2", {"{current_root_dir}/dummy_main.c"}, - {"{current_build_dir}/dummy_main.o"}, Dep2Cb); - cgen.AddIdInfo("id1", {"{current_root_dir}/dummy_main.cpp"}, - {"{current_build_dir}/dummy_main.o"}, Dep1Cb); - cgen.AddDependencyCb(DependencyCb); - cgen.Build(); - - mock().expectOneCall("Dep1Cb"); - buildcc::env::m::CommandExpect_Execute(1, true); - mock().expectOneCall("Dep2Cb"); - buildcc::env::m::CommandExpect_Execute(1, true); - buildcc::m::CustomGeneratorRunner(cgen); - - buildcc::internal::CustomGeneratorSerialization serialization( - cgen.GetBinaryPath()); - CHECK_TRUE(serialization.LoadFromFile()); - CHECK_EQUAL(serialization.GetLoad().internal_ids.size(), 2); - } -} - static bool FileDep1Cb(const buildcc::CustomGeneratorContext &ctx) { mock().actualCall("FileDep1Cb"); for (const auto &o : ctx.outputs) { @@ -455,157 +284,6 @@ static bool FileDep2Cb(const buildcc::CustomGeneratorContext &ctx) { return true; } -TEST(CustomGeneratorTestGroup, AddDependency_FileDep) { - constexpr const char *const kGenName = "add_dependency_file_dep"; - { - buildcc::CustomGenerator cgen(kGenName, ""); - cgen.AddIdInfo("id1", {"{current_root_dir}/dummy_main.c"}, - {"{current_build_dir}/dummy_main.o"}, FileDep1Cb); - cgen.AddIdInfo("id2", {"{current_build_dir}/dummy_main.o"}, {}, FileDep2Cb); - cgen.AddDependencyCb(DependencyCb); - cgen.Build(); - - mock().expectOneCall("FileDep1Cb"); - mock().expectOneCall("FileDep2Cb"); - buildcc::m::CustomGeneratorRunner(cgen); - - buildcc::internal::CustomGeneratorSerialization serialization( - cgen.GetBinaryPath()); - CHECK_TRUE(serialization.LoadFromFile()); - CHECK_EQUAL(serialization.GetLoad().internal_ids.size(), 2); - } -} - -TEST(CustomGeneratorTestGroup, AddDependency_FileDep_WithRebuild) { - constexpr const char *const kGenName = "add_dependency_file_dep_with_rebuild"; - - fs::path kInputFile = - (BUILD_DIR / kGenName / "dummy_main.c").make_preferred(); - UT_PRINT(kInputFile.string().c_str()); - fs::create_directories(BUILD_DIR / kGenName); - CHECK_TRUE(buildcc::env::save_file(kInputFile.string().c_str(), "", false)); - - { - buildcc::CustomGenerator cgen(kGenName, ""); - cgen.AddIdInfo("id1", {"{current_build_dir}/dummy_main.c"}, - {"{current_build_dir}/dummy_main.o"}, FileDep1Cb); - cgen.AddIdInfo("id2", {"{current_build_dir}/dummy_main.o"}, {}, FileDep2Cb); - cgen.AddDependencyCb(DependencyCb); - cgen.Build(); - - mock().expectOneCall("FileDep1Cb"); - mock().expectOneCall("FileDep2Cb"); - buildcc::m::CustomGeneratorRunner(cgen); - - buildcc::internal::CustomGeneratorSerialization serialization( - cgen.GetBinaryPath()); - CHECK_TRUE(serialization.LoadFromFile()); - CHECK_EQUAL(serialization.GetLoad().internal_ids.size(), 2); - - CHECK(buildcc::env::get_task_state() == buildcc::env::TaskState::SUCCESS); - } - - // Same, no change - { - buildcc::CustomGenerator cgen(kGenName, ""); - cgen.AddIdInfo("id1", {"{current_build_dir}/dummy_main.c"}, - {"{current_build_dir}/dummy_main.o"}, FileDep1Cb); - cgen.AddIdInfo("id2", {"{current_build_dir}/dummy_main.o"}, {}, FileDep2Cb); - cgen.AddDependencyCb(DependencyCb); - cgen.Build(); - - buildcc::m::CustomGeneratorRunner(cgen); - - buildcc::internal::CustomGeneratorSerialization serialization( - cgen.GetBinaryPath()); - CHECK_TRUE(serialization.LoadFromFile()); - CHECK_EQUAL(serialization.GetLoad().internal_ids.size(), 2); - - CHECK(buildcc::env::get_task_state() == buildcc::env::TaskState::SUCCESS); - } - - // reset - fs::remove_all(BUILD_DIR / kGenName / "dummy_main.o"); - - // Remove id1, should cause id2 to fail - // NOTE, dirty_ == false is not made true when id2 is run, however id removed - // sets dirty_ == true - { - buildcc::CustomGenerator cgen(kGenName, ""); - cgen.AddIdInfo("id2", {"{current_build_dir}/dummy_main.o"}, {}, FileDep2Cb); - cgen.AddDependencyCb(DependencyCb); - cgen.Build(); - - buildcc::m::CustomGeneratorExpect_IdRemoved(1, &cgen); - buildcc::m::CustomGeneratorRunner(cgen); - - buildcc::internal::CustomGeneratorSerialization serialization( - cgen.GetBinaryPath()); - CHECK_TRUE(serialization.LoadFromFile()); - CHECK_EQUAL(serialization.GetLoad().internal_ids.size(), 0); - - CHECK(buildcc::env::get_task_state() == buildcc::env::TaskState::FAILURE); - } - - // reset - buildcc::env::set_task_state(buildcc::env::TaskState::SUCCESS); - fs::remove_all(BUILD_DIR / kGenName / "dummy_main.o"); - - // Added - { - buildcc::CustomGenerator cgen(kGenName, ""); - cgen.AddIdInfo("id1", {"{current_build_dir}/dummy_main.c"}, - {"{current_build_dir}/dummy_main.o"}, FileDep1Cb); - cgen.AddIdInfo("id2", {"{current_build_dir}/dummy_main.o"}, {}, FileDep2Cb); - cgen.AddDependencyCb(DependencyCb); - cgen.Build(); - - buildcc::m::CustomGeneratorExpect_IdAdded(1, &cgen); - buildcc::m::CustomGeneratorExpect_IdAdded(1, &cgen); - mock().expectOneCall("FileDep1Cb"); - mock().expectOneCall("FileDep2Cb"); - buildcc::m::CustomGeneratorRunner(cgen); - - buildcc::internal::CustomGeneratorSerialization serialization( - cgen.GetBinaryPath()); - CHECK_TRUE(serialization.LoadFromFile()); - CHECK_EQUAL(serialization.GetLoad().internal_ids.size(), 2); - - CHECK(buildcc::env::get_task_state() == buildcc::env::TaskState::SUCCESS); - } - - // reset - buildcc::env::set_task_state(buildcc::env::TaskState::SUCCESS); - fs::remove_all(BUILD_DIR / kGenName / "dummy_main.o"); - - buildcc::m::blocking_sleep(1); - buildcc::env::save_file(kInputFile.string().c_str(), "", false); - - // Update id1:dummy_main.c -> updated dummy_main.o -> should rerun id2 as well - { - buildcc::CustomGenerator cgen(kGenName, ""); - - cgen.AddIdInfo("id1", {"{current_build_dir}/dummy_main.c"}, - {"{current_build_dir}/dummy_main.o"}, FileDep1Cb); - cgen.AddIdInfo("id2", {"{current_build_dir}/dummy_main.o"}, {}, FileDep2Cb); - cgen.AddDependencyCb(DependencyCb); - cgen.Build(); - - mock().expectOneCall("FileDep1Cb"); - mock().expectOneCall("FileDep2Cb"); - buildcc::m::CustomGeneratorRunner(cgen); - - buildcc::internal::CustomGeneratorSerialization serialization( - cgen.GetBinaryPath()); - CHECK_TRUE(serialization.LoadFromFile()); - CHECK_EQUAL(serialization.GetLoad().internal_ids.size(), 2); - - CHECK(buildcc::env::get_task_state() == buildcc::env::TaskState::SUCCESS); - } - - buildcc::env::set_task_state(buildcc::env::TaskState::SUCCESS); -} - static bool RealGenerateCb(const buildcc::CustomGeneratorContext &ctx) { (void)ctx; mock().actualCall("RealGenerateCb"); @@ -666,209 +344,6 @@ TEST(CustomGeneratorTestGroup, RealGenerate_Basic) { buildcc::env::set_task_state(buildcc::env::TaskState::SUCCESS); } -TEST(CustomGeneratorTestGroup, RealGenerate_RemoveAndAdd) { - constexpr const char *const kGenName = "real_generator_remove_and_add"; - { - buildcc::CustomGenerator cgen(kGenName, ""); - cgen.AddIdInfo("id1", {"{current_root_dir}/dummy_main.cpp"}, - {"{current_build_dir}/dummy_main.o"}, RealGenerateCb); - cgen.AddIdInfo("id2", {"{current_root_dir}/dummy_main.c"}, - {"{current_build_dir}/dummy_main.o"}, RealGenerateCb); - cgen.Build(); - - mock().expectOneCall("RealGenerateCb"); - buildcc::env::m::CommandExpect_Execute(1, true); - mock().expectOneCall("RealGenerateCb"); - buildcc::env::m::CommandExpect_Execute(1, true); - buildcc::m::CustomGeneratorRunner(cgen); - - buildcc::internal::CustomGeneratorSerialization serialization( - cgen.GetBinaryPath()); - CHECK_TRUE(serialization.LoadFromFile()); - CHECK_EQUAL(serialization.GetLoad().internal_ids.size(), 2); - auto imap = serialization.GetLoad().internal_ids; - CHECK_EQUAL(imap.at("id1").internal_inputs.size(), 1); - CHECK_EQUAL(imap.at("id2").internal_inputs.size(), 1); - - CHECK_EQUAL(imap.at("id1").outputs.size(), 1); - CHECK_EQUAL(imap.at("id2").outputs.size(), 1); - } - - // Same, no change - { - buildcc::CustomGenerator cgen(kGenName, ""); - cgen.AddIdInfo("id1", {"{current_root_dir}/dummy_main.cpp"}, - {"{current_build_dir}/dummy_main.o"}, RealGenerateCb); - cgen.AddIdInfo("id2", {"{current_root_dir}/dummy_main.c"}, - {"{current_build_dir}/dummy_main.o"}, RealGenerateCb); - cgen.Build(); - - buildcc::m::CustomGeneratorRunner(cgen); - - buildcc::internal::CustomGeneratorSerialization serialization( - cgen.GetBinaryPath()); - CHECK_TRUE(serialization.LoadFromFile()); - CHECK_EQUAL(serialization.GetLoad().internal_ids.size(), 2); - auto imap = serialization.GetLoad().internal_ids; - CHECK_EQUAL(imap.at("id1").internal_inputs.size(), 1); - CHECK_EQUAL(imap.at("id2").internal_inputs.size(), 1); - - CHECK_EQUAL(imap.at("id1").outputs.size(), 1); - CHECK_EQUAL(imap.at("id2").outputs.size(), 1); - } - - // Map Removed - { - buildcc::CustomGenerator cgen(kGenName, ""); - cgen.AddIdInfo("id1", {"{current_root_dir}/dummy_main.cpp"}, - {"{current_build_dir}/dummy_main.o"}, RealGenerateCb); - - cgen.Build(); - - buildcc::m::CustomGeneratorExpect_IdRemoved(1, &cgen); - buildcc::m::CustomGeneratorRunner(cgen); - - buildcc::internal::CustomGeneratorSerialization serialization( - cgen.GetBinaryPath()); - CHECK_TRUE(serialization.LoadFromFile()); - CHECK_EQUAL(serialization.GetLoad().internal_ids.size(), 1); - auto imap = serialization.GetLoad().internal_ids; - CHECK_EQUAL(imap.at("id1").internal_inputs.size(), 1); - CHECK_EQUAL(imap.at("id1").outputs.size(), 1); - - CHECK_THROWS(std::out_of_range, imap.at("id2")); - } - - // Map Added Failure - { - buildcc::CustomGenerator cgen(kGenName, ""); - cgen.AddIdInfo("id1", {"{current_root_dir}/dummy_main.cpp"}, - {"{current_build_dir}/dummy_main.o"}, RealGenerateCb); - cgen.AddIdInfo("id2", {"{current_root_dir}/dummy_main.c"}, - {"{current_build_dir}/dummy_main.o"}, RealGenerateCb); - cgen.AddDependencyCb([](auto &&task_map) { - task_map.at("id1").precede(task_map.at("id2")); - }); - cgen.Build(); - - buildcc::m::CustomGeneratorExpect_IdAdded(1, &cgen); - mock().expectOneCall("RealGenerateCb"); - buildcc::env::m::CommandExpect_Execute(1, false); - buildcc::m::CustomGeneratorRunner(cgen); - - buildcc::internal::CustomGeneratorSerialization serialization( - cgen.GetBinaryPath()); - CHECK_TRUE(serialization.LoadFromFile()); - CHECK_EQUAL(serialization.GetLoad().internal_ids.size(), 1); - auto imap = serialization.GetLoad().internal_ids; - CHECK_EQUAL(imap.at("id1").internal_inputs.size(), 1); - CHECK_EQUAL(imap.at("id1").outputs.size(), 1); - CHECK_THROWS(std::out_of_range, imap.at("id2")); - CHECK(buildcc::env::get_task_state() == buildcc::env::TaskState::FAILURE); - } - - buildcc::env::set_task_state(buildcc::env::TaskState::SUCCESS); - - // Map Added Success - { - buildcc::CustomGenerator cgen(kGenName, ""); - cgen.AddIdInfo("id1", {"{current_root_dir}/dummy_main.cpp"}, - {"{current_build_dir}/dummy_main.o"}, RealGenerateCb); - cgen.AddIdInfo("id2", {"{current_root_dir}/dummy_main.c"}, - {"{current_build_dir}/dummy_main.o"}, RealGenerateCb); - cgen.Build(); - - buildcc::m::CustomGeneratorExpect_IdAdded(1, &cgen); - mock().expectOneCall("RealGenerateCb"); - buildcc::env::m::CommandExpect_Execute(1, true); - buildcc::m::CustomGeneratorRunner(cgen); - - buildcc::internal::CustomGeneratorSerialization serialization( - cgen.GetBinaryPath()); - CHECK_TRUE(serialization.LoadFromFile()); - CHECK_EQUAL(serialization.GetLoad().internal_ids.size(), 2); - auto imap = serialization.GetLoad().internal_ids; - CHECK_EQUAL(imap.at("id1").internal_inputs.size(), 1); - CHECK_EQUAL(imap.at("id2").internal_inputs.size(), 1); - - CHECK_EQUAL(imap.at("id1").outputs.size(), 1); - CHECK_EQUAL(imap.at("id2").outputs.size(), 1); - } -} - -TEST(CustomGeneratorTestGroup, RealGenerate_Update_Failure) { - constexpr const char *const kGenName = "real_generator_update_failure"; - - { - buildcc::CustomGenerator cgen(kGenName, ""); - buildcc::env::save_file( - (cgen.GetBuildDir() / "dummy_main.c").string().c_str(), "", false); - buildcc::env::save_file( - (cgen.GetBuildDir() / "dummy_main.cpp").string().c_str(), "", false); - - cgen.AddIdInfo("id1", {"{current_build_dir}/dummy_main.c"}, - {"{current_build_dir}/dummy_main.o"}, RealGenerateCb); - cgen.AddIdInfo("id2", {"{current_build_dir}/dummy_main.cpp"}, - {"{current_build_dir}/other_dummy_main.o"}, RealGenerateCb); - cgen.AddDependencyCb([](auto &&task_map) { - task_map.at("id1").precede(task_map.at("id2")); - }); - cgen.Build(); - - mock().expectOneCall("RealGenerateCb"); - buildcc::env::m::CommandExpect_Execute(1, true); - mock().expectOneCall("RealGenerateCb"); - buildcc::env::m::CommandExpect_Execute(1, true); - buildcc::m::CustomGeneratorRunner(cgen); - - buildcc::internal::CustomGeneratorSerialization serialization( - cgen.GetBinaryPath()); - CHECK_TRUE(serialization.LoadFromFile()); - CHECK_EQUAL(serialization.GetLoad().internal_ids.size(), 2); - auto imap = serialization.GetLoad().internal_ids; - CHECK_EQUAL(imap.at("id1").internal_inputs.size(), 1); - CHECK_EQUAL(imap.at("id2").internal_inputs.size(), 1); - - CHECK_EQUAL(imap.at("id1").outputs.size(), 1); - CHECK_EQUAL(imap.at("id2").outputs.size(), 1); - } - - buildcc::m::blocking_sleep(1); - - // Updated Input file Failure - UT_PRINT("Updated Input file: Failure\r\n"); - { - buildcc::CustomGenerator cgen(kGenName, ""); - buildcc::env::save_file( - (cgen.GetBuildDir() / "dummy_main.cpp").string().c_str(), "", false); - - cgen.AddIdInfo("id1", {"{current_build_dir}/dummy_main.c"}, - {"{current_build_dir}/dummy_main.o"}, RealGenerateCb); - cgen.AddIdInfo("id2", {"{current_build_dir}/dummy_main.cpp"}, - {"{current_build_dir}/other_dummy_main.o"}, RealGenerateCb); - cgen.AddDependencyCb([](auto &&task_map) { - task_map.at("id1").precede(task_map.at("id2")); - }); - cgen.Build(); - - mock().expectOneCall("RealGenerateCb"); - buildcc::env::m::CommandExpect_Execute(1, false); - buildcc::m::CustomGeneratorRunner(cgen); - - buildcc::internal::CustomGeneratorSerialization serialization( - cgen.GetBinaryPath()); - CHECK_TRUE(serialization.LoadFromFile()); - CHECK_EQUAL(serialization.GetLoad().internal_ids.size(), 1); - auto imap = serialization.GetLoad().internal_ids; - CHECK_EQUAL(imap.at("id1").internal_inputs.size(), 1); - CHECK_EQUAL(imap.at("id1").outputs.size(), 1); - - CHECK(buildcc::env::get_task_state() == buildcc::env::TaskState::FAILURE); - } - - buildcc::env::set_task_state(buildcc::env::TaskState::SUCCESS); -} - TEST(CustomGeneratorTestGroup, RealGenerate_Update_Success) { constexpr const char *const kGenName = "real_generator_update_success"; From da0ef697a62eceb2f689fb8b5dacbfade0cb770c Mon Sep 17 00:00:00 2001 From: Niket Naidu Date: Sat, 19 Nov 2022 06:00:13 -0800 Subject: [PATCH 05/19] Update gcovr.cmake --- cmake/coverage/gcovr.cmake | 1 + 1 file changed, 1 insertion(+) diff --git a/cmake/coverage/gcovr.cmake b/cmake/coverage/gcovr.cmake index d127fe42..4d97ebbe 100644 --- a/cmake/coverage/gcovr.cmake +++ b/cmake/coverage/gcovr.cmake @@ -9,6 +9,7 @@ else() message("GCOVR at ${gcovr_program}") set(GCOVR_REMOVE_OPTIONS + --exclude "(.+/)?third_party(.+/)?" --exclude "(.+/)?spdlog(.+/)?" --exclude "(.+/)?fmt(.+/)?" --exclude "(.+/)?taskflow(.+/)?" From 9352f01933a1255ecbc6f5f5a72c9e4aedfd3864 Mon Sep 17 00:00:00 2001 From: Niket Naidu Date: Sat, 19 Nov 2022 06:03:58 -0800 Subject: [PATCH 06/19] Update test_custom_generator.cpp --- .../test/target/test_custom_generator.cpp | 74 ------------------- 1 file changed, 74 deletions(-) diff --git a/buildcc/lib/target/test/target/test_custom_generator.cpp b/buildcc/lib/target/test/target/test_custom_generator.cpp index 0faa6940..0812c1de 100644 --- a/buildcc/lib/target/test/target/test_custom_generator.cpp +++ b/buildcc/lib/target/test/target/test_custom_generator.cpp @@ -124,48 +124,6 @@ bool SuccessCb(const buildcc::CustomGeneratorContext &ctx) { return true; } -// Behaviour -// Initial: A | B -> Passes -// Changes: (GID:NEW)[A -> B] -> No rebuild triggered - -// Behaviour -// Initial: A | B -> Fails -// Changes: (GID:NEW)[A -> B] -> rebuild triggered due to previous failure - -// ! IMPORTANT -// * NOTE, It is users responsibility to make sure that when A -> B, A's data -// change should automatically trigger B - -// For example: Say A -> B i.e B depends on A -// In a typical scenario, B would depend on A's output -// To make sure B is triggered when A changes. Make sure you use A's output in -// B's userblob. -// In this way whenever A changes, B's userblob automatically becomes "newer" -// and triggers a rebuild as well - -// Say, A gives out "rebuild = true/false" as its output -// We can use this "rebuild" variable in B's userblob -// When A runs and "rebuild" changes from false to true, During the `TaskRunner` -// we check B's userblob and automatically invoke the `CheckChanged` virtual -// call -// TODO, Create a testcase for the above scenario (Advanced_DependencyRebuild -// scenario) - -// DONE, Make B fail because it properly depends on A -static bool rebuild_value{false}; -static bool ProperDependency1(const buildcc::CustomGeneratorContext &ctx) { - (void)ctx; - mock().actualCall("ProperDependency1"); - rebuild_value = true; - return true; -} - -static bool ProperDependency2(const buildcc::CustomGeneratorContext &ctx) { - (void)ctx; - mock().actualCall("ProperDependency2"); - return rebuild_value; -} - TEST(CustomGeneratorTestGroup, DefaultArgumentUsage) { buildcc::CustomGenerator cgen("default_argument_usage", ""); cgen.AddPatterns({ @@ -252,38 +210,6 @@ TEST(CustomGeneratorTestGroup, FailureCases) { buildcc::env::set_task_state(buildcc::env::TaskState::SUCCESS); } -static bool Dep1Cb(const buildcc::CustomGeneratorContext &ctx) { - (void)ctx; - mock().actualCall("Dep1Cb"); - return buildcc::env::Command::Execute(""); -} - -static bool Dep2Cb(const buildcc::CustomGeneratorContext &ctx) { - (void)ctx; - mock().actualCall("Dep2Cb"); - return buildcc::env::Command::Execute(""); -} - -static void DependencyCb(std::unordered_map &&task_map) { - task_map.at("id1").precede(task_map.at("id2")); -} - -static bool FileDep1Cb(const buildcc::CustomGeneratorContext &ctx) { - mock().actualCall("FileDep1Cb"); - for (const auto &o : ctx.outputs) { - CHECK_TRUE(buildcc::env::save_file(o.string().c_str(), "", false)); - } - return true; -} - -static bool FileDep2Cb(const buildcc::CustomGeneratorContext &ctx) { - mock().actualCall("FileDep2Cb"); - for (const auto &i : ctx.inputs) { - CHECK_TRUE(fs::exists(i)); - } - return true; -} - static bool RealGenerateCb(const buildcc::CustomGeneratorContext &ctx) { (void)ctx; mock().actualCall("RealGenerateCb"); From 33a11113cade000d8eb636921cc3cce5cf0b57e3 Mon Sep 17 00:00:00 2001 From: Niket Naidu Date: Sun, 20 Nov 2022 13:37:42 -0800 Subject: [PATCH 07/19] Removed groups from Custom Generator --- .../target/include/target/custom_generator.h | 31 +----------- .../target/include/target/file_generator.h | 1 - .../include/target/template_generator.h | 1 - .../src/custom_generator/custom_generator.cpp | 50 ------------------- .../test/target/test_custom_generator.cpp | 31 ------------ 5 files changed, 1 insertion(+), 113 deletions(-) diff --git a/buildcc/lib/target/include/target/custom_generator.h b/buildcc/lib/target/include/target/custom_generator.h index 53917afb..4c3a7e70 100644 --- a/buildcc/lib/target/include/target/custom_generator.h +++ b/buildcc/lib/target/include/target/custom_generator.h @@ -145,10 +145,6 @@ class CustomGenerator : public internal::BuilderInterface { const GenerateCb &generate_cb, std::shared_ptr blob_handler = nullptr); - // TODO, Doc - void AddGroupInfo(const std::string &group_id, - std::initializer_list ids); - // Callbacks /** * @brief Setup dependencies between Tasks using their `id` @@ -178,22 +174,6 @@ class CustomGenerator : public internal::BuilderInterface { const std::string &Get(const std::string &file_identifier) const; private: - struct GroupMetadata { - std::vector ids; - // DependencyCb dependency_cb; - // void InvokeDependencyCb(std::unordered_map - // &®istered_tasks) const noexcept { - // if (dependency_cb) { - // try { - // dependency_cb(std::move(registered_tasks)); - // } catch (...) { - // // TODO, Put a logging message here - // env::set_task_state(env::TaskState::FAILURE); - // } - // } - // } - }; - struct Comparator { Comparator(const internal::CustomGeneratorSchema &s, const UserCustomGeneratorSchema &us) @@ -293,18 +273,12 @@ class CustomGenerator : public internal::BuilderInterface { private: void Initialize(); - tf::Task CreateGroupTask(tf::FlowBuilder &builder, - const GroupMetadata &group_metadata); - tf::Task CreateTaskRunner(tf::Subflow &subflow, const std::string &id); void TaskRunner(const std::string &id); void GenerateTask(); void BuildGenerate(); - // void InvokeDependencyCb(std::unordered_map - // &®istered_tasks) const noexcept; - // Recheck states void IdRemoved(); void IdAdded(); @@ -321,7 +295,7 @@ class CustomGenerator : public internal::BuilderInterface { // Serialization UserCustomGeneratorSchema user_; - std::unordered_map grouped_ids_; + // TODO, Remove this as well std::unordered_set ungrouped_ids_; // Comparator @@ -333,9 +307,6 @@ class CustomGenerator : public internal::BuilderInterface { // Internal env::Command command_; - - // // Callbacks - // DependencyCb dependency_cb_; }; } // namespace buildcc diff --git a/buildcc/lib/target/include/target/file_generator.h b/buildcc/lib/target/include/target/file_generator.h index d49fddbe..9897b5fc 100644 --- a/buildcc/lib/target/include/target/file_generator.h +++ b/buildcc/lib/target/include/target/file_generator.h @@ -68,7 +68,6 @@ class FileGenerator : public CustomGenerator { // Restrict access to certain custom generator APIs private: - using CustomGenerator::AddGroupInfo; using CustomGenerator::AddIdInfo; using CustomGenerator::Build; diff --git a/buildcc/lib/target/include/target/template_generator.h b/buildcc/lib/target/include/target/template_generator.h index 12c89aec..73c892e7 100644 --- a/buildcc/lib/target/include/target/template_generator.h +++ b/buildcc/lib/target/include/target/template_generator.h @@ -42,7 +42,6 @@ class TemplateGenerator : public CustomGenerator { // Restrict access to certain custom generator APIs private: - using CustomGenerator::AddGroupInfo; using CustomGenerator::AddIdInfo; using CustomGenerator::Build; diff --git a/buildcc/lib/target/src/custom_generator/custom_generator.cpp b/buildcc/lib/target/src/custom_generator/custom_generator.cpp index 26c8fd32..9dd1ed90 100644 --- a/buildcc/lib/target/src/custom_generator/custom_generator.cpp +++ b/buildcc/lib/target/src/custom_generator/custom_generator.cpp @@ -76,25 +76,6 @@ void CustomGenerator::AddIdInfo( ungrouped_ids_.emplace(id); } -void CustomGenerator::AddGroupInfo(const std::string &group_id, - std::initializer_list ids) { - // Verify that the ids exist - // Remove those ids from ungrouped_ids - for (const auto &id : ids) { - env::assert_fatal(user_.ids.find(id) != user_.ids.end(), - fmt::format("Id '{}' is not found", id)); - ungrouped_ids_.erase(id); - } - - env::assert_fatal(grouped_ids_.find(group_id) == grouped_ids_.end(), - fmt::format("Group Id '{}' duplicate found", group_id)); - - // Group map is used to group similar ids in a single subflow - GroupMetadata group_metadata; - group_metadata.ids = ids; - grouped_ids_.try_emplace(group_id, std::move(group_metadata)); -} - void CustomGenerator::Build() { (void)serialization_.LoadFromFile(); @@ -148,11 +129,6 @@ void CustomGenerator::BuildGenerate() { (void)id; IdAdded(); } - - // For GROUPS - // Group Removed - // Group Added - // Group Updated } } @@ -168,13 +144,6 @@ void CustomGenerator::GenerateTask() { std::unordered_map registered_tasks; - // Grouped tasks - for (const auto &[group_id, group_metadata] : grouped_ids_) { - auto group_task = CreateGroupTask(subflow, group_metadata); - group_task.name(group_id); - registered_tasks.try_emplace(group_id, group_task); - } - // Ungrouped tasks for (const auto &id : ungrouped_ids_) { auto task = CreateTaskRunner(subflow, id); @@ -205,25 +174,6 @@ void CustomGenerator::GenerateTask() { generate_task.name(kGenerateTaskName); } -tf::Task CustomGenerator::CreateGroupTask(tf::FlowBuilder &builder, - const GroupMetadata &group_metadata) { - return builder.emplace([&](tf::Subflow &s) { - if (env::get_task_state() != env::TaskState::SUCCESS) { - return; - } - - std::unordered_map registered_tasks; - for (const auto &id : group_metadata.ids) { - auto task = CreateTaskRunner(s, id); - task.name(id); - registered_tasks.try_emplace(id, task); - } - - // NOTE, Do not call detach otherwise this will fail - s.join(); - }); -} - tf::Task CustomGenerator::CreateTaskRunner(tf::Subflow &subflow, const std::string &id) { return subflow.emplace([&, id]() { diff --git a/buildcc/lib/target/test/target/test_custom_generator.cpp b/buildcc/lib/target/test/target/test_custom_generator.cpp index 0812c1de..383077d8 100644 --- a/buildcc/lib/target/test/target/test_custom_generator.cpp +++ b/buildcc/lib/target/test/target/test_custom_generator.cpp @@ -84,37 +84,6 @@ TEST(CustomGeneratorTestGroup, Basic_Failure) { CHECK_EQUAL(internal_map.size(), 1); } -TEST(CustomGeneratorTestGroup, Basic_Group) { - buildcc::CustomGenerator cgen("basic_group", ""); - cgen.AddIdInfo("id1", {"{current_root_dir}/dummy_main.c"}, - {"{current_build_dir}/dummy_main.o"}, BasicGenerateCb); - cgen.AddIdInfo("id2", {"{current_root_dir}/dummy_main.cpp"}, {}, - BasicGenerateCb); - cgen.AddGroupInfo("grouped_id1_and_id2", {"id1", "id2"}); - cgen.Build(); - - mock().expectOneCall("BasicGenerateCb").andReturnValue(true); - mock().expectOneCall("BasicGenerateCb").andReturnValue(true); - buildcc::m::CustomGeneratorRunner(cgen); - - // Serialization check - { - buildcc::internal::CustomGeneratorSerialization serialization( - cgen.GetBinaryPath()); - CHECK_TRUE(serialization.LoadFromFile()); - - const auto &internal_map = serialization.GetLoad().internal_ids; - CHECK_EQUAL(internal_map.size(), 2); - const auto &id1_info = internal_map.at("id1"); - CHECK_EQUAL(id1_info.internal_inputs.size(), 1); - CHECK_EQUAL(id1_info.outputs.size(), 1); - - const auto &id2_info = internal_map.at("id2"); - CHECK_EQUAL(id2_info.internal_inputs.size(), 1); - CHECK_EQUAL(id2_info.outputs.size(), 0); - } -} - bool FailureCb(const buildcc::CustomGeneratorContext &ctx) { (void)ctx; return false; From 240881e36b80dfcb65da896d8f9dcee4673e2e75 Mon Sep 17 00:00:00 2001 From: Niket Naidu Date: Sun, 20 Nov 2022 21:41:19 -0800 Subject: [PATCH 08/19] Removed ungrouped ids from custom generator --- .../target/include/target/custom_generator.h | 44 +++---------------- .../src/custom_generator/custom_generator.cpp | 20 ++++----- 2 files changed, 16 insertions(+), 48 deletions(-) diff --git a/buildcc/lib/target/include/target/custom_generator.h b/buildcc/lib/target/include/target/custom_generator.h index 4c3a7e70..6544868b 100644 --- a/buildcc/lib/target/include/target/custom_generator.h +++ b/buildcc/lib/target/include/target/custom_generator.h @@ -214,44 +214,20 @@ class CustomGenerator : public internal::BuilderInterface { } } - void CompareGroups() { - const auto &prev_groups = schema.internal_groups; - const auto &curr_groups = user_schema.internal_groups; - - for (const auto &[prev_group_id, _] : prev_groups) { - if (curr_groups.find(prev_group_id) == curr_groups.end()) { - // Group Removed condition - group_state_info.at(State::kRemoved).insert(prev_group_id); - } - } - - for (const auto &[curr_group_id, curr_group_info] : curr_groups) { - if (prev_groups.find(curr_group_id) == prev_groups.end()) { - // Group Added condition - group_state_info.at(State::kAdded).insert(curr_group_id); - } else { - // Group Check Later condition - // NOTE, We cannot perform the check now since the input/metadata - // might be added during runtime (when task executes) - group_state_info.at(State::kCheckLater).insert(curr_group_id); - } - } - } - - const std::unordered_set &RemovedIds() const { + const std::unordered_set &GetRemovedIds() const { return id_state_info.at(State::kRemoved); } - const std::unordered_set &AddedIds() const { + const std::unordered_set &GetAddedIds() const { return id_state_info.at(State::kAdded); } - bool AddedId(const std::string &id) const { - return id_state_info.at(State::kAdded).count(id) == 1; + const std::unordered_set &GetCheckLaterIds() const { + return id_state_info.at(State::kCheckLater); } - bool AddedGroupId(const std::string &group_id) const { - return group_state_info.at(State::kAdded).count(group_id) == 1; + bool IsIdAdded(const std::string &id) const { + return id_state_info.at(State::kAdded).count(id) == 1; } private: @@ -262,12 +238,6 @@ class CustomGenerator : public internal::BuilderInterface { {State::kAdded, std::unordered_set()}, {State::kCheckLater, std::unordered_set()}, }; - - std::unordered_map> group_state_info{ - {State::kRemoved, std::unordered_set()}, - {State::kAdded, std::unordered_set()}, - {State::kCheckLater, std::unordered_set()}, - }; }; private: @@ -295,8 +265,6 @@ class CustomGenerator : public internal::BuilderInterface { // Serialization UserCustomGeneratorSchema user_; - // TODO, Remove this as well - std::unordered_set ungrouped_ids_; // Comparator Comparator comparator_; diff --git a/buildcc/lib/target/src/custom_generator/custom_generator.cpp b/buildcc/lib/target/src/custom_generator/custom_generator.cpp index 9dd1ed90..b612e51a 100644 --- a/buildcc/lib/target/src/custom_generator/custom_generator.cpp +++ b/buildcc/lib/target/src/custom_generator/custom_generator.cpp @@ -73,7 +73,6 @@ void CustomGenerator::AddIdInfo( schema.blob_handler = std::move(blob_handler); user_.ids.try_emplace(id, std::move(schema)); - ungrouped_ids_.emplace(id); } void CustomGenerator::Build() { @@ -117,15 +116,15 @@ void CustomGenerator::BuildGenerate() { // For IDS comparator_.CompareIds(); - const bool is_removed = !comparator_.RemovedIds().empty(); - const bool is_added = !comparator_.AddedIds().empty(); + const bool is_removed = !comparator_.GetRemovedIds().empty(); + const bool is_added = !comparator_.GetAddedIds().empty(); dirty_ = is_removed || is_added; if (is_removed) { IdRemoved(); } - for (const auto &id : comparator_.AddedIds()) { + for (const auto &id : comparator_.GetAddedIds()) { (void)id; IdAdded(); } @@ -142,13 +141,15 @@ void CustomGenerator::GenerateTask() { // Selected ids for build BuildGenerate(); - std::unordered_map registered_tasks; + // Create runner for each added/updated id + for (const auto &id : comparator_.GetAddedIds()) { + auto task = CreateTaskRunner(subflow, id); + task.name(id); + } - // Ungrouped tasks - for (const auto &id : ungrouped_ids_) { + for (const auto &id : comparator_.GetCheckLaterIds()) { auto task = CreateTaskRunner(subflow, id); task.name(id); - registered_tasks.try_emplace(id, task); } // NOTE, Do not call detach otherwise this will fail @@ -165,7 +166,6 @@ void CustomGenerator::GenerateTask() { env::assert_fatal(serialization_.StoreToFile(), fmt::format("Store failed for {}", name_)); } - } catch (...) { env::set_task_state(env::TaskState::FAILURE); } @@ -202,7 +202,7 @@ void CustomGenerator::TaskRunner(const std::string &id) { // Run const auto ¤t_id_info = user_.ids.at(id); - bool run = comparator_.AddedId(id); + bool run = comparator_.IsIdAdded(id); if (!run) { const auto &previous_info = serialization_.GetLoad().internal_ids.at(id); run = From 19ce132f606ebb883b7c932f4c186b7eb7b5855a Mon Sep 17 00:00:00 2001 From: Niket Naidu Date: Sun, 20 Nov 2022 23:07:20 -0800 Subject: [PATCH 09/19] Updated comparator variable names --- .../target/include/target/custom_generator.h | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/buildcc/lib/target/include/target/custom_generator.h b/buildcc/lib/target/include/target/custom_generator.h index 6544868b..e3475993 100644 --- a/buildcc/lib/target/include/target/custom_generator.h +++ b/buildcc/lib/target/include/target/custom_generator.h @@ -175,9 +175,9 @@ class CustomGenerator : public internal::BuilderInterface { private: struct Comparator { - Comparator(const internal::CustomGeneratorSchema &s, + Comparator(const internal::CustomGeneratorSchema &loaded, const UserCustomGeneratorSchema &us) - : schema(s), user_schema(us) {} + : loaded_schema_(loaded), current_schema_(us) {} enum class State { kRemoved, @@ -186,54 +186,54 @@ class CustomGenerator : public internal::BuilderInterface { }; void AddAllIds() { - const auto &curr_ids = user_schema.ids; + const auto &curr_ids = current_schema_.ids; for (const auto &[id, _] : curr_ids) { - id_state_info.at(State::kAdded).insert(id); + id_state_info_.at(State::kAdded).insert(id); } } void CompareIds() { - const auto &prev_ids = schema.internal_ids; - const auto &curr_ids = user_schema.ids; + const auto &prev_ids = loaded_schema_.internal_ids; + const auto &curr_ids = current_schema_.ids; for (const auto &[prev_id, _] : prev_ids) { if (curr_ids.find(prev_id) == curr_ids.end()) { // Id Removed condition, previous id is not present in the current run - id_state_info.at(State::kRemoved).insert(prev_id); + id_state_info_.at(State::kRemoved).insert(prev_id); } } for (const auto &[curr_id, _] : curr_ids) { if (prev_ids.find(curr_id) == prev_ids.end()) { // Id Added condition - id_state_info.at(State::kAdded).insert(curr_id); + id_state_info_.at(State::kAdded).insert(curr_id); } else { // Id Check Later condition - id_state_info.at(State::kCheckLater).insert(curr_id); + id_state_info_.at(State::kCheckLater).insert(curr_id); } } } const std::unordered_set &GetRemovedIds() const { - return id_state_info.at(State::kRemoved); + return id_state_info_.at(State::kRemoved); } const std::unordered_set &GetAddedIds() const { - return id_state_info.at(State::kAdded); + return id_state_info_.at(State::kAdded); } const std::unordered_set &GetCheckLaterIds() const { - return id_state_info.at(State::kCheckLater); + return id_state_info_.at(State::kCheckLater); } bool IsIdAdded(const std::string &id) const { - return id_state_info.at(State::kAdded).count(id) == 1; + return id_state_info_.at(State::kAdded).count(id) == 1; } private: - const internal::CustomGeneratorSchema &schema; - const UserCustomGeneratorSchema &user_schema; - std::unordered_map> id_state_info{ + const internal::CustomGeneratorSchema &loaded_schema_; + const UserCustomGeneratorSchema ¤t_schema_; + std::unordered_map> id_state_info_{ {State::kRemoved, std::unordered_set()}, {State::kAdded, std::unordered_set()}, {State::kCheckLater, std::unordered_set()}, From 950d840a54bb1e4a8237c679dda4fabf4810c4fb Mon Sep 17 00:00:00 2001 From: Niket Naidu Date: Sun, 20 Nov 2022 23:37:27 -0800 Subject: [PATCH 10/19] Added IsChanged function to Comparator --- .../target/include/target/custom_generator.h | 17 +++++++++++++ .../src/custom_generator/custom_generator.cpp | 24 +++++-------------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/buildcc/lib/target/include/target/custom_generator.h b/buildcc/lib/target/include/target/custom_generator.h index e3475993..9961f3dd 100644 --- a/buildcc/lib/target/include/target/custom_generator.h +++ b/buildcc/lib/target/include/target/custom_generator.h @@ -214,6 +214,23 @@ class CustomGenerator : public internal::BuilderInterface { } } + bool IsChanged(const std::string &id) const { + const auto &previous_id_info = loaded_schema_.internal_ids.at(id); + const auto ¤t_id_info = current_schema_.ids.at(id); + + bool changed = internal::CheckPaths(previous_id_info.internal_inputs, + current_id_info.internal_inputs) != + internal::PathState::kNoChange; + changed = changed || internal::CheckChanged(previous_id_info.outputs, + current_id_info.outputs); + if (!changed && current_id_info.blob_handler != nullptr) { + changed = + changed || current_id_info.blob_handler->CheckChanged( + previous_id_info.userblob, current_id_info.userblob); + } + return changed; + } + const std::unordered_set &GetRemovedIds() const { return id_state_info_.at(State::kRemoved); } diff --git a/buildcc/lib/target/src/custom_generator/custom_generator.cpp b/buildcc/lib/target/src/custom_generator/custom_generator.cpp index b612e51a..3a3f289f 100644 --- a/buildcc/lib/target/src/custom_generator/custom_generator.cpp +++ b/buildcc/lib/target/src/custom_generator/custom_generator.cpp @@ -200,27 +200,15 @@ void CustomGenerator::TaskRunner(const std::string &id) { : std::vector(); } - // Run - const auto ¤t_id_info = user_.ids.at(id); - bool run = comparator_.IsIdAdded(id); - if (!run) { - const auto &previous_info = serialization_.GetLoad().internal_ids.at(id); - run = - internal::CheckPaths(previous_info.internal_inputs, - current_id_info.internal_inputs) != - internal::PathState::kNoChange || - internal::CheckChanged(previous_info.outputs, current_id_info.outputs); - if (!run && current_id_info.blob_handler != nullptr) { - run = current_id_info.blob_handler->CheckChanged( - previous_info.userblob, current_id_info.userblob); - } - } + // Compute runnable + bool run = comparator_.IsIdAdded(id) ? true : comparator_.IsChanged(id); + const auto ¤t_id_info = user_.ids.at(id); if (run) { dirty_ = true; - buildcc::CustomGeneratorContext ctx(command_, current_id_info.inputs, - current_id_info.outputs, - current_id_info.userblob); + CustomGeneratorContext ctx(command_, current_id_info.inputs, + current_id_info.outputs, + current_id_info.userblob); bool success = current_id_info.generate_cb(ctx); env::assert_fatal(success, fmt::format("Generate Cb failed for id {}", id)); } From fa205292987926217f26d1fb764c44b2727a77a5 Mon Sep 17 00:00:00 2001 From: Niket Naidu Date: Sun, 20 Nov 2022 23:52:43 -0800 Subject: [PATCH 11/19] Added a ConvertToInternal function for UserCustomGeneratorSchema --- .../lib/target/include/target/custom_generator.h | 9 +++++++-- .../src/custom_generator/custom_generator.cpp | 13 +++---------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/buildcc/lib/target/include/target/custom_generator.h b/buildcc/lib/target/include/target/custom_generator.h index 9961f3dd..1dea3836 100644 --- a/buildcc/lib/target/include/target/custom_generator.h +++ b/buildcc/lib/target/include/target/custom_generator.h @@ -51,8 +51,6 @@ class CustomGeneratorContext { // clang-format off using GenerateCb = std::function; - -using DependencyCb = std::function &&)>; // clang-format on class CustomBlobHandler { @@ -91,6 +89,13 @@ struct UserCustomGeneratorSchema : public internal::CustomGeneratorSchema { fs_unordered_set inputs; // TODO, Remove GenerateCb generate_cb; std::shared_ptr blob_handler{nullptr}; + + void ConvertToInternal() { + internal_inputs = internal::path_schema_convert( + inputs, internal::Path::CreateExistingPath); + userblob = blob_handler != nullptr ? blob_handler->GetSerializedData() + : std::vector(); + } }; using UserIdPair = std::pair; diff --git a/buildcc/lib/target/src/custom_generator/custom_generator.cpp b/buildcc/lib/target/src/custom_generator/custom_generator.cpp index 3a3f289f..02c851d0 100644 --- a/buildcc/lib/target/src/custom_generator/custom_generator.cpp +++ b/buildcc/lib/target/src/custom_generator/custom_generator.cpp @@ -189,20 +189,13 @@ tf::Task CustomGenerator::CreateTaskRunner(tf::Subflow &subflow, } void CustomGenerator::TaskRunner(const std::string &id) { - // Convert - { - auto ¤t_id_info = user_.ids.at(id); - current_id_info.internal_inputs = internal::path_schema_convert( - current_id_info.inputs, internal::Path::CreateExistingPath); - current_id_info.userblob = - current_id_info.blob_handler != nullptr - ? current_id_info.blob_handler->GetSerializedData() - : std::vector(); - } + // Convert to internal + user_.ids.at(id).ConvertToInternal(); // Compute runnable bool run = comparator_.IsIdAdded(id) ? true : comparator_.IsChanged(id); + // Invoke generator callback const auto ¤t_id_info = user_.ids.at(id); if (run) { dirty_ = true; From 6d03060684b6ea830d1b163cd6e7865564600269 Mon Sep 17 00:00:00 2001 From: Niket Naidu Date: Wed, 23 Nov 2022 04:24:45 -0800 Subject: [PATCH 12/19] Removed groups from custom_generator_schema --- buildcc/schema/include/schema/custom_generator_schema.h | 7 ------- 1 file changed, 7 deletions(-) diff --git a/buildcc/schema/include/schema/custom_generator_schema.h b/buildcc/schema/include/schema/custom_generator_schema.h index 0a171f2c..d7027b98 100644 --- a/buildcc/schema/include/schema/custom_generator_schema.h +++ b/buildcc/schema/include/schema/custom_generator_schema.h @@ -27,11 +27,9 @@ struct CustomGeneratorSchema { private: static constexpr const char *const kName = "name"; static constexpr const char *const kIds = "ids"; - static constexpr const char *const kGroups = "groups"; public: using IdKey = std::string; - using GroupKey = std::string; struct IdInfo { private: static constexpr const char *const kInputs = "inputs"; @@ -57,23 +55,18 @@ struct CustomGeneratorSchema { }; using IdPair = std::pair; - using GroupInfo = std::unordered_set; - using GroupPair = std::pair; std::string name; std::unordered_map internal_ids; - std::unordered_map internal_groups; friend void to_json(json &j, const CustomGeneratorSchema &schema) { j[kName] = schema.name; j[kIds] = schema.internal_ids; - j[kGroups] = schema.internal_groups; } friend void from_json(const json &j, CustomGeneratorSchema &schema) { j.at(kName).get_to(schema.name); j.at(kIds).get_to(schema.internal_ids); - j.at(kGroups).get_to(schema.internal_groups); } }; From 2bfff729043ce0702e0b6b3300ba06e08e9a09f0 Mon Sep 17 00:00:00 2001 From: Niket Naidu Date: Wed, 23 Nov 2022 21:16:16 -0800 Subject: [PATCH 13/19] Shifted classes to seperate include files --- .../lib/target/cmake/common_target_src.cmake | 3 + .../target/include/target/custom_generator.h | 53 +---------------- .../custom_generator/custom_blob_handler.h | 59 +++++++++++++++++++ .../custom_generator_context.h | 45 ++++++++++++++ 4 files changed, 110 insertions(+), 50 deletions(-) create mode 100644 buildcc/lib/target/include/target/custom_generator/custom_blob_handler.h create mode 100644 buildcc/lib/target/include/target/custom_generator/custom_generator_context.h diff --git a/buildcc/lib/target/cmake/common_target_src.cmake b/buildcc/lib/target/cmake/common_target_src.cmake index 5a10bfe8..92822197 100644 --- a/buildcc/lib/target/cmake/common_target_src.cmake +++ b/buildcc/lib/target/cmake/common_target_src.cmake @@ -27,6 +27,9 @@ set(COMMON_TARGET_SRCS include/target/api/target_getter.h # Generator + include/target/custom_generator/custom_generator_context.h + include/target/custom_generator/custom_blob_handler.h + src/custom_generator/custom_generator.cpp include/target/custom_generator.h src/generator/file_generator.cpp diff --git a/buildcc/lib/target/include/target/custom_generator.h b/buildcc/lib/target/include/target/custom_generator.h index 1dea3836..786cb7db 100644 --- a/buildcc/lib/target/include/target/custom_generator.h +++ b/buildcc/lib/target/include/target/custom_generator.h @@ -30,60 +30,13 @@ #include "schema/custom_generator_serialization.h" #include "schema/path.h" +#include "custom_generator/custom_blob_handler.h" +#include "custom_generator/custom_generator_context.h" + #include "target/common/target_env.h" namespace buildcc { -// TODO, Shift to a different file -// TODO, Check if we need the "id" here as well -class CustomGeneratorContext { -public: - CustomGeneratorContext(const env::Command &c, const fs_unordered_set &i, - const fs_unordered_set &o, - const std::vector &ub) - : command(c), inputs(i), outputs(o), userblob(ub) {} - - const env::Command &command; - const fs_unordered_set &inputs; - const fs_unordered_set &outputs; - const std::vector &userblob; -}; - -// clang-format off -using GenerateCb = std::function; -// clang-format on - -class CustomBlobHandler { -public: - CustomBlobHandler() = default; - virtual ~CustomBlobHandler() = default; - - bool CheckChanged(const std::vector &previous, - const std::vector ¤t) const { - env::assert_fatal( - Verify(previous), - "Stored blob is corrupted or User verification is incorrect"); - env::assert_fatal( - Verify(current), - "Current blob is corrupted or User verification is incorrect"); - return !IsEqual(previous, current); - }; - - std::vector GetSerializedData() const { - auto serialized_data = Serialize(); - env::assert_fatal( - Verify(serialized_data), - "Serialized data is corrupted or Serialize function is incorrect"); - return serialized_data; - } - -private: - virtual bool Verify(const std::vector &serialized_data) const = 0; - virtual bool IsEqual(const std::vector &previous, - const std::vector ¤t) const = 0; - virtual std::vector Serialize() const = 0; -}; - struct UserCustomGeneratorSchema : public internal::CustomGeneratorSchema { struct UserIdInfo : internal::CustomGeneratorSchema::IdInfo { fs_unordered_set inputs; // TODO, Remove diff --git a/buildcc/lib/target/include/target/custom_generator/custom_blob_handler.h b/buildcc/lib/target/include/target/custom_generator/custom_blob_handler.h new file mode 100644 index 00000000..1c268301 --- /dev/null +++ b/buildcc/lib/target/include/target/custom_generator/custom_blob_handler.h @@ -0,0 +1,59 @@ +/* + * Copyright 2021-2022 Niket Naidu. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef TARGET_CUSTOM_GENERATOR_CUSTOM_BLOB_HANDLER_H_ +#define TARGET_CUSTOM_GENERATOR_CUSTOM_BLOB_HANDLER_H_ + +#include + +#include "env/assert_fatal.h" + +namespace buildcc { + +class CustomBlobHandler { +public: + CustomBlobHandler() = default; + virtual ~CustomBlobHandler() = default; + + bool CheckChanged(const std::vector &previous, + const std::vector ¤t) const { + env::assert_fatal( + Verify(previous), + "Stored blob is corrupted or User verification is incorrect"); + env::assert_fatal( + Verify(current), + "Current blob is corrupted or User verification is incorrect"); + return !IsEqual(previous, current); + }; + + std::vector GetSerializedData() const { + auto serialized_data = Serialize(); + env::assert_fatal( + Verify(serialized_data), + "Serialized data is corrupted or Serialize function is incorrect"); + return serialized_data; + } + +private: + virtual bool Verify(const std::vector &serialized_data) const = 0; + virtual bool IsEqual(const std::vector &previous, + const std::vector ¤t) const = 0; + virtual std::vector Serialize() const = 0; +}; + +} // namespace buildcc + +#endif diff --git a/buildcc/lib/target/include/target/custom_generator/custom_generator_context.h b/buildcc/lib/target/include/target/custom_generator/custom_generator_context.h new file mode 100644 index 00000000..207015d9 --- /dev/null +++ b/buildcc/lib/target/include/target/custom_generator/custom_generator_context.h @@ -0,0 +1,45 @@ +/* + * Copyright 2021-2022 Niket Naidu. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef TARGET_CUSTOM_GENERATOR_CUSTOM_GENERATOR_CONTEXT_H_ +#define TARGET_CUSTOM_GENERATOR_CUSTOM_GENERATOR_CONTEXT_H_ + +#include "schema/path.h" + +#include "env/command.h" + +namespace buildcc { + +class CustomGeneratorContext { +public: + CustomGeneratorContext(const env::Command &c, const fs_unordered_set &i, + const fs_unordered_set &o, + const std::vector &ub) + : command(c), inputs(i), outputs(o), userblob(ub) {} + + const env::Command &command; + const fs_unordered_set &inputs; + const fs_unordered_set &outputs; + const std::vector &userblob; +}; + +// clang-format off +using GenerateCb = std::function; +// clang-format on + +} // namespace buildcc + +#endif From 0156aecec3766ead1781e840363092e98c6457b6 Mon Sep 17 00:00:00 2001 From: Niket Naidu Date: Wed, 23 Nov 2022 21:16:23 -0800 Subject: [PATCH 14/19] Update custom_generator.cpp --- .../lib/target/src/custom_generator/custom_generator.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/buildcc/lib/target/src/custom_generator/custom_generator.cpp b/buildcc/lib/target/src/custom_generator/custom_generator.cpp index 02c851d0..a7deb78f 100644 --- a/buildcc/lib/target/src/custom_generator/custom_generator.cpp +++ b/buildcc/lib/target/src/custom_generator/custom_generator.cpp @@ -102,12 +102,6 @@ void CustomGenerator::Initialize() { tf_.name(name_); } -/** - * @brief Check which ids need to be rebuilt - * - * @param gen_selected_ids - * @param dummy_gen_selected_ids - */ void CustomGenerator::BuildGenerate() { if (!serialization_.IsLoaded()) { comparator_.AddAllIds(); From a0316aaf5a584facbc44cae4e9318e22ecf7ac8d Mon Sep 17 00:00:00 2001 From: Niket Naidu Date: Wed, 23 Nov 2022 21:37:49 -0800 Subject: [PATCH 15/19] Pass blob_handler shared_ptr by ref --- buildcc/lib/target/include/target/custom_generator.h | 11 ++++++----- .../target/src/custom_generator/custom_generator.cpp | 5 ++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/buildcc/lib/target/include/target/custom_generator.h b/buildcc/lib/target/include/target/custom_generator.h index 786cb7db..47006db0 100644 --- a/buildcc/lib/target/include/target/custom_generator.h +++ b/buildcc/lib/target/include/target/custom_generator.h @@ -97,11 +97,12 @@ class CustomGenerator : public internal::BuilderInterface { * @param generate_cb User-defined generate callback to build outputs from the * provided inputs */ - void AddIdInfo(const std::string &id, - const std::unordered_set &inputs, - const std::unordered_set &outputs, - const GenerateCb &generate_cb, - std::shared_ptr blob_handler = nullptr); + void + AddIdInfo(const std::string &id, + const std::unordered_set &inputs, + const std::unordered_set &outputs, + const GenerateCb &generate_cb, + const std::shared_ptr &blob_handler = nullptr); // Callbacks /** diff --git a/buildcc/lib/target/src/custom_generator/custom_generator.cpp b/buildcc/lib/target/src/custom_generator/custom_generator.cpp index a7deb78f..b60283e8 100644 --- a/buildcc/lib/target/src/custom_generator/custom_generator.cpp +++ b/buildcc/lib/target/src/custom_generator/custom_generator.cpp @@ -55,7 +55,7 @@ void CustomGenerator::AddIdInfo( const std::string &id, const std::unordered_set &inputs, const std::unordered_set &outputs, const GenerateCb &generate_cb, - std::shared_ptr blob_handler) { + const std::shared_ptr &blob_handler) { env::assert_fatal(user_.ids.find(id) == user_.ids.end(), fmt::format("Duplicate id {} detected", id)); ASSERT_FATAL(generate_cb, "Invalid callback provided"); @@ -70,8 +70,7 @@ void CustomGenerator::AddIdInfo( schema.outputs.emplace(std::move(output)); } schema.generate_cb = generate_cb; - schema.blob_handler = std::move(blob_handler); - + schema.blob_handler = blob_handler; user_.ids.try_emplace(id, std::move(schema)); } From fdc784959073c6b21a221296b1255efc7abace73 Mon Sep 17 00:00:00 2001 From: Niket Naidu Date: Wed, 23 Nov 2022 21:57:32 -0800 Subject: [PATCH 16/19] Update custom_generator.h --- .../target/include/target/custom_generator.h | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/buildcc/lib/target/include/target/custom_generator.h b/buildcc/lib/target/include/target/custom_generator.h index 47006db0..eef212ba 100644 --- a/buildcc/lib/target/include/target/custom_generator.h +++ b/buildcc/lib/target/include/target/custom_generator.h @@ -104,23 +104,6 @@ class CustomGenerator : public internal::BuilderInterface { const GenerateCb &generate_cb, const std::shared_ptr &blob_handler = nullptr); - // Callbacks - /** - * @brief Setup dependencies between Tasks using their `id` - * For example: `task_map["id1"].precede(task_map["id2"])` - * - * IMPORTANT: Successor tasks will not automatically run if dependent task is - * run. - * The Dependency callback only sets precedence (order in which your tasks - * should run) - * Default behaviour when dependency callback is not supplied: All task `id`s - * run in parallel. - * - * @param dependency_cb Unordered map of `id` and `task` - * The map can be safely mutated. - */ - // void AddDependencyCb(const DependencyCb &dependency_cb); - void Build() override; // Getters From 6a0e4937adc54ab30cd92051f01f3f1ba340624c Mon Sep 17 00:00:00 2001 From: Niket Naidu Date: Wed, 23 Nov 2022 22:25:01 -0800 Subject: [PATCH 17/19] Update cppcheck.cmake --- cmake/tool/cppcheck.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/tool/cppcheck.cmake b/cmake/tool/cppcheck.cmake index 20d98554..9f11dbe6 100644 --- a/cmake/tool/cppcheck.cmake +++ b/cmake/tool/cppcheck.cmake @@ -13,7 +13,7 @@ if(${BUILDCC_CPPCHECK}) set(CPPCHECK_ADDITIONAL_OPTIONS --std=c++17 -q - --error-exitcode=1 + # --error-exitcode=1 --cppcheck-build-dir=${CMAKE_CURRENT_BINARY_DIR}/cppcheck_output ) set(CPPCHECK_CHECK_DIR From a4a52b2fca29ee6c060d5b675e3ea168bacaf4c9 Mon Sep 17 00:00:00 2001 From: Niket Naidu Date: Thu, 24 Nov 2022 06:01:17 -0800 Subject: [PATCH 18/19] Updated tests for custom_generator --- .../test/target/test_custom_generator.cpp | 95 ++++++++++++++++--- 1 file changed, 80 insertions(+), 15 deletions(-) diff --git a/buildcc/lib/target/test/target/test_custom_generator.cpp b/buildcc/lib/target/test/target/test_custom_generator.cpp index 383077d8..4d1fe169 100644 --- a/buildcc/lib/target/test/target/test_custom_generator.cpp +++ b/buildcc/lib/target/test/target/test_custom_generator.cpp @@ -24,7 +24,8 @@ TEST_GROUP(CustomGeneratorTestGroup) }; // clang-format on -fs::path BUILD_DIR = fs::current_path() / "intermediate" / "custom_generator"; +const fs::path BUILD_DIR = + fs::current_path() / "intermediate" / "custom_generator"; static bool BasicGenerateCb(const buildcc::CustomGeneratorContext &ctx) { (void)ctx; @@ -61,6 +62,77 @@ TEST(CustomGeneratorTestGroup, Basic) { } } +TEST(CustomGeneratorTestGroup, BasicRebuild) { + constexpr const char *const kName = "basic_rebuild"; + + { + buildcc::CustomGenerator cgen(kName, ""); + cgen.AddIdInfo("id1", {"{current_root_dir}/dummy_main.c"}, + {"{current_build_dir}/dummy_main.o"}, BasicGenerateCb); + cgen.AddIdInfo("id2", {"{current_root_dir}/dummy_main.cpp"}, {}, + BasicGenerateCb); + cgen.Build(); + + mock().expectOneCall("BasicGenerateCb").andReturnValue(true); + mock().expectOneCall("BasicGenerateCb").andReturnValue(true); + buildcc::m::CustomGeneratorRunner(cgen); + } + + { + buildcc::CustomGenerator cgen(kName, ""); + cgen.AddIdInfo("id1", {"{current_root_dir}/dummy_main.c"}, + {"{current_build_dir}/dummy_main.o"}, BasicGenerateCb); + cgen.AddIdInfo("id2", {"{current_root_dir}/dummy_main.cpp"}, {}, + BasicGenerateCb); + cgen.Build(); + + buildcc::m::CustomGeneratorRunner(cgen); + } +} + +TEST(CustomGeneratorTestGroup, BasicRebuild_Add_Remove) { + constexpr const char *const kName = "basic_rebuild_add_remove"; + + { + buildcc::CustomGenerator cgen(kName, ""); + cgen.AddIdInfo("id1", {"{current_root_dir}/dummy_main.c"}, + {"{current_build_dir}/dummy_main.o"}, BasicGenerateCb); + cgen.AddIdInfo("id2", {"{current_root_dir}/dummy_main.cpp"}, {}, + BasicGenerateCb); + cgen.Build(); + + mock().expectOneCall("BasicGenerateCb").andReturnValue(true); + mock().expectOneCall("BasicGenerateCb").andReturnValue(true); + buildcc::m::CustomGeneratorRunner(cgen); + } + + // Remove + { + buildcc::CustomGenerator cgen(kName, ""); + cgen.AddIdInfo("id1", {"{current_root_dir}/dummy_main.c"}, + {"{current_build_dir}/dummy_main.o"}, BasicGenerateCb); + // ID2 Removed + cgen.Build(); + + buildcc::m::CustomGeneratorExpect_IdRemoved(1, &cgen); + buildcc::m::CustomGeneratorRunner(cgen); + } + + // Add + { + buildcc::CustomGenerator cgen(kName, ""); + cgen.AddIdInfo("id1", {"{current_root_dir}/dummy_main.c"}, + {"{current_build_dir}/dummy_main.o"}, BasicGenerateCb); + cgen.AddIdInfo("id2", {"{current_root_dir}/dummy_main.cpp"}, {}, + BasicGenerateCb); + cgen.Build(); + + buildcc::m::CustomGeneratorExpect_IdAdded(1, &cgen); + mock().expectOneCall("BasicGenerateCb").andReturnValue(true); + buildcc::m::CustomGeneratorRunner(cgen); + } +} + TEST(CustomGeneratorTestGroup, Basic_Failure) { buildcc::CustomGenerator cgen("basic_failure", ""); cgen.AddIdInfo("id1", {"{current_root_dir}/dummy_main.c"}, {}, @@ -84,22 +156,15 @@ TEST(CustomGeneratorTestGroup, Basic_Failure) { CHECK_EQUAL(internal_map.size(), 1); } -bool FailureCb(const buildcc::CustomGeneratorContext &ctx) { - (void)ctx; - return false; -} -bool SuccessCb(const buildcc::CustomGeneratorContext &ctx) { - (void)ctx; - return true; -} - TEST(CustomGeneratorTestGroup, DefaultArgumentUsage) { buildcc::CustomGenerator cgen("default_argument_usage", ""); - cgen.AddPatterns({ - {"dummy_main_c", "{current_root_dir}/dummy_main.c"}, - {"dummy_main_o", "{current_build_dir}/dummy_main.o"}, - {"dummy_main_cpp", "{current_root_dir}/dummy_main.cpp"}, - }); + cgen.AddPatterns({{"dummy_main_c", "{current_root_dir}/dummy_main.c"}, + {"dummy_main_o", "{current_build_dir}/dummy_main.o"}, + {"dummy_main_cpp", "{current_root_dir}/dummy_main.cpp"}, + {"hello", "world"}}); + STRCMP_EQUAL(cgen.ParsePattern("{hello}").c_str(), "world"); + STRCMP_EQUAL(cgen.Get("hello").c_str(), "world"); + cgen.AddIdInfo("id1", {"{dummy_main_c}"}, {"{dummy_main_o}"}, BasicGenerateCb); cgen.AddIdInfo("id2", {"{dummy_main_cpp}"}, {}, BasicGenerateCb); From 03061bdaf53c413f2ce399564255837b133e7b13 Mon Sep 17 00:00:00 2001 From: Niket Naidu Date: Thu, 24 Nov 2022 06:15:10 -0800 Subject: [PATCH 19/19] Updated custom_generator blob_handler implementation --- buildcc/lib/target/include/target/custom_generator.h | 8 +++++--- buildcc/lib/target/test/target/test_custom_generator.cpp | 1 + 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/buildcc/lib/target/include/target/custom_generator.h b/buildcc/lib/target/include/target/custom_generator.h index eef212ba..0712cde5 100644 --- a/buildcc/lib/target/include/target/custom_generator.h +++ b/buildcc/lib/target/include/target/custom_generator.h @@ -166,9 +166,11 @@ class CustomGenerator : public internal::BuilderInterface { changed = changed || internal::CheckChanged(previous_id_info.outputs, current_id_info.outputs); if (!changed && current_id_info.blob_handler != nullptr) { - changed = - changed || current_id_info.blob_handler->CheckChanged( - previous_id_info.userblob, current_id_info.userblob); + // We only check blob handler if not changed by inputs/outputs + // Checking blob_handler could be expensive so this optimization is made + // to run only when changed == false + changed = current_id_info.blob_handler->CheckChanged( + previous_id_info.userblob, current_id_info.userblob); } return changed; } diff --git a/buildcc/lib/target/test/target/test_custom_generator.cpp b/buildcc/lib/target/test/target/test_custom_generator.cpp index 4d1fe169..839c91f7 100644 --- a/buildcc/lib/target/test/target/test_custom_generator.cpp +++ b/buildcc/lib/target/test/target/test_custom_generator.cpp @@ -34,6 +34,7 @@ static bool BasicGenerateCb(const buildcc::CustomGeneratorContext &ctx) { TEST(CustomGeneratorTestGroup, Basic) { buildcc::CustomGenerator cgen("basic", ""); + STRCMP_EQUAL(cgen.GetName().c_str(), "basic"); cgen.AddIdInfo("id1", {"{current_root_dir}/dummy_main.c"}, {"{current_build_dir}/dummy_main.o"}, BasicGenerateCb); cgen.AddIdInfo("id2", {"{current_root_dir}/dummy_main.cpp"}, {},