Skip to content

add php:7.0-alpine #176

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 11 commits into from
Mar 18, 2016
Merged

add php:7.0-alpine #176

merged 11 commits into from
Mar 18, 2016

Conversation

ncopa
Copy link
Contributor

@ncopa ncopa commented Jan 11, 2016

Before you merge this I have a few notes and questions:

  • I did not include the php_recode module. I have not seen anything actually use it, so I think we can leave it out til someone asks for it. (the recode seems to be old and unmaintained and has various issues. I made it compile but I doubt it will actually work). If anybody can give me a specific example of an application using it, then I'll make it work.
  • I only did the cli variant for Alpine for now. Do we want fpm and apache variants too? If so, how do we do that? What do we call those variants? php:7.0-alpine-fpm and php:7.0-alpine-apache?
  • Do we also want php:5.6-alpine-*?

@soullivaneuh
Copy link

Good one.

👍 For apache / fpm variant and 5.6.

@tianon
Copy link
Member

tianon commented Jan 29, 2016

We'll definitely want to do this across all versions, but I think starting with just one probably makes sense (and then replicating from there). I'm not sure what to do about Apache, FPM, and ZTS versions -- they exist specifically because they're non-trivial to create and they can't really be created after the fact from a bare CLI image without recompiling all of PHP (at which point making a new image makes sense). Apache is the one I'm most worried about since it's going to be functionally different (since it'll be based on Alpine's Apache or the httpd image instead of what Debian does), but I think Alpine+FPM is definitely an easy sell (and will work really well).

