On 2023-01-26 10:20:58 +0100, Peter Eisentraut wrote:
> On 19.01.23 21:45, Andres Freund wrote:
> > Hi,
> >
> > On 2023-01-19 21:37:15 +0100, Peter Eisentraut wrote:
> > > On 11.01.23 12:05, Peter Eisentraut wrote:
> > > > 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 don't think it would. The / operator understands absolute paths and
> > doesn't
> > add the "first component" if the second component is absolute.
>
> Oh, that is interesting. In that case, this is not the right patch. We
> should proceed with my previous patch in [0] then.
WFM.
I still think it'd be slightly more legible if we tested the prefix for
postgres|pgsql once, rather than do the per-variable .contains() checks on the
"combined" path. But it's a pretty minor difference, and I'd have no problem
with you comitting your version.
Basically:
is_pg_prefix = dir_prefix.contains('pgsql) or dir_prefix.contains('postgres')
...
if not (is_pg_prefix or dir_data.contains('pgsql') or
dir_data.contains('postgres'))
instead of "your":
if not ((dir_prefix/dir_data).contains('pgsql') or
(dir_prefix/dir_data).contains('postgres'))
Greetings,
Andres Freund
