CMake: Move error and warning handling

... from CMakeBuildConfiguration to CMakeBuildSystem.

Less back-and-forth this way.

Also, prefer plain buildConfiguration() over cmakeBuildConfiguration()
back-pointers.

Change-Id: Ie1341302ecc10e53d71ca68b7b6eb5f46cfdad5f
Reviewed-by: Cristian Adam <cristian.adam@qt.io>

src/plugins/cmakeprojectmanager/cmakebuildconfiguration.cpp

@@ -355,9 +355,11 @@ CMakeBuildSettingsWidget::CMakeBuildSettingsWidget(CMakeBuildConfiguration *bc)
         }.setSpacing(0)
     }.attachTo(details, false);
 
+    auto buildSystem = static_cast<CMakeBuildSystem *>(bc->buildSystem());
+
     updateAdvancedCheckBox();
-    setError(bc->error());
-    setWarning(bc->warning());
+    setError(buildSystem->error());
+    setWarning(buildSystem->warning());
 
     connect(bc->buildSystem(), &BuildSystem::parsingStarted, this, [this] {
         updateButtonState();
@@ -388,8 +390,7 @@ CMakeBuildSettingsWidget::CMakeBuildSettingsWidget(CMakeBuildConfiguration *bc)
         updateConfigurationStateSelection();
     });
 
-    auto cbc = static_cast<CMakeBuildSystem *>(bc->buildSystem());
-    connect(cbc, &CMakeBuildSystem::configurationCleared, this, [this]() {
+    connect(buildSystem, &CMakeBuildSystem::configurationCleared, this, [this]() {
         updateConfigurationStateSelection();
     });
 
@@ -1424,7 +1425,7 @@ void CMakeBuildConfiguration::setConfigurationChanges(const CMakeConfig &config)
 // FIXME: Run clean steps when a setting starting with "ANDROID_BUILD_ABI_" is changed.
 // FIXME: Warn when kit settings are overridden by a project.
 
-void CMakeBuildConfiguration::clearError(ForceEnabledChanged fec)
+void CMakeBuildSystem::clearError(ForceEnabledChanged fec)
 {
     if (!m_error.isEmpty()) {
         m_error.clear();
@@ -1432,7 +1433,7 @@ void CMakeBuildConfiguration::clearError(ForceEnabledChanged fec)
     }
     if (fec == ForceEnabledChanged::True) {
         qCDebug(cmakeBuildConfigurationLog) << "Emitting enabledChanged signal";
-        emit enabledChanged();
+        emit buildConfiguration()->enabledChanged();
     }
 }
 
@@ -1477,7 +1478,7 @@ void CMakeBuildConfiguration::filterConfigArgumentsFromAdditionalCMakeArguments(
     aspect<AdditionalCMakeOptionsAspect>()->setValue(ProcessArgs::joinArgs(unknownOptions));
 }
 
-void CMakeBuildConfiguration::setError(const QString &message)
+void CMakeBuildSystem::setError(const QString &message)
 {
     qCDebug(cmakeBuildConfigurationLog) << "Setting error to" << message;
     QTC_ASSERT(!message.isEmpty(), return );
@@ -1487,27 +1488,27 @@ void CMakeBuildConfiguration::setError(const QString &message)
         m_error = message;
     if (oldMessage.isEmpty() != !message.isEmpty()) {
         qCDebug(cmakeBuildConfigurationLog) << "Emitting enabledChanged signal";
-        emit enabledChanged();
+        emit buildConfiguration()->enabledChanged();
     }
     TaskHub::addTask(BuildSystemTask(Task::TaskType::Error, message));
-    emit errorOccurred(m_error);
+    emit cmakeBuildConfiguration()->errorOccurred(m_error);
 }
 
-void CMakeBuildConfiguration::setWarning(const QString &message)
+void CMakeBuildSystem::setWarning(const QString &message)
 {
     if (m_warning == message)
         return;
     m_warning = message;
     TaskHub::addTask(BuildSystemTask(Task::TaskType::Warning, message));
-    emit warningOccurred(m_warning);
+    emit cmakeBuildConfiguration()->warningOccurred(m_warning);
 }
 
-QString CMakeBuildConfiguration::error() const
+QString CMakeBuildSystem::error() const
 {
     return m_error;
 }
 
-QString CMakeBuildConfiguration::warning() const
+QString CMakeBuildSystem::warning() const
 {
     return m_warning;
 }

src/plugins/cmakeprojectmanager/cmakebuildconfiguration.h

@@ -59,9 +59,6 @@ public:
     QStringList initialCMakeArguments() const;
     CMakeConfig initialCMakeConfiguration() const;
 
-    QString error() const;
-    QString warning() const;
-
     static Utils::FilePath
     shadowBuildDirectory(const Utils::FilePath &projectFilePath, const ProjectExplorer::Kit *k,
                          const QString &bcName, BuildConfiguration::BuildType buildType);
@@ -100,20 +97,11 @@ private:
 
     virtual CMakeConfig signingFlags() const;
 
-    enum ForceEnabledChanged { False, True };
-    void clearError(ForceEnabledChanged fec = ForceEnabledChanged::False);
-
     void setConfigurationFromCMake(const CMakeConfig &config);
     void setConfigurationChanges(const CMakeConfig &config);
 
     void setInitialCMakeArguments(const QStringList &args);
 
-    void setError(const QString &message);
-    void setWarning(const QString &message);
-
-    QString m_error;
-    QString m_warning;
-
     CMakeConfig m_configurationFromCMake;
     CMakeConfig m_configurationChanges;
     Internal::CMakeBuildSystem *m_buildSystem = nullptr;

src/plugins/cmakeprojectmanager/cmakebuildsystem.cpp

@@ -188,8 +188,8 @@ CMakeBuildSystem::CMakeBuildSystem(CMakeBuildConfiguration *bc)
         return type;
     });
 
