From 770d87709ee4d67e24b4a8d4b3fd2bdd0eff2745 Mon Sep 17 00:00:00 2001 From: hjk Date: Thu, 20 May 2021 13:14:15 +0200 Subject: [PATCH] Utils: Join SynchronousProcess::run and runBlocking implementations Make functionality dependent on an (intentionally ugly) setProcessUserEventWhileRunning call. Also, back-paddle a bit on API combination of QtcProcess and SynchronousPrceoss for now and prevent the QtcProcess-and- runBlocking and SynchronousProcess-and-start combinations. Goal is still to have all in QtcProcess in the end, but this may take a while. Change-Id: Ic146ec5db0ab8dc9613e5b2af5f4dc90bc7465ca Reviewed-by: Christian Stenger --- src/libs/utils/qtcprocess.cpp | 146 ++++++++---------- src/libs/utils/qtcprocess.h | 20 +-- src/libs/utils/shellcommand.cpp | 3 +- src/plugins/android/androidbuildapkstep.cpp | 3 +- .../androidcreatekeystorecertificate.cpp | 3 +- src/plugins/android/androiddeployqtstep.cpp | 3 +- src/plugins/android/androidmanager.cpp | 12 +- src/plugins/android/androidsdkmanager.cpp | 6 +- src/plugins/clearcase/clearcaseplugin.cpp | 3 +- src/plugins/ios/iosprobe.cpp | 3 +- src/plugins/perforce/perforceplugin.cpp | 3 +- .../customwizardscriptgenerator.cpp | 3 +- 12 files changed, 102 insertions(+), 106 deletions(-) diff --git a/src/libs/utils/qtcprocess.cpp b/src/libs/utils/qtcprocess.cpp index 1d568ec2331..2ecb538ea9b 100644 --- a/src/libs/utils/qtcprocess.cpp +++ b/src/libs/utils/qtcprocess.cpp @@ -134,6 +134,7 @@ public: bool m_timeOutMessageBoxEnabled = false; bool m_waitingForUser = false; bool m_isSynchronousProcess = false; + bool m_processUserEvents = false; }; void QtcProcessPrivate::clearForRun() @@ -744,6 +745,11 @@ SynchronousProcess::~SynchronousProcess() disconnect(this, nullptr, this, nullptr); } +void SynchronousProcess::setProcessUserEventWhileRunning() +{ + d->m_processUserEvents = true; +} + void QtcProcess::setTimeoutS(int timeoutS) { QTC_CHECK(d->m_isSynchronousProcess); @@ -783,14 +789,14 @@ static bool isGuiThread() } #endif -void QtcProcess::run() +void SynchronousProcess::runBlocking() { QTC_CHECK(d->m_isSynchronousProcess); // FIXME: Implement properly if (d->m_commandLine.executable().needsDevice()) { // writeData ? - start(); + QtcProcess::start(); waitForFinished(); @@ -801,99 +807,75 @@ void QtcProcess::run() return; }; - qCDebug(processLog).noquote() << "Starting:" << d->m_commandLine.toUserOutput(); + qCDebug(processLog).noquote() << "Starting blocking:" << d->m_commandLine.toUserOutput() + << " process user events: " << d->m_processUserEvents; ExecuteOnDestruction logResult([this] { qCDebug(processLog) << *this; }); d->clearForRun(); d->m_binary = d->m_commandLine.executable(); - // using QProcess::start() and passing program, args and OpenMode results in a different - // quoting of arguments than using QProcess::setArguments() beforehand and calling start() - // only with the OpenMode - if (!d->m_writeData.isEmpty()) { - connect(this, &QProcess::started, this, [this] { - write(d->m_writeData); - closeWriteChannel(); - }); - } - setOpenMode(d->m_writeData.isEmpty() ? QIODevice::ReadOnly : QIODevice::ReadWrite); - start(); - // On Windows, start failure is triggered immediately if the - // executable cannot be found in the path. Do not start the - // event loop in that case. - if (!d->m_startFailure) { - d->m_timer.start(); + if (d->m_processUserEvents) { + if (!d->m_writeData.isEmpty()) { + connect(this, &QProcess::started, this, [this] { + write(d->m_writeData); + closeWriteChannel(); + }); + } + setOpenMode(d->m_writeData.isEmpty() ? QIODevice::ReadOnly : QIODevice::ReadWrite); + QtcProcess::start(); + + // On Windows, start failure is triggered immediately if the + // executable cannot be found in the path. Do not start the + // event loop in that case. + if (!d->m_startFailure) { + d->m_timer.start(); #ifdef QT_GUI_LIB - if (isGuiThread()) - QApplication::setOverrideCursor(Qt::WaitCursor); + if (isGuiThread()) + QApplication::setOverrideCursor(Qt::WaitCursor); #endif - d->m_eventLoop.exec(QEventLoop::ExcludeUserInputEvents); + d->m_eventLoop.exec(QEventLoop::ExcludeUserInputEvents); + d->m_stdOut.append(readAllStandardOutput(), false); + d->m_stdErr.append(readAllStandardError(), false); + + d->m_timer.stop(); +#ifdef QT_GUI_LIB + if (isGuiThread()) + QApplication::restoreOverrideCursor(); +#endif + } + } else { + setOpenMode(QIODevice::ReadOnly); + QtcProcess::start(); + if (!waitForStarted(d->m_maxHangTimerCount * 1000)) { + d->m_result = QtcProcess::StartFailed; + return; + } + closeWriteChannel(); + if (!waitForFinished(d->m_maxHangTimerCount * 1000)) { + d->m_result = QtcProcess::Hang; + terminate(); + if (!waitForFinished(1000)) { + kill(); + waitForFinished(1000); + } + } + + if (state() != QProcess::NotRunning) + return; + + d->m_exitCode = exitCode(); + if (d->m_result == QtcProcess::StartFailed) { + if (exitStatus() != QProcess::NormalExit) + d->m_result = QtcProcess::TerminatedAbnormally; + else + d->m_result = d->interpretExitCode(d->m_exitCode); + } d->m_stdOut.append(readAllStandardOutput(), false); d->m_stdErr.append(readAllStandardError(), false); - - d->m_timer.stop(); -#ifdef QT_GUI_LIB - if (isGuiThread()) - QApplication::restoreOverrideCursor(); -#endif } } -void QtcProcess::runBlocking() -{ - QTC_CHECK(d->m_isSynchronousProcess); - // FIXME: Implement properly - if (d->m_commandLine.executable().needsDevice()) { - - // writeData ? - start(); - - waitForFinished(); - - d->m_result = QtcProcess::Finished; - d->m_exitCode = exitCode(); - d->m_stdOut.rawData += readAllStandardOutput(); - d->m_stdErr.rawData += readAllStandardError(); - return; - }; - - qCDebug(processLog).noquote() << "Starting blocking:" << d->m_commandLine.toUserOutput(); - ExecuteOnDestruction logResult([this] { qCDebug(processLog) << *this; }); - - d->clearForRun(); - - d->m_binary = d->m_commandLine.executable(); - setOpenMode(QIODevice::ReadOnly); - start(); - if (!waitForStarted(d->m_maxHangTimerCount * 1000)) { - d->m_result = QtcProcess::StartFailed; - return; - } - closeWriteChannel(); - if (!waitForFinished(d->m_maxHangTimerCount * 1000)) { - d->m_result = QtcProcess::Hang; - terminate(); - if (!waitForFinished(1000)) { - kill(); - waitForFinished(1000); - } - } - - if (state() != QProcess::NotRunning) - return; - - d->m_exitCode = exitCode(); - if (d->m_result == QtcProcess::StartFailed) { - if (exitStatus() != QProcess::NormalExit) - d->m_result = QtcProcess::TerminatedAbnormally; - else - d->m_result = d->interpretExitCode(d->m_exitCode); - } - d->m_stdOut.append(readAllStandardOutput(), false); - d->m_stdErr.append(readAllStandardError(), false); -} - void QtcProcess::setStdOutCallback(const std::function &callback) { QTC_CHECK(d->m_isSynchronousProcess); diff --git a/src/libs/utils/qtcprocess.h b/src/libs/utils/qtcprocess.h index b19760a6ce4..d2c1da1d617 100644 --- a/src/libs/utils/qtcprocess.h +++ b/src/libs/utils/qtcprocess.h @@ -89,16 +89,6 @@ public: // FIXME: This is currently only used in run(), not in start() void setWriteData(const QByteArray &writeData); - // Starts a nested event loop and runs the command - void run(); - - // Starts the command blocking the UI fully - void runBlocking(); - - // FIXME: Remove. Kept for downstream for a while. - void run(const CommandLine &cmd) { setCommand(cmd); run(); } - void runBlocking(const CommandLine &cmd) { setCommand(cmd); runBlocking(); } - void setStdOutCallback(const std::function &callback); void setStdErrCallback(const std::function &callback); @@ -156,6 +146,16 @@ class QTCREATOR_UTILS_EXPORT SynchronousProcess : public QtcProcess public: SynchronousProcess(); ~SynchronousProcess() override; + + // Force the use of 'runBlocking' for now. + void start() = delete; + + // This starts a nested event loop when running the command. + void setProcessUserEventWhileRunning(); // Avoid. + + // Starts the command and waits for finish. User input processing depends + // on whether setProcessUserEventWhileRunning was called. + void runBlocking(); }; } // namespace Utils diff --git a/src/libs/utils/shellcommand.cpp b/src/libs/utils/shellcommand.cpp index dd811cc259d..0da6738bfe9 100644 --- a/src/libs/utils/shellcommand.cpp +++ b/src/libs/utils/shellcommand.cpp @@ -422,7 +422,8 @@ void ShellCommand::runSynchronous(SynchronousProcess &process, if (d->m_codec) process.setCodec(d->m_codec); - process.run(); + process.setProcessUserEventWhileRunning(); + process.runBlocking(); } const QVariant &ShellCommand::cookie() const diff --git a/src/plugins/android/androidbuildapkstep.cpp b/src/plugins/android/androidbuildapkstep.cpp index fa6351b43b9..8dcc9d62e32 100644 --- a/src/plugins/android/androidbuildapkstep.cpp +++ b/src/plugins/android/androidbuildapkstep.cpp @@ -1010,7 +1010,8 @@ QAbstractItemModel *AndroidBuildApkStep::keystoreCertificates() SynchronousProcess keytoolProc; keytoolProc.setTimeoutS(30); keytoolProc.setCommand({AndroidConfigurations::currentConfig().keytoolPath(), params}); - keytoolProc.run(); + keytoolProc.setProcessUserEventWhileRunning(); + keytoolProc.runBlocking(); if (keytoolProc.result() > QtcProcess::FinishedError) QMessageBox::critical(nullptr, tr("Error"), tr("Failed to run keytool.")); else diff --git a/src/plugins/android/androidcreatekeystorecertificate.cpp b/src/plugins/android/androidcreatekeystorecertificate.cpp index 8bd787c8ba1..9549f5157f2 100644 --- a/src/plugins/android/androidcreatekeystorecertificate.cpp +++ b/src/plugins/android/androidcreatekeystorecertificate.cpp @@ -199,7 +199,8 @@ void AndroidCreateKeystoreCertificate::buttonBoxAccepted() SynchronousProcess genKeyCertProc; genKeyCertProc.setTimeoutS(15); genKeyCertProc.setCommand(command); - genKeyCertProc.run(); + genKeyCertProc.setProcessUserEventWhileRunning(); + genKeyCertProc.runBlocking(); if (genKeyCertProc.result() != QtcProcess::Finished || genKeyCertProc.exitCode() != 0) { QMessageBox::critical(this, tr("Error"), diff --git a/src/plugins/android/androiddeployqtstep.cpp b/src/plugins/android/androiddeployqtstep.cpp index fcb1106ffb4..99203853ad3 100644 --- a/src/plugins/android/androiddeployqtstep.cpp +++ b/src/plugins/android/androiddeployqtstep.cpp @@ -483,7 +483,8 @@ void AndroidDeployQtStep::runCommand(const CommandLine &command) OutputFormat::NormalMessage); buildProc.setCommand(command); - buildProc.run(); + buildProc.setProcessUserEventWhileRunning(); + buildProc.runBlocking(); if (buildProc.result() != QtcProcess::Finished || buildProc.exitCode() != 0) { const QString error = buildProc.exitMessage(); emit addOutput(error, OutputFormat::ErrorMessage); diff --git a/src/plugins/android/androidmanager.cpp b/src/plugins/android/androidmanager.cpp index 6023e80b2d3..cf50cee4017 100644 --- a/src/plugins/android/androidmanager.cpp +++ b/src/plugins/android/androidmanager.cpp @@ -539,7 +539,8 @@ bool AndroidManager::checkKeystorePassword(const QString &keystorePath, const QS SynchronousProcess proc; proc.setTimeoutS(10); proc.setCommand(cmd); - proc.run(); + proc.setProcessUserEventWhileRunning(); + proc.runBlocking(); return proc.result() == QtcProcess::Finished && proc.exitCode() == 0; } @@ -556,7 +557,8 @@ bool AndroidManager::checkCertificatePassword(const QString &keystorePath, const SynchronousProcess proc; proc.setTimeoutS(10); proc.setCommand({AndroidConfigurations::currentConfig().keytoolPath(), arguments}); - proc.run(); + proc.setProcessUserEventWhileRunning(); + proc.runBlocking(); return proc.result() == QtcProcess::Finished && proc.exitCode() == 0; } @@ -570,7 +572,8 @@ bool AndroidManager::checkCertificateExists(const QString &keystorePath, SynchronousProcess proc; proc.setTimeoutS(10); proc.setCommand({AndroidConfigurations::currentConfig().keytoolPath(), arguments}); - proc.run(); + proc.setProcessUserEventWhileRunning(); + proc.runBlocking(); return proc.result() == QtcProcess::Finished && proc.exitCode() == 0; } @@ -727,7 +730,8 @@ SdkToolResult AndroidManager::runCommand(const CommandLine &command, cmdProc.setWriteData(writeData); qCDebug(androidManagerLog) << "Running command (sync):" << command.toUserOutput(); cmdProc.setCommand(command); - cmdProc.run(); + cmdProc.setProcessUserEventWhileRunning(); + cmdProc.runBlocking(); cmdResult.m_stdOut = cmdProc.stdOut().trimmed(); cmdResult.m_stdErr = cmdProc.stdErr().trimmed(); cmdResult.m_success = cmdProc.result() == QtcProcess::Finished; diff --git a/src/plugins/android/androidsdkmanager.cpp b/src/plugins/android/androidsdkmanager.cpp index 1800d67ccdf..d42c83587d2 100644 --- a/src/plugins/android/androidsdkmanager.cpp +++ b/src/plugins/android/androidsdkmanager.cpp @@ -155,7 +155,8 @@ static bool sdkManagerCommand(const AndroidConfig &config, const QStringList &ar proc.setTimeoutS(timeout); proc.setTimeOutMessageBoxEnabled(true); proc.setCommand({config.sdkManagerToolPath(), newArgs}); - proc.run(); + proc.setProcessUserEventWhileRunning(); + proc.runBlocking(); if (output) *output = proc.allOutput(); return proc.result() == QtcProcess::Finished; @@ -196,7 +197,8 @@ static void sdkManagerCommand(const AndroidConfig &config, const QStringList &ar &proc, &SynchronousProcess::stopProcess); } proc.setCommand({config.sdkManagerToolPath(), newArgs}); - proc.run(); + proc.setProcessUserEventWhileRunning(); + proc.runBlocking(); if (assertionFound) { output.success = false; output.stdOutput = proc.stdOut(); diff --git a/src/plugins/clearcase/clearcaseplugin.cpp b/src/plugins/clearcase/clearcaseplugin.cpp index 4b1d0090b88..2654b3fbe53 100644 --- a/src/plugins/clearcase/clearcaseplugin.cpp +++ b/src/plugins/clearcase/clearcaseplugin.cpp @@ -2359,7 +2359,8 @@ QString ClearCasePluginPrivate::runExtDiff(const QString &workingDir, const QStr process.setWorkingDirectory(workingDir); process.setCodec(outputCodec ? outputCodec : QTextCodec::codecForName("UTF-8")); process.setCommand(diff); - process.run(); + process.setProcessUserEventWhileRunning(); + process.runBlocking(); if (process.result() != QtcProcess::Finished) return QString(); return process.allOutput(); diff --git a/src/plugins/ios/iosprobe.cpp b/src/plugins/ios/iosprobe.cpp index bfcb923ae71..7adaea6d61b 100644 --- a/src/plugins/ios/iosprobe.cpp +++ b/src/plugins/ios/iosprobe.cpp @@ -68,7 +68,8 @@ void XcodeProbe::detectDeveloperPaths() Utils::SynchronousProcess selectedXcode; selectedXcode.setTimeoutS(5); selectedXcode.setCommand({"/usr/bin/xcode-select", {"--print-path"}}); - selectedXcode.run(); + selectedXcode.setProcessUserEventWhileRunning(); + selectedXcode.runBlocking(); if (selectedXcode.result() != QtcProcess::Finished) qCWarning(probeLog) << QString::fromLatin1("Could not detect selected Xcode using xcode-select"); diff --git a/src/plugins/perforce/perforceplugin.cpp b/src/plugins/perforce/perforceplugin.cpp index 237f0a30d4b..6861ff9a0e2 100644 --- a/src/plugins/perforce/perforceplugin.cpp +++ b/src/plugins/perforce/perforceplugin.cpp @@ -1273,7 +1273,8 @@ PerforceResponse PerforcePluginPrivate::synchronousProcess(const QString &workin } process.setTimeOutMessageBoxEnabled(true); process.setCommand({m_settings.p4BinaryPath.value(), args}); - process.run(); + process.setProcessUserEventWhileRunning(); + process.runBlocking(); PerforceResponse response; response.error = true; diff --git a/src/plugins/projectexplorer/customwizard/customwizardscriptgenerator.cpp b/src/plugins/projectexplorer/customwizard/customwizardscriptgenerator.cpp index 90f4ba3ba75..daea2672e23 100644 --- a/src/plugins/projectexplorer/customwizard/customwizardscriptgenerator.cpp +++ b/src/plugins/projectexplorer/customwizard/customwizardscriptgenerator.cpp @@ -113,7 +113,8 @@ static bool qDebug("In %s, running:\n%s\n", qPrintable(workingDirectory), qPrintable(cmd.toUserOutput())); process.setCommand(cmd); - process.run(); + process.setProcessUserEventWhileRunning(); + process.runBlocking(); if (process.result() != Utils::QtcProcess::Finished) { *errorMessage = QString("Generator script failed: %1").arg(process.exitMessage()); const QString stdErr = process.stdErr();