On Sun, Aug 21, 2016 at 3:02 AM, Tom Lane <[email protected]> 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
[email protected]. 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 ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
