RemoteLinux: Generalize deploy step

Instead of three different deploy steps we use only the RSyncDeployStep
which can be configured to each of the three file transfer methods.

The SupportsRSync constant is moved to projectexplorer so that it can be
used by all devices.

The RSyncDeployStep checks which transfer method is available based on
source and target devices. An option is added for the user to force
a different method in case of issues.

Change-Id: I2b1eb39b5bd4a30d0f879d18317b7677d127f48f
Reviewed-by: hjk <hjk@qt.io>

src/plugins/boot2qt/qdbdeployconfigurationfactory.cpp

@@ -10,6 +10,7 @@
 #include <projectexplorer/devicesupport/idevice.h>
 #include <projectexplorer/kitinformation.h>
 #include <projectexplorer/project.h>
+#include <projectexplorer/projectexplorerconstants.h>
 #include <projectexplorer/target.h>
 
 #include <remotelinux/remotelinux_constants.h>
@@ -37,14 +38,14 @@ QdbDeployConfigurationFactory::QdbDeployConfigurationFactory()
         if (buildDevice && buildDevice->rootPath().needsDevice())
             return false;
         return !device
-               || (device && device->extraData(RemoteLinux::Constants::SupportsRSync).toBool());
+               || (device && device->extraData(ProjectExplorer::Constants::SUPPORTS_RSYNC).toBool());
     });
     addInitialStep(RemoteLinux::Constants::DirectUploadStepId, [](Target *target) {
         auto device = DeviceKitAspect::device(target->kit());
         auto buildDevice = BuildDeviceKitAspect::device(target->kit());
         if (buildDevice && buildDevice->rootPath().needsDevice())
             return false;
-        return device && !device->extraData(RemoteLinux::Constants::SupportsRSync).toBool();
+        return device && !device->extraData(ProjectExplorer::Constants::SUPPORTS_RSYNC).toBool();
     });
     // This step is for:
     // a) A remote build device, as they do not support real rsync yet.

src/plugins/projectexplorer/devicesupport/desktopdevice.cpp

@@ -135,4 +135,14 @@ FilePath DesktopDevice::rootPath() const
     return IDevice::rootPath();
 }
 
+void DesktopDevice::fromMap(const QVariantMap &map)
+{
+    IDevice::fromMap(map);
+
+    const FilePath rsync = FilePath::fromString("rsync").searchInPath();
+    const FilePath sftp = FilePath::fromString("sftp").searchInPath();
+    setExtraData(Constants::SUPPORTS_RSYNC, rsync.isExecutableFile());
+    setExtraData(Constants::SUPPORTS_SFTP, sftp.isExecutableFile());
+}
+
 } // namespace ProjectExplorer

src/plugins/projectexplorer/devicesupport/desktopdevice.h

@@ -37,6 +37,8 @@ public:
     Utils::FilePath rootPath() const override;
     Utils::FilePath filePath(const QString &pathOnDevice) const override;
 
+    void fromMap(const QVariantMap &map) override;
+
 protected:
     DesktopDevice();
 

src/plugins/projectexplorer/projectexplorerconstants.h

@@ -213,6 +213,10 @@ const char SESSION_TASKFILE_KEY[] = "TaskList.File";
 const char CLEAR_SYSTEM_ENVIRONMENT_KEY[] = "ProjectExplorer.BuildConfiguration.ClearSystemEnvironment";
 const char USER_ENVIRONMENT_CHANGES_KEY[] = "ProjectExplorer.BuildConfiguration.UserEnvironmentChanges";
 
+// Called "RemoteLinux." for backwards compatibility
+const char SUPPORTS_RSYNC[] = "RemoteLinux.SupportsRSync";
+const char SUPPORTS_SFTP[] = "RemoteLinux.SupportsSftp";
+
 // UI texts
 PROJECTEXPLORER_EXPORT QString msgAutoDetected();
 PROJECTEXPLORER_EXPORT QString msgAutoDetectedToolTip();

src/plugins/remotelinux/linuxdevicetester.cpp

@@ -3,11 +3,11 @@
 
 #include "linuxdevicetester.h"
 
-#include "remotelinux_constants.h"
 #include "remotelinuxtr.h"
 
 #include <projectexplorer/devicesupport/deviceusedportsgatherer.h>
 #include <projectexplorer/devicesupport/filetransfer.h>
+#include <projectexplorer/projectexplorerconstants.h>
 
 #include <utils/algorithm.h>
 #include <utils/process.h>
