phpearth / docker-php

🐳 Docker For PHP developers - Docker images with PHP, Nginx, OpenLiteSpeed, Apache, Lighttpd, and Alpine
https://docs.php.earth/docker
MIT License
261 stars 80 forks source link

php: add PHP_INI_DIR variable to container #34

Closed glensc closed 4 years ago

glensc commented 5 years ago
# this project
$ docker run --rm -it app:latest sh -c 'ls -ld $PHP_INI_DIR/conf.d'
drwxr-xr-x    2 root     root          4096 May 26 00:52 /etc/php/7.1/conf.d

# official docker php image
$ docker run --rm -it php sh -c 'ls -ld $PHP_INI_DIR/conf.d'
drwxr-sr-x 1 root staff 4096 May  8 02:36 /usr/local/etc/php/conf.d
glensc commented 5 years ago

@petk WDYT? if approved, I'll change other Dockerfiles as well.

petk commented 5 years ago

Sure... This would be useful for example for knowing where to install some extension's ini configuration file or something similar?

glensc commented 5 years ago

yes. I use it for that purpose exactly:

  1. from php:7.2 or php:7.3
  2. copy blah.ini $PHP_INI_DIR/conf.d

and I do not need to adjust the path when I change base image ;)

glensc commented 5 years ago

added to all docker files, can be merged now.

for the same reason I'd like to add $PHP_VERSON=7.2 variable. append to this PR, or create new one?

petk commented 5 years ago

Thanks for the info. I would suggest using this workflow with the ini files (for the readme example):

RUN ... \
    && echo "memory_limit = 512M" >> $PHP_INI_DIR/php.ini \
    && ...

copy-ing entire INI files is not very well organized because the settings might change a lot from version to version.

This ini dir is also basically entire configuration directory here... Let me check a bit, otherwise looks good to me.

glensc commented 5 years ago

I prefer a new file because:

I can remove the readme modification from this and you can write your own instructions.

glensc commented 5 years ago

also, my instructions did not suggest copy whole .ini file, but only add project specific settings.

glensc commented 4 years ago

ping @petk

glensc commented 4 years ago

btw, using this project here: https://github.com/eventum/docker/pull/2/files

petk commented 4 years ago

If I'm not mistaken, you need to get the location of the ini directory after you install PHP. So basically, with php 7.4 you can also use php-config --ini-dir and php-config ----ini-path I'm not sure why it took so long to get this option in, yes... But now it will be available in PHP directly.

Hopefully here everything works correctly then. Thanks for the pull request.

glensc commented 4 years ago

you can't use scripts if you use COPY, see discussion here: https://github.com/docker-library/php/pull/721

petk commented 4 years ago

Yup, that's true. And that's why also echo "ini.directive" >> ... is better and adds less layers to image.

petk commented 4 years ago

Now I remember also why there was a single ENV with new lines defined :D - it adds that many layers less. Another measurement of the image simplicity :D

glensc commented 4 years ago

yeah, but echo has other downsides: no suitable version control (adding another echo will likely git blame previous line as well), total mess with syntax (dosini syntax wrapped in shell syntax, including escaping); no layers caching (all commands in single RUN re-run); more difficult to share (copy the file between projects).

typically, single ENV vs multiple ENV add only metadata, not data layers, so not really an issue.

if you're worried about layers data, you could use multi staged build and still be efficient in layers caching:

FROM alpine AS build
....

FROM scratch
COPY --from=build / .

or, to keep alpine layer:

FROM alpine AS base
FROM base AS build
....

FROM base
COPY --from=build / .

there's also docker build --squash which does basically the first example, but that likely can't be automated using dockerhub.

note: this is just a rant, no need real reason to change anything now :)

petk commented 4 years ago

I'll check, thanks for the info. Last time I was checking this, there were some recommendations to keep the layers at minimum for such base images that are used further on...

glensc commented 4 years ago

some random thoughts:

I'd keep alpine layer intact. as I might have it cached locally and provides some means to identify layers added by this project. and having multiple layers actually improves download efficiency on first pull, layers are downloaded in parallel, so typically four smaller ones come down faster than one very huge. and of course, this all depends on the downlink.

glensc commented 4 years ago

@petk any estimation when the image gets rebuilt?

➔ docker run --rm docker.io/phpearth/php:7.1-cli env|grep PHP_INI_DIR -c
0