ProcessInterface: Add readyRead() signal

And get rid of readyReadStandard[Output/Error]() signals.
Get rid of readAllStandard[Output/Error]() accessors, as
this data is passed with readyRead() signal.

Task-number: QTCREATORBUG-27358
Change-Id: I01341feba650e650761f97e90f83b6042c899251
Reviewed-by: hjk <hjk@qt.io>
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>

src/libs/utils/launchersocket.cpp

@@ -33,8 +33,6 @@
 #include <QLocalSocket>
 #include <QMutexLocker>
 
-#include <iostream>
-
 namespace Utils {
 namespace Internal {
 
@@ -184,29 +182,7 @@ void CallerHandle::handleStarted(const StartedSignal *launcherSignal)
 void CallerHandle::handleReadyRead(const ReadyReadSignal *launcherSignal)
 {
     QTC_ASSERT(isCalledFromCallersThread(), return);
-    if (m_setup->m_processChannelMode == QProcess::MergedChannels) {
+    emit readyRead(launcherSignal->stdOut(), launcherSignal->stdErr());
-        m_stdout += launcherSignal->stdOut();
-        m_stdout += launcherSignal->stdErr();
-        if (!m_stdout.isEmpty())
-            emit readyReadStandardOutput();
-    } else {
-        if (m_setup->m_processChannelMode == QProcess::ForwardedOutputChannel
-                || m_setup->m_processChannelMode == QProcess::ForwardedChannels) {
-            std::cout << launcherSignal->stdOut().constData() << std::flush;
-        } else {
-            m_stdout += launcherSignal->stdOut();
-            if (!m_stdout.isEmpty())
-                emit readyReadStandardOutput();
-        }
-        if (m_setup->m_processChannelMode == QProcess::ForwardedErrorChannel
-                || m_setup->m_processChannelMode == QProcess::ForwardedChannels) {
-            std::cerr << launcherSignal->stdErr().constData() << std::flush;
-        } else {
-            m_stderr += launcherSignal->stdErr();
-            if (!m_stderr.isEmpty())
-                emit readyReadStandardError();
-        }
-    }
 }
 
 void CallerHandle::handleDone(const DoneSignal *launcherSignal)
@@ -297,18 +273,6 @@ void CallerHandle::kill()
     sendStopPacket(StopProcessPacket::SignalType::Kill);
 }
 
-QByteArray CallerHandle::readAllStandardOutput()
-{
-    QTC_ASSERT(isCalledFromCallersThread(), return {});
-    return readAndClear(m_stdout);
-}
-
-QByteArray CallerHandle::readAllStandardError()
-{
-    QTC_ASSERT(isCalledFromCallersThread(), return {});
-    return readAndClear(m_stderr);
-}
-
 qint64 CallerHandle::processId() const
 {
     QTC_ASSERT(isCalledFromCallersThread(), return 0);

src/libs/utils/launchersocket.h

@@ -93,9 +93,6 @@ public:
     void terminate();
     void kill();
 
-    QByteArray readAllStandardOutput();
-    QByteArray readAllStandardError();
-
     qint64 processId() const;
 
     void start(const QString &program, const QStringList &arguments);
@@ -113,8 +110,7 @@ public:
 signals:
     void started();
     void done(const Utils::ProcessResultData &resultData);
-    void readyReadStandardOutput();
+    void readyRead(const QByteArray &outputData, const QByteArray &errorData);
-    void readyReadStandardError();
 
 private:
     bool waitForSignal(int msecs, SignalType newSignal);
@@ -153,8 +149,6 @@ private:
     std::atomic<QProcess::ProcessState> m_processState = QProcess::NotRunning;
     std::unique_ptr<StartProcessPacket> m_startPacket;
     int m_processId = 0;
-    QByteArray m_stdout;
-    QByteArray m_stderr;
 
     QString m_command;
     QStringList m_arguments;

src/libs/utils/processinterface.h

@@ -49,7 +49,6 @@ public:
     Environment m_environment;
     Environment m_remoteEnvironment;
     QByteArray m_writeData;
-    QProcess::ProcessChannelMode m_processChannelMode = QProcess::SeparateChannels;
     QVariantHash m_extraData;
     QString m_standardInputFile;
     QString m_nativeArguments; // internal, dependent on specific code path
@@ -86,8 +85,6 @@ public:
     virtual void kill() = 0;
     virtual void close() = 0;
 
-    virtual QByteArray readAllStandardOutput() = 0;
-    virtual QByteArray readAllStandardError() = 0;
     virtual qint64 write(const QByteArray &data) = 0;
 
     virtual qint64 processId() const = 0;
@@ -102,9 +99,8 @@ public:
 
 signals:
     void started();
+    void readyRead(const QByteArray &outputData, const QByteArray &errorData);
     void done(const Utils::ProcessResultData &resultData);
-    void readyReadStandardOutput();
-    void readyReadStandardError();
 
 protected:
     ProcessSetupData::Ptr m_setup;
@@ -123,11 +119,8 @@ public:
     {
         m_target->setParent(this);
         connect(m_target, &ProcessInterface::started, this, &ProcessInterface::started);
+        connect(m_target, &ProcessInterface::readyRead, this, &ProcessInterface::readyRead);
         connect(m_target, &ProcessInterface::done, this, &ProcessInterface::done);
-        connect(m_target, &ProcessInterface::readyReadStandardOutput,
-                this, &ProcessInterface::readyReadStandardOutput);
-        connect(m_target, &ProcessInterface::readyReadStandardError,
-                this, &ProcessInterface::readyReadStandardError);
     }
 
     void start() override { m_target->start(); }
@@ -136,8 +129,6 @@ public:
     void kill() override { m_target->kill(); }
     void close() override { m_target->close(); }
 
-    QByteArray readAllStandardOutput() override { return m_target->readAllStandardOutput(); }
-    QByteArray readAllStandardError() override { return m_target->readAllStandardError(); }
     qint64 write(const QByteArray &data) override { return m_target->write(data); }
 
     qint64 processId() const override { return m_target->processId(); }

src/libs/utils/qtcprocess.cpp

@@ -56,6 +56,7 @@
 #include <atomic>
 #include <chrono>
 #include <functional>
+#include <iostream>
 #include <limits>
 #include <memory>
 
@@ -337,16 +338,15 @@ public:
                 this, &QProcessImpl::handleFinished);
         connect(m_process, &QProcess::errorOccurred,
                 this, &QProcessImpl::handleError);
-        connect(m_process, &QProcess::readyReadStandardOutput,
+        connect(m_process, &QProcess::readyReadStandardOutput, this, [this] {
-                this, &ProcessInterface::readyReadStandardOutput);
+            emit readyRead(m_process->readAllStandardOutput(), {});
-        connect(m_process, &QProcess::readyReadStandardError,
+        });
-                this, &ProcessInterface::readyReadStandardError);
+        connect(m_process, &QProcess::readyReadStandardError, this, [this] {
+            emit readyRead({}, m_process->readAllStandardError());
+        });
     }
     ~QProcessImpl() final { ProcessReaper::reap(m_process); }
 
-    QByteArray readAllStandardOutput() final { return m_process->readAllStandardOutput(); }
-    QByteArray readAllStandardError() final { return m_process->readAllStandardError(); }
-
     void interrupt() final { ProcessHelper::interruptProcess(m_process); }
     void terminate() final { ProcessHelper::terminateProcess(m_process); }
     void kill() final { m_process->kill(); }
@@ -372,7 +372,6 @@ private:
         m_process->setProcessEnvironment(m_setup->m_environment.toProcessEnvironment());
         m_process->setWorkingDirectory(m_setup->m_workingDirectory.path());
         m_process->setStandardInputFile(m_setup->m_standardInputFile);
-        m_process->setProcessChannelMode(m_setup->m_processChannelMode);
         if (m_setup->m_lowPriority)
             m_process->setLowPriority();
         if (m_setup->m_unixTerminalDisabled)
@@ -423,12 +422,10 @@ public:
         m_handle->setProcessSetupData(m_setup);
         connect(m_handle, &CallerHandle::started,
                 this, &ProcessInterface::started);
+        connect(m_handle, &CallerHandle::readyRead,
+                this, &ProcessInterface::readyRead);
         connect(m_handle, &CallerHandle::done,
                 this, &ProcessInterface::done);
-        connect(m_handle, &CallerHandle::readyReadStandardOutput,
-                this, &ProcessInterface::readyReadStandardOutput);
-        connect(m_handle, &CallerHandle::readyReadStandardError,
-                this, &ProcessInterface::readyReadStandardError);
     }
     ~ProcessLauncherImpl() final
     {
@@ -437,9 +434,6 @@ public:
         m_handle = nullptr;
     }
 
-    QByteArray readAllStandardOutput() final { return m_handle->readAllStandardOutput(); }
-    QByteArray readAllStandardError() final { return m_handle->readAllStandardError(); }
-
     void interrupt() final
     {
         if (m_setup->m_useCtrlCStub) // bypass launcher and interrupt directly
@@ -504,26 +498,10 @@ public:
 
         connect(m_process.get(), &ProcessInterface::started,
                 this, &QtcProcessPrivate::emitStarted);
+        connect(m_process.get(), &ProcessInterface::readyRead,
+                this, &QtcProcessPrivate::handleReadyRead);
         connect(m_process.get(), &ProcessInterface::done,
                 this, &QtcProcessPrivate::handleDone);
-        connect(m_process.get(), &ProcessInterface::readyReadStandardOutput,
-                this, &QtcProcessPrivate::handleReadyReadStandardOutput);
-        connect(m_process.get(), &ProcessInterface::readyReadStandardError,
-                this, &QtcProcessPrivate::handleReadyReadStandardError);
-    }
-
-    void handleReadyReadStandardOutput()
-    {
-        m_stdOut.append(m_process->readAllStandardOutput());
-        m_hangTimerCount = 0;
-        emitReadyReadStandardOutput();
-    }
-
-    void handleReadyReadStandardError()
-    {
-        m_stdErr.append(m_process->readAllStandardError());
-        m_hangTimerCount = 0;
-        emitReadyReadStandardError();
     }
 
     CommandLine fullCommandLine() const
@@ -558,6 +536,7 @@ public:
     ProcessSetupData m_setup;
 
     void slotTimeout();
+    void handleReadyRead(const QByteArray &outputData, const QByteArray &errorData);
     void handleDone(const ProcessResultData &data);
     void handleError();
     void clearForRun();
@@ -570,6 +549,7 @@ public:
 
     ProcessResult interpretExitCode(int exitCode);
 
+    QProcess::ProcessChannelMode m_processChannelMode = QProcess::SeparateChannels;
     ProcessResultData m_resultData;
 
     QTextCodec *m_codec = QTextCodec::codecForLocale();
@@ -1154,7 +1134,8 @@ qint64 QtcProcess::applicationMainThreadID() const
 
 void QtcProcess::setProcessChannelMode(QProcess::ProcessChannelMode mode)
 {
-    d->m_setup.m_processChannelMode = mode;
+    QTC_CHECK(state() == QProcess::NotRunning);
+    d->m_processChannelMode = mode;
 }
 
 QProcess::ProcessState QtcProcess::state() const
@@ -1489,11 +1470,6 @@ void QtcProcess::runBlocking(EventLoopMode eventLoopMode)
             d->m_eventLoop = &eventLoop;
             eventLoop.exec(QEventLoop::ExcludeUserInputEvents);
             d->m_eventLoop = nullptr;
-            d->m_stdOut.append(d->m_process->readAllStandardOutput());
-            d->m_stdErr.append(d->m_process->readAllStandardError());
-            d->m_stdOut.handleRest();
-            d->m_stdErr.handleRest();
-
             timer.stop();
 #ifdef QT_GUI_LIB
             if (isGuiThread())
@@ -1512,10 +1488,6 @@ void QtcProcess::runBlocking(EventLoopMode eventLoopMode)
                 kill();
                 waitForFinished(1000);
             }
-            d->m_stdOut.append(d->m_process->readAllStandardOutput());
-            d->m_stdErr.append(d->m_process->readAllStandardError());
-            d->m_stdOut.handleRest();
-            d->m_stdErr.handleRest();
         }
     }
 }
@@ -1567,6 +1539,37 @@ void QtcProcessPrivate::slotTimeout()
     }
 }
 
+void QtcProcessPrivate::handleReadyRead(const QByteArray &outputData, const QByteArray &errorData)
+{
+    // TODO: check why we need this timer?
+    m_hangTimerCount = 0;
+    // TODO: store a copy of m_processChannelMode on start()? Currently we assert that state
+    // is NotRunning when setting the process channel mode.
+    if (m_processChannelMode == QProcess::MergedChannels) {
+        m_stdOut.append(outputData);
+        m_stdOut.append(errorData);
+        if (!outputData.isEmpty() || !errorData.isEmpty())
+            emitReadyReadStandardOutput();
+    } else {
+        if (m_processChannelMode == QProcess::ForwardedOutputChannel
+                || m_processChannelMode == QProcess::ForwardedChannels) {
+            std::cout << outputData.constData() << std::flush;
+        } else {
+            m_stdOut.append(outputData);
+            if (!outputData.isEmpty())
+                emitReadyReadStandardOutput();
+        }
+        if (m_processChannelMode == QProcess::ForwardedErrorChannel
+                || m_processChannelMode == QProcess::ForwardedChannels) {
+            std::cerr << errorData.constData() << std::flush;
+        } else {
+            m_stdErr.append(errorData);
+            if (!errorData.isEmpty())
+                emitReadyReadStandardError();
+        }
+    }
+}
+
 void QtcProcessPrivate::handleDone(const ProcessResultData &data)
 {
     m_resultData = data;
@@ -1616,9 +1619,6 @@ void QtcProcessPrivate::handleError()
     // Was hang detected before and killed?
     if (m_result != ProcessResult::Hang)
         m_result = ProcessResult::StartFailed;
-    if (m_eventLoop)
-        m_eventLoop->quit();
-
     emitErrorOccurred(m_resultData.m_error);
 }
 

src/libs/utils/terminalprocess_p.h

@@ -45,9 +45,6 @@ public:
     TerminalImpl();
     ~TerminalImpl() final;
 
-    QByteArray readAllStandardOutput() final { QTC_CHECK(false); return {}; }
-    QByteArray readAllStandardError() final { QTC_CHECK(false); return {}; }
-
     qint64 write(const QByteArray &) final { QTC_CHECK(false); return -1; }
 
     void terminate() final { stopProcess(); }