diff --git a/cmake/NSIS.template.in b/cmake/NSIS.template.in index 956b3349a..a23d733e8 100644 --- a/cmake/NSIS.template.in +++ b/cmake/NSIS.template.in @@ -22,6 +22,8 @@ Var LooseDecksBackupDone Var LooseDeckNoticeShown Var StaleFindHandle Var StaleFindName +Var DataBackupFindHandle +Var DataBackupFindName Var IsUpgrade Var AbortReason Var WaitPid @@ -399,10 +401,28 @@ ${AndIf} ${FileExists} "$OldInstallDir\data" !insertmacro CockLog "pre-uninstall backup: copying $OldInstallDir\data (downloaded pics excluded)" DetailPrint "Backing up portable user data before removing previous version..." CreateDirectory "$DataBackupDir\data" - CopyFiles /SILENT "$OldInstallDir\data\*.*" "$DataBackupDir\data\" - ; Downloaded pictures are large and re-downloadable, so drop them from the - ; safety backup - but keep user-made custom pics (data\pics\CUSTOM). - RMDir /r "$DataBackupDir\data\pics" + ; Copy every top-level entry except pics (downloaded pictures are large and + ; re-downloadable): copying everything first and deleting pics afterwards + ; would duplicate gigabytes into %TEMP% just to throw them away. + FindFirst $DataBackupFindHandle $DataBackupFindName "$OldInstallDir\data\*.*" + ${If} $DataBackupFindHandle != "" + backup_data_loop: + ${If} $DataBackupFindName != "." + ${AndIf} $DataBackupFindName != ".." + ${AndIf} $DataBackupFindName != "pics" + ${If} ${FileExists} "$OldInstallDir\data\$DataBackupFindName\*.*" + CreateDirectory "$DataBackupDir\data\$DataBackupFindName" + CopyFiles /SILENT "$OldInstallDir\data\$DataBackupFindName\*.*" "$DataBackupDir\data\$DataBackupFindName\" + ${Else} + CopyFiles /SILENT "$OldInstallDir\data\$DataBackupFindName" "$DataBackupDir\data\" + ${EndIf} + ${EndIf} + ClearErrors + FindNext $DataBackupFindHandle $DataBackupFindName + IfErrors 0 backup_data_loop + FindClose $DataBackupFindHandle + ${EndIf} + ; Keep user-made custom pics (data\pics\CUSTOM). ${If} ${FileExists} "$OldInstallDir\data\pics\CUSTOM" CreateDirectory "$DataBackupDir\data\pics\CUSTOM" CopyFiles /SILENT "$OldInstallDir\data\pics\CUSTOM\*.*" "$DataBackupDir\data\pics\CUSTOM\" diff --git a/cockatrice/src/interface/window_main.cpp b/cockatrice/src/interface/window_main.cpp index 53b4fb35d..1b0e2b779 100644 --- a/cockatrice/src/interface/window_main.cpp +++ b/cockatrice/src/interface/window_main.cpp @@ -684,6 +684,29 @@ void MainWindow::warnAboutDataPathsInProgramDir() offenders.clear(); } else { offenders = 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) { + const QString suffix = QFileInfo(fileName).suffix().toLower(); + if (suffix != QStringLiteral("cod") && suffix != QStringLiteral("dec") && suffix != QStringLiteral("dek") && + suffix != QStringLiteral("mwdeck")) { + continue; + } + const QString looseDeck = QDir::cleanPath(appDirHandle.absoluteFilePath(fileName)); + bool alreadyCovered = false; + for (const QString &existing : offenders) { + if (PathsSettings::isInsideDir(looseDeck, existing)) { + alreadyCovered = true; + break; + } + } + if (!alreadyCovered) { + offenders.append(looseDeck); + } + } if (offenders.isEmpty()) { return; } @@ -717,21 +740,12 @@ void MainWindow::warnAboutDataPathsInProgramDir() bool backupReused = false; bool backupSkippedDueToSize = false; qint64 backupBytes = 0; - const QString recentSession = UserDataBackup::findRecentBackupSession(backupRoot, 24); - QStringList recentLocations; - bool recentUsable = false; - if (!recentSession.isEmpty()) { - recentLocations = UserDataBackup::listSessionBackupLocations(recentSession); - // An empty source set must never reuse a session: sessionCoversSources is - // vacuously true for it, which would claim an unrelated recent backup - // covers paths that were never backed up. - recentUsable = !sourcesToBackUp.isEmpty() && !recentLocations.isEmpty() && - UserDataBackup::isBackupSessionComplete(recentSession) && - UserDataBackup::sessionCoversSources(recentSession, sourcesToBackUp); - } - if (recentUsable) { - sessionDir = recentSession; - backupLocations = recentLocations; + // Search all recent sessions for a complete, covering one: a partial failure must not + // pin the newest slot and force a full re-copy on every later launch. + const QString reusableSession = UserDataBackup::findRecentUsableBackupSession(backupRoot, sourcesToBackUp, 24); + if (!reusableSession.isEmpty()) { + sessionDir = reusableSession; + backupLocations = UserDataBackup::listSessionBackupLocations(reusableSession); backupComplete = true; backupReused = true; qCInfo(MainWindowLog) << "Reusing recent at-risk user data backup at" << sessionDir; @@ -753,13 +767,14 @@ void MainWindow::warnAboutDataPathsInProgramDir() backupComplete = UserDataBackup::backupAtRiskPaths(sourcesToBackUp, backupRoot, &backupLocations, &sessionDir); UserDataBackup::pruneOldBackupSessions(backupRoot, 5); - if (backupLocations.isEmpty() && !recentLocations.isEmpty()) { - // Fresh copy failed entirely: fall back to the recent session so the - // dialog still points at something, flagged incomplete below - but - // only when it actually covers the current sources. - if (!recentSession.isEmpty() && UserDataBackup::sessionCoversSources(recentSession, sourcesToBackUp)) { - sessionDir = recentSession; - backupLocations = recentLocations; + if (backupLocations.isEmpty()) { + // Fresh copy failed entirely: fall back to the newest covering session so the + // dialog still points at something, flagged incomplete below. + const QString fallbackSession = + UserDataBackup::findRecentCoveringBackupSession(backupRoot, sourcesToBackUp, 24); + if (!fallbackSession.isEmpty()) { + sessionDir = fallbackSession; + backupLocations = UserDataBackup::listSessionBackupLocations(fallbackSession); backupComplete = false; } } @@ -823,6 +838,18 @@ void MainWindow::warnAboutDataPathsInProgramDir() if (!backupComplete) { message += tr("\n\nThe backup may be incomplete; please back up these folders manually as well."); } + bool hasLooseDecks = false; + for (const QString &offender : offenders) { + const QString suffix = QFileInfo(offender).suffix().toLower(); + if (suffix == QStringLiteral("cod") || suffix == QStringLiteral("dec") || + suffix == QStringLiteral("dek") || suffix == QStringLiteral("mwdeck")) { + hasLooseDecks = true; + break; + } + } + if (hasLooseDecks) { + message += tr("\n\nDeck files ending in .txt are not backed up - move those manually."); + } } else if (backupSkippedDueToSize) { message += tr("\n\nThe automatic safety backup was skipped because of its size: %1. " "Please back up these folders manually.") diff --git a/libcockatrice_utility/libcockatrice/utility/user_data_backup.cpp b/libcockatrice_utility/libcockatrice/utility/user_data_backup.cpp index 4885fb4fc..476a9c788 100644 --- a/libcockatrice_utility/libcockatrice/utility/user_data_backup.cpp +++ b/libcockatrice_utility/libcockatrice/utility/user_data_backup.cpp @@ -80,7 +80,19 @@ bool copyDirectoryRecursively(const QString &sourceDir, const QString &destDir) it.next(); QString entryRel = source.relativeFilePath(it.filePath()); QString target = QDir(destDir).absoluteFilePath(entryRel); - if (it.fileInfo().isDir()) { + if (it.fileInfo().isSymLink()) { + // Copy the link itself best-effort without dereferencing it. QDirIterator does not + // follow symlinks here, so a symlink-to-directory must not take the isDir() branch + // below (which would create an empty dir and silently drop the contents). + QDir().mkpath(QFileInfo(target).absolutePath()); + if (QFile::exists(target) && !QFile::remove(target)) { + ok = false; + continue; + } + if (!QFile::copy(it.filePath(), target)) { + ok = false; + } + } else if (it.fileInfo().isDir()) { if (!QDir().mkpath(target)) { ok = false; } @@ -284,10 +296,10 @@ QString findRecentBackupSession(const QString &backupRoot, int maxAgeHours) if (!info.exists()) { continue; } - // Sessions are newest-first, so once an entry is too old every later - // one is older as well and the search can stop. + // 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) { - break; + continue; } // Skip empty sessions from failed attempts: they must never block a retry. if (listSessionBackupLocations(session).isEmpty()) { @@ -298,6 +310,58 @@ QString findRecentBackupSession(const QString &backupRoot, int maxAgeHours) return {}; } +QString findRecentCoveringBackupSession(const QString &backupRoot, const QStringList &sourcePaths, 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; + } + if (info.lastModified().secsTo(now) >= static_cast(maxAgeHours) * 3600) { + continue; + } + if (listSessionBackupLocations(session).isEmpty()) { + continue; + } + if (!sessionCoversSources(session, sourcePaths)) { + continue; + } + return session; + } + return {}; +} + +QString findRecentUsableBackupSession(const QString &backupRoot, const QStringList &sourcePaths, int maxAgeHours) +{ + if (sourcePaths.isEmpty()) { + return {}; + } + const QStringList sessions = listBackupSessions(backupRoot); + const QDateTime now = QDateTime::currentDateTime(); + for (const QString &session : sessions) { + const QFileInfo info(session); + if (!info.exists()) { + continue; + } + if (info.lastModified().secsTo(now) >= static_cast(maxAgeHours) * 3600) { + continue; + } + if (listSessionBackupLocations(session).isEmpty()) { + continue; + } + if (!isBackupSessionComplete(session)) { + continue; + } + if (!sessionCoversSources(session, sourcePaths)) { + continue; + } + return session; + } + return {}; +} + bool isBackupSessionComplete(const QString &sessionDir) { const QDir session(sessionDir); diff --git a/libcockatrice_utility/libcockatrice/utility/user_data_backup.h b/libcockatrice_utility/libcockatrice/utility/user_data_backup.h index ed3fda7d1..c40b8b0e3 100644 --- a/libcockatrice_utility/libcockatrice/utility/user_data_backup.h +++ b/libcockatrice_utility/libcockatrice/utility/user_data_backup.h @@ -28,7 +28,8 @@ constexpr qint64 MAX_AUTO_BACKUP_BYTES = 500LL * 1024 * 1024; /** * @brief Recursively copies @p sourceDir into @p destDir, creating it as needed. * @returns true when every file was copied (or there was nothing to copy). - * Symlinks are copied best-effort via QFile::copy; a dangling link fails that entry only. + * Symlinks (file or directory) are copied best-effort via QFile::copy without + * dereferencing them; a dangling link fails that entry only. */ bool copyDirectoryRecursively(const QString &sourceDir, const QString &destDir); @@ -96,11 +97,30 @@ QStringList listBackupSessions(const QString &backupRoot); * * 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, - * and an expired newest entry stops the search since sessions are newest-first. + * 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. + */ +QString findRecentUsableBackupSession(const QString &backupRoot, const QStringList &sourcePaths, int maxAgeHours = 24); + +/** + * @brief Returns the newest non-empty session covering @p sourcePaths, regardless of completeness. + * + * Used as a fallback pointer when a fresh copy fails entirely, so the dialog can still + * point at salvaged content. Completeness is left to the caller to report. + */ +QString +findRecentCoveringBackupSession(const QString &backupRoot, const QStringList &sourcePaths, int maxAgeHours = 24); + /** * @brief Reports whether @p sessionDir holds a complete backup. * @returns true when the session carries the complete marker, or when it predates diff --git a/tests/user_data_backup_test.cpp b/tests/user_data_backup_test.cpp index 192a95e28..cfb1b0a73 100644 --- a/tests/user_data_backup_test.cpp +++ b/tests/user_data_backup_test.cpp @@ -447,6 +447,98 @@ TEST(UserDataBackup, ManifestIsHiddenFromSessionLocations) } } +TEST(UserDataBackup, FindRecentUsableSkipsIncompleteNewest) +{ + QTemporaryDir sourceDir; + QTemporaryDir backupRoot; + ASSERT_TRUE(sourceDir.isValid()); + ASSERT_TRUE(backupRoot.isValid()); + QFile deck(QDir(sourceDir.path()).absoluteFilePath("mydeck.cod")); + ASSERT_TRUE(deck.open(QIODevice::WriteOnly | QIODevice::Text)); + deck.write("\n"); + deck.close(); + + QStringList locations; + QString oldSession; + QString newSession; + ASSERT_TRUE(UserDataBackup::backupAtRiskPaths({sourceDir.path()}, backupRoot.path(), &locations, &oldSession)); + ASSERT_TRUE(UserDataBackup::backupAtRiskPaths({sourceDir.path()}, backupRoot.path(), &locations, &newSession)); + ASSERT_NE(oldSession, newSession); + + // Newest session becomes incomplete: usable search must fall back to the older complete one. + ASSERT_TRUE(QFile::remove(QDir(newSession).absoluteFilePath("backup.complete"))); + QFile marker(QDir(newSession).absoluteFilePath("backup.incomplete")); + ASSERT_TRUE(marker.open(QIODevice::WriteOnly | QIODevice::Text)); + marker.write("incomplete\n"); + marker.close(); + + EXPECT_EQ(UserDataBackup::findRecentUsableBackupSession(backupRoot.path(), {sourceDir.path()}), oldSession); + // Covering search still finds the newest session even though it is incomplete. + EXPECT_EQ(UserDataBackup::findRecentCoveringBackupSession(backupRoot.path(), {sourceDir.path()}), newSession); +} + +TEST(UserDataBackup, FindRecentUsableRequiresCoveringSet) +{ + QTemporaryDir sourceA; + QTemporaryDir sourceB; + QTemporaryDir backupRoot; + ASSERT_TRUE(sourceA.isValid()); + ASSERT_TRUE(sourceB.isValid()); + ASSERT_TRUE(backupRoot.isValid()); + QFile fa(QDir(sourceA.path()).absoluteFilePath("a.cod")); + ASSERT_TRUE(fa.open(QIODevice::WriteOnly | QIODevice::Text)); + fa.write("a"); + fa.close(); + QFile fb(QDir(sourceB.path()).absoluteFilePath("b.cod")); + ASSERT_TRUE(fb.open(QIODevice::WriteOnly | QIODevice::Text)); + fb.write("b"); + fb.close(); + + QStringList locations; + QString oldSession; + QString newSession; + ASSERT_TRUE(UserDataBackup::backupAtRiskPaths({sourceA.path()}, backupRoot.path(), &locations, &oldSession)); + ASSERT_TRUE(UserDataBackup::backupAtRiskPaths({sourceB.path()}, backupRoot.path(), &locations, &newSession)); + + // Newest covers B only, so a query for A must reuse the older session. + EXPECT_EQ(UserDataBackup::findRecentUsableBackupSession(backupRoot.path(), {sourceA.path()}), oldSession); + EXPECT_EQ(UserDataBackup::findRecentUsableBackupSession(backupRoot.path(), {sourceB.path()}), newSession); + EXPECT_TRUE( + UserDataBackup::findRecentUsableBackupSession(backupRoot.path(), {sourceA.path(), sourceB.path()}).isEmpty()); + EXPECT_TRUE(UserDataBackup::findRecentUsableBackupSession(backupRoot.path(), {}).isEmpty()); +} + +TEST(UserDataBackup, CopyDirectoryDoesNotSilentlyEmptyDirSymlink) +{ + QTemporaryDir sourceDir; + QTemporaryDir destParent; + ASSERT_TRUE(sourceDir.isValid()); + ASSERT_TRUE(destParent.isValid()); + QDir source(sourceDir.path()); + ASSERT_TRUE(source.mkpath("real")); + QFile inner(source.absoluteFilePath("real/card.cod")); + ASSERT_TRUE(inner.open(QIODevice::WriteOnly | QIODevice::Text)); + inner.write("x"); + inner.close(); + if (!QFile::link(source.absoluteFilePath("real"), source.absoluteFilePath("link"))) { + GTEST_SKIP() << "Symlinks not permitted in this environment"; + } + + QString dest = QDir(destParent.path()).absoluteFilePath("copy"); + const bool ok = UserDataBackup::copyDirectoryRecursively(sourceDir.path(), dest); + const QFileInfo linkCopy(QDir(dest).absoluteFilePath("link")); + // Old behavior created an empty plain dir and reported success, silently dropping contents. + // New behavior copies the link best-effort; if that fails it must report failure. + if (ok) { + EXPECT_TRUE(linkCopy.exists() || linkCopy.isSymLink()); + if (linkCopy.isDir() && !linkCopy.isSymLink()) { + EXPECT_FALSE(QDir(linkCopy.absoluteFilePath()).isEmpty()); + } + } else { + EXPECT_TRUE(linkCopy.isSymLink() || !linkCopy.exists() || !QDir(linkCopy.absoluteFilePath()).isEmpty()); + } +} + TEST(UserDataBackup, SingularBackupClearsOutParamOnTotalFailure) { QTemporaryDir backupRoot;