From 75b43de14a64773bfa9852a79698f3893b143282 Mon Sep 17 00:00:00 2001
From: Marcus Tillmanns <marcus.tillmanns@qt.io>
Date: Mon, 21 Nov 2022 15:24:33 +0100
Subject: [PATCH] Docker: Cleanup docker process interface

Instead of creating the actual command in two places
this change moves everything into one function.

Instead of sending the environment and working directory
into the shell as arguments, we send it via the correct
docker exec cli arguments.

Instead of creating QStringLists and adding them
to a CommandLine we directly add them to the
CommandLine instead.

This also fixes an issue when the working directory
contains spaces.

Fixes: QTCREATORBUG-28476
Change-Id: I4f5b39a2dd4c86d20717dbb53003f1eb60f6c089
Reviewed-by: hjk <hjk@qt.io>
---
 src/plugins/docker/dockerdevice.cpp | 79 +++++++++++++++--------------
 1 file changed, 41 insertions(+), 38 deletions(-)

diff --git a/src/plugins/docker/dockerdevice.cpp b/src/plugins/docker/dockerdevice.cpp
index 0544d3412bd..5ca1037249c 100644
--- a/src/plugins/docker/dockerdevice.cpp
+++ b/src/plugins/docker/dockerdevice.cpp
@@ -82,8 +82,6 @@ Q_LOGGING_CATEGORY(dockerDeviceLog, "qtc.docker.device", QtWarningMsg);
 
 namespace Docker::Internal {
 
-const QString s_pidMarker = "__qtc$$qtc__";
-
 class ContainerShell : public Utils::DeviceShell
 {
 public:
@@ -156,7 +154,10 @@ public:
 
     Environment environment();
 
-    CommandLine withDockerExecCmd(const CommandLine &cmd, bool interactive = false);
+    CommandLine withDockerExecCmd(const CommandLine &cmd,
+                                  Environment *env = nullptr,
+                                  FilePath *workDir = nullptr,
+                                  bool interactive = false);
 
     bool prepareForBuild(const Target *target);
     Tasks validateMounts() const;
@@ -202,9 +203,6 @@ private:
     qint64 write(const QByteArray &data) override;
     void sendControlSignal(ControlSignal controlSignal) override;
 
-private:
-    CommandLine fullLocalCommandLine(bool interactive);
-
 private:
     DockerDevicePrivate *m_devicePrivate = nullptr;
     // Store the IDevice::ConstPtr in order to extend the lifetime of device for as long
@@ -216,29 +214,6 @@ private:
     bool m_hasReceivedFirstOutput = false;
 };
 
-CommandLine DockerProcessImpl::fullLocalCommandLine(bool interactive)
-{
-    QStringList args;
-
-    if (!m_setup.m_workingDirectory.isEmpty()) {
-        QTC_CHECK(DeviceManager::deviceForPath(m_setup.m_workingDirectory) == m_device);
-        args.append({"cd", m_setup.m_workingDirectory.path()});
-        args.append("&&");
-    }
-
-    args.append({"echo", s_pidMarker, "&&"});
-
-    const Environment &env = m_setup.m_environment;
-    for (auto it = env.constBegin(); it != env.constEnd(); ++it)
-        args.append(env.key(it) + "='" + env.expandedValueForKey(env.key(it)) + '\'');
-
-    args.append("exec");
-    args.append({m_setup.m_commandLine.executable().path(), m_setup.m_commandLine.arguments()});
-
-    CommandLine shCmd("/bin/sh", {"-c", args.join(" ")});
-    return m_devicePrivate->withDockerExecCmd(shCmd, interactive);
-}
-
 DockerProcessImpl::DockerProcessImpl(IDevice::ConstPtr device, DockerDevicePrivate *devicePrivate)
     : m_devicePrivate(devicePrivate)
     , m_device(std::move(device))
@@ -304,7 +279,14 @@ void DockerProcessImpl::start()
     if (m_setup.m_lowPriority)
         m_process.setLowPriority();
 
-    m_process.setCommand(fullLocalCommandLine(m_setup.m_processMode == ProcessMode::Writer));
+    const bool interactive = m_setup.m_processMode == ProcessMode::Writer;
+    const CommandLine fullCommandLine = m_devicePrivate
+                                            ->withDockerExecCmd(m_setup.m_commandLine,
+                                                                &m_setup.m_environment,
+                                                                &m_setup.m_workingDirectory,
+                                                                interactive);
+
+    m_process.setCommand(fullCommandLine);
     m_process.start();
 }
 
@@ -445,23 +427,44 @@ void DockerDevice::updateContainerAccess() const
     d->updateContainerAccess();
 }
 
-CommandLine DockerDevicePrivate::withDockerExecCmd(const CommandLine &cmd, bool interactive)
+CommandLine DockerDevicePrivate::withDockerExecCmd(const CommandLine &cmd,
+                                                   Environment *env,
+                                                   FilePath *workDir,
+                                                   bool interactive)
 {
     if (!m_settings)
         return {};
 
     updateContainerAccess();
 
-    QStringList args;
+    CommandLine dockerCmd{m_settings->dockerBinaryPath.filePath(), {"exec"}};
 
-    args << "exec";
     if (interactive)
-        args << "-i";
-    args << m_container;
+        dockerCmd.addArg("-i");
 
-    CommandLine dcmd{m_settings->dockerBinaryPath.filePath(), args};
-    dcmd.addCommandLineAsArgs(cmd, CommandLine::Raw);
-    return dcmd;
+    if (env) {
+        for (auto it = env->constBegin(); it != env->constEnd(); ++it) {
+            dockerCmd.addArg("-e");
+            dockerCmd.addArg(env->key(it) + "=" + env->expandedValueForKey(env->key(it)));
+        }
+    }
+
+    if (workDir && !workDir->isEmpty())
+        dockerCmd.addArgs({"-w", workDir->path()});
+
+    dockerCmd.addArg(m_container);
+    dockerCmd.addArgs({"/bin/sh", "-c"});
+
+    CommandLine exec("exec");
+    exec.addCommandLineAsArgs(cmd);
+
+    CommandLine echo("echo");
+    echo.addArgs("__qtc$$qtc__", CommandLine::Raw);
+    echo.addCommandLineWithAnd(exec);
+
+    dockerCmd.addCommandLineAsSingleArg(echo);
+
+    return dockerCmd;
 }
 
 void DockerDevicePrivate::stopCurrentContainer()