Skip to content

Fix tests when stdout is unbuffered #13924

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

Merged
merged 1 commit into from
Apr 16, 2024
Merged

Conversation

arnaud-lb
Copy link
Member

@arnaud-lb arnaud-lb commented Apr 9, 2024

These tests spawn a sub-process with -d extension=phar.so, which emits a warning on stdout when the extension is compiled statically in the php binary.

In glibc, stdout is buffered, and we also don't flush it explicitly. The process is later killed with SIGTERM, so the buffer is never flushed, and the warning is not visible.

With musl, stdout is unbuffered (or maybe line-buffered, or the buffer is smaller), so the warning is visible, causing the tests to fail.

In this PR I add -d extension=phar.so only if phar was compiled as a dynamic module, so that no warning is emitted.

This is out of scope of this PR, but we should probably flush after emitting a warning. Also, I think that warnings go to stdout as part of the display_errors functionality, but ideally they should be written to stderr when using the CLI SAPI.

@arnaud-lb arnaud-lb mentioned this pull request Apr 9, 2024
@arnaud-lb arnaud-lb marked this pull request as ready for review April 9, 2024 17:09
Copy link
Member

@iluuu1994 iluuu1994 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@arnaud-lb arnaud-lb merged commit 326dc17 into php:master Apr 16, 2024
10 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