[Installer][Client] Fix review findings: stale backup, empty reuse, relocate, portable warning

NSIS: wipe TEMP backup once per run to avoid resurrecting deleted files,
check data dir existence in relocate so dir-only trees are moved.

Client: never reuse a recent session for an empty source set, only fall
back to a covering session on copy failure, warn fresh portable installs
instead of staying silent.
This commit is contained in:
Lukas Brübach 2026-10-13 11:12:11 +02:00
parent 53e8421885
commit 3bde484244
2 changed files with 32 additions and 14 deletions

View file

@ -383,13 +383,15 @@ Function BackupUserDataBeforeUninstall
${If} $OldInstallDir == "" ${If} $OldInstallDir == ""
StrCpy $OldInstallDir $INSTDIR StrCpy $OldInstallDir $INSTDIR
${EndIf} ${EndIf}
; No wipe of $TEMP\CockatriceDataBackup here: %TEMP% survives across installer ; %TEMP% survives across installer runs, so a previous run's safety copy would
; runs, so the previous run's safety copy stays on disk until this run overwrites ; otherwise merge into this run: CopyFiles merges/overwrites, resurrecting files
; it. Stale content can never leak into this run because restore is gated on the ; the user deleted since (e.g. deck A from run 1 reappearing alongside deck B
; per-run $DataBackupDone / $LooseDecksBackupDone flags below, not on what happens ; from run 2) or restoring stale content after a failed copy. Wipe once per
; to be on disk. CopyFiles merges/overwrites, so calling this twice in sequence ; installer run - $DataBackupDir is empty until the first backup of this run,
; (32-bit then 64-bit uninstall) can only add safety, never delete the first ; so the second call in sequence (32-bit then 64-bit uninstall) still merges.
; call's copy. ${If} $DataBackupDir == ""
RMDir /r "$TEMP\CockatriceDataBackup"
${EndIf}
${If} ${FileExists} "$OldInstallDir\portable.dat" ${If} ${FileExists} "$OldInstallDir\portable.dat"
${AndIf} ${FileExists} "$OldInstallDir\data" ${AndIf} ${FileExists} "$OldInstallDir\data"
StrCpy $DataBackupDir "$TEMP\CockatriceDataBackup" StrCpy $DataBackupDir "$TEMP\CockatriceDataBackup"
@ -485,7 +487,7 @@ StrCmp $DataBackupDir "" done
StrCmp $OldInstallDir "" done StrCmp $OldInstallDir "" done
StrCmp $OldInstallDir $INSTDIR done StrCmp $OldInstallDir $INSTDIR done
${If} $DataBackupDone == "1" ${If} $DataBackupDone == "1"
${AndIf} ${FileExists} "$OldInstallDir\data\*.*" ${AndIf} ${FileExists} "$OldInstallDir\data"
!insertmacro CockLog "relocate: copying restored data $OldInstallDir\data -> $INSTDIR\data" !insertmacro CockLog "relocate: copying restored data $OldInstallDir\data -> $INSTDIR\data"
CreateDirectory "$INSTDIR\data" CreateDirectory "$INSTDIR\data"
CopyFiles /SILENT "$OldInstallDir\data\*.*" "$INSTDIR\data\" CopyFiles /SILENT "$OldInstallDir\data\*.*" "$INSTDIR\data\"

View file

@ -674,8 +674,12 @@ void MainWindow::warnAboutDataPathsInProgramDir()
} }
backupSources.append(offender); backupSources.append(offender);
} }
// 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 (backupSources.isEmpty()) { if (backupSources.isEmpty()) {
return; qCInfo(MainWindowLog) << "Portable install with no existing data files, warning without backup";
} }
offenders.clear(); offenders.clear();
} else { } else {
@ -718,7 +722,11 @@ void MainWindow::warnAboutDataPathsInProgramDir()
bool recentUsable = false; bool recentUsable = false;
if (!recentSession.isEmpty()) { if (!recentSession.isEmpty()) {
recentLocations = UserDataBackup::listSessionBackupLocations(recentSession); recentLocations = UserDataBackup::listSessionBackupLocations(recentSession);
recentUsable = !recentLocations.isEmpty() && UserDataBackup::isBackupSessionComplete(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); UserDataBackup::sessionCoversSources(recentSession, sourcesToBackUp);
} }
if (recentUsable) { if (recentUsable) {
@ -747,10 +755,13 @@ void MainWindow::warnAboutDataPathsInProgramDir()
UserDataBackup::pruneOldBackupSessions(backupRoot, 5); UserDataBackup::pruneOldBackupSessions(backupRoot, 5);
if (backupLocations.isEmpty() && !recentLocations.isEmpty()) { if (backupLocations.isEmpty() && !recentLocations.isEmpty()) {
// Fresh copy failed entirely: fall back to the recent session so the // Fresh copy failed entirely: fall back to the recent session so the
// dialog still points at something, flagged incomplete below. // dialog still points at something, flagged incomplete below - but
sessionDir = recentSession; // only when it actually covers the current sources.
backupLocations = recentLocations; if (!recentSession.isEmpty() && UserDataBackup::sessionCoversSources(recentSession, sourcesToBackUp)) {
backupComplete = false; sessionDir = recentSession;
backupLocations = recentLocations;
backupComplete = false;
}
} }
} }
} }
@ -788,6 +799,11 @@ void MainWindow::warnAboutDataPathsInProgramDir()
"before uninstalling or updating.\n\nReplays and downloaded pictures would not have been " "before uninstalling or updating.\n\nReplays and downloaded pictures would not have been "
"included in that backup either.") "included in that backup either.")
.arg(sizeInfo); .arg(sizeInfo);
} else if (sourcesToBackUp.isEmpty()) {
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\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.");
} else { } else {
message = tr("This portable install keeps its data inside the program directory, where the " 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 " "uninstallers shipped in Cockatrice 3.1.0/3.1.1 delete everything on uninstall or "