CMake: Quieten soft-assert on small project loads

The directory tree scanner and the project parsing work independently of
each other. Add logic to combine these two sets of results into one
parsing state.

Change-Id: I46e94f0e866b40ee7225235c536c742cecf11b45
Reviewed-by: Tim Jenssen <tim.jenssen@qt.io>

src/plugins/cmakeprojectmanager/builddirmanager.cpp

@@ -62,10 +62,6 @@ BuildDirManager::BuildDirManager(CMakeBuildConfiguration *bc) :
     m_buildConfiguration(bc)
 {
     QTC_ASSERT(bc, return);
-
-    m_reparseTimer.setSingleShot(true);
-
-    connect(&m_reparseTimer, &QTimer::timeout, this, &BuildDirManager::parse);
 }
 
 BuildDirManager::~BuildDirManager() = default;
@@ -209,7 +205,7 @@ void BuildDirManager::maybeForceReparseOnceReaderReady()
     // The critical keys *must* be set in cmake configuration, so those were already
     // handled above.
     if (mustReparse || kcit != targetConfig.constEnd())
-        parseOnceReaderReady(true);
+        emit requestReparse(true);
 }
 
 bool BuildDirManager::isParsing() const
@@ -232,7 +228,7 @@ void BuildDirManager::becameDirty()
     if (!tool->isAutoRun())
         return;
 
-    m_reparseTimer.start(1000);
+    emit requestReparse(false);
 }
 
 void BuildDirManager::forceReparse()
@@ -272,11 +268,6 @@ void BuildDirManager::resetData()
     m_buildTargets.clear();
 }
 
-bool BuildDirManager::updateCMakeStateBeforeBuild()
-{
-    return m_reparseTimer.isActive();
-}
-
 bool BuildDirManager::persistCMakeState()
 {
     if (!m_tempDir)
@@ -455,7 +446,7 @@ void BuildDirManager::maybeForceReparse()
         return;
 
     if (!m_reader || !m_reader->hasData()) {
-        forceReparse();
+        emit requestReparse(true);
         return;
     }
 

src/plugins/cmakeprojectmanager/builddirmanager.h

@@ -67,7 +67,6 @@ public:
     void forceReparseWithoutCheckingForChanges();
     void maybeForceReparse(); // Only reparse if the configuration has changed...
     void resetData();
-    bool updateCMakeStateBeforeBuild();
     bool persistCMakeState();
 
     void generateProjectTree(CMakeProjectNode *root,
@@ -83,6 +82,7 @@ public:
     CMakeBuildConfiguration *buildConfiguration() const { return m_buildConfiguration; }
 
 signals:
+    void requestReparse(bool urgent) const;
     void configurationStarted() const;
     void dataAvailable() const;
     void errorOccured(const QString &err) const;
@@ -109,8 +109,6 @@ private:
     mutable std::unique_ptr<Utils::TemporaryDirectory> m_tempDir = nullptr;
     mutable CMakeConfig m_cmakeCache;
 
-    QTimer m_reparseTimer;
-
     std::unique_ptr<BuildDirReader> m_reader;
 
     mutable QList<CMakeBuildTarget> m_buildTargets;

src/plugins/cmakeprojectmanager/cmakebuildconfiguration.cpp

@@ -142,10 +142,12 @@ void CMakeBuildConfiguration::ctor()
                                            target()->kit(),
                                            displayName(), BuildConfiguration::Unknown));
 
