Re: [PATCH 00/14] Add submodule test harness

[Junio C Hamano <gitster@pobox.com>]

Johannes Sixt <j6t@kdbg.org> writes:

> Am 08.07.2014 21:34, schrieb Jens Lehmann:
>>> And Msysgit complains 
>>> error: fchmod on c:/xxxt/trash directory.t7613-merge-submodule/submodule_update_repo/.git/modules/sub1/config.lock failed: Function not implemented
>> 
>> I'm not sure what this is about, seems to happen during the "cp -R" of
>> the repo under .git/modules into the submodule.
>
> No. It happens because fchmod() is not implemented in our Windows port.
>
> Please see my band-aid patch at
> http://thread.gmane.org/gmane.comp.version-control.git/248154/focus=20266
> The sub-thread ended inconclusive.

We need to start somewhere, and a no-op fchmod() in your patch may
be as a good place to start as anything.  At least we would then
keep the old behaviour without introducing any new failure.

An alternative might be to use chmod() after we are done writing to
the config.lock in order to avoid the use of fchmod() altogether,
which I think can replace the existing two callsites of fchmod().
That approach might be a more expedient, but may turn out to be
undesirable in the longer term.

I also wonder if this "carry forward the original permission bits
when updating an existing file by first writing the updated contents
into a lockfile and then renaming it after we are done" pattern
ought to be done in the lockfile API at commit_lock_file() time (Duy
and Michael Cc'ed for their input, as they have recently touched
lockfile API implementation in their series somewhat), not at the
level of the user of the lockfile API like Eric's patch daa22c6f
(config: preserve config file permissions on edits, 2014-05-06) did
only for the config file.