Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate to GitHub Actions #1766

Closed

Conversation

jack-worman
Copy link
Contributor

@jack-worman jack-worman commented Feb 28, 2022

Future issues:

  • Some Windows tests are failing
  • PHP 8.2 workflows fail

@jack-worman jack-worman force-pushed the Migrate_to_github_actions branch 16 times, most recently from 728028c to daa4774 Compare March 5, 2022 22:40
@terrafrost
Copy link
Member

Secure shell functional test not set up

What would it take to get those setup? The SSH functional tests have def found legit issues and regressions and altho I'm okay not having them for a short amount of time I don't see it as being something I'd be happy with for the long term.

The other stuff is fine. We never had Windows tests before. They'd be nice but at least it doesn't feel like back peddling by not having them!

Also, I note that the README.md changes don't replace the Travis Build Status indicator. https://docs.github.com/en/actions/monitoring-and-troubleshooting-workflows/adding-a-workflow-status-badge talks about how to add that.

Would you be up for doing this for the 1.0 and 2.0 branches as well? The master branch is (currently) 99% the same as the 3.0 branch. The only (current) difference is that the master branch has #1751 whereas the 3.0 branch doesn't.

It looks like GitHub Actions supports PHP down as low as 5.3: https://github.com/marketplace/actions/setup-php-action phpseclib 2.0 is, in theory, supposed to work on PHP 5.3+ so having it unit tested down to 5.3 would be good, composer issues notwithstanding. phpseclib 1.0 is supposed to, in theory, work on PHP 4.4+ but trying to do unit tests for PHP versions that old is just not reasonable. If short array syntax (which was introduced in PHP 5.4+) crops up in the 1.0 branch it's presence results in a bug report I'll fix it but I'm not necessarily proactively fixing it (altho linting, as #1710 attempted to add, would be cool)

I'm playing around with a cherry-picked version of this to the 1.0 branch myself. Still a work in progress (altho it is behind your latest changes, now, since you seem to be doing rebasing)

@jack-worman jack-worman force-pushed the Migrate_to_github_actions branch 8 times, most recently from 9430e2f to a5f9af7 Compare March 6, 2022 19:59
@jack-worman
Copy link
Contributor Author

jack-worman commented Mar 6, 2022

  • SSH functional tests now working (only for ubuntu os)
  • Replaced travis status badge with github status badge
  • I'll make new PR's for doing the 1.0 and 2.0 branches

@terrafrost
Copy link
Member

I cherry-picked this to the 3.0 branch and merged it into master!

@terrafrost terrafrost closed this Mar 8, 2022
@jack-worman jack-worman deleted the Migrate_to_github_actions branch March 9, 2022 00:17
@terrafrost
Copy link
Member

I'll make new PR's for doing the 1.0 and 2.0 branches

For phpseclib 1.0... for linting for versions of PHP below 5.3... maybe we could use the 4.4-5.2 Docker containers at https://github.com/phpseclib/docker-php

@terrafrost
Copy link
Member

In Travis CI you could have allow select builds to fail using allow_failures. Do you know if GitHub Actions allows this?

The only versions of PHP the unit tests are now failing on is 8.2 and 5.6.

On 8.2 it's due to a "PHP Fatal error: Uncaught Error: Call to undefined function each() in /Users/runner/work/phpseclib/phpseclib/vendor/phpunit/phpunit/src/Util/Getopt.php:38
Stack trace:
" error, which seems to have more to do with PHPUnit than it does with phpseclib.

On 5.6... I have no idea what it's due to. There's an intermittent segfault that's happening on the GitHub Actions build. Travis CI never had segfaults with 5.6 and I'm not able to reproduce it locally either. I thought that maybe I could isolate it down to a single unit test by selectively deleting unit tests but alas that did not help. It seems like it's combination of tests that's causing the error. Since it's a segfault I don't see how I can get any info on it from GitHub Actions. Maybe if they were using Docker containers to do the different PHP versions I could duplicate it locally but idk. In lieu of that I think imma just have to accept failing tests on PHP 5.6, sadly.

@terrafrost
Copy link
Member

Actually, I was able to reproduce the PHP 5.6 segfault on Ubuntu. Due to it's intermittent nature I guess I just hadn't tried it enough times lol. I was able to reproduce it in gdb as well and here's what the where said after I reproduced it in gdb:

#0  0x00005555558327e3 in ?? ()
#1  0x0000555555850044 in destroy_op_array ()
#2  0x000055555586ab78 in zend_hash_destroy ()
#3  0x000055555584fddc in destroy_zend_class ()
#4  0x0000555555869536 in ?? ()
#5  0x000055555586b162 in zend_hash_reverse_apply ()
#6  0x000055555584b77b in ?? ()
#7  0x000055555585c456 in zend_deactivate ()
#8  0x00005555557f72d7 in php_request_shutdown ()
#9  0x000055555590cf5f in ?? ()
#10 0x0000555555501177 in ?? ()
#11 0x00007ffff573a2e1 in __libc_start_main (main=0x555555500d10, argc=5, argv=0x7fffffffea38,
    init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>,
    stack_end=0x7fffffffea28) at ../csu/libc-start.c:291
#12 0x00005555555012ba in _start ()

Here's what list said:

