RemoteLinux: Rework TarPackage related build steps

Change-Id: I13c27e89186ab5451e1f11cf3cf28cfd9ed10a9d
Reviewed-by: Christian Stenger <christian.stenger@qt.io>

src/plugins/remotelinux/remotelinux_constants.h

@@ -13,6 +13,7 @@ const char DeployToGenericLinux[] = "DeployToGenericLinux";
 const char DirectUploadStepId[] = "RemoteLinux.DirectUploadStep";
 const char MakeInstallStepId[] = "RemoteLinux.MakeInstall";
 const char TarPackageCreationStepId[]  = "MaemoTarPackageCreationStep";
+const char TarPackageFilePathId[] = "TarPackageFilePath";
 const char TarPackageDeployStepId[] = "MaemoUploadAndInstallTarPackageStep";
 const char RsyncDeployStepId[] = "RemoteLinux.RsyncDeployStep";
 const char CustomCommandDeployStepId[] = "RemoteLinux.GenericRemoteLinuxCustomCommandDeploymentStep";

src/plugins/remotelinux/remotelinuxplugin.cpp

@@ -55,7 +55,7 @@ public:
     RemoteLinuxRunConfigurationFactory runConfigurationFactory;
     RemoteLinuxCustomRunConfigurationFactory customRunConfigurationFactory;
     RemoteLinuxDeployConfigurationFactory deployConfigurationFactory;
-    GenericDeployStepFactory<TarPackageCreationStep> tarPackageCreationStepFactory;
+    TarPackageCreationStepFactory tarPackageCreationStepFactory;
     TarPackageDeployStepFactory tarPackageDeployStepFactory;
     GenericDeployStepFactory<GenericDirectUploadStep> genericDirectUploadStepFactory;
     GenericDeployStepFactory<RsyncDeployStep> rsyncDeployStepFactory;

src/plugins/remotelinux/tarpackagecreationstep.cpp

@@ -9,6 +9,7 @@
 
 #include <projectexplorer/buildmanager.h>
 #include <projectexplorer/deploymentdata.h>
+#include <projectexplorer/projectexplorerconstants.h>
 #include <projectexplorer/project.h>
 #include <projectexplorer/target.h>
 
@@ -22,12 +23,13 @@
 using namespace ProjectExplorer;
 using namespace Utils;
 
