ProjectExplorer: Funnel environment fetching through device access

Easier nowadays. It's synchronous now, so conceptually worse,
but 'env' is not worse than 'ls', and this is used regularly
all over the place also for larger data sets without being a problem.

Change-Id: I2c2b1db8c07ff5c128700d4a1deefd710967568a
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

src/libs/utils/devicefileaccess.cpp

@@ -179,6 +179,12 @@ void DeviceFileAccess::iterateDirectory(const FilePath &filePath,
     QTC_CHECK(false);
 }
 
+Environment DeviceFileAccess::deviceEnvironment() const
+{
+    QTC_CHECK(false);
+    return {};
+}
+
 expected_str<QByteArray> DeviceFileAccess::fileContents(const FilePath &filePath,
                                                         qint64 limit,
                                                         qint64 offset) const
@@ -534,6 +540,11 @@ void DesktopDeviceFileAccess::iterateDirectory(const FilePath &filePath,
     }
 }
 
+Environment DesktopDeviceFileAccess::deviceEnvironment() const
+{
+    return Environment::systemEnvironment();
+}
+
 expected_str<QByteArray> DesktopDeviceFileAccess::fileContents(const FilePath &filePath,
                                                                qint64 limit,
                                                                qint64 offset) const
@@ -1077,4 +1088,11 @@ void UnixDeviceFileAccess::iterateDirectory(const FilePath &filePath,
     iterateLsOutput(filePath, entries, filter, callBack);
 }
 
-} // namespace Utils
+Environment UnixDeviceFileAccess::deviceEnvironment() const
+{
+    const RunResult result = runInShell({"env", {}, OsType::OsTypeLinux});
+    const QString out = QString::fromUtf8(result.stdOut);
+    return Environment(out.split('\n', Qt::SkipEmptyParts), OsTypeLinux);
+}
+
+} // Utils

src/libs/utils/devicefileaccess.h

@@ -15,6 +15,8 @@ class QTCREATOR_UTILS_EXPORT DeviceFileAccess
 public:
     virtual ~DeviceFileAccess();
 
+    virtual Environment deviceEnvironment() const;
+
 protected:
     friend class FilePath;
 
@@ -123,6 +125,8 @@ protected:
             const FilePath::IterateDirCallback &callBack,
             const FileFilter &filter) const override;
 
+    Environment deviceEnvironment() const override;
+
     expected_str<QByteArray> fileContents(const FilePath &filePath,
                                           qint64 limit,
                                           qint64 offset) const override;
@@ -172,6 +176,7 @@ protected:
             const FilePath::IterateDirCallback &callBack,
             const FileFilter &filter) const override;
 
+    Environment deviceEnvironment() const override;
     expected_str<QByteArray> fileContents(const FilePath &filePath,
                                           qint64 limit,
                                           qint64 offset) const override;

src/plugins/projectexplorer/devicesupport/desktopdevice.cpp

@@ -80,22 +80,6 @@ DeviceProcessSignalOperation::Ptr DesktopDevice::signalOperation() const
     return DeviceProcessSignalOperation::Ptr(new DesktopProcessSignalOperation());
 }
 
-class DesktopDeviceEnvironmentFetcher : public DeviceEnvironmentFetcher
-{
-public:
-    DesktopDeviceEnvironmentFetcher() = default;
-
-    void start() override
-    {
-        emit finished(Utils::Environment::systemEnvironment(), true);
-    }
-};
-
-DeviceEnvironmentFetcher::Ptr DesktopDevice::environmentFetcher() const
-{
-    return DeviceEnvironmentFetcher::Ptr(new DesktopDeviceEnvironmentFetcher());
-}
-
 PortsGatheringMethod DesktopDevice::portsGatheringMethod() const
 {
     return {

src/plugins/projectexplorer/devicesupport/desktopdevice.h

@@ -28,7 +28,6 @@ public:
     DeviceProcessList *createProcessListModel(QObject *parent) const override;
     ProjectExplorer::PortsGatheringMethod portsGatheringMethod() const override;
     DeviceProcessSignalOperation::Ptr signalOperation() const override;
-    DeviceEnvironmentFetcher::Ptr environmentFetcher() const override;
     QUrl toolControlChannel(const ControlChannelHint &) const override;
     bool usableAsBuildDevice() const override;
 

src/plugins/projectexplorer/devicesupport/idevice.cpp

@@ -251,8 +251,9 @@ FileTransferInterface *IDevice::createFileTransferInterface(
 
 Environment IDevice::systemEnvironment() const
 {
-    QTC_CHECK(false);
+    DeviceFileAccess *access = fileAccess();
-    return Environment::systemEnvironment();
+    QTC_ASSERT(access, return Environment::systemEnvironment());
+    return access->deviceEnvironment();
 }
 
 /*!
@@ -391,11 +392,6 @@ DeviceProcessSignalOperation::Ptr IDevice::signalOperation() const
     return {};
 }
 
-DeviceEnvironmentFetcher::Ptr IDevice::environmentFetcher() const
-{
-    return {};
-}
-
 IDevice::DeviceState IDevice::deviceState() const
 {
     return d->deviceState;
@@ -652,8 +648,6 @@ void DeviceProcessSignalOperation::setDebuggerCommand(const FilePath &cmd)
 
 DeviceProcessSignalOperation::DeviceProcessSignalOperation() = default;
 
-DeviceEnvironmentFetcher::DeviceEnvironmentFetcher() = default;
-
 void DeviceProcessKiller::start()
 {
     m_signalOperation.reset();

src/plugins/projectexplorer/devicesupport/idevice.h

@@ -76,21 +76,6 @@ protected:
     QString m_errorMessage;
 };
 
-class PROJECTEXPLORER_EXPORT DeviceEnvironmentFetcher : public QObject
-{
-    Q_OBJECT
-public:
-    using Ptr = QSharedPointer<DeviceEnvironmentFetcher>;
-
-    virtual void start() = 0;
-
-signals:
-    void finished(const Utils::Environment &env, bool success);
-
-protected:
-    explicit DeviceEnvironmentFetcher();
-};
-
 class PROJECTEXPLORER_EXPORT PortsGatheringMethod final
 {
 public:
@@ -163,7 +148,6 @@ public:
     virtual DeviceTester *createDeviceTester() const;
 
     virtual DeviceProcessSignalOperation::Ptr signalOperation() const;
-    virtual DeviceEnvironmentFetcher::Ptr environmentFetcher() const;
 
     enum DeviceState { DeviceReadyToUse, DeviceConnected, DeviceDisconnected, DeviceStateUnknown };
     DeviceState deviceState() const;

src/plugins/remotelinux/CMakeLists.txt

@@ -25,7 +25,6 @@ add_qtc_plugin(RemoteLinux
     remotelinuxdeployconfiguration.cpp remotelinuxdeployconfiguration.h
     remotelinuxenvironmentaspect.cpp remotelinuxenvironmentaspect.h
     remotelinuxenvironmentaspectwidget.cpp remotelinuxenvironmentaspectwidget.h
-    remotelinuxenvironmentreader.cpp remotelinuxenvironmentreader.h
     remotelinuxplugin.cpp remotelinuxplugin.h
     remotelinuxqmltoolingsupport.cpp remotelinuxqmltoolingsupport.h
     remotelinuxrunconfiguration.cpp remotelinuxrunconfiguration.h

src/plugins/remotelinux/linuxdevice.cpp

@@ -9,7 +9,6 @@
 #include "linuxprocessinterface.h"
 #include "publickeydeploymentdialog.h"
 #include "remotelinux_constants.h"
-#include "remotelinuxenvironmentreader.h"
 #include "remotelinuxsignaloperation.h"
 #include "remotelinuxtr.h"
 #include "sshprocessinterface.h"
@@ -1043,31 +1042,6 @@ DeviceProcessSignalOperation::Ptr LinuxDevice::signalOperation() const
     return DeviceProcessSignalOperation::Ptr(new RemoteLinuxSignalOperation(sharedFromThis()));
 }
 
-class LinuxDeviceEnvironmentFetcher : public DeviceEnvironmentFetcher
-{
-public:
-    LinuxDeviceEnvironmentFetcher(const IDevice::ConstPtr &device)
-        : m_reader(device)
-    {
-        connect(&m_reader, &Internal::RemoteLinuxEnvironmentReader::finished,
-                this, &LinuxDeviceEnvironmentFetcher::readerFinished);
-        connect(&m_reader, &Internal::RemoteLinuxEnvironmentReader::error,
-                this, &LinuxDeviceEnvironmentFetcher::readerError);
-    }
-
-private:
-    void start() override { m_reader.start(); }
-    void readerFinished() { emit finished(m_reader.remoteEnvironment(), true); }
-    void readerError() { emit finished(Environment(), false); }
-
-    Internal::RemoteLinuxEnvironmentReader m_reader;
-};
-
-DeviceEnvironmentFetcher::Ptr LinuxDevice::environmentFetcher() const
-{
-    return DeviceEnvironmentFetcher::Ptr(new LinuxDeviceEnvironmentFetcher(sharedFromThis()));
-}
-
 bool LinuxDevice::usableAsBuildDevice() const
 {
     return true;
@@ -1097,11 +1071,6 @@ ProcessInterface *LinuxDevice::createProcessInterface() const
     return new LinuxProcessInterface(this);
 }
 
-Environment LinuxDevice::systemEnvironment() const
-{
-    return {}; // FIXME. See e.g. Docker implementation.
-}
-
 LinuxDevicePrivate::LinuxDevicePrivate(LinuxDevice *parent)
     : q(parent)
 {

src/plugins/remotelinux/linuxdevice.h

@@ -29,7 +29,6 @@ public:
     bool hasDeviceTester() const override { return true; }
     ProjectExplorer::DeviceTester *createDeviceTester() const override;
     ProjectExplorer::DeviceProcessSignalOperation::Ptr signalOperation() const override;
-    ProjectExplorer::DeviceEnvironmentFetcher::Ptr environmentFetcher() const override;
     bool usableAsBuildDevice() const override;
 
     QString userAtHost() const;
@@ -41,7 +40,6 @@ public:
     Utils::ProcessInterface *createProcessInterface() const override;
     ProjectExplorer::FileTransferInterface *createFileTransferInterface(
             const ProjectExplorer::FileTransferSetupData &setup) const override;
-    Utils::Environment systemEnvironment() const override;
 
 protected:
     LinuxDevice();

src/plugins/remotelinux/remotelinux.qbs

@@ -55,8 +55,6 @@ Project {
             "remotelinuxenvironmentaspect.h",
             "remotelinuxenvironmentaspectwidget.cpp",
             "remotelinuxenvironmentaspectwidget.h",
-            "remotelinuxenvironmentreader.cpp",
-            "remotelinuxenvironmentreader.h",
             "remotelinuxplugin.cpp",
             "remotelinuxplugin.h",
             "remotelinuxqmltoolingsupport.cpp",

src/plugins/remotelinux/remotelinuxenvironmentaspectwidget.cpp

@@ -5,7 +5,6 @@
 
 #include "linuxdevice.h"
 #include "remotelinuxenvironmentaspect.h"
-#include "remotelinuxenvironmentreader.h"
 #include "remotelinuxtr.h"
 
 #include <coreplugin/icore.h>
@@ -14,38 +13,35 @@
 #include <projectexplorer/kitinformation.h>
 #include <projectexplorer/target.h>
 
+#include <utils/devicefileaccess.h>
 #include <utils/qtcassert.h>
 
-#include <QCoreApplication>
 #include <QMessageBox>
 #include <QPushButton>
 
 using namespace ProjectExplorer;
-using namespace RemoteLinux::Internal;
 using namespace Utils;
 
 namespace RemoteLinux {
 
-const QString FetchEnvButtonText = Tr::tr("Fetch Device Environment");
-
 RemoteLinuxEnvironmentAspectWidget::RemoteLinuxEnvironmentAspectWidget
         (RemoteLinuxEnvironmentAspect *aspect, Target *target)
     : EnvironmentAspectWidget(aspect)
-    , m_fetchButton(new QPushButton(FetchEnvButtonText))
 {
-    addWidget(m_fetchButton);
+    auto fetchButton = new QPushButton(Tr::tr("Fetch Device Environment"));
+    addWidget(fetchButton);
 
-    IDevice::ConstPtr device = DeviceKitAspect::device(target->kit());
+    connect(target, &ProjectExplorer::Target::kitChanged, [this, aspect] {
+        aspect->setRemoteEnvironment({});
+    });
 
-    m_deviceEnvReader = new RemoteLinuxEnvironmentReader(device, this);
+    connect(fetchButton, &QPushButton::clicked, this, [this, aspect, target] {
-    connect(target, &ProjectExplorer::Target::kitChanged,
+        if (IDevice::ConstPtr device = DeviceKitAspect::device(target->kit())) {
-            m_deviceEnvReader, &RemoteLinuxEnvironmentReader::handleCurrentDeviceConfigChanged);
+            DeviceFileAccess *access = device->fileAccess();
-
+            QTC_ASSERT(access, return);
-    connect(m_fetchButton, &QPushButton::clicked, this, &RemoteLinuxEnvironmentAspectWidget::fetchEnvironment);
+            aspect->setRemoteEnvironment(access->deviceEnvironment());
-    connect(m_deviceEnvReader, &RemoteLinuxEnvironmentReader::finished,
+        }
-            this, &RemoteLinuxEnvironmentAspectWidget::fetchEnvironmentFinished);
+    });
-    connect(m_deviceEnvReader, &RemoteLinuxEnvironmentReader::error,
-            this, &RemoteLinuxEnvironmentAspectWidget::fetchEnvironmentError);
 
     const EnvironmentWidget::OpenTerminalFunc openTerminalFunc
             = [target](const Environment &env) {
@@ -60,40 +56,8 @@ RemoteLinuxEnvironmentAspectWidget::RemoteLinuxEnvironmentAspectWidget
         QTC_ASSERT(linuxDevice, return);
         linuxDevice->openTerminal(env, FilePath());
     };
+
     envWidget()->setOpenTerminalFunc(openTerminalFunc);
 }
 
-void RemoteLinuxEnvironmentAspectWidget::fetchEnvironment()
-{
-    disconnect(m_fetchButton, &QPushButton::clicked,
-               this, &RemoteLinuxEnvironmentAspectWidget::fetchEnvironment);
-    connect(m_fetchButton, &QPushButton::clicked,
-            this, &RemoteLinuxEnvironmentAspectWidget::stopFetchEnvironment);
-    m_fetchButton->setText(Tr::tr("Cancel Fetch Operation"));
-    m_deviceEnvReader->start();
-}
-
-void RemoteLinuxEnvironmentAspectWidget::fetchEnvironmentFinished()
-{
-    disconnect(m_fetchButton, &QPushButton::clicked,
-               this, &RemoteLinuxEnvironmentAspectWidget::stopFetchEnvironment);
-    connect(m_fetchButton, &QPushButton::clicked,
-            this, &RemoteLinuxEnvironmentAspectWidget::fetchEnvironment);
-    m_fetchButton->setText(FetchEnvButtonText);
-    qobject_cast<RemoteLinuxEnvironmentAspect *>(aspect())->setRemoteEnvironment(
-                m_deviceEnvReader->remoteEnvironment());
-}
-
-void RemoteLinuxEnvironmentAspectWidget::fetchEnvironmentError(const QString &error)
-{
-    QMessageBox::warning(this, Tr::tr("Device Error"),
-        Tr::tr("Fetching environment failed: %1").arg(error));
-}
-
-void RemoteLinuxEnvironmentAspectWidget::stopFetchEnvironment()
-{
-    m_deviceEnvReader->stop();
-    fetchEnvironmentFinished();
-}
-
 } // RemoteLinux

src/plugins/remotelinux/remotelinuxenvironmentaspectwidget.h

@@ -5,16 +5,10 @@
 
 #include <projectexplorer/environmentaspectwidget.h>
 
-QT_BEGIN_NAMESPACE
-class QPushButton;
-QT_END_NAMESPACE
-
 namespace RemoteLinux {
 
 class RemoteLinuxEnvironmentAspect;
 
-namespace Internal { class RemoteLinuxEnvironmentReader; }
-
 class RemoteLinuxEnvironmentAspectWidget : public ProjectExplorer::EnvironmentAspectWidget
 {
     Q_OBJECT
@@ -22,15 +16,6 @@ class RemoteLinuxEnvironmentAspectWidget : public ProjectExplorer::EnvironmentAs
 public:
     RemoteLinuxEnvironmentAspectWidget(RemoteLinuxEnvironmentAspect *aspect,
                                        ProjectExplorer::Target *target);
-
-private:
-    void fetchEnvironment();
-    void fetchEnvironmentFinished();
-    void fetchEnvironmentError(const QString &error);
-    void stopFetchEnvironment();
-
-    Internal::RemoteLinuxEnvironmentReader *m_deviceEnvReader = nullptr;
-    QPushButton *m_fetchButton = nullptr;
 };
 
 } // namespace RemoteLinux

src/plugins/remotelinux/remotelinuxenvironmentreader.cpp

@@ -1,97 +0,0 @@
-// Copyright (C) 2016 The Qt Company Ltd.
-// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0+ OR GPL-3.0 WITH Qt-GPL-exception-1.0
-
-#include "remotelinuxenvironmentreader.h"
-
-#include "remotelinuxtr.h"
-
-#include <projectexplorer/devicesupport/idevice.h>
-
-#include <utils/qtcprocess.h>
-#include <utils/stringutils.h>
-
-using namespace ProjectExplorer;
-using namespace Utils;
-
-namespace RemoteLinux {
-namespace Internal {
-
-RemoteLinuxEnvironmentReader::RemoteLinuxEnvironmentReader(const IDevice::ConstPtr &device,
-                                                           QObject *parent)
-    : QObject(parent)
-    , m_env(Utils::OsTypeLinux)
-    , m_device(device)
-{
-}
-
-void RemoteLinuxEnvironmentReader::start()
-{
-    if (!m_device) {
-        emit error(Tr::tr("Error: No device"));
-        setFinished();
-        return;
-    }
-    m_deviceProcess = new QtcProcess(this);
-    connect(m_deviceProcess, &QtcProcess::done,
-            this, &RemoteLinuxEnvironmentReader::handleDone);
-    m_deviceProcess->setCommand({m_device->filePath("env"), {}});
-    m_deviceProcess->start();
-}
-
-void RemoteLinuxEnvironmentReader::stop()
-{
-    if (!m_deviceProcess)
-        return;
-    m_deviceProcess->disconnect(this);
-    m_deviceProcess->deleteLater();
-    m_deviceProcess = nullptr;
-}
-
-void RemoteLinuxEnvironmentReader::handleCurrentDeviceConfigChanged()
-{
-    m_env.clear();
-    setFinished();
-}
-
-void RemoteLinuxEnvironmentReader::handleDone()
-{
-    if (m_deviceProcess->result() != ProcessResult::FinishedWithSuccess) {
-        emit error(Tr::tr("Error: %1").arg(m_deviceProcess->errorString()));
-        setFinished();
-        return;
-    }
-
-    m_env.clear();
-    QString errorMessage;
-    if (m_deviceProcess->exitStatus() != QProcess::NormalExit) {
-        errorMessage = m_deviceProcess->errorString();
-    } else if (m_deviceProcess->exitCode() != 0) {
-        errorMessage = Tr::tr("Process exited with code %1.")
-                .arg(m_deviceProcess->exitCode());
-    }
-
-    if (!errorMessage.isEmpty()) {
-        errorMessage = Tr::tr("Error running 'env': %1").arg(errorMessage);
-        const QString remoteStderr
-                = QString::fromUtf8(m_deviceProcess->readAllStandardError()).trimmed();
-        if (!remoteStderr.isEmpty())
-            errorMessage += QLatin1Char('\n') + Tr::tr("Remote stderr was: \"%1\"").arg(remoteStderr);
-        emit error(errorMessage);
-    } else {
-        const QString remoteOutput = QString::fromUtf8(m_deviceProcess->readAllStandardOutput());
-        if (!remoteOutput.isEmpty()) {
-            m_env = Utils::Environment(remoteOutput.split(QLatin1Char('\n'),
-                Qt::SkipEmptyParts), Utils::OsTypeLinux);
-        }
-    }
-    setFinished();
-}
-
-void RemoteLinuxEnvironmentReader::setFinished()
-{
-    stop();
-    emit finished();
-}
-
-} // namespace Internal
-} // namespace RemoteLinux

src/plugins/remotelinux/remotelinuxenvironmentreader.h

@@ -1,44 +0,0 @@
-// Copyright (C) 2016 The Qt Company Ltd.
-// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0+ OR GPL-3.0 WITH Qt-GPL-exception-1.0
-
-#pragma once
-
-#include <projectexplorer/devicesupport/idevicefwd.h>
-#include <utils/environment.h>
-
-#include <QObject>
-#include <QSharedPointer>
-
-namespace Utils { class QtcProcess; }
-
-namespace RemoteLinux {
-namespace Internal {
-
-class RemoteLinuxEnvironmentReader : public QObject
-{
-    Q_OBJECT
-
-public:
-    RemoteLinuxEnvironmentReader(const ProjectExplorer::IDeviceConstPtr &device,
-                                 QObject *parent = nullptr);
-    void start();
-    void stop();
-
-    Utils::Environment remoteEnvironment() const { return m_env; }
-    void handleCurrentDeviceConfigChanged();
-
-signals:
-    void finished();
-    void error(const QString &error);
-
-private:
-    void handleDone();
-    void setFinished();
-
-    Utils::Environment m_env;
-    ProjectExplorer::IDeviceConstPtr m_device;
-    Utils::QtcProcess *m_deviceProcess = nullptr;
-};
-
-} // namespace Internal
-} // namespace RemoteLinux