1       ../sysdeps/x86_64/dl-procinfo.c: No such file or directory.

idk that there's gonna be much I'm gonna be able to do about this. I mean, maybe in theory, there are some code changes phpseclib could make to remedy this but maybe not. And if not... it's not like I can submit a bug report about this feature because PHP 5.6 is EOL.

@terrafrost
Copy link
Member

I almost wonder if we're seeing something like zendtech/ZendOptimizerPlus#146 going on for PHP 5.6. Some functions that are being unit tested employee randomization. Like prime number generation for RSA keys. You randomly generate a bunch of numbers and test each one to see if it's prime. And even the primality testing algorithm, itself (Miller-Rabin) employees randomization, by design. More specifically, Miller-Rabin is a probabilistic primality test - non-probabilistic (aka determinstic) primality tests exists, even in P, but they're galactic algorithms, so they're not very useful IRL.

Anyway, I just mention that because, assuming that the issue is that too many objects are being created the randomized algorithms would (probably) explain why the problem is intermittent.

Maybe the solution to this is to make it so that some unit tests simply aren't ran on PHP 5.6.

@jack-worman
Copy link
Contributor Author

jack-worman commented Mar 13, 2022

I didn't know 5.6 was failing sometimes, I never ran into it yet.

It sure is strange that it only happens after all the tests have ran. Must be something PHPUnit is doing at the very end.

Edit: Funnily enough, I just hit the problem in my new PR #1768

@terrafrost
Copy link
Member

terrafrost commented Mar 14, 2022

Looks like the segfault's are for a multitude of reasons:

where output:

#0  __memcmp_sse4_1 () at ../sysdeps/x86_64/multiarch/memcmp-sse4.S:878
#1  0x000055555586b668 in zend_hash_quick_find ()
#2  0x0000555555900dc8 in ?? ()
#3  0x000055555589559e in execute_ex ()
#4  0x000055555584cb7f in zend_call_function ()
#5  0x0000555555874c45 in zend_call_method ()
#6  0x0000555555885421 in zend_std_cast_object_tostring ()
#7  0x000055555585d365 in zend_make_printable_zval ()
#8  0x00005555558c09ea in ?? ()
#9  0x000055555589559e in execute_ex ()
#10 0x000055555585dd51 in zend_execute_scripts ()
#11 0x00005555557f87b0 in php_execute_script ()
#12 0x000055555590dc0f in ?? ()
#13 0x0000555555501177 in ?? ()
#14 0x00007ffff573a2e1 in __libc_start_main (main=0x555555500d10, argc=5, argv=0x7fffffffea28,
    init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>,
    stack_end=0x7fffffffea18) at ../csu/libc-start.c:291
#15 0x00005555555012ba in _start ()

list output:

873     in ../sysdeps/x86_64/multiarch/memcmp-sse4.S

Thus far I've gotten 5 segfaults in gdb. 80% of them are the ../sysdeps/x86_64/dl-procinfo.c: No such file or directory. segfault.

That said, the where output is different even when it's the dl-procinfo.c error. eg. after having added gc_collect_cycles() to RSA/ModeTest.php:

#0  0x000055555587ca80 in ?? ()
#1  0x000055555587ca6c in ?? ()
#2  0x000055555587cab6 in ?? ()
#3  0x000055555587cf15 in ?? ()
#4  0x000055555587d005 in ?? ()
#5  0x000055555587d503 in gc_collect_cycles ()
#6  0x000055555587db66 in gc_zval_possible_root ()
#7  0x000055555590bc65 in ?? ()
#8  0x000055555589559e in execute_ex ()
#9  0x000055555584cb7f in zend_call_function ()
#10 0x0000555555874c45 in zend_call_method ()
#11 0x0000555555885421 in zend_std_cast_object_tostring ()
#12 0x000055555585d365 in zend_make_printable_zval ()
#13 0x00005555558c09ea in ?? ()
#14 0x000055555589559e in execute_ex ()
#15 0x000055555585dd51 in zend_execute_scripts ()
#16 0x00005555557f87b0 in php_execute_script ()
#17 0x000055555590dc0f in ?? ()
#18 0x0000555555501177 in ?? ()
#19 0x00007ffff573a2e1 in __libc_start_main (main=0x555555500d10, argc=5, argv=0x7fffffffea28,
    init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>,
    stack_end=0x7fffffffea18) at ../csu/libc-start.c:291
#20 0x00005555555012ba in _start ()

Here's where destroy_op_array is defined (which is referenced in the first stack trace that I posted):

https://github.com/php/php-src/blob/PHP-5.6.40/Zend/zend_opcode.c#L354

Nothing references dl-procinfo.c but, then again, we do have one ?? symbol.

I think it's gonna be best just to drop 5.6 from the unit tests. It can still be linted - just not unit tested :|

Like even with my proposals for PHP 4.4 / phpseclib 1.0... my proposal isn't that we do unit tests on PHP 4.4. Rather, it's just that linting be done with the Docker containers (if possible). If people have specific issues with PHP 5.6 (or PHP 4.4 for phpseclib 1.0) they can create bug reports on github.

@jack-worman
Copy link
Contributor Author

jack-worman commented Mar 14, 2022

I'll mark the PHP 5.6 tests as "continue-on-error" in #1768 (allows failure)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants