-
Notifications
You must be signed in to change notification settings - Fork 34
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 NTS support #24
base: default
Are you sure you want to change the base?
Add NTS support #24
Conversation
The current implementation only builds ZTS (Zend Thread Safety) builds of PHP. This PR adds support for also building NTS (Non Thread Safe) builds, complete with `-nts` suffix to differentiate them from the ZTS default builds.
Addresses travis-ci/travis-ci#2800 and travis-ci/travis-ci#985 (partially; would still need the API calls to be adjusted to build with configs for |
How widely is it used? This effectively doubles the number of builds we will be running, and if it's a niche feature, I am not sure if it is worth the effort. |
Many Linux distros provide only the NTS build, and getting the ZTS build requires manual installation from another source, or building locally. Extension developers especially need to run tests on both flavors to ensure their extensions function properly on both, but even just general PHP devs will be running, much of the time, without ZTS enabled. |
@BanzaiMan if your using PHP-FPM that is needed for example with Nginx you’re running NTS. Also when running docker it makes sense to run PHP in a separate container with PHP-FPM. Using Apache you can use both. |
Fedora includes both, with separate binaries and libraries for each mode. AFAICT SUSE and Ubuntu builds are NTS only. I would suspect Debian matches Ubuntu, but have not checked. |
It's more that Ubuntu matches Debian, really. The main difference is in more recent versions and different default settings. Generally the compilation settings are left untouched. |
@danhunsaker We could get in to a discussion about the semantics about when two things match, who matches to whom, but that's quite a digression here. 😉 To clarify, I was merely attempting to say in a succinct way that since Ubuntu tends not to deviate much from upstream Debian, it would follow that the Debian packages are probably also built NTS. |
I'd love to see support for this. I'd even make a non-ZTS enabled version the default and make it opt-in rather than opt-out but I can see this suddenly breaking builds so perhaps unwanted. |
I didn't even know ZTS was still a thing until I discovered Travis still used it haha. Wow, what a throwback 🤣 |
Building on what @driesvints said, the default could be switched for PHP 8.0+, assuming ZTS is even supported for it still, there. 7.x and below would stay ZTS, and 8.0 and above would be NTS, unless you explicitly specify the other in your config. Minimal impact while switching to a saner default. In any case, it would be really helpful to have both versions available for reasons outlined above, so I really hope this one doesn't just languish and die due to inaction. |
@BanzaiMan any updates regarding this? |
It's not clear from this discussion why there is interest in having NTS builds. ZTS allows the use of a couple of additional (threading-related) extensions, and is also the preferred testing mode for extensions. The only use-case mentioned is that extension authors may wish to test on both NTS and ZTS, which is a legit use-case, but high quality testing of extensions probably needs different PHP builds anyway than what Travis provides (at least assertion-enabled, but probably also with asan and ubsan). |
@nikic I agree it's like @danhunsaker described a time saver for people building extensions. So it will support that use-case without affecting anyone else. |
55f5d7d
to
065efad
Compare
019d867
to
bbeee8a
Compare
The current implementation only builds ZTS (Zend Thread Safety) builds of PHP. This PR adds support for also building NTS (Non Thread Safe) builds, complete with
-nts
suffix to differentiate them from the ZTS default builds.