search

git-ecosystem / git-credential-manager

Secure, cross-platform Git credential storage with authentication to GitHub, Azure Repos, and other popular Git hosting services.
Other
6.67k stars 1.73k forks source link

Migrate from .NET Framework on Windows #113

Open mjcheetham opened 4 years ago

mjcheetham commented 4 years ago

We are currently not actually using the .NET Core on Windows. We should actually use the .NET Core runtime!

The reason we are currently using .NET Framework instead is because MSAL does (didn't) support embedded browser on Windows when targeting .NET Core or .NET Standard. We can use the UseCustomWebUi extension point to restore this functionality.

Regenhardt commented 4 years ago

So your readme straight up lies saying "...built on .NET Core"? Could be "...built on .NET" or "...built on .NET Framework and soon .NET Core" instead.

rimrul commented 4 years ago

No. The macOS build is built on .NET Core. So the readme is merely inacurate.

dennisameling commented 3 years ago

@mjcheetham would you accept a PR that migrates the Windows bits to .NET Core? Moving to .NET Core should also unlock building natively for Windows ARM64. We're working on a semi-native ARM64 Git for Windows and while using the x86 GCMC works for now (through emulation), having a native binary would be fantastic.

Is there a specific reason why you want to use an embedded browser rather than the user's default browser? This is the official recommendation:

We recommend that you use the platform default, and this is typically the system browser. The system browser is better at remembering the users that have logged in before. If you need to change this behavior, use WithUseEmbeddedWebView(bool)

Note to self: this might be a viable approach for an embedded Web UI

mjcheetham commented 3 years ago

Hi @dennisameling,

Is there a specific reason why you want to use an embedded browser rather than the user's default browser?

There are/were a few reasons:

  1. To maintain the existing behaviour from Git Credential Manager for Window (GCMW) which used an embedded browser pop-up via the old ADAL auth library
  2. Without an intermediate window (take a look at GitHub and Bitbucket's UI flows with OAuth) where we can tell the user we're about to start a new browser window, just launching a browser window for some random authentication isn't a great user experience.

[..] would you accept a PR that migrates the Windows bits to .NET Core?

We're not wedded to the .NET Framework as the target runtime on Windows but need to consider "moving the user's cheese" for the UX. 🧀 ⚠️

We'd also need to be mindful of the impact of shipping a copy the .NET Core runtime with GCM Core has on installer/zip file size, and how bundlers of GCMC would respond. Git for Windows's libexec\git-core directory would now contain a helluva lot of binaries for example!

We can look at using the single-file publishing model of course to reduce the number of binaries shipped, and trimming to reduce the size. .NET 5 would likely be the preferred target runtime to do this trimming and single-file publishing with, due to several improvements made in the latest release.

Note to self: this might be a viable approach for an embedded Web UI

As I mentioned in this issue "We can use the UseCustomWebUi extension point to restore this functionality." - it's just a case of having enough time to get around to do that 😉

Ideally MSAL.NET would expose the WinForms-based embedded browser when running on .NET Core on Windows, but that seems to be combined with/is waiting on their replacement of the old browser with "WebView2" (which itself has runtime distribution considerations).

So to answer your question, "yes" we would accept a PR to move to .NET Core on Windows, but we'd also need to ensure the following at the same time:

(Note: on macOS we've recently removed the embedded browser workaround/native helper to rely on the default browser flow, but on Windows we have to be more considerate of the experience change that would bring with the various consumers of GCM which includes Sourcetree and Visual Studio).

vtbassmatt commented 3 years ago

I'm in favor of this change - being on two different and diverging runtimes isn't hurting us now but smells like the kind of tech debt which will hurt us later. Let's sequence this after a move to .NET 5, and we can spend the time between now and then figuring out how much 🧀 -moving we'll do to VS and Sourcetree.

vtbassmatt commented 2 years ago

As nice as this would be, the tradeoffs still don't really make sense. The bloat of bringing along the runtime is unacceptable to our distribution partners in Git for Windows, and the Win7 story would get needlessly complicated.

We should reevaluate this when: 1) we can drop Win7 and 2) some .NET flavor can get us down to 3x our size (as opposed to the 10x it is in .NET 5).

mjcheetham commented 1 year ago

Update on dropping .NET Framework on Windows.. work in MSAL, .NET 6 and 7 means that we may be at a point where we can consider trying this again.

There is still one aspect that means we may not be able to fully embrace single-file publishing and that's Windows 7 support. Msys2 (that Git for Windows depends on) has signaled that it will be dropping Windows 7 and 8.0 support: https://github.com/msys2/msys2.github.io/blob/source/web/news.md#2023-01-15---dropping-support-for-windows-7-and-80

dscho commented 1 year ago

Msys2 (that Git for Windows depends on) has signaled that it will be dropping Windows 7 and 8.0 support: https://github.com/msys2/msys2.github.io/blob/source/web/news.md#2023-01-15---dropping-support-for-windows-7-and-80

So has the Git for Windows project.

mjcheetham commented 12 months ago

Update on Windows 7 issues and single-file publishing.. the latest docs indicate that we can use single file publishing if we are using the .NET 6.0.3 runtime or later:

[IMPORTANT] To run a single file app on Windows 7, you must use .NET Runtime 6.0.3 or later.

https://learn.microsoft.com/en-us/dotnet/core/deploying/single-file/overview

mjcheetham commented 12 months ago

This work should now be entirely achievable with the latest .NET 8 release candidate, from my testing! 🎉

We can logically break this work down in to a few steps:

  1. Update Mac and Linux targets from .NET 7 to .NET 8 (in RC now, with GA due Nov 2023)

    • .NET 7 is not an LTS release, so we should be doing this anyway!

  2. Remove any .NET Framework-only code/projects

    • Drop WPF UI helpers; we've not been using them by default in Windows builds for a number of releases now.

  3. Retarget Windows builds to .NET 8 and remove conditionally compiled code

    • Drop all the #if NETFRAMEWORK code which is no longer required

  4. Prepare codebase for trimming

    • Enable source generation for System.Text.Json
    • Enable compiled binding for Avalonia UI components

  5. Enable trimming and single-file publishing

rimrul commented 11 months ago

Retarget Windows builds to .NET 8

That might be a slight issue. Cygwin has explicitly announced that they intend to keep supporting Windows 8.1 for the forseable future. AFAIK Msys2 and Git for Windows have similar plans. But .NET 7 dropped support for Windows 8.1.

dscho commented 11 months ago

Cygwin has explicitly announced that they intend to keep supporting Windows 8.1 for the forseable future.

Historically, we have been following their lead in Git for Windows. Because we had to. Like, when we still wanted to support Windows XP and Cygwin ended support for Windows XP, Git for Windows could not bear the torch to resurrect and maintain that support.

In this instance, I think it might actually be the other way around: Git for Windows could say that, you know, Windows 8.1 support ended on January 10, 2023. If you need to use Git on that Windows version, either build it yourself or use an older version of Git for Windows.

I'm not saying that this decision has been made, though. I'm just saying that it is an option, maybe the easiest option we have.

rimrul commented 11 months ago

We could also potentially disallow the Git Credential Manager option in the Git for Windows installer on Windows 8.1 and recommend a standalone install of the latest .NET Framework based version in that situation.