So, I think we should go with alpine being a variant of each version variant here (ie, php:7-fpm-alpine, etc), with a directory structure of 7.0/fpm/alpine/Dockerfile (with 7.0/alpine/Dockerfile as CLI like you've got here).

@tianon
Copy link
Member

tianon commented Jan 29, 2016

Current diff:

--- 7.0/Dockerfile  2016-01-28 14:18:16.090223059 -0800
+++ /dev/fd/63  2016-01-29 10:50:37.928788095 -0800
@@ -1,46 +1,44 @@
-FROM debian:jessie
-
-# persistent / runtime deps
-RUN apt-get update && apt-get install -y ca-certificates curl librecode0 libsqlite3-0 libxml2 --no-install-recommends && rm -r /var/lib/apt/lists/*
-
-# phpize deps
-RUN apt-get update && apt-get install -y autoconf file g++ gcc libc-dev make pkg-config re2c --no-install-recommends && rm -r /var/lib/apt/lists/*
+FROM alpine:3.3

 ENV PHP_INI_DIR /usr/local/etc/php
 RUN mkdir -p $PHP_INI_DIR/conf.d

+ENV PHP_VERSION 7.0.2
+ENV PHP_FILENAME php-7.0.2.tar.xz
+ENV PHP_SHA256 556121271a34c442b48e3d7fa3d3bbb4413d91897abbb92aaeced4a7df5f2ab2
+
+
 ##<autogenerated>##
 ##</autogenerated>##

+# --enable-mysqlnd is included below because it's harder to compile after the fact the extensions are (since it's a plugin for several extensions, not an extension in itself)
 ENV GPG_KEYS 1A4E8B7277C42E53DBA9C7B9BCAA30EA9C0D5763
 RUN set -xe \
+   && apk add --no-cache --virtual .build-deps \
+       gnupg \
+       curl-dev \
+       autoconf \
+       file \
+       gcc \
+       libc-dev \
+       make \
+       pkgconf \
+       re2c \
+       libxml2-dev \
+       libedit-dev \
+       sqlite-dev \
+       openssl-dev \
+       libxml2-dev \
    && for key in $GPG_KEYS; do \
        gpg --keyserver ha.pool.sks-keyservers.net --recv-keys "$key"; \
-   done
-
-ENV PHP_VERSION 7.0.2
-ENV PHP_FILENAME php-7.0.2.tar.xz
-ENV PHP_SHA256 556121271a34c442b48e3d7fa3d3bbb4413d91897abbb92aaeced4a7df5f2ab2
-
-# --enable-mysqlnd is included below because it's harder to compile after the fact the extensions are (since it's a plugin for several extensions, not an extension in itself)
-RUN buildDeps=" \
-       $PHP_EXTRA_BUILD_DEPS \
-       libcurl4-openssl-dev \
-       libreadline6-dev \
-       librecode-dev \
-       libsqlite3-dev \
-       libssl-dev \
-       libxml2-dev \
-       xz-utils \
-   " \
-   && set -x \
-   && apt-get update && apt-get install -y $buildDeps --no-install-recommends && rm -rf /var/lib/apt/lists/* \
+   done \
    && curl -fSL "http://php.net/get/$PHP_FILENAME/from/this/mirror" -o "$PHP_FILENAME" \
    && echo "$PHP_SHA256 *$PHP_FILENAME" | sha256sum -c - \
    && curl -fSL "http://php.net/get/$PHP_FILENAME.asc/from/this/mirror" -o "$PHP_FILENAME.asc" \
    && gpg --verify "$PHP_FILENAME.asc" \
-   && mkdir -p /usr/src/php \
-   && tar -xf "$PHP_FILENAME" -C /usr/src/php --strip-components=1 \
+   && mkdir -p /usr/src \
+   && tar -Jxf "$PHP_FILENAME" -C /usr/src \
+   && mv /usr/src/php-$PHP_VERSION /usr/src/php \
    && rm "$PHP_FILENAME"* \
    && cd /usr/src/php \
    && ./configure \
@@ -51,14 +49,21 @@
        --enable-mysqlnd \
        --with-curl \
        --with-openssl \
-       --with-readline \
-       --with-recode \
+       --with-libedit \
        --with-zlib \
    && make -j"$(nproc)" \
    && make install \
    && { find /usr/local/bin /usr/local/sbin -type f -executable -exec strip --strip-all '{}' + || true; } \
-   && apt-get purge -y --auto-remove -o APT::AutoRemove::RecommendsImportant=false -o APT::AutoRemove::SuggestsImportant=false $buildDeps \
-   && make clean
+   && make clean \
+   && runDeps="$( \
+       scanelf --needed --nobanner --recursive /usr/local \
+           | awk '{ gsub(/,/, "\nso:", $2); print "so:" $2 }' \
+           | sort -u \
+           | xargs -r apk info --installed \
+           | sort -u \
+   )" \
+   && apk add --virtual .php-rundeps $runDeps \
+   && apk del .build-deps

 COPY docker-php-ext-* /usr/local/bin/

@tianon
Copy link
Member

tianon commented Jan 29, 2016

Also, I think we shouldn't be deviating on recode -- we should either keep it in Alpine or remove it in the main variants. It was added in #89, so maybe @xuhdev can explain why it was added there? (might just be that it cannot be added after the fact, which is fair)

@xuhdev
Copy link
Contributor

xuhdev commented Jan 29, 2016

I vaguely remember if you don't have recode preinstalled, you cannot install it with phpsize later.

@tianon
Copy link
Member

tianon commented Jan 29, 2016

@xuhdev yeah, I figured that was probably all -- I was hoping that maybe you remembered some specific project you'd needed it for, but completeness is as good an argument as any too 👍 (thanks for chiming in)

So perhaps it is safe to remove, but I think we'll need to do it consistently (and then we're more likely to find folks who actually need it).

@xuhdev
Copy link
Contributor

xuhdev commented Jan 29, 2016

It looks like phpmyadmin may need it...

@soullivaneuh
Copy link

For information, the official image of phpmyadmin, based on Alpine: https://github.com/phpmyadmin/docker/blob/master/Dockerfile

@ncopa
Copy link
Contributor Author

ncopa commented Feb 1, 2016

When I looked at building recode for alpine I could not find anything that actually used it and it seemed to old, unmaintained and obsolete.

phpmyadmin is the first software i found that uses recode. But it uses it only as fallback if iconv is missing.

I would prefer leave recode off with the alpine images for now, and if someone complains about stuff broken due to lacking recode I'll add it then. People needing it badly can always use the debian variants til the alpine php image has been updated.

@sylus
Copy link

sylus commented Feb 18, 2016

Just moved this comment to appropriate issue :)

I was testing out the fpm alpine variant and I think there is an issue with the the following code:

modules=""
for module; do
    if [ -z "$module" ]; then
        continue
    fi
    if [ -f "$module.so" ] && ! [ -f "$module" ]; then
        # allow ".so" to be optional
        module="$module.so"
    fi
    if ! [ -f "$module" ]; then
        echo >&2 "error: $(readlink -f "$module") does not exist"
        echo >&2
        usage >&2
        exit 1
    fi
    modules="$modules $module"
done

When running docker-php-ext-enable + install and going to: /usr/local/etc/php/conf.d/

-rw-r--r--   13 root     root            17 Feb 18 06:18 docker-php-ext- gd.ini
-rw-r--r--   13 root     root            23 Feb 18 06:18 docker-php-ext- mbstring.ini
-rw-r--r--   13 root     root            21 Feb 18 06:18 docker-php-ext- mcrypt.ini
-rw-r--r--    7 root     root            21 Feb 18 06:25 docker-php-ext- mysqli.ini
-rw-r--r--    7 root     root            24 Feb 18 06:26 docker-php-ext- pdo_mysql.ini
-rw-r--r--    7 root     root            24 Feb 18 06:26 docker-php-ext- pdo_pgsql.ini
-rw-r--r--    7 root     root            20 Feb 18 06:26 docker-php-ext- pgsql.ini
-rw-r--r--    7 root     root            21 Feb 18 06:26 docker-php-ext- xdebug.ini
-rw-r--r--   13 root     root            18 Feb 18 06:18 docker-php-ext- zip.ini

You can notice there is whitespace before the extension. This whitespace is also present when checking if library is already enabled so that check won't ever work either.

@sylus
Copy link

sylus commented Feb 18, 2016

I temporarily fixed this by adding the second line below right after we call basename to remove the .so extension.

        ext="$(basename "$module")"
        ext="$(echo "$module" | tr -d '[:space:]')"
        ext="${ext%.*}"

@ncopa
Copy link
Contributor Author

ncopa commented Feb 19, 2016

@sylus it should be fixed with commit ba77bb41fa9f078526e01a470d10c268bff2eae1

@sylus
Copy link

sylus commented Feb 19, 2016

Thanks a bunch @ncopa I really appreciate your help and the awesome work you do for alpine!


ENV PHP_VERSION 5.6.18
ENV PHP_FILENAME php-5.6.18.tar.xz
ENV PHP_SHA256 54dd9106c3469bc7028644d72ac140af00655420bbaaf4a742a64e9ed02ec1b0

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest to use the form:

ENV PHP_VERSION=5.6.18 \
         PHP_FILENAME=php-5.6.18.tar.xz \
         PHP_SHA256=54dd9106c3469bc7028644d72ac140af00655420bbaaf4a742a64e9ed02ec1b0

Just so as to reduce the number of layers (one layer means an additional download after all). I'd probably regroup all ENV together when possible.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that consecutive ENV lines imply more layers.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They do, you can check either in imagelayer or using docker run --rm -v /var/run/docker.sock:/var/run/docker.sock nate/dockviz images -t

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Docker 1.10 and above it won't make much of a difference, but it's a good practice to combine them

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thaJeztah ok, thank you for the tip ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @tianon prefers consistency in this case.

@ncopa
Copy link
Contributor Author

ncopa commented Mar 3, 2016

@tianon I have rebased the patches. What is preventing this from being merged?

@mbrevda
Copy link

mbrevda commented Mar 7, 2016

Would be good to update the apache versions, too

libedit-dev \
sqlite-dev \
openssl-dev \
libxml2-dev \
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is listed twice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch. thanks!

@shouze
Copy link
Contributor

shouze commented Mar 14, 2016

This would be great to merge now. I would only miss the apache variant... but should be added later no?

ENV PHP_INI_DIR /usr/local/etc/php
RUN mkdir -p $PHP_INI_DIR/conf.d

ENV PHP_VERSION 7.0.3

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this file should be removed before commit

@andypost
Copy link

except copy-paste remains that's great to have asap ;)

@ncopa
Copy link
Contributor Author

ncopa commented Mar 17, 2016

i removed the *.orig. Good catch!

i think it should be ready for merge now.

@tianon
Copy link
Member

tianon commented Mar 18, 2016

👍

tianon added a commit that referenced this pull request Mar 18, 2016
@tianon tianon merged commit e1ab9ff into docker-library:master Mar 18, 2016
@thaJeztah
Copy link
Contributor

Nice! 🎉

tianon added a commit to infosiftr/stackbrew that referenced this pull request Mar 18, 2016
- `php`: add Alpine variants (docker-library/php#176, docker-library/php#202)
- `pypy`: pip 8.1.1
- `python`: pip 8.1.1
tianon added a commit to infosiftr/stackbrew that referenced this pull request Mar 18, 2016
- `php`: add Alpine variants (docker-library/php#176, docker-library/php#202)
- `pypy`: pip 8.1.1
- `python`: pip 8.1.1
@mbrevda
Copy link

mbrevda commented Mar 19, 2016

The alpine based image is about 28% smaller than the original, which is good. An image built using the alpine package manager is 96% smaller than the original! Is there anyway to get closer to the smaller build?

php                   alpine              e40e7aabdb36        17 hours ago        376.5 MB
php                   latest              8c8eb06f7c64        17 hours ago        483.8 MB
frolvlad/alpine-php   latest              9eda42b8cf2e        2 weeks ago         18.22 MB

@shouze
Copy link
Contributor

shouze commented Mar 19, 2016

I'm a bit surprised too that alpine images resulting from this PR are not smaller.
If I deep into cli image for example (docker run --rm -it php:alpine ash):

/ # du -sh /usr/*
10.6M   /usr/bin
26.9M   /usr/include
73.8M   /usr/lib
63.6M   /usr/libexec
30.5M   /usr/local
16.0K   /usr/sbin
39.4M   /usr/share
185.3M  /usr/src
2.6M    /usr/x86_64-alpine-linux-musl
/ # du -sh /usr/src/*
185.3M  /usr/src/php
/ # du -sh /usr/src/php/ext
119.4M  /usr/src/php/ext
/ # du -sh /usr/share/*
16.0K   /usr/share/aclocal
1.7M    /usr/share/autoconf
712.0K  /usr/share/ca-certificates
132.0K  /usr/share/gcc-5.3.0
24.0K   /usr/share/gdb
3.7M    /usr/share/misc
26.5M   /usr/share/perl5
6.7M    /usr/share/terminfo
20.0K   /usr/share/zsh

So...

  • De we need the php exts source code? => 119MB
  • Do we need perl5? => 26.5MB
  • Do we need zsh (no so important)?

Also, as most projects using/inheriting from the alpine image use it to produce & 'final' image for their apps, would be useful to provide a cleanup command that go further, removing for example gcc, the whole source code & so on at the end of the build no?

@andypost
Copy link

suppose there should be script for 1% of container consumer to add new extensions (pull, build and clean-up)

COPY docker-php-ext-* /usr/local/bin/

##<autogenerated>##
CMD ["php", "-a"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably a good idea to have "php" in ENTRYPOINT and "-a" in CMD (see https://www.ctl.io/developers/blog/post/dockerfile-entrypoint-vs-cmd/ => "ENTRYPOINT and CMD" especially) (same for other php versions dockerfiles)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, we want to keep official images consistent so that a docker run -it --rm random-image valid-shell works: https://github.com/docker-library/official-images#consistency. If there is a need for an entrypoint script to do something else like a chown, then it could be added in the way described in the official images.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a great article and could be beneficial to many docker users.

@ncopa ncopa deleted the php-alpine branch March 21, 2016 11:52
@ncopa
Copy link
Contributor Author

ncopa commented Mar 21, 2016

It is the .persistent-deps and .phpize-deps that blows up the size. We should create a new ticket for that since it is not related this pull request.

@mbrevda mbrevda mentioned this pull request Mar 27, 2016
RichardScothern pushed a commit to RichardScothern/official-images that referenced this pull request Jun 14, 2016
- `php`: add Alpine variants (docker-library/php#176, docker-library/php#202)
- `pypy`: pip 8.1.1
- `python`: pip 8.1.1
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.