(#3461,#3487) Prevent dependency resolution from downgrading packages

Invoke-Tests.ps1

@@ -54,12 +54,53 @@ else {
 if (-not (Test-Path "$TestPath/packages") -or -not $SkipPackaging) {
     $null = New-Item -Path "$TestPath/packages" -ItemType Directory -Force
     # Get and pack packages
-    $nuspecs = Get-ChildItem -Path $PSScriptRoot/src/chocolatey.tests.integration, $PSScriptRoot/tests/packages -Recurse -Include *.nuspec | Where-Object FullName -notmatch 'bin'
+    $nuspecs = Get-ChildItem -Path $PSScriptRoot/src/chocolatey.tests.integration, $PSScriptRoot/tests/packages -Recurse -Include *.nuspec | Where-Object FullName -NotMatch 'bin'
     Get-ChildItem -Path $PSScriptRoot/tests/packages -Recurse -Include *.nupkg | Copy-Item -Destination "$TestPath/packages"
 
-    foreach ($file in $nuspecs) {
-        Write-Host "Packaging $file"
-        $null = choco pack $file.FullName --out "$TestPath/packages"
+    $packFailures = foreach ($file in $nuspecs) {
+        # Include allow-unofficial in case an unofficial Chocolatey has been installed globally for testing
+        $packOutput = choco pack $file.FullName --out "$TestPath/packages" --allow-unofficial
+        if ($LASTEXITCODE -ne 0) {
+            [pscustomobject]@{
+                Package = $file.FullName
+                ExitCode = $LASTEXITCODE
+                Output = $packOutput
+            }
+            Write-Warning "Failed to pack $file"
+        }
+        else {
+            Write-Host "Packaged $file"
+        }
+    }
+
+    if ($null -ne $packFailures) {
+        foreach ($failure in $packFailures) {
+            Write-Warning "$($failure.Package) failed to pack with exit code: $($failure.ExitCode)"
+            $failure.Output | Write-Warning
+        }
+        # If you want to stop things, change this to a throw.
+        # This is not currently throwing as there are two packages that are supposed to fail.
+        Write-Error "$($packFailures.Count) packages failed to pack."
+    }
+}
+
+if (-not (Test-Path "$TestPath/all-packages") -or -not $SkipPackaging) {
+    $null = New-Item -Path "$TestPath/all-packages" -ItemType Directory -Force
+
+    # These are the package ids that are loaded into the all packages test repository.
+    $AllPackagesRepository = @(
+        'isdependency'
+        'hasdependency'
+        'hasnesteddependency'
+        'downgradesdependency'
+        'dependencyfailure'
+        'hasfailingnesteddependency'
+        'failingdependency'
+        'isexactversiondependency'
+    )
+
+    foreach ($package in $AllPackagesRepository) {
+        $null = Copy-Item "$TestPath/packages/$package.*.nupkg" "$TestPath/all-packages/"
     }
 }
 
@@ -97,6 +138,7 @@ try {
 
     Import-Module $PSScriptRoot\tests\helpers\common-helpers.psm1 -Force
     $null = Invoke-Choco source add --name hermes --source "$TestPath/packages"
+    $null = Invoke-Choco source add --name hermes-all --source "$TestPath/all-packages"
     Enable-ChocolateyFeature -Name allowGlobalConfirmation
     $PesterConfiguration = [PesterConfiguration]@{
         Run        = @{

src/chocolatey/StringResources.cs

@@ -56,5 +56,13 @@ public static class EnvironmentVariables
             [Browsable(false)]
             internal const string PackageNuspecVersion = "packageNuspecVersion";
         }
+
+        public static class ErrorMessages
+        {
+            [EditorBrowsable(EditorBrowsableState.Never)]
+            [Browsable(false)]
+            internal const string UnableToDowngrade = "A newer version of {0} (v{1}) is already installed.{2} Use --allow-downgrade or --force to attempt to install older versions.";
+            internal const string DependencyFailedToInstall = "Failed to install {0} because a previous dependency failed.";
+        }
     }
 }

src/chocolatey/infrastructure.app/services/NugetService.cs

@@ -578,6 +578,12 @@ public virtual ConcurrentDictionary<string, PackageResult> Install(ChocolateyCon
 
             foreach (var packageName in packageNames.OrEmpty())
             {
+                if (packageResultsToReturn.ContainsKey(packageName))
+                {
+                    // We have already processed this package (likely during dependency resolution of another package).
+                    continue;
+                }
+
                 // We need to ensure we are using a clean configuration file
                 // before we start reading it.
                 config.RevertChanges();
@@ -599,6 +605,7 @@ public virtual ConcurrentDictionary<string, PackageResult> Install(ChocolateyCon
                 if (installedPackage != null && (version == null || version == installedPackage.PackageMetadata.Version) && !config.Force)
                 {
                     var logMessage = "{0} v{1} already installed.{2} Use --force to reinstall, specify a version to install, or try upgrade.".FormatWith(installedPackage.Name, installedPackage.Version, Environment.NewLine);
+                    // We need a temporary PackageResult so that we can add to the Messages collection.
                     var nullResult = packageResultsToReturn.GetOrAdd(packageName, installedPackage);
                     nullResult.Messages.Add(new ResultMessage(ResultType.Warn, logMessage));
                     nullResult.Messages.Add(new ResultMessage(ResultType.Inconclusive, logMessage));
@@ -619,9 +626,9 @@ public virtual ConcurrentDictionary<string, PackageResult> Install(ChocolateyCon
 
                 if (installedPackage != null && version != null && version < installedPackage.PackageMetadata.Version && !config.AllowDowngrade)
                 {
-                    var logMessage = "A newer version of {0} (v{1}) is already installed.{2} Use --allow-downgrade or --force to attempt to install older versions.".FormatWith(installedPackage.Name, installedPackage.Version, Environment.NewLine);
-                    var nullResult = packageResultsToReturn.GetOrAdd(packageName, installedPackage);
-                    nullResult.Messages.Add(new ResultMessage(ResultType.Error, logMessage));
+                    var logMessage = StringResources.ErrorMessages.UnableToDowngrade.FormatWith(installedPackage.Name, installedPackage.Version, Environment.NewLine);
+                    packageResultsToReturn.GetOrAdd(packageName, installedPackage)
+                        .Messages.Add(new ResultMessage(ResultType.Error, logMessage));
                     this.Log().Error(ChocolateyLoggers.Important, logMessage);
                     continue;
                 }
@@ -793,6 +800,26 @@ Version was specified as '{0}'. It is possible that version
 
                 foreach (SourcePackageDependencyInfo packageDependencyInfo in resolvedPackages)
                 {
+                    // Don't attempt to action this package if dependencies failed.
+                    if (packageDependencyInfo != null && packageResultsToReturn.Any(r => r.Value.Success != true && packageDependencyInfo.Dependencies.Any(d => d.Id.Equals(r.Value.Identity.Id, StringComparison.OrdinalIgnoreCase))))
+                    {
+                        var logMessage = StringResources.ErrorMessages.DependencyFailedToInstall.FormatWith(packageDependencyInfo.Id);
+                        packageResultsToReturn
+                            .GetOrAdd(
+                                packageDependencyInfo.Id,
+                                new PackageResult(packageDependencyInfo.Id, packageDependencyInfo.Version.ToFullStringChecked(), string.Empty)
+                            )
+                            .Messages.Add(new ResultMessage(ResultType.Error, logMessage));
+                        this.Log().Error(ChocolateyLoggers.Important, logMessage);
+
+                        if (config.Features.StopOnFirstPackageFailure)
+                        {
+                            throw new ApplicationException("Stopping further execution as {0} has failed install.".FormatWith(packageDependencyInfo.Id));
+                        }
+
+                        continue;
+                    }
+
                     var packageRemoteMetadata = packagesToInstall.FirstOrDefault(p => p.Identity.Equals(packageDependencyInfo));
 
                     if (packageRemoteMetadata is null)
@@ -809,6 +836,22 @@ Version was specified as '{0}'. It is possible that version
                     var packageToUninstall = packagesToUninstall.FirstOrDefault(p => p.PackageMetadata.Id.Equals(packageDependencyInfo.Id, StringComparison.OrdinalIgnoreCase));
                     if (packageToUninstall != null)
                     {
+                        // Are we attempting a downgrade? We need to ensure it's allowed...
+                        if (!config.AllowDowngrade && packageToUninstall.Identity.HasVersion && packageDependencyInfo.HasVersion && packageDependencyInfo.Version < packageToUninstall.Identity.Version)
+                        {
+                            var logMessage = StringResources.ErrorMessages.UnableToDowngrade.FormatWith(packageToUninstall.Name, packageToUninstall.Version, Environment.NewLine);
+                            packageResultsToReturn.GetOrAdd(packageToUninstall.Name, packageToUninstall)
+                                .Messages.Add(new ResultMessage(ResultType.Error, logMessage));
+                            this.Log().Error(ChocolateyLoggers.Important, logMessage);
+
+                            if (config.Features.StopOnFirstPackageFailure)
+                            {
+                                throw new ApplicationException("Stopping further execution as {0} has failed install.".FormatWith(packageToUninstall.Identity.Id));
+                            }
+
+                            continue;
+                        }
+
                         shouldAddForcedResultMessage = true;
                         BackupAndRunBeforeModify(packageToUninstall, config, beforeModifyAction);
                         packageToUninstall.InstallLocation = pathResolver.GetInstallPath(packageToUninstall.Identity);
@@ -1051,6 +1094,12 @@ public virtual ConcurrentDictionary<string, PackageResult> Upgrade(ChocolateyCon
 
             foreach (var packageName in config.PackageNames.Split(new[] { ApplicationParameters.PackageNamesSeparator }, StringSplitOptions.RemoveEmptyEntries).OrEmpty())
             {
+                if (packageResultsToReturn.ContainsKey(packageName))
+                {
+                    // We have already processed this package (likely during dependency resolution of another package).
+                    continue;
+                }
+
                 // We need to ensure we are using a clean configuration file
                 // before we start reading it.
                 config.RevertChanges();
@@ -1138,9 +1187,9 @@ public virtual ConcurrentDictionary<string, PackageResult> Upgrade(ChocolateyCon
 
                 if (version != null && version < installedPackage.PackageMetadata.Version && !config.AllowDowngrade)
                 {
-                    var logMessage = "A newer version of {0} (v{1}) is already installed.{2} Use --allow-downgrade or --force to attempt to upgrade to older versions.".FormatWith(installedPackage.PackageMetadata.Id, installedPackage.Version, Environment.NewLine);
-                    var nullResult = packageResultsToReturn.GetOrAdd(packageName, new PackageResult(installedPackage.PackageMetadata, pathResolver.GetInstallPath(installedPackage.PackageMetadata.Id)));
-                    nullResult.Messages.Add(new ResultMessage(ResultType.Error, logMessage));
+                    var logMessage = StringResources.ErrorMessages.UnableToDowngrade.FormatWith(installedPackage.PackageMetadata.Id, installedPackage.Version, Environment.NewLine);
+                    packageResultsToReturn.GetOrAdd(packageName, new PackageResult(installedPackage.PackageMetadata, pathResolver.GetInstallPath(installedPackage.PackageMetadata.Id)))
+                        .Messages.Add(new ResultMessage(ResultType.Error, logMessage));
                     this.Log().Error(ChocolateyLoggers.Important, logMessage);
                     continue;
                 }
@@ -1559,6 +1608,25 @@ public virtual ConcurrentDictionary<string, PackageResult> Upgrade(ChocolateyCon
                                 break;
                             }
 
+                            // Don't attempt to action this package if dependencies failed.
+                            if (packageResultsToReturn.Any(r => r.Value.Success != true && packageDependencyInfo.Dependencies.Any(d => d.Id.Equals(r.Value.Identity.Id, StringComparison.OrdinalIgnoreCase))))
+                            {
+                                var logMessage = StringResources.ErrorMessages.DependencyFailedToInstall.FormatWith(packageDependencyInfo.Id, packageDependencyInfo.Version);
+                                packageResultsToReturn.GetOrAdd(
+                                        packageDependencyInfo.Id, 
+                                        new PackageResult(packageDependencyInfo.Id, packageDependencyInfo.Version.ToFullStringChecked(), string.Empty)
+                                    )
+                                    .Messages.Add(new ResultMessage(ResultType.Error, logMessage));
+                                this.Log().Error(ChocolateyLoggers.Important, logMessage);
+
+                                if (config.Features.StopOnFirstPackageFailure)
+                                {
+                                    throw new ApplicationException("Stopping further execution as {0} has failed.".FormatWith(packageDependencyInfo.Id));
+                                }
+
+                                continue;
+                            }
+
                             var packageRemoteMetadata = packagesToInstall.FirstOrDefault(p => p.Identity.Equals(packageDependencyInfo));
 
                             if (packageRemoteMetadata is null)
@@ -1576,6 +1644,32 @@ public virtual ConcurrentDictionary<string, PackageResult> Upgrade(ChocolateyCon
                             {
                                 if (packageToUninstall != null)
                                 {
+                                    // Are we attempting a downgrade? We need to ensure it's allowed...
+                                    if (!config.AllowDowngrade && packageToUninstall.Identity.HasVersion && packageDependencyInfo.HasVersion && packageDependencyInfo.Version < packageToUninstall.Identity.Version)
+                                    {
+                                        var logMessage = StringResources.ErrorMessages.UnableToDowngrade.FormatWith(packageToUninstall.Name, packageToUninstall.Version, Environment.NewLine);
+                                        packageResultsToReturn.GetOrAdd(packageToUninstall.Name, packageToUninstall)
+                                            .Messages.Add(new ResultMessage(ResultType.Error, logMessage));
+                                        this.Log().Error(ChocolateyLoggers.Important, logMessage);
+
+                                        if (config.Features.StopOnFirstPackageFailure)
+                                        {
+                                            throw new ApplicationException("Stopping further execution as {0} has failed.".FormatWith(packageDependencyInfo.Id));
+                                        }
+
+                                        continue;
+                                    }
+
+                                    // Package was previously marked inconclusive (not upgraded). We need remove the message since we are now upgrading it.
+                                    // Packages that are inconclusive but successful are not labeled as successful at the end of the run.
+                                    var checkResult = packageResultsToReturn.GetOrAdd(packageToUninstall.Name, packageToUninstall);
+
+                                    while (checkResult.Inconclusive)
+                                    {
+                                        checkResult.Messages.Remove(checkResult.Messages.FirstOrDefault((m) => m.MessageType == ResultType.Inconclusive));
+                                    }
+
+
                                     var oldPkgInfo = _packageInfoService.Get(packageToUninstall.PackageMetadata);
 
                                     BackupAndRunBeforeModify(packageToUninstall, oldPkgInfo, config, beforeUpgradeAction);

tests/Vagrantfile

@@ -69,6 +69,7 @@ Vagrant.configure("2") do |config|
   config.vm.provision "shell", name: "clear-packages", inline: <<-SHELL
     Write-Host "($(Get-Date -Format "dddd MM/dd/yyyy HH:mm:ss K")) Clearing the packages directory"
     Remove-Item "$env:TEMP/chocolateyTests/packages" -Recurse -Force -ErrorAction SilentlyContinue
+    Remove-Item "$env:TEMP/chocolateyTests/all-packages" -Recurse -Force -ErrorAction SilentlyContinue
   SHELL
   config.vm.provision "shell", name: "test", inline: <<-SHELL
     # Copy changed files.

tests/helpers/common/Chocolatey/Get-ChocolateyInstalledPackages.ps1

@@ -0,0 +1,3 @@
+function Get-ChocolateyInstalledPackages {
+    (Invoke-Choco list -r).Lines | ConvertFrom-ChocolateyOutput -Command List
+}

tests/packages/dependencyfailure/dependencyfailure.nuspec

@@ -0,0 +1,23 @@
+<?xml version="1.0" encoding="utf-8"?>
+<package xmlns="http://schemas.microsoft.com/packaging/2015/06/nuspec.xsd">
+  <metadata>
+    <id>dependencyfailure</id>
+    <version>1.0.0</version>
+    <title>dependencyfailure (Install)</title>
+    <authors>__REPLACE_AUTHORS_OF_SOFTWARE_COMMA_SEPARATED__</authors>
+    <projectUrl>https://_Software_Location_REMOVE_OR_FILL_OUT_</projectUrl>
+    <tags>dependencyfailure admin SPACE_SEPARATED</tags>
+    <summary>Package includes a dependency for a package that is known to fail installation.</summary>
+    <description>
+      Package includes a dependency for a package that is known to fail installation.
+
+      It can be used in tests to ensure that failing dependencies do not allow the package to install.
+    </description>
+    <dependencies>
+      <dependency id="failingdependency" version="1.0.0" />
+    </dependencies>
+  </metadata>
+  <files>
+    <file src="tools\**" target="tools" />
+  </files>
+</package>

tests/packages/dependencyfailure/tools/chocolateyinstall.ps1

@@ -0,0 +1 @@
+Write-Host "I'm successful!"

tests/packages/failingdependency/failingdependency.nuspec

@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="utf-8"?>
+<package xmlns="http://schemas.microsoft.com/packaging/2015/06/nuspec.xsd">
+  <metadata>
+    <id>failingdependency</id>
+    <version>1.0.0</version>
+    <title>dependencyfailure (Install)</title>
+    <authors>__REPLACE_AUTHORS_OF_SOFTWARE_COMMA_SEPARATED__</authors>
+    <projectUrl>https://_Software_Location_REMOVE_OR_FILL_OUT_</projectUrl>
+    <tags>dependencyfailure admin SPACE_SEPARATED</tags>
+    <summary>Package that fails to install. Used as part of dependency tree.</summary>
+    <description>Package that fails to install. Used as part of dependency tree.</description>
+  </metadata>
+  <files>
+    <file src="tools\**" target="tools" />
+  </files>
+</package>

tests/packages/failingdependency/tools/chocolateyinstall.ps1

@@ -0,0 +1,2 @@
+Write-Error "This should fail!"
+$env:ChocolateyExitCode = '15608'