-
-
Notifications
You must be signed in to change notification settings - Fork 825
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
Add install and runtime status warnings if MySQL utf8mb4 is not supported #13425
Conversation
(Standard links)
|
…rted. This will allow CiviCRM to require utf8mb4 support at a future date. See also dev/mail#37 dev/core#339 dev/core#392
I'm in favour of the outcome sought by this PR. Would it be worthwhile to see testing on a diverse set of instances before merge? I don't know the options on all of the versions of MySQL out there http://stats.civicrm.org/?tab=technology (there are a fair number of Percona installs too). |
The test query is part of Drupal 7 so it's already pretty well tested at this point. The install requirements there are MySQL >= 5.0.15, MariaDB >= 5.1.44 or Percona Server >= 5.1.70 |
Eileen requested additional community input. I'm a +1 on this. I also imagine it's fairly rare that this will be triggered - most distros that ship a version of MySQL/etc. that Civi supports will support utf8mb4 out of the box - but that's perhaps why it IS important to catch this for configs which aren't supported. |
LGTM. Flagging future compatibility issues as early as possible is a great approach. 👍 |
Merging based on community review. |
This check seems to cause a fatal error on a client site during login. Backtrace below:
|
@mattwire Can you open a gitlab issue and cross-ref here? (what version of mysql?) |
This falls over on key length for a VARCHAR(255) field rather than unavailability of utf8mb4 per se. I imagine that will happen on many environments. The server where it's reported here is CentOS 6 with cPanel. I have seen a similar error in a different context when setting up a site locally on Debian 9 (Stretch) with MariaDB 10.1.37 . There I resolved it by adding this to MariaDB config:
That's not default on either of the above environments so I don't share the optimism that it's fairly rare that this will be triggered. I understand that the code is intended to catch the exception and a fix has been submitted for this at #13682, so we shouldn't see a fatal error going forward. Just wanted to offer the above as evidence about the likelihood of environments not passing the check. |
This should issue a warning on an out-of-the-box configuration of MySQL < 5.7 and MariaDB < 10.2 |
Overview
At a future date, it would be a good idea for CiviCRM to require utf8mb4 support and migrate the schema to utf8mb4.
To work towards this goal, we should add install and runtime status warnings if MySQL utf8mb4 is not supported.
Before
Civi instances could have a setup that doesn't support utf8mb4 (i.e. versions of MySQL that require additional configuration).
After
A warning is displayed on the system status page, as well as at install time, if utf8mb4 is not supported by the stack.
Technical Details
This is incredibly redundant, as there seem to be multiple places where requirement checks are made. If it makes sense to abstract it and put the code in one place, I would gladly do so.
Comments
See also discussion @ https://lab.civicrm.org/dev/mail/issues/37 https://lab.civicrm.org/dev/core/issues/339 https://lab.civicrm.org/dev/core/issues/392