From 98ff1d32f92073cfafbf0daffec9f1a98f92c1eb Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Fri, 20 Jan 2023 10:44:21 +0100 Subject: [PATCH] TaskTree: Remove GroupConfig / GroupAction Use TaskAction for dynamic group setup. Get rid of possibility of selecting group's children to be started inside group setup. Use dynamic children's setup instead, so that each individual child may decide itself whether it wants to be started or not. Change-Id: Ie31f707791db0dc26be92663ca6238b8e49c3b98 Reviewed-by: hjk Reviewed-by: Qt CI Bot --- src/libs/utils/tasktree.cpp | 57 +++++++--------------- src/libs/utils/tasktree.h | 19 +------- src/plugins/clangtools/clangtoolrunner.cpp | 11 ++--- src/plugins/git/gitclient.cpp | 4 +- tests/auto/utils/tasktree/tst_tasktree.cpp | 45 ++++++----------- 5 files changed, 42 insertions(+), 94 deletions(-) diff --git a/src/libs/utils/tasktree.cpp b/src/libs/utils/tasktree.cpp index 02d3bfa9b53..477785acc89 100644 --- a/src/libs/utils/tasktree.cpp +++ b/src/libs/utils/tasktree.cpp @@ -6,6 +6,8 @@ #include "guard.h" #include "qtcassert.h" +#include + namespace Utils { namespace Tasking { @@ -151,7 +153,6 @@ public: ~TaskContainer(); TaskAction start(); TaskAction startChildren(); - int selectChildren(); // returns the skipped child count void stop(); bool isRunning() const; int taskCount() const; @@ -170,14 +171,12 @@ public: TaskTreePrivate *m_taskTreePrivate = nullptr; TaskContainer *m_parentContainer = nullptr; const int m_parallelLimit = 1; - WorkflowPolicy m_workflowPolicy = WorkflowPolicy::StopOnError; + const WorkflowPolicy m_workflowPolicy = WorkflowPolicy::StopOnError; const TaskItem::GroupHandler m_groupHandler; QList m_storageList; QList m_storageIdList; int m_taskCount = 0; - GroupConfig m_groupConfig; QList m_children; - QList m_selectedChildren; int m_doneCount = -1; bool m_successBit = true; Guard m_startGuard; @@ -351,32 +350,27 @@ TaskContainer::~TaskContainer() TaskAction TaskContainer::start() { m_doneCount = 0; - m_groupConfig = {}; - m_selectedChildren.clear(); createStorages(); + TaskAction groupAction = TaskAction::Continue; if (m_groupHandler.m_setupHandler || m_groupHandler.m_dynamicSetupHandler) { StorageActivator activator(*this); GuardLocker locker(m_taskTreePrivate->m_guard); if (m_groupHandler.m_setupHandler) m_groupHandler.m_setupHandler(); if (m_groupHandler.m_dynamicSetupHandler) - m_groupConfig = m_groupHandler.m_dynamicSetupHandler(); + groupAction = m_groupHandler.m_dynamicSetupHandler(); } - if (m_groupConfig.action == GroupAction::StopWithDone - || m_groupConfig.action == GroupAction::StopWithError) { - const bool success = m_groupConfig.action == GroupAction::StopWithDone; + if (groupAction == TaskAction::StopWithDone || groupAction == TaskAction::StopWithError) { + const bool success = groupAction == TaskAction::StopWithDone; const int skippedTaskCount = taskCount(); m_taskTreePrivate->advanceProgress(skippedTaskCount); invokeEndHandler(success); return toTaskAction(success); } - const int skippedTaskCount = selectChildren(); - m_taskTreePrivate->advanceProgress(skippedTaskCount); - - if (m_selectedChildren.isEmpty()) { + if (m_children.isEmpty()) { invokeEndHandler(true); return TaskAction::StopWithDone; // TODO: take workflow policy into account? } @@ -389,13 +383,13 @@ TaskAction TaskContainer::start() TaskAction TaskContainer::startChildren() { - const int childCount = m_selectedChildren.size(); + const int childCount = m_children.size(); for (int i = m_doneCount; i < childCount; ++i) { const int limit = currentLimit(); if (i >= limit) break; - const TaskAction action = m_selectedChildren.at(i)->start(); + const TaskAction action = m_children.at(i)->start(); if (action == TaskAction::Continue) continue; @@ -406,7 +400,7 @@ TaskAction TaskContainer::startChildren() int skippedTaskCount = 0; // Skip scheduled but not run yet. The current (i) was already notified. for (int j = i + 1; j < limit; ++j) - skippedTaskCount += m_selectedChildren.at(j)->taskCount(); + skippedTaskCount += m_children.at(j)->taskCount(); m_taskTreePrivate->advanceProgress(skippedTaskCount); return finalizeAction; @@ -414,37 +408,20 @@ TaskAction TaskContainer::startChildren() return TaskAction::Continue; } -int TaskContainer::selectChildren() -{ - if (m_groupConfig.action != GroupAction::ContinueSelected) { - m_selectedChildren = m_children; - return 0; - } - m_selectedChildren.clear(); - int skippedTaskCount = 0; - for (int i = 0; i < m_children.size(); ++i) { - if (m_groupConfig.childrenToRun.contains(i)) - m_selectedChildren.append(m_children.at(i)); - else - skippedTaskCount += m_children.at(i)->taskCount(); - } - return skippedTaskCount; -} - void TaskContainer::stop() { if (!isRunning()) return; - const int childCount = m_selectedChildren.size(); + const int childCount = m_children.size(); const int limit = currentLimit(); for (int i = m_doneCount; i < limit; ++i) - m_selectedChildren.at(i)->stop(); + m_children.at(i)->stop(); int skippedTaskCount = 0; for (int i = limit; i < childCount; ++i) - skippedTaskCount += m_selectedChildren.at(i)->taskCount(); + skippedTaskCount += m_children.at(i)->taskCount(); m_taskTreePrivate->advanceProgress(skippedTaskCount); m_doneCount = -1; @@ -462,7 +439,7 @@ int TaskContainer::taskCount() const int TaskContainer::currentLimit() const { - const int childCount = m_selectedChildren.size(); + const int childCount = m_children.size(); return m_parallelLimit ? qMin(m_doneCount + m_parallelLimit, childCount) : childCount; } @@ -479,7 +456,7 @@ TaskAction TaskContainer::childDone(bool success) ++m_doneCount; updateSuccessBit(success); - if (m_doneCount == m_selectedChildren.size()) { + if (m_doneCount == m_children.size()) { invokeEndHandler(m_successBit); groupDone(m_successBit); return toTaskAction(m_successBit); @@ -528,7 +505,7 @@ void TaskContainer::invokeEndHandler(bool success) void TaskContainer::resetSuccessBit() { - if (m_selectedChildren.isEmpty()) + if (m_children.isEmpty()) m_successBit = true; if (m_workflowPolicy == WorkflowPolicy::StopOnDone diff --git a/src/libs/utils/tasktree.h b/src/libs/utils/tasktree.h index fabf7fe4286..a82d5a235d7 100644 --- a/src/libs/utils/tasktree.h +++ b/src/libs/utils/tasktree.h @@ -5,8 +5,8 @@ #include "utils_global.h" +#include #include -#include #include namespace Utils { @@ -101,14 +101,6 @@ enum class WorkflowPolicy { Optional // Returns always done after all children finished }; -enum class GroupAction -{ - ContinueAll, - ContinueSelected, - StopWithDone, - StopWithError -}; - enum class TaskAction { Continue, @@ -116,13 +108,6 @@ enum class TaskAction StopWithError }; -class GroupConfig -{ -public: - GroupAction action = GroupAction::ContinueAll; - QSet childrenToRun = {}; -}; - class QTCREATOR_UTILS_EXPORT TaskItem { public: @@ -135,7 +120,7 @@ public: // Called when group entered / after group ended with success or failure using GroupSimpleHandler = std::function; // Called when group entered - using GroupSetupHandler = std::function; + using GroupSetupHandler = std::function; struct TaskHandler { TaskCreateHandler m_createHandler; diff --git a/src/plugins/clangtools/clangtoolrunner.cpp b/src/plugins/clangtools/clangtoolrunner.cpp index 1d74b4b3db9..08e93996233 100644 --- a/src/plugins/clangtools/clangtoolrunner.cpp +++ b/src/plugins/clangtools/clangtoolrunner.cpp @@ -122,25 +122,24 @@ TaskItem clangToolTask(const AnalyzeInputData &input, }; const auto onGroupSetup = [=] { - const GroupConfig error = GroupConfig{GroupAction::StopWithError}; if (setupHandler && !setupHandler()) - return error; + return TaskAction::StopWithError; ClangToolStorage *data = storage.activeStorage(); data->name = clangToolName(input.tool); data->executable = toolExecutable(input.tool); if (!data->executable.isExecutableFile()) { qWarning() << "Can't start:" << data->executable << "as" << data->name; - return error; + return TaskAction::StopWithError; } QTC_CHECK(!input.unit.arguments.contains(QLatin1String("-o"))); QTC_CHECK(!input.unit.arguments.contains(input.unit.file)); - QTC_ASSERT(FilePath::fromString(input.unit.file).exists(), return error); + QTC_ASSERT(FilePath::fromString(input.unit.file).exists(), return TaskAction::StopWithError); data->outputFilePath = createOutputFilePath(input.outputDirPath, input.unit.file); - QTC_ASSERT(!data->outputFilePath.isEmpty(), return error); + QTC_ASSERT(!data->outputFilePath.isEmpty(), return TaskAction::StopWithError); - return GroupConfig{GroupAction::ContinueAll}; + return TaskAction::Continue; }; const auto onProcessSetup = [=](QtcProcess &process) { process.setEnvironment(input.environment); diff --git a/src/plugins/git/gitclient.cpp b/src/plugins/git/gitclient.cpp index 67cca2d70e7..c5a7f4cfbbd 100644 --- a/src/plugins/git/gitclient.cpp +++ b/src/plugins/git/gitclient.cpp @@ -344,8 +344,8 @@ ShowController::ShowController(IDocument *document, const QString &id) const auto desciptionDetailsSetup = [storage] { if (!storage->m_postProcessDescription) - return GroupConfig{GroupAction::StopWithDone}; - return GroupConfig(); + return TaskAction::StopWithDone; + return TaskAction::Continue; }; const auto setupBranches = [this, storage](QtcProcess &process) { diff --git a/tests/auto/utils/tasktree/tst_tasktree.cpp b/tests/auto/utils/tasktree/tst_tasktree.cpp index 6d61b471829..7ca6aea8152 100644 --- a/tests/auto/utils/tasktree/tst_tasktree.cpp +++ b/tests/auto/utils/tasktree/tst_tasktree.cpp @@ -251,7 +251,7 @@ void tst_TaskTree::processTree_data() Process(std::bind(setupDynamicProcess, _1, 2, TaskAction::Continue), readResult, readError), Group { DynamicSetup([storage] { storage->m_log.append({0, Handler::GroupSetup}); - return GroupConfig{GroupAction::StopWithError}; }), + return TaskAction::StopWithError; }), Process(std::bind(setupDynamicProcess, _1, 3, TaskAction::Continue), readResult, readError) }, Process(std::bind(setupDynamicProcess, _1, 4, TaskAction::Continue), readResult, readError) @@ -520,10 +520,9 @@ void tst_TaskTree::processTree_data() {-1, Handler::GroupDone}}; QTest::newRow("Optional") << optionalRoot << storage << optionalLog << true << true << 2; - const auto stopWithDoneSetup = [] { return GroupConfig{GroupAction::StopWithDone}; }; - const auto stopWithErrorSetup = [] { return GroupConfig{GroupAction::StopWithError}; }; - const auto continueAllSetup = [] { return GroupConfig{GroupAction::ContinueAll}; }; - const auto continueSelSetup = [] { return GroupConfig{GroupAction::ContinueSelected, {0, 2}}; }; + const auto stopWithDoneSetup = [] { return TaskAction::StopWithDone; }; + const auto stopWithErrorSetup = [] { return TaskAction::StopWithError; }; + const auto continueSetup = [] { return TaskAction::Continue; }; const auto constructDynamicSetup = [=](const DynamicSetup &dynamicSetup) { return Group { Storage(storage), @@ -554,30 +553,18 @@ void tst_TaskTree::processTree_data() QTest::newRow("DynamicSetupError") << dynamicSetupErrorRoot << storage << dynamicSetupErrorLog << true << false << 4; - const Group dynamicSetupAllRoot = constructDynamicSetup({continueAllSetup}); - const Log dynamicSetupAllLog{{1, Handler::Setup}, - {1, Handler::Done}, - {2, Handler::Setup}, - {2, Handler::Done}, - {3, Handler::Setup}, - {3, Handler::Done}, - {4, Handler::Setup}, - {4, Handler::Done}, - {-1, Handler::GroupDone}}; - QTest::newRow("DynamicSetupAll") << dynamicSetupAllRoot << storage << dynamicSetupAllLog - << true << true << 4; - - const Group dynamicSetupSelRoot = constructDynamicSetup({continueSelSetup}); - const Log dynamicSetupSelLog{{1, Handler::Setup}, - {1, Handler::Done}, - {2, Handler::Setup}, - {2, Handler::Done}, - {4, Handler::Setup}, - {4, Handler::Done}, - {-1, Handler::GroupDone}}; - QTest::newRow("DynamicSetupSelected") << dynamicSetupSelRoot << storage << dynamicSetupSelLog - << true << true << 4; - + const Group dynamicSetupContinueRoot = constructDynamicSetup({continueSetup}); + const Log dynamicSetupContinueLog{{1, Handler::Setup}, + {1, Handler::Done}, + {2, Handler::Setup}, + {2, Handler::Done}, + {3, Handler::Setup}, + {3, Handler::Done}, + {4, Handler::Setup}, + {4, Handler::Done}, + {-1, Handler::GroupDone}}; + QTest::newRow("DynamicSetupContinue") << dynamicSetupContinueRoot << storage + << dynamicSetupContinueLog << true << true << 4; } void tst_TaskTree::processTree()