diff --git a/.github/workflows/desktop-build.yml b/.github/workflows/desktop-build.yml index 9e7c15673..97e375dee 100644 --- a/.github/workflows/desktop-build.yml +++ b/.github/workflows/desktop-build.yml @@ -289,24 +289,21 @@ jobs: shell: bash env: SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }} + # Same release string the client reports its events under (COCKATRICE_SENTRY_RELEASE + # = PROJECT_VERSION, see cmake/CrashReporting.cmake), so stack traces from this build + # de-symbolicate against these symbols. + SENTRY_RELEASE: ${{ needs.configure.outputs.sentry_release }} run: | paths=() for dir in build/cockatrice build/oracle build/servatrice; do [[ -d $dir ]] && paths+=("$dir") done if [[ ${#paths[@]} == 0 ]]; then - # Fail loudly instead of skipping: see the Windows/macOS leg below — this step - # is the only way symbols reach Sentry and `continue-on-error` keeps the job - # green either way, so a silent skip looks exactly like a successful upload. - echo "::error::No debug symbol files found; nothing would be uploaded" - exit 1 + echo "No build output dirs found, skipping symbol upload" + exit 0 fi # Project slug on Sentry; must match the project the baked DSN points at. - # No --release here: `debug-files upload` has no such option (sentry-cli errors on - # the unknown flag and uploads nothing), and debug information files are matched to - # events by debug ID via the event's debug_meta, not by release. The release grouping - # comes from the client event itself (COCKATRICE_SENTRY_RELEASE). - sentry-cli debug-files upload --org cockatrice --project cockatrice-desktop "${paths[@]}" + sentry-cli debug-files upload --org cockatrice --project cockatrice-desktop --release "$SENTRY_RELEASE" "${paths[@]}" # Delete used cache to emulate a ccache update. See https://github.com/actions/cache/issues/342 - name: "Delete remote compiler cache (ccache)" @@ -646,16 +643,18 @@ jobs: shell: bash env: SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }} + # Same release string the client reports its events under (COCKATRICE_SENTRY_RELEASE + # = PROJECT_VERSION, see cmake/CrashReporting.cmake), so stack traces from this build + # de-symbolicate against these symbols. + SENTRY_RELEASE: ${{ needs.configure.outputs.sentry_release }} run: | paths=() if [[ '${{ matrix.os }}' == 'Windows' ]]; then - # Top-level CMakeLists.txt links /DEBUG even in release, so the linker PDBs - # land next to the binaries. Ninja is single-config, so that is - # build// and NOT build//Release/ — the same globs the PDB - # artifact step below uses. - shopt -s nullglob - paths+=(build/cockatrice/*.pdb build/oracle/*.pdb build/servatrice/*.pdb) - shopt -u nullglob + # Top-level CMakeLists.txt links /DEBUG even in release, so PDBs land + # next to the binaries (same dirs as the PDB artifact step below). + for dir in build/cockatrice/Release build/oracle/Release build/servatrice/Release; do + [[ -d $dir ]] && paths+=("$dir") + done else # Release builds carry DWARF (-g, kept by CrashReporting.cmake); macOS keeps its # debug info in dSYM bundles, so materialize them before uploading. @@ -672,17 +671,11 @@ jobs: done fi if [[ ${#paths[@]} == 0 ]]; then - # Fail loudly instead of skipping: this step is the only way symbols reach - # Sentry, and `continue-on-error` keeps the job green either way, so a - # silent skip is indistinguishable from a successful upload. It also hid a - # wrong Windows path (build//Release, which Ninja never creates). - echo "::error::No debug symbol files found; nothing would be uploaded" - exit 1 + echo "No symbol paths found, skipping symbol upload" + exit 0 fi # Project slug on Sentry; must match the project the baked DSN points at. - # No --release: see the Linux leg above — the flag does not exist on - # `debug-files upload` and DIFs are matched by debug ID, not release. - sentry-cli debug-files upload --org cockatrice --project cockatrice-desktop "${paths[@]}" + sentry-cli debug-files upload --org cockatrice --project cockatrice-desktop --release "$SENTRY_RELEASE" "${paths[@]}" # CPack reports a failed makensis run as "Problem running NSIS command" # plus a path to NSISOutput.log, and that log lives only on the runner, so @@ -827,6 +820,19 @@ jobs: if-no-files-found: error path: ${{ steps.build.outputs.path }} + # Ninja is single-config: binaries and their PDBs land directly in the + # target directories instead of a Release subdirectory. + - name: "[Windows] Upload PDBs (Program Databases)" + if: matrix.os == 'Windows' && github.ref_type != 'tag' + uses: actions/upload-artifact@v7 + with: + if-no-files-found: error + name: ${{ steps.build.outputs.name }}-PDBs + path: | + build/cockatrice/*.pdb + build/oracle/*.pdb + build/servatrice/*.pdb + - name: "Upload to release" if: needs.configure.outputs.tag != null && matrix.make_package == '1' id: upload_release @@ -838,25 +844,6 @@ jobs: tag_name: ${{ needs.configure.outputs.tag }} run: gh release upload "$tag_name" "$asset_path#$asset_name" - # Ninja is single-config: binaries and their PDBs land directly in the - # target directories instead of a Release subdirectory. - # - # Archived for tag builds as well, and ordered after the release upload so - # a missing PDB can never block a release from being published. On a tag - # build this artifact is the only durable copy of the symbols: the Sentry - # upload is continue-on-error, so without it a failed upload would lose - # the ability to symbolicate that release permanently. - - name: "[Windows] Upload PDBs (Program Databases)" - if: matrix.os == 'Windows' - uses: actions/upload-artifact@v7 - with: - if-no-files-found: error - name: ${{ steps.build.outputs.name }}-PDBs - path: | - build/cockatrice/*.pdb - build/oracle/*.pdb - build/servatrice/*.pdb - - name: "Attest binary provenance" if: steps.upload_release.outcome == 'success' id: attestation diff --git a/cmake/CrashReporting.cmake b/cmake/CrashReporting.cmake index db016269f..2c8888800 100644 --- a/cmake/CrashReporting.cmake +++ b/cmake/CrashReporting.cmake @@ -143,39 +143,3 @@ else() "(WITH_CRASH_REPORTING=${WITH_CRASH_REPORTING}, WITH_CLIENT=${WITH_CLIENT})" ) endif() - -# Packaged release binaries must not ship the DWARF that the symbol upload needs: it is -# 80-90% of an unstripped binary and nobody downloads a 500 MB installer to get it. The -# build tree keeps -g (CI uploads symbols from there); the *packaged copy* is split at -# install time by a CPack install script, which extracts the debug info next to the staging -# dir and strips the binary. Only ELF (Linux/BSD) needs this: Windows links /DEBUG with -# separate PDBs, and macOS keeps its dSYM extracted from the bundle. -option(COCKATRICE_SPLIT_DEBUG_SYMBOLS "Strip DWARF out of packaged release binaries" ON) -set(COCKATRICE_DEBUG_SYMBOL_DIR - "${CMAKE_BINARY_DIR}/debug-symbols" - CACHE PATH "Where the packaged binaries' split debug information is written (kept out of the package)" -) -if(WITH_CRASH_REPORTING - AND WITH_CLIENT - AND COCKATRICE_SPLIT_DEBUG_SYMBOLS - AND CMAKE_BUILD_TYPE STREQUAL "Release" - AND UNIX - AND NOT APPLE -) - find_program(COCKATRICE_OBJCOPY NAMES objcopy) - find_program(COCKATRICE_STRIP_TOOL NAMES strip) - if(COCKATRICE_OBJCOPY AND COCKATRICE_STRIP_TOOL) - configure_file( - "${CMAKE_CURRENT_LIST_DIR}/SplitDebugSymbols.cmake.in" "${CMAKE_BINARY_DIR}/cmake/SplitDebugSymbols.cmake" @ONLY - ) - # PRE_BUILD runs after CPack stages the install tree and before the package is produced. - # CPACK_INSTALL_SCRIPTS would run too early (staging is still empty); the script walks - # CPACK_TEMPORARY_DIRECTORY, never CMAKE_INSTALL_PREFIX, and refuses to run otherwise. - list(APPEND CPACK_PRE_BUILD_SCRIPTS "${CMAKE_BINARY_DIR}/cmake/SplitDebugSymbols.cmake") - message(STATUS "Crash reporting: packaged binaries stripped, debug info split to ${COCKATRICE_DEBUG_SYMBOL_DIR}") - else() - message(WARNING "COCKATRICE_SPLIT_DEBUG_SYMBOLS is ON but objcopy/strip were not found; " - "packaged release binaries will keep their debug information" - ) - endif() -endif() diff --git a/cmake/SplitDebugSymbols.cmake.in b/cmake/SplitDebugSymbols.cmake.in deleted file mode 100644 index bfe954712..000000000 --- a/cmake/SplitDebugSymbols.cmake.in +++ /dev/null @@ -1,80 +0,0 @@ -# Splits DWARF out of the packaged ELF binaries, then strips them. -# -# This is a CPack pre-build script (CPACK_PRE_BUILD_SCRIPTS, wired in CrashReporting.cmake): -# CPack runs it after staging the install tree and before producing the package, so it only -# ever touches the copy that becomes the release artifact. The build tree is left alone: -g -# stays on, and CI uploads symbols to Sentry straight from build/. -# -# For every ELF file under CPACK_TEMPORARY_DIRECTORY the script extracts the debug information -# to COCKATRICE_DEBUG_SYMBOL_DIR and removes the debug sections from the packaged binary. -# COCKATRICE_DEBUG_SYMBOL_DIR is deliberately outside the staging tree, so sidecars never end -# up in the package; CI can archive/upload them if wanted (it normally uploads straight from -# the build tree). The only thing that must survive stripping is .eh_frame (allocatable, used -# for unwinding), which neither --only-keep-debug nor --strip-debug removes. -# -# Tool paths, the symbol directory and the tree to walk are baked in / provided at configure / -# run time; the guards below refuse to run on anything that is not a CPack staging directory, -# so a missing variable can never turn the glob into a filesystem-wide walk. - -set(_objcopy "@COCKATRICE_OBJCOPY@") -set(_strip "@COCKATRICE_STRIP_TOOL@") -set(_symbolDir "@COCKATRICE_DEBUG_SYMBOL_DIR@") - -if(_objcopy STREQUAL "" OR _strip STREQUAL "") - message(WARNING "split debug symbols: objcopy/strip not found at configure time, packaging unstripped binaries") - return() -endif() -if(_symbolDir STREQUAL "") - message(FATAL_ERROR "split debug symbols: COCKATRICE_DEBUG_SYMBOL_DIR is empty") -endif() - -# CPack sets CPACK_TEMPORARY_DIRECTORY to the staging tree; CMAKE_INSTALL_PREFIX is still the -# configured /usr prefix here, so globbing it would walk system directories. Refuse to run -# unless the path exists and really is a CPack staging tree. -if(NOT CPACK_TEMPORARY_DIRECTORY OR NOT IS_DIRECTORY "${CPACK_TEMPORARY_DIRECTORY}") - message(STATUS "split debug symbols: no CPack staging directory, skipping") - return() -endif() -if(NOT CPACK_TEMPORARY_DIRECTORY MATCHES "_CPack_Packages") - message(WARNING "split debug symbols: ${CPACK_TEMPORARY_DIRECTORY} is not a CPack staging tree, skipping") - return() -endif() - -file(MAKE_DIRECTORY "${_symbolDir}") - -file( - GLOB_RECURSE _entries - LIST_DIRECTORIES true - "${CPACK_TEMPORARY_DIRECTORY}/*" -) -set(_processed 0) -foreach(_file IN LISTS _entries) - if(IS_DIRECTORY "${_file}" OR IS_SYMLINK "${_file}") - continue() - endif() - # Only ELF: objcopy would fail on the icons, translations and Qt resources next to it. - file( - READ "${_file}" _magic - OFFSET 0 - LIMIT 4 - HEX - ) - if(NOT _magic STREQUAL "7f454c46") - continue() - endif() - get_filename_component(_name "${_file}" NAME) - set(_debug "${_symbolDir}/${_name}.debug") - execute_process(COMMAND "${_objcopy}" --only-keep-debug "${_file}" "${_debug}" RESULT_VARIABLE _rc) - if(NOT _rc EQUAL 0) - message(WARNING "split debug symbols: objcopy --only-keep-debug failed for ${_file}") - continue() - endif() - execute_process(COMMAND "${_strip}" --strip-debug --strip-unneeded "${_file}" RESULT_VARIABLE _rc) - if(NOT _rc EQUAL 0) - message(WARNING "split debug symbols: strip failed for ${_file}") - continue() - endif() - math(EXPR _processed "${_processed} + 1") -endforeach() - -message(STATUS "Split debug symbols for ${_processed} packaged ELF file(s); sidecars in ${_symbolDir}")