search

thias / puppet-php

Puppet module to manage PHP
Other
49 stars 67 forks source link

Automatically detect PHP extension directory #4

Open ddeboer opened 11 years ago

ddeboer commented 11 years ago

When adding, for instance, the XDebug module, one currently has to write:

php::module::ini { 'pecl-xdebug':
  zend => '/usr/lib64/php/modules',
}

With this PR it becomes easier to include Zend extensions like XDebug:

php::module::ini { 'pecl-xdebug':
  zend => true,
}

In this case, the PHP extension directory is automatically detected through a Puppet fact. If you need to override the module directory, you can still do so by supplying a string for the zend property:

php::module::ini { 'pecl-xdebug':
  zend => '/some/other/php/modules/dir',
}
thias commented 11 years ago

This is an interesting idea. Unfortunately, the php-config command is provided by the php-devel package on RHEL, meaning that many servers won't have it available (mine don't, I keep devel packages off of production systems).

And the default you set if the fact isn't present is 64bit specific, which is something best avoided.

How would you make the fact more robust? Fall back to iterating through known directories until one is found, when the php-config command is unavailable?

ddeboer commented 11 years ago

Good points. I made the fact architecture-aware and independent from php-devel.

Of course, the most reliable output is from php -i itself, but that will not be available in the Puppet run during which PHP is installed (only in subsequent runs). So I guess we do need to iterate through a set of known directories. Any suggestions as to which directories they should be?

thias commented 10 years ago

Sorry for taking soooooooooooo long. I'm still not entirely convinced about this change, but I would be okay to include it with a minor clean up : There currently are two separate fallbacks, one in the fact and the other in params.pp. The one in params.pp is uglier since it harcodes the lib64 path, so it will be incorrect on 32bit systems. The one is the fact is less useful, as it's nice to have a fallback if pluginsync is disabled (no fact)... so... if you can clean that up, I'll include the change.

I think just removing the fallback in params.pp would be fine, since people passing true will be doing so explicitly.