Re: Consecutive git gc fails on Windows network share

Re: Consecutive git gc fails on Windows network share

[Torsten Bögershausen]

(No top-posting, please see my comments below)
On 2014-01-20 15.02, Jochen wrote:
>On 01/16/2014 10:55 AM, Jochen Haag wrote:
> Hello,
>
> we want to report the following issue, because it seems to be not an
> intended behaviour. Maybe someone can have a look at it at some point.
> It is not urgent for us.
>
> After upgrading from Git version 1.8.1.msysgit.1 to 1.8.5.2.msysgit.0 we
> observed that a consecutive git gc fails, in case the Git repo is
> located on a Windows network share. Operating system used is Windows 7
> 64 bit SP1.
>
> In case git gc fails temporary packs and index remain in .git/objects/pack/.
>
> "Consecutive" means, that git gc is called on an already packed
> repository (e.g. git gc is called more than once).
>
> This is the output given in case of error:
>
> U:\GitEnv>git gc
> Counting objects: 139, done.
> Delta compression using up to 8 threads.
> Compressing objects: 100% (71/71), done.
> Writing objects: 100% (139/139), done.
> Total 139 (delta 65), reused 139 (delta 65)
> fatal: renaming
> '.git/objects/pack/.tmp-7920-pack-ad9d069e7c27ce19cc5b6cde82b4a38bedbf5884.pack'
> failed: Permission denied
> error: failed to run repack
>
> And here the output with GIT_TRACE = 1:
>
> U:\GitEnv>git gc
> trace: built-in: git 'gc'
> trace: run_command: 'pack-refs' '--all' '--prune'
> trace: built-in: git 'pack-refs' '--all' '--prune'
> trace: run_command: 'reflog' 'expire' '--all'
> trace: built-in: git 'reflog' 'expire' '--all'
> trace: run_command: 'repack' '-d' '-l' '-A'
> '--unpack-unreachable=2.weeks.ago'
> trace: built-in: git 'repack' '-d' '-l' '-A'
> '--unpack-unreachable=2.weeks.ago'
> trace: run_command: 'pack-objects' '--keep-true-parents'
> '--honor-pack-keep' '--non-empty' '--all' '--reflog'
> '--unpack-unreachable=2.weeks.ago' '--local' '--delta-base-offset'
> '.git/objects/pack/.tmp-7612-pack'
> trace: built-in: git 'pack-objects' '--keep-true-parents'
> '--honor-pack-keep' '--non-empty' '--all' '--reflog'
> '--unpack-unreachable=2.weeks.ago' '--local' '--delta-base-offset'
> '.git/objects/pack/.tmp-7612-pack'
> Counting objects: 139, done.
> Delta compression using up to 8 threads.
> Compressing objects: 100% (71/71), done.
> Writing objects: 100% (139/139), done.
> Total 139 (delta 65), reused 139 (delta 65)
> fatal: renaming
> '.git/objects/pack/.tmp-7612-pack-ad9d069e7c27ce19cc5b6cde82b4a38bedbf5884.pack'
> failed: Permission denied
> error: failed to run repack
>
> After running git gc twice, this is what is left in .git/objects/pack/:
>
> U:\GitEnv\.git\objects\pack>ls -al
> total 53676
> drwxr-xr-x    1 hgj2fe   Administ        0 Jan 16 10:43 .
> drwxr-xr-x    1 hgj2fe   Administ        0 Jan 14 12:52 ..
> -r--r--r--    1 hgj2fe   Administ     4964 Jan 16 10:43
> .tmp-7612-pack-ad9d069e7c27ce19cc5b6cde82b4a38bedbf5884.idx
> -r--r--r--    1 hgj2fe   Administ 18315618 Jan 16 10:43
> .tmp-7612-pack-ad9d069e7c27ce19cc5b6cde82b4a38bedbf5884.pack
> -r--r--r--    1 hgj2fe   Administ     4964 Jan 16 10:40
> .tmp-7920-pack-ad9d069e7c27ce19cc5b6cde82b4a38bedbf5884.idx
> -r--r--r--    1 hgj2fe   Administ 18315618 Jan 16 10:40
> .tmp-7920-pack-ad9d069e7c27ce19cc5b6cde82b4a38bedbf5884.pack
> -r--r--r--    1 hgj2fe   Administ     4964 Jan 14 12:52
> pack-ad9d069e7c27ce19cc5b6cde82b4a38bedbf5884.idx
> -r--r--r--    1 hgj2fe   Administ 18315618 Jan 14 12:52
> pack-ad9d069e7c27ce19cc5b6cde82b4a38bedbf5884.pack
>
> In case we remove the read-only flag of the pack and index manually
> before running git gc again, no problem occurs:
>
> U:\GitEnv\.git\objects\pack>git gc
> trace: built-in: git 'gc'
> trace: run_command: 'pack-refs' '--all' '--prune'
> trace: built-in: git 'pack-refs' '--all' '--prune'
> trace: run_command: 'reflog' 'expire' '--all'
> trace: built-in: git 'reflog' 'expire' '--all'
> trace: run_command: 'repack' '-d' '-l' '-A'
> '--unpack-unreachable=2.weeks.ago'
> trace: built-in: git 'repack' '-d' '-l' '-A'
> '--unpack-unreachable=2.weeks.ago'
> trace: run_command: 'pack-objects' '--keep-true-parents'
> '--honor-pack-keep' '--non-empty' '--all' '--reflog'
> '--unpack-unreachable=2.weeks.ago' '--local' '--delta-base-offset'
> 'U:/GitEnv/.git/objects/pack/.tmp-3736-pack'
> trace: built-in: git 'pack-objects' '--keep-true-parents'
> '--honor-pack-keep' '--non-empty' '--all' '--reflog'
> '--unpack-unreachable=2.weeks.ago' '--local' '--delta-base-offset'
> 'U:/GitEnv/.git/objects/pack/.tmp-3736-pack'
> Counting objects: 139, done.
> Delta compression using up to 8 threads.
> Compressing objects: 100% (71/71), done.
> Writing objects: 100% (139/139), done.
> Total 139 (delta 65), reused 139 (delta 65)
> trace: run_command: 'prune-packed'
> trace: built-in: git 'prune-packed'
> trace: run_command: 'update-server-info'
> trace: built-in: git 'update-server-info'
> trace: run_command: 'prune' '--expire' '2.weeks.ago'
> trace: built-in: git 'prune' '--expire' '2.weeks.ago'
> trace: run_command: 'rerere' 'gc'
> trace: built-in: git 'rerere' 'gc'
>
> The problem might be related to this commit:
> https://github.com/msysgit/git/commit/a1bbc6c0176f1fa2d4aa571cc0183a1f0ff9b285
>
>
> Best regards,
>
> Jochen
------
> 
> 
> Am Freitag, 17. Januar 2014 19:02:07 UTC+1 schrieb Torsten Bögershausen:
> 
> 
>     So, please, what happens if you revert that commit?
>     It could be good if you can test it and share the results with the list
>     /Torsten
> 
> 
> Instead of reverting we did some more analysis.
> 
> In repack.c we found the following code:
> 
>         /* Now the ones with the same name are out of the way... */
>         for_each_string_list_item(item, &names) {
>                 for (ext = 0; ext < 2; ext++) {
>                         char *fname, *fname_old;
>                         struct stat statbuffer;
>                         fname = mkpathdup("%s/pack-%s%s",
>                                         packdir, item->string, exts[ext]);
>                         fname_old = mkpathdup("%s-%s%s",
>                                         packtmp, item->string, exts[ext]);
>                         if (!stat(fname_old, &statbuffer)) {
>                                 statbuffer.st_mode &= ~(S_IWUSR | S_IWGRP | S_IWOTH);
>                                 chmod(fname_old, statbuffer.st_mode);
>                         }
>                         if (rename(fname_old, fname))
>                                 die_errno(_("renaming '%s' failed"), fname_old);
>                         free(fname);
>                         free(fname_old);
>                 }
>         }
> 
> The rename command replaces a mv -f command of the original shell script. Obviously the -f option can also override a read-only file but rename can not on a network share.
> 
> We changed the code as followed:
> 
>         /* Now the ones with the same name are out of the way... */
>         for_each_string_list_item(item, &names) {
>                 for (ext = 0; ext < 2; ext++) {
>                         char *fname, *fname_old;
>                         struct stat statbuffer;
>                         fname = mkpathdup("%s/pack-%s%s",
>                                         packdir, item->string, exts[ext]);
>                         fname_old = mkpathdup("%s-%s%s",
>                                         packtmp, item->string, exts[ext]);
>                         if (!stat(fname_old, &statbuffer)) {
>                                 statbuffer.st_mode &= ~(S_IWUSR | S_IWGRP | S_IWOTH);
>                                 chmod(fname_old, statbuffer.st_mode);
>                         }
>+++                         if (!stat(fname, &statbuffer)) {
>+++                                 statbuffer.st_mode |= (S_IWUSR | S_IWGRP | S_IWOTH);
>+++                                 chmod(fname, statbuffer.st_mode);
>+++                         }
>                         if (rename(fname_old, fname))
>                                 die_errno(_("renaming '%s' failed"), fname_old);
>                         free(fname);
>                         free(fname_old);
>                 }
>         }
> 
> Before rename is called the destination file is made writeable. This worked for us. We are not sure if this is a good implementation.

Jochen,
thanks for sharing the code changes with us.

I allowed me to 
a) reconstruct the original mail,
b) add "+++" at the places where you added the stat() and chmod(),
c) and to send the question "is this a good implementation ?" to upstream git.

I think your implementation makes sense.
/Torsten
 

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Consecutive git gc fails on Windows network share

[Stefan Beller]

On 20.01.2014 18:16, Torsten Bögershausen wrote:
> (No top-posting, please see my comments below)
> On 2014-01-20 15.02, Jochen wrote:
>> On 01/16/2014 10:55 AM, Jochen Haag wrote:
>> Hello,
>>
>> we want to report the following issue, because it seems to be not an
>> intended behaviour. Maybe someone can have a look at it at some point.
>> It is not urgent for us.
>>
>> After upgrading from Git version 1.8.1.msysgit.1 to 1.8.5.2.msysgit.0 we
>> observed that a consecutive git gc fails, in case the Git repo is
>> located on a Windows network share. Operating system used is Windows 7
>> 64 bit SP1.
>>
>> In case git gc fails temporary packs and index remain in .git/objects/pack/.
>>
>> "Consecutive" means, that git gc is called on an already packed
>> repository (e.g. git gc is called more than once).
>>
>> This is the output given in case of error:
>>
>> U:\GitEnv>git gc
>> Counting objects: 139, done.
>> Delta compression using up to 8 threads.
>> Compressing objects: 100% (71/71), done.
>> Writing objects: 100% (139/139), done.
>> Total 139 (delta 65), reused 139 (delta 65)
>> fatal: renaming
>> '.git/objects/pack/.tmp-7920-pack-ad9d069e7c27ce19cc5b6cde82b4a38bedbf5884.pack'
>> failed: Permission denied
>> error: failed to run repack
>>
>> And here the output with GIT_TRACE = 1:
>>
>> U:\GitEnv>git gc
>> trace: built-in: git 'gc'
>> trace: run_command: 'pack-refs' '--all' '--prune'
>> trace: built-in: git 'pack-refs' '--all' '--prune'
>> trace: run_command: 'reflog' 'expire' '--all'
>> trace: built-in: git 'reflog' 'expire' '--all'
>> trace: run_command: 'repack' '-d' '-l' '-A'
>> '--unpack-unreachable=2.weeks.ago'
>> trace: built-in: git 'repack' '-d' '-l' '-A'
>> '--unpack-unreachable=2.weeks.ago'
>> trace: run_command: 'pack-objects' '--keep-true-parents'
>> '--honor-pack-keep' '--non-empty' '--all' '--reflog'
>> '--unpack-unreachable=2.weeks.ago' '--local' '--delta-base-offset'
>> '.git/objects/pack/.tmp-7612-pack'
>> trace: built-in: git 'pack-objects' '--keep-true-parents'
>> '--honor-pack-keep' '--non-empty' '--all' '--reflog'
>> '--unpack-unreachable=2.weeks.ago' '--local' '--delta-base-offset'
>> '.git/objects/pack/.tmp-7612-pack'
>> Counting objects: 139, done.
>> Delta compression using up to 8 threads.
>> Compressing objects: 100% (71/71), done.
>> Writing objects: 100% (139/139), done.
>> Total 139 (delta 65), reused 139 (delta 65)
>> fatal: renaming
>> '.git/objects/pack/.tmp-7612-pack-ad9d069e7c27ce19cc5b6cde82b4a38bedbf5884.pack'
>> failed: Permission denied
>> error: failed to run repack
>>
>> After running git gc twice, this is what is left in .git/objects/pack/:
>>
>> U:\GitEnv\.git\objects\pack>ls -al
>> total 53676
>> drwxr-xr-x    1 hgj2fe   Administ        0 Jan 16 10:43 .
>> drwxr-xr-x    1 hgj2fe   Administ        0 Jan 14 12:52 ..
>> -r--r--r--    1 hgj2fe   Administ     4964 Jan 16 10:43
>> .tmp-7612-pack-ad9d069e7c27ce19cc5b6cde82b4a38bedbf5884.idx
>> -r--r--r--    1 hgj2fe   Administ 18315618 Jan 16 10:43
>> .tmp-7612-pack-ad9d069e7c27ce19cc5b6cde82b4a38bedbf5884.pack
>> -r--r--r--    1 hgj2fe   Administ     4964 Jan 16 10:40
>> .tmp-7920-pack-ad9d069e7c27ce19cc5b6cde82b4a38bedbf5884.idx
>> -r--r--r--    1 hgj2fe   Administ 18315618 Jan 16 10:40
>> .tmp-7920-pack-ad9d069e7c27ce19cc5b6cde82b4a38bedbf5884.pack
>> -r--r--r--    1 hgj2fe   Administ     4964 Jan 14 12:52
>> pack-ad9d069e7c27ce19cc5b6cde82b4a38bedbf5884.idx
>> -r--r--r--    1 hgj2fe   Administ 18315618 Jan 14 12:52
>> pack-ad9d069e7c27ce19cc5b6cde82b4a38bedbf5884.pack
>>
>> In case we remove the read-only flag of the pack and index manually
>> before running git gc again, no problem occurs:
>>
>> U:\GitEnv\.git\objects\pack>git gc
>> trace: built-in: git 'gc'
>> trace: run_command: 'pack-refs' '--all' '--prune'
>> trace: built-in: git 'pack-refs' '--all' '--prune'
>> trace: run_command: 'reflog' 'expire' '--all'
>> trace: built-in: git 'reflog' 'expire' '--all'
>> trace: run_command: 'repack' '-d' '-l' '-A'
>> '--unpack-unreachable=2.weeks.ago'
>> trace: built-in: git 'repack' '-d' '-l' '-A'
>> '--unpack-unreachable=2.weeks.ago'
>> trace: run_command: 'pack-objects' '--keep-true-parents'
>> '--honor-pack-keep' '--non-empty' '--all' '--reflog'
>> '--unpack-unreachable=2.weeks.ago' '--local' '--delta-base-offset'
>> 'U:/GitEnv/.git/objects/pack/.tmp-3736-pack'
>> trace: built-in: git 'pack-objects' '--keep-true-parents'
>> '--honor-pack-keep' '--non-empty' '--all' '--reflog'
>> '--unpack-unreachable=2.weeks.ago' '--local' '--delta-base-offset'
>> 'U:/GitEnv/.git/objects/pack/.tmp-3736-pack'
>> Counting objects: 139, done.
>> Delta compression using up to 8 threads.
>> Compressing objects: 100% (71/71), done.
>> Writing objects: 100% (139/139), done.
>> Total 139 (delta 65), reused 139 (delta 65)
>> trace: run_command: 'prune-packed'
>> trace: built-in: git 'prune-packed'
>> trace: run_command: 'update-server-info'
>> trace: built-in: git 'update-server-info'
>> trace: run_command: 'prune' '--expire' '2.weeks.ago'
>> trace: built-in: git 'prune' '--expire' '2.weeks.ago'
>> trace: run_command: 'rerere' 'gc'
>> trace: built-in: git 'rerere' 'gc'
>>
>> The problem might be related to this commit:
>> https://github.com/msysgit/git/commit/a1bbc6c0176f1fa2d4aa571cc0183a1f0ff9b285
>>
>>
>> Best regards,
>>
>> Jochen
> ------
>>
>>
>> Am Freitag, 17. Januar 2014 19:02:07 UTC+1 schrieb Torsten Bögershausen:
>>
>>
>>     So, please, what happens if you revert that commit?
>>     It could be good if you can test it and share the results with the list
>>     /Torsten
>>
>>
>> Instead of reverting we did some more analysis.
>>
>> In repack.c we found the following code:
>>
>>         /* Now the ones with the same name are out of the way... */
>>         for_each_string_list_item(item, &names) {
>>                 for (ext = 0; ext < 2; ext++) {
>>                         char *fname, *fname_old;
>>                         struct stat statbuffer;
>>                         fname = mkpathdup("%s/pack-%s%s",
>>                                         packdir, item->string, exts[ext]);
>>                         fname_old = mkpathdup("%s-%s%s",
>>                                         packtmp, item->string, exts[ext]);
>>                         if (!stat(fname_old, &statbuffer)) {
>>                                 statbuffer.st_mode &= ~(S_IWUSR | S_IWGRP | S_IWOTH);
>>                                 chmod(fname_old, statbuffer.st_mode);
>>                         }
>>                         if (rename(fname_old, fname))
>>                                 die_errno(_("renaming '%s' failed"), fname_old);
>>                         free(fname);
>>                         free(fname_old);
>>                 }
>>         }
>>
>> The rename command replaces a mv -f command of the original shell script. Obviously the -f option can also override a read-only file but rename can not on a network share.
>>
>> We changed the code as followed:
>>
>>         /* Now the ones with the same name are out of the way... */
>>         for_each_string_list_item(item, &names) {
>>                 for (ext = 0; ext < 2; ext++) {
>>                         char *fname, *fname_old;
>>                         struct stat statbuffer;
>>                         fname = mkpathdup("%s/pack-%s%s",
>>                                         packdir, item->string, exts[ext]);
>>                         fname_old = mkpathdup("%s-%s%s",
>>                                         packtmp, item->string, exts[ext]);
>>                         if (!stat(fname_old, &statbuffer)) {
>>                                 statbuffer.st_mode &= ~(S_IWUSR | S_IWGRP | S_IWOTH);
>>                                 chmod(fname_old, statbuffer.st_mode);
>>                         }
>> +++                         if (!stat(fname, &statbuffer)) {
>> +++                                 statbuffer.st_mode |= (S_IWUSR | S_IWGRP | S_IWOTH);
>> +++                                 chmod(fname, statbuffer.st_mode);
>> +++                         }
>>                         if (rename(fname_old, fname))
>>                                 die_errno(_("renaming '%s' failed"), fname_old);
>>                         free(fname);
>>                         free(fname_old);
>>                 }
>>         }
>>
>> Before rename is called the destination file is made writeable. This worked for us. We are not sure if this is a good implementation.
> 
> Jochen,
> thanks for sharing the code changes with us.
> 
> I allowed me to 
> a) reconstruct the original mail,
> b) add "+++" at the places where you added the stat() and chmod(),
> c) and to send the question "is this a good implementation ?" to upstream git.
> 
> I think your implementation makes sense.
> /Torsten
>  
> 

