Skip to content

fix ldap.h detection without pkgconfig #19005

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

Merged
merged 1 commit into from
Jul 3, 2025

Conversation

remicollet
Copy link
Member

Failing with old openldap, without pkgconfig (ex: openldap-2.4.46 on RJEL-8)


checking for lber ldap... no
configure: error: Package requirements (lber ldap) were not met:

Package 'lber', required by 'virtual:world', not found
Package 'ldap', required by 'virtual:world', not found

PKG_CHECK_MODULES should have an action-if-not-found arg

This also re-add search in default paths if not set by option

@remicollet
Copy link
Member Author

ping @NattyNarwhal authbor of 3677871

@remicollet remicollet requested a review from NattyNarwhal July 2, 2025 07:21
remicollet referenced this pull request Jul 2, 2025
* Use pkg-config for ext/ldap without a directory

The existing check is not very good. OpenLDAP has been shipping a
pkg-config file for a while now, and that's preferable over trying to
manually find it.

Note that for older OpenLDAP or non-OpenLDAP implementations, a
directory can still be passed to use the old logic. Note that Oracle
LDAP is busted and going away soon; I'm not sure for other LDAP
implementations.

Tested on macOS 14.

* Convert added ifs to AS_IF
Copy link
Member

@NattyNarwhal NattyNarwhal left a comment

Choose a reason for hiding this comment

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

Makes sense. I would personally tend towards setting the i.e. LDAP_LIBS variables, but we want the defaults to work.

@remicollet remicollet merged commit 840dc19 into php:master Jul 3, 2025
9 checks passed
@remicollet remicollet deleted the issue-ldap-h branch July 3, 2025 13:24
@remicollet
Copy link
Member Author

Thanks for reviews
Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants