From 08677c0b014cc44d944e32d462f502a67c948404 Mon Sep 17 00:00:00 2001 From: Tobias Hunger Date: Fri, 28 Jul 2017 11:16:18 +0200 Subject: [PATCH] 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 --- .../cmakeprojectmanager/builddirmanager.cpp | 15 +- .../cmakeprojectmanager/builddirmanager.h | 4 +- .../cmakebuildconfiguration.cpp | 10 +- .../cmakebuildconfiguration.h | 3 + .../cmakeprojectmanager/cmakeproject.cpp | 197 +++++++++++------- .../cmakeprojectmanager/cmakeproject.h | 23 +- .../cmakeprojectmanager.cpp | 3 +- 7 files changed, 154 insertions(+), 101 deletions(-) diff --git a/src/plugins/cmakeprojectmanager/builddirmanager.cpp b/src/plugins/cmakeprojectmanager/builddirmanager.cpp index 67ae9387af0..b64898d2b4f 100644 --- a/src/plugins/cmakeprojectmanager/builddirmanager.cpp +++ b/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; } diff --git a/src/plugins/cmakeprojectmanager/builddirmanager.h b/src/plugins/cmakeprojectmanager/builddirmanager.h index 489ca67ff7f..4db167d0428 100644 --- a/src/plugins/cmakeprojectmanager/builddirmanager.h +++ b/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 m_tempDir = nullptr; mutable CMakeConfig m_cmakeCache; - QTimer m_reparseTimer; - std::unique_ptr m_reader; mutable QList m_buildTargets; diff --git a/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.cpp b/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.cpp index 59bdaf1168b..a62fa7f4af4 100644 --- a/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.cpp +++ b/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]() { - project->handleParsingStarted(this); + this, [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(project())->mustUpdateCMakeStateBeforeBuild(); } void CMakeBuildConfiguration::runCMake() diff --git a/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.h b/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.h index 5cc58bbdcb4..45a45ede73d 100644 --- a/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.h +++ b/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); diff --git a/src/plugins/cmakeprojectmanager/cmakeproject.cpp b/src/plugins/cmakeprojectmanager/cmakeproject.cpp index 57de95ad1fa..2ab0037bd0e 100644 --- a/src/plugins/cmakeprojectmanager/cmakeproject.cpp +++ b/src/plugins/cmakeprojectmanager/cmakeproject.cpp @@ -51,6 +51,7 @@ #include #include +#include #include #include #include @@ -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); - - Target *t = activeTarget(); - if (!t || t->activeBuildConfiguration() != bc) - return; - - if (!m_treeScanner.isFinished() || bc->isParsing()) - return; + QTC_ASSERT(bc == (t ? t->activeBuildConfiguration() : nullptr), return); + QTC_ASSERT(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 (activeTarget()) - bc = qobject_cast(activeTarget()->activeBuildConfiguration()); + if (isParsing()) + return; - if (bc) + startParsingProject(PARSE); +} + +void CMakeProject::runCMakeAndScanProjectTree() +{ + if (isParsing()) + return; + + startParsingProject(static_cast(PARSE | SCAN)); +} + +void CMakeProject::startParsingProject(const CMakeProject::DataCollectionAction action) +{ + const bool runParse = action & PARSE; + const bool runScan = action & SCAN; + + CMakeBuildConfiguration *bc = activeTarget() + ? qobject_cast(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(pc)) { + if (!bc->isActive()) + return; + } else if (!qobject_cast(pc)) { return; - m_treeScanner.asyncScanForFiles(projectDirectory()); - 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(activeTarget()->activeBuildConfiguration()); - - foreach (Target *t, targets()) { - foreach (BuildConfiguration *bc, t->buildConfigurations()) { + for (Target *t : targets()) { + for (BuildConfiguration *bc : t->buildConfigurations()) { auto i = qobject_cast(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(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(t->activeBuildConfiguration()); - if (!bc) + m_waitingForParse = false; + m_combinedScanAndParseResult = m_combinedScanAndParseResult && true; + + combineScanAndParse(bc); +} + +void CMakeProject::handleParsingError(CMakeBuildConfiguration *bc) +{ + QTC_CHECK(m_waitingForParse); + + if (!bc || !bc->isActive()) return; - updateProjectData(bc); + 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); diff --git a/src/plugins/cmakeprojectmanager/cmakeproject.h b/src/plugins/cmakeprojectmanager/cmakeproject.h index 1d3e047683f..2a6964fc8b6 100644 --- a/src/plugins/cmakeprojectmanager/cmakeproject.h +++ b/src/plugins/cmakeprojectmanager/cmakeproject.h @@ -37,6 +37,7 @@ #include #include +#include #include @@ -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 buildTargets() const; - void handleActiveTargetChanged(); - void handleActiveBuildConfigurationChanged(); - void handleParsingStarted(const Internal::CMakeBuildConfiguration *bc); + enum DataCollectionAction { PARSE = 1, SCAN = 2 }; + void startParsingProject(const DataCollectionAction a); + + 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 m_buildTargets; @@ -134,10 +138,17 @@ private: QList m_extraCompilers; Internal::TreeScanner m_treeScanner; + + bool m_waitingForScan = false; + bool m_waitingForParse = false; + bool m_combinedScanAndParseResult = false; + QHash m_mimeBinaryCache; QList m_allFiles; mutable std::unique_ptr m_projectImporter; + QTimer m_delayedParsingTimer; + friend class Internal::CMakeBuildConfiguration; friend class Internal::CMakeBuildSettingsWidget; }; diff --git a/src/plugins/cmakeprojectmanager/cmakeprojectmanager.cpp b/src/plugins/cmakeprojectmanager/cmakeprojectmanager.cpp index 7464261e8c3..420e0d0bd2d 100644 --- a/src/plugins/cmakeprojectmanager/cmakeprojectmanager.cpp +++ b/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->runCMake(); // by my experience: every rescan run requires cmake run too + cmakeProject->runCMakeAndScanProjectTree();// by my experience: every rescan run requires cmake run too }