From 55161882900b459da6bc76e2d5c48e3c32ea4b34 Mon Sep 17 00:00:00 2001 From: David Schulz Date: Tue, 10 Jan 2023 10:21:56 +0100 Subject: [PATCH] Docker: validate clangd version Do not automatically setup a clangd that is too old and mark them as invalid if they are manually selected. Change-Id: Ie9662a8821df8fc678eabc4b8a08375723b4d1c3 Reviewed-by: Christian Kandeler Reviewed-by: Reviewed-by: hjk --- src/libs/utils/CMakeLists.txt | 1 + src/libs/utils/clangutils.cpp | 66 +++++++++++++++++++ src/libs/utils/clangutils.h | 20 ++++++ src/libs/utils/utils.qbs | 2 + .../cppeditor/cppcodemodelsettings.cpp | 34 +--------- src/plugins/cppeditor/cppcodemodelsettings.h | 4 +- .../cppeditor/cppcodemodelsettingspage.cpp | 14 +--- src/plugins/docker/dockerdevicewidget.cpp | 10 ++- 8 files changed, 104 insertions(+), 47 deletions(-) create mode 100644 src/libs/utils/clangutils.cpp create mode 100644 src/libs/utils/clangutils.h diff --git a/src/libs/utils/CMakeLists.txt b/src/libs/utils/CMakeLists.txt index 71eb7008be0..76f4e8a314a 100644 --- a/src/libs/utils/CMakeLists.txt +++ b/src/libs/utils/CMakeLists.txt @@ -21,6 +21,7 @@ add_qtc_library(Utils changeset.cpp changeset.h checkablemessagebox.cpp checkablemessagebox.h classnamevalidatinglineedit.cpp classnamevalidatinglineedit.h + clangutils.cpp clangutils.h codegeneration.cpp codegeneration.h commandline.cpp commandline.h completinglineedit.cpp completinglineedit.h diff --git a/src/libs/utils/clangutils.cpp b/src/libs/utils/clangutils.cpp new file mode 100644 index 00000000000..3f2fb69909d --- /dev/null +++ b/src/libs/utils/clangutils.cpp @@ -0,0 +1,66 @@ +// Copyright (C) 2023 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0 + +#include "clangutils.h" + +#include "filepath.h" +#include "qtcprocess.h" +#include "utilstr.h" + +#include + +namespace Utils { + +static QVersionNumber getClangdVersion(const FilePath &clangdFilePath) +{ + QtcProcess clangdProc; + clangdProc.setCommand({clangdFilePath, {"--version"}}); + clangdProc.runBlocking(); + if (clangdProc.result() != ProcessResult::FinishedWithSuccess) + return {}; + const QString output = clangdProc.allOutput(); + static const QString versionPrefix = "clangd version "; + const int prefixOffset = output.indexOf(versionPrefix); + if (prefixOffset == -1) + return {}; + return QVersionNumber::fromString(output.mid(prefixOffset + versionPrefix.length())); +} + +QVersionNumber clangdVersion(const FilePath &clangd) +{ + static QHash> versionCache; + const QDateTime timeStamp = clangd.lastModified(); + const auto it = versionCache.find(clangd); + if (it == versionCache.end()) { + const QVersionNumber version = getClangdVersion(clangd); + versionCache.insert(clangd, {timeStamp, version}); + return version; + } + if (it->first != timeStamp) { + it->first = timeStamp; + it->second = getClangdVersion(clangd); + } + return it->second; +} + +bool checkClangdVersion(const FilePath &clangd, QString *error) +{ + const QVersionNumber version = clangdVersion(clangd); + if (version >= minimumClangdVersion()) + return true; + if (error) { + *error = version.isNull() + ? Tr::tr("Failed to retrieve clangd version: Unexpected clangd output.") + : Tr::tr("The clangd version is %1, but %2 or greater is required.") + .arg(version.toString()) + .arg(minimumClangdVersion().majorVersion()); + } + return false; +} + +QVersionNumber minimumClangdVersion() +{ + return QVersionNumber(14); +} + +} // namespace Utils diff --git a/src/libs/utils/clangutils.h b/src/libs/utils/clangutils.h new file mode 100644 index 00000000000..2ea0cddc978 --- /dev/null +++ b/src/libs/utils/clangutils.h @@ -0,0 +1,20 @@ +// Copyright (C) 2023 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0 + +#pragma once + +#include "utils_global.h" + +QT_BEGIN_NAMESPACE +class QVersionNumber; +QT_END_NAMESPACE + +namespace Utils { + +class FilePath; + +QTCREATOR_UTILS_EXPORT QVersionNumber clangdVersion(const FilePath &clangd); +QTCREATOR_UTILS_EXPORT bool checkClangdVersion(const FilePath &clangd, QString *error = nullptr); +QTCREATOR_UTILS_EXPORT QVersionNumber minimumClangdVersion(); + +} // namespace Utils diff --git a/src/libs/utils/utils.qbs b/src/libs/utils/utils.qbs index d35973591dd..11c0e342826 100644 --- a/src/libs/utils/utils.qbs +++ b/src/libs/utils/utils.qbs @@ -64,6 +64,8 @@ Project { "changeset.h", "checkablemessagebox.cpp", "checkablemessagebox.h", + "clangutils.cpp", + "clangutils.h", "classnamevalidatinglineedit.cpp", "classnamevalidatinglineedit.h", "codegeneration.cpp", diff --git a/src/plugins/cppeditor/cppcodemodelsettings.cpp b/src/plugins/cppeditor/cppcodemodelsettings.cpp index 1e5ad709b6c..b8edf452854 100644 --- a/src/plugins/cppeditor/cppcodemodelsettings.cpp +++ b/src/plugins/cppeditor/cppcodemodelsettings.cpp @@ -332,38 +332,6 @@ void ClangdSettings::setData(const Data &data) } } -static QVersionNumber getClangdVersion(const FilePath &clangdFilePath) -{ - Utils::QtcProcess clangdProc; - clangdProc.setCommand({clangdFilePath, {"--version"}}); - clangdProc.start(); - if (!clangdProc.waitForFinished()) - return{}; - const QString output = clangdProc.allOutput(); - static const QString versionPrefix = "clangd version "; - const int prefixOffset = output.indexOf(versionPrefix); - if (prefixOffset == -1) - return {}; - return QVersionNumber::fromString(output.mid(prefixOffset + versionPrefix.length())); -} - -QVersionNumber ClangdSettings::clangdVersion(const FilePath &clangdFilePath) -{ - static QHash> versionCache; - const QDateTime timeStamp = clangdFilePath.lastModified(); - const auto it = versionCache.find(clangdFilePath); - if (it == versionCache.end()) { - const QVersionNumber version = getClangdVersion(clangdFilePath); - versionCache.insert(clangdFilePath, {timeStamp, version}); - return version; - } - if (it->first != timeStamp) { - it->first = timeStamp; - it->second = getClangdVersion(clangdFilePath); - } - return it->second; -} - static FilePath getClangHeadersPathFromClang(const FilePath &clangdFilePath) { const FilePath clangFilePath = clangdFilePath.absolutePath().pathAppended("clang") @@ -391,7 +359,7 @@ static FilePath getClangHeadersPath(const FilePath &clangdFilePath) if (!headersPath.isEmpty()) return headersPath; - const QVersionNumber version = ClangdSettings::clangdVersion(clangdFilePath); + const QVersionNumber version = Utils::clangdVersion(clangdFilePath); QTC_ASSERT(!version.isNull(), return {}); static const QStringList libDirs{"lib", "lib64"}; for (const QString &libDir : libDirs) { diff --git a/src/plugins/cppeditor/cppcodemodelsettings.h b/src/plugins/cppeditor/cppcodemodelsettings.h index 222a0c6a361..589ebdea2a5 100644 --- a/src/plugins/cppeditor/cppcodemodelsettings.h +++ b/src/plugins/cppeditor/cppcodemodelsettings.h @@ -6,6 +6,7 @@ #include "clangdiagnosticconfig.h" #include "cppeditor_global.h" +#include #include #include @@ -159,8 +160,7 @@ public: void setData(const Data &data); Data data() const { return m_data; } - static QVersionNumber clangdVersion(const Utils::FilePath &clangdFilePath); - QVersionNumber clangdVersion() const { return clangdVersion(clangdFilePath()); } + QVersionNumber clangdVersion() const { return Utils::clangdVersion(clangdFilePath()); } Utils::FilePath clangdIncludePath() const; static Utils::FilePath clangdUserConfigFilePath(); diff --git a/src/plugins/cppeditor/cppcodemodelsettingspage.cpp b/src/plugins/cppeditor/cppcodemodelsettingspage.cpp index 61db55a5cda..bbf7b3ce073 100644 --- a/src/plugins/cppeditor/cppcodemodelsettingspage.cpp +++ b/src/plugins/cppeditor/cppcodemodelsettingspage.cpp @@ -441,17 +441,9 @@ ClangdSettingsWidget::ClangdSettingsWidget(const ClangdSettings::Data &settingsD if (!d->clangdChooser.isValid()) return; const Utils::FilePath clangdPath = d->clangdChooser.filePath(); - const QVersionNumber clangdVersion = ClangdSettings::clangdVersion(clangdPath); - if (clangdVersion.isNull()) { - labelSetter.setWarning(Tr::tr("Failed to retrieve clangd version: " - "Unexpected clangd output.")); - return; - } - if (clangdVersion < QVersionNumber(14)) { - labelSetter.setWarning(Tr::tr("The clangd version is %1, but %2 or greater is required.") - .arg(clangdVersion.toString()).arg(14)); - return; - } + QString errorMessage; + if (!Utils::checkClangdVersion(clangdPath, &errorMessage)) + labelSetter.setWarning(errorMessage); }; connect(&d->clangdChooser, &Utils::PathChooser::textChanged, this, updateWarningLabel); updateWarningLabel(); diff --git a/src/plugins/docker/dockerdevicewidget.cpp b/src/plugins/docker/dockerdevicewidget.cpp index 6cb1098cc3f..b69e9232836 100644 --- a/src/plugins/docker/dockerdevicewidget.cpp +++ b/src/plugins/docker/dockerdevicewidget.cpp @@ -8,6 +8,7 @@ #include "dockertr.h" #include +#include #include #include #include @@ -108,6 +109,10 @@ DockerDeviceWidget::DockerDeviceWidget(const IDevice::Ptr &device) m_clangdExecutable->setHistoryCompleter("Docker.ClangdExecutable.History"); m_clangdExecutable->setAllowPathFromDevice(true); m_clangdExecutable->setFilePath(m_data.clangdExecutable); + m_clangdExecutable->setValidationFunction( + [chooser = m_clangdExecutable](FancyLineEdit *, QString *error) { + return Utils::checkClangdVersion(chooser->filePath(), error); + }); connect(m_clangdExecutable, &PathChooser::rawPathChanged, this, [this, dockerDevice] { m_data.clangdExecutable = m_clangdExecutable->filePath(); @@ -177,7 +182,10 @@ DockerDeviceWidget::DockerDeviceWidget(const IDevice::Ptr &device) logView->clear(); dockerDevice->updateContainerAccess(); - const FilePath clangdPath = dockerDevice->rootPath().withNewPath("clangd").searchInPath(); + const FilePath clangdPath = dockerDevice->systemEnvironment() + .searchInPath("clangd", {}, [](const FilePath &clangd) { + return Utils::checkClangdVersion(clangd); + }); if (!clangdPath.isEmpty()) m_clangdExecutable->setFilePath(clangdPath);