patch, 12 partsMINGW: compat/bswap.h: include stdint.h

> bswap.h uses uint32_t type which might not be defined.
> This patch adds direct include so bswap.h can be safely included.
>
> Signed-off-by: Marat Radchenko <marat@slonopotamus.org>
> ---
>  compat/bswap.h | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/compat/bswap.h b/compat/bswap.h
> index 120c6c1..d170447 100644
> --- a/compat/bswap.h
> +++ b/compat/bswap.h
> @@ -5,6 +5,8 @@
>   * operation.
>   */
>
> +#include <stdint.h>
> +
>  /*
>   * Default version that the compiler ought to optimize properly with
>   * constant values.

> On Mon, Apr 28, 2014 at 04:45:43PM +0200, Erik Faye-Lund wrote:
>> bswap.h is included after stdint.h from git-compat-util.h anyway...
>
> That only becomes true after PATCH 05 when talking about MinGW.
>
> Will drop this one.

> On Mon, Apr 28, 2014 at 05:02:09PM +0200, Erik Faye-Lund wrote:
>> msysGit has a declaration of it in io.h as well. But it's not a
>> preprocessor-definition... Are you saying that it's a
>> preprocessor-define in mingw-w64, that points to a 64-bit version? If
>> so, looks good.
>
> MinGW is x86 only.
>
> MinGW-W64, a separate project, provides both x86 and x86_64.
>
> And here's relevant part of io.h from MingW-W64:
> http://sourceforge.net/apps/trac/mingw-w64/browser/trunk/mingw-w64-headers/crt/io.h?rev=5437#L321

> On 2014-04-28 15.51, Marat Radchenko wrote:
> > mingw-w64 has lseek defined in io.h.
> []
> >  #define off_t off64_t
> > +#ifndef lseek
> >  #define lseek _lseeki64
> > +#endif
> Is the commit message in line with the code?
> 
> I would have expected something in this style:
> 
> #if defined(__x86_64__) && ! defined(lseek))
> #include <io.h>
> #endif 

> On 2014-04-28 15.51, Marat Radchenko wrote:
>> mingw-w64 has lseek defined in io.h.
> []
>>  #define off_t off64_t
>> +#ifndef lseek
>>  #define lseek _lseeki64
>> +#endif
> Is the commit message in line with the code?
>
> I would have expected something in this style:
>
> #if defined(__x86_64__) && ! defined(lseek))
> #include <io.h>
> #endif

> [...]
> > -STRIP ?= strip
> > +STRIP = $(CROSS_COMPILE)strip
> 
> Before, STRIP from the environment took precedence over STRIP from the
> makefile.  Switching to the more usual 'environment can't be trusted'
> convention is a good change, but please mention it in the commit
> message.

> > -STRIP ?= strip
> > +STRIP = $(CROSS_COMPILE)strip
> 
> Before, STRIP from the environment took precedence over STRIP from the
> makefile.  Switching to the more usual 'environment can't be trusted'
> convention is a good change, but please mention it in the commit
> message.

> Simply expanded variables are defined by lines using ‘:=’ or ‘::=’ (see Setting Variables).
> Both forms are equivalent in GNU make; however only the ‘::=’ form is described by the POSIX
> standard (support for ‘::=’ was added to the POSIX standard in 2012, so older versions of make
> won't accept this form either).
>
> The value of a simply expanded variable is scanned once and for all, expanding any references
> to other variables and functions, when the variable is defined. The actual value of the simply
> expanded variable is the result of expanding the text that you write. It does not contain any
> references to other variables; it contains their values as of the time this variable was defined.
> Therefore,
>
>    x := foo
>    y := $(x) bar
>    x := later
> is equivalent to
>
>    y := foo bar
>    x := later
>
> When a simply expanded variable is referenced, its value is substituted verbatim.

>>> -STRIP ?= strip
>>> +STRIP = $(CROSS_COMPILE)strip
>>
>> Before, STRIP from the environment took precedence over STRIP from the
>> makefile.  Switching to the more usual 'environment can't be trusted'
>> convention is a good change, but please mention it in the commit
>> message.
>
> Taken from [1]:
>
>> Simply expanded variables are defined by

> @@ -401,8 +404,11 @@ htmldir_relative = $(patsubst $(prefix)/%,%,$(htmldir))
>  
>  export prefix bindir sharedir sysconfdir gitwebdir localedir
>  
> -CC = cc
> -AR = ar
> +AR = $(CROSS_COMPILE)ar
> +CC = $(CROSS_COMPILE)cc
> +GCOV = $(CROSS_COMPILE)gcov
> +STRIP = $(CROSS_COMPILE)strip

> diff --git a/config.mak.uname b/config.mak.uname
> index 5d301da..6c2e6df 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -511,7 +511,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
>  	EXTLIBS += -lws2_32
>  	GITLIBS += git.res
>  	PTHREAD_LIBS =
> -	RC = windres -O coff
> +	RC = $(CROSS_COMPILE)windres -O coff

> > -	RC = windres -O coff
> > +	RC = $(CROSS_COMPILE)windres -O coff
> 
> I don't think this is the best.
> 
> We should probably have this in the Makefile:
> 
>   RC = $(CROSS_COMPILE)windres
> 
> And then config.mak.uname should have
> 
>   RCFLAGS += -O coff

> On Mon, Apr 28, 2014 at 12:37:42PM -0500, Felipe Contreras wrote:
> > > +CC = $(CROSS_COMPILE)cc
> > 
> > Nice.
> 
> Actually, not. You still have to override CC because it is
> $(CROSS_COMPILE)*g*cc. Any thoughts how to handle this?

>>> +CC = $(CROSS_COMPILE)cc
>>
>> Nice.
>
> Actually, not. You still have to override CC because it is
> $(CROSS_COMPILE)*g*cc. Any thoughts how to handle this?

> Marat Radchenko wrote:
> > On Mon, Apr 28, 2014 at 12:37:42PM -0500, Felipe Contreras wrote:
> 
> >>> +CC = $(CROSS_COMPILE)cc
> >>
> >> Nice.
> >
> > Actually, not. You still have to override CC because it is
> > $(CROSS_COMPILE)*g*cc. Any thoughts how to handle this?
> 
> One possibility would be something like
> 
> 	ifdef CROSS_COMPILE
> 	CC = $(CROSS_COMPILE)gcc
> 	else
> 	CC = cc
> 	endif

> Also, pass -D__USE_MINGW_ANSI_STDIO=0 to select MSVCRT-compatible
> macro definitions.
>
> Signed-off-by: Marat Radchenko <marat@slonopotamus.org>
> ---
>  compat/mingw.h    |  2 --
>  compat/msvc.h     |  3 +++
>  config.mak.uname  |  3 ++-
>  git-compat-util.h | 11 ++++++-----
>  4 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/compat/mingw.h b/compat/mingw.h
> index 262b300..c502a22 100644
> --- a/compat/mingw.h
> +++ b/compat/mingw.h
> @@ -342,8 +342,6 @@ static inline char *mingw_find_last_dir_sep(const char *path)
>  }
>  #define find_last_dir_sep mingw_find_last_dir_sep
>  #define PATH_SEP ';'
> -#define PRIuMAX "I64u"
> -#define PRId64 "I64d"
>
>  void mingw_open_html(const char *path);
>  #define open_html mingw_open_html
> diff --git a/compat/msvc.h b/compat/msvc.h
> index 580bb55..cb41ce3 100644
> --- a/compat/msvc.h
> +++ b/compat/msvc.h
> @@ -15,6 +15,9 @@
>  #define strtoull     _strtoui64
>  #define strtoll      _strtoi64
>
> +#define PRIuMAX "I64u"
> +#define PRId64 "I64d"
> +
>  static __inline int strcasecmp (const char *s1, const char *s2)
>  {
>         int size1 = strlen(s1);
> diff --git a/config.mak.uname b/config.mak.uname
> index 6c2e6df..e5edae6 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -321,6 +321,7 @@ ifeq ($(uname_S),Windows)
>         NO_PREAD = YesPlease
>         NEEDS_CRYPTO_WITH_SSL = YesPlease
>         NO_LIBGEN_H = YesPlease
> +       NO_INTTYPES_H = UnfortunatelyYes
>         NO_POLL = YesPlease
>         NO_SYMLINK_HEAD = YesPlease
>         NO_IPV6 = YesPlease
> @@ -502,7 +503,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
>         NO_INET_NTOP = YesPlease
>         NO_POSIX_GOODIES = UnfortunatelyYes
>         DEFAULT_HELP_FORMAT = html
> -       COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -D_USE_32BIT_TIME_T -DNOGDI -Icompat -Icompat/win32
> +       COMPAT_CFLAGS += -D__USE_MINGW_ANSI_STDIO=0 -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 \
> diff --git a/git-compat-util.h b/git-compat-util.h
> index f6d3a46..aa57a04 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -85,6 +85,12 @@
>  #define _NETBSD_SOURCE 1
>  #define _SGI_SOURCE 1
>
> +#ifndef NO_INTTYPES_H
> +#include <inttypes.h>
> +#else
> +#include <stdint.h>
> +#endif
> +

> Just checking that I understand: Does this mean that we now require an
> MSVC-version that has stdint.h? If so, I'm not against such a case.
> IMO, the biggest benefit of using MSVC is not building on legacy
> systems, but being able to use it's debugger. And for that purpose
> it's probably OK to increase the required version.

> On Mon, Apr 28, 2014 at 04:53:52PM +0200, Erik Faye-Lund wrote:
>> Just checking that I understand: Does this mean that we now require an
>> MSVC-version that has stdint.h? If so, I'm not against such a case.
>> IMO, the biggest benefit of using MSVC is not building on legacy
>> systems, but being able to use it's debugger. And for that purpose
>> it's probably OK to increase the required version.
>
> Ouch, that was not intentional. What minimal MSVC version is currently
> supported and who decides if it is OK to increase required one?

> On Mon, Apr 28, 2014 at 3:51 PM, Marat Radchenko <marat@slonopotamus.org> wrote:
> > HAVE_LIBCHARSET_H and NO_R_TO_GCC_LINKER are not specific to
> > msysGit, they're general MinGW settings.
> 
> Actually, HAVE_LIBCHARSET_H is. It's only present because we have
> libiconv installed.

> On Mon, Apr 28, 2014 at 04:58:11PM +0200, Erik Faye-Lund wrote:
>> On Mon, Apr 28, 2014 at 3:51 PM, Marat Radchenko <marat@slonopotamus.org> wrote:
>> > HAVE_LIBCHARSET_H and NO_R_TO_GCC_LINKER are not specific to
>> > msysGit, they're general MinGW settings.
>>
>> Actually, HAVE_LIBCHARSET_H is. It's only present because we have
>> libiconv installed.
>
> 1. What are other ways to provide iconv on MinGW?

> > 1. What are other ways to provide iconv on MinGW?
> 
> I'm not sure I understand. To set HAVE_LIBCHARSET_H, we need to have
> libcharset.h. MinGW doesn't supply by default to my knowledge, so we
> get it from iconv. The THIS_IS_MSYSGIT file is there for us to be able
> to pick the right defaults for msysGit, and us having libcharset is
> indeed a msysGit-detail. Not all iconv-flavors supply libcharset.h, so
> this tells a particularity about the one we have in msysGit.

> On Mon, Apr 28, 2014 at 3:51 PM, Marat Radchenko <marat@slonopotamus.org> wrote:
> > Also, fix `warning: passing argument 2 of 'mingw_main' from
> > incompatible pointer type` in http-fetch.c and remote-curl.c.
> 
> These seems completely unrelated, perhaps it should be split in two?

> On Mon, Apr 28, 2014 at 05:26:38PM +0200, Erik Faye-Lund wrote:
>> On Mon, Apr 28, 2014 at 3:51 PM, Marat Radchenko <marat@slonopotamus.org> wrote:
>> > Also, fix `warning: passing argument 2 of 'mingw_main' from
>> > incompatible pointer type` in http-fetch.c and remote-curl.c.
>>
>> These seems completely unrelated, perhaps it should be split in two?
>
> Okay, will split. Though there is a connection - until this patch,
> http-fetch.c and remote-curl.c never built for MinGW.

> On MinGW-W64, MsgWaitForMultipleObjects is guarded with #ifndef NOGDI.
>
> Unfortunately, including wingdi.h brings in #define ERROR 0 which
> conflicts with several Git internal enums. So, #undef ERROR.
>
> Signed-off-by: Marat Radchenko <marat@slonopotamus.org>
> ---
>  config.mak.uname  | 4 ++--
>  git-compat-util.h | 2 ++
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/config.mak.uname b/config.mak.uname
> index a376b32..4883fd5 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -363,7 +363,7 @@ ifeq ($(uname_S),Windows)
>         COMPAT_OBJS = compat/msvc.o compat/winansi.o \
>                 compat/win32/pthread.o compat/win32/syslog.o \
>                 compat/win32/dirent.o
> -       COMPAT_CFLAGS = -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/regex -Icompat/win32 -DSTRIP_EXTENSION=\".exe\"
> +       COMPAT_CFLAGS = -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/regex -Icompat/win32 -DSTRIP_EXTENSION=\".exe\"
>         BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib
>         EXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib invalidcontinue.obj
>         PTHREAD_LIBS =
> @@ -503,7 +503,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
>         NO_INET_NTOP = YesPlease
>         NO_POSIX_GOODIES = UnfortunatelyYes
>         DEFAULT_HELP_FORMAT = html
> -       COMPAT_CFLAGS += -D__USE_MINGW_ANSI_STDIO=0 -D__USE_MINGW_ACCESS -D_USE_32BIT_TIME_T -DNOGDI -Icompat -Icompat/win32
> +       COMPAT_CFLAGS += -D__USE_MINGW_ANSI_STDIO=0 -D__USE_MINGW_ACCESS -D_USE_32BIT_TIME_T -Icompat -Icompat/win32
>         COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
>         COMPAT_OBJS += compat/mingw.o compat/winansi.o \
>                 compat/win32/pthread.o compat/win32/syslog.o \
> diff --git a/git-compat-util.h b/git-compat-util.h
> index aa57a04..48bf0f7 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -98,6 +98,8 @@
>  #define WIN32_LEAN_AND_MEAN  /* stops windows.h including winsock.h */
>  #include <winsock2.h>
>  #include <windows.h>
> +/* wingdi.h defines ERROR=0, undef it to avoid conflicts */
> +#undef ERROR
>  #define GIT_WINDOWS_NATIVE
>  #endif

