sous-chefs / php

Development repository for the php cookbook
https://supermarket.chef.io/cookbooks/php
Apache License 2.0
443 stars 491 forks source link

shell_out() without { default_env: false } param causes pecl install to fail #256

Open followfung opened 5 years ago

followfung commented 5 years ago

:ghost: (Not A) Brief Description

When trying to install PHP7.2 from SCL on CentOS 7, installing a pecl package (imagick for example) using the php_pear resource fails because the dependencies cannot be found even when they are installed. But SSH-ing into the machine and running pecl install imagick via command line, runs without issue.

I think I've narrowed down the problem to the usage of shell_out() and how it handles the PATH environment variable. Calling it without the { default_env: false } flag causes the chef-client to inject /opt/chef/embedded/bin to the beginning of the $PATH so that when command runs, it picks up the pkg-config embedded in Chef instead of the one installed on the system.

Calling shell_out() without the flag: checking for pkg-config... /opt/chef/embedded/bin/pkg-config

Calling shell_out() with the flag: checking for pkg-config... /usr/bin/pkg-config

I can't figure out why it works without problems when using the default php version installed by CentOS, but either way I think that php_pear should explicitly tell chef-client to not muck with $PATH for consistency. Even the execute resource uses this flag to have the output match what you would see if you were to run it manually via command line (See: https://github.com/chef/chef/blob/master/lib/chef/resource/execute.rb#L72).

I'm not sure if this would break for other platforms, but it seems to work fine when updating calls to shell_out() to include this flag. Example below:

def pear_shell_out(command)
  p = shell_out!(command, { default_env: false })
  # pear/pecl commands return a 0 on failures...we'll grep for it
  p.invalid! if p.stdout.split('\n').last =~ /^ERROR:.+/i
  p
end

For reference:

:pancakes: Cookbook version

7.0.0

:woman_cook: Chef-Infra Version

Chef 14.2+

EDIT (2020-02-20):

I'm not sure how to comment further so I'm making an edit here...

Sorry, I wasn't clear. The default_env flag is true by default in Chef 14. The fix that I suggested is to modify the pear_shell_out() method (https://github.com/sous-chefs/php/blob/master/resources/pear.rb#L187) to call shell_out()! with { default_env: false } flag.

The problem only occurs when using a PHP version provided in SCL (e.g.: CentOS 7 running PHP7.2 from SCL). In order to install PHP7.2 I've overridden a lot of the default attributes.

Overrides for reference:

override['php']['packages'] = %w(rh-php72 rh-php72-php-devel rh-php72-php-json rh-php72-php-ldap rh-php72-php-mbstring rh-php72-php-mysqlnd rh-php72-php-opcache rh-php72-php-pdo rh-php72-php-xml)
override['php']['pear'] = '/opt/rh/rh-php72/root/bin/pear'
override['php']['conf_dir'] = '/etc/opt/rh/rh-php72'
override['php']['ext_conf_dir'] = '/etc/opt/rh/rh-php72/php.d'
override['php']['ext_dir'] = '/opt/rh/rh-php72/root/usr/lib64/php/modules'
override['php']['fpm_package'] = 'rh-php72-php-fpm'
override['php']['fpm_pooldir'] = '/etc/opt/rh/rh-php72/php-fpm.d'
override['php']['fpm_default_conf'] = '/etc/opt/rh/rh-php72/php-fpm.d/www.conf'
override['php']['fpm_service'] = 'rh-php72-php-fpm'

To be clear, when using the default PHP that is provided by CentOS 7 (i.e.: without using SCL), there are no issues. I've tested the fix and it works fine for my use case, but I'm not sure if it will break on other platforms.

damacus commented 4 years ago

I do wonder if this is a simple as default_env: true here instead? If you fancy giving that a go be our guest. And please do shout up if you need any help in debugging this