From ab11c4373b53953915cb6886356172a991660b8f Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Mon, 4 Apr 2022 13:17:36 +0200 Subject: [PATCH] QtcProcess: Introduce done() signal Introduce QtcProcess::done() signal. It's similar to QtcProcess::finished() signal, but also emitted when process failed to start (so after QProcess::FailedToStart error occurred). SshRemoteProcess::finished() signal was already behaving like this. So, we remove special handling of FailedToStart error in this class and connect all clients of SshRemoteProcess to done() signal, instead of finished(). Task-number: QTCREATORBUG-27232 Change-Id: If4240e2172f3f706e812bca669a1d5b24bdc3285 Reviewed-by: hjk --- src/libs/ssh/sshremoteprocess.cpp | 9 +----- src/libs/ssh/sshremoteprocess.h | 1 - src/libs/ssh/sshremoteprocessrunner.cpp | 10 +++--- src/libs/utils/qtcprocess.cpp | 17 ++++++---- src/libs/utils/qtcprocess.h | 3 +- .../devicesupport/sshdeviceprocess.cpp | 8 ++--- .../genericdirectuploadservice.cpp | 4 +-- src/plugins/remotelinux/linuxdevicetester.cpp | 13 ++++---- src/plugins/remotelinux/rsyncdeploystep.cpp | 2 +- tests/auto/ssh/tst_ssh.cpp | 31 ++++++++++--------- tests/manual/ssh/shell/shell.cpp | 9 +++--- 11 files changed, 55 insertions(+), 52 deletions(-) diff --git a/src/libs/ssh/sshremoteprocess.cpp b/src/libs/ssh/sshremoteprocess.cpp index 088c8f868bd..97ca54dcc85 100644 --- a/src/libs/ssh/sshremoteprocess.cpp +++ b/src/libs/ssh/sshremoteprocess.cpp @@ -62,14 +62,7 @@ void SshRemoteProcess::emitFinished() { if (exitStatus() == QProcess::CrashExit) m_errorString = tr("The ssh process crashed: %1").arg(errorString()); - emit finished(); -} - -void SshRemoteProcess::emitErrorOccurred(QProcess::ProcessError error) -{ - if (error == QProcess::FailedToStart) - emit finished(); - emit errorOccurred(error); + QtcProcess::emitFinished(); } void SshRemoteProcess::start() diff --git a/src/libs/ssh/sshremoteprocess.h b/src/libs/ssh/sshremoteprocess.h index 99aa01a1cec..8f80ac1a5a4 100644 --- a/src/libs/ssh/sshremoteprocess.h +++ b/src/libs/ssh/sshremoteprocess.h @@ -50,7 +50,6 @@ public: protected: void emitFinished() override; - void emitErrorOccurred(QProcess::ProcessError error) override; private: QString m_remoteCommand; diff --git a/src/libs/ssh/sshremoteprocessrunner.cpp b/src/libs/ssh/sshremoteprocessrunner.cpp index c1259f4d9e1..617ffc000c4 100644 --- a/src/libs/ssh/sshremoteprocessrunner.cpp +++ b/src/libs/ssh/sshremoteprocessrunner.cpp @@ -36,6 +36,8 @@ running a remote process over an SSH connection. */ +using namespace Utils; + namespace QSsh { namespace Internal { namespace { @@ -109,13 +111,13 @@ void SshRemoteProcessRunner::handleConnected() setState(Connected); d->m_process = d->m_connection->createRemoteProcess(d->m_command); - connect(d->m_process.get(), &SshRemoteProcess::started, + connect(d->m_process.get(), &QtcProcess::started, this, &SshRemoteProcessRunner::handleProcessStarted); - connect(d->m_process.get(), &SshRemoteProcess::finished, + connect(d->m_process.get(), &QtcProcess::done, this, &SshRemoteProcessRunner::handleProcessFinished); - connect(d->m_process.get(), &SshRemoteProcess::readyReadStandardOutput, + connect(d->m_process.get(), &QtcProcess::readyReadStandardOutput, this, &SshRemoteProcessRunner::readyReadStandardOutput); - connect(d->m_process.get(), &SshRemoteProcess::readyReadStandardError, + connect(d->m_process.get(), &QtcProcess::readyReadStandardError, this, &SshRemoteProcessRunner::readyReadStandardError); d->m_process->start(); } diff --git a/src/libs/utils/qtcprocess.cpp b/src/libs/utils/qtcprocess.cpp index 7bd20d76b38..fee06952f46 100644 --- a/src/libs/utils/qtcprocess.cpp +++ b/src/libs/utils/qtcprocess.cpp @@ -590,6 +590,8 @@ public: void handleError(QProcess::ProcessError error); void clearForRun(); + void emitErrorOccurred(QProcess::ProcessError error); + ProcessResult interpretExitCode(int exitCode); QTextCodec *m_codec = QTextCodec::codecForLocale(); @@ -701,11 +703,7 @@ void QtcProcess::emitStarted() void QtcProcess::emitFinished() { emit finished(); -} - -void QtcProcess::emitErrorOccurred(QProcess::ProcessError error) -{ - emit errorOccurred(error); + emit done(); } void QtcProcess::setProcessInterface(ProcessInterface *interface) @@ -1647,7 +1645,14 @@ void QtcProcessPrivate::handleError(QProcess::ProcessError error) if (m_eventLoop) m_eventLoop->quit(); - q->emitErrorOccurred(error); + emitErrorOccurred(error); +} + +void QtcProcessPrivate::emitErrorOccurred(QProcess::ProcessError error) +{ + emit q->errorOccurred(error); + if (error == QProcess::FailedToStart) + emit q->done(); } } // namespace Utils diff --git a/src/libs/utils/qtcprocess.h b/src/libs/utils/qtcprocess.h index 1e73b0176f3..d7a2dcb7fd3 100644 --- a/src/libs/utils/qtcprocess.h +++ b/src/libs/utils/qtcprocess.h @@ -189,6 +189,8 @@ public: signals: void started(); void finished(); + void done(); // The same as finished() with the addition it's being also emitted after + // FailedToStart error occurred. void errorOccurred(QProcess::ProcessError error); void readyReadStandardOutput(); void readyReadStandardError(); @@ -197,7 +199,6 @@ protected: // TODO: remove these methods on QtcProcess de-virtualization virtual void emitStarted(); virtual void emitFinished(); - virtual void emitErrorOccurred(QProcess::ProcessError error); private: void setProcessInterface(ProcessInterface *interface); diff --git a/src/plugins/projectexplorer/devicesupport/sshdeviceprocess.cpp b/src/plugins/projectexplorer/devicesupport/sshdeviceprocess.cpp index 689ee1b398c..8d9c44b5e39 100644 --- a/src/plugins/projectexplorer/devicesupport/sshdeviceprocess.cpp +++ b/src/plugins/projectexplorer/devicesupport/sshdeviceprocess.cpp @@ -197,14 +197,14 @@ void SshDeviceProcess::handleConnected() setCommand(d->remoteProcess->fullLocalCommandLine(true)); QtcProcess::start(); } else { - connect(d->remoteProcess.get(), &QSsh::SshRemoteProcess::started, + connect(d->remoteProcess.get(), &QtcProcess::started, this, &SshDeviceProcess::handleProcessStarted); - connect(d->remoteProcess.get(), &QSsh::SshRemoteProcess::finished, this, [this] { + connect(d->remoteProcess.get(), &QtcProcess::done, this, [this] { handleProcessFinished(d->remoteProcess->errorString()); }); - connect(d->remoteProcess.get(), &QSsh::SshRemoteProcess::readyReadStandardOutput, + connect(d->remoteProcess.get(), &QtcProcess::readyReadStandardOutput, this, &QtcProcess::readyReadStandardOutput); - connect(d->remoteProcess.get(), &QSsh::SshRemoteProcess::readyReadStandardError, + connect(d->remoteProcess.get(), &QtcProcess::readyReadStandardError, this, &QtcProcess::readyReadStandardError); d->remoteProcess->start(); } diff --git a/src/plugins/remotelinux/genericdirectuploadservice.cpp b/src/plugins/remotelinux/genericdirectuploadservice.cpp index e1f95e1c56b..a146c8748f0 100644 --- a/src/plugins/remotelinux/genericdirectuploadservice.cpp +++ b/src/plugins/remotelinux/genericdirectuploadservice.cpp @@ -200,7 +200,7 @@ void GenericDirectUploadService::runStat(const DeployableFile &file) const QString statCmd = "stat -t " + Utils::ProcessArgs::quoteArgUnix(file.remoteFilePath()); SshRemoteProcess * const statProc = connection()->createRemoteProcess(statCmd).release(); statProc->setParent(this); - connect(statProc, &SshRemoteProcess::finished, this, [this, statProc, state = d->state] { + connect(statProc, &QtcProcess::done, this, [this, statProc, state = d->state] { QTC_ASSERT(d->state == state, return); const DeployableFile file = d->getFileForProcess(statProc); QTC_ASSERT(file.isValid(), return); @@ -341,7 +341,7 @@ void GenericDirectUploadService::chmod() SshRemoteProcess * const chmodProc = connection()->createRemoteProcess(command).release(); chmodProc->setParent(this); - connect(chmodProc, &SshRemoteProcess::finished, this, [this, chmodProc, state = d->state] { + connect(chmodProc, &QtcProcess::done, this, [this, chmodProc, state = d->state] { QTC_ASSERT(state == d->state, return); const DeployableFile file = d->getFileForProcess(chmodProc); QTC_ASSERT(file.isValid(), return); diff --git a/src/plugins/remotelinux/linuxdevicetester.cpp b/src/plugins/remotelinux/linuxdevicetester.cpp index 641e250219d..c427617c069 100644 --- a/src/plugins/remotelinux/linuxdevicetester.cpp +++ b/src/plugins/remotelinux/linuxdevicetester.cpp @@ -38,6 +38,7 @@ using namespace ProjectExplorer; using namespace QSsh; +using namespace Utils; namespace RemoteLinux { namespace Internal { @@ -55,7 +56,7 @@ public: SshRemoteProcessPtr process; DeviceUsedPortsGatherer portsGatherer; SftpTransferPtr sftpTransfer; - Utils::QtcProcess rsyncProcess; + QtcProcess rsyncProcess; State state = Inactive; bool sftpWorks = false; }; @@ -126,7 +127,7 @@ void GenericLinuxDeviceTester::handleConnected() QTC_ASSERT(d->state == Connecting, return); d->process = d->connection->createRemoteProcess("uname -rsm"); - connect(d->process.get(), &SshRemoteProcess::finished, + connect(d->process.get(), &QtcProcess::done, this, &GenericLinuxDeviceTester::handleProcessFinished); emit progressMessage(tr("Checking kernel version...")); @@ -183,7 +184,7 @@ void GenericLinuxDeviceTester::handlePortListReady() emit progressMessage(tr("All specified ports are available.") + QLatin1Char('\n')); } else { QString portList; - foreach (const Utils::Port port, d->portsGatherer.usedPorts()) + foreach (const Port port, d->portsGatherer.usedPorts()) portList += QString::number(port.number()) + QLatin1String(", "); portList.remove(portList.count() - 2, 2); emit errorMessage(tr("The following specified ports are currently in use: %1") @@ -221,18 +222,18 @@ void GenericLinuxDeviceTester::handleSftpFinished(const QString &error) void GenericLinuxDeviceTester::testRsync() { emit progressMessage(tr("Checking whether rsync works...")); - connect(&d->rsyncProcess, &Utils::QtcProcess::errorOccurred, [this] { + connect(&d->rsyncProcess, &QtcProcess::errorOccurred, [this] { if (d->rsyncProcess.error() == QProcess::FailedToStart) handleRsyncFinished(); }); - connect(&d->rsyncProcess, &Utils::QtcProcess::finished, this, [this] { + connect(&d->rsyncProcess, &QtcProcess::finished, this, [this] { handleRsyncFinished(); }); const RsyncCommandLine cmdLine = RsyncDeployStep::rsyncCommand(*d->connection, RsyncDeployStep::defaultFlags()); const QStringList args = QStringList(cmdLine.options) << "-n" << "--exclude=*" << (cmdLine.remoteHostSpec + ":/tmp"); - d->rsyncProcess.setCommand(Utils::CommandLine("rsync", args)); + d->rsyncProcess.setCommand(CommandLine("rsync", args)); d->rsyncProcess.start(); } diff --git a/src/plugins/remotelinux/rsyncdeploystep.cpp b/src/plugins/remotelinux/rsyncdeploystep.cpp index 49d032d26ad..3f159b28998 100644 --- a/src/plugins/remotelinux/rsyncdeploystep.cpp +++ b/src/plugins/remotelinux/rsyncdeploystep.cpp @@ -103,7 +103,7 @@ void RsyncDeployService::createRemoteDirectories() remoteDirs.removeDuplicates(); m_mkdir = connection()->createRemoteProcess("mkdir -p " + ProcessArgs::createUnixArgs(remoteDirs).toString()); - connect(m_mkdir.get(), &SshRemoteProcess::finished, this, [this] { + connect(m_mkdir.get(), &QtcProcess::done, this, [this] { QString userError; const QString error = m_mkdir->errorString(); if (!error.isEmpty()) diff --git a/tests/auto/ssh/tst_ssh.cpp b/tests/auto/ssh/tst_ssh.cpp index cce90e79ba5..dbd014fda0e 100644 --- a/tests/auto/ssh/tst_ssh.cpp +++ b/tests/auto/ssh/tst_ssh.cpp @@ -48,6 +48,7 @@ #include using namespace QSsh; +using namespace Utils; class tst_Ssh : public QObject { @@ -76,10 +77,10 @@ void tst_Ssh::initTestCase() if (!SshTest::checkParameters(params)) SshTest::printSetupHelp(); - Utils::LauncherInterface::setPathToLauncher(qApp->applicationDirPath() + '/' - + QLatin1String(TEST_RELATIVE_LIBEXEC_PATH)); - Utils::TemporaryDirectory::setMasterTemporaryDirectory(QDir::tempPath() - + "/qtc-ssh-autotest-XXXXXX"); + LauncherInterface::setPathToLauncher(qApp->applicationDirPath() + '/' + + QLatin1String(TEST_RELATIVE_LIBEXEC_PATH)); + TemporaryDirectory::setMasterTemporaryDirectory(QDir::tempPath() + + "/qtc-ssh-autotest-XXXXXX"); } void tst_Ssh::errorHandling_data() @@ -118,7 +119,7 @@ void tst_Ssh::errorHandling() params.setUserName(user); params.timeout = 3; params.authenticationType = authType; - params.privateKeyFile = Utils::FilePath::fromString(keyFile); + params.privateKeyFile = FilePath::fromString(keyFile); SshConnection connection(params); QEventLoop loop; bool disconnected = false; @@ -240,12 +241,12 @@ void tst_Ssh::remoteProcessChannels() SshRemoteProcessPtr echoProcess = connection.createRemoteProcess("printf " + QString::fromUtf8(testString) + " >&2"); QEventLoop loop; - connect(echoProcess.get(), &SshRemoteProcess::finished, &loop, &QEventLoop::quit); - connect(echoProcess.get(), &Utils::QtcProcess::readyReadStandardError, + connect(echoProcess.get(), &QtcProcess::done, &loop, &QEventLoop::quit); + connect(echoProcess.get(), &QtcProcess::readyReadStandardError, [&remoteData, p = echoProcess.get()] { remoteData += p->readAllStandardError(); }); - connect(echoProcess.get(), &SshRemoteProcess::readyReadStandardOutput, + connect(echoProcess.get(), &QtcProcess::readyReadStandardOutput, [&remoteStdout, p = echoProcess.get()] { remoteStdout += p->readAllStandardOutput(); }); - connect(echoProcess.get(), &SshRemoteProcess::readyReadStandardError, + connect(echoProcess.get(), &QtcProcess::readyReadStandardError, [&remoteStderr] { remoteStderr = testString; }); echoProcess->start(); QTimer timer; @@ -273,10 +274,10 @@ void tst_Ssh::remoteProcessInput() QVERIFY(waitForConnection(connection)); SshRemoteProcessPtr catProcess = connection.createRemoteProcess("/bin/cat"); - catProcess->setProcessMode(Utils::ProcessMode::Writer); + catProcess->setProcessMode(ProcessMode::Writer); QEventLoop loop; - connect(catProcess.get(), &SshRemoteProcess::started, &loop, &QEventLoop::quit); - connect(catProcess.get(), &SshRemoteProcess::finished, &loop, &QEventLoop::quit); + connect(catProcess.get(), &QtcProcess::started, &loop, &QEventLoop::quit); + connect(catProcess.get(), &QtcProcess::done, &loop, &QEventLoop::quit); QTimer timer; QObject::connect(&timer, &QTimer::timeout, &loop, &QEventLoop::quit); timer.setSingleShot(true); @@ -289,7 +290,7 @@ void tst_Ssh::remoteProcessInput() QVERIFY(catProcess->isRunning()); static QString testString = "x\r\n"; - connect(catProcess.get(), &Utils::QtcProcess::readyReadStandardOutput, &loop, &QEventLoop::quit); + connect(catProcess.get(), &QtcProcess::readyReadStandardOutput, &loop, &QEventLoop::quit); catProcess->write(testString.toUtf8()); timer.start(); @@ -460,7 +461,7 @@ void tst_Ssh::sftp() QVERIFY(success); // The same again, with a non-interactive download. - FilesToTransfer filesToDownload = Utils::transform(filesToUpload, [&](const FileToTransfer &fileToUpload) { + const FilesToTransfer filesToDownload = transform(filesToUpload, [&](const FileToTransfer &fileToUpload) { return FileToTransfer(fileToUpload.targetFile, getDownloadFilePath(dir2ForFilesToDownload, QFileInfo(fileToUpload.sourceFile).fileName())); @@ -574,7 +575,7 @@ void tst_Ssh::sftp() void tst_Ssh::cleanupTestCase() { - Utils::Singleton::deleteAll(); + Singleton::deleteAll(); } bool tst_Ssh::waitForConnection(SshConnection &connection) diff --git a/tests/manual/ssh/shell/shell.cpp b/tests/manual/ssh/shell/shell.cpp index e7e96174fd6..33fe39f29df 100644 --- a/tests/manual/ssh/shell/shell.cpp +++ b/tests/manual/ssh/shell/shell.cpp @@ -35,6 +35,7 @@ #include using namespace QSsh; +using namespace Utils; Shell::Shell(const SshConnectionParameters ¶meters, QObject *parent) : QObject(parent), @@ -70,12 +71,12 @@ void Shell::handleConnectionError() void Shell::handleConnected() { m_shell = m_connection->createRemoteShell(); - connect(m_shell.get(), &SshRemoteProcess::started, this, &Shell::handleShellStarted); - connect(m_shell.get(), &SshRemoteProcess::readyReadStandardOutput, + connect(m_shell.get(), &QtcProcess::started, this, &Shell::handleShellStarted); + connect(m_shell.get(), &QtcProcess::readyReadStandardOutput, this, &Shell::handleRemoteStdout); - connect(m_shell.get(), &SshRemoteProcess::readyReadStandardError, + connect(m_shell.get(), &QtcProcess::readyReadStandardError, this, &Shell::handleRemoteStderr); - connect(m_shell.get(), &SshRemoteProcess::finished, this, &Shell::handleChannelClosed); + connect(m_shell.get(), &QtcProcess::done, this, &Shell::handleChannelClosed); m_shell->start(); }