search

php / php-src

The PHP Interpreter
https://www.php.net
Other
38.23k stars 7.75k forks source link

Use pkg-config on Windows #16752

Open cmb69 opened 6 days ago

cmb69 commented 6 days ago

Description

On POSIX systems, we use pkg-config to configure most (all?) dependency libraries. On Windows, we still look for the headers and libraries "manually" (CHECK_LIB and CHECK_HEADER_ADD_INCLUDE, what is clumsy and likely more constraining than necessary. And maybe worse, it makes it harder to port m4 configurations to w32, or to keep them in sync. It gets especially annoying when we want to check for a certain package version (usually a minimum requirement). Instead I suggest to use pkg-config on Windows, too.

POC I've put a pkg-config.exe in the `PATH`, put a suitable zlib.pc in %DEPS%\lib\pkgconfig, and added that to `PKG_CONFIG_PATH`. Then I've applied the following patch to php-src: ````diff ext/zlib/config.w32 | 6 +++--- win32/build/confutils.js | 45 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/ext/zlib/config.w32 b/ext/zlib/config.w32 index 3bc24d88e1..623d752e57 100644 --- a/ext/zlib/config.w32 +++ b/ext/zlib/config.w32 @@ -3,9 +3,9 @@ ARG_ENABLE("zlib", "ZLIB support", "yes"); if (PHP_ZLIB == "yes") { - if (CHECK_LIB("zlib_a.lib;zlib.lib", "zlib", PHP_ZLIB) && - CHECK_HEADER_ADD_INCLUDE("zlib.h", "CFLAGS", "..\\zlib;" + php_usual_include_suspects)) { - + if (PKG_CHECK_MODULES("ZLIB", "zlib >= 1.2.11")) { + PHP_EVAL_INCLINE(ZLIB_CFLAGS); + PHP_EVAL_LIBLINE(ZLIB_LIBS, "zlib"); EXTENSION("zlib", "zlib.c zlib_fopen_wrapper.c zlib_filter.c", PHP_ZLIB_SHARED, "/D ZLIB_EXPORTS /DZEND_ENABLE_STATIC_TSRMLS_CACHE=1"); AC_DEFINE("HAVE_ZLIB", 1, "Define to 1 if the PHP extension 'zlib' is available."); diff --git a/win32/build/confutils.js b/win32/build/confutils.js index 3623dcf7e2..63bd1b867a 100644 --- a/win32/build/confutils.js +++ b/win32/build/confutils.js @@ -1054,6 +1054,51 @@ function CHECK_HEADER_ADD_INCLUDE(header_name, flag_name, path_to_check, use_env return p; } +function PKG_CHECK_MODULES(prefix, list_of_modules) +{ + var out; + STDOUT.Write("checking for " + list_of_modules + "... "); + if (!(out = execute("pkg-config --cflags " + list_of_modules))) { + STDOUT.WriteLine("no"); + return false; + } + eval(prefix + "_CFLAGS = out"); + if (!(out = execute("pkg-config --libs " + list_of_modules))) { + STDOUT.WriteLine("no"); + return false; + } + eval(prefix + "_LIBS = out"); + STDOUT.WriteLine("yes"); + return true; +} + +function PHP_EVAL_LIBLINE(libline, libs_variable, not_extension) +{ + libs_variable = libs_variable.toUpperCase(); + var args = libline.split(/\s/); + for (var i = 0; i < args.length; i++) { + var arg = args[i]; + if (arg.match(/^-l(.*)/)) { + ADD_FLAG("LIBS_" + libs_variable, RegExp.$1 + ".lib"); + } else if (arg.match(/^-L(.*)/)) { + var path = condense_path(RegExp.$1); + ADD_FLAG("LDFLAGS_" + libs_variable, '/libpath:"' + path + '" '); + ADD_FLAG("ARFLAGS_" + libs_variable, '/libpath:"' + path + '" '); + } + } +} + +function PHP_EVAL_INCLINE(headerline) +{ + var args = headerline.split(/\s/); + for (var i = 0; i < args.length; i++) { + var arg = args[i]; + if (arg.match(/^-I(.*)/)) { + ADD_FLAG("CFLAGS", "/I " + condense_path(RegExp.$1)); + } + } +} + /* XXX check whether some manifest was originally supplied, otherwise keep using the default. */ function generate_version_info_manifest(makefiletarget) { ````

If we want to take this route, we should ship pkg-config with https://github.com/php/php-sdk-binary-tools (we can probably use https://github.com/skeeto/u-config). And the winlibs builds would need to actually contain .pc files. I don't think it's viable to rebuilt all php-src dependency winlibs right away, but when updating to new versions, the *.pc files could be added, and then php-src could be adapted to use them.

Thoughts?

cmb69 commented 5 days ago

Hmm, not quite sure how we should approach the .pc files shipped with winlib builds.

So we could just roll out minimalist .pc files for our builds, not even catering to the "canonical" .pc names, and just have a single .pc file per package (i.e. not openssl.pc, libssl.pc and libcrypto.pc for OpenSSL, but only openssl.pc). That would save a lot of work (including rebuilding the packages), but might not be best for interoperability with custom dependency builds (although these may not be interoperable at all, see https://gitlab.freedesktop.org/pkg-config/pkg-config/-/merge_requests/13).

https://github.com/php/php-src/blob/33ba1a4ab9260b7fc2780fc353baa0d1c68a57f7/ext/snmp/config.w32#L7

is a nice example why not using something like pkg-config is not the best idea. We're unconditionally requiring OpenSSL, even though Net-SNMP might have been built without OpenSSL support.

Perhaps better examples are ext/ssh2 and ext/curl, which also require OpenSSL, although either libssh2 and libcurl could be built with Schannel support instead.

Or ext/zip, which requires to have liblzma, and sets a couple of defines in the CFLAGS, so makes some strong assumptions about how libzip had been built. Wanting to use these flags from .pc files would be a point pro proper .pc files.

cmb69 commented 5 days ago

Or ext/zip, which requires to have liblzma, and sets a couple of defines in the CFLAGS, so makes some strong assumptions about how libzip had been built.

Except for /D LZMA_API_STATIC (which might not even be needed), these defines are not something pkg-config (usually) would report. These flags are set whether some functions in the library exists, and these check are hard to replicate with our Windows AC compat layer, so hard-coding makes some sense (albeit these are still strong assumptions). But this is unrelated to this ticket.

Anyhow, I've build libzip as usual, and got the following libzip.pc:

prefix=C:/Program Files/libzip
exec_prefix=${prefix}
bindir=${prefix}/bin
libdir=${prefix}/lib
includedir=${prefix}/include

zipcmp=${bindir}/zipcmp

Name: libzip
Description: library for handling zip archives
Version: 1.11.2
Libs:  -L${libdir} -lzip
Libs.private:  -ladvapi32 -lbz2 -llzma -lbcrypt -lzlib_a
Cflags: -I${includedir}

That is pretty accurate (besides that -lzip won't work since we're calling it libzip_a.lib; likely an insufficient patch of winlibs), but how would PHP (or any other client) know whether to build against a static or shared library (in this case there is no shared library at all)? Note again that PHP ever only calls PKG_CHECK_MODULES (what assumes shared libraries); that might be appropriate for most systems, but on Windows we have quite a couple of static library dependencies. Why? Maybe for BC reasons, maybe for convenience (to avoid shipping many more DLLs which would not easily be found by Webserver modules), and sometimes likely to avoid problems with libraries wich are not fully thread-safe (e.g. Net-SNMP has init and shutdown routines; cf. libxml2; not sure that the commit message is accurate, but multi-threading can indeed be a problem).

So if we want to use pkg-config on Windows (and I still think this is a good idea for the reasons given in the OP), we should just roll our own .pc files. After all, it's not hard to manually craft (and some automation might still be possible) something like:

prefix=C:/usr
libdir=${prefix}/lib
includedir=${prefix}/include

Name: libzip
Description: winlibs' libzip package
Version: 1.11.2
Requires: libbzip2 zlib liblzma
Libs: -L${libdir} -llibzip_a
Cflags: -I${includedir}

Custom dependency builds could copy these and adapt if needed. And downstream consumers other than PHP (if there are any) would likely not be affected, since we've shipped no .pc files so far, so they probably don't use pkg-config at all.

NattyNarwhal commented 9 hours ago

FWIW, my nitpick here is suggesting pkgconf instead of u-config. It's portable and maintained (unlike fdo p-c), and has been around for years so it should be well tested with libraries (unlike u-c).