mirror of
https://github.com/Cockatrice/Cockatrice.git
synced 2026-11-05 01:23:24 -08:00
1e1f79db0d
Some checks are pending
CodeQL / Analyze (cpp) (push) Waiting to run
CodeQL / Analyze (actions) (push) Waiting to run
Build Desktop / Configure (push) Waiting to run
Build Desktop / Debian 13 (push) Blocked by required conditions
Build Desktop / Debian 12 (push) Blocked by required conditions
Build Desktop / Fedora 44 (push) Blocked by required conditions
Build Desktop / Fedora 43 (push) Blocked by required conditions
Build Desktop / Servatrice_Debian 12 (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 (push) Blocked by required conditions
Build Desktop / Ubuntu 24.04 (push) Blocked by required conditions
Build Desktop / Arch (push) Blocked by required conditions
Build Desktop / macOS 13 Intel (push) Blocked by required conditions
Build Desktop / macOS 14 (push) Blocked by required conditions
Build Desktop / macOS 15 (push) Blocked by required conditions
Build Desktop / macOS 26 Debug (push) Blocked by required conditions
Build Desktop / Windows 10 (push) Blocked by required conditions
Build Docker / Servatrice (arm) (push) Waiting to run
Build Docker / Servatrice (x86) (push) Waiting to run
Build Docker / Publish multi-platform Servatrice image (push) Blocked by required conditions
* [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> |
||
|---|---|---|
| .. | ||
| pch | ||
| CMakeDMGSetup.script | ||
| createversionfile.cmake | ||
| dmgBackground.tif | ||
| FindLibexecinfo.cmake | ||
| FindQtRuntime.cmake | ||
| FindVCredistRuntime.cmake | ||
| getversion.cmake | ||
| gtest-CMakeLists.txt.in | ||
| headerimage.bmp | ||
| Info.plist | ||
| launch-c.in | ||
| launch-cxx.in | ||
| leftimage.bmp | ||
| NSIS.definitions.nsh.in | ||
| NSIS.template.in | ||
| SignMacApplications.cmake | ||
