Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build(win): use UCRT64 environment instead of MinGW64. #2323

Merged
merged 3 commits into from
May 25, 2024
Merged

build(win): use UCRT64 environment instead of MinGW64. #2323

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
13318e7
Use UCRT64 environment instead of MinGW64.
tez011 Mar 29, 2024
6f97ddc
Use default minimal ANSI locale.
tez011 Mar 31, 2024
d15bc84
win: ucrt64: fix non-admin shortcut launch on UCRT64 runtime
psyke83 Mar 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 13 additions & 19 deletions .github/workflows/CI.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -935,29 +935,23 @@ jobs:
- name: Setup Dependencies Windows
uses: msys2/setup-msys2@v2
with:
msystem: ucrt64
update: true
install: >-
base-devel
diffutils
doxygen
git
make
mingw-w64-x86_64-binutils
mingw-w64-x86_64-boost
mingw-w64-x86_64-cmake
mingw-w64-x86_64-curl
mingw-w64-x86_64-graphviz
mingw-w64-x86_64-miniupnpc
mingw-w64-x86_64-nlohmann-json
mingw-w64-x86_64-nodejs
mingw-w64-x86_64-nsis
mingw-w64-x86_64-onevpl
mingw-w64-x86_64-openssl
mingw-w64-x86_64-opus
mingw-w64-x86_64-toolchain
nasm
wget
yasm
mingw-w64-ucrt-x86_64-boost
mingw-w64-ucrt-x86_64-cmake
mingw-w64-ucrt-x86_64-curl
mingw-w64-ucrt-x86_64-graphviz
mingw-w64-ucrt-x86_64-miniupnpc
mingw-w64-ucrt-x86_64-nlohmann-json
mingw-w64-ucrt-x86_64-nodejs
mingw-w64-ucrt-x86_64-nsis
mingw-w64-ucrt-x86_64-onevpl
mingw-w64-ucrt-x86_64-openssl
mingw-w64-ucrt-x86_64-opus
mingw-w64-ucrt-x86_64-toolchain

- name: Setup python
# use this instead of msys2 python due to known issues using wheels, https://www.msys2.org/docs/python/
Expand Down
33 changes: 14 additions & 19 deletions docs/source/building/windows.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Windows

Requirements
------------
First you need to install `MSYS2 <https://www.msys2.org>`__, then startup "MSYS2 MinGW 64-bit" and execute the following
First you need to install `MSYS2 <https://www.msys2.org>`__, then startup "MSYS2 UCRT64" and execute the following
codes.

Update all packages:
Expand All @@ -15,26 +15,21 @@ Install dependencies:
.. code-block:: bash

pacman -S \
base-devel \
cmake \
diffutils \
doxygen \
gcc \
git \
make \
mingw-w64-x86_64-binutils \
mingw-w64-x86_64-boost \
mingw-w64-x86_64-cmake \
mingw-w64-x86_64-curl \
mingw-w64-x86_64-graphviz \
mingw-w64-x86_64-miniupnpc \
mingw-w64-x86_64-nlohmann-json \
mingw-w64-x86_64-nodejs \
mingw-w64-x86_64-onevpl \
mingw-w64-x86_64-openssl \
mingw-w64-x86_64-opus \
mingw-w64-x86_64-rust \
mingw-w64-x86_64-toolchain \
mingw-w64-ucrt-x86_64-boost \
mingw-w64-ucrt-x86_64-cmake \
mingw-w64-ucrt-x86_64-curl \
mingw-w64-ucrt-x86_64-graphviz \
mingw-w64-ucrt-x86_64-miniupnpc \
mingw-w64-ucrt-x86_64-nlohmann-json \
mingw-w64-ucrt-x86_64-nodejs \
mingw-w64-ucrt-x86_64-nsis \
mingw-w64-ucrt-x86_64-onevpl \
mingw-w64-ucrt-x86_64-openssl \
mingw-w64-ucrt-x86_64-opus \
mingw-w64-ucrt-x86_64-rust \
mingw-w64-ucrt-x86_64-toolchain \
python \
python-pip

Expand Down
10 changes: 9 additions & 1 deletion src/config.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1272,17 +1272,25 @@
BOOST_LOG(fatal) << "Failed to apply config: "sv << err.what();
}

if (!config_loaded) {
#ifdef _WIN32
// UCRT64 raises an access denied exception if launching from the shortcut
// as non-admin and the config folder is not yet present; we can defer
// so that service instance will do the work instead.

if (!config_loaded && !shortcut_launch) {
BOOST_LOG(fatal) << "To relaunch Sunshine successfully, use the shortcut in the Start Menu. Do not run Sunshine.exe manually."sv;
std::this_thread::sleep_for(10s);
#else
if (!config_loaded) {

Check warning on line 1284 in src/config.cpp

View check run for this annotation

Codecov / codecov/patch

src/config.cpp#L1284 

Added line #L1284 was not covered by tests
#endif
return -1;
}

#ifdef _WIN32
// We have to wait until the config is loaded to handle these launches,
// because we need to have the correct base port loaded in our config.
// Exception: UCRT64 shortcut_launch instances may have no config loaded due to
// insufficient permissions to create folder; port defaults will be acceptable.
if (service_admin_launch) {
// This is a relaunch as admin to start the service
service_ctrl::start_service();
Expand Down
3 changes: 1 addition & 2 deletions src/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,7 @@ main(int argc, char *argv[]) {
task_pool_util::TaskPool::task_id_t force_shutdown = nullptr;

#ifdef _WIN32
// Switch default C standard library locale to UTF-8 on Windows 10 1803+
setlocale(LC_ALL, ".UTF-8");
setlocale(LC_ALL, "C");
ReenigneArcher marked this conversation as resolved.
Show resolved Hide resolved
#endif

#pragma GCC diagnostic push
Expand Down
5 changes: 3 additions & 2 deletions src_assets/windows/misc/migration/migrate-config.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,11 @@ if exist "%OLD_DIR%\credentials\" (
rem Create the credentials directory if it wasn't migrated or already existing
if not exist "%NEW_DIR%\credentials\" mkdir "%NEW_DIR%\credentials"

rem Disallow read access to the credentials directory for normal users
rem Note: We must use the SID directly because "Administrators" is localized
rem Disallow read access to the credentials directory contents for normal users
rem Note: We must use the SIDs directly because "Users" and "Administrators" are localized
icacls "%NEW_DIR%\credentials" /inheritance:r
icacls "%NEW_DIR%\credentials" /grant:r *S-1-5-32-544:(OI)(CI)(F)
icacls "%NEW_DIR%\credentials" /grant:r *S-1-5-32-545:(R)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still needed with the change to config.cpp?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so?

Test 1:

  • Delete existing config directory
  • Freshly install Sunshine built from current PR branch
  • Set port = 46989
  • Stop sunshine service and relaunch from shortcut as non-admin

Result: Sunshine launches with the correct custom web UI port.

Test 2:

  • Repeat test 1 with build that omits migrate-config.bat OR keep PR build+config from test 1 and remove Users read folder privilege manually.

Result: several second delay on "Waiting for Web UI to be ready....", and UI opens to the (incorrect) default port.


rem Migrate the covers directory
if exist "%OLD_DIR%\covers\" (
Expand Down
Loading