feat(composer): install PECL extension #286

EtienneM · 2022-12-27T16:22:26Z

I went through the PECL download statistics (https://pecl.php.net/package-stats.php) and tested a bunch of the packages from there. The PHP info is here: https://biniou.osc-fr1.scalingo.io/. Some of these packages are really old so I didn't tested them. I tried to find some packages that are still maintained:

Swoole (https://pecl.php.net/package/swoole): works easily
YAML (https://pecl.php.net/package/yaml): also needs the APT buildpack to install libyaml-dev
TimezoneDB (https://pecl.php.net/package/timezonedb): works easily
amqp (https://pecl.php.net/package/amqp): also needs the APT buildpack to install librabbitmq-dev

Fix #249

EtienneM · 2022-12-28T12:51:26Z

lib/composer

+              install_pecl_extension $ext "$ext_version" "$CACHE_DIR" | indent
            fi
+
+            # TODO I don't think this instruction should be executed for all extensions


I still have a question about this instruction. But the question also stands for the script version on master.. Do you know its purpose?

I think this is more a question for @Soulou right ?

We should try to remove it and see if all extensions we're officially supporting in our doc are building as expected.

I assume that some extensions will fail, since it requires external libs that we are packaging.

My guts feeling on this is that:

All extensions which are not pre-packaged by us currently which do not require another external library should enter this new standard process

All extensions which are installed automatically should keep being maintained.

But installing redis and mongodb doesn't really make sense it's not used that often, I would stop installing them automatically

APCU is a automatic code compilation caching optimization, we should keep installing it by default.

All extensions which are pre-packaged by us should keep using fetch_package, so we need to identify them:

[ KEEP] https://github.com/Scalingo/php-buildpack/blob/master/support/ext/amqp → PECL extension requiring external lib, we must keep supporting it

[KEEP] https://github.com/Scalingo/php-buildpack/blob/master/support/ext/apcu → PECL only, but should be automatically installed, so let's keep it fast by prebuilding it

[DROP] https://github.com/Scalingo/php-buildpack/blob/master/support/ext/apfd → PECL only, drop pre-packaging

[KEEP] https://github.com/Scalingo/php-buildpack/blob/master/support/ext/blackfire → Third party lib, we need to keep it

[DROP] https://github.com/Scalingo/php-buildpack/blob/master/support/ext/ds → PECL only, drop pre-packaging

[KEEP] https://github.com/Scalingo/php-buildpack/blob/master/support/ext/event → Requiring webp lib

[KEEP] https://github.com/Scalingo/php-buildpack/blob/master/support/ext/igbinary → Non PECL

[DROP] https://github.com/Scalingo/php-buildpack/blob/master/support/ext/imagick → PECL only

[KEEP] https://github.com/Scalingo/php-buildpack/blob/master/support/ext/lua → Requiring installation of LUA

[DROP] https://github.com/Scalingo/php-buildpack/blob/master/support/ext/mcrypt → PECL only

[KEEP] https://github.com/Scalingo/php-buildpack/blob/master/support/ext/memcached → External lib

[KEEP] https://github.com/Scalingo/php-buildpack/blob/master/support/ext/mongodb → External lib

[KEEP] https://github.com/Scalingo/php-buildpack/blob/master/support/ext/redis → External lib

@EtienneM is my analysis clear, does it match your point of view?

EtienneM · 2023-01-06T15:49:35Z

lib/pecl

+
+  pushd ${extension_name}-${version} > /dev/null
+  /app/vendor/php/bin/phpize > /dev/null
+  ./configure --with-php-config=/app/vendor/php/bin/php-config --with-${extension_name}=${build_dir}/.apt/usr > /dev/null


The problem is that the option --with-${extension_name} is not always useful, or may be named differently. How could we make this configurable? Via an environment variable?

I think there's more: some extensions use the extension=whatever.so directive while some others use the zend_extension=whatever.so.

For now I planned to use a (rather uglyish) case statement to handle all this. But I wanted to discuss it also.

That was before the decision to first have infosec have a look at it.

There is no way to know if an extension need the zen_extension or the extension directive from PECL (i.e. xdebug needs a zen_extension directive but nothing is mentionned on the pecl repository). I think defining env variables to configure the --with-${extension_name} and the extension / zen_extension directives could be a solution to this

Frzk · 2023-01-06T16:19:59Z

The fact that some extensions are really old has been raised in the original issue and it was agreed that we wouldn't care about it.

Yet, infosec was supposed to review all of them and give us a go.

Frzk · 2023-01-06T16:23:21Z

We should also handle the case where there is something like this in the composer.json file:

{
  "require": {
    "ext-mongodb": "*",
    "ext-imagick": "*",
    ...
  }
}

The * meaning latest which is available here: https://pecl.php.net/get/$extension_name

Frzk · 2023-01-06T16:25:42Z

FYI, support for the amqp extension has been added a few months ago through this script: https://github.com/Scalingo/php-buildpack/blob/master/support/ext/amqp

We'll have to decide if we rely on the apt-buildpack or on custom instructions to handle the dependencies of such extensions :)

lib/pecl

…ibaio installed via apt buildpack libaio and oracle-client must be cached to avoid download at each deploy

…ng oci8 extensions

feat(oci8): support `oci8` extensions

… PECL

Soulou · 2023-01-27T14:07:06Z

└> git push bp master  
Enumerating objects: 5, done.
Counting objects: 100% (5/5), done.
Delta compression using up to 8 threads
Compressing objects: 100% (3/3), done.
Writing objects: 100% (3/3), 1.08 KiB | 1.08 MiB/s, done.
Total 3 (delta 1), reused 0 (delta 0), pack-reused 0

Your deployment has been queued and is going to start…
<-- Start deployment of test-bp-pecl -->
Fetching source code
Fetching deployment cache
-----> Cloning custom buildpack: https://github.com/Scalingo/php-buildpack#fix/249/pecl_extensions
-----> Bundling Nginx 1.22.1
Checksums match. Fetching from cache.
-----> Bundling PHP 8.1.13
Checksums match. Fetching from cache.
-----> Bundling platform default extensions
apcu
Checksums match. Fetching from cache.
phpredis
Checksums match. Fetching from cache.
mongodb
Checksums match. Fetching from cache.
-----> Vendoring Composer 2.5.1
Checksums match. Fetching Composer from cache.
-----> Bundling additional extensions
Installing PHP extension: amqp
Installing PHP extension: apfd
Installing PHP extension: calendar
Installing PECL extension ds version 1.4.0
Installing PHP extension: event
Installing PHP extension: ftp
Installing PHP extension: gmp
Installing PHP extension: igbinary
Installing PHP extension: imagick
Installing PHP extension: imap
Installing PHP extension: memcached
Installing dependencies for oci8: libaio-dev
Installing PECL extension oci8 version 3.2.1
Checksums match. Fetching from cache.
Installing PHP extension: sodium
Checksums match. Fetching from cache.
Installing PHP extension: tidy
-----> Installing application dependencies with Composer
No composer.lock file present. Updating dependencies to latest instead of installing from lock file. See https://getcomposer.org/install for more information.
Loading composer repositories with package information
Updating dependencies
Nothing to modify in lock file
Writing lock file
Installing dependencies from lock file
Nothing to install, update or remove
Generating optimized autoload files
-----> Setting up default configuration
-----> Vendoring binaries into slug

-----> cron.json detected, creating cron tasks
 Build complete, shipping your container...
 Waiting for your application to boot... 
<-- [URL] -->

Soulou · 2023-01-27T14:08:32Z

Soulou · 2023-01-27T14:48:02Z

Tested with

scalingo-22 PHP 8.1 / 8.2
scalingo-20 PHP 7.4/8.0/8.1/8.2

All with success

SCedricThomas

If it is working with OCI8 LGTM

EtienneM

If I understand correctly, we have some kind of extra support for oci8 as it has been request by a CST customer, am I right? Other people would need to use the apt buildpack?

lib/composer

lib/apt

lib/composer

lib/apt

lib/pecl_oci8

EtienneM · 2023-01-30T14:03:44Z

lib/package

+    fetch_package "$base_url" "${engine}-${version}" "$location"
+}
+
+function has_package() {


best practice: I'm not sure about what this function does. Maybe add a comment or rename it so that it's clearer? If I understand correctly it makes a HEAD request to our object storage to check if the given package exists, which means that we pre-compiled the extension, isn't it?

lib/apt

guillaume-sc · 2023-01-30T14:31:10Z

If I understand correctly, we have some kind of extra support for oci8 as it has been request by a CST customer, am I right? Other people would need to use the apt buildpack?

At the time of changing our approach from allow-listing specific extension, to support all extensions from PECL, our Product Management identified a customer lead requiring the availability of OCI8 PHP extension. The work was done to achieve both supporting PECL extensions, and ensuring the specific OCI8 extension would work. I currently lack the knowledge to comment whether this could have been achieved by using the apt buildpack, or not.

lib/apt

lib/package

lib/pecl

lib/package

Soulou · 2023-02-02T08:40:33Z

If I understand correctly, we have some kind of extra support for oci8 as it has been request by a CST customer, am I right? Other people would need to use the apt buildpack?

Yes you're right

Frzk

LGTM for a first version.
(some comments still need to be addressed IMHO :))

Co-authored-by: Étienne M. <EtienneM@users.noreply.github.com>

Soulou · 2023-02-03T13:20:25Z

@Frzk you're right, I missed some from Étienne :-D

Soulou · 2023-01-13T09:09:40Z

lib/composer

+              install_pecl_extension $ext "$ext_version" "$CACHE_DIR" | indent
            fi
+
+            # TODO I don't think this instruction should be executed for all extensions


We should try to remove it and see if all extensions we're officially supporting in our doc are building as expected.

I assume that some extensions will fail, since it requires external libs that we are packaging.

My guts feeling on this is that:

All extensions which are not pre-packaged by us currently which do not require another external library should enter this new standard process

All extensions which are installed automatically should keep being maintained.

But installing redis and mongodb doesn't really make sense it's not used that often, I would stop installing them automatically

APCU is a automatic code compilation caching optimization, we should keep installing it by default.

All extensions which are pre-packaged by us should keep using fetch_package, so we need to identify them:

[ KEEP] https://github.com/Scalingo/php-buildpack/blob/master/support/ext/amqp → PECL extension requiring external lib, we must keep supporting it

[KEEP] https://github.com/Scalingo/php-buildpack/blob/master/support/ext/apcu → PECL only, but should be automatically installed, so let's keep it fast by prebuilding it

[DROP] https://github.com/Scalingo/php-buildpack/blob/master/support/ext/apfd → PECL only, drop pre-packaging

[KEEP] https://github.com/Scalingo/php-buildpack/blob/master/support/ext/blackfire → Third party lib, we need to keep it

[DROP] https://github.com/Scalingo/php-buildpack/blob/master/support/ext/ds → PECL only, drop pre-packaging

[KEEP] https://github.com/Scalingo/php-buildpack/blob/master/support/ext/event → Requiring webp lib

[KEEP] https://github.com/Scalingo/php-buildpack/blob/master/support/ext/igbinary → Non PECL

[DROP] https://github.com/Scalingo/php-buildpack/blob/master/support/ext/imagick → PECL only

[KEEP] https://github.com/Scalingo/php-buildpack/blob/master/support/ext/lua → Requiring installation of LUA

[DROP] https://github.com/Scalingo/php-buildpack/blob/master/support/ext/mcrypt → PECL only

[KEEP] https://github.com/Scalingo/php-buildpack/blob/master/support/ext/memcached → External lib

[KEEP] https://github.com/Scalingo/php-buildpack/blob/master/support/ext/mongodb → External lib

[KEEP] https://github.com/Scalingo/php-buildpack/blob/master/support/ext/redis → External lib

@EtienneM is my analysis clear, does it match your point of view?

lib/composer

lib/apt

lib/composer

lib/apt

lib/pecl_oci8

EtienneM self-assigned this Dec 27, 2022

EtienneM force-pushed the fix/249/pecl_extensions branch 24 times, most recently from a0a3461 to 34e3873 Compare December 28, 2022 12:49

EtienneM commented Dec 28, 2022

View reviewed changes

EtienneM commented Jan 6, 2023

View reviewed changes

feat(oci8); oci8 is now available, some code should be 'cleaned'

db3894b

QuentinEscudierScalingo reviewed Jan 24, 2023

View reviewed changes

lib/pecl Outdated Show resolved Hide resolved

QuentinEscudierScalingo and others added 8 commits January 25, 2023 10:38

fix(oci8): oracle client installed in /app/vendor/oracle-client and l…

1ee469d

…ibaio installed via apt buildpack libaio and oracle-client must be cached to avoid download at each deploy

fix(oci8): clean code

7a5a59c

fix(oci8): set -e in compile

2e8f674

Correctly export LD_LIBRARY_PATH from sub-APT buildpack when installi…

336de5f

…ng oci8 extensions

Rever 'set -e'

b6b2566

Fix cache directory in pecl installation

190e897

Merge pull request #291 from Scalingo/feat/290/support_oci8_extension

d253f4d

feat(oci8): support `oci8` extensions

Polish codebase, make code more generic between internal packages and…

83b3b1d

… PECL

Soulou requested a review from SCedricThomas January 27, 2023 14:07

Soulou marked this pull request as ready for review January 27, 2023 14:48

SCedricThomas approved these changes Jan 30, 2023

View reviewed changes

EtienneM commented Jan 30, 2023

View reviewed changes

Frzk reviewed Jan 30, 2023

View reviewed changes

Review + readme

7903a16

Soulou requested a review from Frzk February 2, 2023 09:12

Frzk approved these changes Feb 2, 2023

View reviewed changes

Update lib/apt - better logging

e49d20a

Co-authored-by: Étienne M. <EtienneM@users.noreply.github.com>

Soulou approved these changes Feb 3, 2023

View reviewed changes

Soulou merged commit 3a3283d into master Feb 3, 2023

Soulou deleted the fix/249/pecl_extensions branch February 3, 2023 13:30

This was referenced Sep 21, 2023

Add swoole extension #181

Closed

Extensions cleanup #363

Open

feat(composer): install PECL extension #286

feat(composer): install PECL extension #286

Uh oh!

Conversation

EtienneM commented Dec 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Frzk commented Jan 6, 2023

Uh oh!

Frzk commented Jan 6, 2023

Uh oh!

Frzk commented Jan 6, 2023

Uh oh!

Uh oh!

Soulou commented Jan 27, 2023

Uh oh!

Soulou commented Jan 27, 2023

Uh oh!

Soulou commented Jan 27, 2023

Uh oh!

SCedricThomas left a comment

Choose a reason for hiding this comment

Uh oh!

EtienneM left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

guillaume-sc commented Jan 30, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Soulou commented Feb 2, 2023

Uh oh!

Frzk left a comment

Choose a reason for hiding this comment

Uh oh!

Soulou commented Feb 3, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

EtienneM commented Dec 27, 2022 •

edited

Loading

EtienneM left a comment •

edited

Loading