From de6faa0f15b265d4b6c85135a2c28c881971b46b Mon Sep 17 00:00:00 2001 From: Tobias Hunger Date: Mon, 5 Aug 2019 12:57:01 +0200 Subject: [PATCH] CMake: Better warnings for strange/invalid CMake binaries Warn when cmake is configured to use a non-canonical path. This might trigger problems in CMake itself. Show this warning in the Kit as well as in the CMake options page. Also complain other issues in the CMake options page. Task-number: QTCREATORBUG-22583 Change-Id: I841341db8305f9152543487ce9ceeab2eca0b2b9 Reviewed-by: hjk --- .../cmakekitinformation.cpp | 8 ++ .../cmakeprojectmanager/cmakesettingspage.cpp | 107 +++++++++++++----- src/plugins/cmakeprojectmanager/cmaketool.cpp | 19 ++++ src/plugins/cmakeprojectmanager/cmaketool.h | 5 + 4 files changed, 113 insertions(+), 26 deletions(-) diff --git a/src/plugins/cmakeprojectmanager/cmakekitinformation.cpp b/src/plugins/cmakeprojectmanager/cmakekitinformation.cpp index 9bee2f4fb72..9dfdd03ddce 100644 --- a/src/plugins/cmakeprojectmanager/cmakekitinformation.cpp +++ b/src/plugins/cmakeprojectmanager/cmakekitinformation.cpp @@ -46,6 +46,7 @@ #include #include #include +#include #include #include #include @@ -256,6 +257,13 @@ Tasks CMakeKitAspect::validate(const Kit *k) const "version 3.0 or later.").arg(QString::fromUtf8(version.fullVersion)), Utils::FilePath(), -1, Core::Id(ProjectExplorer::Constants::TASK_CATEGORY_BUILDSYSTEM)); } + if (!tool->isExecutablePathCanonical()) { + result << Task(Task::Warning, + CMakeTool::nonCanonicalPathToCMakeExecutableWarningMessage(), + Utils::FilePath(), + -1, + Core::Id(ProjectExplorer::Constants::TASK_CATEGORY_BUILDSYSTEM)); + } } return result; } diff --git a/src/plugins/cmakeprojectmanager/cmakesettingspage.cpp b/src/plugins/cmakeprojectmanager/cmakesettingspage.cpp index eeeaf34753e..d0420851113 100644 --- a/src/plugins/cmakeprojectmanager/cmakesettingspage.cpp +++ b/src/plugins/cmakeprojectmanager/cmakesettingspage.cpp @@ -27,25 +27,27 @@ #include "cmakesettingspage.h" #include "cmaketoolmanager.h" +#include #include #include -#include -#include #include +#include #include #include #include #include +#include #include +#include #include #include #include #include #include #include -#include #include +#include using namespace Utils; @@ -91,30 +93,45 @@ class CMakeToolTreeItem : public TreeItem Q_DECLARE_TR_FUNCTIONS(CMakeProjectManager::CMakeSettingsPage) public: - CMakeToolTreeItem(const CMakeTool *item, bool changed) : - m_id(item->id()), - m_name(item->displayName()), - m_executable(item->cmakeExecutable()), - m_isAutoRun(item->isAutoRun()), - m_autoCreateBuildDirectory(item->autoCreateBuildDirectory()), - m_autodetected(item->isAutoDetected()), - m_changed(changed) + CMakeToolTreeItem(const CMakeTool *item, bool changed) + : m_id(item->id()) + , m_name(item->displayName()) + , m_executable(item->cmakeExecutable()) + , m_isAutoRun(item->isAutoRun()) + , m_autoCreateBuildDirectory(item->autoCreateBuildDirectory()) + , m_autodetected(item->isAutoDetected()) + , m_changed(changed) { + updateErrorFlags(); m_tooltip = tr("Version: %1
Supports fileApi: %2
Supports server-mode: %3") .arg(QString::fromUtf8(item->version().fullVersion)) .arg(item->hasFileApi() ? tr("yes") : tr("no")) .arg(item->hasServerMode() ? tr("yes") : tr("no")); } - CMakeToolTreeItem(const QString &name, const Utils::FilePath &executable, - bool autoRun, bool autoCreate, bool autodetected) : - m_id(Core::Id::fromString(QUuid::createUuid().toString())), - m_name(name), - m_executable(executable), - m_isAutoRun(autoRun), - m_autoCreateBuildDirectory(autoCreate), - m_autodetected(autodetected) - {} + CMakeToolTreeItem(const QString &name, + const Utils::FilePath &executable, + bool autoRun, + bool autoCreate, + bool autodetected) + : m_id(Core::Id::fromString(QUuid::createUuid().toString())) + , m_name(name) + , m_executable(executable) + , m_isAutoRun(autoRun) + , m_autoCreateBuildDirectory(autoCreate) + , m_autodetected(autodetected) + { + updateErrorFlags(); + } + + void updateErrorFlags() + { + const QFileInfo fi = m_executable.toFileInfo(); + m_pathExists = fi.exists(); + m_pathIsFile = fi.isFile(); + m_pathIsExecutable = fi.isExecutable(); + m_pathIsCanonical = CMakeTool::isCanonicalPath(m_executable); + } CMakeToolTreeItem() = default; @@ -123,7 +140,7 @@ public: QVariant data(int column, int role) const override { switch (role) { - case Qt::DisplayRole: + case Qt::DisplayRole: { switch (column) { case 0: { QString name = m_name; @@ -131,11 +148,12 @@ public: name += tr(" (Default)"); return name; } - case 1: + case 1: { return m_executable.toUserOutput(); } - break; - + } // switch (column) + return QVariant(); + } case Qt::FontRole: { QFont font; font.setBold(m_changed); @@ -143,7 +161,39 @@ public: return font; } case Qt::ToolTipRole: { - return m_tooltip; + QString result = m_tooltip; + QString error; + if (!m_pathExists) { + error = QCoreApplication::translate( + "CMakeProjectManager::Internal::CMakeToolTreeItem", + "CMake executable path does not exist."); + } else if (!m_pathIsFile) { + error = QCoreApplication::translate( + "CMakeProjectManager::Internal::CMakeToolTreeItem", + "CMake executable path is not a file."); + } else if (!m_pathIsExecutable) { + error = QCoreApplication::translate( + "CMakeProjectManager::Internal::CMakeToolTreeItem", + "CMake executable path is not executable."); + } else if (!m_pathIsCanonical) { + error = CMakeTool::nonCanonicalPathToCMakeExecutableWarningMessage(); + } + if (result.isEmpty() || error.isEmpty()) + return QString("%1%2").arg(result).arg(error); + else + return QString("%1

%2").arg(result).arg(error); + } + case Qt::DecorationRole: { + if (column != 0) + return QVariant(); + + const bool hasError = !m_pathExists || !m_pathIsFile || !m_pathIsExecutable; + if (hasError) + return Utils::Icons::CRITICAL.icon(); + const bool hasWarning = !m_pathIsCanonical; + if (hasWarning) + return Utils::Icons::WARNING.icon(); + return QVariant(); } } return QVariant(); @@ -154,6 +204,10 @@ public: QString m_tooltip; FilePath m_executable; bool m_isAutoRun = true; + bool m_pathExists = false; + bool m_pathIsFile = false; + bool m_pathIsExecutable = false; + bool m_pathIsCanonical = false; bool m_autoCreateBuildDirectory = false; bool m_autodetected = false; bool m_changed = true; @@ -240,9 +294,10 @@ void CMakeToolItemModel::updateCMakeTool(const Core::Id &id, const QString &disp treeItem->m_name = displayName; treeItem->m_executable = executable; - treeItem->m_isAutoRun = autoRun; treeItem->m_autoCreateBuildDirectory = autoCreate; + treeItem->updateErrorFlags(); + reevaluateChangedFlag(treeItem); } diff --git a/src/plugins/cmakeprojectmanager/cmaketool.cpp b/src/plugins/cmakeprojectmanager/cmaketool.cpp index f37939174ac..695ae5c483e 100644 --- a/src/plugins/cmakeprojectmanager/cmaketool.cpp +++ b/src/plugins/cmakeprojectmanager/cmaketool.cpp @@ -356,6 +356,25 @@ CMakeTool::ReaderType CMakeTool::readerType() const return m_readerType.value(); } +bool CMakeTool::isCanonicalPath(const Utils::FilePath &path) +{ + const QString canonicalPath = path.toFileInfo().canonicalFilePath(); + return canonicalPath == path.toString(); +} + +bool CMakeTool::isExecutablePathCanonical() const +{ + return isCanonicalPath(cmakeExecutable()); +} + +QString CMakeTool::nonCanonicalPathToCMakeExecutableWarningMessage() +{ + return QCoreApplication::translate( + "CMakeProjectManager::CMakeTool", + "CMake executable path is not canonical and contains \"..\", \".\" " + "or a symbolic link. This might trigger bugs in CMake."); +} + void CMakeTool::readInformation(CMakeTool::QueryType type) const { if ((type == QueryType::GENERATORS && !m_introspection->m_generators.isEmpty()) diff --git a/src/plugins/cmakeprojectmanager/cmaketool.h b/src/plugins/cmakeprojectmanager/cmaketool.h index c8c29665c2f..ef39f0f0d38 100644 --- a/src/plugins/cmakeprojectmanager/cmaketool.h +++ b/src/plugins/cmakeprojectmanager/cmaketool.h @@ -112,6 +112,11 @@ public: ReaderType readerType() const; + static bool isCanonicalPath(const Utils::FilePath &path); + bool isExecutablePathCanonical() const; + + static QString nonCanonicalPathToCMakeExecutableWarningMessage(); + private: enum class QueryType { GENERATORS,