I'm sorry for breaking repack there. The implementation sounds
reasonably to me.

Thanks for reporting.
Do you want to prepare an upstream patch?

Thanks,
Stefan

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [msysGit] Consecutive git gc fails on Windows network share

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1544 bytes --]

Hi Torsten,

On Mon, 20 Jan 2014, Torsten Bögershausen wrote:

> (No top-posting, please see my comments below)

already very good! For extra brownie points, just edit the quoted part to
reflect the abridged version needed to understand the context.

> On 2014-01-20 15.02, Jochen wrote:
> >On 01/16/2014 10:55 AM, Jochen Haag wrote:
> > The rename command replaces a mv -f command of the original shell script. Obviously the -f option can also override a read-only file but rename can not on a network share.
> 
> I allowed me to 
> a) reconstruct the original mail,

Please note that together with an exceedingly flakey internet connection,
not only having to scroll through the mail (most of which was actually not
relevant to your reply) and having to delete most parts again ate up my
complete Git time budget for today. Just something you might want to keep
in mind.

> b) add "+++" at the places where you added the stat() and chmod(),
> c) and to send the question "is this a good implementation ?" to upstream git.
>
> I think your implementation makes sense.

As I said in my other reply, I think that the problem would be addressed
more generally in compat/mingw.c. It is to be doubted highly that upstream
wants to handle cases such as "rename() cannot overwrite read-only files
on Windows" everywhere they call rename() because the platforms upstream
cares about do not have that problem.

