moodlehq / moodle-php-apache

PHP + Apache docker images for Moodle development
61 stars 68 forks source link

Add simple environment-based PHP configuration #167

Closed andrewnicols closed 1 year ago

andrewnicols commented 1 year ago

This adds a layer of common sense on top of the other changes I've worked on to allow easy configuration variable setting.

This is currently based on top of #162 and #163 together, but can be refactored easily.

The tl;dr is:

docker run \
    --name web0 \
    -p 8080:80 \
    -v $PWD/moodle:/var/www/html
    -e INI-upload_max_filesize=200M \
    -e INI-post_max_size=210M \
    moodle-php-apache:latest

Will cause a new php.ini file to be created at `/usr/local/etc/php/conf.d/10-local.ini containing:

upload_max_filesize = 200M
post_max_size = 210M
andrewnicols commented 1 year ago

Note: This is intended to co-exist with the entrypoint changes - the rationale being that we should make it as easy as possible to make small changes, but still allow larger ones.

Adding an environment variable is very easy. Specifying an entrypoint directory with appropriate .ini configuration is significantly harder.

They are not mutually exclusive, and both are optional. We can provide neither, either, or both.

scara commented 1 year ago

Hi @andrewnicols, this is really a nice feature! 💪

My trivial comment: why the INI- prefix? At first glance I would use PHP_INI- (or PHP_INI.) to better cope with the domain of those ENV VAR settings.

The official image is actually using PHP_INI_* (e.g. PHP_INI_DIR) too to provide read-only info.

HTH, Matteo

andrewnicols commented 1 year ago

I'm happy to go with either if anyone has a preference.

stronk7 commented 1 year ago

Fun, I was to comment also about the "INI" prefix when saw @scara already did.

My point was more about future expansions we could do, like adding stuff to apache conf or git conf... or whatever.

In any case, for me PHP_INI/php_ini looks perfect.

andrewnicols commented 1 year ago

I've updated the patch to change it to use PHP_INI instead.

stronk7 commented 1 year ago

Sold, thanks!

I'll be backporting this to php >= 74 images (like entrypoints @ #162).

stronk7 commented 1 year ago

Done, all the php >= 74 images have been rebuilt to include this.