diff --git a/src/chocolatey/infrastructure.app/services/ChocolateyPackageInformationService.cs b/src/chocolatey/infrastructure.app/services/ChocolateyPackageInformationService.cs index ddd97b7909..4450823e60 100644 --- a/src/chocolatey/infrastructure.app/services/ChocolateyPackageInformationService.cs +++ b/src/chocolatey/infrastructure.app/services/ChocolateyPackageInformationService.cs @@ -74,7 +74,8 @@ public ChocolateyPackageInformation Get(IPackageMetadata package) return packageInformation; } - var pkgStorePath = _fileSystem.CombinePaths(ApplicationParameters.ChocolateyPackageInfoStoreLocation, "{0}.{1}".FormatWith(package.Id, package.Version.ToStringSafe())); + var pkgStorePath = GetStorePath(_fileSystem, package.Id, package.Version); + if (!_fileSystem.DirectoryExists(pkgStorePath)) { return packageInformation; @@ -176,7 +177,8 @@ public void Save(ChocolateyPackageInformation packageInformation) return; } - var pkgStorePath = _fileSystem.CombinePaths(ApplicationParameters.ChocolateyPackageInfoStoreLocation, "{0}.{1}".FormatWith(packageInformation.Package.Id, packageInformation.Package.Version.ToStringSafe())); + var pkgStorePath = GetStorePath(_fileSystem, packageInformation.Package.Id, packageInformation.Package.Version); + _fileSystem.EnsureDirectoryExists(pkgStorePath); if (packageInformation.RegistrySnapshot != null) @@ -250,11 +252,57 @@ public void Save(ChocolateyPackageInformation packageInformation) public void Remove(IPackageMetadata package) { - var pkgStorePath = _fileSystem.CombinePaths(ApplicationParameters.ChocolateyPackageInfoStoreLocation, "{0}.{1}".FormatWith(package.Id, package.Version.ToStringSafe())); + var pkgStorePath = GetStorePath(_fileSystem, package.Id, package.Version); if (_config.RegularOutput) this.Log().Info("Removing Package Information for {0}".FormatWith(pkgStorePath)); _fileSystem.DeleteDirectoryChecked(pkgStorePath, recursive: true); } + private static string GetStorePath(IFileSystem fileSystem, string id, NuGetVersion version) + { + var preferredStorePath = fileSystem.CombinePaths(ApplicationParameters.ChocolateyPackageInfoStoreLocation, "{0}.{1}".FormatWith(id, version.ToNormalizedStringChecked())); + + if (fileSystem.DirectoryExists(preferredStorePath)) + { + return preferredStorePath; + } + + var pkgStorePath = fileSystem.CombinePaths(ApplicationParameters.ChocolateyPackageInfoStoreLocation, "{0}.{1}".FormatWith(id, version.ToStringSafe())); + + if (fileSystem.DirectoryExists(pkgStorePath)) + { + return pkgStorePath; + } + + // Legacy handling for package version that was installed originally as 4.0.0.0 + + var versionFull = version.IsPrerelease + ? "{0}-{1}".FormatWith(version.Version.ToStringSafe(), version.Release) + : version.Version.ToStringSafe(); + pkgStorePath = fileSystem.CombinePaths(ApplicationParameters.ChocolateyPackageInfoStoreLocation, "{0}.{1}".FormatWith(id, versionFull)); + + if (fileSystem.DirectoryExists(pkgStorePath)) + { + return pkgStorePath; + } + + // Legacy handling for package versions that was installed originally as "4.2" + + if (version.Version.Revision == 0 && version.Version.Build == 0) + { + versionFull = version.IsPrerelease + ? "{0}.{1}-{2}".FormatWith(version.Major, version.Minor, version.Release) + : "{0}.{1}".FormatWith(version.Major, version.Minor); + pkgStorePath = fileSystem.CombinePaths(ApplicationParameters.ChocolateyPackageInfoStoreLocation, "{0}.{1}".FormatWith(id, versionFull)); + + if (fileSystem.DirectoryExists(pkgStorePath)) + { + return pkgStorePath; + } + } + + return preferredStorePath; + } + #pragma warning disable IDE1006 [Obsolete("This overload is deprecated and will be removed in v3.")] public ChocolateyPackageInformation get_package_information(IPackageMetadata package) diff --git a/src/chocolatey/infrastructure.app/services/NugetService.cs b/src/chocolatey/infrastructure.app/services/NugetService.cs index d3ade05836..5ad6c45b36 100644 --- a/src/chocolatey/infrastructure.app/services/NugetService.cs +++ b/src/chocolatey/infrastructure.app/services/NugetService.cs @@ -1389,6 +1389,10 @@ public virtual ConcurrentDictionary Upgrade(ChocolateyCon continue; } + + // We need to remove the nupkg file explicitly in case it has + // been modified. + EnsureNupkgRemoved(oldPkgInfo.Package); } var endpoint = NuGetEndpointResources.GetResourcesBySource(packageDependencyInfo.Source); @@ -2244,6 +2248,12 @@ public virtual ConcurrentDictionary Uninstall(ChocolateyC this.Log().Warn(logMessage); packageResult.Messages.Add(new ResultMessage(ResultType.Error, errorlogMessage)); + // As the only reason we failed the uninstall is due to left over files, let us + // be good citizens and ensure that the nupkg file is removed so the package will + // not be listed as installed. This is needed to do manually as the package may + // have been optimized by licensed edition of Chocolatey CLI. + EnsureNupkgRemoved(packageToUninstall.PackageMetadata, throwError: false); + // Do not call continueAction again here as it has already been called once. //if (continueAction != null) continueAction.Invoke(packageResult, config); continue; @@ -2418,7 +2428,8 @@ protected void BackupCurrentPackageFiles(ChocolateyConfiguration config, IPackag /// Ensure that the package is deleted or throw an error. /// /// The installed package. - private void EnsureNupkgRemoved(IPackageMetadata removedPackage) + /// Whether failing to remove the nuget package should throw an exception or not. + private void EnsureNupkgRemoved(IPackageMetadata removedPackage, bool throwError = true) { this.Log().Debug(ChocolateyLoggers.Verbose, "Removing nupkg if it still exists."); @@ -2431,7 +2442,7 @@ private void EnsureNupkgRemoved(IPackageMetadata removedPackage) FaultTolerance.TryCatchWithLoggingException( () => _fileSystem.DeleteFile(nupkg), "Error deleting nupkg file", - throwError: true); + throwError: throwError); } protected void NormalizeNuspecCasing(IPackageSearchMetadata packageMetadata, string packageLocation) diff --git a/tests/chocolatey-tests/commands/choco-uninstall.Tests.ps1 b/tests/chocolatey-tests/commands/choco-uninstall.Tests.ps1 index 93e0a13e72..c1b372da63 100644 --- a/tests/chocolatey-tests/commands/choco-uninstall.Tests.ps1 +++ b/tests/chocolatey-tests/commands/choco-uninstall.Tests.ps1 @@ -51,11 +51,7 @@ Describe "choco uninstall" -Tag Chocolatey, UninstallCommand { $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 lib package directory" -Tag FossOnly { + It "Should have removed lib package directory" { "$env:ChocolateyInstall\lib\upgradepackage" | Should -Not -Exist } @@ -131,11 +127,7 @@ Describe "choco uninstall" -Tag Chocolatey, UninstallCommand { "$env:ChocolateyInstall\lib\installpackage\a-locked-file.txt" | Should -Exist } - # 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 { + 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') { "$env:ChocolateyInstall\lib\installpackage\$_" | Should -Not -Exist } @@ -172,23 +164,15 @@ Describe "choco uninstall" -Tag Chocolatey, UninstallCommand { $LockedFile.Dispose() } - It "Exits with Failure (1)" -Tag FossOnly { + It "Exits with Failure (1)" { $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 } - # 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 { + 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') { "$env:ChocolateyInstall\lib\installpackage\$_" | Should -Not -Exist } @@ -200,11 +184,11 @@ Describe "choco uninstall" -Tag Chocolatey, UninstallCommand { "$env:ChocolateyInstall\lib-bkp\upgradepackage" | Should -Not -Exist } - It "Reports no package uninstalled" -Tag FossOnly { + It "Reports no package uninstalled" { $Output.Lines | Should -Contain "Chocolatey uninstalled 0/1 packages. 1 packages failed." } - It "Outputs not able to remove all package files" -Tag FossOnly { + It "Outputs not able to remove all package files" { $Output.String | Should -Match "installpackage - Unable to delete all existing package files. There will be leftover files requiring manual cleanup" } } @@ -222,11 +206,7 @@ Describe "choco uninstall" -Tag Chocolatey, UninstallCommand { $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 { + It "Should have removed <_>" -ForEach @('isdependency', 'hasdependency') { "$env:ChocolateyInstall\lib\$_" | Should -Not -Exist -Because $Output.String } @@ -234,7 +214,7 @@ Describe "choco uninstall" -Tag Chocolatey, UninstallCommand { "$env:ChocolateyInstall\lib\isexactversiondependency" | Should -Exist -Because $Output.String } - It "Outputs <_> was succcesfully uninstalled" -ForEach @('isdependency', 'hasdependency') -Tag FossOnly { + It "Outputs <_> was succcesfully uninstalled" -ForEach @('isdependency', 'hasdependency') { $Output.Lines | Should -Contain "$_ has been successfully uninstalled." -Because $Output.String } @@ -260,11 +240,7 @@ Describe "choco uninstall" -Tag Chocolatey, UninstallCommand { $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 { + It "Should have removed <_>" -ForEach @('isdependency', 'hasdependency', 'isexactversiondependency') { "$env:ChocolateyInstall\lib\$_" | Should -Not -Exist -Because $Output.String } @@ -299,19 +275,11 @@ Describe "choco uninstall" -Tag Chocolatey, UninstallCommand { $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 { + It "Should have uninstall package installpackage" { "$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 { + It "Outputs successful uninstall of installpackage" { $Output.Lines | Should -Contain "installpackage has been successfully uninstalled." -Because $Output.String } @@ -333,11 +301,7 @@ Describe "choco uninstall" -Tag Chocolatey, UninstallCommand { $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 { + It "Should have removed package toplevelwithnesteddependencies" { "$env:ChocolateyInstall\lib\toplevelwithnesteddependencies" | Should -Not -Exist -Because $Output.String } @@ -379,11 +343,7 @@ Describe "choco uninstall" -Tag Chocolatey, UninstallCommand { $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 { + It "Should have removed <_>" -ForEach @('hasdependency', 'hasnesteddependency', 'isdependency', 'isexactversiondependency', 'toplevelhasexactversiondependency', 'toplevelwithnesteddependencies') { "$env:ChocolateyInstall\lib\$_" | Should -Not -Exist -Because $Output.String } @@ -418,10 +378,6 @@ Describe "choco uninstall" -Tag Chocolatey, UninstallCommand { $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', 'isdependency') { "$env:ChocolateyInstall\lib\$_" | Should -Not -Exist -Because $Output.String } diff --git a/tests/chocolatey-tests/commands/choco-upgrade.Tests.ps1 b/tests/chocolatey-tests/commands/choco-upgrade.Tests.ps1 index ef3dc568f0..a9194ec06b 100644 --- a/tests/chocolatey-tests/commands/choco-upgrade.Tests.ps1 +++ b/tests/chocolatey-tests/commands/choco-upgrade.Tests.ps1 @@ -339,23 +339,19 @@ To upgrade a local, or remote file, you may use: } } - # 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. - Context "Upgrading a package when installer is locked" -Tag FossOnly { + Context "Upgrading a package when installer is locked" { BeforeAll { Restore-ChocolateyInstallSnapshot $PackageUnderTest = "hasinnoinstaller" $PackageVersion = '6.2.0.3' - $null = Invoke-Choco install $PackageUnderTest --version 6.2.0.0 --confirm --no-progress + $null = Invoke-Choco install $PackageUnderTest --version 6.2.0.0 --confirm --no-progress --no-reduce $LockedFile = [System.IO.File]::Open("$env:ChocolateyInstall\lib\$PackageUnderTest\tools\helloworld-1.0.0.exe", 'Open', 'Read', 'Read') - $Output = Invoke-Choco upgrade $PackageUnderTest --version $PackageVersion --confirm --no-progress + $Output = Invoke-Choco upgrade $PackageUnderTest --version $PackageVersion --confirm --no-progress --no-reduce } AfterAll { @@ -396,7 +392,7 @@ To upgrade a local, or remote file, you may use: $LockedFile = [System.IO.File]::Open("$env:ChocolateyInstall\lib\$PackageUnderTest\tools\a-locked-file.txt", 'OpenOrCreate', 'Read', 'Read') - $null = Invoke-Choco install $PackageUnderTest --version 6.2.0.0 --confirm --no-progress + $null = Invoke-Choco install $PackageUnderTest --version 6.2.0.0 --confirm --no-progress --no-reduce $Output = Invoke-Choco upgrade $PackageUnderTest --version $PackageVersion --confirm --no-progress } @@ -406,19 +402,11 @@ To upgrade a local, or remote file, you may use: $null = Invoke-Choco uninstall $PackageUnderTest --confirm } - # 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 "Exits with Failure (1)" -Tag FossOnly { + 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 "Keeps file '<_>' in the lib directory" -ForEach @('hasinnoinstaller.nuspec', 'hasinnoinstaller.nupkg', 'tools\chocolateyinstall.ps1', 'tools\chocolateyuninstall.ps1', 'tools\helloworld-1.0.0.exe', 'tools\helloworld-1.0.0.exe.ignore') -Tag FossOnly { + It "Keeps file '<_>' in the lib directory" -ForEach @('hasinnoinstaller.nuspec', 'hasinnoinstaller.nupkg', 'tools\chocolateyinstall.ps1', 'tools\chocolateyuninstall.ps1', 'tools\helloworld-1.0.0.exe', 'tools\helloworld-1.0.0.exe.ignore') { "$env:ChocolateyInstall\lib\$PackageUnderTest\$_" | Should -Exist } @@ -426,12 +414,7 @@ To upgrade a local, or remote file, you may use: "$env:ChocolateyInstall\lib-bkp\$PackageUnderTest" | Should -Not -Exist } - # Only two files are backed up as we was not able to download and extract the new package - # 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 "Creates backup of file '<_>' in lib-bad" -ForEach @('.chocolateyPending', 'tools\a-locked-file.txt') -Tag FossOnly { + It "Creates backup of file '<_>' in lib-bad" -ForEach @('.chocolateyPending', 'tools\a-locked-file.txt') { "$env:ChocolateyInstall\lib-bad\$PackageUnderTest\$PackageVersion\$_" | Should -Exist } @@ -439,11 +422,7 @@ To upgrade a local, or remote file, you may use: "$env:ChocolateyInstall\lib-bad\$PackageUnderTest\$PackageVersion\$_" | Should -Not -Exist } - # 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 a message showing that installation failed." -Tag FossOnly { + It "Outputs a message showing that installation failed." { $Output.Lines | Should -Contain "Chocolatey upgraded 0/1 packages. 1 packages failed." -Because $Output.String }