So we probably need just the same _wchmod we have in mingw_unlink also in
mingw_rename.

Ciao,
Johannes

Re: Consecutive git gc fails on Windows network share

[Erik Faye-Lund]

On Tue, Jan 21, 2014 at 12:25 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> On Mon, 20 Jan 2014, Torsten Bögershausen wrote:
>
>> b) add "+++" at the places where you added the stat() and chmod(),
>> c) and to send the question "is this a good implementation ?" to upstream git.
>>
>> I think your implementation makes sense.
>
> As I said in my other reply, I think that the problem would be addressed
> more generally in compat/mingw.c. It is to be doubted highly that upstream
> wants to handle cases such as "rename() cannot overwrite read-only files
> on Windows" everywhere they call rename() because the platforms upstream
> cares about do not have that problem.

I'm not so sure. A quick test shows me that this is not the case for
NTFS. Since this is over a network-share, the problem is probably
server-side, and can affect other systems as well.

So a work-around might be appropriate for all systems, no?

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Consecutive git gc fails on Windows network share

[Johannes Schindelin]

Hi kusma,

On Tue, 21 Jan 2014, Erik Faye-Lund wrote:

> On Tue, Jan 21, 2014 at 12:25 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > On Mon, 20 Jan 2014, Torsten Bögershausen wrote:
> >
> >> b) add "+++" at the places where you added the stat() and chmod(),
> >> c) and to send the question "is this a good implementation ?" to upstream git.
> >>
> >> I think your implementation makes sense.
> >
> > As I said in my other reply, I think that the problem would be
> > addressed more generally in compat/mingw.c. It is to be doubted highly
> > that upstream wants to handle cases such as "rename() cannot overwrite
> > read-only files on Windows" everywhere they call rename() because the
> > platforms upstream cares about do not have that problem.
> 
> I'm not so sure. A quick test shows me that this is not the case for
> NTFS. Since this is over a network-share, the problem is probably
> server-side, and can affect other systems as well.
> 
> So a work-around might be appropriate for all systems, no?

