Skip to content

Deprecate oci8.old_oci_close_semantics#7258

Merged
kocsismate merged 1 commit intophp:masterfrom
kocsismate:old-oci-close
Jul 19, 2021
Merged

Deprecate oci8.old_oci_close_semantics#7258
kocsismate merged 1 commit intophp:masterfrom
kocsismate:old-oci-close

Conversation

@kocsismate
Copy link
Copy Markdown
Member

@kocsismate kocsismate commented Jul 18, 2021

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

nikic commented Jul 22, 2021

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

@Jan-E
Copy link
Copy Markdown
Contributor

Jan-E commented Jul 22, 2021

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

@kocsismate
Copy link
Copy Markdown
Member Author

kocsismate commented Jul 22, 2021

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
Copy Markdown
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
Copy Markdown
Member Author

kocsismate 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.

Thanks! Testing is highly appreciated :)

@Jan-E
Copy link
Copy Markdown
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
Copy Markdown
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