threads / discuss / 7754

cg-log "fatal: bad revision ''"

Subject: cg-log "fatal: bad revision ''"

## tl;dr

6 messages between Apr 20, 2007 and Apr 23, 2007.

replies: 5people: 4as markdown or json

MichaelTiloDressel@t-online.de· Apr 20, 2007, 11:54 UTC · lore
Hi,

I moved a subdirectory in my working direcotry. If I do cg-log on any of the files in the new subdirectory I get the error:

Use of uninitialized value in hash element at /usr/lib/cogito/cg-Xfollowrenames line 192, <$kid> line 12. fatal: bad revision ''

I tried the moving of the subdirectory in two ways:
1. using cg-mv
2. git mv
But  in both cases the result is the same.

Cheers, Michael

Petr Baudis· Apr 20, 2007, 13:58 UTC · re: MichaelTiloDressel@t-online.de · lore

Re: cg-log "fatal: bad revision ''"

  Hi,
On Fri, Apr 20, 2007 at 01:54:14PM CEST, MichaelTiloDressel@t-online.de wrote:
> I moved a subdirectory in my working direcotry.
> If I do cg-log on any of the files in the new subdirectory I get the
> error:
  renames following in cg-log is known to be broken, please turn it off
or use newer Cogito version which has it by default off.
-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Ever try. Ever fail. No matter. // Try again. Fail again. Fail better.
		-- Samuel Beckett
Michael Dressel· Apr 20, 2007, 17:03 UTC · re: Petr Baudis · lore

Re: cg-log "fatal: bad revision ''"

On Fri, 20 Apr 2007, Petr Baudis wrote:
Show 9 quoted lines
>   Hi,
> 
> On Fri, Apr 20, 2007 at 01:54:14PM CEST, MichaelTiloDressel@t-online.de wrote:
> > I moved a subdirectory in my working direcotry.
> > If I do cg-log on any of the files in the new subdirectory I get the
> > error:
> 
>   renames following in cg-log is known to be broken, please turn it off
> or use newer Cogito version which has it by default off.

Thanx for the reply. Is there a way to follow renaming at all, either with cogito or git?

Cheers, Michael

Show 7 quoted lines
> 
> -- 
> 				Petr "Pasky" Baudis
> Stuff: http://pasky.or.cz/
> Ever try. Ever fail. No matter. // Try again. Fail again. Fail better.
> 		-- Samuel Beckett
> 
Linus Torvalds· Apr 20, 2007, 23:25 UTC · re: Michael Dressel · lore

Re: cg-log "fatal: bad revision ''"

On Fri, 20 Apr 2007, Michael Dressel wrote:
> 
> Thanx for the reply. Is there a way to follow renaming at all, either 
> with cogito or git?

"git blame" will follow renames, but doesn't really show things as a log, more just as a "these commits mattered" thing.

You can use something like this patch to then make it *look* like a "log" function.

Junio - I really think that something along these lines is worth doing, but I wonder what the right semantics are. This patch is stupid, and just keeps the "blame" semantics, just showing it in a "log format". It does mean that when I do

	git blame -C --log block/ll_rw_block.c
it will show entries for the old drivers/block/ll_rw_blk.c location.

HOWEVER, it's still broken, because when you do logging, you don't actually care about the diffs at all, so *any* difference from the previous version should show up, even if the lines don't show up in the end result. So this doesn't really do what people actually *want* to do, it does somethign much more complicated ;)

		Linus
---
 builtin-blame.c |   21 +++++++++++++++++----
 1 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/builtin-blame.c b/builtin-blame.c
index 8919b02..ee98753 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -16,6 +16,7 @@
 #include "quote.h"
 #include "xdiff-interface.h"
 #include "cache-tree.h"
+#include "log-tree.h"
 
 static char blame_usage[] =
 "git-blame [-c] [-l] [-t] [-f] [-n] [-p] [-L n,m] [-S <revs-file>] [-M] [-C] [-C] [--contents <filename>] [--incremental] [commit] [--] file\n"
@@ -30,6 +31,7 @@ static char blame_usage[] =
 "  -L n,m              Process only line range n,m, counting from 1\n"
 "  -M, -C              Find line movements within and across files\n"
 "  --incremental       Show blame entries as we find them, incrementally\n"
+"  --log               Show blame entries as a log\n"
 "  --contents file     Use <file>'s contents as the final image\n"
 "  -S revs-file        Use revisions from revs-file instead of calling git-rev-list\n";
 
@@ -42,6 +44,7 @@ static int show_root;
 static int blank_boundary;
 static int incremental;
 static int cmd_is_annotate;
+static int log;
 
 #ifndef DEBUG
 #define DEBUG 0
@@ -1370,11 +1373,15 @@ static void write_filename_info(const char *path)
  * The blame_entry is found to be guilty for the range.  Mark it
  * as such, and show it in incremental output.
  */