> nedalloc was initially added in f0ed82 to fix slowness of standard WinXP
> memory allocator. Since WinXP is EOLed, this point is no longer valid.
>
> The actual reason behind this commit is incompatibility of malloc.c.h
> with MinGW-W64 headers. Alternative solution implies updating nedalloc
> to something newer.

>> nedalloc was initially added in f0ed82 to fix slowness of standard WinXP
>> memory allocator. Since WinXP is EOLed, this point is no longer valid.
>>
>> The actual reason behind this commit is incompatibility of malloc.c.h
>> with MinGW-W64 headers. Alternative solution implies updating nedalloc
>> to something newer.
>
> Did you measure that malloc on newer Windows-versions are actually
> faster? AFAIK, malloc does a lot more inside the CRT than in the
> kernel...

> On Mon, Apr 28, 2014 at 3:51 PM, Marat Radchenko <marat@slonopotamus.org> wrote:
> > nedalloc was initially added in f0ed82 to fix slowness of standard WinXP
> > memory allocator. Since WinXP is EOLed, this point is no longer valid.
> >
> > The actual reason behind this commit is incompatibility of malloc.c.h
> > with MinGW-W64 headers. Alternative solution implies updating nedalloc
> > to something newer.
> 
> Did you measure that malloc on newer Windows-versions are actually
> faster? AFAIK, malloc does a lot more inside the CRT than in the
> kernel...

