Subject: doc: pgevent.dll location

From: "Ryohei Takahashi (Fujitsu)" <[email-masked]>
Date: Tue, 5 Nov 2024 03:02:23 +0000

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

---

From: Gurjeet Singh <[email-masked]>
Date: Mon, 4 Nov 2024 22:00:10 -0800

> 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

---

From: Robert Haas <[email-masked]>
Date: Tue, 5 Nov 2024 10:09:54 -0500

> 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

---

From: Tom Lane <[email-masked]>
Date: Tue, 05 Nov 2024 10:15:48 -0500

> I think we will need to find someone who can confirm whether or not

More to the point: if that does happen, isn't it a bug to be fixed
rather than documented?

			regards, tom lane

---

From: Dave Page <[email-masked]>
Date: Tue, 5 Nov 2024 15:27:03 +0000

> I think we will need to find someone who can confirm whether or not

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

---

From: "Ryohei Takahashi (Fujitsu)" <[email-masked]>
Date: Wed, 6 Nov 2024 12:57:42 +0000

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

---

From: Peter Eisentraut <[email-masked]>
Date: Wed, 6 Nov 2024 17:11:43 +0100

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

---

From: Dave Page <[email-masked]>
Date: Thu, 7 Nov 2024 08:16:14 +0000

Hi

> Again, this is just reading the code, but it seems to be backwards from

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/

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

---

From: Amit Kapila <[email-masked]>
Date: Mon, 11 Nov 2024 16:41:51 +0530

On Thu, Nov 7, 2024 at 1:46 PM Dave Page <[email-masked]> wrote:
> 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.

---

From: Robert Haas <[email-masked]>
Date: Tue, 12 Nov 2024 11:02:53 -0500

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

---

From: Peter Eisentraut <[email-masked]>
Date: Tue, 12 Nov 2024 17:52:27 +0100

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

---

From: Robert Haas <[email-masked]>
Date: Tue, 12 Nov 2024 12:27:37 -0500

> of a plugin and less a shared library, so I think that approach would be

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.

---

From: Peter Eisentraut <[email-masked]>
Date: Wed, 13 Nov 2024 09:53:36 +0100

> be to change src/bin/pgevent/meson.build to use shared_module() instead 

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.
From 064013af8020112b5ee96538e9bf928c623b74e0 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email-masked]>
Date: Wed, 13 Nov 2024 09:49:25 +0100
Subject: [PATCH] meson: Build pgevent as shared_module rather than
 shared_library

This matches the behavior of the makefiles and the old MSVC build
system.  The main effect is that the build result gets installed into
pkglibdir rather than bindir.  The documentation says to locate the
library in pkglibdir, so this makes the code match the documentation
again.

Reported-by: Ryohei Takahashi (Fujitsu) <[email-masked]>
Discussion: https://www.postgresql.org/message-id/flat/TY3PR01MB118912125614599641CA881B782522%40TY3PR01MB11891.jpnprd01.prod.outlook.com

ci-os-only: windows
---
 src/bin/pgevent/meson.build | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/bin/pgevent/meson.build b/src/bin/pgevent/meson.build
index 5c2d296fef9..f7422438664 100644
--- a/src/bin/pgevent/meson.build
+++ b/src/bin/pgevent/meson.build
@@ -21,13 +21,11 @@ if cc.get_id() == 'msvc'
   pgevent_link_args += '/ignore:4104'
 endif
 
-pgevent = shared_library('pgevent',
+pgevent = shared_module('pgevent',
   pgevent_sources,
   dependencies: [frontend_code],
   link_args: pgevent_link_args,
   vs_module_defs: 'pgevent.def',
-  kwargs: default_lib_args + {
-    'name_prefix': '',
-  },
+  kwargs: default_mod_args,
 )
 bin_targets += pgevent
-- 
2.47.0

---

From: "Ryohei Takahashi (Fujitsu)" <[email-masked]>
Date: Fri, 15 Nov 2024 13:53:52 +0000

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

---

From: Peter Eisentraut <[email-masked]>
Date: Tue, 26 Nov 2024 18:15:45 +0100

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

---

From: "Ryohei Takahashi (Fujitsu)" <[email-masked]>
Date: Fri, 29 Nov 2024 11:41:39 +0000

Hi,

Thank you for your commit.

Regards,
Ryohei Takahashi