On Wed, Dec 8, 2010 at 01:05, Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxx> wrote: > Johannes Sixt wrote: >> Instead of removing the macros, wouldn't we be much safer with just >> >> #ifndef S_IWUSR >> >> ? ... > > Er... no. > > Commit 4091bfc (which added these macros) does not provide any motivation > for the change, and I'm having a hard time trying to imagine a useful > purpose for this part of the commit. (I'm not saying there isn't one - just > that I can't see it :-P ) Sorry for not finding the time to respond to the thread in [1] about two months ago where this issue about my commit was first raised. While it's true that my commit message does not contain any detailed information about its motivation, it says the defines were "missing", suggesting a compile error. Indeed, I remember that back then my msysgit working tree did not compile with MSVC if I didn't have these defines (and I vaguely remember that this was caused by MSVC using different a header file than MinGW, or in a different order, or something similar). However, I'm not able to reproduce this anymore. I checked out 4091bfc^ and 4091bfc, and both compile file with MSVC for me now, the latter just giving a lot of the mentioned macro redefinition warnings. Maybe this was caused my me using older MSVC project files with a newer code base ... I probably should have run contrib/buildsystems/generate again. After defining LF_FACESIZE and TMPF_TRUETYPE in winansi.c, and INTMAX_MAX in git-compat-util.h, I was also able to compile the v1.7.3.2.msysgit.0 tag with MSVC. If I revert 4091bfc on top of it, it still compiles fine for me. > So, once again, I see no reason to keep them ... Unless you know otherwise. I agree to remove the lines and vote in favor of Ramsay's patch. Feel free to add me as Signed-off-by or Acked-by. [1] http://thread.gmane.org/gmane.comp.version-control.git/158144/focus=158409 -- Sebastian Schuberth -- 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