Add ability to uninstall 5.0 #170
Conversation
sfoslund
force-pushed
the
RemoveUpperBound
branch
from
57e731d to
ff7ccfc
Compare
|
Looks good. I noticed there's some code that examines the DisplayVersion properties from the registry. Because of how the actual versions are calculated, the version in the DisplayName and the DisplayVersion will be different for 5.x installs. For example, for 5.0.100-preview.2-20176.6, the actual file version (which would also be used for the DisplayVersion) will be 5.1.20.17606. Not sure if that impacts what the tool expects to receive as input, but just wanted to call it out. |
Yeah, I noticed that so I removed some code that was using DisplayVersion before. I think the only uses now are taken directly from CoreScan and are just checking that it's not empty- we can always assume it's non-empty, right? |
Correct. The DisplayVersion should always be set. |
| private static readonly Regex _hostingBundleVersionDisplayNameRegex = new Regex(string.Format( | ||
| _runtimeVersionBasicRegexFormat, | ||
| _previewVersionHostingBundleDisplayNameRegex.ToString())); | ||
| private static readonly Regex _hostingBundleAuxVersionDisplayNameRegex = new Regex(string.Format( |
There was a problem hiding this comment.
these are removed due to applying corescan logic?
There was a problem hiding this comment.
Yes, they're no longer needed once we convert to corescan
| [InlineData("Microsoft ASP.NET Core 3.0.0 Preview 6 Build 19307.2 - Shared Framework")] | ||
| [InlineData("Microsoft ASP.NET Core 2.2.6 - Shared Framework")] | ||
| [InlineData("Microsoft ASP.NET Core 2.2.0 Preview 3 Build 35497 - Shared Framework")] | ||
| [InlineData("Microsoft ASP.NET Core 2.1.0 Release Candidate 1 - Shared Framework")] |
There was a problem hiding this comment.
why these tests are removed? due to corescan logic does not support it?
There was a problem hiding this comment.
Half of the deleted tests are moved here since they are no longer directly testing the regex. The other half are removed because corescan's logic is less restrictive about which bundles are considered .NET bundles. With the regex's we were looking for very specific display name strings and with the corescan logic we're less exact.
For example, out original logic might not have marked Microsoft ASP.NET Core 2.1.0 Release Candidate 1 - Shared Framework as a .NET bundle because there are extra spaced around the -. The corescan logic doesn't care about that kind of thing, which is good because it will be more resillient to small format changes.
| } | ||
|
|
||
| private static bool IsDotNetCoreBundleUninstaller(int? windowsInstaller) | ||
| internal static bool IsNetCoreBundle(string displayName, string displayVersion, string uninstallString, string bundleVersion) |
There was a problem hiding this comment.
@wli3 Most of this method comes from CS
| var hasAuxVersion = cachePathMatch.Groups[Regexes.AuxVersionGroupName].Success; | ||
| var footnote = hasAuxVersion ? | ||
| string.Format(LocalizableStrings.HostingBundleFootnoteFormat, displayName, versionString) : | ||
| null; | ||
|
|
||
| if (string.IsNullOrEmpty(displayName) || string.IsNullOrEmpty(versionString)) | ||
| // Classify the bundle type |
There was a problem hiding this comment.
This section is also taken from corescan
|
@wli3 the 2 main sections that were taken from corescan were marked above: |
Tested manually on windows and mac