* [Windows] Purge every runtime DLL on upgrade instead of a named few
A user updating from 3.0.2 to 3.1 got "abseil_dll.dll is either not designed
to run on Windows or it contains an error" and could not start the client.
abseil_dll.dll comes from vcpkg, not Qt. It rides into the install directory
on the "*.dll" glob in cockatrice/CMakeLists.txt, alongside libprotobuf.dll,
liblzma.dll and z.dll, and none of those were named in the upgrade purge. So
on an upgrade the old uninstaller's RMDir /r skipped whichever of them was
locked, the purge did not look for them, HasStaleRuntimeFiles did not look
for them either, and the installer reported success - having written the new
executables next to the previous version's protobuf runtime. Windows then
refused to load it.
The mask list was also already wrong where it existed: it matches zlib*.dll
while vcpkg ships z.dll.
Delete every top-level DLL instead of naming library families. The payload is
written after the purge, so a DLL the new build still needs is put straight
back; what the purge has to guarantee is only that nothing from the previous
build is left to be loaded next to the new executables. Enumerated families
cannot do that - every transitive dependency that arrives between two
releases is one more name to forget, and the three previous rounds of this
work each reasoned about Qt because that is what the reports named at the
time.
HasStaleRuntimeFiles collapses to the matching *.dll probe, so the abort
fires on any survivor rather than only on the families it was told about.
IsExistingCockatriceInstall stays on Qt6*.dll on purpose: it decides whether a
directory is ours at all, and a folder that merely holds some DLLs of its own
must not be purged. Qt6*.dll is enough even when only the protobuf runtime
survived, because the whole Qt set is mapped by the same process that maps
abseil_dll.dll and so is locked or free with it.
* [CI] Assert the whole Windows runtime, not just the Qt modules
required-qt-runtime.txt listed only the Qt modules COCKATRICE_QT_MODULES
names, so it could not see any other runtime go missing. The three previous
rounds of installer work all reasoned about Qt*.dll because that was all the
build knew how to ask about, and the one runtime that actually broke the
reported upgrade - abseil_dll.dll - was outside the set entirely.
- Rename it to required-runtime.txt and take the third party runtime names
from the vcpkg bin directory the app-local copies come from, so a
dependency bump cannot leave the list behind and a versioned name such as
libcrypto-3-x64.dll becoming libcrypto-4-x64.dll cannot either
- Union the three applications' module lists rather than the client's alone:
Servatrice contributes Qt6Sql.dll, servatrice.exe is installed next to
cockatrice.exe and cannot start without it, and nothing asserted it
- Add Core to the client's module list. It is the one module nothing can do
without and it only ever arrived transitively, so Qt6Core.dll was shipped
and never checked - the new unknown-runtime check flagged it on the first
run
- Fail the build on a missing runtime, as before, and warn on a runtime
nothing requires. Only the test framework is a hard failure there: "no
shipped executable links gtest" is a fact, whereas a runtime missing from
the manifest may be one the manifest failed to derive, and a check that has
never run before should not be able to block a release
The warning is meant to be read. Qt6::Quick pulls in Qml, QmlModels,
QmlWorkerScript, QuickBasic, QuickTemplates2 and OpenGL transitively and none
are named, so the manifest is known to be a partial list; the first Windows
run reports the real one, which is evidence rather than a guess about what Qt
links. Fold that back in when it lands.
* [CI] Stop deriving the required runtime from the vcpkg bin directory
The first Windows run of this check failed, and the seven files it named as
missing are not runtime at all: gmock.dll, gmock_main.dll, legacy.dll,
libprotobuf-lite.dll, libprotoc.dll and pthreadVC3/VCE3/VSE3.dll. None of them
is linked by any shipped executable, so a correct package leaves them out.
vcpkg_installed/x64-windows/bin is everything vcpkg installed, not everything
the applications import. The protoc compiler, the GoogleMock library, a port
tool and three CMake FindThreads probes sit beside the genuine runtimes, and a
directory listing cannot tell them apart. Asserting it fails the build over a
package that is right - which is what happened, and it is the same mistake as
the one that produced this bug report: reasoning about a list of names instead
of about the build.
The real source of truth is the three executables' import tables, which
fixup_bundle already walks to decide what to copy. Until CMake can read that
result, the Qt half is what can be derived honestly, from the same module lists
the targets are built against. So assert that, and report the rest rather than
require it:
- Drop the vcpkg bin glob, its gtest filter and the vcpkg-runtime-known marker.
All three existed only to support the glob.
- Report top level DLLs the manifest does not name without failing. These are
expected: protobuf, Abseil, OpenSSL, liblzma, ffmpeg and the Qt debug DLLs all
arrive on the build output glob because nothing in the build declares them by
name. The list is a report of what the package carries, not a verdict, and it
is how the missing Qt6Qml/Quick/OpenGL entries above became visible.
- Report a leaked gtest or gmock instead of failing on it. "No shipped
executable links this" is a fact, but a check that has never passed on a real
package has not earned the right to block a release, and the first run of an
enforcing version is exactly when its own wrong assumptions show up.
- Skip DLLs under a subdirectory, which are platform plugins rather than
imported runtimes, and the NSIS plugins, which extract to $PLUGINSDIR and so
reach the comparison looking like payload.
Verified against the paths 7-Zip actually listed in the failing package: the
check passes on it, still fails when Qt6Sql.dll or the qwindows plugin is
removed, and still reports gtest.dll and gmock.dll if either is added.
The purge in cmake/NSIS.template.in covers the runtimes the manifest does not
name. That fix is independent of any list and is what makes the reported update
failure go away.
* [Installer] Fix the /R upgrade aborting before it does anything
WaitForLauncherToExit read the wrong registers. System.dll writes its
.rN outputs to $N and its .RN outputs to $RN, but the wait used
p.r0/i.r1 while testing $R1. $R1 therefore stayed empty, and the "did
not exit" test compared "" with "0" - true every time. Every in-app
update passes /PID=, so every upgrade hit AbortInstall before writing
anything; because /R also sets silent, the override just vanished with
exit code 1 and no message. The bug came in with #7372 and is inherited
by this branch, not introduced by it.
Use the uppercase .RN forms, and treat a null or plugin-failed
OpenProcess as the launcher already being gone - the case the old
${Errors} check never caught, because System.dll does not set the NSIS
error flag from the callee's return value.
Also restore the abort dialog for the in-app updater: AbortInstall and
the two inline warnings gated their message box on ${IfNot} ${Silent},
which is false for a /R upgrade, so any abort on that path was equally
silent. The unattended case is a /S install without /R, not /R, so show
the box for $ReinstallMode = 1 or an interactive install. The
stale-runtime abort now goes through AbortInstall so that decision
lives in one place.
* [Installer] Log every upgrade step to %TEMP%\Cockatrice-installer.log
* [Installer] Actually append to the upgrade log (FileSeek END)
* [Installer] Wait for the old uninstaller before replacing files
An NSIS uninstaller copies itself to %TEMP% and runs from there so that it
can delete its own directory; the process the installer started exits
immediately. A plain ExecWait therefore returns before the uninstall has
run, and the temp copy's "RMDir /r $INSTDIR" then executes concurrently
with the install section, deleting files the payload just wrote. That is
why an upgrade could leave Qt6Core/Gui/Multimedia and the other
alphabetically-first DLLs missing while the tail of the directory survived:
a recursive delete marching from the front while the final listing ran.
Pass _?=$INSTDIR to the old uninstaller so it runs in place and ExecWait
actually blocks until the uninstall completes. _?= must be the last
argument and unquoted, even for paths with spaces. Applied to both the
silent /R path (AutoUninstallIfNeeded) and the interactive path
(componentsPagePre).
* [Installer] Tighten the wording of the upgrade comments
Drop the dash-heavy phrasing from the comments this PR adds, and say plainly
in the _?= comment that ExecWait returns before the old uninstaller has run
and the temp copy then deletes the payload the section is still writing.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Installer] Abort instead of installing over leftover runtime files
The silent upgrade could leave a mix of old and new Qt DLLs next to the new
executables, which is what makes the next start fail with "The procedure entry
point X could not be located in the dynamic link library ...Qt6QuickWidgets.dll"
or "The code execution cannot proceed because Qt6Multimedia.dll was not found".
The check that was supposed to catch this probed only Qt6Core.dll and
Qt6Network.dll, so it was blind to the two files the reports name. It also only
warned and then installed anyway, so a caught problem was still compounded.
- Scan every file the purge targets with FindFirst, which takes the same
wildcards the Delete/RMDir calls use, instead of two hardcoded names
- Widen IsExistingCockatriceInstall from Qt6Core.dll to Qt6*.dll so a directory
left holding only some Qt modules is still recognised as an install
- Gate the scan on that, so a first-time install into a directory that happens to
hold unrelated Qt5 DLLs is not turned into an abort
- Abort with a non-zero exit code instead of warning, so the new payload is
never written next to files of the previous version
- Skip the message box on an unattended /S install, which would block on it
forever, matching the two existing abort sites
* [Installer] Cancel a silent upgrade when the running-instance guard fails
The running instance guard is what stands between a locked install directory
and the mixed Qt DLL set that breaks the next start, but it failed open in two
places and let the install carry on anyway:
- powershell.exe could not be started: the check warned and returned
- the process lookup answered "could not determine": WaitForAppToClose returned
on $R0 != 1, so "unknown" was treated exactly like "not running"
On a silent upgrade there is nobody at the keyboard to close the applications in
response to that warning, which is precisely the state in which the launcher was
still holding its Qt DLLs mapped.
- Treat "could not determine" as its own state rather than folding it into "not
running", in all three places WaitForAppToClose inspects the result
- Cancel a silent upgrade in both cases; keep an interactive install on the
existing warn-and-continue path, since refusing there would leave an installer
that can never complete and the stale-runtime check still applies
- Route every abort through one AbortInstall so the dialog-vs-silent decision
lives in a single place, and stop the two pre-existing warning dialogs that a
/S install without /R would have blocked on forever
* [Installer] Wait on the process that launched the update installer
The in-app updater starts the installer and only then quits itself, so the
installer has to decide whether the launching process is still alive while it is
running. It did that by polling powershell.exe for an image name under the
install directory, which is a proxy for the real question - is the process that
started me still holding its Qt DLLs mapped - and a proxy that can answer "not
running" or "could not determine" while the process is very much alive. Either
answer let the install carry on into the mixed Qt DLL set.
Take /PID= from the command line and wait on that process handle directly:
- OpenProcess with PROCESS_SYNCHRONIZE only, which is all that is needed to wait
and the least likely to be refused for an unrelated reason
- WaitForSingleObject for the same 60 s the name based check allows, so a client
slow to flush its card database is not squeezed
- Treat an unopenable handle as "already exited", which is what it means for a
dead process
- Cancel the upgrade if the launcher has not exited, rather than replacing files
it is still using
Run it before EnsureAppsNotRunning, not after: it needs no powershell.exe, so it
is the one check that still works on a machine where the process lookup cannot
run, and it is what lets that case keep upgrading instead of being cancelled by
what was only ever a proxy for it. Keep an older client, which passes no /PID=,
on the existing path.
An older installer ignores /PID=, and /D= stays the last parameter either way.
* [Client] Pass our process id to the update installer
The updater starts the installer and only then quits itself, so the installer
begins replacing files while this process still has every Qt runtime DLL mapped,
and a mapped DLL cannot be deleted. It has to work out that it must wait, and
until now all it had was its own image name to go on.
Pass /PID= so it can wait on this process directly instead of guessing, and
cancel the update rather than replace files this process is still using. /D=
stays the last parameter, which NSIS requires.
* [CI] Fail the build when the installer is missing a Qt runtime DLL
The Windows payload is assembled by globbing every *.dll out of the build output
directories, so nothing but luck decides whether a module the client links
statically - Qt6Multimedia.dll, Qt6QuickWidgets.dll - reaches the installer at
all. A module that is left out is invisible until a user installs an update and
the client then refuses to start, with "The code execution cannot proceed because
Qt6Multimedia.dll was not found" or a missing procedure entry point in a
Qt6QuickWidgets.dll carried over from the previous version.
Derive the required list from SERVATRICE_QT_MODULES, COCKATRICE_QT_MODULES and
ORACLE_QT_MODULES, the same module lists the targets are actually built against,
so it cannot drift from what the binaries need, and write it next to the CMake
cache as required-qt-runtime.txt. Add the platform plugin, without which the
client cannot start at all.
Assert it in the existing 7-Zip pass that already rejects build-tree artifacts,
comparing exact relative paths so a same-named file in the wrong directory does
not satisfy a requirement, and normalizing the archive's own separators so it
does not matter which one NSIS chose. Missing manifest, missing entry or missing
7-Zip-independent failures all stop the build rather than warn, which is the
whole point: the previous check already failed hard for artifacts, and a warning
here is what let a broken package ship.
* [Client] Point the Windows qt.conf translations path at the real one
The Windows qt.conf carried Translations = Resources/translations, copied from
the macOS bundle block, but the Windows install has no Resources directory: the
.qm files are installed to COCKATRICE_WIN32_QM_INSTALL_DIR, which is
translations, and main.cpp looks for them at applicationDirPath() +
"/translations". Leave the macOS value alone, which is correct for it.
* [Installer] Close the /? help string so the installer compiles again
The last line of the continued MessageBox string ended in $\n\" instead of $\n".
NSIS reads \" as an escaped quote, so the string never terminated and makensis
rejected the script, which took the CPack packaging step down with it. The
Windows build failed on "Problem running NSIS command" before ever reaching the
new required-runtime check.
* [Installer] Give FindFirst the two arguments it takes
FindFirst takes the pattern and the output variable and leaves the search
handle in $0. There is no handle argument, and FindNext taking only a variable
is the giveaway: the handle has to live somewhere implicit, and it is $0.
The call passed three arguments, so makensis rejected the script and the CPack
packaging step failed with "Problem running NSIS command" before ever reaching
the required-runtime check.
This was not only a compile error. The handle variable was never populated, so
the stale-runtime check that commit 844a1a9b added could never see a match: it
probed $StaleFindHandle, which stayed empty, and the widened wildcard scan over
the purge set was a no-op. Pass the pattern, test $0, and close $0. No FindNext
is needed, because $R0 is a yes/no flag rather than a list.
Drops the now unused StaleFindHandle Var.
* [CI] Dump the NSIS compile log when packaging fails
CPack reports a failed makensis run as "Problem running NSIS command" plus a
path to NSISOutput.log, and that log exists only on the runner, so the actual
compile error never reaches the workflow output. An installer script that does
not compile is therefore undiagnosable from CI alone, which is exactly how a
three-argument FindFirst cost two build cycles here.
* [CI] Run the NSIS log dump under bash
The dump step used the default pwsh shell, where `find` resolves to Windows'
text-search find.exe rather than a file finder, so it died with "FIND:
Parameter format not correct" instead of printing anything. Pin shell: bash.
* [Installer] Test the error flag with LogicLib's ${Errors} condition
makensis rejected the script with "macro _If requires 4 parameter(s), passed
2" at the ClearErrors/OpenProcess pair. A ${If} condition cannot be a bare
$Var: LogicLib needs either a comparison or one of its flag macros, and a lone
$Errors splits into too few arguments for _If. ${Errors} is the documented flag
condition and is the same mechanism as the ${Silent} test already used here.
The dump step earned its keep - this took one run to locate rather than another
round of guessing.
* [Installer] Give FindFirst its handle back and stop blocking silent aborts
makensis: "FindFirst expects 3 parameters, got 2. Usage: FindFirst
$(user_var: handle output) $(user_var: filename output) filespec".
The previous commit was wrong about this. FindFirst does take the handle
explicitly, and FindNext finds it in $0 as a side effect of the FindFirst that
set it, which is why FindNext takes only a variable. The three-argument form the
stale-runtime check was written with was correct all along, and the two-argument
form broke it. Restored, along with Var StaleFindHandle.
That same commit inverted the interactive check in both abort paths.
`${If} $ReinstallMode = 1` with `${OrIfNot} ${Silent}` shows the message box
exactly when the install is the silent /R upgrade - the one case the comment
says must never block, since nothing will dismiss it and the installer hangs
until it is killed. Replaced with `${IfNot} ${Silent}`, so the box appears only
for an interactive install and a silent abort goes straight to SetErrorLevel.
Audited every construct added by this branch against master, which compiles:
all 9 LogicLib condition shapes have a master precedent, FindFirst is the only
FindFirst and is back to 3 arguments, Function/Section and block depth balance,
and the 12 lines with an odd number of quotes are the same pre-existing string
continuations master has.
* [CI] Strip the CR 7-Zip leaves on its paths before comparing them
The installer check reported all 14 required files missing, including
Qt6Core-era basics an 82 MB package obviously contains, so the payload was fine
and the comparison was broken. 7-Zip writes CRLF on Windows, sed 's/^Path = //p'
kept the \r on every line, and grep -qxF matches whole lines - so every entry
carried a trailing \r and nothing matched. The build-tree artifact check next to
it uses an unanchored grep -E and was unaffected, which is what made the two
checks disagree.
Reproduced locally against a 7z archive with CRLF injected: the old pipeline
matches nothing, the new one finds the present files and reports only the
genuinely absent one.
Also match case-insensitively. That is how the installed tree behaves - the
uninstaller clears both $INSTDIR\plugins and $INSTDIR\Plugins - so a
Plugins vs plugins difference in the archive is not a missing file.
* [CI] Print both sides of the installer path comparison, and strip its prefixes
The check still reported all 14 files missing after the CR fix, so the CR was
not the cause and the real difference between 7-Zip's output and the manifest
is still unknown. Inferring it again is what produced two wrong fixes in a row,
so on failure the check now prints the required list and the first 40 paths
7-Zip lists. A bare "missing" line cannot distinguish a missing file from a
misread listing, and twice it was the latter.
Also strip $INSTDIR/, $OUTDIR/, $PLUGINSDIR/ and ./ prefixes from the listed
paths. Written as separate -e expressions rather than one alternation: with |
as the sed delimiter an escaped \| is a literal pipe, so the alternation
version silently matched nothing at all.
Verified against a 7z archive fed through seven path shapes - plain, Windows
backslashes with CRLF, each prefix, lowercase plugins, and all of them combined
- each reporting only the file genuinely absent from the archive.
* [CI] Strip CR from the manifest side of the installer path check
The dump added last run settled it: 7-Zip listed 209 paths including Qt6Gui.dll,
Qt6Multimedia.dll and Plugins/platforms/qwindows.dll verbatim, as plain relative
paths with no prefix - so the archive side was already clean and the previous
fix was right to strip CR there but not sufficient.
The trailing CR was on the other side. CMake writes required-qt-runtime.txt
with the platform line ending, so on Windows $entry arrived as
"Qt6Gui.dll\r", which cannot whole-line match a clean "Qt6Gui.dll". Every
entry then read as missing while the listing held it, which is precisely what
the two sides of the dump showed.
Reproduced locally with a CRLF manifest against a 7z archive: without the strip
every entry is reported missing, with it only the genuinely absent file is.
Checked across plain paths, Windows backslashes with CRLF, $INSTDIR, $OUTDIR,
$PLUGINSDIR, ./ and / prefixes.
The literal $INSTDIR-style prefixes are written as [$] bracket expressions so
shellcheck does not read them as unexpanded variables; that clears the SC2016
that failed the format job, without a disable directive.
* [CI] Reword the bracket-expression comment so the linter stops parsing it
A comment whose first word is shellcheck is read as a directive, so
"# shellcheck does not read these as unexpanded shell variables" was parsed as
a directive named "does" and failed with SC1073 and SC1072 - which cascaded into
the SC1009 on the else clause above it. The SC2016 it was meant to document is
already gone now that the prefixes are written as [$] bracket expressions, so
the note only had to stop looking like a directive.
* [CI] Indent the diagnostic path lists without sed, and undo comment drift
SC2001 asks for parameter expansion instead of sed 's/^/ /'. Echoing
${manifest//$'\n'/$'\n '} prepends the two spaces directly and keeps the
diagnostic block readable. Both occurrences are converted; only the one reading
from a here-string was reported, but the piped one has the same shape.
Also restore master's wording of the build-tree artifact comment. An earlier
edit inserted "in " ("not the install)" became "not in the install)"). It has no
functional effect, but it was an unintended change to a line this branch has no
reason to touch, and .ci/compile.sh now differs from master only by additions.
* [CI] Ship the Qt module runtimes the Windows package check requires
The installer check reports Qt6Concurrent.dll and Qt6ShaderTools.dll missing
from the Windows package. Both are genuinely absent, not another comparison
artifact - the manifest now matches 12 of 14 entries and the dual-list
diagnostic shows the other two are not in the archive at all.
Nothing deploys them. fixup_bundle copies only the DLLs it finds in
Cockatrice.exe's import table, and the *.dll glob only sees what was already
copied next to the executable. Qt6::Concurrent is linked but never imported,
because QtConcurrent::run and QFutureWatcher are header-only in Qt 6, so the
linker drops the import; Qt6::ShaderTools is skipped the same way. The package
has shipped without them and the client still starts, but that is luck rather
than intent, and required-qt-runtime.txt asks for both.
Install the runtime of every linked module explicitly so the installer contains
what the check demands. An unused DLL costs a few hundred kilobytes; a missing
one is what stops the client starting after an update, and stays invisible until
then.
Derive both the manifest and the install list from the imported target's TYPE,
so a statically linked Qt module - which ships no runtime at all - is skipped by
each and the two can never disagree. Scope the manifest to the client rather
than the union across server, client and Oracle: only the client is packaged on
Windows, and the other two module lists are subsets of the client's, so this
changes nothing today.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Windows] Close running instances and purge stale runtime DLLs during update
* [Windows] Scope running-instance handling to install dir and fix NSIS build
- Drop !include nsExec.nsh: nsExec is a plugin DLL, not a header, so
makensis aborts before NSIS can build the installer.
- Match processes by image name and executable path under $INSTDIR via
PowerShell, then close (WM_CLOSE) and force-stop only those PIDs, so an
unrelated oracle.exe (Oracle DB) is never killed on a silent /R update.
- Gate the stale-runtime-DLL purge on $INSTDIR\cockatrice.exe existing, so
a first-time install can't recursively delete an unrelated Plugins
directory.
* [Windows] Make process detection rely on an explicit PowerShell exit code
* [Windows] Fix exit-code detection in the uninstaller process check
* [Windows] Pass the install dir to PowerShell without quoting pitfalls
* [Windows] Fail closed when a process path is unreadable
* [Windows] Only pre-check running instances for /R updates
* [Windows] Wait on a wall-clock 60 s deadline instead of a loop counter
* [Windows] Verify the force close and abort if the process survived
* [Windows] Defer silent updates instead of force-killing the client
* [Windows] Purge OpenSSL 3 DLLs on portable upgrades and broaden the stale-DLL guard
* [Windows] Replace the GetTickCount macro with a direct System::Call
* [Windows] Only close running instances whose path is readable
CloseMatchingApps reused the fail-closed detection filter, which counts a
process whose Path cannot be read (Access denied) as a match. The installer
runs elevated, so CloseMainWindow() would then be delivered to an unrelated
oracle.exe / servatrice.exe belonging to some other program, silently closing
a third party application during a Cockatrice update.
The set of processes we act on now has to be the strict one: a readable path
under $INSTDIR. Unreadable paths are simply not ours.
* [Windows] Match running instances by a readable path under the install dir
The Path read in the Where-Object filter raises "Access is denied" for
processes the installer cannot open, and both branches of the try/catch
turned that into a match. Together with mapping every non-zero nsExec result
to "running" this made the guard a hard stop rather than a fail-closed one: a
machine with an Oracle database service (or a servatrice service) running as
SYSTEM always has a process named oracle / servatrice whose path cannot be
read, so every silent update aborted after 60 s and every interactive install
re-asked for Retry indefinitely.
A process whose path cannot be read is not one of ours - the updater is
launched by the application it updates and therefore runs in the same user
session, and an interactive install is elevated and can read the paths of the
user's own processes. Both the detection and the close path now require a
readable path under $INSTDIR, which is what the comment above the helpers
already claimed.
* [Windows] Distinguish "could not determine" from "definitely running"
Mapping every non-zero nsExec result to "running" folds nsExec's "error"
return (powershell.exe could not be started at all - removed, blocked by
policy, refused by antivirus) into "Cockatrice is running". That state can
never clear: closing applications does not help, so silent installs aborted
after the full 60 s wait and interactive installs could not get past the
Retry/Cancel prompt.
IsAppRunning now reports three states (0 not running, 1 running, 2 could not
determine) and a new IsProcessLookupAvailable preflight checks that
powershell.exe can run at all. A broken PowerShell is reported to the user
once - with a message box, since a silent install would otherwise show
nothing - and the install continues instead of being blocked forever. Only a
definite "running" is now waited for.
* [Windows] Force-stop the console applications after the grace period
Servatrice and Oracle are console applications, so CloseMainWindow()
returns false and does nothing for them. Every silent update therefore
waited the full 60 s and then aborted, and every interactive install looped
on the Retry prompt, for any user running Servatrice out of the install
directory.
They now get a second shutdown path: once the grace period is over they are
force-stopped by PID with the same strict install-dir filter, since they hold
no unsaved user data. Cockatrice keeps the graceful-only treatment - a
force-kill there could destroy an unsaved deck - and still aborts instead.
* [Windows] Tell the user when a silent install or uninstall is aborted
On the /R path the installer runs with SetSilent and SetAutoClose, so the
DetailPrint before Abort went nowhere: the user clicks Help -> Check for
Client Updates, the download finishes, Cockatrice closes its window, and
nothing else happens - no update and no error, because the client launches the
installer detached and never looks at its exit code.
Every silent abort path now shows a message box telling the user which
application is still open and that they should close it and run the
installer again. The same applies to the silent uninstall, where the only
output is a log file nobody reads.
* [Windows] Treat leftover runtime files as an install marker for the purge
The guard on the stale-DLL purge only looked for the three executables and
uninstall.exe, so it skipped precisely the case the purge exists for: a
previous failed update whose uninstaller removed what it could and silently
skipped the Qt DLLs that a running instance had locked. None of the four
markers is left in that directory, and the stale DLLs then survive into the
new install.
A leftover Qt6Core.dll (what we ship today) or the qt.conf written by a
portable install now count as an install marker as well.
* [Windows] Run the section instance check before any file is deleted
Call EnsureAppsNotRunning sat after the portable-upgrade block, which had
already deleted the old cockatrice.exe, Qt*.dll and plugins directory. An
interactive install ("Portable mode" pointing at an existing portable folder)
has $ReinstallMode = 0, so the .onInit guard is skipped and locked files were
silently skipped - exactly the mixed-Qt-DLL state this change is about.
The check now runs right after SetOutPath, before the first Delete/RMDir in
the section.
* [Windows] Guard the old install before the old uninstaller deletes files
The interactive upgrade path removes the previous installation long before the
install section: componentsPagePre runs the old UninstallString on the
components page, i.e. before the directory page and before any call of ours
can check anything. Every uninstaller shipped before this change has no
un.EnsureAppsNotRunning, so for exactly the versions affected by #1576 the old
files were deleted while cockatrice.exe was still running and the new
installer had no say in it.
componentsPagePre now runs the same check before both ExecWait calls. At that
point $INSTDIR is still only the default, not the directory the old install
lives in, so the new EnsureOldInstallNotRunning guards the location recorded
in the registry instead. The helpers now work on a $GuardDir the caller
picks; the normal entry points keep seeding it with $INSTDIR.
* [Windows] Do not abort the section check past the point of no return
"nothing has been replaced yet, so aborting just defers the update" only holds
for the guard in .onInit. On the /R path AutoUninstallIfNeeded has already run
the old uninstaller, which RMDir /r's the whole install directory, and the
purge below it deletes the runtime files - so an Abort from the section would
leave the user with the old version uninstalled and the new one not installed
at all, which is strictly worse than the bug this change fixes.
The ordering now guarantees the check already passed: .onInit guards the /R
path before the uninstall, so the section only repeats it where that is not
already true, i.e. for interactive installs, where nothing of this installer
has been deleted yet and an Abort is the correct outcome.
As a safety net for whatever the purge could not remove (a still locked file,
or a check that could not run at all), HasStaleRuntimeFiles reports surviving
runtime files with a message box - also on a silent update, where there is no
other way to let the user know the install came out incomplete.
* [Windows] Only block the in-app updater with a message box
The message box on the silent abort would also fire for a plain /S install,
which is exactly the unattended case: a package manager (or a CI job) running
the installer with a leftover Cockatrice process would now hang forever on a
dialog nobody can see instead of aborting.
The box is therefore limited to /R, the path the in-app updater takes, where
the user just clicked Help -> Check for Client Updates, the client launches
the installer detached and never inspects its exit code, and the window is
about to disappear - without a message, the update would silently not happen.
Every silent abort also sets an error level now, so an unattended caller can
see that the run failed instead of having to read the log.
The silent uninstaller reports through log and exit code for the same reason:
it is run unattended by the /R updater itself (QuietUninstallString) and by
package managers. The installer that invoked it tells the user about the
incomplete result itself.
* [Windows] Treat an unset guard directory as undeterminable
$GuardDir is set by the three entry points into the check, but it is now what
decides whether a process counts as one of ours, and an empty string would
match every process that merely shares an image name - the same false positive
the strict path filter exists to avoid. Report it as "could not determine"
instead of running the check, so a future caller that forgets to set it warns
the user rather than closing or killing a third party process.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Build] Keep Windows installs free of build-tree artifacts
Several Windows packaging gaps could leak Visual Studio CMake build
output into the installed application or the NSIS installer:
- The per-app DLL sweep used
${CMAKE_BINARY_DIR}/${PROJECT_NAME}/${CMAKE_BUILD_TYPE}, which is
empty on multi-config generators, collapsing the recursive
DIRECTORY install into the whole build tree (containing *.dir,
*_autogen, .qt, .qsb, x64, ...). Point it at the real per-config
output with $<TARGET_FILE_DIR:...> and exclude build artifacts.
- install(FILES ${OPENSSL_INCLUDE_DIRS} ...) tried to install OpenSSL
include directories as files. CMake refuses this
("install FILES given directory"); it only slipped through CI
because the vcpkg OpenSSL config leaves the variable empty. Remove it;
fixup_bundle already ships the OpenSSL runtime DLLs.
- The NSIS uninstaller only deleted *.exe/*.dll and a few known files,
so build-tree leftovers survived an uninstall/reinstall cycle. Wipe
the whole directory tree instead.
- Add a Windows CI gate that lists the packaged installer with 7-Zip
and fails the build if any build-tree artifact path is found.
* Update .ci/compile.sh
Co-authored-by: tooomm <tooomm@users.noreply.github.com>
* [Build] Rework Windows installer artifact exclusions per review
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
Co-authored-by: tooomm <tooomm@users.noreply.github.com>
* [CI] Remove Qt5
Took 10 minutes
Took 9 minutes
* Revert CI failure and fix up comments
Took 4 minutes
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Application] Add single instance guard and mime types.
Took 2 hours 39 minutes
Took 18 minutes
Took 5 minutes
Took 12 seconds
Took 11 seconds
* Rework
Took 30 minutes
Took 50 seconds
* Only enforce single instance if launched with arguments.
Took 5 minutes
* Prototype intents
Took 53 minutes
Took 6 seconds
* Connect/disconnect and join game/room intents.
Took 3 hours 14 minutes
Took 2 seconds
Took 15 seconds
* Fix include.
Took 1 minute
Took 23 seconds
Took 2 seconds
* Mac handling.
Took 10 minutes
Took 12 seconds
Took 3 minutes
* Lint.
Took 3 minutes
* Rebase.
Took 3 minutes
Took 17 seconds
* Implement UrlSchemeEventFilter
Took 10 minutes
Took 7 seconds
* Qt Moc
Took 3 minutes
* Modern PList.
Took 21 minutes
Took 1 minute
* Debug output.
Took 6 minutes
Took 19 minutes
* Watch file:// prefix.
Took 15 minutes
Took 7 seconds
* Better handler.
Took 6 minutes
* Don't store reference in member
Took 5 minutes
* Move impl to cpp, fix lifetime issues.
Took 11 minutes
Took 2 minutes
* Better single-instance handoff, url intent harded
copy game link context-menu
Polish for installers
Took 35 minutes
Took 8 seconds
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* Add -R option in Windows NSIS script
Took 23 minutes
* Small fix.
Took 3 minutes
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* Portable mode for windows
* fix start menu in portable mode
* Make gcc an happy puppy
* Clean old installation if we are installing over an old portable mode installation
* Default to Desktop\CockatricePortable
* Settings dialog fixes
* wording
* Change version in about window and installer name
* Change version in about window and installer name
* Cmake get version updates
* dev snapshot format changed
* alignment changes
* main.cpp: removed path checking and db loading
* card database: merge card loading methods into a single one
* settings cache: take care of returning safe paths for decks,
replays, etc..
* main window: if db loading fails (eg. first run), propose to run
oracle
NSIS: propose to run cockatrice instead of oracle
Rework card database loading
* Move carddatabase-related method out of deckeditor tab
* Load cards in another thread and render them progressively
* Optimize database reload after enabled sets change
Fix deck editor column width
* removed the noCard hack.
* getCard() no more creates cards instead of just returning existing
ones
* Fix the “edit tokens” dialog.
* PictureLoader: avoid trying to download twice the same card
* PictureLoader: correct return of card background
* AbstractCardItem: avoid recalculating card color at every paint
Use a different file to save custom tokens
Misc required improvements
* Use nullptr;
* Refactor CardInfoWidget to use CardInfoPicture and CardInfoText
instead of duplicating code;
* Added CardInfo::getColorChar()
* Fixed some potential crashes
* removed dead code related to CardInfoWidget
* Don't require a restart after adding a new custom sets file
* Bump CMake requirements to 3.1
* uninstaller: delete zlib1.dll (first issue in #876)
* require the user to uninstall the old version before installing the
new one
* use project-version as branding name (fourth issue in #876)
Remove argument that if added by default with out having the proper configuration defined during compiling would cause cockatrice to not open properly.
* Move all cmake-related files in a new “cmake” folder
* move nsis files in there, too, since they are templates parsed by cake
* retrieve git version once when cmake is run, and use it for both
creating version_string.h/cpp and for package naming
