Skip to content
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

MySqliDriver::getResource() fixed usage of resource being closed prior to the call in PHP 8 #410

Merged
merged 2 commits into from
Dec 12, 2021
Merged

MySqliDriver::getResource() fixed usage of resource being closed prior to the call in PHP 8 #410

merged 2 commits into from
Dec 12, 2021

Conversation

dakujem
Copy link
Contributor

@dakujem dakujem commented Dec 11, 2021

closes #409

@dakujem dakujem changed the title Fix error in MySqliDriver::getResource with resource being closed prior to the call in PHP 8 WIP: Fix error in MySqliDriver::getResource with resource being closed prior to the call in PHP 8 Dec 11, 2021
@dakujem dakujem force-pushed the feat/fix-error-in-MySqliDriver-getResource-with-closed-resource branch 6 times, most recently from 1fe1ace to 1dacacb Compare December 11, 2021 19:31
@dakujem
Copy link
Contributor Author

dakujem commented Dec 11, 2021

@dg I'm unable to configure the tests, help needed. I'm able to repliate the error via github actions in my fork, but appveyor uses different DB config provider.

@dakujem
Copy link
Contributor Author

dakujem commented Dec 11, 2021

Also, I tried the is_resource condition, but the condition fails right after construction for some reason. I reimplemented it using a try-catch block.

@dakujem dakujem force-pushed the feat/fix-error-in-MySqliDriver-getResource-with-closed-resource branch from 1dacacb to 357041e Compare December 11, 2021 20:55
@dakujem
Fix error in MySqliDriver::getResource with closed resource
fdfdbfb 
Fixes error in PHP 8 when calling MySqliDriver::getResource with the resource being closed prior to the call
@dakujem
Driver test
8517eec
@dakujem dakujem force-pushed the feat/fix-error-in-MySqliDriver-getResource-with-closed-resource branch from 357041e to 8517eec Compare December 11, 2021 20:58
@dg dg changed the title WIP: Fix error in MySqliDriver::getResource with resource being closed prior to the call in PHP 8 MySqliDriver::getResource() fixed usage of resource being closed prior to the call in PHP 8 Dec 12, 2021
@dg dg merged commit 8c2e425 into dg:master Dec 12, 2021
@dg
Copy link
Owner

dg commented Dec 12, 2021

Thanks

dg pushed a commit that referenced this pull request Dec 12, 2021
@dakujem @dg
MySqliDriver::getResource() fixed access to resource being closed pri…
1e12c56 
…or to the call in PHP 8 (#410)
@dakujem
Copy link
Contributor Author

dakujem commented Dec 12, 2021

@dg The tests need to be updated. It was marked WIP for that reason. The code is ok, but I was unable to make the tests pass on appveyor. There's also the unused mysqli-driver config left dangling in the config files.

dg pushed a commit that referenced this pull request Dec 12, 2021
@dakujem @dg
MySqliDriver::getResource() fixed access to resource being closed pri…
f93d157 
…or to the call in PHP 8 (#410)
dg pushed a commit that referenced this pull request Dec 12, 2021
@dakujem @dg
MySqliDriver::getResource() fixed access to resource being closed pri…
7b3a6e6 
…or to the call in PHP 8 (#410)
dg pushed a commit that referenced this pull request Dec 12, 2021
@dakujem @dg
MySqliDriver::getResource() fixed access to resource being closed pri…
7d82ce2 
…or to the call in PHP 8 (#410)
@dg
Copy link
Owner

dg commented Dec 12, 2021

I used a test like this: 7d82ce2#diff-1a3150d75da0d135bea2c37b25a578bf7aa75308db33a297abd067e27ffe558f

@dakujem
Copy link
Contributor Author

dakujem commented Dec 14, 2021

If you're okay with the test, fine. Still, there's the dangling code that should be removed: 8c2e425#diff-2e359a78bf001eae825dda38b8e9565c483c4a62089440534f2129aea7115538R26

Should I open a new PR?

@dakujem dakujem mentioned this pull request Dec 14, 2021
Open
@dg
Copy link
Owner

dg commented Dec 14, 2021

Yeah, I put the code away.

@dakujem
Copy link
Contributor Author

dakujem commented Dec 14, 2021

Oh, sorry, I did not notice.

@dakujem dakujem deleted the feat/fix-error-in-MySqliDriver-getResource-with-closed-resource branch December 20, 2021 09:32
@dakujem
Copy link
Contributor Author

dakujem commented Jan 11, 2022

Hello.
Is there any plan for a 4.2.x patch release? This issue currently prevents me from migrating to 8. Thank you in advance!

@dg
Copy link
Owner

dg commented Jan 11, 2022

So use dev-4.2.x in the meantime.

@dakujem
Copy link
Contributor Author

dakujem commented Jan 13, 2022

That works fine for development, not for production.

Jakub-Fajkus pushed a commit to peckadesign/dibi that referenced this pull request Feb 9, 2022
@dakujem @Jakub-Fajkus
MySqliDriver::getResource() fixed access to resource being closed pri…
bdad6ef 
…or to the call in PHP 8 (dg#410)

(cherry picked from commit 7d82ce2)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@dakujem @dg