AutotoolsPM: Replace QThread with TaskTree

Don't create a permanent thread for just one parsing.
Use TaskTree with async recipe instead.

Change-Id: I8eef0082c5d39a45fb0d8751c632ba02a955faef
Reviewed-by: hjk <hjk@qt.io>

src/plugins/autotoolsprojectmanager/CMakeLists.txt

@@ -11,6 +11,5 @@ add_qtc_plugin(AutotoolsProjectManager
     autotoolsprojectplugin.cpp
     configurestep.cpp configurestep.h
     makefileparser.cpp makefileparser.h
-    makefileparserthread.cpp makefileparserthread.h
     makestep.cpp makestep.h
 )

src/plugins/autotoolsprojectmanager/autotoolsbuildsystem.cpp

@@ -3,7 +3,7 @@
 
 #include "autotoolsbuildsystem.h"
 
-#include "makefileparserthread.h"
+#include "makefileparser.h"
 
 #include <projectexplorer/buildconfiguration.h>
 #include <projectexplorer/projectupdater.h>
@@ -12,10 +12,11 @@
 
 #include <qtsupport/qtcppkitinfo.h>
 
-#include <utils/filesystemwatcher.h>
+#include <utils/async.h>
 #include <utils/qtcassert.h>
 
 using namespace ProjectExplorer;
+using namespace Tasking;
 using namespace Utils;
 
 namespace AutotoolsProjectManager::Internal {
@@ -28,28 +29,35 @@ AutotoolsBuildSystem::AutotoolsBuildSystem(Target *target)
     connect(target->project(), &Project::projectFileIsDirty, this, [this] { requestParse(); });
 }
 
-AutotoolsBuildSystem::~AutotoolsBuildSystem()
+AutotoolsBuildSystem::~AutotoolsBuildSystem() = default;
-{
-    delete m_cppCodeModelUpdater;
 
-    if (m_makefileParserThread)
+static void parseMakefile(QPromise<MakefileParserOutputData> &promise, const QString &makefile)
-        m_makefileParserThread->wait();
+{
+    MakefileParser parser(makefile);
+    if (parser.parse())
+        promise.addResult(parser.outputData());
+    else
+        promise.future().cancel();
 }
 
 void AutotoolsBuildSystem::triggerParsing()
 {
-    // The thread is still busy parsing a previous configuration.
+    const Storage<std::optional<ParseGuard>> storage;
-    // Wait until the thread has been finished and delete it.
-    // TODO: Discuss whether blocking is acceptable.
-    if (m_makefileParserThread)
-        m_makefileParserThread->wait();
 
-    // Parse the makefile asynchronously in a thread
+    const auto onSetup = [this, storage](Async<MakefileParserOutputData> &async) {
-    m_makefileParserThread.reset(new MakefileParserThread(this));
+        *storage = guardParsingRun();
+        async.setConcurrentCallData(parseMakefile, projectFilePath().toString());
+    };
+    const auto onDone = [this, storage](const Async<MakefileParserOutputData> &async) {
+        (*storage)->markAsSuccess();
+        makefileParsingFinished(async.result());
+    };
 
-    connect(m_makefileParserThread.get(), &MakefileParserThread::done,
+    const Group recipe {
-            this, &AutotoolsBuildSystem::makefileParsingFinished);
+        storage,
-    m_makefileParserThread->start();
+        AsyncTask<MakefileParserOutputData>(onSetup, onDone, CallDoneIf::Success)
+    };
+    m_parserRunner.start(recipe);
 }
 
 static QStringList filterIncludes(const QString &absSrc, const QString &absBuild,
@@ -69,37 +77,28 @@ static QStringList filterIncludes(const QString &absSrc, const QString &absBuild
     return result;
 }
 
-void AutotoolsBuildSystem::makefileParsingFinished()
+void AutotoolsBuildSystem::makefileParsingFinished(const MakefileParserOutputData &outputData)
 {
-    // The parsing has been cancelled by the user. Don't show any project data at all.
-    if (m_makefileParserThread->isCanceled()) {
-        m_makefileParserThread.release()->deleteLater();
-        return;
-    }
-
-    if (m_makefileParserThread->hasError())
-        qWarning("Parsing of makefile contained errors.");
-
     m_files.clear();
 
-    QSet<Utils::FilePath> filesToWatch;
+    QSet<FilePath> filesToWatch;
 
     // Apply sources to m_files, which are returned at AutotoolsBuildSystem::files()
     const QFileInfo fileInfo = projectFilePath().toFileInfo();
     const QDir dir = fileInfo.absoluteDir();
-    const QStringList files = m_makefileParserThread->sources();
+    const QStringList files = outputData.m_sources;
     for (const QString& file : files)
         m_files.append(dir.absoluteFilePath(file));
 
     // Watch for changes of Makefile.am files. If a Makefile.am file
     // has been changed, the project tree must be reparsed.
-    const QStringList makefiles = m_makefileParserThread->makefiles();
+    const QStringList makefiles = outputData.m_makefiles;
     for (const QString &makefile : makefiles) {
         const QString absMakefile = dir.absoluteFilePath(makefile);
 
         m_files.append(absMakefile);
 
-        filesToWatch.insert(Utils::FilePath::fromString(absMakefile));
+        filesToWatch.insert(FilePath::fromString(absMakefile));
     }
 
     // Add configure.ac file to project and watch for changes.
@@ -109,12 +108,12 @@ void AutotoolsBuildSystem::makefileParsingFinished()
         const QString absConfigureAc = dir.absoluteFilePath(configureAc);
         m_files.append(absConfigureAc);
 
-        filesToWatch.insert(Utils::FilePath::fromString(absConfigureAc));
+        filesToWatch.insert(FilePath::fromString(absConfigureAc));
     }
 
     auto newRoot = std::make_unique<ProjectNode>(project()->projectDirectory());
     for (const QString &f : std::as_const(m_files)) {
-        const Utils::FilePath path = Utils::FilePath::fromString(f);
+        const FilePath path = FilePath::fromString(f);
         newRoot->addNestedNode(std::make_unique<FileNode>(path,
                                                           FileNode::fileTypeForFileName(path)));
     }
@@ -128,8 +127,8 @@ void AutotoolsBuildSystem::makefileParsingFinished()
     rpp.setDisplayName(project()->displayName());
     rpp.setProjectFileLocation(projectFilePath().toString());
     rpp.setQtVersion(kitInfo.projectPartQtVersion);
-    const QStringList cflags = m_makefileParserThread->cflags();
+    const QStringList cflags = outputData.m_cflags;
-    QStringList cxxflags = m_makefileParserThread->cxxflags();
+    QStringList cxxflags = outputData.m_cxxflags;
     if (cxxflags.isEmpty())
         cxxflags = cflags;
 
@@ -142,15 +141,12 @@ void AutotoolsBuildSystem::makefileParsingFinished()
 
     const QString absBuild = bc ? bc->buildDirectory().toString() : QString();
 
-    rpp.setIncludePaths(filterIncludes(absSrc, absBuild, m_makefileParserThread->includePaths()));
+    rpp.setIncludePaths(filterIncludes(absSrc, absBuild, outputData.m_includePaths));
-    rpp.setMacros(m_makefileParserThread->macros());
+    rpp.setMacros(outputData.m_macros);
     rpp.setFiles(m_files);
 
     m_cppCodeModelUpdater->update({project(), kitInfo, activeParseEnvironment(), {rpp}});
 
-    m_makefileParserThread.release()->deleteLater();
+    emitBuildSystemUpdated();}
-
-    emitBuildSystemUpdated();
-}
 
 } // AutotoolsProjectManager::Internal

src/plugins/autotoolsprojectmanager/autotoolsbuildsystem.h

@@ -5,13 +5,13 @@
 
 #include <projectexplorer/buildsystem.h>
 
