Skip to content

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