Re: Patch: initdb: "'" for QUOTE_PATH (non-windows)
От | Michael Paquier |
---|---|
Тема | Re: Patch: initdb: "'" for QUOTE_PATH (non-windows) |
Дата | |
Msg-id | CAB7nPqTSM8ydS=nyW_rb3nE=sEQgrhAtXKEWGejPnehvWRsjjg@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: Patch: initdb: "'" for QUOTE_PATH (non-windows) (Tom Lane <tgl@sss.pgh.pa.us>) |
Ответы |
Re: Patch: initdb: "'" for QUOTE_PATH (non-windows)
(Tom Lane <tgl@sss.pgh.pa.us>)
|
Список | pgsql-hackers |
On Sun, Aug 21, 2016 at 3:02 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > I wrote: >> A bigger issue here is that it seems fundamentally wrong for initdb to be >> including libpq, because it surely is never meant to be communicating >> with a running postmaster. Not sure what to do about that. We could >> consider moving pqexpbuffer out of libpq into fe_utils, but I wonder >> whether that would break any third-party code. We've never advertised >> pqexpbuffer.h as a supported API of libpq, but it's probably handy enough >> that people use it anyway. I suppose we could duplicate it in fe_utils >> and libpq, though that's a tad ugly. Thoughts? > > I looked into this and soon found that fe_utils/string_utils.o has > dependencies on libpq that are much wider than just pqexpbuffer :-(. > It might be a project to think about sorting that out sometime, but it > looks like it would be an awful lot of work just to avoid having initdb > depend on libpq.so. So I now think this objection is impractical. > I'll just annotate the makefile entry to say that initdb itself doesn't > use libpq. pqexpbuffer.c is an independent piece of facility, so we could move it to src/common and leverage the dependency a bit, and have libpq link to the source file itself at build phase. The real problem is the call to PQmblen in psqlscan.l... And this, I am not sure how this could be refactored cleanly. >> Another perhaps-only-cosmetic issue is that now initdb prints quotes >> whether they are needed or not. I find this output pretty ugly: >> ... >> That's not really the fault of this patch perhaps. Maybe we could adjust >> appendShellString so it doesn't add quotes if they are clearly >> unnecessary. > > I still think this is worth fixing, but it's a simple modification. > Will take care of it. > > This item is listed in the commitfest as a bug fix, but given the lack of > previous complaints, and the fact that the printed command isn't really > meant to be blindly copied-and-pasted anyway (you're at least meant to > replace "logfile" with something), I do not think it needs back-patching. Agreed on that. Only first-time users do a copy-paste of the command anyway, so the impact is very narrow. And actually, I had a look at the build failure that you reported in 3855.1471713949@sss.pgh.pa.us. While that was because of a copy of libpq.so that I had in my own LD_LIBRARY_PATH, shouldn't all the other frontend utilities depending on fe_utils also use $(libpq_pgport) instead of -lpq? I think that you saw the error for initdb because that's the first one to be built in src/bin/, and I think that Ryan used -lpq in his patch because the same pattern is used everywhere else. It seems to me as well that submake-libpq and submake-libpgfeutils should be listed in initdb's Makefile when building the binary. Am I missing something? Please see the patch attached to see what I mean. Thinking harder about that, it may be better to even add a variable in Makefile.global.in for utilities in need of fe_utils, like that for example: libpg_feutils = -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport) Thoughts? -- Michael
Вложения
В списке pgsql-hackers по дате отправления: