Refactor ext/mbstring/libmbfl/config.h usage#13713
Conversation
|
@petk Thanks for working on this. I don't know enough about this part of the build system to know if this PR is a good idea or not. 😆 |
arnaud-lb
left a comment
arnaud-lb
left a comment
There was a problem hiding this comment.
The change looks good to me. This causes ext/mbstring/libmbfl/ to diverge a bit more from the upstream, but as it's officially not kept in sync, I guess it's fine.
It can be sometimes useful to be able to build a single extension from php-src |
😄 PHP's vendored copy of libmbfl has diverged so much from upstream that it hardly makes sense to call it "libmbfl" any more. (Well, maybe that's an exaggeration, but not a big one.) |
|
Yes, I agree. I'll change this differently without hardcoding the config.h... The built-in config.h file is kind of weird. Like something done in half. Even if it is completely forked library only used in php-src now. It's still good to separate these things as much as possible. |
petk
force-pushed
the
patch-mbstring-config
branch
from
35ed3c5 to
f97e8d5
Compare
|
Ok, in the appended new commit there is now only the main/php_config.h file included. I think that when building with phpize the extension's config.h is a bit redundant and misses few CPP macros. So the new appended commit is targeted here. |
petk
force-pushed
the
patch-mbstring-config
branch
from
f97e8d5 to
078f9de
Compare
|
I've now added more changes here. Details in the commit. Probably the |
petk
force-pushed
the
patch-mbstring-config
branch
2 times, most recently
from
d1e7800 to
ab53468
Compare
This moves libmbfl configuration to mbstring extension level and syncs the include style used as of PHP-8.4 to be able to do simultaneous in-source and out-of-source builds. The HAVE_CONFIG_H file is when doing phpize build using Autotools. Checks for strcasecmp and strings.h are also moved to mbstring extension only. The libmbfl/config.h header is now also NOT installed anymore as it is a private header used only in C files as part of the build interface (during build phase only). The libmbfl/config.h is included with angled style so the include flags take precedence and simultaneous php-src building in-source and out-of-source can be performed.
petk
force-pushed
the
patch-mbstring-config
branch
from
ab53468 to
1601eed
Compare
|
For the time being, I've rebased this PR against current master branch and also removed the libmbfl/config.h from being installed as it was not meant to be a public header. Yes, this is "fun". Having libraries bundled in php-src and diverging them from being standalone is quite messy but no other way is possible here. I'll check more later on. |
3 participants
This simplifies the libmbfl/config.h usage and removes duplicate compile definitions on Windows (HAVE_STRICMP).
The _WIN32 symbol is always defined when compiling on Windows with any current compiler, the HAVE_CONFIG_H symbol is defined only when doing phpize build inside mbstring extension (an edge case but if that is used by someone perhaps), otherwise the regular main/php_config.h is used.
This is sort of an addition to make it managing changes in #13516 easier. The include style of angle brackets is based on the linked PR. Perhaps we don't even need phpize build for the mbstring extension since it is part of the php-src core. But I'm not sure what is the accepted usage for this so I've added it like it was previously in the config.m4 file (the config.h included with absolute path).
I'm adding also @arnaud-lb in the PR to have an overview a bit and when @alexdowad has time for checking this a bit.
Regarding Windows, I have another change planned there with this HAVE_STRICMP but that is beyond the scope here and can be done in the future.