Skip to content

Add zend_worklist.h to PHP_INSTALL_HEADERS #12571

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
Oct 30, 2023

Conversation

iluuu1994
Copy link
Member

Fixes GH-12565

@devnexen
Copy link
Member

I would say LGTM, but not sure that is really the issue in the aforementioned ticket.
What I mean is maybe, since there was no -I/usr/src/php/Zend, the build pointed to an existing php installation rather than the PHP's source content, if that makes sense ?
But your change is ok, do not see any harm.

@iluuu1994
Copy link
Member Author

@devnexen I don't know exactly how the Docker image is set up, but I could imagine PHP being built, installed, and then opcache being built against the installed sources. This would be missing this header. This header was added to zend_jit.c as of a few days ago, with the new JIT.

Copy link
Member

@devnexen devnexen left a comment

Choose a reason for hiding this comment

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

fair enough

@mvorisek
Copy link
Contributor

@devnexen I don't know exactly how the Docker image is set up, but I could imagine PHP being built, installed, and then opcache being built against the installed sources. This would be missing this header. This header was added to zend_jit.c as of a few days ago, with the new JIT.

yes, this is exactly what I did and what https://hub.docker.com/_/php (official PHP docker image) does

@iluuu1994 iluuu1994 merged commit 2efe366 into php:master Oct 30, 2023
@iluuu1994
Copy link
Member Author

@mvorisek Can you confirm that this fixes your build?

@mvorisek
Copy link
Contributor

@mvorisek Can you confirm that this fixes your build?

I can confirm the issue is gone, thank you both ❤️

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.

Make of opcache ext is failing on master
3 participants