Skip to content

Conversation

devnexen
Copy link
Member

No description provided.

@devnexen devnexen marked this pull request as ready for review September 12, 2025 11:41
@devnexen devnexen requested a review from Girgias September 12, 2025 11:42
@devnexen
Copy link
Member Author

Do not know if I need any more warning in other places. There does not seem any CI for using Oracle Instant Client.

ext/ldap/ldap.c Outdated
RETURN_THROWS();
}

zend_error(E_DEPRECATED, "Calling ldap_connect_wallet is deprecated");
Copy link
Member

Choose a reason for hiding this comment

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

I guess you should consistently use () when referring to a function name. You did so on line 976 but not here.

Copy link
Member

Choose a reason for hiding this comment

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

Also this should use the #[\Deprecated] attribute in the stub instead of triggering this in C.

@devnexen devnexen force-pushed the deprecate_oracle_ldap branch from 4975330 to 5132197 Compare September 12, 2025 15:39
@devnexen devnexen requested a review from a team September 12, 2025 18:20
Copy link
Member

@DanielEScherzer DanielEScherzer left a comment

Choose a reason for hiding this comment

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

RM review, left some notes inline but this is approved, technical review not performed

GSLC_SSL_NO_UATH
GSLC_SSL_ONEWAY_UATH
GSLC_SSL_TWOWAY_UATH

Copy link
Member

Choose a reason for hiding this comment

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

please link to the RFC

UPGRADING Outdated
Comment on lines 441 to 442
ldap_connect with wallet support
ldap_connect_wallet
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ldap_connect with wallet support
ldap_connect_wallet
ldap_connect() with wallet support
ldap_connect_wallet()

elif test -f $LDAP_LIBDIR/libclntsh.$SHLIB_SUFFIX_NAME.12.1 || test -f $LDAP_LIBDIR/$MACHINE_INCLUDES/libclntsh.$SHLIB_SUFFIX_NAME.12.1; then
PHP_ADD_LIBRARY_WITH_PATH([clntsh], [$LDAP_LIBDIR], [LDAP_SHARED_LIBADD])
AC_DEFINE([HAVE_ORALDAP], [1])
AC_MSG_WARN([Build with Oracle Instant Client is deprecated])
Copy link
Member

Choose a reason for hiding this comment

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

should we include the version of the deprecation here? I'm not familiar with the SOP for deprecations in the build configurations so if that isn't standard no need to include it

Copy link
Member Author

Choose a reason for hiding this comment

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

No that s a good point I ll add the version

@devnexen devnexen force-pushed the deprecate_oracle_ldap branch from 3b22db1 to 3efb7f2 Compare September 19, 2025 08:46
@devnexen devnexen merged commit a46be36 into php:master Sep 19, 2025
9 checks passed
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.

The ldap extension cannot be built on Solaris with Oracle Instant Client

3 participants