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

[RFC] PHP should be compiled with ZTS or not ? #154774

Closed
drupol opened this issue Jan 12, 2022 · 10 comments
Closed

[RFC] PHP should be compiled with ZTS or not ? #154774

drupol opened this issue Jan 12, 2022 · 10 comments
Labels
0.kind: bug 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md

Comments

@drupol
Copy link
Contributor

drupol commented Jan 12, 2022

Hi,

I'm opening this RFC hoping to gather comments from the Apache and PHP maintainers.

We know that there are two ways to compile the PHP interpreter (yeah there are many others but in this thread, only those flags are relevant):

  • NTS: Non-Thread Safe
  • ZTS: Zend-Thread Safe

Me, as a PHP developer, I remember these rules:

  • When PHP is meant to be used with mod_php in Apache -> it's better to use ZTS
  • When PHP is meant to used with CGI/FASTCGI/Nginx/CommandLine -> It's better NTS

The default PHP in Nixos is built with ZTS and I think it should be NTS. NTS is also the default in many other distributions.

What's your point of view about that?

Relevant links:

CC:

@jtojnar
Copy link
Member

jtojnar commented Jan 12, 2022

Even if we change this line to false:

, ztsSupport ? apxs2Support

PHP will still be built with ZTS due to

https://github.com/php/php-src/blob/5b01c4863fe9e4bc2702b2bbf66d292d23001a18/sapi/apache2handler/config.m4#L119

$ nix shell -f . apacheHttpd.dev -c sh -c '$(apxs -q SBINDIR)/$(apxs -q TARGET) -V' | grep threaded
  threaded:     yes (fixed thread count)

@talyz
Copy link
Contributor

talyz commented Jan 12, 2022

Yeah, I guess the question is if we should disable apxs2 by default and maybe provide a separate package with it enabled.

@drupol
Copy link
Contributor Author

drupol commented Jan 12, 2022

Yeah, I guess the question is if we should disable apxs2 by default and maybe provide a separate package with it enabled.

Could be an idea yeah !

@drupol
Copy link
Contributor Author

drupol commented Jan 14, 2022

Do you have something to say here @lovek323 ?

@lovek323
Copy link
Member

Like you say, it would be great to be able to have the CLI php binary using NTS by default, if apxs is the only thing that's blocking this, a separate package for that sounds good to me.

@drupol
Copy link
Contributor Author

drupol commented Jan 22, 2022

@lovek323 Thanks for your input !

@jtojnar how about making 2 packages:

  • php: This would be the default PHP package, without apxs2support and NTS
  • php-apache: This would be the PHP package, with apxs2support and ZTS

WDYT?

@aanderse
Copy link
Member

I'll mention that it is 2022 and people really shouldn't be using mod_php anymore... we have php-fpm which is almost always a better solution. @drupol I think your proposed solution is reasonable 👍

https://cwiki.apache.org/confluence/display/httpd/PHP

@drupol
Copy link
Contributor Author

drupol commented Jan 23, 2022

I'll mention that it is 2022 and people really shouldn't be using mod_php anymore... we have php-fpm which is almost always a better solution. @drupol I think your proposed solution is reasonable 👍

https://cwiki.apache.org/confluence/display/httpd/PHP

I definitely agree and this is why I opened this rfc.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 31, 2022
@etu
Copy link
Contributor

etu commented Nov 13, 2022

Well, the changes is merged now 🙂 🎉

@etu etu closed this as completed Nov 13, 2022
@drupol
Copy link
Contributor Author

drupol commented Nov 13, 2022

Excellent :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: bug 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md
Projects
None yet
Development

No branches or pull requests

6 participants