Re: Using VC build git (new patch)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Sat, 15 Aug 2009, Johannes Schindelin wrote:

> 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



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]