(#2503) Add ability to export saved arguments #2506

TheCakeIsNaOH · 2022-01-05T05:08:17Z

Description Of Changes

This adds the --include-remembered-arguments option which is used to
export any remembered arguments. This reuses the
set_package_config_for_upgrade method in the NugetService to read and
parse the saved arguments.

Changes the set_package_config_for_upgrade method in the nuget service
to be public so it can be accessed for purposes of parsing the saved
arguments so they can be exported.

Motivation and Context

This is rounding out the feature set of the export command, so it is able to export the saved arguments.

Testing

Install a package with arguments: .\choco.exe install iperf2 --allow-unofficial --pre --ignore-dependencies --forcex86 --ia="/test" --override-arguments --params="/test" --args-global --params-global -m --allow-downgrade --timeout=0 --fail-on-stderr --use-system-powershell
Install a package without extra arguments: .\choco.exe install wget --allow-unofficial
Install a package with some of the same arguments as the first package had: .\choco.exe install z3 --package-parameters="'/z3param'" --forcex86
Run choco export --allow-unofficial --include-remembered-arguments
Open up the resulting packages.config file and check that it is something like this:

<?xml version="1.0" encoding="utf-8"?>
<packages>
  <package id="iperf2" prerelease="true" ignoreDependencies="true" forceX86="true" installArguments="/test" overrideArguments="true" applyInstallArgumentsToDependencies="true" packageParameters="/test" applyPackageParametersToDependencies="true" allowDowngrade="true" allowMultipleVersions="true" timeout="0" failOnStderr="true" useSystemPowershell="true" />
  <package id="Wget" />
  <package id="z3" packageParameters="/z3param" forceX86="true" />
</packages>

This validates that the remembered argument are export correctly, that they are not reused between packages, and that they can be set again in a later package.

Change Types Made

Bug fix (non-breaking change)
Feature / Enhancement (non-breaking change)
Breaking change (fix or feature that could cause existing functionality to change)

Related Issue

Fixes #2503
Depends on/based on #3003

Change Checklist

Requires a change to the documentation
Documentation has been updated
Tests to cover my changes, have been added
All new and existing tests passed.
N/A PowerShell v2 compatibility checked.

TheCakeIsNaOH · 2022-03-06T05:29:05Z

I'm open to suggestions on more unit tests that should be added, and if the cacheLocation should be exported or not.

coveralls · 2022-06-27T18:18:47Z

Coverage increased (+0.3%) to 27.881% when pulling eb3fe6b on TheCakeIsNaOH:export-remembered-args into 3892cfb on chocolatey:develop.

gep13

Noticed a couple things on this one. Let me know if you have any questions about my comments.

src/chocolatey/infrastructure.app/commands/ChocolateyExportCommand.cs

gep13 · 2022-08-12T20:22:40Z

src/chocolatey/infrastructure.app/commands/ChocolateyExportCommand.cs

+
+                                    //Mirrors the arguments captured in ChocolateyPackageService.capture_arguments()
+
+                                    if (configuration.Prerelease) xw.WriteAttributeString("prerelease", "true");


These strings, for example prerelease are starting to feel a little bit like "magic" strings, as they are being used in multiple places as bare strings. Being used here, as well as when naming the attributes in the PackagesConfigFilePackageSetting class. Should we break these out into a new class, of const string's, or similar, so we can define them once, and use in multiple places?

Hmm, why not use the PackagesConfigFilePackageSetting (or PackagesConfigFileSetting) class to do the xml serialization, that way the strings would be in only one place?

Hmm, that is not a bad idea. That would mean that the serialized XML could be used elsewhere. Although, I don't immediately see a need for it anywhere else, but putting it in one of these classes would mean that we "simplify" the command class, and do the work elsewhere.

I've switched to serializing that class to create the xml. It required adding members so the boolean options are not serialized if they are not specified, but otherwise I like the implementation.

src/chocolatey/infrastructure.app/configuration/ChocolateyConfiguration.cs

TheCakeIsNaOH · 2022-08-13T18:46:42Z

Converted to draft, as I found some inconsistent behavior with some arguments not being included, will do some debugging.

src/chocolatey.resources/helpers/ChocolateyTabExpansion.ps1

TheCakeIsNaOH · 2022-09-23T20:45:12Z

I've fixed the inconsistent behavior I think, and updated the manual testing. This is ready for a re-review.

TheCakeIsNaOH · 2023-01-28T04:15:23Z

This is draft until #3003 is merged/closed, as the implementation of this depends on what happens with that PR.

gep13 · 2023-12-14T11:22:33Z

Need to discuss what would happen here with regard to sensitive arguments that might have been passed in with Chocolatey Licensed Extension, i.e. --package-parameters-sensitive. We can't allow those to be included in the exported file.

TheCakeIsNaOH · 2024-01-09T22:51:47Z

Need to discuss what would happen here with regard to sensitive arguments that might have been passed in with Chocolatey Licensed Extension, i.e. --package-parameters-sensitive. We can't allow those to be included in the exported file.

Given that each item has to have support manually typed in, that is not a concern, at least not in the open source code base, as the open source code base does not support --package-parameters-sensitive in the .config file format and that there is no other explicit handling for it.

TheCakeIsNaOH · 2024-01-10T00:21:47Z

I've rebased this to depend on #3003

nopeless · 2024-03-07T17:21:03Z

@TheCakeIsNaOH thanks for the work. Makes it easier to export my current package config

This helps ensure that package IDs are handled in a case insensitive manner.

This allows overriding of remembered package parameters, install arguments, cache location and execution timeout during upgrade. So a user can pass in different package parameters or arguments without having to completely reinstall the package or turn off remembered arguments. Switch arguments cannot be checked, because the lack of a switch normally would mean that they are just relying on the remembered args to remember it, so there is no way to determine if an override of the remembered args is appropriate.

…ration This adds a new method, GetPackageConfigFromRememberedArguments which is similar to SetConfigFromRememberedArguments, but operates using a different method. First, a OptionSet 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 has a more general name 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.

Instead of exporting via building an xml document manually, this creates a PackagesConfigFilePackageSetting and serializes that to create the xml document that is saved. This allows for usage of the same class as is used to read in packages.config files to export those files. The reason the various "Specified" members are added to the PackagesConfigFilePackageSetting class is so if an element is not set during export, it will not show up at all in the resulting serialized packages.config file.

This adds the --include-remembered-arguments option which is used to export any remembered arguments. It reuses the GetPackageConfigFromRememberedArguments method in the NugetService to read and parse the remembered arguments.

TheCakeIsNaOH changed the title ~~Export remembered args~~ (#2503) Add ability to export saved arguments Jan 5, 2022

TheCakeIsNaOH force-pushed the export-remembered-args branch 2 times, most recently from d2c4fc9 to dac10e7 Compare March 6, 2022 05:28

TheCakeIsNaOH force-pushed the export-remembered-args branch 2 times, most recently from 6333235 to 265ec90 Compare March 9, 2022 16:30

TheCakeIsNaOH force-pushed the export-remembered-args branch from 265ec90 to ace284f Compare March 21, 2022 16:19

TheCakeIsNaOH force-pushed the export-remembered-args branch from ace284f to 6dfe263 Compare June 27, 2022 15:14

TheCakeIsNaOH force-pushed the export-remembered-args branch from 6dfe263 to b7b4012 Compare August 11, 2022 23:04

TheCakeIsNaOH mentioned this pull request Aug 11, 2022

(#2603) Export pin status #2618

Open

7 tasks

gep13 requested changes Aug 12, 2022

View reviewed changes

TheCakeIsNaOH force-pushed the export-remembered-args branch from b7b4012 to ee96706 Compare August 12, 2022 22:05

TheCakeIsNaOH requested a review from gep13 August 13, 2022 18:45

TheCakeIsNaOH marked this pull request as draft August 13, 2022 18:46

gep13 reviewed Aug 13, 2022

View reviewed changes

src/chocolatey.resources/helpers/ChocolateyTabExpansion.ps1 Outdated Show resolved Hide resolved

TheCakeIsNaOH force-pushed the export-remembered-args branch from 2840467 to eb3fe6b Compare September 23, 2022 20:42

TheCakeIsNaOH requested a review from gep13 September 23, 2022 20:44

TheCakeIsNaOH marked this pull request as ready for review September 23, 2022 20:44

TheCakeIsNaOH force-pushed the export-remembered-args branch 2 times, most recently from 1e9c0d1 to 097f703 Compare December 23, 2022 01:21

TheCakeIsNaOH force-pushed the export-remembered-args branch 2 times, most recently from 1438bc6 to 0b51a7d Compare January 13, 2023 15:26

TheCakeIsNaOH marked this pull request as draft January 28, 2023 04:15

TheCakeIsNaOH force-pushed the export-remembered-args branch from 0b51a7d to 25541d9 Compare January 10, 2024 00:20

TheCakeIsNaOH marked this pull request as ready for review January 10, 2024 00:21

TheCakeIsNaOH added 5 commits April 27, 2024 21:20

(chocolatey#508) Make package id comparisons case insensitive

edd1f23

This helps ensure that package IDs are handled in a case insensitive manner.

TheCakeIsNaOH force-pushed the export-remembered-args branch from 25541d9 to db7ea32 Compare April 28, 2024 02:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(#2503) Add ability to export saved arguments #2506

(#2503) Add ability to export saved arguments #2506

TheCakeIsNaOH commented Jan 5, 2022 •

edited

Loading

TheCakeIsNaOH commented Mar 6, 2022 •

edited

Loading

coveralls commented Jun 27, 2022 •

edited

Loading

gep13 left a comment

gep13 Aug 12, 2022

TheCakeIsNaOH Aug 13, 2022

gep13 Aug 13, 2022

TheCakeIsNaOH Sep 23, 2022

TheCakeIsNaOH commented Aug 13, 2022

TheCakeIsNaOH commented Sep 23, 2022

TheCakeIsNaOH commented Jan 28, 2023

gep13 commented Dec 14, 2023

TheCakeIsNaOH commented Jan 9, 2024

TheCakeIsNaOH commented Jan 10, 2024

nopeless commented Mar 7, 2024


		//Mirrors the arguments captured in ChocolateyPackageService.capture_arguments()

		if (configuration.Prerelease) xw.WriteAttributeString("prerelease", "true");

(#2503) Add ability to export saved arguments #2506

Are you sure you want to change the base?

(#2503) Add ability to export saved arguments #2506

Conversation

TheCakeIsNaOH commented Jan 5, 2022 • edited Loading

Description Of Changes

Motivation and Context

Testing

Change Types Made

Related Issue

Change Checklist

TheCakeIsNaOH commented Mar 6, 2022 • edited Loading

coveralls commented Jun 27, 2022 • edited Loading

gep13 left a comment

Choose a reason for hiding this comment

gep13 Aug 12, 2022

Choose a reason for hiding this comment

TheCakeIsNaOH Aug 13, 2022

Choose a reason for hiding this comment

gep13 Aug 13, 2022

Choose a reason for hiding this comment

TheCakeIsNaOH Sep 23, 2022

Choose a reason for hiding this comment

TheCakeIsNaOH commented Aug 13, 2022

TheCakeIsNaOH commented Sep 23, 2022

TheCakeIsNaOH commented Jan 28, 2023

gep13 commented Dec 14, 2023

TheCakeIsNaOH commented Jan 9, 2024

TheCakeIsNaOH commented Jan 10, 2024

nopeless commented Mar 7, 2024

TheCakeIsNaOH commented Jan 5, 2022 •

edited

Loading

TheCakeIsNaOH commented Mar 6, 2022 •

edited

Loading

coveralls commented Jun 27, 2022 •

edited

Loading