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.

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

Fix broken windows installer #26511

Merged
merged 1 commit into from
Jan 14, 2024

Commits on Jan 13, 2024

  1. Fix broken installer

    Closes ppy#26510.
    
    Time for a rant.
    
    Technically, this "broke" with 9e8d07d,
    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:
    
        https://github.com/clowd/Clowd.Squirrel/blob/24427217482deeeb9f2cacac555525edfc7bd9ac/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:
    
        https://github.com/ppy/osu-framework/blob/92db55a52742047b42c0b4b25327ce28bd991b44/osu.Framework/Development/DebugUtils.cs#L16-L34
    
    which seems *much* saner, no?)
    
    Now, why did this break with 9e8d07d
    *specifically*, you might wonder?
    
    Well the reason is this line:
    
        https://github.com/ppy/osu/blob/3d3f58c2523f519504d9156a684538e2aa0c094c/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="/ppy#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`:
    
        https://github.com/clowd/Clowd.Squirrel/blob/24427217482deeeb9f2cacac555525edfc7bd9ac/src/Squirrel/SquirrelAwareApp.cs#L85-L88
    
    and it will also refuse to create version shortcuts:
    
        https://github.com/clowd/Clowd.Squirrel/blob/24427217482deeeb9f2cacac555525edfc7bd9ac/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.
    bdach committed Jan 13, 2024
    Configuration menu
    Copy the full SHA
    1e3c332 View commit details
    Browse the repository at this point in the history