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 -- 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