Sync HAVE_OPENSSL* symbols #14333
Merged
Sync HAVE_OPENSSL* symbols #14333
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
petk
added a commit
to petk/pecl-web_services-oauth
that referenced
this pull request
May 26, 2024
This replaces the HAVE_OPENSSL_EXT condition with more reliable zend_hash_str_exists. If the openssl extension is loaded as a shared module on *nix, this symbol is also defined but openssl shared object (openssl.so) might not be loaded at the runtime. See: php/php-src#14333
This syncs few inconsistencies between the Windows and Autotools build systems: - HAVE_OPENSSL_EXT is now defined in the same style on both systems (undefined - extension is not available, defined to 1 - extension is available) - HAVE_OPENSSL removed as it was only defined on Windows
petk
force-pushed
the
patch-openssl-have-symbols
branch
from
3523f9d to
fcfe8bf
Compare
|
Then merge coming up here. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello, there is one more inconsistency in the build systems that I'd like to sync.
I've left this one as the last "HAVE_* symbols sync" in the PHP-8.4-dev branch (hopefully it is the last for the time being). These symbols are very error prone defined in the form of
HAVE_<extension>but I'll leave this be for now.So, now we have for the ext/openssl the
HAVE_OPENSSLdefined to 1 on Windows, andHAVE_OPENSSL_EXTdefined to 1 on both. On Windows theHAVE_OPENSSL_EXTis additionally defined to value 0 if the ext/openssl is built as shared. From my point of view this is not consistent and doesn't make much sense to be practical in extensions. Although, I understand the motivation of 0 and 1 value.My suggestion here would be:
HAVE_OPENSSL_EXTin the same style on both systems (undefined - extension is not available, defined to 1 - extension is available)HAVE_OPENSSLas it was only defined on Windows and it was in the past defined also elsewhereI've quickly scanned the PECL community open source extensions and so far only oath is using this HAVE_OPENSSL_EXT symbol with
#if HAVE_OPENSSL_EXT, which can be resolved quite simple. I'll also recheck the extensions again but it doesn't seem to be problematic.Your thoughts are welcome. Thanks.