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

add dtrace build #1367

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

tombokombo
Copy link

@tombokombo tombokombo commented Feb 7, 2023

Hi, this PR introduce dtrace variant of php image, but debian omly, it would be really problematic on alpine.

Closes #813

Signed-off-by: tombokombo <tombo@sysart.tech>
@yosifkit
Copy link
Member

yosifkit commented Feb 8, 2023

Unfortunately adding more variants of PHP is not something we want to support and maintain. Full build time of what we already have is around 2-3 hours (on each architecture). We do not have bandwidth for more PHP variants (#530 (comment), #879 (comment)).

Also, this would seem to make more sense as a variant of each of the current variants (i.e. fpm-dtrace) rather than a variant unto itself which would double the number of Debian variants, greatly increasing queue build times.

@tombokombo
Copy link
Author

With docker buildx there are possible optimizations eg. part of Dockerfile for installing dependencies could be cached and shared for each distro-arch build, this could save some bandwidth but I understand that most of time is spent in compilation process.
Regarding fpm, if somebody needs dtrace-fpm, he could use simple multistage dockerfile and just copy fpm binary with confs and deps to dtrace image, without any compilation, but only case that there will be dtrace image available.

@tianon
Copy link
Member

tianon commented Feb 8, 2023

See also #813

@tombokombo
Copy link
Author

@tianon i've checked php source, should have no impact when disabled by env USE_ZEND_DTRACE=0
@yosifkit would you accept PR with dtrace enabled for all debian builds?

@tianon
Copy link
Member

tianon commented Dec 19, 2023

Sorry for the delay! That sounds like it might be OK, but my high level concerns beyond performance impact would be size impact and whether it's appropriate to enable across all the multitude of CPU architectures we support (I see Debian has been oscillating back and forth on this for a while and I'm guessing that's probably FTBFS related: https://salsa.debian.org/php-team/php/-/commits/debian/main/8.3/debian?search=dtrace 😬). 🙇

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.

Default dtrace support
3 participants