diff --git a/autotests/CMakeLists.txt b/autotests/CMakeLists.txt --- a/autotests/CMakeLists.txt +++ b/autotests/CMakeLists.txt @@ -75,6 +75,7 @@ ecm_add_tests( favicontest.cpp processlauncherjobtest.cpp + commandlauncherjobtest.cpp NAME_PREFIX "kiogui-" LINK_LIBRARIES KF5::KIOCore KF5::KIOGui Qt5::Test ) diff --git a/autotests/commandlauncherjobtest.h b/autotests/commandlauncherjobtest.h new file mode 100644 --- /dev/null +++ b/autotests/commandlauncherjobtest.h @@ -0,0 +1,42 @@ +/* + This file is part of the KDE libraries + Copyright (c) 2020 David Faure + + This library is free software; you can redistribute it and/or modify + it under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation; either version 2 of the License or ( at + your option ) version 3 or, at the discretion of KDE e.V. ( which shall + act as a proxy as in section 14 of the GPLv3 ), any later version. + + This library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Library General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with this library; see the file COPYING.LIB. If not, write to + the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, + Boston, MA 02110-1301, USA. +*/ + +#ifndef COMMANDLAUNCHERJOBTEST_H +#define COMMANDLAUNCHERJOBTEST_H + +#include +#include + +class CommandLauncherJobTest : public QObject +{ + Q_OBJECT + +private Q_SLOTS: + void initTestCase(); + + void startProcess_data(); + void startProcess(); + + void doesNotFailOnNonExistingExecutable(); +}; + +#endif /* COMMANDLAUNCHERJOBTEST_H */ + diff --git a/autotests/commandlauncherjobtest.cpp b/autotests/commandlauncherjobtest.cpp new file mode 100644 --- /dev/null +++ b/autotests/commandlauncherjobtest.cpp @@ -0,0 +1,110 @@ +/* + This file is part of the KDE libraries + Copyright (c) 2020 David Faure + + This library is free software; you can redistribute it and/or modify + it under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation; either version 2 of the License or ( at + your option ) version 3 or, at the discretion of KDE e.V. ( which shall + act as a proxy as in section 14 of the GPLv3 ), any later version. + + This library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Library General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with this library; see the file COPYING.LIB. If not, write to + the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, + Boston, MA 02110-1301, USA. +*/ + +#include "commandlauncherjobtest.h" +#include "commandlauncherjob.h" + +#include "kiotesthelper.h" // createTestFile etc. + +#ifdef Q_OS_UNIX +#include // kill +#endif + +#include +#include +#include +#include + +QTEST_GUILESS_MAIN(CommandLauncherJobTest) + +void CommandLauncherJobTest::initTestCase() +{ + QStandardPaths::setTestModeEnabled(true); +} + +static void createSrcFile(const QString path) +{ + QFile srcFile(path); + QVERIFY2(srcFile.open(QFile::WriteOnly), qPrintable(srcFile.errorString())); + srcFile.write("Hello world\n"); +} + +void CommandLauncherJobTest::startProcess_data() +{ + QTest::addColumn("useExec"); + + QTest::newRow("exec") << true; + QTest::newRow("waitForStarted") << false; +} + +void CommandLauncherJobTest::startProcess() +{ + QFETCH(bool, useExec); + + // Given a command + QTemporaryDir tempDir; + const QString srcDir = tempDir.path(); + const QString srcFile = srcDir + "/srcfile"; + createSrcFile(srcFile); + QVERIFY(QFile::exists(srcFile)); + +#ifdef Q_OS_WIN + QString command = "copy.exe"; +#else + QString command = "cp"; +#endif + + command += QStringLiteral(" %1 destfile").arg(srcFile); + + // When running a CommandLauncherJob + KIO::CommandLauncherJob *job = new KIO::CommandLauncherJob(command, this); + job->setWorkingDirectory(srcDir); + if (useExec) { + QVERIFY(job->exec()); + } else { + job->start(); + QVERIFY(job->waitForStarted()); + } + const qint64 pid = job->pid(); + + // Then the service should be executed (which copies the source file to "dest") + QVERIFY(pid != 0); + const QString dest = srcDir + "/destfile"; + QTRY_VERIFY2(QFile::exists(dest), qPrintable(dest)); + QVERIFY(QFile::exists(srcDir + "/srcfile")); + QVERIFY(QFile::remove(dest)); // cleanup + + // Just to make sure + QTRY_COMPARE(KProcessRunner::instanceCount(), 0); +} + +void CommandLauncherJobTest::doesNotFailOnNonExistingExecutable() +{ + // Given a command that uses an executable that doesn't exist + const QString command = "does_not_exist foo bar"; + + // When running a CommandLauncherJob + KIO::CommandLauncherJob *job = new KIO::CommandLauncherJob(command, this); + job->setExecutable("really_does_not_exist"); + + // Then it doesn't actually fail. QProcess is starting /bin/sh, which works... + QVERIFY(job->exec()); +} diff --git a/src/gui/CMakeLists.txt b/src/gui/CMakeLists.txt --- a/src/gui/CMakeLists.txt +++ b/src/gui/CMakeLists.txt @@ -3,6 +3,7 @@ set(kiogui_SRCS faviconrequestjob.cpp processlauncherjob.cpp + commandlauncherjob.cpp kprocessrunner.cpp ) @@ -47,6 +48,7 @@ HEADER_NAMES FavIconRequestJob ProcessLauncherJob + CommandLauncherJob PREFIX KIO REQUIRED_HEADERS KIO_namespaced_gui_HEADERS diff --git a/src/gui/commandlauncherjob.h b/src/gui/commandlauncherjob.h new file mode 100644 --- /dev/null +++ b/src/gui/commandlauncherjob.h @@ -0,0 +1,118 @@ +/* + This file is part of the KDE libraries + Copyright (c) 2020 David Faure + + This library is free software; you can redistribute it and/or modify + it under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation; either version 2 of the License or ( at + your option ) version 3 or, at the discretion of KDE e.V. ( which shall + act as a proxy as in section 14 of the GPLv3 ), any later version. + + This library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Library General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with this library; see the file COPYING.LIB. If not, write to + the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, + Boston, MA 02110-1301, USA. +*/ + +#ifndef KIO_COMMANDLAUNCHERJOB_H +#define KIO_COMMANDLAUNCHERJOB_H + +#include "kiogui_export.h" +#include + +namespace KIO { + +class CommandLauncherJobPrivate; + +/** + * @brief CommandLauncherJob runs a command and watches it while running. + * + * It creates a startup notification and finishes it on success or on error (for the taskbar). + * It also emits an error message if necessary (e.g. "program not found"). + * + * The job finishes when the application is successfully started. At that point you can + * query the PID. + * + * @since 5.69 + */ +class KIOGUI_EXPORT CommandLauncherJob : public KJob +{ +public: + /** + * @brief Creates a CommandLauncherJob + * @param command the shell command to run + * @param parent the parent QObject + * + * Please consider also calling setExecutable and setIcon for better startup notification. + */ + explicit CommandLauncherJob(const QString &command, QObject *parent = nullptr); + + /** + * Destructor + * Note that jobs auto-delete themselves after emitting result + */ + ~CommandLauncherJob() override; + + /** + * Allows to set the name of the executable, used in the startup notification. + * (see KStartupInfoData::setBin) + * @param executable executable name, with or without a path + * Alternatively, use setDesktopName. + */ + void setExecutable(const QString &executable); + + /** + * @brief Sets the icon for the startup notification + * @param iconName name of the icon, to be loaded from the current icon theme + * Alternatively, use setDesktopName. + */ + void setIcon(const QString &iconName); + + /** + * Allows to set the name of the desktop file (e.g. "org.kde.dolphin", the extension is optional) + * This is an alternative solution for setIcon and setExecutable, i.e. the icon + * will be taken from the desktop file, and the executable inferred from the Exec line. + */ + void setDesktopName(const QString &desktopName); + + /** + * @brief setStartupId sets the startupId of the new application + * @param startupId Application startup notification id, if any (otherwise ""). + */ + void setStartupId(const QByteArray &startupId); + + /** + * @brief Sets the working directory from which to run the command + * @param workingDirectory path of a local directory + */ + void setWorkingDirectory(const QString &workingDirectory); + + /** + * @brief start starts the job. You must call this, after all the setters. + */ + void start() override; + + /** + * Blocks until the process has started. + */ + bool waitForStarted(); + + /** + * @return the PID of the application that was started. + * Available after the job emits result(). + */ + qint64 pid() const; + +private: + friend class CommandLauncherJobPrivate; + QScopedPointer d; +}; + +} // namespace KIO + +#endif diff --git a/src/gui/commandlauncherjob.cpp b/src/gui/commandlauncherjob.cpp new file mode 100644 --- /dev/null +++ b/src/gui/commandlauncherjob.cpp @@ -0,0 +1,115 @@ +/* + This file is part of the KDE libraries + Copyright (c) 2020 David Faure + + This library is free software; you can redistribute it and/or modify + it under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation; either version 2 of the License or ( at + your option ) version 3 or, at the discretion of KDE e.V. ( which shall + act as a proxy as in section 14 of the GPLv3 ), any later version. + + This library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Library General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with this library; see the file COPYING.LIB. If not, write to + the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, + Boston, MA 02110-1301, USA. +*/ + +#include "commandlauncherjob.h" +#include "kprocessrunner_p.h" +#include "kiogui_debug.h" +#include + +class KIO::CommandLauncherJobPrivate +{ +public: + explicit CommandLauncherJobPrivate(const QString &command) + : m_command(command) {} + + QString m_command; + QString m_desktopName; + QString m_executable; + QString m_iconName; + QString m_workingDirectory; + QByteArray m_startupId; + KProcessRunner *m_processRunner = nullptr; + qint64 m_pid = 0; +}; + +KIO::CommandLauncherJob::CommandLauncherJob(const QString &command, QObject *parent) + : KJob(parent), d(new CommandLauncherJobPrivate(command)) +{ +} + +KIO::CommandLauncherJob::~CommandLauncherJob() +{ + // Do *NOT* delete the KProcessRunner instances here. + // We need it to keep running so it can terminate startup notification on process exit. +} + +void KIO::CommandLauncherJob::setExecutable(const QString &executable) +{ + d->m_executable = executable; +} + +void KIO::CommandLauncherJob::setIcon(const QString &iconName) +{ + d->m_iconName = iconName; +} + +void KIO::CommandLauncherJob::setDesktopName(const QString &desktopName) +{ + d->m_desktopName = desktopName; +} + +void KIO::CommandLauncherJob::setStartupId(const QByteArray &startupId) +{ + d->m_startupId = startupId; +} + +void KIO::CommandLauncherJob::setWorkingDirectory(const QString &workingDirectory) +{ + d->m_workingDirectory = workingDirectory; +} + +void KIO::CommandLauncherJob::start() +{ + // Some fallback for lazy callers, not 100% accurate though + if (d->m_executable.isEmpty()) { + const QStringList args = KShell::splitArgs(d->m_command); + if (!args.isEmpty()) { + d->m_executable = args.first(); + } + } + if (d->m_iconName.isEmpty()) { + d->m_iconName = d->m_executable; + } + d->m_processRunner = new KProcessRunner(d->m_command, d->m_desktopName, d->m_executable, + d->m_iconName, d->m_startupId, + d->m_workingDirectory); + connect(d->m_processRunner, &KProcessRunner::error, this, [this](const QString &errorText) { + setError(KJob::UserDefinedError); + setErrorText(errorText); + emitResult(); + }); + connect(d->m_processRunner, &KProcessRunner::processStarted, this, [this]() { + d->m_pid = d->m_processRunner->pid(); + emitResult(); + }); +} + +bool KIO::CommandLauncherJob::waitForStarted() +{ + const bool ret = d->m_processRunner->waitForStarted(); + qApp->sendPostedEvents(d->m_processRunner); // so slotStarted gets called + return ret; +} + +qint64 KIO::CommandLauncherJob::pid() const +{ + return d->m_pid; +} diff --git a/src/gui/kprocessrunner.cpp b/src/gui/kprocessrunner.cpp --- a/src/gui/kprocessrunner.cpp +++ b/src/gui/kprocessrunner.cpp @@ -99,29 +99,32 @@ } } - // m_executable can be a full shell command, so here is not 100% reliable. - // E.g. it could be "cd", which isn't an existing binary. It's just a heuristic anyway. - const QString bin = KIO::DesktopExecParser::executableName(m_executable); - init(service, bin, service->name(), service->icon(), asn); + init(service, service->name(), service->icon(), asn); } -KProcessRunner::KProcessRunner(const QString &cmd, const QString &execName, const QString &iconName, const QByteArray &asn, const QString &workingDirectory) +KProcessRunner::KProcessRunner(const QString &cmd, const QString &desktopName, const QString &execName, const QString &iconName, const QByteArray &asn, const QString &workingDirectory) : m_process{new KProcess}, m_executable(execName) { ++s_instanceCount; m_process->setShellCommand(cmd); if (!workingDirectory.isEmpty()) { m_process->setWorkingDirectory(workingDirectory); } - QString bin = KIO::DesktopExecParser::executableName(m_executable); - KService::Ptr service = KService::serviceByDesktopName(bin); - init(service, bin, - execName /*user-visible name*/, - iconName, asn); + if (!desktopName.isEmpty()) { + KService::Ptr service = KService::serviceByDesktopName(desktopName); + if (service) { + if (m_executable.isEmpty()) { + m_executable = KIO::DesktopExecParser::executablePath(service->exec()); + } + init(service, service->name(), service->icon(), asn); + return; + } + } + init(KService::Ptr(), execName /*user-visible name*/, iconName, asn); } -void KProcessRunner::init(const KService::Ptr &service, const QString &bin, const QString &userVisibleName, const QString &iconName, const QByteArray &asn) +void KProcessRunner::init(const KService::Ptr &service, const QString &userVisibleName, const QString &iconName, const QByteArray &asn) { if (service && !service->entryPath().isEmpty() && !KDesktopFile::isAuthorizedDesktopFile(service->entryPath())) { @@ -141,6 +144,9 @@ m_startupId.setupStartupEnv(); KStartupInfoData data; data.setHostname(); + // When it comes from a desktop file, m_executable can be a full shell command, so here is not 100% reliable. + // E.g. it could be "cd", which isn't an existing binary. It's just a heuristic anyway. + const QString bin = KIO::DesktopExecParser::executableName(m_executable); data.setBin(bin); if (!userVisibleName.isEmpty()) { data.setName(userVisibleName); diff --git a/src/gui/kprocessrunner_p.h b/src/gui/kprocessrunner_p.h --- a/src/gui/kprocessrunner_p.h +++ b/src/gui/kprocessrunner_p.h @@ -64,15 +64,15 @@ /** * Run a shell command * @param cmd must be a shell command. No need to append "&" to it. - * @param execName the name of the executable, if known. This improves startup notification, - * as well as honoring various flags coming from the desktop file for this executable, if there's one. + * @param desktopName name of the desktop file, if known. + * @param execName the name of the executable, if known. * @param iconName icon for the startup notification * @param asn Application startup notification id, if any (otherwise ""). * @param workingDirectory the working directory for the started process. The default * (if passing an empty string) is the user's document path. * This allows a command like "kwrite file.txt" to find file.txt from the right place. */ - KProcessRunner(const QString &cmd, const QString &execName, const QString &iconName, + KProcessRunner(const QString &cmd, const QString &desktopName, const QString &execName, const QString &iconName, const QByteArray &asn = {}, const QString &workingDirectory = {}); /** @@ -104,14 +104,14 @@ void slotProcessStarted(); private: - void init(const KService::Ptr &service, const QString &bin, const QString &userVisibleName, + void init(const KService::Ptr &service, const QString &userVisibleName, const QString &iconName, const QByteArray &asn); void startProcess(); void terminateStartupNotification(); void emitDelayedError(const QString &errorMsg); std::unique_ptr m_process; - const QString m_executable; // can be a full path + QString m_executable; // can be a full path KStartupInfoId m_startupId; qint64 m_pid = 0; diff --git a/src/gui/processlauncherjob.cpp b/src/gui/processlauncherjob.cpp --- a/src/gui/processlauncherjob.cpp +++ b/src/gui/processlauncherjob.cpp @@ -53,7 +53,7 @@ KIO::ProcessLauncherJob::~ProcessLauncherJob() { // Do *NOT* delete the KProcessRunner instances here. - // We need it to keep running so it can do terminate startup notification on process exit. + // We need it to keep running so it can terminate startup notification on process exit. } void KIO::ProcessLauncherJob::setUrls(const QList &urls) diff --git a/src/widgets/krun.cpp b/src/widgets/krun.cpp --- a/src/widgets/krun.cpp +++ b/src/widgets/krun.cpp @@ -65,6 +65,7 @@ #include #include #include +#include #include #include @@ -106,18 +107,21 @@ qEnvironmentVariableIsSet("SNAP"); } -static qint64 runProcessRunner(KProcessRunner *processRunner, QWidget *widget) +static qint64 runProcessLauncherJob(KIO::ProcessLauncherJob *job, QWidget *widget) { QObject *receiver = widget ? static_cast(widget) : static_cast(qApp); - QObject::connect(processRunner, &KProcessRunner::error, receiver, [widget](const QString &errorString) { - QEventLoopLocker locker; - KMessageBox::sorry(widget, errorString); + QObject::connect(job, &KJob::result, receiver, [widget](KJob *job) { + if (job->error()) { + QEventLoopLocker locker; + KMessageBox::sorry(widget, job->errorString()); + } }); - processRunner->waitForStarted(); - return processRunner->pid(); + job->start(); + job->waitForStarted(); + return job->pid(); } -static qint64 runProcessLauncherJob(KIO::ProcessLauncherJob *job, QWidget *widget) +static qint64 runCommandLauncherJob(KIO::CommandLauncherJob *job, QWidget *widget) { QObject *receiver = widget ? static_cast(widget) : static_cast(qApp); QObject::connect(job, &KJob::result, receiver, [widget](KJob *job) { @@ -721,17 +725,16 @@ bool KRun::runCommand(const QString &cmd, const QString &execName, const QString &iconName, QWidget *window, const QByteArray &asn, const QString &workingDirectory) { - //qDebug() << "runCommand " << cmd << "," << execName; + auto *job = new KIO::CommandLauncherJob(cmd); + job->setExecutable(execName); + job->setIcon(iconName); + job->setStartupId(asn); + job->setWorkingDirectory(workingDirectory); - // QTBUG-59017 Calling winId() on an embedded widget will break interaction - // with it on high-dpi multi-screen setups (cf. also Bug 363548), hence using - // its parent window instead if (window) { window = window->window(); } - - auto *processRunner = new KProcessRunner(cmd, execName, iconName, asn, workingDirectory); - return runProcessRunner(processRunner, window); + return runCommandLauncherJob(job, window); } KRun::KRun(const QUrl &url, QWidget *window,