Fix Solaris tests and enable CI #20709
Conversation
psumbera
requested review from
TimWolla,
bukka,
derickr,
devnexen,
dstogov,
kamil-tekiela and
ndossche
as code owners
github-actions
bot
added
Extension: calendar
Extension: ctype
Extension: date
Extension: fileinfo
Extension: iconv
Extension: libxml
Extension: mysqli
Extension: opcache
Extension: pcntl
Extension: posix
Extension: sockets
Extension: standard
Extension: xsl
Extension: zend_test
Extension: zlib
SAPI: cli
labels
psumbera
force-pushed
the
github-solaris-ci
branch
2 times, most recently
from
bbc1c07 to
9c9436d
Compare
|
cc @iluuu1994 at least for the CI's aspect. |
| --SKIPIF-- | ||
| <?php | ||
| if (!function_exists("pcntl_setcpuaffinity")) die("skip pcntl_setcpuaffinity is not available"); | ||
| if (PHP_OS_FAMILY === 'Solaris') die("skip CPU affinity not supported on Solaris"); |
There was a problem hiding this comment.
is the test failing and in which manner ? because as far as I see it is supported by solaris.
There was a problem hiding this comment.
It fails like this:
Running selected tests.
TEST 1/1 [ext/pcntl/tests/pcntl_cpuaffinity.phpt]
========DIFF========
001- bool(true)
002- array(0) {
003- }
004- array(0) {
005- }
006- bool(true)
007- pcntl_setcpuaffinity(): Argument #2 ($cpu_ids) must not be empty
008- pcntl_setcpuaffinity(): Argument #2 ($cpu_ids) cpu id invalid value (def)
009- pcntl_setcpuaffinity(): Argument #2 ($cpu_ids) cpu id must be between 0 and %d (%d)
010- pcntl_setcpuaffinity(): Argument #2 ($cpu_ids) cpu id must be between 0 and %d (-1024)
011- pcntl_getcpuaffinity(): Argument #1 ($process_id) invalid process (-1024)
012- pcntl_setcpuaffinity(): Argument #2 ($cpu_ids) value must be of type int|string, array given
001+ Warning: pcntl_setcpuaffinity(): pset_create: Cannot allocate memory in /root/php-src/ext/pcntl/tests/pcntl_cpuaffinity.php on line 3
002+ bool(false)
003+
004+ Warning: pcntl_getcpuaffinity(): pset_create: Cannot allocate memory in /root/php-src/ext/pcntl/tests/pcntl_cpuaffinity.php on line 4
005+
006+ Fatal error: Uncaught TypeError: array_diff(): Argument #2 must be of type array, false given in /root/php-src/ext/pcntl/tests/pcntl_cpuaffinity.php:5
007+ Stack trace:
008+ #0 /root/php-src/ext/pcntl/tests/pcntl_cpuaffinity.php(5): array_diff(Array, false)
009+ #1 {main}
010+ thrown in /root/php-src/ext/pcntl/tests/pcntl_cpuaffinity.php on line 5
========DONE========
FAIL pcntl_getcpuaffinity() and pcntl_setcpuaffinity() [ext/pcntl/tests/pcntl_cpuaffinity.phpt]
There was a problem hiding this comment.
the test might need adaptation rather than disabling it, pset_create seems to set errno to ENOMEM here. We can devise about it later..
| # define MIN(x, y) ((x) > (y)? (y) : (x)) | ||
| #endif | ||
|
|
||
| #define SEG_ALLOC_SIZE_MAX 32*1024*1024 |
There was a problem hiding this comment.
it seems a slightly unrelated change to me (I do not say it is right or wrong tough).
There was a problem hiding this comment.
On Solaris, OPcache uses the SysV SHM allocator (shared_alloc_shm.c), which hard-limits individual segments to 32 MB (SEG_ALLOC_SIZE_MAX). With JIT enabled, OPcache reserves a default 64 MB buffer (ZEND_JIT_DEFAULT_BUFFER_SIZE) from the last shared segment during zend_shared_alloc_startup(). This makes startup fail with “Insufficient shared memory!” because the last segment can never satisfy the reservation.
There was a problem hiding this comment.
sure, but what I understood from the comment is opcache on solaris can't work without this change ? If that is the case, this change needs to be a PR on its own as a bug fix (from the lowest branch it needs to apply).
.github/actions/solaris/action.yml
Outdated
| release: "11.4-gcc" | ||
| usesh: true | ||
| copyback: false | ||
| # Temporarily disable sqlite, as FreeBSD ships it with disabled double quotes. We'll need to fix our tests. |
There was a problem hiding this comment.
is it really the same for solaris ?
There was a problem hiding this comment.
No, you are right. I have removed the note and also configure options --without-sqlite3 and --without-pdo-sqlite.
There was a problem hiding this comment.
nice ! I struggle whether the CI part needs to have its own PR or not but other members might not be bothered by it, we'll see..
psumbera
force-pushed
the
github-solaris-ci
branch
from
9c9436d to
924f86d
Compare
| default => 8*1024*1024, | ||
| }, | ||
| 'SunOS' => 10 * 1024 * 1024, | ||
| 'SunOS' => preg_match('/(omnios|illumos|smartos|oi-|openindiana|joyent)/i', php_uname('v')) |
There was a problem hiding this comment.
does it represent all flavors of illumos ? maybe the reverse logic might be simpler ?
There was a problem hiding this comment.
I hope it does. Solaris 11.4 returns e.g. 11.4.90.211.0 and Solaris 10 e.g.Generic_150400-29. Not sure when Illuoms did relevant change. But it must have been originally the same value as is in Solaris.
| die("skip Test is not valid for Windows"); | ||
| } | ||
| if (PHP_OS_FAMILY === 'Solaris') { | ||
| die("skip Solaris uses ' 8:08:08 AM' for %r (time format differs)"); |
There was a problem hiding this comment.
as a follow-up PR, might be worth doing solaris only versions of tests related to time formats.
2 participants
No description provided.