On Sun, Aug 21, 2016 at 3:02 AM, Tom Lane <t...@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.1471713...@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
diff --git a/src/bin/initdb/Makefile b/src/bin/initdb/Makefile index 531cc97..394eae0 100644 --- a/src/bin/initdb/Makefile +++ b/src/bin/initdb/Makefile @@ -30,7 +30,7 @@ OBJS= initdb.o findtimezone.o localtime.o encnames.o $(WIN32RES) all: initdb -initdb: $(OBJS) | submake-libpgport +initdb: $(OBJS) | submake-libpq submake-libpgport submake-libpgfeutils $(CC) $(CFLAGS) $(OBJS) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X) # We used to pull in all of libpq to get encnames.c, but that diff --git a/src/bin/pg_dump/Makefile b/src/bin/pg_dump/Makefile index 25336a5..08b01d7 100644 --- a/src/bin/pg_dump/Makefile +++ b/src/bin/pg_dump/Makefile @@ -17,7 +17,7 @@ top_builddir = ../../.. include $(top_builddir)/src/Makefile.global override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS) -LDFLAGS += -L$(top_builddir)/src/fe_utils -lpgfeutils -lpq +LDFLAGS += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport) OBJS= pg_backup_archiver.o pg_backup_db.o pg_backup_custom.o \ pg_backup_null.o pg_backup_tar.o pg_backup_directory.o \ diff --git a/src/bin/pg_upgrade/Makefile b/src/bin/pg_upgrade/Makefile index 8823288..aa3c45b 100644 --- a/src/bin/pg_upgrade/Makefile +++ b/src/bin/pg_upgrade/Makefile @@ -12,7 +12,7 @@ OBJS = check.o controldata.o dump.o exec.o file.o function.o info.o \ tablespace.o util.o version.o $(WIN32RES) override CPPFLAGS := -DDLSUFFIX=\"$(DLSUFFIX)\" -I$(srcdir) -I$(libpq_srcdir) $(CPPFLAGS) -LDFLAGS += -L$(top_builddir)/src/fe_utils -lpgfeutils -lpq +LDFLAGS += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport) all: pg_upgrade diff --git a/src/bin/pgbench/Makefile b/src/bin/pgbench/Makefile index 1503d00..228eed5 100644 --- a/src/bin/pgbench/Makefile +++ b/src/bin/pgbench/Makefile @@ -10,7 +10,7 @@ include $(top_builddir)/src/Makefile.global OBJS = pgbench.o exprparse.o $(WIN32RES) override CPPFLAGS := -I. -I$(srcdir) -I$(libpq_srcdir) $(CPPFLAGS) -LDFLAGS += -L$(top_builddir)/src/fe_utils -lpgfeutils -lpq +LDFLAGS += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport) ifneq ($(PORTNAME), win32) override CFLAGS += $(PTHREAD_CFLAGS) diff --git a/src/bin/psql/Makefile b/src/bin/psql/Makefile index 1f6a289..2c727c5 100644 --- a/src/bin/psql/Makefile +++ b/src/bin/psql/Makefile @@ -19,7 +19,7 @@ include $(top_builddir)/src/Makefile.global REFDOCDIR= $(top_srcdir)/doc/src/sgml/ref override CPPFLAGS := -I. -I$(srcdir) -I$(libpq_srcdir) $(CPPFLAGS) -LDFLAGS += -L$(top_builddir)/src/fe_utils -lpgfeutils -lpq +LDFLAGS += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport) OBJS= command.o common.o help.o input.o stringutils.o mainloop.o copy.o \ startup.o prompt.o variables.o large_obj.o describe.o \ diff --git a/src/bin/scripts/Makefile b/src/bin/scripts/Makefile index 8c107b1..7f218f5 100644 --- a/src/bin/scripts/Makefile +++ b/src/bin/scripts/Makefile @@ -19,7 +19,7 @@ include $(top_builddir)/src/Makefile.global PROGRAMS = createdb createlang createuser dropdb droplang dropuser clusterdb vacuumdb reindexdb pg_isready override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS) -LDFLAGS += -L$(top_builddir)/src/fe_utils -lpgfeutils -lpq +LDFLAGS += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport) all: $(PROGRAMS)
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers