threads / discuss / 19102

error: Unable to append to .git/logs/refs/remotes/origin/master: Permission denied

Subject: error: Unable to append to .git/logs/refs/remotes/origin/master: Permission denied

## tl;dr

8 messages between Apr 28, 2009 and May 25, 2009.

replies: 7people: 3as markdown or json

Ingo Molnar· Apr 28, 2009, 07:31 UTC · lore

I had a portion of a repo owned by root accidentally, and the next time i pulled as user mingo Git gave me this warning and suggestion:

 aldebaran:~/git> git pull
 error: Unable to append to .git/logs/refs/remotes/origin/master: Permission denied
 From e2:git
  ! 66996ec..95110d7  master     -> origin/master  (unable to update local ref)
  * [new tag]         v1.6.3-rc2 -> v1.6.3-rc2
 error: some local refs could not be updated; try running
  'git remote prune origin' to remove any old, conflicting branches

Obviousy Git cannot update the ref there so the failure is OK, but the git-remote advice it gives is confusing IMHO: the 'git remote prune origin' cannot fix anything. (and it is clear from the fetch permission failure that there's no chance to fix anything here.)

I suspect there are other, more typical failure modes where that advice is useful - just wanted to point out that it's confusing here.

	Ingo
Jeff King· Apr 29, 2009, 03:29 UTC · re: Ingo Molnar · lore

Re: error: Unable to append to .git/logs/refs/remotes/origin/master: Permission denied

On Tue, Apr 28, 2009 at 09:31:38AM +0200, Ingo Molnar wrote:
Show 16 quoted lines
>  aldebaran:~/git> git pull
>  error: Unable to append to .git/logs/refs/remotes/origin/master: Permission denied
>  From e2:git
>   ! 66996ec..95110d7  master     -> origin/master  (unable to update local ref)
>   * [new tag]         v1.6.3-rc2 -> v1.6.3-rc2
>  error: some local refs could not be updated; try running
>   'git remote prune origin' to remove any old, conflicting branches
> 
> Obviousy Git cannot update the ref there so the failure is OK, but 
> the git-remote advice it gives is confusing IMHO: the 'git remote 
> prune origin' cannot fix anything. (and it is clear from the fetch 
> permission failure that there's no chance to fix anything here.)
> 
> I suspect there are other, more typical failure modes where that 
> advice is useful - just wanted to point out that it's confusing 
> here.
Yeah, I knew that when I wrote it:
  $ git log -1 --format=%s%n%b f3cb169
  fetch: give a hint to the user when local refs fail to update
  There are basically two categories of update failures for
  local refs:
    1. problems outside of git, like disk full, bad
       permissions, etc.
    2. D/F conflicts on tracking branch ref names
  In either case, there should already have been an error
  message. In case '1', hopefully enough information has
  already been given that the user can fix it. In the case of
  '2', we can hint that the user can clean up their tracking
  branch area by using 'git remote prune'.
  Note that we don't actually know _which_ case we have, so
  the user will receive the hint in case 1, as well. In this
  case the suggestion won't do any good, but hopefully the
  user is smart enough to figure out that it's just a hint.

Note the repeated use of "hopefully". :) Maybe the earlier message is too hidden to rely on. We might be able to get by with checking "errno" for ENOTDIR after trying to lock the ref and using a different message, but I don't know how portable that will be.

-Peff
Jeff King· Apr 29, 2009, 04:07 UTC · re: Jeff King · lore

Re: error: Unable to append to .git/logs/refs/remotes/origin/master: Permission denied

On Tue, Apr 28, 2009 at 11:29:43PM -0400, Jeff King wrote:
> Note the repeated use of "hopefully". :) Maybe the earlier message is
> too hidden to rely on. We might be able to get by with checking "errno"
> for ENOTDIR after trying to lock the ref and using a different message,
> but I don't know how portable that will be.

Hmm, that actually doesn't work. errno is properly EACCESS in your example, but the D/F problem doesn't actually set errno, since it is git itself, and not a failed syscall, that determines that "foo/bar" is not available because "foo" exists (and git must do it, because "foo" may be a packed ref).

So I think we would need to simulate the errno setting, like the patch below. That should generate the hint only when it would actually be useful.

---
diff --git a/builtin-fetch.c b/builtin-fetch.c
index 0bb290b..ad00bd2 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -181,9 +181,9 @@ static int s_update_ref(const char *action,
 	lock = lock_any_ref_for_update(ref->name,
 				       check_old ? ref->old_sha1 : NULL, 0);
 	if (!lock)
-		return 2;
+		return errno == ENOTDIR ? 2 : 1;
 	if (write_ref_sha1(lock, ref->new_sha1, msg) < 0)
-		return 2;
+		return errno == ENOTDIR ? 2 : 1;
 	return 0;
 }
 
diff --git a/refs.c b/refs.c
index e65a3b4..79795d0 100644
--- a/refs.c
+++ b/refs.c
@@ -893,8 +893,10 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char
 	 * name is a proper prefix of our refname.
 	 */
 	if (missing &&
-            !is_refname_available(ref, NULL, get_packed_refs(), 0))
+            !is_refname_available(ref, NULL, get_packed_refs(), 0)) {
+		last_errno = ENOTDIR;
 		goto error_return;
+	}
 
 	lock->lk = xcalloc(1, sizeof(struct lock_file));
Ingo Molnar· Apr 29, 2009, 07:32 UTC · re: Jeff King · lore

Re: error: Unable to append to .git/logs/refs/remotes/origin/master: Permission denied

* Jeff King <peff@peff.net> wrote:
Show 17 quoted lines
> On Tue, Apr 28, 2009 at 11:29:43PM -0400, Jeff King wrote:
> 
> > Note the repeated use of "hopefully". :) Maybe the earlier 
> > message is too hidden to rely on. We might be able to get by 
> > with checking "errno" for ENOTDIR after trying to lock the ref 
> > and using a different message, but I don't know how portable 
> > that will be.
> 
> Hmm, that actually doesn't work. errno is properly EACCESS in your 
> example, but the D/F problem doesn't actually set errno, since it 
> is git itself, and not a failed syscall, that determines that 
> "foo/bar" is not available because "foo" exists (and git must do 
> it, because "foo" may be a packed ref).
> 
> So I think we would need to simulate the errno setting, like the 
> patch below. That should generate the hint only when it would 
> actually be useful.

it wasnt hard to figure out what's going on. So this was more of a FYI, not really a bug report. Maybe if someone tries to pull into a read-only repo the same could happen? My particular breakage (of a single ref being root-owned - the rest was mingo owned) is atypical enough to be ignored.

If there's no easy/clean solution then please ignore my report.
	Ingo
Jeff King· Apr 29, 2009, 08:06 UTC · re: Ingo Molnar · lore

Re: error: Unable to append to .git/logs/refs/remotes/origin/master: Permission denied

On Wed, Apr 29, 2009 at 09:32:56AM +0200, Ingo Molnar wrote:
Show 9 quoted lines
> > So I think we would need to simulate the errno setting, like the 
> > patch below. That should generate the hint only when it would 
> > actually be useful.
> 
> it wasnt hard to figure out what's going on. So this was more of a 
> FYI, not really a bug report. Maybe if someone tries to pull into a 
> read-only repo the same could happen? My particular breakage (of a 
> single ref being root-owned - the rest was mingo owned) is atypical 
> enough to be ignored.

Actually, it is a little bit tough to get your breakage. A pure read-only repo would error out much earlier (permission denied on FETCH_HEAD or writing to object db). But if yours was just "accidentally fetched once as root", then that doesn't seem too uncommon.

> If there's no easy/clean solution then please ignore my report.
I think the patch I posted isn't too bad. We'll see what others say.
-Peff
Jeff King· May 25, 2009, 10:37 UTC · re: Jeff King · lore

[PATCH 1/2] lock_ref: inform callers of unavailable ref

One of the ways that locking might fail is that there is a DF conflict between two refs (e.g., you want to lock "foo/bar" but "foo" already exists). In this case, we return an error, but there is no way for the caller to know the specific problem.

This patch sets errno to ENOTDIR, which is the most sensible code. It's what we would see if the refs were stored purely in the filesystem (but these days we must check the namespace manually due to packed refs).

Signed-off-by: Jeff King <peff@peff.net>
---
We introduce a caller who cares in the next patch.
 refs.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/refs.c b/refs.c
index 45ad556..24438c6 100644
--- a/refs.c
+++ b/refs.c
@@ -893,8 +893,10 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char
 	 * name is a proper prefix of our refname.
 	 */
 	if (missing &&
-            !is_refname_available(ref, NULL, get_packed_refs(), 0))
+	     !is_refname_available(ref, NULL, get_packed_refs(), 0)) {
+		last_errno = ENOTDIR;
 		goto error_return;
+	}
 
 	lock->lk = xcalloc(1, sizeof(struct lock_file));
 

-- 
1.6.3.1.250.g01b8b.dirty
Jeff King· May 25, 2009, 10:40 UTC · re: Jeff King · lore

[PATCH 2/2] fetch: report ref storage DF errors more accurately

When we fail to store a fetched ref, we recommend that the user try running "git prune" to remove up any old refs that have been deleted by the remote, which would clear up any DF conflicts. However, ref storage might fail for other reasons (e.g., permissions problems) in which case the advice is useless and misleading.

This patch detects when there is an actual DF situation and only issues the advice when one is found.

Signed-off-by: Jeff King <peff@peff.net>
---
This is a followup to Ingo's bug report here:
  http://thread.gmane.org/gmane.comp.version-control.git/117751
 builtin-fetch.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/builtin-fetch.c b/builtin-fetch.c
index 77acabf..1eec64e 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -167,6 +167,9 @@ static struct ref *get_ref_map(struct transport *transport,
 	return ref_map;
 }
 
+#define STORE_REF_ERROR_OTHER 1
+#define STORE_REF_ERROR_DF_CONFLICT 2
+
 static int s_update_ref(const char *action,
 			struct ref *ref,
 			int check_old)
@@ -181,9 +184,11 @@ static int s_update_ref(const char *action,
 	lock = lock_any_ref_for_update(ref->name,
 				       check_old ? ref->old_sha1 : NULL, 0);
 	if (!lock)
-		return 2;
+		return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
+					  STORE_REF_ERROR_OTHER;
 	if (write_ref_sha1(lock, ref->new_sha1, msg) < 0)
-		return 2;
+		return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
+					  STORE_REF_ERROR_OTHER;
 	return 0;
 }
 
@@ -386,7 +391,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 	}
 	free(url);
 	fclose(fp);
-	if (rc & 2)
+	if (rc & STORE_REF_ERROR_DF_CONFLICT)
 		error("some local refs could not be updated; try running\n"
 		      " 'git remote prune %s' to remove any old, conflicting "
 		      "branches", remote_name);

-- 
1.6.3.1.250.g01b8b.dirty

← back to recent threads