@@ -177,9 +177,9 @@ GroupItem GenericLinuxDeviceTesterPrivate::transferTask(FileTransferMethod metho
         const QString methodName = FileTransfer::transferMethodName(method);
         emit q->progressMessage(Tr::tr("\"%1\" is functional.\n").arg(methodName));
         if (method == FileTransferMethod::Rsync)
-            m_device->setExtraData(Constants::SupportsRSync, true);
+            m_device->setExtraData(Constants::SUPPORTS_RSYNC, true);
         else if (method == FileTransferMethod::Sftp)
-            m_device->setExtraData(Constants::SupportsSftp, true);
+            m_device->setExtraData(Constants::SUPPORTS_SFTP, true);
         else
             storage->useGenericCopy = true;
     };
@@ -197,12 +197,12 @@ GroupItem GenericLinuxDeviceTesterPrivate::transferTask(FileTransferMethod metho
         }
         emit q->errorMessage(error);
         if (method == FileTransferMethod::Rsync)
-            m_device->setExtraData(Constants::SupportsRSync, false);
+            m_device->setExtraData(Constants::SUPPORTS_RSYNC, false);
         else if (method == FileTransferMethod::Sftp)
-            m_device->setExtraData(Constants::SupportsSftp, false);
+            m_device->setExtraData(Constants::SUPPORTS_SFTP, false);
 
-        const QVariant supportsRSync = m_device->extraData(Constants::SupportsRSync);
+        const QVariant supportsRSync = m_device->extraData(Constants::SUPPORTS_RSYNC);
-        const QVariant supportsSftp = m_device->extraData(Constants::SupportsSftp);
+        const QVariant supportsSftp = m_device->extraData(Constants::SUPPORTS_SFTP);
         if (supportsRSync.isValid() && !supportsRSync.toBool()
             && supportsSftp.isValid() && !supportsSftp.toBool()) {
             const QString generic = FileTransfer::transferMethodName(FileTransferMethod::GenericCopy);

src/plugins/remotelinux/remotelinux_constants.h

@@ -19,8 +19,6 @@ const char RsyncDeployStepId[] = "RemoteLinux.RsyncDeployStep";
 const char CustomCommandDeployStepId[] = "RemoteLinux.GenericRemoteLinuxCustomCommandDeploymentStep";
 const char KillAppStepId[] = "RemoteLinux.KillAppStep";
 
-const char SupportsRSync[] = "RemoteLinux.SupportsRSync";
-const char SupportsSftp[] = "RemoteLinux.SupportsSftp";
 const char SourceProfile[] = "RemoteLinux.SourceProfile";
 const char LinkDevice[] = "RemoteLinux.LinkDevice";
 

src/plugins/remotelinux/remotelinuxdeployconfiguration.cpp

@@ -17,24 +17,6 @@ using namespace ProjectExplorer;
 
 namespace RemoteLinux::Internal {
 
-FileTransferMethod defaultTransferMethod(Kit *kit)
-{
-    auto runDevice = DeviceKitAspect::device(kit);
-    auto buildDevice = BuildDeviceKitAspect::device(kit);
-
-    if (runDevice != buildDevice) {
-        // FIXME: That's not the full truth, we need support from the build
-        // device, too.
-        if (runDevice && runDevice->extraData(Constants::SupportsRSync).toBool())
-            return FileTransferMethod::Rsync;
-    }
-
-    if (runDevice && runDevice->extraData(Constants::SupportsSftp).toBool())
-        return FileTransferMethod::Sftp;
-
-    return FileTransferMethod::GenericCopy;
-}
-
 RemoteLinuxDeployConfigurationFactory::RemoteLinuxDeployConfigurationFactory()
 {
     setConfigBaseId(RemoteLinux::Constants::DeployToGenericLinux);
@@ -58,17 +40,8 @@ RemoteLinuxDeployConfigurationFactory::RemoteLinuxDeployConfigurationFactory()
     addInitialStep(Constants::MakeInstallStepId, needsMakeInstall);
     addInitialStep(Constants::KillAppStepId);
 
-    // Todo: Check: Instead of having three different steps here, have one
+    // TODO: Rename RsyncDeployStep to something more generic.
-    // and shift the logic into the implementation there?
+    addInitialStep(Constants::RsyncDeployStepId);
-    addInitialStep(Constants::RsyncDeployStepId, [](Target *target) {
-        return defaultTransferMethod(target->kit()) == FileTransferMethod::Rsync;
-    });
-    addInitialStep(Constants::DirectUploadStepId, [](Target *target) {
-        return defaultTransferMethod(target->kit()) == FileTransferMethod::Sftp;
-    });
-    addInitialStep(ProjectExplorer::Constants::COPY_FILE_STEP, [](Target *target) {
-        return defaultTransferMethod(target->kit()) == FileTransferMethod::GenericCopy;
-    });
 }
 
 } // RemoteLinux::Internal

src/plugins/remotelinux/rsyncdeploystep.cpp

@@ -9,6 +9,7 @@
 
 #include <projectexplorer/buildsystem.h>
 #include <projectexplorer/deploymentdata.h>
+#include <projectexplorer/devicesupport/devicemanager.h>
 #include <projectexplorer/devicesupport/filetransfer.h>
 #include <projectexplorer/devicesupport/idevice.h>
 #include <projectexplorer/kitinformation.h>
@@ -42,6 +43,7 @@ private:
 
     mutable FilesToTransfer m_files;
     bool m_ignoreMissingFiles = false;
+    FileTransferMethod m_preferredTransferMethod = FileTransferMethod::Rsync;
     QString m_flags;
 };
 
