{"thread":{"id":"33652","subject":"[PATCH 2/2] cygwin: Remove the CYGWIN_V15_WIN32API build variable","startedAt":"2013-04-27T19:41:44Z","lastAt":"2013-04-30T17:05:35Z","messageCount":3,"participants":["Ramsay Jones","Jonathan Nieder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"215736","messageId":"517C29F8.7090709@ramsay1.demon.co.uk","threadId":"33652","inReplyTo":null,"subject":"[PATCH 2/2] cygwin: Remove the CYGWIN_V15_WIN32API build variable","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2013-04-27T19:41:44Z","receivedAt":"2013-04-27T19:41:44Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nSigned-off-by: Ramsay Jones \n---\n Makefile | 7 -------\n compat/cygwin.c | 5 -----\n config.mak.uname | 1 -\n 3 files changed, 13 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 0f931a2..32c49ba 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -290,10 +290,6 @@ all::\n #\n # Define NO_REGEX if you have no or inferior regex support in your C library.\n #\n-# Define CYGWIN_V15_WIN32API if you are using Cygwin v1.7.x but are not\n-# using the current w32api packages. The recommended approach, however,\n-# is to update your installation if compilation errors occur.\n-#\n # Define HAVE_DEV_TTY if your system can open /dev/tty to interact with the\n # user.\n #\n@@ -1449,9 +1445,6 @@ ifdef NO_REGEX\n \tCOMPAT_CFLAGS += -Icompat/regex\n \tCOMPAT_OBJS += compat/regex/regex.o\n endif\n-ifdef CYGWIN_V15_WIN32API\n-\tCOMPAT_CFLAGS += -DCYGWIN_V15_WIN32API\n-endif\n \n ifdef USE_NED_ALLOCATOR\n COMPAT_CFLAGS += -Icompat/nedmalloc\ndiff --git a/compat/cygwin.c b/compat/cygwin.c\nindex 871b41d..91ce5d4 100644\n--- a/compat/cygwin.c\n+++ b/compat/cygwin.c\n@@ -1,14 +1,9 @@\n #define CYGWIN_C\n #define WIN32_LEAN_AND_MEAN\n-#ifdef CYGWIN_V15_WIN32API\n-#include \"../git-compat-util.h\"\n-#include \"win32.h\"\n-#else\n #include \n #include \n #include \"win32.h\"\n #include \"../git-compat-util.h\"\n-#endif\n #include \"../cache.h\" /* to read configuration */\n \n /*\ndiff --git a/config.mak.uname b/config.mak.uname\nindex d78fd3d..28aa239 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -158,7 +158,6 @@ ifeq ($(uname_O),Cygwin)\n \t\tNO_SYMLINK_HEAD = YesPlease\n \t\tNO_IPV6 = YesPlease\n \t\tOLD_ICONV = UnfortunatelyYes\n-\t\tCYGWIN_V15_WIN32API = YesPlease\n \tendif\n \tNO_THREAD_SAFE_PREAD = YesPlease\n \tNEEDS_LIBICONV = YesPlease\n-- \n1.8.2\n"},{"id":"215833","messageId":"20130429060123.GD8031@elie.Belkin","threadId":"33652","inReplyTo":"517C29F8.7090709@ramsay1.demon.co.uk","subject":"Re: [PATCH 2/2] cygwin: Remove the CYGWIN_V15_WIN32API build variable","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-04-29T06:01:24Z","receivedAt":"2013-04-29T06:01:24Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nRamsay Jones wrote:\n \n> Signed-off-by: Ramsay Jones \n\nLet me try to understand this.\n\nBefore v1.8.1.1~7^2~2 (Update cygwin.c for new mingw-64 win32 api\nheaders, 2012-11-11), compat/cygwin.c did\n\n\t#define CYGWIN_C\n\t#define WIN32_LEAN_AND_MEAN\n\t#include \"../git-compat-util.h\"\n\t#include \"win32.h\"\n\nAfterward, on modern Cygwin it changed to\n\n\t#define CYGWIN_C\n\t#define WIN32_LEAN_AND_MEAN\n\t#include \n\t#include \n\t#include \"win32.h\"\n\t#include \"../git-compat-util.h\"\n\ncompat/win32.h does\n\n\t#ifndef WIN32 /* Not defined by Cygwin */\n\t#include \n\t#endif\n\nand then defines some inline functions relying on the win32 api.\n\ngit-compat-util.h defines various feature request macros and includes\nmany system headers, so that simple swap of lines was a huge change.\nIt's not obvious to me what part was important for making this work\nwith modern Cygwin win32api. Mark or others, do you remember what\nwent wrong with the original \"git-compat-util.h and then\ncompat/win32.h\" order (e.g., did it break the build? what was the\ncompiler message?) when upgrading win32api?\n\nUnfortunately this was a breaking change: systems with the *old*\nversion of win32api would only compile with the old order of header\ninclusions. The new ordering produced the following symptom:\n\n\tIn file included from compat/../git-compat-util.h:90,\n\t\t\t from compat/cygwin.c:9:\n\t/usr/lib/gcc/i686-pc-cygwin/3.4.4/../../../../include/w32api/winsock2.h:103:2: \n\twarning: #warning \"fd_set and associated macros have been defined in sys/types. \n\tThis may cause runtime problems with W32 sockets\"\n\tIn file included from /usr/include/sys/socket.h:16,\n\t\t\t from compat/../git-compat-util.h:131,\n\t\t\t from compat/cygwin.c:9:\n\t/usr/include/cygwin/socket.h:29: error: redefinition of `struct sockaddr'\n [...]\n\nAlex Riesen (cc-ed) noticed that removing the following lines from\ngit-compat-util.h alleviated this new symptom:\n\n\t#ifdef WIN32 /* Both MinGW and MSVC */\n\t# if defined (_MSC_VER)\n\t# define _WIN32_WINNT 0x0502\n\t# endif\n\t#define WIN32_LEAN_AND_MEAN /* stops windows.h including winsock.h */\n\t#include \n\t#include \n\t#endif\n\nAfter all, on Cygwin there is no reason to include winsock or the\nwin32api from git-compat-util.h. Presumably one of ,\n, and causes the WIN32 macro to be defined on\nthese systems with old win32api, and chaos ensues.\n\nAll in all, it wasn't a bad thing, since it revealed that the WIN32\nmacro doesn't mean what most of the git codebase assumes it means\n(hence patch 1/2, which fixes that).\n\nThe reordering made in v1.8.1.1~7^2~2 still seems like voodoo to me,\nbut at least it works. This patch applies that same order for\neveryone. Systems that would previously use the \"I have old win32api\nand don't need that reordering\" codepath don't need to be\nspecial-cased any more, since *their* particular brand of trouble is\navoided by being careful about how to use the WIN32 macro.\n\nThe upshot:\n\n - No change on modern setups. To uninformed people like me I feel\n like there is still something subtle going on that is not well\n understood, but hey, this patch doesn't break it. :)\n\n - Tested to still work on setups that previously needed\n CYGWIN_V15_WIN32API. Yay!\n\n - This drops an #ifdef, which means less code that is never tested\n to keep up to date.\n\nWith or without a few words of explanation in the commit message to\nsave some time for the next confused person looking this over,\n\nReviewed-by: Jonathan Nieder \n"},{"id":"216004","messageId":"7vtxmodk4g.fsf@alter.siamese.dyndns.org","threadId":"33652","inReplyTo":"20130429060123.GD8031@elie.Belkin","subject":"Re: [PATCH 2/2] cygwin: Remove the CYGWIN_V15_WIN32API build variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-30T17:05:35Z","receivedAt":"2013-04-30T17:05:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> The reordering made in v1.8.1.1~7^2~2 still seems like voodoo to me,\n> but at least it works. This patch applies that same order for\n> everyone. Systems that would previously use the \"I have old win32api\n> and don't need that reordering\" codepath don't need to be\n> special-cased any more, since *their* particular brand of trouble is\n> avoided by being careful about how to use the WIN32 macro.\n>\n> The upshot:\n>\n> - No change on modern setups. To uninformed people like me I feel\n> like there is still something subtle going on that is not well\n> understood, but hey, this patch doesn't break it. :)\n>\n> - Tested to still work on setups that previously needed\n> CYGWIN_V15_WIN32API. Yay!\n>\n> - This drops an #ifdef, which means less code that is never tested\n> to keep up to date.\n>\n> With or without a few words of explanation in the commit message to\n> save some time for the next confused person looking this over,\n>\n> Reviewed-by: Jonathan Nieder \n\nThanks. Ramsay, I tend to agree with Jonathan that this change\ndeserves a bit more explanation.\n"}]}