tPl0ch / puppet-composer

A very flexible puppet module to install and use the composer PHP dependency manager.
http://forge.puppetlabs.com/tPl0ch/composer
MIT License
25 stars 60 forks source link

Add support for managing dependencies elsewhere #37

Open lieutdan13 opened 10 years ago

lieutdan13 commented 10 years ago

I'm running into an issue with package dependencies, duplicate declarations, and declaration ordering, which is apparently a bug in Puppet itself. More specifically the issue is with your Package[$php_package] declaration in the composer class.

What I would like is for the class to completely ignore the $php_package parameter which would allow me to manage it elsewhere. I would like to propose a solution, but I'm not sure what your coding conventions are for this type of thing.

Proposed solution:

class { 'composer': php_package => '' }

class composer(...) inherits composer::params {
    ...
    if $php_package != '' and defined(Package[$php_package]) == false {
        package { $php_package: ensure => present, }
    }
    ...
}
lieutdan13 commented 10 years ago

Ok. I tried my solution and it turns out that the $php_package parameter is used elsewhere inside the class. That was an oversight... I'll have to think about an alternate solution, without having to add more parameters tot he class.

andyshinn commented 10 years ago

Yea, there will need to be some cleanup to make this happen. It is possible. But there are a couple of ways to solve it.

One way would be for us to conditionally require an external PHP module (like https://github.com/thias/puppet-php). Then we could remove references to Package[$php_package].

Another would be the route you suggested, conditionally include the Package[$php_package] resource if $php_package is not undef.

Is there a specific Puppet PHP module you are using or do you just declare package { 'php5-cli': } yourself somewhere?

minorOffense commented 9 years ago

Actually, you might be able to use ensure_packages from the puppet labs stdlib. We just discovered that a few weeks ago and have since moved most of our package dependencies to use that. Helps avoid conflicts.

https://forge.puppetlabs.com/puppetlabs/stdlib

tPl0ch commented 9 years ago

@minorOffense Looks like a good solution to me. Thanks for pointing that out!

tPl0ch commented 9 years ago

@andyshinn I am currently adding some more little features like dump-autoload, clear-cache and run-script. Together with this change I'd say a 2.0 release is there.

andyshinn commented 9 years ago

Sounds good. I've already tested the fix is simple enough with ensure_packages($php_package, { ensure => present })