Fix MSVC compile errors and cleanup stat definitions

PATCH In review Karsten Blees 4 participants 9 messages Jun 15, 2016

lore

Share & tools

lore

Karsten Blees

author

2016-06-15 22:58 UTC #0 lore raw

cover Fix MSVC compile errors and cleanup stat definitions

A few minor fixes for the MSVC build.

Also here: https://github.com/kblees/git/tree/kb/fix-msvc-stat-definitions

Karsten Blees (3):
MSVC: fix compile errors due to missing libintl.h
MSVC: fix compile errors due to macro redefinitions
MSVC: fix stat definition hell

 compat/mingw.h   | 21 +++++++++++++++++----
 compat/msvc.h    | 15 ---------------
 config.mak.uname |  1 +
 3 files changed, 18 insertions(+), 19 deletions(-)

Signature · 1 line

1.8.4.8243.gbcbdefd

Karsten Blees

author

2016-06-15 22:58 UTC #1 lore raw

PATCH 1/3 MSVC: fix compile errors due to missing libintl.h

Set NO_GETTEXT in config.mak.uname to get rid of libintl.h dependency.

Signed-off-by Karsten Blees <[email protected]>

---
 config.mak.uname | 1 +
 1 file changed, 1 insertion(+)

config.mak.uname +1 -0

index b27f51d..64b7f49 100644
@@ -340,6 +340,7 @@ ifeq ($(uname_S),Windows)
 	OBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo
 	NO_REGEX = YesPlease
 	NO_CURL = YesPlease
+	NO_GETTEXT = YesPlease
 	NO_PYTHON = YesPlease
 	BLK_SHA1 = YesPlease
 	ETAGS_TARGET = ETAGS

Signature · 1 line

1.8.4.8243.gbcbdefd

Karsten Blees

author

2016-06-15 22:58 UTC #2 lore raw

PATCH 2/3 MSVC: fix compile errors due to macro redefinitions

In reply to Karsten Blees

Skip errno.h definitions if they are already defined.

Signed-off-by Karsten Blees <[email protected]>

---
 compat/mingw.h | 6 ++++++
 1 file changed, 6 insertions(+)

compat/mingw.h +6 -0

index bd0a88b..6b531e4 100644
@@ -32,7 +32,9 @@ typedef int socklen_t;
 #define WEXITSTATUS(x) ((x) & 0xff)
 #define WTERMSIG(x) SIGTERM
 
+#ifndef EWOULDBLOCK
 #define EWOULDBLOCK EAGAIN
+#endif
 #define SHUT_WR SD_SEND
 
 #define SIGHUP 1

@@ -46,8 +48,12 @@ typedef int socklen_t;
 #define F_SETFD 2
 #define FD_CLOEXEC 0x1
 
+#ifndef EAFNOSUPPORT
 #define EAFNOSUPPORT WSAEAFNOSUPPORT
+#endif
+#ifndef ECONNABORTED
 #define ECONNABORTED WSAECONNABORTED
