diff --git a/cmake/NSIS.template.in b/cmake/NSIS.template.in index a23d733e8..811a3cb7f 100644 --- a/cmake/NSIS.template.in +++ b/cmake/NSIS.template.in @@ -18,7 +18,9 @@ Var OldUninstallString Var OldInstallDir Var DataBackupDir Var DataBackupDone +Var DataBackupForDir Var LooseDecksBackupDone +Var InstallSubdirsBackupDone Var LooseDeckNoticeShown Var StaleFindHandle Var StaleFindName @@ -135,6 +137,11 @@ StrCpy $InstallLogPath "$TEMP\Cockatrice-installer.log" ; No backup has been taken yet; RestoreUserDataAfterUninstall must be a no-op ; until BackupUserDataBeforeUninstall sets this after actually copying something. StrCpy $DataBackupDir "" +StrCpy $DataBackupDone "" +StrCpy $DataBackupForDir "" +StrCpy $LooseDecksBackupDone "" +StrCpy $InstallSubdirsBackupDone "" +StrCpy $LooseDeckNoticeShown "" ; No old install location resolved yet; Backup/Restore fall back to $INSTDIR ; when this is empty (the /R path where $INSTDIR already is the old dir). StrCpy $OldInstallDir "" @@ -367,16 +374,30 @@ FunctionEnd ; outside $INSTDIR before invoking it and put it back afterwards. Best effort ; only: a failed backup must never abort the install. ; -; The backup is intentionally left in %TEMP% afterwards as a second safety copy. -; Non-portable data paths beyond loose decks are covered by the client's -; startup safety backup (warnAboutDataPathsInProgramDir). +; The backup is left in %TEMP% afterwards on a best-effort basis only: the next +; installer run that takes a new backup wipes it first (see below), so it is not +; a durable second copy. Anything this backup does not cover - e.g. loose files +; with other extensions - relies on the client's startup safety backup +; (warnAboutDataPathsInProgramDir), which only exists after the first launch of +; the new client. +; +; The 32-bit and 64-bit uninstall blocks below share $TEMP\CockatriceDataBackup. +; When both are registered with different InstallLocations, backing up the second +; directory into the first one's backup would restore the first dir's files into +; the second one. BackupUserDataBeforeUninstall therefore starts over when +; $OldInstallDir changes: the previous pass already restored its own backup, so +; only its leftover %TEMP% copy is lost. RelocateRestoredDataToInstDir only sees +; the last pass, so with two different old dirs plus a new target dir the first +; dir's restored data stays in place instead of moving along - no loss, but the +; user has to copy it manually. ; ; The portable-data branch below is defensive: both callers (AutoUninstallIfNeeded ; on /R, componentsPagePre interactively) only run for non-portable installs, and ; portable installs never write the registry uninstall keys that trigger them. ; It fires only when $OldInstallDir nevertheless contains portable.dat - e.g. a ; stale registry InstallLocation pointing at a portable dir, or portable.dat added -; to a normal install dir. The loose-decks branch is the live one for normal installs. +; to a normal install dir. The loose-decks and install-subdirs branches are the +; live ones for normal installs. Function BackupUserDataBeforeUninstall ; $OldInstallDir is the directory being uninstalled: on the /R path it equals ; $INSTDIR (updater passes /D=), on the interactive path componentsPagePre @@ -394,6 +415,22 @@ ${EndIf} ${If} $DataBackupDir == "" RMDir /r "$TEMP\CockatriceDataBackup" ${EndIf} +; The 32-bit and 64-bit uninstall blocks share one backup dir. When the second +; block points at a different directory, merging would restore the first dir's +; files into the second one, so start over instead. The previous pass already +; restored its own backup; only its leftover %TEMP% copy is discarded. +; StrCmp is case-sensitive while Windows paths are not, so a casing-only +; difference triggers a harmless redundant re-backup of the same content. +${If} $DataBackupDir != "" +${AndIf} $DataBackupForDir != "" +${AndIf} $OldInstallDir != $DataBackupForDir + !insertmacro CockLog "pre-uninstall backup: old dir changed ($DataBackupForDir -> $OldInstallDir), starting a fresh backup" + RMDir /r "$TEMP\CockatriceDataBackup" + StrCpy $DataBackupDir "" + StrCpy $DataBackupDone "" + StrCpy $LooseDecksBackupDone "" + StrCpy $InstallSubdirsBackupDone "" +${EndIf} ${If} ${FileExists} "$OldInstallDir\portable.dat" ${AndIf} ${FileExists} "$OldInstallDir\data" StrCpy $DataBackupDir "$TEMP\CockatriceDataBackup" @@ -448,6 +485,49 @@ ${OrIf} ${FileExists} "$OldInstallDir\*.mwDeck" CopyFiles /SILENT "$OldInstallDir\*.dek" "$DataBackupDir\loose_decks\" CopyFiles /SILENT "$OldInstallDir\*.mwDeck" "$DataBackupDir\loose_decks\" ${EndIf} +; Data folders below the top level (e.g. $OldInstallDir\decks) are deleted by the +; old "RMDir /r $INSTDIR" just like loose decks, and on a 3.1.1 -> new upgrade the +; client's startup backup has never run yet, so stash every top-level directory +; that is not part of the payload. Payload dirs stay in place: they are rewritten +; by the section below, and restoring them could put stale DLLs next to the new +; executables. "data" is skipped once the portable branch above already took it; +; without portable.dat it is user content like any other unknown dir. +FindFirst $DataBackupFindHandle $DataBackupFindName "$OldInstallDir\*.*" +${If} $DataBackupFindHandle != "" +backup_subdirs_loop: + ${If} $DataBackupFindName != "." + ${AndIf} $DataBackupFindName != ".." + ${AndIf} $DataBackupFindName != "plugins" + ${AndIf} $DataBackupFindName != "Plugins" + ${AndIf} $DataBackupFindName != "translations" + ${AndIf} $DataBackupFindName != "sounds" + ${AndIf} $DataBackupFindName != "themes" + ${If} $DataBackupFindName == "data" + ${AndIf} $DataBackupDone == "1" + Goto backup_subdirs_next + ${EndIf} + ${If} ${FileExists} "$OldInstallDir\$DataBackupFindName\*.*" + ${If} $DataBackupDir == "" + StrCpy $DataBackupDir "$TEMP\CockatriceDataBackup" + ${EndIf} + StrCpy $InstallSubdirsBackupDone "1" + !insertmacro CockLog "pre-uninstall backup: copying subdir $OldInstallDir\$DataBackupFindName" + DetailPrint "Backing up $DataBackupFindName stored in the install directory..." + CreateDirectory "$DataBackupDir\install_subdirs\$DataBackupFindName" + CopyFiles /SILENT "$OldInstallDir\$DataBackupFindName\*.*" "$DataBackupDir\install_subdirs\$DataBackupFindName\" + ${EndIf} + ${EndIf} +backup_subdirs_next: + ClearErrors + FindNext $DataBackupFindHandle $DataBackupFindName + IfErrors 0 backup_subdirs_loop + FindClose $DataBackupFindHandle +${EndIf} +; Remember which directory the current backup belongs to, so a later pass for a +; different directory starts over instead of merging (see above). +${If} $DataBackupDir != "" + StrCpy $DataBackupForDir $OldInstallDir +${EndIf} FunctionEnd Function RestoreUserDataAfterUninstall @@ -481,12 +561,37 @@ ${AndIf} ${FileExists} "$DataBackupDir\loose_decks" CopyFiles /SILENT "$DataBackupDir\loose_decks\*.dek" "$OldInstallDir\" CopyFiles /SILENT "$DataBackupDir\loose_decks\*.mwDeck" "$OldInstallDir\" DetailPrint "Restored deck files stored in the install directory." - ; Runs once per uninstall block, and there are two (32-bit, 64-bit): show the - ; notice a single time per installer run. - ${IfNot} ${Silent} - ${AndIf} $LooseDeckNoticeShown != "1" +${EndIf} +${If} $InstallSubdirsBackupDone == "1" +${AndIf} ${FileExists} "$DataBackupDir\install_subdirs" + !insertmacro CockLog "post-uninstall restore: restoring install subdirectories" + CreateDirectory "$OldInstallDir" + FindFirst $DataBackupFindHandle $DataBackupFindName "$DataBackupDir\install_subdirs\*.*" + ${If} $DataBackupFindHandle != "" + restore_subdirs_loop: + ${If} $DataBackupFindName != "." + ${AndIf} $DataBackupFindName != ".." + ${AndIf} ${FileExists} "$DataBackupDir\install_subdirs\$DataBackupFindName\*.*" + !insertmacro CockLog "post-uninstall restore: restoring subdir $OldInstallDir\$DataBackupFindName" + CreateDirectory "$OldInstallDir\$DataBackupFindName" + CopyFiles /SILENT "$DataBackupDir\install_subdirs\$DataBackupFindName\*.*" "$OldInstallDir\$DataBackupFindName\" + ${EndIf} + ClearErrors + FindNext $DataBackupFindHandle $DataBackupFindName + IfErrors 0 restore_subdirs_loop + FindClose $DataBackupFindHandle + ${EndIf} + DetailPrint "Restored folders stored in the install directory." +${EndIf} +; Runs once per uninstall block, and there are two (32-bit, 64-bit): show the +; notice a single time per installer run, covering both loose decks and +; restored subdirectories. +${IfNot} ${Silent} +${AndIf} $LooseDeckNoticeShown != "1" + ${If} $LooseDecksBackupDone == "1" + ${OrIf} $InstallSubdirsBackupDone == "1" StrCpy $LooseDeckNoticeShown "1" - MessageBox MB_OK|MB_ICONINFORMATION "Your decks were stored inside the Cockatrice install folder ($OldInstallDir), which the previous version's uninstaller deletes.$\n$\nThey have been restored from a backup to that folder (deck files ending in .txt are not backed up - move those manually). If you install to a different folder, they are moved there automatically. Please move them out of the install folder afterwards (Settings -> General -> Paths) so a future uninstall cannot delete them again." + MessageBox MB_OK|MB_ICONINFORMATION "Your decks or data folders were stored inside the Cockatrice install folder ($OldInstallDir), which the previous version's uninstaller deletes.$\n$\nThey have been restored from a backup to that folder (loose deck files ending in .txt are not backed up - move those manually). If you install to a different folder, they are moved there automatically. Please move them out of the install folder afterwards (Settings -> General -> Paths) so a future uninstall cannot delete them again." ${EndIf} ${EndIf} done: @@ -500,6 +605,9 @@ FunctionEnd ; target directory, the restored user data would otherwise stay orphaned in the ; old directory while the new install starts empty. Copy it across when they differ ; (a copy, not a move: the source is user data and is deliberately left in place). +; Only the last uninstall pass's $OldInstallDir is relocated (see the shared-backup +; comment above); with two different old dirs the first one's data stays restored +; in place instead of moving along. ; StrCmp is case-sensitive while Windows paths are not; a casing-only difference ; triggers a harmless redundant copy onto itself. Function RelocateRestoredDataToInstDir @@ -527,6 +635,25 @@ ${If} $LooseDecksBackupDone == "1" DetailPrint "Copied restored deck files to the new install folder." ${EndIf} ${EndIf} +${If} $InstallSubdirsBackupDone == "1" +${AndIf} ${FileExists} "$DataBackupDir\install_subdirs" + FindFirst $DataBackupFindHandle $DataBackupFindName "$DataBackupDir\install_subdirs\*.*" + ${If} $DataBackupFindHandle != "" + relocate_subdirs_loop: + ${If} $DataBackupFindName != "." + ${AndIf} $DataBackupFindName != ".." + ${AndIf} ${FileExists} "$OldInstallDir\$DataBackupFindName\*.*" + !insertmacro CockLog "relocate: copying restored subdir $OldInstallDir\$DataBackupFindName -> $INSTDIR\$DataBackupFindName" + CreateDirectory "$INSTDIR\$DataBackupFindName" + CopyFiles /SILENT "$OldInstallDir\$DataBackupFindName\*.*" "$INSTDIR\$DataBackupFindName\" + DetailPrint "Copied restored folder $DataBackupFindName to the new install folder." + ${EndIf} + ClearErrors + FindNext $DataBackupFindHandle $DataBackupFindName + IfErrors 0 relocate_subdirs_loop + FindClose $DataBackupFindHandle + ${EndIf} +${EndIf} done: FunctionEnd diff --git a/cockatrice/src/interface/at_risk_data_warning.cpp b/cockatrice/src/interface/at_risk_data_warning.cpp index 9f905a14b..dfa46096c 100644 --- a/cockatrice/src/interface/at_risk_data_warning.cpp +++ b/cockatrice/src/interface/at_risk_data_warning.cpp @@ -12,7 +12,7 @@ #include #include -inline Q_LOGGING_CATEGORY(AtRiskDataWarningLog, "main_window"); +Q_LOGGING_CATEGORY(AtRiskDataWarningLog, "at_risk_data_warning"); static bool isDeckFileSuffix(const QString &suffix) { @@ -30,22 +30,6 @@ static bool isInsideAnyDir(const QString &path, const QStringList &bases) return false; } -// Merges an at-risk offender into the backup source list: offenders already covered by a -// collected source are skipped, while an offender that is a parent of collected sources -// supersedes them. -static void mergeCoveringSource(QStringList &sources, const QString &offender) -{ - if (isInsideAnyDir(offender, sources)) { - return; - } - for (int i = sources.size() - 1; i >= 0; --i) { - if (PathsSettings::isInsideDir(sources.at(i), offender)) { - sources.removeAt(i); - } - } - sources.append(offender); -} - static bool hasLooseDeckFile(const QStringList &offenders) { for (const QString &offender : offenders) { @@ -65,125 +49,6 @@ static QString resolveBackupRoot(const QString &appDir) return backupRoot; } -// True when a portable offender must stay out of the safety backup: replays (size), -// downloaded pictures (large and re-downloadable - everything under pics/ except the -// seeded CUSTOM dir) and the regenerable redirect/cache dir. -static bool isPortableBackupExcluded(const QString &offender) -{ - auto &paths = SettingsCache::instance().paths(); - const QString replaysPath = QDir::cleanPath(paths.getReplaysPath()); - const QString picsPath = QDir::cleanPath(paths.getPicsPath()); - const QString customPicsPath = QDir::cleanPath(paths.getCustomPicsPath()); - const QString redirectsPath = QDir::cleanPath(paths.getRedirectCachePath()); - const QString cacheDir = QDir::cleanPath(SettingsCache::instance().getCachePath()); - - if (!replaysPath.isEmpty() && PathsSettings::isInsideDir(offender, replaysPath)) { - return true; - } - if (!picsPath.isEmpty() && PathsSettings::isInsideDir(offender, picsPath) && - (customPicsPath.isEmpty() || !PathsSettings::isInsideDir(offender, customPicsPath))) { - return true; - } - if (!redirectsPath.isEmpty() && PathsSettings::isInsideDir(offender, redirectsPath)) { - return true; - } - if (!cacheDir.isEmpty() && PathsSettings::isInsideDir(offender, cacheDir)) { - return true; - } - return false; -} - -AtRiskDataWarning::AtRiskPaths AtRiskDataWarning::collectPortableAtRiskPaths(const QString &appDir) -{ - auto &paths = SettingsCache::instance().paths(); - AtRiskPaths atRisk; - - // Portable data lives inside the install directory by design, so there is - // nothing to reconfigure. Back up the small irreplaceable data instead. - // Downloaded pictures are deliberately excluded: they are large and can - // always be re-downloaded (the installer's pre-uninstall backup drops - // them for the same reason). Replays are excluded here for size; the - // installer backup covers them during installer-driven upgrades, but a - // manual run of the old uninstall.exe is not covered - hence the note - // in the warning text below. - const QString dataDir = SettingsCache::instance().getDataPath(); - for (const QString &sub : - QStringList{QStringLiteral("decks"), QStringLiteral("customsets"), QStringLiteral("settings"), - QStringLiteral("filters"), QStringLiteral("themes"), QStringLiteral("sounds"), - QStringLiteral("pics/CUSTOM")}) { - const QString candidate = QDir::cleanPath(QDir(dataDir).absoluteFilePath(sub)); - if (QFileInfo::exists(candidate)) { - atRisk.sourcesToBackUp.append(candidate); - } - } - - // Card database files live in dataDir by default but are not in the list above. Seed them - // explicitly so they do not depend on the offender merge below to be backed up. - for (const QString &dbFile : - QStringList{paths.getCardDatabasePath(), paths.getTokenDatabasePath(), paths.getSpoilerCardDatabasePath()}) { - if (dbFile.isEmpty()) { - continue; - } - const QString cleanedDbFile = QDir::cleanPath(dbFile); - if (!QFileInfo::exists(cleanedDbFile) || !PathsSettings::isInsideDir(cleanedDbFile, appDir)) { - continue; - } - // isInsideDir is equality-aware and case-insensitive on Windows/macOS, - // so it also covers exact duplicates with different casing or separators. - if (!isInsideAnyDir(cleanedDbFile, atRisk.sourcesToBackUp)) { - atRisk.sourcesToBackUp.append(cleanedDbFile); - } - } - - // A portable user may also have pointed a custom path inside the install - // dir. Offenders outside data/ (e.g. appDir/mydecks) would be wiped by the - // old uninstaller but are not in the hardcoded list above - and neither - // is a custom folder inside data/ itself (e.g. dataDir/mydecks), which the - // old uninstaller deletes all the same. Cover any offender that is not - // already inside a backed-up source; when an offender is a parent of - // already-collected sources it supersedes them. - for (const QString &offender : paths.pathsInsideDir(appDir)) { - if (!QFileInfo::exists(offender)) { - continue; - } - const QString cleanedOffender = QDir::cleanPath(offender); - if (isPortableBackupExcluded(cleanedOffender)) { - continue; - } - mergeCoveringSource(atRisk.sourcesToBackUp, cleanedOffender); - } - - // A fresh portable install has no data on disk yet, but the user still - // needs to know their data will live where the 3.1.0/3.1.1 uninstaller - // deletes it - so fall through to the warning with an empty backup - // instead of returning silently. - if (atRisk.sourcesToBackUp.isEmpty()) { - qCInfo(AtRiskDataWarningLog) << "Portable install with no existing data files, warning without backup"; - } - return atRisk; -} - -QStringList AtRiskDataWarning::collectInstalledAtRiskPaths(const QString &appDir) -{ - QStringList offenders = SettingsCache::instance().paths().pathsInsideDir(appDir); - - // Loose decks sitting directly in the program folder are wiped by the old uninstaller - // just like configured paths. Mirror the installer backup set (DeckLoader extensions - // minus *.txt) so a manual run of the old uninstall.exe is covered too. - const QDir appDirHandle(appDir); - const QStringList topLevelFiles = appDirHandle.entryList(QDir::Files | QDir::Hidden | QDir::NoDotAndDotDot); - for (const QString &fileName : topLevelFiles) { - if (!isDeckFileSuffix(QFileInfo(fileName).suffix().toLower())) { - continue; - } - const QString looseDeck = QDir::cleanPath(appDirHandle.absoluteFilePath(fileName)); - if (!isInsideAnyDir(looseDeck, offenders)) { - offenders.append(looseDeck); - } - } - return offenders; -} - AtRiskDataWarning::AtRiskBackupOutcome AtRiskDataWarning::backUpAtRiskPaths(const QStringList &sources, const QString &appDir) { @@ -208,8 +73,6 @@ AtRiskDataWarning::AtRiskBackupOutcome AtRiskDataWarning::backUpAtRiskPaths(cons // The copy runs synchronously on the UI thread at startup, so cap it: a // pictures folder pointed into the program dir can be gigabytes, and freezing // the client to duplicate it is worse than asking for a manual copy. - // Portable sources are small by construction (downloaded pictures and replays - // are excluded above); this guard bites for non-portable offenders. outcome.bytes = UserDataBackup::estimateBackupSize(sources); if (outcome.bytes >= UserDataBackup::MAX_AUTO_BACKUP_BYTES) { outcome.skippedDueToSize = true; @@ -244,50 +107,6 @@ AtRiskDataWarning::AtRiskBackupOutcome AtRiskDataWarning::backUpAtRiskPaths(cons return outcome; } -QString AtRiskDataWarning::buildPortableAtRiskMessage(const AtRiskBackupOutcome &outcome, bool emptySources) -{ - const QString backupVerb = outcome.reused ? tr("is already available") : tr("has been created"); - const QString sizeInfo = tr("%1 MB (automatic backups are capped at %2 MB)") - .arg(QString::number(outcome.bytes / 1048576.0, 'f', 1), - QString::number(UserDataBackup::MAX_AUTO_BACKUP_BYTES / 1048576)); - if (!outcome.locations.isEmpty()) { - QString message = - tr("This portable install keeps its data inside the program directory, where the " - "uninstallers shipped in Cockatrice 3.1.0/3.1.1 delete everything on uninstall or " - "update.\n\nA safety backup of your decks, custom sets, settings, filters, themes, sounds and " - "custom " - "pictures %1 " - "at:\n%2\n\nReplays and downloaded pictures are not included in this backup. Uninstalling " - "with the old uninstaller still deletes the program " - "directory, so keep your own copy of that backup until you have updated once.") - .arg(backupVerb, outcome.locations.join(QLatin1Char('\n'))); - if (!outcome.complete) { - message += tr("\n\nThe backup may be incomplete; please back up these folders manually as well."); - } - return message; - } - if (outcome.skippedDueToSize) { - return tr("This portable install keeps its data inside the program directory, where the " - "uninstallers shipped in Cockatrice 3.1.0/3.1.1 delete everything on uninstall or " - "update.\n\nThe automatic safety backup was skipped because of its size: %1. " - "Please back up your data folder manually " - "before uninstalling or updating.\n\nReplays and downloaded pictures would not have been " - "included in that backup either.") - .arg(sizeInfo); - } - if (emptySources) { - return tr("This portable install keeps its data inside the program directory, where the " - "uninstallers shipped in Cockatrice 3.1.0/3.1.1 delete everything on uninstall or " - "update.\n\nNone of your data files exist on disk yet, so there was nothing to back up. " - "Files created there later are still at risk until you have updated once."); - } - return tr("This portable install keeps its data inside the program directory, where the " - "uninstallers shipped in Cockatrice 3.1.0/3.1.1 delete everything on uninstall or " - "update.\n\nThe automatic safety backup failed; please back up your data folder manually " - "before uninstalling or updating.\n\nReplays and downloaded pictures would not have been " - "included in that backup either."); -} - QString AtRiskDataWarning::buildInstalledAtRiskMessage(const QStringList &offenders, const QStringList &sources, const AtRiskBackupOutcome &outcome) @@ -342,36 +161,34 @@ void AtRiskDataWarning::warnAboutDataPathsInProgramDir(QWidget *parent) return; } + // Portable installs keep their data inside the program directory by design, but they + // never shipped an uninstaller, and portable updates use the installer's payload + // purge, which preserves data/ - so there is nothing destructive to warn about. + if (SettingsCache::instance().getIsPortableBuild()) { + return; + } + const QString appDir = QCoreApplication::applicationDirPath(); - const bool isPortable = SettingsCache::instance().getIsPortableBuild(); // Sources to back up outside the install directory before warning. The uninstallers // shipped in 3.1.0/3.1.1 delete the whole install tree, so a safety copy is kept // even though the fixed uninstaller preserves foreign files - the destructive one // is already on disk on affected machines. - AtRiskPaths atRisk; - if (isPortable) { - atRisk = collectPortableAtRiskPaths(appDir); - } else { - atRisk.offendersForDisplay = collectInstalledAtRiskPaths(appDir); - if (atRisk.offendersForDisplay.isEmpty()) { - return; - } - // Only existing paths can be copied; a configured-but-never-created path has - // nothing to lose yet and must not turn the warning into a bogus "failed". - for (const QString &existingOffender : atRisk.offendersForDisplay) { - if (QFileInfo::exists(existingOffender)) { - atRisk.sourcesToBackUp.append(existingOffender); - } + const QStringList offenders = collectInstalledAtRiskPaths(appDir); + if (offenders.isEmpty()) { + return; + } + // Only existing paths can be copied; a configured-but-never-created path has + // nothing to lose yet and must not turn the warning into a bogus "failed". + QStringList sourcesToBackUp; + for (const QString &offender : offenders) { + if (QFileInfo::exists(offender)) { + sourcesToBackUp.append(offender); } } - const AtRiskBackupOutcome outcome = backUpAtRiskPaths(atRisk.sourcesToBackUp, appDir); + const AtRiskBackupOutcome outcome = backUpAtRiskPaths(sourcesToBackUp, appDir); - const QString title = - isPortable ? tr("Portable data at risk on uninstall") : tr("Data paths inside the program directory"); - const QString message = - isPortable ? buildPortableAtRiskMessage(outcome, atRisk.sourcesToBackUp.isEmpty()) - : buildInstalledAtRiskMessage(atRisk.offendersForDisplay, atRisk.sourcesToBackUp, outcome); - showAtRiskDataWarning(parent, title, message); + const QString message = buildInstalledAtRiskMessage(offenders, sourcesToBackUp, outcome); + showAtRiskDataWarning(parent, tr("Data paths inside the program directory"), message); } diff --git a/cockatrice/src/interface/at_risk_data_warning.h b/cockatrice/src/interface/at_risk_data_warning.h index fd794d6c0..22cad86fa 100644 --- a/cockatrice/src/interface/at_risk_data_warning.h +++ b/cockatrice/src/interface/at_risk_data_warning.h @@ -1,7 +1,7 @@ #ifndef AT_RISK_DATA_WARNING_H #define AT_RISK_DATA_WARNING_H -#include +#include #include #include #include @@ -12,13 +12,15 @@ class QWidget; * @brief Warns when user data lives inside the program directory and backs it up first. * * The uninstallers shipped in Cockatrice 3.1.0/3.1.1 delete the whole install - * tree, so data paths configured inside the program directory (and portable - * installs, whose data lives there by design) get a safety backup outside the - * install directory before the warning is shown. + * tree, so data paths configured inside the program directory get a safety + * backup outside the install directory before the warning is shown. + * Portable installs keep their data inside the program directory by design but + * never shipped an uninstaller, so they are skipped. */ -class AtRiskDataWarning : public QObject +class AtRiskDataWarning { - Q_OBJECT + Q_DECLARE_TR_FUNCTIONS(AtRiskDataWarning) + public: struct AtRiskBackupOutcome { @@ -29,11 +31,6 @@ public: bool skippedDueToSize = false; ///< true when the automatic copy was skipped for size qint64 bytes = 0; ///< estimated bytes, only set when a fresh copy was attempted }; - struct AtRiskPaths - { - QStringList sourcesToBackUp; ///< existing paths that need a safety copy - QStringList offendersForDisplay; ///< paths listed in the warning (empty for portable) - }; /** * @brief Backs up at-risk data paths and warns about them. @@ -42,10 +39,8 @@ public: static void warnAboutDataPathsInProgramDir(QWidget *parent); private: - static AtRiskPaths collectPortableAtRiskPaths(const QString &appDir); static QStringList collectInstalledAtRiskPaths(const QString &appDir); static AtRiskBackupOutcome backUpAtRiskPaths(const QStringList &sources, const QString &appDir); - static QString buildPortableAtRiskMessage(const AtRiskBackupOutcome &outcome, bool emptySources); static QString buildInstalledAtRiskMessage(const QStringList &offenders, const QStringList &sources, const AtRiskBackupOutcome &outcome); diff --git a/cockatrice/src/interface/window_main.cpp b/cockatrice/src/interface/window_main.cpp index 763f029de..80cb4154b 100644 --- a/cockatrice/src/interface/window_main.cpp +++ b/cockatrice/src/interface/window_main.cpp @@ -606,14 +606,9 @@ MainWindow::MainWindow(QWidget *parent) QTimer::singleShot(0, this, &MainWindow::applyStartupDestination); } -void MainWindow::warnAboutDataPathsInProgramDir() -{ - AtRiskDataWarning::warnAboutDataPathsInProgramDir(this); -} - void MainWindow::startupConfigCheck() { - warnAboutDataPathsInProgramDir(); + AtRiskDataWarning::warnAboutDataPathsInProgramDir(this); const bool isCleanInstall = SettingsCache::instance().network().getClientVersion() == CLIENT_INFO_NOT_SET; diff --git a/cockatrice/src/interface/window_main.h b/cockatrice/src/interface/window_main.h index ee8873000..9e624cd5b 100644 --- a/cockatrice/src/interface/window_main.h +++ b/cockatrice/src/interface/window_main.h @@ -149,9 +149,6 @@ private: void runFirstRunWizard(bool firstRun = false); - /** @brief Warns once when a non-portable install keeps its data paths inside the program directory. */ - void warnAboutDataPathsInProgramDir(); - inline QString getCardUpdaterBinaryName() { return "oracle"; diff --git a/libcockatrice_utility/libcockatrice/utility/user_data_backup.cpp b/libcockatrice_utility/libcockatrice/utility/user_data_backup.cpp index 476a9c788..aa7095545 100644 --- a/libcockatrice_utility/libcockatrice/utility/user_data_backup.cpp +++ b/libcockatrice_utility/libcockatrice/utility/user_data_backup.cpp @@ -33,8 +33,8 @@ bool pathsEqual(const QString &a, const QString &b) void writeCompletionMarker(const QString &sessionDir, bool complete) { - // Best effort: a missing marker only means isBackupSessionComplete() falls - // back to the legacy content check below. + // Best effort: a missing marker only means isBackupSessionComplete() + // reports the session as incomplete. QFile marker(QDir(sessionDir).absoluteFilePath(complete ? COMPLETE_MARKER : INCOMPLETE_MARKER)); if (marker.open(QIODevice::WriteOnly | QIODevice::Text)) { marker.write(complete ? "complete\n" : "incomplete\n"); @@ -132,20 +132,6 @@ bool createBackupSessionDir(const QString &backupRoot, QString *sessionDirOut) return true; } -bool backupAtRiskPath(const QString &sourcePath, const QString &backupRoot, QString *backupLocationOut) -{ - QStringList locations; - const bool ok = backupAtRiskPaths({sourcePath}, backupRoot, &locations, nullptr); - if (backupLocationOut) { - if (!locations.isEmpty()) { - *backupLocationOut = locations.first(); - } else { - backupLocationOut->clear(); - } - } - return ok; -} - bool backupAtRiskPaths(const QStringList &sourcePaths, const QString &backupRoot, QStringList *backupLocationsOut, @@ -204,7 +190,7 @@ bool backupAtRiskPaths(const QStringList &sourcePaths, } if (locations.isEmpty()) { // Nothing salvaged: remove the session again so the empty directory - // neither blocks a retry via findRecentBackupSession() nor evicts a + // neither blocks a retry via the recent-session reuse nor evicts a // good session in pruneOldBackupSessions(). QDir(sessionDir).removeRecursively(); if (backupLocationsOut) { @@ -287,29 +273,6 @@ QStringList listBackupSessions(const QString &backupRoot) return sessions; } -QString findRecentBackupSession(const QString &backupRoot, int maxAgeHours) -{ - const QStringList sessions = listBackupSessions(backupRoot); - const QDateTime now = QDateTime::currentDateTime(); - for (const QString &session : sessions) { - const QFileInfo info(session); - if (!info.exists()) { - continue; - } - // No early break: filename order is not guaranteed to match mtime order - // (clock adjustments, externally touched dirs), and session counts are tiny. - if (info.lastModified().secsTo(now) >= static_cast(maxAgeHours) * 3600) { - continue; - } - // Skip empty sessions from failed attempts: they must never block a retry. - if (listSessionBackupLocations(session).isEmpty()) { - continue; - } - return session; - } - return {}; -} - QString findRecentCoveringBackupSession(const QString &backupRoot, const QStringList &sourcePaths, int maxAgeHours) { const QStringList sessions = listBackupSessions(backupRoot); @@ -368,15 +331,7 @@ bool isBackupSessionComplete(const QString &sessionDir) if (!session.exists()) { return false; } - if (QFileInfo::exists(session.absoluteFilePath(COMPLETE_MARKER))) { - return true; - } - if (QFileInfo::exists(session.absoluteFilePath(INCOMPLETE_MARKER))) { - return false; - } - // Legacy sessions from before markers existed: content means the copy - // reported success at the time, so treat them as complete. - return !listSessionBackupLocations(session.absolutePath()).isEmpty(); + return QFileInfo::exists(session.absoluteFilePath(COMPLETE_MARKER)); } void pruneOldBackupSessions(const QString &backupRoot, int keepCount) @@ -421,8 +376,8 @@ bool sessionCoversSources(const QString &sessionDir, const QStringList &sourcePa { QFile manifest(QDir(sessionDir).absoluteFilePath(SOURCES_MANIFEST)); if (!manifest.open(QIODevice::ReadOnly | QIODevice::Text)) { - // Legacy sessions predate the manifest: coverage cannot be verified, - // so report not-covered and let the caller take a fresh copy. + // No readable manifest (e.g. the write failed): coverage cannot be + // verified, so report not-covered and let the caller take a fresh copy. return false; } QStringList recorded; diff --git a/libcockatrice_utility/libcockatrice/utility/user_data_backup.h b/libcockatrice_utility/libcockatrice/utility/user_data_backup.h index c40b8b0e3..4a4dc1d7f 100644 --- a/libcockatrice_utility/libcockatrice/utility/user_data_backup.h +++ b/libcockatrice_utility/libcockatrice/utility/user_data_backup.h @@ -22,8 +22,9 @@ namespace UserDataBackup { -/** @brief Above this total, the client skips the automatic startup backup and asks for a manual copy. */ -constexpr qint64 MAX_AUTO_BACKUP_BYTES = 500LL * 1024 * 1024; +/** @brief Above this total, the client skips the automatic startup backup and asks for a manual copy. + * Kept small on purpose: the copy runs synchronously on the UI thread at startup. */ +constexpr qint64 MAX_AUTO_BACKUP_BYTES = 200LL * 1024 * 1024; /** * @brief Recursively copies @p sourceDir into @p destDir, creating it as needed. @@ -42,19 +43,6 @@ bool copyDirectoryRecursively(const QString &sourceDir, const QString &destDir); */ bool createBackupSessionDir(const QString &backupRoot, QString *sessionDirOut); -/** - * @brief Copies an at-risk path (file or directory) to a timestamped folder under @p backupRoot. - * @param sourcePath file or directory holding user data (must exist). - * @param backupRoot directory OUTSIDE the install tree that will hold timestamped backups. - * @param backupLocationOut receives the created backup file or directory whenever a target - * was created, even when the copy was only partial (i.e. also on failure with a partial copy). - * Cleared when nothing was salvaged. - * @returns true when the whole source was copied, false when the source is missing or - * any entry failed. A false return with a non-empty @p backupLocationOut means a partial - * backup exists at that location. - */ -bool backupAtRiskPath(const QString &sourcePath, const QString &backupRoot, QString *backupLocationOut = nullptr); - /** * @brief Copies several at-risk paths into a single timestamped session directory. * @param sourcePaths files or directories holding user data; missing entries are skipped. @@ -92,23 +80,11 @@ QString defaultBackupRoot(); */ QStringList listBackupSessions(const QString &backupRoot); -/** - * @brief Returns the newest non-empty session directory younger than @p maxAgeHours, or empty. - * - * Used to throttle startup safety backups so dismissing the warning without - * "Don't show again" does not create a full duplicate on every launch. - * Empty sessions (failed attempts) are skipped so they never block a retry. - * Every session is age-checked individually; no early exit assumes filename - * order matches mtime order (clock adjustments or externally touched dirs - * can break that assumption). - */ -QString findRecentBackupSession(const QString &backupRoot, int maxAgeHours = 24); - /** * @brief Returns the newest session that is non-empty, complete and covering @p sourcePaths, or empty. * - * Unlike findRecentBackupSession(), an incomplete newest session (partial failure) or one - * covering a different source set does not block reuse of an older good session. + * An incomplete newest session (partial failure) or one covering a different + * source set does not block reuse of an older good session. */ QString findRecentUsableBackupSession(const QString &backupRoot, const QStringList &sourcePaths, int maxAgeHours = 24); @@ -123,9 +99,8 @@ findRecentCoveringBackupSession(const QString &backupRoot, const QStringList &so /** * @brief Reports whether @p sessionDir holds a complete backup. - * @returns true when the session carries the complete marker, or when it predates - * markers but holds backup content (legacy sessions); false for partial backups - * and for empty/missing sessions. + * @returns true when the session carries the complete marker, false otherwise + * (partial backups and empty/missing sessions). */ bool isBackupSessionComplete(const QString &sessionDir); @@ -146,9 +121,9 @@ QStringList listSessionBackupLocations(const QString &sessionDir); /** * @brief Reports whether @p sessionDir was recorded as covering all of @p sourcePaths. * @returns true when the session's source manifest lists every entry of @p sourcePaths - * (normalized via QDir::cleanPath, case-insensitive on Windows/macOS). Legacy sessions - * without a manifest return false so a changed source set always triggers a fresh copy - * instead of reusing a backup that may not cover it. + * (normalized via QDir::cleanPath, case-insensitive on Windows/macOS). Sessions + * without a readable manifest return false so a changed source set always triggers + * a fresh copy instead of reusing a backup that may not cover it. */ bool sessionCoversSources(const QString &sessionDir, const QStringList &sourcePaths); diff --git a/tests/user_data_backup_test.cpp b/tests/user_data_backup_test.cpp index cfb1b0a73..b74b26fa3 100644 --- a/tests/user_data_backup_test.cpp +++ b/tests/user_data_backup_test.cpp @@ -98,10 +98,12 @@ TEST(UserDataBackup, BackupDirectoryCreatesTimestampedCopy) deck.write("\n"); deck.close(); - QString backupLocation; - EXPECT_TRUE(UserDataBackup::backupAtRiskPath(sourceDir.path(), backupRoot.path(), &backupLocation)); - EXPECT_TRUE(QFile::exists(backupLocation + "/mydeck.cod")); - EXPECT_TRUE(backupLocation.startsWith(QDir(backupRoot.path()).absolutePath())); + QStringList locations; + QString sessionDir; + EXPECT_TRUE(UserDataBackup::backupAtRiskPaths({sourceDir.path()}, backupRoot.path(), &locations, &sessionDir)); + ASSERT_EQ(locations.size(), 1); + EXPECT_TRUE(QFile::exists(locations.first() + "/mydeck.cod")); + EXPECT_TRUE(locations.first().startsWith(QDir(backupRoot.path()).absolutePath())); } TEST(UserDataBackup, BackupSingleFile) @@ -116,17 +118,23 @@ TEST(UserDataBackup, BackupSingleFile) db.write("\n"); db.close(); - QString backupLocation; - EXPECT_TRUE(UserDataBackup::backupAtRiskPath(cardDb, backupRoot.path(), &backupLocation)); - EXPECT_TRUE(QFile::exists(backupLocation)); - EXPECT_TRUE(backupLocation.endsWith("cards.xml")); + QStringList locations; + EXPECT_TRUE(UserDataBackup::backupAtRiskPaths({cardDb}, backupRoot.path(), &locations, nullptr)); + ASSERT_EQ(locations.size(), 1); + EXPECT_TRUE(QFile::exists(locations.first())); + EXPECT_TRUE(locations.first().endsWith("cards.xml")); } TEST(UserDataBackup, BackupMissingSourceFails) { QTemporaryDir backupRoot; ASSERT_TRUE(backupRoot.isValid()); - EXPECT_FALSE(UserDataBackup::backupAtRiskPath("/nonexistent-cockatrice-path-xyz", backupRoot.path(), nullptr)); + QStringList locations; + QString sessionDir; + EXPECT_FALSE(UserDataBackup::backupAtRiskPaths({QDir(backupRoot.path()).absoluteFilePath("does-not-exist.cod")}, + backupRoot.path(), &locations, &sessionDir)); + EXPECT_TRUE(locations.isEmpty()); + EXPECT_TRUE(sessionDir.isEmpty()); } TEST(UserDataBackup, BackupPathsShareSingleSessionDir) @@ -241,7 +249,7 @@ TEST(UserDataBackup, ListSessionsFindsRecentAndPrunesOld) // Newest first EXPECT_EQ(sessions.first(), secondSession); - const QString recent = UserDataBackup::findRecentBackupSession(backupRoot.path(), 24); + const QString recent = UserDataBackup::findRecentUsableBackupSession(backupRoot.path(), {sourceDir.path()}, 24); EXPECT_EQ(recent, secondSession); const QStringList reusedLocations = UserDataBackup::listSessionBackupLocations(recent); @@ -262,7 +270,10 @@ TEST(UserDataBackup, FindRecentReturnsEmptyWhenNone) QTemporaryDir backupRoot; ASSERT_TRUE(backupRoot.isValid()); EXPECT_TRUE(UserDataBackup::listBackupSessions(backupRoot.path()).isEmpty()); - EXPECT_TRUE(UserDataBackup::findRecentBackupSession(backupRoot.path(), 24).isEmpty()); + EXPECT_TRUE(UserDataBackup::findRecentUsableBackupSession(backupRoot.path(), {backupRoot.path() + "/missing"}, 24) + .isEmpty()); + EXPECT_TRUE(UserDataBackup::findRecentCoveringBackupSession(backupRoot.path(), {backupRoot.path() + "/missing"}, 24) + .isEmpty()); EXPECT_TRUE(UserDataBackup::listSessionBackupLocations(backupRoot.path() + "/missing").isEmpty()); EXPECT_FALSE(UserDataBackup::isBackupSessionComplete(backupRoot.path() + "/missing")); // Pruning an empty root must not fail @@ -292,7 +303,10 @@ TEST(UserDataBackup, EmptySessionsAreSkippedAndPruned) // An empty session from a failed attempt must never block a retry EXPECT_TRUE(UserDataBackup::listSessionBackupLocations(emptySession).isEmpty()); EXPECT_FALSE(UserDataBackup::isBackupSessionComplete(emptySession)); - EXPECT_TRUE(UserDataBackup::findRecentBackupSession(backupRoot.path(), 24).isEmpty()); + EXPECT_TRUE(UserDataBackup::findRecentUsableBackupSession(backupRoot.path(), {backupRoot.path() + "/missing"}, 24) + .isEmpty()); + EXPECT_TRUE(UserDataBackup::findRecentCoveringBackupSession(backupRoot.path(), {backupRoot.path() + "/missing"}, 24) + .isEmpty()); UserDataBackup::pruneOldBackupSessions(backupRoot.path(), 5); EXPECT_FALSE(QDir(emptySession).exists()); } @@ -321,7 +335,7 @@ TEST(UserDataBackup, SuccessfulBackupIsMarkedComplete) } } -TEST(UserDataBackup, IncompleteMarkerReportsIncompleteButRemainsReusable) +TEST(UserDataBackup, IncompleteMarkerIsNotUsableButRemainsCovering) { QTemporaryDir sourceDir; QTemporaryDir backupRoot; @@ -342,11 +356,13 @@ TEST(UserDataBackup, IncompleteMarkerReportsIncompleteButRemainsReusable) marker.close(); EXPECT_FALSE(UserDataBackup::isBackupSessionComplete(sessionDir)); - // Still non-empty, so throttling reuses it and the caller warns instead of copying again - EXPECT_EQ(UserDataBackup::findRecentBackupSession(backupRoot.path(), 24), sessionDir); + // Incomplete sessions are skipped by the usable search but still found by the + // covering search, so a failed copy falls back to the salvaged content. + EXPECT_TRUE(UserDataBackup::findRecentUsableBackupSession(backupRoot.path(), {sourceDir.path()}, 24).isEmpty()); + EXPECT_EQ(UserDataBackup::findRecentCoveringBackupSession(backupRoot.path(), {sourceDir.path()}, 24), sessionDir); } -TEST(UserDataBackup, LegacySessionWithoutMarkerCountsAsComplete) +TEST(UserDataBackup, SessionWithoutMarkerCountsAsIncomplete) { QTemporaryDir backupRoot; ASSERT_TRUE(backupRoot.isValid()); @@ -357,8 +373,12 @@ TEST(UserDataBackup, LegacySessionWithoutMarkerCountsAsComplete) ASSERT_TRUE(deck.open(QIODevice::WriteOnly | QIODevice::Text)); deck.write("\n"); deck.close(); - EXPECT_TRUE(UserDataBackup::isBackupSessionComplete(sessionDir)); - EXPECT_EQ(UserDataBackup::findRecentBackupSession(backupRoot.path(), 24), sessionDir); + EXPECT_FALSE(UserDataBackup::isBackupSessionComplete(sessionDir)); + // No source manifest either, so neither search reuses it. + EXPECT_TRUE(UserDataBackup::findRecentUsableBackupSession(backupRoot.path(), {backupRoot.path() + "/missing"}, 24) + .isEmpty()); + EXPECT_TRUE(UserDataBackup::findRecentCoveringBackupSession(backupRoot.path(), {backupRoot.path() + "/missing"}, 24) + .isEmpty()); } TEST(UserDataBackup, FindRecentIgnoresExpiredSessions) @@ -375,7 +395,8 @@ TEST(UserDataBackup, FindRecentIgnoresExpiredSessions) QStringList locations; QString sessionDir; ASSERT_TRUE(UserDataBackup::backupAtRiskPaths({sourceDir.path()}, backupRoot.path(), &locations, &sessionDir)); - EXPECT_TRUE(UserDataBackup::findRecentBackupSession(backupRoot.path(), 0).isEmpty()); + EXPECT_TRUE(UserDataBackup::findRecentUsableBackupSession(backupRoot.path(), {sourceDir.path()}, 0).isEmpty()); + EXPECT_TRUE(UserDataBackup::findRecentCoveringBackupSession(backupRoot.path(), {sourceDir.path()}, 0).isEmpty()); } TEST(UserDataBackup, EstimateBackupSize) @@ -414,7 +435,7 @@ TEST(UserDataBackup, SessionCoversSourcesRequiresManifestMatch) EXPECT_FALSE(UserDataBackup::sessionCoversSources(sessionDir, {sourceB.path()})); } -TEST(UserDataBackup, LegacySessionWithoutManifestIsNotCovering) +TEST(UserDataBackup, SessionWithoutManifestIsNotCovering) { QTemporaryDir backupRoot; ASSERT_TRUE(backupRoot.isValid()); @@ -539,14 +560,16 @@ TEST(UserDataBackup, CopyDirectoryDoesNotSilentlyEmptyDirSymlink) } } -TEST(UserDataBackup, SingularBackupClearsOutParamOnTotalFailure) +TEST(UserDataBackup, PluralBackupClearsOutParamsOnTotalFailure) { QTemporaryDir backupRoot; ASSERT_TRUE(backupRoot.isValid()); - QString location = QStringLiteral("stale"); + QStringList locations = {QStringLiteral("stale")}; + QString sessionDir = QStringLiteral("stale"); const QString missing = QDir(backupRoot.path()).absoluteFilePath("does-not-exist.cod"); - EXPECT_FALSE(UserDataBackup::backupAtRiskPath(missing, backupRoot.path(), &location)); - EXPECT_TRUE(location.isEmpty()); + EXPECT_FALSE(UserDataBackup::backupAtRiskPaths({missing}, backupRoot.path(), &locations, &sessionDir)); + EXPECT_TRUE(locations.isEmpty()); + EXPECT_TRUE(sessionDir.isEmpty()); } } // namespace