Skip to content
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

(chocolatey-core.extension) breaks Get-PackageParameters function if paramater contains spaces #950

Closed
deckjockey opened this issue Jan 18, 2018 · 12 comments

Comments

@deckjockey
Copy link

Expected Behavior

Get-PackageParameters function supports parameters with spaces:
For example `-Parameters "/ITEM1:value /ITEM2:value with spaces"
https://chocolatey.org/docs/helpers-get-package-parameters

Current Behavior

Get-PackageParameters function supports parameters with spaces:
For example `-Parameters "/ITEM1:value /ITEM2:value with spaces"
https://chocolatey.org/docs/helpers-get-package-parameters

Possible Solution

Should Get-PackageParameters.ps1 be removed from the chocolatey-core.extension package? since it is already a function in chocolatey base package

Steps to Reproduce (for bugs)

===========================================
Install package without Extensions package:

chocolatey 0.10.8

example:

choco install nabtrade-securities-wiki --params="'/SEINSTALL:D:\ccviews\Program Files\securitease_test1'" -y

Parameters with spaces work as expected:

2018-01-18 14:15:53,850 12728 [INFO ] - Chocolatey v0.10.8
2018-01-18 14:15:53,859 12728 [DEBUG] - Chocolatey is running on Windows v 6.1.7601.65536
2018-01-18 14:15:53,862 12728 [DEBUG] - Attempting to delete file "C:/ProgramData/chocolatey/choco.exe.old".
2018-01-18 14:15:53,864 12728 [DEBUG] - Attempting to delete file "C:\ProgramData\chocolatey\choco.exe.old".
2018-01-18 14:15:53,874 12728 [DEBUG] - Command line: "C:\ProgramData\chocolatey\choco.exe" install nabtrade-securities-wiki --params="'/SEINSTALL:D:\ccviews\Program Files\securitease_test1'" -y
2018-01-18 14:15:53,875 12728 [DEBUG] - Received arguments: install nabtrade-securities-wiki --params='/SEINSTALL:D:\ccviews\Program Files\securitease_test1' -y
...
PackageParameters='/SEINSTALL:D:\ccviews\Program Files\securitease_test1'|
...
2018-01-18 14:16:35,474 12728 [DEBUG] - Calling built-in PowerShell host with ['[System.Threading.Thread]::CurrentThread.CurrentCulture = '';[System.Threading.Thread]::CurrentThread.CurrentUICulture = ''; & import-module -name 'C:\ProgramData\chocolatey_test1\helpers\chocolateyInstaller.psm1'; & 'C:\ProgramData\chocolatey_test1\helpers\chocolateyScriptRunner.ps1' -packageScript 'C:\ProgramData\chocolatey_test1\lib\nabtrade-securities-wiki\tools\chocolateyInstall.ps1' -installArguments '' -packageParameters '/SEINSTALL:D:\ccviews\Program Files\securitease_test1'']
...
2018-01-18 14:16:38,082 12728 [DEBUG] - Running 'ChocolateyScriptRunner' for nabtrade-securities-wiki v05.26.00.00 with packageScript 'C:\ProgramData\chocolatey_test1\lib\nabtrade-securities-wiki\tools\chocolateyInstall.ps1', packageFolder:'C:\ProgramData\chocolatey_test1\lib\nabtrade-securities-wiki', installArguments: '', packageParameters: '/SEINSTALL:D:\ccviews\Program Files\securitease_test1',
...
2018-01-18 14:16:38,116 12728 [INFO ] - ================================================================================
2018-01-18 14:16:38,123 12728 [INFO ] - * Getting parameters...
2018-01-18 14:16:38,152 12728 [DEBUG] - Running Get-PackageParameters
2018-01-18 14:16:38,165 12728 [DEBUG] - Parsing $env:ChocolateyPackageParameters and $env:ChocolateyPackageParametersSensitive for parameters
2018-01-18 14:16:38,180 12728 [DEBUG] - Adding package param 'SEINSTALL'='D:\ccviews\Program Files\securitease_test1'
2018-01-18 14:16:38,215 12728 [INFO ] - -> SEINSTALL parameter = D:\ccviews\Program Files\securitease_test1
2018-01-18 14:16:38,221 12728 [INFO ] - ================================================================================

========================================
Install package with Extensions package:

chocolatey 0.10.8
chocolatey-core.extension 1.3.3

example:

choco install nabtrade-securities-wiki --params="'/SEINSTALL:D:\ccviews\Program Files\securitease_test1'" -y

Parameters with spaces no longer work:

2018-01-18 14:23:06,289 13400 [INFO ] - Chocolatey v0.10.8
2018-01-18 14:23:06,298 13400 [DEBUG] - Chocolatey is running on Windows v 6.1.7601.65536
2018-01-18 14:23:06,301 13400 [DEBUG] - Attempting to delete file "C:/ProgramData/chocolatey/choco.exe.old".
2018-01-18 14:23:06,302 13400 [DEBUG] - Attempting to delete file "C:\ProgramData\chocolatey\choco.exe.old".
2018-01-18 14:23:06,310 13400 [DEBUG] - Command line: "C:\ProgramData\chocolatey\choco.exe" install nabtrade-securities-wiki --params="'/SEINSTALL:D:\ccviews\Program Files\securitease_test1'" -y
2018-01-18 14:23:06,312 13400 [DEBUG] - Received arguments: install nabtrade-securities-wiki --params='/SEINSTALL:D:\ccviews\Program Files\securitease_test1' -y
...
PackageParameters='/SEINSTALL:D:\ccviews\Program Files\securitease_test1'|
...
2018-01-18 14:23:45,378 13400 [DEBUG] - Calling built-in PowerShell host with ['[System.Threading.Thread]::CurrentThread.CurrentCulture = '';[System.Threading.Thread]::CurrentThread.CurrentUICulture = ''; & import-module -name 'C:\ProgramData\chocolatey_test1\helpers\chocolateyInstaller.psm1'; & 'C:\ProgramData\chocolatey_test1\helpers\chocolateyScriptRunner.ps1' -packageScript 'C:\ProgramData\chocolatey_test1\lib\nabtrade-securities-wiki\tools\chocolateyInstall.ps1' -installArguments '' -packageParameters '/SEINSTALL:D:\ccviews\Program Files\securitease_test1'']
...
2018-01-18 14:23:48,253 13400 [DEBUG] - Running 'ChocolateyScriptRunner' for nabtrade-securities-wiki v05.26.00.00 with packageScript 'C:\ProgramData\chocolatey_test1\lib\nabtrade-securities-wiki\tools\chocolateyInstall.ps1', packageFolder:'C:\ProgramData\chocolatey_test1\lib\nabtrade-securities-wiki', installArguments: '', packageParameters: '/SEINSTALL:D:\ccviews\Program Files\securitease_test1',
...
2018-01-18 14:23:48,280 13400 [INFO ] - ================================================================================
2018-01-18 14:23:48,288 13400 [INFO ] - * Getting parameters...
2018-01-18 14:23:48,428 13400 [INFO ] - -> SEINSTALL parameter = D:\ccviews\Program
2018-01-18 14:23:48,442 13400 [INFO ] - ================================================================================
after.choco.summary.log
after.chocolatey.log
before.choco.summary.log
before.chocolatey.log
chocolateyInstall.zip

Context

Unable to use parameters with spaces after chocolatey-core.extension is installed, so is a show stopper

Your Environment

  • Package Version used: 1.3.3
  • Operating System and version: Windows v 6.1.7601.65536
  • Chocolatey version: Chocolatey v0.10.8
  • Install/uninstall gist:
@majkinetor
Copy link
Contributor

majkinetor commented Jan 18, 2018

As with any other tool, you must wrap spaces in ' or ".

PS> Get-PackageParameters "/SEINSTALL:'D:\ccviews\Program Files\securitease_test1'"

Name                           Value
----                           -----
SEINSTALL                      D:\ccviews\Program Files\securitease_test1

PS> Get-PackageParameters '/SEINSTALL:"D:\ccviews\Program Files\securitease_test1"'

Name                           Value
----                           -----
SEINSTALL                      D:\ccviews\Program Files\securitease_test1

@majkinetor
Copy link
Contributor

BTW, internal choco helper function is semi-compatible with core function, so it was a poor choice to name it the same IMO. The differences are subtle but they exist as some packages failed just by removing core extension.

Also, on choco help you have example with long names:

Example 4

# Pass in your own values
Get-PackageParameters -Parameters "/Shortcut /InstallDir:'c:\program files\xyz' /NoStartup" | set r
if ($r.Shortcut) {... }
Write-Host $r.InstallDir

@deckjockey
Copy link
Author

The syntax is different depending on whether the Extensions are install or not, which is bad, as you cannot assume the Extensions are installed if a package DOES NOT require them.

So with the extension installed, the following syntax works:

  • choco install nabtrade-securities-wiki --params="/SEINSTALL:'D:\ccviews\Program Files\securitease_test1'" -y

But without the extension, the same syntax fails:

  • choco install nabtrade-securities-wiki --params="/SEINSTALL:'D:\ccviews\Program Files\securitease_test1'" -y
  • Getting parameters...
    -> SEINSTALL parameter = 'D:\ccviews\Program Files\securitease_test1'

  • Copy Wiki files to Securitease folder...
    -> toolsDir = C:\ProgramData\chocolatey_test1\lib\nabtrade-securities-wiki\to
    ols
    ERROR: ERROR: Securitese Folder is missing!
    The install of nabtrade-securities-wiki was NOT successful.
    Error while running 'C:\ProgramData\chocolatey_test1\lib\nabtrade-securities-wik
    i\tools\chocolateyInstall.ps1'.
    See log for details.

