CMakeProjectManager: Make backup of CMake configuration before starting CMake

CMake's fileapi functionality will save the project structure in json
files in the .cmake/api/v1/reply directory.

When issuing a cmake command with -D variables CMake will update its
CMakeCache.txt file even if cmake will fail.

This commit will rename .cmake/api/v1/reply as .cmake/api/v1/reply.prev
and make a copy of CMakeCache.txt before starting CMake, and if
something fails, replace the existing files with the previous values.

Also make sure the changed values are not dissappearing when the
old .cmake/api/v1/reply gets parsed.

Fixes: QTCREATORBUG-24593
Change-Id: I82141786fea7068699e0f761a8978ba1f3203e47
Reviewed-by: Eike Ziller <eike.ziller@qt.io>

src/plugins/cmakeprojectmanager/builddirparameters.cpp

@@ -59,7 +59,7 @@ BuildDirParameters::BuildDirParameters(CMakeBuildConfiguration *bc)
                                                            });
     initialCMakeArguments = Utils::filtered(expandedArguments,
                                             [](const QString &s) { return !s.isEmpty(); });
-    extraCMakeArguments = Utils::transform(bc->extraCMakeArguments(),
+    extraCMakeArguments = Utils::transform(bc->configurationChangesArguments(),
                                              [expander](const QString &s) {
                                                  return expander->expand(s);
                                              });

src/plugins/cmakeprojectmanager/cmakebuildconfiguration.cpp

@@ -470,8 +470,7 @@ void CMakeBuildSettingsWidget::updateButtonState()
     m_resetButton->setEnabled(m_configModel->hasChanges() && !isParsing);
     m_reconfigureButton->setEnabled((!configChanges.isEmpty() || m_configModel->hasCMakeChanges())
                                     && !isParsing);
-    m_buildConfiguration->setExtraCMakeArguments(
+    m_buildConfiguration->setConfigurationChanges(configChanges);
-        Utils::transform(configChanges, [](const CMakeConfigItem &i) { return i.toArgument(); }));
 }
 
 void CMakeBuildSettingsWidget::updateAdvancedCheckBox()
@@ -943,9 +942,14 @@ CMakeConfig CMakeBuildConfiguration::configurationFromCMake() const
     return m_configurationFromCMake;
 }
 
-QStringList CMakeBuildConfiguration::extraCMakeArguments() const
+CMakeConfig CMakeBuildConfiguration::configurationChanges() const
 {
-    return m_extraCMakeArguments;
+    return m_configurationChanges;
+}
+
+QStringList CMakeBuildConfiguration::configurationChangesArguments() const
+{
+    return Utils::transform(m_configurationChanges, [](const CMakeConfigItem &i) { return i.toArgument(); });
 }
 
 QStringList CMakeBuildConfiguration::initialCMakeArguments() const
@@ -953,21 +957,22 @@ QStringList CMakeBuildConfiguration::initialCMakeArguments() const
     return aspect<InitialCMakeArgumentsAspect>()->value().split('\n', Qt::SkipEmptyParts);
 }
 
-void CMakeBuildConfiguration::setExtraCMakeArguments(const QStringList &args)
-{
-    if (m_extraCMakeArguments == args)
-        return;
-
-    qCDebug(cmakeBuildConfigurationLog)
-        << "Extra Args changed from" << m_extraCMakeArguments << "to" << args << "...";
-    m_extraCMakeArguments = args;
-}
-
 void CMakeBuildConfiguration::setConfigurationFromCMake(const CMakeConfig &config)
 {
     m_configurationFromCMake = config;
 }
 
+void CMakeBuildConfiguration::setConfigurationChanges(const CMakeConfig &config)
+{
+    qCDebug(cmakeBuildConfigurationLog)
+        << "Configuration changes before:" << configurationChangesArguments();
+
+    m_configurationChanges = config;
+
+    qCDebug(cmakeBuildConfigurationLog)
+        << "Configuration changes after:" << configurationChangesArguments();
+}
+
 // FIXME: Run clean steps when a setting starting with "ANDROID_BUILD_ABI_" is changed.
 // FIXME: Warn when kit settings are overridden by a project.
 

