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


Reply via email to