Re: 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:
http://www.postgresql.org/message-id/flat/86sk8845pl(dot)fsf(at)ds4(dot)des(dot)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

Noah Misch <noah(at)leadboat(dot)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

On Sun, Jun 08, 2014 at 06:04:46PM -0400, Tom Lane wrote:
> Noah Misch <noah(at)leadboat(dot)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