+#endif
 
 struct passwd {
 	char *pw_name;

Signature · 1 line

1.8.4.8243.gbcbdefd

Karsten Blees

author

2016-06-15 22:58 UTC #3 lore raw

PATCH 3/3 MSVC: fix stat definition hell

In reply to Karsten Blees

In msvc.h, there's a couple of stat related functions defined diffently from mingw.h. When we remove these definitions, the only problem we get is "warning C4005: '_stati64' : macro redefinition" for this line in mingw.h:

#define _stati64(x,y) mingw_stat(x,y)

The reason is that as of MSVCR80.dll (distributed with MSVC 2005), the original _stati64 family of functions was renamed to _stat32i64, and the former function names became macros (pointing to the appropriate function based on the definition of _USE_32BIT_TIME_T).

Defining _stati64 works on MinGW because MinGW by default compiles against the MSVCRT.DLL that is part of Windows (i.e. _stati64 is a function rather than a macro).

Note: MinGW *can* compile for newer MSVC runtime versions, and MSVC apparently can also compile for the Windows MSVCRT.DLL via the DDK (see
http://www.syndicateofideas.com/posts/fighting-the-msvcrt-dll-hell ).

Remove the stat definitions from msvc.h, as they are not compiler related.

In mingw.h, determine the runtime version in use from the definitions of _stati64 and _USE_32BIT_TIME_T, and define stat() accordingly.

This also fixes that stat() in MSVC builds still resolves to mingw_lstat() instead of mingw_stat().

Signed-off-by Karsten Blees <[email protected]>

---
 compat/mingw.h | 15 +++++++++++----
 compat/msvc.h  | 15 ---------------
 2 files changed, 11 insertions(+), 19 deletions(-)

compat/mingw.h +11 -4

index 6b531e4..3c3a9d9 100644
@@ -264,19 +264,26 @@ static inline int getrlimit(int resource, struct rlimit *rlp)
 	return 0;
 }
 
-/* Use mingw_lstat() instead of lstat()/stat() and
- * mingw_fstat() instead of fstat() on Windows.
+/*
+ * Use mingw specific stat()/lstat()/fstat() implementations on Windows.
  */
 #define off_t off64_t
 #define lseek _lseeki64
-#ifndef ALREADY_DECLARED_STAT_FUNCS
+
+/* use struct stat with 64 bit st_size */
 #define stat _stati64
 int mingw_lstat(const char *file_name, struct stat *buf);
 int mingw_stat(const char *file_name, struct stat *buf);
 int mingw_fstat(int fd, struct stat *buf);
 #define fstat mingw_fstat
 #define lstat mingw_lstat
-#define _stati64(x,y) mingw_stat(x,y)
+
+#ifndef _stati64
+# define _stati64(x,y) mingw_stat(x,y)
+#elif defined (_USE_32BIT_TIME_T)
+# define _stat32i64(x,y) mingw_stat(x,y)
+#else
+# define _stat64(x,y) mingw_stat(x,y)
 #endif
 
 int mingw_utime(const char *file_name, const struct utimbuf *times);

compat/msvc.h +0 ~~-15~~

index 96b6d60..580bb55 100644
@@ -24,21 +24,6 @@ static __inline int strcasecmp (const char *s1, const char *s2)
 
 #undef ERROR
 
-/* Use mingw_lstat() instead of lstat()/stat() and mingw_fstat() instead
- * of fstat(). We add the declaration of these functions here, suppressing
- * the corresponding declarations in mingw.h, so that we can use the
- * appropriate structure type (and function) names from the msvc headers.
- */
-#define stat _stat64
-int mingw_lstat(const char *file_name, struct stat *buf);
-int mingw_fstat(int fd, struct stat *buf);
-#define fstat mingw_fstat
-#define lstat mingw_lstat
-#define _stat64(x,y) mingw_lstat(x,y)
-#define ALREADY_DECLARED_STAT_FUNCS
-
 #include "compat/mingw.h"
 
-#undef ALREADY_DECLARED_STAT_FUNCS
-
 #endif

Signature · 1 line

1.8.4.8243.gbcbdefd

Sebastian Schuberth

2016-06-15 22:58 UTC #4 lore raw

Re: [PATCH 3/3] MSVC: fix stat definition hell

Quoted 5 lines · Karsten Blees

In msvc.h, there's a couple of stat related functions defined diffently from mingw.h. When we remove these definitions, the only problem we get is "warning C4005: '_stati64' : macro redefinition" for this line in mingw.h:
#define _stati64(x,y) mingw_stat(x,y)

I have a similar patch at [1] to fix similar compilation issues with MinGW runtime version 4.0, which was recently released. I like your patch better, so I've rebased mine on top of yours:

[PATCH] MinGW: Fix stat definitions to work with MinGW runtime version 4.0

For an overview of changes in mingwrt-4.0 see:

    http://sourceforge.net/p/mingw/mingw-org-wsl/ci/4.0.0/tree/NEWS

Signed-off-by Sebastian Schuberth <[email protected]>

---
 compat/mingw.c   | 1 -
 compat/mingw.h   | 9 +++++++++
 config.mak.uname | 2 +-
 3 files changed, 10 insertions(+), 2 deletions(-)

compat/mingw.c +0 -1

index 96d9ac4..29c051f 100644
@@ -616,7 +616,6 @@ int mingw_stat(const char *file_name, struct stat *buf)
 	return do_stat_internal(1, file_name, buf);
 }
 
