build: drop the .NET Framework target#2365
Conversation
Drop the .NET Framework target (net472) from all projects and build scripts. Windows builds will now target the, only, TFM of `net10.0`. Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Remove all code that was conditional on a .NET Framework target. Now that we're only building against the CoreCLR, this code was dead. Note that we now lose the ability to use the embedded web view for Microsoft authentication, which was a .NET Framework-only feature (and therefore Windows only). Support for embedded web view flows will be restored at a later date via Avalonia WebView. Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Add the SupportedOSPlatform attributes to types that only run on the relevant specific OS platform. This will help ensure we're always gating use of OS-specific behaviour to that specific OS. Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Suppress the "This call site is reachable on all platforms" warning for all test projects - we use custom Xunit attributes to dynamically skip OS-specific tests unless the test is running on the appropriate OS. The analyzer for CA1416 does not know how to handle this. Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
mjcheetham
added
engineering
derrickstolee
left a comment
derrickstolee
left a comment
There was a problem hiding this comment.
I expected the framework version of the code to be deleted with the removal of the #if NETFRAMEWORK macros, but some chunks are keeping the framework version.
I suspect that the project dependencies that changed to match the core version make the framework-specific code now work in the core version, but I'm not able to connect the dots myself.
Thus, I only commented on the places where I saw the code stick instead of a deletion, just as a way for you to confirm that these are all intentianal.
| #else | ||
| // OS broker requires .NET Framework right now until we migrate to .NET 5.0 (net5.0-windows10.x.y.z) | ||
| return false; | ||
| #endif |
There was a problem hiding this comment.
ah, so since we have migrated we now have a different broker? Interesting.
There was a problem hiding this comment.
This is a stale comment. The MSAL team moved to a different model (MSAL Runtime + native broker interop library) since this was written. We will still be using the same broker access model on Windows as before.
| // { | ||
| // throw new Trace2InvalidOperationException(Context.Trace2, | ||
| // "Embedded web view is not available without a desktop session."); | ||
| // } |
There was a problem hiding this comment.
Intentional commented-out-code?
There was a problem hiding this comment.
Yes. There is a larger story behind this, sadly. If we wish to drop .NET Framework but also keep the embedded webview on Windows we'd need to bring along the Chromium-based WebView2 component, which is huge.
Instead we are going to look at one of two options:
- Avalonia WebView (which uses the OS-bundled webview component, or the IE fallback on Windows),
- dropping embedded entirely, and rely on Broker.
I understand the confusion. There are two cases where we had done
Many of these you are rightly confused by are the latter. Since we knew Windows was .NET Framework, and Mac/Linux was .NET Core, we could optimise at compile-time to strip extra code and dependencies from the Core (and therefore Mac/Linux) builds. Now every OS is using .NET Core, we must keep the Windows-only functionality in all builds, but gated behind a |
2 participants
Summary
Git Credential Manager no longer targets .NET Framework. This drops the
net472TFM everywhere, leavingnet10.0as the single target on all platforms (Windows included), then deletes the code that only existed to support the old runtime and tightens our OS-platform guarantees.Why
The product builds against the CoreCLR on every platform now. The .NET Framework target only added a second build pivot, a pile of conditional code, and Windows-only baggage. Collapsing to one modern TFM simplifies the build, the source, and everything downstream (packaging, CI, trimming/AOT work that follows).
What changes
net472from all projects and build scripts. Windows now builds the singlenet10.0TFM like every other platform.#if NETFRAMEWORKcode. With only the CoreCLR left, this code was unreachable.[SupportedOSPlatform]attributes to OS-specific types so use of platform-specific behaviour is gated at compile time.[WindowsFact],[PosixFact], …) skip OS-specific tests at runtime; the analyzer can't see that, so the warning is noise in tests.Caveat
Removing the .NET Framework target also removes the embedded web view for Microsoft authentication — a netfx-only, Windows-only feature. The goal will be to move to the Windows "WAM" broker by default before release to replace the UX offered by embedded wv.