CMake: Make sure a CMakeBuildConfiguration always has a CMakeBuildSystem

For this to work, we need to make sure no parsing is triggered before
the project is fully set up. Otherwise it would be QTCREATORBUG-23816
again...

Change-Id: If81f4c6b9c82283abdaa8a635f93ebe0bcaf8159
Reviewed-by: Cristian Adam <[email protected]>

3 files changed, 118 insertions, 102 deletions

diff --git a/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.cpp b/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.cpp
index 18e62ec9366..37575ef3246 100644
--- a/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.cpp
+++ b/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.cpp
@@ -63,6 +63,8 @@ const char CONFIGURATION_KEY[] = "CMake.Configuration";
 CMakeBuildConfiguration::CMakeBuildConfiguration(Target *target, Core::Id id)
     : BuildConfiguration(target, id)
 {
+    m_buildSystem = new CMakeBuildSystem(this);
+
     setBuildDirectory(shadowBuildDirectory(project()->projectFilePath(),
                                            target->kit(),
                                            displayName(),
@@ -149,21 +151,12 @@ CMakeBuildConfiguration::CMakeBuildConfiguration(Target *target, Core::Id id)
         }
 
         setConfigurationForCMake(config);
-
-        // Only do this after everything has been set up!
-        m_buildSystem = new CMakeBuildSystem(this);
     });
 
     const auto qmlDebuggingAspect = addAspect<QtSupport::QmlDebuggingAspect>();
     qmlDebuggingAspect->setKit(target->kit());
     connect(qmlDebuggingAspect, &QtSupport::QmlDebuggingAspect::changed,
             this, &CMakeBuildConfiguration::configurationForCMakeChanged);
-
-    // m_buildSystem is still nullptr here since it the build directory to be available
-    // before it can get created.
-    //
-    // This means this needs to be done in the lambda for the setInitializer(...) call
-    // defined above as well as in fromMap!
 }
 
 CMakeBuildConfiguration::~CMakeBuildConfiguration()
@@ -182,8 +175,6 @@ QVariantMap CMakeBuildConfiguration::toMap() const
 
 bool CMakeBuildConfiguration::fromMap(const QVariantMap &map)
 {
-    QTC_CHECK(!m_buildSystem);
-
     if (!BuildConfiguration::fromMap(map))
         return false;
 
@@ -194,8 +185,6 @@ bool CMakeBuildConfiguration::fromMap(const QVariantMap &map)
 
     setConfigurationForCMake(conf);
 
-    m_buildSystem = new CMakeBuildSystem(this);
-
     return true;
 }
 
diff --git a/src/plugins/cmakeprojectmanager/cmakebuildsystem.cpp b/src/plugins/cmakeprojectmanager/cmakebuildsystem.cpp
index b0d962d72cb..9b6158b9b65 100644
--- a/src/plugins/cmakeprojectmanager/cmakebuildsystem.cpp
+++ b/src/plugins/cmakeprojectmanager/cmakebuildsystem.cpp
@@ -40,6 +40,7 @@
 #include <cpptools/generatedcodemodelsupport.h>
 #include <projectexplorer/kitinformation.h>
 #include <projectexplorer/projectexplorerconstants.h>
+#include <projectexplorer/session.h>
 #include <projectexplorer/target.h>
 #include <qmljs/qmljsmodelmanagerinterface.h>
 
@@ -196,95 +197,10 @@ CMakeBuildSystem::CMakeBuildSystem(CMakeBuildConfiguration *bc)
         cmakeBuildConfiguration()->clearError(CMakeBuildConfiguration::ForceEnabledChanged::True);
     });
 
