diff --git a/Invoke-Tests.ps1 b/Invoke-Tests.ps1 index 95fc45f412..b11fb8524c 100644 --- a/Invoke-Tests.ps1 +++ b/Invoke-Tests.ps1 @@ -93,7 +93,17 @@ try { Verbosity = 'Minimal' } Filter = @{ - ExcludeTag = 'Background', 'Licensed', 'CCM', 'WIP', 'NonAdmin', 'Internal' + ExcludeTag = @( + 'Background' + 'Licensed' + 'CCM' + 'WIP' + 'NonAdmin' + 'Internal' + if (-not $env:VM_RUNNING -and -not $env:TEST_KITCHEN) { + 'VMOnly' + } + ) } } diff --git a/src/chocolatey/infrastructure.app/configuration/ChocolateyConfiguration.cs b/src/chocolatey/infrastructure.app/configuration/ChocolateyConfiguration.cs index 5a36efb131..549ac89cb8 100644 --- a/src/chocolatey/infrastructure.app/configuration/ChocolateyConfiguration.cs +++ b/src/chocolatey/infrastructure.app/configuration/ChocolateyConfiguration.cs @@ -30,6 +30,9 @@ namespace chocolatey.infrastructure.app.configuration [Serializable] public class ChocolateyConfiguration { + [NonSerialized] + private ChocolateyConfiguration _originalConfiguration; + public ChocolateyConfiguration() { RegularOutput = true; @@ -58,6 +61,86 @@ public ChocolateyConfiguration() #endif } + /// + /// Creates a backup of the current version of the configuration class. + /// + /// One or more objects in the class or child classes are not serializable. + public void start_backup() + { + // We do this the easy way to ensure that we have a clean copy + // of the original configuration file. + _originalConfiguration = this.deep_copy(); + } + + /// + /// Restore the backup that has previously been created to the initial + /// state, without making the class reference types the same to prevent + /// the initial configuration class being updated at the same time if a + /// value changes. + /// + /// Whether a backup that was previously made should be removed after resetting the configuration. + /// No backup has been created before trying to reset the current configuration, and removal of the backup was not requested. + /// + /// This call may make quite a lot of allocations on the Gen0 heap, as such + /// it is best to keep the calls to this method at a minimum. + /// + public void reset_config(bool removeBackup = false) + { + if (_originalConfiguration == null) + { + if (removeBackup) + { + // If we will also be removing the backup, we do not care if it is already + // null or not, as that is the intended state when this method returns. + return; + } + + throw new InvalidOperationException("No backup has been created before trying to reset the current configuration, and removal of the backup was not requested."); + } + + var t = this.GetType(); + + foreach (var property in t.GetProperties(BindingFlags.Public | BindingFlags.Instance)) + { + try + { + var originalValue = property.GetValue(_originalConfiguration, new object[0]); + + if (removeBackup || property.DeclaringType.IsPrimitive || property.DeclaringType.IsValueType || property.DeclaringType == typeof(string)) + { + // If the property is a primitive, a value type or a string, then a copy of the value + // will be created by the .NET Runtime automatically, and we do not need to create a deep clone of the value. + // Additionally, if we will be removing the backup there is no need to create a deep copy + // for any reference types, as such we also set the reference itself so it is not needed + // to allocate more memory. + property.SetValue(this, originalValue, new object[0]); + } + else if (originalValue != null) + { + // We need to do a deep copy of the value so it won't copy the reference itself, + // but rather the actual values we are interested in. + property.SetValue(this, originalValue.deep_copy(), new object[0]); + } + else + { + property.SetValue(this, null, new object[0]); + } + } + catch (Exception ex) + { + throw new ApplicationException("Unable to restore the value for the property '{0}'.".format_with(property.Name), ex); + } + } + + if (removeBackup) + { + // It is enough to set the original configuration to null to + // allow GC to clean it up the next time it runs on the stored Generation + // Heap Table. + _originalConfiguration = null; + } + } + // overrides public override string ToString() { @@ -237,6 +320,7 @@ private void append_output(StringBuilder propertyValues, string append) [Obsolete("Side by Side installation is deprecated, and is pending removal in v2.0.0")] public bool AllowMultipleVersions { get; set; } + public bool AllowDowngrade { get; set; } public bool ForceDependencies { get; set; } public string DownloadChecksum { get; set; } diff --git a/src/chocolatey/infrastructure.app/services/ChocolateyPackageService.cs b/src/chocolatey/infrastructure.app/services/ChocolateyPackageService.cs index b62d9c5106..516768d5b6 100644 --- a/src/chocolatey/infrastructure.app/services/ChocolateyPackageService.cs +++ b/src/chocolatey/infrastructure.app/services/ChocolateyPackageService.cs @@ -22,7 +22,6 @@ namespace chocolatey.infrastructure.app.services using System.IO; using System.Linq; using System.Text; - using builders; using commandline; using configuration; using domain; @@ -31,8 +30,8 @@ namespace chocolatey.infrastructure.app.services using infrastructure.events; using infrastructure.services; using logging; - using NuGet; using nuget; + using NuGet; using platforms; using results; using tolerance; @@ -91,6 +90,7 @@ system admins into something amazing! Singlehandedly solve your organization's struggles with software management and save the day! https://chocolatey.org/compare" }; + private const string PRO_BUSINESS_LIST_MESSAGE = @" Did you know Pro / Business automatically syncs with Programs and Features? Learn more about Package Synchronizer at @@ -742,7 +742,7 @@ private IEnumerable get_packages_from_config(string pac if (pkgSettings.IgnoreDependencies) packageConfig.IgnoreDependencies = true; if (pkgSettings.ApplyInstallArgumentsToDependencies) packageConfig.ApplyInstallArgumentsToDependencies = true; if (pkgSettings.ApplyPackageParametersToDependencies) packageConfig.ApplyPackageParametersToDependencies = true; - + if (!string.IsNullOrWhiteSpace(pkgSettings.Source) && has_source_type(pkgSettings.Source)) { packageConfig.SourceType = pkgSettings.Source; diff --git a/src/chocolatey/infrastructure.app/services/NugetService.cs b/src/chocolatey/infrastructure.app/services/NugetService.cs index 64d3807e29..651cd84e68 100644 --- a/src/chocolatey/infrastructure.app/services/NugetService.cs +++ b/src/chocolatey/infrastructure.app/services/NugetService.cs @@ -19,25 +19,24 @@ namespace chocolatey.infrastructure.app.services using System; using System.Collections.Concurrent; using System.Collections.Generic; - using System.Globalization; using System.IO; using System.Linq; using System.Net; - using NuGet; using adapters; + using chocolatey.infrastructure.app.utility; using commandline; using configuration; using domain; using guards; using logging; using nuget; + using NuGet; using platforms; using results; using tolerance; using DateTime = adapters.DateTime; using Environment = System.Environment; using IFileSystem = filesystem.IFileSystem; - using chocolatey.infrastructure.app.utility; //todo: #2575 - this monolith is too large. Refactor once test coverage is up. @@ -436,13 +435,13 @@ public virtual ConcurrentDictionary install_run(Chocolate uninstallSuccessAction: null, addUninstallHandler: true); - var originalConfig = config.deep_copy(); + config.start_backup(); foreach (string packageName in packageNames.or_empty_list_if_null()) { - // reset config each time through - config = originalConfig.deep_copy(); - + // We need to ensure we are using a clean configuration file + // before we start reading it. + config.reset_config(); //todo: #2577 get smarter about realizing multiple versions have been installed before and allowing that IPackage installedPackage = packageManager.LocalRepository.FindPackage(packageName); @@ -555,6 +554,11 @@ Version was specified as '{0}'. It is possible that version } } + // Reset the configuration again once we are completely done with the processing of + // configurations, and make sure that we are removing any backup that was created + // as part of this run. + config.reset_config(removeBackup: true); + return packageInstalls; } @@ -621,12 +625,13 @@ public virtual ConcurrentDictionary upgrade_run(Chocolate set_package_names_if_all_is_specified(config, () => { config.IgnoreDependencies = true; }); config.IgnoreDependencies = configIgnoreDependencies; - var originalConfig = config.deep_copy(); + config.start_backup(); foreach (string packageName in config.PackageNames.Split(new[] { ApplicationParameters.PackageNamesSeparator }, StringSplitOptions.RemoveEmptyEntries).or_empty_list_if_null()) { - // reset config each time through - config = originalConfig.deep_copy(); + // We need to ensure we are using a clean configuration file + // before we start reading it. + config.reset_config(); IPackage installedPackage = packageManager.LocalRepository.FindPackage(packageName); @@ -878,6 +883,11 @@ public virtual ConcurrentDictionary upgrade_run(Chocolate } } + // Reset the configuration again once we are completely done with the processing of + // configurations, and make sure that we are removing any backup that was created + // as part of this run. + config.reset_config(removeBackup: true); + return packageInstalls; } @@ -897,12 +907,13 @@ public virtual ConcurrentDictionary get_outdated(Chocolat set_package_names_if_all_is_specified(config, () => { config.IgnoreDependencies = true; }); var packageNames = config.PackageNames.Split(new[] { ApplicationParameters.PackageNamesSeparator }, StringSplitOptions.RemoveEmptyEntries).or_empty_list_if_null().ToList(); - var originalConfig = config.deep_copy(); + config.start_backup(); foreach (var packageName in packageNames) { - // reset config each time through - config = originalConfig.deep_copy(); + // We need to ensure we are using a clean configuration file + // before we start reading it. + config.reset_config(); var installedPackage = packageManager.LocalRepository.FindPackage(packageName); var pkgInfo = _packageInfoService.get_package_information(installedPackage); @@ -961,6 +972,11 @@ public virtual ConcurrentDictionary get_outdated(Chocolat } } + // Reset the configuration again once we are completely done with the processing of + // configurations, and make sure that we are removing any backup that was created + // as part of this run. + config.reset_config(removeBackup: true); + return outdatedPackages; } @@ -1374,12 +1390,13 @@ public virtual ConcurrentDictionary uninstall_run(Chocola config.ForceDependencies = false; }); - var originalConfig = config.deep_copy(); + config.start_backup(); foreach (string packageName in config.PackageNames.Split(new[] { ApplicationParameters.PackageNamesSeparator }, StringSplitOptions.RemoveEmptyEntries).or_empty_list_if_null()) { - // reset config each time through - config = originalConfig.deep_copy(); + // We need to ensure we are using a clean configuration file + // before we start reading it. + config.reset_config(); IList installedPackageVersions = new List(); if (string.IsNullOrWhiteSpace(config.Version)) @@ -1507,6 +1524,11 @@ public virtual ConcurrentDictionary uninstall_run(Chocola } } + // Reset the configuration again once we are completely done with the processing of + // configurations, and make sure that we are removing any backup that was created + // as part of this run. + config.reset_config(removeBackup: true); + return packageUninstalls; } diff --git a/tests/Vagrantfile b/tests/Vagrantfile index 2b2e4b2149..fec687b61a 100644 --- a/tests/Vagrantfile +++ b/tests/Vagrantfile @@ -60,6 +60,7 @@ Vagrant.configure("2") do |config| Write-Host "Build complete. Executing tests." # $env:TEST_KITCHEN = 1 + $env:VM_RUNNING = 1 ./Invoke-Tests.ps1 SHELL end diff --git a/tests/chocolatey-tests/choco-upgrade.Tests.ps1 b/tests/chocolatey-tests/choco-upgrade.Tests.ps1 index 269ee5f0de..25b8d785fc 100644 --- a/tests/chocolatey-tests/choco-upgrade.Tests.ps1 +++ b/tests/chocolatey-tests/choco-upgrade.Tests.ps1 @@ -119,4 +119,92 @@ Describe "choco upgrade" -Tag Chocolatey, UpgradeCommand { $Output.Lines | Should -Contain "Chocolatey upgraded 1/1 packages." -Because $Output.String } } + + Context "Upgrading packages while remembering arguments with only one package using arguments" -Tag Internal { + BeforeAll { + Restore-ChocolateyInstallSnapshot + + $null = Enable-ChocolateyFeature useRememberedArgumentsForUpgrades + $null = Invoke-Choco install curl --package-parameters="'/CurlOnlyParam'" --version="7.77.0" --ia="'/CurlIAParam'" --x86 -y + $null = Invoke-Choco install wget --version=1.21.1 -y + + $Output = Invoke-Choco upgrade all -y --debug + } + + It 'Exits with Success (0)' { + $Output.ExitCode | Should -Be 0 + } + + It 'Outputs running curl script with correct arguments' { + $line = $Output.Lines | Where-Object { $_ -match "packageScript.*curl\\tools" } | Select-Object -Last 1 + + $line | Should -Not -BeNullOrEmpty + $line | Should -MatchExactly "\/CurlIAParam" + $line | Should -MatchExactly "\/CurlOnlyParam" + $line | Should -Match "-forceX86" + } + + It 'Outputs running wget script with correct arguments' { + $line = $Output.Lines | Where-Object { $_ -match "packageScript.*wget\\tools" } + + $line | Should -Not -BeNullOrEmpty + $line | Should -Match "installArguments:? ''" + $line | Should -Match "packageParameters:? ''" + $line | Should -Not -Match "-forceX86" + } + } + + # We exclude this test when running CCM, as it will install and remove + # the firefox package which is used through other tests that will be affected. + Context "Upgrading packages while remembering arguments with multiple packages using arguments" -Tag CCMExcluded, Internal, VMOnly { + BeforeAll { + Restore-ChocolateyInstallSnapshot + + $null = Enable-ChocolateyFeature useRememberedArgumentsForUpgrades + $null = Invoke-Choco install curl --package-parameters="'/CurlOnlyParam'" --version="7.77.0" --ia="'/CurlIAParam'" --forcex86 -y + $null = Invoke-Choco install wget --version=1.21.1 -y --forcex86 + $null = Invoke-Choco install firefox --version=99.0.1 --package-parameters="'/l=eu'" -y --ia="'/RemoveDistributionDir=true'" + + $Output = Invoke-Choco upgrade all -y --debug + } + + AfterAll { + $null = Invoke-Choco uninstall firefox -y + } + + It 'Exits with Success (0)' { + $Output.ExitCode | Should -Be 0 + } + + It 'Outputs running curl script with correct arguments' { + $line = $Output.Lines | Where-Object { $_ -match "packageScript.*curl\\tools" } | Select-Object -Last 1 + + $line | Should -Not -BeNullOrEmpty + $line | Should -Match "installArguments:? '/CurlIAParam'" + $line | Should -Match "packageParameters:? '/CurlOnlyParam'" + $line | Should -Match "-forceX86" + } + + It 'Outputs running wget script with correct arguments' { + $line = $Output.Lines | Where-Object { $_ -match "packageScript.*wget\\tools" } | Select-Object -Last 1 + + $line | Should -Not -BeNullOrEmpty + $line | Should -Match "installArguments:? ''" + $line | Should -Match "packageParameters:? ''" + $line | Should -Match "-forceX86" + } + + It 'Outputs firefox using eu as language locale' { + $Output.Lines | Should -Contain "Using locale 'eu'..." -Because $Output.String + } + + It 'Outputs running firefox script with correct arguments' { + $line = $Output.Lines | Where-Object { $_ -match "packageScript.*firefox\\tools" } + + $line | Should -Not -BeNullOrEmpty + $line | Should -Match "installArguments:? '\/RemoveDistributionDir=true'" + $line | Should -Match "packageParameters:? '\/l=eu'" + $line | Should -Not -Match "-forceX86" + } + } }