Make Avalonia UI the default on all platforms and move in-process #1207

mjcheetham · 2023-04-18T21:49:38Z

Today GCM has an architecture that delegates all GUI prompting to (bundled) external executables, using WPF-based helpers on Windows and Avalonia UI helpers on Mac and Linux. Each host provider has its own GUI helper executable, meaning we currently bundle four (GitHub.UI, GitLab.UI, Atlassian.Bitbucket.UI, git-credential-manager.ui [for generic auth]) helpers in addition to the core git-credential-manager CLI executable.

On Linux we use the "publish as a single-file" model for each of these executables, meaning we have a copy of the CLR included for each executable! That means five CLRs!

Now that the Avalonia UI-based GUI helpers have matured, we can look at dropping the WPF helpers and reduce the effort required to ship new GUI by 50% (we no longer need to do everything twice: once with WPF and once with Avalonia UI).

In addition, in a recent spike, I found that it may be possible to bring the Avalonia UI helpers in-process, with nearly zero net-new code. This would help reduce our installation footprint on Linux down to approximately 1/4 of the current size. The single executable model would also help decrease build times, and increase end-user performance since we no longer pay the process startup costs using external processes!

We need to be mindful that this change may result in a small increase in the Windows installation as we now bring in extra libraries used to support Avalonia UI, whereas previously we relied on WPF libraries as included with Windows 'for free'. This is expected to be minimal.

As for the current GUI extensibility model, we are not aware of any consumers, but it would not be much effort to keep the existing external process model, but now just default to using our in-process GUI options in the absence of external configuration. This still gives tools like VS or VSCode options to integrate native UI for GCM in the future.

Initially, we are still keeping the WPF helpers around on Windows but disabled by default.
We can drop these after at least one release using the in-proc Avalonia helpers.

To switch back to the WPF helpers, use credential.devUseLegacyUIHelpers or GCM_DEV_USELEGACYUIHELPERS.

Rename the WPF implementation of DialogWindow to prevent name collisions that would otherwise occur in upcoming commits where we merge the Avalonia implementation of DialogWindow into the Core project.

Explicitly target the WPF Dispatcher type using the fully qualified type name to prevent an ambiguous reference that would otherwise occur in upcomming commits where the Avalonia Dispatcher, and the GCM Dispatcher are merged in to the Core project.

Add the .Windows suffix to the WPF project namespaces to prevent name collisions that would otherwise occur in future commits where the Avalonia UI types are to be merged in to the core projects. Also clean up some unused XML namespace imports.

Set up a message pump on the main thread in the primary GCM executable in preparation to adding in-process, AvaloniaUI-based UI. Also move to explicitly shutdown the dispatcher correctly rather than just killing the process.

Default to using the in-proc Avalonia GUI on all platforms, with a fallback flag that can be set on Windows to continue to use the legacy WPF, out-of-proc UI helpers.

ldennington

This is so exciting! Great work, Matthew! 🚀

ldennington · 2023-04-24T15:44:03Z

src/linux/Packaging.Linux/layout.sh

@@ -81,15 +80,6 @@ $DOTNET_ROOT/dotnet publish "$GCM_SRC" \
 	-p:PublishSingleFile=true \
 	--output="$(make_absolute "$PAYLOAD")" || exit 1

-echo "Publishing core UI helper..."


Yay! So happy these are going away.

Builds are much faster and will be easier to maintain or modify going forwards (at least once we can drop the WPF helpers entirely).

ldennington · 2023-04-24T16:11:59Z

src/shared/Core/Authentication/OAuthAuthentication.cs

+                    // Default to the 'first' choice in the menu
+                    TerminalMenuItem choice = menu.Show(0);
+
+                    if (choice == browserItem) goto case OAuthAuthenticationModes.Browser;


I know this was already here, but just wanted to call out that I think it's elegant - one of the cases where a goto does make sense.

Yeah, the C# goto case keywords used together is a neat way to achieve this sort of control flow, without allowing for spaghetti jumps (can only jump to another case).

ldennington · 2023-04-24T16:31:32Z

src/shared/GitHub/GitHubAuthentication.cs

                var promptCts = new CancellationTokenSource();
                var tokenCts = new CancellationTokenSource();

                // Show the dialog with the device code but don't await its closure
-                Task promptTask = InvokeHelperAsync(command, promptArgs.ToString(), null, promptCts.Token);
+                Task promptTask = ShowDeviceCodeViaHelperAsync(dcr, command, args, promptCts.Token);


Nice - the refactors you've done here make the sequence of events a lot clearer imo.

src/shared/GitLab/GitLabAuthentication.cs

ldennington · 2023-04-24T16:42:22Z

src/shared/Core/Constants.cs

@@ -102,6 +102,7 @@ public static class EnvironmentVariables
            public const string OAuthDeviceEndpoint      = "GCM_OAUTH_DEVICE_ENDPOINT";
            public const string OAuthClientAuthHeader    = "GCM_OAUTH_USE_CLIENT_AUTH_HEADER";
            public const string OAuthDefaultUserName     = "GCM_OAUTH_DEFAULT_USERNAME";
