Closed Bug 237870 Opened 20 years ago Closed 20 years ago
GW builds against wrong sockets DLL
MSVC builds link against wsock32.lib but some makefiles specify -lws2_32 as the MinGW equivalent. This prevents MinGW from building on some platforms.
Assignee: nobody → neil.parkwaycc.co.uk
Status: NEW → ASSIGNED
Comment on attachment 144218 [details] [diff] [review] Proposed patch As this touches protected partitions I'd appreciate it if you checked in any bits to which you agree.
On what platforms does this prevent mingw from building on? ws2_32 is the winsock2 library; it should be on all boxes. Besides, it's required to get full getsockopt functionality. See http://bugzilla.mozilla.org/show_bug.cgi?id=134113#c108 .
I'm sorry, but I don't understand why mingw builds link against ws2_32.dll but msvc builds link against wsock32.dll - the platform I'm trying to build on doesn't have ws2_32.dll (so far I've had no issues relating to wsock32.dll).
The reerenced fcomment in the original mingw bug explains why we need to link against ws2_32.dll for mingw builds. I honestly have no idea why msvc doesn't need to link against the winsock2 dll or even if msvc builds will pass the nspr tests. Passing the nspr tests was a requirement for landing mingw support so here we are. Have you tested to see if the nspr tests will pass with your changes?
Neil: is your platform Windows 95? I know old Windows 95 releases don't have ws2_32.dll. It is possible that we are not defining the right macros when we are compiling with MinGW so that when we link with wsock32.dll it isn't able to use certain socket options that are only implemented in Winsock 2. Unfortunately I am not set up to debug a MinGW build. I hope someone can figure this out. Yeah, I've also been wondering why the MinGW build must link with ws2_32.dll in order to pass all of the NSPR tests. (By the way, the MSVC build passes all of the NSPR tests.)
Assuming "make runtests" in nsprpub/pr/tests is the way to go, the tests don't actually pass for me with the (msvc) mozilla 1.5 release nspr... on the other hand, with the patch applied, mozilla starts up my home page fine...
IIRC, to properly run the tests, you need to: cd pr/tests && sh runtests.ksh
Yes, "make runtests" is obsolete. You need to say "sh runtests.ksh". We have been using the MKS Korn Shell to run runtests.sh. I don't know if that shell script works under Cygwin's bash.
MinGW doesn't define IP_TOS or IP_TTL in winsock.h which is a bug in MinGW as far as incompatibility with msvc is concerned; rather than forcing MinGW to use ws2tcpip.h would something along these lines would be acceptable? [to replace the existing #ifdef WIN32 block in prmapopt.c] #ifdef WIN32 #include <winsock.h> /* MinGW doesn't define these in its winsock.h */ #ifndef IP_TTL #define IP_TTL 7 #endif #ifndef IP_TOS #define IP_TOS 8 #endif #endif
That sounds like a good idea. Please try this: 1. In prmapopt.c, restore the #ifdef WIN32 block to what it was before: #ifdef WINNT #include <winsock.h> #endif 2. In nsprpub/pr/include/md/_win95.h, add the following after the winsock.h inclusion: /* MinGW doesn't define these in its winsock.h */ #ifdef __MINGW32__ #ifndef IP_TTL #define IP_TTL 7 #endif #ifndef IP_TOS #define IP_TOS 8 #endif #endif
(In reply to comment #11) >That sounds like a good idea. Please try this: >1. In prmapopt.c, restore the #ifdef WIN32 block to what it was before: >#ifdef WINNT >#include <winsock.h> >#endif Did you mean WIN32 ? The sockopt test failed with this changed to WINNT.
You are right. I also tried the changes I suggested and found that they don't work. Please try this patch instead.
Attachment #144465 - Attachment is obsolete: true
Comment on attachment 144466 [details] [diff] [review] Updated patch to NSPR v2 I forgot to mention that this patch is essentially what Neil suggested in comment 10. We probably could move the definitions of IP_TTL and IP_TOS before the "primpl.h" inclusion.
(In reply to comment #14) >(From update of attachment 144466 [details] [diff] [review]) This patch PASSED too :-) >We probably could move the definitions of IP_TTL >and IP_TOS before the "primpl.h" inclusion. If you do that, you could possibly separate the #ifdef WINNT and #ifdef __MINGW__ sections each having their own #include <winsock.h> lines.
Neil, thanks for testing NSPR patch v2. In this patch I made the change I suggested: define IP_TTL and IP_TOS before including primpl.h. (Your suggestion would also work. It's mainly a matter of personal taste.) Could you tell me which platforms don't have ws2_32.dll? Current Microsoft documentation tells people to include winsock2.h and link with ws2_32.dll. So it is likely that we will switch to Winsock 2 some time soon.
Attachment #144466 - Attachment is obsolete: true
Comment on attachment 144491 [details] [diff] [review] Updated patch to NSPR v2.1 My mingw build passes the test suite with these changes. r=cls
Attachment #144491 - Flags: review?(cls) → review+
Comment on attachment 144218 [details] [diff] [review] Proposed patch Do you have any issues with the non-nspr changes in this patch?
Comment on attachment 144218 [details] [diff] [review] Proposed patch r=wtc. The non-NSPR changes in this patch are fine.
Comment on attachment 144491 [details] [diff] [review] Updated patch to NSPR v2.1 I've checked in this NSPR patch on the NSPR tip (NSPR 4.5) and NSPRPUB_PRE_4_2_CLIENT_BRANCH (Mozilla 1.7 final). Neil, you just need to check in the non-NSPR changes.
One of those non-NSPR changes was to security/nss/lib/fortcrypt/swfort/pkcs11/Makefile .
Comment on attachment 144218 [details] [diff] [review] Proposed patch The security/nss change in this patch has been checked into the NSS tip (NSS 3.10), NSS_3_9_BRANCH (NSS 3.9.1), and NSS_CLIENT_TAG (Mozilla 1.7 final).
Attachment #144218 - Flags: approval1.7+ → approval1.7?
Checking in Makefile.in; /cvsroot/mozilla/netwerk/build/Makefile.in,v <-- Makefile.in new revision: 1.62; previous revision: 1.61 done Thanks everybody.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.