8000 (#1185) Always pass in hook scripts event if empty by corbob · Pull Request #2834 · chocolatey/choco · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

(#1185) Always pass in hook scripts event if empty #2834

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Sep 27, 2022
Merged

(#1185) Always pass in hook scripts event if empty #2834

merged 1 commit into from
Sep 27, 2022

Conversation

corbob
Copy link
Member
@corbob corbob commented Sep 27, 2022

Description Of Changes

Always pass the hook parameters to chocolateyScriptRunner even if we're just passing them $null

Motivation and Context

There is a manual test that we have where we try to install a package thusly: choco install test-params --package-parameters="/NoDesktop 'Not Displayed' /InstallDir:'C:\tools\package destination'" this was resulting in the following in the log: 2022-09-26 16:08:41,822 7016 [DEBUG] - Running 'ChocolateyScriptRunner' for test-params v0.0.0 with packageScript 'C:\ProgramData\chocolatey\lib\test-params\tools\chocolateyinstall.ps1', packageFolder:'C:\ProgramData\chocolatey\lib\test-params', installArguments: '', packageParameters: '/NoDesktop ', preRunHookScripts: 'Not', postRunHookScripts: 'Displayed /InstallDir:C:\tools\package', which is clearly not correct.

Testing

  1. I have run this through the debugger with the parameters install test-params --package-parameters="/NoDesktop 'Not Displayed' /InstallDir:'C:\tools\package destination'"
  2. Confirmed that it installed as I expected it to.

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)
  • PowerShell code changes.

Related Issue

Fixes #1185

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.
  • PowerShell v2 compatibility checked.

@corbob corbob requested a review from AdmiringWorm September 27, 2022 00:05
@coveralls
Copy link
coveralls commented Sep 27, 2022 •
edited
Loading

Coverage Status

Coverage increased (+0.002%) to 27.635% when pulling 3ac2b5f on corbob:a1185-fix-hooks into b7934fd on chocolatey:develop.

@corbob @AdmiringWorm
(chocolatey#1185) Always pass in hook scripts event if empty
3ac2b5f 
When passing package parameters into chocolateyScriptRunner.ps1, if
there are quotes, PowerShell can get confused. This ensures that we're
specifying the new hook arguments and passing nothing if there should be
nothing.
AdmiringWorm
AdmiringWorm previously approved these changes Sep 27, 2022
View reviewed changes
Copy link
Member
@AdmiringWorm AdmiringWorm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, and I can confirm that this works as expected. Oddly enough, the problem you noticed does not happen in Debug builds, and only when a release artifact is created.

@AdmiringWorm AdmiringWorm merged commit 44c5283 into chocolatey:develop Sep 27, 2022
@corbob corbob deleted the a1185-fix-hooks branch September 27, 2022 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AdmiringWorm AdmiringWorm AdmiringWorm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow post/pre hooks to run on upgrade/install/uninstall
3 participants
@corbob @coveralls @AdmiringWorm
0