From fc411cd0d1806435a9c89e8a10cdfed012663fd9 Mon Sep 17 00:00:00 2001 From: Cristian Adam Date: Thu, 4 Feb 2021 16:22:40 +0100 Subject: [PATCH] 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 --- .../builddirparameters.cpp | 2 +- .../cmakebuildconfiguration.cpp | 33 ++++++++++------- .../cmakebuildconfiguration.h | 6 ++- .../cmakeprojectmanager/cmakebuildsystem.cpp | 28 ++++++++++---- .../cmakeprojectmanager/cmakebuildsystem.h | 2 + .../cmakeprojectmanager/fileapireader.cpp | 37 ++++++++++++++++++- .../cmakeprojectmanager/fileapireader.h | 1 + 7 files changed, 83 insertions(+), 26 deletions(-) diff --git a/src/plugins/cmakeprojectmanager/builddirparameters.cpp b/src/plugins/cmakeprojectmanager/builddirparameters.cpp index 023de6b0717..0c06c329f58 100644 --- a/src/plugins/cmakeprojectmanager/builddirparameters.cpp +++ b/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); }); diff --git a/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.cpp b/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.cpp index 42457c6a775..13cf6c8af2f 100644 --- a/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.cpp +++ b/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( - Utils::transform(configChanges, [](const CMakeConfigItem &i) { return i.toArgument(); })); + m_buildConfiguration->setConfigurationChanges(configChanges); } 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()->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. diff --git a/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.h b/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.h index b9bf2530919..24dbaaca436 100644 --- a/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.h +++ b/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; diff --git a/src/plugins/cmakeprojectmanager/cmakebuildsystem.cpp b/src/plugins/cmakeprojectmanager/cmakebuildsystem.cpp index 636f0c47f56..abd515f1a0f 100644 --- a/src/plugins/cmakeprojectmanager/cmakebuildsystem.cpp +++ b/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); - for (auto &ci : cmakeConfig) - ci.inCMakeCache = true; - cmakeBuildConfiguration()->setConfigurationFromCMake(cmakeConfig); + updateCMakeConfiguration(errorMessage); checkAndReportError(errorMessage); } @@ -722,10 +737,7 @@ void CMakeBuildSystem::handleParsingFailed(const QString &msg) cmakeBuildConfiguration()->setError(msg); QString errorMessage; - CMakeConfig cmakeConfig = m_reader.takeParsedConfiguration(errorMessage); - for (auto &ci : cmakeConfig) - ci.inCMakeCache = true; - cmakeBuildConfiguration()->setConfigurationFromCMake(cmakeConfig); + updateCMakeConfiguration(errorMessage); // ignore errorMessage here, we already got one. m_ctestPath.clear(); diff --git a/src/plugins/cmakeprojectmanager/cmakebuildsystem.h b/src/plugins/cmakeprojectmanager/cmakebuildsystem.h index b69d675be06..283990ed7be 100644 --- a/src/plugins/cmakeprojectmanager/cmakebuildsystem.h +++ b/src/plugins/cmakeprojectmanager/cmakebuildsystem.h @@ -134,6 +134,8 @@ private: const QList &allFiles, bool includeHeadersNode); void checkAndReportError(QString &errorMessage); + void updateCMakeConfiguration(QString &errorMessage); + void updateProjectData(); void updateFallbackProjectData(); QList findExtraCompilers(); diff --git a/src/plugins/cmakeprojectmanager/fileapireader.cpp b/src/plugins/cmakeprojectmanager/fileapireader.cpp index 71fa5601a17..152f22b468f 100644 --- a/src/plugins/cmakeprojectmanager/fileapireader.cpp +++ b/src/plugins/cmakeprojectmanager/fileapireader.cpp @@ -29,6 +29,8 @@ #include "fileapiparser.h" #include "projecttreehelper.h" +#include + #include #include @@ -185,7 +187,8 @@ QList 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)); } diff --git a/src/plugins/cmakeprojectmanager/fileapireader.h b/src/plugins/cmakeprojectmanager/fileapireader.h index ef210ea2a73..25d5e1af1b8 100644 --- a/src/plugins/cmakeprojectmanager/fileapireader.h +++ b/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 m_cmakeProcess;