[PATCH] compat/mingw.c: MSVC build must use ANSI Win32 API's

[PATCH] compat/mingw.c: MSVC build must use ANSI Win32 API's

[Michael Wookey]

MSVC builds define UNICODE which results in the "WIDE" variation of
Win32 API's being used.

Explicitly use the ANSI variation of the API's for compatibility with
msysgit.

Signed-off-by: Michael Wookey <michaelwookey@gmail.com>
---
 compat/mingw.c |   26 +++++++++++++-------------
 1 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 6b5b5b2..39be42f 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -135,7 +135,7 @@ int mingw_open (const char *filename, int oflags, ...)
 	fd = open(filename, oflags, mode);

 	if (fd < 0 && (oflags & O_CREAT) && errno == EACCES) {
-		DWORD attrs = GetFileAttributes(filename);
+		DWORD attrs = GetFileAttributesA(filename);
 		if (attrs != INVALID_FILE_ATTRIBUTES && (attrs & FILE_ATTRIBUTE_DIRECTORY))
 			errno = EISDIR;
 	}
@@ -607,7 +607,7 @@ static char *lookup_prog(const char *dir, const
char *cmd, int isexe, int exe_on
 		return xstrdup(path);
 	path[strlen(path)-4] = '\0';
 	if ((!exe_only || isexe) && access(path, F_OK) == 0)
-		if (!(GetFileAttributes(path) & FILE_ATTRIBUTE_DIRECTORY))
+		if (!(GetFileAttributesA(path) & FILE_ATTRIBUTE_DIRECTORY))
 			return xstrdup(path);
 	return NULL;
 }
@@ -641,14 +641,14 @@ static int env_compare(const void *a, const void *b)
 static pid_t mingw_spawnve(const char *cmd, const char **argv, char **env,
 			   int prepend_cmd)
 {
-	STARTUPINFO si;
+	STARTUPINFOA si;
 	PROCESS_INFORMATION pi;
 	struct strbuf envblk, args;
 	unsigned flags;
 	BOOL ret;

 	/* Determine whether or not we are associated to a console */
-	HANDLE cons = CreateFile("CONOUT$", GENERIC_WRITE,
+	HANDLE cons = CreateFileA("CONOUT$", GENERIC_WRITE,
 			FILE_SHARE_WRITE, NULL, OPEN_EXISTING,
 			FILE_ATTRIBUTE_NORMAL, NULL);
 	if (cons == INVALID_HANDLE_VALUE) {
@@ -717,7 +717,7 @@ static pid_t mingw_spawnve(const char *cmd, const
char **argv, char **env,
 	}

 	memset(&pi, 0, sizeof(pi));
-	ret = CreateProcess(cmd, args.buf, NULL, NULL, TRUE, flags,
+	ret = CreateProcessA(cmd, args.buf, NULL, NULL, TRUE, flags,
 		env ? envblk.buf : NULL, NULL, &si, &pi);

 	if (env)
@@ -965,23 +965,23 @@ int mingw_rename(const char *pold, const char *pnew)
 	if (errno != EEXIST)
 		return -1;
 repeat:
-	if (MoveFileEx(pold, pnew, MOVEFILE_REPLACE_EXISTING))
+	if (MoveFileExA(pold, pnew, MOVEFILE_REPLACE_EXISTING))
 		return 0;
 	/* TODO: translate more errors */
 	gle = GetLastError();
 	if (gle == ERROR_ACCESS_DENIED &&
-	    (attrs = GetFileAttributes(pnew)) != INVALID_FILE_ATTRIBUTES) {
+	    (attrs = GetFileAttributesA(pnew)) != INVALID_FILE_ATTRIBUTES) {
 		if (attrs & FILE_ATTRIBUTE_DIRECTORY) {
 			errno = EISDIR;
 			return -1;
 		}
 		if ((attrs & FILE_ATTRIBUTE_READONLY) &&
-		    SetFileAttributes(pnew, attrs & ~FILE_ATTRIBUTE_READONLY)) {
-			if (MoveFileEx(pold, pnew, MOVEFILE_REPLACE_EXISTING))
+		    SetFileAttributesA(pnew, attrs & ~FILE_ATTRIBUTE_READONLY)) {
+			if (MoveFileExA(pold, pnew, MOVEFILE_REPLACE_EXISTING))
 				return 0;
 			gle = GetLastError();
 			/* revert file attributes on failure */
-			SetFileAttributes(pnew, attrs);
+			SetFileAttributesA(pnew, attrs);
 		}
 	}
 	if (tries < ARRAY_SIZE(delay) && gle == ERROR_ACCESS_DENIED) {
@@ -1006,7 +1006,7 @@ struct passwd *getpwuid(int uid)
 	static struct passwd p;

 	DWORD len = sizeof(user_name);
-	if (!GetUserName(user_name, &len))
+	if (!GetUserNameA(user_name, &len))
 		return NULL;
 	p.pw_name = user_name;
 	p.pw_gecos = "unknown";
@@ -1151,7 +1151,7 @@ void mingw_open_html(const char *unixpath)
 {
 	const char *htmlpath = make_backslash_path(unixpath);
 	printf("Launching default browser to display HTML ...\n");
-	ShellExecute(NULL, "open", htmlpath, NULL, "\\", 0);
+	ShellExecuteA(NULL, "open", htmlpath, NULL, "\\", 0);
 }

 int link(const char *oldpath, const char *newpath)
@@ -1160,7 +1160,7 @@ int link(const char *oldpath, const char *newpath)
 	static T create_hard_link = NULL;
 	if (!create_hard_link) {
 		create_hard_link = (T) GetProcAddress(
-			GetModuleHandle("kernel32.dll"), "CreateHardLinkA");
+			GetModuleHandleA("kernel32.dll"), "CreateHardLinkA");
 		if (!create_hard_link)
 			create_hard_link = (T)-1;
 	}
-- 
1.6.5.rc1.44.ga1675

Re: [PATCH] compat/mingw.c: MSVC build must use ANSI Win32 API's

[Johannes Sixt]

Michael Wookey schrieb:
> MSVC builds define UNICODE which results in the "WIDE" variation of
> Win32 API's being used.
> 
> Explicitly use the ANSI variation of the API's for compatibility with
> msysgit.
> 
> Signed-off-by: Michael Wookey <michaelwookey@gmail.com>

Marius,

I would like to understand why you did not have this issue.

The patch itself looks fine.

-- Hannes

Re: [PATCH] compat/mingw.c: MSVC build must use ANSI Win32 API's

[Marius Storm-Olsen]

Johannes Sixt said the following on 22.09.2009 08:08:
> Michael Wookey schrieb:
>> MSVC builds define UNICODE which results in the "WIDE" variation of
>> Win32 API's being used.
>>
>> Explicitly use the ANSI variation of the API's for compatibility with
>> msysgit.
>>
>> Signed-off-by: Michael Wookey <michaelwookey@gmail.com>
> 
> Marius,
> 
> I would like to understand why you did not have this issue.
> 
> The patch itself looks fine.

I never added the UNICODE define to the Git compile
process with MSVC (Check the Makefile), so then the
windows API should use the ANSI version by default.
And the following patch proved my point (sorry, will
probably wrap):
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1201,6 +1201,12 @@ struct mingw_DIR
        char                    dd_name[1];     /* given path for dir with search pattern (struct is extended) */
 };

+#ifdef UNICODE
+#pragma message("We have UNICODE defined")
+#else
+#pragma message("Nope, UNICODE is not defined here")
+#endif
+
 struct dirent *mingw_readdir(DIR *dir)
 {
        WIN32_FIND_DATAA buf;


> make MSVC=1
    CC compat/msvc.o
msvc.c
d:\msvc\git\compat\mingw.c(223) : warning C4133: 'function' : incompatible types - from '_stati64 *' to '_stat64 *'
d:\msvc\git\compat\mingw.c(636) : warning C4090: 'initializing' : different 'const' qualifiers
d:\msvc\git\compat\mingw.c(637) : warning C4090: 'initializing' : different 'const' qualifiers
d:\msvc\git\compat\mingw.c(787) : warning C4090: 'function' : different 'const' qualifiers
d:\msvc\git\compat\mingw.c(797) : warning C4090: 'function' : different 'const' qualifiers
Nope, UNICODE is not defined here
    AR libgit.a
Microsoft (R) Library Manager Version 9.00.21022.08
Copyright (C) Microsoft Corporation.  All rights reserved.

Michael, how are you trying to compile git? With the IDE or
the GNU Make? Which version of MSVC? If you use the IDE, can
you make sure it doesn't contain the UNICODE define in the
compiler section of the properties of the projects?

In general though, I'm ok with patches which specifies the
correct API, so we won't have the problem, should we decide
to add UNICODE in the future.

--
.marius

Re: [PATCH] compat/mingw.c: MSVC build must use ANSI Win32 API's

[Michael Wookey]

2009/9/22 Marius Storm-Olsen <mstormo@gmail.com>:
> Michael, how are you trying to compile git? With the IDE or
> the GNU Make? Which version of MSVC? If you use the IDE, can
> you make sure it doesn't contain the UNICODE define in the
> compiler section of the properties of the projects?

I'm using the VS 2008 Professional IDE (the solution doesn't open in
VS 2005). I made no changes to the build settings. In the Preprocessor
section of the project, UNICODE is defined.

On another note, I see *many* build warnings for things like
"signed/unsigned compares". I'd be willing to work through these
warnings and fix them. Thoughts?

Re: [PATCH] compat/mingw.c: MSVC build must use ANSI Win32 API's

[Marius Storm-Olsen]

Michael Wookey said the following on 22.09.2009 11:17:
> 2009/9/22 Marius Storm-Olsen <mstormo@gmail.com>:
>> Michael, how are you trying to compile git? With the IDE or the
>> GNU Make? Which version of MSVC? If you use the IDE, can you make
>> sure it doesn't contain the UNICODE define in the compiler
>> section of the properties of the projects?
> 
> I'm using the VS 2008 Professional IDE (the solution doesn't open
> in VS 2005). I made no changes to the build settings. In the
> Preprocessor section of the project, UNICODE is defined.

Were these projects generated with the Vcproj generator in 
contrib/buildsystem, with the Qmake generator, or the projects from 
Frank's repo?


> On another note, I see *many* build warnings for things like 
> "signed/unsigned compares". I'd be willing to work through these 
> warnings and fix them. Thoughts?

Well, first find out why these are a problem with MSVC and not GCC. 
Are the types different on these platforms? signed vs unsigned should 
show up with GCC as well. We need to make sure that we don't fix 
signed/unsigned issues on one platform, just to introduce it to 
another platform.
In any case, it would be good for someone to have a look at these, 
just so we can determine the cause for most of them, and then we 
should figure out on the list how to deal with them.

Just my €0.2..

--
.marius

Re: [PATCH] compat/mingw.c: MSVC build must use ANSI Win32 API's

[Michael Wookey]

2009/9/22 Marius Storm-Olsen <mstormo@gmail.com>:
> Michael Wookey said the following on 22.09.2009 11:17:
>>
>> 2009/9/22 Marius Storm-Olsen <mstormo@gmail.com>:
>>>
>>> Michael, how are you trying to compile git? With the IDE or the
>>> GNU Make? Which version of MSVC? If you use the IDE, can you make
>>> sure it doesn't contain the UNICODE define in the compiler
>>> section of the properties of the projects?
>>
>> I'm using the VS 2008 Professional IDE (the solution doesn't open
>> in VS 2005). I made no changes to the build settings. In the
>> Preprocessor section of the project, UNICODE is defined.
>
> Were these projects generated with the Vcproj generator in
> contrib/buildsystem, with the Qmake generator, or the projects from Frank's
> repo?

The project was generated from the vcproj generator in
contrib/buildsystem from git.git/master.

>> On another note, I see *many* build warnings for things like
>> "signed/unsigned compares". I'd be willing to work through these warnings
>> and fix them. Thoughts?
>
> Well, first find out why these are a problem with MSVC and not GCC. Are the
> types different on these platforms? signed vs unsigned should show up with
> GCC as well. We need to make sure that we don't fix signed/unsigned issues
> on one platform, just to introduce it to another platform.
> In any case, it would be good for someone to have a look at these, just so
> we can determine the cause for most of them, and then we should figure out
> on the list how to deal with them.

Well, at warning level 4, MSVC is quite verbose. Perhaps the current
gcc build flags are more forgiving?

Re: [PATCH] compat/mingw.c: MSVC build must use ANSI Win32 API's

[Michael Wookey]

2009/9/22 Michael Wookey <michaelwookey@gmail.com>:
> 2009/9/22 Marius Storm-Olsen <mstormo@gmail.com>:
>>> On another note, I see *many* build warnings for things like
>>> "signed/unsigned compares". I'd be willing to work through these warnings
>>> and fix them. Thoughts?
>>
>> Well, first find out why these are a problem with MSVC and not GCC. Are the
>> types different on these platforms? signed vs unsigned should show up with
>> GCC as well. We need to make sure that we don't fix signed/unsigned issues
>> on one platform, just to introduce it to another platform.
>> In any case, it would be good for someone to have a look at these, just so
>> we can determine the cause for most of them, and then we should figure out
>> on the list how to deal with them.
>
> Well, at warning level 4, MSVC is quite verbose. Perhaps the current
> gcc build flags are more forgiving?

Ah, gcc does produces similar warnings when "-Wextra" is added to the CFLAGS:

  CFLAGS = -g -O2 -Wall -Wextra

IMHO, warnings such as these should be worked through and fixed, or
there may be some latent bug waiting to appear.

Re: [PATCH] compat/mingw.c: MSVC build must use ANSI Win32 API's

[Johannes Sixt]

Michael Wookey schrieb:
> 2009/9/22 Marius Storm-Olsen <mstormo@gmail.com>:
>> Michael Wookey said the following on 22.09.2009 11:17:
>>> 2009/9/22 Marius Storm-Olsen <mstormo@gmail.com>:
>>>> Michael, how are you trying to compile git? With the IDE or the
>>>> GNU Make? Which version of MSVC? If you use the IDE, can you make
>>>> sure it doesn't contain the UNICODE define in the compiler
>>>> section of the properties of the projects?
>>> I'm using the VS 2008 Professional IDE (the solution doesn't open
>>> in VS 2005). I made no changes to the build settings. In the
>>> Preprocessor section of the project, UNICODE is defined.
>> Were these projects generated with the Vcproj generator in
>> contrib/buildsystem, with the Qmake generator, or the projects from Frank's
>> repo?
> 
> The project was generated from the vcproj generator in
> contrib/buildsystem from git.git/master.

