Restore HHVM support
thekid opened this issue · 21 comments
$ XP_RT=hhvm xp -v
Fatal error: Uncaught exception 'Exception' with message 'This version of the XP Framework
requires PHP 7.0.0+, have PHP 5.6.99-hhvm'
... the upcoming 3.11.0 stable release will be the first release of HHVM with support for the major PHP 7 features.
(source: http://hhvm.com/blog/10859/php-7-support)
This means the checks should be on:
- PHP_VERSION_ID >= 70000
- defined('HHVM_VERSION_ID') && HHVM_VERSION_ID >= 31100
Also, the config setting hhvm.php7.all needs to be set to 1
By the way, installing HHVM on Bash on Windows works exactly as documented for Ubuntu
Let's see how this does on Travis-CI...
OK, here are some of the issues:
Throwable
namespace lang; class ... extends Throwable
raises: Fatal error: Uncaught Error: Class lang\T may not inherit from interface (__SystemLib\Throwable). HHVM seems to prefer its builtin classes over the Throwable
class declared in the lang
namespace. Fixed by fully qualifying parent class names
HHVM doesn't catch exceptions when using catch (\Throwable $t)
- unlike PHP, the Throwable type is not inside global namespace - can be fixed by adding catch (\__SystemLib\Throwable $t)
Strict mode
HHVM uses strict mode by default, causing warnings like preg_match_all() expects parameter 2 to be string, null given
- needs a couple of safeguards or casts
Reflection access to fields is broken in HHVM (due to the strict mode):
$ XP_RT=hhvm xp -w '
class A { public $f; }
$r= new ReflectionProperty("A", "f");
return $r->getValue(new A())
'
Fatal error: Uncaught TypeError: Argument 2 passed to hphp_get_property()
must be an instance of string, null given
See https://docs.hhvm.com/hack/reference/function/hphp_get_property/
HHVM branched off to feature/hhvm-7
branch, too much of a hassle right now.
- PHP version ID: as you found, needs the ini flag setting
- Throwable: should be fixed in 3.20 - if not, please file an issue against HHVM and mention me in the comment: facebook/hhvm@29fc86f
- Strict mode: should also be fixed in 3.20 - if not, file an issue, but I'm not the right person for it: facebook/hhvm@f02a287
Sadly, travis is a pain to use with HHVM: HHVM's only supported by Travis on Ubuntu 14.04, and recent versions of HHVM require 16.04 or newer. You can use docker, but this would likely require a change to your PHP testing too, so might not be worth it.
Docker travis example:
https://github.com/facebook/xhp-lib/blob/master/.travis.yml
https://github.com/facebook/xhp-lib/blob/master/.travis.sh
Strict mode: should also be fixed in 3.20 - if not, file an issue
There actually already is one for it: facebook/hhvm#7709
Throwable: should be fixed in 3.20
It is, thanks @fredemmott!
PHP version ID: as you found, needs the ini flag setting
Nice, HHVM 3.20 now also returns a PHP7-ish version number (if hhvm.php7.all = 1) instead of 5.6.99
$ hhvm --version
HipHop VM 3.20.2 (rel)
Compiler: tags/HHVM-3.20.2-0-gad133f299ece0e45db8536473a8ef77cc4612ae3
Repo schema: 962ff68f0831a12b292f0eb9acd17f705a904446
$ XP_RT=hhvm xp -w '[PHP_VERSION_ID, phpversion()]'
[70199, "7.1.99-hhvm"]
Giving it a try:
$ XP_RT=hhvm xp -cp test.xar xp.unittest.TestRunner src/test/config/unittest/core.ini
# ...
terminate called without an active exception
Core dumped: Aborted
Stack trace in /tmp/stacktrace.1108.log
$ cat /tmp/stacktrace.1108.log
Host: slate
ProcessID: 1108
ThreadID: 140197943919552
ThreadPID: 1108
Name: /usr/bin/xp
Type: Aborted
Runtime: hhvm
Version: tags/HHVM-3.20.2-0-gad133f299ece0e45db8536473a8ef77cc4612ae3
DebuggerCount: 0
Arguments: /usr/bin/class-main.php xp.unittest.TestRunner src/test/config/unittest/core.ini
ThreadType: CLI
# 0 0000000003508796
# 1 0000000001167f92
# 2 00007f825e971390
# 3 00007f8257d95428
# 4 00007f8257d9702a
# 5 00007f82586ef84d
# 6 00007f82586ed6b6
# 7 00007f82586ed701
# 8 00000000011c59e2
# 9 0000000010bc02a4
# 10 000000000a1c829c
# 11 000000000640029e
# 12 0000000002000554
# 13 00000000020f321a
# 14 00000000010caea8
# 15 0000000002cab277
# 16 000000000a07377c
# 17 000000000a0736bd
# 18 000000000a072c2e
# 19 000000000a072485
# 20 000000000a06f720
# 21 000000000e0000c0
# 22 000000000e0000c0
# 23 000000000e0000c0
# 24 000000000640029e
# 25 0000000002000554
# 26 00000000020f321a
# 27 00000000010cb505
# 28 00000000010cbba1
# 29 0000000003510348
# 30 00000000035106c9
# 31 00000000013d575b
# 32 00000000013d5fcb
# 33 00000000013eae07
# 34 00000000013eca37
# 35 00000000011b7938
# 36 0000000000a10f38
# 37 00007f8257d80830
# 38 0000000000a0c0d9
PHP Stacktrace:
#0 lang\Throwable::printStackTrace() called at [/mnt/.../src/test/php/net/xp_framework/unittest/core/ExceptionsTest.class.php:118]
#1 net\xp_framework\unittest\core\ExceptionsTest->printStackTrace()
#2 ReflectionMethod->invokeArgs() called at [/mnt/.../src/main/php/lang/reflect/Method.class.php:88]
#3 lang\reflect\Method->invoke() called at [xar:///mnt/.../test.xar?unittest/TestSuite.class.php:317]
#4 unittest\TestSuite->runInternal() called at [xar:///mnt/.../test.xar?unittest/TestSuite.class.php:529]
#5 unittest\TestSuite->run() called at [xar:///mnt/.../test.xar?xp/unittest/TestRunner.class.php:376]
#6 xp\unittest\TestRunner->run() called at [xar:///mnt/.../test.xar?xp/unittest/TestRunner.class.php:386]
#7 xp\unittest\TestRunner::main() called at [/usr/bin/class-main.php:374]
Can be reproduced with this simpler case:
$ XP_RT=hhvm xp -e '
use io\streams\{Streams, MemoryOutputStream};
$out= new MemoryOutputStream();
fwrite(Streams::writeableFd($out), "Test")
'
terminate called without an active exception
Core dumped: Aborted
Stack trace in /tmp/stacktrace.1173.log
...and can be worked around as follows:
$ git diff
diff --git a/src/main/php/io/streams/Streams.class.php b/src/main/php/io/streams/Streams.class.php
index 18eb641..89524ee 100755
--- a/src/main/php/io/streams/Streams.class.php
+++ b/src/main/php/io/streams/Streams.class.php
@@ -61,7 +61,7 @@ abstract class Streams {
}
public function stream_flush() {
- return self::$streams[$this->id]->flush();
+ return parent::$streams[$this->id]->flush();
}
public function stream_eof() {
With that patch applied, here's how we fare:
XP_RT=hhvm xp -cp test.xar xp.unittest.TestRunner src/test/config/unittest/core.ini
# ...
×: 2122/2196 run (74 skipped), 2053 succeeded, 69 failed
Memory used: 6144.00 kB (6144.00 kB peak)
Time taken: 15.074 seconds
Additionally, by setting hhvm.hack.lang.look_for_typechecker = 0
in hhvm.ini, I can prevent could not find a .hhconfig file errors, and get this:
XP_RT=hhvm xp -cp test.xar xp.unittest.TestRunner src/test/config/unittest/core.ini
# ...
×: 2122/2196 run (74 skipped), 2069 succeeded, 53 failed
Memory used: 6144.00 kB (6144.00 kB peak)
Time taken: 11.620 seconds
After opening facebook/hhvm#7878 (anonymous classes and self::
reference) and working around it (see eeb3fa5), we're now here:
XP_RT=hhvm xp -cp test.xar xp.unittest.TestRunner src/test/config/unittest/core.ini
# ...
×: 2121/2196 run (75 skipped), 2113 succeeded, 8 failed
Memory used: 4096.00 kB (4096.00 kB peak)
Time taken: 51.356 seconds
Reasons these 8 tests are broken:
- hphp_(get|set)_property problem - see facebook/hhvm#7709
- Void return type missing - see facebook/hhvm#7740
- Parameter $args is variadic and has a type constraint (HH\string); variadic params with type constraints are not supported in non-Hack files - see facebook/hhvm#6954
The test net.xp_framework.unittest.io.streams.Bz2CompressingOutputStreamTest fails because the stream wrapper doesn't honor the blocks setting:
stream_filter_append($this->out, 'bzip2.compress', STREAM_FILTER_WRITE, ['blocks' => $level])
// ^^^^^^^^^^^^^^^^^^^^
// This
Worked around by ignoring; opened facebook/hhvm#7879
Added a workaround for the hphp_(get|set)_property problem by calling them directly. In essence, this is what I'm doing:
if (defined('HHVM_VERSION_ID')) {
self::$read= function($class, $reflect, $instance, $public) {
if (null === $instance) {
return hphp_get_static_property($class, $reflect->name, !$public);
} else {
return hphp_get_property($instance, $class, $reflect->name);
}
};
self::$write= function($class, $reflect, $instance, $value, $public) {
if (null === $instance) {
return hphp_set_static_property($class, $reflect->name, $value, !$public);
} else {
return hphp_set_property($instance, $class, $reflect->name, $value);
}
};
} else {
self::$read= function($class, $reflect, $instance, $public) {
$public || $reflect->setAccessible(true);
return $reflect->getValue($instance);
};
self::$write= function($class, $reflect, $instance, $value, $public) {
$public || $reflect->setAccessible(true);
return $reflect->setValue($instance, $value);
};
}
Adding a small overhead with the closure function call, though.
Documentation for these functions is available in ext/reflection/ext_reflection-internals-functions.php
This gets us to:
$ XP_RT=hhvm xp -cp test.xar xp.unittest.TestRunner src/test/config/unittest/core.ini
# ...
×: 2121/2196 run (75 skipped), 2116 succeeded, 5 failed
Memory used: 4096.00 kB (4096.00 kB peak)
Time taken: 49.872 seconds
Added @action(new IgnoredOnHHVM())
to the tests for void return types and variadics with type as long as they're not supported (see facebook/hhvm#7740 and facebook/hhvm#6954) in 9868a57
This gets us to:
$ XP_RT=hhvm xp -cp test.xar xp.unittest.TestRunner src/test/config/unittest/core.ini
# ...
×: 2117/2196 run (79 skipped), 2115 succeeded, 2 failed
Memory used: 4096.00 kB (4096.00 kB peak)
Time taken: 49.337 seconds
"Cherry-picked" c5a7000 to fix variadic parameters always returning array
on HHVM in 569b3a1. This gets us to:
$ XP_RT=hhvm xp -cp test.xar xp.unittest.TestRunner src/test/config/unittest/core.ini
# ...
×: 2117/2196 run (79 skipped), 2116 succeeded, 1 failed
Memory used: 4096.00 kB (4096.00 kB peak)
Time taken: 50.479 seconds
The only test failing now is:
F unittest.TestAssertionFailed(test= ErrorsTest::call_to_member_on_non_..., time= 0.046 seconds) {
unittest.AssertionFailedError{ Caught Exception
lang.XPException (Call to a member function method() on a non-object (null))
instead of expected lang.Error
}
at unittest.TestSuite::run() [line 376 of TestRunner.class.php]
at xp.unittest.TestRunner::run() [line 386 of TestRunner.class.php]
at xp.unittest.TestRunner::main() [line 374 of class-main.php]
}
This resulted from PHP throwing an Error
(which gets wrapped into a lang.Error), while HHVM throws as BadMethodCallException
(which gets wrapped into a lang.XPException). Addressed this in lang.Throwable::wrap()
, which now does:
if ($e instanceof self) {
return $e;
} else if ($e instanceof \BadMethodCallException) { // This condition is new:
$wrapped= new Error($e->getMessage(), $e->getPrevious(), false); // Wrap in lang.Error
} else if ($e instanceof \Exception) {
$wrapped= new XPException($e->getMessage(), $e->getPrevious(), false);
} else if ($e instanceof \Throwable) {
$wrapped= new Error($e->getMessage(), $e->getPrevious(), false);
} else {
throw new IllegalArgumentException('Given argument ...');
}
And now:
$ XP_RT=hhvm xp -cp test.xar xp.unittest.TestRunner src/test/config/unittest/core.ini
# ...
♥: 2117/2196 run (79 skipped), 2117 succeeded, 0 failed
Memory used: 4096.00 kB (4096.00 kB peak)
Time taken: 50.533 seconds
Minimum HHVM version required is HHVM 3.20
Example for older HHVM versions:
$ XP_RT=hhvm xp -v
Fatal error: Uncaught exception 'Exception' with message
'This version of the XP Framework requires HHVM 3.20+, have HHVM 3.18.0'
HHVM branched off to feature/hhvm-7 branch, too much of a hassle right now.
This branch has now been removed again, HHVM support is back; unlike other frameworks, the XP Framework will continue to support HHVM for the time being.
I agree with the authors of HHVM and MongoDB and Symfony 4: End of HHVM support that HHVM is not used the majority of users and that PHP7 has caught up performance-wise in the meantime. However, the HHVM team seems to have been putting quite a bit of effort in to reducing the PHP 7 incompatibilities lately; and one of the XP Framework's goals has been to operate on as many platforms and environments as possible.
See also:
- facebook/hhvm#7198 - HHVM and PHP7 differ on type annotations on internal functions (rtrim in particular) - fixed
- facebook/hhvm#7869 - Support composer in PHP7 mode - fixed
- facebook/hhvm#7876 - Support PHPUnit in PHP7 mode
- symfony/symfony#22758 - Remove HHVM support
- doctrine/orm#6424 - HHVM support
- phpmd/phpmd#473 - Put down Support for HHVM
- joomla/joomla-cms#16232 - remove hhvm ci task
- cakephp/cakephp#10674 - Drop support for HHVM officially?
Here are some problems from running the entire test suite:
F unittest.TestAssertionFailed(test= net.xp_framework.unittest.io.PathTest::normalize_uppercases_drive_letter, time= 0.072 seconds) {
unittest.AssertionFailedError{ expected ["C:/test"] but was ["c:/test"] using: 'equals' }
at unittest.TestCase::assertEquals() [line 314 of PathTest.class.php]
at net.xp_framework.unittest.io.PathTest::normalize_uppercases_drive_letter() [line 0 of StackTraceElement.class.php]
at ReflectionMethod::invokeArgs() [line 88 of Method.class.php]
at lang.reflect.Method::invoke() [line 317 of TestSuite.class.php]
at unittest.TestSuite::runInternal() [line 529 of TestSuite.class.php]
at unittest.TestSuite::run() [line 376 of TestRunner.class.php]
at xp.unittest.TestRunner::run() [line 386 of TestRunner.class.php]
at xp.unittest.TestRunner::main() [line 374 of class-main.php]
}
F unittest.TestAssertionFailed(test= net.xp_framework.unittest.io.PathTest::relative_to("c:/Windows", "C:/", "Windows"), time= 0.002 seconds) {
unittest.AssertionFailedError{ expected ["Windows"] but was ["../../c:/Windows"] using: 'equals' }
at unittest.TestCase::assertEquals() [line 353 of PathTest.class.php]
at net.xp_framework.unittest.io.PathTest::relative_to() [line 0 of StackTraceElement.class.php]
at ReflectionMethod::invokeArgs() [line 88 of Method.class.php]
at lang.reflect.Method::invoke() [line 317 of TestSuite.class.php]
at unittest.TestSuite::runInternal() [line 529 of TestSuite.class.php]
at unittest.TestSuite::run() [line 376 of TestRunner.class.php]
at xp.unittest.TestRunner::run() [line 386 of TestRunner.class.php]
at xp.unittest.TestRunner::main() [line 374 of class-main.php]
}
F unittest.TestAssertionFailed(test= net.xp_framework.unittest.io.PathTest::relative_to("C:\Windows", "c:", "Windows"), time= 0.000 seconds) {
unittest.AssertionFailedError{ expected ["Windows"] but was ["../C:\Windows"] using: 'equals' }
at unittest.TestCase::assertEquals() [line 353 of PathTest.class.php]
at net.xp_framework.unittest.io.PathTest::relative_to() [line 0 of StackTraceElement.class.php]
at ReflectionMethod::invokeArgs() [line 88 of Method.class.php]
at lang.reflect.Method::invoke() [line 317 of TestSuite.class.php]
at unittest.TestSuite::runInternal() [line 529 of TestSuite.class.php]
at unittest.TestSuite::run() [line 376 of TestRunner.class.php]
at xp.unittest.TestRunner::run() [line 386 of TestRunner.class.php]
at xp.unittest.TestRunner::main() [line 374 of class-main.php]
}
These are caused by facebook/hhvm#7880, but can (and have been) easily be worked around:
- if (2 === sscanf($this->path, '%c%*[:]', $drive)) {
+ if (strlen($this->path) > 1 && ':' === $this->path{1}) {
OK, Travis-CI gives us:
$ hhvm --version
HipHop VM 3.18.3 (rel)
Compiler: tags/HHVM-3.18.3-0-g1ddd4af7b1342c20635afbdc67701fbcbdf97bfa
Repo schema: 61b13efcfd4e2fabea72495f96208746a568a688
See https://travis-ci.org/xp-framework/core/jobs/241581199
This won't work, neither will hhvm-nightly
, which is even older(!)
Here's the code that replaces HHVM with a Docker-based version - inspired by @fredemmott's solution above.
wrap() {
local image="$1"
local cmd="$2"
local target="$3"
local wrapper=$(basename $target).in
local wd=$(pwd)
mv $target $wrapper
echo "#!/bin/sh" > $target
echo "docker run --rm -v $wd:/opt/src -v $wd/php.ini:/etc/hhvm/php.ini -w /opt/src $image $cmd $wrapper \$@" >> $target
chmod 755 $target
}
replace_hhvm_with() {
local version="$1"
printf "\033[33;1mReplacing HHVM\033[0m\n"
docker pull hhvm/hhvm:$version
docker run --rm hhvm/hhvm:$version hhvm --version
echo
echo "hhvm.php7.all = 1" > php.ini
echo "hhvm.hack.lang.look_for_typechecker = 0" >> php.ini
wrap hhvm/hhvm:$version "hhvm --php" /home/travis/.phpenv/versions/hhvm/bin/composer
wrap hhvm/hhvm:$version "sh" xp-run
}
# Run HHVM inside Docker as the versions provided by Travis-CI are too old
case "$TRAVIS_PHP_VERSION" in
hhvm-nightly*)
replace_hhvm_with "latest"
;;
hhvm*)
replace_hhvm_with "3.20.1"
;;
esac
HHVM Versions can be found @ https://hub.docker.com/r/hhvm/hhvm/tags/. Currently latest equals 3.20.1, so both hhvm
and hhvm-nightly
will use the same HHVM version. This will change one newer HHVM versions are released by Facebook.
See https://travis-ci.org/xp-framework/core/builds/241608653
Interesting:
PHP7 is charting a new course away from PHP5, and we want to do the same, via a renewed focus on Hack. Consequently, HHVM will not aim to target PHP7. The HHVM team believes that we have a clear path toward making Hack a fantastic language for web development, untethered from its PHP origins.