fix zlib Windows build for 1.3.2+ cmake output naming change#1041
fix zlib Windows build for 1.3.2+ cmake output naming change#1041GhostPirateBob wants to merge 9 commits intocrazywhalecc:mainfrom
Conversation
|
This wouldn't be enough given that lib.json defines the expected filename. |
|
Ah yeah, and openssl.php would break because it hardcodes zlibstatic.lib What if I create a copy for both static lib aliases?
you wouldn't have to change openssl.php, gd_config files or lib.json No downstream affects 🤔 ... I think! updated my PR with fix for those hardcoded references |
|
No, simply rename them everywhere. We generally do not support old versions of libraries. |
src/globals/extra/gd_config_80.w32
Outdated
|
|
||
| // zlib ext support (required) | ||
| if (!CHECK_LIB("zlib_a.lib;zlib.lib", "gd", PHP_GD)) { | ||
| if (!CHECK_LIB("zlib_a.lib;z.lib", "gd", PHP_GD)) { |
There was a problem hiding this comment.
I thought zlib_a.lib no longer exists?
There was a problem hiding this comment.
zlib_a.lib never came from cmake, its just a copy target name the zlib.php builder creates via copy()
It's confusing because zlib_a.lib is (from SPC's naming convention, created by the builder), the cmake output names that changed are different
Good news is nothing in lib.json needs to change! 😀
The expected static lib on Windows is zlib_a.lib, so zlib.php builder creates it:
copy(BUILD_LIB_PATH . '\zs.lib', BUILD_LIB_PATH . '\zlib_a.lib');- Then
isLibraryInstalled()inLibraryBase.phpchecks thatzlib_a.libexists inBUILD_LIB_PATH - Finally
SPCConfigUtilpasses it to the linker
That's the whole chain.
The CHECK_LIB("zlib_a.lib;z.lib", ...) is just a fallback from our copy to z.lib (raw cmake output).
There was a problem hiding this comment.
If zlib_a doesn't really exist, we shouldn't create it for no reason. @crazywhalecc is there some edge case where a weird library expects that name? If not, simply use zs.lib.
There was a problem hiding this comment.
Yeah it's essentially arbitrary, but you're right, it makes life harder and more confusing for anyone new to keep it going.
If all those filenames are changing anyway then you might as well use that chance to make it more consistent and not misleading.
Not technically part of the zlib changes, just a build artifact with a weird Windows history/convention.
I'll change it to all be consistent, legacy Windows cmake artifacts with arbitrary but hardcoded paths passed directly to the linker was probably not a great idea to begin with 😉
There was a problem hiding this comment.
Sweet, much cleaner and straightforward now.
I was able to get it down to +11 -8 lines and +4 of those lines are
// zlib 1.3.2 changed cmake output names on Windows (see #1039):
// zlibstatic.lib -> zs.lib
// zlib.lib -> z.lib
// zlib.dll -> z.dll
Should be good to go
There was a problem hiding this comment.
We don't need the history of version in our builder files. Can remove these 4 lines.
Like I said, we're only ever supporting the latest version of each library.
There was a problem hiding this comment.
Nice, I feel better removing about as much as I add.
Fixes crazywhalecc#1039 zlib 1.3.2 changed cmake output file names on Windows: zlibstatic.lib -> zs.lib, zlib.lib -> z.lib, zlib.dll -> z.dll Update all references to use the new names directly.
282df01 to
e12fb33
Compare
|
I'm still on vacation, and I accidentally shut down my home remote server, so I'm not be able to test this PR these days. I suggest doing two tests in Actions, using a combination of spc-min and spc-max. |
Updated the list of PHP extensions for Windows and changed the base combination setting.
| SET(CMAKE_EXE_LINKER_FLAGS "{$ldflags}") | ||
| SET(CMAKE_FIND_ROOT_PATH "{$buildroot}") | ||
| SET(CMAKE_MSVC_RUNTIME_LIBRARY MultiThreaded) | ||
| SET(ZLIB_LIBRARY "{$buildroot}/lib/zs.lib") |
There was a problem hiding this comment.
That is definitely the wrong place to put this.
There was a problem hiding this comment.
No worries!
I just put that in so I could get through the tests, I'm going through the extension list to ensure my changes didn't break any of the listed compatible ones. But I had to work around a couple of other build issues to finished testing.
Test Extensions: amqp, apcu, bcmath, bz2, calendar, ctype, curl, dba, dom, ds, exif, ffi, fileinfo, filter, ftp, iconv, igbinary, libxml, mbregex, mbstring, mysqli, mysqlnd, opcache, openssl, pdo_mysql, pdo_sqlite, pdo_sqlsrv, phar, rar, redis, session, shmop, simdjson, simplexml, soap, sockets, sqlite3, sqlsrv, ssh2, sysvshm, tokenizer, xml, xmlreader, xmlwriter, yac, yaml, zip, zlib
I was hoping that ZLIB_LIBRARY one I could ask your advice or leave it with you. I wouldn't normally push test code to an active PR but I didn't know a better way to trigger the full workflow and keep testing the extension list without triggering a push (doesn't run properly on my fork)
Anyways, I've got a short list of PHP 8.5 / ZST / Windows extensions issues that are mostly unrelated things, eg WARNING: OpenSSL argon2 hashing not supported in ZTS mode for now which is a new issue, causes an assert to fail assert(function_exists('openssl_password_hash')) etc...
Almost done and then I'll revert that one and leave it with you.
Cheers!
There was a problem hiding this comment.
Openssl hash is only supported on ZTS with php 8.5.
There was a problem hiding this comment.
Cool, I got all the tests to pass with only a few hacky workarounds. I'll revert them now and then post the extensions that still had issues building. Should help to decide how best to handle them.
There was a problem hiding this comment.
I reverted the workarounds for testing and re-wrote the PR since it's changed a bunch,
remove gd as it has pre-existing other issue copy zs.lib to zlib_a.lib to see if it builds at least 7 extensions all rely on zlib_a.lib in their config.w32 windows pdo is enabled by default pdo_sqlsrv compilation errors against PHP 8.5 skip openssl_password_hash check in ZTS mode Fixes assertion fail in text since now PHP 8.5 disables openssl_password_hash in ZTS mode flag is PHP_ZTS not ZTS
07a63d6 to
a66db67
Compare
What this PR does
Fixes #1039 - zlib 1.3.2 changed its cmake output file names on Windows:
zlibstatic.libzs.libzlib.libz.libzlib.dllz.dllThis PR updates all hardcoded references to the old names across 5 files:
src/SPC/builder/windows/library/zlib.php- builder uses new names, copieszs.libtozlib_a.libfor PHP compatibility (see below)src/SPC/builder/windows/library/openssl.php- Makefile patching useszs.libconfig/lib.json-static-libs-windowsupdated tozs.libsrc/globals/extra/gd_config_80.w32-CHECK_LIBuseszs.libsrc/globals/extra/gd_config_81.w32-CHECK_LIBuseszs.libTested on Windows Server 2022 and 2025 with PHP 8.5 ZTS, 48 extensions, all passing.
Why
copy(zs.lib, zlib_a.lib)PHP's own
config.w32files (and those of PECL extensions) hardcodezlib_a.libas the expected static library name. Without the copy, these extensions fail at configure time:ext/zlib/config.w32zlib_a.lib;zlib.libext/zip/config.w32zlib_a.libext/ssh2/config.w32zlib_a.libThese are in PHP's source tree (or PECL), not in SPC, so we can't change them. The copy is the simplest workaround.
ZLIB_LIBRARY hint for FindZLIB.cmake (not in this PR)
While debugging these zlib file paths I found out that
FindZLIB.cmakesearches for library namesz,zlib,zdll,zlib1,zlibstatic. But notzs. This is hard-coded and not much can really be done about it.I wrote this hacky workaround for testing, but not sure what the right fix would be for that one.
The workaround gets
SystemUtil::makeCmakeToolchainFile()linking again. I know that at least libssh2 has this issue, and probably a few others too.PHP 8.5 extension issues found (unrelated to zlib)
While testing the full extension set on PHP 8.5 some unrelated or pre-existing issues cropped up. You can probably ignore them but might save you some time debugging later :)
pdo_sqlsrv -- fails to compile against PHP 8.5
The
_pdo_dbh_tstruct hadquery_stmt_zvalremoved, and thepdo_error_modeenum values changed, I just removed it for testing.gd -- fails to compile due to a missing
gd_pixelate.cIt's not in the bundled libgd source file list,
ext/gd/libgd/gd_interpolation.creferences it, but the file isn't built ¯_(ツ)_/¯Anyways, as this PR stands with the current changes around ~45 of 50 extensions will build on Windows.
Two of them, pdo_sqlsrv and gd I'm pretty sure have no chance and are just broken.
You can get up to 48 of 50 building with
copy(BUILD_LIB_PATH . '\zs.lib', BUILD_LIB_PATH . '\zlibstatic.lib')and a couple of the dodgier workarounds above but I'll leave that for you guys to think about the best approach.Cheers!
gpb