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

Migrate update framework to Velopack #28743

Merged
merged 25 commits into from
Sep 4, 2024
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
0ee8918
initial implementation
smallketchup82 Jun 26, 2024
1025e5b
rewrite the restart function
smallketchup82 Jun 27, 2024
04c8df0
Merge branch 'master' into velopack
smallketchup82 Jun 27, 2024
36a3765
Replace with attemptexit to better display how restarting is borked
smallketchup82 Jun 27, 2024
fae8f5f
Refactor VeloUpdateManager
smallketchup82 Jul 4, 2024
cae3607
Fix up restarting
smallketchup82 Jul 4, 2024
c13f24d
Remove InstallingUpdate progress notification
smallketchup82 Jul 4, 2024
461b791
Remove SimpleUpdateManager
smallketchup82 Jul 4, 2024
9e01cf7
Move setupVelo logic higher up
smallketchup82 Jul 4, 2024
6a03092
Reformat
smallketchup82 Jul 4, 2024
72cf6bb
Allow downgrading
smallketchup82 Jul 4, 2024
4898cff
Restart patch
smallketchup82 Jul 4, 2024
71816c0
Resurrect SimpleUpdateManager as MobileUpdateNotifier
smallketchup82 Jul 5, 2024
fcede9a
Bump velopack version
smallketchup82 Aug 7, 2024
d5b5215
Merge branch 'master' into velopack
smoogipoo Aug 31, 2024
636ee50
Rename to VelopackUpdateManager
smoogipoo Aug 31, 2024
a038799
Update osu.Desktop.csproj
smallketchup82 Aug 31, 2024
b990af6
Use full name
peppy Sep 2, 2024
42e1168
Remove github token variable & pass null for the github token
smallketchup82 Sep 2, 2024
f8a6a6a
Request restart asynchronously to avoid blocking update thread
peppy Sep 2, 2024
68e6fa2
Make comment about velopack's init a bit more loud
peppy Sep 2, 2024
cd9b822
Pass through correct update to apply when calling `WaitExitThenApplyU…
peppy Sep 2, 2024
08224b4
Simplify update process by caching pending update info and early-hand…
peppy Sep 3, 2024
b610233
Don't log probable network failures to sentry
peppy Sep 3, 2024
e564e8c
Add todo about fixing stutter on update application
peppy Sep 4, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion osu.Android/OsuGameAndroid.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public override void SetHost(GameHost host)
host.Window.CursorState |= CursorState.Hidden;
}

protected override UpdateManager CreateUpdateManager() => new SimpleUpdateManager();
protected override UpdateManager CreateUpdateManager() => new MobileUpdateNotifier();

protected override BatteryInfo CreateBatteryInfo() => new AndroidBatteryInfo();

Expand Down
30 changes: 4 additions & 26 deletions osu.Desktop/OsuGameDesktop.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
// See the LICENCE file in the repository root for full licence text.

using System;
using System.Diagnostics;
using System.IO;
using System.Reflection;
using System.Runtime.Versioning;
using System.Threading.Tasks;
using Microsoft.Win32;
using osu.Desktop.Performance;
using osu.Desktop.Security;
Expand Down Expand Up @@ -102,35 +102,13 @@ protected override UpdateManager CreateUpdateManager()
if (!string.IsNullOrEmpty(packageManaged))
return new NoActionUpdateManager();

switch (RuntimeInfo.OS)
{
case RuntimeInfo.Platform.Windows:
Debug.Assert(OperatingSystem.IsWindows());

return new SquirrelUpdateManager();

default:
return new SimpleUpdateManager();
}
return new VelopackUpdateManager();
}

public override bool RestartAppWhenExited()
{
switch (RuntimeInfo.OS)
{
case RuntimeInfo.Platform.Windows:
Debug.Assert(OperatingSystem.IsWindows());

// Of note, this is an async method in squirrel that adds an arbitrary delay before returning
// likely to ensure the external process is in a good state.
//
// We're not waiting on that here, but the outro playing before the actual exit should be enough
// to cover this.
Squirrel.UpdateManager.RestartAppWhenExited().FireAndForget();
return true;
}

return base.RestartAppWhenExited();
Task.Run(() => Velopack.UpdateExe.Start()).FireAndForget();
return true;
}

protected override void LoadComplete()
Expand Down
55 changes: 13 additions & 42 deletions osu.Desktop/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System;
using System.IO;
using System.Runtime.Versioning;
using osu.Desktop.LegacyIpc;
using osu.Desktop.Windows;
using osu.Framework;
Expand All @@ -14,7 +13,7 @@
using osu.Game.IPC;
using osu.Game.Tournament;
using SDL;
using Squirrel;
using Velopack;

namespace osu.Desktop
{
Expand All @@ -31,19 +30,11 @@ public static class Program
[STAThread]
public static void Main(string[] args)
{
/*
Copy link
Member

@peppy peppy Sep 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems a similar method is used for unit test detection, but in some basic testing I couldn't break it in the same way we broke it last time (ie. inserting a Logger.Log call before the setup call).

"needs to be run before anything else" sounds like a common sense thing, but is there an actual reason you found which makes this a requirement? If so, I'd probably want that explicitly mentioned.

Copy link
Contributor Author

@smallketchup82 smallketchup82 Sep 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Velopack docs suggest that you must run it before everything else. However, as you found, that's not necessarily a strict requirement. In another example on Velopack's docs, code is executed before the .Run()

As for me, I decided "So you should put setupVelopack() before everything else, but putting stuff before it isn't the end of the world as it would've been with Squirrel. So I'll tone it down a bit, and just keep the advisory since it's good practice to run it ASAP."

Though, @caesay would likely know best about this

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made the inline comment a bit louder for now. Probably fine to continue.

* 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")
*/
// IMPORTANT DON'T IGNORE: For general sanity, velopack's setup needs to run before anything else.
// This has bitten us in the rear before (bricked updater), and although the underlying issue from
// last time has been fixed, let's not tempt fate.
setupVelopack();

if (OperatingSystem.IsWindows())
{
var windowsVersion = Environment.OSVersion.Version;
Expand All @@ -66,8 +57,6 @@ public static void Main(string[] args)
return;
}
}

setupSquirrel();
}

// NVIDIA profiles are based on the executable name of a process.
Expand Down Expand Up @@ -177,32 +166,14 @@ private static bool trySendIPCMessage(IIpcHost host, string cwd, string[] args)
return false;
}

[SupportedOSPlatform("windows")]
private static void setupSquirrel()
private static void setupVelopack()
{
SquirrelAwareApp.HandleEvents(onInitialInstall: (_, tools) =>
{
tools.CreateShortcutForThisExe();
tools.CreateUninstallerRegistryEntry();
WindowsAssociationManager.InstallAssociations();
}, onAppUpdate: (_, tools) =>
{
tools.CreateUninstallerRegistryEntry();
WindowsAssociationManager.UpdateAssociations();
}, onAppUninstall: (_, tools) =>
{
tools.RemoveShortcutForThisExe();
tools.RemoveUninstallerRegistryEntry();
WindowsAssociationManager.UninstallAssociations();
}, onEveryRun: (_, _, _) =>
{
// While setting the `ProcessAppUserModelId` fixes duplicate icons/shortcuts on the taskbar, it currently
// causes the right-click context menu to function incorrectly.
//
// This may turn out to be non-required after an alternative solution is implemented.
// see https://github.com/clowd/Clowd.Squirrel/issues/24
// tools.SetProcessAppUserModelId();
});
VelopackApp
.Build()
.WithFirstRun(v =>
{
if (OperatingSystem.IsWindows()) WindowsAssociationManager.InstallAssociations();
}).Run();
}
}
}
180 changes: 0 additions & 180 deletions osu.Desktop/Updater/SquirrelUpdateManager.cs

This file was deleted.

Loading
Loading