Install xdebug and xhprof as standard

[andrewnicols]

This also adds a new PHP_EXTENSION-[name]=1 environment variable optino to call docker-php-ext-enable on start.

[scara]

Hi @andrewnicols,
that's another nice feature from you! 💪

I still wonder about the ENV VAR naming; there is a docker PHP base (fat) image - not the one we're using today! - that makes use of:

• PHP_EXTENSION_[extension_name]
• PHP_EXTENSIONS

To be someway future proof should be your PHP_EXTENSION-[extension_name] renamed into PHP_EXTENSION_ENABLE-[extension_name]?

HTH,
Matteo

[andrewnicols]

Thanks Eloy,

I've renamed it to PHP_EXTENSION_[name]=1.

Currently doesn't support an = 0 option (later PR) or the pluar.

[scara]

Hi @andrewnicols,

I've renamed it to PHP_EXTENSION_[name]=1.

In https://github.com/moodlehq/moodle-php-apache/pull/173/files#diff-535707c409375def5e16a434bf94429341e9f0feb2ac1bf55094c22322b7c179R35 there is still "-".

I wonder if we can separate extension installation from extension enabling in the naming convention.

HTH,
Matteo

[andrewnicols]

I wonder if we can separate extension installation from extension enabling in the naming convention.

We do - the PHP_EXTENSION_[name]=[state] is only to enable or disable the plugin, not to install it, which is consistent wtih https://github.com/thecodingmachine/docker-images-php/tree/449290b1e175902944a4484dc1758349bf97cb97#enablingdisabling-extensions-in-the-fat-image.

[scara]

Hi @andrewnicols,

We do

I mean:

• How to install new extensions w/o enabling them, future proof i.e. not required now:
  • Single PHP extension: PHP_EXTENSION=<extension_name>
  • Multiple PHP extensions: PHP_EXTENSIONS=<extension_name>[ <extension_name>[ ...]]
• How to enable extensions:
  • Single PHP extension: PHP_EXTENSION_ENABLE-<extension_name>=<state>
  • [Opt] Multiple PHP extensions: PHP_EXTENSIONS_ENABLE=<extension_name>=<state>[ ...]]

Just an idea based on some other experiences.

HTH,
Matteo

[stronk7]

Hi,

I had this abandoned... apart from the comments from @scara (splitting install and enable), nice to have, though I don't think that's strictly needed in this case (xhprof, xdebug & pcov)... I've had a "déjà vu" here...

I thought we already had commented about that... but... what's wrong with the pcov extension? Unless I'm wrong, it's the BEST one for coverage, better (performace-wise) than the xdebug one, in fact.

Maybe you're confusing it with the phpdbg alternative that, since PHP 8.0 is practically unusable, crashing all the time?

Ciao :-)

[stronk7]

OT, while reviewing older issues and pull requests... maybe we can consider #156 (php-excimer) here to be added too? Alternative would be to ask into that PR to ensure the extension behaves like the 3 being added here (disabled by default).

[stronk7]

Hi,

can we amend the readme about the pcov / phpdbg confusion commented above? With that and considering if we also should add php-excimer... i think this is basically ready.

Ciao :-)

[andrewnicols]

Thanks for the reminder.

I'll try and get this finished over the next few days. I'd suggest maybe leaving php-excimer to a separte pull request based on this one but no objections here.

[stronk7]

I'll try and get this finished over the next few days. I'd suggest maybe leaving php-excimer to a separte pull request based on this one but no objections here.

Sup, once this lands we can point them to the way to add it, perfect!

Thanks!

[andrewnicols]

I've updated the README.md

Re @scara's suggestion to add support for installing extensions, I think you're confusing the dockerfile ARG and ENV.

The fat image you refer to is doing a number of things:

• it supports enabling and disabling extensions using:
  • PHP_EXTENSION_[NAME]=[0|1]
  • PHP_EXTENSIONS=[name [name [name]]]
• it supports installing new extensions by setting a Dockerfile ARG PHP_EXTENSIONS=[name [name [name]]] when building the image

At the moment we are only adding support for enabling extensions.
In the future we will likely add support for disabling them too but it will require a complete restructure of how we install and enable extensions so it is not high priority.

The installation of extensions at build time is much lower priority because it requires actually knowing how to install each plugin - that is any dependencies. You'll notice that the image you refer to has a library of all of the extensions it can support here: https://github.com/thecodingmachine/docker-images-php/tree/449290b1e175902944a4484dc1758349bf97cb97/extensions/8.2 and we would need to do something similar.

This patch is otherwise good to go.

[stronk7]

(sorry for the delay, I missed this one - maybe coz I was near AL... and just saw it now. Will look to it ASAP!)

[stronk7]

Hi @andrewnicols ,

I've taken the liberty of slightly amending your patch for current master (php83-beta2):

• The xmlrpc extension was removed some weeks ago (no Moodle branch supporting php83 needs it).
• Fix a typo in commit message.
• Keep php-solr not installed, because it's not available for php83 yet.

$ git range-diff  andrew/xdebug^1...andrew/xdebug  HEAD^1...HEAD
1:  38ad2e5 ! 1:  ddb1d9f Install xdebug and xhprof as standard
    @@ Metadata
      ## Commit message ##
         Install xdebug and xhprof as standard
     
    -    This also adds a new PHP_EXTENSION-[name]=1 environment variable optino
    +    This also adds a new PHP_EXTENSION-[name]=1 environment variable option
         to call docker-php-ext-enable on start.
     
      ## .github/workflows/test_buildx_and_publish.yml ##
    @@ README.md: docker run \
     +* uuid
     +* xdebug
     +* xhprof
    -+* xmlrpc-beta
     +* xsl
     +* zip
     +
    @@ README.md: docker run \
      * [moodle-docker](https://github.com/moodlehq/moodle-docker) a docker-composer based set of tools to get Moodle development running with zero configuration
     
      ## root/tmp/setup/php-extensions.sh ##
    -@@ root/tmp/setup/php-extensions.sh: docker-php-ext-enable apcu igbinary memcached pcov redis solr timezonedb uuid xm
    +@@ root/tmp/setup/php-extensions.sh: docker-php-ext-enable apcu igbinary memcached pcov redis timezonedb uuid
      
      echo 'apc.enable_cli = On' >> /usr/local/etc/php/conf.d/10-docker-php-ext-apcu.ini

I will be merging this once GHA ends. And then will backport to as many branches as possible.

Ciao :-)

[stronk7]

Done, this has been backported to all branches php >= 7.4 (images are being built right now).

Ciao :-)