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 <christian.stenger@qt.io>

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,15 +807,15 @@ 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()
+    if (d->m_processUserEvents) {
-    // only with the OpenMode
         if (!d->m_writeData.isEmpty()) {
             connect(this, &QProcess::started, this, [this] {
                 write(d->m_writeData);
@@ -817,7 +823,7 @@ void QtcProcess::run()
             });
         }
         setOpenMode(d->m_writeData.isEmpty() ? QIODevice::ReadOnly : QIODevice::ReadWrite);
-    start();
+        QtcProcess::start();
 
         // On Windows, start failure is triggered immediately if the
         // executable cannot be found in the path. Do not start the
@@ -838,34 +844,9 @@ void QtcProcess::run()
                 QApplication::restoreOverrideCursor();
 #endif
         }
-}
+    } else {
-
-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();
+        QtcProcess::start();
         if (!waitForStarted(d->m_maxHangTimerCount * 1000)) {
             d->m_result = QtcProcess::StartFailed;
             return;
@@ -893,6 +874,7 @@ void QtcProcess::runBlocking()
         d->m_stdOut.append(readAllStandardOutput(), false);
         d->m_stdErr.append(readAllStandardError(), false);
     }
+}
 
 void QtcProcess::setStdOutCallback(const std::function<void (const QString &)> &callback)
 {

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<void(const QString &)> &callback);
     void setStdErrCallback(const std::function<void(const QString &)> &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

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

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

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"),

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);

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;

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();

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();

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");

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;

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();