-namespace RemoteLinux {
+namespace RemoteLinux::Internal {
 
 const char IgnoreMissingFilesKey[] = "RemoteLinux.TarPackageCreationStep.IgnoreMissingFiles";
 const char IncrementalDeploymentKey[] = "RemoteLinux.TarPackageCreationStep.IncrementalDeployment";
 
 const int TarBlockSize = 512;
+
 struct TarFileHeader {
     char fileName[100];
     char fileMode[8];
@@ -48,11 +50,29 @@ struct TarFileHeader {
     char padding[12];
 };
 
-namespace Internal {
+class TarPackageCreationStep : public BuildStep
-
-class TarPackageCreationStepPrivate
 {
 public:
+    TarPackageCreationStep(BuildStepList *bsl, Id id);
+
+    FilePath packageFilePath() const;
+
+private:
+    bool init() final;
+    void doRun() final;
+    bool fromMap(const QVariantMap &map) final;
+    QVariantMap toMap() const final;
+    QVariant data(Id id) const final;
+
+    void raiseError(const QString &errorMessage);
+    void raiseWarning(const QString &warningMessage);
+    bool isPackagingNeeded() const;
+    void deployFinished(bool success);
+    void addNeededDeploymentFiles(const DeployableFile &deployable, const Kit *kit);
+    bool runImpl();
+    bool doPackage();
+    bool appendFile(QFile &tarFile, const QFileInfo &fileInfo, const QString &remoteFilePath);
+
     FilePath m_cachedPackageFilePath;
     bool m_deploymentDataModified = false;
     DeploymentTimeInfo m_deployTimes;
@@ -62,26 +82,23 @@ public:
     QList<DeployableFile> m_files;
 };
 
-} // namespace Internal
-
 TarPackageCreationStep::TarPackageCreationStep(BuildStepList *bsl, Id id)
     : BuildStep(bsl, id)
-    , d(new Internal::TarPackageCreationStepPrivate)
 {
     connect(target(), &Target::deploymentDataChanged, this, [this] {
-        d->m_deploymentDataModified = true;
+        m_deploymentDataModified = true;
     });
-    d->m_deploymentDataModified = true;
+    m_deploymentDataModified = true;
 
-    d->m_ignoreMissingFilesAspect = addAspect<BoolAspect>();
+    m_ignoreMissingFilesAspect = addAspect<BoolAspect>();
-    d->m_ignoreMissingFilesAspect->setLabel(Tr::tr("Ignore missing files"),
+    m_ignoreMissingFilesAspect->setLabel(Tr::tr("Ignore missing files"),
                                          BoolAspect::LabelPlacement::AtCheckBox);
-    d->m_ignoreMissingFilesAspect->setSettingsKey(IgnoreMissingFilesKey);
+    m_ignoreMissingFilesAspect->setSettingsKey(IgnoreMissingFilesKey);
 
-    d->m_incrementalDeploymentAspect = addAspect<BoolAspect>();
+    m_incrementalDeploymentAspect = addAspect<BoolAspect>();
-    d->m_incrementalDeploymentAspect->setLabel(Tr::tr("Package modified files only"),
+    m_incrementalDeploymentAspect->setLabel(Tr::tr("Package modified files only"),
-                                            BoolAspect::LabelPlacement::AtCheckBox);
+                                         BoolAspect::LabelPlacement::AtCheckBox);
-    d->m_incrementalDeploymentAspect->setSettingsKey(IncrementalDeploymentKey);
+    m_incrementalDeploymentAspect->setSettingsKey(IncrementalDeploymentKey);
 
     setSummaryUpdater([this] {
         FilePath path = packageFilePath();
@@ -92,30 +109,18 @@ TarPackageCreationStep::TarPackageCreationStep(BuildStepList *bsl, Id id)
     });
 }
 
-TarPackageCreationStep::~TarPackageCreationStep() = default;
-
-Utils::Id TarPackageCreationStep::stepId()
-{
-    return Constants::TarPackageCreationStepId;
-}
-
-QString TarPackageCreationStep::displayName()
-{
-    return Tr::tr("Create tarball");
-}
-
 FilePath TarPackageCreationStep::packageFilePath() const
 {
     if (buildDirectory().isEmpty())
         return {};
-    const QString packageFileName = project()->displayName() + QLatin1String(".tar");
+    const QString packageFileName = project()->displayName() + ".tar";
     return buildDirectory().pathAppended(packageFileName);
 }
 
 bool TarPackageCreationStep::init()
 {
-    d->m_cachedPackageFilePath = packageFilePath();
+    m_cachedPackageFilePath = packageFilePath();
-    d->m_packagingNeeded = isPackagingNeeded();
+    m_packagingNeeded = isPackagingNeeded();
     return true;
 }
 
@@ -128,21 +133,28 @@ bool TarPackageCreationStep::fromMap(const QVariantMap &map)
 {
     if (!BuildStep::fromMap(map))
         return false;
-    d->m_deployTimes.importDeployTimes(map);
+    m_deployTimes.importDeployTimes(map);
     return true;
 }
 
 QVariantMap TarPackageCreationStep::toMap() const
 {
     QVariantMap map = BuildStep::toMap();
-    map.insert(d->m_deployTimes.exportDeployTimes());
+    map.insert(m_deployTimes.exportDeployTimes());
     return map;
 }
 
+QVariant TarPackageCreationStep::data(Id id) const
+{
+    if (id == Constants::TarPackageFilePathId)
+        return packageFilePath().toVariant();
+    return {};
+}
+
 void TarPackageCreationStep::raiseError(const QString &errorMessage)
 {
     emit addTask(DeploymentTask(Task::Error, errorMessage));
-    emit addOutput(errorMessage, BuildStep::OutputFormat::Stderr);
+    emit addOutput(errorMessage, OutputFormat::Stderr);
 }
 
 void TarPackageCreationStep::raiseWarning(const QString &warningMessage)
@@ -154,7 +166,7 @@ void TarPackageCreationStep::raiseWarning(const QString &warningMessage)
 bool TarPackageCreationStep::isPackagingNeeded() const
 {
     const FilePath packagePath = packageFilePath();
-    if (!packagePath.exists() || d->m_deploymentDataModified)
+    if (!packagePath.exists() || m_deploymentDataModified)
         return true;
 
     const DeploymentData &dd = target()->deploymentData();
@@ -177,19 +189,18 @@ void TarPackageCreationStep::deployFinished(bool success)
     const Kit *kit = target()->kit();
 
     // Store files that have been tar'd and successfully deployed
-    const auto files = d->m_files;
+    for (const DeployableFile &file : qAsConst(m_files))
-    for (const DeployableFile &file : files)
+        m_deployTimes.saveDeploymentTimeStamp(file, kit, QDateTime());
-        d->m_deployTimes.saveDeploymentTimeStamp(file, kit, QDateTime());
 }
 
 void TarPackageCreationStep::addNeededDeploymentFiles(
-        const ProjectExplorer::DeployableFile &deployable,
+        const DeployableFile &deployable,
-        const ProjectExplorer::Kit *kit)
+        const Kit *kit)
 {
     const QFileInfo fileInfo = deployable.localFilePath().toFileInfo();
     if (!fileInfo.isDir()) {
-        if (d->m_deployTimes.hasLocalFileChanged(deployable, kit))
+        if (m_deployTimes.hasLocalFileChanged(deployable, kit))
-            d->m_files << deployable;
+            m_files << deployable;
         return;
     }
 
@@ -197,7 +208,7 @@ void TarPackageCreationStep::addNeededDeploymentFiles(
             .entryList(QDir::Files | QDir::Dirs | QDir::NoDotAndDotDot);
 
     if (files.isEmpty()) {
-        d->m_files << deployable;
+        m_files << deployable;
         return;
     }
 
@@ -215,18 +226,18 @@ bool TarPackageCreationStep::runImpl()
 {
     const QList<DeployableFile> &files = target()->deploymentData().allFiles();
 
-    if (d->m_incrementalDeploymentAspect->value()) {
+    if (m_incrementalDeploymentAspect->value()) {
-        d->m_files.clear();
+        m_files.clear();
         for (const DeployableFile &file : files)
             addNeededDeploymentFiles(file, kit());
     } else {
-        d->m_files = files;
+        m_files = files;
     }
 
     const bool success = doPackage();
 
     if (success) {
-        d->m_deploymentDataModified = false;
+        m_deploymentDataModified = false;
         emit addOutput(Tr::tr("Packaging finished successfully."), OutputFormat::NormalMessage);
     } else {
         emit addOutput(Tr::tr("Packaging failed."), OutputFormat::ErrorMessage);
@@ -241,13 +252,13 @@ bool TarPackageCreationStep::runImpl()
 bool TarPackageCreationStep::doPackage()
 {
     emit addOutput(Tr::tr("Creating tarball..."), OutputFormat::NormalMessage);
-    if (!d->m_packagingNeeded) {
+    if (!m_packagingNeeded) {
         emit addOutput(Tr::tr("Tarball up to date, skipping packaging."), OutputFormat::NormalMessage);
         return true;
     }
 
     // TODO: Optimization: Only package changed files
-    const FilePath tarFilePath = d->m_cachedPackageFilePath;
+    const FilePath tarFilePath = m_cachedPackageFilePath;
     QFile tarFile(tarFilePath.toString());
 
     if (!tarFile.open(QIODevice::WriteOnly | QIODevice::Truncate)) {
@@ -256,7 +267,7 @@ bool TarPackageCreationStep::doPackage()
         return false;
     }
 
-    for (const DeployableFile &d : qAsConst(d->m_files)) {
+    for (const DeployableFile &d : qAsConst(m_files)) {
         if (d.remoteDirectory().isEmpty()) {
             emit addOutput(Tr::tr("No remote path specified for file \"%1\", skipping.")
                 .arg(d.localFilePath().toUserOutput()), OutputFormat::ErrorMessage);
@@ -362,7 +373,7 @@ bool TarPackageCreationStep::appendFile(QFile &tarFile, const QFileInfo &fileInf
     const QString &remoteFilePath)
 {
     QString errorMessage;
-    if (!writeHeader(tarFile, fileInfo, remoteFilePath, d->m_cachedPackageFilePath.toUserOutput(),
+    if (!writeHeader(tarFile, fileInfo, remoteFilePath, m_cachedPackageFilePath.toUserOutput(),
                      &errorMessage)) {
         raiseError(errorMessage);
         return false;
@@ -384,7 +395,7 @@ bool TarPackageCreationStep::appendFile(QFile &tarFile, const QFileInfo &fileInf
     if (!file.open(QIODevice::ReadOnly)) {
         const QString message = Tr::tr("Error reading file \"%1\": %2.")
                                 .arg(nativePath, file.errorString());
-        if (d->m_ignoreMissingFilesAspect->value()) {
+        if (m_ignoreMissingFilesAspect->value()) {
             raiseWarning(message);
             return true;
         } else {
@@ -422,4 +433,13 @@ bool TarPackageCreationStep::appendFile(QFile &tarFile, const QFileInfo &fileInf
     return true;
 }
 
-} // namespace RemoteLinux
+TarPackageCreationStepFactory::TarPackageCreationStepFactory()
+{
+    registerStep<TarPackageCreationStep>(Constants::TarPackageCreationStepId);
+    setDisplayName(Tr::tr("Create tarball"));
+
+    setSupportedConfiguration(RemoteLinux::Constants::DeployToGenericLinux);
+    setSupportedStepList(ProjectExplorer::Constants::BUILDSTEPS_DEPLOY);
+}
+
+} // RemoteLinux::Internal

src/plugins/remotelinux/tarpackagecreationstep.h

@@ -3,51 +3,14 @@
 
 #pragma once
 
-#include "remotelinux_export.h"
-
 #include <projectexplorer/buildstep.h>
 
-QT_BEGIN_NAMESPACE
+namespace RemoteLinux::Internal {
-class QFile;
-class QFileInfo;
-QT_END_NAMESPACE
 
-namespace ProjectExplorer { class DeployableFile; }
+class TarPackageCreationStepFactory : public ProjectExplorer::BuildStepFactory
-
-namespace RemoteLinux {
-
-namespace Internal { class TarPackageCreationStepPrivate; }
-
-class REMOTELINUX_EXPORT TarPackageCreationStep : public ProjectExplorer::BuildStep
 {
-    Q_OBJECT
-
 public:
-    explicit TarPackageCreationStep(ProjectExplorer::BuildStepList *bsl, Utils::Id id);
+    TarPackageCreationStepFactory();
-    ~TarPackageCreationStep() override;
-
-    static Utils::Id stepId();
-    static QString displayName();
-
-    Utils::FilePath packageFilePath() const;
-
-private:
-    bool init() override;
-    void doRun() override;
-    bool fromMap(const QVariantMap &map) override;
-    QVariantMap toMap() const override;
-
-    void raiseError(const QString &errorMessage);
-    void raiseWarning(const QString &warningMessage);
-    bool isPackagingNeeded() const;
-    void deployFinished(bool success);
-    void addNeededDeploymentFiles(const ProjectExplorer::DeployableFile &deployable,
-                                  const ProjectExplorer::Kit *kit);
-    bool runImpl();
-    bool doPackage();
-    bool appendFile(QFile &tarFile, const QFileInfo &fileInfo, const QString &remoteFilePath);
-
-    std::unique_ptr<Internal::TarPackageCreationStepPrivate> d;
 };
 
 } // namespace RemoteLinux

src/plugins/remotelinux/tarpackagedeploystep.cpp

@@ -7,7 +7,6 @@
 #include "abstractremotelinuxdeploystep.h"
 #include "remotelinux_constants.h"
 #include "remotelinuxtr.h"
-#include "tarpackagecreationstep.h"
 
 #include <projectexplorer/deployconfiguration.h>
 #include <projectexplorer/devicesupport/filetransfer.h>
@@ -22,74 +21,10 @@
 using namespace ProjectExplorer;
 using namespace Utils;
 
-namespace RemoteLinux {
+namespace RemoteLinux::Internal {
-namespace Internal {
-
-class TarPackageInstaller : public QObject
-{
-    Q_OBJECT
-
-public:
-    TarPackageInstaller();
-
-    void installPackage(const IDeviceConstPtr &deviceConfig,
-                        const QString &packageFilePath,
-                        bool removePackageFile);
-    void cancelInstallation();
-
-signals:
-    void stdoutData(const QString &output);
-    void stderrData(const QString &output);
-    void finished(const QString &errorMsg = QString());
-
-private:
-    IDevice::ConstPtr m_device;
-    QtcProcess m_installer;
-    QtcProcess m_killer;
-};
-
-TarPackageInstaller::TarPackageInstaller()
-{
-    connect(&m_installer, &QtcProcess::readyReadStandardOutput, this, [this] {
-        emit stdoutData(QString::fromUtf8(m_installer.readAllStandardOutput()));
-    });
-    connect(&m_installer, &QtcProcess::readyReadStandardError, this, [this] {
-        emit stderrData(QString::fromUtf8(m_installer.readAllStandardError()));
-    });
-    connect(&m_installer, &QtcProcess::done, this, [this] {
-        const QString errorMessage = m_installer.result() == ProcessResult::FinishedWithSuccess
-                ? QString() : Tr::tr("Installing package failed.") + m_installer.errorString();
-        emit finished(errorMessage);
-    });
-}
-
-void TarPackageInstaller::installPackage(const IDevice::ConstPtr &deviceConfig,
-    const QString &packageFilePath, bool removePackageFile)
-{
-    QTC_ASSERT(m_installer.state() == QProcess::NotRunning, return);
-
-    m_device = deviceConfig;
-
-    QString cmdLine = QLatin1String("cd / && tar xvf ") + packageFilePath;
-    if (removePackageFile)
-        cmdLine += QLatin1String(" && (rm ") + packageFilePath + QLatin1String(" || :)");
-    m_installer.setCommand({m_device->filePath("/bin/sh"), {"-c", cmdLine}});
-    m_installer.start();
-}
-
-void TarPackageInstaller::cancelInstallation()
-{
-    QTC_ASSERT(m_installer.state() != QProcess::NotRunning, return);
-
-    m_killer.setCommand({m_device->filePath("/bin/sh"), {"-c", "pkill tar"}});
-    m_killer.start();
-    m_installer.close();
-}
 
 class TarPackageDeployService : public AbstractRemoteLinuxDeployService
 {
-    Q_OBJECT
-
 public:
     TarPackageDeployService();
     void setPackageFilePath(const FilePath &filePath);
@@ -108,10 +43,18 @@ private:
 
     void setFinished();
 
+    void installPackage(const IDeviceConstPtr &deviceConfig,
+                        const QString &packageFilePath,
+                        bool removePackageFile);
+    void cancelInstallation();
+
     State m_state = Inactive;
     FileTransfer m_uploader;
     FilePath m_packageFilePath;
-    TarPackageInstaller m_installer;
+
+    IDevice::ConstPtr m_device;
+    QtcProcess m_installer;
+    QtcProcess m_killer;
 };
 
 TarPackageDeployService::TarPackageDeployService()
@@ -120,6 +63,41 @@ TarPackageDeployService::TarPackageDeployService()
             &TarPackageDeployService::handleUploadFinished);
     connect(&m_uploader, &FileTransfer::progress, this,
             &TarPackageDeployService::progressMessage);
+
+    connect(&m_installer, &QtcProcess::readyReadStandardOutput, this, [this] {
+        emit stdOutData(QString::fromUtf8(m_installer.readAllStandardOutput()));
+    });
+    connect(&m_installer, &QtcProcess::readyReadStandardError, this, [this] {
+        emit stdErrData(QString::fromUtf8(m_installer.readAllStandardError()));
+    });
+    connect(&m_installer, &QtcProcess::done, this, [this] {
+        const QString errorMessage = m_installer.result() == ProcessResult::FinishedWithSuccess
+                ? QString() : Tr::tr("Installing package failed.") + m_installer.errorString();
+        handleInstallationFinished(errorMessage);
+    });
+}
+
+void TarPackageDeployService::installPackage(const IDevice::ConstPtr &deviceConfig,
+    const QString &packageFilePath, bool removePackageFile)
+{
+    QTC_ASSERT(m_installer.state() == QProcess::NotRunning, return);
+
+    m_device = deviceConfig;
+
+    QString cmdLine = QLatin1String("cd / && tar xvf ") + packageFilePath;
+    if (removePackageFile)
+        cmdLine += QLatin1String(" && (rm ") + packageFilePath + QLatin1String(" || :)");
+    m_installer.setCommand({m_device->filePath("/bin/sh"), {"-c", cmdLine}});
+    m_installer.start();
+}
+
+void TarPackageDeployService::cancelInstallation()
+{
+    QTC_ASSERT(m_installer.state() != QProcess::NotRunning, return);
+
+    m_killer.setCommand({m_device->filePath("/bin/sh"), {"-c", "pkill tar"}});
+    m_killer.start();
+    m_installer.close();
 }
 
 void TarPackageDeployService::setPackageFilePath(const FilePath &filePath)
@@ -161,7 +139,7 @@ void TarPackageDeployService::stopDeployment()
         setFinished();
         break;
     case Installing:
-        m_installer.cancelInstallation();
+        cancelInstallation();
         setFinished();
         break;
     }
@@ -181,13 +159,8 @@ void TarPackageDeployService::handleUploadFinished(const ProcessResultData &resu
     const QString remoteFilePath = uploadDir() + '/' + m_packageFilePath.fileName();
     m_state = Installing;
     emit progressMessage(Tr::tr("Installing package to device..."));
-    connect(&m_installer, &TarPackageInstaller::stdoutData,
+
-            this, &AbstractRemoteLinuxDeployService::stdOutData);
+    installPackage(deviceConfiguration(), remoteFilePath, true);
-    connect(&m_installer, &TarPackageInstaller::stderrData,
-            this, &AbstractRemoteLinuxDeployService::stdErrData);
-    connect(&m_installer, &TarPackageInstaller::finished,
-            this, &TarPackageDeployService::handleInstallationFinished);
-    m_installer.installPackage(deviceConfiguration(), remoteFilePath, true);
 }
 
 void TarPackageDeployService::handleInstallationFinished(const QString &errorMsg)
@@ -224,18 +197,22 @@ public:
         setWidgetExpandedByDefault(false);
 
         setInternalInitializer([this, service] {
-            const TarPackageCreationStep *pStep = nullptr;
+            const BuildStep *tarCreationStep = nullptr;
 
             for (BuildStep *step : deployConfiguration()->stepList()->steps()) {
                 if (step == this)
                     break;
-                if ((pStep = qobject_cast<TarPackageCreationStep *>(step)))
+                if (step->id() == Constants::TarPackageCreationStepId) {
+                    tarCreationStep = step;
                     break;
+                }
             }
-            if (!pStep)
+            if (!tarCreationStep)
                 return CheckResult::failure(Tr::tr("No tarball creation step found."));
 
-            service->setPackageFilePath(pStep->packageFilePath());
+            const FilePath tarFile =
+                    FilePath::fromVariant(tarCreationStep->data(Constants::TarPackageFilePathId));
+            service->setPackageFilePath(tarFile);
             return service->isDeploymentPossible();
         });
     }
@@ -252,7 +229,4 @@ TarPackageDeployStepFactory::TarPackageDeployStepFactory()
     setSupportedStepList(ProjectExplorer::Constants::BUILDSTEPS_DEPLOY);
 }
 
-} // Internal
+} // RemoteLinux::Internal
-} // RemoteLinux
-
-#include "tarpackagedeploystep.moc"

src/plugins/remotelinux/tarpackagedeploystep.h

@@ -5,8 +5,7 @@
 
 #include <projectexplorer/buildstep.h>
 
-namespace RemoteLinux {
+namespace RemoteLinux::Internal {
-namespace Internal {
 
 class TarPackageDeployStepFactory : public ProjectExplorer::BuildStepFactory
 {
@@ -14,5 +13,4 @@ public:
     TarPackageDeployStepFactory();
 };
 
-} // Internal
+} // RemoteLinux::Internal
-} // RemoteLinux