@@ -60,7 +62,14 @@ RsyncDeployStep::RsyncDeployStep(BuildStepList *bsl, Id id)
                                  BoolAspect::LabelPlacement::InExtraLabel);
     ignoreMissingFiles->setValue(false);
 
-    setInternalInitializer([this, ignoreMissingFiles, flags] {
+    auto method = addAspect<SelectionAspect>();
+    method->setDisplayStyle(SelectionAspect::DisplayStyle::ComboBox);
+    method->setDisplayName(Tr::tr("Transfer method:"));
+    method->addOption(Tr::tr("rsync"), Tr::tr("Use rsync if available."));
+    method->addOption(Tr::tr("SFTP"), Tr::tr("Use sftp if available."));
+    method->addOption(Tr::tr("Generic Copy"), Tr::tr("Use generic copy, most likely to succeed."));
+
+    setInternalInitializer([this, ignoreMissingFiles, flags, method] {
         if (BuildDeviceKitAspect::device(kit()) == DeviceKitAspect::device(kit())) {
             // rsync transfer on the same device currently not implemented
             // and typically not wanted.
@@ -69,6 +78,13 @@ RsyncDeployStep::RsyncDeployStep(BuildStepList *bsl, Id id)
         }
         m_ignoreMissingFiles = ignoreMissingFiles->value();
         m_flags = flags->value();
+        if (method->value() == 0)
+            m_preferredTransferMethod = FileTransferMethod::Rsync;
+        else if (method->value() == 1)
+            m_preferredTransferMethod = FileTransferMethod::Sftp;
+        else
+            m_preferredTransferMethod = FileTransferMethod::GenericCopy;
+
         return isDeploymentPossible();
     });
 
@@ -127,10 +143,46 @@ GroupItem RsyncDeployStep::mkdirTask()
     return AsyncTask<ResultType>(onSetup, {}, onError);
 }
 
+static FileTransferMethod supportedTransferMethodFor(const FileToTransfer &fileToTransfer)
+{
+    auto sourceDevice = ProjectExplorer::DeviceManager::deviceForPath(fileToTransfer.m_source);
+    auto targetDevice = ProjectExplorer::DeviceManager::deviceForPath(fileToTransfer.m_target);
+
+    if (sourceDevice && targetDevice) {
+        // TODO: Check if the devices can reach each other via their ip
+        if (sourceDevice->extraData(ProjectExplorer::Constants::SUPPORTS_RSYNC).toBool()
+            && targetDevice->extraData(ProjectExplorer::Constants::SUPPORTS_RSYNC).toBool()) {
+            return FileTransferMethod::Rsync;
+        }
+
+        if (sourceDevice->extraData(ProjectExplorer::Constants::SUPPORTS_SFTP).toBool()
+            && targetDevice->extraData(ProjectExplorer::Constants::SUPPORTS_SFTP).toBool()) {
+            return FileTransferMethod::Sftp;
+        }
+    }
+
+    return FileTransferMethod::GenericCopy;
+}
+
 GroupItem RsyncDeployStep::transferTask()
 {
     const auto setupHandler = [this](FileTransfer &transfer) {
-        transfer.setTransferMethod(FileTransferMethod::Rsync);
+        FileTransferMethod transferMethod = m_preferredTransferMethod;
+
+        if (transferMethod != FileTransferMethod::GenericCopy) {
+            for (const FileToTransfer &fileToTransfer : m_files) {
+                const FileTransferMethod supportedMethod = supportedTransferMethodFor(
+                    fileToTransfer);
+
+                if (supportedMethod != m_preferredTransferMethod) {
+                    transferMethod = FileTransferMethod::GenericCopy;
+                    break;
+                }
+            }
+        }
+
+        transfer.setTransferMethod(transferMethod);
+
         transfer.setRsyncFlags(m_flags);
         transfer.setFilesToTransfer(m_files);
         connect(&transfer, &FileTransfer::progress,