Re: [msysGit] Re: [PULL] Pull request from msysGit

[Pat Thoyts <patthoyts@gmail.com>]

On 7 October 2010 20:30, Peter Harris <pharris@opentext.com> wrote:
> On 2010-10-07 13:17, Ramsay Jones wrote:
>> Now, Peter Harris has already submitted a fix for this, which is
>> currently on the work/msvc-fixes branch, which contains:
>>
>>     358f1be Modify MSVC wrapper script
>>     38bd27d Fix MSVC build
>>
>> The suggested fix is given in commit 38bd27d. However, I prefer a
>> different solution, which is given below:
>>
>> --- >8 ---
>> diff --git a/compat/mingw.h b/compat/mingw.h
>> index afedf3a..445d1a1 100644
>> --- a/compat/mingw.h
>> +++ b/compat/mingw.h
>> @@ -12,12 +12,6 @@ typedef int pid_t;
>>  #define S_ISLNK(x) (((x) & S_IFMT) == S_IFLNK)
>>  #define S_ISSOCK(x) 0
>>
>> -#ifndef _STAT_H_
>> -#define S_IRUSR 0
>> -#define S_IWUSR 0
>> -#define S_IXUSR 0
>> -#define S_IRWXU (S_IRUSR | S_IWUSR | S_IXUSR)
>> -#endif
>>  #define S_IRGRP 0
>>  #define S_IWGRP 0
>>  #define S_IXGRP 0
>> --- 8< ---
>>
>> Note that, for *both* MinGW and MSVC, the deleted #defines
>> are not wanted, pointless and just plain wrong! :-D
>
> I didn't realize that the defines were not wanted for MinGW either.
>
> I heartily approve of removing code rather than just ifdefing around it.
> Please use this version of the patch instead of mine.
>
> Peter Harris

The patch in question has been on the msysGit tree for about 10 months
now. Its somewhat disappointing not to have had it spotted before we
pushed it upstream. Are the msvc builders only working against junio's
repository?

Reverting it seems to make no difference to the msysGit build at all -
presumably because S_IRUSR and friends are all defined in the mingw
<sys/stat.h> anyway. Sebastian - can you recall why this got added?
The commit comment is not all that enlightening.

I also wonder why changes to a compat/mingw.h file should affect the
msvc build. As it has it's own compat/vcbuild and headers in there,
surely it should be independent of mingw-gcc compatability headers?

Pat Thoyts