added a patch to backup settings files in Windows Firmware Installer#330
added a patch to backup settings files in Windows Firmware Installer#330
Conversation
WalkthroughAdds a comprehensive settings backup and cleanup system to the Inno Setup script and bumps the project version. New global variables (BackedUpFilesCount, BackupTimestamp), functions and procedures manage locating, timestamping, backing up (JSON/XML/TTF), and cleaning up backups; backups are stored in timestamped folders. PrepareToInstall now calls BackupAllSettingsFiles before MSI uninstall logic; CurStepChanged performs post-install cleanup and user notification. Adds UninstallDelete entries and updates [Files] to include data/*.ttf and backup-ready placeholders. Updates version constant to "1.14.5". Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@EmotiBitInstaller/EmotiBitInstaller.iss`:
- Around line 86-96: The SETTINGS_FILE_COUNT constant is a fragile manual count
that can fall out of sync with the number of cases in GetSettingsFileInfo; add a
defensive runtime assertion and/or an inline comment to prevent mismatch: update
GetSettingsFileInfo to include a comment referencing SETTINGS_FILE_COUNT and add
a validation (e.g., compare the number of cases returned or the length of the
settings list against SETTINGS_FILE_COUNT) that logs or raises an error if they
differ so BackedUpFilesCount/backup cleanup logic cannot silently skip files.
- Around line 183-194: GetSettingsFileInfo currently lacks a default branch so
an out-of-range Index can leave FileName and DestDir uninitialized; update the
GetSettingsFileInfo procedure to add a default case that assigns safe fallback
values (e.g., empty FileName and a safe DestDir or a clearly named error file)
and optionally logs or raises an error/exception to fail fast when Index is
invalid, ensuring callers (backup/cleanup) never receive uninitialized strings.
…o adds a patch to remove all emotibit directories on uninstall (was an issue if upgrading from MSI to INNO installer)
…ases extensibility. tested working.
nitin710
temporarily deployed
to
RUNNER-WINDOWS
GitHub Actions
Inactive
nitin710
temporarily deployed
to
RUNNER-WINDOWS
GitHub Actions
Inactive
nitin710
temporarily deployed
to
RUNNER-WINDOWS
GitHub Actions
Inactive
nitin710
temporarily deployed
to
RUNNER-WINDOWS
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@EmotiBitInstaller/EmotiBitInstaller.iss`:
- Around line 227-250: Backup currently uses RenameFile in BackupFile (called
from BackupAllSettingsFiles during PrepareToInstall) which moves originals
before the user-facing MSI uninstall prompt; change behavior to preserve
originals by replacing RenameFile with FileCopy in BackupFile so backups are
copies, and/or move the call to BackupAllSettingsFiles out of PrepareToInstall
into CurStepChanged handling for ssInstall so backups occur after cancelable
prompts; update logging to reflect copy vs move and ensure BackedUpFilesCount
increments only on successful FileCopy.
| procedure BackupFile(FilePath: String); | ||
| var | ||
| BackupPath, BackupDir: String; | ||
| begin | ||
| if not FileExists(FilePath) then | ||
| begin | ||
| Log('No file to backup: ' + FilePath); | ||
| Exit; | ||
| end; | ||
|
|
||
| BackupPath := GetBackupFilePath(FilePath); | ||
| BackupDir := ExtractFilePath(BackupPath); | ||
|
|
||
| if not DirExists(BackupDir) then | ||
| ForceDirectories(BackupDir); | ||
|
|
||
| if RenameFile(FilePath, BackupPath) then | ||
| begin | ||
| Log('Backed up: ' + FilePath + ' -> ' + BackupPath); | ||
| BackedUpFilesCount := BackedUpFilesCount + 1; | ||
| end | ||
| else | ||
| Log('Warning: Failed to backup ' + FilePath); | ||
| end; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp && find . -name "*.iss" -type f 2>/dev/null | head -20Repository: EmotiBit/ofxEmotiBit
Length of output: 46
🏁 Script executed:
git ls-files | grep -i "emotibits" | head -20Repository: EmotiBit/ofxEmotiBit
Length of output: 46
🏁 Script executed:
git ls-files | grep -i "\.iss$"Repository: EmotiBit/ofxEmotiBit
Length of output: 103
🏁 Script executed:
wc -l EmotiBitInstaller/EmotiBitInstaller.issRepository: EmotiBit/ofxEmotiBit
Length of output: 107
🏁 Script executed:
sed -n '227,250p' EmotiBitInstaller/EmotiBitInstaller.issRepository: EmotiBit/ofxEmotiBit
Length of output: 635
🏁 Script executed:
sed -n '399,404p' EmotiBitInstaller/EmotiBitInstaller.issRepository: EmotiBit/ofxEmotiBit
Length of output: 356
🏁 Script executed:
grep -n "PrepareToInstall" EmotiBitInstaller/EmotiBitInstaller.issRepository: EmotiBit/ofxEmotiBit
Length of output: 129
🏁 Script executed:
grep -n "BackupAllSettingsFiles\|RestoreSettings\|CancelledInstallation" EmotiBitInstaller/EmotiBitInstaller.issRepository: EmotiBit/ofxEmotiBit
Length of output: 133
🏁 Script executed:
sed -n '391,410p' EmotiBitInstaller/EmotiBitInstaller.issRepository: EmotiBit/ofxEmotiBit
Length of output: 705
🏁 Script executed:
sed -n '284,310p' EmotiBitInstaller/EmotiBitInstaller.issRepository: EmotiBit/ofxEmotiBit
Length of output: 850
🏁 Script executed:
grep -n "CurStepChanged\|function.*Cancel\|procedure.*Abort\|procedure.*Restore" EmotiBitInstaller/EmotiBitInstaller.issRepository: EmotiBit/ofxEmotiBit
Length of output: 189
🏁 Script executed:
grep -n "ssInstall\|ssFinished\|Result :=" EmotiBitInstaller/EmotiBitInstaller.iss | head -20Repository: EmotiBit/ofxEmotiBit
Length of output: 766
🏁 Script executed:
sed -n '371,390p' EmotiBitInstaller/EmotiBitInstaller.issRepository: EmotiBit/ofxEmotiBit
Length of output: 762
🏁 Script executed:
sed -n '371,550p' EmotiBitInstaller/EmotiBitInstaller.iss | grep -A 30 "CurStepChanged"Repository: EmotiBit/ofxEmotiBit
Length of output: 2150
🏁 Script executed:
grep -n "RenameFile\|FileCopy\|DeleteFile\|MoveFile" EmotiBitInstaller/EmotiBitInstaller.issRepository: EmotiBit/ofxEmotiBit
Length of output: 199
🏁 Script executed:
grep -n "procedure.*Restore\|function.*Restore\|RestoreFile\|RestoreSettings" EmotiBitInstaller/EmotiBitInstaller.issRepository: EmotiBit/ofxEmotiBit
Length of output: 46
🌐 Web query:
Inno Setup RenameFile vs FileCopy behavior and differences
💡 Result:
RenameFile (move/rename)
- What it does: Renames (or moves) an existing file or directory from
OldNametoNewName. (documentation.help) - If destination exists: Fails (returns
False) ifNewNamealready exists. (documentation.help) - Cross-drive behavior: Works like a filesystem rename/move, so it’s not suitable for moving across different drives/volumes (typical “move to another drive” needs copy+delete). (stackoverflow.com)
- Practical implications: Usually fast/atomic on the same volume (metadata change, no data duplication). But it will fail if the source is in use/locked, or if permissions block the operation (same as any OS-level rename).
FileCopy (copy)
- What it does: Copies
ExistingFiletoNewFileand preserves timestamp and file attributes. (documentation.help) - If destination exists: Controlled by
FailIfExists:True→ fails ifNewFileexistsFalse→ overwritesNewFile(documentation.help)
- Directories: It’s a single-file operation; it does not create the destination folder (so the target path must already exist). (stackoverflow.com)
- Practical implications: Slower and uses extra disk I/O/space (because it duplicates data), but it’s the correct building block for “move across drives” (copy + then
DeleteFile), whichRenameFilecan’t reliably do. (stackoverflow.com)
Rule of thumb
- Use
RenameFilewhen you want a true rename/move on the same volume and you want it to fail if the destination already exists. (documentation.help) - Use
FileCopywhen you want a duplicate, need optional overwrite control, or you may be moving between different drives (copy+delete). (documentation.help)
Citations:
- 1: https://documentation.help/inno-setup/topic_isxfunc_renamefile.htm?utm_source=openai
- 2: https://documentation.help/inno-setup/topic_isxfunc_renamefile.htm?utm_source=openai
- 3: https://stackoverflow.com/questions/33305164/is-it-possible-to-move-existing-directories-files-with-an-inno-script?utm_source=openai
- 4: https://documentation.help/Inno-Setup/topic_isxfunc_filecopy.htm?utm_source=openai
- 5: https://documentation.help/Inno-Setup/topic_isxfunc_filecopy.htm?utm_source=openai
- 6: https://stackoverflow.com/questions/39535942/inno-setup-filecopy-failing?utm_source=openai
- 7: https://stackoverflow.com/questions/33305164/is-it-possible-to-move-existing-directories-files-with-an-inno-script?utm_source=openai
- 8: https://documentation.help/inno-setup/topic_isxfunc_renamefile.htm?utm_source=openai
- 9: https://documentation.help/Inno-Setup/topic_isxfunc_filecopy.htm?utm_source=openai
🌐 Web query:
Inno Setup PrepareToInstall abort cancel hook restore files rollback
💡 Result:
1) “Abort/cancel” from PrepareToInstall (the supported hook)
PrepareToInstall is explicitly meant to stop Setup before the actual install phase starts. To abort, return a non-empty string (shown on the “Preparing to Install” page). If a restart is required, also set NeedsRestart := True. Setup then exits with a dedicated exit code (7/8). [1][2]
function PrepareToInstall(var NeedsRestart: Boolean): String;
begin
if SomePrecheckFailed then
Result := 'Cannot proceed (reason...)'; // abort before installing anything
end;If you need to force a “restart required” outcome:
function PrepareToInstall(var NeedsRestart: Boolean): String;
begin
NeedsRestart := True;
Result := 'A restart is required before updating. Please reboot and try again.';
end;2) “Abort during install” with rollback (only works in specific events)
Abort; only terminates Setup (with normal cleanup/rollback behavior) when called from certain event functions—most importantly CurStepChanged(ssInstall) for aborting during the install phase. [3]
procedure CurStepChanged(CurStep: TSetupStep);
begin
if CurStep = ssInstall then
if MustStopNow then
Abort; // causes Setup to stop and roll back what it can
end;3) About “restore files / rollback overwritten files”
Inno Setup’s “Rolling back changes” is not a full transactional restore of overwritten files in every scenario (especially upgrades + partial file replacement failures). Practically, you should assume you cannot rely on rollback to restore previous versions of files that were already overwritten, so design to avoid starting file replacement unless you’re sure you can finish. This is why the safest pattern is to detect the problem before ssInstall (via PrepareToInstall) and stop early. [1]
For the specific “locked file during silent upgrade” case, one suggested workaround is to detect (e.g., by analyzing your /LOG) that Restart Manager couldn’t avoid a reboot, and then abort at CurStepChanged(ssInstall) before more replacement happens. [1]
Sources:
[1] Stack Overflow discussion of silent update + locked file + rollback limitations/workarounds (stackoverflow.com)
[2] Inno Setup help: PrepareToInstall behavior + dedicated exit codes (jrsoftware.org)
[3] Inno Setup help: Abort only exits Setup when called from specific events (incl. CurStepChanged(ssInstall)) (documentation.help)
Citations:
- 1: https://stackoverflow.com/questions/77067642/how-to-abort-an-installation-when-replacing-files-would-require-a-restart
- 2: https://jrsoftware.org/ishelp/topic_scriptevents.htm?utm_source=openai
- 3: https://documentation.help/Inno-Setup/topic_isxfunc_abort.htm?utm_source=openai
Backup moves files before the cancelable MSI uninstall prompt—if cancelled, the app loses original settings.
RenameFile moves the original settings to a backup folder. BackupAllSettingsFiles() is called at the start of PrepareToInstall, but the user-facing MSI uninstall confirmation prompt (which allows cancellation) happens after. If the user cancels there, files remain in the backup folder and the app loses its original settings files.
Switch to FileCopy to keep original files in place while creating a safety backup, or defer BackupAllSettingsFiles() until after all cancelable prompts by moving it to CurStepChanged(ssInstall).
💡 Safer backup approach (copy instead of move)
- if RenameFile(FilePath, BackupPath) then
+ if FileCopy(FilePath, BackupPath, False) then
begin
Log('Backed up: ' + FilePath + ' -> ' + BackupPath);
BackedUpFilesCount := BackedUpFilesCount + 1;
end📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| procedure BackupFile(FilePath: String); | |
| var | |
| BackupPath, BackupDir: String; | |
| begin | |
| if not FileExists(FilePath) then | |
| begin | |
| Log('No file to backup: ' + FilePath); | |
| Exit; | |
| end; | |
| BackupPath := GetBackupFilePath(FilePath); | |
| BackupDir := ExtractFilePath(BackupPath); | |
| if not DirExists(BackupDir) then | |
| ForceDirectories(BackupDir); | |
| if RenameFile(FilePath, BackupPath) then | |
| begin | |
| Log('Backed up: ' + FilePath + ' -> ' + BackupPath); | |
| BackedUpFilesCount := BackedUpFilesCount + 1; | |
| end | |
| else | |
| Log('Warning: Failed to backup ' + FilePath); | |
| end; | |
| procedure BackupFile(FilePath: String); | |
| var | |
| BackupPath, BackupDir: String; | |
| begin | |
| if not FileExists(FilePath) then | |
| begin | |
| Log('No file to backup: ' + FilePath); | |
| Exit; | |
| end; | |
| BackupPath := GetBackupFilePath(FilePath); | |
| BackupDir := ExtractFilePath(BackupPath); | |
| if not DirExists(BackupDir) then | |
| ForceDirectories(BackupDir); | |
| if FileCopy(FilePath, BackupPath, False) then | |
| begin | |
| Log('Backed up: ' + FilePath + ' -> ' + BackupPath); | |
| BackedUpFilesCount := BackedUpFilesCount + 1; | |
| end | |
| else | |
| Log('Warning: Failed to backup ' + FilePath); | |
| end; |
🤖 Prompt for AI Agents
In `@EmotiBitInstaller/EmotiBitInstaller.iss` around lines 227 - 250, Backup
currently uses RenameFile in BackupFile (called from BackupAllSettingsFiles
during PrepareToInstall) which moves originals before the user-facing MSI
uninstall prompt; change behavior to preserve originals by replacing RenameFile
with FileCopy in BackupFile so backups are copies, and/or move the call to
BackupAllSettingsFiles out of PrepareToInstall into CurStepChanged handling for
ssInstall so backups occur after cancelable prompts; update logging to reflect
copy vs move and ensure BackedUpFilesCount increments only on successful
FileCopy.
1 participant
Description
Handling settings file updates in EmotiBit applicationsin drive for notes.Requirements
Issues Referenced
Documentation update
Testing
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.