All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: "Mark Levedahl" <mlevedahl@gmail.com>,
	"Alex Riesen" <raa.lkml@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Jason Pyeron" <jpyeron@pdinc.us>,
	git@vger.kernel.org, "Torsten Bögershausen" <tboegi@web.de>,
	"Stephen & Linda Smith" <ischis2@cox.net>,
	"Eric Blake" <eblake@redhat.com>
Subject: Re: Version 1.8.1 does not compile on Cygwin 1.7.14
Date: Tue, 22 Jan 2013 18:31:36 +0000	[thread overview]
Message-ID: <50FEDB08.6030901@ramsay1.demon.co.uk> (raw)
In-Reply-To: <20130120101007.GD16339@elie.Belkin>

Jonathan Nieder wrote:
> Ramsay Jones wrote:
> 
>> --- a/git-compat-util.h
>> +++ b/git-compat-util.h
>> @@ -85,12 +85,6 @@
>>  #define _NETBSD_SOURCE 1
>>  #define _SGI_SOURCE 1
>>  
>> -#ifdef WIN32 /* Both MinGW and MSVC */
>> -#define WIN32_LEAN_AND_MEAN  /* stops windows.h including winsock.h */
>> -#include <winsock2.h>
>> -#include <windows.h>
>> -#endif
> 
> So, do I understand correctly that the above conditional should be
> something like
> 
>  #if defined(WIN32) && !defined(__CYGWIN__)
> 
> to allow dropping the CYGWIN_V15_WIN32API setting?

Yes, replacing the git-compat-util.h hunk above with:

    diff --git a/git-compat-util.h b/git-compat-util.h
    index e5a4b74..a38ae8d 100644
    --- a/git-compat-util.h
    +++ b/git-compat-util.h
    @@ -85,7 +85,7 @@
     #define _NETBSD_SOURCE 1
     #define _SGI_SOURCE 1
 
    -#ifdef WIN32 /* Both MinGW and MSVC */
    +#if defined(WIN32) && !defined(__CYGWIN__) /* Both MinGW and MSVC */
     #define WIN32_LEAN_AND_MEAN  /* stops windows.h including winsock.h */
     #include <winsock2.h>
     #include <windows.h>

will also compile on cygwin 1.5.x

> "defined(WIN32)" is used throughout git to mean "win32 and not
> cygwin", so if I understand correctly we would either need to do

Hmm ... I remember being *very* nervous of commit 435bdf8c ("Make
usage of windows.h lean and mean", 16-09-2009) exactly because it
makes the code (on cygwin) much more fragile with respect to header
include order. ;-)

As I have mentioned here before, the claim that "WIN32 is not defined
on cygwin" is simply nonsense - it depends on if/when certain header
files are included. For example, *as soon as* you include <windows.h>
(and, I suspect, many other win32 headers) then "defined(WIN32)"
is true.

Note that commit 380a4d92 ("Update cygwin.c for new mingw-64 win32 api
headers", 11-11-2012) swaps the include order for the win32.h and
git-compat-util.h header files. [I don't know the details, Mark didn't
elaborate, but it is clearly an include order problem on cygwin 1.7.x :-D ]
This causes compilation errors on cygwin 1.5.x, exactly because win32.h
includes <windows.h>, which defines WIN32, which then leads to
git-compat-util.h including <winsock2.h>.

>  #if defined(WIN32) && defined(__CYGWIN__)
>  # undef WIN32
>  #endif

Hmm, except when you want it defined on cygwin, of course ... ;-)

> Thanks for investigating.

No problem.

I've included the updated patch below, just for completeness.

HTH

ATB,
Ramsay Jones

-- >8 --
Subject: [PATCH] cygwin: Remove the CYGWIN_V15_WIN32API config


Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
 Makefile          | 7 -------
 compat/cygwin.c   | 5 -----
 config.mak.uname  | 1 -
 git-compat-util.h | 2 +-
 4 files changed, 1 insertion(+), 14 deletions(-)

diff --git a/Makefile b/Makefile
index 1b30d7b..1c84f68 100644
--- a/Makefile
+++ b/Makefile
@@ -281,10 +281,6 @@ all::
 #
 # Define NO_REGEX if you have no or inferior regex support in your C library.
 #
-# Define CYGWIN_V15_WIN32API if you are using Cygwin v1.7.x but are not
-# using the current w32api packages. The recommended approach, however,
-# is to update your installation if compilation errors occur.
-#
 # Define HAVE_DEV_TTY if your system can open /dev/tty to interact with the
 # user.
 #
@@ -1402,9 +1398,6 @@ ifdef NO_REGEX
 	COMPAT_CFLAGS += -Icompat/regex
 	COMPAT_OBJS += compat/regex/regex.o
 endif
-ifdef CYGWIN_V15_WIN32API
-	COMPAT_CFLAGS += -DCYGWIN_V15_WIN32API
-endif
 
 ifdef USE_NED_ALLOCATOR
        COMPAT_CFLAGS += -Icompat/nedmalloc
