Support package parameters and install arguments starting with dashes

[jberezanski]

Some Chocolatey packages use package parameters which begin with dashes. One example is the Visual Studio family of packages:

choco install visualstudio2019buildtools --package-parameters "--installPath C:\BT --allWorkloads --passive"

Also, some native installers may accept dashed parameters (again, the Visual Studio Installer is one example).

The existing design of ChocolateyGet assumes that package parameters and install arguments are passed inside the AdditionalArguments dynamic option. However, the parsing code in Install-Package is very simple - it just splits the AdditionalArguments string by dashes and makes no attempt at handling quoting. This falls apart when the package parameters or installer arguments contain tokens which start with a dash.

Rather than attempt to crawl deeper into the rabbit hole of quoting and complex regexes, this PR fixes the issue by promoting package parameters and install arguments to first-class provider options.

Examples which now work correctly:

Install-Package -ProviderName ChocolateyGet -Name visualstudio2019buildtools  -PackageParameters "--installPath C:\BT --passive --locale pl-PL"

configuration c
{
    Import-DscResource -Module @{ModuleName = 'PackageManagement'; RequiredVersion = '1.4.7'}
    Node localhost
    {
        PackageManagement VSComm
        {
            Ensure = 'Present'
            Name = 'visualstudio2019community'
            ProviderName = 'ChocolateyGet'
            RequiredVersion = 'latest'
            Source = 'chocolatey'
            AdditionalParameters = @{ PackageParameters = '--installPath C:\VSComm --nickName Comm' }
        }
    }
}

The existing syntax (-params/-args inside AdditionalArguments) is left intact for backward compatibility and takes precedence, if the user specifies parameters using both methods at once.

[jberezanski]

If you are OK with the design, I can add some tests.

[ethanbergstrom]

Chocolatey packages are supposed to use slashes, not dashes, for package parameters. That's the standard that Chocolatey's Get-PackageParameters cmdlet uses. See https://docs.chocolatey.org/en-us/create/functions/get-packageparameters#parameters-string and https://github.com/chocolatey/choco/blob/master/src/chocolatey.resources/helpers/functions/Get-PackageParameters.ps1#L65-L66 for more information

If the package in question is passing anything and everything to the underlying installer, they really should be chocolatey arguments, not parameters, which are not supported with the free version of Chocolatey. See https://stackoverflow.com/a/37817046/14384982

See https://github.com/chocolatey-community/chocolatey-coreteampackages/blob/master/automatic/sysinternals/tools/chocolateyInstall.ps1#L4 for an example of how Chocolatey package parameters should be handled within the package, which this Visual Studio package is not adhering to.

[jberezanski]

Chocolatey packages are supposed to use slashes, not dashes, for package parameters.

This is just a convention introduced several years after the concept of package parameters first appeared in Chocolatey in 2013. Initially, there had been no facility for parsing the parameters and for 4 years packages had to invent their own parameter style and develop parsing code for it. Eventually, one convention became popular, at first as part of a community extension, and finally made its way into Chocolatey proper in 2017. But still, it was only a suggestion/recommendation, never a hard requirement.

The fact is that there are many packages on chocolatey.org which do not use slashes for parameters and those packages were accepted by moderators without any issues. Some use dashes (examples: visualstudio2015*, visualstudio2017*, visualstudio2019* - over 80 packages in total), others yet another syntax (example: dotnetcore-*). choco.exe supports all of them, because it does not really put any restrictions on the value of --package-parameters.

If the package in question is passing anything and everything to the underlying installer, they really should be chocolatey arguments, not parameters, which are not supported with the free version of Chocolatey

The possibility to pass additional arguments to the underlying installer is very much a feature of the free version of Chocolatey and predates even the concept of package parameters. Here there can be absolutely no control over the format of the arguments, because it depends entirely on the underlying installer. One could, for example, make a Chocolatey package for installing something using WinGet and users of that package would need to pass dashed arguments in --install-arguments if they wanted to tweak the installation, because that is the argument syntax accepted by winget.

     --ia, --installargs, --install-args, --installarguments, --install-arguments=VALUE
     InstallArguments - Install Arguments to pass to the native installer in 
       the package. Defaults to unspecified.

(https://docs.chocolatey.org/en-us/choco/commands/install#options-and-switches)

[ethanbergstrom]

Sorry for the delay.

Thanks for the historical background, that's helpful, and the design looks good to me. Like you mentioned, just add test coverage and it should be good to go.

[jberezanski]

Apologies for the silence, I've been on vacation, Thanks for the green light, I'll get to work on the tests.

[jberezanski]

I added test cases for -PackageParameters. I've never used --install-arguments, so I don't know of any good package to use for testing -InstallArguments. However, the mechanism is exactly the same and the existing tests do not cover --install-arguments either, so perhaps the -PackageParameters tests are sufficient.

I started adding tests for the specific case which motivated this PR (jberezanski@8256861), but ultimately decided against it, for these reasons:

• the only packages with this parameter syntax I know of are the Visual Studio packages;
• even the smallest products from the Visual Studio family (Test Agent, Feedback Client) install a lot of things on the system, including several prerequisite Chocolatey packages, taking a noticeable amount of time and possibly requiring a reboot;
• one of the dependency packages currently fails during uninstallation, so proper cleanup would introduce much complexity in the test code.

[jberezanski]

The history of this branch is rather complicated, with several merges. Would you like me to clean it up, by recreating my changes on top of the current master?

[ethanbergstrom]

Looks good to me, just made a few edits and removed the workaround for Foil behavior.

Regarding the commit history, I'll just squash merge it.

[jberezanski]

Thanks!