[PATCH v1] [meson] add a default option prefix=/usr/local/pgsql

Started by Junwang Zhaoover 3 years ago11 messages
#1Junwang Zhao
zhjwpku@gmail.com
1 attachment(s)

autoconf set PREFIX to /usr/local/pgsql, so I think we should
do the same in meson build.

This will group all the targets generated by postgres in the same directory.

--
Regards
Junwang Zhao

Attachments:

0001-meson-add-a-default-option-prefix-usr-local-pgsql.patchapplication/octet-stream; name=0001-meson-add-a-default-option-prefix-usr-local-pgsql.patchDownload
From 01dbfaaabd022dcb6972f186993c386074d42faf Mon Sep 17 00:00:00 2001
From: Junwang Zhao <zhjwpku@gmail.com>
Date: Fri, 30 Sep 2022 23:11:04 +0800
Subject: [PATCH v1] [meson] add a default option prefix=/usr/local/pgsql

autoconf set PREFIX to /usr/local/pgsql, so I think we should
do the same in meson build.

Signed-off-by: Junwang Zhao <zhjwpku@gmail.com>
---
 meson.build | 1 +
 1 file changed, 1 insertion(+)

diff --git a/meson.build b/meson.build
index 38b2c3aae2..903bbf2950 100644
--- a/meson.build
+++ b/meson.build
@@ -16,6 +16,7 @@ project('postgresql',
   default_options: [
     'warning_level=1', #-Wall equivalent
     'buildtype=release',
+    'prefix=/usr/local/pgsql',
   ]
 )
 
-- 
2.33.0

