You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
just calls the mysql binary which is what generates the warnings about using mysql itself.
The problem is that because mariadb has decided to deprecate calling mysql, we would need to have a way to determine what binary to call before we call one at all. So this will need some thinking as the best way to do that (and then replace the very many uses of mysql in wp-cli so it probably should be in the wp cli project itself).
A simple way is to just assume if mariadb is there to prefer that instead -- but im not sure how accurate that might be. Or to add a new environment variable that lets the user decide?
A simple way is to just assume if mariadb is there to prefer that instead -- but im not sure how accurate that might be. Or to add a new environment variable that lets the user decide?
The former could have some unexpected results if suddenly a different binary is used. So the latter seems safer.
So basically we need /usr/bin/env which mariadb to get the MariaDB, which could be its own util a la get_mariadb_binary_path().
Yea I think something like that is needed! I'm not sure if it is common to have both mariadb and mysql installed -- but even with the deprecated mysql binary that mariadb provides (which causes these warnings) we would still get valid return values for both which mariadb and which mysql so we would need some way of deciding which is the proper one to pick.
So really the problem is on a system that is going to have both a mariadb and mysql binary, what steps do we take to determine the one to use? Is it safe enough to assume that since mariadb provides a mysql binary, but mysql doesn't provide a mariadb binary, that pretty much any case of both existing are going to be a mariadb install?
Maybe a more general get_db_type() check that could include sqlite as well? I don't think we have anything like that but it would be a good first step for either fixing or giving better error messages for most of the commands in this repo ..
Yea I think something like that is needed! I'm not sure if it is common to have both mariadb and mysql installed -- but even with the deprecated mysql binary that mariadb provides (which causes these warnings) we would still get valid return values for both which mariadb and which mysql so we would need some way of deciding which is the proper one to pick.
So really the problem is on a system that is going to have both a mariadb and mysql binary, what steps do we take to determine the one to use? Is it safe enough to assume that since mariadb provides a mysql binary, but mysql doesn't provide a mariadb binary, that pretty much any case of both existing are going to be a mariadb install?
Perhaps we could do this:
If MySQL is not installed but MariaDB is -> use MariaDB
If MySQL is installed and mysql --version says "from 11.7.2-MariaDB" -> use MariaDB
(For this we would run /usr/bin/env mysql --version and simply ignore/silence the deprecation warning)
If MySQL is installed and it's the "real" one -> use MySQL
If MySQL is not installed, well, good luck 😄
This way there shouldn't be any breaking change.
Maybe a more general get_db_type() check that could include sqlite as well? I don't think we have anything like that but it would be a good first step for either fixing or giving better error messages for most of the commands in this repo ..
We could check for SQLite support depending on the SQLITE_DB_DROPIN_VERSION constant. But that of course works only when dealing with an actual WordPress install with the SQLite plugin.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a follow-up to #275 which addressed the
mysqlcheck
andmysqldump
usage, but not usage ofmysql
itself.There was a suggestion to move this utility to the main framework (maybe updating
Utils/get_mysql_binary_path()
?) which we could do in a future step.Then, we could also add a
get_db_type()
util or so to avoid repeating( strpos( Utils\get_mysql_version(), 'MariaDB' ) !== false )
everywhere.