Hi, On 2024-06-19 14:47:50 +0100, Dave Page wrote:
> > I'm *NOT* sure that vcpkg is the way to go, fwiw. It does seem > > advantageous to > > use one of the toolkits thats commonly built for building dependencies on > > windows, which seems to mean vcpkg or conan. > > > > I don't think requiring or expecting vcpkg or conan is reasonable at all, > for a number of reasons: > > - Neither supports all the dependencies at present. > - There are real supply chain verification concerns for vendors. > - That would be a huge change from what we've required in the last 19 > years, with no deprecation notices or warnings for packagers etc. I don't think we should hard-require one specifically. I do think it'd be good if we provided an easy recipe for dependencies to be installed though. I think such flexibility acually means it becomes *more* important to abstract away some of the concrete ways of using the various dependencies. It doesn't make sense for postgres to understand the internals of each dependency on all platforms to a detail that it can cope with all the different ways of linking against them. E.g. libxml can be built with icu, lzma, zlib support. If libxml is built statically, postgres needs to link to all those libraries as well. How can we know which of those dependencies are enabled? Even if we can make that somehow work, it's not reasonable for postgres developers adding a dependency to have to figure out how to probe all of this, when literally no other platform works that way anymore. If you look around at recipes for building postgres on windows, they all have to patch src/tools/msvc (see links at the bottom), because the builtin paths and flags just don't work outside of a single way of acquiring the dependency. The fact that this thread started only now is actually a good example for how broken the approach to internalize all knowledge about building against libraries into postgres is. This could all have been figured out 1+ years ago - but wasn't. Unless you want to require postgres devs to get constantly in the muck on windows, we'll never get that right until just before the release. I don't particularly care how we abstract away the low level linking details on windows. We can use pkgconf, we can use cmake, we can invent our own thing. But it has to be something other than hardcoding windows library paths and compiler flags into our buildsystem. And yes, that might make it a bit harder for a packager on windows, but windows is already a *massive* drag on PG developers, it has to be somewhat manageable. I do think we can make the effort of windows dependency management a lot more reasonable than it is now though, by providing a recipe for acquiring the dependency in some form. It's a lot easier to for packagers and developers to customize ontop of something like that. Frankly, the fact that there's pretty much no automated testing of the various dependencies that's accessible to non-windows devs is not a sustainable situation. > > Btw, I've been working with Bilal to add a few of the dependencies to the > > CI > > images so we can test those automatically. Using vcpkg. We got that nearly > > working, but he's on vacation this week... That does ensure both cmake and > > .pc files are generated, fwiw. > > > > Currently builds gettext, icu, libxml2, libxslt, lz4, openssl, pkgconf, > > python3, tcl, zlib, zstd. > > > That appears to be using Mingw/Msys, which is quite different from a VC++ > build, in part because it's a full environment with its own package manager > and packages that people have put a lot of effort into making work as they > do on unix. Err, that was a copy-paste mistake on my end and doesn't even use the vcpkg generated stuff. Here's an example build with most dependencies enabled (see below for more details): https://cirrus-ci.com/task/6497321108635648?logs=configure#L323 I started hacking a bit further on testing all dependencies, which led me down a few rabbitholes: - kerberos: When built linking against a debug runtime, it spews *ginormous* amounts of information onto stderr. Unfortunately its buildsystem doesn't seperate out debugging output and linking against a debug runtime. Argh. The tests fail even with a non-debug runtime though, due to debugging output in some cases, not sure why: https://cirrus-ci.com/task/5872684519653376?logs=check_world#L502 Separately, the kerberos tests don't seem to be prepared to work on windows :(. So I disabled using it in CI for now. - Linking the backend dynamically against lz4, icu, ssl, xml, xslt, zstd, zlib slows down the tests noticeably (~20%). So I ended up building those statically. I ran into some issue with using a static libintl. I made it work, but for now reverted to a dynamic one. - Enabling nls slows down the tests by about 15%, somewhat painful. This is when statically linking, it's a bit worse when linked dynamically :(. - readline: Instead of the old issue with a compiler error, now we get a compiler crash: https://developercommunity.visualstudio.com/t/tab-completec4023:-fatal-error-C1001:/10685868 The issue is fairly trivial to work around, we just need to break the the if/else chain into two. Probably deserves a bigger refactoring, but that's for another day. I haven't yet looked into a) uuid b) tcl. I think those are the only other missing dependencies. > > Many of them do include at least cmake files on windows if you build them > > though? > The only one that does is libxml2 as far as I can see. And that doesn't > seem to work even if I use --cmake-prefix-path= as you suggested: Ugh, that's because they used a different name for their cmake dependency than for pkg-config. We can add the alternative spelling to meson.build. > > > > And that's why we really need to be able to locate headers and libraries > > > easily by passing paths to meson, as we can't rely on pkgconfig, cmake, > > or > > > things being in some standard directory on Windows. > > > > Except that that often causes hard to diagnose breakages, because that > > doesn't allow including the necessary compiler/linker flags [2]. It's a > > bad model, we shouldn't perpetuate it. If we want to forever make windows > > a complicated annoying stepchild, that's the way to go. > > That is a good point, though I suspect it wouldn't solve your second > example of the Kerberos libraries, as you'll get both 32 and 64 bit libs if > you follow their standard process for building on Windows so you still need > to have code to pick the right ones. vcpkg for one does provide .pc files for kerberos. > > FWIW, at least libzstd, libxml [3], lz4, zlib can generate cmake dependency > > files on windows in their upstream code. > > > > In the case of zstd, it does not if you build with VC++, the Makefile, or > Meson, at least in my testing. When building with meson it does generate a .pc file, which does work with PG as-is. > It looks like it would if you built it with cmake, but I couldn't get that > to work in 10 minutes or so of messing around. > And that's a perfect example of what I'm bleating about - there are often > many ways of building things on Windows and there are definitely many ways > of getting things on Windows, and they're not all equal. Right - but that's precisely which is why it's unreasable for postgres to know all the ins and outs of the different file locations and compiler flags for all those sources. Hence needing to abstract that. > We've either got to be extremely prescriptive in our docs, telling people > precisely what they need to download for each dependency, or how to build it > themselves in the way that will work with PostgreSQL, or the build system > needs to be flexible enough to handle different dependency variations, as > the old VC++ build system was. I'm confused - the old build system wasn't flexible around this stuff *at all*. Everyone had to patch it to get dependencies to work, unless you chose exactly the right source to download from - which was often not documented or outdated. For example: - https://github.com/microsoft/vcpkg/blob/master/ports/libpq/windows/msbuild.patch - https://github.com/conan-io/conan-center-index/blob/1b24f7c74994ec6573e322b7ae4111c10f620ffa/recipes/libpq/all/conanfile.py#L116-L160 - https://github.com/conda-forge/postgresql-feedstock/tree/main/recipe/patches Greetings, Andres Freund