From 27c8f00d73ca624e63c16bc3cf4eb5e25c2bff4e Mon Sep 17 00:00:00 2001 From: hjk Date: Tue, 29 Mar 2022 18:01:08 +0200 Subject: [PATCH] Utils: Un-seat QtcProcess from ProcessInterface I find it too confusing to have that inheritance only to enforce similar interfaces. Change-Id: I9da68ea59c04f709228e0874460e987115b30f56 Reviewed-by: Jarek Kobus --- src/libs/utils/processinterface.h | 2 +- src/libs/utils/qtcprocess.cpp | 129 +++++++++++++++--------------- src/libs/utils/qtcprocess.h | 57 +++++++------ 3 files changed, 98 insertions(+), 90 deletions(-) diff --git a/src/libs/utils/processinterface.h b/src/libs/utils/processinterface.h index 1d94f8deb85..0f8fef684f8 100644 --- a/src/libs/utils/processinterface.h +++ b/src/libs/utils/processinterface.h @@ -131,7 +131,7 @@ public: } void start() override { m_target->start(); } - void interrupt() override { m_target->interrupt(); }; + void interrupt() override { m_target->interrupt(); } void terminate() override { m_target->terminate(); } void kill() override { m_target->kill(); } void close() override { m_target->close(); } diff --git a/src/libs/utils/qtcprocess.cpp b/src/libs/utils/qtcprocess.cpp index ad0b72d2c55..3f914d37311 100644 --- a/src/libs/utils/qtcprocess.cpp +++ b/src/libs/utils/qtcprocess.cpp @@ -564,19 +564,18 @@ public: OtherFailure }; - explicit QtcProcessPrivate(QtcProcess *parent, const ProcessSetupData::Ptr &setup) + explicit QtcProcessPrivate(QtcProcess *parent) : QObject(parent) , q(parent) - , m_setup(setup) {} ProcessInterface *createProcessInterface() { - if (m_setup->m_terminalMode != TerminalMode::Off) + if (m_setup.m_terminalMode != TerminalMode::Off) return new TerminalImpl(); - const ProcessImpl impl = m_setup->m_processImpl == ProcessImpl::Default - ? defaultProcessImpl() : m_setup->m_processImpl; + const ProcessImpl impl = m_setup.m_processImpl == ProcessImpl::Default + ? defaultProcessImpl() : m_setup.m_processImpl; if (impl == ProcessImpl::QProcess) return new QProcessImpl(); return new ProcessLauncherImpl(); @@ -585,7 +584,7 @@ public: void setProcessInterface(ProcessInterface *process) { m_process.reset(process); - m_setup->m_errorString.clear(); + m_setup.m_errorString.clear(); m_process->setParent(this); connect(m_process.get(), &ProcessInterface::started, @@ -616,21 +615,21 @@ public: CommandLine fullCommandLine() const { - if (!m_setup->m_runAsRoot || HostOsInfo::isWindowsHost()) - return m_setup->m_commandLine; + if (!m_setup.m_runAsRoot || HostOsInfo::isWindowsHost()) + return m_setup.m_commandLine; CommandLine rootCommand("sudo", {"-A"}); - rootCommand.addCommandLineAsArgs(m_setup->m_commandLine); + rootCommand.addCommandLineAsArgs(m_setup.m_commandLine); return rootCommand; } Environment fullEnvironment() const { Environment env; - if (m_setup->m_haveEnv) { - if (m_setup->m_environment.size() == 0) + if (m_setup.m_haveEnv) { + if (m_setup.m_environment.size() == 0) qWarning("QtcProcess::start: Empty environment set when running '%s'.", - qPrintable(m_setup->m_commandLine.executable().toString())); - env = m_setup->m_environment; + qPrintable(m_setup.m_commandLine.executable().toString())); + env = m_setup.m_environment; } else { env = Environment::systemEnvironment(); } @@ -643,7 +642,7 @@ public: QtcProcess *q; std::unique_ptr m_process; - ProcessSetupData::Ptr m_setup; + ProcessSetupData m_setup; void slotTimeout(); void slotFinished(); @@ -708,8 +707,8 @@ qint64 ProcessInterface::applicationMainThreadID() const */ QtcProcess::QtcProcess(QObject *parent) - : ProcessInterface(parent), - d(new QtcProcessPrivate(this, m_setup)) + : QObject(parent), + d(new QtcProcessPrivate(this)) { static int qProcessExitStatusMeta = qRegisterMetaType(); static int qProcessProcessErrorMeta = qRegisterMetaType(); @@ -773,90 +772,90 @@ void QtcProcess::setProcessInterface(ProcessInterface *interface) { d->setProcessInterface(interface); // Make a copy, don't share, until we get rid of fullCommandLine() and fullEnvironment() - *d->m_process->m_setup = *d->m_setup; + *d->m_process->m_setup = d->m_setup; } void QtcProcess::setProcessImpl(ProcessImpl processImpl) { - d->m_setup->m_processImpl = processImpl; + d->m_setup.m_processImpl = processImpl; } ProcessMode QtcProcess::processMode() const { - return d->m_setup->m_processMode; + return d->m_setup.m_processMode; } void QtcProcess::setTerminalMode(TerminalMode mode) { - d->m_setup->m_terminalMode = mode; + d->m_setup.m_terminalMode = mode; } TerminalMode QtcProcess::terminalMode() const { - return d->m_setup->m_terminalMode; + return d->m_setup.m_terminalMode; } void QtcProcess::setProcessMode(ProcessMode processMode) { - d->m_setup->m_processMode = processMode; + d->m_setup.m_processMode = processMode; } void QtcProcess::setEnvironment(const Environment &env) { - d->m_setup->m_environment = env; - d->m_setup->m_haveEnv = true; + d->m_setup.m_environment = env; + d->m_setup.m_haveEnv = true; } void QtcProcess::unsetEnvironment() { - d->m_setup->m_environment = Environment(); - d->m_setup->m_haveEnv = false; + d->m_setup.m_environment = Environment(); + d->m_setup.m_haveEnv = false; } const Environment &QtcProcess::environment() const { - return d->m_setup->m_environment; + return d->m_setup.m_environment; } bool QtcProcess::hasEnvironment() const { - return d->m_setup->m_haveEnv; + return d->m_setup.m_haveEnv; } void QtcProcess::setRemoteEnvironment(const Environment &environment) { - d->m_setup->m_remoteEnvironment = environment; + d->m_setup.m_remoteEnvironment = environment; } Environment QtcProcess::remoteEnvironment() const { - return d->m_setup->m_remoteEnvironment; + return d->m_setup.m_remoteEnvironment; } void QtcProcess::setCommand(const CommandLine &cmdLine) { - if (d->m_setup->m_workingDirectory.needsDevice() && cmdLine.executable().needsDevice()) { - QTC_CHECK(d->m_setup->m_workingDirectory.host() == cmdLine.executable().host()); + if (d->m_setup.m_workingDirectory.needsDevice() && cmdLine.executable().needsDevice()) { + QTC_CHECK(d->m_setup.m_workingDirectory.host() == cmdLine.executable().host()); } - d->m_setup->m_commandLine = cmdLine; + d->m_setup.m_commandLine = cmdLine; } const CommandLine &QtcProcess::commandLine() const { - return d->m_setup->m_commandLine; + return d->m_setup.m_commandLine; } FilePath QtcProcess::workingDirectory() const { - return d->m_setup->m_workingDirectory; + return d->m_setup.m_workingDirectory; } void QtcProcess::setWorkingDirectory(const FilePath &dir) { - if (dir.needsDevice() && d->m_setup->m_commandLine.executable().needsDevice()) { - QTC_CHECK(dir.host() == d->m_setup->m_commandLine.executable().host()); + if (dir.needsDevice() && d->m_setup.m_commandLine.executable().needsDevice()) { + QTC_CHECK(dir.host() == d->m_setup.m_commandLine.executable().host()); } - d->m_setup->m_workingDirectory = dir; + d->m_setup.m_workingDirectory = dir; } void QtcProcess::setUseCtrlCStub(bool enabled) @@ -865,7 +864,7 @@ void QtcProcess::setUseCtrlCStub(bool enabled) // Qt Creator otherwise, because they share the same Windows console. // See QTCREATORBUG-11995 for details. #ifndef QT_DEBUG - d->m_setup->m_useCtrlCStub = enabled; + d->m_setup.m_useCtrlCStub = enabled; #else Q_UNUSED(enabled) #endif @@ -877,7 +876,7 @@ void QtcProcess::start() // QTC_ASSERT(state() == QProcess::NotRunning, return); ProcessInterface *processImpl = nullptr; - if (d->m_setup->m_commandLine.executable().needsDevice()) { + if (d->m_setup.m_commandLine.executable().needsDevice()) { if (s_deviceHooks.processImplHook) { // TODO: replace "if" with an assert for the hook processImpl = s_deviceHooks.processImplHook(commandLine().executable()); } @@ -929,7 +928,7 @@ BOOL CALLBACK sendInterruptMessageToAllWindowsOfProcess_enumWnd(HWND hwnd, LPARA void QtcProcess::terminate() { #ifdef Q_OS_WIN - if (d->m_setup->m_useCtrlCStub) + if (d->m_setup.m_useCtrlCStub) EnumWindows(sendShutDownMessageToAllWindowsOfProcess_enumWnd, processId()); else #endif @@ -940,7 +939,7 @@ void QtcProcess::terminate() void QtcProcess::interrupt() { #ifdef Q_OS_WIN - if (d->m_setup->m_useCtrlCStub) + if (d->m_setup.m_useCtrlCStub) EnumWindows(sendInterruptMessageToAllWindowsOfProcess_enumWnd, processId()); else #endif @@ -958,71 +957,71 @@ bool QtcProcess::startDetached(const CommandLine &cmd, const FilePath &workingDi void QtcProcess::setLowPriority() { - d->m_setup->m_lowPriority = true; + d->m_setup.m_lowPriority = true; } void QtcProcess::setDisableUnixTerminal() { - d->m_setup->m_unixTerminalDisabled = true; + d->m_setup.m_unixTerminalDisabled = true; } void QtcProcess::setAbortOnMetaChars(bool abort) { - d->m_setup->m_abortOnMetaChars = abort; + d->m_setup.m_abortOnMetaChars = abort; } void QtcProcess::setRunAsRoot(bool on) { - d->m_setup->m_runAsRoot = on; + d->m_setup.m_runAsRoot = on; } bool QtcProcess::isRunAsRoot() const { - return d->m_setup->m_runAsRoot; + return d->m_setup.m_runAsRoot; } void QtcProcess::setStandardInputFile(const QString &inputFile) { - d->m_setup->m_standardInputFile = inputFile; + d->m_setup.m_standardInputFile = inputFile; } QString QtcProcess::toStandaloneCommandLine() const { QStringList parts; parts.append("/usr/bin/env"); - if (!d->m_setup->m_workingDirectory.isEmpty()) { + if (!d->m_setup.m_workingDirectory.isEmpty()) { parts.append("-C"); - d->m_setup->m_workingDirectory.path(); + d->m_setup.m_workingDirectory.path(); } parts.append("-i"); - if (d->m_setup->m_environment.size() > 0) { - const QStringList envVars = d->m_setup->m_environment.toStringList(); + if (d->m_setup.m_environment.size() > 0) { + const QStringList envVars = d->m_setup.m_environment.toStringList(); std::transform(envVars.cbegin(), envVars.cend(), std::back_inserter(parts), ProcessArgs::quoteArgUnix); } - parts.append(d->m_setup->m_commandLine.executable().path()); - parts.append(d->m_setup->m_commandLine.splitArguments()); + parts.append(d->m_setup.m_commandLine.executable().path()); + parts.append(d->m_setup.m_commandLine.splitArguments()); return parts.join(" "); } void QtcProcess::setExtraData(const QString &key, const QVariant &value) { - d->m_setup->m_extraData.insert(key, value); + d->m_setup.m_extraData.insert(key, value); } QVariant QtcProcess::extraData(const QString &key) const { - return d->m_setup->m_extraData.value(key); + return d->m_setup.m_extraData.value(key); } void QtcProcess::setExtraData(const QVariantHash &extraData) { - d->m_setup->m_extraData = extraData; + d->m_setup.m_extraData = extraData; } QVariantHash QtcProcess::extraData() const { - return d->m_setup->m_extraData; + return d->m_setup.m_extraData; } void QtcProcess::setRemoteProcessHooks(const DeviceProcessHooks &hooks) @@ -1107,7 +1106,7 @@ bool QtcProcess::readDataFromProcess(int timeoutS, } // Prompt user, pretend we have data if says 'No'. const bool hang = !hasData && !finished; - hasData = hang && showTimeOutMessageBox && !askToKill(d->m_setup->m_commandLine.executable().path()); + hasData = hang && showTimeOutMessageBox && !askToKill(d->m_setup.m_commandLine.executable().path()); } while (hasData && !finished); if (syncDebug) qDebug() << "m_setup->m_processChannelMode = mode; + d->m_setup.m_processChannelMode = mode; } QProcess::ProcessError QtcProcess::error() const @@ -1277,7 +1276,7 @@ QString QtcProcess::errorString() const { if (d->m_process) return d->m_process->errorString(); - return d->m_setup->m_errorString; + return d->m_setup.m_errorString; } void QtcProcess::setErrorString(const QString &str) @@ -1285,7 +1284,7 @@ void QtcProcess::setErrorString(const QString &str) if (d->m_process) d->m_process->setErrorString(str); else - d->m_setup->m_errorString = str; + d->m_setup.m_errorString = str; } qint64 QtcProcess::processId() const @@ -1584,7 +1583,7 @@ void QtcProcess::setExitCodeInterpreter(const ExitCodeInterpreter &interpreter) void QtcProcess::setWriteData(const QByteArray &writeData) { - d->m_setup->m_writeData = writeData; + d->m_setup.m_writeData = writeData; } #ifdef QT_GUI_LIB @@ -1598,7 +1597,7 @@ void QtcProcess::runBlocking(EventLoopMode eventLoopMode) { // FIXME: Implement properly - if (d->m_setup->m_commandLine.executable().needsDevice()) { + if (d->m_setup.m_commandLine.executable().needsDevice()) { QtcProcess::start(); waitForFinished(); return; @@ -1696,7 +1695,7 @@ void QtcProcessPrivate::slotTimeout() qDebug() << Q_FUNC_INFO << "HANG detected, killing"; m_waitingForUser = true; const bool terminate = !m_timeOutMessageBoxEnabled - || askToKill(m_setup->m_commandLine.executable().toString()); + || askToKill(m_setup.m_commandLine.executable().toString()); m_waitingForUser = false; if (terminate) { q->stopProcess(); diff --git a/src/libs/utils/qtcprocess.h b/src/libs/utils/qtcprocess.h index 3a6b7a53c41..0bf9bf2c524 100644 --- a/src/libs/utils/qtcprocess.h +++ b/src/libs/utils/qtcprocess.h @@ -30,13 +30,14 @@ #include "environment.h" #include "commandline.h" #include "processenums.h" -#include "processinterface.h" #include "qtcassert.h" #include -QT_FORWARD_DECLARE_CLASS(QDebug) -QT_FORWARD_DECLARE_CLASS(QTextCodec) +QT_BEGIN_NAMESPACE +class QDebug; +class QTextCodec; +QT_END_NAMESPACE class tst_QtcProcess; @@ -45,8 +46,9 @@ namespace Utils { namespace Internal { class QtcProcessPrivate; } class DeviceProcessHooks; +class ProcessInterface; -class QTCREATOR_UTILS_EXPORT QtcProcess : public ProcessInterface +class QTCREATOR_UTILS_EXPORT QtcProcess : public QObject { Q_OBJECT @@ -56,31 +58,31 @@ public: // ProcessInterface related - void start() override; - void interrupt() override; - void terminate() override; - void kill() override; - void close() final; + virtual void start(); + virtual void interrupt(); + virtual void terminate(); + virtual void kill(); + void close(); - QByteArray readAllStandardOutput() override; - QByteArray readAllStandardError() override; - qint64 write(const QByteArray &input) override; + virtual QByteArray readAllStandardOutput(); + virtual QByteArray readAllStandardError(); + virtual qint64 write(const QByteArray &input); - qint64 processId() const override; - QProcess::ProcessState state() const override; - int exitCode() const override; - QProcess::ExitStatus exitStatus() const override; + virtual qint64 processId() const; + virtual QProcess::ProcessState state() const; + virtual int exitCode() const; + virtual QProcess::ExitStatus exitStatus() const; - QProcess::ProcessError error() const final; - QString errorString() const override; - void setErrorString(const QString &str) final; + QProcess::ProcessError error() const; + virtual QString errorString() const; + void setErrorString(const QString &str); - bool waitForStarted(int msecs = 30000) final; - bool waitForReadyRead(int msecs = 30000) final; - bool waitForFinished(int msecs = 30000) final; + bool waitForStarted(int msecs = 30000); + bool waitForReadyRead(int msecs = 30000); + bool waitForFinished(int msecs = 30000); - void kickoffProcess() final; - qint64 applicationMainThreadID() const final; + void kickoffProcess(); + qint64 applicationMainThreadID() const; // ProcessSetupData related @@ -185,6 +187,13 @@ public: QString toStandaloneCommandLine() const; +signals: + void started(); + void finished(); + void errorOccurred(QProcess::ProcessError error); + void readyReadStandardOutput(); + void readyReadStandardError(); + protected: // TODO: remove these methods on QtcProcess de-virtualization virtual void emitStarted();