Tests is failing on Alpine with V8 >= 5.7.434 about php-v8

[pinepain]

Hi! Thank you for reporting. Sorry for late reply, life was too busy past week.

Can you please post what exactly fails in that tests: I have test-report.sh which I use to show output of failed tests: make test | tee result.txt && test-report.sh should do the job.

From your Docker file alpine-libv8, it looks like that you don't copy icudtl.dat, which php-v8 expects to have (and from build flags it looks like that v8 build with icu, which is default behavior), so I guess this is the cause of an issue with failed tests. As tests that doesn't init V8 passes, it also makes me thing that missed ICU data file is the cause of tests failure.

You can give a look how I package libv8 for ubuntu and macOS (homebrew).

In latest changes I make php-v8 be quite demanding to libv8 and it expects vanilla build, which has own ICU. While it's a nice thing, as before I faced with a minor problems as icu versions was different across ubuntu versions and on macOS, it also has a drawback for our case - it has it data as a standalone file rather then built-in. As v8js expects non-icu libv8 and it doesn't initialize it, it should works just fine, but only with non-icu libv8 builds, thought, I'm not quite sure about that, so to validate my assumption, you can run some of Intl.DateTimeFormat examples with v8js.

(RU)
Огромное спасибо! Отписал на указанный в гитхаб профиле почтовый ящик.

[AlexMasterov]

Thanks for the reply.

gcc 6.3.0-r1
icu-dev 58.2-r1
alpine-libv8: Add icudtl.dat in shared libraries + icu_use_data_file=true (Default)

test-result.log
test-report.log

(RU)
Постараюсь позже ответить подробней на почту. Флаги протестирую.

[pinepain]

Hi, thanks for detailed logs!

002+ # Fatal error in ../../src/snapshot/natives-external.cc, line 122
003+ # Check failed: holder_.

Add v8_use_external_startup_data=0 to build flags

TL;DR

It looks like you have startup data as external file, php-v8 expect libv8 built with v8_use_external_startup_data=0. At this time I don't detect external snapshot in build script as having built-in startup data offers better performance and I can't easily come up with a generic use case when end-users will alter external startup data, especially from PHP userland. V8js, is more lenient to libv8 options and it try to detect such variations. While the same approach for blobs detection could be added to php-v8, I'm not fond of supporting of libv8 builds variability at this time, you'd better just add v8_use_external_startup_data=0 to build flags and the issue should be resolved for this particular case.

[AlexMasterov]

alpine-libv8: Add "v8_use_external_startup_data=false" to build flags

New report looks better.

test-result.log
test-report.log

=====================================================================
FAILED TEST SUMMARY
---------------------------------------------------------------------
V8\DateObject [tests/V8DateObject.phpt]
V8\Int32Value [tests/V8Int32Value.phpt]
V8\IntegerValue [tests/V8IntegerValue.phpt]
V8\Uint32Value [tests/V8Uint32Value.phpt]

V8DateObject.phpt
date.timezone is empty in php.ini; this is the only problem? v8js ext. has something similar:

=====================================================================
FAILED TEST SUMMARY
---------------------------------------------------------------------
Test V8::registerExtension() : Register extension with errors [tests/extensions_error.phpt]
Test V8::executeString() : Check timezone handling [tests/timezones.phpt]

[pinepain]

Great! Thanks! Awesome work!

Timezone problem comes from environment timezone, last time i was looking into this specific proble, v8 respect TZ env variable, so having it preset to some standard value should fix test result inconsistency. I'll give it a look later today (as well as fixing float/int argument issue).

[pinepain]

I fixed Int*Value tests, they should be fine now.

As to DateObject test, under the hood it also tests timezone change at runtime. From your test output I can assume that somehow internal date cache is not reset during V8\DateObject::DateTimeConfigurationChangeNotification() call. But it may be that Alpine handles timezone differently from other distros or it's some issue with it libc++ or in ICU.

[AlexMasterov]

This report from the latest commit b33c148.
No change. Hm.

test-result.log
test-report.log

[pinepain]

Hm, my fault, "fixed" nonexistent issue then. I reverted that bogus changes.

double: NAN
128+ V8\IntegerValue->Value(): int(0)
128- TypeError: Argument 2 passed to V8\IntegerValue::__construct() must be of the type integer, float given\n

It seems to be that for some reason NAN is integer zero. I'm a bit surprised. What libc++ does Alpine has and which one do you use to build php-v8? Can you check what NAN is in Alpine?

[AlexMasterov]

musl-libc

Can you check what NAN is in Alpine?

float(NAN)
The same output as here: is_nan().

[pinepain]

It seems to be a bug, https://bugs.php.net/bug.php?id=73954.

I guess there is nothing to do in advance with both NAN and timezone changing issues at this time as they both come from external tools. For now you may locally add --XFAIL-- to known to fail tests or ignore that failures. I'll try to keep you updated about a progress, if any.

[AlexMasterov]

@pinepain
Thank you. I will support your report about this bug.

I think this issue can be closed.

btw, is_nan(NAN) returns FALSE on Alpine (musl) 😃👍