* Git force push fails after a rejected push (unpack failed)? @ 2015-07-07 13:45 X H 2015-07-07 14:13 ` Jeff King 0 siblings, 1 reply; 17+ messages in thread From: X H @ 2015-07-07 13:45 UTC (permalink / raw) To: git@vger.kernel.org Hi, I have a remote repository on a network share configured as a file remote. I have pushed the HEAD of a branch to the remote, then amend the local HEAD. I then try to push the HEAD again but it was rejected. I made multiple attempts and then try to force the push with: $ git push -f origin branch_name but it failed with message: remote: error: unable to write sha1 filename objects/d9/4bfb39cd0be7497e493bd4045111a7b1158134: Permission denied remote: fatal: failed to write object error: unpack failed: unpack-objects abnormal exit To file://xxx ! [remote rejected] branch_name -> branch_name (unpacker error) error: failed to push some refs to 'file://xxx' Sometimes the error is due to a commit sha, sometimes due to a tree sha. It seems that after a rejected push some files are not deleted and since they are read only on the remote, the next push cannot overwrite them. Is it the intended behaviour? I try to reproduce this and it seems it doesn't happen always. git v2.3.5 (git-for-windows) Thank you. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Git force push fails after a rejected push (unpack failed)? 2015-07-07 13:45 Git force push fails after a rejected push (unpack failed)? X H @ 2015-07-07 14:13 ` Jeff King [not found] ` <DUB120-W36B78FEE6DC80BDCB05D7FF6920@phx.gbl> 0 siblings, 1 reply; 17+ messages in thread From: Jeff King @ 2015-07-07 14:13 UTC (permalink / raw) To: X H; +Cc: git@vger.kernel.org On Tue, Jul 07, 2015 at 03:45:50PM +0200, X H wrote: > remote: error: unable to write sha1 filename objects/d9/4bfb39cd0be7497e493bd4045111a7b1158134: Permission denied > remote: fatal: failed to write object > error: unpack failed: unpack-objects abnormal exit It looks like the permissions of one or more of the .git/objects/* directories on the server is not right. Just a guess, but this may be because you have multiple accounts pushing to it, and need to set umasks or use git's core.sharedrepository config to set the permissions for each object. You'll probably have to ssh to the server and "chown" or "chmod" some directories to fix it in the first place, though. > Is it the intended behaviour? I try to reproduce this and it seems it > doesn't happen always. Probably only some object directories have the problem, so it would depend on the set of exact objects you are sending, and possibly even the number of objects (if you send a large number of objects, git will store them as a pack, not as loose objects). -Peff ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <DUB120-W36B78FEE6DC80BDCB05D7FF6920@phx.gbl>]
* Re: Git force push fails after a rejected push (unpack failed)? [not found] ` <DUB120-W36B78FEE6DC80BDCB05D7FF6920@phx.gbl> @ 2015-07-07 19:49 ` Jeff King 2015-07-07 23:05 ` Eric Sunshine 2015-07-08 17:41 ` Johannes Sixt 0 siblings, 2 replies; 17+ messages in thread From: Jeff King @ 2015-07-07 19:49 UTC (permalink / raw) To: X H; +Cc: git@vger.kernel.org On Tue, Jul 07, 2015 at 09:31:25PM +0200, X H wrote: > For the moment, I'm the only one pushing to the remote, always with > the same user (second user is planned). I use git-for-windows which is > based on MSYS2. I have mounted the network share with noacl option so > permissions should be handled by the Windows share. I'm in a group > which has read/write access. I have not configured > core.sharedrepository, I don't think it is useful with noacl since > unix group are not used in this case. The permission for the folder > above the file with permission denied is rw, but this file is read > only so if git try to modify it it won't work. Ah, so this is not a push to a server, but to a share mounted on the local box? That is leaving my realm of expertise. I'm not sure if it could be a misconfiguration in your share setup, or that git is trying to do something that would work on a Unix machine, but not on a Windows share. You might want to ask on the msysgit list: https://groups.google.com/forum/#!forum/msysgit > Why does git try to write a file with the same name? If I amend a > commit isn't the sha modified? Yes, but remember that git stores all of the objects for all of the commits. So for some reason your push is perhaps trying to send an object that the other side already has. Usually this does not happen (the receiver says "I already have these commits, do not bother sending their objects"), but it's possible that you have an object that is not referenced by any commit, or a similar situation. It's hard to say without looking at the repository. -Peff ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Git force push fails after a rejected push (unpack failed)? 2015-07-07 19:49 ` Jeff King @ 2015-07-07 23:05 ` Eric Sunshine 2015-07-08 17:41 ` Johannes Sixt 1 sibling, 0 replies; 17+ messages in thread From: Eric Sunshine @ 2015-07-07 23:05 UTC (permalink / raw) To: Jeff King; +Cc: X H, git@vger.kernel.org On Tue, Jul 7, 2015 at 3:49 PM, Jeff King <peff@peff.net> wrote: > On Tue, Jul 07, 2015 at 09:31:25PM +0200, X H wrote: > >> For the moment, I'm the only one pushing to the remote, always with >> the same user (second user is planned). I use git-for-windows which is >> based on MSYS2. I have mounted the network share with noacl option so >> permissions should be handled by the Windows share. I'm in a group >> which has read/write access. I have not configured >> core.sharedrepository, I don't think it is useful with noacl since >> unix group are not used in this case. The permission for the folder >> above the file with permission denied is rw, but this file is read >> only so if git try to modify it it won't work. > > Ah, so this is not a push to a server, but to a share mounted on the > local box? > > That is leaving my realm of expertise. I'm not sure if it could be a > misconfiguration in your share setup, or that git is trying to do > something that would work on a Unix machine, but not on a Windows share. > You might want to ask on the msysgit list: > > https://groups.google.com/forum/#!forum/msysgit Is this possibly another case of Windows virus scanner interference? That could account for its variable nature. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Git force push fails after a rejected push (unpack failed)? 2015-07-07 19:49 ` Jeff King 2015-07-07 23:05 ` Eric Sunshine @ 2015-07-08 17:41 ` Johannes Sixt 2015-07-08 18:05 ` Jeff King 1 sibling, 1 reply; 17+ messages in thread From: Johannes Sixt @ 2015-07-08 17:41 UTC (permalink / raw) To: Jeff King, X H; +Cc: git@vger.kernel.org Am 07.07.2015 um 21:49 schrieb Jeff King: > On Tue, Jul 07, 2015 at 09:31:25PM +0200, X H wrote: > >> For the moment, I'm the only one pushing to the remote, always with >> the same user (second user is planned). I use git-for-windows which is >> based on MSYS2. I have mounted the network share with noacl option so >> permissions should be handled by the Windows share. I'm in a group >> which has read/write access. I have not configured >> core.sharedrepository, I don't think it is useful with noacl since >> unix group are not used in this case. The permission for the folder >> above the file with permission denied is rw, but this file is read >> only so if git try to modify it it won't work. > > Ah, so this is not a push to a server, but to a share mounted on the > local box? > > That is leaving my realm of expertise. I'm not sure if it could be a > misconfiguration in your share setup, or that git is trying to do > something that would work on a Unix machine, but not on a Windows share. > You might want to ask on the msysgit list: > > https://groups.google.com/forum/#!forum/msysgit > >> Why does git try to write a file with the same name? If I amend a >> commit isn't the sha modified? > > Yes, but remember that git stores all of the objects for all of the > commits. So for some reason your push is perhaps trying to send an > object that the other side already has. Usually this does not happen > (the receiver says "I already have these commits, do not bother sending > their objects"), but it's possible that you have an object that is not > referenced by any commit, or a similar situation. It's hard to say > without looking at the repository. After a non-fast-forward push fails, a subsequent forced push sends the same set of objects, which are already present at the server side, but are dangling objects. Apparently, Git for Windows fails to replace the read-only files that live on the network file system. I have observed this recently, as well. I haven't dug into the detailed failure mode, yet. In my case, I have a daily repack running on the server side (it's a Samba share on a Linux box), that garbage-collects the dangling objects. Usually, the next day the forced push is successful. I know this is not very helpful if you can't wait a day for the next push attempt... -- Hannes ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Git force push fails after a rejected push (unpack failed)? 2015-07-08 17:41 ` Johannes Sixt @ 2015-07-08 18:05 ` Jeff King 2015-07-08 18:33 ` [PATCH] check_and_freshen_file: fix reversed success-check Jeff King ` (2 more replies) 0 siblings, 3 replies; 17+ messages in thread From: Jeff King @ 2015-07-08 18:05 UTC (permalink / raw) To: Johannes Sixt; +Cc: X H, git@vger.kernel.org On Wed, Jul 08, 2015 at 07:41:48PM +0200, Johannes Sixt wrote: > >Yes, but remember that git stores all of the objects for all of the > >commits. So for some reason your push is perhaps trying to send an > >object that the other side already has. Usually this does not happen > >(the receiver says "I already have these commits, do not bother sending > >their objects"), but it's possible that you have an object that is not > >referenced by any commit, or a similar situation. It's hard to say > >without looking at the repository. > > After a non-fast-forward push fails, a subsequent forced push sends the same > set of objects, which are already present at the server side, but are > dangling objects. > > Apparently, Git for Windows fails to replace the read-only files that live > on the network file system. I left one bit out from my original explanation, which is that we generally prefer existing objects to new ones. So we would generally want to throw out the new object rather than try to write it out. I'm not sure why unpack-objects would try to write an object we already have. We also don't write objects directly, of course; we write to a temporary file and try to link them into place. It really sounds more like the "objects/d9" directory is where the permission problems are. But, hmm... The code path should be unpack-objects.c:write_object, which calls sha1_file.cwrite_sha1_file, which then checks has_sha1_file(). These days it uses the freshen_* functions instead of the latter, which does a similar check. But it does report failure if we cannot call utime() on the file, preferring to write it out instead (this is the safer choice from a preventing-prune-corruption perspective). So it's possible that the sequence is: - unpack-objects tries to write object 1234abcd... - write_sha1_file calls freshen_loose_object - we call access("12/34abcd...", F_OK) and see that it does indeed exist - we call utime("12/34abcd...") which fails (presumably due to EPERM); we return failure and assume we must write out the object - write_sha1_file then writes to a temporary file, and tries to link it into place. Now what? If we get EEXIST, we say "OK, somebody else beat us here", and we consider that a success. But presumably we get some other error here (which may even be a Windows-ism), fall back to rename(), and that fails with EPERM, which we then report. If that's the case, then one solution is to have the timestamp-freshening code silently report success, and skip writing out the object. I'm not entirely comfortable with that, just because it is loosening a safety mechanism. But perhaps we could loosen it _only_ in the case of checking the loose object, and when we get EPERM. We know that the next step is going to be writing out that same loose object, which is almost certainly going to fail for the same reason. I dunno. The whole concept of trying to write to an object database for which you do not have permissions seems a little bit weird. This would definitely be a workaround. But I suspect it did work prior to v2.2.0. -Peff ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] check_and_freshen_file: fix reversed success-check 2015-07-08 18:05 ` Jeff King @ 2015-07-08 18:33 ` Jeff King 2015-07-08 19:24 ` Junio C Hamano 2015-07-08 21:03 ` [PATCH] " Johannes Sixt 2015-07-08 20:28 ` Git force push fails after a rejected push (unpack failed)? X H 2015-07-08 20:56 ` Johannes Sixt 2 siblings, 2 replies; 17+ messages in thread From: Jeff King @ 2015-07-08 18:33 UTC (permalink / raw) To: Johannes Sixt; +Cc: Junio C Hamano, René Scharfe, X H, git@vger.kernel.org On Wed, Jul 08, 2015 at 02:05:39PM -0400, Jeff King wrote: > The code path should be unpack-objects.c:write_object, which calls > sha1_file.cwrite_sha1_file, which then checks has_sha1_file(). These > days it uses the freshen_* functions instead of the latter, which does a > similar check. But it does report failure if we cannot call utime() on > the file, preferring to write it out instead (this is the safer choice > from a preventing-prune-corruption perspective). > > So it's possible that the sequence is: > > - unpack-objects tries to write object 1234abcd... > > - write_sha1_file calls freshen_loose_object > > - we call access("12/34abcd...", F_OK) and see that it does indeed > exist > > - we call utime("12/34abcd...") which fails (presumably due to EPERM); > we return failure and assume we must write out the object ...or maybe in the utime() step there is actually a bug, and we report failure for no good reason. Ugh. -- >8 -- Subject: check_and_freshen_file: fix reversed success-check When we want to write out a loose object file, we have always first made sure we don't already have the object somewhere. Since 33d4221 (write_sha1_file: freshen existing objects, 2014-10-15), we also update the timestamp on the file, so that a simultaneous prune knows somebody is likely to reference it soon. If our utime() call fails, we treat this the same as not having the object in the first place; the safe thing to do is write out another copy. However, the loose-object check accidentally inverst the utime() check; it returns failure _only_ when the utime() call actually succeeded. Thus it was failing to protect us there, and in the normal case where utime() succeeds, it caused us to pointlessly write out and link the object. This passed our freshening tests, because writing out the new object is certainly _one_ way of updating its utime. So the normal case of a successful utime() was inefficient, but not wrong. Signed-off-by: Jeff King <peff@peff.net> --- The worst part of this is that I had the _same_ bug in the pack code path when I initially posted what became 33d4221. René noticed during review, and my fix was to invert the return value from freshen_file to match the other functions. But of course doing that without fixing the other caller meant I introduced the same bug there. I'll be curious if this fixes the problem the OP is seeing. If not, then we can dig deeper into the weird EPERM problems around this particular object database. sha1_file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sha1_file.c b/sha1_file.c index 77cd81d..721eadc 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -454,7 +454,7 @@ static int check_and_freshen_file(const char *fn, int freshen) { if (access(fn, F_OK)) return 0; - if (freshen && freshen_file(fn)) + if (freshen && !freshen_file(fn)) return 0; return 1; } -- 2.5.0.rc1.340.ge59e3eb ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] check_and_freshen_file: fix reversed success-check 2015-07-08 18:33 ` [PATCH] check_and_freshen_file: fix reversed success-check Jeff King @ 2015-07-08 19:24 ` Junio C Hamano 2015-07-08 20:33 ` [PATCH v2] " Jeff King 2015-07-08 21:03 ` [PATCH] " Johannes Sixt 1 sibling, 1 reply; 17+ messages in thread From: Junio C Hamano @ 2015-07-08 19:24 UTC (permalink / raw) To: Jeff King; +Cc: Johannes Sixt, René Scharfe, X H, git@vger.kernel.org Jeff King <peff@peff.net> writes: > Subject: check_and_freshen_file: fix reversed success-check > > When we want to write out a loose object file, we have > always first made sure we don't already have the object > somewhere. Since 33d4221 (write_sha1_file: freshen existing > objects, 2014-10-15), we also update the timestamp on the > file, so that a simultaneous prune knows somebody is > likely to reference it soon. > > If our utime() call fails, we treat this the same as not > having the object in the first place; the safe thing to do > is write out another copy. However, the loose-object check > accidentally inverst the utime() check; it returns failure s/inverst/invert/? > _only_ when the utime() call actually succeeded. Thus it was > failing to protect us there, and in the normal case where > utime() succeeds, it caused us to pointlessly write out and > link the object. > > This passed our freshening tests, because writing out the > new object is certainly _one_ way of updating its utime. So > the normal case of a successful utime() was inefficient, but > not wrong. > > Signed-off-by: Jeff King <peff@peff.net> > --- > The worst part of this is that I had the _same_ bug in the pack code > path when I initially posted what became 33d4221. René noticed during > review, and my fix was to invert the return value from freshen_file to > match the other functions. But of course doing that without fixing the > other caller meant I introduced the same bug there. I think each of the functions in the check_and_freshen_* callchain can at least have a comment in front of it, saying what the returned value means, to unconfuse readers. "Return 1 when the thing exists and no further action is necessary; return 0 when the thing does not exist or not in a good state and should be overwritten (if the caller has something to overwrite it with, that is)" or something? Their returning "1" instead of "-1" could be taken as a hint that says "this non-zero return does not signal a _failure_", but it is a rather weak hint. > > I'll be curious if this fixes the problem the OP is seeing. If not, then > we can dig deeper into the weird EPERM problems around this particular > object database. > > sha1_file.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/sha1_file.c b/sha1_file.c > index 77cd81d..721eadc 100644 > --- a/sha1_file.c > +++ b/sha1_file.c > @@ -454,7 +454,7 @@ static int check_and_freshen_file(const char *fn, int freshen) > { > if (access(fn, F_OK)) > return 0; > - if (freshen && freshen_file(fn)) > + if (freshen && !freshen_file(fn)) > return 0; > return 1; > } ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2] check_and_freshen_file: fix reversed success-check 2015-07-08 19:24 ` Junio C Hamano @ 2015-07-08 20:33 ` Jeff King 0 siblings, 0 replies; 17+ messages in thread From: Jeff King @ 2015-07-08 20:33 UTC (permalink / raw) To: Junio C Hamano; +Cc: Johannes Sixt, René Scharfe, X H, git@vger.kernel.org On Wed, Jul 08, 2015 at 12:24:41PM -0700, Junio C Hamano wrote: > > If our utime() call fails, we treat this the same as not > > having the object in the first place; the safe thing to do > > is write out another copy. However, the loose-object check > > accidentally inverst the utime() check; it returns failure > > s/inverst/invert/? Whoops, should be "inverts". > I think each of the functions in the check_and_freshen_* callchain > can at least have a comment in front of it, saying what the returned > value means, to unconfuse readers. "Return 1 when the thing exists > and no further action is necessary; return 0 when the thing does not > exist or not in a good state and should be overwritten (if the > caller has something to overwrite it with, that is)" or something? > > Their returning "1" instead of "-1" could be taken as a hint that > says "this non-zero return does not signal a _failure_", but it is a > rather weak hint. Fair enough. Here's a re-roll with some extra comments, and the typo-fix from above. -- >8 -- Subject: check_and_freshen_file: fix reversed success-check When we want to write out a loose object file, we have always first made sure we don't already have the object somewhere. Since 33d4221 (write_sha1_file: freshen existing objects, 2014-10-15), we also update the timestamp on the file, so that a simultaneous prune knows somebody is likely to reference it soon. If our utime() call fails, we treat this the same as not having the object in the first place; the safe thing to do is write out another copy. However, the loose-object check accidentally inverts the utime() check; it returns failure _only_ when the utime() call actually succeeded. Thus it was failing to protect us there, and in the normal case where utime() succeeds, it caused us to pointlessly write out and link the object. This passed our freshening tests, because writing out the new object is certainly _one_ way of updating its utime. So the normal case was inefficient, but not wrong. While we're here, let's also drop a comment in front of the check_and_freshen functions, making a note of their return type (since it is not our usual "0 for success, -1 for error"). Signed-off-by: Jeff King <peff@peff.net> --- sha1_file.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/sha1_file.c b/sha1_file.c index 77cd81d..e435289 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -443,6 +443,7 @@ void prepare_alt_odb(void) read_info_alternates(get_object_directory(), 0); } +/* Returns 1 if we have successfully freshened the file, 0 otherwise. */ static int freshen_file(const char *fn) { struct utimbuf t; @@ -450,11 +451,18 @@ static int freshen_file(const char *fn) return !utime(fn, &t); } +/* + * All of the check_and_freshen functions return 1 if the file exists and was + * freshened (if freshening was requested), 0 otherwise. If they return + * 0, you should not assume that it is safe to skip a write of the object (it + * either does not exist on disk, or has a stale mtime and may be subject to + * pruning). + */ static int check_and_freshen_file(const char *fn, int freshen) { if (access(fn, F_OK)) return 0; - if (freshen && freshen_file(fn)) + if (freshen && !freshen_file(fn)) return 0; return 1; } -- 2.5.0.rc1.340.ge59e3eb ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] check_and_freshen_file: fix reversed success-check 2015-07-08 18:33 ` [PATCH] check_and_freshen_file: fix reversed success-check Jeff King 2015-07-08 19:24 ` Junio C Hamano @ 2015-07-08 21:03 ` Johannes Sixt 2015-07-09 20:51 ` Johannes Sixt 1 sibling, 1 reply; 17+ messages in thread From: Johannes Sixt @ 2015-07-08 21:03 UTC (permalink / raw) To: Jeff King; +Cc: Junio C Hamano, René Scharfe, X H, git@vger.kernel.org Am 08.07.2015 um 20:33 schrieb Jeff King: > ...or maybe in the utime() step there is actually a bug, and we report > failure for no good reason. Ugh. Ah! That code is less than a year old. When I began to adopt a workflow requiring force-pushes lately, I wondered why I haven't seen these failures earlier, because I did do force pushes in the past, but not that frequently. I thought that I had just been lucky. But this would explain it. -- Hannes ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] check_and_freshen_file: fix reversed success-check 2015-07-08 21:03 ` [PATCH] " Johannes Sixt @ 2015-07-09 20:51 ` Johannes Sixt 2015-07-09 22:48 ` Jeff King 0 siblings, 1 reply; 17+ messages in thread From: Johannes Sixt @ 2015-07-09 20:51 UTC (permalink / raw) To: Jeff King; +Cc: Junio C Hamano, René Scharfe, X H, git@vger.kernel.org Am 08.07.2015 um 23:03 schrieb Johannes Sixt: > Am 08.07.2015 um 20:33 schrieb Jeff King: >> ...or maybe in the utime() step there is actually a bug, and we report >> failure for no good reason. Ugh. > > Ah! That code is less than a year old. When I began to adopt a workflow > requiring force-pushes lately, I wondered why I haven't seen these > failures earlier, because I did do force pushes in the past, but not > that frequently. I thought that I had just been lucky. But this would > explain it. And, in fact, with this patch these particular failures are gone! Thank you so much! -- Hannes ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] check_and_freshen_file: fix reversed success-check 2015-07-09 20:51 ` Johannes Sixt @ 2015-07-09 22:48 ` Jeff King 2015-07-11 22:21 ` X H 0 siblings, 1 reply; 17+ messages in thread From: Jeff King @ 2015-07-09 22:48 UTC (permalink / raw) To: Johannes Sixt; +Cc: Junio C Hamano, René Scharfe, X H, git@vger.kernel.org On Thu, Jul 09, 2015 at 10:51:50PM +0200, Johannes Sixt wrote: > >Ah! That code is less than a year old. When I began to adopt a workflow > >requiring force-pushes lately, I wondered why I haven't seen these > >failures earlier, because I did do force pushes in the past, but not > >that frequently. I thought that I had just been lucky. But this would > >explain it. > > And, in fact, with this patch these particular failures are gone! Thank you > so much! Great, thanks for testing. You can temper your appreciation by noticing that I introduced the bug in the first place. ;) -Peff ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] check_and_freshen_file: fix reversed success-check 2015-07-09 22:48 ` Jeff King @ 2015-07-11 22:21 ` X H 2015-07-13 3:52 ` Jeff King 0 siblings, 1 reply; 17+ messages in thread From: X H @ 2015-07-11 22:21 UTC (permalink / raw) To: Jeff King, Johannes Sixt Cc: Junio C Hamano, René Scharfe, git@vger.kernel.org Le 10/07/2015 0:48, Jeff King a écrit : > On Thu, Jul 09, 2015 at 10:51:50PM +0200, Johannes Sixt wrote: > >>> Ah! That code is less than a year old. When I began to adopt a workflow >>> requiring force-pushes lately, I wondered why I haven't seen these >>> failures earlier, because I did do force pushes in the past, but not >>> that frequently. I thought that I had just been lucky. But this would >>> explain it. >> >> And, in fact, with this patch these particular failures are gone! Thank you >> so much! > > Great, thanks for testing. You can temper your appreciation by noticing > that I introduced the bug in the first place. ;) > > -Peff > Hi, Thank you for the patch. I hope it will solve the problem and permit to have a second user using the same repository. How are the permission handled, is it git that is asking to create a file read only or rw on the remote or is it the environment with umask ans so on that decides it, or Windows when the drive is mounted with noacl? ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] check_and_freshen_file: fix reversed success-check 2015-07-11 22:21 ` X H @ 2015-07-13 3:52 ` Jeff King 2015-07-13 19:58 ` X H 0 siblings, 1 reply; 17+ messages in thread From: Jeff King @ 2015-07-13 3:52 UTC (permalink / raw) To: X H; +Cc: Johannes Sixt, Junio C Hamano, René Scharfe, git@vger.kernel.org On Sun, Jul 12, 2015 at 12:21:33AM +0200, X H wrote: > How are the permission handled, is it git that is asking to create a file > read only or rw on the remote or is it the environment with umask ans so on > that decides it, or Windows when the drive is mounted with noacl? Generally, git follows the umask when creating most files. However, for the object files in the object database, it does drop the "w" bit, as once written, they should never be changed (after all, the filename is a hash of the contents). We don't ever open those files for writing, but we may try to rename another file over them; that might behave differently on Unix versus Windows (or even differently on Windows between local and remote-mounted filesystems). -Peff ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] check_and_freshen_file: fix reversed success-check 2015-07-13 3:52 ` Jeff King @ 2015-07-13 19:58 ` X H 0 siblings, 0 replies; 17+ messages in thread From: X H @ 2015-07-13 19:58 UTC (permalink / raw) To: Jeff King Cc: Johannes Sixt, Junio C Hamano, René Scharfe, git@vger.kernel.org Le 13/07/2015 5:52, Jeff King a écrit : > On Sun, Jul 12, 2015 at 12:21:33AM +0200, X H wrote: > >> How are the permission handled, is it git that is asking to create a file >> read only or rw on the remote or is it the environment with umask ans so on >> that decides it, or Windows when the drive is mounted with noacl? > > Generally, git follows the umask when creating most files. However, for > the object files in the object database, it does drop the "w" bit, as > once written, they should never be changed (after all, the filename is a > hash of the contents). We don't ever open those files for writing, but > we may try to rename another file over them; that might behave > differently on Unix versus Windows (or even differently on Windows > between local and remote-mounted filesystems). > > -Peff > Hi, When mounted on Linux the object files were created rw on the share, but when mounted on Windows the temporary files are created with read-only attributes. Thank you for your patch, forced push are now working from git-on-windows to smb folders shared from another Windows machine. I've not yet tested to push to the share with another user, but I think it should makes no diff. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Git force push fails after a rejected push (unpack failed)? 2015-07-08 18:05 ` Jeff King 2015-07-08 18:33 ` [PATCH] check_and_freshen_file: fix reversed success-check Jeff King @ 2015-07-08 20:28 ` X H 2015-07-08 20:56 ` Johannes Sixt 2 siblings, 0 replies; 17+ messages in thread From: X H @ 2015-07-08 20:28 UTC (permalink / raw) To: Jeff King, Johannes Sixt; +Cc: git@vger.kernel.org Le 8/07/2015 20:05, Jeff King a écrit : > On Wed, Jul 08, 2015 at 07:41:48PM +0200, Johannes Sixt wrote: > >>> Yes, but remember that git stores all of the objects for all of the >>> commits. So for some reason your push is perhaps trying to send an >>> object that the other side already has. Usually this does not happen >>> (the receiver says "I already have these commits, do not bother sending >>> their objects"), but it's possible that you have an object that is not >>> referenced by any commit, or a similar situation. It's hard to say >>> without looking at the repository. >> >> After a non-fast-forward push fails, a subsequent forced push sends the same >> set of objects, which are already present at the server side, but are >> dangling objects. >> >> Apparently, Git for Windows fails to replace the read-only files that live >> on the network file system. > > I left one bit out from my original explanation, which is that > we generally prefer existing objects to new ones. So we would generally > want to throw out the new object rather than try to write it out. I'm > not sure why unpack-objects would try to write an object we already > have. > > We also don't write objects directly, of course; we write to a temporary > file and try to link them into place. It really sounds more like the > "objects/d9" directory is where the permission problems are. But, hmm... > > The code path should be unpack-objects.c:write_object, which calls > sha1_file.cwrite_sha1_file, which then checks has_sha1_file(). These > days it uses the freshen_* functions instead of the latter, which does a > similar check. But it does report failure if we cannot call utime() on > the file, preferring to write it out instead (this is the safer choice > from a preventing-prune-corruption perspective). > > So it's possible that the sequence is: > > - unpack-objects tries to write object 1234abcd... > > - write_sha1_file calls freshen_loose_object > > - we call access("12/34abcd...", F_OK) and see that it does indeed > exist > > - we call utime("12/34abcd...") which fails (presumably due to EPERM); > we return failure and assume we must write out the object > > - write_sha1_file then writes to a temporary file, and tries to link > it into place. Now what? If we get EEXIST, we say "OK, somebody else > beat us here", and we consider that a success. But presumably we get > some other error here (which may even be a Windows-ism), fall back > to rename(), and that fails with EPERM, which we then report. > > If that's the case, then one solution is to have the > timestamp-freshening code silently report success, and skip writing out > the object. I'm not entirely comfortable with that, just because it is > loosening a safety mechanism. But perhaps we could loosen it _only_ in > the case of checking the loose object, and when we get EPERM. We know > that the next step is going to be writing out that same loose object, > which is almost certainly going to fail for the same reason. > > I dunno. The whole concept of trying to write to an object database for > which you do not have permissions seems a little bit weird. This would > definitely be a workaround. But I suspect it did work prior to v2.2.0. > > -Peff > I've made some tests: 1) Linux Mint: - ntfs share mounted (with noacl) since $ chacl -l /the/mountpoint gives chacl: cannot get access ACL on '/the/mountpoint': Operation not supported - Create a remote repository on the ntfs share - Push some test files - ls -al for files in the objects folder gives: -rw-r--r-- - with subsequent amend, push the permissions stays the same. 2) On Windows: - ntfs share mapped as drive by windows - remote repository created by git on windows - Push some test files - ls -al for files in the objects folder gives: -r--r--r-- - chmod all files to add w permission - then amend commit and push, push is successful but all files rewritten by the push are again: -r--r--r-- but all folders under objects are: drwxr-xr-x So I suppose it some issue with Windows or MSYS2. I'm not sure it's a problem related to your explanations, I will read it whit fresh mind. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Git force push fails after a rejected push (unpack failed)? 2015-07-08 18:05 ` Jeff King 2015-07-08 18:33 ` [PATCH] check_and_freshen_file: fix reversed success-check Jeff King 2015-07-08 20:28 ` Git force push fails after a rejected push (unpack failed)? X H @ 2015-07-08 20:56 ` Johannes Sixt 2 siblings, 0 replies; 17+ messages in thread From: Johannes Sixt @ 2015-07-08 20:56 UTC (permalink / raw) To: Jeff King; +Cc: X H, git@vger.kernel.org Am 08.07.2015 um 20:05 schrieb Jeff King: > We also don't write objects directly, of course; we write to a temporary > file and try to link them into place. It really sounds more like the > "objects/d9" directory is where the permission problems are. But, hmm... Not on Windows: A read-only file cannot be overwritten or removed, regardless of the permissions of the directory. We do treat this case mingw_rename, but I have a slight suspicion that this does not work sufficiently reliably on networked file systems. -- Hannes ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2015-07-13 19:59 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-07-07 13:45 Git force push fails after a rejected push (unpack failed)? X H 2015-07-07 14:13 ` Jeff King [not found] ` <DUB120-W36B78FEE6DC80BDCB05D7FF6920@phx.gbl> 2015-07-07 19:49 ` Jeff King 2015-07-07 23:05 ` Eric Sunshine 2015-07-08 17:41 ` Johannes Sixt 2015-07-08 18:05 ` Jeff King 2015-07-08 18:33 ` [PATCH] check_and_freshen_file: fix reversed success-check Jeff King 2015-07-08 19:24 ` Junio C Hamano 2015-07-08 20:33 ` [PATCH v2] " Jeff King 2015-07-08 21:03 ` [PATCH] " Johannes Sixt 2015-07-09 20:51 ` Johannes Sixt 2015-07-09 22:48 ` Jeff King 2015-07-11 22:21 ` X H 2015-07-13 3:52 ` Jeff King 2015-07-13 19:58 ` X H 2015-07-08 20:28 ` Git force push fails after a rejected push (unpack failed)? X H 2015-07-08 20:56 ` Johannes Sixt
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).