pressure-vessel: respect STEAM_COMPAT_FLAGS=search-cwd
If we have "search-cwd" in STEAM_COMPAT_FLAGS
, and we are running the
main program, we need to append to LD_LIBRARY_PATH
the working
directory of the game that we want to execute. Said directory is
expected to be STEAM_COMPAT_INSTALL_PATH
.
Fixes: #46 (closed)
This took me way more than expected. The whole process of how we decide what goes in LD_LIBRARY_PATH
is a bit confusing and sparse in a lot of places.
Then there was the session mode vs the single mode/relaunch to take into consideration.
In this MR there is my latest iteration where I used a similar check to the one in _v2-entry-point
, to evaluate if we are running the main program, and condensed all the necessary bits in just a single place.
/cc @smcv
Merge request reports
Activity
- Resolved by Ludovico de Nittis
I don't like this being in the adverb and done by default, because the purpose of the adverb is to run the command in a modified execution environment, like
env
andsetpriv
. I would prefer it to be like those commands in that it has no practical effect by default, but can be given options to modify the execution environment.For example, we use the adverb in various places in the
SteamLinuxRuntime
wrapper scripts, to run commands with a lock held.The whole process of how we decide what goes in
LD_LIBRARY_PATH
is a bit confusing and sparse in a lot of places.Yes. It's grown organically over time, we don't really have a very clear specification (although you got closer to one in T24167), and it will probably need to change it again for Flatpak support. It might make most sense to stop treating it as "just another environment variable" internally, and have "library search path" as a first-class-citizen variable - a
GPtrArray
ofgchar *
, or similar - that gets composed into a stringLD_LIBRARY_PATH
at the last possible moment.If we have to defer this change until we are ready to build
LD_LIBRARY_PATH
in a different or simpler way, or until we have decided whether relaunching is something we should always be doing, then so be it - it isn't a hugely high priority. I'll try to keep this in mind when I'm refactoring.
added 223 commits
-
9718d984...22e43459 - 222 commits from branch
master
- ad98c482 - pressure-vessel: Add support for the STEAM_COMPAT_FLAGS=search-cwd
-
9718d984...22e43459 - 222 commits from branch
added 1 commit
- f8211395 - pressure-vessel: Add support for the STEAM_COMPAT_FLAGS=search-cwd
mentioned in merge request steamlinuxruntime!33 (closed)
- Resolved by Ludovico de Nittis
added 1 commit
- 0ec97f20 - pressure-vessel: Add support for the STEAM_COMPAT_FLAGS=search-cwd
requested review from @smcv
The part that touches
PvRuntime
seems fine.I agree that
--search-cwd
doesn't seem very explicit about what we're doing.One idea: just parse
STEAM_COMPAT_FLAGS
by splitting it on commas. Don't have aPRESSURE_VESSEL_
environment variable, and don't have a command-line option either. This could be done either inPvRuntime
or inpv-wrap
.Another idea: have
STEAM_COMPAT_FLAGS
, together with a--enable-compat-flag=search-cwd
,--disable-compat-flag=search-cwd
override that takes precedence overSTEAM_COMPAT_FLAGS
.Either way, I think unknown compat flags should be ignored (with at most a
g_info()
).Right, the less bash we have, the better :)
I added the
STEAM_COMPAT_FLAGS
parsing inpv-wrap
(probably safer to handle future flags?) and I avoided adding the enable and disable options simply because right now there isn't a use for them. But if in the future this changes, we can always add them at a later time.
added 1 commit
- 9ed64292 - pressure-vessel: Add support for the env var STEAM_COMPAT_FLAGS
- Resolved by Ludovico de Nittis
Please could you add
Resolves: https://github.com/ValveSoftware/steam-runtime/issues/371
to the commit message? (I'm pretty sure it does.)
added 1 commit
- 35e4e76f - pressure-vessel: Add support for the env var STEAM_COMPAT_FLAGS
730 * The check used is similar to the one in _v2-entry-point. 731 * Try to keep them in sync. 732 */ 733 static gboolean 734 check_main_program (int argc, 735 char *argv[]) 736 { 737 gsize i; 738 const gchar *session_id = g_getenv ("STEAM_COMPAT_SESSION_ID"); 739 if (session_id == NULL) 740 /* Non-session mode, behaving like the main program */ 741 return TRUE; 742 743 for (i = 0; i < argc; i++) 744 { 745 if (g_strcmp0 (argv[i], "waitforexitandrun") == 0) This is parsing command-line options that ought to be opaque to pressure-vessel, which I'm never a big fan of.
It's also not actually going to work, I don't think, because Proton games (which don't need this behaviour!) do run pv-wrap as (simplified):
pressure-vessel-wrap --option --option -- /path/to/proton waitforexitandrun /path/to/hl2.exe
but the case where we actually need
search-cwd
is native Linux games, which run pv-wrap more like this:pressure-vessel-wrap --option --option -- /path/to/hl2.sh
In the short term, I think we can detect the main program by
g_getenv ("SteamAppId")
being non-NULL
.In the long term, I want to cut out most of the shell scripts and move more knowledge about the Steam compat-tool protocol into pressure-vessel itself, but I don't really have a design for that yet. Timo has talked about maybe introducing a v3 compat-tool protocol with explicit setup/open, run and teardown/close steps, which is something I have wanted for a long time, and perhaps moving the compat-tool protocol into pressure-vessel itself should wait for that.
Doesn't that mean that the check for
is_main
in_v2-entry-point
is wrong too?No, because we enter
_v2-entry-point
with a command-line like this:_v2-entry-point --verb=waitforexitandrun -- /path/to/proton waitforexitandrun /path/to/hl2.exe |<-- parsed by v2ep -->|<--treated as opaque from our point of view -->|
and it's the
--verb
option that_v2-entry-point
is paying attention to.If
_v2-entry-point
passed a new option--steam-compat-v2-verb=waitforexitandrun
topv-wrap
, then it would be OK forpv-wrap
to respect that option - but I don't think it's worth plumbing that in right now.changed this line in version 7 of the diff
added 1 commit
- f32f230a - pressure-vessel: Add support for the env var STEAM_COMPAT_FLAGS
enabled an automatic merge when the pipeline for f32f230a succeeds
mentioned in commit dd7c036c