#2Andres Freund
andres@anarazel.de
In reply to: Junwang Zhao (#1)
Re: [PATCH v1] [meson] add a default option prefix=/usr/local/pgsql

Hi,

On 2022-09-30 23:21:22 +0800, Junwang Zhao wrote:

autoconf set PREFIX to /usr/local/pgsql, so I think we should
do the same in meson build.

That makes sense.

One concern with that is that default would also apply to windows - autoconf
didn't have to care about that. I just tried it, and it "just" ends up
installing it into c:/usr/local/pgsql (assuming the build dir is in
c:/<something>). I think that's something we could live with, but it's worth
thinking about.

Greetings,

Andres Freund

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#2)
Re: [PATCH v1] [meson] add a default option prefix=/usr/local/pgsql

Andres Freund <andres@anarazel.de> writes:

On 2022-09-30 23:21:22 +0800, Junwang Zhao wrote:

autoconf set PREFIX to /usr/local/pgsql, so I think we should
do the same in meson build.

That makes sense.

+1

One concern with that is that default would also apply to windows - autoconf
didn't have to care about that. I just tried it, and it "just" ends up
installing it into c:/usr/local/pgsql (assuming the build dir is in
c:/<something>). I think that's something we could live with, but it's worth
thinking about.

Can we have a platform-dependent default? What was the default
behavior with the MSVC scripts?

regards, tom lane

#4Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#3)
Re: [PATCH v1] [meson] add a default option prefix=/usr/local/pgsql

Hi,

On 2022-09-30 11:45:35 -0400, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

One concern with that is that default would also apply to windows - autoconf
didn't have to care about that. I just tried it, and it "just" ends up
installing it into c:/usr/local/pgsql (assuming the build dir is in
c:/<something>). I think that's something we could live with, but it's worth
thinking about.

Can we have a platform-dependent default?

Not easily in that spot, I think.

What was the default behavior with the MSVC scripts?

The install script always needs a target directory. And pg_config_paths is
always set to:
print $o <<EOF;
#define PGBINDIR "/bin"
#define PGSHAREDIR "/share"
#define SYSCONFDIR "/etc"
#define INCLUDEDIR "/include"
#define PKGINCLUDEDIR "/include"
#define INCLUDEDIRSERVER "/include/server"
#define LIBDIR "/lib"
#define PKGLIBDIR "/lib"
#define LOCALEDIR "/share/locale"
#define DOCDIR "/doc"
#define HTMLDIR "/doc"
#define MANDIR "/man"
EOF

Greetings,

Andres Freund

#5Andres Freund
andres@anarazel.de
In reply to: Andres Freund (#4)
Re: [PATCH v1] [meson] add a default option prefix=/usr/local/pgsql

Hi,

On 2022-09-30 08:59:53 -0700, Andres Freund wrote:

On 2022-09-30 11:45:35 -0400, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

One concern with that is that default would also apply to windows - autoconf
didn't have to care about that. I just tried it, and it "just" ends up
installing it into c:/usr/local/pgsql (assuming the build dir is in
c:/<something>). I think that's something we could live with, but it's worth
thinking about.

Can we have a platform-dependent default?

Not easily in that spot, I think.

For background: The reason for that is that meson doesn't yet know what the
host/build environment is, because those can be influenced by
default_options. We can run programs though, so if we really want to set some
platform dependent default, we can.

Greetings,

Andres Freund

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#5)
Re: [PATCH v1] [meson] add a default option prefix=/usr/local/pgsql

Andres Freund <andres@anarazel.de> writes:

On 2022-09-30 08:59:53 -0700, Andres Freund wrote:

On 2022-09-30 11:45:35 -0400, Tom Lane wrote:

Can we have a platform-dependent default?

Not easily in that spot, I think.

For background: The reason for that is that meson doesn't yet know what the
host/build environment is, because those can be influenced by
default_options. We can run programs though, so if we really want to set some
platform dependent default, we can.

Meh. It's not like the existing MSVC script behavior is so sane
that we should strive to retain it.

regards, tom lane

#7Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#6)
Re: [PATCH v1] [meson] add a default option prefix=/usr/local/pgsql

Hi,

On 2022-09-30 13:13:29 -0400, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

On 2022-09-30 08:59:53 -0700, Andres Freund wrote:

On 2022-09-30 11:45:35 -0400, Tom Lane wrote:

Can we have a platform-dependent default?

Not easily in that spot, I think.

For background: The reason for that is that meson doesn't yet know what the
host/build environment is, because those can be influenced by
default_options. We can run programs though, so if we really want to set some
platform dependent default, we can.

Meh. It's not like the existing MSVC script behavior is so sane
that we should strive to retain it.

Agreed - I was just trying to give background. I'm inclined to just go for
Junwang Zhao's patch for now.

Greetings,

Andres Freund

#8Andres Freund
andres@anarazel.de
In reply to: Andres Freund (#7)
Re: [PATCH v1] [meson] add a default option prefix=/usr/local/pgsql

Hi,

On 2022-09-30 10:17:37 -0700, Andres Freund wrote:

Agreed - I was just trying to give background. I'm inclined to just go for
Junwang Zhao's patch for now.

That turns out to break tests on windows right now - but it's not the fault of
the patch. Paths on windows are just evil:

We do the installation for tmp_install with DESTDIR (no surprise) just as in
autoconf. To set PATH etc, we need a path to the bindir inside that. Trivial
on unixoid systems. Not so much on windows. The obvious problematic cases are
things like a prefix of c:/something: Can't just prepend tmp_install/.

I'd hacked that up for c:/ style paths. But that doesn't work for paths like
/usr/local, because they're neither absolute nor relative, but "drive
relative". And then there's like a gazillion other things. A prefix could be
'//computer/share/path/to/' and all other sorts of nastiness.

I see two potential ways of dealing with this reliably on windows: - error out
if a prefix is not drive-local, that's easy enough to check, something like:
normalized_prefix.startswith('/') and not normalized_prefix.startswith('//')
as the installation on windows is relocatable, that's not too bad a
restriction - if on windows call a small python helper to compute the path of
tmp_install + prefix, using the code that meson uses for the purpose

Greetings,

Andres Freund

#9Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#8)
Re: [PATCH v1] [meson] add a default option prefix=/usr/local/pgsql

Andres Freund <andres@anarazel.de> writes:

I see two potential ways of dealing with this reliably on windows: - error out
if a prefix is not drive-local, that's easy enough to check, something like:
normalized_prefix.startswith('/') and not normalized_prefix.startswith('//')
as the installation on windows is relocatable, that's not too bad a
restriction - if on windows call a small python helper to compute the path of
tmp_install + prefix, using the code that meson uses for the purpose

I'd be inclined to keep it simple for now. This seems like something
that could be improved later in a pretty localized way, and it's not
like there's not tons of other things that need work.

regards, tom lane

#10Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#9)
2 attachment(s)
Re: [PATCH v1] [meson] add a default option prefix=/usr/local/pgsql

Hi,

On 2022-09-30 21:19:03 -0400, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

I see two potential ways of dealing with this reliably on windows: - error out
if a prefix is not drive-local, that's easy enough to check, something like:
normalized_prefix.startswith('/') and not normalized_prefix.startswith('//')
as the installation on windows is relocatable, that's not too bad a
restriction - if on windows call a small python helper to compute the path of
tmp_install + prefix, using the code that meson uses for the purpose

I'd be inclined to keep it simple for now. This seems like something
that could be improved later in a pretty localized way, and it's not
like there's not tons of other things that need work.

Just not sure which of the two are simpler, particularly taking docs into
account...

The attached 0001 calls into a meson helper command to do this. Not
particularly pretty, but less code than before, and likely more reliable.

Alternatively, the code meson uses for this is trivial, we could just stash it
in a windows_tempinstall_helper.py as well:

import sys
from pathlib import PureWindowsPath as PurePath

def destdir_join(d1: str, d2: str) -> str:
if not d1:
return d2
# c:\destdir + c:\prefix must produce c:\destdir\prefix
return str(PurePath(d1, *PurePath(d2).parts[1:]))

print(destdir_join(sys.argv[1], sys.argv[2]))

Greetings,

Andres Freund

Attachments:

v2-0001-meson-windows-Determine-path-to-tmp_install-prefi.patchtext/x-diff; charset=us-asciiDownload
From 20db3a7cbe72dcaaf21ec310f486718de4905f91 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Fri, 30 Sep 2022 18:54:11 -0700
Subject: [PATCH v2 1/2] meson: windows: Determine path to tmp_install + prefix
 using meson

Previously some paths (like c:\ or d:/) worked, but plenty others (like
/path/to or //computer/share/path) didn't. As we'd like to change the default
prefix to /usr/local/pgsql, that's a problem.

Instead of trying to do this in meson.build, call out to the implementation
meson install uses. This isn't pretty, but it's more reliable than what we had
before.

Discussion: https://postgr.es/CAEG8a3LGWE-gG6vuddmH91RORhi8gWs0mMB-hcTmP3_NVgM7dg@mail.gmail.com
---
 meson.build | 40 ++++++++++++++++------------------------
 1 file changed, 16 insertions(+), 24 deletions(-)

diff --git a/meson.build b/meson.build
index 0cd09ba4308..0a62be8bc69 100644
--- a/meson.build
+++ b/meson.build
@@ -29,6 +29,7 @@ fs = import('fs')
 pkgconfig = import('pkgconfig')
 
 host_system = host_machine.system()
+build_system = build_machine.system()
 host_cpu = host_machine.cpu_family()
 
 cc = meson.get_compiler('c')
@@ -2739,32 +2740,23 @@ endif
 # Test prep
 ###############################################################
 
-# The determination of where a DESTDIR install points to is ugly, it's somewhat hard
-# to combine two absolute paths portably...
-
-prefix = get_option('prefix')
-
-test_prefix = fs.as_posix(prefix)
-
-if fs.is_absolute(get_option('prefix'))
-  if host_system == 'windows'
-    if prefix.split(':/').length() == 1
-      # just a drive
-      test_prefix = ''
-    else
-      test_prefix = prefix.split(':/')[1]
-    endif
-  else
-    assert(prefix.startswith('/'))
-    test_prefix = './@0@'.format(prefix)
-  endif
-endif
-
-# DESTDIR for the installation used to run tests in
+# DESTDIR for the installation we'll run tests in
 test_install_destdir = meson.build_root() / 'tmp_install/'
-# DESTDIR + prefix appropriately munged
-test_install_location = test_install_destdir / test_prefix
 
+# DESTDIR + prefix appropriately munged
+if build_system != 'windows'
+  # On unixoid systems this is trivial, we just prepend the destdir
+  assert(dir_prefix.startswith('/')) # enforced by meson
+  test_install_location = '@0@@1@'.format(test_install_destdir, dir_prefix)
+else
+  # drives, drive-relative paths, etc make this complicated on windows, call
+  # meson's logic for it
+  command = [
+    meson_bin, meson_args, 'runpython', '-c',
+    'import sys; from mesonbuild.scripts import destdir_join; print(destdir_join(sys.argv[4], sys.argv[5]))',
+    test_install_destdir, dir_prefix]
+  test_install_location = run_command(command, check: true).stdout().strip()
+endif
 
 meson_install_args = meson_args + ['install'] + {
     'meson': ['--quiet', '--only-changed', '--no-rebuild'],
-- 
2.37.3.542.gdd3f6c4cae

v2-0002-meson-Add-prefix-usr-local-pgsql-to-default_optio.patchtext/x-diff; charset=us-asciiDownload
From e191b6b8b262461c8725a8706a68d9fc366a0048 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Fri, 30 Sep 2022 10:51:45 -0700
Subject: [PATCH v2 2/2] meson: Add prefix=/usr/local/pgsql to default_options

autoconf set PREFIX to /usr/local/pgsql, so using the same default for meson
makes sense. The effect on windows is that installation defaults to installing
to C:/usr/local/pgsql rather than meson's default of C:/, which doesn't seem
perfect, but OK enough.

Signed-off-by: Junwang Zhao <zhjwpku@gmail.com>
Author: Junwang Zhao <zhjwpku@gmail.com>
Discussion: https://postgr.es/CAEG8a3LGWE-gG6vuddmH91RORhi8gWs0mMB-hcTmP3_NVgM7dg@mail.gmail.com
---
 meson.build | 1 +
 1 file changed, 1 insertion(+)

diff --git a/meson.build b/meson.build
index 0a62be8bc69..7719270a2e6 100644
--- a/meson.build
+++ b/meson.build
@@ -16,6 +16,7 @@ project('postgresql',
   default_options: [
     'warning_level=1', #-Wall equivalent
     'buildtype=release',
+    'prefix=/usr/local/pgsql',
   ]
 )
 
-- 
2.37.3.542.gdd3f6c4cae

#11Andres Freund
andres@anarazel.de
In reply to: Andres Freund (#10)
Re: [PATCH v1] [meson] add a default option prefix=/usr/local/pgsql

Hi,

On 2022-09-30 19:07:21 -0700, Andres Freund wrote:

The attached 0001 calls into a meson helper command to do this. Not
particularly pretty, but less code than before, and likely more reliable.

I pushed Junwang Zhao's patch together with this change, after adding a
comment to the setting of the default prefix explaining how it affects
windows.

Greetings,

Andres Freund