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.

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

Support PHP 8.3 #5136

Closed
Al2Klimov opened this issue Oct 5, 2023 · 2 comments · Fixed by #5137
Closed

Support PHP 8.3 #5136

Al2Klimov opened this issue Oct 5, 2023 · 2 comments · Fixed by #5137
Assignees
Labels
enhancement New feature or improvement
Milestone

Comments

@Al2Klimov
Copy link
Member

Describe the bug

Calling ldap_connect() with separate hostname and port is deprecated.

https://github.com/php/php-src/blob/php-8.3.0RC1/UPGRADING#L184C5-L184C74

But this is exactly what we do:

https://github.com/Icinga/icingaweb2/blob/v2.12.0/library/Icinga/Protocol/Ldap/LdapConnection.php#L1201

To Reproduce

  1. Try to add LDAP resource
  2. Check PHP deprecation notice on the screen

Expected behavior

LDAP just works.

Screenshots

Bildschirmfoto 2023-10-05 um 16 03 02

Your Environment

  • Icinga Web 2 version and modules (System - About): master
  • Web browser used: Safari
  • Icinga 2 version used (icinga2 --version): -
  • PHP version used (php --version): 8.3.0 RC3
  • Server operating system and version: Fedora 40
@nilmerg
Copy link
Member

nilmerg commented Nov 14, 2023

Other notable changes

Executing proc_get_status() multiple times

https://www.php.net/manual/en/migration83.incompatible.php#migration83.incompatible.core.proc-get-status-multiple-times

Applicable to:

  • icingaweb2/modules/monitoring/library/Monitoring/Command/Transport/RemoteCommandFile.php
  • pdfexport/library/Pdfexport/ShellCommand.php

Both shouldn't be strongly affected.

Uses of traits with static properties

https://www.php.net/manual/en/migration83.incompatible.php#migration83.incompatible.core.traits-with-static-properties

Applicable to:

  • graphite/library/Graphite/Graphing/GraphingTrait.php
  • ipl/web/src/FormElement/ScheduleElement/Common/FieldsUtils.php

The first is mainly used in controllers and clicommands, thus not affected by the new behavior. Though, a widget Graphs also uses it, probably in parallel with one of the controllers/clicommands. Will check whether that's a problem later.
The latter seems to use them instead of constants. Weird, but not a problem.

Assigning a negative index to an empty array

https://www.php.net/manual/en/migration83.incompatible.php#migration83.incompatible.core.negative-index-to-empty-array

Difficult to check. phpstan will hopefully do so.

The range() function has had various changes

https://www.php.net/manual/en/migration83.incompatible.php#migration83.incompatible.standard

Funny. range('0', '9') will now return strings in the array, whereas range('0', '10') does not. (or anything larger than one char) Should not be applicable to us. If it is, phpstan will hopefully detect it.

Saner Increment/Decrement operators

https://www.php.net/manual/en/migration83.deprecated.php#migration83.deprecated.core.saner-inc-dec-operators

Difficult to check. phpstan will hopefully do so.

get_class()/get_parent_class() call without arguments

https://www.php.net/manual/en/migration83.deprecated.php#migration83.deprecated.core.get-class

Not applicable to our code parts, but to a vendor part. (zf1-future's OpenId extension, unused by us)

@nilmerg nilmerg added this to the 2.12.1 milestone Nov 14, 2023
@nilmerg nilmerg added the enhancement New feature or improvement label Nov 14, 2023
@nilmerg
Copy link
Member

nilmerg commented Nov 14, 2023

Will check whether that's a problem later.

Did some testing. Seems to have no effect.

nilmerg pushed a commit that referenced this issue Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants