Skip to content

install build time deps from docker-php-ext-install for alpine #206

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 20, 2016

Conversation

ncopa
Copy link
Contributor

@ncopa ncopa commented Mar 21, 2016

We don't need the build dependencies during runtime, so we temporarily
install them when building extension. This reduces image size with 50%.

@ncopa
Copy link
Contributor Author

ncopa commented Mar 21, 2016

I only did alpine images for now because people who use debian images probably don't care about size anyway.

@ncopa ncopa force-pushed the php-ext-install-alpine branch from 4fdde3a to c5ce7bb Compare March 22, 2016 14:28
@ncopa
Copy link
Contributor Author

ncopa commented Mar 22, 2016

rebased it with the fix to make travis happy

@Lctrs
Copy link

Lctrs commented Mar 24, 2016

I like this idea but it'll make installing extension through PECL a lot more difficult than now. Maybe we can add PECL support to docker-php-ext-install ?

@shouze
Copy link
Contributor

shouze commented Mar 27, 2016

LGTM, not BC. great idea to wrap phpize deps into ext install scripts.

@Lctrs yes, maybe we should just wrap pecl the same way, overriding it with some alias that does the same tricks?

@Lctrs
Copy link

Lctrs commented Mar 27, 2016

@shouze actually, there is a BC for those who are installing extensions through PECL. Best options IMHO would be to add a wrapper around it or add support to it in docker-php-ext-install.

@ncopa ncopa mentioned this pull request Mar 28, 2016
@mbrevda
Copy link

mbrevda commented Mar 28, 2016

Would there be any downsides to using alpine packages instead of building from source? It would seem that would alleviate the need for build deps.

@tianon
Copy link
Member

tianon commented Mar 29, 2016

@ncopa does apk support any method for creating a "persistent" virtual? (are virtuals already persistent?) That way we could list the packages once in the Dockerfile when creating the virtual, and just reference the virtual elsewhere (especially and including for pecl users).

Otherwise, I'm generally +1 to the approach, and agree that it doesn't really make sense for Debian to follow suit.

@deizel
Copy link

deizel commented Mar 29, 2016

@mbrevda: Would there be any downsides to using alpine packages instead of building from source? It would seem that would alleviate the need for build deps.

The list of PHP packages available from apk is not exhaustive, so users will still be need build deps to compile certain extensions (redis, yaml, amqp, to name a few).

The ideal solution would be to fix these omissions upstream (by helping the alpine package maintainers) but, in the interest of time, most will want to build such things manually from their Dockerfile.

Another thing of note, the alpine packages (for php7 in particular) put files in different locations, so some extra work would be required to alleviate/document this if that approach was taken.

RUN ln -s php7 /usr/bin/php
RUN ln -s php-config7 "/usr/bin/php-config"
RUN cd "/usr/include/php7/ext/yaml" && phpize7 && ./configure && make && make install

As you can see, binaries suffixed with 7 (/usr/bin/{php7,phpize7,php-config7}), sources in /usr/include/php7/ext/ instead of /usr/src/php/ext/, etc.

That said, I see advantages with both approaches, though the current one seems to maintain consistency with the other php images with regard to file locations.

@mbrevda
Copy link

mbrevda commented Mar 30, 2016

I guess the approach (binaries vs build) doesn't really matter, it's the end product that were all aiming for! I just hope we can get an image that is as small as with binaries (which is really small).

@ncopa
Copy link
Contributor Author

ncopa commented Mar 31, 2016

@mbrevda: Would there be any downsides to using alpine packages instead of building from source? It would seem that would alleviate the need for build deps.

I would say the biggest downside would that we'd need to take non-docker alpine users' needs in consideration and we'd need coordinate the docker image release with the alpine releases. We would depend on alpine developers for pushing new releases.

Now, I am in the position to adapt how/when alpine linux packages are done, so I believe it would be possible to simply use alpine packages. There would be some benefits doing so too. With the alpine built package we can split a single build into multiple pre-compiled packages. You could have the default runtime very minimal (as you showed, its 80-90% size reduction) and when need you can add the parts you may need later with apk add.

With the current build from source approach this is not possible. This means that currently we need to include more things than strictly necessary because its difficult to add those parts afterwards.

