yiisoft / yii2-docker

Official Docker images suitable for Yii 2.0
https://www.yiiframework.com/
BSD 3-Clause "New" or "Revised" License
384 stars 202 forks source link

Feature/build with mlocati #132

Closed schmunk42 closed 3 years ago

schmunk42 commented 3 years ago

fixes #131

tfrancisci commented 3 years ago

This change has removed the mysql package which is now breaking our builds in other ways. Was this intended? :)

schmunk42 commented 3 years ago

Not at all :)

The requirements check passes?! https://github.com/yiisoft/yii2-docker/runs/2508946919?check_suite_focus=true#step:7:69

tfrancisci commented 3 years ago

The mysql PHP extension is there, but not the mysql client itself...

The old image (From March 25th)

docker run --rm -ti yiisoftware/yii2-php:7.4-apache-21.3.1 mysql --version
usermod: no changes
mysql  Ver 15.1 Distrib 10.3.27-MariaDB, for debian-linux-gnu (x86_64) using readline 5.2

The image from this morning

docker run --rm -ti yiisoftware/yii2-php:7.4-apache mysql --version
usermod: no changes
/usr/local/bin/docker-php-entrypoint: 28: exec: mysql: not found
schmunk42 commented 3 years ago

So the PHP extension should still be there, but the mysql client is missing, right?

> docker run --rm -ti yiisoftware/yii2-php:7.4-apache php -i | grep mysql
Configure Command =>  './configure'  '--build=x86_64-linux-gnu' '--with-config-file-path=/usr/local/etc/php' '--with-config-file-scan-dir=/usr/local/etc/php/conf.d' '--enable-option-checking=fatal' '--with-mhash' '--with-pic' '--enable-ftp' '--enable-mbstring' '--enable-mysqlnd' '--with-password-argon2' '--with-sodium=shared' '--with-pdo-sqlite=/usr' '--with-sqlite3=/usr' '--with-curl' '--with-libedit' '--with-openssl' '--with-zlib' '--with-pear' '--with-libdir=lib/x86_64-linux-gnu' '--with-apxs2' '--disable-cgi' 'build_alias=x86_64-linux-gnu'
/usr/local/etc/php/conf.d/docker-php-ext-pdo_mysql.ini,
mysqlnd
mysqlnd => enabled
Version => mysqlnd 7.4.18
Loaded plugins => mysqlnd,debug_trace,auth_plugin_mysql_native_password,auth_plugin_mysql_clear_password,auth_plugin_caching_sha2_password,auth_plugin_sha256_password
API Extensions => pdo_mysql
PDO drivers => sqlite, mysql, pgsql
pdo_mysql
Client API version => mysqlnd 7.4.18
pdo_mysql.default_socket => no value => no value

It breaks BC (unintended), but I am not sure if we should readd all the database clients.

@yiisoft/reviewers Thoughts?

bizley commented 3 years ago

I would keep them out. Images should be as slim as possible, not everyone needs the clients.

tfrancisci commented 3 years ago

Our pipeline was created by a vendor who, I guess, saw that it was present and so used it. We are now moving our mysql client tasks to use a different image anyway (I agree that it does not make sense to leave them in here). We can move on from this problem now.

Thanks for your work in maintaining this repo!

schmunk42 commented 3 years ago

Our pipeline was created by a vendor who, I guess, saw that it was present and so used it. We are now moving our mysql client tasks to use a different image anyway (I agree that it does not make sense to leave them in here). We can move on from this problem now.

Thanks for your work in maintaining this repo!

You should be able to re-add it just by (or similar)

RUN apt-get install -y mysql-client
marcovtwout commented 3 years ago

@schmunk42 Thanks for the pointer.

Here's an example when building on Debian 10:

RUN apt-get update && apt-get install -y default-mysql-client