Hi, [please quote only those parts that you actually reply to.] On Sun, 16 Aug 2009, Frank Li wrote: > 2009/8/16 Johannes Schindelin <Johannes.Schindelin@xxxxxx>: > > > - 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. As Git _never_ wants to open files in text mode, this is a change that can come _separately_ from your VC patches. It could even be considered a bug fix (MinGW's current GCC implies O_BINARY, but previous ones may not, and it is wrong of us to rely on this magic). > > - disabling link() (why?) > > VC report stack error. I also don't know why. I have not deep debug this > problem. You should. What with Visual Studio's superior debugging capabilities, it should not be a problem. Right? > > - double-#define'ing stat (which puzzles me greatly) > > old define is > #define stat stati64 > #define stati64 msys_stati64. Not exactly. #define stat _stati64 [...] #define _stati64(x,y) mingw_lstat(x,y) And even VC should handle that just fine. > 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. It is a real pity that you did not inline your patch, and that I could not quote it as a consequence, because now we could discuss code properly: The original code was done on purpose as it is, to use the _stati64 struct (which allows long pointers, as Microsoft decided off_t was not good enough to represent 64 bit offsets). I could imagine that your patch breaks that for Visual C++. > > - adding _huge_ .vcproj files (can they not be smaller?) > > It is created by VS2008. I think it is difficult to make smaller. Is there not some magic to make a compact export version? But maybe it is not that bad? I have 72 (out of 1730) files in my repository that are larger, gitk being the king. FWIW I would have loved it if _you_ defended the size of the file, instead of having to do it myself. > > 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. That is no question. We made it easy for you. > 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. That needs to be resolved somehow, and you would get _much_ more help if you had split your patch series properly and this was _one_ patch. > 2. open need binary mode. As I said, this can be a separate change. In my original reply I _implied_ that it is okay, and in this reply I elaborated on that (namely that it might be considered a bug fix). > 3. mingw.c use C99 style to break build at VC. The same applies here as to O_BINARY. > 4. mingw.c use special DIR structure, So I need add open_dir in mingw.c. No, you do not need to add that to mingw.c. MinGW does not need it. So it has no place in mingw.c. > If I don't change mingw.c, I need copy it to new file totally. NOOOO!!! You #include it after making sure that you have #undef'ed and #define'd whatever needs to, and in a separate file -- which is only referenced in the .vcproj -- you can implement whatever Visual C++ misses. > > With these comments, I look forward to your next iteration. > > > > Ciao, > > Dscho Please, please, PLEASE, do not quote things that you are not addressing. And please, please, PLEASE adher to Documentation/SubmittingPatches. Actually, the only thing SubmittingPatches does is spell out how to make things easier for reviewers, should you not know how to do so. Remember: the more time it takes me to cull the quoted parts of your mail, the more time it takes me to have a look at your patches, the more time it takes me to refer to your patch and make suggestions, the more am I inclined to spend my time elsewhere, doing something that is much more enjoyable. Ciao, Dscho