Fix GH-14792: Compilation failure on pdo_* extensions #14797
Merged
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
When building pdo_mysql, pdo_pgsql, or pdo_sqlite from downloaded PHP 8.4 archive, also pdo_sql_parser.h and php_pdo_int.h need to be installed.
SakiTakamachi
approved these changes
Jul 3, 2024
|
Thanks! |
|
Just stumbled upon this while working on #15682. It seems to me this fix is not correct regarding php_pdo_int.h: Lines 19 to 20 in 1a126c5 In my opinion, drivers including php_pdo_int.h should be fixed; if absolutely needed declarations now in php_pdo_int.h can be moved to somewhere else (probably php_pdo.h). |
|
If drivers and this header can be refactored in such way then that would be good idea probably, yes. |
cmb69
added a commit
to cmb69/php-src
that referenced
this pull request
Sep 1, 2024
This is, as the name and a comment in the header imply, an internal header which is not supposed to be used by extensions other than PDO (not even by drivers). Since there is apparently no need to include this header in the parsers of the drivers, we remove these includes, and no longer declare the header to be installed. Given that the header is only exported for a couple of weeks[1], this is not considered to be a BC break, because it's unlikely that external drivers have already been adjusted to use this header, and otherwise they can still be fixed; PHP 8.4 is still in the pre-release stage. [1] <php#14797>
|
I've submitted PR #15688. |
cmb69
added a commit
that referenced
this pull request
Sep 1, 2024
This is, as the name and a comment in the header imply, an internal header which is not supposed to be used by extensions other than PDO (not even by drivers). Since there is apparently no need to include this header in the parsers of the drivers, we remove these includes, and no longer declare the header to be installed. Given that the header is only exported for a couple of weeks[1], this is not considered to be a BC break, because it's unlikely that external drivers have already been adjusted to use this header, and otherwise they can still be fixed; PHP 8.4 is still in the pre-release stage. [1] <#14797> Closes GH-15688.
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.
When building pdo_mysql, pdo_pgsql, or pdo_sqlite from downloaded PHP 8.4 archive, also pdo_sql_parser.h and php_pdo_int.h need to be installed.