Skip to content

(PUP-748) Fix the zypper package provider support for install_options.#2441

Merged
peterhuene merged 2 commits intopuppetlabs:masterfrom
peterhuene:pr/2067
Mar 19, 2014
Merged

(PUP-748) Fix the zypper package provider support for install_options.#2441
peterhuene merged 2 commits intopuppetlabs:masterfrom
peterhuene:pr/2067

Conversation

@peterhuene
Copy link
Contributor

When install_options is a hash, the options are rendered with a space.
This is causing zypper to interpret the option incorrectly when
using Kernel.exec.

For example, this fails with "unknown option '--from 1'":
Kernel.exec 'zypper', 'install', '--from 1', 'vim'

While this succeeds:
Kernel.exec 'zypper', 'install', '--from=1', 'vim'

The fix is to use the supported option syntax that uses
an equal sign instead of a space.

Also flattening the options instead of joining them so that
multiple options in a hash are not interpreted as a single option.

darix and others added 2 commits March 17, 2014 15:23
When install_options is a hash, the options are rendered with a space.
This is causing zypper to interpret the option incorrectly when
using Kernel.exec.

For example, this fails with "unknown option '--from 1'":
Kernel.exec 'zypper', 'install', '--from 1', 'vim'

While this succeeds:
Kernel.exec 'zypper', 'install', '--from=1', 'vim'

The fix is to use the supported option syntax that uses
an equal sign instead of a space.

Also flattening the options instead of joining them so that
multiple options in a hash are not interpreted as a single option.
Remove expected nil when calling execute without install_options.

Add specs for install_options as a hash, array, or string.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note to reviewers: we don't need to quote the value here because this is already handled for us in package_options.rb.

@puppetcla
Copy link

CLA signed by all contributors.

@MosesMendoza
Copy link
Contributor

👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Tiny nitpick - why += here instead of <<?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I had it that way before I refactored the options code a little. I can change it for consistency's sake.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh right, it's that way because install_options returns an array and I wanted the two arrays joined, not the options array appended. i.e. a = [1, 2, 3]; a += [4] => [1, 2, 3, 4] instead of [1, 2, 3] << [4] => [1, 2, 3, [4]]

Copy link
Contributor

Choose a reason for hiding this comment

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

GAAAH. That catches me every time. 👍

peterhuene added a commit that referenced this pull request Mar 19, 2014
(PUP-748) Fix the zypper package provider support for install_options.
@peterhuene peterhuene merged commit 81dace7 into puppetlabs:master Mar 19, 2014
@peterhuene peterhuene deleted the pr/2067 branch March 19, 2014 21:55
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

Successfully merging this pull request may close these issues.

5 participants