Run CMake via canonical path instead of warning

Running CMake via a non-canonical path (including '..' in the path, or
via a symlink) can result in funny errors.

de6faa0f15 added a warning for that
condition. Unfortunately the auto-detection of CMake can return a path
to a symlink (e.g. for CMake from brew on macOS). Also it is helpful to
use a symlink to e.g. manage different CMake versions.

Instead of warning about the condition, and forcing the user to resolve
it manually, simply run CMake via its canonical path when actually
running it from Qt Creator.

Change-Id: I95623b45c5436a6d61c1419b7aba23e2a73a0650
Reviewed-by: Tobias Hunger <tobias.hunger@qt.io>
This commit is contained in:
Eike Ziller
2019-08-12 14:44:29 +02:00
parent b61a5b759b
commit ff503740ce
5 changed files with 16 additions and 48 deletions

View File

@@ -257,13 +257,6 @@ Tasks CMakeKitAspect::validate(const Kit *k) const
"version 3.0 or later.").arg(QString::fromUtf8(version.fullVersion)), "version 3.0 or later.").arg(QString::fromUtf8(version.fullVersion)),
Utils::FilePath(), -1, Core::Id(ProjectExplorer::Constants::TASK_CATEGORY_BUILDSYSTEM)); 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; return result;
} }

View File