-#include <memory>
+#include <solutions/tasking/tasktreerunner.h>
 
 namespace ProjectExplorer { class ProjectUpdater; }
 
 namespace AutotoolsProjectManager::Internal {
 
-class MakefileParserThread;
+class MakefileParserOutputData;
 
 class AutotoolsBuildSystem final : public ProjectExplorer::BuildSystem
 {
@@ -29,15 +29,15 @@ private:
      * takes care listen to file changes for Makefile.am and configure.ac
      * files.
      */
-    void makefileParsingFinished();
+    void makefileParsingFinished(const MakefileParserOutputData &outputData);
 
     /// Return value for AutotoolsProject::files()
     QStringList m_files;
 
     /// Responsible for parsing the makefiles asynchronously in a thread
-    std::unique_ptr<MakefileParserThread> m_makefileParserThread;
+    Tasking::TaskTreeRunner m_parserRunner;
 
-    ProjectExplorer::ProjectUpdater *m_cppCodeModelUpdater = nullptr;
+    std::unique_ptr<ProjectExplorer::ProjectUpdater> m_cppCodeModelUpdater;
 };
 
 } // AutotoolsProjectManager::Internal

src/plugins/autotoolsprojectmanager/autotoolsprojectmanager.qbs

@@ -26,8 +26,6 @@ QtcPlugin {
         "configurestep.h",
         "makefileparser.cpp",
         "makefileparser.h",
-        "makefileparserthread.cpp",
-        "makefileparserthread.h",
         "makestep.cpp",
         "makestep.h",
     ]

src/plugins/autotoolsprojectmanager/makefileparserthread.cpp

@@ -1,95 +0,0 @@
-// Copyright (C) 2016 Openismus GmbH.
-// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
-
-#include "makefileparserthread.h"
-
-#include <QMutexLocker>
-
-namespace AutotoolsProjectManager::Internal {
-
-MakefileParserThread::MakefileParserThread(ProjectExplorer::BuildSystem *bs)
-    : m_parser(bs->projectFilePath().toString()),
-      m_guard(bs->guardParsingRun())
-{
-    connect(this, &QThread::finished, this, &MakefileParserThread::done, Qt::QueuedConnection);
-}
-
-QStringList MakefileParserThread::sources() const
-{
-    QMutexLocker locker(&m_mutex);
-    return m_sources;
-}
-
-QStringList MakefileParserThread::makefiles() const
-{
-    QMutexLocker locker(&m_mutex);
-    return m_makefiles;
-}
-
-QString MakefileParserThread::executable() const
-{
-    QMutexLocker locker(&m_mutex);
-    return m_executable;
-}
-
-QStringList MakefileParserThread::includePaths() const
-{
-    QMutexLocker locker(&m_mutex);
-    return m_includePaths;
-}
-
-ProjectExplorer::Macros MakefileParserThread::macros() const
-{
-    QMutexLocker locker(&m_mutex);
-    return m_macros;
-}
-
-QStringList MakefileParserThread::cflags() const
-{
-    QMutexLocker locker(&m_mutex);
-    return m_cflags;
-}
-
-QStringList MakefileParserThread::cxxflags() const
-{
-    QMutexLocker locker(&m_mutex);
-    return m_cxxflags;
-}
-
-bool MakefileParserThread::hasError() const
-{
-    QMutexLocker locker(&m_mutex);
-    return !m_guard.isSuccess();
-}
-
-bool MakefileParserThread::isCanceled() const
-{
-    // MakefileParser::isCanceled() is thread-safe
-    return m_parser.isCanceled();
-}
-
-void MakefileParserThread::cancel()
-{
-    m_parser.cancel();
-}
-
-void MakefileParserThread::run()
-{
-    const bool success = m_parser.parse();
-
-    // Important: Start locking the mutex _after_ the parsing has been finished, as
-    // this prevents long locks if the caller reads a value before the signal
-    // finished() has been emitted.
-    QMutexLocker locker(&m_mutex);
-    if (success)
-        m_guard.markAsSuccess();
-    m_executable = m_parser.executable();
-    m_sources = m_parser.sources();
-    m_makefiles = m_parser.makefiles();
-    m_includePaths = m_parser.includePaths();
-    m_macros = m_parser.macros();
-    m_cflags = m_parser.cflags();
-    m_cxxflags = m_parser.cxxflags();
-}
-
-} // AutotoolsProjectManager::Internal

