Fall back to from utf8_decode to phutil_utf8v for codepoint counting
ClosedPublic
Actions

Authored by alexmv on Jan 12 2017, 3:02 PM.

Details

Reviewers

epriestley

Group Reviewers

Blessed Reviewers

Commits

rPHU6cd38ce75fea: Fall back to from utf8_decode to phutil_utf8v for codepoint counting

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

Test Failures

Build Status

Buildable 15220
Build 20005: Run Core Tests
Build 20004: arc lint + arc unit

Unit TestsFailed
View All

	Time	Test
	0 ms	PhutilUtilsTestCase::Unknown Unit Message ("") Assertion failed, expected values to be equal (at PhutilUtilsTestCase.php:599). Expected: 'syntax error, unexpected $end, expecting ']' in Unknown on line 1' Actual: 'parse error, expecting `']'' in Unknown on line 1'
	0 ms	AbstractDirectedGraphTestCase::Unknown Unit Message ("") 1 assertion passed.
	0 ms	AbstractDirectedGraphTestCase::Unknown Unit Message ("") 1 assertion passed.
	0 ms	AbstractDirectedGraphTestCase::Unknown Unit Message ("") 2 assertions passed.
	0 ms	AbstractDirectedGraphTestCase::Unknown Unit Message ("") 1 assertion passed.
		View Full Test Results (1 Failed · 89 Passed)

Event Timeline

alexmv created this revision.Jan 12 2017, 3:02 PM

Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptJan 12 2017, 3:02 PM

Herald added a subscriber: epriestley. · View Herald Transcript

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?