diff --git a/.ci/compile.sh b/.ci/compile.sh index 569ae9f6b..b71b45728 100755 --- a/.ci/compile.sh +++ b/.ci/compile.sh @@ -191,7 +191,14 @@ if [[ $USE_CCACHE ]]; then fi fi if [[ $PACKAGE_TYPE ]]; then - flags+=("-DCPACK_GENERATOR=$PACKAGE_TYPE") + if [[ $PACKAGE_TYPE == NSIS || $PACKAGE_TYPE == InnoSetup ]]; then + # The Windows installer is chosen by a CMake option, and that option then adds its CPack + # generator itself. Passing CPACK_GENERATOR as well would list both installers and build + # both in one run. + flags+=("-DCOCKATRICE_WIN_INSTALLER=$PACKAGE_TYPE") + else + flags+=("-DCPACK_GENERATOR=$PACKAGE_TYPE") + fi fi if [[ $USE_VCPKG ]]; then flags+=("-DUSE_VCPKG=1") @@ -741,20 +748,22 @@ if [[ $MAKE_PACKAGE ]]; then fi # 7-Zip writes CRLF on Windows, so the \r has to come off before any # whole-line comparison below. Normalize separators to /, and strip the - # install-root prefixes an NSIS listing may carry, so the entries can be - # compared as exact relative paths. -xF keeps a '.' in a DLL name from - # being a wildcard; -i because that is how the installed tree behaves - - # the uninstaller clears both $INSTDIR\plugins and $INSTDIR\Plugins. - # The archive side and the manifest side have to agree on line endings. - # CMake writes required-runtime.txt with the platform's line ending and - # 7-Zip writes CRLF on Windows, so a trailing \r survives on $entry and - # defeats the whole-line match - every entry then reads as missing even - # though the listing holds it verbatim. + # install-root prefixes a listing may carry - $INSTDIR/$OUTDIR/$PLUGINSDIR + # from NSIS, {app} from Inno Setup - so the entries can be compared as exact + # relative paths. -xF keeps a '.' in a DLL name from being a wildcard; -i + # because that is how the installed tree behaves - the uninstaller clears + # both $INSTDIR\plugins and $INSTDIR\Plugins. The archive side and the + # manifest side have to agree on line endings. CMake writes + # required-runtime.txt with the platform's line ending and 7-Zip writes CRLF + # on Windows, so a trailing \r survives on $entry and defeats the whole-line + # match - every entry then reads as missing even though the listing holds it + # verbatim. # [$] rather than \$ so the literal dollar is a bracket expression and the - # linter does not read these as unexpanded shell variables (SC2016). + # linter does not read these as unexpanded shell variables (SC2016). The + # same goes for the braces of Inno Setup's {app} prefix. installer_paths="$("$seven_zip" l -slt "$package" | tr -d '\r' | sed -n 's/^Path = //p' | - sed -e 's|\\|/|g' -e 's|^[$]INSTDIR/||' -e 's|^[$]OUTDIR/||' -e 's|^[$]PLUGINSDIR/||' -e 's|^\./||' -e 's|^/||')" + sed -e 's|\\|/|g' -e 's|^[$]INSTDIR/||' -e 's|^[$]OUTDIR/||' -e 's|^[$]PLUGINSDIR/||' -e 's|^[{]app[}]/||' -e 's|^\./||' -e 's|^/||')" manifest="$(tr -d '\r' = 3.27 (found ${CMAKE_VERSION})") +endif() + +# CPack may be told -DCPACK_GENERATOR=... by the caller. For the Windows installers the +# choice is made by COCKATRICE_WIN_INSTALLER instead (.ci/compile.sh --package NSIS or +# InnoSetup maps to it), because picking the generator here and the installer tool there can +# silently build both in one run. Only guard against asking for InnoSetup twice. +if(NOT "InnoSetup" IN_LIST CPACK_GENERATOR) + set(CPACK_GENERATOR InnoSetup ${CPACK_GENERATOR}) +endif() + +if(CMAKE_SIZEOF_VOID_P EQUAL 8) + set(CPACK_INNOSETUP_ARCHITECTURE x64) +else() + set(CPACK_INNOSETUP_ARCHITECTURE x86) +endif() + +# Shortcuts and the finish-page run entry come from the executables CPack already knows. +# Only the client is offered: oracle.exe and servatrice.exe are tools, and the NSIS +# installer never created shortcuts for them either. +set(CPACK_PACKAGE_EXECUTABLES "cockatrice" "Cockatrice") +set(CPACK_INNOSETUP_RUN_EXECUTABLES "cockatrice") + +set(CPACK_INNOSETUP_USE_MODERN_WIZARD ON) +set(CPACK_INNOSETUP_PROGRAM_MENU_FOLDER "Cockatrice") +set(CPACK_INNOSETUP_ICON_FILE "${PROJECT_SOURCE_DIR}/cockatrice/resources/appicon.ico") +set(CPACK_INNOSETUP_IGNORE_README_PAGE ON) +# The uninstall entry is the only place a user sees the version they have after an update +# that replaced the folder, so keep it and give it the icon. +set(CPACK_INNOSETUP_CREATE_UNINSTALL_LINK ON) + +# Upgrade identity. Never change this value: it is what makes a new build an upgrade of the +# previous one (same uninstall entry, same "previous app path", same tasks) instead of a +# second, parallel installation. No braces: a literal { in an .iss script opens a constant, +# and a GUID here buys nothing that this string does not already do. +set(CPACK_INNOSETUP_SETUP_AppId "Cockatrice-6F1C5B2A-9D84-4E37-B0A5-2C7E9A31D4F8") +set(CPACK_INNOSETUP_SETUP_AppName "Cockatrice") +set(CPACK_INNOSETUP_SETUP_DefaultGroupName "Cockatrice") + +# An NSIS install registers nothing under Inno's own keys, so the default directory is +# resolved from the legacy registration instead of guessed. See GetDefaultInstallDir in +# cmake/InnoSetup.code.iss. +set(CPACK_INNOSETUP_SETUP_DefaultDirName "{code:GetDefaultInstallDir}") + +set(CPACK_INNOSETUP_SETUP_Compression "lzma2/max") +set(CPACK_INNOSETUP_SETUP_SolidCompression ON) +# Replaces the CockLog/CockLogListing machinery: every run writes a log to %TEMP% without +# any script support, which is what a failed upgrade on a user's machine needs to be +# diagnosable after the fact. +set(CPACK_INNOSETUP_SETUP_SetupLogging ON) + +set(CPACK_INNOSETUP_SETUP_PrivilegesRequired "admin") +# Users without administrator rights get a working installation instead of a dead end; +# /CURRENTUSER covers the same choice when driven by a package manager or the updater. +set(CPACK_INNOSETUP_SETUP_PrivilegesRequiredOverridesAllowed "dialog commandline") + +# Restart Manager, not a PowerShell process scan. Setup detects the applications holding +# the files it is about to replace, closes them, and starts them again afterwards. This is +# the whole of what WaitForAppToClose/GuardAppsNotRunning/ForceCloseApps approximated in +# the NSIS template, including the uninstaller copies that had to repeat it. +set(CPACK_INNOSETUP_SETUP_CloseApplications ON) +set(CPACK_INNOSETUP_SETUP_RestartApplications ON) + +# The payload is built for one architecture only; refuse to install it on a system that +# cannot run it. x64 (not x64compatible) keeps this valid on every Inno Setup 6.x. +if(CPACK_INNOSETUP_ARCHITECTURE STREQUAL "x64") + set(CPACK_INNOSETUP_SETUP_ArchitecturesAllowed "x64") + set(CPACK_INNOSETUP_SETUP_ArchitecturesInstallIn64BitMode "x64") +endif() + +set(CPACK_INNOSETUP_SETUP_UninstallDisplayIcon "{app}\\cockatrice.exe") + +# Sections the generator does not emit (the stale-runtime deletes), and the behaviour it +# cannot express (legacy migration, portable marker, C++ runtime bootstrap). +set(CPACK_INNOSETUP_EXTRA_SCRIPTS "${COCKATRICE_CMAKE_PATH}/InnoSetup.install.iss") +set(CPACK_INNOSETUP_CODE_FILES "${COCKATRICE_CMAKE_PATH}/InnoSetup.code.iss") + +# The installer-log sender the failure hook in InnoSetup.code.iss offers after an upgrade +# that did not finish. CPack passes these as public ISCC macros (/Dname=value), so the extra +# script can embed the helper with a [Files] dontcopy entry and the code can extract it at +# send time. Undefined when the build carries no reporting, which is what the ifdef guards in +# both scripts test. +if(CPACK_SENTRY_ENABLED) + set(CPACK_INNOSETUP_DEFINE_SentryHelperPath "${COCKATRICE_SENTRY_INSTALLER_LOG}") +endif() + +message(STATUS "Windows installer: Inno Setup (CPack InnoSetup generator, ${CPACK_INNOSETUP_ARCHITECTURE})") diff --git a/cmake/InnoSetup.code.iss b/cmake/InnoSetup.code.iss new file mode 100644 index 000000000..27ef5c02f --- /dev/null +++ b/cmake/InnoSetup.code.iss @@ -0,0 +1,257 @@ +// Behaviour for the Windows installer written by CPack's Inno Setup generator. +// +// Included through CPACK_INNOSETUP_CODE_FILES, which places this file inside the generated +// script's code section. Everything here is something the generator cannot express as +// data: the migration off the NSIS installation, the portable marker, and the C++ runtime +// bootstrap. Comments use Pascal braces, so the install-directory constant is spelled out +// as ExpandConstant rather than written inside prose. + +const + // Written by every NSIS build up to and including 3.1.x: HKLM in the 64-bit view for a + // 64-bit installer, and in the 32-bit view by the older 32-bit ones. + LegacyUninstallKey = 'SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall\Cockatrice'; + +var + PortablePage: TCustomPage; + PortableCheck: TNewCheckBox; + // Progress of this run, for the failure hook further down. Setup has no "the installation + // failed" callback, and DeinitializeSetup runs on every way out of the wizard, so the two + // steps that bracket the file work are what distinguishes those ways. + InstallStarted: Boolean; + InstallCompleted: Boolean; + UserCancelled: Boolean; + +// True when the given switch appears on the setup command line. Used for the legacy +// portable switch, which the in-app updater and the older installers pass. +function HasSwitch(const Name: String): Boolean; +var + I: Integer; +begin + Result := False; + for I := 1 to ParamCount do + if CompareText(ParamStr(I), Name) = 0 then + begin + Result := True; + Exit; + end; +end; + +// The install directory recorded by the NSIS installer, or an empty string when this +// machine has no such installation. Read-only access to HKLM, so it works unelevated. +function ReadLegacyInstallDir(): String; +var + Value: String; +begin + Result := ''; + if RegQueryStringValue(HKLM64, LegacyUninstallKey, 'InstallLocation', Value) then + Result := Value + else if RegQueryStringValue(HKLM32, LegacyUninstallKey, 'InstallLocation', Value) then + Result := Value; +end; + +// Scripted constant behind the DefaultDirName directive. An NSIS installation keeps no +// Inno-style "previous app path", so without this an upgrade of a user who picked a custom +// folder in 3.1.x would land in the default folder and leave the old copy behind. +function GetDefaultInstallDir(Param: String): String; +var + Legacy: String; +begin + Legacy := ReadLegacyInstallDir(); + if (Legacy <> '') and DirExists(Legacy) then + begin + Result := Legacy; + Log('Default install directory follows the NSIS installation: ' + Result); + end + else + Result := ExpandConstant('{autopf}\Cockatrice'); +end; + +procedure InitializeWizard(); +begin + PortablePage := CreateCustomPage(wpSelectDir, 'Installation mode', + 'Cockatrice keeps settings, decks and downloaded pictures in your user profile by default. ' + + 'Portable mode keeps them in the install folder instead.'); + PortableCheck := TNewCheckBox.Create(PortablePage); + PortableCheck.Parent := PortablePage.Surface; + PortableCheck.Left := 0; + PortableCheck.Top := 0; + PortableCheck.Width := PortablePage.SurfaceWidth; + PortableCheck.Height := ScaleY(32); + PortableCheck.WordWrap := True; + PortableCheck.Caption := 'Portable: keep settings and decks in the install folder'; + PortableCheck.Checked := HasSwitch('/PORTABLE'); +end; + +function PortableRequested(): Boolean; +begin + Result := (Assigned(PortableCheck) and PortableCheck.Checked) or HasSwitch('/PORTABLE'); +end; + +// The client decides portable mode by the presence of this file next to the executable +// (cache_settings.cpp and logger.cpp), not by its contents. +procedure WritePortableMarker(); +var + Lines: TArrayOfString; +begin + if not PortableRequested() then + Exit; + SetArrayLength(Lines, 1); + Lines[0] := 'Portable install: settings, decks and pictures live beside the executable.'; + if not SaveStringsToFile(ExpandConstant('{app}\portable.dat'), Lines, False) then + MsgBox('Could not write portable.dat into the install folder. Portable mode will not be' + + ' active until it exists.', mbError, MB_OK); +end; + +// Runs the Visual C++ redistributable the payload carries, when it carries one. The file +// name differs between the VS 2017 and VS 2022 layouts, so it is matched rather than named. +// A failure is logged and ignored: the redistributable is usually already present, and the +// application is what reports a real problem. +procedure InstallVCRedist(); +var + FindRec: TFindRec; + ResultCode: Integer; + Redist: String; +begin + if not FindFirst(ExpandConstant('{app}\vc*redist*.exe'), FindRec) then + Exit; + try + repeat + if (FindRec.Attributes and FILE_ATTRIBUTE_DIRECTORY) = 0 then + begin + Redist := ExpandConstant('{app}\') + FindRec.Name; + Log('Installing the C++ runtime: ' + Redist); + if Exec(Redist, '/install /quiet /norestart', '', SW_HIDE, ewWaitUntilTerminated, ResultCode) then + begin + Log('C++ runtime installer exited with ' + IntToStr(ResultCode)); + DeleteFile(Redist); + end + else + Log('Could not start the C++ runtime installer: ' + Redist); + end; + until not FindNext(FindRec); + finally + FindClose(FindRec); + end; +end; + +// Removes the Add/Remove Programs entry the NSIS installer left behind. Without this the +// user sees two entries for Cockatrice and can still run the old uninstaller, whose +// blanket removal of the install directory is the data loss the 3.1.x backup pair was +// written to survive. Deleting the registration is what makes that uninstaller +// unreachable; the binary next to it is removed earlier by the install-delete section. +procedure RemoveLegacyNSISRegistration(); +var + Uninstaller: String; +begin + if not (RegQueryStringValue(HKLM64, LegacyUninstallKey, 'UninstallString', Uninstaller) + or RegQueryStringValue(HKLM32, LegacyUninstallKey, 'UninstallString', Uninstaller)) then + Exit; + RegDeleteKeyIncludingSubkeys(HKLM64, LegacyUninstallKey); + RegDeleteKeyIncludingSubkeys(HKLM32, LegacyUninstallKey); + Log('Removed the legacy NSIS uninstall registration: ' + Uninstaller); +end; + +procedure CurStepChanged(CurStep: TSetupStep); +begin + if CurStep = ssInstall then + InstallStarted := True; + if CurStep = ssPostInstall then + begin + InstallVCRedist(); + WritePortableMarker(); + RemoveLegacyNSISRegistration(); + end; + if CurStep = ssDone then + InstallCompleted := True; +end; + +// Cancel is a decision, not a failure, so the hook below stays quiet after one. +procedure CancelButtonClick(CurPageID: Integer; var Cancel, Confirm: Boolean); +begin + UserCancelled := True; +end; + +#ifdef SentryHelperPath +// The failure hook, which is what the NSIS template's AbortInstall did for updates that +// stopped half way. The three conditions it watched for - the application would not close, +// the stale runtime could not be purged, the launcher did not exit - are Restart Manager's +// and the [InstallDelete] section's job here, so what is left is the outcome rather than the +// cause: a silent run the in-app updater started, which began replacing files and never +// reached the end. That run is the one that needs it, because the updater quits after +// launching Setup and leaves no window on screen to read an error from; an interactive user +// sees the message Setup shows and can find the log in the temporary directory themselves. +procedure OfferToSendInstallerLog(); +var + LogPath, Params: String; + ResultCode: Integer; +begin + LogPath := ExpandConstant('{log}'); + if LogPath = '' then + Exit; + // The sender takes the reason from the log (see SendInstallerLog.ps1.in), so record it + // whether or not the user agrees: it is the first thing to look at in a report either way. + Log('ABORT: the update did not finish installing'); + Log('full installer log: ' + LogPath); + if MsgBox('Send the installer log to the developers to help fix this? The log contains' + + ' file paths from this machine, which may include your Windows user name.', + mbConfirmation, MB_YESNO) <> IDYES then + begin + Log('Sentry send: declined by user'); + Exit; + end; + ExtractTemporaryFile('SendInstallerLog.ps1'); + Params := '-NoProfile -NonInteractive -ExecutionPolicy Bypass -File "' + + ExpandConstant('{tmp}\SendInstallerLog.ps1') + '" -LogPath "' + LogPath + '"'; + ResultCode := -1; + if Exec('powershell.exe', Params, '', SW_HIDE, ewWaitUntilTerminated, ResultCode) + and (ResultCode = 0) then + begin + Log('Sentry send: result=' + IntToStr(ResultCode)); + MsgBox('The installer log was sent. Thank you for helping improve Cockatrice!', + mbInformation, MB_OK); + end + else + begin + Log('Sentry send: result=' + IntToStr(ResultCode)); + MsgBox('The installer log could not be sent. You can still share it manually; it was' + + ' written to:' + #13#10 + LogPath, mbError, MB_OK); + end; +end; +#endif + +// Runs after logging has started and while the log is still open and flushed, which is what +// makes the log of this run available to the sender. +procedure DeinitializeSetup(); +begin +#ifdef SentryHelperPath + // /UPDATER is the client's own mark on the command lines it hands either installer. + // Requiring it keeps this offer out of a package manager's silent install, where a message + // box that nobody answers would block the install - the case the NSIS template kept clear + // of by prompting on its /R switch rather than on silence alone. + if WizardSilent and HasSwitch('/UPDATER') and InstallStarted and not InstallCompleted + and not UserCancelled then + OfferToSendInstallerLog(); +#endif +end; + +// The in-app updater used to pass two switches that belong to the NSIS build: /R (silent +// reinstall) and /PID= (the process to wait for). Setup ignores switches it does not know - +// its command line is an if/else-if chain over the ones it does know, with no fallback that +// rejects the rest (ProcessCommandLine in Setup.MainFunc.pas) - so an older client still +// installs from this one, it just walks the wizard instead of running silently. Logging the +// switches keeps both cases visible in the log of the first upgrade that comes through here. +function InitializeSetup(): Boolean; +var + I: Integer; + Param: String; +begin + Log('Setup command line: ' + GetCmdTail()); + for I := 1 to ParamCount do + begin + Param := ParamStr(I); + if (CompareText(Param, '/R') = 0) or (CompareText(Param, '/UPDATER') = 0) + or (CompareText(Copy(Param, 1, 5), '/PID=') = 0) then + Log('In-app updater switch: ' + Param); + end; + Result := True; +end; diff --git a/cmake/InnoSetup.install.iss b/cmake/InnoSetup.install.iss new file mode 100644 index 000000000..80b7411c5 --- /dev/null +++ b/cmake/InnoSetup.install.iss @@ -0,0 +1,36 @@ +; Sections added to the setup script CPack's Inno Setup generator writes. +; +; Included through CPACK_INNOSETUP_EXTRA_SCRIPTS, which prepends this file to the +; generated script. Repeating a section that the generator also writes is not an error: the +; compiler re-tests every [Header] against the section it is collecting and enumerates the +; lines of each occurrence (FoundSection in TSetupCompiler.EnumIniSection, +; Compiler.SetupCompiler.pas), so the [Files] and [InstallDelete] below merge with the ones +; built from the install() rules. Behaviour belongs in InnoSetup.code.iss. + +[InstallDelete] +; The previous version's runtime, whatever it was called. Replacing a DLL in place is not +; enough: a module the payload no longer carries (Qt6Multimedia.dll while it was still +; shared, Qt6QuickWidgets.dll) would survive the install and then be loaded next to the new +; executable, which is the "procedure entry point ... could not be located" failure after +; an update. Removing every DLL and letting the payload put its own back is what the NSIS +; RetryDllPurge did with 250 lines of retry and diagnostics; with CloseApplications on, +; Restart Manager has already released the handles, so one pass is enough. +Type: files; Name: "{app}\*.dll" +; Plugins are loaded by name from this directory, so a stale one is just as fatal and is +; not covered by the payload's own file list. +Type: filesandordirs; Name: "{app}\Plugins" +; The NSIS uninstaller. Its Add/Remove Programs registration is deleted after a successful +; install (RemoveLegacyNSISRegistration in InnoSetup.code.iss); the binary itself has no +; business staying next to the new one. +Type: files; Name: "{app}\Uninstall.exe" + +; The installer-log sender behind the failure hook in InnoSetup.code.iss. dontcopy embeds +; the file in the setup image without installing it, and ExtractTemporaryFile pulls it out +; into the temporary directory only when something went wrong - what the NSIS template did +; by writing the same helper into $PLUGINSDIR. The path arrives from CMake as a public ISCC +; macro defined only for a build that carries a Sentry DSN; without one there is nothing to +; send, so nothing is embedded and the hook compiles out. +#ifdef SentryHelperPath +[Files] +Source: "{#SentryHelperPath}"; Flags: dontcopy +#endif diff --git a/cmake/SendInstallerLog.ps1.in b/cmake/SendInstallerLog.ps1.in index 585c726dd..f65a0319e 100644 --- a/cmake/SendInstallerLog.ps1.in +++ b/cmake/SendInstallerLog.ps1.in @@ -1,5 +1,5 @@ -# Sends the NSIS installer log to Sentry as an envelope carrying one error -# event plus the log as an attachment. +# Sends the installer log, from either installer, to Sentry as an envelope carrying one +# error event plus the log as an attachment. # # Generated at configure time from this template by CMake configure_file: the # CPACK_SENTRY_* placeholders are baked in then, so the real values only ever @@ -44,16 +44,20 @@ try { } # --- Abort reason, read from the log itself -------------------------------- - # AbortInstall logs "ABORT: ..." first, so the reason travels inside the file - # and nothing reason-shaped crosses the command line (no quoting hazards). - # Select-String yields the matching line alone, so the reason is already one - # physical line for the message; anything else stays in the attached log. - # Non-ASCII paths may garble here; the attachment is byte-exact regardless. + # Each installer records the reason before offering to send, so it travels inside the + # file and nothing reason-shaped crosses the command line (no quoting hazards). + # NSIS writes the line as "ABORT: ..." through CockLog. Inno Setup prefixes every line + # of its log with a timestamp and three spaces, so that prefix is optional here and + # removed together with the marker. Select-String yields the matching line alone, so + # the reason is already one physical line for the message; anything else stays in the + # attached log. Non-ASCII paths may garble here; the attachment is byte-exact + # regardless. + $stamp = '\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}\.\d{3}\s+' $Reason = 'unknown (no ABORT line in log)' if (Test-Path -LiteralPath $LogPath) { - $abortLine = Select-String -LiteralPath $LogPath -Pattern '^ABORT: ' | Select-Object -Last 1 + $abortLine = Select-String -LiteralPath $LogPath -Pattern ('^(' + $stamp + ')?ABORT: ') | Select-Object -Last 1 if ($abortLine) { - $Reason = ($abortLine.Line -replace '^ABORT:\s*', '') + $Reason = ($abortLine.Line -replace ('^(' + $stamp + ')?ABORT:\s*'), '') } } diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_update.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_update.cpp index 11dcd7e27..793470fad 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_update.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_update.cpp @@ -229,16 +229,16 @@ void DlgUpdate::setLabel(const QString &newText) void DlgUpdate::warnInstallerIsWaiting() { - // Modeless on purpose: the installer asks the application to close exactly once (see - // CloseMatchingApps in NSIS.template.in) and only polls afterwards, while Qt drops spontaneous - // close events - a WM_CLOSE from the installer - for windows that are blocked by a modal - // widget (QGuiApplicationPrivate::processCloseEvent). An application modal message box here - // would therefore swallow the installer's one and only request for the rest of its wait. + // Modeless on purpose: the installer asks the application to close exactly once - the NSIS + // template closes matching windows itself (CloseMatchingApps in NSIS.template.in), Setup asks + // through Restart Manager - and then gives up on the update, while Qt drops spontaneous close + // events - a WM_CLOSE from the installer - for windows that are blocked by a modal widget + // (QGuiApplicationPrivate::processCloseEvent). An application modal message box here would + // therefore swallow the installer's one and only request. auto *notification = new QMessageBox(QMessageBox::Warning, tr("Update"), - tr("The update installer is already running and waits about a " - "minute for Cockatrice to close. Cockatrice is still busy, so " - "save your work and close it before then. Otherwise the " - "installer gives up and the update is cancelled."), + tr("The update installer is waiting for Cockatrice to close. " + "Cockatrice is still busy, so save your work and close it. " + "Otherwise the update cannot be installed and is cancelled."), QMessageBox::Ok, parentWidget()); notification->setWindowModality(Qt::NonModal); notification->setAttribute(Qt::WA_DeleteOnClose); @@ -276,19 +276,35 @@ void DlgUpdate::downloadSuccessful(const QUrl &filepath) QProcess process; process.setProgram(installerPath); - // NSIS needs the /D= argument to be an UNQUOTED string, even if it contains spaces. Qt likes to quote arguments if - // they contain spaces, so we use the windows exclusive QProcess::setNativeArguments in the only case where this is - // relevant, which preserves the argument unquoted. - // - // /PID= tells the installer which process to wait for before it replaces anything. We launch the installer first - // and only quit afterwards, so it starts while this process is still alive and still has every Qt runtime DLL - // mapped, and a mapped DLL cannot be deleted. Without the handle the installer can only poll for an image name and - // guess, and a wrong guess is what leaves the update writing new executables next to runtime files of the previous - // version, which fails on the next start with "The procedure entry point ... could not be located in the dynamic - // link library - // ...Qt6QuickWidgets.dll". /D= has to stay the last parameter, which is why it is last here too. + // One command line has to drive either installer, because a package can be built with either + // one and the client cannot tell which it downloaded: + // - /R runs the NSIS installer silently and reinstalls over the existing installation. Setup + // ignores the switches it does not know, and that covers /R, /PID= and /D=. + // - /VERYSILENT and the switches around it are Setup's spelling of the same thing. + // /CLOSEAPPLICATIONS lets Restart Manager close Cockatrice if this process is still shutting + // down once Setup reaches the files, and /NOFORCECLOSEAPPLICATIONS keeps that from ever + // forcing it: a window that refuses to close still cancels the update rather than losing the + // user's work. + // - /PID= tells the NSIS installer which process to wait for before it replaces anything. We + // launch the installer first and only quit afterwards, so it starts while this process is + // still alive and still has every Qt runtime DLL mapped, and a mapped DLL cannot be deleted. + // Without the handle the installer can only poll for an image name and guess, and a wrong + // guess is what leaves the update writing new executables next to runtime files of the + // previous version, which fails on the next start with "The procedure entry point ... could + // not be located in the dynamic link library ...Qt6QuickWidgets.dll". Setup ignores the + // switch; Restart Manager releases the same handles, which is what /CLOSEAPPLICATIONS above + // asks it to do. + // - /UPDATER marks an install that the in-app updater launched. Both installers ignore it + // while running, and only the Inno Setup package reads it: it is what lets the failure + // hook offer the log of a silent update that did not finish, without ever putting a + // message box in front of a package manager's silent install. + // NSIS honours /D= only as the last parameter and requires it unquoted even when it has spaces, + // so it stays last here and the whole line goes through QProcess::setNativeArguments. #ifdef Q_OS_WIN - process.setNativeArguments(QString("/R /PID=%1 /D=%2").arg(QCoreApplication::applicationPid()).arg(appDir)); + process.setNativeArguments(QString("/R /PID=%1 /UPDATER /VERYSILENT /SUPPRESSMSGBOXES /NORESTART" + " /CLOSEAPPLICATIONS /NOFORCECLOSEAPPLICATIONS /D=%2") + .arg(QCoreApplication::applicationPid()) + .arg(appDir)); #else // Linux/macOS: normal argument passing (not relevant since they update differently.) process.setArguments({"/R", QString("/D=%1").arg(appDir)}); @@ -297,14 +313,13 @@ void DlgUpdate::downloadSuccessful(const QUrl &filepath) // Try to open the installer. If it opens, quit Cockatrice if (process.startDetached()) { qCInfo(DlgUpdateLog) << "Opened update installer successfully - closing Cockatrice"; - // Close the main window synchronously so file locks are released before the NSIS installer + // Close the main window synchronously so file locks are released before the installer // (already launched) starts replacing files. This also flushes settings and shuts down the // tabs, but only when the close is actually accepted: MainWindow may veto it for a running // card DB update, an open game, or an unsaved deck, and closeForUpdate() also reports a // close already in progress (reached from a nested event loop while a shutdown prompt is // up). Only quit when the shutdown really ran - otherwise keep running so the user can - // resolve the blocker, and warn them that the installer only waits about a minute - // before it gives up and cancels the update. + // resolve the blocker, and warn them that the installer cannot finish until it closes. if (auto *window = qobject_cast(parent())) { if (window->closeForUpdate()) { QTimer::singleShot(0, qApp, [] { QCoreApplication::exit(0); });