Skip to content

Commit

Permalink
(#2886) Switch remembered args to only change local configuration
Browse files Browse the repository at this point in the history
This renames set_package_config_for_upgrade to
get_package_config_from_remembered_args and changes how it operates.
First, a option set is set up, so only the config that is passed in is
modified, instead of the config that the global options parser was set
with with.
Second, it returns the modified configuration instead of the original
configuration, because it is the local configuration being modified.
Third, it renames the function and changes log messages to be more
general, so it later can more easily be reused for uninstall and export.

This change fixes remembered arguments when Chocolatey is used as a
library, like in ChocolateyGUI, where the config that is passed to
the install_run method is not necessarily the same config object that
was used to set up the global argument parser.

The downside of using a new commandline parser is that it opens up the
possibility of drift between the upgrade/global arguments and this
added parser. However, this is not an issue because the format of the
saved arguments is known, and any added arguments there would not work
without being added here as well, which would be picked up during
development.
  • Loading branch information
TheCakeIsNaOH committed Jun 16, 2023
1 parent 6630a2f commit 8bcb5ad
Showing 1 changed file with 73 additions and 11 deletions.
84 changes: 73 additions & 11 deletions src/chocolatey/infrastructure.app/services/NugetService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1044,7 +1044,7 @@ public virtual ConcurrentDictionary<string, PackageResult> Upgrade(ChocolateyCon
continue;
}

SetConfigFromRememberedArguments(config, pkgInfo);
config = GetPackageConfigFromRememberedArguments(config, pkgInfo);
var pathResolver = NugetCommon.GetPathResolver(_fileSystem);
var nugetProject = new FolderNuGetProject(ApplicationParameters.PackagesLocation, pathResolver, NuGetFramework.AnyFramework);

Expand Down Expand Up @@ -1576,12 +1576,12 @@ public virtual ConcurrentDictionary<string, PackageResult> GetOutdated(Chocolate
}

/// <summary>
/// Sets the configuration for the package upgrade
/// Gets the configuration from remembered arguments
/// </summary>
/// <param name="config">The configuration.</param>
/// <param name="config">The original configuration.</param>
/// <param name="packageInfo">The package information.</param>
/// <returns>The original unmodified configuration, so it can be reset after upgrade</returns>
protected virtual ChocolateyConfiguration SetConfigFromRememberedArguments(ChocolateyConfiguration config, ChocolateyPackageInformation packageInfo)
/// <returns>The modified configuration, so it can be used</returns>
protected virtual ChocolateyConfiguration GetPackageConfigFromRememberedArguments(ChocolateyConfiguration config, ChocolateyPackageInformation packageInfo)
{
if (!config.Features.UseRememberedArgumentsForUpgrades || string.IsNullOrWhiteSpace(packageInfo.Arguments)) return config;

Expand All @@ -1591,7 +1591,7 @@ protected virtual ChocolateyConfiguration SetConfigFromRememberedArguments(Choco
if (!ArgumentsUtility.SensitiveArgumentsProvided(packageArgumentsUnencrypted))
{
sensitiveArgs = false;
this.Log().Debug(ChocolateyLoggers.Verbose, "{0} - Adding remembered arguments for upgrade: {1}".FormatWith(packageInfo.Package.Id, packageArgumentsUnencrypted.EscapeCurlyBraces()));
this.Log().Debug(ChocolateyLoggers.Verbose, "{0} - Adding remembered arguments: {1}".FormatWith(packageInfo.Package.Id, packageArgumentsUnencrypted.EscapeCurlyBraces()));
}

var packageArgumentsSplit = packageArgumentsUnencrypted.Split(new[] { " --" }, StringSplitOptions.RemoveEmptyEntries);
Expand All @@ -1609,14 +1609,74 @@ protected virtual ChocolateyConfiguration SetConfigFromRememberedArguments(Choco

if (sensitiveArgs)
{
this.Log().Debug(ChocolateyLoggers.Verbose, "{0} - Adding '{1}' to upgrade arguments. Values not shown due to detected sensitive arguments".FormatWith(packageInfo.Package.Id, optionName.EscapeCurlyBraces()));
this.Log().Debug(ChocolateyLoggers.Verbose, "{0} - Adding '{1}' to arguments. Values not shown due to detected sensitive arguments".FormatWith(packageInfo.Package.Id, optionName.EscapeCurlyBraces()));
}
packageArguments.Add("--{0}{1}".FormatWith(optionName, string.IsNullOrWhiteSpace(optionValue) ? string.Empty : "=" + optionValue));
}

var originalConfig = config.DeepCopy();
// this changes config globally
ConfigurationOptions.OptionSet.Parse(packageArguments);
var rememberedOptionSet = new OptionSet();

rememberedOptionSet
.Add("pre|prerelease",
"Prerelease - Include Prereleases? Defaults to false.",
option => config.Prerelease = option != null)
.Add("i|ignoredependencies|ignore-dependencies",
"IgnoreDependencies - Ignore dependencies when installing package(s). Defaults to false.",
option => config.IgnoreDependencies = option != null)
.Add("x86|forcex86",
"ForceX86 - Force x86 (32bit) installation on 64 bit systems. Defaults to false.",
option => config.ForceX86 = option != null)
.Add("ia=|installargs=|install-args=|installarguments=|install-arguments=",
"InstallArguments - Install Arguments to pass to the native installer in the package. Defaults to unspecified.",
option => config.InstallArguments = option.remove_surrounding_quotes())
.Add("o|override|overrideargs|overridearguments|override-arguments",
"OverrideArguments - Should install arguments be used exclusively without appending to current package passed arguments? Defaults to false.",
option => config.OverrideArguments = option != null)
.Add("argsglobal|args-global|installargsglobal|install-args-global|applyargstodependencies|apply-args-to-dependencies|apply-install-arguments-to-dependencies",
"Apply Install Arguments To Dependencies - Should install arguments be applied to dependent packages? Defaults to false.",
option => config.ApplyInstallArgumentsToDependencies = option != null)
.Add("params=|parameters=|pkgparameters=|packageparameters=|package-parameters=",
"PackageParameters - Parameters to pass to the package. Defaults to unspecified.",
option => config.PackageParameters = option.remove_surrounding_quotes())
.Add("paramsglobal|params-global|packageparametersglobal|package-parameters-global|applyparamstodependencies|apply-params-to-dependencies|apply-package-parameters-to-dependencies",
"Apply Package Parameters To Dependencies - Should package parameters be applied to dependent packages? Defaults to false.",
option => config.ApplyPackageParametersToDependencies = option != null)
.Add("allowdowngrade|allow-downgrade",
"AllowDowngrade - Should an attempt at downgrading be allowed? Defaults to false.",
option => config.AllowDowngrade = option != null)
.Add("u=|user=",
"User - used with authenticated feeds. Defaults to empty.",
option => config.SourceCommand.Username = option.remove_surrounding_quotes())
.Add("p=|password=",
"Password - the user's password to the source. Defaults to empty.",
option => config.SourceCommand.Password = option.remove_surrounding_quotes())
.Add("cert=",
"Client certificate - PFX pathname for an x509 authenticated feeds. Defaults to empty. Available in 0.9.10+.",
option => config.SourceCommand.Certificate = option.remove_surrounding_quotes())
.Add("cp=|certpassword=",
"Certificate Password - the client certificate's password to the source. Defaults to empty. Available in 0.9.10+.",
option => config.SourceCommand.CertificatePassword = option.remove_surrounding_quotes())
.Add("timeout=|execution-timeout=",
"CommandExecutionTimeout (in seconds) - The time to allow a command to finish before timing out. Overrides the default execution timeout in the configuration of {0} seconds. '0' for infinite starting in 0.10.4.".format_with(config.CommandExecutionTimeoutSeconds.to_string()),
option =>
{
int timeout = 0;
var timeoutString = option.remove_surrounding_quotes();
int.TryParse(timeoutString, out timeout);
if (timeout > 0 || timeoutString.is_equal_to("0"))
{
config.CommandExecutionTimeoutSeconds = timeout;
}
})
.Add("c=|cache=|cachelocation=|cache-location=",
"CacheLocation - Location for download cache, defaults to %TEMP% or value in chocolatey.config file.",
option => config.CacheLocation = option.remove_surrounding_quotes())
.Add("use-system-powershell",
"UseSystemPowerShell - Execute PowerShell using an external process instead of the built-in PowerShell host. Should only be used when internal host is failing. Available in 0.9.10+.",
option => config.Features.UsePowerShellHost = option != null);

rememberedOptionSet.Parse(packageArguments);

// there may be overrides from the user running upgrade
if (!string.IsNullOrWhiteSpace(originalConfig.PackageParameters)) config.PackageParameters = originalConfig.PackageParameters;
Expand All @@ -1626,7 +1686,9 @@ protected virtual ChocolateyConfiguration SetConfigFromRememberedArguments(Choco
if (!string.IsNullOrWhiteSpace(originalConfig.SourceCommand.Certificate)) config.SourceCommand.Certificate = originalConfig.SourceCommand.Certificate;
if (!string.IsNullOrWhiteSpace(originalConfig.SourceCommand.CertificatePassword)) config.SourceCommand.CertificatePassword = originalConfig.SourceCommand.CertificatePassword;

return originalConfig;
// We can't override cache location, execution timeout, or the switches because we don't know here if they were set on the command line

return config;
}

private bool HasMissingDependency(PackageResult package, List<PackageResult> allLocalPackages)
Expand Down Expand Up @@ -2724,7 +2786,7 @@ public virtual ConcurrentDictionary<string, PackageResult> get_outdated(Chocolat

[Obsolete("This overload is deprecated and will be removed in v3.")]
protected virtual ChocolateyConfiguration set_package_config_for_upgrade(ChocolateyConfiguration config, ChocolateyPackageInformation packageInfo)
=> SetConfigFromRememberedArguments(config, packageInfo);
=> GetPackageConfigFromRememberedArguments(config, packageInfo);

[Obsolete("This overload is deprecated and will be removed in v3.")]
protected virtual void ensure_package_files_have_compatible_attributes(ChocolateyConfiguration config, IPackageMetadata installedPackage)
Expand Down

0 comments on commit 8bcb5ad

Please sign in to comment.