-static void found_guilty_entry(struct blame_entry *ent)
+static void found_guilty_entry(struct rev_info *revs, struct blame_entry *ent)
 {
 	if (ent->guilty)
 		return;
 	ent->guilty = 1;
+	if (log) {
+		log_tree_commit(revs, ent->suspect->commit);
+		return;
+	}
 	if (incremental) {
 		struct origin *suspect = ent->suspect;
 
@@ -1443,7 +1450,7 @@ static void assign_blame(struct scoreboard *sb, struct rev_info *revs, int opt)
 		/* Take responsibility for the remaining entries */
 		for (ent = sb->ent; ent; ent = ent->next)
 			if (same_suspect(ent->suspect, suspect))
-				found_guilty_entry(ent);
+				found_guilty_entry(revs, ent);
 		origin_decref(suspect);
 
 		if (DEBUG) /* sanity */
@@ -2072,7 +2079,6 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	cmd_is_annotate = !strcmp(argv[0], "annotate");
 
 	git_config(git_blame_config);
-	save_commit_buffer = 0;
 
 	opt = 0;
 	seen_dashdash = 0;
@@ -2123,6 +2129,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 		}
 		else if (!strcmp("--incremental", arg))
 			incremental = 1;
+		else if (!strcmp("--log", arg))
+			log = 1;
 		else if (!strcmp("--score-debug", arg))
 			output_option |= OUTPUT_SHOW_SCORE;
 		else if (!strcmp("-f", arg) ||
@@ -2244,6 +2252,11 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 
 	init_revisions(&revs, NULL);
 	setup_revisions(unk, argv, &revs, NULL);
+	revs.abbrev = DEFAULT_ABBREV;
+	revs.commit_format = CMIT_FMT_DEFAULT;
+	revs.verbose_header = 1;
+	revs.always_show_header = 1;
+
 	memset(&sb, 0, sizeof(sb));
 
 	/*
@@ -2335,7 +2348,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 
 	assign_blame(&sb, &revs, opt);
 
-	if (incremental)
+	if (incremental || log)
 		return 0;
 
 	coalesce(&sb);
Junio C Hamano· Apr 20, 2007, 23:47 UTC · re: Linus Torvalds · lore

Re: cg-log "fatal: bad revision ''"

Linus Torvalds <torvalds@linux-foundation.org> writes:
Show 27 quoted lines
> On Fri, 20 Apr 2007, Michael Dressel wrote:
>> 
>> Thanx for the reply. Is there a way to follow renaming at all, either 
>> with cogito or git?
>
> "git blame" will follow renames, but doesn't really show things as a log, 
> more just as a "these commits mattered" thing.
>
> You can use something like this patch to then make it *look* like a "log" 
> function.
>
> Junio - I really think that something along these lines is worth doing, 
> but I wonder what the right semantics are. This patch is stupid, and just 
> keeps the "blame" semantics, just showing it in a "log format". It does 
> mean that when I do
>
> 	git blame -C --log block/ll_rw_block.c
>
> it will show entries for the old drivers/block/ll_rw_blk.c location.
>
> HOWEVER, it's still broken, because when you do logging, you don't 
> actually care about the diffs at all, so *any* difference from the 
> previous version should show up, even if the lines don't show up in the 
> end result. So this doesn't really do what people actually *want* to do, 
> it does somethign much more complicated ;)
>
> 		Linus
Yeah, I've been thinking about it but in the back of my head.

I think by hooking in found_guilty_entry(), you show the same commit more than once when the commit is attributed to more than one region. This can probably be solved by remembering if the current suspect commit matched, and show the log once if so.

And while you are remembering if the current <commit,file> pair has some chunks attributed to it in that caller loop, I _think_ you would also want to remember which lines in the suspect (i.e. <commit,file> pair) were blamed, and to which file in which child commit (i.e. previous suspect). Then after the caller loop finds out that there are some lines that are attributed to the current suspect, you can show the log, and diff between the suspects' blobs, limited to the line ranges that mattered.

Currently that information is not recorded in blame_entry. It only has s_lno which indexes into the final image; the necessary information is found in "struct patch", but after it is used to split the blame entries and assign blame to the current suspect, it is discarded. For the purpose of this application you need to record that as well.

While I do not think it is a rocket surgery, the change needed to do this is not entirely trivial.

Michael Dressel· Apr 23, 2007, 22:05 UTC · re: Linus Torvalds · lore

Re: cg-log "fatal: bad revision ''"

On Fri, 20 Apr 2007, Linus Torvalds wrote:
Show 127 quoted lines
> 
> 
> On Fri, 20 Apr 2007, Michael Dressel wrote:
> > 
> > Thanx for the reply. Is there a way to follow renaming at all, either 
> > with cogito or git?
> 
> "git blame" will follow renames, but doesn't really show things as a log, 
> more just as a "these commits mattered" thing.
> 
> You can use something like this patch to then make it *look* like a "log" 
> function.
> 
> Junio - I really think that something along these lines is worth doing, 
> but I wonder what the right semantics are. This patch is stupid, and just 
> keeps the "blame" semantics, just showing it in a "log format". It does 
> mean that when I do
> 
> 	git blame -C --log block/ll_rw_block.c
> 
> it will show entries for the old drivers/block/ll_rw_blk.c location.
> 
> HOWEVER, it's still broken, because when you do logging, you don't 
> actually care about the diffs at all, so *any* difference from the 
> previous version should show up, even if the lines don't show up in the 
> end result. So this doesn't really do what people actually *want* to do, 
> it does somethign much more complicated ;)
> 
> 		Linus
> 
> ---
>  builtin-blame.c |   21 +++++++++++++++++----
>  1 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/builtin-blame.c b/builtin-blame.c
> index 8919b02..ee98753 100644
> --- a/builtin-blame.c
> +++ b/builtin-blame.c
> @@ -16,6 +16,7 @@
>  #include "quote.h"
>  #include "xdiff-interface.h"
>  #include "cache-tree.h"
> +#include "log-tree.h"
>  
>  static char blame_usage[] =
>  "git-blame [-c] [-l] [-t] [-f] [-n] [-p] [-L n,m] [-S <revs-file>] [-M] [-C] [-C] [--contents <filename>] [--incremental] [commit] [--] file\n"
> @@ -30,6 +31,7 @@ static char blame_usage[] =
>  "  -L n,m              Process only line range n,m, counting from 1\n"
>  "  -M, -C              Find line movements within and across files\n"
>  "  --incremental       Show blame entries as we find them, incrementally\n"
> +"  --log               Show blame entries as a log\n"
>  "  --contents file     Use <file>'s contents as the final image\n"
>  "  -S revs-file        Use revisions from revs-file instead of calling git-rev-list\n";
>  
> @@ -42,6 +44,7 @@ static int show_root;
>  static int blank_boundary;
>  static int incremental;
>  static int cmd_is_annotate;
> +static int log;
>  
>  #ifndef DEBUG
>  #define DEBUG 0
> @@ -1370,11 +1373,15 @@ static void write_filename_info(const char *path)
>   * The blame_entry is found to be guilty for the range.  Mark it
>   * as such, and show it in incremental output.
>   */
> -static void found_guilty_entry(struct blame_entry *ent)
> +static void found_guilty_entry(struct rev_info *revs, struct blame_entry *ent)
>  {
>  	if (ent->guilty)
>  		return;
>  	ent->guilty = 1;
> +	if (log) {
> +		log_tree_commit(revs, ent->suspect->commit);
> +		return;
> +	}
>  	if (incremental) {
>  		struct origin *suspect = ent->suspect;
>  
> @@ -1443,7 +1450,7 @@ static void assign_blame(struct scoreboard *sb, struct rev_info *revs, int opt)
>  		/* Take responsibility for the remaining entries */
>  		for (ent = sb->ent; ent; ent = ent->next)
>  			if (same_suspect(ent->suspect, suspect))
> -				found_guilty_entry(ent);
> +				found_guilty_entry(revs, ent);
>  		origin_decref(suspect);
>  
>  		if (DEBUG) /* sanity */
> @@ -2072,7 +2079,6 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
>  	cmd_is_annotate = !strcmp(argv[0], "annotate");
>  
>  	git_config(git_blame_config);
> -	save_commit_buffer = 0;
>  
>  	opt = 0;
>  	seen_dashdash = 0;
> @@ -2123,6 +2129,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
>  		}
>  		else if (!strcmp("--incremental", arg))
>  			incremental = 1;
> +		else if (!strcmp("--log", arg))
> +			log = 1;
>  		else if (!strcmp("--score-debug", arg))
>  			output_option |= OUTPUT_SHOW_SCORE;
>  		else if (!strcmp("-f", arg) ||
> @@ -2244,6 +2252,11 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
>  
>  	init_revisions(&revs, NULL);
>  	setup_revisions(unk, argv, &revs, NULL);
> +	revs.abbrev = DEFAULT_ABBREV;
> +	revs.commit_format = CMIT_FMT_DEFAULT;
> +	revs.verbose_header = 1;
> +	revs.always_show_header = 1;
> +
>  	memset(&sb, 0, sizeof(sb));
>  
>  	/*
> @@ -2335,7 +2348,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
>  
>  	assign_blame(&sb, &revs, opt);
>  
> -	if (incremental)
> +	if (incremental || log)
>  		return 0;
>  
>  	coalesce(&sb);
> 
Thanx for your help.
I have a version 1.5.2.rc0.2.g8ec0-dirty now with your changes patched in. 

The command git blame -C --log myfile produces some alternative information to git log. I thought it would be a "simple" bug in cg-Xfollowrenames. As I learned from the follow-up postings it's more involved.

Cheers, Michael

← back to recent threads