MinGW/Cygwin build snags
First, when I tried to add an Assert() call to a file in src/port, a MinGW-w64
build failed like this:
Creating library file: libpostgres.a
../../src/port/libpgport_srv.a(fls_srv.o): In function `fls':
/home/nm/src/pg/mingw-postgresql/src/port/fls.c:63: undefined reference to `__imp_assert_enabled'
Since src/makefiles/Makefile.win32 does not set BUILDING_DLL for src/port,
PGDLLIMPORT is set improperly for code to be linked directly into the backend.
Makefile.win32 does set BUILDING_DLL for src/common. (Makefile.cygwin has the
same discrepancy, though I haven't checked whether it causes an actual build
failure there. The MSVC build system sets BUILDING_DLL for both src/port and
src/common files.) This affects any reference to a data symbol from src/port.
The fix is straightforward enough: cause Makefile.{win32,cygwin} to treat
src/port like src/common.
Second, src/template/{win32,cygwin} completely replaces LDFLAGS, so overriding
LDFLAGS on the "configure" command line is ineffective. Those scripts should
instead add to the existing LDFLAGS, like other templates do for CPPFLAGS.
Several other templates completely override CFLAGS; that's undesirable for the
same reason. I don't have ready access to those affected configurations, so
I'm leaving the CFLAGS damage alone. A fix for the Makefile.win32 half of the
original problem appeared as part of a larger patch:
/messages/by-id/86sk8845pl.fsf@ds4.des.no
Both of these changes fix bugs, but I plan not to back-patch. Builds that
work today won't see any change, and I found no other user reports.
--
Noah Misch
EnterpriseDB http://www.enterprisedb.com
Attachments:
pgport-dll-v1.patchtext/plain; charset=us-asciiDownload
diff --git a/src/makefiles/Makefile.cygwin b/src/makefiles/Makefile.cygwin
index bd83e5f..bb2efed 100644
--- a/src/makefiles/Makefile.cygwin
+++ b/src/makefiles/Makefile.cygwin
@@ -28,6 +28,10 @@ ifneq (,$(findstring src/common,$(subdir)))
override CPPFLAGS+= -DBUILDING_DLL
endif
+ifneq (,$(findstring src/port,$(subdir)))
+override CPPFLAGS+= -DBUILDING_DLL
+endif
+
ifneq (,$(findstring timezone,$(subdir)))
override CPPFLAGS+= -DBUILDING_DLL
endif
diff --git a/src/makefiles/Makefile.win32 b/src/makefiles/Makefile.win32
index b18621b..9cb84f2 100644
--- a/src/makefiles/Makefile.win32
+++ b/src/makefiles/Makefile.win32
@@ -27,6 +27,10 @@ ifneq (,$(findstring src/common,$(subdir)))
override CPPFLAGS+= -DBUILDING_DLL
endif
+ifneq (,$(findstring src/port,$(subdir)))
+override CPPFLAGS+= -DBUILDING_DLL
+endif
+
ifneq (,$(findstring timezone,$(subdir)))
override CPPFLAGS+= -DBUILDING_DLL
endif
template-ldflags-preserve-v2.patchtext/plain; charset=us-asciiDownload
diff --git a/src/template/cygwin b/src/template/cygwin
index e484fe6..b6ea0de 100644
--- a/src/template/cygwin
+++ b/src/template/cygwin
@@ -6,4 +6,4 @@ SRCH_LIB="/usr/local/lib"
# pg_toupper() etc. in both libpq and pgport
# we'd prefer to use --disable-auto-import to match MSVC linking behavior,
# but support for it in Cygwin is too haphazard
-LDFLAGS="-Wl,--allow-multiple-definition -Wl,--enable-auto-import"
+LDFLAGS="$LDFLAGS -Wl,--allow-multiple-definition -Wl,--enable-auto-import"
diff --git a/src/template/win32 b/src/template/win32
index dc5b77e..7da9719 100644
--- a/src/template/win32
+++ b/src/template/win32
@@ -3,4 +3,4 @@
# --allow-multiple-definition is required to link pg_dump because it finds
# pg_toupper() etc. in both libpq and pgport
# --disable-auto-import is to ensure we get MSVC-like linking behavior
-LDFLAGS="-Wl,--allow-multiple-definition -Wl,--disable-auto-import"
+LDFLAGS="$LDFLAGS -Wl,--allow-multiple-definition -Wl,--disable-auto-import"
Noah Misch <noah@leadboat.com> writes:
First, when I tried to add an Assert() call to a file in src/port, a MinGW-w64
build failed like this:
Creating library file: libpostgres.a
../../src/port/libpgport_srv.a(fls_srv.o): In function `fls':
/home/nm/src/pg/mingw-postgresql/src/port/fls.c:63: undefined reference to `__imp_assert_enabled'
FWIW, I think we had consensus to remove the assert_enabled variable
entirely. Not that it wouldn't be good to fix this, but Assert per se
won't need it after we do that.
Since src/makefiles/Makefile.win32 does not set BUILDING_DLL for src/port,
PGDLLIMPORT is set improperly for code to be linked directly into the backend.
Makefile.win32 does set BUILDING_DLL for src/common. (Makefile.cygwin has the
same discrepancy, though I haven't checked whether it causes an actual build
failure there. The MSVC build system sets BUILDING_DLL for both src/port and
src/common files.) This affects any reference to a data symbol from src/port.
The fix is straightforward enough: cause Makefile.{win32,cygwin} to treat
src/port like src/common.
I wonder whether these cases shouldn't distinguish between the "frontend"
and "backend" builds of src/port/ and src/common/. In particular, it
seems odd that we're getting this type of failure in the backend build.
Second, src/template/{win32,cygwin} completely replaces LDFLAGS, so overriding
LDFLAGS on the "configure" command line is ineffective. Those scripts should
instead add to the existing LDFLAGS, like other templates do for CPPFLAGS.
Several other templates completely override CFLAGS; that's undesirable for the
same reason. I don't have ready access to those affected configurations, so
I'm leaving the CFLAGS damage alone.
+1 for doing something about CFLAGS while we're at it.
Both of these changes fix bugs, but I plan not to back-patch.
Agreed; the lack of complaints to date suggests that we should leave
well enough alone in the back branches.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sun, Jun 08, 2014 at 06:04:46PM -0400, Tom Lane wrote:
Noah Misch <noah@leadboat.com> writes:
Since src/makefiles/Makefile.win32 does not set BUILDING_DLL for src/port,
PGDLLIMPORT is set improperly for code to be linked directly into the backend.
Makefile.win32 does set BUILDING_DLL for src/common. (Makefile.cygwin has the
same discrepancy, though I haven't checked whether it causes an actual build
failure there. The MSVC build system sets BUILDING_DLL for both src/port and
src/common files.) This affects any reference to a data symbol from src/port.
The fix is straightforward enough: cause Makefile.{win32,cygwin} to treat
src/port like src/common.I wonder whether these cases shouldn't distinguish between the "frontend"
and "backend" builds of src/port/ and src/common/. In particular, it
seems odd that we're getting this type of failure in the backend build.
Good question; they need not distinguish. !BUILDING_DLL means that the code
being compiled will access backend symbols through dynamic linking to
postgres.exe. Server modules, such as plpgsql, build with !BUILDING_DLL, and
normal backend code builds with BUILDING_DLL. The setting is irrelevant for
"frontend"/non-server code, which does not link to backend symbols at all.
Second, src/template/{win32,cygwin} completely replaces LDFLAGS, so overriding
LDFLAGS on the "configure" command line is ineffective. Those scripts should
instead add to the existing LDFLAGS, like other templates do for CPPFLAGS.
Several other templates completely override CFLAGS; that's undesirable for the
same reason. I don't have ready access to those affected configurations, so
I'm leaving the CFLAGS damage alone.+1 for doing something about CFLAGS while we're at it.
Scratch that; configure.in has logic to discard template script CFLAGS changes
if the user had specified CFLAGS.
--
Noah Misch
EnterpriseDB http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers