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

Test stream_context_tcp_nodelay_server on Windows #17308

Merged
merged 1 commit into from
Jan 6, 2025

Conversation

bukka
Copy link
Member

@bukka bukka commented Dec 31, 2024

This is to fix stream_context_tcp_nodelay_server which currently does not have socket extension available on Windows in worker.

@bukka bukka changed the base branch from master to PHP-8.1 December 31, 2024 14:31
@bukka bukka force-pushed the stream_test_tcp_nodelay_server branch from 1d71c71 to a278619 Compare December 31, 2024 14:31
@cmb69
Copy link
Member

cmb69 commented Dec 31, 2024

Ah, this doesn't look like a Windows specific issue, but is more generally related to whether ext/socket is built shared or static. On Windows it's usually the former. The simplest solution would be to do what we already do regarding ext/openssl in CI:

rem work-around for some spawned PHP processes requiring OpenSSL
echo extension=php_openssl.dll >> %PHP_BUILD_DIR%\php.ini

Adding echo extension=php_sockets.dll >> %PHP_BUILD_DIR%\php.ini should do for now.

A more general solution would be to load ext/sockets when spawning the worker:

if (defined("PHP_WINDOWS_VERSION_MAJOR")) {
$ini = php_ini_loaded_file();
$cmd = sprintf(
'%s %s "%s" %s',
PHP_BINARY, $ini ? "-n -c $ini" : "",
__FILE__,
WORKER_ARGV_VALUE
);

Additionally passing -d extension=sockets if !extension_loaded("sockets") might do (unless php_sockets.dll is not available, or there are some other issues regarding php.ini).

@bukka bukka force-pushed the stream_test_tcp_nodelay_server branch from a278619 to 1e7783a Compare December 31, 2024 23:28
@bukka
Copy link
Member Author

bukka commented Dec 31, 2024

Ah ok, I was actually looking to that file and completely missed that part. Thanks for pointing that out. I went for the simplest solution as it is also used for OpenSSL. It seems that extension_loaded is true in the test (there is --EXTENSIONS-- check) so it would probably require extra command to check it in the spawned process unless I'm missing something.

@bukka bukka force-pushed the stream_test_tcp_nodelay_server branch from 1e7783a to 9e1b582 Compare January 5, 2025 13:44
@bukka bukka merged commit 9e1b582 into php:PHP-8.1 Jan 6, 2025
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants