xp-framework/core

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:

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(!)

Restored unittests success by running HHVM in Docker:

hhvm-in-docker-on-travis

https://travis-ci.org/xp-framework/core/jobs/241593079

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.

http://hhvm.com/blog/2017/09/18/the-future-of-hhvm.html