Another argument against using alpine packages would be less consistency, however I think that it would be worth it. Why have different variants if they are all the same anyway?

@deizel : The list of PHP packages available from apk is not exhaustive,

The list of php packages available from apk is still bigger than from the current Docker file, which is none. If you need 3rd party module you need to use docker-php-ext-install to compile those from source. So it would not make significant difference. (you'd need apk add php-dev in addition to build tools).

@deizel : Another thing of note, the alpine packages (for php7 in particular) put files in different locations,

This was only a temporary thing to give users possibility to test php7 early. I think we need to clean up php7.

@mbrevda
Copy link

mbrevda commented Mar 31, 2016

As above, the approach is less important than the final size. I see benefits to installing from source (flexibility in compile time opts, less opinionated, etd) and benefits to packages (it greatly simplifies everything).

I would say stick to source - provided the entire environment is cleaned up afterwards and the size is about the same. Otherwise, there is a great benefit to packages.

@ncopa
Copy link
Contributor Author

ncopa commented Mar 31, 2016

@mbrevda the problem with building from source is that once you have cleaned away the stuff you don't need for the runtime (headers, static libs, pecl etc), then you cannot easily get them back later. So those need to stay.

This is exactly the problem a package manager like apk is supposed to solve.

@ncopa
Copy link
Contributor Author

ncopa commented Mar 31, 2016

@tianon i don't follow what you mean with "persistant virtuals".

But we have the same problem in the other languages, gem, pip, npm where you may need build time dependencies that is not needed during runtime.

@shouze
Copy link
Contributor

shouze commented Mar 31, 2016

@ncopa @mbrevda about alpine packages... I'm not sure to understand, are we talking about alpine packages of php extensions? If yes, can be risky if we're not sure of sapi compatibility no?

Otherwise, the only remaining issue I've noted past week was manual pecl installations no? Putting a wrapper around pecl to install / uninstall .phpize-deps looks good to me.

If we're talking about use cases, personaly I build my own containers inherited from php:alpine one's so I NEED stuff not needed for the runtime (headers, static libs, pecl etc) but I could choose to trash them at the end of the build of my custom container. Not sure this should save to much space on the resulting container as the previous layer got them in. Maybe there's something to do with docker ONBUILD about that?

@mbrevda
Copy link

mbrevda commented Mar 31, 2016

@shouze I hear your point. I guess ideal now means to find the best medium between a custom build and size. Perhaps instructions need to be included on how to blow away the dev tools once the last-in-chain desides they are no longer needed.

@tianon
Copy link
Member

tianon commented Mar 31, 2016

@ncopa sorry, I mean something like this:

$ docker run -it --rm alpine:3.3 sh
/ # apk add --no-cache --virtual .test ca-certificates
fetch http://dl-cdn.alpinelinux.org/alpine/v3.3/main/x86_64/APKINDEX.tar.gz
fetch http://dl-cdn.alpinelinux.org/alpine/v3.3/community/x86_64/APKINDEX.tar.gz
(1/3) Installing openssl (1.0.2g-r0)
(2/3) Installing ca-certificates (20160104-r2)
(3/3) Installing .test (0)
Executing busybox-1.24.1-r7.trigger
Executing ca-certificates-20160104-r2.trigger
OK: 6 MiB in 14 packages
/ # apk del .test
WARNING: Ignoring APKINDEX.5a59b88b.tar.gz: No such file or directory
WARNING: Ignoring APKINDEX.7c1f02d6.tar.gz: No such file or directory
(1/3) Purging .test (0)
(2/3) Purging ca-certificates (20160104-r2)
(3/3) Purging openssl (1.0.2g-r0)
Executing busybox-1.24.1-r7.trigger
OK: 5 MiB in 11 packages
/ # apk add --no-cache .test
fetch http://dl-cdn.alpinelinux.org/alpine/v3.3/main/x86_64/APKINDEX.tar.gz
fetch http://dl-cdn.alpinelinux.org/alpine/v3.3/community/x86_64/APKINDEX.tar.gz
ERROR: unsatisfiable constraints:
  .test (missing):
    required by: world[.test]

Is there any way to make .test persist after apk del ?

@shouze
Copy link
Contributor

shouze commented Mar 31, 2016

Perhaps instructions need to be included on how to blow away the dev tools once the last-in-chain desides they are no longer needed

@mbrevda looks good to me, end users of that kind of images are not noob users I guess.

@franz-josef-kaiser
Copy link

looks good to me, end users of that kind of images are not noob users I guess.

@shouze Sidenote: You should assume that Alpine based images will become the new standard.

@ncopa
Copy link
Contributor Author

ncopa commented Apr 4, 2016

Is there any way to make .test persist after apk del ?

Not really. Well, we could always make an official alpine package of it, but i doubt we want that.

Other alternative is to use an ENV var:

ENV PHP_BUILD_DEPS="libc-dev gcc make php-dev"

So that you could:

...
RUN apk add --no-cache --virtual .php-builddeps $PHP_BUILD_DEPS \
    ...
    && apk del .php-builddeps

@franz-josef-kaiser
Copy link

@ncopa Out of 7 participants, you got 3 (+1 incl. yourself), that's 50+% … LGTM.

@shouze
Copy link
Contributor

shouze commented Apr 17, 2016

@ncopa the ENV var is pretty good as we can easily reinstall builddeps that way when we onehrit from an alpine Dockerfile.

@yosifkit
Copy link
Member

👍 on the ENV var idea for the phpize deps; probably put it right before installing those deps on the debian versions. What do you think @tianon?

@tianon
Copy link
Member

tianon commented Apr 18, 2016

Yeah, I think that makes sense, especially so our Debian<->Alpine diff stays sane and still lines up reasonably (although still only using the var at run-time on Alpine -- apk add is much faster than apt-get update && apt-get install .... and thus I think this is reasonable there, but not for Debian).

@ncopa
Copy link
Contributor Author

ncopa commented Apr 19, 2016

Would something like this work?

diff --git a/Dockerfile-alpine.template b/Dockerfile-alpine.template
index e126463..3f76e67 100644
--- a/Dockerfile-alpine.template
+++ b/Dockerfile-alpine.template
@@ -26,21 +26,16 @@ ENV PHP_VERSION %%PHP_VERSION%%
 ENV PHP_FILENAME %%PHP_FILENAME%%
 ENV PHP_SHA256 %%PHP_SHA256%%

+ENV PHPIZE_DEPS autoconf file g++ gcc libc-dev make pkgconf re2c
+
 RUN set -xe \
        && apk add --no-cache --virtual .build-deps \
-               autoconf \
+               $PHPIZE_DEPS \
                curl-dev \
-               file \
-               g++ \
-               gcc \
                gnupg \
-               libc-dev \
                libedit-dev \
                libxml2-dev \
-               make \
                openssl-dev \
-               pkgconf \
-               re2c \
                sqlite-dev \
        && curl -fSL "http://php.net/get/$PHP_FILENAME/from/this/mirror" -o "$PHP_FILENAME" \
        && echo "$PHP_SHA256 *$PHP_FILENAME" | sha256sum -c - \

@franz-josef-kaiser
Copy link

@ncopa it would be great if there would be actual ARG usage, so one can use

ARG PACKAGES
ENV PHPIZE_DEPS "autoconf file g++ gcc libc-dev make pkgconf re2c ${PACKAGES:-}"

RUN set -xe \
        && apk add --no-cache --virtual .build-deps \
               $PHPIZE_DEPS \
               curl-dev \
               gnupg \
               libedit-dev \
               libxml2-dev \
               openssl-dev \
               sqlite-dev \
        && curl -fSL "http://php.net/get/$PHP_FILENAME/from/this/mirror" -o "$PHP_FILENAME" \
        && echo "$PHP_SHA256 *$PHP_FILENAME" | sha256sum -c - \
        …

This would enable us to customize the build using something along the lines:

docker build -t php-fpm:5.6 --build-arg PACKAGES="foo bar baz" .

@tianon
Copy link
Member

tianon commented Apr 20, 2016

@ncopa yeah, looks sane to me -- then on Alpine that can be used directly from the docker-php-... scripts 👍

@franz-josef-kaiser I'm not sure I understand what value having an ARG for that would add over just using something like the following, can you elaborate?

FROM php:5.6-fpm
RUN apt-get update && apt-get install -y foo bar baz

or

FROM php:5.6-fpm-alpine
ENV PHPIZE_DEPS $PHPIZE_DEPS foo bar baz

or even better yet

FROM php:5.6-fpm-alpine
# using an inline var change so they're not permanently added
RUN PHPIZE_DEPS="foo bar baz $PHPIZE_DEPS" docker-php-ext-install ....

@franz-josef-kaiser
Copy link

@ncopa It's just the possibility to install things in the base box in case I do not need a custom local box. For that case it would save me from building a custom box if I need it for some throw away case.

@shouze
Copy link
Contributor

shouze commented May 19, 2016

ping @tianon @yosifkit are we all agreee about the ENV var introduction as suggested by @ncopa in #206 (comment)?

@ncopa could you push it then we could merge & move forward on alpine builds for the good? 😄

@yosifkit
Copy link
Member

@shouze, yeah that change looks good to me.

@ncopa, To make a quicker review could you do one commit to adjust all the templates in the root and then a second commit for running update to apply it to all the versions?

ncopa added 2 commits May 20, 2016 16:17
We don't need the build dependencies during runtime, so we temporarily
install them when building extension. This reduces image size with 50%.

We also provide a PHPIZE_DEPS environment variable with the needed deps.
@ncopa ncopa force-pushed the php-ext-install-alpine branch from c5ce7bb to 1ca3e1d Compare May 20, 2016 14:19
@ncopa
Copy link
Contributor Author

ncopa commented May 20, 2016

@yosifkit rebased patch

@yosifkit
Copy link
Member

LGTM

@yosifkit yosifkit merged commit 60a7ce3 into docker-library:master May 20, 2016
@shouze
Copy link
Contributor

shouze commented May 20, 2016

@yosifkit I guess it's not auto builds? Looks like images are not yet available on docker hub.

@yosifkit
Copy link
Member

@shouze, unfortunately no. We need to make a PR to official images.

See a change merged here that doesn't show up on the Docker Hub yet? Check the "library/php" manifest file in the docker-library/official-images repo, especially PRs with the "library/php" label on that repo. For more information about the official images process, see the docker-library/official-images readme.
https://github.com/docker-library/php#about-this-repo

@ncopa ncopa deleted the php-ext-install-alpine branch May 23, 2016 18:19
tianon added a commit to infosiftr/stackbrew that referenced this pull request May 23, 2016
- `java`: 8u91 (Debian)
- `php`: adjust `phpize` deps handling for slightly smaller Alpine images (docker-library/php#206, docker-library/php@d1c1222)
@kusmierz
Copy link

@ncopa @tianon what with docker-php-ext-configure? There is phpize command, but no dependencies installed... So trying

docker-php-ext-configure pdo_pgsql -with-pgsql=/usr/include/postgresql/ \
&& docker-php-ext-install pdo_pgsql

rises an error:

Cannot find autoconf. Please check your autoconf installation and the
$PHP_AUTOCONF environment variable. Then, rerun this script.

ERROR: Service 'xxx' failed to build: The command '/bin/sh -c set -ex     && apk --no-cache add postgresql-dev sqlite-dev     && docker-php-ext-install pdo pdo_sqlite pdo_mysql     && docker-php-ext-configure pdo_pgsql -with-pgsql=/usr/include/postgresql/     && docker-php-ext-install pdo_pgsql' returned a non-zero code: 1

@shouze
Copy link
Contributor

shouze commented Jun 2, 2016

ping @kusmierz I've tried to fix with #237

@teohhanhui
Copy link

You broke existing builds with this... I expected more concern for maintaining BC 😞

e.g. pecl install apcu-5.1.3

teohhanhui added a commit to teohhanhui/docker-tripviss-api-ci that referenced this pull request Jun 3, 2016
teohhanhui added a commit to teohhanhui/docker-tripviss-api-ci that referenced this pull request Jun 3, 2016
teohhanhui added a commit to tripviss/docker-tripviss-api-ci that referenced this pull request Jun 3, 2016
@shouze
Copy link
Contributor

shouze commented Jun 3, 2016

@teohhanhui I guess it worth the break as the result is smaller images. It's super easy even if you need some pecl install to apk add php extension dependencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants