Subject: Re: meson oddities

From: Peter Eisentraut <[email-masked]>
Date: Wed, 4 Jan 2023 12:35:35 +0100

On 16.11.22 18:07, Andres Freund wrote:
>>> If I just want to install postgres into a prefix without 'postgresql' added in
>>> a bunch of directories, e.g. because I already have pg-$version to be in the
>>> prefix, there's really no good way to do so - you can't even specify
>>> --sysconfdir or such, because we just override that path.
>>
>> At least for the libraries, the point of the 'postgresql' subdir IMO
>> is to keep backend-loadable extensions separate from random libraries.
>> It's not great that we may fail to do that depending on what the
>> initial part of the library path is.
> 
> Agreed, extensions really should never be in a path searched by the dynamic
> linker, even if the prefix contains 'postgres'.
> 
> To me that's a separate thing from adding postgresql to datadir, sysconfdir,
> includedir, docdir... On a green field I'd say the 'extension library'
> directory should just always be extensions/ or such.

I think we should get the two build systems to produce the same 
installation layout when given equivalent options.

Unless someone comes up with a proposal to address the above broader 
issues, also taking into account current packaging practices etc., then 
I think we should do a short-term solution to either port the 
subdir-appending to the meson scripts or remove it from the makefiles 
(or maybe a bit of both).

---

From: Andres Freund <[email-masked]>
Date: Wed, 4 Jan 2023 11:35:02 -0800

Hi,

On 2023-01-04 12:35:35 +0100, Peter Eisentraut wrote:
> I think we should get the two build systems to produce the same installation
> layout when given equivalent options.

I'm not convinced that that's the right thing to do. Distributions have
helper infrastructure for buildsystems - why should we make it harder for them
by deviating further from the buildsystem defaults?

I have yet to hear an argument why installing libraries below
/usr/[local]/lib/{x86_64,i386,...}-linux-{gnu,musl,...}/ is the wrong thing to
do on Debian based systems (or similar, choosing lib64 over lib on RH based
systems).  But at the same time I haven't heard of an argument why we should
break existing scripts building with autoconf for this. To me a different
buildsystem is a convenient point to adapt to build path from the last decade.

> Unless someone comes up with a proposal to address the above broader issues,
> also taking into account current packaging practices etc., then I think we
> should do a short-term solution to either port the subdir-appending to the
> meson scripts or remove it from the makefiles (or maybe a bit of both).

Just to be clear, with 'subdir-appending' you mean libdir defaulting to
'lib/x86_64-linux-gnu' (or similar)? Or do you mean adding 'postgresql' into
various dirs when the path doesn't already contain postgres?

I did try to mirror the 'postgresql' adding bit in the meson build.

Greetings,

Andres Freund

---

From: Robert Haas <[email-masked]>
Date: Wed, 4 Jan 2023 16:06:15 -0500

> by deviating further from the buildsystem defaults?

If we don't do as Peter suggests, then any difference between the
results of one build system and the other could either be a bug or an
intentional deviation. There will be no easy way to know which it is.
And if or when people switch build systems, stuff will be randomly
different, and they won't understand why.

I hear your point too. It's unpleasant for you to spend a lot of
effort overriding meson's behavior if the result is arguably worse
than the default, and it has the effect of carrying forward in
perpetuity hacks that may not have been a good idea in the first
place, or may not be a good idea any more. Those seem like valid
concerns, too.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

---

From: Peter Eisentraut <[email-masked]>
Date: Wed, 4 Jan 2023 23:17:30 +0100

> I did try to mirror the 'postgresql' adding bit in the meson build.

I meant the latter, which I see is already in there, but it doesn't 
actually fully work.  It only looks at the subdirectory (like "lib"), 
not the whole path (like "/usr/local/pgsql/lib").  With the attached 
patch I have it working and I get the same installation layout from both 
build systems.

From 579411b5c443ce4c6f45f30627bbf891a53c8f77 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email-masked]>
Date: Wed, 4 Jan 2023 23:14:10 +0100
Subject: [PATCH] meson: Fix installation path computation

---
 meson.build | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/meson.build b/meson.build
index 8999170b4d..634dc7d167 100644
--- a/meson.build
+++ b/meson.build
@@ -467,19 +467,19 @@ dir_prefix = get_option('prefix')
 dir_bin = get_option('bindir')
 
 dir_data = get_option('datadir')
-if not (dir_data.contains('pgsql') or dir_data.contains('postgres'))
+if not ((dir_prefix/dir_data).contains('pgsql') or (dir_prefix/dir_data).contains('postgres'))
   dir_data = dir_data / pkg
 endif
 
 dir_sysconf = get_option('sysconfdir')
-if not (dir_sysconf.contains('pgsql') or dir_sysconf.contains('postgres'))
+if not ((dir_prefix/dir_sysconf).contains('pgsql') or (dir_prefix/dir_sysconf).contains('postgres'))
   dir_sysconf = dir_sysconf / pkg
 endif
 
 dir_lib = get_option('libdir')
 
 dir_lib_pkg = dir_lib
-if not (dir_lib_pkg.contains('pgsql') or dir_lib_pkg.contains('postgres'))
+if not ((dir_prefix/dir_lib_pkg).contains('pgsql') or (dir_prefix/dir_lib_pkg).contains('postgres'))
   dir_lib_pkg = dir_lib_pkg / pkg
 endif
 
@@ -489,7 +489,7 @@ dir_include = get_option('includedir')
 
 dir_include_pkg = dir_include
 dir_include_pkg_rel = ''
-if not (dir_include_pkg.contains('pgsql') or dir_include_pkg.contains('postgres'))
+if not ((dir_prefix/dir_include_pkg).contains('pgsql') or (dir_prefix/dir_include_pkg).contains('postgres'))
   dir_include_pkg = dir_include_pkg / pkg
   dir_include_pkg_rel = pkg
 endif
-- 
2.39.0

---

From: Andres Freund <[email-masked]>
Date: Wed, 4 Jan 2023 14:53:54 -0800

Hi,

On 2023-01-04 23:17:30 +0100, Peter Eisentraut wrote:
> I meant the latter, which I see is already in there, but it doesn't actually
> fully work.  It only looks at the subdirectory (like "lib"), not the whole
> path (like "/usr/local/pgsql/lib").  With the attached patch I have it
> working and I get the same installation layout from both build systems.

Oh, oops. I tested this at some point, but I guess I over-simplified it at
some point.

Then I have zero objections to this. One question below though.

>    dir_data = dir_data / pkg

Hm. Perhaps we should just test once whether prefix contains pgsql/postgres,
and then just otherwise leave the test as is? There afaict can't be a
dir_prefix/dir_* that matches postgres/pgsql that won't also match either of
the components.

Greetings,

Andres Freund

---

From: Peter Eisentraut <[email-masked]>
Date: Wed, 11 Jan 2023 12:05:34 +0100

> dir_prefix/dir_* that matches postgres/pgsql that won't also match either of

You mean something like

     dir_prefix_contains_pg =
       (dir_prefix.contains('pgsql') or dir_prefix.contains('postgres'))

and

     if not (dir_prefix_contains_pg or
            (dir_data.contains('pgsql') or dir_data.contains('postgres'))

Seems more complicated to me.

I think there is also an adjacent issue:  The subdir options may be 
absolute or relative.  So if you specify --prefix=/usr/local and 
--sysconfdir=/etc/postgresql, then

     config_paths_data.set_quoted('SYSCONFDIR', dir_prefix / dir_sysconf)

would produce something like /usr/local/etc/postgresql.

I think maybe we should make all the dir_* variables absolute right at 
the beginning, like

     dir_lib = get_option('libdir')
     if not fs.is_absolute(dir_lib)
         dir_lib = dir_prefix / dir_lib
     endif

And then the appending stuff could be done after that, keeping the 
current code.