Re: Meson far from ready on Windows

Hi

On Thu, 20 Jun 2024 at 21:58, Andres Freund <andres(at)anarazel(dot)de> wrote:

>
> > 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.
>

That is precisely what https://github.com/dpage/winpgbuild/ was intended
for - and it works well for PG <= 16.

> 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?
>

I don't think it's unreasonable to not support static linking, but I take
your point.

> 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.
>

I've been responsible for the Windows installers at EDB since we started
work on them, and prior to that built the original ones with Magnus. Since
v8.0, I've implemented multiple frameworks for building those packages, and
for building PostgreSQL as a dependency of other things (e.g. pgAdmin).
I've done that using builds of dependencies found at random places on the
internet, building them all myself, and a mixture.

I have never once had to patch the MSVC build system. The most I've ever
had to do is copy/rename a .lib file - zlib, which for some reason uses
different naming depending on how you build it. I vaguely recall that
OpenSSL had a similar issue in the distant past.

My point is that we seem to be heading from minor hacks to get things
working in some corner cases, towards requiring packagers and other people
building PostgreSQL on Windows having to do significant work to make the
dependencies look as we now expect. I suspect even more people will end up
patching the Meson build system as it might actually be easier to get
things to work.

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.
>

<rant>
Right now I'd be happy to just have the old MSVC build system back until we
can figure out a less complicated way to get to Meson (which I fully
support).

My assumption all along was that Meson would replace autoconf etc. before
anything happened with MSVC, precisely because that's the type of
environment all the Postgres devs work in primarily. Instead we seem to
have taken what I think is a flawed approach of entirely replacing the
build system on the platform none of the devs use, whilst leaving the new
system as an experimental option on the platforms it will have had orders
of magnitude more testing.

What makes it worse, is that I don't believe anyone was warned about such a
drastic change. Packagers were told about the git archive changes to the
tarball generation, but that's it (and we've said before, we can't expect
packagers to follow all the activity on -hackers).
</rant>

> 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.
>

Well as I noted, that is the point of my Github repo above. You can just go
download the binaries from the all_deps action - you can even download the
config.pl and buildenv.pl that will work with those dependencies (those
files are artefacts of the postgresql action).

We/I *could* add cmake/pc file generation to that tool, which would make
things work nicely with PostgreSQL 17 of course - however my original aim
for the project was to build all the dependencies in their officially
documented way, using MSVC (or UCRT64 if MSVC can't be used) for maximum
compatibility with the PG build, specifically eliminating or at least
minimising any custom build steps/hacks. As it turns out, I think the only
hack I really have is to avoid having to do an otherwise unnecessary 32bit
build of krb5.

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

OK.

> 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.
>

Urgh, makes sense.

>
>
> - 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.
>

Curious. I wonder if that translates into a general 20% performance hit.
Presumably it would for anything that looks similar to whatever test/tests
are affected.

>
> 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 :(.
>

That one I can imagine being in psql, so maybe not a big issue for most
real world use cases.

>
>
> - 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.
>

We really need to replace ossp-uuid on Windows anyway. It's basically
abandoned these days. I haven't looked to see if the alternatives work on
Windows now.

>
>
> > > 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.
>

Yes - that's in the vcpkg repo. I suspect they're adding pc and cmake files
for a lot of things.

> > 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.
>

As I noted above - as the "owner" of the official packages, I never did
despite using a variety of upstream sources.

>
> For example:
> -
> https://github.com/microsoft/vcpkg/blob/master/ports/libpq/windows/msbuild.patch

That one looks almost entirely related to making PostgreSQL itself fit into
vcpkg's view of the world. It's changing the installation footprint, and
pulling some paths from their own variables. If they're changing our
installation footprint, it's likely they're doing the same for other
packages.

>
> -
> https://github.com/conan-io/conan-center-index/blob/1b24f7c74994ec6573e322b7ae4111c10f620ffa/recipes/libpq/all/conanfile.py#L116-L160

Same for that one. It's making many of those changes for non-Windows
platforms as well.

>
> -
> https://github.com/conda-forge/postgresql-feedstock/tree/main/recipe/patches

That one is interesting. It fixes the same zlib and OpenSSL issues I
mentioned being the one fix I did myself, albeit by renaming libraries
originally, and later by actually following the upstream build instructions
correctly.

It also makes essentially the same fix for krb5 that I hacked into my
Github Action, but similarly that isn't actually needed at all if you
follow the documented krb5 build process, which produces 32 and 64 bit
binaries.

Additionally, it also fixes a GSSAPI related bug which I reported a week or
two back here and for which there is a patch waiting to be committed, and
replaces some setenv calls with _putenv_.

There are a couple more patches in there, but they're Linux related from a
quick glance.

In short, I don't really see anything in those examples that are general
issues (aside from the bugs of course).

--
Dave Page
pgAdmin: https://www.pgadmin.org
PostgreSQL: https://www.postgresql.org
EDB: https://www.enterprisedb.com