RE: doc: pgevent.dll location

Hi,

PostgreSQL 17.0 Documentation teach us to run the following command.

regsvr32 pgsql_library_directory/pgevent.dll

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.

How do you think about this?

Regards,
Ryohei Takahashi

On Mon, Nov 4, 2024 at 7:02 PM Ryohei Takahashi (Fujitsu)
<r(dot)takahashi_2(at)fujitsu(dot)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(at)singh(dot)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(at)gmail(dot)com> writes:
> On Tue, Nov 5, 2024 at 1:00 AM Gurjeet Singh <gurjeet(at)singh(dot)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(at)gmail(dot)com> wrote:

> On Tue, Nov 5, 2024 at 1:00 AM Gurjeet Singh <gurjeet(at)singh(dot)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:

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(at)eisentraut(dot)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(at)pgadmin(dot)org> wrote:
>
> Hi
>
> On Wed, 6 Nov 2024 at 16:11, Peter Eisentraut <peter(at)eisentraut(dot)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/
>

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(at)eisentraut(dot)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(at)eisentraut(dot)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(at)eisentraut(dot)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(at)eisentraut(dot)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(at)eisentraut(dot)org> wrote:
> On 12.11.24 18:27, Robert Haas wrote:
> > On Tue, Nov 12, 2024 at 11:52 AM Peter Eisentraut <peter(at)eisentraut(dot)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(at)eisentraut(dot)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

On 12.11.24 17:52, Peter Eisentraut 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().

I developed the attached patch for this.

I haven't tested it locally, but I checked in the CI logs that the build
commands for pgevent don't change with this. CI doesn't test meson
install, it seems, so can't actually tell whether this changes the
installation directory successfully, but I think it should work.
Someone with Windows handy please give this a test.

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