What's the status of this? Do Sebastian Schuberth's patches

http://article.gmane.org/gmane.comp.version-control.msysgit/7152
http://article.gmane.org/gmane.comp.version-control.msysgit/7153

make a difference?

-- Hannes

Re: [PATCH] compat/mingw.c: MSVC build must use ANSI Win32 API's

[Michael Wookey]

2009/9/28 Johannes Sixt <j.sixt@viscovery.net>:
> Michael Wookey schrieb:
>> 2009/9/22 Marius Storm-Olsen <mstormo@gmail.com>:
>>> Michael Wookey said the following on 22.09.2009 11:17:
>>>> 2009/9/22 Marius Storm-Olsen <mstormo@gmail.com>:
>>>>> Michael, how are you trying to compile git? With the IDE or the
>>>>> GNU Make? Which version of MSVC? If you use the IDE, can you make
>>>>> sure it doesn't contain the UNICODE define in the compiler
>>>>> section of the properties of the projects?
>>>> I'm using the VS 2008 Professional IDE (the solution doesn't open
>>>> in VS 2005). I made no changes to the build settings. In the
>>>> Preprocessor section of the project, UNICODE is defined.
>>> Were these projects generated with the Vcproj generator in
>>> contrib/buildsystem, with the Qmake generator, or the projects from Frank's
>>> repo?
>>
>> The project was generated from the vcproj generator in
>> contrib/buildsystem from git.git/master.
>
> What's the status of this?

