diff --git a/.editorconfig b/.editorconfig index 6a69cea353..e5dc8c8bdc 100644 --- a/.editorconfig +++ b/.editorconfig @@ -57,7 +57,8 @@ csharp_style_pattern_matching_over_as_with_null_check = true:suggestion csharp_style_prefer_extended_property_pattern = true:suggestion csharp_style_var_for_built_in_types = true:suggestion csharp_style_var_when_type_is_apparent = true:suggestion -csharp_style_var_elsewhere = false:suggestion +csharp_style_var_elsewhere = true:suggestion + [*.{cs,vb}] #### Naming styles #### diff --git a/src/chocolatey.tests.integration/scenarios/InfoScenarios.cs b/src/chocolatey.tests.integration/scenarios/InfoScenarios.cs index 7e9ed292c1..89f0bb94e6 100644 --- a/src/chocolatey.tests.integration/scenarios/InfoScenarios.cs +++ b/src/chocolatey.tests.integration/scenarios/InfoScenarios.cs @@ -185,8 +185,8 @@ public void Should_log_package_count_as_warning() public class When_searching_for_non_normalized_exact_package : CommandScenariosBase { - private string NonNormalizedVersion = "004.0.01.0"; - private string NormalizedVersion = "4.0.1"; + private const string NonNormalizedVersion = "004.0.01.0"; + private const string NormalizedVersion = "4.0.1"; public override void Context() { @@ -226,8 +226,8 @@ public void Should_log_package_count_as_warning() public class When_searching_for_non_normalized_exact_package_with_version_specified : CommandScenariosBase { - private string NonNormalizedVersion = "004.0.01.0"; - private string NormalizedVersion = "4.0.1"; + private const string NonNormalizedVersion = "004.0.01.0"; + private const string NormalizedVersion = "4.0.1"; public override void Context() { @@ -269,8 +269,8 @@ public void Should_log_package_count_as_warning() public class When_searching_for_non_normalized_exact_package_with_non_normalized_version_specified : CommandScenariosBase { - private string NonNormalizedVersion = "004.0.01.0"; - private string NormalizedVersion = "4.0.1"; + private const string NonNormalizedVersion = "004.0.01.0"; + private const string NormalizedVersion = "4.0.1"; public override void Context() { diff --git a/src/chocolatey.tests.integration/scenarios/UninstallScenarios.cs b/src/chocolatey.tests.integration/scenarios/UninstallScenarios.cs index 2a89acb514..cfa932c664 100644 --- a/src/chocolatey.tests.integration/scenarios/UninstallScenarios.cs +++ b/src/chocolatey.tests.integration/scenarios/UninstallScenarios.cs @@ -453,10 +453,20 @@ public void Should_uninstall_the_package_from_the_lib_directory() DirectoryAssert.DoesNotExist(packageDir); } - [Fact] - [Pending("Does not work under .Net 4.8, See issue #2690")] - [Broken] - public void Should_not_be_able_delete_the_rollback() + // Locking is inconsistent between client and server operating systems in .NET 4.8. + // On a server, if a file is Read and delete locked it can't be deleted, but on client systems it can. + [Fact, Platform(Exclude = "WindowsServer10")] + public void Should_have_deleted_the_rollback() + { + var packageDir = Path.Combine(Scenario.get_top_level(), "lib-bkp", Configuration.PackageNames); + + DirectoryAssert.DoesNotExist(packageDir); + } + + // Locking is inconsistent between client and server operating systems in .NET 4.8. + // On a server, if a file is Read and delete locked it can't be deleted, but on client systems it can. + [Fact, Platform("WindowsServer10")] + public void Should_not_have_deleted_the_rollback_on_server() { var packageDir = Path.Combine(Scenario.get_top_level(), "lib-bkp", Configuration.PackageNames); diff --git a/src/chocolatey.tests.integration/scenarios/UpgradeScenarios.cs b/src/chocolatey.tests.integration/scenarios/UpgradeScenarios.cs index 55b7fc4561..ada93f35bd 100644 --- a/src/chocolatey.tests.integration/scenarios/UpgradeScenarios.cs +++ b/src/chocolatey.tests.integration/scenarios/UpgradeScenarios.cs @@ -1932,20 +1932,20 @@ public class When_upgrading_a_package_with_a_read_and_delete_share_locked_file : { private PackageResult _packageResult; - private FileStream fileStream; + private FileStream _fileStream; public override void Context() { base.Context(); var fileToOpen = Path.Combine(Scenario.get_top_level(), "lib", Configuration.PackageNames, "tools", "chocolateyInstall.ps1"); - fileStream = new FileStream(fileToOpen, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.Read | FileShare.Delete); + _fileStream = new FileStream(fileToOpen, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.Read | FileShare.Delete); } public override void AfterObservations() { base.AfterObservations(); - fileStream.Close(); - fileStream.Dispose(); + _fileStream.Close(); + _fileStream.Dispose(); } public override void Because() @@ -1978,10 +1978,20 @@ public void Should_upgrade_the_package() } } - [Fact] - [Pending("Does not work under .Net 4.8, See issue #2690")] - [Broken] - public void Should_not_be_able_delete_the_rollback() + // Locking is inconsistent between client and server operating systems in .NET 4.8. + // On a server, if a file is Read and delete locked it can't be deleted, but on client systems it can. + [Fact, Platform(Exclude = "WindowsServer10")] + public void Should_have_deleted_the_rollback() + { + var packageDir = Path.Combine(Scenario.get_top_level(), "lib-bkp", Configuration.PackageNames); + + DirectoryAssert.DoesNotExist(packageDir); + } + + // Locking is inconsistent between client and server operating systems in .NET 4.8. + // On a server, if a file is Read and delete locked it can't be deleted, but on client systems it can. + [Fact, Platform("WindowsServer10")] + public void Should_not_have_deleted_the_rollback_on_server() { var packageDir = Path.Combine(Scenario.get_top_level(), "lib-bkp", Configuration.PackageNames); @@ -2045,20 +2055,20 @@ public class When_upgrading_a_package_with_an_exclusively_locked_file : Scenario { private PackageResult _packageResult; - private FileStream fileStream; + private FileStream _fileStream; public override void Context() { base.Context(); var fileToOpen = Path.Combine(Scenario.get_top_level(), "lib", Configuration.PackageNames, "tools", "chocolateyInstall.ps1"); - fileStream = new FileStream(fileToOpen, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None); + _fileStream = new FileStream(fileToOpen, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None); } public override void AfterObservations() { base.AfterObservations(); - fileStream.Close(); - fileStream.Dispose(); + _fileStream.Close(); + _fileStream.Dispose(); } public override void Because() @@ -2306,7 +2316,7 @@ public void Should_match_the_upgrade_version_of_one_dot_one_dot_zero() public class When_upgrading_a_package_that_does_not_exist : ScenariosBase { - private PackageResult packageResult; + private PackageResult _packageResult; public override void Context() { @@ -2317,7 +2327,7 @@ public override void Context() public override void Because() { Results = Service.Upgrade(Configuration); - packageResult = Results.FirstOrDefault().Value; + _packageResult = Results.FirstOrDefault().Value; } [Fact] @@ -2355,26 +2365,26 @@ public void Should_contain_a_message_that_no_packages_were_upgraded() [Fact] public void Should_not_have_a_successful_package_result() { - packageResult.Success.ShouldBeFalse(); + _packageResult.Success.ShouldBeFalse(); } [Fact] public void Should_not_have_inconclusive_package_result() { - packageResult.Inconclusive.ShouldBeFalse(); + _packageResult.Inconclusive.ShouldBeFalse(); } [Fact] public void Should_not_have_warning_package_result() { - packageResult.Warning.ShouldBeFalse(); + _packageResult.Warning.ShouldBeFalse(); } [Fact] public void Should_have_an_error_package_result() { bool errorFound = false; - foreach (var message in packageResult.Messages) + foreach (var message in _packageResult.Messages) { if (message.MessageType == ResultType.Error) { @@ -2389,7 +2399,7 @@ public void Should_have_an_error_package_result() public void Should_have_expected_error_in_package_result() { bool errorFound = false; - foreach (var message in packageResult.Messages) + foreach (var message in _packageResult.Messages) { if (message.MessageType == ResultType.Error) { @@ -2564,7 +2574,7 @@ public void Should_have_expected_error_in_package_result() [Platform(Exclude = "Mono")] public class When_upgrading_a_package_that_errors : ScenariosBase { - private PackageResult packageResult; + private PackageResult _packageResult; public override void Context() { @@ -2575,7 +2585,7 @@ public override void Context() public override void Because() { Results = Service.Upgrade(Configuration); - packageResult = Results.FirstOrDefault().Value; + _packageResult = Results.FirstOrDefault().Value; } [Fact] @@ -2637,26 +2647,26 @@ public void Should_contain_a_warning_message_that_it_was_unable_to_upgrade_a_pac [Fact] public void Should_not_have_a_successful_package_result() { - packageResult.Success.ShouldBeFalse(); + _packageResult.Success.ShouldBeFalse(); } [Fact] public void Should_not_have_inconclusive_package_result() { - packageResult.Inconclusive.ShouldBeFalse(); + _packageResult.Inconclusive.ShouldBeFalse(); } [Fact] public void Should_not_have_warning_package_result() { - packageResult.Warning.ShouldBeFalse(); + _packageResult.Warning.ShouldBeFalse(); } [Fact] public void Should_have_an_error_package_result() { bool errorFound = false; - foreach (var message in packageResult.Messages) + foreach (var message in _packageResult.Messages) { if (message.MessageType == ResultType.Error) { @@ -2671,7 +2681,7 @@ public void Should_have_an_error_package_result() public void Should_have_expected_error_in_package_result() { bool errorFound = false; - foreach (var message in packageResult.Messages) + foreach (var message in _packageResult.Messages) { if (message.MessageType == ResultType.Error) { diff --git a/src/chocolatey/infrastructure.app/services/ChocolateyPackageService.cs b/src/chocolatey/infrastructure.app/services/ChocolateyPackageService.cs index 134c23764e..f532157d36 100644 --- a/src/chocolatey/infrastructure.app/services/ChocolateyPackageService.cs +++ b/src/chocolatey/infrastructure.app/services/ChocolateyPackageService.cs @@ -1087,7 +1087,7 @@ private void BuildInstallExample(string packageName, StringBuilder sb, string co private int ReportActionSummary(ConcurrentDictionary packageResults, string actionName) { - var successes = packageResults.OrEmpty().Where(p => p.Value.Success && !p.Value.Inconclusive); + var successes = packageResults.OrEmpty().Where(p => p.Value.Success && !p.Value.Inconclusive).OrderBy(p => p.Value.Name); var failures = packageResults.Count(p => !p.Value.Success); var warnings = packageResults.Count(p => p.Value.Warning); var rebootPackages = packageResults.Count(p => new[] { 1641, 3010 }.Contains(p.Value.ExitCode)); @@ -1117,7 +1117,7 @@ private int ReportActionSummary(ConcurrentDictionary pack { this.Log().Info(""); this.Log().Warn("Warnings:"); - foreach (var warning in packageResults.Where(p => p.Value.Warning).OrEmpty()) + foreach (var warning in packageResults.Where(p => p.Value.Warning).OrderBy(p => p.Value.Name).OrEmpty()) { var warningMessage = warning.Value.Messages.FirstOrDefault(m => m.MessageType == ResultType.Warn); this.Log().Warn(" - {0}{1}".FormatWith(warning.Value.Name, warningMessage != null ? " - {0}".FormatWith(warningMessage.Message) : string.Empty)); @@ -1128,7 +1128,7 @@ private int ReportActionSummary(ConcurrentDictionary pack { this.Log().Info(""); this.Log().Warn("Packages requiring reboot:"); - foreach (var reboot in packageResults.Where(p => new[] { 1641, 3010 }.Contains(p.Value.ExitCode)).OrEmpty()) + foreach (var reboot in packageResults.Where(p => new[] { 1641, 3010 }.Contains(p.Value.ExitCode)).OrderBy(p => p.Value.Name).OrEmpty()) { this.Log().Warn(" - {0}{1}".FormatWith(reboot.Value.Name, reboot.Value.ExitCode != 0 ? " (exit code {0})".FormatWith(reboot.Value.ExitCode) : string.Empty)); } @@ -1141,7 +1141,7 @@ The recent package changes indicate a reboot is necessary. { this.Log().Info(""); this.Log().Error("Failures"); - foreach (var failure in packageResults.Where(p => !p.Value.Success).OrEmpty()) + foreach (var failure in packageResults.Where(p => !p.Value.Success).OrderBy(p => p.Value.Name).OrEmpty()) { var errorMessage = failure.Value.Messages.FirstOrDefault(m => m.MessageType == ResultType.Error); this.Log().Error( diff --git a/src/chocolatey/infrastructure.app/services/NugetService.cs b/src/chocolatey/infrastructure.app/services/NugetService.cs index b72212059e..e72edc2357 100644 --- a/src/chocolatey/infrastructure.app/services/NugetService.cs +++ b/src/chocolatey/infrastructure.app/services/NugetService.cs @@ -2034,6 +2034,8 @@ public virtual ConcurrentDictionary Uninstall(ChocolateyC config.CreateBackup(); + var packageVersionsToRemove = new SortedSet(PackageResultDependencyComparer.Instance); + foreach (string packageName in config.PackageNames.Split(new[] { ApplicationParameters.PackageNamesSeparator }, StringSplitOptions.RemoveEmptyEntries).OrEmpty()) { // We need to ensure we are using a clean configuration file @@ -2061,7 +2063,8 @@ public virtual ConcurrentDictionary Uninstall(ChocolateyC continue; } - var packageVersionsToRemove = installedPackageVersions.ToList(); + packageVersionsToRemove.AddRange(installedPackageVersions); + if (!config.AllVersions && installedPackageVersions.Count > 1) { if (config.PromptForConfirmation) @@ -2092,7 +2095,7 @@ public virtual ConcurrentDictionary Uninstall(ChocolateyC if (selection.IsEqualTo(abortChoice)) continue; if (selection.IsEqualTo(allVersionsChoice)) { - packageVersionsToRemove = installedPackageVersions.ToList(); + packageVersionsToRemove.AddRange(installedPackageVersions.ToList()); if (config.RegularOutput) this.Log().Info(() => "You selected to remove all versions of {0}".FormatWith(packageName)); } else @@ -2103,106 +2106,148 @@ public virtual ConcurrentDictionary Uninstall(ChocolateyC } } } + } - foreach (var installedPackage in packageVersionsToRemove) + foreach (var installedPackage in packageVersionsToRemove) + { + //Need to get this again for dependency resolution + allLocalPackages = GetInstalledPackages(config); + var packagesToUninstall = new HashSet(); + var localPackagesDependencyInfos = new HashSet(PackageIdentityComparer.Default); + var pathResolver = NugetCommon.GetPathResolver(_fileSystem); + var nugetProject = new FolderNuGetProject(ApplicationParameters.PackagesLocation, pathResolver, NuGetFramework.AnyFramework); + + var pkgInfo = _packageInfoService.Get(installedPackage.PackageMetadata); + if (pkgInfo != null && pkgInfo.IsPinned) { - //Need to get this again for dependency resolution - allLocalPackages = GetInstalledPackages(config); - var packagesToUninstall = new HashSet(); - var localPackagesDependencyInfos = new HashSet(PackageIdentityComparer.Default); - var pathResolver = NugetCommon.GetPathResolver(_fileSystem); - var nugetProject = new FolderNuGetProject(ApplicationParameters.PackagesLocation, pathResolver, NuGetFramework.AnyFramework); + var logMessage = "{0} is pinned. Skipping pinned package.".FormatWith(installedPackage.Name); + var pinnedResult = packageResultsToReturn.GetOrAdd(installedPackage.Name, new PackageResult(installedPackage.Name, null, null)); + pinnedResult.Messages.Add(new ResultMessage(ResultType.Warn, logMessage)); + pinnedResult.Messages.Add(new ResultMessage(ResultType.Inconclusive, logMessage)); + if (config.RegularOutput) this.Log().Warn(ChocolateyLoggers.Important, logMessage); + continue; + } - var pkgInfo = _packageInfoService.Get(installedPackage.PackageMetadata); - if (pkgInfo != null && pkgInfo.IsPinned) + if (performAction) + { + var allPackagesIdentities = allLocalPackages.Where(p => !p.Identity.Equals(installedPackage)).Select(p => p.SearchMetadata.Identity).ToList(); + localPackagesDependencyInfos.AddRange(allLocalPackages + .Select( + p => new SourcePackageDependencyInfo( + p.SearchMetadata.Identity, + p.PackageMetadata.DependencyGroups.SelectMany(x => x.Packages).ToList(), + true, + localRepositorySource, + null, + null))); + var uninstallationContext = new UninstallationContext(removeDependencies: config.ForceDependencies, forceRemove: config.Force); + + ICollection resolvedPackages = null; + try { - string logMessage = "{0} is pinned. Skipping pinned package.".FormatWith(packageName); - var pinnedResult = packageResultsToReturn.GetOrAdd(packageName, new PackageResult(packageName, null, null)); - pinnedResult.Messages.Add(new ResultMessage(ResultType.Warn, logMessage)); - pinnedResult.Messages.Add(new ResultMessage(ResultType.Inconclusive, logMessage)); - if (config.RegularOutput) this.Log().Warn(ChocolateyLoggers.Important, logMessage); + resolvedPackages = UninstallResolver.GetPackagesToBeUninstalled(installedPackage.Identity, localPackagesDependencyInfos, allPackagesIdentities, uninstallationContext); + } + catch (Exception ex) + { + this.Log().Warn("[NuGet]: {0}", ex.Message); + var result = packageResultsToReturn.GetOrAdd(installedPackage.Name + "." + installedPackage.Version, (key) => new PackageResult(installedPackage.PackageMetadata, pathResolver.GetInstallPath(installedPackage.PackageMetadata.Id))); + result.Messages.Add(new ResultMessage(ResultType.Error, ex.Message)); continue; } - if (performAction) + if (resolvedPackages is null) { - var allPackagesIdentities = allLocalPackages.Where(p => !p.Identity.Equals(installedPackage)).Select(p => p.SearchMetadata.Identity).ToList(); - localPackagesDependencyInfos.AddRange(allLocalPackages - .Select( - p => new SourcePackageDependencyInfo( - p.SearchMetadata.Identity, - p.PackageMetadata.DependencyGroups.SelectMany(x => x.Packages).ToList(), - true, - localRepositorySource, - null, - null))); - var uninstallationContext = new UninstallationContext(removeDependencies: config.ForceDependencies, forceRemove: config.Force); - var resolvedPackages = UninstallResolver.GetPackagesToBeUninstalled(installedPackage.Identity, localPackagesDependencyInfos, allPackagesIdentities, uninstallationContext); - packagesToUninstall.AddRange(allLocalPackages.Where(p => resolvedPackages.Contains(p.Identity))); + var result = packageResultsToReturn.GetOrAdd(installedPackage.Name + "." + installedPackage.Version, (key) => new PackageResult(installedPackage.PackageMetadata, pathResolver.GetInstallPath(installedPackage.PackageMetadata.Id))); + result.Messages.Add(new ResultMessage(ResultType.Error, "Unable to resolve dependency context. Not able to uninstall package.")); + continue; + } - foreach (var packageToUninstall in packagesToUninstall) + packagesToUninstall.AddRange(allLocalPackages.Where(p => resolvedPackages.Contains(p.Identity))); + + foreach (var packageToUninstall in packagesToUninstall) + { + try { - try + this.Log().Info(ChocolateyLoggers.Important, @" +{0} v{1}", packageToUninstall.Name, packageToUninstall.Identity.Version.ToNormalizedStringChecked()); + + var uninstallPkgInfo = _packageInfoService.Get(packageToUninstall.PackageMetadata); + BackupAndRunBeforeModify(packageToUninstall, uninstallPkgInfo, config, beforeUninstallAction); + + var key = packageToUninstall.Name + "." + packageToUninstall.Version.ToStringSafe(); + + if (packageResultsToReturn.TryRemove(key, out PackageResult removedPackage)) + { + // If we are here, we have already tried this package, as such we need + // to remove the package but keep any messages. This is required as + // Search Metadata may be null which is required later. + packageToUninstall.Messages.AddRange(removedPackage.Messages); + } + + var packageResult = packageResultsToReturn.GetOrAdd(key, packageToUninstall); + + if (!packageResult.Success) { - var uninstallPkgInfo = _packageInfoService.Get(packageToUninstall.PackageMetadata); - BackupAndRunBeforeModify(packageToUninstall, uninstallPkgInfo, config, beforeUninstallAction); + // Remove any previous error messages, otherwise the uninstall may show + // up as failing. + packageResult.Messages.RemoveAll(m => m.MessageType == ResultType.Error); + } - var packageResult = packageResultsToReturn.GetOrAdd(packageToUninstall.Name + "." + packageToUninstall.Version.ToStringSafe(), packageToUninstall); - packageResult.InstallLocation = packageToUninstall.InstallLocation; - string logMessage = "{0}{1} v{2}{3}".FormatWith(Environment.NewLine, packageToUninstall.Name, packageToUninstall.Version.ToStringSafe(), config.Force ? " (forced)" : string.Empty); - packageResult.Messages.Add(new ResultMessage(ResultType.Debug, ApplicationParameters.Messages.ContinueChocolateyAction)); + packageResult.InstallLocation = packageToUninstall.InstallLocation; + var logMessage = "{0}{1} v{2}{3}".FormatWith(Environment.NewLine, packageToUninstall.Name, packageToUninstall.Version.ToStringSafe(), config.Force ? " (forced)" : string.Empty); + packageResult.Messages.Add(new ResultMessage(ResultType.Debug, ApplicationParameters.Messages.ContinueChocolateyAction)); - if (continueAction != null) continueAction.Invoke(packageResult, config); + if (continueAction != null) continueAction.Invoke(packageResult, config); - if (packageToUninstall != null) + if (packageToUninstall != null) + { + packageToUninstall.InstallLocation = pathResolver.GetInstallPath(packageToUninstall.Identity); + try { - packageToUninstall.InstallLocation = pathResolver.GetInstallPath(packageToUninstall.Identity); - try - { - //It does not throw or return false if it fails to delete something... - //var ableToDelete = nugetProject.DeletePackage(packageToUninstall.Identity, projectContext, CancellationToken.None, shouldDeleteDirectory: false).GetAwaiter().GetResult(); + //It does not throw or return false if it fails to delete something... + //var ableToDelete = nugetProject.DeletePackage(packageToUninstall.Identity, projectContext, CancellationToken.None, shouldDeleteDirectory: false).GetAwaiter().GetResult(); - // If we have gotten here, it means we may only have files left to remove. - RemoveInstallationFilesUnsafe(packageToUninstall.PackageMetadata, pkgInfo); - } - catch (Exception ex) - { - string errorlogMessage = "{0}:{1} {2}".FormatWith("Unable to delete all existing package files. There will be leftover files requiring manual cleanup", Environment.NewLine, ex.Message); - this.Log().Warn(logMessage); - packageResult.Messages.Add(new ResultMessage(ResultType.Error, errorlogMessage)); + // If we have gotten here, it means we may only have files left to remove. + RemoveInstallationFilesUnsafe(packageToUninstall.PackageMetadata, pkgInfo); + } + catch (Exception ex) + { + var errorlogMessage = "{0}:{1} {2}".FormatWith("Unable to delete all existing package files. There will be leftover files requiring manual cleanup", Environment.NewLine, ex.Message); + this.Log().Warn(logMessage); + packageResult.Messages.Add(new ResultMessage(ResultType.Error, errorlogMessage)); - // Do not call continueAction again here as it has already been called once. - //if (continueAction != null) continueAction.Invoke(packageResult, config); - continue; - } + // Do not call continueAction again here as it has already been called once. + //if (continueAction != null) continueAction.Invoke(packageResult, config); + continue; } + } - this.Log().Info(ChocolateyLoggers.Important, " {0} has been successfully uninstalled.".FormatWith(packageToUninstall.Name)); + this.Log().Info(ChocolateyLoggers.Important, " {0} has been successfully uninstalled.".FormatWith(packageToUninstall.Name)); - EnsureNupkgRemoved(packageToUninstall.PackageMetadata); - RemoveInstallationFiles(packageToUninstall.PackageMetadata, uninstallPkgInfo); - } - catch (Exception ex) + EnsureNupkgRemoved(packageToUninstall.PackageMetadata); + RemoveInstallationFiles(packageToUninstall.PackageMetadata, uninstallPkgInfo); + } + catch (Exception ex) + { + var logMessage = "{0} not uninstalled. An error occurred during uninstall:{1} {2}".FormatWith(installedPackage.Name, Environment.NewLine, ex.Message); + this.Log().Error(ChocolateyLoggers.Important, logMessage); + var result = packageResultsToReturn.GetOrAdd(packageToUninstall.Name + "." + packageToUninstall.Version.ToStringSafe(), new PackageResult(packageToUninstall.PackageMetadata, pathResolver.GetInstallPath(packageToUninstall.PackageMetadata.Id))); + result.Messages.Add(new ResultMessage(ResultType.Error, logMessage)); + if (result.ExitCode == 0) result.ExitCode = 1; + + if (config.Features.StopOnFirstPackageFailure) { - var logMessage = "{0} not uninstalled. An error occurred during uninstall:{1} {2}".FormatWith(packageName, Environment.NewLine, ex.Message); - this.Log().Error(ChocolateyLoggers.Important, logMessage); - var result = packageResultsToReturn.GetOrAdd(packageToUninstall.Name + "." + packageToUninstall.Version.ToStringSafe(), new PackageResult(packageToUninstall.PackageMetadata, pathResolver.GetInstallPath(packageToUninstall.PackageMetadata.Id))); - result.Messages.Add(new ResultMessage(ResultType.Error, logMessage)); - if (result.ExitCode == 0) result.ExitCode = 1; - if (config.Features.StopOnFirstPackageFailure) - { - throw new ApplicationException("Stopping further execution as {0} has failed uninstallation".FormatWith(packageToUninstall.Name)); - } - // do not call continueAction - will result in multiple passes + throw new ApplicationException("Stopping further execution as {0} has failed uninstallation".FormatWith(packageToUninstall.Name)); } + // do not call continueAction - will result in multiple passes } } - else - { - // continue action won't be found b/c we are not actually uninstalling (this is noop) - var result = packageResultsToReturn.GetOrAdd(installedPackage.Name + "." + installedPackage.Version.ToStringSafe(), new PackageResult(installedPackage.PackageMetadata, pathResolver.GetInstallPath(installedPackage.PackageMetadata.Id))); - if (continueAction != null) continueAction.Invoke(result, config); - } + } + else + { + // continue action won't be found b/c we are not actually uninstalling (this is noop) + var result = packageResultsToReturn.GetOrAdd(installedPackage.Name + "." + installedPackage.Version.ToStringSafe(), new PackageResult(installedPackage.PackageMetadata, pathResolver.GetInstallPath(installedPackage.PackageMetadata.Id))); + if (continueAction != null) continueAction.Invoke(result, config); } } @@ -2639,5 +2684,47 @@ public virtual void remove_installation_files(IPackageMetadata removedPackage, C public IEnumerable get_all_installed_packages(ChocolateyConfiguration config) => GetInstalledPackages(config); #pragma warning restore IDE1006 + + private class PackageResultDependencyComparer : IComparer + { + private PackageResultDependencyComparer() + { + } + + public static IComparer Instance { get; } = new PackageResultDependencyComparer(); + + public int Compare(PackageResult x, PackageResult y) + { + if (ReferenceEquals(x, y)) + { + return 0; + } + + foreach (PackageDependency dependency in x.PackageMetadata?.DependencyGroups.SelectMany(d => d.Packages).OrEmpty()) + { + if (y.Name.IsEqualTo(dependency.Id)) + { + return -1; + } + } + + foreach (PackageDependency dependency in y.PackageMetadata?.DependencyGroups.SelectMany(d => d.Packages).OrEmpty()) + { + if (x.Name.IsEqualTo(dependency.Id)) + { + return 1; + } + } + + var result = string.Compare(x.Name, y.Name, StringComparison.OrdinalIgnoreCase); + + if (result == 0) + { + return x.Identity.Version.CompareTo(y.Identity.Version); + } + + return result; + } + } } } diff --git a/tests/chocolatey-tests/commands/choco-uninstall.Tests.ps1 b/tests/chocolatey-tests/commands/choco-uninstall.Tests.ps1 index 23d69caee1..cfd4131e0b 100644 --- a/tests/chocolatey-tests/commands/choco-uninstall.Tests.ps1 +++ b/tests/chocolatey-tests/commands/choco-uninstall.Tests.ps1 @@ -37,8 +37,7 @@ Describe "choco uninstall" -Tag Chocolatey, UninstallCommand { } } - # Broken in latest Chocolatey CLI v2.0.0-beta - Context "Uninstalling a package when chocolateyBeforeModify fails" -Tag Broken { + Context "Uninstalling a package when chocolateyBeforeModify fails" { BeforeAll { Restore-ChocolateyInstallSnapshot @@ -47,12 +46,16 @@ Describe "choco uninstall" -Tag Chocolatey, UninstallCommand { $Output = Invoke-Choco uninstall upgradepackage --confirm } - # Broken since v1.3.1 + # Broken since v1.0.0 It "Exits with Success (0)" -Tag Broken { $Output.ExitCode | Should -Be 0 -Because $Output.String } - It "Should have removed lib package directory" { + # This does not work as expected when runnning in test kitchen and with Chocolatey Licensed Extension installed. + # It is believed to be a problem with test kitchen, or the VM that we are using that is causing the issue. + # The result is that versioned backup files of each file in the package is created, instead of the package being + # removed. Consider the test partially broken. + It "Should have removed lib package directory" -Tag FossOnly { "$env:ChocolateyInstall\lib\upgradepackage" | Should -Not -Exist } @@ -124,7 +127,11 @@ Describe "choco uninstall" -Tag Chocolatey, UninstallCommand { "$env:ChocolateyInstall\lib\installpackage\a-locked-file.txt" | Should -Exist } - It "Should have removed file '<_>' in lib directory" -ForEach @('installpackage.nupkg', 'installpackage.nuspec', 'tools\casemismatch.exe', 'tools\Casemismatch.exe.ignore', 'tools\chocolateyBeforeModify.ps1', 'tools\chocolateyinstall.ps1', 'tools\chocolateyuninstall.ps1', 'tools\console.exe', 'tools\graphical.exe', 'tools\graphical.exe.gui', 'tools\not.installed.exe', 'tools\not.installed.exe.ignore', 'tools\simplefile.txt') { + # This does not work as expected when runnning in test kitchen and with Chocolatey Licensed Extension installed. + # It is believed to be a problem with test kitchen, or the VM that we are using that is causing the issue. + # The result is that versioned backup files of each file in the package is created, instead of the package being + # removed. Consider the test partially broken. + It "Should have removed file '<_>' in lib directory" -ForEach @('installpackage.nupkg', 'installpackage.nuspec', 'tools\casemismatch.exe', 'tools\Casemismatch.exe.ignore', 'tools\chocolateyBeforeModify.ps1', 'tools\chocolateyinstall.ps1', 'tools\chocolateyuninstall.ps1', 'tools\console.exe', 'tools\graphical.exe', 'tools\graphical.exe.gui', 'tools\not.installed.exe', 'tools\not.installed.exe.ignore', 'tools\simplefile.txt') -Tag FossOnly { "$env:ChocolateyInstall\lib\installpackage\$_" | Should -Not -Exist } @@ -161,15 +168,23 @@ Describe "choco uninstall" -Tag Chocolatey, UninstallCommand { $LockedFile.Dispose() } - It "Exits with Failure (1)" { + It "Exits with Failure (1)" -Tag FossOnly { $Output.ExitCode | Should -Be 1 -Because $Output.String } + It "Exits with Success (0)" -Tag Licensed { + $Output.ExitCode | Should -Be 0 -Because $Output.String + } + It "Should have kept locked file in lib directory" { "$env:ChocolateyInstall\lib\installpackage\a-locked-file.txt" | Should -Exist } - It "Should have removed file '<_>' in lib directory" -ForEach @('installpackage.nupkg', 'installpackage.nuspec', 'tools\casemismatch.exe', 'tools\Casemismatch.exe.ignore', 'tools\chocolateyBeforeModify.ps1', 'tools\chocolateyinstall.ps1', 'tools\chocolateyuninstall.ps1', 'tools\console.exe', 'tools\graphical.exe', 'tools\graphical.exe.gui', 'tools\not.installed.exe', 'tools\not.installed.exe.ignore', 'tools\simplefile.txt') { + # This does not work as expected when runnning in test kitchen and with Chocolatey Licensed Extension installed. + # It is believed to be a problem with test kitchen, or the VM that we are using that is causing the issue. + # The result is that versioned backup files of each file in the package is created, instead of the package being + # removed. Consider the test partially broken. + It "Should have removed file '<_>' in lib directory" -ForEach @('installpackage.nupkg', 'installpackage.nuspec', 'tools\casemismatch.exe', 'tools\Casemismatch.exe.ignore', 'tools\chocolateyBeforeModify.ps1', 'tools\chocolateyinstall.ps1', 'tools\chocolateyuninstall.ps1', 'tools\console.exe', 'tools\graphical.exe', 'tools\graphical.exe.gui', 'tools\not.installed.exe', 'tools\not.installed.exe.ignore', 'tools\simplefile.txt') -Tag FossOnly { "$env:ChocolateyInstall\lib\installpackage\$_" | Should -Not -Exist } @@ -181,15 +196,210 @@ Describe "choco uninstall" -Tag Chocolatey, UninstallCommand { "$env:ChocolateyInstall\lib-bkp\upgradepackage" | Should -Not -Exist } - It "Reports no package uninstalled" { + It "Reports no package uninstalled" -Tag FossOnly { $Output.Lines | Should -Contain "Chocolatey uninstalled 0/1 packages. 1 packages failed." } - It "Outputs not able to remove all package files" { + It "Outputs not able to remove all package files" -Tag FossOnly { $Output.String | Should -Match "installpackage - Unable to delete all existing package files. There will be leftover files requiring manual cleanup" } } + Context "When specifying multiple packages where one is a dependency should not fail uninstallation" { + BeforeAll { + Restore-ChocolateyInstallSnapshot + + $null = Invoke-Choco install hasdependency --confirm + + $Output = Invoke-Choco uninstall isdependency hasdependency --confirm + } + + It "Exits with Success (0)" { + $Output.ExitCode | Should -Be 0 -Because $Output.String + } + + # When a file exists before initial installation, it will be considered as part of the + # package files. This is NuGet behavior. This happens during existing files for upgrades as well. + # We might want to rollback files in this case, but it is not possible as the backup has been removed before + # any locked files are being tried to be removed. + It "Should have removed <_>" -ForEach @('isdependency', 'hasdependency') -Tag FossOnly { + "$env:ChocolateyInstall\lib\$_" | Should -Not -Exist -Because $Output.String + } + + It "Should not have removed isexactversiondependency" { + "$env:ChocolateyInstall\lib\isexactversiondependency" | Should -Exist -Because $Output.String + } + + It "Outputs <_> was succcesfully uninstalled" -ForEach @('isdependency', 'hasdependency') -Tag FossOnly { + $Output.Lines | Should -Contain "$_ has been successfully uninstalled." -Because $Output.String + } + + It "Does not output isexactversiondependency being uninstalled" { + $Output.Lines | Should -Not -Contain "isexactversiondependency has been successfully uninstalled." -Because $Output.String + } + + It "Outputs 2/2 packages uninstalled" { + $Output.Lines | Should -Contain "Chocolatey uninstalled 2/2 packages." -Because $Output.String + } + } + + Context "When specifying multiple packages where one is a dependency should not fail uninstallation (forced dependencies)" { + BeforeAll { + Restore-ChocolateyInstallSnapshot + + $null = Invoke-Choco install hasdependency --confirm + + $Output = Invoke-Choco uninstall isdependency hasdependency --confirm --force-dependencies + } + + It "Exits with Success (0)" { + $Output.ExitCode | Should -Be 0 -Because $Output.String + } + + # This does not work as expected when runnning in test kitchen and with Chocolatey Licensed Extension installed. + # It is believed to be a problem with test kitchen, or the VM that we are using that is causing the issue. + # The result is that versioned backup files of each file in the package is created, instead of the package being + # removed. Consider the test partially broken. + It "Should have removed <_>" -ForEach @('isdependency', 'hasdependency', 'isexactversiondependency') -Tag FossOnly { + "$env:ChocolateyInstall\lib\$_" | Should -Not -Exist -Because $Output.String + } + + It "Outputs <_> was succcesfully uninstalled" -ForEach @('isdependency', 'hasdependency', 'isexactversiondependency') { + $Output.Lines | Should -Contain "$_ has been successfully uninstalled." -Because $Output.String + } + + It "Outputs 3/3 packages uninstalled" { + $Output.Lines | Should -Contain "Chocolatey uninstalled 3/3 packages." -Because $Output.String + } + } + + Context "When specifying non-existing package before and after failing package does not abort execution" { + BeforeAll { + $null = Invoke-Choco install uninstallfailure installpackage --confirm + + $Output = Invoke-Choco uninstall packageA uninstallfailure packageB installpackage --confirm + } + + It "Exits with Failure (-1)" { + $Output.ExitCode | Should -Be -1 -Because $Output.String + } + + It "Outputs package not existing (<_>)" -ForEach @('packageA', 'packageB') { + $Output.Lines | Should -Contain "$_ is not installed. Cannot uninstall a non-existent package." -Because $Output.String + $Output.Lines | Should -Contain "- $_ - $_ is not installed. Cannot uninstall a non-existent package." -Because $Output.String + } + + It "Outputs failing to uninstall package uninstallfailure" { + $Output.Lines | Should -Contain "uninstallfailure not uninstalled. An error occurred during uninstall:" -Because $Output.String + $Output.Lines | Should -Contain "uninstallfailure uninstall not successful." -Because $Output.String + $Output.String | Should -Match "- uninstallfailure \(exited -1\) - Error while running" + } + + # This does not work as expected when runnning in test kitchen and with Chocolatey Licensed Extension installed. + # It is believed to be a problem with test kitchen, or the VM that we are using that is causing the issue. + # The result is that versioned backup files of each file in the package is created, instead of the package being + # removed. Consider the test partially broken. + It "Should have uninstall package installpackage" -Tag FossOnly { + "$env:ChocolateyInstall\lib\installpackage" | Should -Not -Exist -Because $Output.String + } + + # This does not work as expected when runnning in test kitchen and with Chocolatey Licensed Extension installed. + # It is believed to be a problem with test kitchen, or the VM that we are using that is causing the issue. + # The result is that versioned backup files of each file in the package is created, instead of the package being + # removed. Consider the test partially broken. + It "Outputs successful uninstall of installpackage" -Tag FossOnly { + $Output.Lines | Should -Contain "installpackage has been successfully uninstalled." -Because $Output.String + } + + It "Outputs 1/3 successful uninstalls" { + $Output.Lines | Should -Contain "Chocolatey uninstalled 1/4 packages. 3 packages failed." -Because $Output.String + } + } + + Context "When specifying multiple packages where one is a nested dependency should not fail uninstallation" { + BeforeAll { + Restore-ChocolateyInstallSnapshot + + $null = Invoke-Choco install toplevelwithnesteddependencies --confirm + + $Output = Invoke-Choco uninstall isdependency toplevelwithnesteddependencies --confirm + } + + It "Exits with Failure (1)" { + $Output.ExitCode | Should -Be 1 -Because $Output.String + } + + # This does not work as expected when runnning in test kitchen and with Chocolatey Licensed Extension installed. + # It is believed to be a problem with test kitchen, or the VM that we are using that is causing the issue. + # The result is that versioned backup files of each file in the package is created, instead of the package being + # removed. Consider the test partially broken. + It "Should have removed package toplevelwithnesteddependencies" -Tag FossOnly { + "$env:ChocolateyInstall\lib\toplevelwithnesteddependencies" | Should -Not -Exist -Because $Output.String + } + + It "Should not have removed <_>" -ForEach @('hasdependency', 'hasnesteddependency', 'isdependency', 'isexactversiondependency', 'toplevelhasexactversiondependency') { + "$env:ChocolateyInstall\lib\$_" | Should -Exist -Because $Output.String + } + + It "Outputs toplevelwithnesteddependencies was succcesfully uninstalled" { + $Output.Lines | Should -Contain "toplevelwithnesteddependencies has been successfully uninstalled." -Because $Output.String + } + + It "Does not output <_> being uninstalled" -ForEach @('hasdependency', 'hasnesteddependency', 'isdependency', 'isexactversiondependency', 'toplevelhasexactversiondependency') { + $Output.Lines | Should -Not -Contain "$_ has been successfully uninstalled." -Because $Output.String + } + + It "Outputs warning about package being unable to be uninstalled due to being a dependency" { + $Output.Lines | Should -Contain "[NuGet]: Unable to uninstall 'isdependency.2.1.0' because 'hasdependency.2.0.1' depends on it." + } + + It "Outputs 1/2 packages uninstalled with 1 failed package" { + $Output.Lines | Should -Contain "Chocolatey uninstalled 1/2 packages. 1 packages failed." -Because $Output.String + } + + It "Outputs failure to uninstall one of the packages" { + $Output.Lines | Should -Contain "- isdependency - Unable to uninstall 'isdependency.2.1.0' because 'hasdependency.2.0.1' depends on it." + } + } + + Context "When specifying multiple packages where one is a nested dependency should not fail uninstallation (forced dependencies)" { + BeforeAll { + Restore-ChocolateyInstallSnapshot + + $null = Invoke-Choco install toplevelwithnesteddependencies --confirm + + $Output = Invoke-Choco uninstall isdependency toplevelwithnesteddependencies --confirm --force-dependencies + } + + It "Exits with Success (0)" { + $Output.ExitCode | Should -Be 0 -Because $Output.String + } + + # This does not work as expected when runnning in test kitchen and with Chocolatey Licensed Extension installed. + # It is believed to be a problem with test kitchen, or the VM that we are using that is causing the issue. + # The result is that versioned backup files of each file in the package is created, instead of the package being + # removed. Consider the test partially broken. + It "Should have removed <_>" -ForEach @('hasdependency', 'hasnesteddependency', 'isdependency', 'isexactversiondependency', 'toplevelhasexactversiondependency', 'toplevelwithnesteddependencies') -Tag FossOnly { + "$env:ChocolateyInstall\lib\$_" | Should -Not -Exist -Because $Output.String + } + + It "Outputs <_> was succcesfully uninstalled" -ForEach @('hasdependency', 'hasnesteddependency', 'isdependency', 'isexactversiondependency', 'toplevelhasexactversiondependency', 'toplevelwithnesteddependencies') { + $Output.Lines | Should -Contain "$_ has been successfully uninstalled." -Because $Output.String + } + + It "Outputs warning about package being unable to be uninstalled due to being a dependency" { + $Output.Lines | Should -Contain "[NuGet]: Unable to uninstall 'isdependency.2.1.0' because 'hasdependency.2.0.1' depends on it." + } + + It "Outputs 7/7 packages uninstalled" { + $Output.Lines | Should -Contain "Chocolatey uninstalled 7/7 packages." -Because $Output.String + } + + It "Does not output failure to uninstall one of the packages" { + $Output.Lines | Should -Not -Contain "- isdependency - Unable to uninstall 'isdependency.2.1.0' because 'hasdependency.2.0.1' depends on it." + } + } + # This needs to be the last test in this block, to ensure NuGet configurations aren't being created. Test-NuGetPaths }