+            public const string GcmDevUseLegacyUiHelpers = "GCM_DEV_USELEGACYUIHELPERS";


Why the use of DEV in the name here?

This isn't something we want to necessarily advertise or be something that users can rely on being there for the long term. It's a 'hidden' setting – we don't document it.

src/shared/GitLab/GitLabAuthentication.cs

ldennington · 2023-04-24T17:06:08Z

src/shared/Core/Core.csproj

@@ -21,12 +21,13 @@
    <PackageReference Include="Microsoft.Identity.Client" Version="4.52.0" />
    <PackageReference Include="Microsoft.Identity.Client.Extensions.Msal" Version="2.28.0" />
    <PackageReference Include="System.CommandLine" Version="2.0.0-beta1.21216.1" />
-    <PackageReference Include="Avalonia" Version="0.10.18" />
-    <PackageReference Include="Avalonia.Desktop" Version="0.10.18" />
+    <PackageReference Include="Avalonia" Version="11.0.0-preview6" />


Wow! Big jump.

Yeah they moved from 0.10 to dropping the 0. prefix with v11

ldennington · 2023-04-24T17:25:37Z

src/shared/Core/UI/ViewModels/WindowViewModel.cs

+        public bool ShowCustomWindowBorder
+        {
+            // Draw the window border explicitly on Windows
+            get => ShowCustomChrome && PlatformUtils.IsWindows();


I'm not quite understanding the Custom Chrome concept. Any docs I can take a look at to learn more?

The "client area" is the part of a window that we can draw into. Typically this is the box 'inside' of the window titlebar, border and controls that the OS provides.

When we "extend" the client area, we can now draw over the entire window surface, including the title bar and window controls. Note that on macOS the traffic-light controls are still always drawn, but on Windows we are left without any "X" control.

The last column shows the custom window chrome and controls we draw. Here we actually explicitly disable the traffic-light controls on macOS.

The preference on macOS is the middle option, and on Windows it's the right-most one.

Oh and on macOS the thin 1-pixel window border is always drawn, but on Windows it's not.
This means on Windows we want to manually draw a 1-px border or else our window can blend in to the background.

Simplify where we check for user interaction and/or terminal prompts during authentication, to only need to check at the start of each `GetAuthenticationAsync` call. Note that is is a behaviour change - we now require user interaction to be allowed to perform authentication even with only 1 mode available. This is technically more 'correct' because when the user disables interactivity, we should never require interaction to continue. Although we are not asking the user to select a mode of authentication, we still either need them to enter a username/password, or respond to a browser OAuth flow.

Support dark theme resource dictionaries.

To keep the size of the Windows distribution small, directly target the Win32 Avalonia packages rather than the Desktop package that includes macOS and Linux dependencies.

**Changes:** - Support ports in URL-scoped config (#825) - Support URL-scoped enterprise default settings (#1149) - Add support for client TLS certificates (#1152) - Add TRACE2 support(#1131, #1151, #1156, #1162) - Better browser detection inside of WSL (#1148) - Handle expired OAuth refresh token for generic auth (#1196) - Target *-latest runner images in CI workflow (#1178) - Various bug fixes: - Ensure we create a WindowsProcessManager on Windows (#1146) - Ensure we start child processes created with ProcessManager (#1177) - Fix app path name of Windows dropping file extension (#1181) - Ensure we init IEnvironment before SessionManager (#1167) - git: consistently read from stdout before exit wait (#1136) - trace2: guard against null pipe client in dispose (#1135) - Make Avalonia UI the default Windows and move to in-process (#1207) - Add Git configuration options for trace & debug (#1228) - Transition from Nerdbank.GitVersioning to a version file (#1231) - Add support for using the current Windows user for WAM on DevBox (#1197) - Various documentation updates: - org-rename: update references to GitCredentialManager (#1141) - issue templates: remove core suffix (#1180) - readme: add link to project roadmap (#1204) - docs: add bitbucket app password requirements (#1213) - .net tool: clarify install instructions (#1126) - docs: call out different GCM install paths in WSL docs (#1168) - docs: add trace2 to config/env documentation (#1230)

mjcheetham requested a review from ldennington April 18, 2023 21:49

mjcheetham force-pushed the inproc branch 2 times, most recently from eb36ba7 to cc6d0c3 Compare April 18, 2023 21:56

mjcheetham mentioned this pull request Apr 18, 2023

Use in-process GCM Avalonia-based UI helpers #1104

Closed

2 tasks

mjcheetham force-pushed the inproc branch 2 times, most recently from c436534 to 4230893 Compare April 18, 2023 23:05

mjcheetham self-assigned this Apr 19, 2023

This comment was marked as abuse.

Sign in to view

mjcheetham added 19 commits April 21, 2023 08:24

gcm-ui: fix WPF device code view namespaces

0b0ad60

wpf: rename DialogWindow to WpfDialogWindow

a5735f9

Rename the WPF implementation of DialogWindow to prevent name collisions that would otherwise occur in upcoming commits where we merge the Avalonia implementation of DialogWindow into the Core project.

gcm: set up message pump in main GCM executable

c0551fd

Set up a message pump on the main thread in the primary GCM executable in preparation to adding in-process, AvaloniaUI-based UI. Also move to explicitly shutdown the dispatcher correctly rather than just killing the process.

core: merge Core.UI.Avn and Core.UI into Core

1f3e99e

gcm-ui: merge GCM.UI.Avn in to Core

242ac22

bitbucket: merge .UI.Avn and .UI in to Atlassian.Bitbucket

23473cd

github: merge .UI.Avn and .UI in to GitHub

aab6fef

gitlab: merge .UI.Avn and .UI in to GitLab

fd3cbbf

basic: rename ByUi method to ViaHelper

cb13e2b

oauth: extract Helper and TTY prompt methods

20df3d7

bitbucket: extract Helper and TTY prompt methods

2162a98

github: extract Helper and TTY prompt methods

2441216

gitlab: extract Helper and TTY prompt methods

c6c9cb1

ui: use in-proc by default with fallback option for WPF

73e0e86

Default to using the in-proc Avalonia GUI on all platforms, with a fallback flag that can be set on Windows to continue to use the legacy WPF, out-of-proc UI helpers.

ui: default to showing GCM branding in generic prompts

7f6b07c

ui: set default GUI window title

4108595

ui: add window parent/cancellation helpers

7794843

mjcheetham force-pushed the inproc branch from 3a80e83 to 27b3451 Compare April 21, 2023 17:55

mjcheetham changed the title ~~Drop WPF UI helpers and move all UI in-process~~ Make Avalonia UI the default on all platforms and move in-process Apr 21, 2023

mjcheetham added the gui Specific to graphical user interface controls label Apr 21, 2023

mjcheetham marked this pull request as ready for review April 21, 2023 17:58

ldennington approved these changes Apr 24, 2023

View reviewed changes

ldennington mentioned this pull request Apr 24, 2023

Large installation size on Linux #1212

Closed

mjcheetham added 4 commits April 24, 2023 14:48

basic: add in-proc UI implementation

5756930

oauth: add in-proc UI implementation

f120a12

bitbucket: add in-proc UI implementation

4a8d5e4

github: add in-proc UI implementation

5c8d929

mjcheetham force-pushed the inproc branch from 27b3451 to 7481114 Compare April 24, 2023 21:48

gitlab: add in-proc UI implementation

2a37974

mjcheetham force-pushed the inproc branch from 7481114 to 815c988 Compare April 24, 2023 22:35

mjcheetham added 13 commits April 24, 2023 15:44

gitlab: move logos to resource files

fd5fbc3

ui: only include Avn.Diagnostics in debug

de1bd0f

ui: target latest version of AvaloniaUI

2cd5d69

ui: support dark theme base colour resources

fd66094

Support dark theme resource dictionaries.

ui: use custom window chrome on Windows

bd742ce

ui: fix deadlock in Avalonia runloop start on Windows

99e4c69

ui: enable IDE Avalonia designer

473d951

ui: add app version to about window

37f6100

ui: workaround AvaloniaUI/Avalonia #10296

b77ea32

ui: fix textbox layout bug in v11

67092bb

ui: draw window border on windows

6ec9124

windows: simplify Inno setup defn and reduce payload size

56d540f

To keep the size of the Windows distribution small, directly target the Win32 Avalonia packages rather than the Desktop package that includes macOS and Linux dependencies.

mjcheetham force-pushed the inproc branch from 815c988 to 56d540f Compare April 24, 2023 22:45

mjcheetham merged commit d6a4cf3 into git-ecosystem:main Apr 24, 2023

mjcheetham deleted the inproc branch April 24, 2023 23:45

ldennington mentioned this pull request May 2, 2023

dependencies: update to latest versions #1143

Closed

mjcheetham mentioned this pull request May 2, 2023

Create GCM 2.1.0 release #1235

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make Avalonia UI the default on all platforms and move in-process #1207

Make Avalonia UI the default on all platforms and move in-process #1207

mjcheetham commented Apr 18, 2023 •

edited

Loading

This comment was marked as abuse.

ldennington left a comment

ldennington Apr 24, 2023

mjcheetham Apr 24, 2023

ldennington Apr 24, 2023

mjcheetham Apr 24, 2023

ldennington Apr 24, 2023

ldennington Apr 24, 2023

mjcheetham Apr 24, 2023

ldennington Apr 24, 2023

mjcheetham Apr 24, 2023

ldennington Apr 24, 2023

mjcheetham Apr 24, 2023

mjcheetham Apr 24, 2023 •

edited

Loading

Make Avalonia UI the default on all platforms and move in-process #1207

Make Avalonia UI the default on all platforms and move in-process #1207

Conversation

mjcheetham commented Apr 18, 2023 • edited Loading

This comment was marked as abuse.

ldennington left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjcheetham Apr 24, 2023 • edited Loading

Choose a reason for hiding this comment

mjcheetham commented Apr 18, 2023 •

edited

Loading

mjcheetham Apr 24, 2023 •

edited

Loading