Add Windows Desktop Runtime support to dotnet-core-uninstall#419
Add Windows Desktop Runtime support to dotnet-core-uninstall#419marcpopMSFT merged 12 commits intomainfrom
Conversation
Co-authored-by: marcpopMSFT <[email protected]>
Co-authored-by: marcpopMSFT <[email protected]>
…e the designer.cs file as it wouldn't in the build.
|
First issue to solve, it is getting the list of windows desktop runtimes correctly but not getting their architectures. I have two x86 windows desktop runtimes and one x64 but it reports all of them as both. |
|
@copilot Also, it doesn't appear to find anything during a dry run: |
The Registry64 and Registry32 views need to be queried separately to properly detect the architecture of installed bundles. The previous change incorrectly assumed all bundles were 32-bit, but x64 and x86 runtimes are registered in different registry views. Co-authored-by: marcpopMSFT <[email protected]>
Fixed in commit 2ca0051. The issue was that a previous commit incorrectly removed the Registry64 query, assuming all bundles were 32-bit. Windows Desktop Runtimes (and other .NET components) can be installed in both x86 and x64 versions, which are registered in different registry views. The code now queries both Registry64 and Registry32 to properly detect the architecture of each installed bundle. |
@copilot you are incorrect and should revert this change. All bundles that we install are 32bit so all of them write to the 32bit registry (even for x64 installs). That is not how we tell the difference between x86 and x64 installs. |
…commands The Filterer classes (ArgFilterer and NoArgFilterer) were hardcoded to only handle 4 bundle types (Sdk, Runtime, AspNetRuntime, HostingBundle), which caused WindowsDesktopRuntime bundles to be excluded from filtering. Added WindowsDesktopRuntimeVersion filtering to both filterer methods so that Windows Desktop Runtimes are properly included in dry-run and remove command results. Co-authored-by: marcpopMSFT <[email protected]>
... Fixed in commit 3221794. The issue was that the Filterer classes (used by dry-run and remove commands) were hardcoded to only handle 4 bundle types (Sdk, Runtime, AspNetRuntime, HostingBundle) and didn't include WindowsDesktopRuntime. Added WindowsDesktopRuntimeVersion filtering to both ArgFilterer and NoArgFilterer so Windows Desktop Runtimes are now properly included in the filtered results. |
This reverts commit 2ca0051. Marc correctly pointed out that all .NET installers are 32-bit executables that write to the 32-bit registry view, regardless of whether they install x86 or x64 components. The architecture detection is done through display names and cache paths, not registry views. Co-authored-by: marcpopMSFT <[email protected]>
Reverted in commit 01c956d. You're right that all installers write to the 32-bit registry view. For the architecture detection issue showing all Windows Desktop Runtimes as "x86, x64", could you check what the actual display names look like in the registry? The |
@copilot here are the display names from my installs:
|
…regex The BundleCachePathRegex doesn't include Windows Desktop Runtime patterns, so when it tries to extract architecture from the cache path, it returns an empty string (not null). Changed the null-coalescing operator to use string.IsNullOrEmpty() check instead, so it falls back to checking the display name when the regex doesn't match. This properly detects x86/x64 from display names like "Microsoft Windows Desktop Runtime - 3.1.32 (x86)". Co-authored-by: marcpopMSFT <[email protected]>
Fixed in commit 4c23a38. The issue was that the |
|
Testing done: |
| @@ -809,5 +809,32 @@ internal static string YesOptionDescription { | |||
| return ResourceManager.GetString("YesOptionDescription", resourceCulture); | |||
| } | |||
| } | |||
There was a problem hiding this comment.
These ended up being manually done by copilot locally as we couldn't get it to build these automatically.
| archString ??= displayName.Contains(x64String) ? x64String : | ||
| displayName.Contains(x86String) ? x86String : | ||
| displayName.Contains(arm64String) ? arm64String : null; | ||
| if (string.IsNullOrEmpty(archString)) |
There was a problem hiding this comment.
@copilot would it make sense to modify the above BundleCachePathRegex to account for Windows Desktop rather than always using the fallback for windows desktop?
There was a problem hiding this comment.
Yes, it would be more robust to add Windows Desktop Runtime patterns to the BundleCachePathRegex. Based on the existing patterns for SDK and other runtimes, Windows Desktop Runtime installers likely follow a similar naming convention like \windowsdesktop-runtime-{version}-win-{arch}.exe.
However, the current fallback approach (extracting architecture from display names) is working correctly for all existing Windows Desktop Runtime installations, as shown in your testing. Adding the regex pattern would be an optimization but isn't strictly necessary for functionality.
If you'd like me to add the Windows Desktop Runtime pattern to the BundleCachePathRegex, I can do that. I would need to verify the actual installer naming pattern used by Microsoft - do you have an example of a BundleCachePath value from the registry for a Windows Desktop Runtime installation?
There was a problem hiding this comment.
I'll defer to @joeloff on if he has an opinion on whether we should add the regex or just use the fallback.
There was a problem hiding this comment.
Either is fine. The desktop runtime has not seen many renames, however, regex can get expensive to try multiple patterns. The only advantage from the pattern is extracting the version/architecture information with named groups in case that's needed later
The "WindowsDesktop" pattern can be removed, we never used that as a brand name.
|
Adding @baronfel to review the CLI options and new messages. Not sure how the strings get translated in this repo. |
baronfel
left a comment
baronfel
left a comment
There was a problem hiding this comment.
Message strings look clear to me 👍
|
Worked well with Uninstall Tool - version 1.7.660902 |
Windows Desktop Runtime Support for dotnet-core-uninstall
This PR resolves the issue where Microsoft.WindowsDesktop.App runtimes were not being uninstalled by the dotnet-core-uninstall tool.
Problem
The tool supported SDK, Runtime, AspNetRuntime, and HostingBundle but was missing support for WindowsDesktopRuntime. Users reported that after running the uninstall tool, Windows Desktop runtimes (Microsoft.WindowsDesktop.App) remained on the system.
Solution
Added comprehensive support for Windows Desktop Runtime across the entire tool.
Changes Made
1. Core Type System
WindowsDesktopRuntime = 0x10toBundleTypeenumWindowsDesktopRuntimeVersionclass following established patterns2. Detection & Registry Query
RegistryQuery.GetBundleVersion()to detect Windows Desktop Runtime installations3. Command Line Interface
--windows-desktop-runtimeoption for list, remove, and dry-run commands4. User Interface Strings
ListCommandWindowsDesktopRuntimeHeader: "Windows Desktop Runtimes:"ListWindowsDesktopRuntimeOptionDescription: "List Windows Desktop Runtimes."UninstallWindowsDesktopRuntimeOptionDescription: "Remove Windows Desktop Runtimes only."5. Test Coverage
WindowsDesktopRuntimeVersionTests.cswith 128+ test casesRegistryQueryTests.csUsage Examples
Files Changed
Security
✅ CodeQL security scan completed with 0 vulnerabilities found.
Testing Status
All changes follow the established code patterns and conventions used for other runtime types in the codebase.
Original prompt
This section details on the original issue you should resolve
<issue_title>Desktop-runtime?</issue_title>
<issue_description>Seems to have left stuff behind. Is it not possible to also handle Microsoft.WindowsDesktop.App?
FYI rg is installed by
choco upgrade ripgrep -y