-    // Kit changed:
-    connect(KitManager::instance(), &KitManager::kitUpdated, this, [this](Kit *k) {
-        if (k != kit())
-            return; // not for us...
-        // Build configuration has not changed, but Kit settings might have:
-        // reparse and check the configuration, independent of whether the reader has changed
-        qCDebug(cmakeBuildSystemLog) << "Requesting parse due to kit being updated";
-        m_buildDirManager.setParametersAndRequestParse(BuildDirParameters(cmakeBuildConfiguration()),
-                                                       BuildDirManager::REPARSE_CHECK_CONFIGURATION);
-    });
-
-    // Became active/inactive:
-    connect(project(), &Project::activeTargetChanged, this, [this](Target *t) {
-        if (t == target()) {
-            // Build configuration has switched:
-            // * Check configuration if reader changes due to it not existing yet:-)
-            // * run cmake without configuration arguments if the reader stays
-            qCDebug(cmakeBuildSystemLog) << "Requesting parse due to active target changed";
-            m_buildDirManager
-                .setParametersAndRequestParse(BuildDirParameters(cmakeBuildConfiguration()),
-                                              BuildDirManager::REPARSE_CHECK_CONFIGURATION);
-        } else {
-            m_buildDirManager.stopParsingAndClearState();
-        }
-    });
-    connect(target(), &Target::activeBuildConfigurationChanged, this, [this](BuildConfiguration *bc) {
-        if (cmakeBuildConfiguration()->isActive()) {
-            if (cmakeBuildConfiguration() == bc) {
-                // Build configuration has switched:
-                // * Check configuration if reader changes due to it not existing yet:-)
-                // * run cmake without configuration arguments if the reader stays
-                qCDebug(cmakeBuildSystemLog) << "Requesting parse due to active BC changed";
-                m_buildDirManager
-                    .setParametersAndRequestParse(BuildDirParameters(cmakeBuildConfiguration()),
-                                                  BuildDirManager::REPARSE_CHECK_CONFIGURATION);
-            } else {
-                m_buildDirManager.stopParsingAndClearState();
-            }
-        }
-    });
-
-    // BuildConfiguration changed:
-    connect(cmakeBuildConfiguration(), &CMakeBuildConfiguration::environmentChanged, this, [this]() {
-        if (cmakeBuildConfiguration()->isActive()) {
-            // The environment on our BC has changed:
-            // * Error out if the reader updates, cannot happen since all BCs share a target/kit.
-            // * run cmake without configuration arguments if the reader stays
-            qCDebug(cmakeBuildSystemLog) << "Requesting parse due to environment change";
-            m_buildDirManager
-                .setParametersAndRequestParse(BuildDirParameters(cmakeBuildConfiguration()),
-                                              BuildDirManager::REPARSE_CHECK_CONFIGURATION);
-        }
-    });
-    connect(cmakeBuildConfiguration(), &CMakeBuildConfiguration::buildDirectoryChanged, this, [this]() {
-        if (cmakeBuildConfiguration()->isActive()) {
-            // The build directory of our BC has changed:
-            // * Error out if the reader updates, cannot happen since all BCs share a target/kit.
-            // * run cmake without configuration arguments if the reader stays
-            //   If no configuration exists, then the arguments will get added automatically by
-            //   the reader.
-            qCDebug(cmakeBuildSystemLog) << "Requesting parse due to build directory change";
-            m_buildDirManager
-                .setParametersAndRequestParse(BuildDirParameters(cmakeBuildConfiguration()),
-                                              BuildDirManager::REPARSE_CHECK_CONFIGURATION);
-        }
-    });
-    connect(cmakeBuildConfiguration(), &CMakeBuildConfiguration::configurationForCMakeChanged, this, [this]() {
-        if (cmakeBuildConfiguration()->isActive()) {
-            // The CMake configuration has changed on our BC:
-            // * Error out if the reader updates, cannot happen since all BCs share a target/kit.
-            // * run cmake with configuration arguments if the reader stays
-            qCDebug(cmakeBuildSystemLog) << "Requesting parse due to cmake configuration change";
-            m_buildDirManager
-                .setParametersAndRequestParse(BuildDirParameters(cmakeBuildConfiguration()),
-                                              BuildDirManager::REPARSE_FORCE_CONFIGURATION);
-        }
-    });
-
-    connect(project(), &Project::projectFileIsDirty, this, [this]() {
-        if (cmakeBuildConfiguration()->isActive() && !isParsing()) {
-            const auto cmake = CMakeKitAspect::cmakeTool(cmakeBuildConfiguration()->target()->kit());
-            if (cmake && cmake->isAutoRun()) {
-                qCDebug(cmakeBuildSystemLog) << "Requesting parse due to dirty project file";
-                m_buildDirManager.setParametersAndRequestParse(BuildDirParameters(
-                                                                   cmakeBuildConfiguration()),
-                                                               BuildDirManager::REPARSE_DEFAULT);
-            }
-        }
-    });
+    connect(SessionManager::instance(),
+            &SessionManager::projectAdded,
+            this,
+            &CMakeBuildSystem::wireUpConnections);
 }
 
 CMakeBuildSystem::~CMakeBuildSystem()
@@ -638,6 +554,115 @@ void CMakeBuildSystem::handleParsingFailed(const QString &msg)
     handleParsingError();
 }
 