-    connect(&m_reader, &FileApiReader::configurationStarted, this, [this]() {
-        cmakeBuildConfiguration()->clearError(CMakeBuildConfiguration::ForceEnabledChanged::True);
+    connect(&m_reader, &FileApiReader::configurationStarted, this, [this] {
+        clearError(ForceEnabledChanged::True);
     });
 
     connect(&m_reader,
@@ -217,9 +217,9 @@ CMakeBuildSystem::~CMakeBuildSystem()
 
 void CMakeBuildSystem::triggerParsing()
 {
-    qCDebug(cmakeBuildSystemLog) << cmakeBuildConfiguration()->displayName() << "Parsing has been triggered";
+    qCDebug(cmakeBuildSystemLog) << buildConfiguration()->displayName() << "Parsing has been triggered";
 
-    if (!cmakeBuildConfiguration()->isActive()) {
+    if (!buildConfiguration()->isActive()) {
         qCDebug(cmakeBuildSystemLog)
             << "Parsing has been triggered: SKIPPING since BC is not active -- clearing state.";
         stopParsingAndClearState();
@@ -317,8 +317,7 @@ FilePaths CMakeBuildSystem::filesGeneratedFrom(const FilePath &sourceFile) const
     }
 
     const FilePath relativePath = baseDirectory.relativePath(project);
-    FilePath generatedFilePath = cmakeBuildConfiguration()->buildDirectory().resolvePath(
-        relativePath);
+    FilePath generatedFilePath = buildConfiguration()->buildDirectory().resolvePath(relativePath);
 
     if (sourceFile.suffix() == "ui") {
         generatedFilePath = generatedFilePath
@@ -356,11 +355,11 @@ void CMakeBuildSystem::setParametersAndRequestParse(const BuildDirParameters &pa
 {
     project()->clearIssues();
 
-    qCDebug(cmakeBuildSystemLog) << cmakeBuildConfiguration()->displayName()
+    qCDebug(cmakeBuildSystemLog) << buildConfiguration()->displayName()
                                  << "setting parameters and requesting reparse"
                                  << reparseParametersString(reparseParameters);
 
-    if (!cmakeBuildConfiguration()->isActive()) {
+    if (!buildConfiguration()->isActive()) {
         qCDebug(cmakeBuildSystemLog) << "setting parameters and requesting reparse: SKIPPING since BC is not active -- clearing state.";
         stopParsingAndClearState();
         return; // ignore request, this build configuration is not active!
@@ -504,7 +503,7 @@ void CMakeBuildSystem::clearCMakeCache()
 
 void CMakeBuildSystem::combineScanAndParse(bool restoredFromBackup)
 {
-    if (cmakeBuildConfiguration()->isActive()) {
+    if (buildConfiguration()->isActive()) {
         if (m_waitingForParse)
             return;
 
@@ -543,7 +542,7 @@ void CMakeBuildSystem::combineScanAndParse(bool restoredFromBackup)
 void CMakeBuildSystem::checkAndReportError(QString &errorMessage)
 {
     if (!errorMessage.isEmpty()) {
-        cmakeBuildConfiguration()->setError(errorMessage);
+        setError(errorMessage);
         errorMessage.clear();
     }
 }
@@ -554,7 +553,7 @@ void CMakeBuildSystem::updateProjectData()
 
     QTC_ASSERT(m_treeScanner.isFinished() && !m_reader.isParsing(), return );
 
-    cmakeBuildConfiguration()->project()->setExtraProjectFiles(m_reader.projectFilesToWatch());
+    buildConfiguration()->project()->setExtraProjectFiles(m_reader.projectFilesToWatch());
 
     CMakeConfig patchedConfig = cmakeBuildConfiguration()->configurationFromCMake();
     {
@@ -621,7 +620,7 @@ void CMakeBuildSystem::updateProjectData()
     QString errorMessage;
     RawProjectParts rpps = m_reader.createRawProjectParts(errorMessage);
     if (!errorMessage.isEmpty())
-        cmakeBuildConfiguration()->setError(errorMessage);
+        setError(errorMessage);
     qCDebug(cmakeBuildSystemLog) << "Raw project parts created." << errorMessage;
 
     {
@@ -639,7 +638,7 @@ void CMakeBuildSystem::updateProjectData()
             }
         }
 
-        m_cppCodeModelUpdater->update({p, kitInfo, cmakeBuildConfiguration()->environment(), rpps},
+        m_cppCodeModelUpdater->update({p, kitInfo, buildConfiguration()->environment(), rpps},
                                       m_extraCompilers);
     }
     {
@@ -648,7 +647,7 @@ void CMakeBuildSystem::updateProjectData()
         QStringList extraHeaderPaths;
         QList<QByteArray> moduleMappings;
         for (const RawProjectPart &rpp : qAsConst(rpps)) {
-            FilePath moduleMapFile = cmakeBuildConfiguration()->buildDirectory()
+            FilePath moduleMapFile = buildConfiguration()->buildDirectory()
                     .pathAppended("qml_module_mappings/" + rpp.buildSystemTarget);
             if (moduleMapFile.exists()) {
                 QFile mmf(moduleMapFile.toString());
@@ -673,7 +672,7 @@ void CMakeBuildSystem::updateProjectData()
     }
     updateInitialCMakeExpandableVars();
 
-    emit cmakeBuildConfiguration()->buildTypeChanged();
+    emit buildConfiguration()->buildTypeChanged();
 
     qCDebug(cmakeBuildSystemLog) << "All CMake project data up to date.";
 }
@@ -747,12 +746,12 @@ void CMakeBuildSystem::updateCMakeConfiguration(QString &errorMessage)
 
 void CMakeBuildSystem::handleParsingSucceeded(bool restoredFromBackup)
 {
-    if (!cmakeBuildConfiguration()->isActive()) {
+    if (!buildConfiguration()->isActive()) {
         stopParsingAndClearState();
         return;
     }
 
-    cmakeBuildConfiguration()->clearError();
+    clearError();
 
     QString errorMessage;
     {
@@ -785,7 +784,7 @@ void CMakeBuildSystem::handleParsingSucceeded(bool restoredFromBackup)
 
 void CMakeBuildSystem::handleParsingFailed(const QString &msg)
 {
-    cmakeBuildConfiguration()->setError(msg);
+    setError(msg);
 
     QString errorMessage;
     updateCMakeConfiguration(errorMessage);
@@ -875,7 +874,7 @@ FilePath CMakeBuildSystem::buildDirectory(const BuildDirParameters &parameters)
 {
     const FilePath bdir = parameters.buildDirectory;
 
-    if (!cmakeBuildConfiguration()->createBuildDirectory())
+    if (!buildConfiguration()->createBuildDirectory())
         handleParsingFailed(
             tr("Failed to create build directory \"%1\".").arg(bdir.toUserOutput()));
 
@@ -884,7 +883,7 @@ FilePath CMakeBuildSystem::buildDirectory(const BuildDirParameters &parameters)
 
 void CMakeBuildSystem::stopParsingAndClearState()
 {
-    qCDebug(cmakeBuildSystemLog) << cmakeBuildConfiguration()->displayName()
+    qCDebug(cmakeBuildSystemLog) << buildConfiguration()->displayName()
                                  << "stopping parsing run!";
     m_reader.stop();
     m_reader.resetData();
@@ -913,7 +912,7 @@ int CMakeBuildSystem::takeReparseParameters()
 
 void CMakeBuildSystem::runCTest()
 {
-    if (!cmakeBuildConfiguration()->error().isEmpty() || m_ctestPath.isEmpty()) {
+    if (!m_error.isEmpty() || m_ctestPath.isEmpty()) {
         qCDebug(cmakeBuildSystemLog) << "Cancel ctest run after failed cmake run";
         emit testInformationUpdated();
         return;
@@ -1110,7 +1109,7 @@ DeploymentData CMakeBuildSystem::deploymentData() const
     DeploymentData result;
 
     QDir sourceDir = project()->projectDirectory().toString();
-    QDir buildDir = cmakeBuildConfiguration()->buildDirectory().toString();
+    QDir buildDir = buildConfiguration()->buildDirectory().toString();
 
     QString deploymentPrefix;
     QString deploymentFilePath = sourceDir.filePath("QtCreatorDeployment.txt");

src/plugins/cmakeprojectmanager/cmakebuildsystem.h

@@ -108,10 +108,19 @@ public:
 
     CMakeProject *project() const;
 
+    QString error() const;
+    QString warning() const;
+
 signals:
     void configurationCleared();
 
 private:
+    enum ForceEnabledChanged { False, True };
+    void clearError(ForceEnabledChanged fec = ForceEnabledChanged::False);
+
+    void setError(const QString &message);
+    void setWarning(const QString &message);
+
     // Actually ask for parsing:
     enum ReparseParameters {
         REPARSE_DEFAULT = 0, // Nothing special:-)
@@ -188,6 +197,9 @@ private:
     Utils::FilePath m_ctestPath;
     QList<ProjectExplorer::TestCaseInfo> m_testNames;
     Utils::FutureSynchronizer m_futureSynchronizer;
+
+    QString m_error;
+    QString m_warning;
 };
 
 } // namespace Internal