-#undef fstat
 int mingw_fstat(int fd, struct stat *buf)
 {
 	HANDLE fh = (HANDLE)_get_osfhandle(fd);

compat/mingw.h +9 -0

index b521900..0d2faac 100644
@@ -278,11 +278,20 @@ static inline int getrlimit(int resource, struct rlimit *rlp)
 #define lseek _lseeki64
 
 /* use struct stat with 64 bit st_size */
+#ifdef stat
+#undef stat
+#endif
 #define stat _stati64
 int mingw_lstat(const char *file_name, struct stat *buf);
 int mingw_stat(const char *file_name, struct stat *buf);
 int mingw_fstat(int fd, struct stat *buf);
+#ifdef fstat
+#undef fstat
+#endif
 #define fstat mingw_fstat
+#ifdef lstat
+#undef lstat
+#endif
 #define lstat mingw_lstat
 
 #ifndef _stati64

config.mak.uname +1 -1

index 9249ee3..983ecc1 100644
@@ -499,7 +499,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 	NO_POSIX_GOODIES = UnfortunatelyYes
 	DEFAULT_HELP_FORMAT = html
 	NO_D_INO_IN_DIRENT = YesPlease
-	COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat -Icompat/win32
+	COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -D_USE_32BIT_TIME_T -DNOGDI -Icompat -Icompat/win32
 	COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
 	COMPAT_OBJS += compat/mingw.o compat/winansi.o \
 		compat/win32/pthread.o compat/win32/syslog.o \

Signature · 8 lines

1.8.3.mingw.1.2.g56240b5.dirty

I don't think we should squash this patch to yours, however, because yours addressed MSVC compilation issues, and mine address MinGW compilation issues. But my patch could go to your branch.

[1] https://github.com/sschuberth/git/commit/841cdf60faa134eef031a7cf6d6692473a18cb65

-- 
Sebastian Schuberth

Eric Sunshine

2016-06-15 22:58 UTC #5 lore raw

Re: [PATCH 3/3] MSVC: fix stat definition hell

In reply to Karsten Blees

Quoted 1 line · Karsten Blees

In msvc.h, there's a couple of stat related functions defined diffently

s/diffently/differently/

Quoted 27 lines

from mingw.h. When we remove these definitions, the only problem we get is "warning C4005: '_stati64' : macro redefinition" for this line in mingw.h:
#define _stati64(x,y) mingw_stat(x,y)
The reason is that as of MSVCR80.dll (distributed with MSVC 2005), the original _stati64 family of functions was renamed to _stat32i64, and the former function names became macros (pointing to the appropriate function based on the definition of _USE_32BIT_TIME_T).
Defining _stati64 works on MinGW because MinGW by default compiles against the MSVCRT.DLL that is part of Windows (i.e. _stati64 is a function rather than a macro).
Note: MinGW *can* compile for newer MSVC runtime versions, and MSVC apparently can also compile for the Windows MSVCRT.DLL via the DDK (see
http://www.syndicateofideas.com/posts/fighting-the-msvcrt-dll-hell ).
Remove the stat definitions from msvc.h, as they are not compiler related.
In mingw.h, determine the runtime version in use from the definitions of _stati64 and _USE_32BIT_TIME_T, and define stat() accordingly.
This also fixes that stat() in MSVC builds still resolves to mingw_lstat() instead of mingw_stat().
Signed-off-by Karsten Blees <[email protected]>

Junio C Hamano

2016-06-15 22:58 UTC #6 lore raw

In reply to Karsten Blees

Quoted 9 lines · Karsten Blees

A few minor fixes for the MSVC build.
Also here: https://github.com/kblees/git/tree/kb/fix-msvc-stat-definitions
Karsten Blees (3):
MSVC: fix compile errors due to missing libintl.h
MSVC: fix compile errors due to macro redefinitions
MSVC: fix stat definition hell

Quoted 4 lines

 compat/mingw.h   | 21 +++++++++++++++++----
 compat/msvc.h    | 15 ---------------
 config.mak.uname |  1 +
 3 files changed, 18 insertions(+), 19 deletions(-)

I won't be a good person to review, suggest improvements on, and/or judge these patches. I'm Cc'ing regulars who work on mingw port for their help and Ack.

Thanks.

Sebastian Schuberth

Acked-by

2016-06-15 22:58 UTC #7 lore raw

Quoted 11 lines · Junio C Hamano

Quoted 9 lines · Karsten Blees
A few minor fixes for the MSVC build.
Also here: https://github.com/kblees/git/tree/kb/fix-msvc-stat-definitions
Karsten Blees (3):
MSVC: fix compile errors due to missing libintl.h
MSVC: fix compile errors due to macro redefinitions
MSVC: fix stat definition hell

Quoted 8 lines

Quoted 4 lines
 compat/mingw.h   | 21 +++++++++++++++++----
 compat/msvc.h    | 15 ---------------
 config.mak.uname |  1 +
 3 files changed, 18 insertions(+), 19 deletions(-)
I won't be a good person to review, suggest improvements on, and/or judge these patches. I'm Cc'ing regulars who work on mingw port for their help and Ack.

Acked-by Sebastian Schuberth <[email protected]>

Signature · 1 line

Sebastian Schuberth

Junio C Hamano

2016-06-15 22:58 UTC #8 lore raw

Quoted 18 lines · Sebastian Schuberth

Quoted 16 lines · Junio C Hamano
Quoted 13 lines · Karsten Blees
A few minor fixes for the MSVC build.
Also here: https://github.com/kblees/git/tree/kb/fix-msvc-stat-definitions
Karsten Blees (3):
MSVC: fix compile errors due to missing libintl.h
MSVC: fix compile errors due to macro redefinitions
MSVC: fix stat definition hell
 compat/mingw.h   | 21 +++++++++++++++++----
 compat/msvc.h    | 15 ---------------
 config.mak.uname |  1 +
 3 files changed, 18 insertions(+), 19 deletions(-)

Quoted 5 lines

Quoted 3 lines
I won't be a good person to review, suggest improvements on, and/or judge these patches. I'm Cc'ing regulars who work on mingw port for their help and Ack.
Acked-by Sebastian Schuberth <[email protected]>

Thanks; will queue these three and your follow-up on top.