Re: Using VC build git (new patch)

From: Frank Li <lznuaa@gmail.com>
Date: Aug 16, 2009, 04:07 UTC
Message-ID: <1976ea660908152107s8b8f3e5l3c2f043fb3e4d62@mail.gmail.com>
In-Reply-To: <alpine.DEB.1.00.0908151908300.8306@pacific.mpi-cbg.de>

2009/8/16 Johannes Schindelin <Johannes.Schindelin@gmx.de>:

Show 23 quoted lines

> 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++

          #undef ERROR is necessary.
          #undef in mingw.h is only for clean some warning.

>
>        - the addition of O_BINARY in the file modes

         Yes, default is text mode if no O_BINARY, _read _write will
do union code and cr\cf convert.

>
>        - disabling link() (why?)

         VC report stack error. I also don't know why.
         I have not deep debug this problem.

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

          old define is
          #define stat    stati64
          #define stati64 msys_stati64.

          stat is used for both struct and function name.
         In C code:
              struct stat a;
         stat -> stati64 -> msys_stati64,  so compiler report struct
msys_stati64 have not defined.

Show 18 quoted lines

>
>        - 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

           Do you means add
           #ifdef _MSC_VER
           #include git-compat-util.h
           #endif

Show 7 quoted lines

>
>          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?)

           It is created by VS2008.  I think it is difficult to make smaller.

Show 14 quoted lines

>
> 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.

VC build will reuse msysgit work. I will reduce change to mingw.c and mingw.h But there are some problem at mingw.c and mingw.h

1. stat defination.  Because both struct and function use the same name stat.
2. open need binary mode.
3. mingw.c use C99 style to break build at VC.
4. mingw.c use special DIR structure, So I need add open_dir in mingw.c.

If I don't change mingw.c, I need copy it to new file totally.

Show 9 quoted lines

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