patch, 2 partsreflog test: add more tests for 'reflog delete'

This adds more tests for 'reflog delete' and marks it as broken, as currently a call to 'git reflog delete HEAD@{1}' deletes entries in the currently checked out branch's log, not the HEAD log.

Noticed by John Wiegley

Signed-off-by: Pieter de Bie <pdebie@ai.rug.nl>
---

	johnw on IRC noticed this. This adds a test that shows the
	problem. The next patch fixes the issue but I'm not sure of
	the implementation. Perhaps we just shouldn't resolve symbolic
	refs? I'm not really sure which functions to use then.

 t/t1410-reflog.sh |   22 ++++++++++++++++++----
 1 files changed, 18 insertions(+), 4 deletions(-)

dwim_ref() used to resolve HEAD to its symlink (like refs/heads/master), making a call to 'git reflog delete HEAD@{1}' to actually delete the second entry in the master reflog.

This patch makes a special case for HEAD (as that's the only non-branch reflog we keep), fixing the issue.

Signed-off-by: Pieter de Bie <pdebie@ai.rug.nl>
---
 builtin-reflog.c  |   15 ++++++++++++---
 t/t1410-reflog.sh |    2 +-
 2 files changed, 13 insertions(+), 4 deletions(-)

Pieter de Bie <pdebie@ai.rug.nl> writes:

What happens to remotes/origin/HEAD that points at remotes/origin/master?

Junio C Hamano <gitster@pobox.com> writes:

Perhaps this might work better?

 builtin-reflog.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

On Aug 10, 2008, at 3:01 AM, Junio C Hamano wrote:

This is also what add_reflog_for_walk() does, but that function tries to resolve the argv[i] part first, without doing the dwim_log().

Perhaps we can also do this to allow "git reflog expire master" instead of "git reflog expire refs/heads/master"?

--<8--
Subject: [PATCH] builtin-reflog: Allow reflog expire to name partial ref

This allows you to specify 'git reflog expire master' without needing to give the full refname like 'git reflog expire refs/heads/master'

Signed-off-by: Pieter de Bie <pdebie@ai.rug.nl>
---
 builtin-reflog.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

On Sonntag, 10. August 2008, Pieter de Bie wrote:

This runs into an endless loop in the error case because it doesn't increase i.

-- Hannes

Pieter de Bie <pdebie@ai.rug.nl> writes:

Sorry, I do not understand what you meant by the above comment.

 - "This is also what add_reflog_for_walk() does" -- I take it you mean
   the use of dwim_log() instead of dwim_ref()?

 - "... but that function tries to resolve the argv[i] part first" -- do
   you mean the resolve_ref("HEAD"...) call inside "if (!*branch)"
   codepath?

   That one serves different purposes than "delete HEAD@{42}".  It is
   about showing "@{42}" --- in order to show reflog for "the current
   branch", it figures out the current branch by resolving "HEAD".

In any case, what confuses me is I cannot tell if you do or do not have issues that I did not think of with the "s/dwim_ref/dwim_log/" change. Are you saying "no that cannot be a correct fix; see the way dwim_log() is used in add_reflog_for_walk() -- it does more than your one-liner"?

By the way, I think the idea of "Perhaps we can also..." part is good.

On Aug 10, 2008, at 8:52 PM, Junio C Hamano wrote:

Sorry, it was early in the morning ;)

Yes

No, I meant this part:

reflogs = read_complete_reflog(branch);
if (!reflogs || reflogs->nr == 0)
	if (dwim_log(branch, strlen(branch), sha1, &b) == 1) {
		branch = b;
		reflogs = read_complete_reflog(branch);
	}

Which seems to suggest that the read_complete_reflog() may produce different results if dwim_log() is not called. However, I did not follow the codepath to see why.

I think the change looks ok. The only 'problem' I had was the chunk above, because I do not know if the double call to read_complete_reflog, once without a dwim_log and optionally once with, is significant. However, I'm not familiar enough with the code to make any observation other than that, which is why my reply had no conclusion ;)

Hope that clears things up.

I'll send in a better patch.

- Pieter

This allows you to specify 'git reflog expire master' without needing to give the full refname like 'git reflog expire refs/heads/master'

Signed-off-by: Pieter de Bie <pdebie@ai.rug.nl>
---
 builtin-reflog.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)