Re: [PATCH 2/2] config: use chmod() instead of fchmod()

[Karsten Blees <karsten.blees@gmail.com>]

Am 17.07.2014 00:16, schrieb Junio C Hamano:
> Karsten Blees <karsten.blees@gmail.com> writes:
> 
>> There is no fchmod() on native Windows platforms (MinGW and MSVC), and the
>> equivalent Win32 API (SetFileInformationByHandle) requires Windows Vista.
>>
>> Use chmod() instead.
>>
>> Signed-off-by: Karsten Blees <blees@dcon.de>
>> ---
> 
> I am wondering if it is saner to just revert the fchmod() patch and
> replace it with something along the lines of
> 
> http://thread.gmane.org/gmane.comp.version-control.git/251682/focus=253219
> 

I also think it makes a lot of sense to handle permissions centrally.

However, with this patch, the permissions of the target file will
additionally be limited by umask (by passing them to open()), and then
overridden completely if core.sharedRepository is set.

Perhaps the lockfile API should respect the location of the lock files
(i.e. use core.sharedRepository in .git, 0666 in the work-tree, and
copy permissions anywhere else).

Another thing I find strange is that, by doing copy/replace, git silently
overwrites readonly files. If we grab the permissions from the source
file anyway, we should perhaps add 'if (!(perms & 0222)) error("file
is readonly");', or even 'access(filename, W_OK)'?

> Having said that, these are the only two callers of fchmod()
> currently in our code base, so I'll queue this patch to allow us to
> kick the problem-can down the road ;-)
> 

Thanks.