Skip to content

Rename config0.m4 to config.m4 to avoid phpize failure #89

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 1 commit into from
Apr 21, 2015

Conversation

xuhdev
Copy link
Contributor

@xuhdev xuhdev commented Apr 13, 2015

phpize in docker-php-ext-install would fail if it cannot find config.m4. Some modules, such as zlib, sqlite3 use the name cnofig0.m4 instead of config.m4. This commit performs auto renaming after php is installed.

@md5
Copy link
Contributor

md5 commented Apr 13, 2015

@yosifkit
Copy link
Member

I think with the bugs linked by @md5, I would be hesitant to change what is a "bug in phpize". Given my reservations, I think we should make it easier for users to phpize these extensions. I would like to see it a little safer (right now it would clobber a config.m4 if both existed). I also wonder if we should do more like one of the bug comments suggested and make a config.m4 file just with includes for any other config files in the directory.

@md5
Copy link
Contributor

md5 commented Apr 14, 2015

@yosifkit Another possibility would be to add all the ones that use config0.m4 to the ./configure. I'm not sure off hand how that would affect image size, but it seems to be what the maintainers recommend in those bugs.

@tianon
Copy link
Member

tianon commented Apr 14, 2015 via email

@md5
Copy link
Contributor

md5 commented Apr 14, 2015

I had assumed that config9 was actually a typo.

@tianon
Copy link
Member

tianon commented Apr 14, 2015 via email

@md5
Copy link
Contributor

md5 commented Apr 14, 2015

$ docker run -it --rm php find /usr/src/php -name 'config[0-9]*.m4'
/usr/src/php/sapi/cgi/config9.m4
/usr/src/php/ext/openssl/config0.m4
/usr/src/php/ext/recode/config9.m4
/usr/src/php/ext/date/config0.m4
/usr/src/php/ext/pcre/config0.m4
/usr/src/php/ext/libxml/config0.m4
/usr/src/php/ext/zlib/config0.m4
/usr/src/php/ext/ereg/config0.m4
/usr/src/php/ext/sqlite3/config0.m4
/usr/src/php/ext/mysqlnd/config9.m4

Not a typo 👍

@xuhdev
Copy link
Contributor Author

xuhdev commented Apr 15, 2015

Ideally if those extensions are configured during php installation, maybe you wanna see the size? I think some of them (sqlite3, pcre, date) are really important.

@yosifkit
Copy link
Member

A size difference would be great. Could you make a comparison of enabling the ones we don't already have, that are unable to be phpized, to the current image? Maybe just 5.6 for a start?

@xuhdev
Copy link
Contributor Author

xuhdev commented Apr 16, 2015

I applied this patch to the 5.6 CLI version

diff --git a/5.6/Dockerfile b/5.6/Dockerfile
index c8976b3ed5c6..fb3ea3134f8c 100644
--- a/5.6/Dockerfile
+++ b/5.6/Dockerfile
@@ -1,7 +1,7 @@
 FROM debian:jessie

 # persistent / runtime deps
-RUN apt-get update && apt-get install -y ca-certificates curl libxml2 --no-install-recommends && rm -r /var/lib/apt/lists/*
+RUN apt-get update && apt-get install -y ca-certificates curl libpcre3 libxml2 librecode0 --no-install-recommends && rm -r /var/lib/apt/lists/*

 # phpize deps
 RUN apt-get update && apt-get install -y autoconf file gcc libc-dev make pkg-config re2c --no-install-recommends && rm -r /var/lib/apt/lists/*
@@ -21,7 +21,9 @@ RUN buildDeps=" \
                $PHP_EXTRA_BUILD_DEPS \
                bzip2 \
                libcurl4-openssl-dev \
+               libpcre3-dev \
                libreadline6-dev \
+               librecode-dev \
                libssl-dev \
                libxml2-dev \
        " \
@@ -42,7 +44,9 @@ RUN buildDeps=" \
                --enable-mysqlnd \
                --with-curl \
                --with-openssl \
+               --with-pcre \
                --with-readline \
+               --with-recode \
                --with-zlib \
        && make -j"$(nproc)" \
        && make install \

Sizes:

test            406.9 MB
php            405.1 MB

I don't think it's really a problem...

@md5
Copy link
Contributor

md5 commented Apr 16, 2015

@xuhdev 👍

@yosifkit
Copy link
Member

That looks great to me 👍.

@md5
Copy link
Contributor

md5 commented Apr 16, 2015

I may check this myself later if I have some time, but are sqlite3 and ereg already enabled somehow?

@xuhdev
Copy link
Contributor Author

xuhdev commented Apr 16, 2015

@md5 I think if you didn't add the option --disable-sqlite3, sqlite3 is enabled. I cannot find any option related to ereg though... don't know what it is

@md5
Copy link
Contributor

md5 commented Apr 16, 2015

Thanks @xuhdev 👍

@md5
Copy link
Contributor

md5 commented Apr 16, 2015

Looks like ereg is there even without your changes:

$ docker run -it --rm php php -m | grep ereg
ereg

@yosifkit
Copy link
Member

@xuhdev, want to just swap this PR or make a new one to just enabling the couple extensions? 1.8MB is a reasonable amount to me (and won't require changing the source files).

Extensions added: pcre, recode, sqlite3
@xuhdev
Copy link
Contributor Author

xuhdev commented Apr 18, 2015

@yosifkit Updated the commit. Note that I added the sqlite3 development files (otherwise the sqlite3 extension will not be built) and the size difference increases to 2.7M.

@yosifkit
Copy link
Member

LGTM

1 similar comment
@tianon
Copy link
Member

tianon commented Apr 21, 2015

LGTM

tianon added a commit that referenced this pull request Apr 21, 2015
Rename config0.m4 to config.m4 to avoid phpize failure
@tianon tianon merged commit e903963 into docker-library:master Apr 21, 2015
@tianon tianon mentioned this pull request Jan 29, 2016
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.

4 participants