From 5c7083f75cbe404756f5b7e38223abdf5572caca Mon Sep 17 00:00:00 2001 From: Lieven Hey Date: Tue, 12 Sep 2023 16:26:43 +0200 Subject: [PATCH 1/7] rewrite MultiConfigWidget This change makes it more usable by making the usage easier. Simply add the edit widget as a child using addChild and be done. Now you only need to call saveCurrentConfig and be done. Restoring and saving are handled by the widget. --- src/devicesettings.ui | 124 ++++++++++++ src/multiconfigwidget.cpp | 223 ++++++++++++++------- src/multiconfigwidget.h | 49 +++-- src/multiconfigwidget.ui | 115 +++++++++++ src/recordpage.cpp | 43 ++--- src/recordpage.h | 2 - src/recordpage.ui | 161 +++++++++------- src/settingsdialog.cpp | 106 ++-------- src/settingsdialog.h | 1 - src/unwindsettingspage.ui | 397 ++++++++++++++++++++------------------ 10 files changed, 759 insertions(+), 462 deletions(-) create mode 100644 src/devicesettings.ui create mode 100644 src/multiconfigwidget.ui diff --git a/src/devicesettings.ui b/src/devicesettings.ui new file mode 100644 index 00000000..06ba06c0 --- /dev/null +++ b/src/devicesettings.ui @@ -0,0 +1,124 @@ + + + DeviceSettings + + + + 0 + 0 + 400 + 300 + + + + Form + + + + 0 + + + 0 + + + 0 + + + 0 + + + 0 + + + 0 + + + + + Hostname: + + + + + + + + + + Username: + + + + + + + + + + Password: + + + + + + + + 0 + + + 0 + + + 0 + + + 0 + + + 0 + + + + + QLineEdit::Password + + + + + + + Qt::Horizontal + + + + 40 + 20 + + + + + + + + Copy SSH Key + + + + + + + + + + Options: + + + + + + + + + + + diff --git a/src/multiconfigwidget.cpp b/src/multiconfigwidget.cpp index 5225111f..29d572b6 100644 --- a/src/multiconfigwidget.cpp +++ b/src/multiconfigwidget.cpp @@ -1,119 +1,208 @@ /* SPDX-FileCopyrightText: Lieven Hey - SPDX-FileCopyrightText: Milian Wolff - SPDX-FileCopyrightText: 2016-2022 Klarälvdalens Datakonsult AB, a KDAB Group company, info@kdab.com + SPDX-FileCopyrightText: 2022-2023 Klarälvdalens Datakonsult AB, a KDAB Group company, info@kdab.com SPDX-License-Identifier: GPL-2.0-or-later */ #include "multiconfigwidget.h" +#include + #include +#include #include #include #include +#include + +#include "ui_multiconfigwidget.h" + MultiConfigWidget::MultiConfigWidget(QWidget* parent) : QWidget(parent) + , m_configWidget(new Ui::MultiConfigWidget) { - auto* layout = new QHBoxLayout(this); - layout->setContentsMargins(0, 0, 0, 0); - - m_comboBox = new QComboBox(this); - m_comboBox->setEditable(true); - m_comboBox->setInsertPolicy(QComboBox::InsertAtCurrent); - m_comboBox->setDisabled(true); - layout->addWidget(m_comboBox); - - connect(m_comboBox->lineEdit(), &QLineEdit::editingFinished, this, [this] { - m_config.deleteGroup(m_comboBox->currentData().toString()); - saveConfigAs(m_comboBox->currentText()); - m_comboBox->setItemData(m_comboBox->currentIndex(), m_comboBox->currentText()); - m_config.sync(); - }); + m_configWidget->setupUi(this); + + connect(m_configWidget->currentConfigComboBox, qOverload(&QComboBox::currentIndexChanged), this, + [this](int) { loadConfig(currentConfig()); }); - connect(m_comboBox, QOverload::of(&QComboBox::currentIndexChanged), this, - [this] { selectConfig(m_comboBox->currentData().toString()); }); + connect(this, &MultiConfigWidget::configsChanged, this, [this]() { + m_configWidget->currentConfigComboBox->setDisabled(m_configWidget->currentConfigComboBox->count() == 0); + }); - m_copyButton = new QPushButton(this); - m_copyButton->setText(tr("Copy Config")); - layout->addWidget(m_copyButton); + m_configWidget->currentConfigComboBox->setEnabled(true); - connect(m_copyButton, &QPushButton::clicked, this, [this] { - const QString name = tr("Config %1").arg(m_comboBox->count() + 1); - saveConfigAs(name); - m_comboBox->addItem(name, name); - m_comboBox->setDisabled(false); + connect(m_configWidget->copyButton, &QPushButton::pressed, this, [this] { + const auto name = m_configWidget->currentConfigComboBox->currentText(); + auto copyName = QStringLiteral("Copy of %1").arg(name); + if (name.isEmpty()) { + copyName = QStringLiteral("Config"); + } + saveConfig(copyName); + m_configWidget->currentConfigComboBox->addItem(copyName, copyName); + m_configWidget->currentConfigComboBox->setCurrentIndex(m_configWidget->currentConfigComboBox->count() - 1); + emit configsChanged(); }); - m_removeButton = new QPushButton(this); - m_removeButton->setText(tr("Remove Config")); - layout->addWidget(m_removeButton); + connect(m_configWidget->deleteButton, &QPushButton::pressed, this, [this] { + const auto currentConfig = m_configWidget->currentConfigComboBox->currentText(); + m_configWidget->currentConfigComboBox->removeItem(m_configWidget->currentConfigComboBox->currentIndex()); + m_group.deleteGroup(currentConfig); + emit configsChanged(); + }); - connect(m_removeButton, &QPushButton::clicked, this, [this] { - m_config.deleteGroup(m_comboBox->currentData().toString()); - m_comboBox->removeItem(m_comboBox->currentIndex()); + connect(m_configWidget->currentConfigComboBox->lineEdit(), &QLineEdit::returnPressed, this, [this] { + const auto oldName = m_configWidget->currentConfigComboBox->currentData().toString(); + if (!oldName.isEmpty()) { + m_group.deleteGroup(oldName); + } - if (m_comboBox->count() == 0) { - m_comboBox->setDisabled(true); - } else { - selectConfig(m_comboBox->currentData().toString()); + auto newName = m_configWidget->currentConfigComboBox->currentText(); + if (newName.isEmpty()) { + newName = QStringLiteral("Config %1").arg(m_configWidget->currentConfigComboBox->currentIndex()); } + m_configWidget->currentConfigComboBox->setItemData(m_configWidget->currentConfigComboBox->currentIndex(), + newName); + saveConfig(newName); }); - - setLayout(layout); } MultiConfigWidget::~MultiConfigWidget() = default; -QString MultiConfigWidget::currentConfig() const +void MultiConfigWidget::setChildWidget(QWidget* widget, const QVector& formWidgets) { - return m_comboBox->currentData().toString(); + m_formWidgets = formWidgets; + m_child = widget; + m_child->setParent(this); + auto placeholder = m_configWidget->layout->replaceWidget(m_configWidget->placeholder, m_child); + Q_ASSERT(placeholder); + delete placeholder; } -void MultiConfigWidget::setConfig(const KConfigGroup& group) +void MultiConfigWidget::setConfigGroup(const KConfigGroup& group) { - m_comboBox->clear(); - m_config = group; + m_group = group; + if (!m_group.isValid()) { + return; + } + + m_configWidget->currentConfigComboBox->clear(); + auto configGroups = configs(); + for (const auto& config : configGroups) { + m_configWidget->currentConfigComboBox->addItem(config, config); + } + + m_configWidget->currentConfigComboBox->setCurrentIndex(0); - if (!m_config.isValid()) + if (!configGroups.isEmpty()) { + loadConfig(currentConfig()); + } + emit configsChanged(); +} + +void MultiConfigWidget::loadConfig(const QString& name) +{ + if (m_saving) { return; + } - const auto groups = m_config.groupList(); - for (const auto& config : groups) { - if (m_config.hasGroup(config)) { - // item data is used to get the old name after renaming - m_comboBox->addItem(config, config); - m_comboBox->setDisabled(false); + if (m_child == nullptr) { + return; + } + + if (name.isEmpty() || !m_group.isValid()) { + return; + } + + if (!m_group.hasGroup(name)) { + return; + } + + const auto& group = m_group.group(name); + + for (auto formWidget : std::as_const(m_formWidgets)) { + const auto& name = formWidget->objectName(); + if (auto widget = qobject_cast(formWidget)) { + auto text = group.readEntry(name, QString {}); + widget->setText(text); + } else if (auto widget = qobject_cast(formWidget)) { + auto text = group.readEntry(name, QString {}); + widget->setText(text); + } else if (auto widget = qobject_cast(formWidget)) { + auto items = group.readEntry(name, QString {}).split(QLatin1Char(':')); + widget->setItems(items); + } else if (auto widget = qobject_cast(formWidget)) { + auto value = group.readEntry(name, QString {}); + if (value.isEmpty()) { + continue; + } + auto index = widget->findText(value); + if (index == -1) { + index = widget->count(); + widget->addItem(value); + } + widget->setCurrentIndex(index); + } else { + qWarning() << formWidget->metaObject()->className() << "is not supported in MultiConfigWidget"; } } } -void MultiConfigWidget::saveConfigAs(const QString& name) +void MultiConfigWidget::saveConfig(const QString& name) { - if (!name.isEmpty()) { - emit saveConfig(m_config.group(name)); + if (!m_child) { + return; + } + + if (name.isEmpty() || !m_group.isValid()) { + return; + } + + m_saving = true; + + auto group = m_group.group(name); + + for (const auto formWidget : std::as_const(m_formWidgets)) { + const auto& name = formWidget->objectName(); + if (auto widget = qobject_cast(formWidget)) { + group.writeEntry(name, widget->text()); + } else if (auto widget = qobject_cast(formWidget)) { + group.writeEntry(name, widget->text()); + } else if (auto widget = qobject_cast(formWidget)) { + auto data = widget->items().join(QLatin1Char(':')); + group.writeEntry(name, data); + } else if (auto widget = qobject_cast(formWidget)) { + group.writeEntry(name, widget->currentText()); + } else { + qWarning() << formWidget->metaObject()->className() << "is not supported in MultiConfigWidget"; + } } + + m_saving = false; } -void MultiConfigWidget::updateCurrentConfig() +void MultiConfigWidget::saveCurrentConfig() { - if (m_comboBox->currentIndex() != -1) { - saveConfigAs(m_comboBox->currentData().toString()); - } + saveConfig(currentConfig()); } -void MultiConfigWidget::selectConfig(const QString& name) +QString MultiConfigWidget::currentConfig() const { - m_config.sync(); - if (!name.isEmpty() && m_config.hasGroup(name)) { - emit restoreConfig(m_config.group(name)); - } + return m_configWidget->currentConfigComboBox->currentText(); } -void MultiConfigWidget::restoreCurrent() +QStringList MultiConfigWidget::configs() const { - if (m_comboBox->currentIndex() != -1) { - selectConfig(m_comboBox->currentData().toString()); - } + auto configs = m_group.groupList(); + // KConfig is weird in regards to deleted groups + // they are still in groupList but are no longer valid + // TODO: C++20 use filter + configs.erase(std::remove_if(configs.begin(), configs.end(), + [group = m_group](const QString& groupName) { + return !group.hasGroup(groupName) || !group.group(groupName).exists(); + }), + configs.end()); + return configs; } diff --git a/src/multiconfigwidget.h b/src/multiconfigwidget.h index 5f7b71f0..d51cc9a2 100644 --- a/src/multiconfigwidget.h +++ b/src/multiconfigwidget.h @@ -1,7 +1,6 @@ /* SPDX-FileCopyrightText: Lieven Hey - SPDX-FileCopyrightText: Milian Wolff - SPDX-FileCopyrightText: 2016-2022 Klarälvdalens Datakonsult AB, a KDAB Group company, info@kdab.com + SPDX-FileCopyrightText: 2022-2023 Klarälvdalens Datakonsult AB, a KDAB Group company, info@kdab.com SPDX-License-Identifier: GPL-2.0-or-later */ @@ -11,32 +10,46 @@ #include #include -class QComboBox; -class QPushButton; +namespace Ui { +class MultiConfigWidget; +} +/** this widget allows fast switching between different configurations + * of m_child + * saveConfig will automatically save all changes to m_group + * use setChildWidget for an unparented one and moveWidgetToChild for a parented one + * */ class MultiConfigWidget : public QWidget { Q_OBJECT public: MultiConfigWidget(QWidget* parent = nullptr); - ~MultiConfigWidget(); + ~MultiConfigWidget() override; - QString currentConfig() const; + /** widget is the widget containing the form which will be shown inside the MultiConfigWidget + * formWidgets is a list of all user editable widgets in widget that will automatically be saved and restored + * each widget in formWidgets needs a unique name **/ + void setChildWidget(QWidget* widget, const QVector& formWidgets); -signals: - void saveConfig(KConfigGroup group); - void restoreConfig(const KConfigGroup& group); + /** set group where everything should be saved in **/ + void setConfigGroup(const KConfigGroup& group); + + QString currentConfig() const; public slots: - void setConfig(const KConfigGroup& group); - void saveConfigAs(const QString& name); - void updateCurrentConfig(); - void selectConfig(const QString& name); - void restoreCurrent(); + void loadConfig(const QString& name); + void saveConfig(const QString& name); + void saveCurrentConfig(); + +signals: + void configsChanged(); + void currentConfigChanged(); private: - KConfigGroup m_config; - QComboBox* m_comboBox = nullptr; - QPushButton* m_copyButton = nullptr; - QPushButton* m_removeButton = nullptr; + QStringList configs() const; + std::unique_ptr m_configWidget; + QWidget* m_child = nullptr; + QVector m_formWidgets; + KConfigGroup m_group; + bool m_saving = false; }; diff --git a/src/multiconfigwidget.ui b/src/multiconfigwidget.ui new file mode 100644 index 00000000..736c75e8 --- /dev/null +++ b/src/multiconfigwidget.ui @@ -0,0 +1,115 @@ + + + MultiConfigWidget + + + + 0 + 0 + 400 + 300 + + + + + 0 + 0 + + + + Form + + + + 0 + + + 0 + + + 0 + + + 0 + + + 0 + + + + + + 0 + 0 + + + + + 0 + + + 0 + + + 0 + + + 0 + + + 0 + + + + + true + + + true + + + QComboBox::InsertAtCurrent + + + QComboBox::AdjustToContents + + + + + + + Qt::Horizontal + + + + 40 + 20 + + + + + + + + Copy + + + + + + + Delete + + + + + + + + + + + + + + diff --git a/src/recordpage.cpp b/src/recordpage.cpp index a6935e4f..665c2c26 100644 --- a/src/recordpage.cpp +++ b/src/recordpage.cpp @@ -40,7 +40,6 @@ #include #include -#include "multiconfigwidget.h" #include "perfoutputwidgetkonsole.h" #include "perfoutputwidgettext.h" #include "perfrecord.h" @@ -192,13 +191,19 @@ RecordPage::RecordPage(QWidget* parent) } }); - connect(m_recordHost, &RecordHost::clientApplicationChanged, this, [this](const QString& filePath) { - const auto config = applicationConfig(filePath); - ui->workingDirectory->setText(config.readEntry("workingDir", QString())); - ui->applicationParametersBox->setText(config.readEntry("params", QString())); + ui->multiConfig->setChildWidget(ui->launchWidget, {ui->applicationParametersBox, ui->workingDirectory}); - m_multiConfig->setConfig(applicationConfig(ui->applicationName->text())); - }); + connect(m_recordHost, &RecordHost::clientApplicationChanged, this, + [this](const QString& app) { ui->multiConfig->setConfigGroup(applicationConfig(app)); }); + + connect(ui->workingDirectory, qOverload(&KUrlRequester::returnPressed), this, + [this](const QString& cwd) { + ui->multiConfig->saveCurrentConfig(); + m_recordHost->setCurrentWorkingDirectory(cwd); + }); + + connect(ui->applicationParametersBox, &QLineEdit::editingFinished, this, + [this] { ui->multiConfig->saveCurrentConfig(); }); ui->compressionComboBox->addItem(tr("Disabled"), -1); ui->compressionComboBox->addItem(tr("Enabled (Default Level)"), 0); @@ -271,30 +276,6 @@ RecordPage::RecordPage(QWidget* parent) connect(m_perfOutput, &PerfOutputWidget::sendInput, this, [this](const QByteArray& input) { m_perfRecord->sendInput(input); }); - auto saveFunction = [this](KConfigGroup group) { - group.writeEntry("params", ui->applicationParametersBox->text()); - group.writeEntry("workingDir", ui->workingDirectory->text()); - }; - - auto restoreFunction = [this](const KConfigGroup& group) { - ui->applicationParametersBox->setText(group.readEntry("params", "")); - ui->workingDirectory->setText(group.readEntry("workingDir", "")); - setError({}); - }; - - m_multiConfig = new MultiConfigWidget(this); - - connect(m_multiConfig, &MultiConfigWidget::saveConfig, this, saveFunction); - connect(m_multiConfig, &MultiConfigWidget::restoreConfig, this, restoreFunction); - - m_multiConfig->setConfig(applicationConfig(ui->applicationName->text())); - - ui->launchAppBox->layout()->addWidget(m_multiConfig); - - connect(ui->applicationParametersBox, &QLineEdit::editingFinished, m_multiConfig, - &MultiConfigWidget::updateCurrentConfig); - connect(ui->workingDirectory, &KUrlRequester::textChanged, m_multiConfig, &MultiConfigWidget::updateCurrentConfig); - auto columnResizer = new KColumnResizer(this); columnResizer->addWidgetsFromLayout(ui->formLayout); columnResizer->addWidgetsFromLayout(ui->formLayout_1); diff --git a/src/recordpage.h b/src/recordpage.h index ad2c09e8..85b5087f 100644 --- a/src/recordpage.h +++ b/src/recordpage.h @@ -27,7 +27,6 @@ class RecordPage; class PerfRecord; class ProcessModel; class ProcessFilterModel; -class MultiConfigWidget; class PerfOutputWidget; namespace KParts { @@ -73,7 +72,6 @@ private slots: QTimer* m_updateRuntimeTimer; KParts::ReadOnlyPart* m_konsolePart = nullptr; QTemporaryFile* m_konsoleFile = nullptr; - MultiConfigWidget* m_multiConfig; PerfOutputWidget* m_perfOutput; ProcessModel* m_processModel; diff --git a/src/recordpage.ui b/src/recordpage.ui index 0c53b30f..3db5f6dc 100644 --- a/src/recordpage.ui +++ b/src/recordpage.ui @@ -54,74 +54,95 @@ Launch Application - - - QFormLayout::AllNonFixedFieldsGrow - - - - - Path to the application to be recorded - - - App&lication: - - - applicationName - - - - - - - Path to the application to be recorded - - - - - - - - - - Optional parameters to pass to the application being recorded - - - Parame&ters: - - - applicationParametersBox - - - - - - - Qt::NoContextMenu - - - Optional parameters to pass to the application being recorded - - - - - - - Directory to store the perf data file while recording - - - Wor&king Directory: - - - workingDirectory - - + + + - - - - Directory to store the perf data file while recording - + + + + + 0 + + + 0 + + + 0 + + + 0 + + + 0 + + + + + Path to the application to be recorded + + + + + + + + + + Path to the application to be recorded + + + App&lication: + + + applicationName + + + + + + + Optional parameters to pass to the application being recorded + + + Parame&ters: + + + applicationParametersBox + + + + + + + Directory to store the perf data file while recording + + + Wor&king Directory: + + + workingDirectory + + + + + + + Directory to store the perf data file while recording + + + + + + + Qt::NoContextMenu + + + Optional parameters to pass to the application being recorded + + + + @@ -620,6 +641,12 @@
kmessagewidget.h
1 + + MultiConfigWidget + QWidget +
multiconfigwidget.h
+ 1 + diff --git a/src/settingsdialog.cpp b/src/settingsdialog.cpp index 92b1a73a..edc810fd 100644 --- a/src/settingsdialog.cpp +++ b/src/settingsdialog.cpp @@ -30,11 +30,6 @@ #include namespace { -KConfigGroup config() -{ - return KSharedConfig::openConfig()->group("PerfPaths"); -} - QPushButton* setupMultiPath(KEditListWidget* listWidget, QLabel* buddy, QWidget* previous) { auto editor = new KUrlRequester(listWidget); @@ -85,44 +80,18 @@ void SettingsDialog::initSettings() { const auto configName = Settings::instance()->lastUsedEnvironment(); if (!configName.isEmpty()) { - m_configs->selectConfig(configName); - } -} - -void SettingsDialog::initSettings(const QString& sysroot, const QString& appPath, const QString& extraLibPaths, - const QString& debugPaths, const QString& kallsyms, const QString& arch, - const QString& objdump) -{ - auto fromPathString = [](KEditListWidget* listWidget, const QString& string) { - listWidget->setItems(string.split(QLatin1Char(':'), Qt::SkipEmptyParts)); - }; - fromPathString(unwindPage->extraLibraryPaths, extraLibPaths); - fromPathString(unwindPage->debugPaths, debugPaths); - - unwindPage->lineEditSysroot->setText(sysroot); - unwindPage->lineEditApplicationPath->setText(appPath); - unwindPage->lineEditKallsyms->setText(kallsyms); - unwindPage->lineEditObjdump->setText(objdump); - - int itemIndex = 0; - if (!arch.isEmpty()) { - itemIndex = unwindPage->comboBoxArchitecture->findText(arch); - if (itemIndex == -1) { - itemIndex = unwindPage->comboBoxArchitecture->count(); - unwindPage->comboBoxArchitecture->addItem(arch); - } + unwindPage->multiConfig->loadConfig(configName); } - unwindPage->comboBoxArchitecture->setCurrentIndex(itemIndex); } QString SettingsDialog::sysroot() const { - return unwindPage->lineEditSysroot->text(); + return unwindPage->sysroot->text(); } QString SettingsDialog::appPath() const { - return unwindPage->lineEditApplicationPath->text(); + return unwindPage->appPath->text(); } QString SettingsDialog::extraLibPaths() const @@ -137,18 +106,18 @@ QString SettingsDialog::debugPaths() const QString SettingsDialog::kallsyms() const { - return unwindPage->lineEditKallsyms->text(); + return unwindPage->kallsyms->text(); } QString SettingsDialog::arch() const { - const auto sArch = unwindPage->comboBoxArchitecture->currentText(); + const auto sArch = unwindPage->arch->currentText(); return (sArch == QLatin1String("auto-detect")) ? QString() : sArch; } QString SettingsDialog::objdump() const { - return unwindPage->lineEditObjdump->text(); + return unwindPage->objdump->text(); } QString SettingsDialog::perfMapPath() const @@ -178,61 +147,22 @@ void SettingsDialog::addPathSettingsPage() auto item = addPage(page, tr("Unwinding")); item->setHeader(tr("Unwind Options")); item->setIcon(icon()); - unwindPage->setupUi(page); - auto lastExtraLibsWidget = setupMultiPath(unwindPage->extraLibraryPaths, unwindPage->extraLibraryPathsLabel, - unwindPage->lineEditApplicationPath); - setupMultiPath(unwindPage->debugPaths, unwindPage->debugPathsLabel, lastExtraLibsWidget); - - auto* label = new QLabel(this); - label->setText(tr("Config:")); - - auto saveFunction = [this](KConfigGroup group) { - group.writeEntry("sysroot", sysroot()); - group.writeEntry("appPath", appPath()); - group.writeEntry("extraLibPaths", extraLibPaths()); - group.writeEntry("debugPaths", debugPaths()); - group.writeEntry("kallsyms", kallsyms()); - group.writeEntry("arch", arch()); - group.writeEntry("objdump", objdump()); - }; - - auto restoreFunction = [this](const KConfigGroup& group) { - const auto sysroot = group.readEntry("sysroot"); - const auto appPath = group.readEntry("appPath"); - const auto extraLibPaths = group.readEntry("extraLibPaths"); - const auto debugPaths = group.readEntry("debugPaths"); - const auto kallsyms = group.readEntry("kallsyms"); - const auto arch = group.readEntry("arch"); - const auto objdump = group.readEntry("objdump"); - initSettings(sysroot, appPath, extraLibPaths, debugPaths, kallsyms, arch, objdump); - ::config().writeEntry("lastUsed", m_configs->currentConfig()); - }; - - m_configs = new MultiConfigWidget(this); - m_configs->setConfig(config()); - m_configs->restoreCurrent(); - - connect(m_configs, &MultiConfigWidget::saveConfig, this, saveFunction); - connect(m_configs, &MultiConfigWidget::restoreConfig, this, restoreFunction); - - unwindPage->formLayout->insertRow(0, label, m_configs); - - connect(this, &KPageDialog::accepted, this, [this] { m_configs->updateCurrentConfig(); }); - - for (auto field : {unwindPage->lineEditSysroot, unwindPage->lineEditApplicationPath, unwindPage->lineEditKallsyms, - unwindPage->lineEditObjdump}) { - connect(field, &KUrlRequester::textEdited, m_configs, &MultiConfigWidget::updateCurrentConfig); - connect(field, &KUrlRequester::urlSelected, m_configs, &MultiConfigWidget::updateCurrentConfig); - } + unwindPage->multiConfig->setConfigGroup(KSharedConfig::openConfig()->group("PerfPaths")); + unwindPage->multiConfig->setChildWidget(unwindPage->widget, + {unwindPage->sysroot, unwindPage->appPath, unwindPage->extraLibraryPaths, + unwindPage->debugPaths, unwindPage->kallsyms, unwindPage->arch, + unwindPage->objdump}); - connect(unwindPage->comboBoxArchitecture, QOverload::of(&QComboBox::currentIndexChanged), m_configs, - &MultiConfigWidget::updateCurrentConfig); + auto lastExtraLibsWidget = + setupMultiPath(unwindPage->extraLibraryPaths, unwindPage->extraLibraryPathsLabel, unwindPage->appPath); + setupMultiPath(unwindPage->debugPaths, unwindPage->debugPathsLabel, lastExtraLibsWidget); - connect(unwindPage->debugPaths, &KEditListWidget::changed, m_configs, &MultiConfigWidget::updateCurrentConfig); - connect(unwindPage->extraLibraryPaths, &KEditListWidget::changed, m_configs, - &MultiConfigWidget::updateCurrentConfig); + connect(buttonBox(), &QDialogButtonBox::accepted, this, [this] { + unwindPage->multiConfig->saveCurrentConfig(); + Settings::instance()->setLastUsedEnvironment(unwindPage->multiConfig->currentConfig()); + }); } void SettingsDialog::keyPressEvent(QKeyEvent* event) diff --git a/src/settingsdialog.h b/src/settingsdialog.h index 0b396d1a..ed8a6476 100644 --- a/src/settingsdialog.h +++ b/src/settingsdialog.h @@ -58,5 +58,4 @@ class SettingsDialog : public KPageDialog std::unique_ptr debuginfodPage; std::unique_ptr disassemblyPage; std::unique_ptr callgraphPage; - MultiConfigWidget* m_configs; }; diff --git a/src/unwindsettingspage.ui b/src/unwindsettingspage.ui index 5c044135..066a7164 100644 --- a/src/unwindsettingspage.ui +++ b/src/unwindsettingspage.ui @@ -16,186 +16,210 @@ true - - - - - Path to the sysroot. Leave empty to use the local machine. - - - local machine - - - - - - - Path to the application binary and library. - - - Application Path: - - - lineEditApplicationPath - - - - - - - Path to the application binary and library. - - - auto-detect - - - - - - - Qt::TabFocus - - - List of paths that contain additional libraries. - - - - - - - List of that contain debug information. - - - Debug Paths: - - - - - - - Qt::TabFocus - - - List of paths that contain debug information. - - - - - - - Path to the kernel symbol mapping. - - - Kallsyms: - - - lineEditKallsyms - - - - - - - Path to the kernel symbol mapping. - - - auto-detect - - - - - - - - - - System architecture, e.g. x86_64, arm, aarch64 etc. - - - Architecture: - - - comboBoxArchitecture - - - - - - - System architecture, e.g. x86_64, arm, aarch64 etc. - - - true - - - - auto-detect - - - - - x64 - - - - - ARMv7 (32 bit) - - - - - ARMv8 (64 bit) - - - - - - - - <qt>The <tt>objdump</tt> binary that will be used for the disassembly view. Leave empty to let it be auto-detected.</qt> - - - Objdump: - - - lineEditObjdump - - + + + 0 + + + 0 + + + 0 + + + 0 + + + 0 + + + - - - - <qt>The <tt>objdump</tt> binary that will be used for the disassembly view. Leave empty to let it be auto-detected.</qt> - - - auto-detect - - - - - - - - - - List of paths that contain additional libraries. - - - Extra Library Paths: - - - - - - - Path to the sysroot. Leave empty to use the local machine. - - - Sysroot: - - - lineEditSysroot - + + + + + + + Path to the application binary and library. + + + auto-detect + + + + + + + Path to the sysroot. Leave empty to use the local machine. + + + local machine + + + + + + + Qt::TabFocus + + + List of paths that contain additional libraries. + + + + + + + List of paths that contain additional libraries. + + + Extra Library Paths: + + + + + + + Qt::TabFocus + + + List of paths that contain debug information. + + + + + + + Path to the kernel symbol mapping. + + + auto-detect + + + + + + + + + + Path to the kernel symbol mapping. + + + Kallsyms: + + + kallsyms + + + + + + + List of that contain debug information. + + + Debug Paths: + + + + + + + Path to the application binary and library. + + + Application Path: + + + appPath + + + + + + + Path to the sysroot. Leave empty to use the local machine. + + + Sysroot: + + + sysroot + + + + + + + System architecture, e.g. x86_64, arm, aarch64 etc. + + + Architecture: + + + arch + + + + + + + System architecture, e.g. x86_64, arm, aarch64 etc. + + + true + + + + auto-detect + + + + + x64 + + + + + ARMv7 (32 bit) + + + + + ARMv8 (64 bit) + + + + + + + + <qt>The <tt>objdump</tt> binary that will be used for the disassembly view. Leave empty to let it be auto-detected.</qt> + + + auto-detect + + + + + + + + + + <qt>The <tt>objdump</tt> binary that will be used for the disassembly view. Leave empty to let it be auto-detected.</qt> + + + Objdump: + + + objdump + + + + @@ -225,16 +249,13 @@ QWidget
keditlistwidget.h
+ + MultiConfigWidget + QWidget +
multiconfigwidget.h
+ 1 + - - lineEditSysroot - lineEditApplicationPath - extraLibraryPaths - debugPaths - lineEditKallsyms - comboBoxArchitecture - lineEditObjdump - From f4029d15ae7dee35d1ed10c3d543e993f3e33065 Mon Sep 17 00:00:00 2001 From: Lieven Hey Date: Tue, 12 Sep 2023 16:27:09 +0200 Subject: [PATCH 2/7] add ssh settings page you can edit you devices, copy your ssh key and select you ssh binary on this page device are stored directly in a KConfigGroup, the deviceList property of Settings is only there to pass the devices around to other widgets --- src/recordhost.h | 1 + src/settings.cpp | 32 +++++++ src/settings.h | 25 ++++++ src/settingsdialog.cpp | 69 +++++++++++++++ src/settingsdialog.h | 3 + src/sshsettingspage.ui | 188 +++++++++++++++++++++++++++++++++++++++++ 6 files changed, 318 insertions(+) create mode 100644 src/sshsettingspage.ui diff --git a/src/recordhost.h b/src/recordhost.h index c914193c..04b1aafb 100644 --- a/src/recordhost.h +++ b/src/recordhost.h @@ -14,6 +14,7 @@ enum class RecordType { LaunchApplication, + LaunchRemoteApplication, AttachToProcess, ProfileSystem, NUM_RECORD_TYPES diff --git a/src/settings.cpp b/src/settings.cpp index 03c33a9c..3af62262 100644 --- a/src/settings.cpp +++ b/src/settings.cpp @@ -243,6 +243,14 @@ void Settings::loadFromFile() connect(this, &Settings::showBranchesChanged, [sharedConfig](bool showBranches) { sharedConfig->group("Disassembly").writeEntry("showBranches", showBranches); }); + + setSSHPath(sharedConfig->group("SSH").readEntry("ssh")); + setSSHCopyKeyPath(sharedConfig->group("SSH").readEntry("ssh-copy-id")); + + connect(this, &Settings::sshPathChanged, this, + [sharedConfig](const QString& path) { sharedConfig->group("SSH").writeEntry("ssh", path); }); + connect(this, &Settings::sshCopyIdPathChanged, this, + [sharedConfig](const QString& path) { sharedConfig->group("SSH").writeEntry("ssh-copy-id", path); }); } void Settings::setSourceCodePaths(const QString& paths) @@ -268,3 +276,27 @@ void Settings::setShowBranches(bool showBranches) emit showBranchesChanged(m_showBranches); } } + +void Settings::setSSHPath(const QString& sshPath) +{ + if (m_sshPath != sshPath) { + m_sshPath = sshPath; + emit sshPathChanged(m_sshPath); + } +} + +void Settings::setSSHCopyKeyPath(const QString& sshCopyIdPath) +{ + if (m_sshCopyIdPath != sshCopyIdPath) { + m_sshCopyIdPath = sshCopyIdPath; + emit sshCopyIdPathChanged(m_sshCopyIdPath); + } +} + +void Settings::setDevices(const QStringList& devices) +{ + if (m_devices != devices) { + m_devices = devices; + emit devicesChanged(m_devices); + } +} diff --git a/src/settings.h b/src/settings.h index b038a5af..fd42be0a 100644 --- a/src/settings.h +++ b/src/settings.h @@ -159,6 +159,21 @@ class Settings : public QObject return m_showBranches; } + QString sshPath() const + { + return m_sshPath; + } + + QString sshCopyIdPath() const + { + return m_sshCopyIdPath; + } + + QStringList devices() const + { + return m_devices; + } + void loadFromFile(); signals: @@ -182,6 +197,9 @@ class Settings : public QObject void sourceCodePathsChanged(const QString& paths); void perfPathChanged(const QString& perfPath); void showBranchesChanged(bool showBranches); + void sshPathChanged(const QString& sshPath); + void sshCopyIdPathChanged(const QString& sshCopyIdPath); + void devicesChanged(const QStringList& devices); public slots: void setPrettifySymbols(bool prettifySymbols); @@ -206,6 +224,9 @@ public slots: void setSourceCodePaths(const QString& paths); void setPerfPath(const QString& path); void setShowBranches(bool showBranches); + void setSSHPath(const QString& path); + void setSSHCopyKeyPath(const QString& path); + void setDevices(const QStringList& devices); private: Settings() = default; @@ -239,4 +260,8 @@ public slots: QColor m_callgraphColor; QString m_perfPath; + QString m_sshPath; + QString m_sshCopyIdPath; + + QStringList m_devices; }; diff --git a/src/settingsdialog.cpp b/src/settingsdialog.cpp index edc810fd..ff81f52f 100644 --- a/src/settingsdialog.cpp +++ b/src/settingsdialog.cpp @@ -13,6 +13,8 @@ #include "ui_disassemblysettingspage.h" #include "ui_flamegraphsettingspage.h" #include "ui_perfsettingspage.h" +#include "ui_sourcepathsettings.h" +#include "ui_sshsettingspage.h" #include "ui_unwindsettingspage.h" #include "multiconfigwidget.h" @@ -26,6 +28,7 @@ #include #include #include +#include #include @@ -63,6 +66,7 @@ SettingsDialog::SettingsDialog(QWidget* parent) #if KGraphViewerPart_FOUND , callgraphPage(new Ui::CallgraphSettingsPage) #endif + , sshSettingsPage(new Ui::SSHSettingsPage) { addPerfSettingsPage(); addPathSettingsPage(); @@ -72,6 +76,7 @@ SettingsDialog::SettingsDialog(QWidget* parent) addCallgraphPage(); #endif addSourcePathPage(); + addSSHPage(); } SettingsDialog::~SettingsDialog() = default; @@ -272,3 +277,67 @@ void SettingsDialog::addSourcePathPage() settings->setShowBranches(disassemblyPage->showBranches->isChecked()); }); } + +void SettingsDialog::addSSHPage() +{ + auto page = new QWidget(this); + auto item = addPage(page, tr("SSH Settings")); + item->setHeader(tr("SSH Settings Page")); + item->setIcon(QIcon::fromTheme(QStringLiteral("preferences-system-windows-behavior"))); + sshSettingsPage->setupUi(page); + sshSettingsPage->messageWidget->hide(); + sshSettingsPage->errorWidget->hide(); + + auto configGroup = KSharedConfig::openConfig()->group("SSH"); + sshSettingsPage->deviceConfig->setChildWidget( + sshSettingsPage->deviceSettings, + {sshSettingsPage->username, sshSettingsPage->hostname, sshSettingsPage->options}); + sshSettingsPage->deviceConfig->setConfigGroup(configGroup); + + connect(sshSettingsPage->copySshKeyButton, &QPushButton::pressed, this, [this] { + auto* copyKey = new QProcess(this); + + auto path = sshSettingsPage->sshCopyIdPath->text(); + if (path.isEmpty()) { + path = QStandardPaths::findExecutable(QStringLiteral("ssh-copy-id")); + } + if (path.isEmpty()) { + sshSettingsPage->messageWidget->setText(tr("Could not find ssh-copy-id")); + sshSettingsPage->messageWidget->show(); + return; + } + + copyKey->setProgram(path); + + QStringList arguments = {}; + auto options = sshSettingsPage->options->text(); + if (!options.isEmpty()) { + arguments.append(options.split(QLatin1Char(' '))); + } + arguments.append( + QStringLiteral("%1@%2").arg(sshSettingsPage->username->text(), sshSettingsPage->hostname->text())); + copyKey->setArguments(arguments); + + connect(copyKey, qOverload(&QProcess::finished), this, + [this, copyKey](int code, QProcess::ExitStatus) { + if (code == 0) { + sshSettingsPage->messageWidget->setText(QStringLiteral("Copy key successfully")); + sshSettingsPage->messageWidget->show(); + } else { + sshSettingsPage->errorWidget->setText(QStringLiteral("Failed to copy key")); + sshSettingsPage->errorWidget->show(); + } + copyKey->deleteLater(); + }); + copyKey->start(); + }); + + connect(buttonBox(), &QDialogButtonBox::accepted, this, [this, configGroup] { + sshSettingsPage->deviceConfig->saveCurrentConfig(); + + auto settings = Settings::instance(); + settings->setDevices(configGroup.groupList()); + settings->setSSHPath(sshSettingsPage->sshBinary->text()); + settings->setSSHCopyKeyPath(sshSettingsPage->sshCopyIdPath->text()); + }); +} diff --git a/src/settingsdialog.h b/src/settingsdialog.h index ed8a6476..1288093f 100644 --- a/src/settingsdialog.h +++ b/src/settingsdialog.h @@ -19,6 +19,7 @@ class DebuginfodPage; class CallgraphSettingsPage; class DisassemblySettingsPage; class PerfSettingsPage; +class SSHSettingsPage; } class MultiConfigWidget; @@ -51,6 +52,7 @@ class SettingsDialog : public KPageDialog void addDebuginfodPage(); void addCallgraphPage(); void addSourcePathPage(); + void addSSHPage(); std::unique_ptr perfPage; std::unique_ptr unwindPage; @@ -58,4 +60,5 @@ class SettingsDialog : public KPageDialog std::unique_ptr debuginfodPage; std::unique_ptr disassemblyPage; std::unique_ptr callgraphPage; + std::unique_ptr sshSettingsPage; }; diff --git a/src/sshsettingspage.ui b/src/sshsettingspage.ui new file mode 100644 index 00000000..7151bfd8 --- /dev/null +++ b/src/sshsettingspage.ui @@ -0,0 +1,188 @@ + + + SSHSettingsPage + + + + 0 + 0 + 400 + 300 + + + + Form + + + + 0 + + + 0 + + + 0 + + + 0 + + + 0 + + + + + + + + KMessageWidget::Error + + + + + + + + + + + 0 + + + 0 + + + 0 + + + 0 + + + 0 + + + 0 + + + + + Hostname: + + + + + + + + + + Username: + + + + + + + + + + Options: + + + + + + + + + + Copy SSH Key + + + + + + + + + + Qt::Horizontal + + + + + + + + 0 + + + 0 + + + 0 + + + 0 + + + 0 + + + 0 + + + + + ssh: + + + + + + + ssh + + + + + + + ssh-copy-id: + + + + + + + ssh-copy-id + + + + + + + + + + + KUrlRequester + QWidget +
kurlrequester.h
+ + + KMessageWidget + QFrame +
kmessagewidget.h
+ 1 + + + MultiConfigWidget + QWidget +
multiconfigwidget.h
+ 1 + + + + + From d7ecf5e7fdfffbc11672be88bb975a21205121f1 Mon Sep 17 00:00:00 2001 From: Lieven Hey Date: Wed, 13 Sep 2023 16:49:24 +0200 Subject: [PATCH 3/7] update record host to work with remote devices Changes to existing logic: RecordHost now saved the client app args RecordHost no longer differentiate between local and remote when setting the output file name, since the file is always saved on the host New class RemoteDevice, this class handles communication between the host and the remote device. It does this by using a multiplexed ssh connection. This is faster since we only need to do authentification once and can then reuse the existing connection. --- src/CMakeLists.txt | 1 + src/recordhost.cpp | 190 ++++++++++++++++++-------- src/recordhost.h | 11 ++ src/recordpage.cpp | 6 +- src/remotedevice.cpp | 188 +++++++++++++++++++++++++ src/remotedevice.h | 50 +++++++ src/settings.cpp | 6 + tests/integrationtests/CMakeLists.txt | 1 + 8 files changed, 393 insertions(+), 60 deletions(-) create mode 100644 src/remotedevice.cpp create mode 100644 src/remotedevice.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 66e453ab..b3220c20 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -55,6 +55,7 @@ set(HOTSPOT_SRCS errnoutil.cpp recordhost.cpp copyabletreeview.cpp + remotedevice.cpp # ui files: mainwindow.ui aboutdialog.ui diff --git a/src/recordhost.cpp b/src/recordhost.cpp index e588948c..94abcb35 100644 --- a/src/recordhost.cpp +++ b/src/recordhost.cpp @@ -142,12 +142,35 @@ RecordHost::PerfCapabilities fetchLocalPerfCapabilities(const QString& perfPath) return capabilities; } + +RecordHost::PerfCapabilities fetchRemotePerfCapabilities(const RemoteDevice& device) +{ + RecordHost::PerfCapabilities capabilities; + + const auto buildOptions = + device.getProgramOutput({QStringLiteral("perf"), QStringLiteral("version"), QStringLiteral("--build-options")}); + const auto help = device.getProgramOutput({QStringLiteral("perf"), QStringLiteral("--help")}); + + capabilities.canCompress = Zstd_FOUND && buildOptions.contains("zszd: [ on ]"); + capabilities.canSwitchEvents = help.contains("--switch-events"); + capabilities.canSampleCpu = help.contains("--sample-cpu"); + + // TODO: implement + capabilities.canProfileOffCpu = false; + capabilities.privilegesAlreadyElevated = false; + + capabilities.canUseAio = false; // AIO doesn't work with perf streaming + capabilities.canElevatePrivileges = false; // we currently don't support this + + return capabilities; +} } RecordHost::RecordHost(QObject* parent) : QObject(parent) , m_checkPerfCapabilitiesJob(this) , m_checkPerfInstalledJob(this) + , m_remoteDevice(this) { connect(this, &RecordHost::errorOccurred, this, [this](const QString& message) { m_error = message; }); @@ -162,6 +185,10 @@ RecordHost::RecordHost(QObject* parent) connectIsReady(&RecordHost::pidsChanged); connectIsReady(&RecordHost::currentWorkingDirectoryChanged); + connect(&m_remoteDevice, &RemoteDevice::connected, this, &RecordHost::checkRequirements); + + connect(&m_remoteDevice, &RemoteDevice::connected, this, [this] { emit isReadyChanged(isReady()); }); + setHost(QStringLiteral("localhost")); } @@ -171,7 +198,13 @@ bool RecordHost::isReady() const { switch (m_recordType) { case RecordType::LaunchApplication: - // client application is already validated in the setter + // client application is already validated in the setter + if (m_clientApplication.isEmpty() && m_cwd.isEmpty()) + return false; + break; + case RecordType::LaunchRemoteApplication: + if (!m_remoteDevice.isConnected()) + return false; if (m_clientApplication.isEmpty() && m_cwd.isEmpty()) return false; break; @@ -212,38 +245,19 @@ void RecordHost::setHost(const QString& host) m_clientApplication.clear(); emit clientApplicationChanged(m_clientApplication); + m_clientApplicationArguments.clear(); + emit clientApplicationArgumentsChanged(m_clientApplicationArguments); + m_perfCapabilities = {}; emit perfCapabilitiesChanged(m_perfCapabilities); - const auto perfPath = perfBinaryPath(); - m_checkPerfCapabilitiesJob.startJob([perfPath](auto&&) { return fetchLocalPerfCapabilities(perfPath); }, - [this](RecordHost::PerfCapabilities capabilities) { - Q_ASSERT(QThread::currentThread() == thread()); - - m_perfCapabilities = capabilities; - emit perfCapabilitiesChanged(m_perfCapabilities); - }); - - m_checkPerfInstalledJob.startJob( - [isLocal = isLocal(), perfPath](auto&&) { - if (isLocal) { - if (perfPath.isEmpty()) { - return !QStandardPaths::findExecutable(QStringLiteral("perf")).isEmpty(); - } - - return QFileInfo::exists(perfPath); - } - - qWarning() << "remote is not implemented"; - return false; - }, - [this](bool isInstalled) { - if (!isInstalled) { - emit errorOccurred(tr("perf is not installed")); - } - m_isPerfInstalled = isInstalled; - emit isPerfInstalledChanged(isInstalled); - }); + m_remoteDevice.disconnect(); + if (isLocal()) { + checkRequirements(); + } else { + // checkRequirements will be called via RemoteDevice::connected + m_remoteDevice.connectToDevice(m_host); + } } void RecordHost::setCurrentWorkingDirectory(const QString& cwd) @@ -264,16 +278,25 @@ void RecordHost::setCurrentWorkingDirectory(const QString& cwd) m_cwd = cwd; emit currentWorkingDirectoryChanged(cwd); } - return; + } else { + if (!m_remoteDevice.checkIfDirectoryExists(cwd)) { + emit errorOccurred(tr("Working directory folder cannot be found: %1").arg(cwd)); + } else { + emit errorOccurred({}); + m_cwd = cwd; + emit currentWorkingDirectoryChanged(m_cwd); + } } - - qWarning() << "is not implemented for remote"; } void RecordHost::setClientApplication(const QString& clientApplication) { Q_ASSERT(QThread::currentThread() == thread()); + if (m_clientApplication == clientApplication) { + return; + } + if (isLocal()) { QFileInfo application(KShell::tildeExpand(clientApplication)); if (!application.exists()) { @@ -295,38 +318,48 @@ void RecordHost::setClientApplication(const QString& clientApplication) if (m_cwd.isEmpty()) { setCurrentWorkingDirectory(application.dir().absolutePath()); } - return; + } else { + if (!m_remoteDevice.checkIfFileExists(clientApplication)) { + emit errorOccurred(tr("Application file cannot be found: %1").arg(clientApplication)); + } else { + emit errorOccurred({}); + m_clientApplication = clientApplication; + emit clientApplicationChanged(m_clientApplication); + } } - - qWarning() << "is not implemented for remote"; } -void RecordHost::setOutputFileName(const QString& filePath) +void RecordHost::setClientApplicationArguments(const QString& arguments) { - if (isLocal()) { - const auto perfDataExtension = QStringLiteral(".data"); - - const QFileInfo file(filePath); - const QFileInfo folder(file.absolutePath()); - - if (!folder.exists()) { - emit errorOccurred(tr("Output file directory folder cannot be found: %1").arg(folder.path())); - } else if (!folder.isDir()) { - emit errorOccurred(tr("Output file directory folder is not valid: %1").arg(folder.path())); - } else if (!folder.isWritable()) { - emit errorOccurred(tr("Output file directory folder is not writable: %1").arg(folder.path())); - } else if (!file.absoluteFilePath().endsWith(perfDataExtension)) { - emit errorOccurred(tr("Output file must end with %1").arg(perfDataExtension)); - } else { - emit errorOccurred({}); - m_outputFileName = filePath; - emit outputFileNameChanged(m_outputFileName); - } + Q_ASSERT(QThread::currentThread() == thread()); - return; + if (m_clientApplicationArguments != arguments) { + m_clientApplicationArguments = arguments; + emit clientApplicationArgumentsChanged(m_clientApplicationArguments); } +} - qWarning() << "is not implemented for remote"; +void RecordHost::setOutputFileName(const QString& filePath) +{ + const auto perfDataExtension = QStringLiteral(".data"); + const QFileInfo file(filePath); + const QFileInfo folder(file.absolutePath()); + + // the recording data is streamed from the device (currently) so there is no need to use different logic for local + // vs remote + if (!folder.exists()) { + emit errorOccurred(tr("Output file directory folder cannot be found: %1").arg(folder.path())); + } else if (!folder.isDir()) { + emit errorOccurred(tr("Output file directory folder is not valid: %1").arg(folder.path())); + } else if (!folder.isWritable()) { + emit errorOccurred(tr("Output file directory folder is not writable: %1").arg(folder.path())); + } else if (!file.absoluteFilePath().endsWith(perfDataExtension)) { + emit errorOccurred(tr("Output file must end with %1").arg(perfDataExtension)); + } else { + emit errorOccurred({}); + m_outputFileName = filePath; + emit outputFileNameChanged(m_outputFileName); + } } void RecordHost::setRecordType(RecordType type) @@ -368,3 +401,44 @@ QString RecordHost::perfBinaryPath() const } return {}; } + +void RecordHost::checkRequirements() +{ + const auto perfPath = perfBinaryPath(); + m_checkPerfCapabilitiesJob.startJob( + [isLocal = isLocal(), &remoteDevice = m_remoteDevice, perfPath](auto&&) { + if (isLocal) { + return fetchLocalPerfCapabilities(perfPath); + } else { + return fetchRemotePerfCapabilities(remoteDevice); + } + }, + [this](RecordHost::PerfCapabilities capabilities) { + Q_ASSERT(QThread::currentThread() == thread()); + + m_perfCapabilities = capabilities; + emit perfCapabilitiesChanged(m_perfCapabilities); + }); + + m_checkPerfInstalledJob.startJob( + [isLocal = isLocal(), &remoteDevice = m_remoteDevice, perfPath](auto&&) { + if (isLocal) { + if (perfPath.isEmpty()) { + return !QStandardPaths::findExecutable(QStringLiteral("perf")).isEmpty(); + } + + return QFileInfo::exists(perfPath); + } else { + return remoteDevice.checkIfProgramExists(QStringLiteral("perf")); + } + + return false; + }, + [this](bool isInstalled) { + if (!isInstalled) { + emit errorOccurred(tr("perf is not installed")); + } + m_isPerfInstalled = isInstalled; + emit isPerfInstalledChanged(isInstalled); + }); +} diff --git a/src/recordhost.h b/src/recordhost.h index 04b1aafb..2bf40256 100644 --- a/src/recordhost.h +++ b/src/recordhost.h @@ -8,6 +8,7 @@ #pragma once #include "jobtracker.h" +#include "remotedevice.h" #include @@ -60,6 +61,12 @@ class RecordHost : public QObject } void setClientApplication(const QString& clientApplication); + QString clientApplicationArguments() const + { + return m_clientApplicationArguments; + } + void setClientApplicationArguments(const QString& arguments); + QString outputFileName() const { return m_outputFileName; @@ -105,6 +112,7 @@ class RecordHost : public QObject void hostChanged(); void currentWorkingDirectoryChanged(const QString& cwd); // Maybe QUrl void clientApplicationChanged(const QString& clientApplication); + void clientApplicationArgumentsChanged(const QString& arguments); void perfCapabilitiesChanged(RecordHost::PerfCapabilities perfCapabilities); void isPerfInstalledChanged(bool isInstalled); void outputFileNameChanged(const QString& outputFileName); @@ -112,12 +120,14 @@ class RecordHost : public QObject void pidsChanged(); private: + void checkRequirements(); bool isLocal() const; QString m_host; QString m_error; QString m_cwd; QString m_clientApplication; + QString m_clientApplicationArguments; QString m_outputFileName; PerfCapabilities m_perfCapabilities; JobTracker m_checkPerfCapabilitiesJob; @@ -125,6 +135,7 @@ class RecordHost : public QObject RecordType m_recordType = RecordType::LaunchApplication; bool m_isPerfInstalled = false; QStringList m_pids; + RemoteDevice m_remoteDevice; }; Q_DECLARE_METATYPE(RecordHost::PerfCapabilities) diff --git a/src/recordpage.cpp b/src/recordpage.cpp index 665c2c26..c8d133b0 100644 --- a/src/recordpage.cpp +++ b/src/recordpage.cpp @@ -202,8 +202,10 @@ RecordPage::RecordPage(QWidget* parent) m_recordHost->setCurrentWorkingDirectory(cwd); }); - connect(ui->applicationParametersBox, &QLineEdit::editingFinished, this, - [this] { ui->multiConfig->saveCurrentConfig(); }); + connect(ui->applicationParametersBox, &QLineEdit::editingFinished, this, [this] { + ui->multiConfig->saveCurrentConfig(); + m_recordHost->setClientApplicationArguments(ui->applicationParametersBox->text()); + }); ui->compressionComboBox->addItem(tr("Disabled"), -1); ui->compressionComboBox->addItem(tr("Enabled (Default Level)"), 0); diff --git a/src/remotedevice.cpp b/src/remotedevice.cpp new file mode 100644 index 00000000..6ec7b496 --- /dev/null +++ b/src/remotedevice.cpp @@ -0,0 +1,188 @@ +/* + SPDX-FileCopyrightText: Lieven Hey + SPDX-FileCopyrightText: 2023 Klarälvdalens Datakonsult AB, a KDAB Group company, info@kdab.com + + SPDX-License-Identifier: GPL-2.0-or-later +*/ + +#include "remotedevice.h" + +#include +#include +#include + +#include + +#include "settings.h" + +// TODO: remove +#include + +namespace { +QStringList sshArgs(const QString& dir) +{ + return {QStringLiteral("-o"), QStringLiteral("ControlMaster=auto"), QStringLiteral("-o"), + QStringLiteral("ControlPath=%1/ssh").arg(dir)}; +} + +void setupProcess(QProcess* process, const QString& sshBinary, const KConfigGroup& config, const QString& path, + const QStringList& args) +{ + process->setProgram(sshBinary); + const auto options = config.readEntry("options", QString {}); + const auto username = config.readEntry("username", QString {}); + const auto hostname = config.readEntry("hostname", QString {}); + + auto arguments = sshArgs(path); + if (!options.isEmpty()) { + arguments.append(options.split(QLatin1Char(' '))); + } + + arguments.append(QStringLiteral("%1@%2").arg(username, hostname)); + if (!args.isEmpty()) { + arguments.append(args); + } + process->setArguments(arguments); +} +} + +RemoteDevice::RemoteDevice(QObject* parent) + : QObject(parent) +{ + Q_ASSERT(m_tempDir.isValid()); + m_watcher.addPath(m_tempDir.path()); + + connect(&m_watcher, &QFileSystemWatcher::directoryChanged, this, [this](const QString& path) { + // this could also be a delete so we need to check if the socket exists + if (QFile::exists(path + QStringLiteral("/ssh"))) { + // ssh only creates that file when the connection was established + emit connected(); + } + }); + + auto findSSHBinary = [this](const QString& binary) { + if (binary.isEmpty()) { + m_sshBinary = QStandardPaths::findExecutable(QStringLiteral("ssh")); + } else { + m_sshBinary = binary; + } + }; + + auto settings = Settings::instance(); + findSSHBinary(settings->sshPath()); + connect(settings, &Settings::sshPathChanged, this, findSSHBinary); +} + +RemoteDevice::~RemoteDevice() = default; + +void RemoteDevice::connectToDevice(const QString& device) +{ + if (m_connection) { + disconnect(); + } + auto config = KSharedConfig::openConfig()->group("SSH"); + if (!config.hasGroup(device) && config.group(device).exists()) { + emit failedToConnect(); + return; + } + m_config = config.group(device); + + m_connection = sshProcess({}); + + connect(m_connection, qOverload(&QProcess::finished), this, + [connection = m_connection, path = m_tempDir.path(), this](int exitCode, QProcess::ExitStatus) { + if (exitCode != 0) { + emit failedToConnect(); + } else { + emit disconnected(); + } + qDebug() << exitCode << connection->readAll(); + connection->deleteLater(); + }); + + m_connection->start(); +} + +void RemoteDevice::disconnect() +{ + if (m_connection) { + if (m_connection->state() == QProcess::ProcessState::Running) { + // ssh stops once you clode the write channel + // we then use the finished signal for cleanup + m_connection->closeWriteChannel(); + } + + m_connection = nullptr; + } +} + +bool RemoteDevice::isConnected() const +{ + return QFile(m_tempDir.path() + QStringLiteral("/ssh")).exists(); +} + +bool RemoteDevice::checkIfProgramExists(const QString& program) const +{ + auto ssh = sshProcess({QStringLiteral("command"), program}); + ssh->start(); + ssh->waitForFinished(); + auto exitCode = ssh->exitCode(); + ssh->deleteLater(); + // 128 -> not found + // perf will return 1 and display the help message + return exitCode != 128; +} + +bool RemoteDevice::checkIfDirectoryExists(const QString& directory) const +{ + auto ssh = sshProcess({QStringLiteral("test"), QStringLiteral("-d"), directory}); + ssh->start(); + ssh->waitForFinished(); + auto exitCode = ssh->exitCode(); + ssh->deleteLater(); + return exitCode == 0; +} + +bool RemoteDevice::checkIfFileExists(const QString& file) const +{ + auto ssh = sshProcess({QStringLiteral("test"), QStringLiteral("-f"), file}); + ssh->start(); + ssh->waitForFinished(); + auto exitCode = ssh->exitCode(); + ssh->deleteLater(); + return exitCode == 0; +} + +QByteArray RemoteDevice::getProgramOutput(const QStringList& args) const +{ + auto ssh = sshProcess(args); + ssh->start(); + ssh->waitForFinished(); + auto output = ssh->readAllStandardOutput(); + ssh->deleteLater(); + return output; +} + +QProcess* RemoteDevice::sshProcess(const QStringList& args) +{ + if (!m_config.isValid()) { + return nullptr; + } + + auto process = new QProcess(this); + setupProcess(process, m_sshBinary, m_config, m_tempDir.path(), args); + + return process; +} + +QProcess* RemoteDevice::sshProcess(const QStringList& args) const +{ + if (!m_config.isValid()) { + return nullptr; + } + + auto process = new QProcess(nullptr); + setupProcess(process, m_sshBinary, m_config, m_tempDir.path(), args); + + return process; +} diff --git a/src/remotedevice.h b/src/remotedevice.h new file mode 100644 index 00000000..9c9de552 --- /dev/null +++ b/src/remotedevice.h @@ -0,0 +1,50 @@ +/* + SPDX-FileCopyrightText: Lieven Hey + SPDX-FileCopyrightText: 2023 Klarälvdalens Datakonsult AB, a KDAB Group company, info@kdab.com + + SPDX-License-Identifier: GPL-2.0-or-later +*/ + +#pragma once + +#include + +#include +#include + +#include + +class QProcess; + +class RemoteDevice : public QObject +{ + Q_OBJECT +public: + RemoteDevice(QObject* parent = nullptr); + ~RemoteDevice() override; + + void connectToDevice(const QString& device); + void disconnect(); + + bool isConnected() const; + + bool checkIfProgramExists(const QString& program) const; + bool checkIfDirectoryExists(const QString& directory) const; + bool checkIfFileExists(const QString& file) const; + QByteArray getProgramOutput(const QStringList& args) const; + +signals: + void connected(); + void disconnected(); + void failedToConnect(); + +private: + QProcess* sshProcess(const QStringList& args); + QProcess* sshProcess(const QStringList& args) const; + + QProcess* m_connection = nullptr; + QTemporaryDir m_tempDir; + QFileSystemWatcher m_watcher; + KConfigGroup m_config; + QString m_sshBinary; +}; diff --git a/src/settings.cpp b/src/settings.cpp index 3af62262..bed0bc97 100644 --- a/src/settings.cpp +++ b/src/settings.cpp @@ -247,10 +247,16 @@ void Settings::loadFromFile() setSSHPath(sharedConfig->group("SSH").readEntry("ssh")); setSSHCopyKeyPath(sharedConfig->group("SSH").readEntry("ssh-copy-id")); + const auto& ssh = sharedConfig->group("SSH"); + setSSHPath(ssh.readEntry("ssh")); + setSSHCopyKeyPath(ssh.readEntry("ssh-copy-id")); + connect(this, &Settings::sshPathChanged, this, [sharedConfig](const QString& path) { sharedConfig->group("SSH").writeEntry("ssh", path); }); connect(this, &Settings::sshCopyIdPathChanged, this, [sharedConfig](const QString& path) { sharedConfig->group("SSH").writeEntry("ssh-copy-id", path); }); + + setDevices(ssh.groupList()); } void Settings::setSourceCodePaths(const QString& paths) diff --git a/tests/integrationtests/CMakeLists.txt b/tests/integrationtests/CMakeLists.txt index 48e751f9..000f72c8 100644 --- a/tests/integrationtests/CMakeLists.txt +++ b/tests/integrationtests/CMakeLists.txt @@ -7,6 +7,7 @@ ecm_add_test( ../../src/perfcontrolfifowrapper.cpp ../../src/perfrecord.cpp ../../src/recordhost.cpp + ../../src/remotedevice.cpp ../../src/settings.cpp ../../src/util.cpp ../../src/errnoutil.cpp From 5f913da736fdc052295dd7cfd79b008ea5989758 Mon Sep 17 00:00:00 2001 From: Lieven Hey Date: Fri, 15 Sep 2023 13:31:09 +0200 Subject: [PATCH 4/7] integrate RecordHost in PerfRecord There is some logic left from the time before RecordHost. This patch removes these fragments --- src/perfrecord.cpp | 27 +++++------------------ src/perfrecord.h | 6 +++-- src/recordhost.cpp | 2 +- src/recordhost.h | 8 +++---- src/recordpage.cpp | 23 ++++++++++++++----- tests/integrationtests/tst_perfparser.cpp | 10 +++++---- 6 files changed, 39 insertions(+), 37 deletions(-) diff --git a/src/perfrecord.cpp b/src/perfrecord.cpp index e3adc589..a40d4d41 100644 --- a/src/perfrecord.cpp +++ b/src/perfrecord.cpp @@ -21,6 +21,7 @@ #include #include +#include #include #if KWINDOWSYSTEM_VERSION >= QT_VERSION_CHECK(5, 101, 0) #include @@ -174,27 +175,11 @@ void PerfRecord::record(const QStringList& perfOptions, const QString& outputPat runPerf(actuallyElevatePrivileges(elevatePrivileges), options, outputPath, {}); } -void PerfRecord::record(const QStringList& perfOptions, const QString& outputPath, bool elevatePrivileges, - const QString& exePath, const QStringList& exeOptions, const QString& workingDirectory) +void PerfRecord::record(const QStringList& perfOptions, const QString& outputPath, bool elevatePrivileges) { - QFileInfo exeFileInfo(exePath); - - if (!exeFileInfo.exists()) { - exeFileInfo.setFile(QStandardPaths::findExecutable(exePath)); - } - - if (!exeFileInfo.exists()) { - emit recordingFailed(tr("File '%1' does not exist.").arg(exePath)); - return; - } - if (!exeFileInfo.isFile()) { - emit recordingFailed(tr("'%1' is not a file.").arg(exePath)); - return; - } - if (!exeFileInfo.isExecutable()) { - emit recordingFailed(tr("File '%1' is not executable.").arg(exePath)); - return; - } + auto exePath = m_host->clientApplication(); + auto exeOptions = m_host->clientApplicationArguments(); + auto workingDirectory = m_host->currentWorkingDirectory(); QStringList options = perfOptions; if (actuallyElevatePrivileges(elevatePrivileges)) { @@ -211,7 +196,7 @@ void PerfRecord::record(const QStringList& perfOptions, const QString& outputPat m_perfControlFifo.requestStart(); } else { - options.append(exeFileInfo.absoluteFilePath()); + options.append(exePath); options += exeOptions; runPerf(false, options, outputPath, workingDirectory); } diff --git a/src/perfrecord.h b/src/perfrecord.h index ebcc4ce2..9f000691 100644 --- a/src/perfrecord.h +++ b/src/perfrecord.h @@ -24,8 +24,8 @@ class PerfRecord : public QObject explicit PerfRecord(const RecordHost* host, QObject* parent = nullptr); ~PerfRecord(); - void record(const QStringList& perfOptions, const QString& outputPath, bool elevatePrivileges, - const QString& exePath, const QStringList& exeOptions, const QString& workingDirectory = QString()); + void record(const QStringList& perfOptions, const QString& outputPath, bool elevatePrivileges); + void record(const QStringList& perfOptions, const QString& outputPath, bool elevatePrivileges, const QStringList& pids); void recordSystem(const QStringList& perfOptions, const QString& outputPath); @@ -55,4 +55,6 @@ class PerfRecord : public QObject bool runPerf(bool elevatePrivileges, const QStringList& perfOptions, const QString& outputPath, const QString& workingDirectory = QString()); + + bool runRemotePerf(const QStringList& perfOptions, const QString& outputPath, const QString& workingDirectory = {}); }; diff --git a/src/recordhost.cpp b/src/recordhost.cpp index 94abcb35..e6d89eca 100644 --- a/src/recordhost.cpp +++ b/src/recordhost.cpp @@ -329,7 +329,7 @@ void RecordHost::setClientApplication(const QString& clientApplication) } } -void RecordHost::setClientApplicationArguments(const QString& arguments) +void RecordHost::setClientApplicationArguments(const QStringList& arguments) { Q_ASSERT(QThread::currentThread() == thread()); diff --git a/src/recordhost.h b/src/recordhost.h index 2bf40256..9711740d 100644 --- a/src/recordhost.h +++ b/src/recordhost.h @@ -61,11 +61,11 @@ class RecordHost : public QObject } void setClientApplication(const QString& clientApplication); - QString clientApplicationArguments() const + QStringList clientApplicationArguments() const { return m_clientApplicationArguments; } - void setClientApplicationArguments(const QString& arguments); + void setClientApplicationArguments(const QStringList& arguments); QString outputFileName() const { @@ -112,7 +112,7 @@ class RecordHost : public QObject void hostChanged(); void currentWorkingDirectoryChanged(const QString& cwd); // Maybe QUrl void clientApplicationChanged(const QString& clientApplication); - void clientApplicationArgumentsChanged(const QString& arguments); + void clientApplicationArgumentsChanged(const QStringList& arguments); void perfCapabilitiesChanged(RecordHost::PerfCapabilities perfCapabilities); void isPerfInstalledChanged(bool isInstalled); void outputFileNameChanged(const QString& outputFileName); @@ -127,7 +127,7 @@ class RecordHost : public QObject QString m_error; QString m_cwd; QString m_clientApplication; - QString m_clientApplicationArguments; + QStringList m_clientApplicationArguments; QString m_outputFileName; PerfCapabilities m_perfCapabilities; JobTracker m_checkPerfCapabilitiesJob; diff --git a/src/recordpage.cpp b/src/recordpage.cpp index c8d133b0..01959d1d 100644 --- a/src/recordpage.cpp +++ b/src/recordpage.cpp @@ -204,7 +204,7 @@ RecordPage::RecordPage(QWidget* parent) connect(ui->applicationParametersBox, &QLineEdit::editingFinished, this, [this] { ui->multiConfig->saveCurrentConfig(); - m_recordHost->setClientApplicationArguments(ui->applicationParametersBox->text()); + m_recordHost->setClientApplicationArguments(KShell::splitArgs(ui->applicationParametersBox->text())); }); ui->compressionComboBox->addItem(tr("Disabled"), -1); @@ -628,14 +628,27 @@ void RecordPage::onStartRecordingButtonClicked(bool checked) switch (recordType) { case RecordType::LaunchApplication: { const auto applicationName = m_recordHost->clientApplication(); - const auto appParameters = ui->applicationParametersBox->text(); + const auto appParameters = m_recordHost->clientApplicationArguments(); auto workingDir = m_recordHost->currentWorkingDirectory(); if (workingDir.isEmpty()) { workingDir = ui->workingDirectory->placeholderText(); } - rememberApplication(applicationName, appParameters, workingDir, ui->applicationName->comboBox()); - m_perfRecord->record(perfOptions, outputFile, elevatePrivileges, applicationName, - KShell::splitArgs(appParameters), workingDir); + rememberApplication(applicationName, appParameters.join(QLatin1Char(' ')), workingDir, + ui->applicationName->comboBox()); + m_perfRecord->record(perfOptions, outputFile, elevatePrivileges); + break; + } + case RecordType::LaunchRemoteApplication: { + // TODO: network record + const auto applicationName = m_recordHost->clientApplication(); + const auto appParameters = m_recordHost->clientApplicationArguments(); + auto workingDir = m_recordHost->currentWorkingDirectory(); + if (workingDir.isEmpty()) { + workingDir = ui->workingDirectory->placeholderText(); + } + rememberApplication(applicationName, appParameters.join(QLatin1Char(' ')), workingDir, + ui->applicationName->comboBox()); + m_perfRecord->record(perfOptions, outputFile, elevatePrivileges); break; } case RecordType::AttachToProcess: { diff --git a/tests/integrationtests/tst_perfparser.cpp b/tests/integrationtests/tst_perfparser.cpp index 184bff7e..43d14b51 100644 --- a/tests/integrationtests/tst_perfparser.cpp +++ b/tests/integrationtests/tst_perfparser.cpp @@ -19,8 +19,6 @@ #include "perfparser.h" #include "perfrecord.h" #include "recordhost.h" -#include "unistd.h" -#include "util.h" #include "../testutils.h" #include @@ -429,7 +427,9 @@ private slots: QSignalSpy recordingFinishedSpy(&perf, &PerfRecord::recordingFinished); QSignalSpy recordingFailedSpy(&perf, &PerfRecord::recordingFailed); - perf.record({QStringLiteral("--no-buildid-cache")}, tempFile.fileName(), false, exePath, exeOptions); + host.setClientApplication(exePath); + host.setClientApplicationArguments(exeOptions); + perf.record({QStringLiteral("--no-buildid-cache")}, tempFile.fileName(), false); perf.sendInput(QByteArrayLiteral("some input\n")); QVERIFY(recordingFinishedSpy.wait()); @@ -735,11 +735,13 @@ private slots: QSignalSpy recordingFinishedSpy(&perf, &PerfRecord::recordingFinished); QSignalSpy recordingFailedSpy(&perf, &PerfRecord::recordingFailed); + host.setClientApplication(exePath); + host.setClientApplicationArguments(exeOptions); // always add `-c 1000000`, as perf's frequency mode is too unreliable for testing purposes perf.record( perfOptions + QStringList {QStringLiteral("-c"), QStringLiteral("1000000"), QStringLiteral("--no-buildid-cache")}, - fileName, false, exePath, exeOptions); + fileName, false); VERIFY_OR_THROW(recordingFinishedSpy.wait(10000)); From 2689f1bd01e956c5d7cddeaf0ea95046a23a7602 Mon Sep 17 00:00:00 2001 From: Lieven Hey Date: Wed, 4 Oct 2023 13:32:34 +0200 Subject: [PATCH 5/7] replace QPointer with std::uniqur_ptr in PerfRecord --- src/perfrecord.cpp | 12 +++++------- src/perfrecord.h | 5 +++-- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/perfrecord.cpp b/src/perfrecord.cpp index a40d4d41..fa59b9fb 100644 --- a/src/perfrecord.cpp +++ b/src/perfrecord.cpp @@ -58,7 +58,6 @@ PerfRecord::~PerfRecord() stopRecording(); if (m_perfRecordProcess) { m_perfRecordProcess->waitForFinished(100); - delete m_perfRecordProcess; } } @@ -70,9 +69,8 @@ bool PerfRecord::runPerf(bool elevatePrivileges, const QStringList& perfOptions, m_perfControlFifo.requestStop(); m_perfControlFifo.close(); m_perfRecordProcess->kill(); - m_perfRecordProcess->deleteLater(); } - m_perfRecordProcess = new QProcess(this); + m_perfRecordProcess = std::make_unique(this); m_perfRecordProcess->setProcessChannelMode(QProcess::MergedChannels); const auto outputFileInfo = QFileInfo(outputPath); @@ -91,7 +89,7 @@ bool PerfRecord::runPerf(bool elevatePrivileges, const QStringList& perfOptions, return false; } - connect(m_perfRecordProcess.data(), static_cast(&QProcess::finished), + connect(m_perfRecordProcess.get(), static_cast(&QProcess::finished), this, [this](int exitCode, QProcess::ExitStatus exitStatus) { Q_UNUSED(exitStatus) @@ -108,17 +106,17 @@ bool PerfRecord::runPerf(bool elevatePrivileges, const QStringList& perfOptions, m_userTerminated = false; }); - connect(m_perfRecordProcess.data(), &QProcess::errorOccurred, this, [this](QProcess::ProcessError error) { + connect(m_perfRecordProcess.get(), &QProcess::errorOccurred, this, [this](QProcess::ProcessError error) { Q_UNUSED(error) if (!m_userTerminated) { emit recordingFailed(m_perfRecordProcess->errorString()); } }); - connect(m_perfRecordProcess.data(), &QProcess::started, this, + connect(m_perfRecordProcess.get(), &QProcess::started, this, [this] { emit recordingStarted(m_perfRecordProcess->program(), m_perfRecordProcess->arguments()); }); - connect(m_perfRecordProcess.data(), &QProcess::readyRead, this, [this]() { + connect(m_perfRecordProcess.get(), &QProcess::readyRead, this, [this]() { const auto output = QString::fromUtf8(m_perfRecordProcess->readAll()); emit recordingOutput(output); }); diff --git a/src/perfrecord.h b/src/perfrecord.h index 9f000691..c0935c4b 100644 --- a/src/perfrecord.h +++ b/src/perfrecord.h @@ -12,7 +12,8 @@ #include "perfcontrolfifowrapper.h" #include -#include + +#include class QProcess; class RecordHost; @@ -45,7 +46,7 @@ class PerfRecord : public QObject private: const RecordHost* m_host = nullptr; - QPointer m_perfRecordProcess; + std::unique_ptr m_perfRecordProcess; InitiallyStoppedProcess m_targetProcessForPrivilegedPerf; PerfControlFifoWrapper m_perfControlFifo; QString m_outputPath; From 2b56d338bef2383810456ab956452b3598968731 Mon Sep 17 00:00:00 2001 From: Lieven Hey Date: Mon, 18 Sep 2023 15:29:36 +0200 Subject: [PATCH 6/7] add recording via ssh this patch finally runs perf via ssh on a remote device perf is run with -o - to stream the recording to the host in this case stderr contains the output of the program run --- src/perfrecord.cpp | 118 ++++++++++++++++++++++++++--------------- src/perfrecord.h | 9 +++- src/recordhost.cpp | 50 +++++++++++------ src/recordhost.h | 14 ++++- src/recordpage.cpp | 50 ++++++++++++++--- src/recordpage.h | 2 +- src/recordpage.ui | 19 +++++++ src/remotedevice.cpp | 41 +++++++------- src/remotedevice.h | 9 ++-- src/settingsdialog.cpp | 7 +++ 10 files changed, 220 insertions(+), 99 deletions(-) diff --git a/src/perfrecord.cpp b/src/perfrecord.cpp index fa59b9fb..f04f5c76 100644 --- a/src/perfrecord.cpp +++ b/src/perfrecord.cpp @@ -70,60 +70,30 @@ bool PerfRecord::runPerf(bool elevatePrivileges, const QStringList& perfOptions, m_perfControlFifo.close(); m_perfRecordProcess->kill(); } - m_perfRecordProcess = std::make_unique(this); - m_perfRecordProcess->setProcessChannelMode(QProcess::MergedChannels); - const auto outputFileInfo = QFileInfo(outputPath); - const auto folderPath = outputFileInfo.dir().path(); - const auto folderInfo = QFileInfo(folderPath); - if (!folderInfo.exists()) { - emit recordingFailed(tr("Folder '%1' does not exist.").arg(folderPath)); - return false; - } - if (!folderInfo.isDir()) { - emit recordingFailed(tr("'%1' is not a folder.").arg(folderPath)); - return false; - } - if (!folderInfo.isWritable()) { - emit recordingFailed(tr("Folder '%1' is not writable.").arg(folderPath)); - return false; - } - - connect(m_perfRecordProcess.get(), static_cast(&QProcess::finished), - this, [this](int exitCode, QProcess::ExitStatus exitStatus) { - Q_UNUSED(exitStatus) + m_outputPath = outputPath; + m_userTerminated = false; - const auto outputFileInfo = QFileInfo(m_outputPath); - if ((exitCode == EXIT_SUCCESS || (exitCode == SIGTERM && m_userTerminated) || outputFileInfo.size() > 0) - && outputFileInfo.exists()) { - if (exitCode != EXIT_SUCCESS && !m_userTerminated) { - emit debuggeeCrashed(); - } - emit recordingFinished(m_outputPath); - } else { - emit recordingFailed(tr("Failed to record perf data, error code %1.").arg(exitCode)); - } - m_userTerminated = false; - }); + if (m_host->isLocal()) { + return runPerfLocal(elevatePrivileges, perfOptions, outputPath, workingDirectory); + } else { + return runPerfRemote(perfOptions, outputPath, workingDirectory); + } +} - connect(m_perfRecordProcess.get(), &QProcess::errorOccurred, this, [this](QProcess::ProcessError error) { - Q_UNUSED(error) - if (!m_userTerminated) { - emit recordingFailed(m_perfRecordProcess->errorString()); - } - }); +bool PerfRecord::runPerfLocal(bool elevatePrivileges, const QStringList& perfOptions, const QString& outputPath, + const QString& workingDirectory) +{ + m_perfRecordProcess = std::make_unique(this); + m_perfRecordProcess->setProcessChannelMode(QProcess::MergedChannels); - connect(m_perfRecordProcess.get(), &QProcess::started, this, - [this] { emit recordingStarted(m_perfRecordProcess->program(), m_perfRecordProcess->arguments()); }); + connectRecordingProcessErrors(); connect(m_perfRecordProcess.get(), &QProcess::readyRead, this, [this]() { const auto output = QString::fromUtf8(m_perfRecordProcess->readAll()); emit recordingOutput(output); }); - m_outputPath = outputPath; - m_userTerminated = false; - if (!workingDirectory.isEmpty()) { m_perfRecordProcess->setWorkingDirectory(workingDirectory); } @@ -160,6 +130,36 @@ bool PerfRecord::runPerf(bool elevatePrivileges, const QStringList& perfOptions, return true; } +bool PerfRecord::runPerfRemote(const QStringList& perfOptions, const QString& outputPath, + const QString& workingDirectory) +{ + m_perfRecordProcess = m_host->remoteDevice()->runPerf(workingDirectory, perfOptions); + + auto output = new QFile(outputPath, m_perfRecordProcess.get()); + if (!output->open(QIODevice::WriteOnly)) { + emit recordingFailed(QStringLiteral("Failed to create output file: %1").arg(outputPath)); + return false; + } + + connect(m_perfRecordProcess.get(), &QProcess::readyReadStandardOutput, m_perfRecordProcess.get(), + [process = m_perfRecordProcess.get(), output] { + auto data = process->readAllStandardOutput(); + output->write(data); + }); + connect(m_perfRecordProcess.get(), &QProcess::readyReadStandardError, m_perfRecordProcess.get(), + [this] { emit recordingOutput(QString::fromUtf8(m_perfRecordProcess->readAllStandardError())); }); + + connect(m_perfRecordProcess.get(), qOverload(&QProcess::finished), this, [output] { + output->close(); + output->deleteLater(); + }); + + connectRecordingProcessErrors(); + + m_perfRecordProcess->start(); + return true; +} + void PerfRecord::record(const QStringList& perfOptions, const QString& outputPath, bool elevatePrivileges, const QStringList& pids) { @@ -245,3 +245,33 @@ bool PerfRecord::actuallyElevatePrivileges(bool elevatePrivileges) const const auto capabilities = m_host->perfCapabilities(); return elevatePrivileges && capabilities.canElevatePrivileges && !capabilities.privilegesAlreadyElevated; } + +void PerfRecord::connectRecordingProcessErrors() +{ + connect(m_perfRecordProcess.get(), qOverload(&QProcess::finished), this, + [this](int exitCode, QProcess::ExitStatus exitStatus) { + Q_UNUSED(exitStatus) + + const auto outputFileInfo = QFileInfo(m_outputPath); + if ((exitCode == EXIT_SUCCESS || (exitCode == SIGTERM && m_userTerminated) || outputFileInfo.size() > 0) + && outputFileInfo.exists()) { + if (exitCode != EXIT_SUCCESS && !m_userTerminated) { + emit debuggeeCrashed(); + } + emit recordingFinished(m_outputPath); + } else { + emit recordingFailed(tr("Failed to record perf data, error code %1.").arg(exitCode)); + } + m_userTerminated = false; + }); + + connect(m_perfRecordProcess.get(), &QProcess::errorOccurred, this, [this](QProcess::ProcessError error) { + Q_UNUSED(error) + if (!m_userTerminated) { + emit recordingFailed(m_perfRecordProcess->errorString()); + } + }); + + connect(m_perfRecordProcess.get(), &QProcess::started, this, + [this] { emit recordingStarted(m_perfRecordProcess->program(), m_perfRecordProcess->arguments()); }); +} diff --git a/src/perfrecord.h b/src/perfrecord.h index c0935c4b..429dc1d5 100644 --- a/src/perfrecord.h +++ b/src/perfrecord.h @@ -12,10 +12,10 @@ #include "perfcontrolfifowrapper.h" #include +#include #include -class QProcess; class RecordHost; class PerfRecord : public QObject @@ -57,5 +57,12 @@ class PerfRecord : public QObject bool runPerf(bool elevatePrivileges, const QStringList& perfOptions, const QString& outputPath, const QString& workingDirectory = QString()); + bool runPerfLocal(bool elevatePrivileges, const QStringList& perfOptions, const QString& outputPath, + const QString& workingDirectory = QString()); + bool runPerfRemote(const QStringList& perfOptions, const QString& outputPath, + const QString& workingDirectory = QString()); + bool runRemotePerf(const QStringList& perfOptions, const QString& outputPath, const QString& workingDirectory = {}); + + void connectRecordingProcessErrors(); }; diff --git a/src/recordhost.cpp b/src/recordhost.cpp index e6d89eca..0e02fb16 100644 --- a/src/recordhost.cpp +++ b/src/recordhost.cpp @@ -143,13 +143,13 @@ RecordHost::PerfCapabilities fetchLocalPerfCapabilities(const QString& perfPath) return capabilities; } -RecordHost::PerfCapabilities fetchRemotePerfCapabilities(const RemoteDevice& device) +RecordHost::PerfCapabilities fetchRemotePerfCapabilities(const std::unique_ptr& device) { RecordHost::PerfCapabilities capabilities; - const auto buildOptions = - device.getProgramOutput({QStringLiteral("perf"), QStringLiteral("version"), QStringLiteral("--build-options")}); - const auto help = device.getProgramOutput({QStringLiteral("perf"), QStringLiteral("--help")}); + const auto buildOptions = device->getProgramOutput( + {QStringLiteral("perf"), QStringLiteral("version"), QStringLiteral("--build-options")}); + const auto help = device->getProgramOutput({QStringLiteral("perf"), QStringLiteral("--help")}); capabilities.canCompress = Zstd_FOUND && buildOptions.contains("zszd: [ on ]"); capabilities.canSwitchEvents = help.contains("--switch-events"); @@ -170,7 +170,6 @@ RecordHost::RecordHost(QObject* parent) : QObject(parent) , m_checkPerfCapabilitiesJob(this) , m_checkPerfInstalledJob(this) - , m_remoteDevice(this) { connect(this, &RecordHost::errorOccurred, this, [this](const QString& message) { m_error = message; }); @@ -185,10 +184,6 @@ RecordHost::RecordHost(QObject* parent) connectIsReady(&RecordHost::pidsChanged); connectIsReady(&RecordHost::currentWorkingDirectoryChanged); - connect(&m_remoteDevice, &RemoteDevice::connected, this, &RecordHost::checkRequirements); - - connect(&m_remoteDevice, &RemoteDevice::connected, this, [this] { emit isReadyChanged(isReady()); }); - setHost(QStringLiteral("localhost")); } @@ -203,7 +198,7 @@ bool RecordHost::isReady() const return false; break; case RecordType::LaunchRemoteApplication: - if (!m_remoteDevice.isConnected()) + if (!m_remoteDevice || !m_remoteDevice->isConnected()) return false; if (m_clientApplication.isEmpty() && m_cwd.isEmpty()) return false; @@ -238,6 +233,17 @@ void RecordHost::setHost(const QString& host) m_host = host; emit hostChanged(); + if (!isLocal()) { + m_remoteDevice = std::make_unique(this); + + connect(m_remoteDevice.get(), &RemoteDevice::connected, this, &RecordHost::checkRequirements); + connect(m_remoteDevice.get(), &RemoteDevice::connected, this, [this] { emit isReadyChanged(isReady()); }); + connect(m_remoteDevice.get(), &RemoteDevice::failedToConnect, this, + [this] { emit errorOccurred(tr("Failed to connect to: %1").arg(m_host)); }); + } else { + m_remoteDevice = nullptr; + } + // invalidate everything m_cwd.clear(); emit currentWorkingDirectoryChanged(m_cwd); @@ -251,12 +257,11 @@ void RecordHost::setHost(const QString& host) m_perfCapabilities = {}; emit perfCapabilitiesChanged(m_perfCapabilities); - m_remoteDevice.disconnect(); if (isLocal()) { checkRequirements(); } else { // checkRequirements will be called via RemoteDevice::connected - m_remoteDevice.connectToDevice(m_host); + m_remoteDevice->connectToDevice(m_host); } } @@ -279,7 +284,7 @@ void RecordHost::setCurrentWorkingDirectory(const QString& cwd) emit currentWorkingDirectoryChanged(cwd); } } else { - if (!m_remoteDevice.checkIfDirectoryExists(cwd)) { + if (!m_remoteDevice->checkIfDirectoryExists(cwd)) { emit errorOccurred(tr("Working directory folder cannot be found: %1").arg(cwd)); } else { emit errorOccurred({}); @@ -319,7 +324,9 @@ void RecordHost::setClientApplication(const QString& clientApplication) setCurrentWorkingDirectory(application.dir().absolutePath()); } } else { - if (!m_remoteDevice.checkIfFileExists(clientApplication)) { + if (!m_remoteDevice->isConnected()) { + emit errorOccurred(tr("Hotspot is not connected to the remote device")); + } else if (!m_remoteDevice->checkIfFileExists(clientApplication)) { emit errorOccurred(tr("Application file cannot be found: %1").arg(clientApplication)); } else { emit errorOccurred({}); @@ -345,8 +352,8 @@ void RecordHost::setOutputFileName(const QString& filePath) const QFileInfo file(filePath); const QFileInfo folder(file.absolutePath()); - // the recording data is streamed from the device (currently) so there is no need to use different logic for local - // vs remote + // the recording data is streamed from the device (currently) so there is no need to use different logic for + // local vs remote if (!folder.exists()) { emit errorOccurred(tr("Output file directory folder cannot be found: %1").arg(folder.path())); } else if (!folder.isDir()) { @@ -429,7 +436,7 @@ void RecordHost::checkRequirements() return QFileInfo::exists(perfPath); } else { - return remoteDevice.checkIfProgramExists(QStringLiteral("perf")); + return remoteDevice->checkIfProgramExists(QStringLiteral("perf")); } return false; @@ -442,3 +449,12 @@ void RecordHost::checkRequirements() emit isPerfInstalledChanged(isInstalled); }); } + +void RecordHost::disconnectFromDevice() +{ + if (!isLocal()) { + if (m_remoteDevice->isConnected()) { + m_remoteDevice->disconnect(); + } + } +} diff --git a/src/recordhost.h b/src/recordhost.h index 9711740d..6bdce773 100644 --- a/src/recordhost.h +++ b/src/recordhost.h @@ -12,6 +12,8 @@ #include +#include + enum class RecordType { LaunchApplication, @@ -104,6 +106,15 @@ class RecordHost : public QObject // list of pids to record void setPids(const QStringList& pids); + bool isLocal() const; + + const RemoteDevice* remoteDevice() const + { + return m_remoteDevice.get(); + } + + void disconnectFromDevice(); + signals: /// disallow "start" on recordpage until this is ready and that should only be the case when there's no error void isReadyChanged(bool isReady); @@ -121,7 +132,6 @@ class RecordHost : public QObject private: void checkRequirements(); - bool isLocal() const; QString m_host; QString m_error; @@ -135,7 +145,7 @@ class RecordHost : public QObject RecordType m_recordType = RecordType::LaunchApplication; bool m_isPerfInstalled = false; QStringList m_pids; - RemoteDevice m_remoteDevice; + std::unique_ptr m_remoteDevice; }; Q_DECLARE_METATYPE(RecordHost::PerfCapabilities) diff --git a/src/recordpage.cpp b/src/recordpage.cpp index 01959d1d..6f919118 100644 --- a/src/recordpage.cpp +++ b/src/recordpage.cpp @@ -43,6 +43,7 @@ #include "perfoutputwidgetkonsole.h" #include "perfoutputwidgettext.h" #include "perfrecord.h" +#include "settings.h" namespace { bool isIntel() @@ -81,11 +82,15 @@ void updateStartRecordingButtonState(const RecordHost* host, const std::unique_p return; } + // TODO: move stuff to RecordHost bool enabled = false; switch (selectedRecordType(ui)) { case RecordType::LaunchApplication: enabled = ui->applicationName->url().isValid(); break; + case RecordType::LaunchRemoteApplication: + enabled = host->isReady(); + break; case RecordType::AttachToProcess: enabled = ui->processesTableView->selectionModel()->hasSelection(); break; @@ -207,6 +212,20 @@ RecordPage::RecordPage(QWidget* parent) m_recordHost->setClientApplicationArguments(KShell::splitArgs(ui->applicationParametersBox->text())); }); + auto settings = Settings::instance(); + connect(settings, &Settings::devicesChanged, this, [this](const QStringList& devices) { + ui->deviceComboBox->clear(); + ui->deviceComboBox->insertItem(0, QStringLiteral("localhost")); + ui->deviceComboBox->insertItems(1, devices); + ui->deviceComboBox->setCurrentIndex(0); + }); + ui->deviceComboBox->insertItem(0, QStringLiteral("localhost")); + ui->deviceComboBox->insertItems(1, settings->devices()); + ui->deviceComboBox->setCurrentIndex(0); + + connect(ui->deviceComboBox, qOverload(&QComboBox::currentIndexChanged), this, + [this] { m_recordHost->setHost(ui->deviceComboBox->currentText()); }); + ui->compressionComboBox->addItem(tr("Disabled"), -1); ui->compressionComboBox->addItem(tr("Enabled (Default Level)"), 0); ui->compressionComboBox->addItem(tr("Level 1 (Fastest)"), 1); @@ -254,6 +273,7 @@ RecordPage::RecordPage(QWidget* parent) }); m_recordHost->setHost(QStringLiteral("localhost")); + m_recordHost->setRecordType(RecordType::LaunchApplication); ui->applicationName->comboBox()->setEditable(true); ui->applicationName->setMode(KFile::File | KFile::ExistingOnly | KFile::LocalOnly); @@ -289,7 +309,11 @@ RecordPage::RecordPage(QWidget* parent) connect(ui->startRecordingButton, &QPushButton::toggled, this, &RecordPage::onStartRecordingButtonClicked); connect(ui->workingDirectory, &KUrlRequester::textChanged, m_recordHost, &RecordHost::currentWorkingDirectoryChanged); - connect(ui->viewPerfRecordResultsButton, &QPushButton::clicked, this, [this] { emit openFile(m_resultsFile); }); + connect(ui->viewPerfRecordResultsButton, &QPushButton::clicked, this, [this] { + // we are done recording, disconnect from device + m_recordHost->disconnectFromDevice(); + emit openFile(m_resultsFile); + }); connect(ui->outputFile, &KUrlRequester::textChanged, this, &RecordPage::onOutputFileNameChanged); connect(ui->outputFile, static_cast(&KUrlRequester::returnPressed), this, &RecordPage::onOutputFileNameSelected); @@ -297,15 +321,18 @@ RecordPage::RecordPage(QWidget* parent) ui->recordTypeComboBox->addItem(QIcon::fromTheme(QStringLiteral("run-build")), tr("Launch Application"), QVariant::fromValue(RecordType::LaunchApplication)); + ui->recordTypeComboBox->addItem(QIcon::fromTheme(QStringLiteral("run-build")), tr("Launch Remote Application"), + QVariant::fromValue(RecordType::LaunchRemoteApplication)); ui->recordTypeComboBox->addItem(QIcon::fromTheme(QStringLiteral("run-install")), tr("Attach To Process(es)"), QVariant::fromValue(RecordType::AttachToProcess)); ui->recordTypeComboBox->addItem(QIcon::fromTheme(QStringLiteral("run-build-install-root")), tr("Profile System"), QVariant::fromValue(RecordType::ProfileSystem)); - connect(ui->recordTypeComboBox, qOverload(&QComboBox::currentIndexChanged), this, - &RecordPage::updateRecordType); + connect(ui->recordTypeComboBox, qOverload(&QComboBox::currentIndexChanged), m_recordHost, [this] { m_recordHost->setRecordType(ui->recordTypeComboBox->currentData().value()); }); - connect(m_recordHost, &RecordHost::clientApplicationChanged, this, &RecordPage::updateRecordType); + + connect(m_recordHost, &RecordHost::recordTypeChanged, this, &RecordPage::updateRecordType); + updateRecordType(RecordType::LaunchApplication); { ui->callGraphComboBox->addItem(tr("None"), QVariant::fromValue(QString())); @@ -511,7 +538,7 @@ void RecordPage::showRecordPage() { m_resultsFile.clear(); setError({}); - updateRecordType(); + m_recordHost->setRecordType(RecordType::LaunchApplication); ui->viewPerfRecordResultsButton->setEnabled(false); } @@ -748,17 +775,24 @@ void RecordPage::setError(const QString& message) ui->applicationRecordErrorMessage->setVisible(!message.isEmpty()); } -void RecordPage::updateRecordType() +void RecordPage::updateRecordType(RecordType recordType) { setError({}); - const auto recordType = selectedRecordType(ui); - ui->launchAppBox->setVisible(recordType == RecordType::LaunchApplication); + ui->launchAppBox->setVisible(recordType == RecordType::LaunchApplication + || recordType == RecordType::LaunchRemoteApplication); ui->attachAppBox->setVisible(recordType == RecordType::AttachToProcess); + ui->remoteDeviceBox->setVisible(recordType == RecordType::LaunchRemoteApplication); m_perfOutput->setInputVisible(recordType == RecordType::LaunchApplication); m_perfOutput->clear(); + if (recordType == RecordType::LaunchRemoteApplication) { + ui->applicationName->clear(); + } else if (recordType == RecordType::LaunchApplication) { + restoreCombobox(config(), QStringLiteral("applications"), ui->applicationName->comboBox()); + } + if (recordType == RecordType::AttachToProcess) { updateProcesses(); } diff --git a/src/recordpage.h b/src/recordpage.h index 85b5087f..0ed4fa41 100644 --- a/src/recordpage.h +++ b/src/recordpage.h @@ -59,7 +59,7 @@ private slots: private: void recordingStopped(); - void updateRecordType(); + void updateRecordType(RecordType type); void appendOutput(const QString& text); void setError(const QString& message); diff --git a/src/recordpage.ui b/src/recordpage.ui index 3db5f6dc..16a2b52e 100644 --- a/src/recordpage.ui +++ b/src/recordpage.ui @@ -49,6 +49,25 @@ + + + + Remote Device + + + + + + Device: + + + + + + + + + diff --git a/src/remotedevice.cpp b/src/remotedevice.cpp index 6ec7b496..fe162fb4 100644 --- a/src/remotedevice.cpp +++ b/src/remotedevice.cpp @@ -15,9 +15,6 @@ #include "settings.h" -// TODO: remove -#include - namespace { QStringList sshArgs(const QString& dir) { @@ -25,8 +22,8 @@ QStringList sshArgs(const QString& dir) QStringLiteral("ControlPath=%1/ssh").arg(dir)}; } -void setupProcess(QProcess* process, const QString& sshBinary, const KConfigGroup& config, const QString& path, - const QStringList& args) +void setupProcess(const std::unique_ptr& process, const QString& sshBinary, const KConfigGroup& config, + const QString& path, const QStringList& args) { process->setProgram(sshBinary); const auto options = config.readEntry("options", QString {}); @@ -73,7 +70,12 @@ RemoteDevice::RemoteDevice(QObject* parent) connect(settings, &Settings::sshPathChanged, this, findSSHBinary); } -RemoteDevice::~RemoteDevice() = default; +RemoteDevice::~RemoteDevice() +{ + if (m_connection) { + disconnect(); + } +} void RemoteDevice::connectToDevice(const QString& device) { @@ -89,15 +91,14 @@ void RemoteDevice::connectToDevice(const QString& device) m_connection = sshProcess({}); - connect(m_connection, qOverload(&QProcess::finished), this, - [connection = m_connection, path = m_tempDir.path(), this](int exitCode, QProcess::ExitStatus) { + connect(m_connection.get(), qOverload(&QProcess::finished), this, + [path = m_tempDir.path(), this](int exitCode, QProcess::ExitStatus) { if (exitCode != 0) { emit failedToConnect(); } else { emit disconnected(); } - qDebug() << exitCode << connection->readAll(); - connection->deleteLater(); + m_connection = nullptr; }); m_connection->start(); @@ -107,12 +108,10 @@ void RemoteDevice::disconnect() { if (m_connection) { if (m_connection->state() == QProcess::ProcessState::Running) { - // ssh stops once you clode the write channel + // ssh stops once you close the write channel // we then use the finished signal for cleanup m_connection->closeWriteChannel(); } - - m_connection = nullptr; } } @@ -159,30 +158,26 @@ QByteArray RemoteDevice::getProgramOutput(const QStringList& args) const ssh->start(); ssh->waitForFinished(); auto output = ssh->readAllStandardOutput(); - ssh->deleteLater(); return output; } -QProcess* RemoteDevice::sshProcess(const QStringList& args) +std::unique_ptr RemoteDevice::sshProcess(const QStringList& args) const { if (!m_config.isValid()) { return nullptr; } - auto process = new QProcess(this); + auto process = std::make_unique(nullptr); setupProcess(process, m_sshBinary, m_config, m_tempDir.path(), args); return process; } -QProcess* RemoteDevice::sshProcess(const QStringList& args) const +std::unique_ptr RemoteDevice::runPerf(const QString& cwd, const QStringList& perfOptions) const { - if (!m_config.isValid()) { - return nullptr; - } - - auto process = new QProcess(nullptr); - setupProcess(process, m_sshBinary, m_config, m_tempDir.path(), args); + const auto perfCommand = QStringLiteral("perf record -o - %1 ").arg(perfOptions.join(QLatin1Char(' '))); + const QString command = QStringLiteral("cd %1 ; %2").arg(cwd, perfCommand); + auto process = sshProcess({QStringLiteral("sh"), QStringLiteral("-c"), QStringLiteral("\"%1\"").arg(command)}); return process; } diff --git a/src/remotedevice.h b/src/remotedevice.h index 9c9de552..592672e9 100644 --- a/src/remotedevice.h +++ b/src/remotedevice.h @@ -14,6 +14,8 @@ #include +#include + class QProcess; class RemoteDevice : public QObject @@ -33,16 +35,17 @@ class RemoteDevice : public QObject bool checkIfFileExists(const QString& file) const; QByteArray getProgramOutput(const QStringList& args) const; + std::unique_ptr runPerf(const QString& cdw, const QStringList& perfOptions) const; + signals: void connected(); void disconnected(); void failedToConnect(); private: - QProcess* sshProcess(const QStringList& args); - QProcess* sshProcess(const QStringList& args) const; + std::unique_ptr sshProcess(const QStringList& args) const; - QProcess* m_connection = nullptr; + std::unique_ptr m_connection = nullptr; QTemporaryDir m_tempDir; QFileSystemWatcher m_watcher; KConfigGroup m_config; diff --git a/src/settingsdialog.cpp b/src/settingsdialog.cpp index ff81f52f..e257af40 100644 --- a/src/settingsdialog.cpp +++ b/src/settingsdialog.cpp @@ -288,6 +288,13 @@ void SettingsDialog::addSSHPage() sshSettingsPage->messageWidget->hide(); sshSettingsPage->errorWidget->hide(); + bool ksshaskpassFound = !QStandardPaths()::findExe(QStringLiteral("ksshaskpass")).isEmpty(); + if (!qEnvironmentVariableIsSet("SSH_ASKPASS") && !ksshaskpassFound) { + sshSettingsPage->errorWidget->setText( + tr("SSH_ASKPASS is not set please install ksshaskpass or set SSH_ASKPASS")); + sshSettingsPage->errorWidget->show(); + } + auto configGroup = KSharedConfig::openConfig()->group("SSH"); sshSettingsPage->deviceConfig->setChildWidget( sshSettingsPage->deviceSettings, From 0d5ce75afc103dff38dd9b077dca68d78c086e40 Mon Sep 17 00:00:00 2001 From: Lieven Hey Date: Thu, 5 Oct 2023 12:36:42 +0200 Subject: [PATCH 7/7] use qcoro for async ssh control using coroutines allows use async io without having to deal with multithreading --- CMakeLists.txt | 9 ++++++- src/CMakeLists.txt | 1 + src/recordhost.cpp | 36 ++++++++++++++++--------- src/remotedevice.cpp | 38 ++++++++++++++++----------- src/remotedevice.h | 6 +++-- tests/integrationtests/CMakeLists.txt | 1 + 6 files changed, 61 insertions(+), 30 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ca790783..caba3e9d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -23,7 +23,7 @@ if(NOT CMAKE_BUILD_TYPE) ) endif() -set(CMAKE_CXX_STANDARD 17) +set(CMAKE_CXX_STANDARD 20) set(CMAKE_CXX_STANDARD_REQUIRED ON) option(APPIMAGE_BUILD "configure build for bundling in an appimage" OFF) @@ -58,6 +58,13 @@ find_package( REQUIRED ) +find_package( + QCoro${QT_MAJOR_VERSION} + COMPONENTS Core + REQUIRED +) +qcoro_enable_coroutines() + find_package(LibElf REQUIRED) find_package(ElfUtils REQUIRED) find_package(ECM 1.0.0 NO_MODULE REQUIRED) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index b3220c20..11212e10 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -120,6 +120,7 @@ target_link_libraries( models PrefixTickLabels KDAB::kddockwidgets + QCoro::Core ) if(KFArchive_FOUND) diff --git a/src/recordhost.cpp b/src/recordhost.cpp index 0e02fb16..ff503b0c 100644 --- a/src/recordhost.cpp +++ b/src/recordhost.cpp @@ -11,6 +11,9 @@ #include #include #include +#include + +#include #include #include @@ -284,13 +287,17 @@ void RecordHost::setCurrentWorkingDirectory(const QString& cwd) emit currentWorkingDirectoryChanged(cwd); } } else { - if (!m_remoteDevice->checkIfDirectoryExists(cwd)) { - emit errorOccurred(tr("Working directory folder cannot be found: %1").arg(cwd)); - } else { - emit errorOccurred({}); - m_cwd = cwd; - emit currentWorkingDirectoryChanged(m_cwd); - } + QTimer::singleShot(0, this, [this, cwd]() -> QCoro::Task { + bool exists = co_await m_remoteDevice->checkIfDirectoryExists(cwd); + if (!exists) { + emit errorOccurred(tr("Working directory folder cannot be found: %1").arg(cwd)); + } else { + emit errorOccurred({}); + m_cwd = cwd; + emit currentWorkingDirectoryChanged(m_cwd); + } + co_return; + }); } } @@ -326,12 +333,17 @@ void RecordHost::setClientApplication(const QString& clientApplication) } else { if (!m_remoteDevice->isConnected()) { emit errorOccurred(tr("Hotspot is not connected to the remote device")); - } else if (!m_remoteDevice->checkIfFileExists(clientApplication)) { - emit errorOccurred(tr("Application file cannot be found: %1").arg(clientApplication)); } else { - emit errorOccurred({}); - m_clientApplication = clientApplication; - emit clientApplicationChanged(m_clientApplication); + QTimer::singleShot(0, this, [this, clientApplication]() -> QCoro::Task { + bool exists = co_await m_remoteDevice->checkIfFileExists(clientApplication); + if (!exists) { + emit errorOccurred(tr("Application file cannot be found: %1").arg(clientApplication)); + } else { + emit errorOccurred({}); + m_clientApplication = clientApplication; + emit clientApplicationChanged(m_clientApplication); + } + }); } } } diff --git a/src/remotedevice.cpp b/src/remotedevice.cpp index fe162fb4..8cc89ecf 100644 --- a/src/remotedevice.cpp +++ b/src/remotedevice.cpp @@ -11,6 +11,8 @@ #include #include +#include + #include #include "settings.h" @@ -74,6 +76,10 @@ RemoteDevice::~RemoteDevice() { if (m_connection) { disconnect(); + QTimer::singleShot(0, this, [connection = m_connection.release()]() -> QCoro::Task { + co_await qCoro(connection).waitForFinished(); + delete connection; + }); } } @@ -122,34 +128,36 @@ bool RemoteDevice::isConnected() const bool RemoteDevice::checkIfProgramExists(const QString& program) const { + Q_ASSERT(QThread::currentThread() != thread()); + // this is only used to check if perf is installed and is run in a background thread auto ssh = sshProcess({QStringLiteral("command"), program}); ssh->start(); ssh->waitForFinished(); auto exitCode = ssh->exitCode(); - ssh->deleteLater(); // 128 -> not found // perf will return 1 and display the help message return exitCode != 128; } -bool RemoteDevice::checkIfDirectoryExists(const QString& directory) const +QCoro::Task RemoteDevice::checkIfDirectoryExists(const QString& directory) const { - auto ssh = sshProcess({QStringLiteral("test"), QStringLiteral("-d"), directory}); - ssh->start(); - ssh->waitForFinished(); - auto exitCode = ssh->exitCode(); - ssh->deleteLater(); - return exitCode == 0; + auto _ssh = sshProcess({QStringLiteral("test"), QStringLiteral("-d"), directory}); + auto ssh = qCoro(_ssh.get()); + co_await ssh.start(); + co_await ssh.waitForFinished(); + auto exitCode = _ssh->exitCode(); + co_return exitCode == 0; } -bool RemoteDevice::checkIfFileExists(const QString& file) const +QCoro::Task RemoteDevice::checkIfFileExists(const QString& file) const { - auto ssh = sshProcess({QStringLiteral("test"), QStringLiteral("-f"), file}); - ssh->start(); - ssh->waitForFinished(); - auto exitCode = ssh->exitCode(); - ssh->deleteLater(); - return exitCode == 0; + auto _ssh = sshProcess({QStringLiteral("test"), QStringLiteral("-f"), file}); + + auto ssh = qCoro(_ssh.get()); + co_await ssh.start(); + co_await ssh.waitForFinished(); + auto exitCode = _ssh->exitCode(); + co_return exitCode == 0; } QByteArray RemoteDevice::getProgramOutput(const QStringList& args) const diff --git a/src/remotedevice.h b/src/remotedevice.h index 592672e9..5bc19a96 100644 --- a/src/remotedevice.h +++ b/src/remotedevice.h @@ -12,6 +12,8 @@ #include #include +#include + #include #include @@ -31,8 +33,8 @@ class RemoteDevice : public QObject bool isConnected() const; bool checkIfProgramExists(const QString& program) const; - bool checkIfDirectoryExists(const QString& directory) const; - bool checkIfFileExists(const QString& file) const; + QCoro::Task checkIfDirectoryExists(const QString& directory) const; + QCoro::Task checkIfFileExists(const QString& file) const; QByteArray getProgramOutput(const QStringList& args) const; std::unique_ptr runPerf(const QString& cdw, const QStringList& perfOptions) const; diff --git a/tests/integrationtests/CMakeLists.txt b/tests/integrationtests/CMakeLists.txt index 000f72c8..06b51f86 100644 --- a/tests/integrationtests/CMakeLists.txt +++ b/tests/integrationtests/CMakeLists.txt @@ -22,6 +22,7 @@ ecm_add_test( KF${QT_MAJOR_VERSION}::WindowSystem KF${QT_MAJOR_VERSION}::KIOCore KF${QT_MAJOR_VERSION}::Parts + QCoro::Core TEST_NAME tst_perfparser )