+void CMakeBuildSystem::wireUpConnections(const Project *p)
+{
+    if (p != project())
+        return; // That's not us...
+
+    disconnect(SessionManager::instance(), nullptr, this, nullptr);
+
+    // At this point the entire project will be fully configured, so let's connect everything and
+    // trigger an initial parser run
+
+    // Kit changed:
+    connect(KitManager::instance(), &KitManager::kitUpdated, this, [this](Kit *k) {
+        if (k != kit())
+            return; // not for us...
+        // Build configuration has not changed, but Kit settings might have:
+        // reparse and check the configuration, independent of whether the reader has changed
+        qCDebug(cmakeBuildSystemLog) << "Requesting parse due to kit being updated";
+        m_buildDirManager.setParametersAndRequestParse(BuildDirParameters(cmakeBuildConfiguration()),
+                                                       BuildDirManager::REPARSE_CHECK_CONFIGURATION);
+    });
+
+    // Became active/inactive:
+    connect(project(), &Project::activeTargetChanged, this, [this](Target *t) {
+        if (t == target()) {
+            // Build configuration has switched:
+            // * Check configuration if reader changes due to it not existing yet:-)
+            // * run cmake without configuration arguments if the reader stays
+            qCDebug(cmakeBuildSystemLog) << "Requesting parse due to active target changed";
+            m_buildDirManager
+                .setParametersAndRequestParse(BuildDirParameters(cmakeBuildConfiguration()),
+                                              BuildDirManager::REPARSE_CHECK_CONFIGURATION);
+        } else {
+            m_buildDirManager.stopParsingAndClearState();
+        }
+    });
+    connect(target(), &Target::activeBuildConfigurationChanged, this, [this](BuildConfiguration *bc) {
+        if (cmakeBuildConfiguration()->isActive()) {
+            if (cmakeBuildConfiguration() == bc) {
+                // Build configuration has switched:
+                // * Check configuration if reader changes due to it not existing yet:-)
+                // * run cmake without configuration arguments if the reader stays
+                qCDebug(cmakeBuildSystemLog) << "Requesting parse due to active BC changed";
+                m_buildDirManager
+                    .setParametersAndRequestParse(BuildDirParameters(cmakeBuildConfiguration()),
+                                                  BuildDirManager::REPARSE_CHECK_CONFIGURATION);
+            } else {
+                m_buildDirManager.stopParsingAndClearState();
+            }
+        }
+    });
+
+    // BuildConfiguration changed:
+    connect(cmakeBuildConfiguration(), &CMakeBuildConfiguration::environmentChanged, this, [this]() {
+        if (cmakeBuildConfiguration()->isActive()) {
+            // The environment on our BC has changed:
+            // * Error out if the reader updates, cannot happen since all BCs share a target/kit.
+            // * run cmake without configuration arguments if the reader stays
+            qCDebug(cmakeBuildSystemLog) << "Requesting parse due to environment change";
+            m_buildDirManager
+                .setParametersAndRequestParse(BuildDirParameters(cmakeBuildConfiguration()),
+                                              BuildDirManager::REPARSE_CHECK_CONFIGURATION);
+        }
+    });
+    connect(cmakeBuildConfiguration(), &CMakeBuildConfiguration::buildDirectoryChanged, this, [this]() {
+        if (cmakeBuildConfiguration()->isActive()) {
+            // The build directory of our BC has changed:
+            // * Error out if the reader updates, cannot happen since all BCs share a target/kit.
+            // * run cmake without configuration arguments if the reader stays
+            //   If no configuration exists, then the arguments will get added automatically by
+            //   the reader.
+            qCDebug(cmakeBuildSystemLog) << "Requesting parse due to build directory change";
+            m_buildDirManager
+                .setParametersAndRequestParse(BuildDirParameters(cmakeBuildConfiguration()),
+                                              BuildDirManager::REPARSE_CHECK_CONFIGURATION);
+        }
+    });
+    connect(cmakeBuildConfiguration(),
+            &CMakeBuildConfiguration::configurationForCMakeChanged,
+            this,
+            [this]() {
+                if (cmakeBuildConfiguration()->isActive()) {
+                    // The CMake configuration has changed on our BC:
+                    // * Error out if the reader updates, cannot happen since all BCs share a target/kit.
+                    // * run cmake with configuration arguments if the reader stays
+                    qCDebug(cmakeBuildSystemLog)
+                        << "Requesting parse due to cmake configuration change";
+                    m_buildDirManager
+                        .setParametersAndRequestParse(BuildDirParameters(cmakeBuildConfiguration()),
+                                                      BuildDirManager::REPARSE_FORCE_CONFIGURATION);
+                }
+            });
+
+    connect(project(), &Project::projectFileIsDirty, this, [this]() {
+        if (cmakeBuildConfiguration()->isActive() && !isParsing()) {
+            const auto cmake = CMakeKitAspect::cmakeTool(cmakeBuildConfiguration()->target()->kit());
+            if (cmake && cmake->isAutoRun()) {
+                qCDebug(cmakeBuildSystemLog) << "Requesting parse due to dirty project file";
+                m_buildDirManager.setParametersAndRequestParse(BuildDirParameters(
+                                                                   cmakeBuildConfiguration()),
+                                                               BuildDirManager::REPARSE_DEFAULT);
+            }
+        }
+    });
+
+    // Force initial parsing run:
+    m_buildDirManager.setParametersAndRequestParse(BuildDirParameters(cmakeBuildConfiguration()),
+                                                   BuildDirManager::REPARSE_CHECK_CONFIGURATION);
+}
+
 CMakeBuildConfiguration *CMakeBuildSystem::cmakeBuildConfiguration() const
 {
     return static_cast<CMakeBuildConfiguration *>(BuildSystem::buildConfiguration());
diff --git a/src/plugins/cmakeprojectmanager/cmakebuildsystem.h b/src/plugins/cmakeprojectmanager/cmakebuildsystem.h
index d3a79ee2aba..796d91e565c 100644
--- a/src/plugins/cmakeprojectmanager/cmakebuildsystem.h
+++ b/src/plugins/cmakeprojectmanager/cmakebuildsystem.h
@@ -101,6 +101,8 @@ private:
     void handleParsingSucceeded();
     void handleParsingFailed(const QString &msg);
 
+    void wireUpConnections(const ProjectExplorer::Project *p);
+
     BuildDirManager m_buildDirManager;
 
     ProjectExplorer::TreeScanner m_treeScanner;