-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Simplify PDO include paths #14444
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
Simplify PDO include paths #14444
Conversation
PDO include paths can be simplified and synced as done in other extensions: either the project root directory or the phpincludedir (for the system installation). The 'ext' include is automatically appended when doing phpize build. In php-src it is only present on Windows build. The PHP_CHECK_PDO_INCLUDES is left intact working as before and checks if PDO headers are found.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this makes sense, less magic overall :)
Edit: Maybe a UPGRADING.INTERNALS note for external PDO drivers could be useful? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok odbc, agree with needing a note for internals upgrading
Of course. I can add this to the upgrading file. Except that PDO extensions out there are not affected by this change. There is still the PHP_CHECK_PDO_INCLUDES macro available and creates the |
This was added as requested in the pull request php#14444 but actually isn't important to log like this here. Existing PHP extensions using PDO still have their own way of adding the -I flag as before and aren't affected by this change.
Hello, I'm looking at the UPGRADING.INTERNALS change here again and basically this doesn't need to be noted in it. Because probably this wasn't clear enough back then that this change doesn't affect other PDO extensions out there. I'll remove the change in the changelog if you're all fine with this: #15816 |
PDO include paths can be simplified and synced as done in other extensions: either the project root directory or the phpincludedir (for the system installation). The 'ext' include is automatically appended when doing phpize build. In php-src it is only present on Windows build. The PHP_CHECK_PDO_INCLUDES is left intact working as before and checks if PDO headers are found.
Unless I'm missing something that makes the pdo_* extensions different with these includes.