I was hoping that this gets included into git.git because it fixes a
real issue with MSVC builds. Since Junio is away, perhaps Shawn can
take the patch into his interim tree?

BTW - would you mind giving this patch an ack?

> Do Sebastian Schuberth's patches
> http://article.gmane.org/gmane.comp.version-control.msysgit/7152
> http://article.gmane.org/gmane.comp.version-control.msysgit/7153
>
> make a difference?

Unfortunately, no. Those patches do not make a difference.

Re: [PATCH] compat/mingw.c: MSVC build must use ANSI Win32 API's

[Johannes Sixt]

Michael Wookey schrieb:
> 2009/9/28 Johannes Sixt <j.sixt@viscovery.net>:
>> Michael Wookey schrieb:
>>> 2009/9/22 Marius Storm-Olsen <mstormo@gmail.com>:
>>>> Michael Wookey said the following on 22.09.2009 11:17:
>>>>> 2009/9/22 Marius Storm-Olsen <mstormo@gmail.com>:
>>>>>> Michael, how are you trying to compile git? With the IDE or the
>>>>>> GNU Make? Which version of MSVC? If you use the IDE, can you make
>>>>>> sure it doesn't contain the UNICODE define in the compiler
>>>>>> section of the properties of the projects?
>>>>> I'm using the VS 2008 Professional IDE (the solution doesn't open
>>>>> in VS 2005). I made no changes to the build settings. In the
>>>>> Preprocessor section of the project, UNICODE is defined.
>>>> Were these projects generated with the Vcproj generator in
>>>> contrib/buildsystem, with the Qmake generator, or the projects from Frank's
>>>> repo?
>>> The project was generated from the vcproj generator in
>>> contrib/buildsystem from git.git/master.
>> What's the status of this?
> 
> I was hoping that this gets included into git.git because it fixes a
> real issue with MSVC builds. Since Junio is away, perhaps Shawn can
> take the patch into his interim tree?
> 
> BTW - would you mind giving this patch an ack?

