Re: [PATCH 09/14] Avoid including windows.h in winansi.c for MSVC build

3 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 09/14] Avoid including windows.h in winansi.c for MSVC build

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:47:17

Marius Storm-Olsen [off-list ref] writes:
From: Frank Li <redacted>

compat/msvc.h includes winsock2.h which conflicts with windows.h.
msvc.h also defines the oldest Windows API version required.
The first sentence sort-of makes sense; compat/msvc.h will be included by
git-compat-util.h and including <windows.h> here will bring conflicting
definitions, so we avoid doing so when on MSC.

The second sentence does not make any sense to me.  It may be correctly
stating a fact (i.e. "defines required WAPI version"), but it is unclear 
what relevance it has to this change to stop including <windows.h>.

Having said that, the first sentence hints me that perhaps you guys should
include (conditionally only on windows but not with MSC) <windows.h> not
from this file, but from git-compat-util.h?
quoted hunk
Signed-off-by: Frank Li <redacted>
Signed-off-by: Marius Storm-Olsen <redacted>
---
 compat/winansi.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/compat/winansi.c b/compat/winansi.c
index 9217c24..0d79845 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -2,7 +2,9 @@
  * Copyright 2008 Peter Harris <git@peter.is-a-geek.org>
  */
 
+#ifndef _MSC_VER
 #include <windows.h>
+#endif
 #include "../git-compat-util.h"
 
 /*
-- 
1.6.3.msysgit.0.18.gef407

Re: [PATCH 09/14] Avoid including windows.h in winansi.c for MSVC build

From: Reece Dunn <hidden>
Date: 2016-06-15 22:47:17

2009/8/21 Junio C Hamano [off-list ref]:
Marius Storm-Olsen [off-list ref] writes:
quoted
From: Frank Li <redacted>

compat/msvc.h includes winsock2.h which conflicts with windows.h.
msvc.h also defines the oldest Windows API version required.
The first sentence sort-of makes sense; compat/msvc.h will be included by
git-compat-util.h and including <windows.h> here will bring conflicting
definitions, so we avoid doing so when on MSC.

The second sentence does not make any sense to me.  It may be correctly
stating a fact (i.e. "defines required WAPI version"), but it is unclear
what relevance it has to this change to stop including <windows.h>.
The way things are configured, windows.h is pulling in winsock.h. In
git-compat-util.h, winsock2.h is included which has conflicting
definitions of various functions and data structures.
Having said that, the first sentence hints me that perhaps you guys should
include (conditionally only on windows but not with MSC) <windows.h> not
from this file, but from git-compat-util.h?
It would make sense for windows.h to be included in git-compat-util.h.

According to http://social.msdn.microsoft.com/Forums/en-US/windowssdk/thread/4a90b143-1fb8-43e9-a54c-956127e0c579,
the following will work:

#define _WINSOCKAPI_    // stops windows.h including winsock.h
#include <winsock2.h>
#include <windows.h>

Also, if you define WIN32_LEAN_AND_MEAN, windows.h will pull in a
subset of the Windows header files (which also improves compilation
times). Adding this may prevent it from pulling in winsock.h. This
would be a better approach (and would make sense to go into
git-compat-util.h).

I don't have access to a Windows dev box at the moment, so can't
verify that this does indeed work.

- Reece

Re: [PATCH 09/14] Avoid including windows.h in winansi.c for MSVC build

From: Marius Storm-Olsen <hidden>
Date: 2016-06-15 22:47:17

On Fri, Aug 21, 2009 at 11:16 PM, Reece Dunn[off-list ref] wrote:
2009/8/21 Junio C Hamano [off-list ref]:
quoted
Marius Storm-Olsen [off-list ref] writes:
quoted
From: Frank Li <redacted>

compat/msvc.h includes winsock2.h which conflicts with windows.h.
msvc.h also defines the oldest Windows API version required.
The first sentence sort-of makes sense; compat/msvc.h will be included by
git-compat-util.h and including <windows.h> here will bring conflicting
definitions, so we avoid doing so when on MSC.

The second sentence does not make any sense to me.  It may be correctly
stating a fact (i.e. "defines required WAPI version"), but it is unclear
what relevance it has to this change to stop including <windows.h>.
The way things are configured, windows.h is pulling in winsock.h. In
git-compat-util.h, winsock2.h is included which has conflicting
definitions of various functions and data structures.
quoted
Having said that, the first sentence hints me that perhaps you guys should
include (conditionally only on windows but not with MSC) <windows.h> not
from this file, but from git-compat-util.h?
It would make sense for windows.h to be included in git-compat-util.h.

According to http://social.msdn.microsoft.com/Forums/en-US/windowssdk/thread/4a90b143-1fb8-43e9-a54c-956127e0c579,
the following will work:

#define _WINSOCKAPI_    // stops windows.h including winsock.h
#include <winsock2.h>
#include <windows.h>

Also, if you define WIN32_LEAN_AND_MEAN, windows.h will pull in a
subset of the Windows header files (which also improves compilation
times). Adding this may prevent it from pulling in winsock.h. This
would be a better approach (and would make sense to go into
git-compat-util.h).

I don't have access to a Windows dev box at the moment, so can't
verify that this does indeed work.

- Reece
I'll check this on Monday, and send out a v3 of the series,
incorporating the pieces commented on, and clarifying some of the
still confusing commit msgs.

--
.marius
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help