+    connect(m_buildDirManager.get(), &BuildDirManager::requestReparse,
+            this, [this](bool urgent) { emit requestReparse(this, urgent); });
     connect(m_buildDirManager.get(), &BuildDirManager::dataAvailable,
             this, [this, project]() {
         clearError();
-        project->updateProjectData(this);
+        project->handleParsingSuccess(this);
     });
     connect(m_buildDirManager.get(), &BuildDirManager::errorOccured,
             this, [this, project](const QString &msg) {
@@ -153,9 +155,9 @@ void CMakeBuildConfiguration::ctor()
         project->handleParsingError(this);
     });
     connect(m_buildDirManager.get(), &BuildDirManager::configurationStarted,
-            this, [this, project]() {
+            this, [this]() {
-        project->handleParsingStarted(this);
         clearError(ForceEnabledChanged::True);
+        emit parsingStarted(this);
     });
 
     connect(this, &CMakeBuildConfiguration::environmentChanged,
@@ -188,7 +190,7 @@ bool CMakeBuildConfiguration::persistCMakeState()
 
 bool CMakeBuildConfiguration::updateCMakeStateBeforeBuild()
 {
-    return m_buildDirManager->updateCMakeStateBeforeBuild();
+    return static_cast<CMakeProject *>(project())->mustUpdateCMakeStateBeforeBuild();
 }
 
 void CMakeBuildConfiguration::runCMake()

src/plugins/cmakeprojectmanager/cmakebuildconfiguration.h

@@ -97,6 +97,9 @@ public:
     void buildTarget(const QString &buildTarget);
 
 signals:
+    void requestReparse(CMakeBuildConfiguration *, bool isUrgent);
+    void parsingStarted(CMakeBuildConfiguration *);
+
     void errorOccured(const QString &message);
     void warningOccured(const QString &message);
 

src/plugins/cmakeprojectmanager/cmakeproject.cpp

@@ -51,6 +51,7 @@
 #include <qmljs/qmljsmodelmanagerinterface.h>
 
 #include <utils/algorithm.h>
+#include <utils/asconst.h>
 #include <utils/qtcassert.h>
 #include <utils/stringutils.h>
 #include <utils/hostosinfo.h>
@@ -76,12 +77,27 @@ using namespace Internal;
 CMakeProject::CMakeProject(const FileName &fileName) : Project(Constants::CMAKEMIMETYPE, fileName),
     m_cppCodeModelUpdater(new CppTools::CppProjectUpdater(this))
 {
+    m_delayedParsingTimer.setSingleShot(true);
+
+    connect(&m_delayedParsingTimer, &QTimer::timeout,
+            this, [this]() { startParsingProject(PARSE); });
+
     setId(CMakeProjectManager::Constants::CMAKEPROJECT_ID);
     setProjectContext(Core::Context(CMakeProjectManager::Constants::PROJECTCONTEXT));
     setProjectLanguages(Core::Context(ProjectExplorer::Constants::CXX_LANGUAGE_ID));
     setDisplayName(projectDirectory().fileName());
 
-    connect(this, &CMakeProject::activeTargetChanged, this, &CMakeProject::handleActiveTargetChanged);
+    connect(this, &Project::activeProjectConfigurationChanged,
+            this, &CMakeProject::handleActiveProjectConfigurationChanged);
+
+    subscribeSignal(&CMakeBuildConfiguration::requestReparse,
+                    this, [this](CMakeBuildConfiguration *bc, bool isUrgent) {
+        if (bc->isActive()) {
+            m_delayedParsingTimer.setInterval(isUrgent ? 0 : 1000);
+            m_delayedParsingTimer.start();
+        }
+    });
+
     connect(&m_treeScanner, &TreeScanner::finished, this, &CMakeProject::handleTreeScanningFinished);
 
     m_treeScanner.setFilter([this](const Utils::MimeType &mimeType, const Utils::FileName &fn) {
@@ -116,8 +132,6 @@ CMakeProject::CMakeProject(const FileName &fileName) : Project(Constants::CMAKEM
         }
         return type;
     });
-
-    scanProjectTree();
 }
 
 CMakeProject::~CMakeProject()
@@ -134,14 +148,11 @@ CMakeProject::~CMakeProject()
 
 void CMakeProject::updateProjectData(CMakeBuildConfiguration *bc)
 {
+    Target *const t = activeTarget();
+
     QTC_ASSERT(bc, return);
-
+    QTC_ASSERT(bc == (t ? t->activeBuildConfiguration() : nullptr), return);
-    Target *t = activeTarget();
+    QTC_ASSERT(m_treeScanner.isFinished() && !bc->isParsing(), return);
-    if (!t || t->activeBuildConfiguration() != bc)
-        return;
-
-    if (!m_treeScanner.isFinished() || bc->isParsing())
-        return;
 
     Kit *k = t->kit();
 
@@ -189,19 +200,6 @@ void CMakeProject::updateProjectData(CMakeBuildConfiguration *bc)
     emit fileListChanged();
 
     emit bc->emitBuildTypeChanged();
-
-    emitParsingFinished(true);
-}
-
-void CMakeProject::handleParsingError(CMakeBuildConfiguration *bc)
-{
-    QTC_ASSERT(bc, return);
-
-    Target *t = activeTarget();
-    if (!t || t->activeBuildConfiguration() != bc)
-        return;
-
-    emitParsingFinished(false);
 }
 
 void CMakeProject::updateQmlJSCodeModel()
@@ -263,12 +261,49 @@ bool CMakeProject::supportsKit(Kit *k, QString *errorMessage) const
 
 void CMakeProject::runCMake()
 {
-    CMakeBuildConfiguration *bc = nullptr;
+    if (isParsing())
-    if (activeTarget())
+        return;
-        bc = qobject_cast<CMakeBuildConfiguration *>(activeTarget()->activeBuildConfiguration());
 
-    if (bc)
+    startParsingProject(PARSE);
+}
+
+void CMakeProject::runCMakeAndScanProjectTree()
+{
+    if (isParsing())
+        return;
+
+    startParsingProject(static_cast<DataCollectionAction>(PARSE | SCAN));
+}
+
+void CMakeProject::startParsingProject(const CMakeProject::DataCollectionAction action)
+{
+    const bool runParse = action & PARSE;
+    const bool runScan = action & SCAN;
+
+    CMakeBuildConfiguration *bc = activeTarget()
+            ? qobject_cast<CMakeBuildConfiguration *>(activeTarget()->activeBuildConfiguration())
+            : nullptr;
+    if (!bc)
+        return;
+
+    if (!m_treeScanner.isFinished() || m_waitingForScan)
+        return;
+
+    emitParsingStarted();
+
+    m_waitingForParse = runParse;
+    m_waitingForScan = runScan;
+    m_combinedScanAndParseResult = true;
+
+    if (runParse)
         bc->runCMake();
+
+    if (runScan) {
+        m_treeScanner.asyncScanForFiles(projectDirectory());
+        Core::ProgressManager::addTask(m_treeScanner.future(),
+                                       tr("Scan \"%1\" project tree").arg(displayName()),
+                                       "CMake.Scan.Tree");
+    }
 }
 
 void CMakeProject::buildCMakeTarget(const QString &buildTarget)
@@ -330,75 +365,84 @@ bool CMakeProject::setupTarget(Target *t)
     return true;
 }
 
-void CMakeProject::scanProjectTree()
+void CMakeProject::handleActiveProjectConfigurationChanged(ProjectConfiguration *pc)
 {
-    if (!m_treeScanner.isFinished())
+    if (auto bc = qobject_cast<CMakeBuildConfiguration *>(pc)) {
+        if (!bc->isActive())
             return;
-    m_treeScanner.asyncScanForFiles(projectDirectory());
+    } else if (!qobject_cast<Target *>(pc)) {
-    Core::ProgressManager::addTask(m_treeScanner.future(),
-                                   tr("Scan \"%1\" project tree").arg(displayName()),
-                                   "CMake.Scan.Tree");
-}
-
-void CMakeProject::handleActiveTargetChanged()
-{
-    if (m_connectedTarget) {
-        disconnect(m_connectedTarget, &Target::activeBuildConfigurationChanged,
-                   this, &CMakeProject::handleActiveBuildConfigurationChanged);
-        disconnect(m_connectedTarget, &Target::kitChanged,
-                   this, &CMakeProject::handleActiveBuildConfigurationChanged);
-    }
-
-    m_connectedTarget = activeTarget();
-
-    if (m_connectedTarget) {
-        connect(m_connectedTarget, &Target::activeBuildConfigurationChanged,
-                this, &CMakeProject::handleActiveBuildConfigurationChanged);
-        connect(m_connectedTarget, &Target::kitChanged,
-                this, &CMakeProject::handleActiveBuildConfigurationChanged);
-    }
-
-    handleActiveBuildConfigurationChanged();
-}
-
-void CMakeProject::handleActiveBuildConfigurationChanged()
-{
-    if (!activeTarget() || !activeTarget()->activeBuildConfiguration())
         return;
-    auto activeBc = qobject_cast<CMakeBuildConfiguration *>(activeTarget()->activeBuildConfiguration());
+    }
 
-    foreach (Target *t, targets()) {
+    for (Target *t : targets()) {
-        foreach (BuildConfiguration *bc, t->buildConfigurations()) {
+        for (BuildConfiguration *bc : t->buildConfigurations()) {
             auto i = qobject_cast<CMakeBuildConfiguration *>(bc);
             QTC_ASSERT(i, continue);
-            if (i == activeBc)
+            if (i->isActive()) {
+                m_waitingForParse = true;
                 i->maybeForceReparse();
-            else
+            } else {
                 i->resetData();
             }
         }
     }
-
-void CMakeProject::handleParsingStarted(const CMakeBuildConfiguration *bc)
-{
-    if (activeTarget() && activeTarget()->activeBuildConfiguration() == bc)
-        emitParsingStarted();
 }
 
 void CMakeProject::handleTreeScanningFinished()
 {
+    QTC_CHECK(m_waitingForScan);
+
     qDeleteAll(m_allFiles);
     m_allFiles = Utils::transform(m_treeScanner.release(), [](const FileNode *fn) { return fn; });
 
     auto t = activeTarget();
-    if (!t)
+    auto bc = qobject_cast<CMakeBuildConfiguration*>(t ? t->activeBuildConfiguration() : nullptr);
+    QTC_ASSERT(bc, return);
+
+    m_combinedScanAndParseResult = m_combinedScanAndParseResult && true;
+    m_waitingForScan = false;
+
+    combineScanAndParse(bc);
+}
+
+void CMakeProject::handleParsingSuccess(CMakeBuildConfiguration *bc)
+{
+    QTC_CHECK(m_waitingForParse);
+
+    if (!bc || !bc->isActive())
         return;
 
-    auto bc = qobject_cast<CMakeBuildConfiguration*>(t->activeBuildConfiguration());
+    m_waitingForParse = false;
-    if (!bc)
+    m_combinedScanAndParseResult = m_combinedScanAndParseResult && true;
+
+    combineScanAndParse(bc);
+}
+
+void CMakeProject::handleParsingError(CMakeBuildConfiguration *bc)
+{
+    QTC_CHECK(m_waitingForParse);
+
+    if (!bc || !bc->isActive())
         return;
 
+    m_waitingForParse = false;
+    m_combinedScanAndParseResult = false;
+
+    combineScanAndParse(bc);
+}
+
+
+void CMakeProject::combineScanAndParse(CMakeBuildConfiguration *bc)
+{
+    QTC_ASSERT(bc && bc->isActive(), return);
+
+    if (m_waitingForParse || m_waitingForScan)
+        return;
+
+    if (m_combinedScanAndParseResult)
         updateProjectData(bc);
+
+    emitParsingFinished(m_combinedScanAndParseResult);
 }
 
 CMakeBuildTarget CMakeProject::buildTargetForTitle(const QString &title)
@@ -539,6 +583,11 @@ void CMakeProject::updateApplicationAndDeploymentTargets()
     t->setDeploymentData(deploymentData);
 }
 
+bool CMakeProject::mustUpdateCMakeStateBeforeBuild()
+{
+    return m_delayedParsingTimer.isActive();
+}
+
 void CMakeProject::createGeneratedCodeModelSupport()
 {
     qDeleteAll(m_extraCompilers);

src/plugins/cmakeprojectmanager/cmakeproject.h

@@ -37,6 +37,7 @@
 
 #include <QFuture>
 #include <QHash>
+#include <QTimer>
 
 #include <memory>
 
@@ -99,7 +100,7 @@ public:
     bool supportsKit(ProjectExplorer::Kit *k, QString *errorMessage = 0) const final;
 
     void runCMake();
-    void scanProjectTree();
+    void runCMakeAndScanProjectTree();
 
     // Context menu actions:
     void buildCMakeTarget(const QString &buildTarget);
@@ -113,12 +114,15 @@ protected:
 private:
     QList<CMakeBuildTarget> buildTargets() const;
 
-    void handleActiveTargetChanged();
+    enum DataCollectionAction { PARSE = 1, SCAN = 2 };
-    void handleActiveBuildConfigurationChanged();
+    void startParsingProject(const DataCollectionAction a);
-    void handleParsingStarted(const Internal::CMakeBuildConfiguration *bc);
+
+    void handleActiveProjectConfigurationChanged(ProjectExplorer::ProjectConfiguration *pc);
     void handleTreeScanningFinished();
-    void updateProjectData(Internal::CMakeBuildConfiguration *bc);
+    void handleParsingSuccess(Internal::CMakeBuildConfiguration *bc);
     void handleParsingError(Internal::CMakeBuildConfiguration *bc);
+    void combineScanAndParse(Internal::CMakeBuildConfiguration *bc);
+    void updateProjectData(Internal::CMakeBuildConfiguration *bc);
     void updateQmlJSCodeModel();
 
     void createGeneratedCodeModelSupport();
@@ -126,7 +130,7 @@ private:
     void updateTargetRunConfigurations(ProjectExplorer::Target *t);
     void updateApplicationAndDeploymentTargets();
 
-    ProjectExplorer::Target *m_connectedTarget = nullptr;
+    bool mustUpdateCMakeStateBeforeBuild();
 
     // TODO probably need a CMake specific node structure
     QList<CMakeBuildTarget> m_buildTargets;
@@ -134,10 +138,17 @@ private:
     QList<ProjectExplorer::ExtraCompiler *> m_extraCompilers;
 
     Internal::TreeScanner m_treeScanner;
+
+    bool m_waitingForScan = false;
+    bool m_waitingForParse = false;
+    bool m_combinedScanAndParseResult = false;
+
     QHash<QString, bool> m_mimeBinaryCache;
     QList<const ProjectExplorer::FileNode *> m_allFiles;
     mutable std::unique_ptr<Internal::CMakeProjectImporter> m_projectImporter;
 
+    QTimer m_delayedParsingTimer;
+
     friend class Internal::CMakeBuildConfiguration;
     friend class Internal::CMakeBuildSettingsWidget;
 };

src/plugins/cmakeprojectmanager/cmakeprojectmanager.cpp

@@ -152,6 +152,5 @@ void CMakeManager::rescanProject(Project *project)
     if (!cmakeProject || !cmakeProject->activeTarget() || !cmakeProject->activeTarget()->activeBuildConfiguration())
         return;
 
-    cmakeProject->scanProjectTree();
+    cmakeProject->runCMakeAndScanProjectTree();// by my experience: every rescan run requires cmake run too
-    cmakeProject->runCMake(); // by my experience: every rescan run requires cmake run too
 }