TSRM cleanup for PHP8 #3976
Closed
TSRM cleanup for PHP8 #3976
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
weltling
approved these changes
Mar 24, 2019
There was a problem hiding this comment.
Looks ok for 8.0, as for me. Pthreads is now very wide and it utilizes platform specific features as well, where applicable.
Regarding GNUPTH - the use case could be currently CLI only, but there are no extensions making it possible. Furthermore, there are no SAPI requiring it in first place. Non preemptive threads could be an interesting topic in any regard, just that we have no practical case for that right now or in the near future.
I'd be great if @dstogov could verify as well.
Thanks.
|
|
KalleZ
approved these changes
Mar 24, 2019
|
I don't see any problems. |
|
Merged as bd73607 Thanks. |
cmb69
added a commit
to cmb69/web-rmtools
that referenced
this pull request
Mar 25, 2019
Commit bd73607 of php-src[1] broke the ext/ibase build; following Joe's reasoning[2] we disable the ext/interbase builds (at least for now). [1] <http://git.php.net/?p=php-src.git;a=commit;h=bd73607b9e4811e7caa9d2ff4d227626ffd35dab> [2] <php/php-src#3976 (comment)>
php-pulls
pushed a commit
to php/web-rmtools
that referenced
this pull request
Mar 26, 2019
Commit bd73607 of php-src[1] broke the ext/ibase build; following Joe's reasoning[2] we disable the ext/interbase builds (at least for now). [1] <http://git.php.net/?p=php-src.git;a=commit;h=bd73607b9e4811e7caa9d2ff4d227626ffd35dab> [2] <php/php-src#3976 (comment)>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This contains cleanup for TSRM in PHP 8, removing old defines, API's and integrations that are not used (or don't work):
What we are left with is the bare minimum, working ZTS and TSRM layer, with simple requirements and a clean API.
Note that this could target 7.4, however, I feel more comfortable targeting PHP 8 in light of the fact that this removes TSRMLS_D, _DC, _C, and _CC which we even had in php-src, and have been widespread for most of the last ~19 years. In addition, I'm not able to determine the reason for support GNUPTH and other related obscure libraries: I know that ZTS expects pthreads, our use of native TLS is predicated upon POSIX.1c or w32 threads, nevertheless I'd like to give anyone who thinks these obscure integrations are necessary ample time to come forward and work on those integrations - at least GNUPTH is able to operate in a POSIX.1c compliant way, and so it may in theory be possible to integrate with it, although by default and in testing it does not work.
Note also, the windows build is broken by interbase, which is using TSRM api's incorrectly and unsafely, I think interbase is targeted for removal in PHP8, and I don't see a safe way to change it to build, I leave these changes to (a non-existent) maintainer of the extension.
EDIT: I disabled interbase on av, so that we can see the rest of the build run ...