[BUG] Empty reflogs and "git log -g"

[BUG] Empty reflogs and "git log -g"

[Santi Béjar]

Hi *,

  when a reflog entry do not have a reflog message the refs@{num}
syntax gives a different result than with 'git log -g'. Actually 'git
log -g' just skips this ref.

Regards,

Santi

Re: [BUG] Empty reflogs and "git log -g"

[Santi Béjar]

On 2/8/07, Santi Béjar <sbejar@gmail.com> wrote:
> Hi *,
>
>   when a reflog entry do not have a reflog message the refs@{num}
> syntax gives a different result than with 'git log -g'. Actually 'git
> log -g' just skips this ref.
>

Also:

$ git log origin/master

works, but

$ git log -g origin/master

does not. You have to put the remotes prefix.

Santi

Re: [BUG] Empty reflogs and "git log -g"

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1590 bytes --]

Hi,

On Thu, 8 Feb 2007, Santi Béjar wrote:

> On 2/8/07, Santi Béjar <sbejar@gmail.com> wrote:
> > Hi *,
> > 
> >   when a reflog entry do not have a reflog message the refs@{num}
> > syntax gives a different result than with 'git log -g'. Actually 'git
> > log -g' just skips this ref.
> > 
> 
> Also:
> 
> $ git log origin/master
> 
> works, but
> 
> $ git log -g origin/master
> 
> does not. You have to put the remotes prefix.

I hesitate "fixing" that. I mean, if you want to know about the remote 
branch "master" why not be precise? Yes, "git log" _is_ nice if you want 
to be sloppy.

But if you _have_ to have it, see attached patch. I will not push for it 
in time for 1.5.0, though, since I have a track record of not getting 
patches right on the first try.

Ciao,
Dscho

---

 reflog-walk.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/reflog-walk.c b/reflog-walk.c
index 653ec95..430faef 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -174,6 +174,19 @@ void add_reflog_for_walk(struct reflog_walk_info *info,
 			branch = xstrdup(head);
 		}
 		reflogs = read_complete_reflog(branch);
+		if (!reflogs || reflogs->nr == 0) {
+			unsigned char sha1[20];
+			char *b;
+			if (dwim_ref(branch, strlen(branch), sha1, &b) == 1) {
+				if (reflogs) {
+					free(reflogs->ref);
+					free(reflogs);
+				}
+				free(branch);
+				branch = b;
+				reflogs = read_complete_reflog(branch);
+			}
+		}
 		if (!reflogs || reflogs->nr == 0)
 			die("No reflogs found for '%s'", branch);
 		path_list_insert(branch, &info->complete_reflogs)->util

Re: [BUG] Empty reflogs and "git log -g"

[Nicolas Pitre]

On Thu, 8 Feb 2007, Johannes Schindelin wrote:

> But if you _have_ to have it, see attached patch. I will not push for it 
> in time for 1.5.0, though, since I have a track record of not getting 
> patches right on the first try.

Well.... to that effect I'd suggest you use dwim_log() instead.
Otherwise it'll break the HEAD reflog.


Nicolas

Re: [BUG] Empty reflogs and "git log -g"

[Johannes Schindelin]

Hi,

On Thu, 8 Feb 2007, Nicolas Pitre wrote:

> On Thu, 8 Feb 2007, Johannes Schindelin wrote:
> 
> > But if you _have_ to have it, see attached patch. I will not push for it 
> > in time for 1.5.0, though, since I have a track record of not getting 
> > patches right on the first try.
> 
> Well.... to that effect I'd suggest you use dwim_log() instead.
> Otherwise it'll break the HEAD reflog.

Thanks! I did not see that... See, I told you about my first tries with 
patches...

Ciao,
Dscho

Re: [BUG] Empty reflogs and "git log -g"

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 410 bytes --]

Hi,

On Thu, 8 Feb 2007, Santi Béjar wrote:

>  when a reflog entry do not have a reflog message the refs@{num} syntax 
> gives a different result than with 'git log -g'. Actually 'git log -g' 
> just skips this ref.

I could be that some older git version produces empty messages. But I'd 
regard the empty messages to be the problem. Or is the current Git version 
still producing such entries?

Ciao,
Dscho

Re: [BUG] Empty reflogs and "git log -g"

[Jakub Narebski]

Johannes Schindelin wrote:

> On Thu, 8 Feb 2007, Santi Béjar wrote:
> 
>>  when a reflog entry do not have a reflog message the refs@{num} syntax 
>> gives a different result than with 'git log -g'. Actually 'git log -g' 
>> just skips this ref.
> 
> I could be that some older git version produces empty messages. But I'd 
> regard the empty messages to be the problem. Or is the current Git version 
> still producing such entries?

StGIT used to produce no reflog messages; I don't know if this has improved.
But you can have old reflog entries with empty messages; git log -g should
deal with them IMHO.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [BUG] Empty reflogs and "git log -g"

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1700 bytes --]

Hi,

[Cc'ing the mailing list, not the newsgroup]

On Thu, 8 Feb 2007, Jakub Narebski wrote:

> Johannes Schindelin wrote:
> 
> > On Thu, 8 Feb 2007, Santi Béjar wrote:
> > 
> >>  when a reflog entry do not have a reflog message the refs@{num} syntax 
> >> gives a different result than with 'git log -g'. Actually 'git log -g' 
> >> just skips this ref.
> > 
> > I could be that some older git version produces empty messages. But I'd 
> > regard the empty messages to be the problem. Or is the current Git version 
> > still producing such entries?
> 
> StGIT used to produce no reflog messages; I don't know if this has 
> improved. But you can have old reflog entries with empty messages; git 
> log -g should deal with them IMHO.

I just tried. An empty string is not enough. The tab before the message 
has to be lacking, too.

Here's a small patch, if you have to have it.

---

 refs.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/refs.c b/refs.c
index 7e07fc4..ba5bd2d 100644
--- a/refs.c
+++ b/refs.c
@@ -1189,12 +1189,14 @@ int for_each_reflog_ent(const char *ref, each_reflog_ent_fn fn, void *cb_data)
 		    !message || message[0] != ' ' ||
 		    (message[1] != '+' && message[1] != '-') ||
 		    !isdigit(message[2]) || !isdigit(message[3]) ||
-		    !isdigit(message[4]) || !isdigit(message[5]) ||
-		    message[6] != '\t')
+		    !isdigit(message[4]) || !isdigit(message[5]))
 			continue; /* corrupt? */
 		email_end[1] = '\0';
 		tz = strtol(message + 1, NULL, 10);
-		message += 7;
+		if (message[6] != '\t')
+			message += 6;
+		else
+			message += 7;
 		ret = fn(osha1, nsha1, buf+82, timestamp, tz, message, cb_data);
 		if (ret)
 			break;

Re: [BUG] Empty reflogs and "git log -g"

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Thu, 8 Feb 2007, Jakub Narebski wrote:
>
>> StGIT used to produce no reflog messages; I don't know if this has 
>> improved. But you can have old reflog entries with empty messages; git 
>> log -g should deal with them IMHO.
>
> I just tried. An empty string is not enough. The tab before the message 
> has to be lacking, too.
>
> Here's a small patch, if you have to have it.

I think this is necessary for v1.5.0.  I'd appreciate a properly
signed-off log message.

>
> ---
>
>  refs.c |    8 +++++---
>  1 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 7e07fc4..ba5bd2d 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1189,12 +1189,14 @@ int for_each_reflog_ent(const char *ref, each_reflog_ent_fn fn, void *cb_data)
>  		    !message || message[0] != ' ' ||
>  		    (message[1] != '+' && message[1] != '-') ||
>  		    !isdigit(message[2]) || !isdigit(message[3]) ||
> -		    !isdigit(message[4]) || !isdigit(message[5]) ||
> -		    message[6] != '\t')
> +		    !isdigit(message[4]) || !isdigit(message[5]))
>  			continue; /* corrupt? */
>  		email_end[1] = '\0';
>  		tz = strtol(message + 1, NULL, 10);
> -		message += 7;
> +		if (message[6] != '\t')
> +			message += 6;
> +		else
> +			message += 7;
>  		ret = fn(osha1, nsha1, buf+82, timestamp, tz, message, cb_data);
>  		if (ret)
>  			break;

[PATCH] for_each_reflog_ent: be forgiving about missing message

[Johannes Schindelin]

Some reflogs are/were generated without a message; do not plainly
ignore those entries.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Thu, 8 Feb 2007, Junio C Hamano wrote:

	> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
	> 
	> > On Thu, 8 Feb 2007, Jakub Narebski wrote:
	> >
	> >> StGIT used to produce no reflog messages; I don't know if 
	> >> this has improved. But you can have old reflog entries with 
	> >> empty messages; git log -g should deal with them IMHO.
	> >
	> > I just tried. An empty string is not enough. The tab before 
	> > the message has to be lacking, too.
	> >
	> > Here's a small patch, if you have to have it.
	> 
	> I think this is necessary for v1.5.0.  I'd appreciate a properly
	> signed-off log message.

	Voila!

 refs.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/refs.c b/refs.c
index 7e07fc4..ba5bd2d 100644
--- a/refs.c
+++ b/refs.c
@@ -1189,12 +1189,14 @@ int for_each_reflog_ent(const char *ref, each_reflog_ent_fn fn, void *cb_data)
 		    !message || message[0] != ' ' ||
 		    (message[1] != '+' && message[1] != '-') ||
 		    !isdigit(message[2]) || !isdigit(message[3]) ||
-		    !isdigit(message[4]) || !isdigit(message[5]) ||
-		    message[6] != '\t')
+		    !isdigit(message[4]) || !isdigit(message[5]))
 			continue; /* corrupt? */
 		email_end[1] = '\0';
 		tz = strtol(message + 1, NULL, 10);
-		message += 7;
+		if (message[6] != '\t')
+			message += 6;
+		else
+			message += 7;
 		ret = fn(osha1, nsha1, buf+82, timestamp, tz, message, cb_data);
 		if (ret)
 			break;

[PATCH] log --reflog: use dwim_log

