Make libphutil compatible with utf8mb4
ClosedPublic
Actions

Authored by epriestley on Sep 28 2014, 9:12 PM.

Details

Reviewers

btrahan

Maniphest Tasks

T1191: Comments are truncated at first non-base-plane character

Commits

rPHU29136aa2b421: Make libphutil compatible with utf8mb4

Summary

Ref T1191. Two changes here:

Try to use the utf8mb4 character set. Fall back to utf8 if it's not available. This may be slightly worse for old MySQL, but is far, far easier than trying to keeep a cache around somewhere with availability information.
Prepare to transition to utf8mb4. I'm just going to switch this over in the future. I considered trying to save the migration state in the environment somewhere, but it gets really, really messy because we have to inspect the migration state in order to build a connection object. There's also no advantage in the overwhelming majority of cases. We can do something more messy here eventually if it ends up being more complicated than I think.

Test Plan

Wrote comments full of emoji locally.

Diff Detail

Repository

rPHU libphutil

Branch

utf8mb4

Lint

Lint Errors

Severity	Location	Code	Message
Error	src/aphront/storage/connection/mysql/AphrontMySQLiDatabaseConnection.php:136	XHP45	PHP Compatibility
Advice	src/aphront/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php:327	XHP16	TODO Comment

Unit

No Test Coverage

Build Status

Buildable 2694
Build 2698: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

epriestley updated this revision to Diff 25441.Sep 28 2014, 9:12 PM

epriestley retitled this revision from to Make libphutil compatible with utf8mb4.

epriestley updated this object.

epriestley edited the test plan for this revision. (Show Details)

epriestley added a reviewer: btrahan.

epriestley added a task: T1191: Comments are truncated at first non-base-plane character.

Herald added a subscriber: epriestley. · View Herald TranscriptSep 28 2014, 9:12 PM

Harbormaster completed remote builds in B2694: Diff 25441.Sep 28 2014, 9:13 PM

btrahan accepted this revision.Sep 30 2014, 4:56 PM

btrahan edited edge metadata.

btrahan added inline comments.

src/aphront/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php
339	there's an extra %

This revision is now accepted and ready to land.Sep 30 2014, 4:56 PM

epriestley added inline comments.Sep 30 2014, 5:02 PM

src/aphront/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php
339	This is intentional, since we don't want `pht()` to interpret `%B` as a conversion.

btrahan added inline comments.Sep 30 2014, 5:09 PM

src/aphront/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php
339	oh, whoops, righto

Closed by commit rPHU29136aa2b421: Make libphutil compatible with utf8mb4 (authored by epriestley, committed by epriestley).Oct 1 2014, 3:48 PM

This revision was automatically updated to reflect the committed changes.

@epriestley, @btrahan - wasn't sure if I should open a new task for this or put it here

src/aphront/storage/connection/mysql/AphrontMySQLiDatabaseConnection.php
70–73	This is causing massive haemorrhage when it fails... (Server version: 5.1.73) In DarkConsole: mysqli::set_charset() [<a href='mysqli.set-charset'>mysqli.set-charset</a>]: Error executing query at [AphrontMySQLiDatabaseConnection.php:70] (12 times) Same when running CLI scripts.

Can you clarify what "massive haemorrhage" means? Just errors in the log, or is the page fataling?

If it's errors, can you try adding @ in front of the call line, like this?

$ok = @$conn->set_charset('utf8mb4');
     ^^^

I can't reproduce any issues locally (I have utf8mb4, but setting it to "zebra" or whatever) -- the call fails and returns an error, but doesn't spew. Could just be version differences or whatever, though.