From a53fb90ef94ff3d2097539eeee38cc0b5bdf8d55 Mon Sep 17 00:00:00 2001 From: Jason Petersen Date: Sun, 6 Mar 2016 20:32:02 -0700 Subject: [PATCH] Fix various build issues I came across several places we weren't as flexible or resilient as we should have been in our build logic. They include: * Not using `DESTDIR` in the install-header destination * Allowing callers to specify `VPATH` or `srcdir` (which breaks) * Using absolute path for SCRIPTS (9.5 prepends srcdir) * Including libpq-int in a confusing way (extracted this function) * Having server includes come first during csql build (client must) In particular, I hit all of these attempting to build with pg_buildext in Debian. It passes in an explicit VPATH, as well as srcdir (breaking all recursive make invocations), and also uses DESTDIR during install. In addition, a PGDG-enabled Debian box will have the latest libpq-dev headers (e.g. 9.5) even when building against an older server version (e.g. 9.4). This leads to problems when including e.g. `c.h`, which is ambiguous. While compiling more client-side code (csql), we need to ensure the newer libpq headers are included _first_, so I fixed that. --- Makefile | 6 ++-- Makefile.global.in | 3 +- src/backend/distributed/Makefile | 3 +- .../distributed/test/connection_cache.c | 5 +-- .../distributed/test/connection_utils.c | 31 +++++++++++++++++++ src/bin/csql/Makefile | 5 ++- .../distributed/test_helper_functions.h | 2 ++ 7 files changed, 46 insertions(+), 9 deletions(-) create mode 100644 src/backend/distributed/test/connection_utils.c diff --git a/Makefile b/Makefile index 93a3ce457..9d8fc8bcc 100644 --- a/Makefile +++ b/Makefile @@ -18,11 +18,11 @@ extension: install-extension: extension $(MAKE) -C src/backend/distributed/ install install-headers: extension - $(MKDIR_P) '$(includedir_server)/distributed/' + $(MKDIR_P) '$(DESTDIR)$(includedir_server)/distributed/' # generated headers are located in the build directory - $(INSTALL_DATA) src/include/citus_config.h '$(includedir_server)/' + $(INSTALL_DATA) src/include/citus_config.h '$(DESTDIR)$(includedir_server)/' # the rest in the source tree - $(INSTALL_DATA) $(citus_abs_srcdir)/src/include/distributed/*.h '$(includedir_server)/distributed/' + $(INSTALL_DATA) $(citus_abs_srcdir)/src/include/distributed/*.h '$(DESTDIR)$(includedir_server)/distributed/' clean-extension: $(MAKE) -C src/backend/distributed/ clean .PHONY: extension install-extension clean-extension diff --git a/Makefile.global.in b/Makefile.global.in index 9fc3459ab..09825f93c 100644 --- a/Makefile.global.in +++ b/Makefile.global.in @@ -17,9 +17,10 @@ PGXS:=$(shell $(PG_CONFIG) --pgxs) # Support for VPATH builds (i.e. builds from outside the source tree) vpath_build=@vpath_build@ ifeq ($(vpath_build),yes) - VPATH:=$(citus_abs_srcdir) + override VPATH:=$(citus_abs_srcdir) USE_VPATH:=$(VPATH) citus_top_srcdir:=$(citus_abs_top_srcdir) + override srcdir=$(VPATH) else citus_top_srcdir:=$(citus_top_builddir) endif diff --git a/src/backend/distributed/Makefile b/src/backend/distributed/Makefile index 052800bbe..039d1e8b4 100644 --- a/src/backend/distributed/Makefile +++ b/src/backend/distributed/Makefile @@ -7,8 +7,7 @@ MODULE_big = citus EXTENSION = citus EXTVERSION = 5.0 DATA_built = $(EXTENSION)--$(EXTVERSION).sql -# Install scripts in build and/or source directory, but only once if those are the same -SCRIPTS = $(sort $(wildcard $(citus_top_builddir)/src/bin/scripts/* $(citus_top_srcdir)/src/bin/scripts/*)) +SCRIPTS = ../../bin/scripts/copy_to_distributed_table # directories with source files SUBDIRS = . commands executor master planner relay test utils worker diff --git a/src/backend/distributed/test/connection_cache.c b/src/backend/distributed/test/connection_cache.c index 691020a55..1d325a7bf 100644 --- a/src/backend/distributed/test/connection_cache.c +++ b/src/backend/distributed/test/connection_cache.c @@ -13,7 +13,8 @@ #include "postgres.h" #include "c.h" #include "fmgr.h" -#include "libpq-int.h" + +#include "libpq-fe.h" #include #include @@ -143,7 +144,7 @@ set_connection_status_bad(PG_FUNCTION_ARGS) } /* set the connection status */ - connection->status = CONNECTION_BAD; + SetConnectionStatus(connection, CONNECTION_BAD); PG_RETURN_BOOL(true); } diff --git a/src/backend/distributed/test/connection_utils.c b/src/backend/distributed/test/connection_utils.c new file mode 100644 index 000000000..8ec243eb6 --- /dev/null +++ b/src/backend/distributed/test/connection_utils.c @@ -0,0 +1,31 @@ +/*------------------------------------------------------------------------- + * + * test/src/connection_utils.c + * + * This file isolates a test function which modifies private connection + * state, ensuring the correct ("internal") headers are included, rather + * than the version-specific server ones. Without this kludge, builds on + * certain platforms (those which install a single libpq version but can + * have multiple PostgreSQL server versions) will faile. + * + * Copyright (c) 2014-2015, Citus Data, Inc. + * + *------------------------------------------------------------------------- + */ + +#include "internal/c.h" +#include "libpq-fe.h" +#include "internal/libpq-int.h" + +#include "distributed/test_helper_functions.h" + +/* + * SetConnectionStatus simply uses the internal headers to access "private" + * fields of the connection struct in order to force a cache connection to a + * particular status. + */ +void +SetConnectionStatus(PGconn *connection, ConnStatusType status) +{ + connection->status = status; +} diff --git a/src/bin/csql/Makefile b/src/bin/csql/Makefile index 2c1ae79af..d444c535e 100644 --- a/src/bin/csql/Makefile +++ b/src/bin/csql/Makefile @@ -28,7 +28,10 @@ PG_LIBS = $(libpq) include $(citus_top_builddir)/Makefile.global -override CPPFLAGS += -I$(libpq_srcdir) -I$(top_srcdir)/src/bin/csql +# ensure client includes occur before server +client_includes := $(shell $(PG_CONFIG) --includedir)/internal + +override CPPFLAGS := -I$(client_includes) -I$(libpq_srcdir) -I$(citus_abs_top_srcdir)/src/bin/csql $(CPPFLAGS) # psqlscan is compiled as part of mainloop mainloop.o: psqlscan.c diff --git a/src/include/distributed/test_helper_functions.h b/src/include/distributed/test_helper_functions.h index 410239099..f60ca2d66 100644 --- a/src/include/distributed/test_helper_functions.h +++ b/src/include/distributed/test_helper_functions.h @@ -16,6 +16,7 @@ #include "postgres.h" #include "c.h" #include "fmgr.h" +#include "libpq-fe.h" #include "utils/array.h" @@ -29,6 +30,7 @@ /* function declarations for generic test functions */ extern ArrayType * DatumArrayToArrayType(Datum *datumArray, int datumCount, Oid datumTypeId); +extern void SetConnectionStatus(PGconn *connection, ConnStatusType status); /* fake FDW for use in tests */ extern Datum fake_fdw_handler(PG_FUNCTION_ARGS);