From 15b7c3039a41eab75f5dcabab53de82372d2ecfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Mr=C3=A1zek?= Date: Tue, 9 Jun 2015 23:23:46 +0200 Subject: GH-1060 update tweaks * download to multimc folder hierarchy * use rename, not copy * keep backup after update * clean previous backup before update * it's not 'copy', it's 'replace' --- application/MainWindow.cpp | 3 +- application/MultiMC.cpp | 78 ++++++++++++++++++++++++------------------ logic/updater/DownloadTask.cpp | 4 +-- logic/updater/DownloadTask.h | 7 +++- logic/updater/GoUpdate.h | 4 +-- tests/tst_DownloadTask.cpp | 2 +- 6 files changed, 58 insertions(+), 40 deletions(-) diff --git a/application/MainWindow.cpp b/application/MainWindow.cpp index 8c827806..ad4cbf7a 100644 --- a/application/MainWindow.cpp +++ b/application/MainWindow.cpp @@ -988,7 +988,8 @@ void MainWindow::downloadUpdates(GoUpdate::Status status) ProgressDialog updateDlg(this); status.rootPath = MMC->rootPath; - GoUpdate::DownloadTask updateTask(status, &updateDlg); + auto dlPath = PathCombine(MMC->root(), "update", "XXXXXX"); + GoUpdate::DownloadTask updateTask(status, dlPath, &updateDlg); // If the task succeeds, install the updates. if (updateDlg.exec(&updateTask)) { diff --git a/application/MultiMC.cpp b/application/MultiMC.cpp index 1d251323..18a819ab 100644 --- a/application/MultiMC.cpp +++ b/application/MultiMC.cpp @@ -662,18 +662,20 @@ void MultiMC::installUpdates(const QString updateFilesDir, GoUpdate::OperationLi #error Unsupported operating system. #endif - QString backupPath = PathCombine(root(), "update-backup"); - QString trashPath = PathCombine(root(), "update-trash"); - if(!ensureFolderPathExists(backupPath)) + QString backupPath = PathCombine(root(), "update", "backup"); + + // clean up the backup folder. it should be empty before we start + if(!deletePath(backupPath)) { - qWarning() << "couldn't create folder" << backupPath; - return; + qWarning() << "couldn't remove previous backup folder" << backupPath; } - if(!ensureFolderPathExists(trashPath)) + // and it should exist. + if(!ensureFolderPathExists(backupPath)) { - qWarning() << "couldn't create folder" << trashPath; + qWarning() << "couldn't create folder" << backupPath; return; } + struct BackupEntry { QString orig; @@ -681,30 +683,34 @@ void MultiMC::installUpdates(const QString updateFilesDir, GoUpdate::OperationLi }; enum Failure { - Copy, + Replace, Delete, Start, Nothing } failedOperationType = Nothing; - QString failedFile; QList backups; QList trashcan; + + // perform the update operations for(auto op: operations) { switch(op.type) { - case GoUpdate::Operation::OP_COPY: + // replace = move original out to backup, if it exists, move the new file in its place + case GoUpdate::Operation::OP_REPLACE: { QFileInfo replaced (PathCombine(root(), op.dest)); if(replaced.exists()) { - QString backupFilePath = PathCombine(backupPath, replaced.fileName()); + QString backupName = op.dest; + backupName.replace('/', '_'); + QString backupFilePath = PathCombine(backupPath, backupName); if(!QFile::rename(replaced.absoluteFilePath(), backupFilePath)) { - qWarning() << "Couldn't rename:" << replaced.absoluteFilePath() << "to" << backupFilePath; - failedOperationType = Copy; + qWarning() << "Couldn't move:" << replaced.absoluteFilePath() << "to" << backupFilePath; + failedOperationType = Replace; failedFile = op.dest; goto FAILED; } @@ -713,17 +719,26 @@ void MultiMC::installUpdates(const QString updateFilesDir, GoUpdate::OperationLi be.backup = backupFilePath; backups.append(be); } - // FIXME: use rename instead. - if(!QFile::copy(op.file, replaced.absoluteFilePath())) + // make sure the folder we are putting this into exists + if(!ensureFilePathExists(replaced.absoluteFilePath())) + { + qWarning() << "REPLACE: Couldn't create folder:" << replaced.absoluteFilePath(); + failedOperationType = Replace; + failedFile = op.dest; + goto FAILED; + } + // now move the new file in + if(!QFile::rename(op.file, replaced.absoluteFilePath())) { - qWarning() << "CPY: Couldn't copy:" << op.file << "to" << replaced.absoluteFilePath(); - failedOperationType = Copy; + qWarning() << "REPLACE: Couldn't move:" << op.file << "to" << replaced.absoluteFilePath(); + failedOperationType = Replace; failedFile = op.dest; goto FAILED; } QFile::setPermissions(replaced.absoluteFilePath(), unixModeToPermissions(op.mode)); } break; + // delete = move original to backup case GoUpdate::Operation::OP_DELETE: { QString trashFilePath = PathCombine(backupPath, op.file); @@ -732,7 +747,7 @@ void MultiMC::installUpdates(const QString updateFilesDir, GoUpdate::OperationLi { if(!QFile::rename(origFilePath, trashFilePath)) { - qWarning() << "DEL: Couldn't move:" << op.file << "to" << trashFilePath; + qWarning() << "DELETE: Couldn't move:" << op.file << "to" << trashFilePath; failedFile = op.file; failedOperationType = Delete; goto FAILED; @@ -758,15 +773,6 @@ void MultiMC::installUpdates(const QString updateFilesDir, GoUpdate::OperationLi failedOperationType = Start; goto FAILED; } - // now clean up the backed up stuff. - for(auto backup:backups) - { - QFile::remove(backup.backup); - } - for(auto backup:trashcan) - { - QFile::remove(backup.backup); - } qApp->quit(); return; @@ -787,13 +793,17 @@ FAILED: if(!QFile::rename(backup.backup, backup.orig)) { revertOK = false; - qWarning() << "removing new" << backup.orig << "failed!"; + qWarning() << "restoring" << backup.orig << "failed!"; } } for(auto backup:trashcan) { qWarning() << "restoring" << backup.orig << "from" << backup.backup; - revertOK &= QFile::rename(backup.backup, backup.orig); + if(!QFile::rename(backup.backup, backup.orig)) + { + revertOK = false; + qWarning() << "restoring" << backup.orig << "failed!"; + } } QString msg; if(!revertOK) @@ -804,11 +814,13 @@ FAILED: } else switch (failedOperationType) { - case Copy: - msg = tr("Couldn't replace file %1. Changes were reverted.\nSee the MultiMC log file for details.").arg(failedFile); + case Replace: + msg = tr("Couldn't replace file %1. Changes were reverted.\n" + "See the MultiMC log file for details.").arg(failedFile); break; case Delete: - msg = tr("Couldn't remove file %1. Changes were reverted.\nSee the MultiMC log file for details.").arg(failedFile); + msg = tr("Couldn't remove file %1. Changes were reverted.\n" + "See the MultiMC log file for details.").arg(failedFile); break; case Start: msg = tr("The new version didn't start and the update was rolled back."); @@ -817,7 +829,7 @@ FAILED: default: return; } - QMessageBox::critical(nullptr, tr("Update failed"), msg); + QMessageBox::critical(nullptr, tr("Update failed!"), msg); } void MultiMC::setIconTheme(const QString& name) diff --git a/logic/updater/DownloadTask.cpp b/logic/updater/DownloadTask.cpp index 4a42f583..76741748 100644 --- a/logic/updater/DownloadTask.cpp +++ b/logic/updater/DownloadTask.cpp @@ -27,8 +27,8 @@ namespace GoUpdate { -DownloadTask::DownloadTask(Status status, QObject *parent) - : Task(parent) +DownloadTask::DownloadTask(Status status, QString target, QObject *parent) + : Task(parent), m_updateFilesDir(target) { m_status = status; diff --git a/logic/updater/DownloadTask.h b/logic/updater/DownloadTask.h index 3bc504fc..d77c2b2c 100644 --- a/logic/updater/DownloadTask.h +++ b/logic/updater/DownloadTask.h @@ -30,7 +30,12 @@ class DownloadTask : public Task Q_OBJECT public: - explicit DownloadTask(Status status, QObject* parent = 0); + /** + * Create a download task + * + * target is a template - XXXXXX at the end will be replaced with a random generated string, ensuring uniqueness + */ + explicit DownloadTask(Status status, QString target, QObject* parent = 0); /// Get the directory that will contain the update files. QString updateFilesDir(); diff --git a/logic/updater/GoUpdate.h b/logic/updater/GoUpdate.h index 21976f8f..479f1eeb 100644 --- a/logic/updater/GoUpdate.h +++ b/logic/updater/GoUpdate.h @@ -68,7 +68,7 @@ struct Operation { static Operation CopyOp(QString fsource, QString fdest, int fmode=0644) { - return Operation{OP_COPY, fsource, fdest, fmode}; + return Operation{OP_REPLACE, fsource, fdest, fmode}; } static Operation DeleteOp(QString file) { @@ -84,7 +84,7 @@ struct Operation //! Specifies the type of operation that this is. enum Type { - OP_COPY, + OP_REPLACE, OP_DELETE, } type; diff --git a/tests/tst_DownloadTask.cpp b/tests/tst_DownloadTask.cpp index eb58762b..3ff98228 100644 --- a/tests/tst_DownloadTask.cpp +++ b/tests/tst_DownloadTask.cpp @@ -40,7 +40,7 @@ QDebug operator<<(QDebug dbg, const Operation::Type &t) { switch (t) { - case Operation::OP_COPY: + case Operation::OP_REPLACE: dbg << "OP_COPY"; break; case Operation::OP_DELETE: -- cgit v1.2.3