Skip to content

Improper parsing of Chocolatey parameters #405

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
GuyWhoKnowsTheGuy opened this issue Sep 9, 2019 · 7 comments · Fixed by #412
Closed

Improper parsing of Chocolatey parameters #405

GuyWhoKnowsTheGuy opened this issue Sep 9, 2019 · 7 comments · Fixed by #412
Assignees
Labels
5 - Released The issue has been resolved, and released to the public for consumption Bug Issues where something has happened which was not expected or intended
Milestone

Comments

@GuyWhoKnowsTheGuy
Copy link

What You Are Seeing?

Failure to parse Chocolatey parameter

What is Expected?

Correctly parsing Chocolatey parameter

How Did You Get This To Happen? (Steps to Reproduce)

boxstarter script.ps1

where script.ps1 only contains

choco install -y cmake --installargs "ADD_CMAKE_TO_PATH=System"

However,
choco install -y cmake --installargs="ADD_CMAKE_TO_PATH=System" works fine, which makes me think it's simply a parsing issue.

Output Log

Full Log Output

Boxstarter starting Calling Chocolatey to install --installargs. This may take several minutes to complete...
Boxstarter starting Calling Chocolatey to install --installargs. This may take several minutes to complete...
Installing the following packages:
cmake;ADD_CMAKE_TO_PATH=System
By installing you accept licenses for the packages.
cmake v3.15.2 already installed.
Use --force to reinstall, specify a version to install, or try upgrade.
ADD_CMAKE_TO_PATH=System not installed. The package was not found with the source(s) listed.
If you specified a particular version and are receiving this message, it is possible that the package name exists but the version does not.
Version: ""
Source(s): "https://chocolatey.org/api/v2/"

@mwallner mwallner added the Bug Issues where something has happened which was not expected or intended label Sep 11, 2019
@mwallner
Copy link
Member

this is somehow related to #265 - Boxstarter definitely needs some work on it's command parsing, as it simply tries to install an array of packages after a choco install command.

note, this should only applies to "Boxstarter packages", not when using BoxstarterShell.

@mwallner
Copy link
Member

note that #404 does NOT resolve this bug.

@pauby
Copy link
Member

pauby commented Oct 3, 2019

@mwallner You happy to take this one on?

@mwallner mwallner self-assigned this Oct 10, 2019
@mwallner
Copy link
Member

In theory Call-Chocolatey already handles arbitrary arguments passed to chocolatey.

function Call-Chocolatey {
    param(
        [string]$command,
        [string[]]$packageNames=@('')
    )
    $chocoArgs = @($command, $packageNames)
    $chocoArgs += Format-ExeArgs $command @args
    Write-BoxstarterMessage "Passing the following args to Chocolatey: $chocoArgs" -Verbose

.. relates to #145

@mwallner
Copy link
Member

Oh, the code in https://github.com/chocolatey/boxstarter/blob/master/Boxstarter.Chocolatey/Chocolatey.ps1#L301 only checks for = separated parameters, imo this is the correct place for fixing this.

@mwallner mwallner added this to the v2.13.0 milestone Oct 10, 2019
@mwallner
Copy link
Member

I've just written a couple of tests and verified: there definitely is a bug in choco command parsing that leads to argument reordering before those args are passed to choco.exe.

i.e.:

choco Install -y pkg --source blah --installargs "ADD_CMAKE_TO_PATH=System"

I've added a couple of Debug-print statements to function chocolatey:

Write-BoxstarterMessage "chocolatey wrapper: PSBoundParameters: '$($PSBoundParameters.Keys | % { "`r`n  $($_) -> $($PSBoundParameters[$_])" })`r`n'" -Verbose
Write-BoxstarterMessage "chocolatey wrapper: args: $($args | % { $_ })" -Verbose

which gives me

VERBOSE: Boxstarter: chocolatey wrapper: PSBoundParameters: '
  command -> Install
  packageNames -> --source
'
VERBOSE: Boxstarter: chocolatey wrapper: args: -y pkg blah --installargs ADD_CMAKE_TO_PATH=System

that finally leads to

Install --source -y pkg blah --installargs ADD_CMAKE_TO_PATH=System

being passed to choco.exe.
(which is bad, as '--source blah' needs to be kept in correct order, also the quotes of the '--installargs' parameter are lost).

After all, function cinst, function choco, function cup as well as function chocolatey need to be fixed to keep the original parameter order.

@pauby
Copy link
Member

pauby commented Aug 7, 2020

Build is failing in AppVeyor. Once that is resolved we can get to merging this.

Awesome work @mwallner. Thanks for picking this up and completing it!

gep13 pushed a commit to mwallner/boxstarter that referenced this issue Aug 17, 2020
gep13 pushed a commit to mwallner/boxstarter that referenced this issue Aug 17, 2020
mwallner added a commit to mwallner/boxstarter that referenced this issue Aug 22, 2020
mwallner added a commit to mwallner/boxstarter that referenced this issue Oct 12, 2020
pauby pushed a commit that referenced this issue Oct 13, 2020
* GH-405: fix package parameter handling

* fix bad package parameter handling

* objects are objects, not strings

* GH-405: fix missing named params

* ditch .metals vscode folder

* ignore .metals vscode folder

* .gitignore should have newline at end of file

* GH-405: Get-PackageNamesFromInvocationLine fix

* GH-405: fix edge case Get-PackageNamesFromInvocationLine
@pauby pauby changed the title Improper Parsing of Chocolatey Parameter Improper parsing of Chocolatey parameters Oct 19, 2020
@pauby pauby added the 5 - Released The issue has been resolved, and released to the public for consumption label Oct 19, 2020
HolisticDeveloper pushed a commit to HolisticDeveloper/boxstarter that referenced this issue Oct 18, 2021


* 'master' of https://github.com/HolisticDeveloper/boxstarter: (58 commits)
  Update chocolatey uninstall url
  Fix chocolatey links
  (chocolateyGH-446) Updates documentation with important info
  (chocolateyGH-443) Adds Windows Explorer item check box view Even though Boxstarter has existing functionality to modify Windows Explorer's settings, the existing functionality did not allow for user's to configure the "Use check boxes to select items" setting.  This commit adds that functionality to the existing set-windowsexploreroptions function.
  (chocolateyGH-451) more accurate regex
  (chocolateyGH-451) StopOnPackageFailiure not stripped without RebootCodes
  (release v2.13.0) Updated version numbers
  chocolateyGH-444 do not pass StopOnPackageFailure to choco
  Merge pull request from GHSA-rpgx-h675-r3jf
  (chocolateyGH-405) Fix package parameter handling (chocolatey#412)
  (chocolateyGH-434) Fix disabling bing search for Win release 2004+ (chocolatey#438)
  chocolateygh-435 docs for DelegateChocoSources
  (build) Replace direct usage of cinst
  (build) Pin all packages to specific version
  (build) Remove deprecated package
  (build) Make project files consistent
  (build) Pin to specific version of nuget.commandline
  (build) Remove VS import for web targets
  (build) Trying to fix AppVeyor build
  (doc) Update template
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Released The issue has been resolved, and released to the public for consumption Bug Issues where something has happened which was not expected or intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants