From 65658f411bb303f56a589b7f0f1df179dd8eed99 Mon Sep 17 00:00:00 2001 From: Tobias Hunger Date: Fri, 7 Jun 2019 17:13:49 +0200 Subject: [PATCH] CMake: Unify error reporting for builddirmanager's information retrieval Use a dedicated errrorMessage out parameter for error reporting in the builddirmanager methods related to information retrieval. Those are called after the parsing has finished. This frees the errrorOccured signal of the builddirmanager to be used only when the parsing itself has failed. Change-Id: Ieefc32c0386769479177a6bd4bc4a0e77df5db7b Reviewed-by: Cristian Adam Reviewed-by: Tobias Hunger --- .../cmakeprojectmanager/builddirmanager.cpp | 24 ++++++++----- .../cmakeprojectmanager/builddirmanager.h | 9 ++--- .../cmakeprojectmanager/builddirreader.h | 9 ++--- .../cmakeprojectmanager/cmakeproject.cpp | 36 +++++++++++++++---- .../cmakeprojectmanager/cmakeproject.h | 3 ++ .../cmakeprojectmanager/servermodereader.cpp | 13 ++++--- .../cmakeprojectmanager/servermodereader.h | 9 ++--- .../cmakeprojectmanager/tealeafreader.cpp | 19 +++++----- .../cmakeprojectmanager/tealeafreader.h | 9 ++--- 9 files changed, 88 insertions(+), 43 deletions(-) diff --git a/src/plugins/cmakeprojectmanager/builddirmanager.cpp b/src/plugins/cmakeprojectmanager/builddirmanager.cpp index aff84521426..4c06e4fddde 100644 --- a/src/plugins/cmakeprojectmanager/builddirmanager.cpp +++ b/src/plugins/cmakeprojectmanager/builddirmanager.cpp @@ -138,7 +138,10 @@ bool BuildDirManager::hasConfigChanged() const QByteArrayList criticalKeys = {GENERATOR_KEY, CMAKE_COMMAND_KEY, CMAKE_C_COMPILER_KEY, CMAKE_CXX_COMPILER_KEY}; - const CMakeConfig currentConfig = takeCMakeConfiguration(); + QString errorMessage; + const CMakeConfig currentConfig = takeCMakeConfiguration(errorMessage); + if (!errorMessage.isEmpty()) + return false; const CMakeTool *tool = m_parameters.cmakeTool(); QTC_ASSERT(tool, return false); // No cmake... we should not have ended up here in the first place @@ -299,19 +302,21 @@ void BuildDirManager::parse(int reparseParameters) reparseParameters & REPARSE_FORCE_CONFIGURATION); } -void BuildDirManager::generateProjectTree(CMakeProjectNode *root, const QList &allFiles) const +void BuildDirManager::generateProjectTree(CMakeProjectNode *root, + const QList &allFiles, + QString &errorMessage) const { QTC_ASSERT(!m_isHandlingError, return); QTC_ASSERT(m_reader, return); - m_reader->generateProjectTree(root, allFiles); + m_reader->generateProjectTree(root, allFiles, errorMessage); } -CppTools::RawProjectParts BuildDirManager::createRawProjectParts() const +CppTools::RawProjectParts BuildDirManager::createRawProjectParts(QString &errorMessage) const { QTC_ASSERT(!m_isHandlingError, return {}); QTC_ASSERT(m_reader, return {}); - return m_reader->createRawProjectParts(); + return m_reader->createRawProjectParts(errorMessage); } void BuildDirManager::clearCache() @@ -344,7 +349,7 @@ static CMakeBuildTarget utilityTarget(const QString &title, const BuildDirManage return target; } -QList BuildDirManager::takeBuildTargets() const +QList BuildDirManager::takeBuildTargets(QString &errorMessage) const { QList result = { utilityTarget(CMakeBuildStep::allTarget(), this), utilityTarget(CMakeBuildStep::cleanTarget(), this), @@ -353,7 +358,8 @@ QList BuildDirManager::takeBuildTargets() const QTC_ASSERT(!m_isHandlingError, return result); if (m_reader) { - result.append(Utils::filtered(m_reader->takeBuildTargets(), [](const CMakeBuildTarget &bt) { + result.append(Utils::filtered(m_reader->takeBuildTargets(errorMessage), + [](const CMakeBuildTarget &bt) { return bt.title != CMakeBuildStep::allTarget() && bt.title != CMakeBuildStep::cleanTarget() && bt.title != CMakeBuildStep::installTarget() @@ -363,12 +369,12 @@ QList BuildDirManager::takeBuildTargets() const return result; } -CMakeConfig BuildDirManager::takeCMakeConfiguration() const +CMakeConfig BuildDirManager::takeCMakeConfiguration(QString &errorMessage) const { if (!m_reader) return CMakeConfig(); - CMakeConfig result = m_reader->takeParsedConfiguration(); + CMakeConfig result = m_reader->takeParsedConfiguration(errorMessage); for (auto &ci : result) ci.inCMakeCache = true; diff --git a/src/plugins/cmakeprojectmanager/builddirmanager.h b/src/plugins/cmakeprojectmanager/builddirmanager.h index 8c166fc0fdd..0b364c764a5 100644 --- a/src/plugins/cmakeprojectmanager/builddirmanager.h +++ b/src/plugins/cmakeprojectmanager/builddirmanager.h @@ -79,11 +79,12 @@ public: void parse(int reparseParameters); void generateProjectTree(CMakeProjectNode *root, - const QList &allFiles) const; - CppTools::RawProjectParts createRawProjectParts() const; + const QList &allFiles, + QString &errorMessage) const; + CppTools::RawProjectParts createRawProjectParts(QString &errorMessage) const; - QList takeBuildTargets() const; - CMakeConfig takeCMakeConfiguration() const; + QList takeBuildTargets(QString &errorMessage) const; + CMakeConfig takeCMakeConfiguration(QString &errorMessage) const; static CMakeConfig parseCMakeConfiguration(const Utils::FilePath &cacheFile, QString *errorMessage); diff --git a/src/plugins/cmakeprojectmanager/builddirreader.h b/src/plugins/cmakeprojectmanager/builddirreader.h index 94e85ca1878..04f243df793 100644 --- a/src/plugins/cmakeprojectmanager/builddirreader.h +++ b/src/plugins/cmakeprojectmanager/builddirreader.h @@ -62,11 +62,12 @@ public: virtual bool isParsing() const = 0; - virtual QList takeBuildTargets() = 0; - virtual CMakeConfig takeParsedConfiguration() = 0; + virtual QList takeBuildTargets(QString &errorMessage) = 0; + virtual CMakeConfig takeParsedConfiguration(QString &errorMessage) = 0; virtual void generateProjectTree(CMakeProjectNode *root, - const QList &allFiles) = 0; - virtual CppTools::RawProjectParts createRawProjectParts() const = 0; + const QList &allFiles, + QString &errorMessage) = 0; + virtual CppTools::RawProjectParts createRawProjectParts(QString &errorMessage) const = 0; signals: void isReadyNow() const; diff --git a/src/plugins/cmakeprojectmanager/cmakeproject.cpp b/src/plugins/cmakeprojectmanager/cmakeproject.cpp index e85976093ba..c836d09ef26 100644 --- a/src/plugins/cmakeprojectmanager/cmakeproject.cpp +++ b/src/plugins/cmakeprojectmanager/cmakeproject.cpp @@ -109,12 +109,15 @@ CMakeProject::CMakeProject(const FilePath &fileName) : Project(Constants::CMAKEM }); connect(&m_buildDirManager, &BuildDirManager::errorOccured, this, [this](const QString &msg) { + reportError(msg); CMakeBuildConfiguration *bc = m_buildDirManager.buildConfiguration(); if (bc) { - bc->setError(msg); - bc->setConfigurationFromCMake(m_buildDirManager.takeCMakeConfiguration()); + QString errorMessage; + bc->setConfigurationFromCMake(m_buildDirManager.takeCMakeConfiguration(errorMessage)); + // ignore errorMessage here, we already got one. handleParsingError(bc); } + }); connect(&m_buildDirManager, &BuildDirManager::parsingStarted, this, [this]() { @@ -265,14 +268,17 @@ CMakeProject::~CMakeProject() void CMakeProject::updateProjectData(CMakeBuildConfiguration *bc) { const CMakeBuildConfiguration *aBc = activeBc(this); + QString errorMessage; QTC_ASSERT(bc, return); QTC_ASSERT(bc == aBc, return); QTC_ASSERT(m_treeScanner.isFinished() && !m_buildDirManager.isParsing(), return); - const QList buildTargets = m_buildDirManager.takeBuildTargets(); + const QList buildTargets = m_buildDirManager.takeBuildTargets(errorMessage); + checkAndReportError(errorMessage); bc->setBuildTargets(buildTargets); - const CMakeConfig cmakeConfig = m_buildDirManager.takeCMakeConfiguration(); + const CMakeConfig cmakeConfig = m_buildDirManager.takeCMakeConfiguration(errorMessage); + checkAndReportError(errorMessage); bc->setConfigurationFromCMake(cmakeConfig); CMakeConfig patchedConfig = cmakeConfig; @@ -336,7 +342,8 @@ void CMakeProject::updateProjectData(CMakeBuildConfiguration *bc) QtSupport::CppKitInfo kitInfo(this); QTC_ASSERT(kitInfo.isValid(), return); - CppTools::RawProjectParts rpps = m_buildDirManager.createRawProjectParts(); + CppTools::RawProjectParts rpps = m_buildDirManager.createRawProjectParts(errorMessage); + checkAndReportError(errorMessage); for (CppTools::RawProjectPart &rpp : rpps) { rpp.setQtVersion(kitInfo.projectPartQtVersion); // TODO: Check if project actually uses Qt. @@ -395,7 +402,9 @@ CMakeProject::generateProjectTree(const QList &allFiles) const return nullptr; auto root = std::make_unique(projectDirectory()); - m_buildDirManager.generateProjectTree(root.get(), allFiles); + QString errorMessage; + m_buildDirManager.generateProjectTree(root.get(), allFiles, errorMessage); + checkAndReportError(errorMessage); return root; } @@ -529,6 +538,13 @@ bool CMakeProject::setupTarget(Target *t) return true; } +void CMakeProject::reportError(const QString &errorMessage) const +{ + CMakeBuildConfiguration *bc = m_buildDirManager.buildConfiguration(); + if (bc) + bc->setError(errorMessage); +} + void CMakeProject::handleTreeScanningFinished() { QTC_CHECK(m_waitingForScan); @@ -644,6 +660,14 @@ bool CMakeProject::mustUpdateCMakeStateBeforeBuild() return m_delayedParsingTimer.isActive(); } +void CMakeProject::checkAndReportError(QString &errorMessage) const +{ + if (!errorMessage.isEmpty()) { + reportError(errorMessage); + errorMessage.clear(); + } +} + QList CMakeProject::findExtraCompilers() const { QList extraCompilers; diff --git a/src/plugins/cmakeprojectmanager/cmakeproject.h b/src/plugins/cmakeprojectmanager/cmakeproject.h index 3af409f1b64..109583fa161 100644 --- a/src/plugins/cmakeprojectmanager/cmakeproject.h +++ b/src/plugins/cmakeprojectmanager/cmakeproject.h @@ -81,6 +81,9 @@ public: void clearCMakeCache(); bool mustUpdateCMakeStateBeforeBuild(); + void checkAndReportError(QString &errorMessage) const; + void reportError(const QString &errorMessage) const; + protected: RestoreResult fromMap(const QVariantMap &map, QString *errorMessage) final; bool setupTarget(ProjectExplorer::Target *t) final; diff --git a/src/plugins/cmakeprojectmanager/servermodereader.cpp b/src/plugins/cmakeprojectmanager/servermodereader.cpp index 790663da326..f5a96dc77f2 100644 --- a/src/plugins/cmakeprojectmanager/servermodereader.cpp +++ b/src/plugins/cmakeprojectmanager/servermodereader.cpp @@ -189,8 +189,9 @@ bool ServerModeReader::isParsing() const return static_cast(m_future); } -QList ServerModeReader::takeBuildTargets() +QList ServerModeReader::takeBuildTargets(QString &errorMessage) { + Q_UNUSED(errorMessage) const QList result = transform(m_targets, [](const Target *t) -> CMakeBuildTarget { CMakeBuildTarget ct; ct.title = t->name; @@ -220,16 +221,19 @@ QList ServerModeReader::takeBuildTargets() return result; } -CMakeConfig ServerModeReader::takeParsedConfiguration() +CMakeConfig ServerModeReader::takeParsedConfiguration(QString &errorMessage) { + Q_UNUSED(errorMessage) CMakeConfig config = m_cmakeConfiguration; m_cmakeConfiguration.clear(); return config; } void ServerModeReader::generateProjectTree(CMakeProjectNode *root, - const QList &allFiles) + const QList &allFiles, + QString &errorMessage) { + Q_UNUSED(errorMessage) // Split up cmake inputs into useful chunks: std::vector> cmakeFilesSource; std::vector> cmakeFilesBuild; @@ -268,8 +272,9 @@ void ServerModeReader::generateProjectTree(CMakeProjectNode *root, std::move(cmakeFilesOther)); } -CppTools::RawProjectParts ServerModeReader::createRawProjectParts() const +CppTools::RawProjectParts ServerModeReader::createRawProjectParts(QString &errorMessage) const { + Q_UNUSED(errorMessage) CppTools::RawProjectParts rpps; int counter = 0; diff --git a/src/plugins/cmakeprojectmanager/servermodereader.h b/src/plugins/cmakeprojectmanager/servermodereader.h index 4d18a08fe74..a3b29fd871f 100644 --- a/src/plugins/cmakeprojectmanager/servermodereader.h +++ b/src/plugins/cmakeprojectmanager/servermodereader.h @@ -55,11 +55,12 @@ public: bool isParsing() const final; - QList takeBuildTargets() final; - CMakeConfig takeParsedConfiguration() final; + QList takeBuildTargets(QString &errorMessage) final; + CMakeConfig takeParsedConfiguration(QString &errorMessage) final; void generateProjectTree(CMakeProjectNode *root, - const QList &allFiles) final; - CppTools::RawProjectParts createRawProjectParts() const final; + const QList &allFiles, + QString &errorMessage) final; + CppTools::RawProjectParts createRawProjectParts(QString &errorMessage) const final; private: void createNewServer(); diff --git a/src/plugins/cmakeprojectmanager/tealeafreader.cpp b/src/plugins/cmakeprojectmanager/tealeafreader.cpp index 6f8c738dcb5..2ed39cfb741 100644 --- a/src/plugins/cmakeprojectmanager/tealeafreader.cpp +++ b/src/plugins/cmakeprojectmanager/tealeafreader.cpp @@ -204,23 +204,22 @@ bool TeaLeafReader::isParsing() const return m_cmakeProcess && m_cmakeProcess->state() != QProcess::NotRunning; } -QList TeaLeafReader::takeBuildTargets() +QList TeaLeafReader::takeBuildTargets(QString &errorMessage) { + Q_UNUSED(errorMessage) return m_buildTargets; } -CMakeConfig TeaLeafReader::takeParsedConfiguration() +CMakeConfig TeaLeafReader::takeParsedConfiguration(QString &errorMessage) { const FilePath cacheFile = m_parameters.workDirectory.pathAppended("CMakeCache.txt"); if (!cacheFile.exists()) return { }; - QString errorMessage; CMakeConfig result = BuildDirManager::parseCMakeConfiguration(cacheFile, &errorMessage); if (!errorMessage.isEmpty()) { - emit errorOccured(errorMessage); return { }; } @@ -229,16 +228,19 @@ CMakeConfig TeaLeafReader::takeParsedConfiguration() const FilePath canonicalSourceOfBuildDir = sourceOfBuildDir.canonicalPath(); const FilePath canonicalSourceDirectory = m_parameters.sourceDirectory.canonicalPath(); if (canonicalSourceOfBuildDir != canonicalSourceDirectory) { // Uses case-insensitive compare where appropriate - emit errorOccured(tr("The build directory is not for %1 but for %2") + errorMessage = tr("The build directory is not for %1 but for %2") .arg(canonicalSourceOfBuildDir.toUserOutput(), - canonicalSourceDirectory.toUserOutput())); + canonicalSourceDirectory.toUserOutput()); return { }; } return result; } -void TeaLeafReader::generateProjectTree(CMakeProjectNode *root, const QList &allFiles) +void TeaLeafReader::generateProjectTree(CMakeProjectNode *root, + const QList &allFiles, + QString &errorMessage) { + Q_UNUSED(errorMessage) if (m_files.size() == 0) return; @@ -317,8 +319,9 @@ static void processCMakeIncludes(const CMakeBuildTarget &cbt, const ToolChain *t } } -CppTools::RawProjectParts TeaLeafReader::createRawProjectParts() const +CppTools::RawProjectParts TeaLeafReader::createRawProjectParts(QString &errorMessage) const { + Q_UNUSED(errorMessage) const ToolChain *tcCxx = ToolChainManager::findToolChain(m_parameters.cxxToolChainId); const ToolChain *tcC = ToolChainManager::findToolChain(m_parameters.cToolChainId); const FilePath sysroot = m_parameters.sysRoot; diff --git a/src/plugins/cmakeprojectmanager/tealeafreader.h b/src/plugins/cmakeprojectmanager/tealeafreader.h index a305d0276a3..d1ad3026eb4 100644 --- a/src/plugins/cmakeprojectmanager/tealeafreader.h +++ b/src/plugins/cmakeprojectmanager/tealeafreader.h @@ -56,11 +56,12 @@ public: bool isParsing() const final; - QList takeBuildTargets() final; - CMakeConfig takeParsedConfiguration() final; + QList takeBuildTargets(QString &errorMessage) final; + CMakeConfig takeParsedConfiguration(QString &errorMessage) final; void generateProjectTree(CMakeProjectNode *root, - const QList &allFiles) final; - CppTools::RawProjectParts createRawProjectParts() const final; + const QList &allFiles, + QString &errorMessage) final; + CppTools::RawProjectParts createRawProjectParts(QString &errorMessage) const final; private: void extractData();