Re: [HACKERS] Latest ecpg patch broke MSVC build
In hindsight, all these ecpg changes should have been made between beta1 and beta2 when we have time to deal with the fallout, not right before beta1. Or considered new features and held back for 8.4. Not picking on Michael, but the resemblance to the /contrib discussion is striking. Ecpg is another part of core PostgreSQL that lives by slightly different rules. /Magnus ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] Latest ecpg patch broke MSVC build
On Tue, Oct 09, 2007 at 08:15:35AM +0200, Magnus Hagander wrote: In hindsight, all these ecpg changes should have been made between beta1 and beta2 when we have time to deal with the fallout, not right before beta1. This one I totally agree with. Or considered new features and held back for 8.4. Not picking on Michael, but the resemblance to the /contrib discussion is striking. Ecpg is another part of core PostgreSQL that lives by slightly different rules. But this one I don't. At least not the new features part. Had I considered the patch a new feature I wouldn't have committed it. To me it looked like a bug fix and I still see it as such. Yes, we could have documented the bug instead, but still I don't see how we could argue that getting multithreading to work on Windows is a feature when it's already working on all other platforms a and is also compilable, but not working in some/most cases, on Windows. Michael -- Michael Meskes Email: Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) ICQ: 179140304, AIM/Yahoo: michaelmeskes, Jabber: [EMAIL PROTECTED] Go SF 49ers! Go Rhein Fire! Use Debian GNU/Linux! Use PostgreSQL! ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] Latest ecpg patch broke MSVC build
On Tue, Oct 09, 2007 at 10:00:51AM +0200, Michael Meskes wrote: On Tue, Oct 09, 2007 at 08:15:35AM +0200, Magnus Hagander wrote: In hindsight, all these ecpg changes should have been made between beta1 and beta2 when we have time to deal with the fallout, not right before beta1. This one I totally agree with. Or considered new features and held back for 8.4. Not picking on Michael, but the resemblance to the /contrib discussion is striking. Ecpg is another part of core PostgreSQL that lives by slightly different rules. But this one I don't. At least not the new features part. Had I considered the patch a new feature I wouldn't have committed it. To me it looked like a bug fix and I still see it as such. Yes, we could have documented the bug instead, but still I don't see how we could argue that getting multithreading to work on Windows is a feature when it's already working on all other platforms a and is also compilable, but not working in some/most cases, on Windows. We'retalking abuot different patches I think ;-) Things like: http://archives.postgresql.org/pgsql-committers/2007-09/msg00465.php http://archives.postgresql.org/pgsql-committers/2007-09/msg00408.php aren't win32 fixes. They're making parts of ecpg thread-safe that weren't before. And they're the ones that *caused* the win32 specific patches to be needed. That said, I'm sure one could argue they were bug-fixes, but I'm fairliy certain they would *not* be accepted as bug fixes if it were backend code. //Magnus ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] Latest ecpg patch broke MSVC build
In hindsight, all these ecpg changes should have been made between beta1 and beta2 when we have time to deal with the fallout, not right before beta1. --- Tom Lane wrote: This morning's ecpg patch certainly seems to have been snake-bit. Although the Windows gcc buildfarm members seem happy, the MSVC ones are all failing with Linking... Creating library Release\libecpg\libecpg.lib and object Release\libecpg\libecpg.exp libecpg.exp : error LNK2001: unresolved external symbol [EMAIL PROTECTED] .\Release\libecpg\libecpg.dll : fatal error LNK1120: 1 unresolved externals I see that DllMain() got added to misc.c, so it's not obvious what's wrong here. Some adjustment needed in the MSVC build scripts maybe? regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://postgres.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] Latest ecpg patch broke MSVC build
Since this is an actual API library, perhaps a proper fix is to create a .def file listing the exports in it, the same way we do for libpq? And then we could (should!) also filter the exports the same ways as we do for libpq these days. (see the exports.txt file in libpq) I'll try to find time to look forther at this meanwhile, but if someone can confirm that donig an explicit export list is a good way to go, I can confirm that donig that fixes the build problem :-) //Magnus ---(end of broadcast)--- TIP 6: explain analyze is your friend According to: Module-Definition (.def) File EXPORT http://msdn2.microsoft.com/en-us/library/ms856515.aspx whitespace is required between the name and the ordinal in a .def-file, hence in the .def-file DllMain @12 should be used instead of [EMAIL PROTECTED] you're reading the problem wrong. The 12 is not the ordinal, it's a part of the decorated name. /Magnus ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] Latest ecpg patch broke MSVC build
Magnus Hagander schrieb:
Since this is an actual API library, perhaps a proper fix is to create a
.def file listing the exports in it, the same way we do for libpq? And then
we could (should!) also filter the exports the same ways as we do for libpq
these days.
(see the exports.txt file in libpq)
I'll try to find time to look forther at this meanwhile, but if someone can
confirm that donig an explicit export list is a good way to go, I can
confirm that donig that fixes the build problem :-)
//Magnus
---(end of broadcast)---
TIP 6: explain analyze is your friend
According to:
Module-Definition (.def) File EXPORT
http://msdn2.microsoft.com/en-us/library/ms856515.aspx
whitespace is required between the name and the ordinal in a
.def-file, hence in the .def-file DllMain @12 should be used
instead of [EMAIL PROTECTED]
you're reading the problem wrong. The 12 is not the ordinal, it's a part of
the decorated name.
/Magnus
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
but, we are compiling C so the names shouldn't be decorated.
undecorating yields the same name:
c:\hannes\undname [EMAIL PROTECTED]
Microsoft (R) C++ Name Undecorator
Copyright (C) Microsoft Corporation. All rights reserved.
Undecoration of :- [EMAIL PROTECTED]
is :- [EMAIL PROTECTED]
compiling a little test program:
cat dllmain.c EOF
#include windows.h
BOOL WINAPI
DllMain(HANDLE module, DWORD reason, LPVOID reserved)
{
return TRUE;
}
EOF
yields the same exported symbol
C:\Hannescl /c dllmain.c
Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 14.00.50727.762
for 80x86
Copyright (C) Microsoft Corporation. All rights reserved.
C:\Hannesdumpbin /symbols dllmain.obj
Microsoft (R) COFF/PE Dumper Version 8.00.50727.762
Copyright (C) Microsoft Corporation. All rights reserved.
Dump of file dllmain.obj
File Type: COFF OBJECT
COFF SYMBOL TABLE
[snip]
008 SECT3 notype ()External | [EMAIL PROTECTED]
[snip]
When creating a new dll with VC 6.0, same here
C:\Hannes\testdll\Debugdumpbin /symbols testdll.obj | grep Main
01B SECT6 notype ()External | [EMAIL PROTECTED]
as with libecpg
C:\Hannes\pgsql\Debug\libecpgdumpbin /symbols misc.obj | grep Main
05E 07B0 SECT5 notype ()External | [EMAIL PROTECTED]
Am I missing something?
-Hannes
---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match
Re: [HACKERS] Latest ecpg patch broke MSVC build
On Wed, Oct 03, 2007 at 09:35:51AM +0200, Hannes Eder wrote: Magnus Hagander schrieb: Since this is an actual API library, perhaps a proper fix is to create a .def file listing the exports in it, the same way we do for libpq? And then we could (should!) also filter the exports the same ways as we do for libpq these days. (see the exports.txt file in libpq) I'll try to find time to look forther at this meanwhile, but if someone can confirm that donig an explicit export list is a good way to go, I can confirm that donig that fixes the build problem :-) //Magnus ---(end of broadcast)--- TIP 6: explain analyze is your friend According to: Module-Definition (.def) File EXPORT http://msdn2.microsoft.com/en-us/library/ms856515.aspx whitespace is required between the name and the ordinal in a .def-file, hence in the .def-file DllMain @12 should be used instead of [EMAIL PROTECTED] you're reading the problem wrong. The 12 is not the ordinal, it's a part of the decorated name. /Magnus ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster but, we are compiling C so the names shouldn't be decorated. We're not talking C++ decoration, we're talking Windows API decoration. Take a look at for example: http://www.geocities.com/yongweiwu/stdcall.htm (there is a reference on MSDN as well, btu I can't find it right now) The @12 is 12 bytes in the argument list to the function. The reason is to make sure the caller calls it with the right number of arguments so as to prevent stack issues. //Magnus ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] Latest ecpg patch broke MSVC build
Note that unless there's some tools issue, DllMain doesn't need to be exported to function properly. A DLL's initialization routine is marked as the entry point in the PE header, same as main() in classic C. It might be simpler to just get rid of the export. ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] Latest ecpg patch broke MSVC build
Magnus Hagander schrieb: On Sun, Sep 30, 2007 at 11:30:35PM -0400, Andrew Dunstan wrote: Tom Lane wrote: This morning's ecpg patch certainly seems to have been snake-bit. Although the Windows gcc buildfarm members seem happy, the MSVC ones are all failing with Linking... Creating library Release\libecpg\libecpg.lib and object Release\libecpg\libecpg.exp libecpg.exp : error LNK2001: unresolved external symbol [EMAIL PROTECTED] .\Release\libecpg\libecpg.dll : fatal error LNK1120: 1 unresolved externals I see that DllMain() got added to misc.c, so it's not obvious what's wrong here. Some adjustment needed in the MSVC build scripts maybe? It is building with thread.c but it should not be unless I am misreading It's been building with thread.c before this patch. And the problem doesn't go away if you ermove thread.c. The problem seems to be that it tries to export a decorated DllMain ([EMAIL PROTECTED]) which is listed in the object file, but it's unable to export it. Will need to dig further. The reason it doesn't happen on mingw is likely the horrible kludge that is export-all-symbols-in-all-files that we've only partially been able to emulate. Since this is an actual API library, perhaps a proper fix is to create a .def file listing the exports in it, the same way we do for libpq? And then we could (should!) also filter the exports the same ways as we do for libpq these days. (see the exports.txt file in libpq) I'll try to find time to look forther at this meanwhile, but if someone can confirm that donig an explicit export list is a good way to go, I can confirm that donig that fixes the build problem :-) //Magnus ---(end of broadcast)--- TIP 6: explain analyze is your friend According to: Module-Definition (.def) File EXPORT http://msdn2.microsoft.com/en-us/library/ms856515.aspx whitespace is required between the name and the ordinal in a .def-file, hence in the .def-file DllMain @12 should be used instead of [EMAIL PROTECTED] Therefor I think the fix should be addressed in tools/msvc/gendef.pl, see attached diff. Anyway there is just a single occurence of an ordinal in the .def-files: $ grep '@' `find -name *.def` ./libecpg/LIBECPG.def: DllMain @12 The ordinal 12 seems to be the default for DllMain. -Hannes *** ../pgsql-cvshead/src/tools/msvc/gendef.pl Thu May 3 16:04:03 2007 --- src/tools/msvc/gendef.pl Wed Oct 3 00:53:23 2007 *** *** 38,43 --- 38,46 next if $pieces[6] =~ /^__NULL_IMPORT/; next if $pieces[6] =~ /^\?\?_C/; + # whitespace required between name and ordinal + $pieces[6] =~ s/@/ @/; + push @def, $pieces[6]; } close(F); ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] Latest ecpg patch broke MSVC build
On Sun, Sep 30, 2007 at 11:30:35PM -0400, Andrew Dunstan wrote: Tom Lane wrote: This morning's ecpg patch certainly seems to have been snake-bit. Although the Windows gcc buildfarm members seem happy, the MSVC ones are all failing with Linking... Creating library Release\libecpg\libecpg.lib and object Release\libecpg\libecpg.exp libecpg.exp : error LNK2001: unresolved external symbol [EMAIL PROTECTED] .\Release\libecpg\libecpg.dll : fatal error LNK1120: 1 unresolved externals I see that DllMain() got added to misc.c, so it's not obvious what's wrong here. Some adjustment needed in the MSVC build scripts maybe? It is building with thread.c but it should not be unless I am misreading It's been building with thread.c before this patch. And the problem doesn't go away if you ermove thread.c. The problem seems to be that it tries to export a decorated DllMain ([EMAIL PROTECTED]) which is listed in the object file, but it's unable to export it. Will need to dig further. The reason it doesn't happen on mingw is likely the horrible kludge that is export-all-symbols-in-all-files that we've only partially been able to emulate. Since this is an actual API library, perhaps a proper fix is to create a .def file listing the exports in it, the same way we do for libpq? And then we could (should!) also filter the exports the same ways as we do for libpq these days. (see the exports.txt file in libpq) I'll try to find time to look forther at this meanwhile, but if someone can confirm that donig an explicit export list is a good way to go, I can confirm that donig that fixes the build problem :-) //Magnus ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] Latest ecpg patch broke MSVC build
On Sun, Sep 30, 2007 at 11:46:00PM -0400, Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: It is building with thread.c but it should not be unless I am misreading the Makefile. The makefile processing in Project.pm doesn't look nearly powerful enough to handle this: # thread.c is needed only for non-WIN32 implementation of path.c ifneq ($(PORTNAME), win32) OBJS += thread.o endif Hmm, sounds like a problem, but why was it not a problem before? It's not realliy a problem since the stuff in thread.c is #ifdefed away on Windows in most cases anyway. All we do is import a small piece of code we'll never use.. //Magnus ---(end of broadcast)--- TIP 6: explain analyze is your friend
[HACKERS] Latest ecpg patch broke MSVC build
This morning's ecpg patch certainly seems to have been snake-bit. Although the Windows gcc buildfarm members seem happy, the MSVC ones are all failing with Linking... Creating library Release\libecpg\libecpg.lib and object Release\libecpg\libecpg.exp libecpg.exp : error LNK2001: unresolved external symbol [EMAIL PROTECTED] .\Release\libecpg\libecpg.dll : fatal error LNK1120: 1 unresolved externals I see that DllMain() got added to misc.c, so it's not obvious what's wrong here. Some adjustment needed in the MSVC build scripts maybe? regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] Latest ecpg patch broke MSVC build
Tom Lane wrote: This morning's ecpg patch certainly seems to have been snake-bit. Although the Windows gcc buildfarm members seem happy, the MSVC ones are all failing with Linking... Creating library Release\libecpg\libecpg.lib and object Release\libecpg\libecpg.exp libecpg.exp : error LNK2001: unresolved external symbol [EMAIL PROTECTED] .\Release\libecpg\libecpg.dll : fatal error LNK1120: 1 unresolved externals I see that DllMain() got added to misc.c, so it's not obvious what's wrong here. Some adjustment needed in the MSVC build scripts maybe? It is building with thread.c but it should not be unless I am misreading the Makefile. The makefile processing in Project.pm doesn't look nearly powerful enough to handle this: # thread.c is needed only for non-WIN32 implementation of path.c ifneq ($(PORTNAME), win32) OBJS += thread.o endif It will ignore the if and endif lines and process the OBJS line :-( A quick fix is probably to put some whitespace in front of OBJS, although that seems horribly fragile. cheers andrew ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] Latest ecpg patch broke MSVC build
Andrew Dunstan [EMAIL PROTECTED] writes: It is building with thread.c but it should not be unless I am misreading the Makefile. The makefile processing in Project.pm doesn't look nearly powerful enough to handle this: # thread.c is needed only for non-WIN32 implementation of path.c ifneq ($(PORTNAME), win32) OBJS += thread.o endif Hmm, sounds like a problem, but why was it not a problem before? regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] Latest ecpg patch broke MSVC build
Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: It is building with thread.c but it should not be unless I am misreading the Makefile. The makefile processing in Project.pm doesn't look nearly powerful enough to handle this: # thread.c is needed only for non-WIN32 implementation of path.c ifneq ($(PORTNAME), win32) OBJS += thread.o endif Hmm, sounds like a problem, but why was it not a problem before? Good point. I don't know. I guess something else must be causing the build failure. cheers andrew ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
