Re: [PATCH] pg_regress and non-default unix socket path
От | Christoph Berg |
---|---|
Тема | Re: [PATCH] pg_regress and non-default unix socket path |
Дата | |
Msg-id | 20130414105354.GA27954@msgid.df7cb.de обсуждение исходный текст |
Ответ на | Re: [PATCH] pg_regress and non-default unix socket path (Tom Lane <tgl@sss.pgh.pa.us>) |
Список | pgsql-hackers |
Re: Tom Lane 2013-04-12 <20318.1365786018@sss.pgh.pa.us> > Robert Haas <robertmhaas@gmail.com> writes: > > The hunk that changes the messages might need some thought so that it > > doesn't cause a translation regression. But in general I see no > > reason not to do this before we release beta1. It seems safe enough, > > and changes that reduce the need for packagers to carry private > > patches are, I think, generally a good thing. > > It looks to me like this is asking for pg_regress to adopt a nonstandard > interpretation of PGHOST, which doesn't seem like a wise thing at all, > especially if it's not documented. If you are saying pg_regress isn't always honoring PGHOST because it (re-)sets the variable itself, that's not the fault of my patch. My patch fixes pg_regress to make "--host /tmp" work, which is just the same thing as "psql --host /tmp" does. It's undocumented just like the existing behavior (--host works in temp-install mode) is undocumented. I can fix that, my original goal was to change as little as possible in the code. The only thing that doesn't work anymore is that you cannot set both listen_addresses *and* unix_socket_directories at the same time anymore. Does Red Hat need that functionality? I don't think regression tests need that flexibility so it's worth the trouble. (The proper fix would probably be to get rid of --host in temp-install mode and expose listen_addresses and unix_socket_director{y,ies} directly.) > FWIW, the equivalent thing in the Red Hat/Fedora packages can be seen > in this patch: > > http://pkgs.fedoraproject.org/cgit/postgresql.git/plain/postgresql-var-run-socket.patch This is exactly the opposite of what my patch is doing. The Red Hat patch hardcodes /tmp, which my patch is not. We use to have such a patch, and it was bad, because we needed to apply and revert the patch at built time, such that the version in the .deb didn't have the hardcoded path. For reference, here's the old Debian patch (the 9.1 packages still have it, the new one is in 9.2): http://anonscm.debian.org/loggerhead/pkg-postgresql/postgresql-9.1/trunk/annotate/head:/debian/pg_regress-in-tmp.patch In lines 139 ff of debian/rules, the patch gets applied and reverted: http://anonscm.debian.org/loggerhead/pkg-postgresql/postgresql-9.1/trunk/annotate/head:/debian/rules > which would not get noticeably shorter if we hacked pg_regress in the > suggested way. AFAICT, instead of touching pg_regress.c, Red Hat's > patch would need to do something to the regression Makefiles if we > wanted to use this implementation. I'm not convinced that'd be better > at all. TBH, if this is committed, the Red Hat patches will probably > end up reverting it. You are already setting PGHOST in contrib/pg_upgrade/test.sh. This would just need to be replaced by "EXTRA_REGRESS_OPTS='--host=/tmp'". Then your pg_regress.c chunk could go away. (To put it another way, you don't seem to be using pg_regress --host here, you shouldn't be affected by this change.) I'm open to suggestions of how to better fix this in pg_regress. Though could we perhaps first apply this patch as a first step in the right direction? Christoph -- cb@df7cb.de | http://www.df7cb.de/
В списке pgsql-hackers по дате отправления: