git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Compile fix for MSVC: Move poll.h out of sys-folder
@ 2011-11-18 13:47 Vincent van Ravesteijn
  2011-11-18 14:09 ` Erik Faye-Lund
  0 siblings, 1 reply; 5+ messages in thread
From: Vincent van Ravesteijn @ 2011-11-18 13:47 UTC (permalink / raw)
  To: git; +Cc: gitster, kusmabite, msysgit, j.sixt, Vincent van Ravesteijn

In v1.7.7.1-432-g0f77dea (Oct 24 2011; Erik Faye-Lund; mingw: move
poll out of sys-folder) poll.h was moved out of the compat/win32/sys
folder. As the change in the Makefile also affects the MSVC build,
the same must be done for poll.h in compat/vcbuild/include/sys/poll.h.

Signed-off-by: Vincent van Ravesteijn <vfr@lyx.org>
---
 compat/vcbuild/include/poll.h     |    1 +
 compat/vcbuild/include/sys/poll.h |    1 -
 2 files changed, 1 insertions(+), 1 deletions(-)
 create mode 100644 compat/vcbuild/include/poll.h
 delete mode 100644 compat/vcbuild/include/sys/poll.h

diff --git a/compat/vcbuild/include/poll.h b/compat/vcbuild/include/poll.h
new file mode 100644
index 0000000..0d8552a
--- /dev/null
+++ b/compat/vcbuild/include/poll.h
@@ -0,0 +1 @@
+/* Intentionally empty file to support building git with MSVC */
diff --git a/compat/vcbuild/include/sys/poll.h b/compat/vcbuild/include/sys/poll.h
deleted file mode 100644
index 0d8552a..0000000
--- a/compat/vcbuild/include/sys/poll.h
+++ /dev/null
@@ -1 +0,0 @@
-/* Intentionally empty file to support building git with MSVC */
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] Compile fix for MSVC: Move poll.h out of sys-folder
  2011-11-18 13:47 [PATCH] Compile fix for MSVC: Move poll.h out of sys-folder Vincent van Ravesteijn
@ 2011-11-18 14:09 ` Erik Faye-Lund
  2011-11-18 14:28   ` Erik Faye-Lund
  2011-11-18 14:42   ` Vincent van Ravesteijn
  0 siblings, 2 replies; 5+ messages in thread
From: Erik Faye-Lund @ 2011-11-18 14:09 UTC (permalink / raw)
  To: Vincent van Ravesteijn; +Cc: git, gitster, msysgit, j.sixt

On Fri, Nov 18, 2011 at 2:47 PM, Vincent van Ravesteijn <vfr@lyx.org> wrote:
> In v1.7.7.1-432-g0f77dea (Oct 24 2011; Erik Faye-Lund; mingw: move
> poll out of sys-folder) poll.h was moved out of the compat/win32/sys
> folder. As the change in the Makefile also affects the MSVC build,
> the same must be done for poll.h in compat/vcbuild/include/sys/poll.h.
>
> Signed-off-by: Vincent van Ravesteijn <vfr@lyx.org>
> ---
>  compat/vcbuild/include/poll.h     |    1 +
>  compat/vcbuild/include/sys/poll.h |    1 -
>  2 files changed, 1 insertions(+), 1 deletions(-)
>  create mode 100644 compat/vcbuild/include/poll.h
>  delete mode 100644 compat/vcbuild/include/sys/poll.h
>

This looks strange to me. vcbuild/include/poll.h will only prevent the
correct header from being included, while compiling an linking against
compat/win32/poll.[co]... That seems dangerous to me, because the
interface might be declared differently.

Instead, I think compat/vcbuild/include/poll.h should be removed, and
_WIN32_WINNT set to a value below 0x600. That way the poll-stuff
doesn't get pulled in by winsock2.h (as it's Vista and above only).

This was already discussed in your "[PATCHv2] Compile fix for MSVC" thread:
http://mid.gmane.org/CABPQNSaCRRRpEQPG1Mb4DovkMdQSBhHTm-i7y5M4iT+ndHX4XA@mail.gmail.com

Here's the patch that fixes it. I still can't build Junio's master,
due to sys/resource.h missing. This comes from ebae9ff ("compat: add
missing #include <sys/resource.h>"), and is only guarded against
MinGW, not MSVC...

$ git diff
diff --git a/compat/mingw.h b/compat/mingw.h
index dfb0e87..a06269d 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -1,3 +1,4 @@
+#define _WIN32_WINNT 0x0501
 #include <winsock2.h>
 #include <ws2tcpip.h>

diff --git a/git-compat-util.h b/git-compat-util.h
index 5ef8ff7..c52be6c 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -85,6 +85,7 @@
 #define _SGI_SOURCE 1

 #ifdef WIN32 /* Both MinGW and MSVC */
+#define _WIN32_WINNT 0x0501
 #define WIN32_LEAN_AND_MEAN  /* stops windows.h including winsock.h */
 #include <winsock2.h>
 #include <windows.h>

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] Compile fix for MSVC: Move poll.h out of sys-folder
  2011-11-18 14:09 ` Erik Faye-Lund
@ 2011-11-18 14:28   ` Erik Faye-Lund
  2011-11-18 16:47     ` Vincent van Ravesteijn
  2011-11-18 14:42   ` Vincent van Ravesteijn
  1 sibling, 1 reply; 5+ messages in thread
From: Erik Faye-Lund @ 2011-11-18 14:28 UTC (permalink / raw)
  To: Vincent van Ravesteijn; +Cc: git, gitster, msysgit, j.sixt

On Fri, Nov 18, 2011 at 3:09 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> On Fri, Nov 18, 2011 at 2:47 PM, Vincent van Ravesteijn <vfr@lyx.org> wrote:
>> In v1.7.7.1-432-g0f77dea (Oct 24 2011; Erik Faye-Lund; mingw: move
>> poll out of sys-folder) poll.h was moved out of the compat/win32/sys
>> folder. As the change in the Makefile also affects the MSVC build,
>> the same must be done for poll.h in compat/vcbuild/include/sys/poll.h.
>>
>> Signed-off-by: Vincent van Ravesteijn <vfr@lyx.org>
>> ---
>>  compat/vcbuild/include/poll.h     |    1 +
>>  compat/vcbuild/include/sys/poll.h |    1 -
>>  2 files changed, 1 insertions(+), 1 deletions(-)
>>  create mode 100644 compat/vcbuild/include/poll.h
>>  delete mode 100644 compat/vcbuild/include/sys/poll.h
>>
>
> This looks strange to me. vcbuild/include/poll.h will only prevent the
> correct header from being included, while compiling an linking against
> compat/win32/poll.[co]... That seems dangerous to me, because the
> interface might be declared differently.
>
> Instead, I think compat/vcbuild/include/poll.h should be removed, and
> _WIN32_WINNT set to a value below 0x600. That way the poll-stuff
> doesn't get pulled in by winsock2.h (as it's Vista and above only).
>
> This was already discussed in your "[PATCHv2] Compile fix for MSVC" thread:
> http://mid.gmane.org/CABPQNSaCRRRpEQPG1Mb4DovkMdQSBhHTm-i7y5M4iT+ndHX4XA@mail.gmail.com
>
> Here's the patch that fixes it.

Johannes Schindelin was nice enough to create a commit based on my
e-mail, feel free to pick it up and submit it:

https://github.com/msysgit/git/commit/9ca803910f3625bf686699f6b0bf71a8c68bccae

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Compile fix for MSVC: Move poll.h out of sys-folder
  2011-11-18 14:09 ` Erik Faye-Lund
  2011-11-18 14:28   ` Erik Faye-Lund
@ 2011-11-18 14:42   ` Vincent van Ravesteijn
  1 sibling, 0 replies; 5+ messages in thread
From: Vincent van Ravesteijn @ 2011-11-18 14:42 UTC (permalink / raw)
  To: kusmabite; +Cc: git, gitster, msysgit, j.sixt

> This looks strange to me. vcbuild/include/poll.h will only prevent the
> correct header from being included, while compiling an linking against
> compat/win32/poll.[co]... That seems dangerous to me, because the
> interface might be declared differently.
>
> Instead, I think compat/vcbuild/include/poll.h should be removed,

Yes, it should just be removed. Having poll.h in the sys directory was 
wrong and the patch I sent was needed to make 0f77dea compile.

Your patch is indeed necessary to compile later revisions as well.

> I still can't build Junio's master,
> due to sys/resource.h missing. This comes from ebae9ff ("compat: add
> missing #include<sys/resource.h>"), and is only guarded against
> MinGW, not MSVC...

The fix for this is already in 'next' now. So, with your patch, 'next' 
does compile.

Vincent

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Compile fix for MSVC: Move poll.h out of sys-folder
  2011-11-18 14:28   ` Erik Faye-Lund
@ 2011-11-18 16:47     ` Vincent van Ravesteijn
  0 siblings, 0 replies; 5+ messages in thread
From: Vincent van Ravesteijn @ 2011-11-18 16:47 UTC (permalink / raw)
  To: kusmabite; +Cc: git, gitster, msysgit, j.sixt

Op 18-11-2011 15:28, Erik Faye-Lund schreef:
> On Fri, Nov 18, 2011 at 3:09 PM, Erik Faye-Lund<kusmabite@gmail.com>  wrote:
>> On Fri, Nov 18, 2011 at 2:47 PM, Vincent van Ravesteijn<vfr@lyx.org>  wrote:
>>> In v1.7.7.1-432-g0f77dea (Oct 24 2011; Erik Faye-Lund; mingw: move
>>> poll out of sys-folder) poll.h was moved out of the compat/win32/sys
>>> folder. As the change in the Makefile also affects the MSVC build,
>>> the same must be done for poll.h in compat/vcbuild/include/sys/poll.h.
>>>
>>> Signed-off-by: Vincent van Ravesteijn<vfr@lyx.org>
>>> ---
>>>   compat/vcbuild/include/poll.h     |    1 +
>>>   compat/vcbuild/include/sys/poll.h |    1 -
>>>   2 files changed, 1 insertions(+), 1 deletions(-)
>>>   create mode 100644 compat/vcbuild/include/poll.h
>>>   delete mode 100644 compat/vcbuild/include/sys/poll.h
>>>
>> This looks strange to me. vcbuild/include/poll.h will only prevent the
>> correct header from being included, while compiling an linking against
>> compat/win32/poll.[co]... That seems dangerous to me, because the
>> interface might be declared differently.
>>
>> Instead, I think compat/vcbuild/include/poll.h should be removed, and
>> _WIN32_WINNT set to a value below 0x600. That way the poll-stuff
>> doesn't get pulled in by winsock2.h (as it's Vista and above only).
>>
>> This was already discussed in your "[PATCHv2] Compile fix for MSVC" thread:
>> http://mid.gmane.org/CABPQNSaCRRRpEQPG1Mb4DovkMdQSBhHTm-i7y5M4iT+ndHX4XA@mail.gmail.com
>>
>> Here's the patch that fixes it.
> Johannes Schindelin was nice enough to create a commit based on my
> e-mail, feel free to pick it up and submit it:
>
> https://github.com/msysgit/git/commit/9ca803910f3625bf686699f6b0bf71a8c68bccae

I resended a patch series including this one.

Vincent

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2011-11-18 16:47 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-18 13:47 [PATCH] Compile fix for MSVC: Move poll.h out of sys-folder Vincent van Ravesteijn
2011-11-18 14:09 ` Erik Faye-Lund
2011-11-18 14:28   ` Erik Faye-Lund
2011-11-18 16:47     ` Vincent van Ravesteijn
2011-11-18 14:42   ` Vincent van Ravesteijn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).