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

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

[Ingo Molnar]

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

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

[Jeff King]

On Tue, Apr 28, 2009 at 09:31:38AM +0200, Ingo Molnar wrote:

>  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

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

[Jeff King]

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

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

[Ingo Molnar]

* Jeff King <peff@peff.net> wrote:

> 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

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

[Jeff King]

On Wed, Apr 29, 2009 at 09:32:56AM +0200, Ingo Molnar wrote:

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

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

[Jeff King]

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

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

[Jeff King]

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

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

[Junio C Hamano]

Thanks.