FEXInterpreter: Support portable installs #3929
Conversation
Sonicadvance1
force-pushed
the
portable
branch
from
8a2d262 to
fdf1cbe
Compare
Sonicadvance1
force-pushed
the
portable
branch
2 times, most recently
from
3ca853e to
897bace
Compare
Sonicadvance1
force-pushed
the
portable
branch
from
897bace to
a6d589f
Compare
|
This is working as intended and can now be reviewed/merged. |
Sonicadvance1
force-pushed
the
portable
branch
from
a6d589f to
27abb8b
Compare
Source/Tools/FEXLoader/FEXLoader.cpp
Outdated
| * @return true if the binfmt_misc handlers are installed and being used | ||
| */ | ||
| bool QueryInterpreterInstalled(const bool ExecutedWithFD, const FEX::Config::PortableInformation& Portable) { | ||
| // We know the FEX interpreter is installed through a couple paths. |
There was a problem hiding this comment.
Unclear what this line is trying to say. Are we checking any paths here to figure out if the FEX interpreter is installed? Which paths? None of the bullet points after mention any paths.
On third read, it seems you're using "path" in a figurative way, which is confusing in code that also does filesystem operations... Is the intent here that we have multiple methods of checking whether FEXInterpreter is installed?
There was a problem hiding this comment.
Yes, if the paths exist, or if ExecutedWithFD then we know that the Linux kernel binfmt_misc interpreter is installed. /proc doesn't need to be mounted inside of a chroot for example, so those two paths checks could not exist, but binfmt_misc interpreter is still installed.
There was a problem hiding this comment.
This is probably clearer:
// Check if FEX's binfmt_misc handlers are both installed.
// The explicit check can be omitted if FEX was executed from an FD,
// since this only happens if the kernel launched FEX through binfmt_misc
There was a problem hiding this comment.
Actually, scratch that, let me revisit this.
There was a problem hiding this comment.
I guess my proposal captures the essence of it, though it would be nice if the nuance of /proc possibly not being visible could be factored in as well. Then again /proc is typically mounted in a chroot, so this may be too much of an edge case to bother going into the detail.
Anyway, seems good enough.
Sonicadvance1
changed the title
Sonicadvance1
force-pushed
the
portable
branch
3 times, most recently
from
509ae13 to
5a5a5ec
Compare
|
PR title doesn't say "portable-ish" anymore, why did it say so before? |
Oh right, It's effectively portable as long as you don't care about the quirks that can occur by not having binfmt_misc handlers installed. Which is why I said "portable-ish" before, but it is a non-issue for most things we care about. |
Sonicadvance1
force-pushed
the
portable
branch
from
5a5a5ec to
c413d79
Compare
There was a problem hiding this comment.
Oh right, It's effectively portable as long as you don't care about the quirks that can occur by not having binfmt_misc handlers installed. Which is why I said "portable-ish" before, but it is a non-issue for most things we care about.
Ah, that makes sense. Sounds fine to advertise it without the "-ish" qualifier indeed then, since it's more of a side effect of binfmt_misc not being used than a limitation of portable mode.
A couple things to note here.
This expects multiple environment variables to be set
FEX_PORTABLE=1FEX_THUNKHOSTLIBS=<Local install path for thunks>FEX_THUNKHOSTLIBS32=<Local install path for thunks>FEX_THUNKGUESTLIBS=<Local install path for thunks>FEX_THUNKGUESTLIBS32=<Local install path for thunks>Some optional environment variables that can be set
<FEXInterpreter>/fex-emu/$HOME/.fex-emu/Config.jsonand$HOME/.fex-emu/AppConfig/<Files>.json<FEXInterpreter>/fex-emu/$HOME/.fex-emu/Telemetry/and various other data bits like IR cache, code cache and probably in the future more thingsThis also disables FEX trying to use the binfmt_misc handler even if it is installed, so it stays disjoint from the system install.
It also disables the three global config layers since it doesn't make sense in this config.
This isn't the default and shouldn't be expected as the default use case. It also likely breaks namespaces and chroots so shouldn't be used in those cases most of the time.