147 commits

Author	SHA1	Message	Date
BruebachL	7ca697348b	[CI] Give ccache room to hold a configuration (#7667) ... * [CI] Re-assert the ccache timestamp sloppiness after configure CMakeLists.txt sets sloppiness=pch_defines,time_macros at configure time, and ccache --set-config replaces the whole list rather than extending it. Setting the four entries before cmake would therefore leave the build with only two, so the entries CI needs have to be written after the configure step and before the build. The mtime and ctime entries cover files created during the build itself: moc/uic/rcc output and protobuf headers are generated after ccache starts, so without them every TU that includes one is read as a race and never cached. Checked-out sources are older than the compile that follows them and do not trip the check. * [Build] Keep risky ccache sloppiness out of the global config USE_CCACHE defaults to ON, so CMakeLists.txt runs on a developer machine, and ccache --set-config writes the user's global ccache.conf. The include_file_mtime/include_file_ctime entries turn off the modified-during-compile check for every project on the machine, not only this one, so they belong in CI (compile.sh) and not here. The two entries PCH caching needs stay. * [CI] Size each compiler cache against the cache quota The 600M bound sat below what a leg writes: ccache spent each build deleting the entries the next run asked for, with 2624-3720 cleanup passes and about 8% of calls served in the runs inspected on 2026-11-04. Raising it needs room first. The repository's 10 GB quota currently holds 1.1 GB of CodeQL trap, 1.1 GB of vcpkg, 0.7 GB of Qt and 2.1 GB of docker buildkit layers. Two of those are now cheap to reclaim: - docker-release wrote its layer cache with mode=max, but the build stage sits behind COPY . ., so its compile layer is invalidated by every source change and was never read back. mode=min keeps only the final image stage. - the five nightly-only legs kept their own compiler caches, 600M each, that no pull request ever reads. They now keep none, and the prune job retires their leftover keys immediately instead of waiting for GitHub's whole-entry LRU eviction to pick them. That funds per-leg bounds: 700M for one configuration, 850M for the Debian 13 release that also runs the suite, 900M for Fedora 44's Debug and Release, 800M for the instrumented leg, 400M for the server-only leg, and 700M for each macOS leg. 6.45G of ccache plus the 3.2G of fixed entries leaves about 350M under the quota. The prune step also covers the buildkit entries by age now, so the mode=max generation of layer blobs is reclaimed instead of living forever: a layer a build needs is downloaded and its date refreshed, and dropping one that an index still lists only costs docker-release a rebuilt layer, which gates nothing. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-11-04 10:06:56 +01:00
BruebachL	3f7aa4ca82	[CI] Split the sanitizer leg from the Ubuntu packaging leg (#7666) ... The sanitize leg re-ran the Release packaging build after the instrumented one, so 18 minutes of Release compile queued behind the sanitizer run in a single job. Give sanitize its own leg with its own image and ccache, and keep the plain Ubuntu leg publishing the DEB. - The coverage leg's packaging comment now points at the leg that actually publishes the DEB. - The -DENABLE_SANITIZERS=OFF comment no longer describes a debug-sanitize then release-package sequence that no job runs. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-11-04 07:24:24 +01:00
BruebachL	5fe7f186d3	[CI] Make the hdiutil retry wrapper report failure (#7643) ... * [CI] Make the hdiutil retry wrapper report failure CPack reads the wrapper's exit status to decide whether the disk image was created, so a zero exit turns a failed hdiutil into a later "Problem copying the package" error that hides hdiutil's own message. - Propagate hdiutil's real status instead of collapsing every failure to 1. - Name the failing subcommand (create/attach/detach/convert) so the message says which CPack step gave up. - Count attempts rather than failures in the retry line. * Update .ci/compile.sh Co-authored-by: tooomm <tooomm@users.noreply.github.com> --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de> Co-authored-by: tooomm <tooomm@users.noreply.github.com>	2026-11-02 07:01:55 +01:00
sentry[bot]	bbc7e462ff	[Installer] Graceful close and a way out of a stuck update (#7687) ... * [Installer] Stop calling the payload free while an executable is locked The guard answered "nothing is holding the payload" from two lookups that can both miss a running instance: Restart Manager does not reliably report a program's own image or the modules it mapped, and the image path of a process running under another account cannot be read from the probe at all. A user log showed exactly that: the first guard passed while cockatrice.exe was running from the directory, the old uninstaller silently skipped the file it could not delete, and the update aborted half an uninstall later with "cancelled by user" after a close attempt the running window ignored. Ask the file itself. A running image is opened without write sharing, so the write open replacing it needs is refused with ERROR_SHARING_VIOLATION for as long as the program runs, while a file that is merely being read still accepts it. When neither lookup named anybody and a payload executable refuses that open, the probe answers that the file is in use and that the program holding it could not be identified - the guard's "could not confirm" dialog, whose Retry works once the user has closed it - instead of passing the payload as free. Only the payload executables are tested, not every file the purge removes: a locked executable proves a program is running and a mapped DLL belongs to a running executable, while a locked DLL or qt.conf can be the asking process itself, which is the loop the guard already had. The open is retried once after 300 ms, and only for the files that refused, so a scanner holding a file for a moment cannot stop an install for nothing. The Windows probe check now covers that answer: a payload executable held by the asking process, so that it cannot be named, has to come back as in use rather than as free. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de> * [Installer] Close running clients over a local socket The installer could only ask a running client to close with WM_CLOSE, which an instance hidden in the notification area or sitting on its own confirmation cannot answer usefully. Give every client a per-process local socket and have the holder probe ask it there first, falling back to WM_CLOSE for clients from before the socket. * [Installer] Offer to check again or update by hand when files stay locked Replace the two-answer guard prompt with force close, close by hand and check again, or cancel. Retry the automatic close while the set of holders changes, and point a cancelled run at the latest release so a user who does not want anything killed has a way forward that is not cancelling the update again. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-31 15:32:59 +01:00
sentry[bot]	0369aaffa9	[Installer] Name only the payload programs installed in the directory (#7684) ... The running-instance lookup named every process whose image path began with the install directory, subfolders included. Installing into a folder that holds another application in a subfolder therefore reported that application as holding the previous version, and nothing the guard offers could clear it: the close asks cockatrice.exe only, and the end answer refuses anything that is not a payload program. Both answers were no-ops, so the update could only abort and tell the user to close a program that was never touching the files. Name only the programs the payload installs, and only when the image lies in the install directory itself. A program below the directory that really does hold a payload file is still named by the Restart Manager lookup, which is the lookup for that question. The Windows probe check now covers a payload-named program in a subfolder and a program the payload does not install; both have to come back as nothing. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-30 13:16:50 +01:00
sentry[bot]	30d3afc7b4	[Updater] Close a running Cockatrice when the installer asks (#7672) ... * [Updater] Refuse a close that arrives while one is already asking MainWindow::closeEvent() had a re-entrancy guard that returned without touching the event. A QCloseEvent is accepted unless something ignores it, so a second close request - an installer replacing our files posts one every 30 s - accepted the close while the first one was still blocked on its own confirmation. close() then hid this window, hid the confirmation dialog with it, and left the process running inside the nested loop that dialog had opened: no window, no way to answer it, and every later close request accepted into the same dead end. That is what an update looked like when it could not close a running client, and it is why the installer could name the process, bring it forward and ask it to close for minutes without anything happening. Track how far the shutdown has got instead. While a confirmation is up, a close from anywhere else is refused, which leaves the question in front of the user; once the shutdown has been accepted, a duplicate close (Qt delivers the event twice on some platforms) is accepted again so it cannot cancel the quit. * [Installer] Leave an instance that is asking a question alone An instance that answered a close by hiding rather than quitting - the state the client used to get into - has no visible window left, and the confirmation it is waiting on is hidden with the window that owns it. The close probe only ever looked at visible windows, so it restored the main window, posted WM_CLOSE into the same dead end, and reported "still running" without saying why. Show every window the instance left hidden before choosing the one to close, and skip the close when the window it would reach is disabled: a modal dialog of the instance's own is up, and posting the close then only hides the question again. Name what is left of the window in the report as well - gone, still open, hidden again, and whether the thread answers at all - so the log tells a refused close from one that was never delivered. * [Installer] Offer to end a Cockatrice that will not close A close is a request, and an instance that does not answer it - waiting on a question nobody is looking at, or stuck where a close cannot reach - leaves the user with no way forward: the files cannot be replaced while it runs, and clicking Yes again only repeats the request. After a second close that changed nothing, offer the only thing that can free the files: end it. The prompt says what it costs - an unsaved deck, a running game or a setting it has not written yet - the default button is the safe one, and it is a separate answer rather than a relabelled "Yes", so it is not reached by clicking through the close prompt. Only cockatrice.exe is ever ended: Oracle writes the card database in place and Servatrice is a service, so both are named and left to the user, exactly as they are for the close. The probe gains an --end-holders mode for it, and the uninstaller's guard gets the same offer. * [Installer] Never name the process that is doing the asking The old uninstaller is started with _?= so it runs in place, from the very directory it is emptying, and an installer can be downloaded into the folder it installs to. Both are then processes running from the directory, so the probe named them - and the guard asked the user to close the program that was asking. That question cannot be answered: a close and the end offer both skip anything that is not cockatrice.exe, so the guard repeated itself until the update or the uninstall was cancelled. The probe and the processes that started it are now never named, by either lookup. The walk up the parent chain stops at the first program the payload installs, because in an in-app update the installer's own parent is the Cockatrice instance being replaced - the one the guard exists to name. The Windows check held its locked file in the script that starts the probe, so that holder is excluded by design now and the check read it as "nothing found". It holds the file from a process of its own instead, and asserts the exclusion itself: a file that only the asking process holds comes back as nothing. * [Installer] Restore only the windows a user could answer Showing every hidden window an instance owns put Qt's own bookkeeping windows on screen: the tray icon keeps one to receive the TaskbarCreated message, styled WS_OVERLAPPED and titled QTrayIconMessageWindow. Restored, it is an empty white rectangle in the taskbar with no title bar to close it - it has no window procedure that would hide it again, and the process is busy in the very loop the restore is meant to break, so it stayed until the process was force-ended. Only framed, non-empty windows are restored and considered for the close now. That also keeps the close off Qt's tray window, which answers WM_CLOSE by asking the application to terminate - a quit that skips the confirmation the close is supposed to reach. * [Installer] Uninstall the previous version before installing over it A regular install over an existing one asked "a previous version must be uninstalled", then ran the old uninstaller interactively. That uninstaller put its confirmation page behind the installer window, where it could not be answered, and it only removed the installation after the installer had already written the new one over it. Run the previous version's uninstaller silently instead. The installer has already asked the question the user answered, so a second, hidden confirmation is pure risk: with /S there is no window to be behind, and ExecWait is then waiting for something that cannot outlive it. QuietUninstallString is used when the old installation has one, otherwise /S is appended. ExecWait only waits while the uninstaller runs in place: one that did not get a usable _?= copies itself to %TEMP% and returns at once, while the copy keeps deleting - which is how an uninstall can finish after the install it was supposed to precede. Log the exit code and wait for the old cockatrice.exe to disappear before restoring user data or writing anything, and abort with a message instead of installing into a directory that is still being emptied. Cancelling the "must be uninstalled" prompt now quits the installer; it used to skip the page and install over the old version anyway. * [Installer] Do not show windows the application has hidden The guard restored every hidden window an instance owned before asking it to close, so that a confirmation hidden together with its window would come back. Windows does as it is told and shows them; the application does not, because Qt still believes they are hidden and never paints them. What the user gets is a blank white rectangle, on screen and on the taskbar, that answers nothing - and the question it was meant to bring back is not answerable in it either. An instance with no window a user could answer is named and left to the user instead, and the prompt says what to do about it: open it from the notification area. Ending it stays the last resort, offered once a close has been asked for twice. The abort after a failed uninstall now says what it saw - the old version is still there after its uninstaller ran - instead of promising a removal that is not happening, which is the state a cancelled old uninstaller leaves behind. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-27 11:19:49 +01:00
sentry[bot]	d42b9607c5	[Installer] Check for running instances with the Restart Manager probe (#7671) ... * [Installer] Check for running instances with the Restart Manager probe The running-instance guard was built on powershell.exe started from the 32-bit NSIS stub, which is the 32-bit Windows PowerShell: its Get-Process cannot read the path of a 64-bit process, so a running 64-bit Cockatrice was reported as not running, the update went ahead, and the purge then failed on the DLLs that instance still had mapped. Use tools/installer-holder-probe instead - the same Restart Manager lookup the abort path already had - before anything is replaced and in the uninstaller as well. It is bitness-agnostic, so the Sysnative and WMI fallbacks are gone. The guard looks only at the payload, asks Cockatrice to close once by itself (WM_CLOSE, the same request as the window's close button) and then offers close again / retry after closing by hand / cancel for whatever the close cannot reach - Oracle writes the card database in place, Servatrice has no window - without force-stopping anything. A failed purge offers the same close-and-retry, tries one last purge when nobody is named, and every dialog shows in every mode: a silent reinstall stays silent until something goes wrong. * [Installer] Name processes running from the install directory The running-instance guard answers from the Restart Manager, which reports who has a file open. A running instance does not have to have one open: its own image and the modules it loaded are mapped into it rather than opened by it, and a mapping is not reliably reported. That is the state the probe was added for - every DLL still on disk, an instance running from the directory, and "nothing is holding them any more" as the answer. A guard built on file handles alone is blind to exactly that instance, so it passes, the purge fails, the holder lookup after it names nobody either, and the install ends in a warning that no running program was found: no close is attempted and no retry is offered. A manual install with another instance open never asks it to close at all. Name every process whose image lives in the directory being replaced as well, with the same bitness-safe QueryFullProcessImageNameW the probe already uses to label its holders. The image lookup is listed first - it is the instance the user is asked to close - and the Restart Manager entries follow with the pids already named dropped, so the close still reaches everything that holds a file. A Restart Manager call that fails only makes the probe unable to answer when the image lookup named nobody either. Ask instead of dead-ending when the probe cannot answer at all: Retry checks again, Ignore carries on, Abort cancels, in the installer and the uninstaller alike, and the abort dialog says the check failed instead of claiming no running program was found. The Windows CI probe check covers the new lookup with a process started from the directory that holds no file open. * [Installer] Offer to wait again when the launcher has not exited A client that is still closing after 60 s is usually not hung: it is showing its own "quit anyway?" confirmation for an unsaved deck, an open game or a card database update, and the user is the one who has to answer it. The installer answered that with a plain warning and stopped, so the only way forward was to close Cockatrice by hand and run the update again - which on an in-app update means finding the downloaded installer. Ask instead: Retry brings the client to the front, asks it to close again (WM_CLOSE, nothing forced) and keeps waiting on its handle, Cancel stops as before. There is no "install anyway" answer because a live process has its image and Qt DLLs mapped, so writing the payload over them is the broken install this wait exists to prevent - and the guard that runs right after names the same process by its image path and stops there, so ignoring would only move the same question a moment later. * [Installer] Ask about a launcher that has not exited after ten seconds A minute of silence is a long time to sit in front of an installer that has no window yet, and the case it is waiting for is not a slow flush: a client that started an update exits in well under a second, so a minute means it is almost always showing its own "quit anyway?" confirmation with nobody looking at it. Wait ten seconds for the first question instead, and the full grace for the ones after it. The normal path is untouched - WaitForSingleObject returns as soon as the handle signals, so a client that exits in 200 ms still costs 200 ms - and answering the first question once does not turn into a stream of prompts while the user is in Cockatrice's window. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-26 19:25:10 +01:00
BruebachL	bbc6003f0a	[CrashReporting] Name the files and processes behind a failed installer purge (#7590) ... * [CrashReporting] Log which files and processes blocked the installer purge A purge that fails aborts the upgrade with a message that can only guess at the cause, and the log only listed which DLLs survived. Record what Windows reports for each survivor and which processes still have the install directory mapped, then repeat the holder list in the abort message. The diagnostics are additive and run only on the abort path: they open each survivor for DELETE access and close it again without deleting, so a failed upgrade can never end up half-removed. The purge itself is unchanged. * [CrashReporting] Name the holders of a failed purge with Restart Manager The process-and-module scan could only name a holder whose image lives in the install directory or that loaded one of its DLLs, so the case this diagnostic exists for - an antivirus scanner holding a plain handle - was reported as "no holder named, only a code". Reaching the Restart Manager from the NSIS script would have meant marshalling RM_PROCESS_INFO through System::Call as a table of hand-computed offsets, which cannot be checked without a Windows toolchain, and that is why it was dropped when this diagnostics was written. Move the call into tools/installer-holder-probe: a small executable the same Windows build compiles, linking nothing from this project so it still starts when the installed runtime is what the purge could not replace. It rides inside the installer and is extracted to $PLUGINSDIR only when the purge has already failed, so the successful path is unchanged and nothing new is installed. .ci/compile.sh runs it against a file held with FileShare.None on the Windows runners, so the lookup is exercised on real Windows instead of only compiled. * [CrashReporting] Fix the holder probe check and cap the holder line The probe block ran after the package steps, where the working directory is already the build dir, so its $BUILD_DIR-prefixed paths named build/build and the PowerShell file could not be written. Move it ahead of install/package, where the build dir is still the cwd, and name both files relative to it. Guard it on MAKE_NO_CLIENT, since the probe is only built with the client. Cap the rendered holder line at HOLDER_LINE_CHAR_MAX: the count limit alone does not keep long service names inside the NSIS string that also carries the abort text and the log path. Whole entries are kept and the rest counted, with the first holder always shown. Also run cmake-format on the probe's CMakeLists so the lint job accepts it. * [CrashReporting] Report a read-only purge survivor and bound the probe check --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-24 00:46:54 +02:00
BruebachL	9f59f5c495	[CrashReporting] Breakpad capture, session detection and symbol upload (#7523) ... * [CrashReporting] Breakpad everywhere, session detection, symbol CI (inc 3) Breakpad on all platforms (no crashpad packaging); remove the Windows MiniDumpWithFullMemory filter and the installer WER LocalDumps full-dump registration. QLockFile per-process session tracking replaces the Windows-only .dmp scan, placed after parser.process and the instance hand-off so help/version exits and handed-off secondaries never leak a lock. CI: Release-only DSN bake from SENTRY_DSN secret, tag-only sentry-cli symbol upload (org cockatrice, project native). Tests: 533/533 ctest green, format clean, code gate PASS. * [CrashReporting] Enable http_retry, harden symbol upload, fix NSIS context Sets sentry_options_set_http_retry so the cache_keep-held crash envelope in Ask mode is written with a retry filename and actually flushes on giveConsent() instead of sitting in <db>/cache until pruning. Adds continue-on-error to both sentry-cli install steps so a Sentry registry outage or broken Git Bash install cannot block release assets. Cleans the legacy WER CrashDumps folder under SetShellVarContext current (under all it resolves to ProgramData, not the user's AppData) in installer and uninstaller; registry delete works in either context. * [CrashReporting] Extract the startup crash-report flow from main() per review * [CrashReporting] Break up the CrashReporter class doc per review * [CrashReporting] Drop the invalid --release from the symbol upload `debug-files upload` never accepted `--release` (checked against the pinned sentry-cli 3.8.0: the subcommand declares --org/--project and a set of type/id/scan options, and execute() reads no release; the only global flags are --url/--header/--auth-token/--log-level/--quiet/--allow-failure). Clap rejects the unknown flag before any upload happens, so every tag build has been shipping without symbols, with continue-on-error keeping the release pipeline green. Release association is also the wrong mechanism: debug information files are matched to events by debug ID via the event's debug_meta. The Sentry release of a native event comes from the event itself (COCKATRICE_SENTRY_RELEASE), not from the DIF upload. Remove the flag and the now-unused SENTRY_RELEASE env from both invocations (Linux, and Windows/macOS). The symbol upload stays continue-on-error (a Sentry outage must not block the release assets); revisit once it has proven itself on a tag. * [CrashReporting] Split debug information out of packaged release binaries -g is required so the CI symbol upload has DWARF to send, but it was left in the shipped binaries too: the Linux Release client is 563 MiB, 552 MiB of it debug info, and every distribution channel pays for that. Keep the build tree unchanged (CI uploads symbols from build/) and strip only the packaged copy. A CPack pre-build script extracts each ELF's debug info to COCKATRICE_DEBUG_SYMBOL_DIR and then runs `strip --strip-debug --strip-unneeded`. .eh_frame is allocatable and survives, so unwinding is unaffected, and the build ID is identical in the stripped binary and the sidecar. Measured on a local Release build (GCC -O2 -g, WITH_CRASH_REPORTING=ON), bytes: binary build tree packaged before packaged after .debug sidecar cockatrice 590,722,560 590,722,560 28,086,480 579,623,576 oracle 53,432,608 53,432,608 1,649,888 52,075,344 tar.gz of both trees: 289,964,521 -> 19,018,058 (~15x smaller). Only ELF (Linux/BSD) is handled: Windows already links /DEBUG with separate PDBs, and macOS extracts a dSYM from the bundle. Opt-out via COCKATRICE_SPLIT_DEBUG_SYMBOLS; skipped with a warning when objcopy/strip are missing. CPACK_PRE_BUILD_SCRIPTS (not CPACK_INSTALL_SCRIPTS, which runs before staging) invokes it, and the script refuses to run unless CPACK_TEMPORARY_DIRECTORY is a real CPack staging tree so a missing variable can never turn its glob into a filesystem-wide walk. * [CrashReporting] Upload Windows PDBs from where Ninja actually puts them The Windows branch of the symbol upload looked in build/<target>/Release, which is the Visual Studio generator's layout. The Windows leg uses Ninja, which is single-config: linker PDBs land in build/<target>/, next to the binaries — the same paths the "[Windows] Upload PDBs" artifact step below globs with if-no-files-found: error. So paths was always empty on Windows, the step printed "No symbol paths found, skipping symbol upload" and exited 0: a silent no-op behind continue-on-error. Every Windows build would then report "A required debug information file was missing" for cockatrice.exe with a PDB-style debug id (GUID-age), exactly as if no upload step existed at all. Use the artifact step's globs. Also turn the empty-path case into a failure in both symbol steps: a silent skip is indistinguishable from a successful upload (which is how the wrong path went unnoticed), and continue-on-error already keeps a broken upload from failing the release. Verified by extracting both steps' run blocks and executing the Windows branch against a Ninja-style tree: it now passes the three linker PDBs to sentry-cli (and not the CMakeFiles/*.pdb intermediates), and exits non-zero with an ::error:: annotation when there is nothing to upload. * [CrashReporting] Archive PDBs on tag builds so a failed upload is recoverable The "[Windows] Upload PDBs" artifact step skipped tag builds (github.ref_type != 'tag'), so for a release the symbols existed only inside the Sentry upload — which is continue-on-error. A failed upload therefore lost the ability to symbolicate that release permanently: no rebuild produces the same PDB, since the debug id is a per-build GUID+age. That is how every Windows crash reported so far became unreadable. Archive the PDBs for tag builds as well, and order the step after "Upload to release" so a missing PDB can never block a release from being published. Retention stays at the repository default; nothing consumes the artifact automatically, it is the manual recovery path (gh run download <run> -n <name>-PDBs, then sentry-cli debug-files upload). Verified by re-parsing the workflow and asserting the step order (release -> PDBs -> attest) and the broadened condition. * [CrashReporting] Upload the Windows executables with their PDBs Sentry needs two things per image on Windows: symbolication and stack unwinding. The PDB covers symbolication, but for a 64-bit PE the Call Frame Information (.pdata/.xdata) lives in the executable and is not copied into the PDB, so uploading PDBs alone leaves the image at "Stack Unwinding: Missing". That is exactly what the event's Images panel showed for cockatrice.exe, while Sentry's own Microsoft symbol server resolved KERNELBASE.dll and ucrtbase.dll from the same dump. sentry-cli also needs the pair in the same invocation: only then does it associate the PE's code identifier with the PDB's debug identifier, otherwise "the identifiers diverge and Sentry might not be able to resolve the correct file for symbolication" (docs: Debug Identifiers). Do the same for macOS, where the dSYM carries the debug information but the Mach-O carries the unwind information, and include the executables in the symbol artifact so a re-upload still has the pair to offer. Verified by extracting the step and running both branches against stub harnesses: Windows passes the three exe/pdb pairs in a single sentry-cli invocation and still fails loudly on an empty tree; macOS passes the dSYM and the Mach-O together. * [CrashReporting] Upload the shipped Qt runtime DLLs too Qt ships no PDBs with its MSVC SDK, so every Qt frame in a crash report resolved to nothing: an image with neither PDB nor PE has no unwind information (on 64-bit Windows the Call Frame Information lives in the PE and is not copied into the PDB) and no symbol source to fall back on. The list is required-runtime.txt, the manifest .ci/compile.sh already asserts the installer against, so what gets uploaded is exactly what an install carries. Qt frames now get unwinding plus export-table names; line numbers still need Qt's debug information files, which the SDK omits. The guards are not cosmetic: the step runs under `bash -eo pipefail`, where a sed over a missing file or an unmatched ls glob aborts the whole step, and continue-on-error makes that look identical to a successful upload. * [CrashReporting] Upload only the binaries, archive symbols only on tags The Linux leg handed sentry-cli whole build directories: 471 candidate files, of which 7 were object files that can never match a stack frame. Only the three linked binaries carry the build-ids Sentry looks up; the packaged copy is split and stripped but keeps that same build-id. The Windows symbol archive ran on every packaging build even though only tag builds upload to Sentry, so a PR build paid for an artifact nothing could ever consume. Tags keep it as the durable copy for a failed upload. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-19 14:57:04 +02:00
BruebachL	6385f3fab3	[CrashReporting] Mock crash-report dialog and reporting preference (#7521) ... * [CrashReporting] Mock dialog, sentry skeleton and CI prerequisites Mock: DlgCrashReport preview dialog (no account needed, redacted log preview), CrashReportingMode Ask/Always/Never setting in DebugSettings, --simulate-crash flag and Help-menu preview entry. Send writes a local mock JSON only. Vendor sentry-native 0.17.3 via FetchContent behind WITH_CRASH_REPORTING (breakpad everywhere, backend none under sanitizers) plus a CrashReporter facade with early init and post-loop shutdown. DSN is a placeholder: empty CMake default with COCKATRICE_SENTRY_DSN env override, so SDK init is skipped and mock behavior is unchanged until a real DSN is configured. CI prerequisites riding here so every layer below builds green: libcurl dev plus CA bundles in the Linux images and CodeQL env, sentry Qt integration off (its own find_package cannot see the macOS thin Qt layout), and the StrFunc Using construction makensis requires. Tests: ctest green, format clean, code plus UI gates PASS. * [CrashReporting] Forward mode changes to SDK and fix Linux CI opt-in Connects PersonalSettings::crashReportingModeChanged to giveConsent()/revokeConsent() so switching from AlwaysSend to NeverSend (or Ask) mid-session reaches the SDK instead of leaving consent given until restart. Forwards WITH_CRASH_REPORTING into the docker container the way CCACHE_DIR is, so the Linux package leg actually builds with crash reporting ON instead of silently building OFF. * [CrashReporting] Give crash-report logging its own category per review * [CrashReporting] Drop unused DebugSettings constructor visibility per review * [CrashReporting] Document reporting modes on the enum per review * [CrashReporting] Match the dump scan to the writer's temp fallback Review nit: when the handler's SHGetKnownFolderPath lookup fails it writes to <temp>/../.., but the startup scan looked in QDir::tempPath(), so a dump produced through the fallback would never be found and its crash would never offer to be reported. Walk up the same two levels in the scan. The path is Windows-only (#ifdef Q_OS_WIN); the statements were compiled and run standalone against Qt 6 to confirm the API use. * [CrashReporting] Complete the dump-scan fallback path The previous commit walked the scan's fallback up two levels but dropped the CrashDumps/Cockatrice suffix that the writer appends unconditionally, so a dump written through the fallback was still invisible to the scan. Evaluate the writer's expression verbatim rather than mirroring it with QDir: cdUp() is existence-checked and silently leaves the path alone when the directory is missing, parent_path() is not, and QDir::absoluteFilePath() joins the shared suffix without risking a doubled separator when the walk lands on a root. Verified by compiling the block verbatim against Qt 6 with -Wextra -Werror and comparing it with the writer's join for both the normal and the fallback case. Also record that %LOCALAPPDATA% is the scan's stand-in for FOLDERID_LocalAppData and why the two normally agree. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-19 14:57:03 +02:00
BruebachL	fea12a74e2	[CI] Build Windows releases with Ninja (#7380) ... The A/B run measured Ninja against the Visual Studio generator across ten builds: Ninja medians sit at ~17 min against ~21 for MSBuild, and more importantly Ninja already matches the Linux and macOS legs, runs the compiler itself and parallelizes the whole graph instead of one MSBuild node at a time. Single runs on shared 4-core hosts swing by minutes either way (VS and Ninja legs even ran in different Azure regions inside one matrix), so this switches on medians and engineering coherence, not on any one timing. The Ninja leg is now the release leg: it keeps the -Win10 suffix and the vcpkg scope (named after the vs2026 toolchain, whose cl.exe is unchanged, so the warm archives keep restoring), uploads PDBs from the single-config layout and ships installers to releases. The experiment scaffolding goes away with the generator it measured.	2026-10-14 19:13:52 +02:00
BruebachL	0c10798ef4	[CI] Build the Windows solution on more than one MSBuild node (#7379) ... cmake --build only passes a job count to MSBuild when it is asked for one, and .ci/compile.sh never asked, so every Visual Studio generator build ran single-node: projects were built one after another and MTT only ever parallelized the files of whichever project happened to be building. Ask for one node per core, overridable with BUILD_PARALLEL_LEVEL, and fall back to a single node when the core count is not exposed. Alongside it set EnforceProcessCountAcrossBuilds and CL_MPCount, the knobs Microsoft's build throughput write-up pairs with MTT: without them the node count and MTT multiply into more cl.exe processes than the runner has cores for. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-14 16:15:01 +02:00
BruebachL	eefe0bd672	[CI] Collect an MSBuild performance summary and binary log on request (#7378) ... The Windows job spends about 28 minutes inside one "Build project" group, with nothing in it that says where the time goes: MSBuild runs single-node because no --parallel is passed, so projects build one after another and it is not visible which projects, tasks or phases are responsible. Add a --profile flag that makes the Windows build write -flp:PerformanceSummary and -bl, repeat the summary in the job log so the numbers need no download, and report the core count, which decides how much parallelism is worth asking for. The binary log is uploaded as an artifact only on the runs that asked for it. The workflow drives it from a build_profile matrix key that is empty by default, so no build changes behaviour until someone sets it. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-14 14:27:58 +02:00
BruebachL	25c85cf0bb	[CI] Key the vcpkg binary cache per ABI and let pull requests heal it (#7377) ... * [CI] Key the vcpkg binary cache per ABI and let pull requests heal it The cache had no prefix, and TAServers/vcpkg-cache restores every entry it finds under the prefix rather than only the hashes vcpkg asks for. With all legs sharing the single vcpkg/ namespace, each of the five of them downloads all 66 archives of all five on every run. Each restore also refreshes the entry's last access date, so a dead ABI generation stays fresh forever and GitHub's least recently used eviction can never reach it. Name the scope in the key after the platform, target and toolchain it belongs to, so a leg restores only its own archives. macOS 14, 15 and 26 on the same Xcode build identical binaries when they share the lowest deployment target, so point all three at 14 and let them share one scope instead of three. A pull request cannot write the shared cache either. Its entries are scoped to refs/pull/<n>/merge, so nothing it saves is ever readable again, and letting it write anyway only burns the repository's quota, which is what the previous attempt at this did. Keep the build read-only and hand the archives it had to build itself to a new workflow running in the default branch scope, which saves them where master can restore them. Forks stay read-only, they are the one case whose archives must not be trusted. Because the prefix changes every key, add a job that deletes the entries of retired scopes, which is the one-time clean up of the current flat namespace, and the per-run ones pull requests leave behind, which age based eviction also never reaches. Also pin the Linux ccache cache to its own subdirectory, since the directory is shared with the docker build context, and drop the clang-format install from the build images, which format nothing at runtime. * [CI] Create the vcpkg cache directory when a scope is still empty * [CI] Let a pull request write the vcpkg archives it builds * [CI] Upload the vcpkg handover as one archive instead of loose files * [CI] Hand the vcpkg cache over from one leg per scope * [CI] Gate vcpkg cache writes to master pushes --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-14 08:14:16 +02:00
BruebachL	bdda3dedfa	[CI] Publish a gcovr line-coverage report from the test run (#7404) ... * [Tests] Register each test case with CTest instead of each test binary Registering a test cost four edits across a dozen CMakeLists files: add_test, add_executable, a conditional add_dependencies on gtest, and target_link_libraries. The conditional branch was the trap -- it is only needed when GTest was fetched and add_subdirectory'd, and omitting it yields undefined gtest symbols at link time -- so it was easy to leave out and impossible to notice until the link failed. Fold all four steps into cockatrice_add_test() and migrate every test directory onto it, with a cockatrice_add_manual_test() wrapper for the three benchmarks that are built but deliberately kept out of ctest. The calls now read as a declaration of what a test is rather than a transcript of how to register one. While rewriting the registrations, replace add_test(NAME x COMMAND x) with gtest_discover_tests(). Naming the binary as a single CTest entry meant a CI failure reported only which binary failed, and ctest -R Suite.Case could not select an individual case. Discovery raises the suite from 35 opaque entries to 469 real ones: ctest -R ThemeMigrationTest.IsIdempotent now runs exactly that case, and ctest -L server / -LE bench work off labels applied at registration. Give every test a 60s TIMEOUT. CTest's implicit default is 1500s, so one hung socket wait would stall CI for 25 minutes, and single_instance_manager_test already does real QLocalSocket waits. Dropping per-case granularity costs wall-clock time, since the suite becomes process-spawn bound rather than CPU bound: serial ctest went from 3.8s to 30.8s. Run it with -j in CI to recover it -- 7.6s at -j4, and 3.5s at -j8, both at or below the old serial time. The binaries are parallel-safe: the only test claiming a fixed resource scopes its local socket name to its own pid, so concurrent copies cannot collide. Also delete dummy_test, which asserted 1 == 1 and only proved the harness was wired up -- something the other 468 cases already establish. No test is added, removed, or changed by this commit; the 469 cases are the same ones the 35 binaries contained. * [Tests] Make QtTest available to test targets QtTest was not among the modules test targets could link, so the Qt model and signal contracts had no way to be verified from a test: QAbstractItemModelTester and QSignalSpy both live there. Add Test to _TEST_NEEDED in cmake/FindQtRuntime.cmake. That is the single place the union of test-only Qt modules is declared, so this is the only file that needs to change to make the module reachable from every test target. Nothing uses it yet. This lands the capability ahead of the model-layer tests that need it, so those can be added as tests alone rather than as test plus build-system plumbing. * [Tests] Correct the note on the oracle test compression defines add_definitions() populates the directory's COMPILE_DEFINITIONS property, which is consumed at generate time, so it reaches every target in the directory no matter where it is called relative to add_executable(). The previous note claimed the flags stayed confined to the benchmark because the find_package block came after it; oracle_importer_test, created above that block, carries both defines too. Verified harmless rather than a live bug: HAS_ZLIB and HAS_LZMA gate code only in oracle/src/pages.cpp and oracle/src/pagetemplates.cpp, and no test target compiles either. The ordering and the leaked defines are both pre-existing, so this only corrects the explanation. * [Tests] Carry one CTest label per test instead of several gtest_discover_tests() can hand CTest exactly one label. GoogleTestAddTests.cmake accumulates its generated script with `foreach(arg ${ARGN})`, so a label list reaches the CTestTestfile as bare `LABELS ci server`, and CTest reads only the first value of a property and drops the rest without a word. The `ci` label that is appended to every registered test was therefore absent from every test that also asked for `server` or `bench`: `ctest -L ci` reported 400 of 469 cases, and `ctest --print-labels` never listed a second label anywhere in the build. Reduce each test to exactly one of `ci`, `server`, or `bench`, which is all the selections need. The three now partition the suite exactly, 400 + 64 + 5 = 469, and `ctest -LE bench` still means everything but benchmarks. A second label is now a configure-time FATAL_ERROR rather than a line that quietly means something else. Anything orthogonal to a test's kind belongs in a helper flag beside the label, so it composes with the group instead of competing for the single slot. Also raise DISCOVERY_TIMEOUT off GoogleTestAddTests' 5s default. A plain debug binary answers --gtest_list_tests in 0.04s, but that budget is thin once several binaries discover concurrently behind a -j build, and being over it fails at the *link* step with a bare "Process terminated due to timeout" that points nowhere near the cause. * [Tests] Link gtest_main on the fetched-gtest path The downloaded-gtest branch set GTEST_BOTH_LIBRARIES to `gtest` alone, while FindGTest sets it to both halves. Every test target links that list rather than picking an entry point, so on this path nothing provides main() and any test source that does not define its own fails at link with `undefined reference to main` pointing at crt1.o, naming neither gtest nor the line responsible. That stayed latent while every test carried its own four-line main(). A test that omits it now fails, and the error points at the C runtime rather than at gtest, which is a long way from the cause. Restoring both halves makes the two paths agree and drops the boilerplate requirement on new test sources. The 18 sources that still define main() keep linking: gtest_main.cc defines nothing but main, so the linker leaves its object in the archive unused rather than reporting a duplicate. * [Build] Add an ENABLE_SANITIZERS option for ASan and UBSan Adds -DENABLE_SANITIZERS, off by default, building with AddressSanitizer and UndefinedBehaviorSanitizer. The bulk of the work is in the three ways the obvious implementation silently does the wrong thing. Probe the flags on the link step, not just the compile step. check_cxx_compiler_flag() only compiles, so every sanitizer flag comes back unsupported, because linking an ASan object without -fsanitize=address leaves __asan_init undefined. The result is not a build failure but a build that appears instrumented and is not, which is worse. Use check_cxx_source_compiles() with CMAKE_REQUIRED_LINK_OPTIONS instead, and treat an unsupported sanitizer pair as fatal since the cosmetic flags are allowed to degrade. Opt vendored sources out per source, through a cockatrice_disable_sanitizers_for() helper rather than a list of set_source_files_properties() calls at the root. The helper has to exist because CMake resolves a source-file property against the directory scope its target was declared in: setting the property from the root with absolute paths is a no-op, silently, which is why the call sites sit next to the add_library and add_executable that pull the vendored code in. SFMT, minizip, the LZMA SDK and Qxt SMTP are excluded. oracle/src/qt-json is in the history but not in the tree, removed in #7214. Also mark those sources SKIP_PRECOMPILE_HEADERS. The PCH is built with the sanitizer flags, so it defines __SANITIZE_ADDRESS__; a translation unit compiled with -fno-sanitize=all does not and GCC then refuses the PCH, which is a fatal -Werror=invalid-pch in the servatrice and oracle builds. -fno-sanitize=vptr is not optional. Qt constructs QObject subclasses into raw storage and writes the vptr afterwards, so the vptr check reports on essentially every QObject instantiation, and it has no value here because nothing outside moc-generated code inherits QObject. Verified on the resulting build: our own translation units carry ASan and UBSan calls, SFMT.o carries neither, and the full suite of 464 cases passes clean under ASAN_OPTIONS=detect_leaks=0:abort_on_error=1 with UBSAN_OPTIONS=halt_on_error=0. The test suite needs two adjustments to make this option usable at all, both of which land here because they are consequences of COCKATRICE_SANITIZERS existing rather than independent changes. DISCOVERY_TIMEOUT rises from 60s to 300s under sanitizers. An instrumented binary takes 2.8s to answer --gtest_list_tests against 0.04s uninstrumented, and discovery runs several binaries at once behind a -j build. One target is excluded via a NO_SANITIZE flag, server_developer_role_test. Reaching google::protobuf::TextFormat::Printer's destructor aborts on an absl assertion inside system protobuf 33.4 whenever the consumer translation unit is built without NDEBUG, which a Debug sanitizer build is. This reproduces with no Cockatrice code at all, just "TextFormat::Printer p; p.PrintToString(msg, &out);" built with -fsanitize=address, so it is upstream rather than ours. It is a flag and not a fourth label because "skip this in the sanitizer leg" has to compose with a test's group rather than compete for the single label slot, and because a flag excludes by not registering the cases at all, which cannot rot the way a label silently can. The target still builds and still runs on the legs without sanitizers. * [CI] Run the suite once under ASan and UBSan Adds --sanitize to .ci/compile.sh, mirroring --test, and turns it on for one entry of the existing Linux Docker matrix: Ubuntu 26.04, which already had a test-enabled leg. That reuses the image, the ccache layer and the existing step shape, so the CI diff is a matrix key plus the argument plumbing. Debian 13 and Fedora 44 keep covering the uninstrumented suite, so nothing is lost by instrumenting one leg. Sanitizers stay off the release and package legs, which build with --release for packaging: instrumenting those would change the shipped binaries' optimization profile and build time for no benefit. macOS and Windows stay uninstrumented too, since ASan is unreliable against the Qt build there and MSVC has no equivalent without /fsanitize=address. --sanitize does not imply --test, so an instrumented build can be produced locally without paying for ctest. When both are given, ctest runs with detect_leaks=0 because Qt and the sanitizer runtimes both leak at exit by design and LeakSanitizer's default buries real findings under hundreds of process-teardown reports, and with UBSan halt_on_error=0 so one run collects many findings instead of stopping at the first. ASan keeps abort_on_error=1, since a bad access leaves the process unusable and continuing only produces a second, more confusing report from the wreckage. * [CI] Publish a gcovr line-coverage report from the test run Sanitizers tell us what a run did wrong; they say nothing about what it never reached. This adds the other half: an instrumented build whose report says which of our own lines no test executes. Add an ENABLE_COVERAGE option that compiles with --coverage -O0 -g, mirroring the ENABLE_SANITIZERS opt-in including the per-source carve-out that keeps vendored trees out of the denominator. -O0 is deliberate: at higher optimisation levels GCC merges and relocates lines, so the report would describe code that no longer exists in that form. Only GCC is supported, since Clang's intermediate format needs llvm-cov; asking for coverage under Clang is a hard error rather than a build that silently produces unreadable .gcno files. Keep the filters in a committed gcovr.cfg rather than in the invocation. The tests themselves are excluded because they are compiled with the same instrumentation and are only ever executed by the run doing the measuring, so leaving them in would report a healthier project than we have. The process entry points and the client's window chrome go too: each needs a display, an interactive terminal or a live database before it does anything a headless test could observe, so they would otherwise sit at 0% forever and bury real movement. Give coverage its own matrix leg rather than a second flag on the sanitizer leg. The two answer different questions, and combining them slows both down without telling us more. The new variant key gives the leg its own Docker image and ccache entry so the two legs cannot race on the same tag. The leg publishes an HTML report with per-file pages, LCOV, Cobertura XML and a JSON summary. gcovr is a report-time dependency rather than a build dependency, so a missing one warns and skips instead of failing an otherwise green build. * [CI] Resolve the coverage source root before the build dir cd The previous commit computed SRC_ROOT after `cd "$BUILD_DIR"`, under a comment claiming it ran before. `$0` is relative to the directory the script was invoked from, so from inside the build dir `dirname "$0"` no longer resolves and the subshell's `cd` fails. Under `set -e` that is fatal rather than an empty variable, which took down every leg in the matrix, coverage or not, including the ones this change does not otherwise touch. Move it above the cd and guard it on MAKE_COVERAGE, since the coverage report is the only consumer. Resolving it unconditionally meant a coverage-only detail was paid for by every build in the fleet, and gave every one of them a way to fail. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-11 15:05:28 +02:00
BruebachL	8822ade49d	[Tests] Register each test case with CTest instead of each test binary (#7399) ... * [Tests] Register each test case with CTest instead of each test binary Registering a test cost four edits across a dozen CMakeLists files: add_test, add_executable, a conditional add_dependencies on gtest, and target_link_libraries. The conditional branch was the trap -- it is only needed when GTest was fetched and add_subdirectory'd, and omitting it yields undefined gtest symbols at link time -- so it was easy to leave out and impossible to notice until the link failed. Fold all four steps into cockatrice_add_test() and migrate every test directory onto it, with a cockatrice_add_manual_test() wrapper for the three benchmarks that are built but deliberately kept out of ctest. The calls now read as a declaration of what a test is rather than a transcript of how to register one. While rewriting the registrations, replace add_test(NAME x COMMAND x) with gtest_discover_tests(). Naming the binary as a single CTest entry meant a CI failure reported only which binary failed, and ctest -R Suite.Case could not select an individual case. Discovery raises the suite from 35 opaque entries to 469 real ones: ctest -R ThemeMigrationTest.IsIdempotent now runs exactly that case, and ctest -L server / -LE bench work off labels applied at registration. Give every test a 60s TIMEOUT. CTest's implicit default is 1500s, so one hung socket wait would stall CI for 25 minutes, and single_instance_manager_test already does real QLocalSocket waits. Dropping per-case granularity costs wall-clock time, since the suite becomes process-spawn bound rather than CPU bound: serial ctest went from 3.8s to 30.8s. Run it with -j in CI to recover it -- 7.6s at -j4, and 3.5s at -j8, both at or below the old serial time. The binaries are parallel-safe: the only test claiming a fixed resource scopes its local socket name to its own pid, so concurrent copies cannot collide. Also delete dummy_test, which asserted 1 == 1 and only proved the harness was wired up -- something the other 468 cases already establish. No test is added, removed, or changed by this commit; the 469 cases are the same ones the 35 binaries contained. * [Tests] Make QtTest available to test targets QtTest was not among the modules test targets could link, so the Qt model and signal contracts had no way to be verified from a test: QAbstractItemModelTester and QSignalSpy both live there. Add Test to _TEST_NEEDED in cmake/FindQtRuntime.cmake. That is the single place the union of test-only Qt modules is declared, so this is the only file that needs to change to make the module reachable from every test target. Nothing uses it yet. This lands the capability ahead of the model-layer tests that need it, so those can be added as tests alone rather than as test plus build-system plumbing. * [Tests] Correct the note on the oracle test compression defines add_definitions() populates the directory's COMPILE_DEFINITIONS property, which is consumed at generate time, so it reaches every target in the directory no matter where it is called relative to add_executable(). The previous note claimed the flags stayed confined to the benchmark because the find_package block came after it; oracle_importer_test, created above that block, carries both defines too. Verified harmless rather than a live bug: HAS_ZLIB and HAS_LZMA gate code only in oracle/src/pages.cpp and oracle/src/pagetemplates.cpp, and no test target compiles either. The ordering and the leaked defines are both pre-existing, so this only corrects the explanation. * [Tests] Carry one CTest label per test instead of several gtest_discover_tests() can hand CTest exactly one label. GoogleTestAddTests.cmake accumulates its generated script with `foreach(arg ${ARGN})`, so a label list reaches the CTestTestfile as bare `LABELS ci server`, and CTest reads only the first value of a property and drops the rest without a word. The `ci` label that is appended to every registered test was therefore absent from every test that also asked for `server` or `bench`: `ctest -L ci` reported 400 of 469 cases, and `ctest --print-labels` never listed a second label anywhere in the build. Reduce each test to exactly one of `ci`, `server`, or `bench`, which is all the selections need. The three now partition the suite exactly, 400 + 64 + 5 = 469, and `ctest -LE bench` still means everything but benchmarks. A second label is now a configure-time FATAL_ERROR rather than a line that quietly means something else. Anything orthogonal to a test's kind belongs in a helper flag beside the label, so it composes with the group instead of competing for the single slot. Also raise DISCOVERY_TIMEOUT off GoogleTestAddTests' 5s default. A plain debug binary answers --gtest_list_tests in 0.04s, but that budget is thin once several binaries discover concurrently behind a -j build, and being over it fails at the *link* step with a bare "Process terminated due to timeout" that points nowhere near the cause. * [Tests] Link gtest_main on the fetched-gtest path The downloaded-gtest branch set GTEST_BOTH_LIBRARIES to `gtest` alone, while FindGTest sets it to both halves. Every test target links that list rather than picking an entry point, so on this path nothing provides main() and any test source that does not define its own fails at link with `undefined reference to main` pointing at crt1.o, naming neither gtest nor the line responsible. That stayed latent while every test carried its own four-line main(). A test that omits it now fails, and the error points at the C runtime rather than at gtest, which is a long way from the cause. Restoring both halves makes the two paths agree and drops the boilerplate requirement on new test sources. The 18 sources that still define main() keep linking: gtest_main.cc defines nothing but main, so the linker leaves its object in the archive unused rather than reporting a duplicate. * [Build] Run the test suite under ASan and UBSan (#7400) * [Build] Add an ENABLE_SANITIZERS option for ASan and UBSan Adds -DENABLE_SANITIZERS, off by default, building with AddressSanitizer and UndefinedBehaviorSanitizer. The bulk of the work is in the three ways the obvious implementation silently does the wrong thing. Probe the flags on the link step, not just the compile step. check_cxx_compiler_flag() only compiles, so every sanitizer flag comes back unsupported, because linking an ASan object without -fsanitize=address leaves __asan_init undefined. The result is not a build failure but a build that appears instrumented and is not, which is worse. Use check_cxx_source_compiles() with CMAKE_REQUIRED_LINK_OPTIONS instead, and treat an unsupported sanitizer pair as fatal since the cosmetic flags are allowed to degrade. Opt vendored sources out per source, through a cockatrice_disable_sanitizers_for() helper rather than a list of set_source_files_properties() calls at the root. The helper has to exist because CMake resolves a source-file property against the directory scope its target was declared in: setting the property from the root with absolute paths is a no-op, silently, which is why the call sites sit next to the add_library and add_executable that pull the vendored code in. SFMT, minizip, the LZMA SDK and Qxt SMTP are excluded. oracle/src/qt-json is in the history but not in the tree, removed in #7214. Also mark those sources SKIP_PRECOMPILE_HEADERS. The PCH is built with the sanitizer flags, so it defines __SANITIZE_ADDRESS__; a translation unit compiled with -fno-sanitize=all does not and GCC then refuses the PCH, which is a fatal -Werror=invalid-pch in the servatrice and oracle builds. -fno-sanitize=vptr is not optional. Qt constructs QObject subclasses into raw storage and writes the vptr afterwards, so the vptr check reports on essentially every QObject instantiation, and it has no value here because nothing outside moc-generated code inherits QObject. Verified on the resulting build: our own translation units carry ASan and UBSan calls, SFMT.o carries neither, and the full suite of 464 cases passes clean under ASAN_OPTIONS=detect_leaks=0:abort_on_error=1 with UBSAN_OPTIONS=halt_on_error=0. The test suite needs two adjustments to make this option usable at all, both of which land here because they are consequences of COCKATRICE_SANITIZERS existing rather than independent changes. DISCOVERY_TIMEOUT rises from 60s to 300s under sanitizers. An instrumented binary takes 2.8s to answer --gtest_list_tests against 0.04s uninstrumented, and discovery runs several binaries at once behind a -j build. One target is excluded via a NO_SANITIZE flag, server_developer_role_test. Reaching google::protobuf::TextFormat::Printer's destructor aborts on an absl assertion inside system protobuf 33.4 whenever the consumer translation unit is built without NDEBUG, which a Debug sanitizer build is. This reproduces with no Cockatrice code at all, just "TextFormat::Printer p; p.PrintToString(msg, &out);" built with -fsanitize=address, so it is upstream rather than ours. It is a flag and not a fourth label because "skip this in the sanitizer leg" has to compose with a test's group rather than compete for the single label slot, and because a flag excludes by not registering the cases at all, which cannot rot the way a label silently can. The target still builds and still runs on the legs without sanitizers. * [CI] Run the suite once under ASan and UBSan Adds --sanitize to .ci/compile.sh, mirroring --test, and turns it on for one entry of the existing Linux Docker matrix: Ubuntu 26.04, which already had a test-enabled leg. That reuses the image, the ccache layer and the existing step shape, so the CI diff is a matrix key plus the argument plumbing. Debian 13 and Fedora 44 keep covering the uninstrumented suite, so nothing is lost by instrumenting one leg. Sanitizers stay off the release and package legs, which build with --release for packaging: instrumenting those would change the shipped binaries' optimization profile and build time for no benefit. macOS and Windows stay uninstrumented too, since ASan is unreliable against the Qt build there and MSVC has no equivalent without /fsanitize=address. --sanitize does not imply --test, so an instrumented build can be produced locally without paying for ctest. When both are given, ctest runs with detect_leaks=0 because Qt and the sanitizer runtimes both leak at exit by design and LeakSanitizer's default buries real findings under hundreds of process-teardown reports, and with UBSan halt_on_error=0 so one run collects many findings instead of stopping at the first. ASan keeps abort_on_error=1, since a bad access leaves the process unusable and continuing only produces a second, more confusing report from the wreckage. * [Build] Fix sanitizer flag leak into release package and make UBSan fail CI - compile.sh always passes -DENABLE_SANITIZERS explicitly so the release-package reconfigure in the same build dir cannot inherit ON from the sanitized debug step (which shipped libasan/libubsan DEB deps). - Sanitizers.cmake resets COCKATRICE_SANITIZERS to OFF when the option is off (CACHE INTERNAL otherwise sticks forever, keeping NO_SANITIZE skips and the 300s discovery timeout). - UBSan now fails the job: -fno-sanitize-recover=undefined at compile time plus UBSAN_OPTIONS halt_on_error=1 (recover mode exits 0, so the old halt_on_error=0 could never fail). --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de> --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-11 14:50:09 +02:00
BruebachL	1e1f79db0d	[Windows] Wait for uninstaller, rework .dll bundling, add logging (#7407) ... * [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>	2026-10-09 20:09:34 +02:00
BruebachL	ecd955bc60	[Installer] Wait on the launching process so auto-updates cannot write over a mixed Qt runtime (#7372) ... * [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>	2026-10-02 15:15:29 +02:00
BruebachL	a5e94d8a4f	[Build] Keep Windows installs free of build-tree artifacts (#7316) ... * [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>	2026-09-20 20:21:52 +02:00
tooomm	b3c426cd43	Alphabetical ordering of Qt modules/packages (#7334) ... * ordering * Update docker-release.yml * Revert "Update docker-release.yml" This reverts commit e908d92184.	2026-09-20 19:24:55 +02:00
BruebachL	14ecfff700	[Build] Add precompiled headers for Qt-backed executables (#7235) ... * [Build] Add precompiled headers for Qt-backed executables Reparsing QtCore/QtGui/QtWidgets/QtNetwork in ~460 client translation units is the dominant compilation cost. Precompile the two common layers: - qtcore_pch.h (Qt Core only; safe even for headless Servatrice) - qtwidgets_pch.h (adds Gui/Widgets/Network; used by Cockatrice and Oracle) target_precompile_headers() requires CMake 3.16, now the project minimum. Estimated 30-50% faster client rebuilds. * [Build] Format qtwidgets precompiled header clang-format include regrouping and a missing trailing newline. * [Build] Add PCH-aware ccache sloppiness config; format cmake/pch headers --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-05 19:40:36 +02:00
BruebachL	d6fbfb32a1	[Security] Use a CSPRNG for salts, tokens, and RNG seeding (#7192) ... * [Security] Use a CSPRNG for salts, tokens, and RNG seeding Password salts and activation tokens were generated with the global SFMT RNG, which was seeded from a 32-bit timestamp, making registration salts and activation tokens predictable. The game RNG used the same timestamp seed across restarts. Add CryptoUtil backed by OpenSSL RAND_bytes and use it for salt/token generation and to seed RNG_SFMT with a 64-bit CSPRNG value in both the client and server. Link libcockatrice_utility against OpenSSL::Crypto. Took 30 seconds Took 25 minutes * Lint. Took 4 minutes Took 36 seconds --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-04 13:49:18 +02:00
BruebachL	2b7b4e8168	[App] Add onboarding wizard (#7064) ... * [App] Add onboarding wizard Took 10 minutes Took 3 minutes Took 7 minutes Took 9 minutes Took 2 minutes Took 1 minute Took 7 minutes * Adjust CI Took 14 minutes Took 56 seconds Took 2 seconds Took 3 seconds * Adjust CI again Took 14 minutes Took 2 seconds * Comments and fixes Took 9 seconds Took 1 minute * Rebase. Took 5 minutes Took 50 seconds Took 15 seconds * Comments. Took 7 minutes * CI lol Took 3 minutes * CI again lol Took 4 minutes * Drop some settings, add some new ones. Took 19 minutes * Resize when expanding section Took 4 minutes Took 3 minutes Took 7 minutes --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-08-24 23:05:58 +02:00
tooomm	f425dcc93e	Update & add arch/platform labels in release template (#7149) ... * Update & add arch/platform labels * Update release_template.md	2026-08-23 12:56:00 +02:00
tooomm	32619f4d55	Add --x-abi-tools-use-exact-version to vcpkg invocation (#7068)	2026-08-05 23:14:29 +02:00
tooomm	dc152e89f7	CI: Print colored diff for lint check (#6975) ... * print colored diff in gha * use spaces	2026-06-08 19:19:28 +02:00
tooomm	03d54265fe	escape ampersand (#6900)	2026-05-21 22:02:09 +02:00
SlightlyCircuitous	f7eeaeddcb	Drop builds for Debian 11 and Ubuntu 22.04 (#6643) ... * Delete .ci/Debian11 directory * Delete .ci/Servatrice_Debian11 directory * Delete .ci/Ubuntu22.04 directory * Update desktop-build.yml * Update release_template.md * Add servatrice debian 12 * Update desktop-build.yml	2026-05-09 00:28:03 +02:00
ZeldaZach	63143f9416	Fix triplet source for macOS13 Intel	2026-05-08 11:02:50 -04:00
Jeremy Letto	c5702cc8b6	Remove webclient (extracted to Webatrice repo) (#6848) ... The webclient has been extracted to https://github.com/seavor/Webatrice and the Playwright e2e suite has moved to Sockatrice. Cockatrice keeps no copy. Deleted: - webclient/ (entire tree, 349 files) - .github/workflows/web-build.yml, web-lint.yml - .husky/pre-commit (the only tracked husky hook; managed entirely from webclient's package.json which no longer exists) Edited: - .tx/config: dropped the webclient resource block; Webatrice/.tx/config now manages its own translations - .github/workflows/translations-pull.yml: removed webclient locales from add-paths - .github/workflows/desktop-build.yml, desktop-lint.yml: removed dead '!webclient/**' and '!.husky/**' path-filter exclusions - .github/dependabot.yml: removed commented-out npm/webclient block - Doxyfile: removed webclient/ from EXCLUDE list - .ci/release_template.md: dropped Webatrice section (Webatrice now cuts its own releases) - README.md: dropped 'first work on a webclient' line, added Webatrice to Related Repositories, updated translation paragraph and build badges History preserved: every webclient commit remains recoverable via git log on master before this commit.	2026-05-08 09:14:53 -04:00
SlightlyCircuitous	dac611f0f1	Add Fedora 44 build and drop Fedora 42 build (#6836) ... * Add Fedora 44; Drop Fedora 42 * Delete .ci/Fedora42 directory * Create Dockerfile * Update release_template.md * Skip debug	2026-04-30 21:09:49 +02:00
ebbit1q	f97864b72b	set the minimum qt6 version to 6.4 (#6811) ... * set the minimum qt6 version to 6.4 this will be separate from the qt5 removal after release * add include for optional	2026-04-14 00:59:46 +02:00
ebbit1q	e977f123ce	add ccache eviction for files older than 7 days (#6795) ... * add ccache eviction for files older than 7 days also clean up some of the scripts to be more internally consistent * move name build into the docker container again * remove one extra empty line [skip ci] * allow canceling concurrent builds on master for both desktop and docker * add ccache eviction age to macos as well	2026-04-11 19:18:12 +02:00
SlightlyCircuitous	690a00aa6c	Add Ubuntu 26.04 "Resolute Racoon" build (#6766) ... * Create 26.04 Dockerfile * Update desktop-build.yml * Update release_template.md * Add ca-certificates package to build	2026-04-03 20:40:42 +02:00
tooomm	51c684251f	Add Docker to release template (#6732)	2026-03-24 00:16:26 +01:00
tooomm	96f436b65e	CI: Resolve to latest Qt version in range for Windows as well (#6700) ... * CI: Resolve to latest Qt version in range for Windows as well * install aqt * Check dedicated win version	2026-03-15 17:47:48 +01:00
tooomm	42cec10457	CI: Cleanup (#6677) ... * Cleanup vcpkg matrix * Add filename with extension as output * fix name -> fullname, cleanup	2026-03-09 21:32:47 +01:00
tooomm	852429f248	remove unused zip command (#6676)	2026-03-08 23:48:30 +01:00
tooomm	c375cdbb1a	CI: Upload artifact files directly (#6654) ... * Upload artifact files directly * match pdbs name * Update name variable	2026-03-08 18:03:01 +01:00
tooomm	c6dc7eee64	CI: Remove Windows 7 build (#6625) ... * Update release_template.md * Remove Win7	2026-02-22 22:11:58 +01:00
Bruno Alexandre Rosa	72a85b58cf	ci: make fat qt libs thin (#6281) ... * ci: strip fat qt binaries * parallelize * cache thin qt * print libs * change qt install dir in the action * move qt install logic to separate job * lookup only * debug: show contents of QTDIR * enableCrossOsArchive also when saving * check one dir up * change install dir * keep debugging * try deleting cache * force delete cache * pass gh_token * pass missing params * use api * change cache key, disable cross os archive * move job directly to steps * add comments * set cache param directly * address comments * fixup * Update .ci/thin_macos_qtlib.sh * resolve qt version * move resolution to separate script * use single line for run: * improve error handling in new scripts --------- Co-authored-by: ebbit1q <ebbit1q@gmail.com>	2026-01-04 01:00:05 +01:00
BruebachL	7f1d891e26	[Deprecation] Remove DBConverter from sources. (#6431) ... Took 10 minutes Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2025-12-20 15:25:13 +01:00
tooomm	73ce5e051c	Add min os and arch inspection (#6259)	2025-11-16 15:09:26 +01:00
SlightlyCircuitous	3c85ca9cbc	Remove Fedora 41 Build and Add Fedora 43 Build (#6305) ... * Create Dockerfile * Update Release Template * Delete .ci/Fedora41 directory * remove F41, add F43	2025-11-11 20:18:20 -05:00
ebbit1q	c9ccab8771	Servatrice build failure (#6243) ... * attempts to fix the problem * add test to show the problem * fix workflow * move logger to cockatrice * more attempts * undo stuff * mark different libraries as gui * fix SC2145 * rename servatrice only build	2025-11-11 14:34:39 +01:00
ebbit1q	d206a70b8a	update format.sh (#6240) ... * update format.sh add shellcheck to format.sh add statement macros to .clang-format add no clang format to format.sh add changed file list to format.sh diff rename --cf-version to --print-version in format.sh lint files * enable --shell on ci runs * remove useless semicolons removes the semicolons after empty function definitions these semicolons are optional, they don't do anything this will have functions be consistently formatted if we want to keep the option to have these on the same line like they were before we should use the option AllowShortFunctionsOnASingleLine: None * fix script * update echo line in lint_cpp.sh which doesn't lint cpp only at all	2025-11-07 15:00:39 +01:00
Bruno Alexandre Rosa	bbec4d2c7e	ci: unify vcpkg jobs (take 2) (#6263) ... * ci: unify vcpkg jobs * use build matrix variables: package_suffix simplifying some convoluted logic, one variable at a time. work in progress. * use build matrix variables: artifact_name * use build matrix variables: qt stuff * display cmake flags in builds * add type to windows builds * use build matrix variables: cmake stuff * use build matrix variables: USE_CCACHE * formatting * more formatting * spaces * address review comments	2025-11-06 23:20:47 +01:00
Bruno Alexandre Rosa	817a3f979e	build: target macos 13 for x86 binaries (#6221) ... * build: target older macos * cleanup * Align xcode versions * Simplify --x86-macos * use cmake flag in compile.sh i.s.o. env var in yml * more cleanups * adress initial reviews * generate triplet file in compile.sh * fix triplet name * pass matrix.target as version * small refactor * another minor refactor * ci: fix ccache cleaning * add more comments * try passing triplets config as cmake variables * Revert "try passing triplets config as cmake variables" This reverts commit 77e83e8590. * move logic inside runner == macos if * move logic to env var * simplify script * format script "I just thinks it looks better like this" * make script work for arm as well, might be useful * use hyphen * use DVCPKG_HOST_TRIPLET instead of DVCPKG_TARGET_TRIPLET * use DVCPKG_HOST_TRIPLET AND DVCPKG_TARGET_TRIPLET --------- Co-authored-by: ebbit1q <ebbit1q@gmail.com>	2025-10-22 01:00:25 +02:00
ebbit1q	be1403c920	add hdiutil repeat script (#6231)	2025-10-08 10:10:44 +02:00
RickyRister	9c58e6f90f	Don't use vcpkg on local macOS (#6225) ... * Don't use vcpkg on local macOS * fix typo	2025-10-05 13:31:46 +02:00