As I said, the patch looks fine. However, in the commit message you say:

  MSVC builds define UNICODE which results in the "WIDE" variation of
  Win32 API's being used.

But since Marius has built the code without your patch, this justification
must be incomplete. I won't give a formal Ack until this is clarified.

Please work with Marius to figure out why your build uses UNICODE while
Marius's doesn't.

-- Hannes

Re: [PATCH] compat/mingw.c: MSVC build must use ANSI Win32 API's

[Michael Wookey]

2009/9/28 Johannes Sixt <j.sixt@viscovery.net>:
> Michael Wookey schrieb:
>> 2009/9/28 Johannes Sixt <j.sixt@viscovery.net>:
>>> Michael Wookey schrieb:
>>>> 2009/9/22 Marius Storm-Olsen <mstormo@gmail.com>:
>>>>> Michael Wookey said the following on 22.09.2009 11:17:
>>>>>> 2009/9/22 Marius Storm-Olsen <mstormo@gmail.com>:
>>>>>>> Michael, how are you trying to compile git? With the IDE or the
>>>>>>> GNU Make? Which version of MSVC? If you use the IDE, can you make
>>>>>>> sure it doesn't contain the UNICODE define in the compiler
>>>>>>> section of the properties of the projects?
>>>>>> I'm using the VS 2008 Professional IDE (the solution doesn't open
>>>>>> in VS 2005). I made no changes to the build settings. In the
>>>>>> Preprocessor section of the project, UNICODE is defined.
>>>>> Were these projects generated with the Vcproj generator in
>>>>> contrib/buildsystem, with the Qmake generator, or the projects from Frank's
>>>>> repo?
>>>> The project was generated from the vcproj generator in
>>>> contrib/buildsystem from git.git/master.
>>> What's the status of this?
>>
>> I was hoping that this gets included into git.git because it fixes a
>> real issue with MSVC builds. Since Junio is away, perhaps Shawn can
>> take the patch into his interim tree?
>>
>> BTW - would you mind giving this patch an ack?
>
> As I said, the patch looks fine. However, in the commit message you say:
>
>  MSVC builds define UNICODE which results in the "WIDE" variation of
>  Win32 API's being used.
>
> But since Marius has built the code without your patch, this justification
> must be incomplete. I won't give a formal Ack until this is clarified.
>
> Please work with Marius to figure out why your build uses UNICODE while
> Marius's doesn't.

Well, the command line builds have always worked fine. The definition
of UNICODE was limited to building in the IDE. That detail was
unfortunately missing from the original commit message.

It seems that the project file that is generated by Vcproj.pm
(inadvertently?) defines UNICODE. Perhaps the patch below is better
than my original workaround. If you think so, I'll create a formal
patch.

[ sorry if the patch wraps ]

diff --git a/contrib/buildsystems/Generators/Vcproj.pm
b/contrib/buildsystems/Generators/Vcproj.pm
index 00ec0c1..a648756 100644
--- a/contrib/buildsystems/Generators/Vcproj.pm
+++ b/contrib/buildsystems/Generators/Vcproj.pm
@@ -173,7 +173,7 @@ sub createLibProject {
                                Optimization="0"
                                InlineFunctionExpansion="1"
                                AdditionalIncludeDirectories="$includes"
-
PreprocessorDefinitions="UNICODE,WIN32,_DEBUG,$defines"
+                               PreprocessorDefinitions="WIN32,_DEBUG,$defines"
                                MinimalRebuild="true"
                                RuntimeLibrary="1"
                                UsePrecompiledHeader="0"

Re: [PATCH] compat/mingw.c: MSVC build must use ANSI Win32 API's

[Michael Wookey]

> It seems that the project file that is generated by Vcproj.pm
> (inadvertently?) defines UNICODE. Perhaps the patch below is better
> than my original workaround. If you think so, I'll create a formal
> patch.
>
> [ sorry if the patch wraps ]
>

scrub the previous patch... there were more instances of UNICODE
defined (for release and debug builds). The patch below takes care of
them all.

diff --git a/contrib/buildsystems/Generators/Vcproj.pm
b/contrib/buildsystems/Generators/Vcproj.pm
index 00ec0c1..a215911 100644
--- a/contrib/buildsystems/Generators/Vcproj.pm
+++ b/contrib/buildsystems/Generators/Vcproj.pm
@@ -173,7 +173,7 @@ sub createLibProject {
                                Optimization="0"
                                InlineFunctionExpansion="1"
                                AdditionalIncludeDirectories="$includes"
-
PreprocessorDefinitions="UNICODE,WIN32,_DEBUG,$defines"
+                               PreprocessorDefinitions="WIN32,_DEBUG,$defines"
                                MinimalRebuild="true"
                                RuntimeLibrary="1"
                                UsePrecompiledHeader="0"
@@ -239,7 +239,7 @@ sub createLibProject {
                                InlineFunctionExpansion="1"
                                EnableIntrinsicFunctions="true"
                                AdditionalIncludeDirectories="$includes"
-
PreprocessorDefinitions="UNICODE,WIN32,NDEBUG,$defines"
+                               PreprocessorDefinitions="WIN32,NDEBUG,$defines"
                                RuntimeLibrary="0"
                                EnableFunctionLevelLinking="true"
                                UsePrecompiledHeader="0"
@@ -395,7 +395,7 @@ sub createAppProject {
                                Optimization="0"
                                InlineFunctionExpansion="1"
                                AdditionalIncludeDirectories="$includes"
-
PreprocessorDefinitions="UNICODE,WIN32,_DEBUG,$defines"
+                               PreprocessorDefinitions="WIN32,_DEBUG,$defines"
                                MinimalRebuild="true"
                                RuntimeLibrary="1"
                                UsePrecompiledHeader="0"
@@ -466,7 +466,7 @@ sub createAppProject {
                                InlineFunctionExpansion="1"
                                EnableIntrinsicFunctions="true"
                                AdditionalIncludeDirectories="$includes"
-
PreprocessorDefinitions="UNICODE,WIN32,NDEBUG,$defines"
+                               PreprocessorDefinitions="WIN32,NDEBUG,$defines"
                                RuntimeLibrary="0"

Re: [PATCH] compat/mingw.c: MSVC build must use ANSI Win32 API's

[Marius Storm-Olsen]

Michael Wookey said the following on 28.09.2009 11:55:
>> It seems that the project file that is generated by Vcproj.pm
>> (inadvertently?) defines UNICODE. Perhaps the patch below is better
>> than my original workaround. If you think so, I'll create a formal
>> patch.
>>
>> [ sorry if the patch wraps ]
>>
> 
> scrub the previous patch... there were more instances of UNICODE
> defined (for release and debug builds). The patch below takes care of
> them all.

Yup, IMO this is the correct patch, since it will follow the Makefile 
more closely. So, if we then decide to add UNICODE in the Makefile, 
the generated files will follow.

Make it into a proper patch, and I'll ack.

--
.marius