boxen / puppet-php

PHP ALL THE BOXEN
boxen.github.com
MIT License
11 stars 29 forks source link

Allow for PHP installation on mac Sierra. #97

Closed typhonius closed 7 years ago

typhonius commented 7 years ago

Follows up on #96 and allows PHP installation on Sierra. There are a few changes in here which might need to be reverted from previous commits to a more global default (such as timezone and memory limits) but that can be discussed in this issue.

typhonius commented 7 years ago

From the php.ini values within this PR, I'm thinking the following should be changed:

I'm undecided on the rest of the php.ini changes but anything can be overridden in custom manifests with something akin to the following:

  file { "${php::config::configdir}/${version}/conf.d/memory_limit.ini":
    content => '[Memory Limit Override]
memory_limit = 2G',
    require => Php::Version[$version],
  }
jacobbednarz commented 7 years ago

Agree @typhonius - we need saner defaults. I'm +1 to all the recommendations you've mentioned above.

We could always turn the php.ini file into a template file and have it configured via php::config.

typhonius commented 7 years ago

I've made changes to this PR based on the discussion. In a lot of instances I've used the default values recommended in php.ini as this would be what users would expect. I would agree that an ideal state would see php.ini become completely templatised and parameters able to be added from user's manifests. Until such time though, configuration can be overridden as detailed above.

jacobbednarz commented 7 years ago

Once CI is green, I'm :+1:

typhonius commented 7 years ago

I managed to get the tests to work locally but I had to remove the following. Is there a way to make the boxen::env_script resource type available within this module or is that generally not recommended.

  # Add composer global bin to $PATH
  boxen::env_script { 'composer':
    source => 'puppet:///modules/php/composer_global_bin.sh',
    require => Exec['download-php-composer']
  }
typhonius commented 7 years ago

@jacobbednarz I can't seem to get ruby 1.8.7 to install on my Sierra laptop. Are we still supporting this 1.8.7 or shall we bump the travis requirements. Noting as an aside that tests pass on 2.0.0 and 1.9.3 locally.

jacobbednarz commented 7 years ago

1.8.7 is fine to go in the bin. Details in boxen/boxen#204