1
0
mirror of https://github.com/ppy/osu.git synced 2024-09-21 14:47:25 +08:00

Fix broken installer

Closes https://github.com/ppy/osu/issues/26510.

Time for a rant.

Technically, this "broke" with 9e8d07d314,
but it is actually an end result of upstream behaviours that I am
failing to find a better description for than "utterly broken".

Squirrel (the installer we use) has unit tests. Which is great, power
to them. However, the method in which that testing is implemented leads
to epic levels of WTF breakage.

To determine whether Squirrel is being tested right now, it is checking
all currently loaded assemblies, and determining that if any loaded
assembly contains the magic string of "NUNIT" - among others - it must
be being tested right now:

    2442721748/src/Squirrel/SimpleSplat/PlatformModeDetector.cs (L17-L32)

If one assumes that there is no conceivable way that an NUnit assembly
*may* be loaded *without* it being a test context, this *may* seem sane.
Foreshadowing.

(Now, to avoid being hypocritical, we also do this, *but* we do this
by checking if the *entry* assembly is an NUnit:

    92db55a527/osu.Framework/Development/DebugUtils.cs (L16-L34)

which seems *much* saner, no?)

Now, why did this break with 9e8d07d314
*specifically*, you might wonder?

Well the reason is this line:

    3d3f58c252/osu.Desktop/NVAPI.cs (L183)

Yes you are reading this correctly, it's not NVAPI anything itself that
breaks this, it is *a log statement*. To be precise, what the log
statement *does* to provoke this, is calling into framework. That causes
the framework assembly to load, *which* transitively loads the
`nunit.framework` assembly.

(If you ever find yourself wanting to find out this sort of cursed
knowledge - I hope you never need to - you can run something along
the lines of

    dotnet-trace collect --providers Microsoft-Windows-DotNETRuntime:4 -- .\osu!.exe

then open the resulting trace in PerfView, and then search the
`Microsoft-Windows-DotNETRuntime/AssemblyLoader/Start` log for
the cursed assembly. In this case, the relevant entry said something
along the lines of

    HasStack="True"
    ThreadID="23,924"
    ProcessorNumber="0"
    ClrInstanceID="6"
    AssemblyName="nunit.framework, Version=3.13.3.0, Culture=neutral, PublicKeyToken=2638cd05610744eb"
    AssemblyPath=""
    RequestingAssembly="osu.Framework, Version=2024.113.0.0, Culture=neutral, PublicKeyToken=null"
    AssemblyLoadContext="Default"
    RequestingAssemblyLoadContext="Default"
    ActivityID="/#21032/1/26/"

Either that or just comment the log line for kicks. But the above
is *much* faster.)

Now, what *happens* if Squirrel "detects" that it is being "tested"?
Well, it will refuse to close after executing the "hooks" defined via
`SquirrelAwareApp`:

    2442721748/src/Squirrel/SquirrelAwareApp.cs (L85-L88)

and it will also refuse to create version shortcuts:

    2442721748/src/Squirrel/UpdateManager.Shortcuts.cs (L63-L65)

Sounds familiar, don't it?

There are days on which I tire of computers. Today is one of them.
This commit is contained in:
Bartłomiej Dach 2024-01-13 21:39:32 +01:00
parent 3d3f58c252
commit 1e3c332658
No known key found for this signature in database

View File

@ -30,12 +30,19 @@ namespace osu.Desktop
[STAThread]
public static void Main(string[] args)
{
// NVIDIA profiles are based on the executable name of a process.
// Lazer and stable share the same executable name.
// Stable sets this setting to "Off", which may not be what we want, so let's force it back to the default "Auto" on startup.
NVAPI.ThreadedOptimisations = NvThreadControlSetting.OGL_THREAD_CONTROL_DEFAULT;
// run Squirrel first, as the app may exit after these run
/*
* WARNING: DO NOT PLACE **ANY** CODE ABOVE THE FOLLOWING BLOCK!
*
* Logic handling Squirrel MUST run before EVERYTHING if you do not want to break it.
* To be more precise: Squirrel is internally using a rather... crude method to determine whether it is running under NUnit,
* namely by checking loaded assemblies:
* https://github.com/clowd/Clowd.Squirrel/blob/24427217482deeeb9f2cacac555525edfc7bd9ac/src/Squirrel/SimpleSplat/PlatformModeDetector.cs#L17-L32
*
* If it finds ANY assembly from the ones listed above - REGARDLESS of the reason why it is loaded -
* the app will then do completely broken things like:
* - not creating system shortcuts (as the logic is if'd out if "running tests")
* - not exiting after the install / first-update / uninstall hooks are ran (as the `Environment.Exit()` calls are if'd out if "running tests")
*/
if (OperatingSystem.IsWindows())
{
var windowsVersion = Environment.OSVersion.Version;
@ -59,6 +66,11 @@ namespace osu.Desktop
setupSquirrel();
}
// NVIDIA profiles are based on the executable name of a process.
// Lazer and stable share the same executable name.
// Stable sets this setting to "Off", which may not be what we want, so let's force it back to the default "Auto" on startup.
NVAPI.ThreadedOptimisations = NvThreadControlSetting.OGL_THREAD_CONTROL_DEFAULT;
// Back up the cwd before DesktopGameHost changes it
string cwd = Environment.CurrentDirectory;