Skip to content

Deprecate oci8.old_oci_close_semantics #7258

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 19, 2021
Merged

Deprecate oci8.old_oci_close_semantics #7258

merged 1 commit into from
Jul 19, 2021

Conversation

kocsismate
Copy link
Member

Updated the tests blindly...

@kocsismate kocsismate requested a review from cjbj July 18, 2021 14:48
kocsismate
kocsismate commented Jul 18, 2021
View reviewed changes
@cjbj
Copy link
Contributor

cjbj commented Jul 19, 2021

I will make time this week to run tests; I'm happy to tidy up after you merge, if you want to make headway before I get to it.

@nikic nikic added this to the PHP 8.1 milestone Jul 19, 2021
nikic
nikic approved these changes Jul 19, 2021
View reviewed changes
Copy link
Member

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As feature freeze is already tomorrow, let's go with the merge now and potential test fixup later.

@kocsismate kocsismate merged commit e120d52 into php:master Jul 19, 2021
@kocsismate kocsismate deleted the old-oci-close branch July 19, 2021 09:56
@Jan-E
Copy link
Contributor

Jan-E commented Jul 22, 2021

Curious: how do you avoid the deprecation warning? I tried it with with the Windows PHP 8.1.0 beta1 binaries at https://windows.php.net/qa/ and could not find a way.

@nikic
Copy link
Member

nikic commented Jul 22, 2021

@Jan-E Yeah, that's an implementation bug.

@Jan-E
Copy link
Contributor

Jan-E commented Jul 22, 2021

OK. I suppose that will be fixed in beta 2?

@kocsismate
Copy link
Member Author

OK. I suppose that will be fixed in beta 2?

Yes. I've just pushed the suggested change to fix this issue: 87c181a

@Jan-E
Copy link
Contributor

Jan-E commented Jul 22, 2021

OK, I will apply the patch for my own builds of PHP 8.1.0 beta1 (for Windows) to see if it works.

@kocsismate
Copy link
Member Author

OK, I will apply the patch for my own builds of PHP 8.1.0 beta1 (for Windows) to see if it works.

Thanks! Testing is highly appreciated :)

@Jan-E
Copy link
Contributor

Jan-E commented Jul 22, 2021

Works. This is a phpinfo() dump of the patched PHP 8.1.0 beta1 with a lot of extensions:
https://phpdev.toolsforresearch.com/php-8.1.0beta1-nts-Win32-vs16-x64.htm

@Jan-E
Copy link
Contributor

Jan-E commented Jul 22, 2021

Confirmed to be working for all 4 variants of the Windows build (x64/x86, TS/NTS). See https://www.apachelounge.com/viewtopic.php?t=6617 for the phpinfo dumps and the complete packages.

jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Mar 9, 2022
@jrfnl
PHP 8.1 | RemovedIniDirectives: handle deprecated oci8 ini directive
dcb09a9 
> The `oci8.old_oci_close_semantics` INI directive is deprecated.

Includes unit tests.

Refs:
* https://www.php.net/manual/en/migration81.deprecated.php#migration81.deprecated.oci8
* https://wiki.php.net/rfc/deprecations_php_8_1#oci8old_oci_close_semantics_ini_setting
* php/php-src#7258
* php/php-src@e120d52
@jrfnl jrfnl mentioned this pull request Mar 9, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikic nikic nikic approved these changes

@cjbj cjbj Awaiting requested review from cjbj

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
PHP 8.1
Development

Successfully merging this pull request may close these issues.
4 participants
@kocsismate @cjbj @Jan-E @nikic