-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Conversation
I was going to ask whether this breaks the LGTM |
Nice idea! LGTM |
Hmmm... it looks like a good idea, but I see a problem. After running 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 |
|
It seems like just removing the 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 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 |
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.
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. |
39386fe
to
82a7d91
Compare
@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:
Since the /usr/src/php is deleted at the end of docker-php-ext-install it still will work |
Yes, |
@yosifkit I see, that makes sense, I thought you mean the change wouldn't be backwards compatible for those users.
|
@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? |
Fantastic, thanks @shouze great job |
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.