diff --git a/compat/cygwin.c b/compat/cygwin.c
index 5428858..0a9aa6d 100644
--- a/compat/cygwin.c
+++ b/compat/cygwin.c
@@ -1,13 +1,8 @@
 #define WIN32_LEAN_AND_MEAN
-#ifdef CYGWIN_V15_WIN32API
-#include "../git-compat-util.h"
-#include "win32.h"
-#else
 #include <sys/stat.h>
 #include <sys/errno.h>
 #include "win32.h"
 #include "../git-compat-util.h"
-#endif
 #include "../cache.h" /* to read configuration */
 
 static inline void filetime_to_timespec(const FILETIME *ft, struct timespec *ts)
diff --git a/config.mak.uname b/config.mak.uname
index bea34f0..5e493c9 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -158,7 +158,6 @@ ifeq ($(uname_O),Cygwin)
 		NO_SYMLINK_HEAD = YesPlease
 		NO_IPV6 = YesPlease
 		OLD_ICONV = UnfortunatelyYes
-		CYGWIN_V15_WIN32API = YesPlease
 	endif
 	NO_THREAD_SAFE_PREAD = YesPlease
 	NEEDS_LIBICONV = YesPlease
diff --git a/git-compat-util.h b/git-compat-util.h
index e5a4b74..a38ae8d 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -85,7 +85,7 @@
 #define _NETBSD_SOURCE 1
 #define _SGI_SOURCE 1
 
-#ifdef WIN32 /* Both MinGW and MSVC */
+#if defined(WIN32) && !defined(__CYGWIN__) /* Both MinGW and MSVC */
 #define WIN32_LEAN_AND_MEAN  /* stops windows.h including winsock.h */
 #include <winsock2.h>
 #include <windows.h>
-- 
1.8.1

  parent reply	other threads:[~2013-01-22 18:41 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-06  2:04 Version 1.8.1 does not compile on Cygwin 1.7.14 Stephen & Linda Smith
2013-01-06  3:37 ` Jason Pyeron
2013-01-06  4:22   ` Jason Pyeron
2013-01-06  6:20 ` Stephen & Linda Smith
2013-01-06  6:29   ` Jason Pyeron
2013-01-06  7:23     ` Torsten Bögershausen
2013-01-06  9:32       ` Jonathan Nieder
2013-01-06  9:42         ` Torsten Bögershausen
2013-01-06  9:57           ` Jonathan Nieder
2013-01-06 11:48             ` Mark Levedahl
2013-01-06 12:09               ` Jonathan Nieder
2013-01-06 14:09                 ` Stephen & Linda Smith
2013-01-06 19:54                 ` Junio C Hamano
2013-01-06 20:51                   ` Torsten Bögershausen
2013-01-06 21:34                     ` Mark Levedahl
2013-01-06 21:09                   ` Mark Levedahl
2013-01-06 21:33                     ` Jason Pyeron
2013-01-06 21:35                     ` Junio C Hamano
2013-01-06 21:46                       ` Jason Pyeron
2013-01-06 22:00                       ` Mark Levedahl
2013-01-06 22:16                   ` Mark Levedahl
2013-01-07  5:37                     ` Jason Pyeron
2013-01-07  7:29                       ` Junio C Hamano
2013-01-07  9:10                         ` Pyeron, Jason J CTR (US)
2013-01-08  3:12                         ` Mark Levedahl
2013-01-11 20:08                           ` Alex Riesen
2013-01-11 20:17                             ` Alex Riesen
2013-01-13 18:58                               ` Mark Levedahl
2013-01-15 18:47                                 ` Ramsay Jones
2013-01-20 10:10                                   ` Jonathan Nieder
2013-01-20 10:48                                     ` Torsten Bögershausen
2013-01-20 11:06                                       ` Jonathan Nieder
2013-01-21  5:20                                         ` [msysGit] " Torsten Bögershausen
2013-01-22 18:38                                           ` Ramsay Jones
2013-01-22 18:31                                     ` Ramsay Jones [this message]
2013-01-25 23:58                                       ` Mark Levedahl
2013-01-26  0:11                                         ` Junio C Hamano
2013-01-26  0:34                                           ` Eric Blake
2013-01-26  1:03                                             ` [PATCH/RFC] mingw: rename WIN32 cpp macro to NATIVE_WINDOWS Jonathan Nieder
2013-01-26 14:11                                               ` Mark Levedahl
2013-01-26 17:21                                               ` Torsten Bögershausen
2013-01-28 18:29                                               ` Ramsay Jones
2013-02-25  6:44                                                 ` Junio C Hamano
2013-02-26  4:08                                                   ` Mark Levedahl
2013-02-26 16:40                                                     ` Torsten Bögershausen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50FEDB08.6030901@ramsay1.demon.co.uk \
    --to=ramsay@ramsay1.demon.co.uk \
    --cc=eblake@redhat.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=ischis2@cox.net \
    --cc=jpyeron@pdinc.us \
    --cc=jrnieder@gmail.com \
    --cc=mlevedahl@gmail.com \
    --cc=raa.lkml@gmail.com \
    --cc=tboegi@web.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.