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 <hjk@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>

src/libs/utils/tasktree.cpp

@@ -6,6 +6,8 @@
 #include "guard.h"
 #include "qtcassert.h"
 
+#include <QSet>
+
 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<TreeStorageBase> m_storageList;
     QList<int> m_storageIdList;
     int m_taskCount = 0;
-    GroupConfig m_groupConfig;
     QList<TaskNode *> m_children;
-    QList<TaskNode *> 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
+    if (groupAction == TaskAction::StopWithDone || groupAction == TaskAction::StopWithError) {
-        || m_groupConfig.action == GroupAction::StopWithError) {
+        const bool success = groupAction == TaskAction::StopWithDone;
-        const bool success = m_groupConfig.action == GroupAction::StopWithDone;
         const int skippedTaskCount = taskCount();
         m_taskTreePrivate->advanceProgress(skippedTaskCount);
         invokeEndHandler(success);
         return toTaskAction(success);
     }
 
-    const int skippedTaskCount = selectChildren();
+    if (m_children.isEmpty()) {
-    m_taskTreePrivate->advanceProgress(skippedTaskCount);
-
-    if (m_selectedChildren.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

src/libs/utils/tasktree.h

@@ -5,8 +5,8 @@
 
 #include "utils_global.h"
 
+#include <QHash>
 #include <QObject>
-#include <QSet>
 #include <QSharedPointer>
 
 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<int> 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<void()>;
     // Called when group entered
-    using GroupSetupHandler = std::function<GroupConfig()>;
+    using GroupSetupHandler = std::function<TaskAction()>;
 
     struct TaskHandler {
         TaskCreateHandler m_createHandler;

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);

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 TaskAction::StopWithDone;
-        return GroupConfig();
+        return TaskAction::Continue;
     };
 
     const auto setupBranches = [this, storage](QtcProcess &process) {

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 stopWithDoneSetup = [] { return TaskAction::StopWithDone; };
-    const auto stopWithErrorSetup = [] { return GroupConfig{GroupAction::StopWithError}; };
+    const auto stopWithErrorSetup = [] { return TaskAction::StopWithError; };
-    const auto continueAllSetup = [] { return GroupConfig{GroupAction::ContinueAll}; };
+    const auto continueSetup = [] { return TaskAction::Continue; };
-    const auto continueSelSetup = [] { return GroupConfig{GroupAction::ContinueSelected, {0, 2}}; };
     const auto constructDynamicSetup = [=](const DynamicSetup &dynamicSetup) {
         return Group {
             Storage(storage),
@@ -554,8 +553,8 @@ void tst_TaskTree::processTree_data()
     QTest::newRow("DynamicSetupError") << dynamicSetupErrorRoot << storage << dynamicSetupErrorLog
                                        << true << false << 4;
 
-    const Group dynamicSetupAllRoot = constructDynamicSetup({continueAllSetup});
+    const Group dynamicSetupContinueRoot = constructDynamicSetup({continueSetup});
-    const Log dynamicSetupAllLog{{1, Handler::Setup},
+    const Log dynamicSetupContinueLog{{1, Handler::Setup},
                                       {1, Handler::Done},
                                       {2, Handler::Setup},
                                       {2, Handler::Done},
@@ -564,20 +563,8 @@ void tst_TaskTree::processTree_data()
                                       {4, Handler::Setup},
                                       {4, Handler::Done},
                                       {-1, Handler::GroupDone}};
-    QTest::newRow("DynamicSetupAll") << dynamicSetupAllRoot << storage << dynamicSetupAllLog
+    QTest::newRow("DynamicSetupContinue") << dynamicSetupContinueRoot << storage
-                                     << true << true << 4;
+                                          << dynamicSetupContinueLog << 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;
-
 }
 
 void tst_TaskTree::processTree()