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

only install curl if it isn't defined elsewhere #39

Merged
merged 1 commit into from May 13, 2014
Merged

only install curl if it isn't defined elsewhere #39

merged 1 commit into from May 13, 2014

Conversation

ghost
Copy link

@ghost ghost commented May 13, 2014

If ensure_package() is used with a conventional resource declaration
such as package { 'curl': ensure => present, }, it could result in
a duplicate resource error. This commit ensures that curl is not
declared if it's already defined elsewhere, such as a site-wide packages
class (this is fairly common behavior).

Considering that $install_dependencies defaults to false in init.pp,
this commit/PR probably satisfies most of the use cases. But you might
want to consider the use of ensure_package() in the rest of the
dependencies classes.

If ensure_package() is used with a conventional resource declaration
such as `package { 'curl': ensure => present, }`, it could result in
a duplicate resource error. This commit ensures that curl is not
declared if it's already defined elsewhere, such as a site-wide packages
class.
@carlossg
Copy link
Member

if you have package { 'curl': ensure => present, } then ensure_packages will not install curl. Only in the case that your package definition has more attributes

@ghost
Copy link
Author

ghost commented May 13, 2014

Maybe my message / use case wasn't clear enough then; in the interest of brevity I just included a quick in-line case, but you're correct. As soon as you add any other parameters...

package { 'puppet':
  ensure => installed,
  alias  => 'curl',
}

... applying maestrodev/rvm will result in a duplicate resource error. Considering $install_dependencies defaults to false, I wouldn't expect this module to attempt to install any packages. But since curl is necessary to get rvm downloaded and installed, it's clearly needed here, and removing it completely would hurt users more than it would help.

The nicest way to work around all that is to just wrap it in a if ! defined(), as I've done in this PR. This wouldn't affect existing usage of the module, and would allow users with curl defined elsewhere to apply maestrodev/rvm without errors. It worked for me anyway.

carlossg added a commit that referenced this pull request May 13, 2014
only install curl if it isn't defined elsewhere
@carlossg carlossg merged commit 5a4ef08 into voxpupuli:maestrodev May 13, 2014
@carlossg
Copy link
Member

yes, that's a known issue with ensure_packages / ensure_resources, the hash needs to match exactly with ensure => present so it is not redefined

@ghost ghost deleted the fix-curl-resource branch May 13, 2014 17:35
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.

None yet

1 participant