Thread: [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. This will group all the targets generated by postgres in the same directory. -- Regards Junwang Zhao
Attachment
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
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
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
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
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
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
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
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
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
Attachment
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