src/plugins/cmakeprojectmanager/cmakebuildconfiguration.h

@@ -52,8 +52,9 @@ public:
     ~CMakeBuildConfiguration() override;
 
     CMakeConfig configurationFromCMake() const;
+    CMakeConfig configurationChanges() const;
 
-    QStringList extraCMakeArguments() const;
+    QStringList configurationChangesArguments() const;
 
     QStringList initialCMakeArguments() const;
 
@@ -96,8 +97,8 @@ private:
     void clearError(ForceEnabledChanged fec = ForceEnabledChanged::False);
 
     void setConfigurationFromCMake(const CMakeConfig &config);
+    void setConfigurationChanges(const CMakeConfig &config);
 
-    void setExtraCMakeArguments(const QStringList &args);
     void setInitialCMakeArguments(const QStringList &args);
 
     void setError(const QString &message);
@@ -108,6 +109,7 @@ private:
     QString m_warning;
 
     CMakeConfig m_configurationFromCMake;
+    CMakeConfig m_configurationChanges;
     Internal::CMakeBuildSystem *m_buildSystem = nullptr;
 
     QStringList m_extraCMakeArguments;

src/plugins/cmakeprojectmanager/cmakebuildsystem.cpp

@@ -676,6 +676,24 @@ void CMakeBuildSystem::updateFallbackProjectData()
     qCDebug(cmakeBuildSystemLog) << "All fallback CMake project data up to date.";
 }
 
+void CMakeBuildSystem::updateCMakeConfiguration(QString &errorMessage)
+{
+    CMakeConfig cmakeConfig = m_reader.takeParsedConfiguration(errorMessage);
+    for (auto &ci : cmakeConfig)
+        ci.inCMakeCache = true;
+    if (!errorMessage.isEmpty()) {
+        const CMakeConfig changes = cmakeBuildConfiguration()->configurationChanges();
+        for (const auto &ci : changes) {
+            const bool haveConfigItem = Utils::contains(cmakeConfig, [ci](const CMakeConfigItem& i) {
+                return i.key == ci.key;
+            });
+            if (!haveConfigItem)
+                cmakeConfig.append(ci);
+        }
+    }
+    cmakeBuildConfiguration()->setConfigurationFromCMake(cmakeConfig);
+}
+
 void CMakeBuildSystem::handleParsingSucceeded()
 {
     if (!cmakeBuildConfiguration()->isActive()) {
@@ -699,10 +717,7 @@ void CMakeBuildSystem::handleParsingSucceeded()
     }
 
     {
-        CMakeConfig cmakeConfig = m_reader.takeParsedConfiguration(errorMessage);
+        updateCMakeConfiguration(errorMessage);
-        for (auto &ci : cmakeConfig)
-            ci.inCMakeCache = true;
-        cmakeBuildConfiguration()->setConfigurationFromCMake(cmakeConfig);
         checkAndReportError(errorMessage);
     }
 
@@ -722,10 +737,7 @@ void CMakeBuildSystem::handleParsingFailed(const QString &msg)
     cmakeBuildConfiguration()->setError(msg);
 
     QString errorMessage;
-    CMakeConfig cmakeConfig = m_reader.takeParsedConfiguration(errorMessage);
+    updateCMakeConfiguration(errorMessage);
-    for (auto &ci : cmakeConfig)
-        ci.inCMakeCache = true;
-    cmakeBuildConfiguration()->setConfigurationFromCMake(cmakeConfig);
     // ignore errorMessage here, we already got one.
 
     m_ctestPath.clear();

src/plugins/cmakeprojectmanager/cmakebuildsystem.h

@@ -134,6 +134,8 @@ private:
         const QList<const ProjectExplorer::FileNode *> &allFiles, bool includeHeadersNode);
     void checkAndReportError(QString &errorMessage);
 
+    void updateCMakeConfiguration(QString &errorMessage);
+
     void updateProjectData();
     void updateFallbackProjectData();
     QList<ProjectExplorer::ExtraCompiler *> findExtraCompilers();

src/plugins/cmakeprojectmanager/fileapireader.cpp

@@ -29,6 +29,8 @@
 #include "fileapiparser.h"
 #include "projecttreehelper.h"
 
+#include <coreplugin/messagemanager.h>
+
 #include <projectexplorer/projectexplorer.h>
 
 #include <utils/algorithm.h>
@@ -185,7 +187,8 @@ QList<CMakeBuildTarget> FileApiReader::takeBuildTargets(QString &errorMessage){
 
 CMakeConfig FileApiReader::takeParsedConfiguration(QString &errorMessage)
 {
-    Q_UNUSED(errorMessage)
+    if (m_lastCMakeExitCode != 0)
+        errorMessage = tr("CMake returned error code: %1").arg(m_lastCMakeExitCode);
 
     CMakeConfig cache = m_cache;
     m_cache.clear();
@@ -296,6 +299,34 @@ void FileApiReader::endState(const QFileInfo &replyFi)
     });
 }
 
+void FileApiReader::makeBackupConfiguration(bool store)
+{
+    FilePath reply = m_parameters.buildDirectory.pathAppended(".cmake/api/v1/reply");
+    FilePath replyPrev = m_parameters.buildDirectory.pathAppended(".cmake/api/v1/reply.prev");
+    if (!store)
+        std::swap(reply, replyPrev);
+
+    if (reply.exists()) {
+        if (replyPrev.exists())
+            FileUtils::removeRecursively(replyPrev);
+        QDir dir;
+        if (!dir.rename(reply.toString(), replyPrev.toString()))
+            Core::MessageManager::writeFlashing(tr("Failed to rename %1 to %2.")
+                                                .arg(reply.toString(), replyPrev.toString()));
+
+    }
+
+    FilePath cmakeCacheTxt = m_parameters.buildDirectory.pathAppended("CMakeCache.txt");
+    FilePath cmakeCacheTxtPrev = m_parameters.buildDirectory.pathAppended("CMakeCache.txt.prev");
+    if (!store)
+        std::swap(cmakeCacheTxt, cmakeCacheTxtPrev);
+
+    if (!FileUtils::copyIfDifferent(cmakeCacheTxt, cmakeCacheTxtPrev))
+        Core::MessageManager::writeFlashing(tr("Failed to copy %1 to %2.")
+                                            .arg(cmakeCacheTxt.toString(), cmakeCacheTxtPrev.toString()));
+
+}
+
 void FileApiReader::startCMakeState(const QStringList &configurationArguments)
 {
     qCDebug(cmakeFileApiMode) << "FileApiReader: START CMAKE STATE.";
@@ -306,6 +337,7 @@ void FileApiReader::startCMakeState(const QStringList &configurationArguments)
     connect(m_cmakeProcess.get(), &CMakeProcess::finished, this, &FileApiReader::cmakeFinishedState);
 
     qCDebug(cmakeFileApiMode) << ">>>>>> Running cmake with arguments:" << configurationArguments;
+    makeBackupConfiguration(true);
     m_cmakeProcess->run(m_parameters, configurationArguments);
 }
 
@@ -319,6 +351,9 @@ void FileApiReader::cmakeFinishedState(int code, QProcess::ExitStatus status)
     m_lastCMakeExitCode = m_cmakeProcess->lastExitCode();
     m_cmakeProcess.release()->deleteLater();
 
+    if (m_lastCMakeExitCode != 0)
+        makeBackupConfiguration(false);
+
     endState(FileApiParser::scanForCMakeReplyFile(m_parameters.workDirectory));
 }
 

src/plugins/cmakeprojectmanager/fileapireader.h

@@ -89,6 +89,7 @@ private:
     void cmakeFinishedState(int code, QProcess::ExitStatus status);
 
     void replyDirectoryHasChanged(const QString &directory) const;
+    void makeBackupConfiguration(bool store);
 
     std::unique_ptr<CMakeProcess> m_cmakeProcess;