Skip to content

fixing php 8.2 deprecations#30997

Merged
stanley-cheung merged 5 commits intogrpc:masterfrom
brettmc:php82
Oct 26, 2022
Merged

fixing php 8.2 deprecations#30997
stanley-cheung merged 5 commits intogrpc:masterfrom
brettmc:php82

Conversation

@brettmc
Copy link
Copy Markdown
Contributor

@brettmc brettmc commented Sep 15, 2022

PHP 8.2 RC's are out, and grpc complains that Creation of dynamic property Grpc\Call::$channel is deprecated.
Adding a property for channel resolves this, and I also fixed a bunch of deprecations and minor issues in the tests themselves.

@brettmc
fixing php8.2 test deprecations
6e6d2aa 
fixes various dynamic property deprecations in the tests themselves
@brettmc
fixing phpunit9 deprecation
07a5923 
at() matcher is deprecated, to be removed in phpunit 10. switch to withConsecutive
@brettmc
adding channel property to Grpc\Call
1e0bd5e 
this resolves "Creation of dynamic property Grpc\Call::$channel is deprecated" in php 8.2
brettmc
brettmc commented Sep 15, 2022
View reviewed changes
Comment thread src/php/tests/unit_tests/TimevalTest.php Outdated
@brettmc
Copy link
Copy Markdown
Contributor Author

brettmc commented Sep 15, 2022

@stanley-cheung I think you're the right person to review this PR for me.

@brettmc
skip implicit cast test in 8.1+
5796cfa 
implicit casting from float to int is deprecated from 8.1, so users would receive a deprecation warning if trying to do this
@brettmc brettmc mentioned this pull request Sep 16, 2022
Closed
6 tasks
@stanley-cheung stanley-cheung self-assigned this Oct 13, 2022
@stanley-cheung stanley-cheung self-requested a review October 13, 2022 21:12
@stanley-cheung stanley-cheung added kokoro:run release notes: yes Indicates if PR needs to be in release notes labels Oct 13, 2022
@stanley-cheung
Copy link
Copy Markdown
Contributor

stanley-cheung commented Oct 13, 2022

Sorry for the delay. Wasn't aware of this until just now. Will look into this. Thanks for helping fix this.

@stanley-cheung
Copy link
Copy Markdown
Contributor

stanley-cheung commented Oct 15, 2022

@brettmc Can I confirm that, once this PR is patched in, src/php/bin/build_all_docker_images.sh and src/php/bin/run_all_docker_images.sh run to completions with no errors?

@brettmc
Copy link
Copy Markdown
Contributor Author

brettmc commented Oct 17, 2022

@stanley-cheung - confirming that both src/php/bin/build_all_docker_images.sh and src/php/bin/run_all_docker_images.sh run to completion for me with this patch.

stanley-cheung
stanley-cheung approved these changes Oct 26, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@stanley-cheung stanley-cheung left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the contributions!

@stanley-cheung stanley-cheung merged commit 14d1403 into grpc:master Oct 26, 2022
@copybara-service copybara-service Bot added the imported Specifies if the PR has been imported to the internal repository label Oct 26, 2022
@brettmc brettmc deleted the php82 branch November 7, 2022 02:55
@gsdaemon
Copy link
Copy Markdown

gsdaemon commented Mar 21, 2023

2a6cf72
this was not merged unfortunately and it still shows deprecation notices in php 8.2
Notice (8192): Creation of dynamic property Grpc\Call::$channel is deprecated in grpc/src/lib/AbstractCall.php on line 60

@brettmc
Copy link
Copy Markdown
Contributor Author

brettmc commented Mar 21, 2023

Hi @gsdaemon
This was released in v1.51.0: https://github.com/grpc/grpc/blob/v1.51.0/src/php/ext/grpc/call.c#L627 and is also in the most recently released version: https://github.com/grpc/grpc/blob/v1.52.1/src/php/ext/grpc/call.c#L627
Can you confirm the extension version you're using?
It looks like pecl by default is installing an earlier version (I get 1.47.4), so you could try to install an explicit version of 1.51+.
That said, it looks like 1.51+ has a bug that you don't want (I can't run my tests to confirm that the notice has gone away on any version of grpc that has the fix)

@brettmc brettmc mentioned this pull request Jun 13, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stanley-cheung stanley-cheung stanley-cheung approved these changes

Assignees

@stanley-cheung stanley-cheung

Labels

imported Specifies if the PR has been imported to the internal repository lang/php release notes: yes Indicates if PR needs to be in release notes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@brettmc @stanley-cheung @gsdaemon @grpc-kokoro