I do not think that the problem occurs if you run the same commands on
Linux, on a mounted Samba share. So I guess that upstream Git can enjoy
their luxury of not having to care.

In any case, if we would need this also for Linux, doing it for only one
user of rename() would probably not be good enough, either... so something
similar to mingw_rename() would be needed (interfering with mingw_rename
itself, of course).

Ciao,
Dscho

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Consecutive git gc fails on Windows network share

[Erik Faye-Lund]

On Tue, Jan 21, 2014 at 5:57 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi kusma,
>
> On Tue, 21 Jan 2014, Erik Faye-Lund wrote:
>
>> On Tue, Jan 21, 2014 at 12:25 AM, Johannes Schindelin
>> <Johannes.Schindelin@gmx.de> wrote:
>> >
>> > On Mon, 20 Jan 2014, Torsten Bögershausen wrote:
>> >
>> >> b) add "+++" at the places where you added the stat() and chmod(),
>> >> c) and to send the question "is this a good implementation ?" to upstream git.
>> >>
>> >> I think your implementation makes sense.
>> >
>> > As I said in my other reply, I think that the problem would be
>> > addressed more generally in compat/mingw.c. It is to be doubted highly
>> > that upstream wants to handle cases such as "rename() cannot overwrite
>> > read-only files on Windows" everywhere they call rename() because the
>> > platforms upstream cares about do not have that problem.
>>
>> I'm not so sure. A quick test shows me that this is not the case for
>> NTFS. Since this is over a network-share, the problem is probably
>> server-side, and can affect other systems as well.
>>
>> So a work-around might be appropriate for all systems, no?
>
> I do not think that the problem occurs if you run the same commands on
> Linux, on a mounted Samba share. So I guess that upstream Git can enjoy
> their luxury of not having to care.
>
> In any case, if we would need this also for Linux, doing it for only one
> user of rename() would probably not be good enough, either... so something
> similar to mingw_rename() would be needed (interfering with mingw_rename
> itself, of course).
>

Indeed. I was thinking of something along the lines of adding a
xrename in wrapper.c.

But you're probably right that this doesn't happen under Samba; surely
Samba would have added a work-around for such a filesystem by now. So
yeah, you've convinced me. mingw_rename is probably the place to do
that.

The work-around would probably look something like this:

diff --git a/compat/mingw.c b/compat/mingw.c
index a37bbf3..580b820 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1697,6 +1697,14 @@ int mingw_rename(const char *pold, const char *pnew)
      */
     if (!_wrename(wpold, wpnew))
         return 0;
+
+    if (errno == EPERM) {
+        /* read-only files cannot be moved over on network shares */
+        _wchmod(wpnew, 0666);
+        if (!_wrename(wpold, wpnew))
+            return 0;
+    }
+
     if (errno != EEXIST)
         return -1;
 repeat:

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.