[Johannes Schindelin]

Since "git log origin/master" uses dwim_log() to match
"refs/remotes/origin/master", it makes sense to do that for
"git log --reflog", too.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---

	I changed dwim_ref() to dwim_log() as per Nico's request.

 cache.h       |    1 +
 reflog-walk.c |   13 +++++++++++++
 sha1_name.c   |    2 +-
 3 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/cache.h b/cache.h
index 6f55fdc..44941c0 100644
--- a/cache.h
+++ b/cache.h
@@ -304,6 +304,7 @@ extern char *sha1_to_hex(const unsigned char *sha1);	/* static buffer result! */
 extern int read_ref(const char *filename, unsigned char *sha1);
 extern const char *resolve_ref(const char *path, unsigned char *sha1, int, int *);
 extern int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref);
+extern int dwim_log(const char *str, int len, unsigned char *sha1, char **ref);
 
 extern int create_symref(const char *ref, const char *refs_heads_master, const char *logmsg);
 extern int validate_headref(const char *ref);
diff --git a/reflog-walk.c b/reflog-walk.c
index 653ec95..e0f1332 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -174,6 +174,19 @@ void add_reflog_for_walk(struct reflog_walk_info *info,
 			branch = xstrdup(head);
 		}
 		reflogs = read_complete_reflog(branch);
+		if (!reflogs || reflogs->nr == 0) {
+			unsigned char sha1[20];
+			char *b;
+			if (dwim_log(branch, strlen(branch), sha1, &b) == 1) {
+				if (reflogs) {
+					free(reflogs->ref);
+					free(reflogs);
+				}
+				free(branch);
+				branch = b;
+				reflogs = read_complete_reflog(branch);
+			}
+		}
 		if (!reflogs || reflogs->nr == 0)
 			die("No reflogs found for '%s'", branch);
 		path_list_insert(branch, &info->complete_reflogs)->util
diff --git a/sha1_name.c b/sha1_name.c
index d0d9536..c50a378 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -268,7 +268,7 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
 	return refs_found;
 }
 
-static int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
+int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
 {
 	const char **p;
 	int logs_found = 0;
-- 
1.5.0.rc4.2051.g679c-dirty

Re: [PATCH] log --reflog: use dwim_log

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Since "git log origin/master" uses dwim_log() to match
> "refs/remotes/origin/master", it makes sense to do that for
> "git log --reflog", too.

This is a bit sad.

When there is remotes/origin/HEAD symref that points at
remotes/origin/master (and usually there is, in a repository
initialized with the current git-clone), you can say "git log
origin".  Even with this patch, I do not think dwim_log would
allow you to say "git log -g origin" to mean "git log -g
origin/master".

Re: [PATCH] log --reflog: use dwim_log

[Junio C Hamano]

When refs/remotes/gfi/master and refs/remotes/gfi/HEAD exist,
and the latter is a symref that points at the former, dwim_ref()
resolves string "gfi" to "refs/remotes/gfi/master" as expected,
but dwim_log() does not understand "gfi@{1.day}" and needs to be
told "gfi/master@{1.day}".  This is confusing.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

 Junio C Hamano <junkio@cox.net> writes:

 > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
 >
 >> Since "git log origin/master" uses dwim_log() to match
 >> "refs/remotes/origin/master", it makes sense to do that for
 >> "git log --reflog", too.
 >
 > This is a bit sad.
 >
 > When there is remotes/origin/HEAD symref that points at
 > remotes/origin/master (and usually there is, in a repository
 > initialized with the current git-clone), you can say "git log
 > origin".  Even with this patch, I do not think dwim_log would
 > allow you to say "git log -g origin" to mean "git log -g
 > origin/master".

 I think there should be a cleaner way to fix this, but at least
 this seems to work.

 sha1_name.c |   29 +++++++++++++++++++++--------
 1 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index f79a7c9..09c63ed 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -275,16 +275,29 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
 	*log = NULL;
 	for (p = ref_fmt; *p; p++) {
 		struct stat st;
-		char *path = mkpath(*p, len, str);
+		unsigned char hash[20];
+		char path[PATH_MAX];
+		char *ref, *it;
+
+		strcpy(path, mkpath(*p, len, str));
+		ref = resolve_ref(path, hash, 0, NULL);
+		if (!ref)
+			continue;
 		if (!stat(git_path("logs/%s", path), &st) &&
-		    S_ISREG(st.st_mode)) {
-			if (!logs_found++) {
-				*log = xstrdup(path);
-				resolve_ref(path, sha1, 0, NULL);
-			}
-			if (!warn_ambiguous_refs)
-				break;
+		    S_ISREG(st.st_mode))
+			it = path;
+		else if (strcmp(ref, path) &&
+			 !stat(git_path("logs/%s", ref), &st) &&
+			 S_ISREG(st.st_mode))
+			it = ref;
+		else
+			continue;
+		if (!logs_found++) {
+			*log = xstrdup(it);
+			hashcpy(sha1, hash);
 		}
+		if (!warn_ambiguous_refs)
+			break;
 	}
 	return logs_found;
 }