Conversation
There was a problem hiding this comment.
Pull request overview
Updates the Windows Inno Setup installer to improve cleanup behavior during install/upgrade, addressing stale uninstall entries and pre-install service handling (issue #3103).
Changes:
- Enable automatic closing of running applications during setup (
CloseApplications=yes,RestartApplications=no). - Move service stop/delete logic from
[Run]into PascalScript (CurStepChanged) and wait for the service to be fully deleted before continuing. - Add startup cleanup to remove stale uninstall registry entries whose uninstaller executable is missing.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function ExtractExecutablePath(const CommandLine: String): String; | ||
| var | ||
| S: String; | ||
| EndQuote: Integer; | ||
| FirstSpace: Integer; | ||
| begin | ||
| S := Trim(CommandLine); | ||
| if S = '' then begin | ||
| Result := ''; | ||
| exit; | ||
| end; | ||
|
|
||
| if S[1] = '"' then begin | ||
| Delete(S, 1, 1); | ||
| EndQuote := Pos('"', S); | ||
| if EndQuote > 0 then begin | ||
| Result := Copy(S, 1, EndQuote - 1); | ||
| end else begin | ||
| Result := S; | ||
| end; | ||
| exit; | ||
| end; | ||
|
|
||
| FirstSpace := Pos(' ', S); | ||
| if FirstSpace > 0 then begin | ||
| Result := Copy(S, 1, FirstSpace - 1); | ||
| end else begin | ||
| Result := S; | ||
| end; |
There was a problem hiding this comment.
ExtractExecutablePath will incorrectly parse an unquoted executable path that contains spaces (e.g., C:\Program Files\...\unins*.exe /SILENT), returning C:\Program and causing RemoveStaleUninstallEntry to delete a valid uninstall registry key. Consider extracting up to the first .exe (or using a more robust command-line parser) before deciding the entry is stale.
| ServiceStopTimeoutMs = 20000; | ||
| ServicePollIntervalMs = 250; | ||
| UninstallRegSubKey = 'Software\Microsoft\Windows\CurrentVersion\Uninstall\{#SetupSetting("AppId")}_is1'; |
There was a problem hiding this comment.
ServiceStopTimeoutMs is used as the timeout for waiting on service deletion (WaitForServiceDelete), not for waiting on the service to stop. Renaming this constant (or splitting stop vs delete timeouts) would make the intent clearer and reduce confusion for future changes.
Closes https://github.com/getlantern/engineering/issues/3103