From 15d3d099669fb9d6af1fd8da42e82ec56caf2dd6 Mon Sep 17 00:00:00 2001 From: Janne Pulkkinen Date: Thu, 4 May 2023 19:54:50 +0300 Subject: [PATCH] Disable `background-wineserver` by default Disable `background-wineserver` by default, as it resolves two common and longstanding issues with typical Protontricks usage when the `wineserver` process is preserved between commands: * Graphical applications can crash due to an issue likely related to different X11 libraries used inside the Steam Runtime. This issue is tracked in #166. * Console output can become broken. This issue was reported in #223 (and likely other GitHub issues as well). The original reason for implementing the background `wineserver` process was to shorten `wine` command launch time, as Winetricks could take nearly a minute to launch in some situations. This seems to be mostly a non-issue now, with a `wine cmd.exe /c echo %ProgramFiles%` command taking around 0.7 seconds to launch on my system, and `protontricks good` taking around 20 seconds to complete, which is still very reasonable compared to 10 seconds with `--background-wineserver`. Fixes #166. Fixes #223. --- CHANGELOG.md | 3 +++ src/protontricks/cli/launch.py | 2 +- src/protontricks/cli/main.py | 7 ++++--- tests/cli/test_launch.py | 14 +++++++------- tests/cli/test_main.py | 33 ++++++++------------------------- 5 files changed, 23 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 08687bc..8dffb33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Added - Flatpak version of Steam is also detected with non-Flatpak installation of Protontricks +### Changed + - `--background-wineserver` is now disabled by default due to problems with crashing graphical applications and broken console output + ### Fixed - Fix detection of Steam library folders using non-standard capitalizations for `steamapps` - _Steam Linux Runtime - Sniper_ is no longer incorrectly reported as an unsupported runtime diff --git a/src/protontricks/cli/launch.py b/src/protontricks/cli/launch.py index 4c96b3e..f85f263 100644 --- a/src/protontricks/cli/launch.py +++ b/src/protontricks/cli/launch.py @@ -89,7 +89,7 @@ def main(args=None): ) parser.add_argument("executable", type=str) parser.add_argument("exec_args", nargs=argparse.REMAINDER) - parser.set_defaults(background_wineserver=None) + parser.set_defaults(background_wineserver=False) args = parser.parse_args(args) diff --git a/src/protontricks/cli/main.py b/src/protontricks/cli/main.py index c77d287..02174c7 100755 --- a/src/protontricks/cli/main.py +++ b/src/protontricks/cli/main.py @@ -115,7 +115,8 @@ def main(args=None, steam_path=None, steam_root=None): action="store_true", help=( "Launch a background wineserver process to improve Wine command " - "startup time. Default if bwrap is enabled." + "startup time. Disabled by default, as it can cause problems with " + "some graphical applications." ) ) parser.add_argument( @@ -124,10 +125,10 @@ def main(args=None, steam_path=None, steam_root=None): action="store_false", help=( "Do not launch a background wineserver process to improve Wine " - "command startup time. Default if bwrap is not enabled." + "command startup time." ) ) - parser.set_defaults(background_wineserver=None) + parser.set_defaults(background_wineserver=False) parser.add_argument("appid", type=int, nargs="?", default=None) parser.add_argument("winetricks_command", nargs=argparse.REMAINDER) diff --git a/tests/cli/test_launch.py b/tests/cli/test_launch.py index 4cf09e0..6dbc239 100644 --- a/tests/cli/test_launch.py +++ b/tests/cli/test_launch.py @@ -118,12 +118,13 @@ def _set_launch_args(*args, **kwargs): ]) # CLI flags are passed through to the main CLI entrypoint - assert cli_args[0:5] == [ - "--verbose", "--no-runtime", "--no-bwrap", "--no-term", "-c" + assert cli_args[0:6] == [ + "--verbose", "--no-runtime", "--no-bwrap", + "--no-background-wineserver", "--no-term", "-c" ] - assert cli_args[5].startswith("wine ") - assert cli_args[5].endswith("test.exe'") - assert cli_args[6] == "10" + assert cli_args[6].startswith("wine ") + assert cli_args[6].endswith("test.exe'") + assert cli_args[7] == "10" # Steam installation was provided to the main entrypoint assert str(cli_kwargs["steam_path"]) == str(steam_dir) @@ -161,8 +162,7 @@ def _set_launch_args(*args, **kwargs): # entrypoint assert argument in cli_args else: - assert "--background-wineserver" not in cli_args - assert "--no-background-wineserver" not in cli_args + assert "--no-background-wineserver" in cli_args def test_cli_error_handler_uncaught_exception( self, launch_cli, default_proton, steam_app_factory, monkeypatch, diff --git a/tests/cli/test_main.py b/tests/cli/test_main.py index c977d72..b2a5ca3 100644 --- a/tests/cli/test_main.py +++ b/tests/cli/test_main.py @@ -168,10 +168,6 @@ def test_run_winetricks_steam_runtime_v2( # inside the sandbox assert commands[0].args == str(wine_bin_dir / "bwrap-launcher") - # keepalive wineserver process launched in background to improve - # Wine command launch time - assert commands[1].args == str(wine_bin_dir / "wineserver-keepalive") - # winecfg was run command = commands[-1] @@ -282,10 +278,11 @@ def test_run_winetricks_steam_runtime_v2_no_bwrap( @pytest.mark.parametrize( "args,wineserver_launched", [ - # background wineserver enabled for bwrap by default - (["-c", "'echo nothing'", "20"], True), + # background wineserver disabled for bwrap by default + (["-c", "'echo nothing'", "20"], False), - # background wineserver disabled by default for everything else + # background wineserver also disabled by default for everything + # else (["--no-bwrap", "-c", "'echo nothing'", "20"], False), # Manually disable background wineserver @@ -626,8 +623,8 @@ def test_run_bwrap_default( """ Perform commands for two Proton apps, one using a Proton version using the legacy Steam Runtime and another app using newer Steam - Runtime with bwrap. Ensure that the correct defaults for `use_bwrap` - and `background_wineserver` are used in both cases. + Runtime with bwrap. Ensure that the correct default for `use_bwrap` + is used in both cases. Regression test for #150 """ @@ -646,33 +643,19 @@ def test_run_bwrap_default( name="Fake game 2", appid=20, compat_tool_name="new_proton" ) - # bwrap and background wineserver are disabled for the old app by - # default + # bwrap is disabled for the old app by default cli(["-v", "-c", "bash", "10"]) assert any( filter(lambda msg: "Using 'bwrap = False'" in msg, caplog.messages) ) - assert any( - filter( - lambda msg: "Using 'background-wineserver = False'" in msg, - caplog.messages - ) - ) caplog.clear() - # bwrap and background wineserver is enabled for the new app by - # default + # bwrap is enabled for the new app by default. cli(["-v", "-c", "bash", "20"]) assert any( filter(lambda msg: "Using 'bwrap = True'" in msg, caplog.messages) ) - assert any( - filter( - lambda msg: "Using 'background-wineserver = True'" in msg, - caplog.messages - ) - ) @pytest.mark.usefixtures("flatpak_sandbox") def test_select_steam_installation(