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

[zend-ldap] php 8.1 & 8.2 compatibility fixes #159

Merged
merged 4 commits into from
Feb 19, 2023
Merged

[zend-ldap] php 8.1 & 8.2 compatibility fixes #159

merged 4 commits into from
Feb 19, 2023

Conversation

marcing
Copy link
Contributor

@marcing marcing commented Dec 7, 2022 •
edited by falkenhawk
Loading

Zend_Ldap: php 8.1 & 8.2 compatibility fixes.
Tested on openldap 2.5.x - some test assertions had to be adjusted.

Removed calls to ldap_sort while keeping sort features.

ldap_sort got deprecated in php 7.0.0, removed in php 8.0.0

ported from laminas/laminas-ldap@35162e6 and latest version of https://github.com/laminas/laminas-ldap/tree/2.18.x

partikus
partikus previously requested changes Dec 7, 2022
View reviewed changes
phpunit.xml.dist Outdated Show resolved Hide resolved
phpunit.xml.dist Outdated Show resolved Hide resolved
tests/Zend/Ldap/.ci/basedn.ldif Outdated Show resolved Hide resolved
tests/Zend/Ldap/Node/RootDseTest.php Outdated Show resolved Hide resolved
@partikus partikus force-pushed the fix-zend-ldap-openldap branch from c9841bc to f950404 Compare December 19, 2022 14:14
@marcing marcing force-pushed the fix-zend-ldap-openldap branch 7 times, most recently from 57e4e8f to 86efe0b Compare December 23, 2022 19:58
@marcing marcing marked this pull request as ready for review December 25, 2022 11:16
marcing
marcing commented Dec 25, 2022
View reviewed changes
Copy link
Contributor Author

@marcing marcing left a comment

Choose a reason for hiding this comment

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

Changes implemented.

marcing
marcing commented Dec 25, 2022
View reviewed changes
Copy link
Contributor Author

@marcing marcing left a comment

Choose a reason for hiding this comment

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

Changes implemented.

@marcing marcing dismissed partikus’s stale review December 25, 2022 11:18

Changes implemented.

falkenhawk
falkenhawk reviewed Dec 27, 2022
View reviewed changes
Copy link
Member

@falkenhawk falkenhawk left a comment

Choose a reason for hiding this comment

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

thanks a lot for making the ldap tests to run. That was hard.
Please adjust the PR title and description to actually reflect what this is all about.

.github/workflows/tests.yml Outdated Show resolved Hide resolved
.github/workflows/tests.yml Outdated Show resolved Hide resolved
.github/workflows/tests.yml Outdated Show resolved Hide resolved
.github/workflows/tests.yml Outdated Show resolved Hide resolved
packages/zend-ldap/library/Zend/Ldap.php Outdated Show resolved Hide resolved
tests/Zend/Ldap/ChangePasswordTest.php Outdated Show resolved Hide resolved
tests/Zend/Ldap/ConnectTest.php Outdated Show resolved Hide resolved
tests/Zend/Ldap/OriginalOfflineTest.php Show resolved Hide resolved
tests/Zend/Ldap/SearchTest.php Outdated Show resolved Hide resolved
tests/resources/openldap/docker-compose.yml Outdated Show resolved Hide resolved
@falkenhawk falkenhawk linked an issue Dec 27, 2022 that may be closed by this pull request
zf1s/zend-ldap doesn't work with PHP8 #124
Closed
glensc
glensc reviewed Dec 27, 2022
View reviewed changes
.github/workflows/tests.yml Outdated Show resolved Hide resolved
glensc
glensc reviewed Dec 27, 2022
View reviewed changes
.github/workflows/tests.yml Outdated Show resolved Hide resolved
glensc
glensc reviewed Dec 27, 2022
View reviewed changes
.github/workflows/tests.yml Outdated Show resolved Hide resolved
glensc
glensc reviewed Dec 27, 2022
View reviewed changes
.github/workflows/tests.yml Outdated Show resolved Hide resolved
glensc
glensc reviewed Dec 27, 2022
View reviewed changes
.github/workflows/tests.yml Outdated Show resolved Hide resolved
@marcing marcing changed the title [zend-ldap] adjusted tests to run on OpenLDAP: slapd 2.5.13+dfsg-0ubu… [zend-ldap] adjusted tests to run on OpenLDAP 2.5.13, PHP 8.1 & PHP 8.2 Jan 10, 2023
@partikus partikus force-pushed the fix-zend-ldap-openldap branch 3 times, most recently from 122ec21 to 47bc5d2 Compare January 10, 2023 22:42
glensc
glensc reviewed Jan 11, 2023
View reviewed changes
.github/workflows/tests.yml Outdated Show resolved Hide resolved
glensc
glensc reviewed Jan 11, 2023
View reviewed changes
.github/workflows/tests.yml Outdated Show resolved Hide resolved
@partikus partikus force-pushed the fix-zend-ldap-openldap branch 2 times, most recently from a8e342e to d3bf130 Compare January 17, 2023 15:51
falkenhawk
falkenhawk reviewed Jan 17, 2023
View reviewed changes
Copy link
Member

@falkenhawk falkenhawk left a comment

Choose a reason for hiding this comment

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

I don't think any changes around TESTS_ZEND_LDAP_PORT are necessary. imo they can be reverted. Wouldn't the ldap client connect to port 389 by default, whether it's specified or not?

tests/TestConfiguration.ci.php Outdated Show resolved Hide resolved
tests/TestConfiguration.ci.php Show resolved Hide resolved
packages/zend-ldap/library/Zend/Ldap.php Outdated Show resolved Hide resolved
@falkenhawk falkenhawk force-pushed the fix-zend-ldap-openldap branch from d3bf130 to 7d2701f Compare February 19, 2023 19:27
@falkenhawk falkenhawk changed the title [zend-ldap] adjusted tests to run on OpenLDAP 2.5.13, PHP 8.1 & PHP 8.2 [zend-ldap] adjust for openldap 2.5.x and php 8.1 & 8.2 Feb 19, 2023
@falkenhawk falkenhawk changed the title [zend-ldap] adjust for openldap 2.5.x and php 8.1 & 8.2 [zend-ldap] php 8.1 & 8.2 compatibility fixes Feb 19, 2023
@falkenhawk falkenhawk merged commit efdb6ea into master Feb 19, 2023
@falkenhawk falkenhawk deleted the fix-zend-ldap-openldap branch February 19, 2023 19:55
@hungtrinh hungtrinh mentioned this pull request Mar 2, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@glensc glensc glensc left review comments

@falkenhawk falkenhawk falkenhawk approved these changes

@partikus partikus Awaiting requested review from partikus

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

zf1s/zend-ldap doesn't work with PHP8
4 participants
@marcing @glensc @partikus @falkenhawk