@majkinetor
Copy link
Contributor

majkinetor commented Jan 23, 2018

I tried both variants on my computer and both work for me. You should provide repeatable example using only chocolatey Powershell modules. In any case, even if so, the question doesn't belong here as there is nothing wrong with chocolatey-core extension in this manner.

as you cannot assume the Extensions are installed if a package DOES NOT require them.

Why not using extension anyway and be done with it ?

@ferventcoder
Copy link
Contributor

BTW, internal choco helper function is semi-compatible with core function, so it was a poor choice to name it the same IMO. The differences are subtle but they exist as some packages failed just by removing core extension.

Actually the very essence of items in core.extension is that they should ultimately end up in Chocolatey, so I'm perplexed a bit at this statement.

@ferventcoder
Copy link
Contributor

ferventcoder commented Jan 23, 2018

This is correct - choco install nabtrade-securities-wiki --params="'/SEINSTALL:D:\ccviews\Program Files\securitease_test1 /NEXTPARAM:something something'" -y.

This is a bug: choco install nabtrade-securities-wiki --params="/SEINSTALL:'D:\ccviews\Program Files\securitease_test1' /NEXTPARAM:'something something'" -y

Any guesses why the second one won't work in powershell.exe? Actually it looks like it does work in cmd.exe and powershell.exe, however it doesn't follow our prescribed documentation, which was painstakingly tested in multiple shells and programs.

If you are not following the prescribed https://chocolatey.org/docs/commands-reference#how-to-pass-options-switches, you are fracturing the community and the documentation. So you have a bug on your hands. Please fix it.

@ferventcoder ferventcoder reopened this Jan 23, 2018
@ferventcoder
Copy link
Contributor

ferventcoder commented Jan 23, 2018

The differences are subtle but they exist as some packages failed just by removing core extension.

If this is true, why haven't there been more bug reports over at https://github.com/chocolatey/choco? Things between the two methods should be compatible.

Here's one we've fixed: chocolatey/choco#1459 (Urls).

The original implementation in 0.10.8 wasn't quite good enough, so we've flipped over to a regex - https://github.com/chocolatey/choco/blob/516c6cc673ebe90cc97eaa3e4d307ad88ab12752/src/chocolatey.resources/helpers/functions/Get-PackageParameters.ps1#L147 (and yes, every bit of that is required).

This likely fixes the issue of incompatibility reported here, however I don't think it would remove the ' from the values. Maybe it should if those are on the front and back.

We also support by default $env:ChocolateyPackageParameters and $env:ChocolateyPackageParametersSensitive, which this function does not yet.

@ferventcoder
Copy link
Contributor

ferventcoder commented Jan 23, 2018

Okay, I adjusted that functionality for 0.10.9, it will remove the ' from the start and end of item value. Once I get an issue created, I can create and push up a commit for that.

@majkinetor
Copy link
Contributor

majkinetor commented Feb 10, 2018

I found out why I thought the native and core funcs are not the same. Its a bug in remembering parameters:

Firefox v58.0.2 [Approved]
firefox package files upgrade completed. Performing other installation steps.
Detected the 32-bit version of Firefox on a 64-bit system. This package will continue to install the 32-bit version of Firefox unless the 32-bit version is uninstalled.
ERROR: Data line '/Service' is not in 'name=value' format.
The upgrade of firefox was NOT successful.
Error while running 'C:\ProgramData\chocolatey\lib\Firefox\tools\chocolateyInstall.ps1'.
 See log for details.
...
...
jdk8 v8.0.162 [Approved]
jdk8 package files upgrade completed. Performing other installation steps.
ERROR: Data line '/ToggleShortcut:ctrl+q' is not in 'name=value' format.
The upgrade of jdk8 was NOT successful.
Error while running 'C:\ProgramData\chocolatey\lib\jdk8\tools\chocolateyInstall.ps1'.
 See log for details.

/Service is parameter of another package (everything). It pops up on random package each time I cup all. The same is for /ToggleShortcut (copyq).

@ferventcoder
Copy link
Contributor

It's a known issue. The trick is discovering why it occurs. The time from when the actual bug occurs to when you notice is quite far apart. That bug is noted at chocolatey/choco#1443

@ferventcoder
Copy link
Contributor

chocolatey/choco#1490 for the incompatibilities addressed in this issue.

@majkinetor
Copy link
Contributor

I don't think there is anything more to add here so I will close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants