build(macos): configure C++ standard and ICU root#5101
Conversation
|
I'm confused about setting CXX_STANDARD here. We already set it in cmake itself, and we always use fetch content in our CI builds, so I'm not sure this is actually an issue. |
|
The existing CXX_STANDARD setting applies only to the In my local build, Homebrew had Boost 1.90 installed, while the project requested exactly 1.89. CMake rejected the newer Homebrew Boost and fell back to FetchContent, so Boost.Locale was built as part of the project. That fetched Boost target then compiled against Homebrew’s ICU headers without a C++17+ default and failed on ICU headers using constructs like Passing That said, if you prefer, setting the project-wide C++ standard near the top-level CMake before dependencies are loaded would be cleaner than passing it from the macOS scripts. I also wouldn't be offended if you chose to reject as it is very much an edge case and easy to work around with |
|
I think this can probably affect Linux as well, so probably we could just set it in the top level CMakelists.txt as well as in the tests. |
|
Right, it probably does. Do you want me to take a crack at it? |
Yes please! |
|
Done. It builds cleanly on macOS, I did not test elsewhere. |
|
And for good measure I updated |
Pass the project C++ standard explicitly so fetched dependencies build against modern Homebrew headers. Point CMake at Homebrew's icu4c@78 prefix to match the dependency installed by the macOS build path. Mirror the manual macOS build script in CI so the two paths stay aligned.
1691f89 to
dce4987
Compare
Bundle ReportChanges will decrease total bundle size by 3.22kB (-0.2%) ⬇️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: sunshine-esmAssets Changed:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5101 +/- ##
==========================================
- Coverage 17.86% 17.85% -0.01%
==========================================
Files 111 111
Lines 24005 24005
Branches 10619 10619
==========================================
- Hits 4289 4287 -2
+ Misses 18169 15502 -2667
- Partials 1547 4216 +2669
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
|
|
The Linux failure was caused by setting the project C++ standard globally without also setting the CUDA standard before the I fixed that by setting |
|
Bug fixes: - Delay g_vdd_indices.push_back() and persist_display_count() until the new display is confirmed visible; roll back VddRemoveDisplay on failure without leaving stale index/persist state. - Free SP_DEVICE_INTERFACE_DETAIL_DATA before break in OpenDeviceHandle to avoid memory leak. - Remove duplicate and misspelled VDD_IOCTL_UNKONWN (same value as VDD_IOCTL_UPDATE). Robustness: - Replace DXGI-based enumerate_dxgi_outputs with EnumDisplayDevices-based enumerate_displays; DXGI_OUTPUT_DESC::DeviceName uses \\.\DISPLAYx format which never contains "PSCCDD0", making the old VDD-detection logic dead. - Rewrite persist_display_count to update only the vdd_display_count line in-place, preserving comments, blank lines, and config file ordering. - Use compare_exchange_strong in start_keepalive to prevent multiple threads (tray + HTTP + startup) from racing on thread creation. Security: - Add check_content_type + validate_csrf_token to addVddDisplay and removeVddDisplay POST endpoints. - Add validate_csrf_token to removeAllVddDisplays. - Fix addVddDisplay to set status=false when add_display fails. - Fix removeVddDisplay status to reflect actual result. - Add vdd::is_initialized check to removeAllVddDisplays; return proper error when driver is not available. Cleanup: - Remove unused $tp import from VirtualDisplay.vue. - Remove extra </table> tag in docs/configuration.md. fix(macos): preserve modifier state in input events (LizardByte#5102) build(macos): configure C++ standard and ICU root (LizardByte#5101) build(deps): Add SUNSHINE_SYSTEM_VULKAN_HEADERS option (LizardByte#5103) Signed-off-by: James Le Cuirot <chewi@gentoo.org> ci: remove moonlight discord release announcement (LizardByte#5099) build(deps): bump packaging/linux/flatpak/deps/shared-modules from `2dfad85` to `8c3f3cf` (LizardByte#5098) build(deps): bump packaging/linux/flatpak/deps/shared-modules Bumps [packaging/linux/flatpak/deps/shared-modules](https://github.com/flathub/shared-modules) from `2dfad85` to `8c3f3cf`. - [Commits](flathub/shared-modules@2dfad85...8c3f3cf) --- updated-dependencies: - dependency-name: packaging/linux/flatpak/deps/shared-modules dependency-version: 8c3f3cfa5a4af9a696ff0bfb3ed0eba404faaf5d dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> build(deps): bump third-party/build-deps from `cd7d45a` to `d8b1d18` (LizardByte#5097) Bumps [third-party/build-deps](https://github.com/LizardByte/build-deps) from `cd7d45a` to `d8b1d18`. - [Release notes](https://github.com/LizardByte/build-deps/releases) - [Commits](LizardByte/build-deps@cd7d45a...d8b1d18) --- updated-dependencies: - dependency-name: third-party/build-deps dependency-version: d8b1d18b7e82f8ee396bdd05e226896fa523b0df dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> fix: building without the system tray enabled (LizardByte#5092)
|
It's either a coincidence or this PR caused #5112 Do you have some idea why? It's not obvious to me looking at the changes here. |
|
I doubt it, the |
|
This was the only change though, unless the bug is in icu itself, which I didn't see any recent changes in. |
|
My windows build box is slow as hell and churning through git checkout recursive right now - i'll know more in maybe half an hour, but my working theory is that none of the mingw toolchain is pinned; something updated and caused a new icu dependency. Plausible? |
|
Seems a bit odd as the last update was 2 weeks ago. https://github.com/msys2/MINGW-packages/tree/master/mingw-w64-icu and all that changed was the pkgrel version. |
|
The more likely path is Boost. MSYS2 currently installs Boost 1.91, but Sunshine asks CMake for exactly Boost 1.89, so the system package is rejected and we fall back to FetchContent. After #5101, I do not yet know that C++23 specifically toggled the ICU dependency, but it's looking more likely with FetchContent being in play. |
|
The good news is that Windows builds and runs cleanly with: cmake -B build -G Ninja -S . -DBOOST_LOCALE_ENABLE_ICU=OFF ... I also checked the codebase for actual Boost.Locale usage. The only real call site I found is Linux-only: src/platform/linux/input/inputtino_keyboard.cpp auto utf8_str = boost::locale::conv::to_utf<wchar_t>(utf8, utf8 + size, "UTF-8"); So Windows does not appear to need Boost.Locale’s ICU backend. macOS may be similar, but I’d keep the first fix scoped to Windows unless you’d prefer a broader cleanup. Would you like me to work on a PR? I should be able to get it done today. Edit: Nevermind, I see you beat me to it. |



Description
Pass the project C++ 23 standard explicitly so fetched dependencies build against modern headers. C++23 was chosen because it's already used for the
sunshinetarget incmake/targets/common.cmakeand by the Homebrew formula inpackaging/sunshine.rb, but those don't apply early enough to dependencies built throughscripts/macos_build.sh. This can break when Boost falls back to FetchContent.Point CMake at Homebrew's icu4c@78 prefix to match the dependency installed by the macOS build path. Without this, CMake can pick up inconsistent ICU headers/configuration from the Apple SDK or system paths while Boost.Locale is built from source.
Mirror the manual macOS build script in CI so the two paths stay aligned, as requested in the script comments.
Screenshot
Issues Fixed or Closed
Roadmap Issues
Type of Change
Checklist
AI Usage