@@ -96,7 +96,7 @@ public:
CMakeToolTreeItem(const CMakeTool *item, bool changed) CMakeToolTreeItem(const CMakeTool *item, bool changed)
: m_id(item->id()) : m_id(item->id())
, m_name(item->displayName()) , m_name(item->displayName())
, m_executable(item->cmakeExecutable()) , m_executable(item->filePath())
, m_isAutoRun(item->isAutoRun()) , m_isAutoRun(item->isAutoRun())
, m_autoCreateBuildDirectory(item->autoCreateBuildDirectory()) , m_autoCreateBuildDirectory(item->autoCreateBuildDirectory())
, m_autodetected(item->isAutoDetected()) , m_autodetected(item->isAutoDetected())
@@ -130,7 +130,6 @@ public:
m_pathExists = fi.exists(); m_pathExists = fi.exists();
m_pathIsFile = fi.isFile(); m_pathIsFile = fi.isFile();
m_pathIsExecutable = fi.isExecutable(); m_pathIsExecutable = fi.isExecutable();
m_pathIsCanonical = CMakeTool::isCanonicalPath(m_executable);
} }
CMakeToolTreeItem() = default; CMakeToolTreeItem() = default;
@@ -175,8 +174,6 @@ public:
error = QCoreApplication::translate( error = QCoreApplication::translate(
"CMakeProjectManager::Internal::CMakeToolTreeItem", "CMakeProjectManager::Internal::CMakeToolTreeItem",
"CMake executable path is not executable."); "CMake executable path is not executable.");
} else if (!m_pathIsCanonical) {
error = CMakeTool::nonCanonicalPathToCMakeExecutableWarningMessage();
} }
if (result.isEmpty() || error.isEmpty()) if (result.isEmpty() || error.isEmpty())
return QString("%1%2").arg(result).arg(error); return QString("%1%2").arg(result).arg(error);
@@ -190,9 +187,6 @@ public:
const bool hasError = !m_pathExists || !m_pathIsFile || !m_pathIsExecutable; const bool hasError = !m_pathExists || !m_pathIsFile || !m_pathIsExecutable;
if (hasError) if (hasError)
return Utils::Icons::CRITICAL.icon(); return Utils::Icons::CRITICAL.icon();
const bool hasWarning = !m_pathIsCanonical;
if (hasWarning)
return Utils::Icons::WARNING.icon();
return QVariant(); return QVariant();
} }
} }
@@ -207,7 +201,6 @@ public:
bool m_pathExists = false; bool m_pathExists = false;
bool m_pathIsFile = false; bool m_pathIsFile = false;
bool m_pathIsExecutable = false; bool m_pathIsExecutable = false;
bool m_pathIsCanonical = false;
bool m_autoCreateBuildDirectory = false; bool m_autoCreateBuildDirectory = false;
bool m_autodetected = false; bool m_autodetected = false;
bool m_changed = true; bool m_changed = true;
@@ -272,7 +265,7 @@ void CMakeToolItemModel::reevaluateChangedFlag(CMakeToolTreeItem *item) const
{ {
CMakeTool *orig = CMakeToolManager::findById(item->m_id); CMakeTool *orig = CMakeToolManager::findById(item->m_id);
item->m_changed = !orig || orig->displayName() != item->m_name item->m_changed = !orig || orig->displayName() != item->m_name
|| orig->cmakeExecutable() != item->m_executable; || orig->filePath() != item->m_executable;
//make sure the item is marked as changed when the default cmake was changed //make sure the item is marked as changed when the default cmake was changed
CMakeTool *origDefTool = CMakeToolManager::defaultCMakeTool(); CMakeTool *origDefTool = CMakeToolManager::defaultCMakeTool();
@@ -333,7 +326,7 @@ void CMakeToolItemModel::apply()
item->m_changed = false; item->m_changed = false;
if (CMakeTool *cmake = CMakeToolManager::findById(item->m_id)) { if (CMakeTool *cmake = CMakeToolManager::findById(item->m_id)) {
cmake->setDisplayName(item->m_name); cmake->setDisplayName(item->m_name);
cmake->setCMakeExecutable(item->m_executable); cmake->setFilePath(item->m_executable);
cmake->setAutorun(item->m_isAutoRun); cmake->setAutorun(item->m_isAutoRun);
cmake->setAutoCreateBuildDirectory(item->m_autoCreateBuildDirectory); cmake->setAutoCreateBuildDirectory(item->m_autoCreateBuildDirectory);
} else { } else {
@@ -346,7 +339,7 @@ void CMakeToolItemModel::apply()
: CMakeTool::ManualDetection; : CMakeTool::ManualDetection;
auto cmake = std::make_unique<CMakeTool>(detection, item->m_id); auto cmake = std::make_unique<CMakeTool>(detection, item->m_id);
cmake->setDisplayName(item->m_name); cmake->setDisplayName(item->m_name);
cmake->setCMakeExecutable(item->m_executable); cmake->setFilePath(item->m_executable);
if (!CMakeToolManager::registerCMakeTool(std::move(cmake))) if (!CMakeToolManager::registerCMakeTool(std::move(cmake)))
item->m_changed = true; item->m_changed = true;
} }

View File

@@ -145,7 +145,7 @@ CMakeTool::CMakeTool(const QVariantMap &map, bool fromSdk) :
if (!fromSdk) if (!fromSdk)
m_isAutoDetected = map.value(CMAKE_INFORMATION_AUTODETECTED, false).toBool(); m_isAutoDetected = map.value(CMAKE_INFORMATION_AUTODETECTED, false).toBool();
setCMakeExecutable(Utils::FilePath::fromString(map.value(CMAKE_INFORMATION_COMMAND).toString())); setFilePath(Utils::FilePath::fromString(map.value(CMAKE_INFORMATION_COMMAND).toString()));
} }
CMakeTool::~CMakeTool() = default; CMakeTool::~CMakeTool() = default;
@@ -155,7 +155,7 @@ Core::Id CMakeTool::createId()
return Core::Id::fromString(QUuid::createUuid().toString()); return Core::Id::fromString(QUuid::createUuid().toString());
} }
void CMakeTool::setCMakeExecutable(const Utils::FilePath &executable) void CMakeTool::setFilePath(const Utils::FilePath &executable)
{ {
if (m_executable == executable) if (m_executable == executable)
return; return;
@@ -167,6 +167,11 @@ void CMakeTool::setCMakeExecutable(const Utils::FilePath &executable)
CMakeToolManager::notifyAboutUpdate(this); CMakeToolManager::notifyAboutUpdate(this);
} }
Utils::FilePath CMakeTool::filePath() const
{
return m_executable;
}
void CMakeTool::setAutorun(bool autoRun) void CMakeTool::setAutorun(bool autoRun)
{ {
if (m_isAutoRun == autoRun) if (m_isAutoRun == autoRun)
@@ -238,9 +243,9 @@ Utils::FilePath CMakeTool::cmakeExecutable() const
if (Utils::HostOsInfo::isMacHost() && m_executable.endsWith(".app")) { if (Utils::HostOsInfo::isMacHost() && m_executable.endsWith(".app")) {
const Utils::FilePath toTest = m_executable.pathAppended("Contents/bin/cmake"); const Utils::FilePath toTest = m_executable.pathAppended("Contents/bin/cmake");
if (toTest.exists()) if (toTest.exists())
return toTest; return toTest.canonicalPath();
} }
return m_executable; return m_executable.canonicalPath();
} }
bool CMakeTool::isAutoRun() const bool CMakeTool::isAutoRun() const
@@ -356,25 +361,6 @@ CMakeTool::ReaderType CMakeTool::readerType() const
return m_readerType.value(); 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 void CMakeTool::readInformation(CMakeTool::QueryType type) const
{ {
if ((type == QueryType::GENERATORS && !m_introspection->m_generators.isEmpty()) if ((type == QueryType::GENERATORS && !m_introspection->m_generators.isEmpty())

View File

@@ -89,10 +89,11 @@ public:
Core::Id id() const { return m_id; } Core::Id id() const { return m_id; }
QVariantMap toMap () const; QVariantMap toMap () const;
void setCMakeExecutable(const Utils::FilePath &executable);
void setAutorun(bool autoRun); void setAutorun(bool autoRun);
void setAutoCreateBuildDirectory(bool autoBuildDir); void setAutoCreateBuildDirectory(bool autoBuildDir);
void setFilePath(const Utils::FilePath &executable);
Utils::FilePath filePath() const;
Utils::FilePath cmakeExecutable() const; Utils::FilePath cmakeExecutable() const;
bool isAutoRun() const; bool isAutoRun() const;
bool autoCreateBuildDirectory() const; bool autoCreateBuildDirectory() const;
@@ -112,11 +113,6 @@ public:
ReaderType readerType() const; ReaderType readerType() const;
static bool isCanonicalPath(const Utils::FilePath &path);
bool isExecutablePathCanonical() const;
static QString nonCanonicalPathToCMakeExecutableWarningMessage();
private: private:
enum class QueryType { enum class QueryType {
GENERATORS, GENERATORS,

View File

@@ -110,7 +110,7 @@ static std::vector<std::unique_ptr<CMakeTool>> autoDetectCMakeTools()
std::vector<std::unique_ptr<CMakeTool>> found; std::vector<std::unique_ptr<CMakeTool>> found;
foreach (const FilePath &command, suspects) { foreach (const FilePath &command, suspects) {
auto item = std::make_unique<CMakeTool>(CMakeTool::AutoDetection, CMakeTool::createId()); auto item = std::make_unique<CMakeTool>(CMakeTool::AutoDetection, CMakeTool::createId());
item->setCMakeExecutable(command); item->setFilePath(command);
item->setDisplayName(CMakeToolManager::tr("System CMake at %1").arg(command.toUserOutput())); item->setDisplayName(CMakeToolManager::tr("System CMake at %1").arg(command.toUserOutput()));
found.emplace_back(std::move(item)); found.emplace_back(std::move(item));