Page MenuHomePhabricator

Fall back to from utf8_decode to phutil_utf8v for codepoint counting
ClosedPublic

Authored by alexmv on Jan 12 2017, 3:02 PM.
Tags
None
Referenced Files
F14441323: D17188.id41333.diff
Thu, Dec 26, 6:34 AM
Unknown Object (File)
Thu, Dec 12, 6:46 AM
Unknown Object (File)
Sun, Dec 8, 2:52 AM
Unknown Object (File)
Thu, Dec 5, 1:42 AM
Unknown Object (File)
Thu, Nov 28, 5:43 PM
Unknown Object (File)
Nov 22 2024, 11:41 AM
Unknown Object (File)
Nov 22 2024, 10:21 AM
Unknown Object (File)
Nov 21 2024, 5:37 PM
Subscribers

Details

Summary

The php7.0 package in Ubuntu 16.04 splits out the php7.0-xml package, which
provides the utf8_decode function. As such, parts of Arcanist throw
exceptions -- most notably PhutilArgumentSpellingCorrector:

$ arc dieff
[2017-01-12 14:57:25] EXCEPTION: (Error) Call to undefined function utf8_decode() at [<phutil>/src/utils/utf8.php:259]
arcanist(head=master, ref.master=d6e112aecf93), phutil(head=master, ref.master=ab80dcf99c6b)
  #0 phutil_utf8_strlen(string) called at [<phutil>/src/parser/argument/PhutilArgumentSpellingCorrector.php:120]
  #1 PhutilArgumentSpellingCorrector::correctSpelling(string, array) called at [<arcanist>/src/configuration/ArcanistConfiguration.php:148]
  #2 ArcanistConfiguration::selectWorkflow(string, array, ArcanistConfigurationManager, PhutilConsole) called at [<arcanist>/scripts/arcanist.php:193]

Since spelling correction is a core feature, and working out-of-the-box is
desirable, fall back to implementing phutil_utf8_strlen in terms of
phutil_utf8v if decode_utf8 is not present.

Fixes: T11713.

Test Plan

Ran arc dieff on an Ubuntu 16.04 machine after `apt-get
install php7.0-cli`, as well as on PHP 5.5.

Diff Detail

Repository
rPHU libphutil
Branch
up-utf8decode
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 15224
Build 20012: Run Core Tests
Build 20011: arc lint + arc unit

Event Timeline

Just remove the line 599 assertEqual() on the error message to make the test suite pass? You could replace the values of $invalid_cases with a human-readable description, or remove them entirely. I don't think we should be testing exact error message text, as this is something we can reasonably expect to be fairly fragile across versions.

This revision now requires changes to proceed.Jan 12 2017, 3:09 PM

I think that's unrelated to this change, and purely based on the yacc version in the environment. Mind if I split that out into a separate diff?

This revision is now accepted and ready to land.Jan 12 2017, 3:13 PM

Rebase and re-run unit tests.

This revision was automatically updated to reflect the committed changes.