> Marat Radchenko wrote:
>> nedalloc was initially added in f0ed82 to fix slowness of standard 
>> WinXP
>> memory allocator. Since WinXP is EOLed, this point is no longer 
>> valid.
>
> The fact that WinXP is EOLed doesn't mean people are not using it any 
> more =/
>
> I think it should be supported by Git at least for a while.
>
> -- 
> Felipe Contreras
> --

> pid_t is available in sys/types.h on both MinGW and MinGW-W64
>
> Signed-off-by: Marat Radchenko <marat@slonopotamus.org>
> ---
>  compat/mingw.h | 1 -
>  compat/msvc.h  | 2 ++
>  2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/compat/mingw.h b/compat/mingw.h
> index c502a22..8850109 100644
> --- a/compat/mingw.h
> +++ b/compat/mingw.h
> @@ -5,7 +5,6 @@
>   * things that are not available in header files
>   */
>
> -typedef int pid_t;
>  typedef int uid_t;
>  typedef int socklen_t;
>  #define hstrerror strerror
> diff --git a/compat/msvc.h b/compat/msvc.h
> index cb41ce3..e2fda48 100644
> --- a/compat/msvc.h
> +++ b/compat/msvc.h
> @@ -18,6 +18,8 @@
>  #define PRIuMAX "I64u"
>  #define PRId64 "I64d"
>
> +typedef int pid_t;
> +
>  static __inline int strcasecmp (const char *s1, const char *s2)
>  {
>         int size1 = strlen(s1);

> fork() is not used in MinGW builds but causes a compiler warning
> on x86_64 MinGW-W64: conflicting types for built-in function 'fork'
>
> Signed-off-by: Marat Radchenko <marat@slonopotamus.org>
> ---
>  compat/mingw.h | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/compat/mingw.h b/compat/mingw.h
> index 8850109..2fbc8ea 100644
> --- a/compat/mingw.h
> +++ b/compat/mingw.h
> @@ -89,8 +89,6 @@ static inline int symlink(const char *oldpath, const char *newpath)
>  { errno = ENOSYS; return -1; }
>  static inline int fchmod(int fildes, mode_t mode)
>  { errno = ENOSYS; return -1; }
> -static inline pid_t fork(void)
> -{ errno = ENOSYS; return -1; }
>  static inline unsigned int alarm(unsigned int seconds)
>  { return 0; }
>  static inline int fsync(int fd)

> This patch series fixes building on modern MinGW and (32bit only yet) MinGW-W64.
>
> *Compilation* tested on:
>  - MSVC (via WinGit environment)
>  - msysGit environment
>  - Linux cross-toolchain i686-pc-mingw32 (4.8.2) with mingw-runtime-3.20.2
>  - Linux cross-toolchain i686-w64-mingw32 (4.8.2) with mingw64-runtime-3.1.0
>
> Stuff still required to make Git build with x86_64 MinGW-W64 toolchain:
>
> 1. Drop -D_USE_32BIT_TIME_T that was added in fa93bb to config.mak.uname
> because time_t cannot be 32bit on x86_64. I haven't yet figured out what
> should break if this define is removed (pointers are welcome) and why it was
> added in the first place.
>
> 2. Stop passing --large-address-aware to linker. I wonder if it does anything
> for 32bit MinGW builds.
>
> 3. Fix several places with mismatched pointer size casts.
>
> Building it from Gentoo Linux:
>
> MinGW:
>
>   crossdev -t i686-pc-mingw32
>   ARCH=x86 emerge-i686-pc-mingw32 -u dev-libs/libiconv sys-libs/zlib net-misc/curl sys-devel/gettext expat
>   cd <git>
>   make CROSS_COMPILE=i686-pc-mingw32- CC=i686-pc-mingw32-gcc NO_OPENSSL=1 MINGW=1 CURLDIR=/usr/i686-pc-mingw32/usr
>
> MinGW-W64 (32 bit):
>
>   crossdev -t i686-w64-mingw32
>   ARCH=x86 emerge-i686-w64-mingw32 -u dev-libs/libiconv sys-libs/zlib net-misc/curl sys-devel/gettext expat
>   cd <git>
>   make CROSS_COMPILE=i686-w64-mingw32- CC=i686-w64-mingw32-gcc NO_OPENSSL=1 MINGW=1 CURLDIR=/usr/i686-w64-mingw32/usr
>
> Debian/Ubuntu build instructions are WIP (xdeb is non-trivial at all).