Skip to content

Reduced the size of the image by about %23 #96

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

Closed
wants to merge 1 commit into from

Conversation

armen
Copy link

@armen armen commented May 7, 2015

It bzips the /usr/src/php to reduce the size of the image. bzip2
package is moved outsize of the $buildDeps just for backwards
compatibility otherwise one would have needed to add bzip2 into
their Dockerfile as build dependency.

@md5
Copy link
Contributor

md5 commented May 7, 2015

I was going to ask whether this breaks the docker-php-ext-install script, but I see now that you updated that script and the companion *-configure script to uncompress the directory as needed. Looks like the docker-php-ext-* scripts are covered by a test (which I apparently wrote and didn't recall...).

LGTM

@thaJeztah
Copy link
Contributor

Nice idea! LGTM

@yosifkit
Copy link
Member

Hmmm... it looks like a good idea, but I see a problem. After running docker-php-ext-install, the src is tarred and zipped again. This would produce a new version of the tar that is not that same as the tar in the image. This would be tracked as a new file in this docker layer, and grow the size of the end image. Are there changes after install in the src directory that need to be kept for later?

FROM php:apache
RUN apt-get update && apt-get install -y \
        libfreetype6-dev \
        libjpeg62-turbo-dev \
        libmcrypt-dev \
        libpng12-dev \
    && docker-php-ext-install iconv mcrypt \
    && docker-php-ext-configure gd --with-freetype-dir=/usr/include/ --with-jpeg-dir=/usr/include/ \
    && docker-php-ext-install gd
# this RUN would end with a new tar of /usr/src/php (with the configure options zipped in)
# now the image takes the space of two bzipped php source tars

@tianon
Copy link
Member

tianon commented May 14, 2015

docker-php-ext-configure gd --with-freetype-dir=/usr/include/ --with-jpeg-dir=/usr/include/ is a perfect example of something that makes changes for the following docker-php-ext-install gd, and will thus balloon image sizes here by duplicating that tarball every layer 😢 (even just mtime changes get stored as a brand new file)

@md5
Copy link
Contributor

md5 commented May 14, 2015

It seems like just removing the tar step would make things OK. In the case of @yosifkit's example, I'd do something like this:

FROM php:apache
RUN apt-get update && apt-get install -y \
        libfreetype6-dev \
        libjpeg62-turbo-dev \
        libmcrypt-dev \
        libpng12-dev \
    && docker-php-ext-configure gd --with-freetype-dir=/usr/include/ --with-jpeg-dir=/usr/include/ \
    && docker-php-ext-install iconv mcrypt gd

That way the unbzipping would only happen once in docker-php-ext-configure and /usr/src/php would be removed from the image in the docker-php-ext-install call. Any subsequent calls to docker-php-ext-configure or docker-php-ext-install would still have to redo unbzipping as needed. This would result in a slower build of the image, but it would not result in the image getting extra copies of the tar file or the unbzipped source getting left around unless someone were to run docker-php-ext-configure without a subsequent docker-php-ext-install or if they were to run them in different RUN statements.

On that last point, something like this would be pretty pathological:

FROM php:apache
RUN apt-get update && apt-get install -y \
        libfreetype6-dev \
        libjpeg62-turbo-dev \
        libmcrypt-dev \
        libpng12-dev
RUN docker-php-ext-configure iconv
RUN docker-php-ext-install iconv
RUN docker-php-ext-configure mcrypt
RUN docker-php-ext-install mcrypt
RUN docker-php-ext-configure gd --with-freetype-dir=/usr/include/ --with-jpeg-dir=/usr/include/
RUN docker-php-ext-install gd

I think you'd end up with three copies of /usr/src/php in the image, each one whited out after the docker-php-ext-install steps.

It bzips the /usr/src/php to reduce the size of the image. bzip2
package is moved outside of the $buildDeps just for backwards
compatibility otherwise one would have needed to add bzip2 into
their Dockerfile as build dependency.
@yosifkit
Copy link
Member

Well, it seems we have at least 13 users that would be affected (github search). There are around 100-400 that use the install and configure scripts.

@armen armen force-pushed the master branch 3 times, most recently from 39386fe to 82a7d91 Compare May 14, 2015 23:59
@armen
Copy link
Author

armen commented May 15, 2015

@md5 I've removed tar step and I think it'll work

@yosifkit But I can't see how they would be affected?

If you mean this:

RUN docker-php-ext-configure gd --with-png-dir=/usr --with-jpeg-dir=/usr
RUN docker-php-ext-install pdo pdo_mysql mcrypt gd mbstring

Since the /usr/src/php is deleted at the end of docker-php-ext-install it still will work

@yosifkit
Copy link
Member

Yes, /usr/src/php/ would be deleted on the second RUN but it would have already been committed to the image in the previous layer and so this second layer will apply a "whiteout" and the src will still take up the space.

@armen
Copy link
Author

armen commented May 16, 2015

@yosifkit I see, that makes sense, I thought you mean the change wouldn't be backwards compatible for those users.
@yosifkit Please correct me if I'm wrong, I think this patch would be beneficial in general but certainly the documentation has to be updated to point out your point. docker-php-ext-configure and docker-php-ext-install should be used in a single RUN otherwise the image would grow in size.

RUN docker-php-ext-configure gd --with-png-dir=/usr --with-jpeg-dir=/usr && docker-php-ext-install pdo pdo_mysql mcrypt gd mbstring

@armen
Copy link
Author

armen commented May 16, 2015

@yosifkit On the second thought now I understand what you mean by affected users, they might end up with bigger image with this patch, is that correct?

@md5
Copy link
Contributor

md5 commented May 16, 2015

@armen Yep. I see a few existing Dockerfiles in @yosifkit's search that have the configure and install steps in different RUN directives (as well as some cases of multiple installs in different RUN directives). Those users would indeed end up with bigger images with this change.

@shouze
Copy link
Contributor

shouze commented Jul 7, 2016

ping @armen I've explored another way to do things in #234.

@shouze
Copy link
Contributor

shouze commented Jul 14, 2016

ping @armen as #256 has been merged, I guess you can close this PR ;)

@armen
Copy link
Author

armen commented Jul 14, 2016

Fantastic, thanks @shouze great job

@armen armen closed this Jul 14, 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.

6 participants