Thread: Re: doc: pgevent.dll location
On Mon, Nov 4, 2024 at 7:02 PM Ryohei Takahashi (Fujitsu) <r.takahashi_2@fujitsu.com> wrote: > > On the other hand, pgevent.dll is moved from lib/ directory to bin/directory on PG 17.0 > > (It seems that it is moved because of meson) > > So, I think the documentation need to be updated like attached. I'm unable to confirm the veracity of the claim that the pgevent.dll file on Windows is now placed in a different directory, and that this is a result of meson builds. But the patch looks good to me; it applies cleanly to REL_17_STABLE and main branches. Best regards, Gurjeet http://Gurje.et
On Tue, Nov 5, 2024 at 1:00 AM Gurjeet Singh <gurjeet@singh.im> wrote: > I'm unable to confirm the veracity of the claim that the pgevent.dll > file on Windows is now placed in a different directory, and that this > is a result of meson builds. But the patch looks good to me; it > applies cleanly to REL_17_STABLE and main branches. I think we will need to find someone who can confirm whether or not the claim is correct. -- Robert Haas EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Nov 5, 2024 at 1:00 AM Gurjeet Singh <gurjeet@singh.im> wrote:
>> I'm unable to confirm the veracity of the claim that the pgevent.dll
>> file on Windows is now placed in a different directory, and that this
>> is a result of meson builds. But the patch looks good to me; it
>> applies cleanly to REL_17_STABLE and main branches.
> I think we will need to find someone who can confirm whether or not
> the claim is correct.
More to the point: if that does happen, isn't it a bug to be fixed
rather than documented?
regards, tom lane
On Tue, 5 Nov 2024 at 15:10, Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, Nov 5, 2024 at 1:00 AM Gurjeet Singh <gurjeet@singh.im> wrote:
> I'm unable to confirm the veracity of the claim that the pgevent.dll
> file on Windows is now placed in a different directory, and that this
> is a result of meson builds. But the patch looks good to me; it
> applies cleanly to REL_17_STABLE and main branches.
I think we will need to find someone who can confirm whether or not
the claim is correct.
It is correct. In v16, it was in lib/, in v17, bin/.
However, on Windows I think it probably makes (marginally) more sense to include it in bin/ - we already have other libraries in there that are linked at compile time. pgevent is something of a special case, but it's probably more similar to those libraries than the extensions etc. that are in lib/.
Related sidenote: I got a complaint today that pg_regress.exe is no longer part of the Windows installation footprint - and indeed, it's not installed at all with Meson builds, which is problematic for those doing CI/CD testing of extensions.
Dave Page
pgAdmin: https://www.pgadmin.org
PostgreSQL: https://www.postgresql.org
Hi, Thank you for your reply. The dll install paths are changed as follows on Windows. (1) pgevent.dll PG16: lib/ PG17: bin/ (2) dll for user (like libpq.dll, libecpg.dll) PG16: Both in lib/ and bin/ PG17: bin/ (3) contrib dll (like amcheck.dll) PG16: lib/ PG17: lib/ I understand that Dave says (1) and (2) should exist on bin/ directory and the paths in PG17 are correct. So, I think it is correct to update documentation. Regards, Ryohei Takahashi
On 06.11.24 13:57, Ryohei Takahashi (Fujitsu) wrote: > The dll install paths are changed as follows on Windows. > > (1) pgevent.dll > PG16: lib/ > PG17: bin/ > > (2) dll for user (like libpq.dll, libecpg.dll) > PG16: Both in lib/ and bin/ > PG17: bin/ > > (3) contrib dll (like amcheck.dll) > PG16: lib/ > PG17: lib/ > > > I understand that Dave says (1) and (2) should exist on bin/ directory > and the paths in PG17 are correct. > > So, I think it is correct to update documentation. I don't have Windows handy to test it out, but looking at the respective build system source files, in master, pgevent is built and installed like a normal shared library in both meson.build and Makefile, so it should end up somewhere in lib. In src/tools/msvc in REL_16_STABLE, I see some code that appears to suggest that it installs in bin. Again, this is just reading the code, but it seems to be backwards from what is claimed earlier. The statements like "in PGxxx it did this" are not precise enough because there are three possible build systems. We need to know what each build system is doing. Also, the consideration of consistency should go in two dimensions: Consistency between versions and consistency between build systems.
Hi
On Wed, 6 Nov 2024 at 16:11, Peter Eisentraut <peter@eisentraut.org> wrote:
On 06.11.24 13:57, Ryohei Takahashi (Fujitsu) wrote:
> The dll install paths are changed as follows on Windows.
>
> (1) pgevent.dll
> PG16: lib/
> PG17: bin/
>
> (2) dll for user (like libpq.dll, libecpg.dll)
> PG16: Both in lib/ and bin/
> PG17: bin/
>
> (3) contrib dll (like amcheck.dll)
> PG16: lib/
> PG17: lib/
>
>
> I understand that Dave says (1) and (2) should exist on bin/ directory
> and the paths in PG17 are correct.
>
> So, I think it is correct to update documentation.
I don't have Windows handy to test it out, but looking at the respective
build system source files, in master, pgevent is built and installed
like a normal shared library in both meson.build and Makefile, so it
should end up somewhere in lib.
In src/tools/msvc in REL_16_STABLE, I see some code that appears to
suggest that it installs in bin.
Again, this is just reading the code, but it seems to be backwards from
what is claimed earlier.
I downloaded the builds of v16 and v17 from
https://github.com/dpage/winpgbuild/actions/runs/11639786998, a project worked on by Andres, Dave Cramer, and myself.
I've just double-checked I didn't get mixed up, and can confirm that in v17, pgevent.dll is installed into bin/ and in v16.4, it's in lib/
The statements like "in PGxxx it did this" are not precise enough
because there are three possible build systems. We need to know what
each build system is doing. Also, the consideration of consistency
should go in two dimensions: Consistency between versions and
consistency between build systems.
Yeah, sorry - I tend not to even think about Cygwin or Msys builds as they haven't been used for "official" builds on Windows in many, many years.
The builds I'm testing are MSVC++ using the old perl based build system for v16, and MSVC++ using Meson for v17. You can see exactly how they're done in the Github Action script for the action run above.
On Thu, Nov 7, 2024 at 1:46 PM Dave Page <dpage@pgadmin.org> wrote: > > Hi > > On Wed, 6 Nov 2024 at 16:11, Peter Eisentraut <peter@eisentraut.org> wrote: >> >> On 06.11.24 13:57, Ryohei Takahashi (Fujitsu) wrote: >> > The dll install paths are changed as follows on Windows. >> > >> > (1) pgevent.dll >> > PG16: lib/ >> > PG17: bin/ >> > >> > (2) dll for user (like libpq.dll, libecpg.dll) >> > PG16: Both in lib/ and bin/ >> > PG17: bin/ >> > >> > (3) contrib dll (like amcheck.dll) >> > PG16: lib/ >> > PG17: lib/ >> > >> > >> > I understand that Dave says (1) and (2) should exist on bin/ directory >> > and the paths in PG17 are correct. >> > >> > So, I think it is correct to update documentation. >> >> I don't have Windows handy to test it out, but looking at the respective >> build system source files, in master, pgevent is built and installed >> like a normal shared library in both meson.build and Makefile, so it >> should end up somewhere in lib. >> >> In src/tools/msvc in REL_16_STABLE, I see some code that appears to >> suggest that it installs in bin. >> >> Again, this is just reading the code, but it seems to be backwards from >> what is claimed earlier. > > > I downloaded the builds of v16 and v17 from > https://github.com/dpage/winpgbuild/actions/runs/11639786998, a project worked on by Andres, Dave Cramer, and myself. > > I've just double-checked I didn't get mixed up, and can confirm that in v17, pgevent.dll is installed into bin/ and inv16.4, it's in lib/ > I can re-confirm that on my Windows setup, pgevent.dll is present in bin/ for PG17 and HEAD and in lib/ for PG16. -- With Regards, Amit Kapila.
On Wed, Nov 6, 2024 at 11:11 AM Peter Eisentraut <peter@eisentraut.org> wrote: > I don't have Windows handy to test it out, but looking at the respective > build system source files, in master, pgevent is built and installed > like a normal shared library in both meson.build and Makefile, so it > should end up somewhere in lib. > > In src/tools/msvc in REL_16_STABLE, I see some code that appears to > suggest that it installs in bin. > > Again, this is just reading the code, but it seems to be backwards from > what is claimed earlier. > > The statements like "in PGxxx it did this" are not precise enough > because there are three possible build systems. We need to know what > each build system is doing. Also, the consideration of consistency > should go in two dimensions: Consistency between versions and > consistency between build systems. To what three systems are you referring? I thought we now only supported building with meson on Windows, and this is a Windows-specific file. Everyone seems to be saying the file has moved in v17, so it seems like we should either move it back or update the documentation as proposed. The question of why it has moved is perhaps worth some investigation, but seems like a secondary issue. -- Robert Haas EDB: http://www.enterprisedb.com
On 12.11.24 17:02, Robert Haas wrote: > On Wed, Nov 6, 2024 at 11:11 AM Peter Eisentraut <peter@eisentraut.org> wrote: >> I don't have Windows handy to test it out, but looking at the respective >> build system source files, in master, pgevent is built and installed >> like a normal shared library in both meson.build and Makefile, so it >> should end up somewhere in lib. >> >> In src/tools/msvc in REL_16_STABLE, I see some code that appears to >> suggest that it installs in bin. >> >> Again, this is just reading the code, but it seems to be backwards from >> what is claimed earlier. >> >> The statements like "in PGxxx it did this" are not precise enough >> because there are three possible build systems. We need to know what >> each build system is doing. Also, the consideration of consistency >> should go in two dimensions: Consistency between versions and >> consistency between build systems. > > To what three systems are you referring? I thought we now only > supported building with meson on Windows, and this is a > Windows-specific file. Everyone seems to be saying the file has moved > in v17, so it seems like we should either move it back or update the > documentation as proposed. The question of why it has moved is perhaps > worth some investigation, but seems like a secondary issue. The three possible build systems are: 1. src/tools/msvc/ 2. meson 3. make The first one was removed, but the second and the third one still exist. I can tell from the build farm logs (fairywren) that on master with meson the installation layout is Installing src\\bin\\pgevent\\pgevent.dll to C:/tools/xmsys64/home/pgrunner/bf/root/HEAD/inst\\bin Installing src\\bin\\pgevent\\pgevent.dll.a to C:/tools/xmsys64/home/pgrunner/bf/root/HEAD/inst\\lib This appears to be the meson default for shared libraries, because we also have Installing src/interfaces/libpq\\libpq.dll to C:/tools/xmsys64/home/pgrunner/bf/root/HEAD/inst\\bin Installing src/interfaces/libpq\\libpq.dll.a to C:/tools/xmsys64/home/pgrunner/bf/root/HEAD/inst\\lib We don't have any coverage on master for make on Windows, but I can see on REL_15_STABLE (fairywren) /usr/bin/install -c -m 755 pgevent.dll '/home/pgrunner/bf/root/REL_15_STABLE/inst/lib/postgresql/pgevent.dll' and I strongly suspect that this is unchanged in master. Note here that meson installs this file as a shared library (like libpq) but make installs this file as a shared module (like plpgsql). Also, looking at the source code of the old msvc build system (Install.pm, sub CopySolutionOutput), it will install this file into lib in the logic that is is a "dll" file that is *not* a shared library (has no SO_MAJOR_VERSION set). So the msvc system appears to be consistent with the make system in that respect. So the most straightforward way to "make it work like it used to" would be to change src/bin/pgevent/meson.build to use shared_module() instead of shared_library(). Based on the explanation in the documentation, this file is really more of a plugin and less a shared library, so I think that approach would be more correct than not.
On Tue, Nov 12, 2024 at 11:52 AM Peter Eisentraut <peter@eisentraut.org> wrote: > So the most straightforward way to "make it work like it used to" would > be to change src/bin/pgevent/meson.build to use shared_module() instead > of shared_library(). > > Based on the explanation in the documentation, this file is really more > of a plugin and less a shared library, so I think that approach would be > more correct than not. I don't understand what the difference is between a shared module and a shared library, so I find it odd that meson treats them differently. However, if the consensus is that moving it back to lib is better than updating the documentation, that's fine with me. -- Robert Haas EDB: http://www.enterprisedb.com
On 12.11.24 18:27, Robert Haas wrote: > On Tue, Nov 12, 2024 at 11:52 AM Peter Eisentraut <peter@eisentraut.org> wrote: >> So the most straightforward way to "make it work like it used to" would >> be to change src/bin/pgevent/meson.build to use shared_module() instead >> of shared_library(). >> >> Based on the explanation in the documentation, this file is really more >> of a plugin and less a shared library, so I think that approach would be >> more correct than not. > > I don't understand what the difference is between a shared module and > a shared library, so I find it odd that meson treats them differently. In this context, a shared module is something like plpgsql or hstore that you dlopen, and a shared library is something like libpq or libecpg that you -l at build time.
On Tue, Nov 12, 2024 at 3:06 PM Peter Eisentraut <peter@eisentraut.org> wrote: > On 12.11.24 18:27, Robert Haas wrote: > > On Tue, Nov 12, 2024 at 11:52 AM Peter Eisentraut <peter@eisentraut.org> wrote: > >> So the most straightforward way to "make it work like it used to" would > >> be to change src/bin/pgevent/meson.build to use shared_module() instead > >> of shared_library(). > >> > >> Based on the explanation in the documentation, this file is really more > >> of a plugin and less a shared library, so I think that approach would be > >> more correct than not. > > > > I don't understand what the difference is between a shared module and > > a shared library, so I find it odd that meson treats them differently. > > In this context, a shared module is something like plpgsql or hstore > that you dlopen, and a shared library is something like libpq or libecpg > that you -l at build time. That's interesting, but I would have thought both of those would go in libdir (or a subdirectory) not bindir. -- Robert Haas EDB: http://www.enterprisedb.com
Hi,
On 2024-11-12 16:11:44 -0500, Robert Haas wrote:
> On Tue, Nov 12, 2024 at 3:06 PM Peter Eisentraut <peter@eisentraut.org> wrote:
> > In this context, a shared module is something like plpgsql or hstore
> > that you dlopen, and a shared library is something like libpq or libecpg
> > that you -l at build time.
>
> That's interesting, but I would have thought both of those would go in
> libdir (or a subdirectory) not bindir.
That's true on unixoid systems, but due to the way library searchpaths work on
windows, you pretty much need to store them alongside the binary. Otherwise
the dynamic linker won't find the library. And because that happens before
application code is executed, you don't have a chance to specify a different
path ([1]). So on windows shared libraries are typically located alongside
the executable. Which we did for src/tools/msvc and do for make/meson based
builds on windows.
In contrast to that, if you do dlopen() with an explicit path, you can
obviously locate the file anywhere. So for postgres extension libs etc we
could keep them in lib/.
But it's not entirely clear how to describe pgevent.dll - which lead to the
change between src/tools/msvc and meson based builds.
FWIW, the main reason for the shared_library() vs shared_module() distinction
are macos and windows (and also AIX), where the build process for them is
different than on most unices. Libraries aren't allowed to have undefined
symbols there. That's fine for typical dynamic libraries, but for plugin style
things, it means they explicitly need to told where the symbols come from. On
windows by listing the symbols, on macos by explicitly linking extensions
against the postgres binary.
Greetings,
Andres Freund
[1] well, there are some hacky ways to execute code earlier and change the
library path, but it's quite complicated
Hi, Thank you for your patch. The patch worked in my Windows build environment and pgevent.dll is installed to lib/ directory. Regards, Ryohei Takahashi
On 15.11.24 14:53, Ryohei Takahashi (Fujitsu) wrote: > Thank you for your patch. > The patch worked in my Windows build environment and pgevent.dll is installed to lib/ directory. Thanks for checking. I have committed this patch and backpatched it to PG16 and PG17. So the next minor releases will presumably have the file back in the correct place.
Hi, Thank you for your commit. Regards, Ryohei Takahashi