Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[1.x] Install "mariadb-client" package for MariaDB users #693

Merged
merged 1 commit into from
May 16, 2024

Conversation

staudenmeir
Copy link
Contributor

@staudenmeir staudenmeir commented May 15, 2024

This PR is related to laravel/framework#51355:
We need to switch the MariaDB's driver dump binary from mysqldump to mariadb-dump at some point as MariaDB will remove its support for mysqldump. The issue is that Sail doesn't support mariadb-dump yet.

I looked into using the dump binaries from the actual mysql/mariadb containers, but that's a nightmare.

Instead, my suggestion is to install the mariadb-client package instead of mysql-client in the app container when the MariaDB service is requested. This package provides the necessary mariadb-dump binary.

As discussed with @driesvints in the other PR, we should only switch to mariadb-dump in Laravel 12 and this PR prepares Sail for that switch. I only adjusted the Dockerfile for PHP 8.3 as I assumed that Laravel 12 will drop support for PHP 8.2.

I don't see any breaking changes for Sail users:

  • With the default value MYSQL_CLIENT="mysql-client", nothing changes for existing users after updating to the latest version.
  • For Sail users that set up a MariaDB app after this PR is released, the dump command will continue working since the mariadb-client package creates a symlink mysqldump -> mariadb-dump.

@driesvints What do you think about this approach?

Copy link

Thanks for submitting a PR!

Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

@driesvints
Copy link
Member

I like this approach @staudenmeir, nice work!

@driesvints driesvints marked this pull request as ready for review May 16, 2024 07:49
@driesvints driesvints requested a review from taylorotwell May 16, 2024 07:49
@taylorotwell taylorotwell merged commit a8e4e74 into laravel:1.x May 16, 2024
5 checks passed
@staudenmeir staudenmeir deleted the mariadb-client branch May 16, 2024 21:46
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants