Add PHP 7.3 support #6
Closed
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
This is similar to what I did for Python ibmdb ibmdb/python-ibmdb#343
Mostly involved finding resources relative to the script directory instead of from the current directory, which make test sets as the source directory. Instead of writing a file and then reading it back in to compare, just build up a string in memory and compare them. Connecting to a missing db alias returns an empty SQLSTATE. I'm not sure if/when that changed, but I've adjusted the code to handle either case. Finally, escape.dat was missing a \r in the file.
The code does an SQLGetData, binding as SQL_CLOB_LOCATOR, but DB2 Connect CLI returns invalid conversion to bind a CLOB to a locator for some reason: 07006[IBM][CLI Driver] CLI0102E Invalid conversion. SQLSTATE=07006 The BLOB path works, using SQLGetData with SQL_BLOB_LOCATOR, so I'm not sure what's wrong with CLOBs. It seems like a bug, but I can't get it to work and the test predates the CI setup, so just expect it to fail for now.
db2_execute calls _php_db2_execute_helper, which allocs curr->value if it is currently NULL. Later, _php_db2_set_symbol is called which frees tmp_curr->value, but it does not set it to NULL, which means on the next time through, _php_db2_execute_helper will skip allocating new memory and instead re-use a previously freed address. Found using Valgrind.
Check for IBM_DB2_TEST_SKIP_CONNECT_FAILURE if it's set to an empty string or 0, instead of skipping the test it continues onward (and probaby fails).
See https://github.com/php/php-src/blob/php-7.3.0/UPGRADING.INTERNALS#L90-L96 Define 7.3 macro equivalents for 7.0-7.2 as well
See https://github.com/php/php-src/blob/php-7.3.0/UPGRADING.INTERNALS#L123-L128 It was only used in two places, which both were completely broken: (Z_TYPE_FLAGS_P(params) & (IS_TYPE_COPYABLE & !IS_TYPE_REFCOUNTED)) will always be false, since !IS_TYPE_REFCOUNTED is 0. I believe !IS_TYPE_REFCOUNTED was meant to be ~IS_TYPE_REFCOUNTED, but regardless the above document says that all checks for IS_TYPE_COPYABLE were replaced with IS_TYPE_ARRAY, which we're already checking so I just removed the pointless check for all releases.
|
When will this be merged? I can't install this on a PHP 7.3 system :( |
|
Merged in dc392b1 |
Merged
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.
Depends on #5
The biggest change is in the removal of the
IS_TYPE_COPYABLEchecks:(Z_TYPE_FLAGS_P(params) & (IS_TYPE_COPYABLE & !IS_TYPE_REFCOUNTED))will always be false, since!IS_TYPE_REFCOUNTEDis 0. I believe!IS_TYPE_REFCOUNTEDwas meant to be~IS_TYPE_REFCOUNTEDsince it's attempting to make a bitmask, but as it is written, it will be 0 and anything bitwise ANDed with 0 is 0, so(IS_TYPE_COPYABLE & !IS_TYPE_REFCOUNTED)is also 0, and thus the whole thing is 0 and therefore false.Since the 7.3 upgrading docs say that all checks for
IS_TYPE_COPYABLEwere replaced withIS_TYPE_ARRAY, which we're already checking, I just removed the pointless check for all releases instead of#ifdefing for just 7.3.All tests are passing. Travis CI status is here: https://travis-ci.com/kadler/pecl-database-ibm_db2/builds/99230685