SSH: Fix non-session based download operation

Apparently this one never worked, but as we don't use it in our own
code, that went unnoticed.

Fixes: QTCREATORBUG-25236
Change-Id: Ia013baf3ca2fd01cc14b72d9c6a5f73426539d00
Reviewed-by: Christian Stenger <christian.stenger@qt.io>

src/libs/ssh/sftptransfer.cpp

@@ -154,7 +154,7 @@ void SftpTransfer::doStart()
             batchFile.write("-mkdir " + QtcProcess::quoteArgUnix(dir).toLocal8Bit() + '\n');
             break;
         case Internal::FileTransferType::Download:
-            if (!QDir::root().mkdir(dir)) {
+            if (!QDir::root().mkpath(dir)) {
                 emitError(tr("Failed to create local directory \"%1\".")
                           .arg(QDir::toNativeSeparators(dir)));
                 return;
@@ -163,7 +163,7 @@ void SftpTransfer::doStart()
         }
     }
     for (const FileToTransfer &f : d->files) {
-        QString sourceFileOrLinkTarget;
+        QString sourceFileOrLinkTarget = f.sourceFile;
         bool link = false;
         if (d->transferType == Internal::FileTransferType::Upload) {
             QFileInfo fi(f.sourceFile);
@@ -172,8 +172,6 @@ void SftpTransfer::doStart()
                 batchFile.write("-rm " + QtcProcess::quoteArgUnix(f.targetFile).toLocal8Bit()
                                 + '\n');
                 sourceFileOrLinkTarget = fi.dir().relativeFilePath(fi.symLinkTarget()); // see QTBUG-5817.
-            } else {
-                sourceFileOrLinkTarget = f.sourceFile;
             }
          }
          batchFile.write(d->transferCommand(link) + ' '

tests/auto/ssh/tst_ssh.cpp

@@ -28,6 +28,7 @@
 #include <ssh/sshconnection.h>
 #include <ssh/sshremoteprocessrunner.h>
 #include <ssh/sshsettings.h>
+#include <utils/algorithm.h>
 #include <utils/environment.h>
 #include <utils/temporarydirectory.h>
 
@@ -371,12 +372,15 @@ void tst_Ssh::sftp()
     // Create and upload 1000 small files and one big file
     QTemporaryDir dirForFilesToUpload;
     QTemporaryDir dirForFilesToDownload;
+    QTemporaryDir dir2ForFilesToDownload;
     QVERIFY2(dirForFilesToUpload.isValid(), qPrintable(dirForFilesToUpload.errorString()));
     QVERIFY2(dirForFilesToDownload.isValid(), qPrintable(dirForFilesToDownload.errorString()));
+    QVERIFY2(dir2ForFilesToDownload.isValid(), qPrintable(dirForFilesToDownload.errorString()));
     static const auto getRemoteFilePath = [](const QString &localFileName) {
         return QString("/tmp/").append(localFileName).append(".upload");
     };
-    const auto getDownloadFilePath = [&dirForFilesToDownload](const QString &localFileName) {
+    const auto getDownloadFilePath = [](const QTemporaryDir &dirForFilesToDownload,
+            const QString &localFileName) {
         return QString(dirForFilesToDownload.path()).append('/').append(localFileName);
     };
     FilesToTransfer filesToUpload;
@@ -464,7 +468,7 @@ void tst_Ssh::sftp()
     for (const QString &fileName : allUploadedFileNames) {
         const QString localFilePath = dirForFilesToUpload.path() + '/' + fileName;
         const QString remoteFilePath = getRemoteFilePath(fileName);
-        const QString downloadFilePath = getDownloadFilePath(fileName);
+        const QString downloadFilePath = getDownloadFilePath(dirForFilesToDownload, fileName);
         const SftpJobId downloadJob = sftpChannel->downloadFile(remoteFilePath, downloadFilePath);
         QVERIFY(downloadJob != SftpInvalidJob);
         jobs << downloadJob;
@@ -477,24 +481,55 @@ void tst_Ssh::sftp()
     QVERIFY(jobs.empty());
 
     // Compare contents of uploaded and downloaded files
-    for (const QString &fileName : allUploadedFileNames) {
+    bool success;
-        QFile originalFile(dirForFilesToUpload.path() + '/' + fileName);
+    const auto compareFiles = [&](const QTemporaryDir &downloadDir) {
-        QVERIFY2(originalFile.open(QIODevice::ReadOnly), qPrintable(originalFile.errorString()));
+        success = false;
-        QFile downloadedFile(dirForFilesToDownload.path() + '/' + fileName);
+        for (const QString &fileName : allUploadedFileNames) {
-        QVERIFY2(downloadedFile.open(QIODevice::ReadOnly),
+            QFile originalFile(dirForFilesToUpload.path() + '/' + fileName);
-                 qPrintable(downloadedFile.errorString()));
+            QVERIFY2(originalFile.open(QIODevice::ReadOnly), qPrintable(originalFile.errorString()));
-        QVERIFY(originalFile.fileName() != downloadedFile.fileName());
+            QFile downloadedFile(getDownloadFilePath(downloadDir, fileName));
-        QCOMPARE(originalFile.size(), downloadedFile.size());
+            QVERIFY2(downloadedFile.open(QIODevice::ReadOnly),
-        qint64 bytesLeft = originalFile.size();
+                     qPrintable(downloadedFile.errorString()));
-        while (bytesLeft > 0) {
+            QVERIFY(originalFile.fileName() != downloadedFile.fileName());
-            const qint64 bytesToRead = qMin(bytesLeft, Q_INT64_C(1024 * 1024));
+            QCOMPARE(originalFile.size(), downloadedFile.size());
-            const QByteArray origBlock = originalFile.read(bytesToRead);
+            qint64 bytesLeft = originalFile.size();
-            const QByteArray copyBlock = downloadedFile.read(bytesToRead);
+            while (bytesLeft > 0) {
-            QCOMPARE(origBlock.size(), bytesToRead);
+                const qint64 bytesToRead = qMin(bytesLeft, Q_INT64_C(1024 * 1024));
-            QCOMPARE(origBlock, copyBlock);
+                const QByteArray origBlock = originalFile.read(bytesToRead);
-            bytesLeft -= bytesToRead;
+                const QByteArray copyBlock = downloadedFile.read(bytesToRead);
+                QCOMPARE(origBlock.size(), bytesToRead);
+                QCOMPARE(origBlock, copyBlock);
+                bytesLeft -= bytesToRead;
+            }
         }
-    }
+        success = true;
+    };
+    compareFiles(dirForFilesToDownload);
+    QVERIFY(success);
+
+    // The same again, with a non-interactive download.
+    FilesToTransfer filesToDownload = Utils::transform(filesToUpload, [&](const FileToTransfer &fileToUpload) {
+        return FileToTransfer(fileToUpload.targetFile,
+                              getDownloadFilePath(dir2ForFilesToDownload,
+                                                  QFileInfo(fileToUpload.sourceFile).fileName()));
+    });
+    const SftpTransferPtr download = connection.createDownload(filesToDownload,
+                                                               FileTransferErrorHandling::Abort);
+    connect(download.get(), &SftpTransfer::done, [&jobError, &loop](const QString &error) {
+        jobError = error;
+        loop.quit();
+    });
+    QObject::connect(&timer, &QTimer::timeout, &loop, &QEventLoop::quit);
+    timer.setSingleShot(true);
+    timer.setInterval(30 * 1000);
+    timer.start();
+    download->start();
+    loop.exec();
+    QVERIFY(timer.isActive());
+    timer.stop();
+    QVERIFY2(jobError.isEmpty(), qPrintable(jobError));
+    compareFiles(dir2ForFilesToDownload);
+    QVERIFY(success);
 
     // Remove the uploaded files on the remote system
     timer.setInterval((params.timeout + 5) * 1000);