From 2c8e47a0a4ecf6e959c8b6be2c662bc42922c5d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Mon, 26 Oct 2026 07:52:44 +0100 Subject: [PATCH 1/2] [Installer] Add an Inno Setup generator next to NSIS. COCKATRICE_WIN_INSTALLER chooses which tool writes the Windows installer. NSIS stays the default, so nothing on the existing path changes; InnoSetup selects CPack's own INNOSETUP generator (CMake >= 3.27) and builds from the same install() rules. Both can be built from one tree and the resulting installers compared on an upgrade from 3.1.x. What the Inno path does differently: - Installs over the previous version instead of running the old uninstaller first, which is what the backup/restore pair in the NSIS template exists to survive. - Closes and restarts a running application through Windows Restart Manager (CloseApplications/RestartApplications) instead of the two PowerShell process guards the template needed. - Removes stale runtime with one [InstallDelete] of *.dll and Plugins, instead of RetryDllPurge and its diagnostics. - Writes a log on every run through SetupLogging, instead of CockLog. - Resolves the install directory from the legacy NSIS registration, and deletes that registration after a successful install so the old uninstaller (whose blanket directory removal caused the 3.1.x data loss) cannot be run after the upgrade. - Keeps the portable mode the client detects, the C++ runtime bootstrap and the /PORTABLE switch. The in-app updater still passes the NSIS switches (/R, /PID=); the installer logs them so the first Windows run settles whether Inno Setup accepts them or the switch-over needs a client version gate. The installer content check in .ci/compile.sh learns to strip Inno Setup's {app} prefix from the paths 7-Zip lists, so the required-runtime comparison works for either archive format. --- .ci/compile.sh | 24 ++--- CMakeLists.txt | 37 ++++++-- cmake/InnoSetup.cmake | 92 +++++++++++++++++++ cmake/InnoSetup.code.iss | 177 ++++++++++++++++++++++++++++++++++++ cmake/InnoSetup.install.iss | 23 +++++ 5 files changed, 333 insertions(+), 20 deletions(-) create mode 100644 cmake/InnoSetup.cmake create mode 100644 cmake/InnoSetup.code.iss create mode 100644 cmake/InnoSetup.install.iss diff --git a/.ci/compile.sh b/.ci/compile.sh index d60683b6e..9cdd1e54b 100755 --- a/.ci/compile.sh +++ b/.ci/compile.sh @@ -590,20 +590,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 is allowed to be told -DCPACK_GENERATOR=... by the caller; do not list the +# generator twice when that caller already asked for it. +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") + +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..2d5a2eeb1 --- /dev/null +++ b/cmake/InnoSetup.code.iss @@ -0,0 +1,177 @@ +// 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; + +// 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 = ssPostInstall then + begin + InstallVCRedist(); + WritePortableMarker(); + RemoveLegacyNSISRegistration(); + end; +end; + +// The 3.1.x client launches the installer with two switches that belong to the NSIS build: +// /R (silent reinstall) and /PID= (the process to wait for). Recording them from the log is +// how the first test of this installer answers the one open question about the migration: +// whether Inno Setup reaches this point at all with them on the command line. If it does, +// the updater keeps working across the switch; if it does not, Setup rejects them before +// running any script and the updater needs a version gate instead. +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(Copy(Param, 1, 5), '/PID=') = 0) then + Log('Legacy 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..bd691316e --- /dev/null +++ b/cmake/InnoSetup.install.iss @@ -0,0 +1,23 @@ +; 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. Only sections the generator never emits may appear here: it writes +; [Files], [Dirs], [Icons], [Run] and [Registry] from the install() rules, and a second +; section of one of those is a compile error. 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" From 0403ac9354eeda89baabd2bb6388c54d36200fff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Mon, 26 Oct 2026 08:29:22 +0100 Subject: [PATCH 2/2] [Installer] Make the Inno Setup installer the default, with its failure hook. COCKATRICE_WIN_INSTALLER now defaults to InnoSetup. NSIS stays selectable for one release as the legacy fallback, and the two can still be built from one tree and compared on a real upgrade. The switches question from the previous commit is settled from Inno Setup's source: the setup command line is an if/else-if chain over the switches it knows, with no fallback that rejects the rest, so /R, /PID= and /D= are ignored rather than refused. The client therefore sends one command line that drives either installer: - /R and /PID= for NSIS, which also needs /D= last and unquoted; - /VERYSILENT and the CloseApplications set for Setup, which releases the mapped runtime DLLs through Restart Manager instead of waiting for a pid; - /UPDATER as the client's own mark, read only by the Inno Setup script. An older client that passes only the NSIS switches still installs; it walks the wizard once instead of running silently. The Sentry offer that AbortInstall carried is ported: a silent run the updater launched, which began replacing files and never reached the end, offers to send its log through the same helper. The helper is embedded with a [Files] dontcopy entry and extracted at send time; a repeated [Files] section merges with the generated one instead of being a compile error. Setup prefixes every log line with a timestamp, so the helper's ABORT scan now allows that prefix. CI installs Inno Setup next to NSIS, dumps ISCCOutput.log and the generated script when the installer fails to compile, and --package NSIS|InnoSetup selects the installer through the CMake option instead of adding a second CPack generator to the run. --- .ci/compile.sh | 9 +- .github/workflows/desktop-build.yml | 29 ++++-- CMakeLists.txt | 34 ++++--- cmake/InnoSetup.cmake | 21 +++- cmake/InnoSetup.code.iss | 96 +++++++++++++++++-- cmake/InnoSetup.install.iss | 19 +++- cmake/SendInstallerLog.ps1.in | 22 +++-- .../interface/widgets/dialogs/dlg_update.cpp | 63 +++++++----- 8 files changed, 220 insertions(+), 73 deletions(-) diff --git a/.ci/compile.sh b/.ci/compile.sh index 9cdd1e54b..25c6dbf4b 100755 --- a/.ci/compile.sh +++ b/.ci/compile.sh @@ -194,7 +194,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") diff --git a/.github/workflows/desktop-build.yml b/.github/workflows/desktop-build.yml index c97b48342..fd3301ee9 100644 --- a/.github/workflows/desktop-build.yml +++ b/.github/workflows/desktop-build.yml @@ -552,10 +552,13 @@ jobs: modules: ${{ matrix.qt_modules }} version: ${{ matrix.qt_version }} - - name: "[Windows] Install NSIS" + # Inno Setup writes the default Windows installer (COCKATRICE_WIN_INSTALLER=InnoSetup); + # NSIS stays installed because the option still selects the old installer, and CI has to + # be able to build both to compare them on a real upgrade. + - name: "[Windows] Install installer tooling" if: matrix.os == 'Windows' shell: bash - run: choco install nsis + run: choco install nsis innosetup # Ninja invokes cl.exe directly and needs the MSVC environment on PATH. - name: "[Windows] Set up MSVC environment" @@ -745,16 +748,26 @@ jobs: # `debug-files upload` and DIFs are matched by debug ID, not release. sentry-cli debug-files upload --org cockatrice --project cockatrice-desktop "${paths[@]}" - # CPack reports a failed makensis run as "Problem running NSIS command" - # plus a path to NSISOutput.log, and that log lives only on the runner, so - # the actual compile error is otherwise nowhere in the workflow output. - # Dump it, or a broken installer script is undiagnosable from CI alone. + # CPack reports a failed installer compile only as a path to a log that lives on the + # runner: NSISOutput.log for makensis, ISCCOutput.log for Inno Setup's compiler. The + # Inno Setup error names a line of the script CPack generated, so that script is + # dumped too - without them a broken installer script is undiagnosable from CI alone. # shell: bash matters here - the default pwsh resolves `find` to Windows' # text-search find.exe, which errors instead of searching. - - name: "Dump NSIS compile log" + - name: "Dump installer compile log" if: failure() && matrix.os == 'Windows' shell: bash - run: find . -name NSISOutput.log -exec cat {} + + run: | + find . -name NSISOutput.log -o -name ISCCOutput.log | while read -r log; do + echo "::group::$log" + cat "$log" + echo "::endgroup::" + done + find . -path '*_CPack_Packages*' -name ISScript.iss | while read -r script; do + echo "::group::$script" + cat "$script" + echo "::endgroup::" + done # A master run hands over the archives it had to build itself so that # vcpkg-cache-import.yml, which runs in the default branch scope, can save them diff --git a/CMakeLists.txt b/CMakeLists.txt index 21fcc964c..c9075a5d4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -327,19 +327,32 @@ if(UNIX) endif() endif() elseif(WIN32) - # Which tool writes the installer. NSIS is the incumbent and stays the default; - # InnoSetup selects CPack's own Inno Setup generator (see cmake/InnoSetup.cmake). Both - # consume the same install() rules, so this is a packaging choice, never a build change, - # and the two installers can be built from one tree and compared on a real upgrade. + # Which tool writes the installer. InnoSetup, through CPack's own Inno Setup generator + # (cmake/InnoSetup.cmake), is the default. NSIS stays selectable for one release as the + # fallback for anyone who needs the old installer, and so the two can still be built from + # one tree and compared on a real upgrade. Both consume the same install() rules, so this + # is a packaging choice, never a build change. set(COCKATRICE_WIN_INSTALLER - "NSIS" - CACHE STRING "Windows installer generator: NSIS (incumbent) or InnoSetup" + "InnoSetup" + CACHE STRING "Windows installer generator: InnoSetup (default) or NSIS (legacy)" ) set_property(CACHE COCKATRICE_WIN_INSTALLER PROPERTY STRINGS NSIS InnoSetup) if(NOT COCKATRICE_WIN_INSTALLER MATCHES "^(NSIS|InnoSetup)$") message(FATAL_ERROR "COCKATRICE_WIN_INSTALLER must be NSIS or InnoSetup, not '${COCKATRICE_WIN_INSTALLER}'") endif() + # Installer-log sender for the failure hooks of both installers (values parsed out of + # COCKATRICE_SENTRY_DSN in cmake/CrashReporting.cmake). Generated only when a DSN was + # parsed: cmake/NSIS.template.in and cmake/InnoSetup.install.iss embed it under the same + # CPACK_SENTRY_ENABLED check, so a build without reporting never references or carries the + # helper. Configured above the installer choice because both branches consume it. + if(CPACK_SENTRY_ENABLED) + configure_file( + "${COCKATRICE_CMAKE_PATH}/SendInstallerLog.ps1.in" "${PROJECT_BINARY_DIR}/SendInstallerLog.ps1" @ONLY + ) + set(COCKATRICE_SENTRY_INSTALLER_LOG "${PROJECT_BINARY_DIR}/SendInstallerLog.ps1") + endif() + if(COCKATRICE_WIN_INSTALLER STREQUAL "InnoSetup") include(${COCKATRICE_CMAKE_PATH}/InnoSetup.cmake) else() @@ -354,15 +367,6 @@ elseif(WIN32) configure_file("${COCKATRICE_CMAKE_PATH}/NSIS.definitions.nsh.in" "${PROJECT_BINARY_DIR}/NSIS.definitions.nsh") endif() - # Installer-log sender for the AbortInstall Sentry hook (values parsed out of COCKATRICE_SENTRY_DSN in - # cmake/CrashReporting.cmake). Generated only when a DSN was parsed: cmake/NSIS.template.in embeds it under - # the same CPACK_SENTRY_ENABLED check, so a build without reporting never references or carries the helper. - if(CPACK_SENTRY_ENABLED) - configure_file( - "${COCKATRICE_CMAKE_PATH}/SendInstallerLog.ps1.in" "${PROJECT_BINARY_DIR}/SendInstallerLog.ps1" @ONLY - ) - endif() - # Every runtime DLL a Windows install has to contain, as paths relative to the # install root. .ci/compile.sh reads this and fails the build when the packaged # installer is missing one of them, or carries a runtime it does not know about. diff --git a/cmake/InnoSetup.cmake b/cmake/InnoSetup.cmake index 6a03415bf..1ddf6c7f4 100644 --- a/cmake/InnoSetup.cmake +++ b/cmake/InnoSetup.cmake @@ -1,9 +1,9 @@ # Windows installer written by Inno Setup, through CPack's own InnoSetup generator. # # Included from the WIN32 branch of the top-level CMakeLists.txt when -# COCKATRICE_WIN_INSTALLER is InnoSetup. It sits next to the NSIS template rather than -# replacing it so both installers can be built from the same tree and compared on a real -# upgrade. +# COCKATRICE_WIN_INSTALLER is InnoSetup, which is the default. The NSIS template is kept +# next to this file rather than deleted so both installers can still be built from one tree +# and compared on a real upgrade. # # The generator writes [Files], [Dirs], [Icons], [Run] and [Registry] from the install() # rules the project already declares, so nothing here repeats them. What is added is only @@ -17,8 +17,10 @@ if(CMAKE_VERSION VERSION_LESS 3.27) message(FATAL_ERROR "COCKATRICE_WIN_INSTALLER=InnoSetup needs CMake >= 3.27 (found ${CMAKE_VERSION})") endif() -# CPack is allowed to be told -DCPACK_GENERATOR=... by the caller; do not list the -# generator twice when that caller already asked for it. +# 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() @@ -89,4 +91,13 @@ set(CPACK_INNOSETUP_SETUP_UninstallDisplayIcon "{app}\\cockatrice.exe") 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 index 2d5a2eeb1..27ef5c02f 100644 --- a/cmake/InnoSetup.code.iss +++ b/cmake/InnoSetup.code.iss @@ -14,6 +14,12 @@ const 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. @@ -147,20 +153,93 @@ 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; -// The 3.1.x client launches the installer with two switches that belong to the NSIS build: -// /R (silent reinstall) and /PID= (the process to wait for). Recording them from the log is -// how the first test of this installer answers the one open question about the migration: -// whether Inno Setup reaches this point at all with them on the command line. If it does, -// the updater keeps working across the switch; if it does not, Setup rejects them before -// running any script and the updater needs a version gate instead. +// 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; @@ -170,8 +249,9 @@ begin for I := 1 to ParamCount do begin Param := ParamStr(I); - if (CompareText(Param, '/R') = 0) or (CompareText(Copy(Param, 1, 5), '/PID=') = 0) then - Log('Legacy in-app updater switch: ' + Param); + 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 index bd691316e..80b7411c5 100644 --- a/cmake/InnoSetup.install.iss +++ b/cmake/InnoSetup.install.iss @@ -1,9 +1,11 @@ ; 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. Only sections the generator never emits may appear here: it writes -; [Files], [Dirs], [Icons], [Run] and [Registry] from the install() rules, and a second -; section of one of those is a compile error. Behaviour belongs in InnoSetup.code.iss. +; 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 @@ -21,3 +23,14 @@ Type: filesandordirs; Name: "{app}\Plugins" ; 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 dfb8c3d63..49f9f7547 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 ca55c7b41..08d747c19 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_update.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_update.cpp @@ -228,16 +228,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); @@ -275,19 +275,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)}); @@ -296,14 +312,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); });