Fix the Windows extension test failures on fable-v3-windows#1208
Conversation
- the meson build assumed meson/ninja/win_flex/win_bison/perl on PATH, which only held on machines that had them installed - add meson, ninja and winflexbison tool packages and declare them via tools@windows on postgresql, together with the existing strawberry-perl - meson no longer ships a Windows MSI, so the wheel is installed offline into a venv: postgres re-invokes meson through find_program, which needs the real meson.exe launcher pip generates
- php.exe already links with /FORCE:MULTIPLE for extension-bundled duplicate symbols, cgi and micro did not - imagick's MagickCore defines gettimeofday, also defined in php's time.obj, and both links failed with LNK2005
- libpq authenticates through SSPI and the static libcrypto behind it reads the system cert store; php.exe failed to link with unresolved Cert* and SSPI symbols when the openssl extension is not in the build
- static-libs@windows listed both libiconv.lib (the dll import lib) and libiconv_a.lib; the import lib came first, so php.exe ended up importing libiconv.dll and the cli smoke test died with STATUS_DLL_NOT_FOUND on machines without it
m-this
force-pushed
the
fix/1207-windows-ext-tests
branch
from
d52c61a to
43b2da0
Compare
|
Another possibility would be structuring to use winget/vcpkg to install these tools, as they don't affect the library or php binary links.
|
| } | ||
| } | ||
| if ($python === null) { | ||
| throw new EnvironmentException('meson needs Python 3 on PATH. Install it from https://www.python.org or with `winget install Python.Python.3.13`.'); |
There was a problem hiding this comment.
I'd prefer bundling the python-win tools with the installation, or embedding the Python installation into the Meson setup process. Otherwise, there would be the issue of requiring manual environment configuration.
Ideally, if the build process is overly complex, I would prefer handling the build in the cloud (similar to icu-static-win) or simply downloading a pre-built package like before. However, if installing a lightweight Python runtime solves the problem and not struggle the procedure, I think that would be acceptable as well.
There was a problem hiding this comment.
I think that's the cleanest option actually, so I went with it: there's now a python-win tool package that just unzips the official python.org nuget package into the pkgroot. It's a plain zip, nothing to install, and it ships venv so meson can build its env from it (with a fallback to system python just in case). Should also work on your LTSC since there's no winget or msys2 involved.
Checked on windows-latest that the venv really comes from the pkgroot python and not the runner's one: https://github.com/m-this/static-php-cli/actions/runs/28941945448
- add a python-win tool package that unzips the official python.org nuget package into the pkgroot (venv and ensurepip included) - meson uses it to build its venv and only falls back to a system python, so no manual environment setup is needed
henderkes
left a comment
henderkes
left a comment
There was a problem hiding this comment.
I think if we have that kind of dependency not handled by doctor, it should be. Winget is available on every Windows 10+ machine now so we really should be using it.
I have been using the LTSC version of Windows for testing, and it does not include winget. I am not entirely sure about the coverage here, but if we intend to use winget, we should first check for its presence and ensure native support within our package system (we could also include msys2 package together). Native support means we could declare these packages as libs' tool dependencies like we just added |
|
The LTSC versions are still missing it? It was a bit of a pain to install manually I remember. It would definitely simplify the dependency stuff though, both in terms of doctor fixing VS build tools as well as perl, python, meson and cmake. If you think it's acceptable to have users install winget if they don't have it, I think we should do it. |
|
Since Python is already installed, I think sticking with the current approach is sufficient for now. Even if we were to switch to winget, that should be handled as an improvement in a future branch. |
3 participants
Dug into the three Windows failures, turned out to be four separate bugs. All fixed here and verified green on
windows-latestwith the same build commands the test bot runs: https://github.com/m-this/static-php-cli/actions/runs/28927716538tools@windows, like jom on openssl. Meson gets installed from its wheel into a venv, since postgres needs a real meson.exe (it re-invokes meson throughfind_program).gdi32.lib, and php-cgi/micro then hit LNK2005 on duplicate symbols; they now link with/FORCE:MULTIPLElike php.exe already did.ext/pgsql/config.w32, same trick as your imagick patch.