From ee2fa69defcd3d4f1feb80a4320431b4beb9ab68 Mon Sep 17 00:00:00 2001 From: Marcus Tillmanns Date: Thu, 22 Jun 2023 09:11:07 +0200 Subject: [PATCH] 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 --- .../boot2qt/qdbdeployconfigurationfactory.cpp | 5 +- .../devicesupport/desktopdevice.cpp | 10 ++++ .../devicesupport/desktopdevice.h | 2 + .../projectexplorerconstants.h | 4 ++ src/plugins/remotelinux/linuxdevicetester.cpp | 14 ++--- .../remotelinux/remotelinux_constants.h | 2 - .../remotelinuxdeployconfiguration.cpp | 31 +--------- src/plugins/remotelinux/rsyncdeploystep.cpp | 56 ++++++++++++++++++- 8 files changed, 82 insertions(+), 42 deletions(-) diff --git a/src/plugins/boot2qt/qdbdeployconfigurationfactory.cpp b/src/plugins/boot2qt/qdbdeployconfigurationfactory.cpp index 3abb700df1d..eb5a180500e 100644 --- a/src/plugins/boot2qt/qdbdeployconfigurationfactory.cpp +++ b/src/plugins/boot2qt/qdbdeployconfigurationfactory.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -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. diff --git a/src/plugins/projectexplorer/devicesupport/desktopdevice.cpp b/src/plugins/projectexplorer/devicesupport/desktopdevice.cpp index 099040d6b24..33ed3287cf0 100644 --- a/src/plugins/projectexplorer/devicesupport/desktopdevice.cpp +++ b/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 diff --git a/src/plugins/projectexplorer/devicesupport/desktopdevice.h b/src/plugins/projectexplorer/devicesupport/desktopdevice.h index d9cafbfda9a..bc4c353b465 100644 --- a/src/plugins/projectexplorer/devicesupport/desktopdevice.h +++ b/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(); diff --git a/src/plugins/projectexplorer/projectexplorerconstants.h b/src/plugins/projectexplorer/projectexplorerconstants.h index 75054120a83..50bacef3a41 100644 --- a/src/plugins/projectexplorer/projectexplorerconstants.h +++ b/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(); diff --git a/src/plugins/remotelinux/linuxdevicetester.cpp b/src/plugins/remotelinux/linuxdevicetester.cpp index 5392fb4245c..b001fb8654f 100644 --- a/src/plugins/remotelinux/linuxdevicetester.cpp +++ b/src/plugins/remotelinux/linuxdevicetester.cpp @@ -3,11 +3,11 @@ #include "linuxdevicetester.h" -#include "remotelinux_constants.h" #include "remotelinuxtr.h" #include #include +#include #include #include @@ -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 supportsSftp = m_device->extraData(Constants::SupportsSftp); + const QVariant supportsRSync = m_device->extraData(Constants::SUPPORTS_RSYNC); + 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); diff --git a/src/plugins/remotelinux/remotelinux_constants.h b/src/plugins/remotelinux/remotelinux_constants.h index 9c428650c73..24821c6f36a 100644 --- a/src/plugins/remotelinux/remotelinux_constants.h +++ b/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"; diff --git a/src/plugins/remotelinux/remotelinuxdeployconfiguration.cpp b/src/plugins/remotelinux/remotelinuxdeployconfiguration.cpp index 94bfbfb981f..7615a4ef553 100644 --- a/src/plugins/remotelinux/remotelinuxdeployconfiguration.cpp +++ b/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 - // and shift the logic into the implementation there? - 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; - }); + // TODO: Rename RsyncDeployStep to something more generic. + addInitialStep(Constants::RsyncDeployStepId); } } // RemoteLinux::Internal diff --git a/src/plugins/remotelinux/rsyncdeploystep.cpp b/src/plugins/remotelinux/rsyncdeploystep.cpp index 83d729108b2..0214f21576b 100644 --- a/src/plugins/remotelinux/rsyncdeploystep.cpp +++ b/src/plugins/remotelinux/rsyncdeploystep.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -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(); + 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(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,