Re: [PATCH 4/6] remote prune: warn dangling symrefs

Jeff King [off-list ref] writes:

On Mon, Feb 09, 2009 at 01:09:23AM -0800, Junio C Hamano wrote:

quoted

If you prune from the remote "frotz" that deleted the ref your tracking
branch remotes/frotz/HEAD points at, the symbolic ref will become
dangling.  We used to detect this as an error condition and issued a
message every time refs are enumerated.

This stops the error message, but moves the warning to "remote prune".

Very nice. As a bonus, this fixes certain (admittedly unlikely) renames,
too (which don't need to pass the BROKEN flag, since ref_rename uses
get_loose_refs directly):

  # without this patch
  $ git symbolic-ref refs/heads/foo/bar refs/heads/nonexistant
  $ git branch -m master foo
  error: refs/heads/foo/bar points nowhere!
  error: there are still refs under 'refs/heads/foo'
  error: unable to lock refs/heads/foo for update
  fatal: Branch rename failed

  # with this patch
  $ git branch -m master foo
  error: 'refs/heads/foo/bar' exists; cannot create 'refs/heads/foo'
  fatal: Branch rename failed

As a bonus, this issues unwarranted warning when creating the initial
commit in an empty repository.

The following fixes it.

-- >8 --
Subject: [PATCH] Squelch overzealous "ignoring dangling symref" in an empty repository

057e713 (Warn use of "origin" when remotes/origin/HEAD is dangling,
2009-02-08) tried to warn dangling refs/remotes/origin/HEAD only when
"origin" was used to refer to it.  There was one corner case a symref is
expected to be dangling and this warning is unwarranted: HEAD in an empty
repository.

This squelches the warning for this special case.

Signed-off-by: Junio C Hamano <redacted>
---
 sha1_name.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 3bd2ef0..2f75179 100644
--- a/sha1_name.c
+++ b/sha1_name.c

@@ -278,7 +278,8 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
 				*ref = xstrdup(r);
 			if (!warn_ambiguous_refs)
 				break;
-		} else if (flag & REF_ISSYMREF)
+		} else if ((flag & REF_ISSYMREF) &&
+			   (len != 4 || strcmp(str, "HEAD")))
 			warning("ignoring dangling symref %s.", fullref);
 	}
 	free(last_branch);

-- 
1.6.2.rc0.55.g7a105

On Wed, Feb 11, 2009 at 09:30:20AM -0800, Junio C Hamano wrote:

Subject: [PATCH] Squelch overzealous "ignoring dangling symref" in an empty repository

057e713 (Warn use of "origin" when remotes/origin/HEAD is dangling,
2009-02-08) tried to warn dangling refs/remotes/origin/HEAD only when
"origin" was used to refer to it.  There was one corner case a symref is
expected to be dangling and this warning is unwarranted: HEAD in an empty
repository.

This squelches the warning for this special case.

Is the special case really about "this is HEAD", or is it about writing
versus reading? For example, in an empty repo, without this patch I now
get:

  $ git init && git show
  warning: ignoring dangling symref HEAD.
  fatal: bad default revision 'HEAD'

which makes a lot of sense. But _writing_ to any dangling symref
shouldn't trigger a warning.

Admittedly, we have gotten by without this warning until now, and I
doubt anyone will want to write to other symrefs that are branches to be
born, so I think in practice your patch is fine.

-Peff

On Wed, Feb 11, 2009 at 01:35:47PM -0500, Jeff King wrote:

Is the special case really about "this is HEAD", or is it about writing
versus reading? For example, in an empty repo, without this patch I now
get:

  $ git init && git show
  warning: ignoring dangling symref HEAD.
  fatal: bad default revision 'HEAD'

which makes a lot of sense. But _writing_ to any dangling symref
shouldn't trigger a warning.

Admittedly, we have gotten by without this warning until now, and I
doubt anyone will want to write to other symrefs that are branches to be
born, so I think in practice your patch is fine.

I looked into what it would take to fix this, and it's a little hairy.
"git commit" calls:

  if (get_sha1("HEAD", head_sha1))
          initial_commit = 1;

So it is fine with HEAD not resolving, but that intent is not passed to
lower-level code. So we could add a "flags" field, but the callstack
looks like:

  dwim_ref
  get_sha1_basic
  get_sha1_1
  get_sha1_with_mode
  get_sha1

all of which would need to pass the flag through, and all of which have
tons of callers (though we could do the get_sha1_with_flags trick).

I don't know if it's worth it.

-Peff