Re: Using VC build git (new patch)

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Aug 15, 2009, 17:29 UTC
Message-ID: <alpine.DEB.1.00.0908151908300.8306@pacific.mpi-cbg.de>
In-Reply-To: <alpine.DEB.1.00.0908151851280.8306@pacific.mpi-cbg.de>

Hi,

On Sat, 15 Aug 2009, Johannes Schindelin wrote:

Show 6 quoted lines

> A single monster patch?
> 
> Hmm.
> 
> Please understand that I will not review that unless it is split up into 
> nice little and reviewable chunks.

Well, I try to improve.

So I had a look at your patch. The changes fall naturally into one of the following categories:

	- decl-after-statement fixes

	- missing #include "git-compat-util.h"

	- converting a K&R style function definition to modern C

	- #undef's only needed for Microsoft Visual C++

	- the addition of O_BINARY in the file modes

	- disabling link() (why?)

	- double-#define'ing stat (which puzzles me greatly)

	- dummy #define'ing of a few compiler attributes

	- adding _MSVC to a conditional to avoid  #define'ing
	  SNPRINTF_SIZE_CORR yourself

	- #define'ing several symbols without leading underscore to the
	  MS-specific version with a leading underscore

	- implementing strcasecmp() in a misnamed file

	- "fixing" the return value of winansi_vfprintf for Microsoft 
	  Visual C++ (I think this fix is wrong)

	- correctly adding a Visual C++-specific conditional to 
	  git-compat-util.h, pager.c, run-command.c, run-command.h, 
	  setup.c and help.c, although the latter five could use some 
	  refactoring into git-compat-util.h

	  Maybe there is also room to change the MinGW-conditional into a
	  NO_TRUSTABLE_FILEMODE one

	- adding several headers missing from Visual C++'s installation

	- adding _huge_ .vcproj files (can they not be smaller?)

As you can see, there is a pretty natural way to split that huge patch into chunks that most people on these lists can review easily, and that would actually be a pleasure to look at.

Even better, there are a few fixes (the first three, if you ask me), which are not even Windows-specific.

Further, I would like to suggest adding a header file compat/msvc.h which contains all the #undef's and #define's necessary only for Visual C++, and which can be #include'd from git-compat-util.h, to better separate your work from the other platforms (who do not want those changes). That should avoid those unwanted changes to mingw.c and mingw.h. You just have to make sure that msvc.h is #include'd before mingw.h.

With these comments, I look forward to your next iteration.

Ciao, Dscho