src/plugins/autotoolsprojectmanager/makefileparserthread.h

@@ -1,123 +0,0 @@
-// Copyright (C) 2016 Openismus GmbH.
-// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
-
-#pragma once
-
-#include "makefileparser.h"
-
-#include <projectexplorer/buildsystem.h>
-#include <projectexplorer/projectmacro.h>
-
-#include <QMutex>
-#include <QStringList>
-#include <QThread>
-#include <QVector>
-
-namespace AutotoolsProjectManager::Internal {
-
-/**
- * @brief Executes the makefile parser in the thread.
- *
- * After the finished() signal has been emitted, the makefile
- * parser output can be read by sources(), makefiles() and executable().
- * A parsing error can be checked by hasError().
- */
-class MakefileParserThread : public QThread
-{
-    Q_OBJECT
-
-    using Macros = ProjectExplorer::Macros;
-
-public:
-    explicit MakefileParserThread(ProjectExplorer::BuildSystem *bs);
-
-    /** @see QThread::run() */
-    void run() override;
-
-    /**
-     * @return List of sources that are set for the _SOURCES target.
-     *         Sources in sub directorties contain the sub directory as
-     *         prefix. Should be invoked, after the signal finished()
-     *         has been emitted.
-     */
-    QStringList sources() const;
-
-    /**
-     * @return List of Makefile.am files from the current directory and
-     *         all sub directories. The values for sub directories contain
-     *         the sub directory as prefix. Should be invoked, after the
-     *         signal finished() has been emitted.
-     */
-    QStringList makefiles() const;
-
-    /**
-     * @return File name of the executable. Should be invoked, after the
-     *         signal finished() has been emitted.
-     */
-    QString executable() const;
-
-    /**
-     * @return List of include paths. Should be invoked, after the signal
-     *         finished() has been emitted.
-     */
-    QStringList includePaths() const;
-
-    /**
-     * @return Concatenated macros. Should be invoked, after the signal
-     *         finished() has been emitted.
-     */
-    Macros macros() const;
-
-    /**
-     * @return List of compiler flags for C. Should be invoked, after the signal
-     *         finished() has been emitted.
-     */
-    QStringList cflags() const;
-
-    /**
-     * @return List of compiler flags for C++. Should be invoked, after the
-     *         signal finished() has been emitted.
-     */
-    QStringList cxxflags() const;
-
-    /**
-     * @return True, if an error occurred during the parsing. Should be invoked,
-     *         after the signal finished() has been emitted.
-     */
-    bool hasError() const;
-
-    /**
-     * @return True, if the parsing has been cancelled by MakefileParserThread::cancel().
-     */
-    bool isCanceled() const;
-
-    /**
-     * Cancels the parsing of the makefile. MakefileParser::hasError() will
-     * return true in this case.
-     */
-    void cancel();
-
-signals:
-    /**
-      * Similar to finished, but emitted from MakefileParserThread thread, i.e. from the
-      * thread where the MakefileParserThread lives in, not the tread that it creates.
-      * This helps to avoid race condition when connecting to finished() signal.
-      */
-    void done();
-
-private:
-    MakefileParser m_parser;    ///< Is not accessible outside the thread
-
-    mutable QMutex m_mutex;
-    QString m_executable;       ///< Return value for MakefileParserThread::executable()
-    QStringList m_sources;      ///< Return value for MakefileParserThread::sources()
-    QStringList m_makefiles;    ///< Return value for MakefileParserThread::makefiles()
-    QStringList m_includePaths; ///< Return value for MakefileParserThread::includePaths()
-    Macros m_macros;            ///< Return value for MakefileParserThread::macros()
-    QStringList m_cflags;       ///< Return value for MakefileParserThread::cflags()
-    QStringList m_cxxflags;     ///< Return value for MakefileParserThread::cxxflags()
-
-    ProjectExplorer::BuildSystem::ParseGuard m_guard;
-};
-
-} // AutotoolsProjectManager::Internal