Skip to content
This repository has been archived by the owner on May 16, 2018. It is now read-only.

[jessie] Look into php extensions that fail #1164

Merged
merged 15 commits into from
Feb 23, 2016
Merged

Conversation

ppp0
Copy link
Member

@ppp0 ppp0 commented Feb 11, 2016

Keeping the same setup (manually build 3.1.2) brought me some problems because of the build-depend libmagickwand whose jessie version breaks the build. We could patch the configure script to point to the correct location of MagickWand-config which before was to be found in /usr/bin - see rflynn/imgmin#51 and /usr/share/doc/imagemagick/NEWS.Debian.gz:

imagemagick (8:6.8.9.9-1) unstable; urgency=high

  Obsolete config scripts (Magick-config, MagickCore-config,
  MagickWand-config, Wand-config and Magick++-config) are
  not multi-arch safe and thus have been removed from /usr/bin.
  .
  Moreover, these scripts are superseded by pkg-config
  facilities.
  .
  However as a courtesy to our users, these scripts have
  been moved to
  /usr/lib/$DEB_HOST_MULTIARCH/ImageMagick-$VERSION/bin-$QUANTUMDEPTH/
  where $DEB_HOST_MULTIARCH is the multi-arch triplet and
  $QUANTUMDEPTH is the current quantum depth. $VERSION is the upstream
  version without the revision number.
  .
  Please note that these scripts will be definitively removed after
  jessie.

 -- Bastien Roucariès <[email protected]>  Sun, 19 Oct 2014 12:12:20 +0200

I would install 3.2.0-RC1 as per Jessie repository and test if we need to upgrade to 3.3.0

@cargomedia/devops comments welcome

@njam
Copy link
Member

njam commented Feb 11, 2016

I've built and installed version 3.2.0RC1 in a wheezy box and can confirm that CM_Image_ImageTest succeeds - so most probably it will work fine!

@ppp0
Copy link
Member Author

ppp0 commented Feb 11, 2016

@njam please review

@njam
Copy link
Member

njam commented Feb 11, 2016

Install config in both cases, so we can add configuration if needed?

@ppp0
Copy link
Member Author

ppp0 commented Feb 11, 2016

So I reintroduced this config and things started to irk
Will fix next week, this whole php5 config thing is flawed

@ppp0 ppp0 changed the title [jessie] Change php extension ImageMagick installation [jessie] Look into php extensions that fail Feb 17, 2016
@ppp0
Copy link
Member Author

ppp0 commented Feb 18, 2016

php5::extension::apc not needed anymore after #1166

@ppp0
Copy link
Member Author

ppp0 commented Feb 18, 2016

php5::extension::opcache not needed in jessie (built-in in php 5.6)

php -v
PHP 5.6.17-0+deb8u1 (cli) (built: Jan 13 2016 09:10:12) 
Copyright (c) 1997-2015 The PHP Group
Zend Engine v2.6.0, Copyright (c) 1998-2015 Zend Technologies
    with Zend OPcache v7.0.6-dev, Copyright (c) 1999-2015, by Zend Technologies

…fig for jessie (extension is now built-in)
@ppp0
Copy link
Member Author

ppp0 commented Feb 18, 2016

some changes here:

  • apc was only needed by revive (depends on Migrate revive to nginx #1166)
  • opcache is a built-in in php for jessie (v5.6)
  • opcache configurabiity
  • bipbip plugin is named apc therefore php5::fpm::with_apc not renamed (still needed for apcu)

@njam please review

@njam
Copy link
Member

njam commented Feb 18, 2016

  • extension/imagick.pp: What is the reason to have this ini_content param?
  • extension/opcache.pp: Looks a bit strange that we guard the installation with lsbmajdistrelease. I think in general we should rather use lsbdistcodename (or lsbdistcodename and lsbmajdistrelease) for it to be unique. For this specific case maybe it's best to add a fact that returns the PHP version?
  • php5/manifests/fpm.pp: What remove config_extension_change - the purpose is to restart php-fpm when the configuration for an extension changes. I think we should still have it?

@ppp0
Copy link
Member Author

ppp0 commented Feb 22, 2016

  • I thought it would make sense to be able to modify imagick's ini file - but it doesn't, it's enough to manage it
  • sgmt
  • In my understanding, this was some sort of debug class unless there's more magic behind it:
class php5::config_extension_change {

  exec { 'echo Triggering refresh because php extensions configuration changed':
    provider    => shell,
    refreshonly => true,
  }
}

@njam
Copy link
Member

njam commented Feb 22, 2016

php5::config_extension_change - the purpose is to restart php-fpm when the configuration for an extension changes! (via notify and subscribe)

@ppp0
Copy link
Member Author

ppp0 commented Feb 22, 2016

uh oh, I tried to introduce a php_version fact which would return a hash such as

{"release" =>"5.6", "full"=>"PHP 5.6.17-0+deb8u1 (cli) (built: Jan 13 2016 09:10:12)"}

The problem is that before php is installed there is no way to get its version by issuing php- v

@ppp0
Copy link
Member Author

ppp0 commented Feb 22, 2016

Alright, ci passes, changes applied

@njam please review

@njam
Copy link
Member

njam commented Feb 22, 2016

  • manifests/extension/imagick.pp: The extension should be installed before the config (to make sure we only restart php-fpm once the extensions is available)
  • extension/opcache.pp: use "codename"
  • manifests/fpm.pp: Keep subscribing to "change"
  • php5/manifests/init.pp: Use "codename"
  • php5/manifests/init.pp: Why include opcache here?

@ppp0
Copy link
Member Author

ppp0 commented Feb 22, 2016

@njam another review, please

@njam
Copy link
Member

njam commented Feb 22, 2016

lgtm

But depends on #1166

ppp0 added a commit that referenced this pull request Feb 23, 2016
[jessie] Look into php extensions that fail
@ppp0 ppp0 merged commit 6117a6a into cargomedia:master Feb 23, 2016
@ppp0 ppp0 removed the in progress label Feb 23, 2016
@ppp0 ppp0 deleted the issue-1164 branch August 23, 2017 12:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants