Skip to content
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

Build SOAP extension as shared instead of static #39

Closed
wants to merge 1 commit into from

Conversation

md5
Copy link
Contributor

@md5 md5 commented Nov 8, 2014

Updates the changes in #35 to build SOAP extension as shared instead of static.

@thaJeztah
Copy link
Contributor

Ah, nice! Definitely +1, SOAP is not the most common extension in PHP, so disabling by default should be fine for most users.

@md5
Copy link
Contributor Author

md5 commented Nov 8, 2014

Certainly. If anyone has come to rely on a statically linked SOAP extension in the last few days, I apologize in advance for this breaking change.

FYI, I've opened docker-library/docs#94 to document the extension situation.

@md5
Copy link
Contributor Author

md5 commented Nov 9, 2014

I just had the urge to try out using phpize on the core extensions and found that it works fine. Here's my POC using mbstring.so as the guinea pig: synctree/php@e2876c6

And here's the result:

$ docker run -it --rm php:mbstring php -dextension=mbstring.so -m | grep mbstring
mbstring

@tianon
Copy link
Member

tianon commented Nov 10, 2014

Very, very interesting POC. 😄

All that really concerns me is the size of /usr/src/php, but it's bound to be smaller than the full size of every single module being pre-built, right? 😈

@md5
Copy link
Contributor Author

md5 commented Nov 10, 2014

Duplicating my comment from the POC commit here:

I just built 5.6-apache locally based on master and this branch and here's what I saw:

php    5.6-apache               da78d41ebecd        31 minutes ago      384.4 MB
php    5.6-apache-with-source   e4df095dba00        40 minutes ago      500 MB

@tianon
Copy link
Member

tianon commented Nov 10, 2014

A little bit hefty, but considering that we used to be at 800-900 MB, I think that's reasonable, and especially compared to the huge amount it would end up being to compile everything shared.

Do you think it would be worthwhile to wrap up your few lines of "compile mbstring" into some kind of helper script to make it easy to say something like docker-phpize mbstring and have it be installed and configured to be used (because otherwise why would you compile it)?

Do you think this method might work for FPM and Apache2 compilation as well? I guess we ought to start here and play with it from there. 😄

@md5
Copy link
Contributor Author

md5 commented Nov 10, 2014

I wasn't sure whether do suggest something like docker-phpize, but I think it would make it a lot easier to add built-in extensions. It would probably have to take a list of additional *-dev packages to install for the build along with the extension name.

As for the FPM and Apache2 compilation, I'm not sure. Looking at the FPM changes, that could probably be done using the approach I used in the POC branch for mbstring.

@tianon
Copy link
Member

tianon commented Nov 10, 2014

Just did a test with the following patch:

diff --git a/5.6/Dockerfile b/5.6/Dockerfile
index afbd82c..15076d7 100644
--- a/5.6/Dockerfile
+++ b/5.6/Dockerfile
@@ -47,7 +47,7 @@ RUN buildDeps=" \
        && make install \
        && { find /usr/local/bin /usr/local/sbin -type f -executable -exec strip --strip-all '{}' + || true; } \
        && apt-get purge -y --auto-remove $buildDeps \
-       && rm -r /usr/src/php
+       && make clean

And got the following size comparison:

b527230b5967        About an hour ago   361.6 MB
2c123ceec26b        5 minutes ago       461.6 MB

Which is basically exactly 100MB. Nice. Let's start here and see where it goes! (Will be sending a PR shortly.)

@tianon
Copy link
Member

tianon commented Nov 10, 2014

As for extra deps, couldn't we just leave installing them as an exercise for the reader? ie, we document that you should RUN apt-get update && apt-get install -y whatever-build-dep before you RUN docker-phpize someextension?

The alternative would be to maintain a list of build-deps necessary per-package, and perhaps runtime deps too, since those are going to be different as well.

@md5
Copy link
Contributor Author

md5 commented Nov 10, 2014

Taking mcrypt as an example, here's what I'd expect to see in a Dockerfile that adds it:

FROM php:5.6
RUN apt-get update && apt-get install -y libmcrypt4
RUN docker-phpize mcrypt libmcrypt-dev

That is, the runtime dependencies would be explicitly installed, but the dev dependencies would be installed and then removed by docker-phpize (or whatever it's called).

@yosifkit
Copy link
Member

@md5, I like the idea, but can I only install one extension per run of docker-phpize? How would I do multiple (just docker-phpize libA libA-dev-pkg && docker-phpize libB libB-dev-pkg ...)?

Maybe it could be docker-phpize [-d|--dep apt-pkg] php-lib ... and could be used as (don't quote me on the deps being correct):

docker-phpize mcrypt -d=libmcrypt-dev mbstring soap -d=libxml2-dev`

Some extensions might have one, multiple, or no build deps, so this allows any of them.

@tianon
Copy link
Member

tianon commented Nov 10, 2014

So with that example, why not just have the users of the image do something more like this:

FROM php:5.6
RUN apt-get update && apt-get install -y libmcrypt4
RUN apt-get update && apt-get install -y libmcrypt-dev \
    && docker-phpize mcrypt \
    && apt-get purge --auto-remove -y libmcrypt-dev

That way, our script doesn't "accidentally" remove something the user didn't expect, and docker-phpize would essentially just be cd /usr/src/php/ext/$EXT && phpize && ./configure && make && make install && echo "extension=$EXT.so" > wherever (or some other better way to gather the list of actual "so" files that were installed, similar to how the generated Makefile itself does so).

@tianon
Copy link
Member

tianon commented Nov 10, 2014

Also, then users who don't care as much about size as we do in the official image could just put libcrypt-dev on that first line and call it a day.

@yosifkit yosifkit mentioned this pull request Nov 10, 2014
@yosifkit
Copy link
Member

So, we want a tool to phpize the libs -> see issue #39.

As far as soap to a shared extension: LGTM

@tianon
Copy link
Member

tianon commented Nov 10, 2014

I'm thinking we should maybe just implement that, and remove --enable-soap along the way.

@yosifkit
Copy link
Member

Do we want to move it to a shared library anyway so that no one gets used to the automatic soap?

@tianon
Copy link
Member

tianon commented Nov 10, 2014

Well, it's not included in docker-library/official-images#308, so I'm not too worried about it yet. We didn't release SOAP yet previously, right?

@md5
Copy link
Contributor Author

md5 commented Nov 10, 2014

I'm fine with having --enable-soap go away and having to build it myself, as long as it's possible and ideally easy to do.

@yosifkit
Copy link
Member

It was merged ibn #35, but I see it did not get pushed into official-images, so it has not been "released".

@md5 md5 closed this Nov 11, 2014
@md5 md5 deleted the enable-soap-shared branch November 27, 2014 00:01
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.

None yet

4 participants